diff --git a/internal/ingress/annotations/authtls/main.go b/internal/ingress/annotations/authtls/main.go index adedb084a0..e288d82c97 100644 --- a/internal/ingress/annotations/authtls/main.go +++ b/internal/ingress/annotations/authtls/main.go @@ -41,9 +41,7 @@ const ( ) var ( - regexChars = regexp.QuoteMeta(`()|=`) authVerifyClientRegex = regexp.MustCompile(`on|off|optional|optional_no_ca`) - commonNameRegex = regexp.MustCompile(`^CN=[/\-.\_\~a-zA-Z0-9` + regexChars + `]*$`) redirectRegex = regexp.MustCompile(`^((https?://)?[A-Za-z0-9\-.]*(:\d+)?/[A-Za-z0-9\-.]*)?$`) ) @@ -81,7 +79,7 @@ var authTLSAnnotations = parser.Annotation{ Documentation: `This annotation defines if the received certificates should be passed or not to the upstream server in the header "ssl-client-cert"`, }, annotationAuthTLSMatchCN: { - Validator: parser.ValidateRegex(commonNameRegex, true), + Validator: parser.CommonNameAnnotationValidator, Scope: parser.AnnotationScopeLocation, Risk: parser.AnnotationRiskHigh, Documentation: `This annotation adds a sanity check for the CN of the client certificate that is sent over using a string / regex starting with "CN="`, diff --git a/internal/ingress/annotations/parser/validators.go b/internal/ingress/annotations/parser/validators.go index 9a46bc8407..64a9d133d2 100644 --- a/internal/ingress/annotations/parser/validators.go +++ b/internal/ingress/annotations/parser/validators.go @@ -117,6 +117,20 @@ func ValidateRegex(regex *regexp.Regexp, removeSpace bool) AnnotationValidator { } } +// CommonNameAnnotationValidator checks whether the annotation value starts with +// 'CN=' and is followed by a valid regex. +func CommonNameAnnotationValidator(s string) error { + if !strings.HasPrefix(s, "CN=") { + return fmt.Errorf("value %s is not a valid Common Name annotation: missing prefix 'CN='", s) + } + + if _, err := regexp.Compile(s[3:]); err != nil { + return fmt.Errorf("value %s is not a valid regex: %w", s, err) + } + + return nil +} + // ValidateOptions receives an array of valid options that can be the value of annotation. // If no valid option is found, it will return an error func ValidateOptions(options []string, caseSensitive, trimSpace bool) AnnotationValidator { diff --git a/internal/ingress/annotations/parser/validators_test.go b/internal/ingress/annotations/parser/validators_test.go index e7aeb15ca4..8523232a2e 100644 --- a/internal/ingress/annotations/parser/validators_test.go +++ b/internal/ingress/annotations/parser/validators_test.go @@ -307,3 +307,59 @@ func TestCheckAnnotationRisk(t *testing.T) { }) } } + +func TestCommonNameAnnotationValidator(t *testing.T) { + tests := []struct { + name string + annotation string + wantErr bool + }{ + { + name: "correct example", + annotation: `CN=(my\.common\.name)`, + wantErr: false, + }, + { + name: "no CN= prefix", + annotation: `(my\.common\.name)`, + wantErr: true, + }, + { + name: "invalid prefix", + annotation: `CN(my\.common\.name)`, + wantErr: true, + }, + { + name: "invalid regex", + annotation: `CN=(my\.common\.name]`, + wantErr: true, + }, + { + name: "wildcard regex", + annotation: `CN=(my\..*\.name)`, + wantErr: false, + }, + { + name: "somewhat complex regex", + annotation: "CN=(my\\.app\\.dev|.*\\.bbb\\.aaaa\\.tld)", + wantErr: false, + }, + { + name: "another somewhat complex regex", + annotation: `CN=(my-app.*\.c\.defg\.net|other.app.com)`, + wantErr: false, + }, + { + name: "nested parenthesis regex", + annotation: `CN=(api-one\.(asdf)?qwer\.webpage\.organization\.org)`, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := CommonNameAnnotationValidator(tt.annotation); (err != nil) != tt.wantErr { + t.Errorf("CommonNameAnnotationValidator() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +}