From 679e2945f20897504ed65d091a014e16543b8bce Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Mon, 4 Apr 2022 15:31:28 +0200 Subject: [PATCH] Disallow name constraint wildcard notation --- authority/admin/api/policy.go | 12 ++- authority/policy.go | 5 +- authority/policy_test.go | 16 ++-- policy/engine_test.go | 4 +- policy/options.go | 143 ++++++++++++++++++---------------- policy/options_117_test.go | 6 ++ policy/options_118_test.go | 6 ++ policy/options_test.go | 10 ++- policy/validate.go | 2 +- 9 files changed, 116 insertions(+), 88 deletions(-) diff --git a/authority/admin/api/policy.go b/authority/admin/api/policy.go index 44344271..b47c957c 100644 --- a/authority/admin/api/policy.go +++ b/authority/admin/api/policy.go @@ -86,8 +86,9 @@ func (par *PolicyAdminResponder) CreateAuthorityPolicy(w http.ResponseWriter, r var createdPolicy *linkedca.Policy if createdPolicy, err = par.auth.CreateAuthorityPolicy(ctx, adm, newPolicy); err != nil { var pe *authority.PolicyError + isPolicyError := errors.As(err, &pe) - if errors.As(err, &pe); pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure { + if isPolicyError && pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure || pe.Typ == authority.ConfigurationFailure { render.Error(w, admin.WrapError(admin.ErrorBadRequestType, pe, "error storing authority policy")) return } @@ -126,7 +127,8 @@ func (par *PolicyAdminResponder) UpdateAuthorityPolicy(w http.ResponseWriter, r var updatedPolicy *linkedca.Policy if updatedPolicy, err = par.auth.UpdateAuthorityPolicy(ctx, adm, newPolicy); err != nil { var pe *authority.PolicyError - if errors.As(err, &pe); pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure { + isPolicyError := errors.As(err, &pe) + if isPolicyError && pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure || pe.Typ == authority.ConfigurationFailure { render.Error(w, admin.WrapError(admin.ErrorBadRequestType, pe, "error updating authority policy")) return } @@ -201,7 +203,8 @@ func (par *PolicyAdminResponder) CreateProvisionerPolicy(w http.ResponseWriter, err := par.auth.UpdateProvisioner(ctx, prov) if err != nil { var pe *authority.PolicyError - if errors.As(err, &pe); pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure { + isPolicyError := errors.As(err, &pe) + if isPolicyError && pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure || pe.Typ == authority.ConfigurationFailure { render.Error(w, admin.WrapError(admin.ErrorBadRequestType, pe, "error creating provisioner policy")) return } @@ -233,7 +236,8 @@ func (par *PolicyAdminResponder) UpdateProvisionerPolicy(w http.ResponseWriter, err := par.auth.UpdateProvisioner(ctx, prov) if err != nil { var pe *authority.PolicyError - if errors.As(err, &pe); pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure { + isPolicyError := errors.As(err, &pe) + if isPolicyError && pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure || pe.Typ == authority.ConfigurationFailure { render.Error(w, admin.WrapError(admin.ErrorBadRequestType, pe, "error updating provisioner policy")) return } diff --git a/authority/policy.go b/authority/policy.go index a88258fe..cc785173 100644 --- a/authority/policy.go +++ b/authority/policy.go @@ -170,7 +170,7 @@ func (a *Authority) checkPolicy(ctx context.Context, currentAdmin *linkedca.Admi if err != nil { return &PolicyError{ Typ: ConfigurationFailure, - err: fmt.Errorf("error creating temporary policy engine: %w", err), + err: err, } } @@ -213,7 +213,8 @@ func isAllowed(engine authPolicy.X509Policy, sans []string) error { ) if allowed, err = engine.AreSANsAllowed(sans); err != nil { var policyErr *policy.NamePolicyError - if errors.As(err, &policyErr); policyErr.Reason == policy.NotAuthorizedForThisName { + isNamePolicyError := errors.As(err, &policyErr) + if isNamePolicyError && policyErr.Reason == policy.NotAuthorizedForThisName { return &PolicyError{ Typ: AdminLockOut, err: fmt.Errorf("the provided policy would lock out %s from the CA. Please update your policy to include %s as an allowed name", sans, sans), diff --git a/authority/policy_test.go b/authority/policy_test.go index 39edf700..87c96a87 100644 --- a/authority/policy_test.go +++ b/authority/policy_test.go @@ -6,10 +6,10 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" "go.step.sm/linkedca" - "github.com/smallstep/assert" authPolicy "github.com/smallstep/certificates/authority/policy" ) @@ -34,7 +34,7 @@ func TestAuthority_checkPolicy(t *testing.T) { }, err: &PolicyError{ Typ: ConfigurationFailure, - err: errors.New("error creating temporary policy engine: cannot parse permitted domain constraint \"**.local\""), + err: errors.New("cannot parse permitted domain constraint \"**.local\": domain constraint \"**.local\" can only have wildcard as starting character"), }, } }, @@ -46,7 +46,7 @@ func TestAuthority_checkPolicy(t *testing.T) { policy: &linkedca.Policy{ X509: &linkedca.X509Policy{ Allow: &linkedca.X509Names{ - Dns: []string{".local"}, + Dns: []string{"*.local"}, }, }, }, @@ -68,7 +68,7 @@ func TestAuthority_checkPolicy(t *testing.T) { policy: &linkedca.Policy{ X509: &linkedca.X509Policy{ Allow: &linkedca.X509Names{ - Dns: []string{".local"}, + Dns: []string{"*.local"}, }, }, }, @@ -177,13 +177,13 @@ func TestAuthority_checkPolicy(t *testing.T) { if tc.err == nil { assert.Nil(t, err) } else { - assert.Type(t, &PolicyError{}, err) + assert.IsType(t, &PolicyError{}, err) pe, ok := err.(*PolicyError) - assert.Fatal(t, ok) + assert.True(t, ok) - assert.Equals(t, tc.err.Typ, pe.Typ) - assert.Equals(t, tc.err.Error(), pe.Error()) + assert.Equal(t, tc.err.Typ, pe.Typ) + assert.Equal(t, tc.err.Error(), pe.Error()) } }) } diff --git a/policy/engine_test.go b/policy/engine_test.go index 38aa709a..dd0b403f 100755 --- a/policy/engine_test.go +++ b/policy/engine_test.go @@ -1500,7 +1500,7 @@ func TestNamePolicyEngine_X509_AllAllowed(t *testing.T) { name: "ok/dns-permitted-wildcard", options: []NamePolicyOption{ AddPermittedDNSDomain("*.local"), - AddPermittedDNSDomain(".x509local"), + AddPermittedDNSDomain("*.x509local"), WithAllowLiteralWildcardNames(), }, cert: &x509.Certificate{ @@ -1665,7 +1665,7 @@ func TestNamePolicyEngine_X509_AllAllowed(t *testing.T) { { name: "ok/uri-permitted-with-port", options: []NamePolicyOption{ - AddPermittedURIDomain(".example.com"), + AddPermittedURIDomain("*.example.com"), }, cert: &x509.Certificate{ URIs: []*url.URL{ diff --git a/policy/options.go b/policy/options.go index fe8f470e..308d46b5 100755 --- a/policy/options.go +++ b/policy/options.go @@ -5,7 +5,6 @@ import ( "net" "strings" - "github.com/pkg/errors" "golang.org/x/net/idna" ) @@ -33,7 +32,7 @@ func WithPermittedDNSDomains(domains []string) NamePolicyOption { for i, domain := range domains { normalizedDomain, err := normalizeAndValidateDNSDomainConstraint(domain) if err != nil { - return errors.Errorf("cannot parse permitted domain constraint %q", domain) + return fmt.Errorf("cannot parse permitted domain constraint %q: %w", domain, err) } normalizedDomains[i] = normalizedDomain } @@ -48,7 +47,7 @@ func AddPermittedDNSDomains(domains []string) NamePolicyOption { for i, domain := range domains { normalizedDomain, err := normalizeAndValidateDNSDomainConstraint(domain) if err != nil { - return errors.Errorf("cannot parse permitted domain constraint %q", domain) + return fmt.Errorf("cannot parse permitted domain constraint %q: %w", domain, err) } normalizedDomains[i] = normalizedDomain } @@ -63,7 +62,7 @@ func WithExcludedDNSDomains(domains []string) NamePolicyOption { for i, domain := range domains { normalizedDomain, err := normalizeAndValidateDNSDomainConstraint(domain) if err != nil { - return errors.Errorf("cannot parse permitted domain constraint %q", domain) + return fmt.Errorf("cannot parse excluded domain constraint %q: %w", domain, err) } normalizedDomains[i] = normalizedDomain } @@ -78,7 +77,7 @@ func AddExcludedDNSDomains(domains []string) NamePolicyOption { for i, domain := range domains { normalizedDomain, err := normalizeAndValidateDNSDomainConstraint(domain) if err != nil { - return errors.Errorf("cannot parse permitted domain constraint %q", domain) + return fmt.Errorf("cannot parse excluded domain constraint %q: %w", domain, err) } normalizedDomains[i] = normalizedDomain } @@ -91,7 +90,7 @@ func WithPermittedDNSDomain(domain string) NamePolicyOption { return func(e *NamePolicyEngine) error { normalizedDomain, err := normalizeAndValidateDNSDomainConstraint(domain) if err != nil { - return errors.Errorf("cannot parse permitted domain constraint %q", domain) + return fmt.Errorf("cannot parse permitted domain constraint %q: %w", domain, err) } e.permittedDNSDomains = []string{normalizedDomain} return nil @@ -102,7 +101,7 @@ func AddPermittedDNSDomain(domain string) NamePolicyOption { return func(e *NamePolicyEngine) error { normalizedDomain, err := normalizeAndValidateDNSDomainConstraint(domain) if err != nil { - return errors.Errorf("cannot parse permitted domain constraint %q", domain) + return fmt.Errorf("cannot parse permitted domain constraint %q: %w", domain, err) } e.permittedDNSDomains = append(e.permittedDNSDomains, normalizedDomain) return nil @@ -113,7 +112,7 @@ func WithExcludedDNSDomain(domain string) NamePolicyOption { return func(e *NamePolicyEngine) error { normalizedDomain, err := normalizeAndValidateDNSDomainConstraint(domain) if err != nil { - return errors.Errorf("cannot parse permitted domain constraint %q", domain) + return fmt.Errorf("cannot parse excluded domain constraint %q: %w", domain, err) } e.excludedDNSDomains = []string{normalizedDomain} return nil @@ -124,7 +123,7 @@ func AddExcludedDNSDomain(domain string) NamePolicyOption { return func(e *NamePolicyEngine) error { normalizedDomain, err := normalizeAndValidateDNSDomainConstraint(domain) if err != nil { - return errors.Errorf("cannot parse permitted domain constraint %q", domain) + return fmt.Errorf("cannot parse excluded domain constraint %q: %w", domain, err) } e.excludedDNSDomains = append(e.excludedDNSDomains, normalizedDomain) return nil @@ -151,7 +150,7 @@ func WithPermittedCIDRs(cidrs []string) NamePolicyOption { for i, cidr := range cidrs { _, nw, err := net.ParseCIDR(cidr) if err != nil { - return errors.Errorf("cannot parse permitted CIDR constraint %q", cidr) + return fmt.Errorf("cannot parse permitted CIDR constraint %q", cidr) } networks[i] = nw } @@ -166,7 +165,7 @@ func AddPermittedCIDRs(cidrs []string) NamePolicyOption { for i, cidr := range cidrs { _, nw, err := net.ParseCIDR(cidr) if err != nil { - return errors.Errorf("cannot parse permitted CIDR constraint %q", cidr) + return fmt.Errorf("cannot parse permitted CIDR constraint %q", cidr) } networks[i] = nw } @@ -181,7 +180,7 @@ func WithExcludedCIDRs(cidrs []string) NamePolicyOption { for i, cidr := range cidrs { _, nw, err := net.ParseCIDR(cidr) if err != nil { - return errors.Errorf("cannot parse excluded CIDR constraint %q", cidr) + return fmt.Errorf("cannot parse excluded CIDR constraint %q", cidr) } networks[i] = nw } @@ -196,7 +195,7 @@ func AddExcludedCIDRs(cidrs []string) NamePolicyOption { for i, cidr := range cidrs { _, nw, err := net.ParseCIDR(cidr) if err != nil { - return errors.Errorf("cannot parse excluded CIDR constraint %q", cidr) + return fmt.Errorf("cannot parse excluded CIDR constraint %q", cidr) } networks[i] = nw } @@ -215,7 +214,7 @@ func WithPermittedIPsOrCIDRs(ipsOrCIDRs []string) NamePolicyOption { } else if ip := net.ParseIP(ipOrCIDR); ip != nil { networks[i] = networkFor(ip) } else { - return errors.Errorf("cannot parse permitted constraint %q as IP nor CIDR", ipOrCIDR) + return fmt.Errorf("cannot parse permitted constraint %q as IP nor CIDR", ipOrCIDR) } } e.permittedIPRanges = networks @@ -233,7 +232,7 @@ func WithExcludedIPsOrCIDRs(ipsOrCIDRs []string) NamePolicyOption { } else if ip := net.ParseIP(ipOrCIDR); ip != nil { networks[i] = networkFor(ip) } else { - return errors.Errorf("cannot parse excluded constraint %q as IP nor CIDR", ipOrCIDR) + return fmt.Errorf("cannot parse excluded constraint %q as IP nor CIDR", ipOrCIDR) } } e.excludedIPRanges = networks @@ -245,7 +244,7 @@ func WithPermittedCIDR(cidr string) NamePolicyOption { return func(e *NamePolicyEngine) error { _, nw, err := net.ParseCIDR(cidr) if err != nil { - return errors.Errorf("cannot parse permitted CIDR constraint %q", cidr) + return fmt.Errorf("cannot parse permitted CIDR constraint %q", cidr) } e.permittedIPRanges = []*net.IPNet{nw} return nil @@ -256,7 +255,7 @@ func AddPermittedCIDR(cidr string) NamePolicyOption { return func(e *NamePolicyEngine) error { _, nw, err := net.ParseCIDR(cidr) if err != nil { - return errors.Errorf("cannot parse permitted CIDR constraint %q", cidr) + return fmt.Errorf("cannot parse permitted CIDR constraint %q", cidr) } e.permittedIPRanges = append(e.permittedIPRanges, nw) return nil @@ -297,7 +296,7 @@ func WithExcludedCIDR(cidr string) NamePolicyOption { return func(e *NamePolicyEngine) error { _, nw, err := net.ParseCIDR(cidr) if err != nil { - return errors.Errorf("cannot parse excluded CIDR constraint %q", cidr) + return fmt.Errorf("cannot parse excluded CIDR constraint %q", cidr) } e.excludedIPRanges = []*net.IPNet{nw} return nil @@ -308,7 +307,7 @@ func AddExcludedCIDR(cidr string) NamePolicyOption { return func(e *NamePolicyEngine) error { _, nw, err := net.ParseCIDR(cidr) if err != nil { - return errors.Errorf("cannot parse excluded CIDR constraint %q", cidr) + return fmt.Errorf("cannot parse excluded CIDR constraint %q", cidr) } e.excludedIPRanges = append(e.excludedIPRanges, nw) return nil @@ -355,7 +354,7 @@ func WithPermittedEmailAddresses(emailAddresses []string) NamePolicyOption { for i, email := range emailAddresses { normalizedEmailAddress, err := normalizeAndValidateEmailConstraint(email) if err != nil { - return err + return fmt.Errorf("cannot parse permitted email constraint %q: %w", email, err) } normalizedEmailAddresses[i] = normalizedEmailAddress } @@ -370,7 +369,7 @@ func AddPermittedEmailAddresses(emailAddresses []string) NamePolicyOption { for i, email := range emailAddresses { normalizedEmailAddress, err := normalizeAndValidateEmailConstraint(email) if err != nil { - return err + return fmt.Errorf("cannot parse permitted email constraint %q: %w", email, err) } normalizedEmailAddresses[i] = normalizedEmailAddress } @@ -385,7 +384,7 @@ func WithExcludedEmailAddresses(emailAddresses []string) NamePolicyOption { for i, email := range emailAddresses { normalizedEmailAddress, err := normalizeAndValidateEmailConstraint(email) if err != nil { - return err + return fmt.Errorf("cannot parse excluded email constraint %q: %w", email, err) } normalizedEmailAddresses[i] = normalizedEmailAddress } @@ -400,7 +399,7 @@ func AddExcludedEmailAddresses(emailAddresses []string) NamePolicyOption { for i, email := range emailAddresses { normalizedEmailAddress, err := normalizeAndValidateEmailConstraint(email) if err != nil { - return err + return fmt.Errorf("cannot parse excluded email constraint %q: %w", email, err) } normalizedEmailAddresses[i] = normalizedEmailAddress } @@ -413,7 +412,7 @@ func WithPermittedEmailAddress(emailAddress string) NamePolicyOption { return func(e *NamePolicyEngine) error { normalizedEmailAddress, err := normalizeAndValidateEmailConstraint(emailAddress) if err != nil { - return err + return fmt.Errorf("cannot parse permitted email constraint %q: %w", emailAddress, err) } e.permittedEmailAddresses = []string{normalizedEmailAddress} return nil @@ -424,7 +423,7 @@ func AddPermittedEmailAddress(emailAddress string) NamePolicyOption { return func(e *NamePolicyEngine) error { normalizedEmailAddress, err := normalizeAndValidateEmailConstraint(emailAddress) if err != nil { - return err + return fmt.Errorf("cannot parse permitted email constraint %q: %w", emailAddress, err) } e.permittedEmailAddresses = append(e.permittedEmailAddresses, normalizedEmailAddress) return nil @@ -435,7 +434,7 @@ func WithExcludedEmailAddress(emailAddress string) NamePolicyOption { return func(e *NamePolicyEngine) error { normalizedEmailAddress, err := normalizeAndValidateEmailConstraint(emailAddress) if err != nil { - return err + return fmt.Errorf("cannot parse excluded email constraint %q: %w", emailAddress, err) } e.excludedEmailAddresses = []string{normalizedEmailAddress} return nil @@ -446,7 +445,7 @@ func AddExcludedEmailAddress(emailAddress string) NamePolicyOption { return func(e *NamePolicyEngine) error { normalizedEmailAddress, err := normalizeAndValidateEmailConstraint(emailAddress) if err != nil { - return err + return fmt.Errorf("cannot parse excluded email constraint %q: %w", emailAddress, err) } e.excludedEmailAddresses = append(e.excludedEmailAddresses, normalizedEmailAddress) return nil @@ -459,7 +458,7 @@ func WithPermittedURIDomains(uriDomains []string) NamePolicyOption { for i, domain := range uriDomains { normalizedURIDomain, err := normalizeAndValidateURIDomainConstraint(domain) if err != nil { - return err + return fmt.Errorf("cannot parse permitted URI domain constraint %q: %w", domain, err) } normalizedURIDomains[i] = normalizedURIDomain } @@ -474,7 +473,7 @@ func AddPermittedURIDomains(uriDomains []string) NamePolicyOption { for i, domain := range uriDomains { normalizedURIDomain, err := normalizeAndValidateURIDomainConstraint(domain) if err != nil { - return err + return fmt.Errorf("cannot parse permitted URI domain constraint %q: %w", domain, err) } normalizedURIDomains[i] = normalizedURIDomain } @@ -483,35 +482,35 @@ func AddPermittedURIDomains(uriDomains []string) NamePolicyOption { } } -func WithPermittedURIDomain(uriDomain string) NamePolicyOption { +func WithPermittedURIDomain(domain string) NamePolicyOption { return func(e *NamePolicyEngine) error { - normalizedURIDomain, err := normalizeAndValidateURIDomainConstraint(uriDomain) + normalizedURIDomain, err := normalizeAndValidateURIDomainConstraint(domain) if err != nil { - return err + return fmt.Errorf("cannot parse permitted URI domain constraint %q: %w", domain, err) } e.permittedURIDomains = []string{normalizedURIDomain} return nil } } -func AddPermittedURIDomain(uriDomain string) NamePolicyOption { +func AddPermittedURIDomain(domain string) NamePolicyOption { return func(e *NamePolicyEngine) error { - normalizedURIDomain, err := normalizeAndValidateURIDomainConstraint(uriDomain) + normalizedURIDomain, err := normalizeAndValidateURIDomainConstraint(domain) if err != nil { - return err + return fmt.Errorf("cannot parse permitted URI domain constraint %q: %w", domain, err) } e.permittedURIDomains = append(e.permittedURIDomains, normalizedURIDomain) return nil } } -func WithExcludedURIDomains(uriDomains []string) NamePolicyOption { +func WithExcludedURIDomains(domains []string) NamePolicyOption { return func(e *NamePolicyEngine) error { - normalizedURIDomains := make([]string, len(uriDomains)) - for i, domain := range uriDomains { + normalizedURIDomains := make([]string, len(domains)) + for i, domain := range domains { normalizedURIDomain, err := normalizeAndValidateURIDomainConstraint(domain) if err != nil { - return err + return fmt.Errorf("cannot parse excluded URI domain constraint %q: %w", domain, err) } normalizedURIDomains[i] = normalizedURIDomain } @@ -520,13 +519,13 @@ func WithExcludedURIDomains(uriDomains []string) NamePolicyOption { } } -func AddExcludedURIDomains(uriDomains []string) NamePolicyOption { +func AddExcludedURIDomains(domains []string) NamePolicyOption { return func(e *NamePolicyEngine) error { - normalizedURIDomains := make([]string, len(uriDomains)) - for i, domain := range uriDomains { + normalizedURIDomains := make([]string, len(domains)) + for i, domain := range domains { normalizedURIDomain, err := normalizeAndValidateURIDomainConstraint(domain) if err != nil { - return err + return fmt.Errorf("cannot parse excluded URI domain constraint %q: %w", domain, err) } normalizedURIDomains[i] = normalizedURIDomain } @@ -535,22 +534,22 @@ func AddExcludedURIDomains(uriDomains []string) NamePolicyOption { } } -func WithExcludedURIDomain(uriDomain string) NamePolicyOption { +func WithExcludedURIDomain(domain string) NamePolicyOption { return func(e *NamePolicyEngine) error { - normalizedURIDomain, err := normalizeAndValidateURIDomainConstraint(uriDomain) + normalizedURIDomain, err := normalizeAndValidateURIDomainConstraint(domain) if err != nil { - return err + return fmt.Errorf("cannot parse excluded URI domain constraint %q: %w", domain, err) } e.excludedURIDomains = []string{normalizedURIDomain} return nil } } -func AddExcludedURIDomain(uriDomain string) NamePolicyOption { +func AddExcludedURIDomain(domain string) NamePolicyOption { return func(e *NamePolicyEngine) error { - normalizedURIDomain, err := normalizeAndValidateURIDomainConstraint(uriDomain) + normalizedURIDomain, err := normalizeAndValidateURIDomainConstraint(domain) if err != nil { - return err + return fmt.Errorf("cannot parse excluded URI domain constraint %q: %w", domain, err) } e.excludedURIDomains = append(e.excludedURIDomains, normalizedURIDomain) return nil @@ -594,26 +593,29 @@ func isIPv4(ip net.IP) bool { func normalizeAndValidateDNSDomainConstraint(constraint string) (string, error) { normalizedConstraint := strings.ToLower(strings.TrimSpace(constraint)) if normalizedConstraint == "" { - return "", errors.Errorf("contraint %q can not be empty or white space string", constraint) + return "", fmt.Errorf("contraint %q can not be empty or white space string", constraint) } if strings.Contains(normalizedConstraint, "..") { - return "", errors.Errorf("domain constraint %q cannot have empty labels", constraint) + return "", fmt.Errorf("domain constraint %q cannot have empty labels", constraint) } - if normalizedConstraint[0] == '*' && normalizedConstraint[1] != '.' { - return "", errors.Errorf("wildcard character in domain constraint %q can only be used to match (full) labels", constraint) + if strings.HasPrefix(normalizedConstraint, ".") { + return "", fmt.Errorf("domain constraint %q with wildcard should start with *", constraint) } if strings.LastIndex(normalizedConstraint, "*") > 0 { - return "", errors.Errorf("domain constraint %q can only have wildcard as starting character", constraint) + return "", fmt.Errorf("domain constraint %q can only have wildcard as starting character", constraint) + } + if normalizedConstraint[0] == '*' && normalizedConstraint[1] != '.' { + return "", fmt.Errorf("wildcard character in domain constraint %q can only be used to match (full) labels", constraint) } if strings.HasPrefix(normalizedConstraint, "*.") { normalizedConstraint = normalizedConstraint[1:] // cut off wildcard character; keep the period } normalizedConstraint, err := idna.Lookup.ToASCII(normalizedConstraint) if err != nil { - return "", errors.Wrapf(err, "domain constraint %q can not be converted to ASCII", constraint) + return "", fmt.Errorf("domain constraint %q can not be converted to ASCII: %w", constraint, err) } if _, ok := domainToReverseLabels(normalizedConstraint); !ok { - return "", errors.Errorf("cannot parse domain constraint %q", constraint) + return "", fmt.Errorf("cannot parse domain constraint %q", constraint) } return normalizedConstraint, nil } @@ -621,7 +623,7 @@ func normalizeAndValidateDNSDomainConstraint(constraint string) (string, error) func normalizeAndValidateEmailConstraint(constraint string) (string, error) { normalizedConstraint := strings.ToLower(strings.TrimSpace(constraint)) if normalizedConstraint == "" { - return "", errors.Errorf("email contraint %q can not be empty or white space string", constraint) + return "", fmt.Errorf("email contraint %q can not be empty or white space string", constraint) } if strings.Contains(normalizedConstraint, "*") { return "", fmt.Errorf("email constraint %q cannot contain asterisk wildcard", constraint) @@ -645,14 +647,14 @@ func normalizeAndValidateEmailConstraint(constraint string) (string, error) { // https://datatracker.ietf.org/doc/html/rfc5280#section-7.5 domainASCII, err := idna.Lookup.ToASCII(mailbox.domain) if err != nil { - return "", errors.Wrapf(err, "email constraint %q domain part %q cannot be converted to ASCII", constraint, mailbox.domain) + return "", fmt.Errorf("email constraint %q domain part %q cannot be converted to ASCII: %w", constraint, mailbox.domain, err) } normalizedConstraint = mailbox.local + "@" + domainASCII } else { var err error normalizedConstraint, err = idna.Lookup.ToASCII(normalizedConstraint) if err != nil { - return "", errors.Wrapf(err, "email constraint %q cannot be converted to ASCII", constraint) + return "", fmt.Errorf("email constraint %q cannot be converted to ASCII: %w", constraint, err) } } if _, ok := domainToReverseLabels(normalizedConstraint); !ok { @@ -664,35 +666,38 @@ func normalizeAndValidateEmailConstraint(constraint string) (string, error) { func normalizeAndValidateURIDomainConstraint(constraint string) (string, error) { normalizedConstraint := strings.ToLower(strings.TrimSpace(constraint)) if normalizedConstraint == "" { - return "", errors.Errorf("URI domain contraint %q cannot be empty or white space string", constraint) + return "", fmt.Errorf("URI domain contraint %q cannot be empty or white space string", constraint) } if strings.Contains(normalizedConstraint, "://") { - return "", errors.Errorf("URI domain constraint %q contains scheme (not supported yet)", constraint) + return "", fmt.Errorf("URI domain constraint %q contains scheme (not supported yet)", constraint) } if strings.Contains(normalizedConstraint, "..") { - return "", errors.Errorf("URI domain constraint %q cannot have empty labels", constraint) + return "", fmt.Errorf("URI domain constraint %q cannot have empty labels", constraint) + } + if strings.HasPrefix(normalizedConstraint, ".") { + return "", fmt.Errorf("URI domain constraint %q with wildcard should start with *", constraint) + } + if strings.LastIndex(normalizedConstraint, "*") > 0 { + return "", fmt.Errorf("URI domain constraint %q can only have wildcard as starting character", constraint) } if strings.HasPrefix(normalizedConstraint, "*.") { normalizedConstraint = normalizedConstraint[1:] // cut off wildcard character; keep the period } - if strings.Contains(normalizedConstraint, "*") { - return "", errors.Errorf("URI domain constraint %q can only have wildcard as starting character", constraint) - } // we're being strict with square brackets in domains; we don't allow them, no matter what if strings.Contains(normalizedConstraint, "[") || strings.Contains(normalizedConstraint, "]") { - return "", errors.Errorf("URI domain constraint %q contains invalid square brackets", constraint) + return "", fmt.Errorf("URI domain constraint %q contains invalid square brackets", constraint) } if _, _, err := net.SplitHostPort(normalizedConstraint); err == nil { // a successful split (likely) with host and port; we don't currently allow ports in the config - return "", errors.Errorf("URI domain constraint %q cannot contain port", constraint) + return "", fmt.Errorf("URI domain constraint %q cannot contain port", constraint) } // check if the host part of the URI domain constraint is an IP if net.ParseIP(normalizedConstraint) != nil { - return "", errors.Errorf("URI domain constraint %q cannot be an IP", constraint) + return "", fmt.Errorf("URI domain constraint %q cannot be an IP", constraint) } normalizedConstraint, err := idna.Lookup.ToASCII(normalizedConstraint) if err != nil { - return "", errors.Wrapf(err, "URI domain constraint %q cannot be converted to ASCII", constraint) + return "", fmt.Errorf("URI domain constraint %q cannot be converted to ASCII: %w", constraint, err) } _, ok := domainToReverseLabels(normalizedConstraint) if !ok { diff --git a/policy/options_117_test.go b/policy/options_117_test.go index bd3d287d..916eefe2 100644 --- a/policy/options_117_test.go +++ b/policy/options_117_test.go @@ -42,6 +42,12 @@ func Test_normalizeAndValidateURIDomainConstraint(t *testing.T) { want: "", wantErr: true, }, + { + name: "fail/no-asterisk", + constraint: ".example.com", + want: "", + wantErr: true, + }, { name: "fail/domain-with-port", constraint: "host.local:8443", diff --git a/policy/options_118_test.go b/policy/options_118_test.go index 059f1177..6fa2ded4 100644 --- a/policy/options_118_test.go +++ b/policy/options_118_test.go @@ -48,6 +48,12 @@ func Test_normalizeAndValidateURIDomainConstraint(t *testing.T) { want: "", wantErr: true, }, + { + name: "fail/no-asterisk", + constraint: ".example.com", + want: "", + wantErr: true, + }, { name: "fail/ipv4", constraint: "127.0.0.1", diff --git a/policy/options_test.go b/policy/options_test.go index 74982fd8..a1c48e1f 100644 --- a/policy/options_test.go +++ b/policy/options_test.go @@ -46,6 +46,12 @@ func Test_normalizeAndValidateDNSDomainConstraint(t *testing.T) { want: "", wantErr: true, }, + { + name: "fail/no-asterisk", + constraint: ".example.com", + want: "", + wantErr: true, + }, { name: "fail/idna-internationalized-domain-name-lookup", constraint: `\00.local`, // invalid IDNA ASCII character @@ -66,13 +72,13 @@ func Test_normalizeAndValidateDNSDomainConstraint(t *testing.T) { }, { name: "ok/idna-internationalized-domain-name-punycode", - constraint: ".xn--fsq.jp", // Example value from https://www.w3.org/International/articles/idn-and-iri/ + constraint: "*.xn--fsq.jp", // Example value from https://www.w3.org/International/articles/idn-and-iri/ want: ".xn--fsq.jp", wantErr: false, }, { name: "ok/idna-internationalized-domain-name-lookup-transformed", - constraint: ".例.jp", // Example value from https://www.w3.org/International/articles/idn-and-iri/ + constraint: "*.例.jp", // Example value from https://www.w3.org/International/articles/idn-and-iri/ want: ".xn--fsq.jp", wantErr: false, }, diff --git a/policy/validate.go b/policy/validate.go index b85eb299..fd611b74 100644 --- a/policy/validate.go +++ b/policy/validate.go @@ -398,7 +398,7 @@ func parseRFC2821Mailbox(in string) (mailbox rfc2821Mailbox, ok bool) { return mailbox, true } -// matchDomainConstraint matches a domain agains the given constraint +// matchDomainConstraint matches a domain against the given constraint 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.