From d204469280a02f4b0d3f0d8a151bfd06785cbbc6 Mon Sep 17 00:00:00 2001 From: max furman Date: Wed, 11 Sep 2019 22:57:55 -0700 Subject: [PATCH] Add a few more validity checks to default ssh cert validator --- authority/provisioner/sign_ssh_options.go | 8 +- .../provisioner/sign_ssh_options_test.go | 192 ++++++++++++++++++ 2 files changed, 197 insertions(+), 3 deletions(-) create mode 100644 authority/provisioner/sign_ssh_options_test.go diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index 7cc809a9..134cabe4 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -264,9 +264,11 @@ func (v *sshCertificateDefaultValidator) Valid(cert *ssh.Certificate) error { case len(cert.ValidPrincipals) == 0: return errors.New("ssh certificate valid principals cannot be empty") case cert.ValidAfter == 0: - return errors.New("ssh certificate valid after cannot be 0") - case cert.ValidBefore == 0: - return errors.New("ssh certificate valid before cannot be 0") + return errors.New("ssh certificate validAfter cannot be 0") + case cert.ValidBefore < uint64(now().Unix()): + return errors.New("ssh certificate validBefore cannot be in the past") + case cert.ValidBefore < cert.ValidAfter: + return errors.New("ssh certificate validBefore cannot be before validAfter") case cert.CertType == ssh.UserCert && len(cert.Extensions) == 0: return errors.New("ssh certificate extensions cannot be empty") case cert.SignatureKey == nil: diff --git a/authority/provisioner/sign_ssh_options_test.go b/authority/provisioner/sign_ssh_options_test.go new file mode 100644 index 00000000..95c6913c --- /dev/null +++ b/authority/provisioner/sign_ssh_options_test.go @@ -0,0 +1,192 @@ +package provisioner + +import ( + "testing" + "time" + + "github.com/pkg/errors" + "github.com/smallstep/assert" + "github.com/smallstep/cli/crypto/keys" + "golang.org/x/crypto/ssh" +) + +func Test_sshCertificateDefaultValidator_Valid(t *testing.T) { + pub, _, err := keys.GenerateDefaultKeyPair() + assert.FatalError(t, err) + sshPub, err := ssh.NewPublicKey(pub) + assert.FatalError(t, err) + v := sshCertificateDefaultValidator{} + tests := []struct { + name string + cert *ssh.Certificate + err error + }{ + { + "fail/zero-nonce", + &ssh.Certificate{}, + errors.New("ssh certificate nonce cannot be empty"), + }, + { + "fail/nil-key", + &ssh.Certificate{Nonce: []byte("foo")}, + errors.New("ssh certificate key cannot be nil"), + }, + { + "fail/zero-serial", + &ssh.Certificate{Nonce: []byte("foo"), Key: sshPub}, + errors.New("ssh certificate serial cannot be 0"), + }, + { + "fail/unexpected-cert-type", + // UserCert = 1, HostCert = 2 + &ssh.Certificate{Nonce: []byte("foo"), Key: sshPub, CertType: 3, Serial: 1}, + errors.New("ssh certificate has an unknown type: 3"), + }, + { + "fail/empty-cert-key-id", + &ssh.Certificate{Nonce: []byte("foo"), Key: sshPub, Serial: 1, CertType: 1}, + errors.New("ssh certificate key id cannot be empty"), + }, + { + "fail/empty-valid-principals", + &ssh.Certificate{Nonce: []byte("foo"), Key: sshPub, Serial: 1, CertType: 1, KeyId: "foo"}, + errors.New("ssh certificate valid principals cannot be empty"), + }, + { + "fail/zero-validAfter", + &ssh.Certificate{ + Nonce: []byte("foo"), + Key: sshPub, + Serial: 1, + CertType: 1, + KeyId: "foo", + ValidPrincipals: []string{"foo"}, + ValidAfter: 0, + }, + errors.New("ssh certificate validAfter cannot be 0"), + }, + { + "fail/validBefore-past", + &ssh.Certificate{ + Nonce: []byte("foo"), + Key: sshPub, + Serial: 1, + CertType: 1, + KeyId: "foo", + ValidPrincipals: []string{"foo"}, + ValidAfter: uint64(time.Now().Add(-10 * time.Minute).Unix()), + ValidBefore: uint64(time.Now().Add(-5 * time.Minute).Unix()), + }, + errors.New("ssh certificate validBefore cannot be in the past"), + }, + { + "fail/validAfter-after-validBefore", + &ssh.Certificate{ + Nonce: []byte("foo"), + Key: sshPub, + Serial: 1, + CertType: 1, + KeyId: "foo", + ValidPrincipals: []string{"foo"}, + ValidAfter: uint64(time.Now().Add(15 * time.Minute).Unix()), + ValidBefore: uint64(time.Now().Add(10 * time.Minute).Unix()), + }, + errors.New("ssh certificate validBefore cannot be before validAfter"), + }, + { + "fail/empty-extensions", + &ssh.Certificate{ + Nonce: []byte("foo"), + Key: sshPub, + Serial: 1, + CertType: 1, + KeyId: "foo", + ValidPrincipals: []string{"foo"}, + ValidAfter: uint64(time.Now().Unix()), + ValidBefore: uint64(time.Now().Add(10 * time.Minute).Unix()), + }, + errors.New("ssh certificate extensions cannot be empty"), + }, + { + "fail/nil-signature-key", + &ssh.Certificate{ + Nonce: []byte("foo"), + Key: sshPub, + Serial: 1, + CertType: 1, + KeyId: "foo", + ValidPrincipals: []string{"foo"}, + ValidAfter: uint64(time.Now().Unix()), + ValidBefore: uint64(time.Now().Add(10 * time.Minute).Unix()), + Permissions: ssh.Permissions{ + Extensions: map[string]string{"foo": "bar"}, + }, + }, + errors.New("ssh certificate signature key cannot be nil"), + }, + { + "fail/nil-signature", + &ssh.Certificate{ + Nonce: []byte("foo"), + Key: sshPub, + Serial: 1, + CertType: 1, + KeyId: "foo", + ValidPrincipals: []string{"foo"}, + ValidAfter: uint64(time.Now().Unix()), + ValidBefore: uint64(time.Now().Add(10 * time.Minute).Unix()), + Permissions: ssh.Permissions{ + Extensions: map[string]string{"foo": "bar"}, + }, + SignatureKey: sshPub, + }, + errors.New("ssh certificate signature cannot be nil"), + }, + { + "ok/userCert", + &ssh.Certificate{ + Nonce: []byte("foo"), + Key: sshPub, + Serial: 1, + CertType: 1, + KeyId: "foo", + ValidPrincipals: []string{"foo"}, + ValidAfter: uint64(time.Now().Unix()), + ValidBefore: uint64(time.Now().Add(10 * time.Minute).Unix()), + Permissions: ssh.Permissions{ + Extensions: map[string]string{"foo": "bar"}, + }, + SignatureKey: sshPub, + Signature: &ssh.Signature{}, + }, + nil, + }, + { + "ok/hostCert", + &ssh.Certificate{ + Nonce: []byte("foo"), + Key: sshPub, + Serial: 1, + CertType: 2, + KeyId: "foo", + ValidPrincipals: []string{"foo"}, + ValidAfter: uint64(time.Now().Unix()), + ValidBefore: uint64(time.Now().Add(10 * time.Minute).Unix()), + SignatureKey: sshPub, + Signature: &ssh.Signature{}, + }, + nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := v.Valid(tt.cert); err != nil { + if assert.NotNil(t, tt.err) { + assert.HasPrefix(t, err.Error(), tt.err.Error()) + } + } else { + assert.Nil(t, tt.err) + } + }) + } +}