From cc26a0b394b363b02f06f9c9e0e70de150504939 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 6 May 2022 13:58:48 +0200 Subject: [PATCH] Explicitly disable wildcard Common Name constraint --- policy/engine_test.go | 1 + policy/options.go | 31 ++++++++++++++++++++-- policy/options_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++ policy/validate.go | 4 +++ 4 files changed, 92 insertions(+), 2 deletions(-) diff --git a/policy/engine_test.go b/policy/engine_test.go index fabfebb9..1280d14d 100755 --- a/policy/engine_test.go +++ b/policy/engine_test.go @@ -2492,6 +2492,7 @@ func TestNamePolicyEngine_X509_AllAllowed(t *testing.T) { t.Run(tt.name, func(t *testing.T) { engine, err := New(tt.options...) assert.NoError(t, err) + assert.NotNil(t, engine) gotErr := engine.IsX509CertificateAllowed(tt.cert) wantErr := tt.wantErr != nil diff --git a/policy/options.go b/policy/options.go index 79507f43..f08f9180 100755 --- a/policy/options.go +++ b/policy/options.go @@ -28,14 +28,30 @@ func WithAllowLiteralWildcardNames() NamePolicyOption { func WithPermittedCommonNames(commonNames ...string) NamePolicyOption { return func(g *NamePolicyEngine) error { - g.permittedCommonNames = commonNames + normalizedCommonNames := make([]string, len(commonNames)) + for i, commonName := range commonNames { + normalizedCommonName, err := normalizeAndValidateCommonName(commonName) + if err != nil { + return fmt.Errorf("cannot parse permitted common name constraint %q: %w", commonName, err) + } + normalizedCommonNames[i] = normalizedCommonName + } + g.permittedCommonNames = normalizedCommonNames return nil } } func WithExcludedCommonNames(commonNames ...string) NamePolicyOption { return func(g *NamePolicyEngine) error { - g.excludedCommonNames = commonNames + normalizedCommonNames := make([]string, len(commonNames)) + for i, commonName := range commonNames { + normalizedCommonName, err := normalizeAndValidateCommonName(commonName) + if err != nil { + return fmt.Errorf("cannot parse excluded common name constraint %q: %w", commonName, err) + } + normalizedCommonNames[i] = normalizedCommonName + } + g.excludedCommonNames = normalizedCommonNames return nil } } @@ -242,6 +258,17 @@ func isIPv4(ip net.IP) bool { return ip.To4() != nil } +func normalizeAndValidateCommonName(constraint string) (string, error) { + normalizedConstraint := strings.ToLower(strings.TrimSpace(constraint)) + if normalizedConstraint == "" { + return "", fmt.Errorf("contraint %q can not be empty or white space string", constraint) + } + if normalizedConstraint == "*" { + return "", fmt.Errorf("wildcard constraint %q is not supported", constraint) + } + return normalizedConstraint, nil +} + func normalizeAndValidateDNSDomainConstraint(constraint string) (string, error) { normalizedConstraint := strings.ToLower(strings.TrimSpace(constraint)) if normalizedConstraint == "" { diff --git a/policy/options_test.go b/policy/options_test.go index d1a62a9f..697afecf 100644 --- a/policy/options_test.go +++ b/policy/options_test.go @@ -8,6 +8,46 @@ import ( "github.com/stretchr/testify/assert" ) +func Test_normalizeAndValidateCommonName(t *testing.T) { + tests := []struct { + name string + constraint string + want string + wantErr bool + }{ + { + name: "fail/empty-constraint", + constraint: "", + want: "", + wantErr: true, + }, + { + name: "fail/wildcard", + constraint: "*", + want: "", + wantErr: true, + }, + { + name: "ok", + constraint: "step", + want: "step", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := normalizeAndValidateCommonName(tt.constraint) + if (err != nil) != tt.wantErr { + t.Errorf("normalizeAndValidateCommonName() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("normalizeAndValidateCommonName() = %v, want %v", got, tt.want) + } + }) + } +} + func Test_normalizeAndValidateDNSDomainConstraint(t *testing.T) { tests := []struct { name string @@ -196,6 +236,24 @@ func TestNew(t *testing.T) { wantErr bool } var tests = map[string]func(t *testing.T) test{ + "fail/with-permitted-common-name": func(t *testing.T) test { + return test{ + options: []NamePolicyOption{ + WithPermittedCommonNames("*"), + }, + want: nil, + wantErr: true, + } + }, + "fail/with-excluded-common-name": func(t *testing.T) test { + return test{ + options: []NamePolicyOption{ + WithExcludedCommonNames(""), + }, + want: nil, + wantErr: true, + } + }, "fail/with-permitted-dns-domains": func(t *testing.T) test { return test{ options: []NamePolicyOption{ diff --git a/policy/validate.go b/policy/validate.go index 968e936d..ee6f7e9c 100644 --- a/policy/validate.go +++ b/policy/validate.go @@ -639,5 +639,9 @@ func matchPrincipalConstraint(principal, constraint string) (bool, error) { // matchCommonNameConstraint performs a string literal equality check against constraint. func matchCommonNameConstraint(commonName, constraint string) (bool, error) { + // wildcard constraint is (currently) not supported for common names + if constraint == "*" { + return false, nil + } return strings.EqualFold(commonName, constraint), nil }