diff --git a/acme/account.go b/acme/account.go index b83defe1..2dd412db 100644 --- a/acme/account.go +++ b/acme/account.go @@ -53,8 +53,9 @@ type PolicyNames struct { // X509Policy contains ACME account level X.509 policy type X509Policy struct { - Allowed PolicyNames `json:"allow"` - Denied PolicyNames `json:"deny"` + Allowed PolicyNames `json:"allow"` + Denied PolicyNames `json:"deny"` + AllowWildcardNames bool `json:"allowWildcardNames"` } // Policy is an ACME Account level policy @@ -81,18 +82,14 @@ func (p *Policy) GetDeniedNameOptions() *policy.X509NameOptions { } } -// IsWildcardLiteralAllowed returns true by default for -// ACME account policies, as authorization is performed on DNS -// level. -func (p *Policy) IsWildcardLiteralAllowed() bool { - return true -} - -// ShouldVerifySubjectCommonName returns true by default -// for ACME account policies, as this is embedded in the -// protocol. -func (p *Policy) ShouldVerifyCommonName() bool { - return true +// AreWildcardNamesAllowed returns if wildcard names +// like *.example.com are allowed to be signed. +// Defaults to false. +func (p *Policy) AreWildcardNamesAllowed() bool { + if p == nil { + return false + } + return p.X509.AllowWildcardNames } // ExternalAccountKey is an ACME External Account Binding key. diff --git a/authority/admin/api/acme.go b/authority/admin/api/acme.go index 026443fa..31949081 100644 --- a/authority/admin/api/acme.go +++ b/authority/admin/api/acme.go @@ -109,6 +109,7 @@ func eakToLinked(k *acme.ExternalAccountKey) *linkedca.EABKey { eak.Policy.X509.Allow.Ips = k.Policy.X509.Allowed.IPRanges eak.Policy.X509.Deny.Dns = k.Policy.X509.Denied.DNSNames eak.Policy.X509.Deny.Ips = k.Policy.X509.Denied.IPRanges + eak.Policy.X509.AllowWildcardNames = k.Policy.X509.AllowWildcardNames } return eak @@ -143,6 +144,7 @@ func linkedEAKToCertificates(k *linkedca.EABKey) *acme.ExternalAccountKey { eak.Policy.X509.Denied.DNSNames = deny.Dns eak.Policy.X509.Denied.IPRanges = deny.Ips } + eak.Policy.X509.AllowWildcardNames = x509.AllowWildcardNames } } diff --git a/authority/admin/api/acme_test.go b/authority/admin/api/acme_test.go index 5094d5f0..3ff32763 100644 --- a/authority/admin/api/acme_test.go +++ b/authority/admin/api/acme_test.go @@ -512,6 +512,7 @@ func Test_linkedEAKToCertificates(t *testing.T) { Dns: []string{"badhost.local"}, Ips: []string{"10.0.0.30"}, }, + AllowWildcardNames: true, }, }, }, @@ -533,6 +534,7 @@ func Test_linkedEAKToCertificates(t *testing.T) { DNSNames: []string{"badhost.local"}, IPRanges: []string{"10.0.0.30"}, }, + AllowWildcardNames: true, }, }, }, diff --git a/authority/policy.go b/authority/policy.go index dd38fd4a..063a464c 100644 --- a/authority/policy.go +++ b/authority/policy.go @@ -311,7 +311,7 @@ func policyToCertificates(p *linkedca.Policy) *authPolicy.Options { } } - opts.X509.AllowWildcardLiteral = x509.AllowWildcardLiteral + opts.X509.AllowWildcardNames = x509.GetAllowWildcardNames() } // fill ssh policy configuration diff --git a/authority/policy/options.go b/authority/policy/options.go index 0f50083d..b93d2cd1 100644 --- a/authority/policy/options.go +++ b/authority/policy/options.go @@ -30,7 +30,7 @@ func (o *Options) GetSSHOptions() *SSHPolicyOptions { type X509PolicyOptionsInterface interface { GetAllowedNameOptions() *X509NameOptions GetDeniedNameOptions() *X509NameOptions - IsWildcardLiteralAllowed() bool + AreWildcardNamesAllowed() bool } // X509PolicyOptions is a container for x509 allowed and denied @@ -42,10 +42,9 @@ type X509PolicyOptions struct { // DeniedNames contains the x509 denied names DeniedNames *X509NameOptions `json:"deny,omitempty"` - // AllowWildcardLiteral indicates if literal wildcard names - // such as *.example.com and @example.com are allowed. Defaults - // to false. - AllowWildcardLiteral bool `json:"allowWildcardLiteral,omitempty"` + // AllowWildcardNames indicates if literal wildcard names + // like *.example.com are allowed. Defaults to false. + AllowWildcardNames bool `json:"allowWildcardNames,omitempty"` } // X509NameOptions models the X509 name policy configuration. @@ -83,13 +82,13 @@ func (o *X509PolicyOptions) GetDeniedNameOptions() *X509NameOptions { return o.DeniedNames } -// IsWildcardLiteralAllowed returns whether the authority allows -// literal wildcard domains and other names to be signed. -func (o *X509PolicyOptions) IsWildcardLiteralAllowed() bool { +// AreWildcardNamesAllowed returns whether the authority allows +// literal wildcard names to be signed. +func (o *X509PolicyOptions) AreWildcardNamesAllowed() bool { if o == nil { return true } - return o.AllowWildcardLiteral + return o.AllowWildcardNames } // SSHPolicyOptionsInterface is an interface for providers of diff --git a/authority/policy/options_test.go b/authority/policy/options_test.go index d7d42093..0fd6e7c6 100644 --- a/authority/policy/options_test.go +++ b/authority/policy/options_test.go @@ -23,21 +23,21 @@ func TestX509PolicyOptions_IsWildcardLiteralAllowed(t *testing.T) { { name: "set-true", options: &X509PolicyOptions{ - AllowWildcardLiteral: true, + AllowWildcardNames: true, }, want: true, }, { name: "set-false", options: &X509PolicyOptions{ - AllowWildcardLiteral: false, + AllowWildcardNames: false, }, want: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := tt.options.IsWildcardLiteralAllowed(); got != tt.want { + if got := tt.options.AreWildcardNamesAllowed(); got != tt.want { t.Errorf("X509PolicyOptions.IsWildcardLiteralAllowed() = %v, want %v", got, tt.want) } }) diff --git a/authority/policy/policy.go b/authority/policy/policy.go index f5f0fce3..52297d65 100644 --- a/authority/policy/policy.go +++ b/authority/policy/policy.go @@ -52,7 +52,7 @@ func NewX509PolicyEngine(policyOptions X509PolicyOptionsInterface) (X509Policy, return nil, nil } - if policyOptions.IsWildcardLiteralAllowed() { + if policyOptions.AreWildcardNamesAllowed() { options = append(options, policy.WithAllowLiteralWildcardNames()) } diff --git a/authority/policy_test.go b/authority/policy_test.go index 3f03abd9..e64752ec 100644 --- a/authority/policy_test.go +++ b/authority/policy_test.go @@ -218,7 +218,7 @@ func Test_policyToCertificates(t *testing.T) { Allow: &linkedca.X509Names{ Dns: []string{"*.local"}, }, - AllowWildcardLiteral: false, + AllowWildcardNames: false, }, }, want: &policy.Options{ @@ -226,7 +226,7 @@ func Test_policyToCertificates(t *testing.T) { AllowedNames: &policy.X509NameOptions{ DNSDomains: []string{"*.local"}, }, - AllowWildcardLiteral: false, + AllowWildcardNames: false, }, }, }, @@ -248,7 +248,7 @@ func Test_policyToCertificates(t *testing.T) { Uris: []string{"https://badhost.local"}, CommonNames: []string{"another name"}, }, - AllowWildcardLiteral: true, + AllowWildcardNames: true, }, Ssh: &linkedca.SSHPolicy{ Host: &linkedca.SSHHostPolicy{ @@ -291,7 +291,7 @@ func Test_policyToCertificates(t *testing.T) { URIDomains: []string{"https://badhost.local"}, CommonNames: []string{"another name"}, }, - AllowWildcardLiteral: true, + AllowWildcardNames: true, }, SSH: &policy.SSHPolicyOptions{ Host: &policy.SSHHostCertificateOptions{ @@ -369,7 +369,7 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) { DeniedNames: &policy.X509NameOptions{ DNSDomains: []string{"badhost.local"}, }, - AllowWildcardLiteral: true, + AllowWildcardNames: true, }, } @@ -428,7 +428,7 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) { DeniedNames: &policy.X509NameOptions{ DNSDomains: []string{"badhost.local"}, }, - AllowWildcardLiteral: true, + AllowWildcardNames: true, }, SSH: &policy.SSHPolicyOptions{ Host: &policy.SSHHostCertificateOptions{ @@ -486,7 +486,7 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) { DeniedNames: &policy.X509NameOptions{ DNSDomains: []string{"badhost.local"}, }, - AllowWildcardLiteral: true, + AllowWildcardNames: true, }, SSH: &policy.SSHPolicyOptions{ Host: &policy.SSHHostCertificateOptions{ @@ -697,7 +697,7 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) { DeniedNames: &policy.X509NameOptions{ DNSDomains: []string{"badhost.local"}, }, - AllowWildcardLiteral: true, + AllowWildcardNames: true, }, }, }, @@ -796,7 +796,7 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) { DeniedNames: &policy.X509NameOptions{ DNSDomains: []string{"badhost.local"}, }, - AllowWildcardLiteral: true, + AllowWildcardNames: true, }, SSH: &policy.SSHPolicyOptions{ Host: &policy.SSHHostCertificateOptions{ @@ -911,7 +911,7 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) { Deny: &linkedca.X509Names{ Dns: []string{"badhost.local"}, }, - AllowWildcardLiteral: true, + AllowWildcardNames: true, }, Ssh: &linkedca.SSHPolicy{ Host: &linkedca.SSHHostPolicy{ @@ -976,7 +976,7 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) { Deny: &linkedca.X509Names{ Dns: []string{"badhost.local"}, }, - AllowWildcardLiteral: true, + AllowWildcardNames: true, }, }, nil }, @@ -996,11 +996,6 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) { t.Errorf("Authority.reloadPolicyEngines() error = %v, wantErr %v", err, tt.wantErr) } - // TODO(hs): fix those - // assert.Equal(t, tt.expected.x509Policy, a.x509Policy) - // assert.Equal(t, tt.expected.sshHostPolicy, a.sshHostPolicy) - // assert.Equal(t, tt.expected.sshUserPolicy, a.sshUserPolicy) - assert.Equal(t, tt.expected, a.policyEngine) }) } diff --git a/authority/provisioner/options.go b/authority/provisioner/options.go index 50af8396..f5c919b4 100644 --- a/authority/provisioner/options.go +++ b/authority/provisioner/options.go @@ -66,10 +66,9 @@ type X509Options struct { // DeniedNames contains the SANs the provisioner is not authorized to sign DeniedNames *policy.X509NameOptions `json:"-"` - // AllowWildcardLiteral indicates if literal wildcard names - // such as *.example.com and @example.com are allowed. Defaults - // to false. - AllowWildcardLiteral bool `json:"-"` + // AllowWildcardNames indicates if literal wildcard names + // like *.example.com are allowed. Defaults to false. + AllowWildcardNames bool `json:"-"` } // HasTemplate returns true if a template is defined in the provisioner options. @@ -95,11 +94,11 @@ func (o *X509Options) GetDeniedNameOptions() *policy.X509NameOptions { return o.DeniedNames } -func (o *X509Options) IsWildcardLiteralAllowed() bool { +func (o *X509Options) AreWildcardNamesAllowed() bool { if o == nil { return true } - return o.AllowWildcardLiteral + return o.AllowWildcardNames } // TemplateOptions generates a CertificateOptions with the template and data diff --git a/authority/provisioner/options_test.go b/authority/provisioner/options_test.go index 7883d045..0bcf9ec3 100644 --- a/authority/provisioner/options_test.go +++ b/authority/provisioner/options_test.go @@ -302,21 +302,21 @@ func TestX509Options_IsWildcardLiteralAllowed(t *testing.T) { { name: "set-true", options: &X509Options{ - AllowWildcardLiteral: true, + AllowWildcardNames: true, }, want: true, }, { name: "set-false", options: &X509Options{ - AllowWildcardLiteral: false, + AllowWildcardNames: false, }, want: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := tt.options.IsWildcardLiteralAllowed(); got != tt.want { + if got := tt.options.AreWildcardNamesAllowed(); got != tt.want { t.Errorf("X509PolicyOptions.IsWildcardLiteralAllowed() = %v, want %v", got, tt.want) } }) diff --git a/policy/engine_test.go b/policy/engine_test.go index dd6db586..fabfebb9 100755 --- a/policy/engine_test.go +++ b/policy/engine_test.go @@ -285,17 +285,6 @@ func TestNamePolicyEngine_matchEmailConstraint(t *testing.T) { want bool wantErr bool }{ - { - name: "fail/asterisk-prefix", - engine: &NamePolicyEngine{}, - mailbox: rfc2821Mailbox{ - local: "mail", - domain: "local", - }, - constraint: "*@example.com", - want: false, - wantErr: true, - }, { name: "fail/asterisk-label", engine: &NamePolicyEngine{}, @@ -307,17 +296,6 @@ func TestNamePolicyEngine_matchEmailConstraint(t *testing.T) { want: false, wantErr: true, }, - { - name: "fail/asterisk-inside-local", - engine: &NamePolicyEngine{}, - mailbox: rfc2821Mailbox{ - local: "mail", - domain: "local", - }, - constraint: "m*il@local", - want: false, - wantErr: true, - }, { name: "fail/asterisk-inside-domain", engine: &NamePolicyEngine{}, @@ -358,7 +336,7 @@ func TestNamePolicyEngine_matchEmailConstraint(t *testing.T) { local: "mail", domain: "local", }, - constraint: ".local", // "wildcard" for the local domain; requires exactly 1 subdomain + constraint: ".local", want: false, wantErr: false, }, @@ -406,6 +384,50 @@ func TestNamePolicyEngine_matchEmailConstraint(t *testing.T) { want: true, wantErr: false, }, + { + name: "ok/asterisk-prefix", + engine: &NamePolicyEngine{}, + mailbox: rfc2821Mailbox{ + local: "mail", + domain: "local", + }, + constraint: "*@example.com", + want: false, + wantErr: false, + }, + { + name: "ok/asterisk-prefix-match", + engine: &NamePolicyEngine{}, + mailbox: rfc2821Mailbox{ + local: "*", + domain: "example.com", + }, + constraint: "*@example.com", + want: true, + wantErr: false, + }, + { + name: "ok/asterisk-inside-local", + engine: &NamePolicyEngine{}, + mailbox: rfc2821Mailbox{ + local: "mail", + domain: "local", + }, + constraint: "m*il@local", + want: false, + wantErr: false, + }, + { + name: "ok/asterisk-inside-local-match", + engine: &NamePolicyEngine{}, + mailbox: rfc2821Mailbox{ + local: "m*il", + domain: "local", + }, + constraint: "m*il@local", + want: true, + wantErr: false, + }, { name: "ok/specific-mail", engine: &NamePolicyEngine{}, diff --git a/policy/validate.go b/policy/validate.go index abd150db..968e936d 100644 --- a/policy/validate.go +++ b/policy/validate.go @@ -125,7 +125,7 @@ func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailA Reason: CannotParseDomain, NameType: EmailNameType, Name: email, - detail: fmt.Sprintf("cannot parse email domain %q", email), + detail: fmt.Errorf("cannot parse email domain %q: %w", email, err).Error(), } } mailbox.domain = domainASCII @@ -577,11 +577,6 @@ func matchIPConstraint(ip net.IP, constraint *net.IPNet) (bool, error) { // 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 { @@ -617,7 +612,7 @@ func (e *NamePolicyEngine) matchURIConstraint(uri *url.URL, constraint string) ( if strings.Contains(host, ":") && !strings.HasSuffix(host, "]") { var err error - host, _, err = net.SplitHostPort(uri.Host) + host, _, err = net.SplitHostPort(host) if err != nil { return false, err }