Refactor policy engines into container

acme/api/order.go

@@ -5,7 +5,6 @@ import (
 	"crypto/x509"
 	"encoding/base64"
 	"encoding/json"
-	"fmt"
 	"net"
 	"net/http"
 	"strings"
@@ -199,14 +198,7 @@ func isIdentifierAllowed(acmePolicy policy.X509Policy, identifier acme.Identifie
 	if acmePolicy == nil {
 		return nil
 	}
-	allowed, err := acmePolicy.AreSANsAllowed([]string{identifier.Value})
-	if err != nil {
-		return err
-	}
-	if !allowed {
-		return fmt.Errorf("acme identifier '%s' not allowed", identifier.Value)
-	}
-	return nil
+	return acmePolicy.AreSANsAllowed([]string{identifier.Value})
 }
 
 func newACMEPolicyEngine(eak *acme.ExternalAccountKey) (policy.X509Policy, error) {

authority/authority.go

@@ -81,9 +81,7 @@ type Authority struct {
 	authorizeSSHRenewFunc provisioner.AuthorizeSSHRenewFunc
 
 	// Policy engines
-	x509Policy    policy.X509Policy
-	sshUserPolicy policy.UserPolicy
-	sshHostPolicy policy.HostPolicy
+	policyEngine *policy.Engine
 
 	adminMutex sync.RWMutex
 }

authority/policy.go

@@ -227,50 +227,19 @@ func (a *Authority) reloadPolicyEngines(ctx context.Context) error {
 		policyOptions = a.config.AuthorityConfig.Policy
 	}
 
-	// if no new or updated policy option is set, clear policy engines that (may have)
-	// been configured before and return early
-	if policyOptions == nil {
-		a.x509Policy = nil
-		a.sshHostPolicy = nil
-		a.sshUserPolicy = nil
-		return nil
-	}
-
-	var (
-		x509Policy    authPolicy.X509Policy
-		sshHostPolicy authPolicy.HostPolicy
-		sshUserPolicy authPolicy.UserPolicy
-	)
-
-	// initialize the x509 allow/deny policy engine
-	if x509Policy, err = authPolicy.NewX509PolicyEngine(policyOptions.GetX509Options()); err != nil {
-		return err
-	}
-
-	// initialize the SSH allow/deny policy engine for host certificates
-	if sshHostPolicy, err = authPolicy.NewSSHHostPolicyEngine(policyOptions.GetSSHOptions()); err != nil {
-		return err
-	}
-
-	// initialize the SSH allow/deny policy engine for user certificates
-	if sshUserPolicy, err = authPolicy.NewSSHUserPolicyEngine(policyOptions.GetSSHOptions()); err != nil {
+	engine, err := authPolicy.New(policyOptions)
+	if err != nil {
 		return err
 	}
 
-	// set all policy engines; all or nothing
-	a.x509Policy = x509Policy
-	a.sshHostPolicy = sshHostPolicy
-	a.sshUserPolicy = sshUserPolicy
+	// only update the policy engine when no error was returned
+	a.policyEngine = engine
 
 	return nil
 }
 
 func isAllowed(engine authPolicy.X509Policy, sans []string) error {
-	var (
-		allowed bool
-		err     error
-	)
-	if allowed, err = engine.AreSANsAllowed(sans); err != nil {
+	if err := engine.AreSANsAllowed(sans); err != nil {
 		var policyErr *policy.NamePolicyError
 		isNamePolicyError := errors.As(err, &policyErr)
 		if isNamePolicyError && policyErr.Reason == policy.NotAllowed {
@@ -285,13 +254,6 @@ func isAllowed(engine authPolicy.X509Policy, sans []string) error {
 		}
 	}
 
-	if !allowed {
-		return &PolicyError{
-			Typ: AdminLockOut,
-			Err: fmt.Errorf("the provided policy would lock out %s from the CA. Please update your policy to include %s as an allowed name", sans, sans),
-		}
-	}
-
 	return nil
 }
 

authority/policy/engine.go

@@ -0,0 +1,114 @@
+package policy
+
+import (
+	"crypto/x509"
+	"errors"
+	"fmt"
+
+	"golang.org/x/crypto/ssh"
+)
+
+// Engine is a container for multiple policies.
+type Engine struct {
+	x509Policy    X509Policy
+	sshUserPolicy UserPolicy
+	sshHostPolicy HostPolicy
+}
+
+// New returns a new Engine using Options.
+func New(options *Options) (*Engine, error) {
+
+	// if no options provided, return early
+	if options == nil {
+		return nil, nil
+	}
+
+	var (
+		x509Policy    X509Policy
+		sshHostPolicy HostPolicy
+		sshUserPolicy UserPolicy
+		err           error
+	)
+
+	// initialize the x509 allow/deny policy engine
+	if x509Policy, err = NewX509PolicyEngine(options.GetX509Options()); err != nil {
+		return nil, err
+	}
+
+	// initialize the SSH allow/deny policy engine for host certificates
+	if sshHostPolicy, err = NewSSHHostPolicyEngine(options.GetSSHOptions()); err != nil {
+		return nil, err
+	}
+
+	// initialize the SSH allow/deny policy engine for user certificates
+	if sshUserPolicy, err = NewSSHUserPolicyEngine(options.GetSSHOptions()); err != nil {
+		return nil, err
+	}
+
+	return &Engine{
+		x509Policy:    x509Policy,
+		sshHostPolicy: sshHostPolicy,
+		sshUserPolicy: sshUserPolicy,
+	}, nil
+}
+
+// IsX509CertificateAllowed evaluates an X.509 certificate against
+// the X.509 policy (if available) and returns an error if one of the
+// names in the certificate is not allowed.
+func (e *Engine) IsX509CertificateAllowed(cert *x509.Certificate) error {
+
+	// return early if there's no policy to evaluate
+	if e == nil || e.x509Policy == nil {
+		return nil
+	}
+
+	// return result of X.509 policy evaluation
+	return e.x509Policy.IsX509CertificateAllowed(cert)
+}
+
+// AreSANsAllowed evaluates the slice of SANs against the X.509 policy
+// (if available) and returns an error if one of the SANs is not allowed.
+func (e *Engine) AreSANsAllowed(sans []string) error {
+
+	// return early if there's no policy to evaluate
+	if e == nil || e.x509Policy == nil {
+		return nil
+	}
+
+	// return result of X.509 policy evaluation
+	return e.x509Policy.AreSANsAllowed(sans)
+}
+
+// IsSSHCertificateAllowed evaluates an SSH certificate against the
+// user or host policy (if configured) and returns an error if one of the
+// principals in the certificate is not allowed.
+func (e *Engine) IsSSHCertificateAllowed(cert *ssh.Certificate) error {
+
+	// return early if there's no policy to evaluate
+	if e == nil || (e.sshHostPolicy == nil && e.sshUserPolicy == nil) {
+		return nil
+	}
+
+	switch cert.CertType {
+	case ssh.HostCert:
+		// when no host policy engine is configured, but a user policy engine is
+		// configured, the host certificate is denied.
+		if e.sshHostPolicy == nil && e.sshUserPolicy != nil {
+			return errors.New("authority not allowed to sign ssh host certificates")
+		}
+
+		// return result of SSH host policy evaluation
+		return e.sshHostPolicy.IsSSHCertificateAllowed(cert)
+	case ssh.UserCert:
+		// 	when no user policy engine is configured, but a host policy engine is
+		// 	configured, the user certificate is denied.
+		if e.sshUserPolicy == nil && e.sshHostPolicy != nil {
+			return errors.New("authority not allowed to sign ssh user certificates")
+		}
+
+		// return result of SSH user policy evaluation
+		return e.sshUserPolicy.IsSSHCertificateAllowed(cert)
+	default:
+		return fmt.Errorf("unexpected ssh certificate type %q", cert.CertType)
+	}
+}

authority/policy_test.go

@@ -330,105 +330,193 @@ func Test_policyToCertificates(t *testing.T) {
 	}
 }
 
+func mustPolicyEngine(t *testing.T, options *policy.Options) *policy.Engine {
+	engine, err := policy.New(options)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return engine
+}
+
 func TestAuthority_reloadPolicyEngines(t *testing.T) {
 
-	existingX509PolicyEngine, err := policy.NewX509PolicyEngine(&policy.X509PolicyOptions{
-		AllowedNames: &policy.X509NameOptions{
-			DNSDomains: []string{"*.hosts.example.com"},
+	existingPolicyEngine, err := policy.New(&policy.Options{
+		X509: &policy.X509PolicyOptions{
+			AllowedNames: &policy.X509NameOptions{
+				DNSDomains: []string{"*.hosts.example.com"},
+			},
+		},
+		SSH: &policy.SSHPolicyOptions{
+			Host: &policy.SSHHostCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					DNSDomains: []string{"*.hosts.example.com"},
+				},
+			},
+			User: &policy.SSHUserCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					EmailAddresses: []string{"@mails.example.com"},
+				},
+			},
 		},
 	})
 	assert.NoError(t, err)
 
-	existingSSHHostPolicyEngine, err := policy.NewSSHHostPolicyEngine(&policy.SSHPolicyOptions{
-		Host: &policy.SSHHostCertificateOptions{
-			AllowedNames: &policy.SSHNameOptions{
-				DNSDomains: []string{"*.hosts.example.com"},
+	newX509Options := &policy.Options{
+		X509: &policy.X509PolicyOptions{
+			AllowedNames: &policy.X509NameOptions{
+				DNSDomains: []string{"*.local"},
 			},
+			DeniedNames: &policy.X509NameOptions{
+				DNSDomains: []string{"badhost.local"},
+			},
+			AllowWildcardLiteral:          true,
+			DisableCommonNameVerification: false,
 		},
-	})
-	assert.NoError(t, err)
+	}
 
-	existingSSHUserPolicyEngine, err := policy.NewSSHUserPolicyEngine(&policy.SSHPolicyOptions{
-		User: &policy.SSHUserCertificateOptions{
-			AllowedNames: &policy.SSHNameOptions{
-				EmailAddresses: []string{"@mails.example.com"},
+	newSSHHostOptions := &policy.Options{
+		SSH: &policy.SSHPolicyOptions{
+			Host: &policy.SSHHostCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					DNSDomains: []string{"*.local"},
+				},
+				DeniedNames: &policy.SSHNameOptions{
+					DNSDomains: []string{"badhost.local"},
+				},
 			},
 		},
-	})
-	assert.NoError(t, err)
+	}
 
-	newX509PolicyEngine, err := policy.NewX509PolicyEngine(&policy.X509PolicyOptions{
-		AllowedNames: &policy.X509NameOptions{
-			DNSDomains: []string{"*.local"},
+	newSSHUserOptions := &policy.Options{
+		SSH: &policy.SSHPolicyOptions{
+			User: &policy.SSHUserCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					Principals: []string{"*"},
+				},
+				DeniedNames: &policy.SSHNameOptions{
+					Principals: []string{"root"},
+				},
+			},
 		},
-		DeniedNames: &policy.X509NameOptions{
-			DNSDomains: []string{"badhost.local"},
+	}
+
+	newSSHOptions := &policy.Options{
+		SSH: &policy.SSHPolicyOptions{
+			Host: &policy.SSHHostCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					DNSDomains: []string{"*.local"},
+				},
+				DeniedNames: &policy.SSHNameOptions{
+					DNSDomains: []string{"badhost.local"},
+				},
+			},
+			User: &policy.SSHUserCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					Principals: []string{"*"},
+				},
+				DeniedNames: &policy.SSHNameOptions{
+					Principals: []string{"root"},
+				},
+			},
 		},
-		AllowWildcardLiteral:          true,
-		DisableCommonNameVerification: false,
-	})
-	assert.NoError(t, err)
+	}
 
-	newSSHHostPolicyEngine, err := policy.NewSSHHostPolicyEngine(&policy.SSHPolicyOptions{
-		Host: &policy.SSHHostCertificateOptions{
-			AllowedNames: &policy.SSHNameOptions{
+	newOptions := &policy.Options{
+		X509: &policy.X509PolicyOptions{
+			AllowedNames: &policy.X509NameOptions{
 				DNSDomains: []string{"*.local"},
 			},
-			DeniedNames: &policy.SSHNameOptions{
+			DeniedNames: &policy.X509NameOptions{
 				DNSDomains: []string{"badhost.local"},
 			},
+			AllowWildcardLiteral:          true,
+			DisableCommonNameVerification: false,
 		},
-	})
-	assert.NoError(t, err)
-
-	newSSHUserPolicyEngine, err := policy.NewSSHUserPolicyEngine(&policy.SSHPolicyOptions{
-		User: &policy.SSHUserCertificateOptions{
-			AllowedNames: &policy.SSHNameOptions{
-				Principals: []string{"*"},
+		SSH: &policy.SSHPolicyOptions{
+			Host: &policy.SSHHostCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					DNSDomains: []string{"*.local"},
+				},
+				DeniedNames: &policy.SSHNameOptions{
+					DNSDomains: []string{"badhost.local"},
+				},
 			},
-			DeniedNames: &policy.SSHNameOptions{
-				Principals: []string{"root"},
+			User: &policy.SSHUserCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					Principals: []string{"*"},
+				},
+				DeniedNames: &policy.SSHNameOptions{
+					Principals: []string{"root"},
+				},
 			},
 		},
-	})
-	assert.NoError(t, err)
+	}
 
-	newAdminX509PolicyEngine, err := policy.NewX509PolicyEngine(&policy.X509PolicyOptions{
-		AllowedNames: &policy.X509NameOptions{
-			DNSDomains: []string{"*.local"},
+	newAdminX509Options := &policy.Options{
+		X509: &policy.X509PolicyOptions{
+			AllowedNames: &policy.X509NameOptions{
+				DNSDomains: []string{"*.local"},
+			},
 		},
-	})
-	assert.NoError(t, err)
+	}
 
-	newAdminSSHHostPolicyEngine, err := policy.NewSSHHostPolicyEngine(&policy.SSHPolicyOptions{
-		Host: &policy.SSHHostCertificateOptions{
-			AllowedNames: &policy.SSHNameOptions{
-				DNSDomains: []string{"*.local"},
+	newAdminSSHHostOptions := &policy.Options{
+		SSH: &policy.SSHPolicyOptions{
+			Host: &policy.SSHHostCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					DNSDomains: []string{"*.local"},
+				},
 			},
 		},
-	})
-	assert.NoError(t, err)
+	}
 
-	newAdminSSHUserPolicyEngine, err := policy.NewSSHUserPolicyEngine(&policy.SSHPolicyOptions{
-		User: &policy.SSHUserCertificateOptions{
-			AllowedNames: &policy.SSHNameOptions{
-				EmailAddresses: []string{"@example.com"},
+	newAdminSSHUserOptions := &policy.Options{
+		SSH: &policy.SSHPolicyOptions{
+			User: &policy.SSHUserCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					EmailAddresses: []string{"@example.com"},
+				},
 			},
 		},
-	})
-	assert.NoError(t, err)
+	}
 
-	type expected struct {
-		x509Policy    policy.X509Policy
-		sshUserPolicy policy.UserPolicy
-		sshHostPolicy policy.HostPolicy
+	newAdminOptions := &policy.Options{
+		X509: &policy.X509PolicyOptions{
+			AllowedNames: &policy.X509NameOptions{
+				DNSDomains: []string{"*.local"},
+			},
+			DeniedNames: &policy.X509NameOptions{
+				DNSDomains: []string{"badhost.local"},
+			},
+			AllowWildcardLiteral:          true,
+			DisableCommonNameVerification: false,
+		},
+		SSH: &policy.SSHPolicyOptions{
+			Host: &policy.SSHHostCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					DNSDomains: []string{"*.local"},
+				},
+				DeniedNames: &policy.SSHNameOptions{
+					DNSDomains: []string{"badhost.local"},
+				},
+			},
+			User: &policy.SSHUserCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					EmailAddresses: []string{"@example.com"},
+				},
+				DeniedNames: &policy.SSHNameOptions{
+					EmailAddresses: []string{"baduser@example.com"},
+				},
+			},
+		},
 	}
+
 	tests := []struct {
 		name     string
 		config   *config.Config
 		adminDB  admin.DB
 		ctx      context.Context
-		expected *expected
+		expected *policy.Engine
 		wantErr  bool
 	}{
 		{
@@ -445,13 +533,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					},
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: true,
-			expected: &expected{
-				x509Policy:    existingX509PolicyEngine,
-				sshUserPolicy: existingSSHUserPolicyEngine,
-				sshHostPolicy: existingSSHHostPolicyEngine,
-			},
+			ctx:      context.Background(),
+			wantErr:  true,
+			expected: existingPolicyEngine,
 		},
 		{
 			name: "fail/standalone-ssh-host-policy",
@@ -469,13 +553,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					},
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: true,
-			expected: &expected{
-				x509Policy:    existingX509PolicyEngine,
-				sshUserPolicy: existingSSHUserPolicyEngine,
-				sshHostPolicy: existingSSHHostPolicyEngine,
-			},
+			ctx:      context.Background(),
+			wantErr:  true,
+			expected: existingPolicyEngine,
 		},
 		{
 			name: "fail/standalone-ssh-user-policy",
@@ -493,13 +573,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					},
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: true,
-			expected: &expected{
-				x509Policy:    existingX509PolicyEngine,
-				sshUserPolicy: existingSSHUserPolicyEngine,
-				sshHostPolicy: existingSSHHostPolicyEngine,
-			},
+			ctx:      context.Background(),
+			wantErr:  true,
+			expected: existingPolicyEngine,
 		},
 		{
 			name: "fail/adminDB.GetAuthorityPolicy-error",
@@ -513,13 +589,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					return nil, errors.New("force")
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: true,
-			expected: &expected{
-				x509Policy:    existingX509PolicyEngine,
-				sshUserPolicy: existingSSHUserPolicyEngine,
-				sshHostPolicy: existingSSHHostPolicyEngine,
-			},
+			ctx:      context.Background(),
+			wantErr:  true,
+			expected: existingPolicyEngine,
 		},
 		{
 			name: "fail/admin-x509-policy",
@@ -539,13 +611,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					}, nil
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: true,
-			expected: &expected{
-				x509Policy:    existingX509PolicyEngine,
-				sshUserPolicy: existingSSHUserPolicyEngine,
-				sshHostPolicy: existingSSHHostPolicyEngine,
-			},
+			ctx:      context.Background(),
+			wantErr:  true,
+			expected: existingPolicyEngine,
 		},
 		{
 			name: "fail/admin-ssh-host-policy",
@@ -567,13 +635,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					}, nil
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: true,
-			expected: &expected{
-				x509Policy:    existingX509PolicyEngine,
-				sshUserPolicy: existingSSHUserPolicyEngine,
-				sshHostPolicy: existingSSHHostPolicyEngine,
-			},
+			ctx:      context.Background(),
+			wantErr:  true,
+			expected: existingPolicyEngine,
 		},
 		{
 			name: "fail/admin-ssh-user-policy",
@@ -595,13 +659,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					}, nil
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: true,
-			expected: &expected{
-				x509Policy:    existingX509PolicyEngine,
-				sshUserPolicy: existingSSHUserPolicyEngine,
-				sshHostPolicy: existingSSHHostPolicyEngine,
-			},
+			ctx:      context.Background(),
+			wantErr:  true,
+			expected: existingPolicyEngine,
 		},
 		{
 			name: "ok/linkedca-unsupported",
@@ -610,14 +670,10 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					EnableAdmin: true,
 				},
 			},
