diff --git a/api/read/read.go b/api/read/read.go index 30f55886..f4067cb8 100644 --- a/api/read/read.go +++ b/api/read/read.go @@ -34,7 +34,7 @@ func ProtoJSON(r io.Reader, m proto.Message) error { } // ProtoJSONWithCheck reads JSON from the request body and stores it in the value -// pointed to by v. Returns false if an error was written; true if not. +// pointed to by m. Returns false if an error was written; true if not. func ProtoJSONWithCheck(w http.ResponseWriter, r io.Reader, m proto.Message) bool { data, err := io.ReadAll(r) if err != nil { @@ -57,6 +57,7 @@ func ProtoJSONWithCheck(w http.ResponseWriter, r io.Reader, m proto.Message) boo if err := protojson.Unmarshal(data, m); err != nil { if errors.Is(err, proto.Error) { var wrapper = struct { + // TODO(hs): more properties in the error response? Message string `json:"message"` }{ Message: err.Error(), diff --git a/authority/admin/api/acme.go b/authority/admin/api/acme.go index f671059e..0f01b009 100644 --- a/authority/admin/api/acme.go +++ b/authority/admin/api/acme.go @@ -53,7 +53,7 @@ func (h *Handler) requireEABEnabled(next http.HandlerFunc) http.HandlerFunc { return } - next(w, r.WithContext(ctx)) + next(w, r) } } diff --git a/authority/admin/api/handler.go b/authority/admin/api/handler.go index c8ad316b..eb0b791a 100644 --- a/authority/admin/api/handler.go +++ b/authority/admin/api/handler.go @@ -35,10 +35,6 @@ func (h *Handler) Route(r api.Router) { return h.extractAuthorizeTokenAdmin(h.requireAPIEnabled(next)) } - requireEABEnabled := func(next http.HandlerFunc) http.HandlerFunc { - return h.requireEABEnabled(next) - } - enabledInStandalone := func(next http.HandlerFunc) http.HandlerFunc { return h.checkAction(next, true) } @@ -47,6 +43,22 @@ func (h *Handler) Route(r api.Router) { return h.checkAction(next, false) } + acmeEABMiddleware := func(next http.HandlerFunc) http.HandlerFunc { + return authnz(h.loadProvisionerByName(h.requireEABEnabled(next))) + } + + authorityPolicyMiddleware := func(next http.HandlerFunc) http.HandlerFunc { + return authnz(enabledInStandalone(next)) + } + + provisionerPolicyMiddleware := func(next http.HandlerFunc) http.HandlerFunc { + return authnz(disabledInStandalone(h.loadProvisionerByName(next))) + } + + acmePolicyMiddleware := func(next http.HandlerFunc) http.HandlerFunc { + return authnz(disabledInStandalone(h.loadProvisionerByName(h.requireEABEnabled(next)))) + } + // Provisioners r.MethodFunc("GET", "/provisioners/{name}", authnz(h.GetProvisioner)) r.MethodFunc("GET", "/provisioners", authnz(h.GetProvisioners)) @@ -62,26 +74,26 @@ func (h *Handler) Route(r api.Router) { r.MethodFunc("DELETE", "/admins/{id}", authnz(h.DeleteAdmin)) // ACME External Account Binding Keys - r.MethodFunc("GET", "/acme/eab/{provisionerName}/{reference}", authnz(h.loadProvisionerByName(requireEABEnabled(h.acmeResponder.GetExternalAccountKeys)))) - r.MethodFunc("GET", "/acme/eab/{provisionerName}", authnz(h.loadProvisionerByName(requireEABEnabled(h.acmeResponder.GetExternalAccountKeys)))) - r.MethodFunc("POST", "/acme/eab/{provisionerName}", authnz(h.loadProvisionerByName(requireEABEnabled(h.acmeResponder.CreateExternalAccountKey)))) - r.MethodFunc("DELETE", "/acme/eab/{provisionerName}/{id}", authnz(h.loadProvisionerByName(requireEABEnabled(h.acmeResponder.DeleteExternalAccountKey)))) + r.MethodFunc("GET", "/acme/eab/{provisionerName}/{reference}", acmeEABMiddleware(h.acmeResponder.GetExternalAccountKeys)) + r.MethodFunc("GET", "/acme/eab/{provisionerName}", acmeEABMiddleware(h.acmeResponder.GetExternalAccountKeys)) + r.MethodFunc("POST", "/acme/eab/{provisionerName}", acmeEABMiddleware(h.acmeResponder.CreateExternalAccountKey)) + r.MethodFunc("DELETE", "/acme/eab/{provisionerName}/{id}", acmeEABMiddleware(h.acmeResponder.DeleteExternalAccountKey)) // Policy - Authority - r.MethodFunc("GET", "/policy", authnz(enabledInStandalone(h.policyResponder.GetAuthorityPolicy))) - r.MethodFunc("POST", "/policy", authnz(enabledInStandalone(h.policyResponder.CreateAuthorityPolicy))) - r.MethodFunc("PUT", "/policy", authnz(enabledInStandalone(h.policyResponder.UpdateAuthorityPolicy))) - r.MethodFunc("DELETE", "/policy", authnz(enabledInStandalone(h.policyResponder.DeleteAuthorityPolicy))) + r.MethodFunc("GET", "/policy", authorityPolicyMiddleware(h.policyResponder.GetAuthorityPolicy)) + r.MethodFunc("POST", "/policy", authorityPolicyMiddleware(h.policyResponder.CreateAuthorityPolicy)) + r.MethodFunc("PUT", "/policy", authorityPolicyMiddleware(h.policyResponder.UpdateAuthorityPolicy)) + r.MethodFunc("DELETE", "/policy", authorityPolicyMiddleware(h.policyResponder.DeleteAuthorityPolicy)) // Policy - Provisioner - r.MethodFunc("GET", "/provisioners/{provisionerName}/policy", authnz(disabledInStandalone(h.loadProvisionerByName(h.policyResponder.GetProvisionerPolicy)))) - r.MethodFunc("POST", "/provisioners/{provisionerName}/policy", authnz(disabledInStandalone(h.loadProvisionerByName(h.policyResponder.CreateProvisionerPolicy)))) - r.MethodFunc("PUT", "/provisioners/{provisionerName}/policy", authnz(disabledInStandalone(h.loadProvisionerByName(h.policyResponder.UpdateProvisionerPolicy)))) - r.MethodFunc("DELETE", "/provisioners/{provisionerName}/policy", authnz(disabledInStandalone(h.loadProvisionerByName(h.policyResponder.DeleteProvisionerPolicy)))) + r.MethodFunc("GET", "/provisioners/{provisionerName}/policy", provisionerPolicyMiddleware(h.policyResponder.GetProvisionerPolicy)) + r.MethodFunc("POST", "/provisioners/{provisionerName}/policy", provisionerPolicyMiddleware(h.policyResponder.CreateProvisionerPolicy)) + r.MethodFunc("PUT", "/provisioners/{provisionerName}/policy", provisionerPolicyMiddleware(h.policyResponder.UpdateProvisionerPolicy)) + r.MethodFunc("DELETE", "/provisioners/{provisionerName}/policy", provisionerPolicyMiddleware(h.policyResponder.DeleteProvisionerPolicy)) // Policy - ACME Account - r.MethodFunc("GET", "/acme/policy/{provisionerName}/{accountID}", authnz(disabledInStandalone(h.loadProvisionerByName(h.requireEABEnabled(h.policyResponder.GetACMEAccountPolicy))))) - r.MethodFunc("POST", "/acme/policy/{provisionerName}/{accountID}", authnz(disabledInStandalone(h.loadProvisionerByName(h.requireEABEnabled(h.policyResponder.CreateACMEAccountPolicy))))) - r.MethodFunc("PUT", "/acme/policy/{provisionerName}/{accountID}", authnz(disabledInStandalone(h.loadProvisionerByName(h.requireEABEnabled(h.policyResponder.UpdateACMEAccountPolicy))))) - r.MethodFunc("DELETE", "/acme/policy/{provisionerName}/{accountID}", authnz(disabledInStandalone(h.loadProvisionerByName(h.requireEABEnabled(h.policyResponder.DeleteACMEAccountPolicy))))) + r.MethodFunc("GET", "/acme/policy/{provisionerName}/{accountID}", acmePolicyMiddleware(h.policyResponder.GetACMEAccountPolicy)) + r.MethodFunc("POST", "/acme/policy/{provisionerName}/{accountID}", acmePolicyMiddleware(h.policyResponder.CreateACMEAccountPolicy)) + r.MethodFunc("PUT", "/acme/policy/{provisionerName}/{accountID}", acmePolicyMiddleware(h.policyResponder.UpdateACMEAccountPolicy)) + r.MethodFunc("DELETE", "/acme/policy/{provisionerName}/{accountID}", acmePolicyMiddleware(h.policyResponder.DeleteACMEAccountPolicy)) } diff --git a/authority/admin/api/middleware.go b/authority/admin/api/middleware.go index 98477a5e..c30eee10 100644 --- a/authority/admin/api/middleware.go +++ b/authority/admin/api/middleware.go @@ -48,7 +48,7 @@ func (h *Handler) extractAuthorizeTokenAdmin(next http.HandlerFunc) http.Handler } } -// loadProvisioner is a middleware that searches for a provisioner +// loadProvisionerByName is a middleware that searches for a provisioner // by name and stores it in the context. func (h *Handler) loadProvisionerByName(next http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { @@ -81,6 +81,13 @@ func (h *Handler) loadProvisionerByName(next http.HandlerFunc) http.HandlerFunc func (h *Handler) checkAction(next http.HandlerFunc, supportedInStandalone bool) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + // temporarily only support the admin nosql DB + if _, ok := h.adminDB.(*nosql.DB); !ok { + render.Error(w, admin.NewError(admin.ErrorNotImplementedType, + "operation not supported")) + return + } + // actions allowed in standalone mode are always supported if supportedInStandalone { next(w, r) diff --git a/authority/admin/api/policy.go b/authority/admin/api/policy.go index 012f497f..da0e1d9c 100644 --- a/authority/admin/api/policy.go +++ b/authority/admin/api/policy.go @@ -64,12 +64,7 @@ func (par *PolicyAdminResponder) CreateAuthorityPolicy(w http.ResponseWriter, r 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 { + if ae, ok := err.(*admin.Error); ok && !ae.IsType(admin.ErrorNotFoundType) { render.Error(w, admin.WrapErrorISE(err, "error retrieving authority policy")) return } @@ -103,12 +98,7 @@ func (par *PolicyAdminResponder) UpdateAuthorityPolicy(w http.ResponseWriter, r 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 { + if ae, ok := err.(*admin.Error); ok && !ae.IsType(admin.ErrorNotFoundType) { render.Error(w, admin.WrapErrorISE(err, "error retrieving authority policy")) return } @@ -256,17 +246,17 @@ func (par *PolicyAdminResponder) DeleteProvisionerPolicy(w http.ResponseWriter, } func (par *PolicyAdminResponder) GetACMEAccountPolicy(w http.ResponseWriter, r *http.Request) { - render.JSON(w, "not implemented yet") + render.JSONStatus(w, "not implemented yet", http.StatusNotImplemented) } func (par *PolicyAdminResponder) CreateACMEAccountPolicy(w http.ResponseWriter, r *http.Request) { - render.JSON(w, "not implemented yet") + render.JSONStatus(w, "not implemented yet", http.StatusNotImplemented) } func (par *PolicyAdminResponder) UpdateACMEAccountPolicy(w http.ResponseWriter, r *http.Request) { - render.JSON(w, "not implemented yet") + render.JSONStatus(w, "not implemented yet", http.StatusNotImplemented) } func (par *PolicyAdminResponder) DeleteACMEAccountPolicy(w http.ResponseWriter, r *http.Request) { - render.JSON(w, "not implemented yet") + render.JSONStatus(w, "not implemented yet", http.StatusNotImplemented) } diff --git a/authority/admin/db/nosql/policy.go b/authority/admin/db/nosql/policy.go index 8e11ddb0..d26e44a0 100644 --- a/authority/admin/db/nosql/policy.go +++ b/authority/admin/db/nosql/policy.go @@ -41,9 +41,6 @@ func (db *DB) unmarshalDBAuthorityPolicy(data []byte, authorityID string) (*dbAu if err := json.Unmarshal(data, dba); err != nil { return nil, errors.Wrapf(err, "error unmarshaling admin %s into dbAdmin", authorityID) } - // if !dba.DeletedAt.IsZero() { - // return nil, admin.NewError(admin.ErrorDeletedType, "admin %s is deleted", authorityID) - // } if dba.AuthorityID != db.authorityID { return nil, admin.NewError(admin.ErrorAuthorityMismatchType, "admin %s is not owned by authority %s", dba.ID, db.authorityID) @@ -63,14 +60,6 @@ func (db *DB) getDBAuthorityPolicy(ctx context.Context, authorityID string) (*db return dbap, nil } -// func (db *DB) unmarshalAuthorityPolicy(data []byte, authorityID string) (*linkedca.Policy, error) { -// dbap, err := db.unmarshalDBAuthorityPolicy(data, authorityID) -// if err != nil { -// return nil, err -// } -// return dbap.convert(), nil -// } - func (db *DB) CreateAuthorityPolicy(ctx context.Context, policy *linkedca.Policy) error { dbap := &dbAuthorityPolicy{ @@ -88,27 +77,6 @@ func (db *DB) CreateAuthorityPolicy(ctx context.Context, policy *linkedca.Policy } func (db *DB) GetAuthorityPolicy(ctx context.Context) (*linkedca.Policy, error) { - // policy := &linkedca.Policy{ - // X509: &linkedca.X509Policy{ - // Allow: &linkedca.X509Names{ - // Dns: []string{".localhost"}, - // }, - // Deny: &linkedca.X509Names{ - // Dns: []string{"denied.localhost"}, - // }, - // }, - // Ssh: &linkedca.SSHPolicy{ - // User: &linkedca.SSHUserPolicy{ - // Allow: &linkedca.SSHUserNames{}, - // Deny: &linkedca.SSHUserNames{}, - // }, - // Host: &linkedca.SSHHostPolicy{ - // Allow: &linkedca.SSHHostNames{}, - // Deny: &linkedca.SSHHostNames{}, - // }, - // }, - // } - dbap, err := db.getDBAuthorityPolicy(ctx, db.authorityID) if err != nil { return nil, err diff --git a/authority/admin/db/nosql/provisioner.go b/authority/admin/db/nosql/provisioner.go index 540e3ae2..71d9c8d6 100644 --- a/authority/admin/db/nosql/provisioner.go +++ b/authority/admin/db/nosql/provisioner.go @@ -19,7 +19,6 @@ type dbProvisioner struct { Type linkedca.Provisioner_Type `json:"type"` Name string `json:"name"` Claims *linkedca.Claims `json:"claims"` - Policy *linkedca.Policy `json:"policy"` Details []byte `json:"details"` X509Template *linkedca.Template `json:"x509Template"` SSHTemplate *linkedca.Template `json:"sshTemplate"` @@ -44,7 +43,6 @@ func (dbp *dbProvisioner) convert2linkedca() (*linkedca.Provisioner, error) { Type: dbp.Type, Name: dbp.Name, Claims: dbp.Claims, - Policy: dbp.Policy, Details: details, X509Template: dbp.X509Template, SshTemplate: dbp.SSHTemplate, @@ -162,7 +160,6 @@ func (db *DB) CreateProvisioner(ctx context.Context, prov *linkedca.Provisioner) Type: prov.Type, Name: prov.Name, Claims: prov.Claims, - Policy: prov.Policy, Details: details, X509Template: prov.X509Template, SSHTemplate: prov.SshTemplate, @@ -190,7 +187,6 @@ func (db *DB) UpdateProvisioner(ctx context.Context, prov *linkedca.Provisioner) } nu.Name = prov.Name nu.Claims = prov.Claims - nu.Policy = prov.Policy nu.Details, err = json.Marshal(prov.Details.GetData()) if err != nil { return admin.WrapErrorISE(err, "error marshaling details when updating provisioner %s", prov.Name) diff --git a/authority/authority.go b/authority/authority.go index 4352bc23..5caec0fb 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -12,6 +12,11 @@ import ( "time" "github.com/pkg/errors" + "golang.org/x/crypto/ssh" + + "go.step.sm/crypto/pemutil" + "go.step.sm/linkedca" + "github.com/smallstep/certificates/authority/admin" adminDBNosql "github.com/smallstep/certificates/authority/admin/db/nosql" "github.com/smallstep/certificates/authority/administrator" @@ -27,9 +32,6 @@ import ( "github.com/smallstep/certificates/scep" "github.com/smallstep/certificates/templates" "github.com/smallstep/nosql" - "go.step.sm/crypto/pemutil" - "go.step.sm/linkedca" - "golang.org/x/crypto/ssh" ) // Authority implements the Certificate Authority internal interface. @@ -220,6 +222,17 @@ func (a *Authority) reloadPolicyEngines(ctx context.Context) error { ) // if admin API is enabled, the CA is running in linked mode if a.config.AuthorityConfig.EnableAdmin { + + // temporarily disable policy loading when LinkedCA is in use + if _, ok := a.adminDB.(*linkedCaClient); ok { + return nil + } + + // temporarily only support the admin nosql DB + if _, ok := a.adminDB.(*adminDBNosql.DB); !ok { + return nil + } + linkedPolicy, err := a.adminDB.GetAuthorityPolicy(ctx) if err != nil { return admin.WrapErrorISE(err, "error getting policy to (re)load policy engines") diff --git a/authority/config/config.go b/authority/config/config.go index 12503965..f23722d9 100644 --- a/authority/config/config.go +++ b/authority/config/config.go @@ -8,13 +8,15 @@ import ( "time" "github.com/pkg/errors" + + "go.step.sm/linkedca" + "github.com/smallstep/certificates/authority/policy" "github.com/smallstep/certificates/authority/provisioner" cas "github.com/smallstep/certificates/cas/apiv1" "github.com/smallstep/certificates/db" kms "github.com/smallstep/certificates/kms/apiv1" "github.com/smallstep/certificates/templates" - "go.step.sm/linkedca" ) const ( diff --git a/authority/linkedca.go b/authority/linkedca.go index 11c8668c..95812895 100644 --- a/authority/linkedca.go +++ b/authority/linkedca.go @@ -15,16 +15,18 @@ import ( "time" "github.com/pkg/errors" - "github.com/smallstep/certificates/authority/admin" - "github.com/smallstep/certificates/db" + "golang.org/x/crypto/ssh" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials" + "go.step.sm/crypto/jose" "go.step.sm/crypto/keyutil" "go.step.sm/crypto/tlsutil" "go.step.sm/crypto/x509util" "go.step.sm/linkedca" - "golang.org/x/crypto/ssh" - "google.golang.org/grpc" - "google.golang.org/grpc/credentials" + + "github.com/smallstep/certificates/authority/admin" + "github.com/smallstep/certificates/db" ) const uuidPattern = "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$" diff --git a/authority/policy/options.go b/authority/policy/options.go index 5c6e6134..e1c33104 100644 --- a/authority/policy/options.go +++ b/authority/policy/options.go @@ -183,6 +183,7 @@ func (o *SSHUserCertificateOptions) GetDeniedNameOptions() *SSHNameOptions { // names configured. func (o *SSHNameOptions) HasNames() bool { return len(o.DNSDomains) > 0 || + len(o.IPRanges) > 0 || len(o.EmailAddresses) > 0 || len(o.Principals) > 0 } diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go index 36713824..99e49c85 100644 --- a/authority/provisioner/jwk.go +++ b/authority/provisioner/jwk.go @@ -210,23 +210,10 @@ func (p *JWK) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er // revocation status. Just confirms that the provisioner that created the // certificate was configured to allow renewals. 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 return p.ctl.AuthorizeRenew(ctx, cert) } -// 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.ctl.Claimer.IsSSHCAEnabled() { diff --git a/authority/provisioner/k8sSA.go b/authority/provisioner/k8sSA.go index b127ed13..ec813b6c 100644 --- a/authority/provisioner/k8sSA.go +++ b/authority/provisioner/k8sSA.go @@ -56,7 +56,6 @@ type K8sSA struct { ctl *Controller x509Policy policy.X509Policy sshHostPolicy policy.HostPolicy - sshUserPolicy policy.UserPolicy } // GetID returns the provisioner unique identifier. The name and credential id @@ -149,11 +148,6 @@ func (p *K8sSA) Init(config Config) (err error) { return err } - // Initialize the SSH allow/deny policy engine for user certificates - if p.sshUserPolicy, err = policy.NewSSHUserPolicyEngine(p.Options.GetSSHOptions()); err != nil { - return err - } - // Initialize the SSH allow/deny policy engine for host certificates if p.sshHostPolicy, err = policy.NewSSHHostPolicyEngine(p.Options.GetSSHOptions()); err != nil { return err @@ -304,7 +298,7 @@ func (p *K8sSA) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOptio // Require and validate all the default fields in the SSH certificate. &sshCertDefaultValidator{}, // Ensure that all principal names are allowed - newSSHNamePolicyValidator(p.sshHostPolicy, p.sshUserPolicy), + newSSHNamePolicyValidator(p.sshHostPolicy, nil), ), nil } diff --git a/authority/provisioner/nebula.go b/authority/provisioner/nebula.go index 44212f35..78da1f4c 100644 --- a/authority/provisioner/nebula.go +++ b/authority/provisioner/nebula.go @@ -45,7 +45,6 @@ type Nebula struct { ctl *Controller x509Policy policy.X509Policy sshHostPolicy policy.HostPolicy - sshUserPolicy policy.UserPolicy } // Init verifies and initializes the Nebula provisioner. @@ -69,11 +68,6 @@ func (p *Nebula) Init(config Config) (err error) { return err } - // Initialize the SSH allow/deny policy engine for user certificates - if p.sshUserPolicy, err = policy.NewSSHUserPolicyEngine(p.Options.GetSSHOptions()); err != nil { - return err - } - // Initialize the SSH allow/deny policy engine for host certificates if p.sshHostPolicy, err = policy.NewSSHHostPolicyEngine(p.Options.GetSSHOptions()); err != nil { return err @@ -276,7 +270,7 @@ func (p *Nebula) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOpti // Require all the fields in the SSH certificate &sshCertDefaultValidator{}, // Ensure that all principal names are allowed - newSSHNamePolicyValidator(p.sshHostPolicy, p.sshUserPolicy), + newSSHNamePolicyValidator(p.sshHostPolicy, nil), ), nil } diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index 369ef056..b56d5fa5 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -95,7 +95,6 @@ type OIDC struct { Options *Options `json:"options,omitempty"` configuration openIDConfiguration keyStore *keyStore - getIdentityFunc GetIdentityFunc ctl *Controller x509Policy policy.X509Policy sshHostPolicy policy.HostPolicy @@ -202,13 +201,6 @@ func (o *OIDC) Init(config Config) (err error) { return err } - // Set the identity getter if it exists, otherwise use the default. - if config.GetIdentityFunc == nil { - o.getIdentityFunc = DefaultIdentityFunc - } else { - o.getIdentityFunc = config.GetIdentityFunc - } - // Initialize the x509 allow/deny policy engine if o.x509Policy, err = policy.NewX509PolicyEngine(o.Options.GetX509Options()); err != nil { return err diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go index df2551a3..bac40e69 100644 --- a/authority/provisioner/sign_options.go +++ b/authority/provisioner/sign_options.go @@ -422,7 +422,7 @@ func (v *x509NamePolicyValidator) Valid(cert *x509.Certificate, _ SignOptions) e if v.policyEngine == nil { return nil } - _, err := v.policyEngine.AreCertificateNamesAllowed(cert) + _, err := v.policyEngine.IsX509CertificateAllowed(cert) return err } diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index a057b2b9..a41b8bc1 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -480,7 +480,7 @@ 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.ArePrincipalsAllowed(cert) + _, err := v.hostPolicyEngine.IsSSHCertificateAllowed(cert) return err case ssh.UserCert: // when no user policy engine is configured, but a host policy engine is @@ -488,7 +488,7 @@ func (v *sshNamePolicyValidator) Valid(cert *ssh.Certificate, _ SignSSHOptions) if v.userPolicyEngine == nil && v.hostPolicyEngine != nil { return errors.New("SSH user certificate not authorized") } - _, err := v.userPolicyEngine.ArePrincipalsAllowed(cert) + _, err := v.userPolicyEngine.IsSSHCertificateAllowed(cert) return err default: return fmt.Errorf("unexpected SSH certificate type %d", cert.CertType) // satisfy return; shouldn't happen diff --git a/authority/provisioner/sshpop.go b/authority/provisioner/sshpop.go index 1e841db6..e8bcce7e 100644 --- a/authority/provisioner/sshpop.go +++ b/authority/provisioner/sshpop.go @@ -97,8 +97,6 @@ func (p *SSHPOP) Init(config Config) (err error) { p.sshPubKeys = config.SSHKeys config.Audiences = config.Audiences.WithFragment(p.GetIDForToken()) - - // Update claims with global ones p.ctl, err = NewController(p, p.Claims, config) return } diff --git a/authority/ssh.go b/authority/ssh.go index 7c3df192..f2913566 100644 --- a/authority/ssh.go +++ b/authority/ssh.go @@ -5,11 +5,11 @@ import ( "crypto/rand" "crypto/x509" "encoding/binary" + "errors" "net/http" "strings" "time" - "github.com/pkg/errors" "github.com/smallstep/certificates/authority/config" "github.com/smallstep/certificates/authority/provisioner" "github.com/smallstep/certificates/db" @@ -250,7 +250,7 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi 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.ArePrincipalsAllowed(certTpl) + allowed, err := a.sshUserPolicy.IsSSHCertificateAllowed(certTpl) if err != nil { return nil, errs.InternalServerErr(err, errs.WithMessage("authority.SignSSH: error creating ssh user certificate"), @@ -267,7 +267,7 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi 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.ArePrincipalsAllowed(certTpl) + allowed, err := a.sshHostPolicy.IsSSHCertificateAllowed(certTpl) if err != nil { return nil, errs.InternalServerErr(err, errs.WithMessage("authority.SignSSH: error creating ssh host certificate"), diff --git a/authority/tls.go b/authority/tls.go index 13babdf1..bae69279 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -238,7 +238,7 @@ func (a *Authority) isAllowedToSign(cert *x509.Certificate) (bool, error) { return true, nil } - return a.x509Policy.AreCertificateNamesAllowed(cert) + return a.x509Policy.IsX509CertificateAllowed(cert) } // AreSANsAllowed evaluates the provided sans against the diff --git a/policy/engine.go b/policy/engine.go index 63d8452a..afaa2416 100755 --- a/policy/engine.go +++ b/policy/engine.go @@ -1,17 +1,13 @@ package policy import ( - "bytes" "crypto/x509" "crypto/x509/pkix" "fmt" "net" "net/url" - "reflect" - "strings" "golang.org/x/crypto/ssh" - "golang.org/x/net/idna" "go.step.sm/crypto/x509util" ) @@ -161,8 +157,8 @@ func removeDuplicateIPRanges(ipRanges []*net.IPNet) []*net.IPNet { return result } -// AreCertificateNamesAllowed verifies that all SANs in a Certificate are allowed. -func (e *NamePolicyEngine) AreCertificateNamesAllowed(cert *x509.Certificate) (bool, error) { +// IsX509CertificateAllowed verifies that all SANs in a Certificate are allowed. +func (e *NamePolicyEngine) IsX509CertificateAllowed(cert *x509.Certificate) (bool, 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. @@ -175,8 +171,8 @@ func (e *NamePolicyEngine) AreCertificateNamesAllowed(cert *x509.Certificate) (b return true, nil } -// AreCSRNamesAllowed verifies that all names in the CSR are allowed. -func (e *NamePolicyEngine) AreCSRNamesAllowed(csr *x509.CertificateRequest) (bool, error) { +// IsX509CertificateRequestAllowed verifies that all names in the CSR are allowed. +func (e *NamePolicyEngine) IsX509CertificateRequestAllowed(csr *x509.CertificateRequest) (bool, 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. @@ -215,8 +211,8 @@ func (e *NamePolicyEngine) IsIPAllowed(ip net.IP) (bool, error) { return true, nil } -// ArePrincipalsAllowed verifies that all principals in an SSH certificate are allowed. -func (e *NamePolicyEngine) ArePrincipalsAllowed(cert *ssh.Certificate) (bool, error) { +// IsSSHCertificateAllowed verifies that all principals in an SSH certificate are allowed. +func (e *NamePolicyEngine) IsSSHCertificateAllowed(cert *ssh.Certificate) (bool, error) { dnsNames, ips, emails, principals, err := splitSSHPrincipals(cert) if err != nil { return false, err @@ -259,7 +255,7 @@ func splitSSHPrincipals(cert *ssh.Certificate) (dnsNames []string, ips []net.IP, case ssh.UserCert: // re-using SplitSANs results in anything that can't be parsed as an IP, URI or email // to be considered a username principal. This allows usernames like h.slatman to be present - // in the SSH certificate. We're exluding IPs and URIs, because they can be confusing + // in the SSH certificate. We're exluding URIs, because they can be confusing // when used in a SSH user certificate. principals, ips, emails, uris = x509util.SplitSANs(cert.ValidPrincipals) if len(uris) > 0 { @@ -271,568 +267,3 @@ func splitSSHPrincipals(cert *ssh.Certificate) (dnsNames []string, ips []net.IP, return } - -// 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 -func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailAddresses []string, uris []*url.URL, principals []string) error { - - // nothing to compare against; return early - if e.totalNumberOfConstraints == 0 { - return nil - } - - // TODO: implement check that requires at least a single name in all of the SANs + subject? - - // TODO: set limit on total of all names validated? In x509 there's a limit on the number of comparisons - // that protects the CA from a DoS (i.e. many heavy comparisons). The x509 implementation takes - // this number as a total of all checks and keeps a (pointer to a) counter of the number of checks - // executed so far. - - // TODO: gather all errors, or return early? Currently we return early on the first wrong name; check might fail for multiple names. - // Perhaps make that an option? - for _, dns := range dnsNames { - // if there are DNS names to check, no DNS constraints set, but there are other permitted constraints, - // 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{ - Reason: NotAuthorizedForThisName, - Detail: fmt.Sprintf("dns %q is not explicitly permitted by any constraint", dns), - } - } - didCutWildcard := false - if strings.HasPrefix(dns, "*.") { - dns = dns[1:] - didCutWildcard = true - } - parsedDNS, err := idna.Lookup.ToASCII(dns) - if err != nil { - return &NamePolicyError{ - Reason: CannotParseDomain, - Detail: fmt.Sprintf("dns %q cannot be converted to ASCII", dns), - } - } - if didCutWildcard { - parsedDNS = "*" + parsedDNS - } - if _, ok := domainToReverseLabels(parsedDNS); !ok { - return &NamePolicyError{ - Reason: CannotParseDomain, - Detail: fmt.Sprintf("cannot parse dns %q", dns), - } - } - if err := checkNameConstraints("dns", dns, parsedDNS, - func(parsedName, constraint interface{}) (bool, error) { - return e.matchDomainConstraint(parsedName.(string), constraint.(string)) - }, e.permittedDNSDomains, e.excludedDNSDomains); err != nil { - return err - } - } - - for _, ip := range ips { - if e.numberOfIPRangeConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 { - return &NamePolicyError{ - Reason: NotAuthorizedForThisName, - Detail: fmt.Sprintf("ip %q is not explicitly permitted by any constraint", ip.String()), - } - } - if err := checkNameConstraints("ip", ip.String(), ip, - func(parsedName, constraint interface{}) (bool, error) { - return matchIPConstraint(parsedName.(net.IP), constraint.(*net.IPNet)) - }, e.permittedIPRanges, e.excludedIPRanges); err != nil { - return err - } - } - - for _, email := range emailAddresses { - if e.numberOfEmailAddressConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 { - 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{ - Reason: CannotParseRFC822Name, - Detail: fmt.Sprintf("invalid rfc822Name %q", mailbox), - } - } - // According to RFC 5280, section 7.5, emails are considered to match if the local part is - // an exact match and the host (domain) part matches the ASCII representation (case-insensitive): - // https://datatracker.ietf.org/doc/html/rfc5280#section-7.5 - domainASCII, err := idna.ToASCII(mailbox.domain) - if err != nil { - return &NamePolicyError{ - Reason: CannotParseDomain, - Detail: fmt.Sprintf("cannot parse email domain %q", email), - } - } - mailbox.domain = domainASCII - if err := checkNameConstraints("email", email, mailbox, - func(parsedName, constraint interface{}) (bool, error) { - return e.matchEmailConstraint(parsedName.(rfc2821Mailbox), constraint.(string)) - }, e.permittedEmailAddresses, e.excludedEmailAddresses); err != nil { - return err - } - } - - // TODO(hs): fix internationalization for URIs (IRIs) - - for _, uri := range uris { - if e.numberOfURIDomainConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 { - return &NamePolicyError{ - Reason: NotAuthorizedForThisName, - Detail: fmt.Sprintf("uri %q is not explicitly permitted by any constraint", uri.String()), - } - } - if err := checkNameConstraints("uri", uri.String(), uri, - func(parsedName, constraint interface{}) (bool, error) { - return e.matchURIConstraint(parsedName.(*url.URL), constraint.(string)) - }, e.permittedURIDomains, e.excludedURIDomains); err != nil { - return err - } - } - - for _, principal := range principals { - if e.numberOfPrincipalConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 { - return &NamePolicyError{ - Reason: NotAuthorizedForThisName, - Detail: fmt.Sprintf("username principal %q is not explicitly permitted by any constraint", principal), - } - } - // TODO: some validation? I.e. allowed characters? - if err := checkNameConstraints("principal", principal, principal, - func(parsedName, constraint interface{}) (bool, error) { - return matchUsernameConstraint(parsedName.(string), constraint.(string)) - }, e.permittedPrincipals, e.excludedPrincipals); err != nil { - return err - } - } - - // if all checks out, all SANs are allowed - return nil -} - -// checkNameConstraints checks that a name, of type nameType is permitted. -// The argument parsedName contains the parsed form of name, suitable for passing -// to the match function. -// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go -func checkNameConstraints( - nameType string, - name string, - parsedName interface{}, - match func(parsedName, constraint interface{}) (match bool, err error), - permitted, excluded interface{}) error { - - excludedValue := reflect.ValueOf(excluded) - - for i := 0; i < excludedValue.Len(); i++ { - constraint := excludedValue.Index(i).Interface() - match, err := match(parsedName, constraint) - if err != nil { - return &NamePolicyError{ - Reason: CannotMatchNameToConstraint, - Detail: err.Error(), - } - } - - if match { - return &NamePolicyError{ - Reason: NotAuthorizedForThisName, - Detail: fmt.Sprintf("%s %q is excluded by constraint %q", nameType, name, constraint), - } - } - } - - permittedValue := reflect.ValueOf(permitted) - - ok := true - for i := 0; i < permittedValue.Len(); i++ { - constraint := permittedValue.Index(i).Interface() - var err error - if ok, err = match(parsedName, constraint); err != nil { - return &NamePolicyError{ - Reason: CannotMatchNameToConstraint, - Detail: err.Error(), - } - } - - if ok { - break - } - } - - if !ok { - return &NamePolicyError{ - Reason: NotAuthorizedForThisName, - Detail: fmt.Sprintf("%s %q is not permitted by any constraint", nameType, name), - } - } - - return nil -} - -// domainToReverseLabels converts a textual domain name like foo.example.com to -// the list of labels in reverse order, e.g. ["com", "example", "foo"]. -// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go -func domainToReverseLabels(domain string) (reverseLabels []string, ok bool) { - for len(domain) > 0 { - if i := strings.LastIndexByte(domain, '.'); i == -1 { - reverseLabels = append(reverseLabels, domain) - domain = "" - } else { - reverseLabels = append(reverseLabels, domain[i+1:]) - domain = domain[:i] - } - } - - if len(reverseLabels) > 0 && reverseLabels[0] == "" { - // An empty label at the end indicates an absolute value. - return nil, false - } - - for _, label := range reverseLabels { - if label == "" { - // Empty labels are otherwise invalid. - return nil, false - } - - for _, c := range label { - if c < 33 || c > 126 { - // Invalid character. - return nil, false - } - } - } - - return reverseLabels, true -} - -// rfc2821Mailbox represents a “mailbox” (which is an email address to most -// people) by breaking it into the “local” (i.e. before the '@') and “domain” -// parts. -// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go -type rfc2821Mailbox struct { - local, domain string -} - -// parseRFC2821Mailbox parses an email address into local and domain parts, -// based on the ABNF for a “Mailbox” from RFC 2821. According to RFC 5280, -// Section 4.2.1.6 that's correct for an rfc822Name from a certificate: “The -// format of an rfc822Name is a "Mailbox" as defined in RFC 2821, Section 4.1.2”. -// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go -func parseRFC2821Mailbox(in string) (mailbox rfc2821Mailbox, ok bool) { - if in == "" { - return mailbox, false - } - - localPartBytes := make([]byte, 0, len(in)/2) - - if in[0] == '"' { - // Quoted-string = DQUOTE *qcontent DQUOTE - // non-whitespace-control = %d1-8 / %d11 / %d12 / %d14-31 / %d127 - // qcontent = qtext / quoted-pair - // qtext = non-whitespace-control / - // %d33 / %d35-91 / %d93-126 - // quoted-pair = ("\" text) / obs-qp - // text = %d1-9 / %d11 / %d12 / %d14-127 / obs-text - // - // (Names beginning with “obs-” are the obsolete syntax from RFC 2822, - // Section 4. Since it has been 16 years, we no longer accept that.) - in = in[1:] - QuotedString: - for { - if in == "" { - return mailbox, false - } - c := in[0] - in = in[1:] - - switch { - case c == '"': - break QuotedString - - case c == '\\': - // quoted-pair - if in == "" { - return mailbox, false - } - if in[0] == 11 || - in[0] == 12 || - (1 <= in[0] && in[0] <= 9) || - (14 <= in[0] && in[0] <= 127) { - localPartBytes = append(localPartBytes, in[0]) - in = in[1:] - } else { - return mailbox, false - } - - case c == 11 || - c == 12 || - // Space (char 32) is not allowed based on the - // BNF, but RFC 3696 gives an example that - // assumes that it is. Several “verified” - // errata continue to argue about this point. - // We choose to accept it. - c == 32 || - c == 33 || - c == 127 || - (1 <= c && c <= 8) || - (14 <= c && c <= 31) || - (35 <= c && c <= 91) || - (93 <= c && c <= 126): - // qtext - localPartBytes = append(localPartBytes, c) - - default: - return mailbox, false - } - } - } else { - // Atom ("." Atom)* - NextChar: - for len(in) > 0 { - // atext from RFC 2822, Section 3.2.4 - c := in[0] - - switch { - case c == '\\': - // Examples given in RFC 3696 suggest that - // escaped characters can appear outside of a - // quoted string. Several “verified” errata - // continue to argue the point. We choose to - // accept it. - in = in[1:] - if in == "" { - return mailbox, false - } - fallthrough - - case ('0' <= c && c <= '9') || - ('a' <= c && c <= 'z') || - ('A' <= c && c <= 'Z') || - c == '!' || c == '#' || c == '$' || c == '%' || - c == '&' || c == '\'' || c == '*' || c == '+' || - c == '-' || c == '/' || c == '=' || c == '?' || - c == '^' || c == '_' || c == '`' || c == '{' || - c == '|' || c == '}' || c == '~' || c == '.': - localPartBytes = append(localPartBytes, in[0]) - in = in[1:] - - default: - break NextChar - } - } - - if len(localPartBytes) == 0 { - return mailbox, false - } - - // From RFC 3696, Section 3: - // “period (".") may also appear, but may not be used to start - // or end the local part, nor may two or more consecutive - // periods appear.” - twoDots := []byte{'.', '.'} - if localPartBytes[0] == '.' || - localPartBytes[len(localPartBytes)-1] == '.' || - bytes.Contains(localPartBytes, twoDots) { - return mailbox, false - } - } - - if in == "" || in[0] != '@' { - return mailbox, false - } - in = in[1:] - - // The RFC species a format for domains, but that's known to be - // violated in practice so we accept that anything after an '@' is the - // domain part. - if _, ok := domainToReverseLabels(in); !ok { - return mailbox, false - } - - mailbox.local = string(localPartBytes) - mailbox.domain = in - return mailbox, true -} - -// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go -func (e *NamePolicyEngine) matchDomainConstraint(domain, constraint string) (bool, error) { - // The meaning of zero length constraints is not specified, but this - // code follows NSS and accepts them as matching everything. - if constraint == "" { - return true, nil - } - - // A single whitespace seems to be considered a valid domain, but we don't allow it. - if domain == " " { - return false, nil - } - - // Block domains that start with just a period - if domain[0] == '.' { - return false, nil - } - - // Block wildcard domains that don't start with exactly "*." (i.e. double wildcards and such) - if domain[0] == '*' && domain[1] != '.' { - return false, nil - } - - // Check if the domain starts with a wildcard and return early if not allowed - if strings.HasPrefix(domain, "*.") && !e.allowLiteralWildcardNames { - return false, nil - } - - // Only allow asterisk at the start of the domain; we don't allow them as part of a domain label or as a (sub)domain label (currently) - if strings.LastIndex(domain, "*") > 0 { - return false, nil - } - - // Don't allow constraints with empty labels in any position - if strings.Contains(constraint, "..") { - return false, nil - } - - domainLabels, ok := domainToReverseLabels(domain) - if !ok { - return false, fmt.Errorf("cannot parse domain %q", domain) - } - - // RFC 5280 says that a leading period in a domain name means that at - // least one label must be prepended, but only for URI and email - // constraints, not DNS constraints. The code also supports that - // behavior for DNS constraints. In our adaptation of the original - // Go stdlib x509 Name Constraint implementation we look for exactly - // one subdomain, currently. - - mustHaveSubdomains := false - if constraint[0] == '.' { - mustHaveSubdomains = true - constraint = constraint[1:] - } - - constraintLabels, ok := domainToReverseLabels(constraint) - if !ok { - return false, fmt.Errorf("cannot parse domain constraint %q", constraint) - } - - // fmt.Println(mustHaveSubdomains) - // fmt.Println(constraintLabels) - // fmt.Println(domainLabels) - - expectedNumberOfLabels := len(constraintLabels) - if mustHaveSubdomains { - // we expect exactly one more label if it starts with the "canonical" x509 "wildcard": "." - // in the future we could extend this to support multiple additional labels and/or more - // complex matching. - expectedNumberOfLabels++ - } - - if len(domainLabels) != expectedNumberOfLabels { - return false, nil - } - - for i, constraintLabel := range constraintLabels { - if !strings.EqualFold(constraintLabel, domainLabels[i]) { - return false, nil - } - } - - return true, nil -} - -// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go -func matchIPConstraint(ip net.IP, constraint *net.IPNet) (bool, error) { - - // TODO(hs): this is code from Go library, but I got some unexpected result: - // with permitted net 127.0.0.0/24, 127.0.0.1 is NOT allowed. When parsing 127.0.0.1 as net.IP - // which is in the IPAddresses slice, the underlying length is 16. The contraint.IP has a length - // of 4 instead. I currently don't believe that this is a bug in Go now, but why is it like that? - // Is there a difference because we're not operating on a sans []string slice? Or is the Go - // implementation stricter regarding IPv4 vs. IPv6? I've been bitten by some unfortunate differences - // between the two before (i.e. IPv4 in IPv6; IP SANS in ACME) - // if len(ip) != len(constraint.IP) { - // return false, nil - // } - - // for i := range ip { - // if mask := constraint.Mask[i]; ip[i]&mask != constraint.IP[i]&mask { - // return false, nil - // } - // } - - contained := constraint.Contains(ip) // TODO(hs): validate that this is the correct behavior; also check IPv4-in-IPv6 (again) - - return contained, nil -} - -// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go -func (e *NamePolicyEngine) matchEmailConstraint(mailbox rfc2821Mailbox, constraint string) (bool, error) { - // TODO(hs): handle literal wildcard case for emails? Does that even make sense? - // If the constraint contains an @, then it specifies an exact mailbox name (currently) - if strings.Contains(constraint, "*") { - return false, fmt.Errorf("email constraint %q cannot contain asterisk", constraint) - } - if strings.Contains(constraint, "@") { - constraintMailbox, ok := parseRFC2821Mailbox(constraint) - if !ok { - return false, fmt.Errorf("cannot parse constraint %q", constraint) - } - return mailbox.local == constraintMailbox.local && strings.EqualFold(mailbox.domain, constraintMailbox.domain), nil - } - - // Otherwise the constraint is like a DNS constraint of the domain part - // of the mailbox. - return e.matchDomainConstraint(mailbox.domain, constraint) -} - -// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go -func (e *NamePolicyEngine) matchURIConstraint(uri *url.URL, constraint string) (bool, error) { - // From RFC 5280, Section 4.2.1.10: - // “a uniformResourceIdentifier that does not include an authority - // component with a host name specified as a fully qualified domain - // name (e.g., if the URI either does not include an authority - // component or includes an authority component in which the host name - // is specified as an IP address), then the application MUST reject the - // certificate.” - - host := uri.Host - if host == "" { - return false, fmt.Errorf("URI with empty host (%q) cannot be matched against constraints", uri.String()) - } - - // Block hosts with the wildcard character; no exceptions, also not when wildcards allowed. - if strings.Contains(host, "*") { - return false, fmt.Errorf("URI host %q cannot contain asterisk", uri.String()) - } - - if strings.Contains(host, ":") && !strings.HasSuffix(host, "]") { - var err error - host, _, err = net.SplitHostPort(uri.Host) - if err != nil { - return false, err - } - } - - if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") || - net.ParseIP(host) != nil { - return false, fmt.Errorf("URI with IP %q cannot be matched against constraints", uri.String()) - } - - // TODO(hs): add checks for scheme, path, etc.; either here, or in a different constraint matcher (to keep this one simple) - - return e.matchDomainConstraint(host, constraint) -} - -// matchUsernameConstraint performs a string literal match against a constraint. -func matchUsernameConstraint(username, constraint string) (bool, error) { - // allow any plain principal username - if constraint == "*" { - return true, nil - } - return strings.EqualFold(username, constraint), nil -} diff --git a/policy/engine_test.go b/policy/engine_test.go index cf406e71..603ef6ce 100755 --- a/policy/engine_test.go +++ b/policy/engine_test.go @@ -2223,16 +2223,16 @@ func TestNamePolicyEngine_X509_AllAllowed(t *testing.T) { t.Run(tt.name, func(t *testing.T) { engine, err := New(tt.options...) assert.FatalError(t, err) - got, err := engine.AreCertificateNamesAllowed(tt.cert) + got, err := engine.IsX509CertificateAllowed(tt.cert) if (err != nil) != tt.wantErr { - t.Errorf("NamePolicyEngine.AreCertificateNamesAllowed() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("NamePolicyEngine.IsX509CertificateAllowed() error = %v, wantErr %v", err, tt.wantErr) return } if err != nil { assert.NotEquals(t, "", err.Error()) // TODO(hs): implement a more specific error comparison? } if got != tt.want { - t.Errorf("NamePolicyEngine.AreCertificateNamesAllowed() = %v, want %v", 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 @@ -2243,7 +2243,7 @@ func TestNamePolicyEngine_X509_AllAllowed(t *testing.T) { IPAddresses: tt.cert.IPAddresses, URIs: tt.cert.URIs, } - got, err = engine.AreCSRNamesAllowed(csr) + got, err = engine.IsX509CertificateRequestAllowed(csr) if (err != nil) != tt.wantErr { t.Errorf("NamePolicyEngine.AreCSRNamesAllowed() error = %v, wantErr %v", err, tt.wantErr) return @@ -2705,13 +2705,13 @@ func TestNamePolicyEngine_SSH_ArePrincipalsAllowed(t *testing.T) { t.Run(tt.name, func(t *testing.T) { engine, err := New(tt.options...) assert.FatalError(t, err) - got, err := engine.ArePrincipalsAllowed(tt.cert) + got, err := engine.IsSSHCertificateAllowed(tt.cert) if (err != nil) != tt.wantErr { - t.Errorf("NamePolicyEngine.ArePrincipalsAllowed() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("NamePolicyEngine.IsSSHCertificateAllowed() error = %v, wantErr %v", err, tt.wantErr) return } if got != tt.want { - t.Errorf("NamePolicyEngine.ArePrincipalsAllowed() = %v, want %v", got, tt.want) + t.Errorf("NamePolicyEngine.IsSSHCertificateAllowed() = %v, want %v", got, tt.want) } }) } diff --git a/policy/ssh.go b/policy/ssh.go index 0b4290d2..1ebecb2e 100644 --- a/policy/ssh.go +++ b/policy/ssh.go @@ -5,5 +5,5 @@ import ( ) type SSHNamePolicyEngine interface { - ArePrincipalsAllowed(cert *ssh.Certificate) (bool, error) + IsSSHCertificateAllowed(cert *ssh.Certificate) (bool, error) } diff --git a/policy/validate.go b/policy/validate.go new file mode 100644 index 00000000..f259515f --- /dev/null +++ b/policy/validate.go @@ -0,0 +1,580 @@ +// Copyright 2011 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. +package policy + +import ( + "bytes" + "fmt" + "net" + "net/url" + "reflect" + "strings" + + "golang.org/x/net/idna" +) + +// 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 +func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailAddresses []string, uris []*url.URL, principals []string) error { + + // nothing to compare against; return early + if e.totalNumberOfConstraints == 0 { + return nil + } + + // TODO: implement check that requires at least a single name in all of the SANs + subject? + + // TODO: set limit on total of all names validated? In x509 there's a limit on the number of comparisons + // that protects the CA from a DoS (i.e. many heavy comparisons). The x509 implementation takes + // this number as a total of all checks and keeps a (pointer to a) counter of the number of checks + // executed so far. + + // TODO: gather all errors, or return early? Currently we return early on the first wrong name; check might fail for multiple names. + // Perhaps make that an option? + for _, dns := range dnsNames { + // if there are DNS names to check, no DNS constraints set, but there are other permitted constraints, + // 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{ + Reason: NotAuthorizedForThisName, + Detail: fmt.Sprintf("dns %q is not explicitly permitted by any constraint", dns), + } + } + didCutWildcard := false + if strings.HasPrefix(dns, "*.") { + dns = dns[1:] + didCutWildcard = true + } + parsedDNS, err := idna.Lookup.ToASCII(dns) + if err != nil { + return &NamePolicyError{ + Reason: CannotParseDomain, + Detail: fmt.Sprintf("dns %q cannot be converted to ASCII", dns), + } + } + if didCutWildcard { + parsedDNS = "*" + parsedDNS + } + if _, ok := domainToReverseLabels(parsedDNS); !ok { + return &NamePolicyError{ + Reason: CannotParseDomain, + Detail: fmt.Sprintf("cannot parse dns %q", dns), + } + } + if err := checkNameConstraints("dns", dns, parsedDNS, + func(parsedName, constraint interface{}) (bool, error) { + return e.matchDomainConstraint(parsedName.(string), constraint.(string)) + }, e.permittedDNSDomains, e.excludedDNSDomains); err != nil { + return err + } + } + + for _, ip := range ips { + if e.numberOfIPRangeConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 { + return &NamePolicyError{ + Reason: NotAuthorizedForThisName, + Detail: fmt.Sprintf("ip %q is not explicitly permitted by any constraint", ip.String()), + } + } + if err := checkNameConstraints("ip", ip.String(), ip, + func(parsedName, constraint interface{}) (bool, error) { + return matchIPConstraint(parsedName.(net.IP), constraint.(*net.IPNet)) + }, e.permittedIPRanges, e.excludedIPRanges); err != nil { + return err + } + } + + for _, email := range emailAddresses { + if e.numberOfEmailAddressConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 { + 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{ + Reason: CannotParseRFC822Name, + Detail: fmt.Sprintf("invalid rfc822Name %q", mailbox), + } + } + // According to RFC 5280, section 7.5, emails are considered to match if the local part is + // an exact match and the host (domain) part matches the ASCII representation (case-insensitive): + // https://datatracker.ietf.org/doc/html/rfc5280#section-7.5 + domainASCII, err := idna.ToASCII(mailbox.domain) + if err != nil { + return &NamePolicyError{ + Reason: CannotParseDomain, + Detail: fmt.Sprintf("cannot parse email domain %q", email), + } + } + mailbox.domain = domainASCII + if err := checkNameConstraints("email", email, mailbox, + func(parsedName, constraint interface{}) (bool, error) { + return e.matchEmailConstraint(parsedName.(rfc2821Mailbox), constraint.(string)) + }, e.permittedEmailAddresses, e.excludedEmailAddresses); err != nil { + return err + } + } + + // TODO(hs): fix internationalization for URIs (IRIs) + + for _, uri := range uris { + if e.numberOfURIDomainConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 { + return &NamePolicyError{ + Reason: NotAuthorizedForThisName, + Detail: fmt.Sprintf("uri %q is not explicitly permitted by any constraint", uri.String()), + } + } + if err := checkNameConstraints("uri", uri.String(), uri, + func(parsedName, constraint interface{}) (bool, error) { + return e.matchURIConstraint(parsedName.(*url.URL), constraint.(string)) + }, e.permittedURIDomains, e.excludedURIDomains); err != nil { + return err + } + } + + for _, principal := range principals { + if e.numberOfPrincipalConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 { + return &NamePolicyError{ + Reason: NotAuthorizedForThisName, + Detail: fmt.Sprintf("username principal %q is not explicitly permitted by any constraint", principal), + } + } + // TODO: some validation? I.e. allowed characters? + if err := checkNameConstraints("principal", principal, principal, + func(parsedName, constraint interface{}) (bool, error) { + return matchUsernameConstraint(parsedName.(string), constraint.(string)) + }, e.permittedPrincipals, e.excludedPrincipals); err != nil { + return err + } + } + + // if all checks out, all SANs are allowed + return nil +} + +// checkNameConstraints checks that a name, of type nameType is permitted. +// The argument parsedName contains the parsed form of name, suitable for passing +// to the match function. +// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go +func checkNameConstraints( + nameType string, + name string, + parsedName interface{}, + match func(parsedName, constraint interface{}) (match bool, err error), + permitted, excluded interface{}) error { + + excludedValue := reflect.ValueOf(excluded) + + for i := 0; i < excludedValue.Len(); i++ { + constraint := excludedValue.Index(i).Interface() + match, err := match(parsedName, constraint) + if err != nil { + return &NamePolicyError{ + Reason: CannotMatchNameToConstraint, + Detail: err.Error(), + } + } + + if match { + return &NamePolicyError{ + Reason: NotAuthorizedForThisName, + Detail: fmt.Sprintf("%s %q is excluded by constraint %q", nameType, name, constraint), + } + } + } + + permittedValue := reflect.ValueOf(permitted) + + ok := true + for i := 0; i < permittedValue.Len(); i++ { + constraint := permittedValue.Index(i).Interface() + var err error + if ok, err = match(parsedName, constraint); err != nil { + return &NamePolicyError{ + Reason: CannotMatchNameToConstraint, + Detail: err.Error(), + } + } + + if ok { + break + } + } + + if !ok { + return &NamePolicyError{ + Reason: NotAuthorizedForThisName, + Detail: fmt.Sprintf("%s %q is not permitted by any constraint", nameType, name), + } + } + + return nil +} + +// domainToReverseLabels converts a textual domain name like foo.example.com to +// the list of labels in reverse order, e.g. ["com", "example", "foo"]. +// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go +func domainToReverseLabels(domain string) (reverseLabels []string, ok bool) { + for len(domain) > 0 { + if i := strings.LastIndexByte(domain, '.'); i == -1 { + reverseLabels = append(reverseLabels, domain) + domain = "" + } else { + reverseLabels = append(reverseLabels, domain[i+1:]) + domain = domain[:i] + } + } + + if len(reverseLabels) > 0 && reverseLabels[0] == "" { + // An empty label at the end indicates an absolute value. + return nil, false + } + + for _, label := range reverseLabels { + if label == "" { + // Empty labels are otherwise invalid. + return nil, false + } + + for _, c := range label { + if c < 33 || c > 126 { + // Invalid character. + return nil, false + } + } + } + + return reverseLabels, true +} + +// rfc2821Mailbox represents a “mailbox” (which is an email address to most +// people) by breaking it into the “local” (i.e. before the '@') and “domain” +// parts. +// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go +type rfc2821Mailbox struct { + local, domain string +} + +// parseRFC2821Mailbox parses an email address into local and domain parts, +// based on the ABNF for a “Mailbox” from RFC 2821. According to RFC 5280, +// Section 4.2.1.6 that's correct for an rfc822Name from a certificate: “The +// format of an rfc822Name is a "Mailbox" as defined in RFC 2821, Section 4.1.2”. +// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go +func parseRFC2821Mailbox(in string) (mailbox rfc2821Mailbox, ok bool) { + if in == "" { + return mailbox, false + } + + localPartBytes := make([]byte, 0, len(in)/2) + + if in[0] == '"' { + // Quoted-string = DQUOTE *qcontent DQUOTE + // non-whitespace-control = %d1-8 / %d11 / %d12 / %d14-31 / %d127 + // qcontent = qtext / quoted-pair + // qtext = non-whitespace-control / + // %d33 / %d35-91 / %d93-126 + // quoted-pair = ("\" text) / obs-qp + // text = %d1-9 / %d11 / %d12 / %d14-127 / obs-text + // + // (Names beginning with “obs-” are the obsolete syntax from RFC 2822, + // Section 4. Since it has been 16 years, we no longer accept that.) + in = in[1:] + QuotedString: + for { + if in == "" { + return mailbox, false + } + c := in[0] + in = in[1:] + + switch { + case c == '"': + break QuotedString + + case c == '\\': + // quoted-pair + if in == "" { + return mailbox, false + } + if in[0] == 11 || + in[0] == 12 || + (1 <= in[0] && in[0] <= 9) || + (14 <= in[0] && in[0] <= 127) { + localPartBytes = append(localPartBytes, in[0]) + in = in[1:] + } else { + return mailbox, false + } + + case c == 11 || + c == 12 || + // Space (char 32) is not allowed based on the + // BNF, but RFC 3696 gives an example that + // assumes that it is. Several “verified” + // errata continue to argue about this point. + // We choose to accept it. + c == 32 || + c == 33 || + c == 127 || + (1 <= c && c <= 8) || + (14 <= c && c <= 31) || + (35 <= c && c <= 91) || + (93 <= c && c <= 126): + // qtext + localPartBytes = append(localPartBytes, c) + + default: + return mailbox, false + } + } + } else { + // Atom ("." Atom)* + NextChar: + for len(in) > 0 { + // atext from RFC 2822, Section 3.2.4 + c := in[0] + + switch { + case c == '\\': + // Examples given in RFC 3696 suggest that + // escaped characters can appear outside of a + // quoted string. Several “verified” errata + // continue to argue the point. We choose to + // accept it. + in = in[1:] + if in == "" { + return mailbox, false + } + fallthrough + + case ('0' <= c && c <= '9') || + ('a' <= c && c <= 'z') || + ('A' <= c && c <= 'Z') || + c == '!' || c == '#' || c == '$' || c == '%' || + c == '&' || c == '\'' || c == '*' || c == '+' || + c == '-' || c == '/' || c == '=' || c == '?' || + c == '^' || c == '_' || c == '`' || c == '{' || + c == '|' || c == '}' || c == '~' || c == '.': + localPartBytes = append(localPartBytes, in[0]) + in = in[1:] + + default: + break NextChar + } + } + + if len(localPartBytes) == 0 { + return mailbox, false + } + + // From RFC 3696, Section 3: + // “period (".") may also appear, but may not be used to start + // or end the local part, nor may two or more consecutive + // periods appear.” + twoDots := []byte{'.', '.'} + if localPartBytes[0] == '.' || + localPartBytes[len(localPartBytes)-1] == '.' || + bytes.Contains(localPartBytes, twoDots) { + return mailbox, false + } + } + + if in == "" || in[0] != '@' { + return mailbox, false + } + in = in[1:] + + // The RFC species a format for domains, but that's known to be + // violated in practice so we accept that anything after an '@' is the + // domain part. + if _, ok := domainToReverseLabels(in); !ok { + return mailbox, false + } + + mailbox.local = string(localPartBytes) + mailbox.domain = in + return mailbox, true +} + +// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go +func (e *NamePolicyEngine) matchDomainConstraint(domain, constraint string) (bool, error) { + // The meaning of zero length constraints is not specified, but this + // code follows NSS and accepts them as matching everything. + if constraint == "" { + return true, nil + } + + // A single whitespace seems to be considered a valid domain, but we don't allow it. + if domain == " " { + return false, nil + } + + // Block domains that start with just a period + if domain[0] == '.' { + return false, nil + } + + // Block wildcard domains that don't start with exactly "*." (i.e. double wildcards and such) + if domain[0] == '*' && domain[1] != '.' { + return false, nil + } + + // Check if the domain starts with a wildcard and return early if not allowed + if strings.HasPrefix(domain, "*.") && !e.allowLiteralWildcardNames { + return false, nil + } + + // Only allow asterisk at the start of the domain; we don't allow them as part of a domain label or as a (sub)domain label (currently) + if strings.LastIndex(domain, "*") > 0 { + return false, nil + } + + // Don't allow constraints with empty labels in any position + if strings.Contains(constraint, "..") { + return false, nil + } + + domainLabels, ok := domainToReverseLabels(domain) + if !ok { + return false, fmt.Errorf("cannot parse domain %q", domain) + } + + // RFC 5280 says that a leading period in a domain name means that at + // least one label must be prepended, but only for URI and email + // constraints, not DNS constraints. The code also supports that + // behavior for DNS constraints. In our adaptation of the original + // Go stdlib x509 Name Constraint implementation we look for exactly + // one subdomain, currently. + + mustHaveSubdomains := false + if constraint[0] == '.' { + mustHaveSubdomains = true + constraint = constraint[1:] + } + + constraintLabels, ok := domainToReverseLabels(constraint) + if !ok { + return false, fmt.Errorf("cannot parse domain constraint %q", constraint) + } + + // fmt.Println(mustHaveSubdomains) + // fmt.Println(constraintLabels) + // fmt.Println(domainLabels) + + expectedNumberOfLabels := len(constraintLabels) + if mustHaveSubdomains { + // we expect exactly one more label if it starts with the "canonical" x509 "wildcard": "." + // in the future we could extend this to support multiple additional labels and/or more + // complex matching. + expectedNumberOfLabels++ + } + + if len(domainLabels) != expectedNumberOfLabels { + return false, nil + } + + for i, constraintLabel := range constraintLabels { + if !strings.EqualFold(constraintLabel, domainLabels[i]) { + return false, nil + } + } + + return true, nil +} + +// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go +func matchIPConstraint(ip net.IP, constraint *net.IPNet) (bool, error) { + + // TODO(hs): this is code from Go library, but I got some unexpected result: + // with permitted net 127.0.0.0/24, 127.0.0.1 is NOT allowed. When parsing 127.0.0.1 as net.IP + // which is in the IPAddresses slice, the underlying length is 16. The contraint.IP has a length + // of 4 instead. I currently don't believe that this is a bug in Go now, but why is it like that? + // Is there a difference because we're not operating on a sans []string slice? Or is the Go + // implementation stricter regarding IPv4 vs. IPv6? I've been bitten by some unfortunate differences + // between the two before (i.e. IPv4 in IPv6; IP SANS in ACME) + // if len(ip) != len(constraint.IP) { + // return false, nil + // } + + // for i := range ip { + // if mask := constraint.Mask[i]; ip[i]&mask != constraint.IP[i]&mask { + // return false, nil + // } + // } + + contained := constraint.Contains(ip) // TODO(hs): validate that this is the correct behavior; also check IPv4-in-IPv6 (again) + + return contained, nil +} + +// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go +func (e *NamePolicyEngine) matchEmailConstraint(mailbox rfc2821Mailbox, constraint string) (bool, error) { + // TODO(hs): handle literal wildcard case for emails? Does that even make sense? + // If the constraint contains an @, then it specifies an exact mailbox name (currently) + if strings.Contains(constraint, "*") { + return false, fmt.Errorf("email constraint %q cannot contain asterisk", constraint) + } + if strings.Contains(constraint, "@") { + constraintMailbox, ok := parseRFC2821Mailbox(constraint) + if !ok { + return false, fmt.Errorf("cannot parse constraint %q", constraint) + } + return mailbox.local == constraintMailbox.local && strings.EqualFold(mailbox.domain, constraintMailbox.domain), nil + } + + // Otherwise the constraint is like a DNS constraint of the domain part + // of the mailbox. + return e.matchDomainConstraint(mailbox.domain, constraint) +} + +// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go +func (e *NamePolicyEngine) matchURIConstraint(uri *url.URL, constraint string) (bool, error) { + // From RFC 5280, Section 4.2.1.10: + // “a uniformResourceIdentifier that does not include an authority + // component with a host name specified as a fully qualified domain + // name (e.g., if the URI either does not include an authority + // component or includes an authority component in which the host name + // is specified as an IP address), then the application MUST reject the + // certificate.” + + host := uri.Host + if host == "" { + return false, fmt.Errorf("URI with empty host (%q) cannot be matched against constraints", uri.String()) + } + + // Block hosts with the wildcard character; no exceptions, also not when wildcards allowed. + if strings.Contains(host, "*") { + return false, fmt.Errorf("URI host %q cannot contain asterisk", uri.String()) + } + + if strings.Contains(host, ":") && !strings.HasSuffix(host, "]") { + var err error + host, _, err = net.SplitHostPort(uri.Host) + if err != nil { + return false, err + } + } + + if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") || + net.ParseIP(host) != nil { + return false, fmt.Errorf("URI with IP %q cannot be matched against constraints", uri.String()) + } + + // TODO(hs): add checks for scheme, path, etc.; either here, or in a different constraint matcher (to keep this one simple) + + return e.matchDomainConstraint(host, constraint) +} + +// matchUsernameConstraint performs a string literal match against a constraint. +func matchUsernameConstraint(username, constraint string) (bool, error) { + // allow any plain principal username + if constraint == "*" { + return true, nil + } + return strings.EqualFold(username, constraint), nil +} diff --git a/policy/x509.go b/policy/x509.go index 0bc35d89..666e1b5c 100644 --- a/policy/x509.go +++ b/policy/x509.go @@ -6,8 +6,8 @@ import ( ) type X509NamePolicyEngine interface { - AreCertificateNamesAllowed(cert *x509.Certificate) (bool, error) - AreCSRNamesAllowed(csr *x509.CertificateRequest) (bool, error) + 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)