diff --git a/authority/provisioner/oidc_test.go b/authority/provisioner/oidc_test.go index 913c8a2b..9972dc2c 100644 --- a/authority/provisioner/oidc_test.go +++ b/authority/provisioner/oidc_test.go @@ -338,8 +338,6 @@ func TestOIDC_AuthorizeSign(t *testing.T) { case *validityValidator: assert.Equals(t, v.min, tt.prov.ctl.Claimer.MinTLSCertDuration()) assert.Equals(t, v.max, tt.prov.ctl.Claimer.MaxTLSCertDuration()) - case emailOnlyIdentity: - assert.Equals(t, string(v), "name@smallstep.com") case *x509NamePolicyValidator: assert.Equals(t, nil, v.policyEngine) case *WebhookController: diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go index bc0d88ff..c3db239a 100644 --- a/authority/provisioner/sign_options.go +++ b/authority/provisioner/sign_options.go @@ -83,31 +83,6 @@ type AttestationData struct { PermanentIdentifier string } -// emailOnlyIdentity is a CertificateRequestValidator that checks that the only -// SAN provided is the given email address. -type emailOnlyIdentity string - -func (e emailOnlyIdentity) Valid(req *x509.CertificateRequest) error { - switch { - case len(req.DNSNames) > 0: - return errs.Forbidden("certificate request cannot contain DNS names") - case len(req.IPAddresses) > 0: - return errs.Forbidden("certificate request cannot contain IP addresses") - case len(req.URIs) > 0: - return errs.Forbidden("certificate request cannot contain URIs") - case len(req.EmailAddresses) == 0: - return errs.Forbidden("certificate request does not contain any email address") - case len(req.EmailAddresses) > 1: - return errs.Forbidden("certificate request contains too many email addresses") - case req.EmailAddresses[0] == "": - return errs.Forbidden("certificate request cannot contain an empty email address") - case req.EmailAddresses[0] != string(e): - return errs.Forbidden("certificate request does not contain the valid email address - got %s, want %s", req.EmailAddresses[0], e) - default: - return nil - } -} - // defaultPublicKeyValidator validates the public key of a certificate request. type defaultPublicKeyValidator struct{} diff --git a/authority/provisioner/sign_options_test.go b/authority/provisioner/sign_options_test.go index 198462c7..01d2a0cd 100644 --- a/authority/provisioner/sign_options_test.go +++ b/authority/provisioner/sign_options_test.go @@ -16,38 +16,6 @@ import ( "go.step.sm/crypto/pemutil" ) -func Test_emailOnlyIdentity_Valid(t *testing.T) { - uri, err := url.Parse("https://example.com/1.0/getUser") - if err != nil { - t.Fatal(err) - } - - type args struct { - req *x509.CertificateRequest - } - tests := []struct { - name string - e emailOnlyIdentity - args args - wantErr bool - }{ - {"ok", "name@smallstep.com", args{&x509.CertificateRequest{EmailAddresses: []string{"name@smallstep.com"}}}, false}, - {"DNSNames", "name@smallstep.com", args{&x509.CertificateRequest{DNSNames: []string{"foo.bar.zar"}}}, true}, - {"IPAddresses", "name@smallstep.com", args{&x509.CertificateRequest{IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1)}}}, true}, - {"URIs", "name@smallstep.com", args{&x509.CertificateRequest{URIs: []*url.URL{uri}}}, true}, - {"no-emails", "name@smallstep.com", args{&x509.CertificateRequest{EmailAddresses: []string{}}}, true}, - {"empty-email", "", args{&x509.CertificateRequest{EmailAddresses: []string{""}}}, true}, - {"multiple-emails", "name@smallstep.com", args{&x509.CertificateRequest{EmailAddresses: []string{"name@smallstep.com", "foo@smallstep.com"}}}, true}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if err := tt.e.Valid(tt.args.req); (err != nil) != tt.wantErr { - t.Errorf("emailOnlyIdentity.Valid() error = %v, wantErr %v", err, tt.wantErr) - } - }) - } -} - func Test_defaultPublicKeyValidator_Valid(t *testing.T) { _shortRSA, err := pemutil.Read("./testdata/certs/short-rsa.csr") assert.FatalError(t, err) diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index 70dffba2..f027c3a6 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -125,35 +125,6 @@ func (o SignSSHOptions) match(got SignSSHOptions) error { return nil } -// sshCertPrincipalsModifier is an SSHCertModifier that sets the -// principals to the SSH certificate. -type sshCertPrincipalsModifier []string - -// Modify the ValidPrincipals value of the cert. -func (o sshCertPrincipalsModifier) Modify(cert *ssh.Certificate, _ SignSSHOptions) error { - cert.ValidPrincipals = []string(o) - return nil -} - -// sshCertKeyIDModifier is an SSHCertModifier that sets the given -// Key ID in the SSH certificate. -type sshCertKeyIDModifier string - -func (m sshCertKeyIDModifier) Modify(cert *ssh.Certificate, _ SignSSHOptions) error { - cert.KeyId = string(m) - return nil -} - -// sshCertTypeModifier is an SSHCertModifier that sets the -// certificate type. -type sshCertTypeModifier string - -// Modify sets the CertType for the ssh certificate. -func (m sshCertTypeModifier) Modify(cert *ssh.Certificate, _ SignSSHOptions) error { - cert.CertType = sshCertTypeUInt32(string(m)) - return nil -} - // sshCertValidAfterModifier is an SSHCertModifier that sets the // ValidAfter in the SSH certificate. type sshCertValidAfterModifier uint64 @@ -172,51 +143,6 @@ func (m sshCertValidBeforeModifier) Modify(cert *ssh.Certificate, _ SignSSHOptio return nil } -// sshCertDefaultsModifier implements a SSHCertModifier that -// modifies the certificate with the given options if they are not set. -type sshCertDefaultsModifier SignSSHOptions - -// Modify implements the SSHCertModifier interface. -func (m sshCertDefaultsModifier) Modify(cert *ssh.Certificate, _ SignSSHOptions) error { - if cert.CertType == 0 { - cert.CertType = sshCertTypeUInt32(m.CertType) - } - if len(cert.ValidPrincipals) == 0 { - cert.ValidPrincipals = m.Principals - } - if cert.ValidAfter == 0 && !m.ValidAfter.IsZero() { - cert.ValidAfter = uint64(m.ValidAfter.Unix()) - } - if cert.ValidBefore == 0 && !m.ValidBefore.IsZero() { - cert.ValidBefore = uint64(m.ValidBefore.Unix()) - } - return nil -} - -// sshDefaultExtensionModifier implements an SSHCertModifier that sets -// the default extensions in an SSH certificate. -type sshDefaultExtensionModifier struct{} - -func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate, _ SignSSHOptions) error { - switch cert.CertType { - // Default to no extensions for HostCert. - case ssh.HostCert: - return nil - case ssh.UserCert: - if cert.Extensions == nil { - cert.Extensions = make(map[string]string) - } - cert.Extensions["permit-X11-forwarding"] = "" - cert.Extensions["permit-agent-forwarding"] = "" - cert.Extensions["permit-port-forwarding"] = "" - cert.Extensions["permit-pty"] = "" - cert.Extensions["permit-user-rc"] = "" - return nil - default: - return errs.BadRequest("ssh certificate has an unknown type '%d'", cert.CertType) - } -} - // sshDefaultDuration is an SSHCertModifier that sets the certificate // ValidAfter and ValidBefore if they have not been set. It will fail if a // CertType has not been set or is not valid. diff --git a/authority/provisioner/sign_ssh_options_test.go b/authority/provisioner/sign_ssh_options_test.go index 1993295b..550a9f13 100644 --- a/authority/provisioner/sign_ssh_options_test.go +++ b/authority/provisioner/sign_ssh_options_test.go @@ -202,97 +202,6 @@ func TestSSHOptions_Match(t *testing.T) { } } -func Test_sshCertPrincipalsModifier_Modify(t *testing.T) { - type test struct { - modifier sshCertPrincipalsModifier - cert *ssh.Certificate - expected []string - } - tests := map[string]func() test{ - "ok": func() test { - a := []string{"foo", "bar"} - return test{ - modifier: sshCertPrincipalsModifier(a), - cert: new(ssh.Certificate), - expected: a, - } - }, - } - for name, run := range tests { - t.Run(name, func(t *testing.T) { - tc := run() - if assert.Nil(t, tc.modifier.Modify(tc.cert, SignSSHOptions{})) { - assert.Equals(t, tc.cert.ValidPrincipals, tc.expected) - } - }) - } -} - -func Test_sshCertKeyIDModifier_Modify(t *testing.T) { - type test struct { - modifier sshCertKeyIDModifier - cert *ssh.Certificate - expected string - } - tests := map[string]func() test{ - "ok": func() test { - a := "foo" - return test{ - modifier: sshCertKeyIDModifier(a), - cert: new(ssh.Certificate), - expected: a, - } - }, - } - for name, run := range tests { - t.Run(name, func(t *testing.T) { - tc := run() - if assert.Nil(t, tc.modifier.Modify(tc.cert, SignSSHOptions{})) { - assert.Equals(t, tc.cert.KeyId, tc.expected) - } - }) - } -} - -func Test_sshCertTypeModifier_Modify(t *testing.T) { - type test struct { - modifier sshCertTypeModifier - cert *ssh.Certificate - expected uint32 - } - tests := map[string]func() test{ - "ok/user": func() test { - return test{ - modifier: sshCertTypeModifier("user"), - cert: new(ssh.Certificate), - expected: ssh.UserCert, - } - }, - "ok/host": func() test { - return test{ - modifier: sshCertTypeModifier("host"), - cert: new(ssh.Certificate), - expected: ssh.HostCert, - } - }, - "ok/default": func() test { - return test{ - modifier: sshCertTypeModifier("foo"), - cert: new(ssh.Certificate), - expected: 0, - } - }, - } - for name, run := range tests { - t.Run(name, func(t *testing.T) { - tc := run() - if assert.Nil(t, tc.modifier.Modify(tc.cert, SignSSHOptions{})) { - assert.Equals(t, tc.cert.CertType, tc.expected) - } - }) - } -} - func Test_sshCertValidAfterModifier_Modify(t *testing.T) { type test struct { modifier sshCertValidAfterModifier @@ -318,176 +227,6 @@ func Test_sshCertValidAfterModifier_Modify(t *testing.T) { } } -func Test_sshCertDefaultsModifier_Modify(t *testing.T) { - type test struct { - modifier sshCertDefaultsModifier - cert *ssh.Certificate - valid func(*ssh.Certificate) - } - tests := map[string]func() test{ - "ok/changes": func() test { - n := time.Now() - va := NewTimeDuration(n.Add(1 * time.Minute)) - vb := NewTimeDuration(n.Add(5 * time.Minute)) - so := SignSSHOptions{ - Principals: []string{"foo", "bar"}, - CertType: "host", - ValidAfter: va, - ValidBefore: vb, - } - return test{ - modifier: sshCertDefaultsModifier(so), - cert: new(ssh.Certificate), - valid: func(cert *ssh.Certificate) { - assert.Equals(t, cert.ValidPrincipals, so.Principals) - assert.Equals(t, cert.CertType, uint32(ssh.HostCert)) - assert.Equals(t, cert.ValidAfter, uint64(so.ValidAfter.RelativeTime(time.Now()).Unix())) - assert.Equals(t, cert.ValidBefore, uint64(so.ValidBefore.RelativeTime(time.Now()).Unix())) - }, - } - }, - "ok/no-changes": func() test { - n := time.Now() - so := SignSSHOptions{ - Principals: []string{"foo", "bar"}, - CertType: "host", - ValidAfter: NewTimeDuration(n.Add(15 * time.Minute)), - ValidBefore: NewTimeDuration(n.Add(25 * time.Minute)), - } - return test{ - modifier: sshCertDefaultsModifier(so), - cert: &ssh.Certificate{ - CertType: uint32(ssh.UserCert), - ValidPrincipals: []string{"zap", "zoop"}, - ValidAfter: 15, - ValidBefore: 25, - }, - valid: func(cert *ssh.Certificate) { - assert.Equals(t, cert.ValidPrincipals, []string{"zap", "zoop"}) - assert.Equals(t, cert.CertType, uint32(ssh.UserCert)) - assert.Equals(t, cert.ValidAfter, uint64(15)) - assert.Equals(t, cert.ValidBefore, uint64(25)) - }, - } - }, - } - for name, run := range tests { - t.Run(name, func(t *testing.T) { - tc := run() - if assert.Nil(t, tc.modifier.Modify(tc.cert, SignSSHOptions{})) { - tc.valid(tc.cert) - } - }) - } -} - -func Test_sshDefaultExtensionModifier_Modify(t *testing.T) { - type test struct { - modifier sshDefaultExtensionModifier - cert *ssh.Certificate - valid func(*ssh.Certificate) - err error - } - tests := map[string]func() test{ - "fail/unexpected-cert-type": func() test { - cert := &ssh.Certificate{CertType: 3} - return test{ - modifier: sshDefaultExtensionModifier{}, - cert: cert, - err: errors.New("ssh certificate has an unknown type '3'"), - } - }, - "ok/host": func() test { - cert := &ssh.Certificate{CertType: ssh.HostCert} - return test{ - modifier: sshDefaultExtensionModifier{}, - cert: cert, - valid: func(cert *ssh.Certificate) { - assert.Len(t, 0, cert.Extensions) - }, - } - }, - "ok/user/extensions-exists": func() test { - cert := &ssh.Certificate{CertType: ssh.UserCert, Permissions: ssh.Permissions{Extensions: map[string]string{ - "foo": "bar", - }}} - return test{ - modifier: sshDefaultExtensionModifier{}, - cert: cert, - valid: func(cert *ssh.Certificate) { - val, ok := cert.Extensions["foo"] - assert.True(t, ok) - assert.Equals(t, val, "bar") - - val, ok = cert.Extensions["permit-X11-forwarding"] - assert.True(t, ok) - assert.Equals(t, val, "") - - val, ok = cert.Extensions["permit-agent-forwarding"] - assert.True(t, ok) - assert.Equals(t, val, "") - - val, ok = cert.Extensions["permit-port-forwarding"] - assert.True(t, ok) - assert.Equals(t, val, "") - - val, ok = cert.Extensions["permit-pty"] - assert.True(t, ok) - assert.Equals(t, val, "") - - val, ok = cert.Extensions["permit-user-rc"] - assert.True(t, ok) - assert.Equals(t, val, "") - }, - } - }, - "ok/user/no-extensions": func() test { - return test{ - modifier: sshDefaultExtensionModifier{}, - cert: &ssh.Certificate{CertType: ssh.UserCert}, - valid: func(cert *ssh.Certificate) { - _, ok := cert.Extensions["foo"] - assert.False(t, ok) - - val, ok := cert.Extensions["permit-X11-forwarding"] - assert.True(t, ok) - assert.Equals(t, val, "") - - val, ok = cert.Extensions["permit-agent-forwarding"] - assert.True(t, ok) - assert.Equals(t, val, "") - - val, ok = cert.Extensions["permit-port-forwarding"] - assert.True(t, ok) - assert.Equals(t, val, "") - - val, ok = cert.Extensions["permit-pty"] - assert.True(t, ok) - assert.Equals(t, val, "") - - val, ok = cert.Extensions["permit-user-rc"] - assert.True(t, ok) - assert.Equals(t, val, "") - }, - } - }, - } - for name, run := range tests { - t.Run(name, func(t *testing.T) { - tc := run() - if err := tc.modifier.Modify(tc.cert, SignSSHOptions{}); err != nil { - if assert.NotNil(t, tc.err) { - assert.HasPrefix(t, err.Error(), tc.err.Error()) - } - } else { - if assert.Nil(t, tc.err) { - tc.valid(tc.cert) - } - } - }) - } -} - func Test_sshCertDefaultValidator_Valid(t *testing.T) { pub, _, err := keyutil.GenerateDefaultKeyPair() assert.FatalError(t, err) diff --git a/authority/provisioner/x5c_test.go b/authority/provisioner/x5c_test.go index 437b7661..72f9f947 100644 --- a/authority/provisioner/x5c_test.go +++ b/authority/provisioner/x5c_test.go @@ -790,8 +790,6 @@ func TestX5C_AuthorizeSSHSign(t *testing.T) { assert.Equals(t, int64(v), tc.claims.Step.SSH.ValidAfter.RelativeTime(nw).Unix()) case sshCertValidBeforeModifier: assert.Equals(t, int64(v), tc.claims.Step.SSH.ValidBefore.RelativeTime(nw).Unix()) - case sshCertDefaultsModifier: - assert.Equals(t, SignSSHOptions(v), SignSSHOptions{CertType: SSHUserCert}) case *sshLimitDuration: assert.Equals(t, v.Claimer, tc.p.ctl.Claimer) assert.Equals(t, v.NotAfter, x5cCerts[0].NotAfter)