-			adminDB: &linkedCaClient{},
-			ctx:     context.Background(),
-			wantErr: false,
-			expected: &expected{
-				x509Policy:    existingX509PolicyEngine,
-				sshUserPolicy: existingSSHUserPolicyEngine,
-				sshHostPolicy: existingSSHHostPolicyEngine,
-			},
+			adminDB:  &linkedCaClient{},
+			ctx:      context.Background(),
+			wantErr:  false,
+			expected: existingPolicyEngine,
 		},
 		{
 			name: "ok/standalone-no-policy",
@@ -627,13 +683,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					Policy:      nil,
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: false,
-			expected: &expected{
-				x509Policy:    nil,
-				sshUserPolicy: nil,
-				sshHostPolicy: nil,
-			},
+			ctx:      context.Background(),
+			wantErr:  false,
+			expected: mustPolicyEngine(t, nil),
 		},
 		{
 			name: "ok/standalone-x509-policy",
@@ -654,14 +706,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					},
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: false,
-			expected: &expected{
-				// expect only the X.509 policy to exist
-				x509Policy:    newX509PolicyEngine,
-				sshHostPolicy: nil,
-				sshUserPolicy: nil,
-			},
+			ctx:      context.Background(),
+			wantErr:  false,
+			expected: mustPolicyEngine(t, newX509Options),
 		},
 		{
 			name: "ok/standalone-ssh-host-policy",
@@ -682,14 +729,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					},
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: false,
-			expected: &expected{
-				// expect only the SSH host policy to exist
-				x509Policy:    nil,
-				sshHostPolicy: newSSHHostPolicyEngine,
-				sshUserPolicy: nil,
-			},
+			ctx:      context.Background(),
+			wantErr:  false,
+			expected: mustPolicyEngine(t, newSSHHostOptions),
 		},
 		{
 			name: "ok/standalone-ssh-user-policy",
@@ -710,14 +752,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					},
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: false,
-			expected: &expected{
-				// expect only the SSH user policy to exist
-				x509Policy:    nil,
-				sshHostPolicy: nil,
-				sshUserPolicy: newSSHUserPolicyEngine,
-			},
+			ctx:      context.Background(),
+			wantErr:  false,
+			expected: mustPolicyEngine(t, newSSHUserOptions),
 		},
 		{
 			name: "ok/standalone-ssh-policy",
@@ -746,14 +783,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					},
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: false,
-			expected: &expected{
-				// expect only the SSH policy engines to exist
-				x509Policy:    nil,
-				sshHostPolicy: newSSHHostPolicyEngine,
-				sshUserPolicy: newSSHUserPolicyEngine,
-			},
+			ctx:      context.Background(),
+			wantErr:  false,
+			expected: mustPolicyEngine(t, newSSHOptions),
 		},
 		{
 			name: "ok/standalone-full-policy",
@@ -792,14 +824,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					},
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: false,
-			expected: &expected{
-				// expect all three policy engines to exist
-				x509Policy:    newX509PolicyEngine,
-				sshHostPolicy: newSSHHostPolicyEngine,
-				sshUserPolicy: newSSHUserPolicyEngine,
-			},
+			ctx:      context.Background(),
+			wantErr:  false,
+			expected: mustPolicyEngine(t, newOptions),
 		},
 		{
 			name: "ok/admin-x509-policy",
@@ -819,13 +846,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					}, nil
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: false,
-			expected: &expected{
-				x509Policy:    newAdminX509PolicyEngine,
-				sshHostPolicy: nil,
-				sshUserPolicy: nil,
-			},
+			ctx:      context.Background(),
+			wantErr:  false,
+			expected: mustPolicyEngine(t, newAdminX509Options),
 		},
 		{
 			name: "ok/admin-ssh-host-policy",
@@ -847,13 +870,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					}, nil
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: false,
-			expected: &expected{
-				x509Policy:    nil,
-				sshHostPolicy: newAdminSSHHostPolicyEngine,
-				sshUserPolicy: nil,
-			},
+			ctx:      context.Background(),
+			wantErr:  false,
+			expected: mustPolicyEngine(t, newAdminSSHHostOptions),
 		},
 		{
 			name: "ok/admin-ssh-user-policy",
@@ -875,13 +894,9 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					}, nil
 				},
 			},
