Add more tests

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)

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")
 }

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() {

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...

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)
+			}
+		})
+	}
+}

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 {

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
 }

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)
 			}
 		})
 	}