From 6bc0513468d631920723c760cd6b3ec593e48141 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Mon, 3 Jan 2022 15:32:58 +0100 Subject: [PATCH] Add more tests --- authority/provisioner/acme.go | 1 + authority/provisioner/jwk.go | 12 + authority/provisioner/policy.go | 4 +- policy/ssh/ssh.go | 2 +- policy/ssh/ssh_test.go | 261 +++++++++++ policy/x509/options.go | 7 + policy/x509/x509.go | 40 +- policy/x509/x509_test.go | 771 ++++++++++++++++++++++++++++++-- 8 files changed, 1063 insertions(+), 35 deletions(-) create mode 100644 policy/ssh/ssh_test.go diff --git a/authority/provisioner/acme.go b/authority/provisioner/acme.go index c6cadf51..83d35e49 100755 --- a/authority/provisioner/acme.go +++ b/authority/provisioner/acme.go @@ -117,6 +117,7 @@ func (p *ACME) AuthorizeOrderIdentifier(ctx context.Context, identifier string) return nil } + // assuming only valid identifiers (IP or DNS) are provided var err error if ip := net.ParseIP(identifier); ip != nil { _, err = p.x509PolicyEngine.IsIPAllowed(ip) diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go index 081eb60c..3ee8113f 100755 --- a/authority/provisioner/jwk.go +++ b/authority/provisioner/jwk.go @@ -158,6 +158,7 @@ func (p *JWK) authorizeToken(token string, audiences []string) (*jwtPayload, err // revoke the certificate with serial number in the `sub` property. func (p *JWK) AuthorizeRevoke(ctx context.Context, token string) error { _, err := p.authorizeToken(token, p.audiences.Revoke) + // TODO(hs): authorize the SANs using x509 name policy allow/deny rules (also for other provisioners with AuthorizeRevoke) return errs.Wrap(http.StatusInternalServerError, err, "jwk.AuthorizeRevoke") } @@ -208,9 +209,19 @@ func (p *JWK) AuthorizeRenew(ctx context.Context, cert *x509.Certificate) error if p.claimer.IsDisableRenewal() { return errs.Unauthorized("jwk.AuthorizeRenew; renew is disabled for jwk provisioner '%s'", p.GetName()) } + // TODO(hs): authorize the SANs using x509 name policy allow/deny rules (also for other provisioners with AuthorizeRewew and AuthorizeSSHRenew) + //return p.authorizeRenew(cert) return nil } +// func (p *JWK) authorizeRenew(cert *x509.Certificate) error { +// if p.x509PolicyEngine == nil { +// return nil +// } +// _, err := p.x509PolicyEngine.AreCertificateNamesAllowed(cert) +// return err +// } + // AuthorizeSSHSign returns the list of SignOption for a SignSSH request. func (p *JWK) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, error) { if !p.claimer.IsSSHCAEnabled() { @@ -288,5 +299,6 @@ func (p *JWK) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, // AuthorizeSSHRevoke returns nil if the token is valid, false otherwise. func (p *JWK) AuthorizeSSHRevoke(ctx context.Context, token string) error { _, err := p.authorizeToken(token, p.audiences.SSHRevoke) + // TODO(hs): authorize the principals using SSH name policy allow/deny rules (also for other provisioners with AuthorizeSSHRevoke) return errs.Wrap(http.StatusInternalServerError, err, "jwk.AuthorizeSSHRevoke") } diff --git a/authority/provisioner/policy.go b/authority/provisioner/policy.go index cf436d70..282eabdc 100644 --- a/authority/provisioner/policy.go +++ b/authority/provisioner/policy.go @@ -12,7 +12,9 @@ func newX509PolicyEngine(x509Opts *X509Options) (*x509policy.NamePolicyEngine, e return nil, nil } - options := []x509policy.NamePolicyOption{} + options := []x509policy.NamePolicyOption{ + x509policy.WithEnableSubjectCommonNameVerification(), // enable x509 Subject Common Name validation by default + } allowed := x509Opts.GetAllowedNameOptions() if allowed != nil && allowed.HasNames() { diff --git a/policy/ssh/ssh.go b/policy/ssh/ssh.go index 95e7d471..dcf5394f 100644 --- a/policy/ssh/ssh.go +++ b/policy/ssh/ssh.go @@ -80,7 +80,7 @@ func (e *NamePolicyEngine) validateNames(dnsNames, emails, userNames []string) e /* No regexes for now. But if we ever implement them, they'd probably look like this */ /*"principal": ["foo.smallstep.com", "/^*\.smallstep\.com$/"]*/ - // Principals can be single user names (mariano, max, mike, ...), hostnames/domains (*.smallstep.com, host.smallstep.com, ...) and emails (max@smallstep.com, @smallstep.com, ...) + // Principals can be single user names (mariano, max, mike, ...), hostnames/domains (*.smallstep.com, host.smallstep.com, ...) and "emails" (max@smallstep.com, @smallstep.com, ...) // All ValidPrincipals can thus be any one of those, and they can be mixed (mike@smallstep.com, mike, ...); we need to split this? // Should we assume a generic engine, or can we do it host vs. user based? If host vs. user based, then it becomes easier w.r.t. dns; hosts will only be DNS, right? // If we assume generic, we _may_ have a harder time distinguishing host vs. user certs. We propose to use host + user specific provisioners, though... diff --git a/policy/ssh/ssh_test.go b/policy/ssh/ssh_test.go new file mode 100644 index 00000000..e56ce592 --- /dev/null +++ b/policy/ssh/ssh_test.go @@ -0,0 +1,261 @@ +package sshpolicy + +import ( + "testing" + + "golang.org/x/crypto/ssh" +) + +func TestNamePolicyEngine_ArePrincipalsAllowed(t *testing.T) { + type fields struct { + options []NamePolicyOption + permittedDNSDomains []string + excludedDNSDomains []string + permittedEmailAddresses []string + excludedEmailAddresses []string + permittedPrincipals []string + excludedPrincipals []string + } + tests := []struct { + name string + fields fields + cert *ssh.Certificate + want bool + wantErr bool + }{ + { + name: "fail/dns-permitted", + fields: fields{ + permittedDNSDomains: []string{".local"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{"host.notlocal"}, + }, + want: false, + wantErr: true, + }, + { + name: "fail/dns-permitted", + fields: fields{ + excludedDNSDomains: []string{".local"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{"host.local"}, + }, + want: false, + wantErr: true, + }, + { + name: "fail/mail-permitted", + fields: fields{ + permittedEmailAddresses: []string{"example.local"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{"user@example.notlocal"}, + }, + want: false, + wantErr: true, + }, + { + name: "fail/mail-excluded", + fields: fields{ + excludedEmailAddresses: []string{"example.local"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{"user@example.local"}, + }, + want: false, + wantErr: true, + }, + { + name: "fail/principal-permitted", + fields: fields{ + permittedPrincipals: []string{"user1"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{"user2"}, + }, + want: false, + wantErr: true, + }, + { + name: "fail/principal-excluded", + fields: fields{ + excludedPrincipals: []string{"user"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{"user"}, + }, + want: false, + wantErr: true, + }, + { + name: "fail/combined-complex-all-badhost.local", + fields: fields{ + permittedDNSDomains: []string{".local"}, + permittedEmailAddresses: []string{"example.local"}, + permittedPrincipals: []string{"user"}, + excludedDNSDomains: []string{"badhost.local"}, + excludedEmailAddresses: []string{"badmail@example.local"}, + excludedPrincipals: []string{"baduser"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{ + "user", + "user@example.local", + "badhost.local", + }, + }, + want: false, + wantErr: true, + }, + { + name: "ok/no-constraints", + fields: fields{}, + cert: &ssh.Certificate{ + ValidPrincipals: []string{"host.example.com"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/dns-permitted", + fields: fields{ + permittedDNSDomains: []string{".local"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{"example.local"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/dns-excluded", + fields: fields{ + excludedDNSDomains: []string{".notlocal"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{"example.local"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/mail-permitted", + fields: fields{ + permittedEmailAddresses: []string{"example.local"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{"user@example.local"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/mail-excluded", + fields: fields{ + excludedEmailAddresses: []string{"example.notlocal"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{"user@example.local"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/principal-permitted", + fields: fields{ + permittedPrincipals: []string{"user"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{"user"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/principal-excluded", + fields: fields{ + excludedPrincipals: []string{"someone"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{"user"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/combined-simple-user-permitted", + fields: fields{ + permittedEmailAddresses: []string{"example.local"}, + permittedPrincipals: []string{"user"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{ + "user", + "user@example.local", + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/combined-simple-all-permitted", + fields: fields{ + permittedDNSDomains: []string{".local"}, + permittedEmailAddresses: []string{"example.local"}, + permittedPrincipals: []string{"user"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{ + "user", + "user@example.local", + "host.local", + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/combined-complex-all", + fields: fields{ + permittedDNSDomains: []string{".local"}, + permittedEmailAddresses: []string{"example.local"}, + permittedPrincipals: []string{"user"}, + excludedDNSDomains: []string{"badhost.local"}, + excludedEmailAddresses: []string{"badmail@example.local"}, + excludedPrincipals: []string{"baduser"}, + }, + cert: &ssh.Certificate{ + ValidPrincipals: []string{ + "user", + "user@example.local", + "host.local", + }, + }, + want: true, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := &NamePolicyEngine{ + options: tt.fields.options, + permittedDNSDomains: tt.fields.permittedDNSDomains, + excludedDNSDomains: tt.fields.excludedDNSDomains, + permittedEmailAddresses: tt.fields.permittedEmailAddresses, + excludedEmailAddresses: tt.fields.excludedEmailAddresses, + permittedPrincipals: tt.fields.permittedPrincipals, + excludedPrincipals: tt.fields.excludedPrincipals, + } + got, err := e.ArePrincipalsAllowed(tt.cert) + if (err != nil) != tt.wantErr { + t.Errorf("NamePolicyEngine.ArePrincipalsAllowed() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("NamePolicyEngine.ArePrincipalsAllowed() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/policy/x509/options.go b/policy/x509/options.go index 68f236cb..d3557876 100755 --- a/policy/x509/options.go +++ b/policy/x509/options.go @@ -12,6 +12,13 @@ type NamePolicyOption func(e *NamePolicyEngine) error // TODO: wrap (more) errors; and prove a set of known (exported) errors +func WithEnableSubjectCommonNameVerification() NamePolicyOption { + return func(e *NamePolicyEngine) error { + e.verifySubjectCommonName = true + return nil + } +} + func WithPermittedDNSDomains(domains []string) NamePolicyOption { return func(e *NamePolicyEngine) error { for _, domain := range domains { diff --git a/policy/x509/x509.go b/policy/x509/x509.go index c8d4dfb2..408251cd 100755 --- a/policy/x509/x509.go +++ b/policy/x509/x509.go @@ -3,6 +3,7 @@ package x509policy import ( "bytes" "crypto/x509" + "crypto/x509/pkix" "fmt" "net" "net/url" @@ -50,6 +51,7 @@ func (e CertificateInvalidError) Error() string { // TODO(hs): implement Stringer interface: describe the contents of the NamePolicyEngine? type NamePolicyEngine struct { options []NamePolicyOption + verifySubjectCommonName bool permittedDNSDomains []string excludedDNSDomains []string permittedIPRanges []*net.IPNet @@ -76,7 +78,13 @@ func New(opts ...NamePolicyOption) (*NamePolicyEngine, error) { // AreCertificateNamesAllowed verifies that all SANs in a Certificate are allowed. func (e *NamePolicyEngine) AreCertificateNamesAllowed(cert *x509.Certificate) (bool, error) { - if err := e.validateNames(cert.DNSNames, cert.IPAddresses, cert.EmailAddresses, cert.URIs); err != nil { + dnsNames, ips, emails, uris := cert.DNSNames, cert.IPAddresses, cert.EmailAddresses, cert.URIs + // when Subject Common Name must be verified in addition to the SANs, it is + // added to the appropriate slice of names. + if e.verifySubjectCommonName { + appendSubjectCommonName(cert.Subject, &dnsNames, &ips, &emails, &uris) + } + if err := e.validateNames(dnsNames, ips, emails, uris); err != nil { return false, err } return true, nil @@ -84,7 +92,13 @@ func (e *NamePolicyEngine) AreCertificateNamesAllowed(cert *x509.Certificate) (b // AreCSRNamesAllowed verifies that all names in the CSR are allowed. func (e *NamePolicyEngine) AreCSRNamesAllowed(csr *x509.CertificateRequest) (bool, error) { - if err := e.validateNames(csr.DNSNames, csr.IPAddresses, csr.EmailAddresses, csr.URIs); err != nil { + dnsNames, ips, emails, uris := csr.DNSNames, csr.IPAddresses, csr.EmailAddresses, csr.URIs + // when Subject Common Name must be verified in addition to the SANs, it is + // added to the appropriate slice of names. + if e.verifySubjectCommonName { + appendSubjectCommonName(csr.Subject, &dnsNames, &ips, &emails, &uris) + } + if err := e.validateNames(dnsNames, ips, emails, uris); err != nil { return false, err } return true, nil @@ -116,6 +130,26 @@ func (e *NamePolicyEngine) IsIPAllowed(ip net.IP) (bool, error) { return true, nil } +// appendSubjectCommonName appends the Subject Common Name to the appropriate slice of names. The logic is +// similar as x509util.SplitSANs: if the subject can be parsed as an IP, it's added to the ips. If it can +// be parsed as an URL, it is added to the URIs. If it contains an @, it is added to emails. When it's none +// of these, it's added to the DNS names. +func appendSubjectCommonName(subject pkix.Name, dnsNames *[]string, ips *[]net.IP, emails *[]string, uris *[]*url.URL) { + commonName := subject.CommonName + if commonName == "" { + return + } + if ip := net.ParseIP(commonName); ip != nil { + *ips = append(*ips, ip) + } else if u, err := url.Parse(commonName); err == nil && u.Scheme != "" { + *uris = append(*uris, u) + } else if strings.Contains(commonName, "@") { + *emails = append(*emails, commonName) + } else { + *dnsNames = append(*dnsNames, commonName) + } +} + // validateNames verifies that all names are allowed. // Its logic follows that of (a large part of) the (c *Certificate) isValid() function // in https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go @@ -508,7 +542,7 @@ func matchIPConstraint(ip net.IP, constraint *net.IPNet) (bool, error) { // return false, nil // } - contained := constraint.Contains(ip) // TODO(hs): validate that this is the correct behavior. + contained := constraint.Contains(ip) // TODO(hs): validate that this is the correct behavior; also check IPv4-in-IPv6 (again) return contained, nil } diff --git a/policy/x509/x509_test.go b/policy/x509/x509_test.go index 99c371ff..103c6009 100755 --- a/policy/x509/x509_test.go +++ b/policy/x509/x509_test.go @@ -2,6 +2,7 @@ package x509policy import ( "crypto/x509" + "crypto/x509/pkix" "net" "net/url" "testing" @@ -9,8 +10,11 @@ import ( "github.com/smallstep/assert" ) -func TestGuard_IsAllowed(t *testing.T) { +func TestNamePolicyEngine_AreCertificateNamesAllowed(t *testing.T) { + // TODO(hs): refactor these tests into using validateNames instead of AreCertificateNamesAllowed + // TODO(hs): the functionality in the policy engine is a nice candidate for trying fuzzing on type fields struct { + verifySubjectCommonName bool permittedDNSDomains []string excludedDNSDomains []string permittedIPRanges []*net.IPNet @@ -23,7 +27,7 @@ func TestGuard_IsAllowed(t *testing.T) { tests := []struct { name string fields fields - csr *x509.CertificateRequest + cert *x509.Certificate want bool wantErr bool }{ @@ -32,23 +36,67 @@ func TestGuard_IsAllowed(t *testing.T) { fields: fields{ permittedDNSDomains: []string{".local"}, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ DNSNames: []string{"www.example.com"}, }, want: false, wantErr: true, }, + { + name: "fail/dns-permitted-single-host", + fields: fields{ + permittedDNSDomains: []string{"host.local"}, + }, + cert: &x509.Certificate{ + DNSNames: []string{"differenthost.local"}, + }, + want: false, + wantErr: true, + }, + { + name: "fail/dns-permitted-no-label", + fields: fields{ + permittedDNSDomains: []string{".local"}, + }, + cert: &x509.Certificate{ + DNSNames: []string{"local"}, + }, + want: false, + wantErr: true, + }, + { + name: "fail/dns-permitted-empty-label", + fields: fields{ + permittedDNSDomains: []string{".local"}, + }, + cert: &x509.Certificate{ + DNSNames: []string{"www..local"}, + }, + want: false, + wantErr: true, + }, { name: "fail/dns-excluded", fields: fields{ excludedDNSDomains: []string{"example.com"}, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ DNSNames: []string{"www.example.com"}, }, want: false, wantErr: true, }, + { + name: "fail/dns-excluded-single-host", + fields: fields{ + excludedDNSDomains: []string{"example.com"}, + }, + cert: &x509.Certificate{ + DNSNames: []string{"example.com"}, + }, + want: false, + wantErr: true, + }, { name: "fail/ipv4-permitted", fields: fields{ @@ -59,7 +107,7 @@ func TestGuard_IsAllowed(t *testing.T) { }, }, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ IPAddresses: []net.IP{net.ParseIP("1.1.1.1")}, }, want: false, @@ -75,7 +123,7 @@ func TestGuard_IsAllowed(t *testing.T) { }, }, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}, }, want: false, @@ -91,7 +139,7 @@ func TestGuard_IsAllowed(t *testing.T) { }, }, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ IPAddresses: []net.IP{net.ParseIP("3001:0db8:85a3:0000:0000:8a2e:0370:7334")}, }, want: false, @@ -107,7 +155,7 @@ func TestGuard_IsAllowed(t *testing.T) { }, }, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ IPAddresses: []net.IP{net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334")}, }, want: false, @@ -118,18 +166,29 @@ func TestGuard_IsAllowed(t *testing.T) { fields: fields{ permittedEmailAddresses: []string{"example.local"}, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ EmailAddresses: []string{"mail@example.com"}, }, want: false, wantErr: true, }, + { + name: "fail/mail-permitted-period-domain", + fields: fields{ + permittedEmailAddresses: []string{".example.local"}, // any address in a domain, but not on the host example.local + }, + cert: &x509.Certificate{ + EmailAddresses: []string{"mail@example.local"}, + }, + want: false, + wantErr: true, + }, { name: "fail/mail-excluded", fields: fields{ excludedEmailAddresses: []string{"example.local"}, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ EmailAddresses: []string{"mail@example.local"}, }, want: false, @@ -140,7 +199,7 @@ func TestGuard_IsAllowed(t *testing.T) { fields: fields{ permittedURIDomains: []string{".example.com"}, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ URIs: []*url.URL{ { Scheme: "https", @@ -151,12 +210,282 @@ func TestGuard_IsAllowed(t *testing.T) { want: false, wantErr: true, }, + { + name: "fail/uri-permitted-period-host", + fields: fields{ + permittedURIDomains: []string{".example.local"}, + }, + cert: &x509.Certificate{ + URIs: []*url.URL{ + { + Scheme: "https", + Host: "example.local", + }, + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/uri-permitted-period-host-certificate", + fields: fields{ + permittedURIDomains: []string{".example.local"}, + }, + cert: &x509.Certificate{ + URIs: []*url.URL{ + { + Scheme: "https", + Host: ".example.local", + }, + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/uri-permitted-empty-host", + fields: fields{ + permittedURIDomains: []string{".example.com"}, + }, + cert: &x509.Certificate{ + URIs: []*url.URL{ + { + Scheme: "https", + Host: "", + }, + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/uri-permitted-port-missing", + fields: fields{ + permittedURIDomains: []string{".example.com"}, + }, + cert: &x509.Certificate{ + URIs: []*url.URL{ + { + Scheme: "https", + Host: "example.local::", + }, + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/uri-permitted-ip", + fields: fields{ + permittedURIDomains: []string{".example.com"}, + }, + cert: &x509.Certificate{ + URIs: []*url.URL{ + { + Scheme: "https", + Host: "127.0.0.1", + }, + }, + }, + want: false, + wantErr: true, + }, { name: "fail/uri-excluded", fields: fields{ excludedURIDomains: []string{".example.local"}, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ + URIs: []*url.URL{ + { + Scheme: "https", + Host: "www.example.local", + }, + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/subject-dns-permitted", + fields: fields{ + verifySubjectCommonName: true, + permittedDNSDomains: []string{".local"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "example.notlocal", + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/subject-dns-excluded", + fields: fields{ + verifySubjectCommonName: true, + excludedDNSDomains: []string{".local"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "example.local", + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/subject-ipv4-permitted", + fields: fields{ + verifySubjectCommonName: true, + permittedIPRanges: []*net.IPNet{ + { + IP: net.ParseIP("127.0.0.1"), + Mask: net.IPv4Mask(255, 255, 255, 0), + }, + }, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "10.10.10.10", + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/subject-ipv4-excluded", + fields: fields{ + verifySubjectCommonName: true, + excludedIPRanges: []*net.IPNet{ + { + IP: net.ParseIP("127.0.0.1"), + Mask: net.IPv4Mask(255, 255, 255, 0), + }, + }, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "127.0.0.1", + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/subject-ipv6-permitted", + fields: fields{ + verifySubjectCommonName: true, + permittedIPRanges: []*net.IPNet{ + { + IP: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334"), + Mask: net.CIDRMask(120, 128), + }, + }, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "2002:0db8:85a3:0000:0000:8a2e:0370:7339", + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/subject-ipv6-excluded", + fields: fields{ + verifySubjectCommonName: true, + excludedIPRanges: []*net.IPNet{ + { + IP: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334"), + Mask: net.CIDRMask(120, 128), + }, + }, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "2001:0db8:85a3:0000:0000:8a2e:0370:7339", + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/subject-email-permitted", + fields: fields{ + verifySubjectCommonName: true, + permittedEmailAddresses: []string{"example.local"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "mail@smallstep.com", + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/subject-email-excluded", + fields: fields{ + verifySubjectCommonName: true, + excludedEmailAddresses: []string{"example.local"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "mail@example.local", + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/subject-uri-permitted", + fields: fields{ + verifySubjectCommonName: true, + permittedURIDomains: []string{".example.com"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "https://www.google.com", + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/subject-uri-excluded", + fields: fields{ + verifySubjectCommonName: true, + excludedURIDomains: []string{".example.com"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "https://www.example.com", + }, + }, + want: false, + wantErr: true, + }, + { + name: "fail/combined-simple-all-badhost.local", + fields: fields{ + verifySubjectCommonName: true, + permittedDNSDomains: []string{".local"}, + permittedIPRanges: []*net.IPNet{{IP: net.ParseIP("127.0.0.1"), Mask: net.IPv4Mask(255, 255, 255, 0)}}, + permittedEmailAddresses: []string{"example.local"}, + permittedURIDomains: []string{".example.local"}, + excludedDNSDomains: []string{"badhost.local"}, + excludedIPRanges: []*net.IPNet{{IP: net.ParseIP("1.1.1.1"), Mask: net.IPv4Mask(255, 255, 255, 0)}}, + excludedEmailAddresses: []string{"badmail@example.local"}, + excludedURIDomains: []string{"https://badwww.example.local"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "badhost.local", + }, + DNSNames: []string{"example.local"}, + IPAddresses: []net.IP{net.ParseIP("127.0.0.130")}, + EmailAddresses: []string{"mail@example.local"}, URIs: []*url.URL{ { Scheme: "https", @@ -170,25 +499,47 @@ func TestGuard_IsAllowed(t *testing.T) { { name: "ok/no-constraints", fields: fields{}, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ DNSNames: []string{"www.example.com"}, }, want: true, wantErr: false, }, { - name: "ok/dns", + name: "ok/empty-dns-constraint", fields: fields{ - permittedDNSDomains: []string{".local"}, + permittedDNSDomains: []string{""}, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ DNSNames: []string{"example.local"}, }, want: true, wantErr: false, }, { - name: "ok/ipv4", + name: "ok/dns-permitted", + fields: fields{ + permittedDNSDomains: []string{".local"}, + }, + cert: &x509.Certificate{ + DNSNames: []string{"example.local"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/dns-excluded", + fields: fields{ + excludedDNSDomains: []string{".notlocal"}, + }, + cert: &x509.Certificate{ + DNSNames: []string{"example.local"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/ipv4-permitted", fields: fields{ permittedIPRanges: []*net.IPNet{ { @@ -197,14 +548,30 @@ func TestGuard_IsAllowed(t *testing.T) { }, }, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ IPAddresses: []net.IP{net.ParseIP("127.0.0.20")}, }, want: true, wantErr: false, }, { - name: "ok/ipv6", + name: "ok/ipv4-excluded", + fields: fields{ + excludedIPRanges: []*net.IPNet{ + { + IP: net.ParseIP("127.0.0.1"), + Mask: net.IPv4Mask(255, 255, 255, 0), + }, + }, + }, + cert: &x509.Certificate{ + IPAddresses: []net.IP{net.ParseIP("10.10.10.10")}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/ipv6-permitted", fields: fields{ permittedIPRanges: []*net.IPNet{ { @@ -213,29 +580,89 @@ func TestGuard_IsAllowed(t *testing.T) { }, }, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ IPAddresses: []net.IP{net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7339")}, }, want: true, wantErr: false, }, { - name: "ok/mail", + name: "ok/ipv6-excluded", + fields: fields{ + excludedIPRanges: []*net.IPNet{ + { + IP: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334"), + Mask: net.CIDRMask(120, 128), + }, + }, + }, + cert: &x509.Certificate{ + IPAddresses: []net.IP{net.ParseIP("2003:0db8:85a3:0000:0000:8a2e:0370:7334")}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/mail-permitted", fields: fields{ permittedEmailAddresses: []string{"example.local"}, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ EmailAddresses: []string{"mail@example.local"}, }, want: true, wantErr: false, }, { - name: "ok/uri", + name: "ok/mail-permitted-with-period-domain", + fields: fields{ + permittedEmailAddresses: []string{".example.local"}, + }, + cert: &x509.Certificate{ + EmailAddresses: []string{"mail@somehost.example.local"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/mail-permitted-with-multiple-labels", + fields: fields{ + permittedEmailAddresses: []string{".example.local"}, + }, + cert: &x509.Certificate{ + EmailAddresses: []string{"mail@sub.www.example.local"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/mail-excluded", + fields: fields{ + excludedEmailAddresses: []string{"example.notlocal"}, + }, + cert: &x509.Certificate{ + EmailAddresses: []string{"mail@example.local"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/mail-excluded-with-period-domain", + fields: fields{ + excludedEmailAddresses: []string{".example.notlocal"}, + }, + cert: &x509.Certificate{ + EmailAddresses: []string{"mail@somehost.example.local"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/uri-permitted", fields: fields{ permittedURIDomains: []string{".example.com"}, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ URIs: []*url.URL{ { Scheme: "https", @@ -247,14 +674,241 @@ func TestGuard_IsAllowed(t *testing.T) { wantErr: false, }, { - name: "ok/combined-simple", + name: "ok/uri-permitted-with-port", fields: fields{ + permittedURIDomains: []string{".example.com"}, + }, + cert: &x509.Certificate{ + URIs: []*url.URL{ + { + Scheme: "https", + Host: "www.example.com:8080", + }, + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/uri-sub-permitted", + fields: fields{ + permittedURIDomains: []string{"example.com"}, + }, + cert: &x509.Certificate{ + URIs: []*url.URL{ + { + Scheme: "https", + Host: "sub.host.example.com", + }, + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/uri-excluded", + fields: fields{ + excludedURIDomains: []string{".google.com"}, + }, + cert: &x509.Certificate{ + URIs: []*url.URL{ + { + Scheme: "https", + Host: "www.example.com", + }, + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/subject-empty", + fields: fields{ + verifySubjectCommonName: true, + permittedDNSDomains: []string{".local"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "", + }, + DNSNames: []string{"example.local"}, + }, + want: true, + wantErr: false, + }, + { + name: "ok/subject-dns-permitted", + fields: fields{ + verifySubjectCommonName: true, + permittedDNSDomains: []string{".local"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "example.local", + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/subject-dns-excluded", + fields: fields{ + verifySubjectCommonName: true, + excludedDNSDomains: []string{".notlocal"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "example.local", + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/subject-ipv4-permitted", + fields: fields{ + verifySubjectCommonName: true, + permittedIPRanges: []*net.IPNet{ + { + IP: net.ParseIP("127.0.0.1"), + Mask: net.IPv4Mask(255, 255, 255, 0), + }, + }, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "127.0.0.20", + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/subject-ipv4-excluded", + fields: fields{ + verifySubjectCommonName: true, + excludedIPRanges: []*net.IPNet{ + { + IP: net.ParseIP("128.0.0.1"), + Mask: net.IPv4Mask(255, 255, 255, 0), + }, + }, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "127.0.0.1", + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/subject-ipv6-permitted", + fields: fields{ + verifySubjectCommonName: true, + permittedIPRanges: []*net.IPNet{ + { + IP: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334"), + Mask: net.CIDRMask(120, 128), + }, + }, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "2001:0db8:85a3:0000:0000:8a2e:0370:7339", + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/subject-ipv6-excluded", + fields: fields{ + verifySubjectCommonName: true, + excludedIPRanges: []*net.IPNet{ + { + IP: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334"), + Mask: net.CIDRMask(120, 128), + }, + }, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "2009:0db8:85a3:0000:0000:8a2e:0370:7339", + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/subject-email-permitted", + fields: fields{ + verifySubjectCommonName: true, + permittedEmailAddresses: []string{"example.local"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "mail@example.local", + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/subject-email-excluded", + fields: fields{ + verifySubjectCommonName: true, + excludedEmailAddresses: []string{"example.notlocal"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "mail@example.local", + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/subject-uri-permitted", + fields: fields{ + verifySubjectCommonName: true, + permittedURIDomains: []string{".example.com"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "https://www.example.com", + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/subject-uri-excluded", + fields: fields{ + verifySubjectCommonName: true, + excludedURIDomains: []string{".google.com"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "https://www.example.com", + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/combined-simple-permitted", + fields: fields{ + verifySubjectCommonName: true, permittedDNSDomains: []string{".local"}, permittedIPRanges: []*net.IPNet{{IP: net.ParseIP("127.0.0.1"), Mask: net.IPv4Mask(255, 255, 255, 0)}}, permittedEmailAddresses: []string{"example.local"}, permittedURIDomains: []string{".example.local"}, }, - csr: &x509.CertificateRequest{ + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "somehost.local", + }, DNSNames: []string{"example.local"}, IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}, EmailAddresses: []string{"mail@example.local"}, @@ -268,12 +922,69 @@ func TestGuard_IsAllowed(t *testing.T) { want: true, wantErr: false, }, - // TODO: more complex uses cases that combine multiple names + { + name: "ok/combined-simple-permitted-without-subject-verification", + fields: fields{ + verifySubjectCommonName: false, + permittedDNSDomains: []string{".local"}, + permittedIPRanges: []*net.IPNet{{IP: net.ParseIP("127.0.0.1"), Mask: net.IPv4Mask(255, 255, 255, 0)}}, + permittedEmailAddresses: []string{"example.local"}, + permittedURIDomains: []string{".example.local"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "forbidden-but-non-verified-domain.example.com", + }, + DNSNames: []string{"example.local"}, + IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}, + EmailAddresses: []string{"mail@example.local"}, + URIs: []*url.URL{ + { + Scheme: "https", + Host: "www.example.local", + }, + }, + }, + want: true, + wantErr: false, + }, + { + name: "ok/combined-simple-all", + fields: fields{ + verifySubjectCommonName: true, + permittedDNSDomains: []string{".local"}, + permittedIPRanges: []*net.IPNet{{IP: net.ParseIP("127.0.0.1"), Mask: net.IPv4Mask(255, 255, 255, 0)}}, + permittedEmailAddresses: []string{"example.local"}, + permittedURIDomains: []string{".example.local"}, + excludedDNSDomains: []string{"badhost.local"}, + excludedIPRanges: []*net.IPNet{{IP: net.ParseIP("127.0.0.128"), Mask: net.IPv4Mask(255, 255, 255, 128)}}, + excludedEmailAddresses: []string{"badmail@example.local"}, + excludedURIDomains: []string{"https://badwww.example.local"}, + }, + cert: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "somehost.local", + }, + DNSNames: []string{"example.local"}, + IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}, + EmailAddresses: []string{"mail@example.local"}, + URIs: []*url.URL{ + { + Scheme: "https", + Host: "www.example.local", + }, + }, + }, + want: true, + wantErr: false, + }, + // TODO: more complex uses cases that combine multiple names and permitted/excluded entries // TODO: check errors (reasons) are as expected } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := &NamePolicyEngine{ + verifySubjectCommonName: tt.fields.verifySubjectCommonName, permittedDNSDomains: tt.fields.permittedDNSDomains, excludedDNSDomains: tt.fields.excludedDNSDomains, permittedIPRanges: tt.fields.permittedIPRanges, @@ -283,16 +994,16 @@ func TestGuard_IsAllowed(t *testing.T) { permittedURIDomains: tt.fields.permittedURIDomains, excludedURIDomains: tt.fields.excludedURIDomains, } - got, err := g.AreCSRNamesAllowed(tt.csr) + got, err := g.AreCertificateNamesAllowed(tt.cert) if (err != nil) != tt.wantErr { - t.Errorf("Guard.IsAllowed() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("NamePolicyEngine.AreCertificateNamesAllowed() error = %v, wantErr %v", err, tt.wantErr) return } if err != nil { assert.NotEquals(t, "", err.Error()) // TODO(hs): make this a complete equality check } if got != tt.want { - t.Errorf("Guard.IsAllowed() = %v, want %v", got, tt.want) + t.Errorf("NamePolicyEngine.AreCertificateNamesAllowed() = %v, want %v", got, tt.want) } }) }