-			ctx:     context.Background(),
-			wantErr: false,
-			expected: &expected{
-				x509Policy:    nil,
-				sshHostPolicy: nil,
-				sshUserPolicy: newAdminSSHUserPolicyEngine,
-			},
+			ctx:      context.Background(),
+			wantErr:  false,
+			expected: mustPolicyEngine(t, newAdminSSHUserOptions),
 		},
 		{
 			name: "ok/admin-full-policy",
@@ -909,23 +924,24 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 								Allow: &linkedca.SSHHostNames{
 									Dns: []string{"*.local"},
 								},
+								Deny: &linkedca.SSHHostNames{
+									Dns: []string{"badhost.local"},
+								},
 							},
 							User: &linkedca.SSHUserPolicy{
 								Allow: &linkedca.SSHUserNames{
 									Emails: []string{"@example.com"},
 								},
+								Deny: &linkedca.SSHUserNames{
+									Emails: []string{"baduser@example.com"},
+								},
 							},
 						},
 					}, nil
 				},
 			},
-			wantErr: false,
-			expected: &expected{
-				// expect all three policy engines to exist
-				x509Policy:    newX509PolicyEngine,
-				sshHostPolicy: newAdminSSHHostPolicyEngine,
-				sshUserPolicy: newAdminSSHUserPolicyEngine,
-			},
+			wantErr:  false,
+			expected: mustPolicyEngine(t, newAdminOptions),
 		},
 		{
 			// both DB and JSON config; DB config is taken if Admin API is enabled
@@ -972,31 +988,27 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
 					}, nil
 				},
 			},
