Check admin subjects before changing policy

acme/api/order.go

@@ -105,6 +105,8 @@ func (h *Handler) NewOrder(w http.ResponseWriter, r *http.Request) {
 	// management of allowed/denied names based on just the name, without having bound to EAB. Still,
 	// EAB is not illogical, because that's the way Accounts are connected to an external system and
 	// thus make sense to also set the allowed/denied names based on that info.
+	// TODO: also perform check on the authority level here already, so that challenges are not performed
+	// and after that the CA fails to sign it. (i.e. h.ca function?)
 
 	for _, identifier := range nor.Identifiers {
 		// TODO: gather all errors, so that we can build subproblems; include the nor.Validate() error here too, like in example?

authority/admin/api/acme.go

@@ -14,7 +14,7 @@ import (
 
 const (
 	// provisionerContextKey provisioner key
-	provisionerContextKey = ContextKey("provisioner")
+	provisionerContextKey = admin.ContextKey("provisioner")
 )
 
 // CreateExternalAccountKeyRequest is the type for POST /admin/acme/eab requests

authority/admin/api/admin.go

@@ -26,8 +26,8 @@ type adminAuthority interface {
 	UpdateProvisioner(ctx context.Context, nu *linkedca.Provisioner) error
 	RemoveProvisioner(ctx context.Context, id string) error
 	GetAuthorityPolicy(ctx context.Context) (*linkedca.Policy, error)
-	StoreAuthorityPolicy(ctx context.Context, policy *linkedca.Policy) error
-	UpdateAuthorityPolicy(ctx context.Context, policy *linkedca.Policy) error
+	StoreAuthorityPolicy(ctx context.Context, admin *linkedca.Admin, policy *linkedca.Policy) error
+	UpdateAuthorityPolicy(ctx context.Context, admin *linkedca.Admin, policy *linkedca.Policy) error
 	RemoveAuthorityPolicy(ctx context.Context) error
 }
 

authority/admin/api/admin_test.go

@@ -139,11 +139,11 @@ func (m *mockAdminAuthority) GetAuthorityPolicy(ctx context.Context) (*linkedca.
 	return nil, errors.New("not implemented yet")
 }
 
-func (m *mockAdminAuthority) StoreAuthorityPolicy(ctx context.Context, policy *linkedca.Policy) error {
+func (m *mockAdminAuthority) StoreAuthorityPolicy(ctx context.Context, admin *linkedca.Admin, policy *linkedca.Policy) error {
 	return errors.New("not implemented yet")
 }
 
-func (m *mockAdminAuthority) UpdateAuthorityPolicy(ctx context.Context, policy *linkedca.Policy) error {
+func (m *mockAdminAuthority) UpdateAuthorityPolicy(ctx context.Context, admin *linkedca.Admin, policy *linkedca.Policy) error {
 	return errors.New("not implemented yet")
 }
 

authority/admin/api/handler.go

@@ -30,8 +30,7 @@ func NewHandler(auth adminAuthority, adminDB admin.DB, acmeDB acme.DB, acmeRespo
 func (h *Handler) Route(r api.Router) {
 
 	authnz := func(next nextHTTP) nextHTTP {
-		//return h.extractAuthorizeTokenAdmin(h.requireAPIEnabled(next))
-		return h.requireAPIEnabled(next) // TODO(hs): remove this; temporarily no auth checks for simple testing...
+		return h.extractAuthorizeTokenAdmin(h.requireAPIEnabled(next))
 	}
 
 	requireEABEnabled := func(next nextHTTP) nextHTTP {

authority/admin/api/middleware.go

@@ -7,6 +7,7 @@ import (
 	"github.com/smallstep/certificates/api"
 	"github.com/smallstep/certificates/authority/admin"
 	"github.com/smallstep/certificates/authority/admin/db/nosql"
+	"go.step.sm/linkedca"
 )
 
 type nextHTTP = func(http.ResponseWriter, *http.Request)
@@ -27,6 +28,7 @@ func (h *Handler) requireAPIEnabled(next nextHTTP) nextHTTP {
 // extractAuthorizeTokenAdmin is a middleware that extracts and caches the bearer token.
 func (h *Handler) extractAuthorizeTokenAdmin(next nextHTTP) nextHTTP {
 	return func(w http.ResponseWriter, r *http.Request) {
+
 		tok := r.Header.Get("Authorization")
 		if tok == "" {
 			api.WriteError(w, admin.NewError(admin.ErrorUnauthorizedType,
@@ -40,7 +42,7 @@ func (h *Handler) extractAuthorizeTokenAdmin(next nextHTTP) nextHTTP {
 			return
 		}
 
-		ctx := context.WithValue(r.Context(), adminContextKey, adm)
+		ctx := context.WithValue(r.Context(), admin.AdminContextKey, adm)
 		next(w, r.WithContext(ctx))
 	}
 }
@@ -49,13 +51,14 @@ func (h *Handler) extractAuthorizeTokenAdmin(next nextHTTP) nextHTTP {
 func (h *Handler) checkAction(next nextHTTP, supportedInStandalone bool) nextHTTP {
 	return func(w http.ResponseWriter, r *http.Request) {
 
-		// actions allowed in standalone mode are always allowed
+		// actions allowed in standalone mode are always supported
 		if supportedInStandalone {
 			next(w, r)
 			return
 		}
 
-		// when in standalone mode, actions are not supported
+		// when not in standalone mode and using a nosql.DB backend,
+		// actions are not supported
 		if _, ok := h.adminDB.(*nosql.DB); ok {
 			api.WriteError(w, admin.NewError(admin.ErrorNotImplementedType,
 				"operation not supported in standalone mode"))
@@ -67,11 +70,12 @@ func (h *Handler) checkAction(next nextHTTP, supportedInStandalone bool) nextHTT
 	}
 }
 
-// ContextKey is the key type for storing and searching for ACME request
-// essentials in the context of a request.
-type ContextKey string
-
-const (
-	// adminContextKey account key
-	adminContextKey = ContextKey("admin")
-)
+// adminFromContext searches the context for a *linkedca.Admin.
+// Returns the admin or an error.
+func adminFromContext(ctx context.Context) (*linkedca.Admin, error) {
+	val, ok := ctx.Value(admin.AdminContextKey).(*linkedca.Admin)
+	if !ok || val == nil {
+		return nil, admin.NewError(admin.ErrorBadRequestType, "admin not in context")
+	}
+	return val, nil
+}

authority/admin/api/middleware_test.go

@@ -152,7 +152,7 @@ func TestHandler_extractAuthorizeTokenAdmin(t *testing.T) {
 			req.Header["Authorization"] = []string{"token"}
 			createdAt := time.Now()
 			var deletedAt time.Time
-			admin := &linkedca.Admin{
+			adm := &linkedca.Admin{
 				Id:            "adminID",
 				AuthorityId:   "authorityID",
 				Subject:       "admin",
@@ -164,20 +164,20 @@ func TestHandler_extractAuthorizeTokenAdmin(t *testing.T) {
 			auth := &mockAdminAuthority{
 				MockAuthorizeAdminToken: func(r *http.Request, token string) (*linkedca.Admin, error) {
 					assert.Equals(t, "token", token)
-					return admin, nil
+					return adm, nil
 				},
 			}
 			next := func(w http.ResponseWriter, r *http.Request) {
 				ctx := r.Context()
-				a := ctx.Value(adminContextKey) // verifying that the context now has a linkedca.Admin
+				a := ctx.Value(admin.AdminContextKey) // verifying that the context now has a linkedca.Admin
 				adm, ok := a.(*linkedca.Admin)
 				if !ok {
 					t.Errorf("expected *linkedca.Admin; got %T", a)
 					return
 				}
 				opts := []cmp.Option{cmpopts.IgnoreUnexported(linkedca.Admin{}, timestamppb.Timestamp{})}
-				if !cmp.Equal(admin, adm, opts...) {
-					t.Errorf("linkedca.Admin diff =\n%s", cmp.Diff(admin, adm, opts...))
+				if !cmp.Equal(adm, adm, opts...) {
+					t.Errorf("linkedca.Admin diff =\n%s", cmp.Diff(adm, adm, opts...))
 				}
 				w.Write(nil) // mock response with status 200
 			}

authority/admin/api/policy.go

@@ -87,8 +87,14 @@ func (par *PolicyAdminResponder) CreateAuthorityPolicy(w http.ResponseWriter, r
 		return
 	}
 
-	if err := par.auth.StoreAuthorityPolicy(ctx, newPolicy); err != nil {
-		api.WriteError(w, admin.WrapErrorISE(err, "error storing authority policy"))
+	adm, err := adminFromContext(ctx)
+	if err != nil {
+		api.WriteError(w, admin.WrapErrorISE(err, "error retrieving admin from context"))
+		return
+	}
+
+	if err := par.auth.StoreAuthorityPolicy(ctx, adm, newPolicy); err != nil {
+		api.WriteError(w, admin.WrapError(admin.ErrorBadRequestType, err, "error storing authority policy"))
 		return
 	}
 
@@ -103,25 +109,49 @@ func (par *PolicyAdminResponder) CreateAuthorityPolicy(w http.ResponseWriter, r
 
 // UpdateAuthorityPolicy handles the PUT /admin/authority/policy request
 func (par *PolicyAdminResponder) UpdateAuthorityPolicy(w http.ResponseWriter, r *http.Request) {
-	var policy = new(linkedca.Policy)
-	if err := api.ReadProtoJSON(r.Body, policy); err != nil {
+
+	ctx := r.Context()
+	policy, err := par.auth.GetAuthorityPolicy(ctx)
+
+	shouldWriteError := false
+	if ae, ok := err.(*admin.Error); ok {
+		shouldWriteError = !ae.IsType(admin.ErrorNotFoundType)
+	}
+
+	if shouldWriteError {
+		api.WriteError(w, admin.WrapErrorISE(err, "error retrieving authority policy"))
+		return
+	}
+
+	if policy == nil {
+		api.JSONNotFound(w)
+		return
+	}
+
+	var newPolicy = new(linkedca.Policy)
+	if err := api.ReadProtoJSON(r.Body, newPolicy); err != nil {
 		api.WriteError(w, err)
 		return
 	}
 
-	ctx := r.Context()
-	if err := par.auth.UpdateAuthorityPolicy(ctx, policy); err != nil {
-		api.WriteError(w, admin.WrapErrorISE(err, "error updating authority policy"))
+	adm, err := adminFromContext(ctx)
+	if err != nil {
+		api.WriteError(w, admin.WrapErrorISE(err, "error retrieving admin from context"))
+		return
+	}
+
+	if err := par.auth.UpdateAuthorityPolicy(ctx, adm, newPolicy); err != nil {
+		api.WriteError(w, admin.WrapError(admin.ErrorBadRequestType, err, "error updating authority policy"))
 		return
 	}
 
-	newPolicy, err := par.auth.GetAuthorityPolicy(ctx)
+	newlyStoredPolicy, err := par.auth.GetAuthorityPolicy(ctx)
 	if err != nil {
 		api.WriteError(w, admin.WrapErrorISE(err, "error retrieving authority policy after updating"))
 		return
 	}
 
-	api.ProtoJSONStatus(w, newPolicy, http.StatusOK)
+	api.ProtoJSONStatus(w, newlyStoredPolicy, http.StatusOK)
 }
 
 // DeleteAuthorityPolicy handles the DELETE /admin/authority/policy request

authority/admin/context.go

@@ -0,0 +1,10 @@
+package admin
+
+// ContextKey is the key type for storing and searching for
+// Admin API objects in request contexts.
+type ContextKey string
+
+const (
+	// AdminContextKey account key
+	AdminContextKey = ContextKey("admin")
+)

authority/administrator/collection.go

@@ -78,7 +78,7 @@ func (c *Collection) LoadByProvisioner(provName string) ([]*linkedca.Admin, bool
 }
 
 // Store adds an admin to the collection and enforces the uniqueness of
-// admin IDs and amdin subject <-> provisioner name combos.
+// admin IDs and admin subject <-> provisioner name combos.
 func (c *Collection) Store(adm *linkedca.Admin, prov provisioner.Interface) error {
 	// Input validation.
 	if adm.ProvisionerId != prov.GetID() {

authority/authority.go

@@ -219,15 +219,19 @@ func (a *Authority) reloadPolicyEngines(ctx context.Context) error {
 	if a.config.AuthorityConfig.EnableAdmin {
 		linkedPolicy, err := a.adminDB.GetAuthorityPolicy(ctx)
 		if err != nil {
-			return admin.WrapErrorISE(err, "error getting policy to initialize authority")
+			return admin.WrapErrorISE(err, "error getting policy to (re)load policy engines")
 		}
 		policyOptions = policyToCertificates(linkedPolicy)
 	} else {
 		policyOptions = a.config.AuthorityConfig.Policy
 	}
 
-	// return early if no policy options set
+	// 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
 	}
 
@@ -574,7 +578,7 @@ func (a *Authority) init() error {
 		return err
 	}
 
-	// Load Policy Engines
+	// Load x509 and SSH Policy Engines
 	if err := a.reloadPolicyEngines(context.Background()); err != nil {
 		return err
 	}

authority/policy.go

@@ -2,10 +2,15 @@ package authority
 
 import (
 	"context"
+	"fmt"
+
+	"github.com/pkg/errors"
 
-	"github.com/smallstep/certificates/authority/admin"
-	"github.com/smallstep/certificates/authority/policy"
 	"go.step.sm/linkedca"
+
+	"github.com/smallstep/certificates/authority/admin"
+	authPolicy "github.com/smallstep/certificates/authority/policy"
+	policy "github.com/smallstep/certificates/policy"
 )
 
 func (a *Authority) GetAuthorityPolicy(ctx context.Context) (*linkedca.Policy, error) {
@@ -20,31 +25,39 @@ func (a *Authority) GetAuthorityPolicy(ctx context.Context) (*linkedca.Policy, e
 	return policy, nil
 }
 
-func (a *Authority) StoreAuthorityPolicy(ctx context.Context, policy *linkedca.Policy) error {
+func (a *Authority) StoreAuthorityPolicy(ctx context.Context, adm *linkedca.Admin, policy *linkedca.Policy) error {
 	a.adminMutex.Lock()
 	defer a.adminMutex.Unlock()
 
+	if err := a.checkPolicy(ctx, adm, policy); err != nil {
+		return err
+	}
+
 	if err := a.adminDB.CreateAuthorityPolicy(ctx, policy); err != nil {
 		return err
 	}
 
 	if err := a.reloadPolicyEngines(ctx); err != nil {
-		return admin.WrapErrorISE(err, "error reloading admin resources when creating authority policy")
+		return admin.WrapErrorISE(err, "error reloading policy engines when creating authority policy")
 	}
 
 	return nil
 }
 
-func (a *Authority) UpdateAuthorityPolicy(ctx context.Context, policy *linkedca.Policy) error {
+func (a *Authority) UpdateAuthorityPolicy(ctx context.Context, adm *linkedca.Admin, policy *linkedca.Policy) error {
 	a.adminMutex.Lock()
 	defer a.adminMutex.Unlock()
 
+	if err := a.checkPolicy(ctx, adm, policy); err != nil {
+		return err
+	}
+
 	if err := a.adminDB.UpdateAuthorityPolicy(ctx, policy); err != nil {
 		return err
 	}
 
 	if err := a.reloadPolicyEngines(ctx); err != nil {
-		return admin.WrapErrorISE(err, "error reloading admin resources when updating authority policy")
+		return admin.WrapErrorISE(err, "error reloading policy engines when updating authority policy")
 	}
 
 	return nil
@@ -59,34 +72,84 @@ func (a *Authority) RemoveAuthorityPolicy(ctx context.Context) error {
 	}
 
 	if err := a.reloadPolicyEngines(ctx); err != nil {
-		return admin.WrapErrorISE(err, "error reloading admin resources when deleting authority policy")
+		return admin.WrapErrorISE(err, "error reloading policy engines when deleting authority policy")
 	}
 
 	return nil
 }
 
-func policyToCertificates(p *linkedca.Policy) *policy.Options {
+func (a *Authority) checkPolicy(ctx context.Context, adm *linkedca.Admin, p *linkedca.Policy) error {
+
+	// convert the policy; return early if nil
+	policyOptions := policyToCertificates(p)
+	if policyOptions == nil {
+		return nil
+	}
+
+	engine, err := authPolicy.NewX509PolicyEngine(policyOptions.GetX509Options())
+	if err != nil {
+		return admin.WrapErrorISE(err, "error creating temporary policy engine")
+	}
+
+	// TODO(hs): Provide option to force the policy, even when the admin subject would be locked out?
+
+	sans := []string{adm.Subject}
+	if err := isAllowed(engine, sans); err != nil {
+		return err
+	}
+
+	// TODO(hs): perform the check for other admin subjects too?
+	// What logic to use for that: do all admins need access? Only super admins? At least one?
+
+	return nil
+}
+
+func isAllowed(engine authPolicy.X509Policy, sans []string) error {
+	var (
+		allowed bool
+		err     error
+	)
+	if allowed, err = engine.AreSANsAllowed(sans); err != nil {
+		var policyErr *policy.NamePolicyError
+		if errors.As(err, &policyErr); policyErr.Reason == policy.NotAuthorizedForThisName {
+			return 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)
+		} else {
+			return err
+		}
+	}
+
+	if !allowed {
+		return 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
+}
+
+func policyToCertificates(p *linkedca.Policy) *authPolicy.Options {
+
 	// return early
 	if p == nil {
 		return nil
 	}
+
 	// prepare full policy struct
-	opts := &policy.Options{
-		X509: &policy.X509PolicyOptions{
-			AllowedNames: &policy.X509NameOptions{},
-			DeniedNames:  &policy.X509NameOptions{},
+	opts := &authPolicy.Options{
+		X509: &authPolicy.X509PolicyOptions{
+			AllowedNames: &authPolicy.X509NameOptions{},
+			DeniedNames:  &authPolicy.X509NameOptions{},
 		},
-		SSH: &policy.SSHPolicyOptions{
-			Host: &policy.SSHHostCertificateOptions{
-				AllowedNames: &policy.SSHNameOptions{},
-				DeniedNames:  &policy.SSHNameOptions{},
+		SSH: &authPolicy.SSHPolicyOptions{
+			Host: &authPolicy.SSHHostCertificateOptions{
+				AllowedNames: &authPolicy.SSHNameOptions{},
+				DeniedNames:  &authPolicy.SSHNameOptions{},
 			},
-			User: &policy.SSHUserCertificateOptions{
-				AllowedNames: &policy.SSHNameOptions{},
-				DeniedNames:  &policy.SSHNameOptions{},
+			User: &authPolicy.SSHUserCertificateOptions{
+				AllowedNames: &authPolicy.SSHNameOptions{},
+				DeniedNames:  &authPolicy.SSHNameOptions{},
 			},
 		},
 	}
+
 	// fill x509 policy configuration
 	if p.X509 != nil {
 		if p.X509.Allow != nil {
@@ -102,6 +165,7 @@ func policyToCertificates(p *linkedca.Policy) *policy.Options {
 			opts.X509.DeniedNames.URIDomains = p.X509.Deny.Uris
 		}
 	}
+
 	// fill ssh policy configuration
 	if p.Ssh != nil {
 		if p.Ssh.Host != nil {

authority/tls.go

@@ -191,26 +191,20 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Sign
 		}
 	}
 
-	// If a policy is configured, perform allow/deny policy check on authority level
-	// TODO: policy currently also applies to admin token certs; how to circumvent?
-	// Allow any name of an admin in the DB? Or in the admin collection?
-	todoRemoveThis := false
-	if todoRemoveThis && a.x509Policy != nil {
-		allowed, err := a.x509Policy.AreCertificateNamesAllowed(leaf)
-		if err != nil {
-			return nil, errs.InternalServerErr(err,
-				errs.WithKeyVal("csr", csr),
-				errs.WithKeyVal("signOptions", signOpts),
-				errs.WithMessage("error creating certificate"),
-			)
-		}
-		if !allowed {
-			// TODO: include SANs in error message?
-			return nil, errs.ApplyOptions(
-				errs.ForbiddenErr(errors.New("authority not allowed to sign"), "error creating certificate"),
-				opts...,
-			)
-		}
+	// Check if authority is allowed to sign the certificate
+	var allowedToSign bool
+	if allowedToSign, err = a.isAllowedToSign(leaf); err != nil {
+		return nil, errs.InternalServerErr(err,
+			errs.WithKeyVal("csr", csr),
+			errs.WithKeyVal("signOptions", signOpts),
+			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
@@ -236,6 +230,61 @@ 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.
+// It first checks if the certificate contains an admin subject that exists in the
+// collection of admins. The CA is always allowed to sign those. If the cert contains
+// different names and a policy is configured, the policy will be executed against
+// the cert to see if the CA is allowed to sign it.
+func (a *Authority) isAllowedToSign(cert *x509.Certificate) (bool, error) {
+
+	// // check if certificate is an admin identity token certificate and the admin subject exists
+	// b := isAdminIdentityTokenCertificate(cert)
+	// _ = b
+
+	// if isAdminIdentityTokenCertificate(cert) && a.admins.HasSubject(cert.Subject.CommonName) {
+	// 	return true, nil
+	// }
+
+	// if no policy is configured, the cert is implicitly allowed
+	if a.x509Policy == nil {
+		return true, nil
+	}
+
+	return a.x509Policy.AreCertificateNamesAllowed(cert)
+}
+
+func isAdminIdentityTokenCertificate(cert *x509.Certificate) bool {
+
+	// TODO: remove this check
+
+	if cert.Issuer.CommonName != "" {
+		return false
+	}
+
+	subject := cert.Subject.CommonName
+	if subject == "" {
+		return false
+	}
+
+	dnsNames := cert.DNSNames
+	if len(dnsNames) != 1 {
+		return false
+	}
+
+	if dnsNames[0] != subject {
+		return false
+	}
+
+	extras := cert.ExtraExtensions
+	if len(extras) != 1 {
+		return false
+	}
+
+	extra := extras[0]
+
+	return extra.Id.Equal(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 37476, 9000, 64, 1})
+}
+
 // Renew creates a new Certificate identical to the old certificate, except
 // with a validity window that begins 'now'.
 func (a *Authority) Renew(oldCert *x509.Certificate) ([]*x509.Certificate, error) {

policy/engine.go

@@ -10,9 +10,10 @@ import (
 	"reflect"
 	"strings"
 
-	"go.step.sm/crypto/x509util"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/net/idna"
+
+	"go.step.sm/crypto/x509util"
 )
 
 type NamePolicyReason int
@@ -39,7 +40,7 @@ type NamePolicyError struct {
 	Detail string
 }
 
-func (e NamePolicyError) Error() string {
+func (e *NamePolicyError) Error() string {
 	switch e.Reason {
 	case NotAuthorizedForThisName:
 		return "not authorized to sign for this name: " + e.Detail
@@ -295,7 +296,7 @@ func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailA
 		// then return error, because DNS should be explicitly configured to be allowed in that case. In case there are
 		// (other) excluded constraints, we'll allow a DNS (implicit allow; currently).
 		if e.numberOfDNSDomainConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
-			return NamePolicyError{
+			return &NamePolicyError{
 				Reason: NotAuthorizedForThisName,
 				Detail: fmt.Sprintf("dns %q is not explicitly permitted by any constraint", dns),
 			}
@@ -307,7 +308,7 @@ func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailA
 		}
 		parsedDNS, err := idna.Lookup.ToASCII(dns)
 		if err != nil {
-			return NamePolicyError{
+			return &NamePolicyError{
 				Reason: CannotParseDomain,
 				Detail: fmt.Sprintf("dns %q cannot be converted to ASCII", dns),
 			}
@@ -316,7 +317,7 @@ func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailA
 			parsedDNS = "*" + parsedDNS
 		}
 		if _, ok := domainToReverseLabels(parsedDNS); !ok {
-			return NamePolicyError{
+			return &NamePolicyError{
 				Reason: CannotParseDomain,
 				Detail: fmt.Sprintf("cannot parse dns %q", dns),
 			}
@@ -331,7 +332,7 @@ func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailA
 
 	for _, ip := range ips {
 		if e.numberOfIPRangeConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
-			return NamePolicyError{
+			return &NamePolicyError{
 				Reason: NotAuthorizedForThisName,
 				Detail: fmt.Sprintf("ip %q is not explicitly permitted by any constraint", ip.String()),
 			}
@@ -346,14 +347,14 @@ func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailA
 
 	for _, email := range emailAddresses {
 		if e.numberOfEmailAddressConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
-			return NamePolicyError{
+			return &NamePolicyError{
 				Reason: NotAuthorizedForThisName,
 				Detail: fmt.Sprintf("email %q is not explicitly permitted by any constraint", email),
 			}
 		}
 		mailbox, ok := parseRFC2821Mailbox(email)
 		if !ok {
-			return NamePolicyError{
+			return &NamePolicyError{
 				Reason: CannotParseRFC822Name,
 				Detail: fmt.Sprintf("invalid rfc822Name %q", mailbox),
 			}
@@ -363,7 +364,7 @@ func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailA
 		// https://datatracker.ietf.org/doc/html/rfc5280#section-7.5
 		domainASCII, err := idna.ToASCII(mailbox.domain)
 		if err != nil {
-			return NamePolicyError{
+			return &NamePolicyError{
 				Reason: CannotParseDomain,
 				Detail: fmt.Sprintf("cannot parse email domain %q", email),
 			}
@@ -381,7 +382,7 @@ func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailA
 
 	for _, uri := range uris {
 		if e.numberOfURIDomainConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
-			return NamePolicyError{
+			return &NamePolicyError{
 				Reason: NotAuthorizedForThisName,
 				Detail: fmt.Sprintf("uri %q is not explicitly permitted by any constraint", uri.String()),
 			}
@@ -396,7 +397,7 @@ func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailA
 
 	for _, principal := range principals {
 		if e.numberOfPrincipalConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
-			return NamePolicyError{
+			return &NamePolicyError{
 				Reason: NotAuthorizedForThisName,
 				Detail: fmt.Sprintf("username principal %q is not explicitly permitted by any constraint", principal),
 			}
@@ -431,14 +432,14 @@ func checkNameConstraints(
 		constraint := excludedValue.Index(i).Interface()
 		match, err := match(parsedName, constraint)
 		if err != nil {
-			return NamePolicyError{
+			return &NamePolicyError{
 				Reason: CannotMatchNameToConstraint,
 				Detail: err.Error(),
 			}
 		}
 
 		if match {
-			return NamePolicyError{
+			return &NamePolicyError{
 				Reason: NotAuthorizedForThisName,
 				Detail: fmt.Sprintf("%s %q is excluded by constraint %q", nameType, name, constraint),
 			}
@@ -452,7 +453,7 @@ func checkNameConstraints(
 		constraint := permittedValue.Index(i).Interface()
 		var err error
 		if ok, err = match(parsedName, constraint); err != nil {
-			return NamePolicyError{
+			return &NamePolicyError{
 				Reason: CannotMatchNameToConstraint,
 				Detail: err.Error(),
 			}
@@ -464,7 +465,7 @@ func checkNameConstraints(
 	}
 
 	if !ok {
-		return NamePolicyError{
+		return &NamePolicyError{
 			Reason: NotAuthorizedForThisName,
 			Detail: fmt.Sprintf("%s %q is not permitted by any constraint", nameType, name),
 		}

policy/engine_test.go

@@ -13,7 +13,7 @@ import (
 )
 
 // TODO(hs): the functionality in the policy engine is a nice candidate for trying fuzzing on
-// TODO(hs): more complex uses cases that combine multiple names and permitted/excluded entries
+// TODO(hs): more complex use cases that combine multiple names and permitted/excluded entries
 
 func TestNamePolicyEngine_matchDomainConstraint(t *testing.T) {
 	tests := []struct {

policy/options_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+
 	"github.com/smallstep/assert"
 )