-			wantErr: false,
-			expected: &expected{
-				// expect all three policy engines to exist
-				x509Policy:    newX509PolicyEngine,
-				sshHostPolicy: nil,
-				sshUserPolicy: nil,
-			},
+			wantErr:  false,
+			expected: mustPolicyEngine(t, newX509Options),
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			a := &Authority{
-				config:        tt.config,
-				adminDB:       tt.adminDB,
-				x509Policy:    existingX509PolicyEngine,
-				sshUserPolicy: existingSSHUserPolicyEngine,
-				sshHostPolicy: existingSSHHostPolicyEngine,
+				config:       tt.config,
+				adminDB:      tt.adminDB,
+				policyEngine: existingPolicyEngine,
 			}
 			if err := a.reloadPolicyEngines(tt.ctx); (err != nil) != tt.wantErr {
 				t.Errorf("Authority.reloadPolicyEngines() error = %v, wantErr %v", err, tt.wantErr)
 			}
 
-			assert.Equal(t, tt.expected.x509Policy, a.x509Policy)
-			assert.Equal(t, tt.expected.sshHostPolicy, a.sshHostPolicy)
-			assert.Equal(t, tt.expected.sshUserPolicy, a.sshUserPolicy)
+			// TODO(hs): fix those
+			// assert.Equal(t, tt.expected.x509Policy, a.x509Policy)
+			// assert.Equal(t, tt.expected.sshHostPolicy, a.sshHostPolicy)
+			// assert.Equal(t, tt.expected.sshUserPolicy, a.sshUserPolicy)
+
+			assert.Equal(t, tt.expected, a.policyEngine)
 		})
 	}
 }

authority/provisioner/acme.go

@@ -118,9 +118,9 @@ func (p *ACME) AuthorizeOrderIdentifier(ctx context.Context, identifier ACMEIden
 	var err error
 	switch identifier.Type {
 	case IP:
-		_, err = x509Policy.IsIPAllowed(net.ParseIP(identifier.Value))
+		err = x509Policy.IsIPAllowed(net.ParseIP(identifier.Value))
 	case DNS:
-		_, err = x509Policy.IsDNSAllowed(identifier.Value)
+		err = x509Policy.IsDNSAllowed(identifier.Value)
 	default:
 		err = fmt.Errorf("invalid ACME identifier type '%s' provided", identifier.Type)
 	}

authority/provisioner/sign_options.go

@@ -422,8 +422,7 @@ func (v *x509NamePolicyValidator) Valid(cert *x509.Certificate, _ SignOptions) e
 	if v.policyEngine == nil {
 		return nil
 	}
-	_, err := v.policyEngine.IsX509CertificateAllowed(cert)
-	return err
+	return v.policyEngine.IsX509CertificateAllowed(cert)
 }
 
 // var (

authority/provisioner/sign_ssh_options.go

@@ -480,16 +480,14 @@ func (v *sshNamePolicyValidator) Valid(cert *ssh.Certificate, _ SignSSHOptions)
 		if v.hostPolicyEngine == nil && v.userPolicyEngine != nil {
 			return errors.New("SSH host certificate not authorized")
 		}
-		_, err := v.hostPolicyEngine.IsSSHCertificateAllowed(cert)
-		return err
+		return v.hostPolicyEngine.IsSSHCertificateAllowed(cert)
 	case ssh.UserCert:
 		// when no user policy engine is configured, but a host policy engine is
 		// configured, the user certificate is denied.
 		if v.userPolicyEngine == nil && v.hostPolicyEngine != nil {
 			return errors.New("SSH user certificate not authorized")
 		}
-		_, err := v.userPolicyEngine.IsSSHCertificateAllowed(cert)
-		return err
+		return v.userPolicyEngine.IsSSHCertificateAllowed(cert)
 	default:
 		return fmt.Errorf("unexpected SSH certificate type %d", cert.CertType) // satisfy return; shouldn't happen
 	}

authority/ssh.go

@@ -246,63 +246,21 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi
 		return nil, errs.InternalServer("authority.SignSSH: unexpected ssh certificate type: %d", certTpl.CertType)
 	}
 
-	switch certTpl.CertType {
-	case ssh.UserCert:
-		// when no user policy engine is configured, but a host policy engine is
-		// configured, the user certificate is denied.
-		if a.sshUserPolicy == nil && a.sshHostPolicy != nil {
-			return nil, errs.ForbiddenErr(errors.New("authority not allowed to sign ssh user certificates"), "authority.SignSSH: error creating ssh user certificate")
-		}
-		if a.sshUserPolicy != nil {
-			allowed, err := a.sshUserPolicy.IsSSHCertificateAllowed(certTpl)
-			if err != nil {
-				var pe *policy.NamePolicyError
-				if errors.As(err, &pe) && pe.Reason == policy.NotAllowed {
-					return nil, &errs.Error{
-						// NOTE: custom forbidden error, so that denied name is sent to client
-						// as well as shown in the logs.
-						Status: http.StatusForbidden,
-						Err:    fmt.Errorf("authority not allowed to sign: %w", err),
-						Msg:    fmt.Sprintf("The request was forbidden by the certificate authority: %s", err.Error()),
-					}
-				}
-				return nil, errs.InternalServerErr(err,
-					errs.WithMessage("authority.SignSSH: error creating ssh user certificate"),
-				)
-			}
-			if !allowed {
-				return nil, errs.ForbiddenErr(errors.New("authority not allowed to sign"), "authority.SignSSH: error creating ssh user certificate")
+	// Check if authority is allowed to sign the certificate
+	if err := a.isAllowedToSignSSHCertificate(certTpl); err != nil {
+		var pe *policy.NamePolicyError
+		if errors.As(err, &pe) && pe.Reason == policy.NotAllowed {
+			return nil, &errs.Error{
+				// NOTE: custom forbidden error, so that denied name is sent to client
+				// as well as shown in the logs.
+				Status: http.StatusForbidden,
+				Err:    fmt.Errorf("authority not allowed to sign: %w", err),
+				Msg:    fmt.Sprintf("The request was forbidden by the certificate authority: %s", err.Error()),
 			}
 		}
-	case ssh.HostCert:
-		// when no host policy engine is configured, but a user policy engine is
-		// configured, the host certificate is denied.
-		if a.sshHostPolicy == nil && a.sshUserPolicy != nil {
-			return nil, errs.ForbiddenErr(errors.New("authority not allowed to sign ssh host certificates"), "authority.SignSSH: error creating ssh user certificate")
-		}
-		if a.sshHostPolicy != nil {
-			allowed, err := a.sshHostPolicy.IsSSHCertificateAllowed(certTpl)
-			if err != nil {
-				var pe *policy.NamePolicyError
-				if errors.As(err, &pe) && pe.Reason == policy.NotAllowed {
-					return nil, &errs.Error{
-						// NOTE: custom forbidden error, so that denied name is sent to client
-						// as well as shown in the logs.
-						Status: http.StatusForbidden,
-						Err:    fmt.Errorf("authority not allowed to sign: %w", err),
-						Msg:    fmt.Sprintf("The request was forbidden by the certificate authority: %s", err.Error()),
-					}
-				}
-				return nil, errs.InternalServerErr(err,
-					errs.WithMessage("authority.SignSSH: error creating ssh host certificate"),
-				)
-			}
-			if !allowed {
-				return nil, errs.ForbiddenErr(errors.New("authority not allowed to sign"), "authority.SignSSH: error creating ssh host certificate")
-			}
-		}
-	default:
-		return nil, errs.InternalServer("authority.SignSSH: unexpected ssh certificate type: %d", certTpl.CertType)
+		return nil, errs.InternalServerErr(err,
+			errs.WithMessage("authority.SignSSH: error creating ssh certificate"),
+		)
 	}
 
 	// Sign certificate.
@@ -325,6 +283,11 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi
 	return cert, nil
 }
 
+// isAllowedToSignSSHCertificate checks if the Authority is allowed to sign the SSH certificate.
+func (a *Authority) isAllowedToSignSSHCertificate(cert *ssh.Certificate) error {
+	return a.policyEngine.IsSSHCertificateAllowed(cert)
+}
+
 // RenewSSH creates a signed SSH certificate using the old SSH certificate as a template.
 func (a *Authority) RenewSSH(ctx context.Context, oldCert *ssh.Certificate) (*ssh.Certificate, error) {
 	if oldCert.ValidAfter == 0 || oldCert.ValidBefore == 0 {

authority/ssh_test.go

@@ -191,21 +191,28 @@ func TestAuthority_SignSSH(t *testing.T) {
 	}, sshutil.CreateTemplateData(sshutil.UserCert, "key-id", []string{"user"}))
 	assert.FatalError(t, err)
 
-	policyOptions := &policy.SSHPolicyOptions{
-		User: &policy.SSHUserCertificateOptions{
-			AllowedNames: &policy.SSHNameOptions{
-				Principals: []string{"user"},
+	userPolicyOptions := &policy.Options{
+		SSH: &policy.SSHPolicyOptions{
+			User: &policy.SSHUserCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					Principals: []string{"user"},
+				},
 			},
 		},
-		Host: &policy.SSHHostCertificateOptions{
-			AllowedNames: &policy.SSHNameOptions{
-				DNSDomains: []string{"*.test.com"},
+	}
+	userPolicy, err := policy.New(userPolicyOptions)
+	assert.FatalError(t, err)
+
+	hostPolicyOptions := &policy.Options{
+		SSH: &policy.SSHPolicyOptions{
+			Host: &policy.SSHHostCertificateOptions{
+				AllowedNames: &policy.SSHNameOptions{
+					DNSDomains: []string{"*.test.com"},
+				},
 			},
 		},
 	}
-	userPolicy, err := policy.NewSSHUserPolicyEngine(policyOptions)
-	assert.FatalError(t, err)
-	hostPolicy, err := policy.NewSSHHostPolicyEngine(policyOptions)
+	hostPolicy, err := policy.New(hostPolicyOptions)
 	assert.FatalError(t, err)
 
 	now := time.Now()
@@ -213,8 +220,7 @@ func TestAuthority_SignSSH(t *testing.T) {
 	type fields struct {
 		sshCAUserCertSignKey ssh.Signer
 		sshCAHostCertSignKey ssh.Signer
-		sshUserPolicy        policy.UserPolicy
-		sshHostPolicy        policy.HostPolicy
+		policyEngine         *policy.Engine
 	}
 	type args struct {
 		key      ssh.PublicKey
@@ -234,49 +240,48 @@ func TestAuthority_SignSSH(t *testing.T) {
 		want    want
 		wantErr bool
 	}{
-		{"ok-user", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions}}, want{CertType: ssh.UserCert}, false},
-		{"ok-host", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{hostTemplate, hostOptions}}, want{CertType: ssh.HostCert}, false},
-		{"ok-user-only", fields{signer, nil, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions}}, want{CertType: ssh.UserCert}, false},
-		{"ok-host-only", fields{nil, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{hostTemplate, hostOptions}}, want{CertType: ssh.HostCert}, false},
-		{"ok-opts-type-user", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user"}, []provisioner.SignOption{userTemplate}}, want{CertType: ssh.UserCert}, false},
-		{"ok-opts-type-host", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host"}, []provisioner.SignOption{hostTemplate}}, want{CertType: ssh.HostCert}, false},
-		{"ok-opts-principals", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{userTemplateWithUser}}, want{CertType: ssh.UserCert, Principals: []string{"user"}}, false},
-		{"ok-opts-principals", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"foo.test.com", "bar.test.com"}}, []provisioner.SignOption{hostTemplateWithHosts}}, want{CertType: ssh.HostCert, Principals: []string{"foo.test.com", "bar.test.com"}}, false},
-		{"ok-opts-valid-after", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user", ValidAfter: provisioner.NewTimeDuration(now)}, []provisioner.SignOption{userTemplate}}, want{CertType: ssh.UserCert, ValidAfter: uint64(now.Unix())}, false},
-		{"ok-opts-valid-before", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host", ValidBefore: provisioner.NewTimeDuration(now)}, []provisioner.SignOption{hostTemplate}}, want{CertType: ssh.HostCert, ValidBefore: uint64(now.Unix())}, false},
-		{"ok-cert-validator", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertValidator("")}}, want{CertType: ssh.UserCert}, false},
-		{"ok-cert-modifier", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertModifier("")}}, want{CertType: ssh.UserCert}, false},
-		{"ok-opts-validator", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsValidator("")}}, want{CertType: ssh.UserCert}, false},
-		{"ok-opts-modifier", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsModifier("")}}, want{CertType: ssh.UserCert}, false},
-		{"ok-custom-template", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userCustomTemplate, userOptions}}, want{CertType: ssh.UserCert, Principals: []string{"user", "admin"}}, false},
-		{"ok-user-policy", fields{signer, signer, userPolicy, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{userTemplateWithUser}}, want{CertType: ssh.UserCert, Principals: []string{"user"}}, false},
-		{"ok-host-policy", fields{signer, signer, nil, hostPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"foo.test.com", "bar.test.com"}}, []provisioner.SignOption{hostTemplateWithHosts}}, want{CertType: ssh.HostCert, Principals: []string{"foo.test.com", "bar.test.com"}}, false},
-		{"fail-opts-type", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "foo"}, []provisioner.SignOption{userTemplate}}, want{}, true},
-		{"fail-cert-validator", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertValidator("an error")}}, want{}, true},
-		{"fail-cert-modifier", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertModifier("an error")}}, want{}, true},
-		{"fail-opts-validator", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsValidator("an error")}}, want{}, true},
-		{"fail-opts-modifier", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsModifier("an error")}}, want{}, true},
-		{"fail-bad-sign-options", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, "wrong type"}}, want{}, true},
-		{"fail-no-user-key", fields{nil, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user"}, []provisioner.SignOption{userTemplate}}, want{}, true},
-		{"fail-no-host-key", fields{signer, nil, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host"}, []provisioner.SignOption{hostTemplate}}, want{}, true},
-		{"fail-bad-type", fields{signer, nil, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, sshTestModifier{CertType: 100}}}, want{}, true},
-		{"fail-custom-template", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userFailTemplate, userOptions}}, want{}, true},
-		{"fail-custom-template-syntax-error-file", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userJSONSyntaxErrorTemplateFile, userOptions}}, want{}, true},
-		{"fail-custom-template-syntax-value-file", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userJSONValueErrorTemplateFile, userOptions}}, want{}, true},
-		{"fail-user-policy", fields{signer, signer, userPolicy, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"root"}}, []provisioner.SignOption{userTemplateWithRoot}}, want{}, true},
-		{"fail-user-policy-with-host-cert", fields{signer, signer, userPolicy, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"foo.test.com"}}, []provisioner.SignOption{hostTemplateWithExampleDotCom}}, want{}, true},
-		{"fail-user-policy-with-bad-user", fields{signer, signer, userPolicy, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{badUserTemplate}}, want{}, true},
-		{"fail-host-policy", fields{signer, signer, nil, hostPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"example.com"}}, []provisioner.SignOption{hostTemplateWithExampleDotCom}}, want{}, true},
-		{"fail-host-policy-with-user-cert", fields{signer, signer, nil, hostPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{userTemplateWithUser}}, want{}, true},
-		{"fail-host-policy-with-bad-host", fields{signer, signer, nil, hostPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"example.com"}}, []provisioner.SignOption{badHostTemplate}}, want{}, true},
+		{"ok-user", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions}}, want{CertType: ssh.UserCert}, false},
+		{"ok-host", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{hostTemplate, hostOptions}}, want{CertType: ssh.HostCert}, false},
+		{"ok-user-only", fields{signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions}}, want{CertType: ssh.UserCert}, false},
+		{"ok-host-only", fields{nil, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{hostTemplate, hostOptions}}, want{CertType: ssh.HostCert}, false},
+		{"ok-opts-type-user", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user"}, []provisioner.SignOption{userTemplate}}, want{CertType: ssh.UserCert}, false},
+		{"ok-opts-type-host", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host"}, []provisioner.SignOption{hostTemplate}}, want{CertType: ssh.HostCert}, false},
+		{"ok-opts-principals", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{userTemplateWithUser}}, want{CertType: ssh.UserCert, Principals: []string{"user"}}, false},
+		{"ok-opts-principals", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"foo.test.com", "bar.test.com"}}, []provisioner.SignOption{hostTemplateWithHosts}}, want{CertType: ssh.HostCert, Principals: []string{"foo.test.com", "bar.test.com"}}, false},
+		{"ok-opts-valid-after", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user", ValidAfter: provisioner.NewTimeDuration(now)}, []provisioner.SignOption{userTemplate}}, want{CertType: ssh.UserCert, ValidAfter: uint64(now.Unix())}, false},
+		{"ok-opts-valid-before", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host", ValidBefore: provisioner.NewTimeDuration(now)}, []provisioner.SignOption{hostTemplate}}, want{CertType: ssh.HostCert, ValidBefore: uint64(now.Unix())}, false},
+		{"ok-cert-validator", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertValidator("")}}, want{CertType: ssh.UserCert}, false},
+		{"ok-cert-modifier", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertModifier("")}}, want{CertType: ssh.UserCert}, false},
+		{"ok-opts-validator", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsValidator("")}}, want{CertType: ssh.UserCert}, false},
+		{"ok-opts-modifier", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsModifier("")}}, want{CertType: ssh.UserCert}, false},
+		{"ok-custom-template", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userCustomTemplate, userOptions}}, want{CertType: ssh.UserCert, Principals: []string{"user", "admin"}}, false},
+		{"ok-user-policy", fields{signer, signer, userPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{userTemplateWithUser}}, want{CertType: ssh.UserCert, Principals: []string{"user"}}, false},
+		{"ok-host-policy", fields{signer, signer, hostPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"foo.test.com", "bar.test.com"}}, []provisioner.SignOption{hostTemplateWithHosts}}, want{CertType: ssh.HostCert, Principals: []string{"foo.test.com", "bar.test.com"}}, false},
+		{"fail-opts-type", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{CertType: "foo"}, []provisioner.SignOption{userTemplate}}, want{}, true},
+		{"fail-cert-validator", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertValidator("an error")}}, want{}, true},
+		{"fail-cert-modifier", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertModifier("an error")}}, want{}, true},
+		{"fail-opts-validator", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsValidator("an error")}}, want{}, true},
+		{"fail-opts-modifier", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsModifier("an error")}}, want{}, true},
+		{"fail-bad-sign-options", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, "wrong type"}}, want{}, true},
+		{"fail-no-user-key", fields{nil, signer, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user"}, []provisioner.SignOption{userTemplate}}, want{}, true},
+		{"fail-no-host-key", fields{signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host"}, []provisioner.SignOption{hostTemplate}}, want{}, true},
+		{"fail-bad-type", fields{signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, sshTestModifier{CertType: 100}}}, want{}, true},
+		{"fail-custom-template", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userFailTemplate, userOptions}}, want{}, true},
+		{"fail-custom-template-syntax-error-file", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userJSONSyntaxErrorTemplateFile, userOptions}}, want{}, true},
+		{"fail-custom-template-syntax-value-file", fields{signer, signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userJSONValueErrorTemplateFile, userOptions}}, want{}, true},
+		{"fail-user-policy", fields{signer, signer, userPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"root"}}, []provisioner.SignOption{userTemplateWithRoot}}, want{}, true},
+		{"fail-user-policy-with-host-cert", fields{signer, signer, userPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"foo.test.com"}}, []provisioner.SignOption{hostTemplateWithExampleDotCom}}, want{}, true},
+		{"fail-user-policy-with-bad-user", fields{signer, signer, userPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{badUserTemplate}}, want{}, true},
+		{"fail-host-policy", fields{signer, signer, hostPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"example.com"}}, []provisioner.SignOption{hostTemplateWithExampleDotCom}}, want{}, true},
+		{"fail-host-policy-with-user-cert", fields{signer, signer, hostPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{userTemplateWithUser}}, want{}, true},
+		{"fail-host-policy-with-bad-host", fields{signer, signer, hostPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"example.com"}}, []provisioner.SignOption{badHostTemplate}}, want{}, true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			a := testAuthority(t)
 			a.sshCAUserCertSignKey = tt.fields.sshCAUserCertSignKey
 			a.sshCAHostCertSignKey = tt.fields.sshCAHostCertSignKey
-			a.sshUserPolicy = tt.fields.sshUserPolicy
-			a.sshHostPolicy = tt.fields.sshHostPolicy
+			a.policyEngine = tt.fields.policyEngine
 
 			got, err := a.SignSSH(context.Background(), tt.args.key, tt.args.opts, tt.args.signOpts...)
 			if (err != nil) != tt.wantErr {

authority/tls.go

@@ -200,8 +200,7 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Sign
 	}
 
 	// Check if authority is allowed to sign the certificate
-	var allowedToSign bool
-	if allowedToSign, err = a.isAllowedToSign(leaf); err != nil {
+	if err := a.isAllowedToSignX509Certificate(leaf); err != nil {
 		var pe *policy.NamePolicyError
 		if errors.As(err, &pe) && pe.Reason == policy.NotAllowed {
 			return nil, errs.ApplyOptions(&errs.Error{
@@ -218,12 +217,6 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Sign
 			errs.WithMessage("error creating certificate"),
 		)
 	}
-	if !allowedToSign {
-		return nil, errs.ApplyOptions(
-			errs.ForbiddenErr(errors.New("authority not allowed to sign"), "error creating certificate"),
-			opts...,
-		)
-	}
 
 	// Sign certificate
 	lifetime := leaf.NotAfter.Sub(leaf.NotBefore.Add(signOpts.Backdate))
@@ -248,31 +241,16 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Sign
 	return fullchain, nil
 }
 
-// isAllowedToSign checks if the Authority is allowed to sign the X.509 certificate.
-func (a *Authority) isAllowedToSign(cert *x509.Certificate) (bool, error) {
-
-	// if no policy is configured, the cert is implicitly allowed
-	if a.x509Policy == nil {
-		return true, nil
-	}
-
-	return a.x509Policy.IsX509CertificateAllowed(cert)
+// isAllowedToSignX509Certificate checks if the Authority is allowed
+// to sign the X.509 certificate.
+func (a *Authority) isAllowedToSignX509Certificate(cert *x509.Certificate) error {
+	return a.policyEngine.IsX509CertificateAllowed(cert)
 }
 
 // AreSANsAllowed evaluates the provided sans against the
 // authority X.509 policy.
 func (a *Authority) AreSANsAllowed(ctx context.Context, sans []string) error {
-
-	// no policy configured; return early
-	if a.x509Policy == nil {
-		return nil
-	}
-
-	// evaluate the X.509 policy for the provided sans
-	var err error
-	_, err = a.x509Policy.AreSANsAllowed(sans)
-
-	return err
+	return a.policyEngine.AreSANsAllowed(sans)
 }
 
 // Renew creates a new Certificate identical to the old certificate, except

authority/tls_test.go

@@ -523,14 +523,16 @@ ZYtQ9Ot36qc=
 					return nil
 				},
 			}
-			policyOptions := &policy.X509PolicyOptions{
-				DeniedNames: &policy.X509NameOptions{
-					DNSDomains: []string{"test.smallstep.com"},
+			options := &policy.Options{
+				X509: &policy.X509PolicyOptions{
+					DeniedNames: &policy.X509NameOptions{
+						DNSDomains: []string{"test.smallstep.com"},
+					},
 				},
 			}
-			engine, err := policy.NewX509PolicyEngine(policyOptions)
+			engine, err := policy.New(options)
 			assert.FatalError(t, err)
-			aa.x509Policy = engine
+			aa.policyEngine = engine
 			return &signTest{
 				auth:            aa,
 				csr:             csr,
@@ -696,15 +698,17 @@ ZYtQ9Ot36qc=
 					return nil
 				},
 			}
-			policyOptions := &policy.X509PolicyOptions{
-				AllowedNames: &policy.X509NameOptions{
-					DNSDomains: []string{"*.smallstep.com"},
+			options := &policy.Options{
+				X509: &policy.X509PolicyOptions{
+					AllowedNames: &policy.X509NameOptions{
+						DNSDomains: []string{"*.smallstep.com"},
+					},
+					DisableCommonNameVerification: true, // TODO(hs): allows "smallstep test"; do we want to keep it like this?
 				},
-				DisableCommonNameVerification: true, // TODO(hs): allows "smallstep test"; do we want to keep it like this?
 			}
-			engine, err := policy.NewX509PolicyEngine(policyOptions)
+			engine, err := policy.New(options)
 			assert.FatalError(t, err)
-			aa.x509Policy = engine
+			aa.policyEngine = engine
 			return &signTest{
 				auth:            aa,
 				csr:             csr,

policy/engine.go

@@ -15,11 +15,10 @@ import (
 type NamePolicyReason int
 
 const (
-	_ NamePolicyReason = iota
 	// NotAllowed results when an instance of NamePolicyEngine
 	// determines that there's a constraint which doesn't permit
 	// a DNS or another type of SAN to be signed (or otherwise used).
-	NotAllowed
+	NotAllowed NamePolicyReason = iota + 1
 	// CannotParseDomain is returned when an error occurs
 	// when parsing the domain part of SAN or subject.
 	CannotParseDomain
@@ -198,7 +197,7 @@ func removeDuplicateIPNets(items []*net.IPNet) (ret []*net.IPNet) {
 }
 
 // IsX509CertificateAllowed verifies that all SANs in a Certificate are allowed.
-func (e *NamePolicyEngine) IsX509CertificateAllowed(cert *x509.Certificate) (bool, error) {
+func (e *NamePolicyEngine) IsX509CertificateAllowed(cert *x509.Certificate) error {
 	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.
@@ -206,13 +205,13 @@ func (e *NamePolicyEngine) IsX509CertificateAllowed(cert *x509.Certificate) (boo
 		appendSubjectCommonName(cert.Subject, &dnsNames, &ips, &emails, &uris)
 	}
 	if err := e.validateNames(dnsNames, ips, emails, uris, []string{}); err != nil {
-		return false, err
+		return err
 	}
-	return true, nil
+	return nil
 }
 
 // IsX509CertificateRequestAllowed verifies that all names in the CSR are allowed.
-func (e *NamePolicyEngine) IsX509CertificateRequestAllowed(csr *x509.CertificateRequest) (bool, error) {
+func (e *NamePolicyEngine) IsX509CertificateRequestAllowed(csr *x509.CertificateRequest) error {
 	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.
@@ -220,47 +219,47 @@ func (e *NamePolicyEngine) IsX509CertificateRequestAllowed(csr *x509.Certificate
 		appendSubjectCommonName(csr.Subject, &dnsNames, &ips, &emails, &uris)
 	}
 	if err := e.validateNames(dnsNames, ips, emails, uris, []string{}); err != nil {
-		return false, err
+		return err
 	}
-	return true, nil
+	return nil
 }
 
 // AreSANSAllowed verifies that all names in the slice of SANs are allowed.
 // The SANs are first split into DNS names, IPs, email addresses and URIs.
-func (e *NamePolicyEngine) AreSANsAllowed(sans []string) (bool, error) {
+func (e *NamePolicyEngine) AreSANsAllowed(sans []string) error {
 	dnsNames, ips, emails, uris := x509util.SplitSANs(sans)
 	if err := e.validateNames(dnsNames, ips, emails, uris, []string{}); err != nil {
-		return false, err
+		return err
 	}
-	return true, nil
+	return nil
 }
 
 // IsDNSAllowed verifies a single DNS domain is allowed.
-func (e *NamePolicyEngine) IsDNSAllowed(dns string) (bool, error) {
+func (e *NamePolicyEngine) IsDNSAllowed(dns string) error {
 	if err := e.validateNames([]string{dns}, []net.IP{}, []string{}, []*url.URL{}, []string{}); err != nil {
-		return false, err
+		return err
 	}
-	return true, nil
+	return nil
 }
 
 // IsIPAllowed verifies a single IP domain is allowed.
-func (e *NamePolicyEngine) IsIPAllowed(ip net.IP) (bool, error) {
+func (e *NamePolicyEngine) IsIPAllowed(ip net.IP) error {
 	if err := e.validateNames([]string{}, []net.IP{ip}, []string{}, []*url.URL{}, []string{}); err != nil {
-		return false, err
+		return err
 	}
-	return true, nil
+	return nil
 }
 
 // IsSSHCertificateAllowed verifies that all principals in an SSH certificate are allowed.
-func (e *NamePolicyEngine) IsSSHCertificateAllowed(cert *ssh.Certificate) (bool, error) {
+func (e *NamePolicyEngine) IsSSHCertificateAllowed(cert *ssh.Certificate) error {
 	dnsNames, ips, emails, principals, err := splitSSHPrincipals(cert)
 	if err != nil {
-		return false, err
+		return err
 	}
 	if err := e.validateNames(dnsNames, ips, emails, []*url.URL{}, principals); err != nil {
-		return false, err
+		return err
 	}
-	return true, nil
+	return nil
 }
 
 // appendSubjectCommonName appends the Subject Common Name to the appropriate slice of names. The logic is

policy/engine_test.go

@@ -2391,16 +2391,16 @@ func TestNamePolicyEngine_X509_AllAllowed(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			engine, err := New(tt.options...)
 			assert.NoError(t, err)
-			got, err := engine.IsX509CertificateAllowed(tt.cert)
+			gotErr := engine.IsX509CertificateAllowed(tt.cert)
 			wantErr := tt.wantErr != nil
 
-			if (err != nil) != wantErr {
-				t.Errorf("NamePolicyEngine.IsX509CertificateAllowed() error = %v, wantErr %v", err, tt.wantErr)
+			if (gotErr != nil) != wantErr {
+				t.Errorf("NamePolicyEngine.IsX509CertificateAllowed() error = %v, wantErr %v", gotErr, tt.wantErr)
 				return
 			}
-			if err != nil {
+			if gotErr != nil {
 				var npe *NamePolicyError
-				assert.True(t, errors.As(err, &npe))
+				assert.True(t, errors.As(gotErr, &npe))
 				assert.NotEqual(t, "", npe.Error())
 				assert.Equal(t, tt.wantErr.Reason, npe.Reason)
 				assert.Equal(t, tt.wantErr.NameType, npe.NameType)
@@ -2408,9 +2408,6 @@ func TestNamePolicyEngine_X509_AllAllowed(t *testing.T) {
 				assert.NotEqual(t, "", npe.Detail())
 				//assert.Equals(t, tt.err.Reason, npe.Reason) // NOTE: reason detail is skipped; it's a detail
 			}
-			if got != tt.want {
-				t.Errorf("NamePolicyEngine.IsX509CertificateAllowed() = %v, want %v", got, tt.want)
-			}
 
 			// Perform the same tests for a CSR, which are similar to Certificates
 			csr := &x509.CertificateRequest{
@@ -2420,15 +2417,15 @@ func TestNamePolicyEngine_X509_AllAllowed(t *testing.T) {
 				IPAddresses:    tt.cert.IPAddresses,
 				URIs:           tt.cert.URIs,
 			}
-			got, err = engine.IsX509CertificateRequestAllowed(csr)
+			gotErr = engine.IsX509CertificateRequestAllowed(csr)
 			wantErr = tt.wantErr != nil
-			if (err != nil) != wantErr {
-				t.Errorf("NamePolicyEngine.AreCSRNamesAllowed() error = %v, wantErr %v", err, tt.wantErr)
+			if (gotErr != nil) != wantErr {
+				t.Errorf("NamePolicyEngine.AreCSRNamesAllowed() error = %v, wantErr %v", gotErr, tt.wantErr)
 				return
 			}
-			if err != nil {
+			if gotErr != nil {
 				var npe *NamePolicyError
-				assert.True(t, errors.As(err, &npe))
+				assert.True(t, errors.As(gotErr, &npe))
 				assert.NotEqual(t, "", npe.Error())
 				assert.Equal(t, tt.wantErr.Reason, npe.Reason)
 				assert.Equal(t, tt.wantErr.NameType, npe.NameType)
@@ -2436,22 +2433,19 @@ func TestNamePolicyEngine_X509_AllAllowed(t *testing.T) {
 				assert.NotEqual(t, "", npe.Detail())
 				//assert.Equals(t, tt.err.Reason, npe.Reason) // NOTE: reason detail is skipped; it's a detail
 			}
-			if got != tt.want {
-				t.Errorf("NamePolicyEngine.AreCSRNamesAllowed() = %v, want %v", got, tt.want)
-			}
 
 			// Perform the same tests for a slice of SANs
 			includeSubject := engine.verifySubjectCommonName // copy behavior of the engine when Subject has to be included as a SAN
 			sans := extractSANs(tt.cert, includeSubject)
-			got, err = engine.AreSANsAllowed(sans)
+			gotErr = engine.AreSANsAllowed(sans)
 			wantErr = tt.wantErr != nil
-			if (err != nil) != wantErr {
-				t.Errorf("NamePolicyEngine.AreSANsAllowed() error = %v, wantErr %v", err, tt.wantErr)
+			if (gotErr != nil) != wantErr {
+				t.Errorf("NamePolicyEngine.AreSANsAllowed() error = %v, wantErr %v", gotErr, tt.wantErr)
 				return
 			}
-			if err != nil {
+			if gotErr != nil {
 				var npe *NamePolicyError
-				assert.True(t, errors.As(err, &npe))
+				assert.True(t, errors.As(gotErr, &npe))
 				assert.NotEqual(t, "", npe.Error())
 				assert.Equal(t, tt.wantErr.Reason, npe.Reason)
 				assert.Equal(t, tt.wantErr.NameType, npe.NameType)
@@ -2459,9 +2453,6 @@ func TestNamePolicyEngine_X509_AllAllowed(t *testing.T) {
 				assert.NotEqual(t, "", npe.Detail())
 				//assert.Equals(t, tt.err.Reason, npe.Reason) // NOTE: reason detail is skipped; it's a detail
 			}
-			if got != tt.want {
-				t.Errorf("NamePolicyEngine.AreSANsAllowed() = %v, want %v", got, tt.want)
-			}
 		})
 	}
 }
@@ -2955,15 +2946,15 @@ func TestNamePolicyEngine_SSH_ArePrincipalsAllowed(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			engine, err := New(tt.options...)
 			assert.NoError(t, err)
-			got, err := engine.IsSSHCertificateAllowed(tt.cert)
+			gotErr := engine.IsSSHCertificateAllowed(tt.cert)
 			wantErr := tt.wantErr != nil
-			if (err != nil) != wantErr {
-				t.Errorf("NamePolicyEngine.IsSSHCertificateAllowed() error = %v, wantErr %v", err, tt.wantErr)
+			if (gotErr != nil) != wantErr {
+				t.Errorf("NamePolicyEngine.IsSSHCertificateAllowed() error = %v, wantErr %v", gotErr, tt.wantErr)
 				return
 			}
-			if err != nil {
+			if gotErr != nil {
 				var npe *NamePolicyError
-				assert.True(t, errors.As(err, &npe))
+				assert.True(t, errors.As(gotErr, &npe))
 				assert.NotEqual(t, "", npe.Error())
 				assert.Equal(t, tt.wantErr.Reason, npe.Reason)
 				assert.Equal(t, tt.wantErr.NameType, npe.NameType)
@@ -2971,9 +2962,6 @@ func TestNamePolicyEngine_SSH_ArePrincipalsAllowed(t *testing.T) {
 				assert.NotEqual(t, "", npe.Detail())
 				//assert.Equals(t, tt.err.Reason, npe.Reason) // NOTE: reason detail is skipped; it's a detail
 			}
-			if got != tt.want {
-				t.Errorf("NamePolicyEngine.IsSSHCertificateAllowed() = %v, want %v", got, tt.want)
-			}
 		})
 	}
 }

policy/ssh.go

@@ -5,5 +5,5 @@ import (
 )
 
 type SSHNamePolicyEngine interface {
-	IsSSHCertificateAllowed(cert *ssh.Certificate) (bool, error)
+	IsSSHCertificateAllowed(cert *ssh.Certificate) error
 }

policy/x509.go

@@ -6,9 +6,9 @@ import (
 )
 
 type X509NamePolicyEngine interface {
-	IsX509CertificateAllowed(cert *x509.Certificate) (bool, error)
-	IsX509CertificateRequestAllowed(csr *x509.CertificateRequest) (bool, error)
-	AreSANsAllowed(sans []string) (bool, error)
-	IsDNSAllowed(dns string) (bool, error)
-	IsIPAllowed(ip net.IP) (bool, error)
+	IsX509CertificateAllowed(cert *x509.Certificate) error
+	IsX509CertificateRequestAllowed(csr *x509.CertificateRequest) error
+	AreSANsAllowed(sans []string) error
+	IsDNSAllowed(dns string) error
+	IsIPAllowed(ip net.IP) error
 }