From 6b2eb671bfd4e2c2edc3754a71a0b28a63b31e06 Mon Sep 17 00:00:00 2001 From: Haywood Shannon <5781935+haywoodsh@users.noreply.github.com> Date: Fri, 10 Jun 2022 13:54:17 +0100 Subject: [PATCH 1/7] add validation functions --- internal/k8s/validation_test.go | 46 ++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/internal/k8s/validation_test.go b/internal/k8s/validation_test.go index e1db3213cd..5c18067fca 100644 --- a/internal/k8s/validation_test.go +++ b/internal/k8s/validation_test.go @@ -1394,6 +1394,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { { annotations: map[string]string{ "nginx.com/jwt-realm": "my-jwt-realm", + "nginx.com/jwt-key": "my-jwk", }, specServices: map[string]bool{}, isPlus: true, @@ -1487,9 +1488,51 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { }, msg: "invalid nginx.com/jwt-token annotation, nginx plus only", }, + { + annotations: map[string]string{ + "nginx.com/jwt-token": "true", + "nginx.com/jwt-key": "my-jwk", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.com/jwt-token annotation", + }, + { + annotations: map[string]string{ + "nginx.com/jwt-realm": "realm$1", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.com/jwt-realm: Invalid value: "realm$1": a valid annotation value must have all '"' escaped and must not contain any '$' or end with an unescaped '\' (e.g. 'My Realm', or 'Cafe App', regex used for validation is '([^"$\\]|\\[^$])*')`, + }, + msg: "valid nginx.com/jwt-token annotation", + }, + { + annotations: map[string]string{ + "nginx.com/jwt-key": "app/jwk-secret", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.com/jwt-realm: Invalid value: "realm$1": a valid annotation value must have all '"' escaped and must not contain any '$' or end with an unescaped '\' (e.g. 'My Realm', or 'Cafe App', regex used for validation is '([^"$\\]|\\[^$])*')`, + }, + msg: "valid nginx.com/jwt-token annotation", + }, { annotations: map[string]string{ "nginx.com/jwt-token": "$cookie_auth_token", + "nginx.com/jwt-key": "my-jwk", }, specServices: map[string]bool{}, isPlus: true, @@ -1517,6 +1560,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { { annotations: map[string]string{ "nginx.com/jwt-login-url": "https://login.example.com", + "nginx.com/jwt-key": "my-jwk", }, specServices: map[string]bool{}, isPlus: true, @@ -2050,7 +2094,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { expectedErrors: []string{ `annotations.nginx.org/ssl-services: Invalid value: "service-1,service-2": must be a comma-separated list of services. The following services were not found: service-2`, }, - msg: "invalid nginx.org/ssl-services annotation, service does not exist", + msg: "invalid nginx.org/ssl-se§rvices annotation, service does not exist", }, { From 9cf8c0fcfa6fea82cfab3eb0fbee90dd0bce8331 Mon Sep 17 00:00:00 2001 From: Haywood Shannon <5781935+haywoodsh@users.noreply.github.com> Date: Tue, 14 Jun 2022 09:57:22 +0100 Subject: [PATCH 2/7] validate jwt login url and add unit tests --- internal/k8s/validation_test.go | 68 ++++++++++++++++++++++----------- 1 file changed, 45 insertions(+), 23 deletions(-) diff --git a/internal/k8s/validation_test.go b/internal/k8s/validation_test.go index 5c18067fca..63dc893e7d 100644 --- a/internal/k8s/validation_test.go +++ b/internal/k8s/validation_test.go @@ -1394,7 +1394,6 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { { annotations: map[string]string{ "nginx.com/jwt-realm": "my-jwt-realm", - "nginx.com/jwt-key": "my-jwk", }, specServices: map[string]bool{}, isPlus: true, @@ -1476,63 +1475,59 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { { annotations: map[string]string{ - "nginx.com/jwt-token": "true", + "nginx.com/jwt-key": "my_jwk", }, specServices: map[string]bool{}, - isPlus: false, + isPlus: true, appProtectEnabled: false, appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - "annotations.nginx.com/jwt-token: Forbidden: annotation requires NGINX Plus", + "annotations.nginx.com/jwt-key: Invalid value: \"my_jwk\": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')", }, - msg: "invalid nginx.com/jwt-token annotation, nginx plus only", + msg: "invalid nginx.com/jwt-key annotation, containing '_", }, { annotations: map[string]string{ "nginx.com/jwt-token": "true", - "nginx.com/jwt-key": "my-jwk", }, specServices: map[string]bool{}, - isPlus: true, + isPlus: false, appProtectEnabled: false, appProtectDosEnabled: false, internalRoutesEnabled: false, - expectedErrors: nil, - msg: "valid nginx.com/jwt-token annotation", + expectedErrors: []string{ + "annotations.nginx.com/jwt-token: Forbidden: annotation requires NGINX Plus", + }, + msg: "invalid nginx.com/jwt-token annotation, nginx plus only", }, { annotations: map[string]string{ - "nginx.com/jwt-realm": "realm$1", + "nginx.com/jwt-token": "$cookie_auth_token", }, specServices: map[string]bool{}, isPlus: true, appProtectEnabled: false, appProtectDosEnabled: false, internalRoutesEnabled: false, - expectedErrors: []string{ - `annotations.nginx.com/jwt-realm: Invalid value: "realm$1": a valid annotation value must have all '"' escaped and must not contain any '$' or end with an unescaped '\' (e.g. 'My Realm', or 'Cafe App', regex used for validation is '([^"$\\]|\\[^$])*')`, - }, - msg: "valid nginx.com/jwt-token annotation", + expectedErrors: nil, + msg: "valid nginx.com/jwt-token annotation", }, { annotations: map[string]string{ - "nginx.com/jwt-key": "app/jwk-secret", + "nginx.com/jwt-token": "true", }, specServices: map[string]bool{}, isPlus: true, appProtectEnabled: false, appProtectDosEnabled: false, internalRoutesEnabled: false, - expectedErrors: []string{ - `annotations.nginx.com/jwt-realm: Invalid value: "realm$1": a valid annotation value must have all '"' escaped and must not contain any '$' or end with an unescaped '\' (e.g. 'My Realm', or 'Cafe App', regex used for validation is '([^"$\\]|\\[^$])*')`, - }, - msg: "valid nginx.com/jwt-token annotation", + expectedErrors: nil, + msg: "Invalid nginx.com/jwt-token annotation", }, { annotations: map[string]string{ - "nginx.com/jwt-token": "$cookie_auth_token", - "nginx.com/jwt-key": "my-jwk", + "nginx.com/jwt-token": "true", }, specServices: map[string]bool{}, isPlus: true, @@ -1540,7 +1535,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: nil, - msg: "valid nginx.com/jwt-token annotation", + msg: "INvalid nginx.com/jwt-token annotation", }, { @@ -1560,7 +1555,6 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { { annotations: map[string]string{ "nginx.com/jwt-login-url": "https://login.example.com", - "nginx.com/jwt-key": "my-jwk", }, specServices: map[string]bool{}, isPlus: true, @@ -1626,6 +1620,34 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { }, msg: "invalid nginx.com/jwt-login-url annotation, hostname missing", }, + { + annotations: map[string]string{ + "nginx.com/jwt-login-url": "login.example.com", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.nginx.com/jwt-login-url: Invalid value: \"login.example.com\": scheme required, please use the prefix http(s)://", + }, + msg: "invalid nginx.com/jwt-login-url annotation, scheme missing", + }, + { + annotations: map[string]string{ + "nginx.com/jwt-login-url": "http:", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.nginx.com/jwt-login-url: Invalid value: \"http:\": hostname required", + }, + msg: "invalid nginx.com/jwt-login-url annotation, hostname missing", + }, { annotations: map[string]string{ From 5ab16f6fe230b43c258b79c2749cb50cfc8b00ef Mon Sep 17 00:00:00 2001 From: Haywood Shannon <5781935+haywoodsh@users.noreply.github.com> Date: Tue, 14 Jun 2022 15:43:07 +0100 Subject: [PATCH 3/7] Remove unused code --- internal/k8s/validation_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/k8s/validation_test.go b/internal/k8s/validation_test.go index 63dc893e7d..f8048eb734 100644 --- a/internal/k8s/validation_test.go +++ b/internal/k8s/validation_test.go @@ -2116,7 +2116,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { expectedErrors: []string{ `annotations.nginx.org/ssl-services: Invalid value: "service-1,service-2": must be a comma-separated list of services. The following services were not found: service-2`, }, - msg: "invalid nginx.org/ssl-se§rvices annotation, service does not exist", + msg: "invalid nginx.org/ssl-services annotation, service does not exist", }, { From 4ac26a636a5d28db68e3038020be5e66e2a5e450 Mon Sep 17 00:00:00 2001 From: Haywood Shannon <5781935+haywoodsh@users.noreply.github.com> Date: Thu, 16 Jun 2022 12:15:30 +0100 Subject: [PATCH 4/7] validate jwt token --- internal/k8s/validation.go | 6 ++++ internal/k8s/validation_test.go | 30 +++++++++++++++---- pkg/apis/configuration/validation/policy.go | 6 ++-- .../configuration/validation/policy_test.go | 8 ++--- 4 files changed, 37 insertions(+), 13 deletions(-) diff --git a/internal/k8s/validation.go b/internal/k8s/validation.go index 1c2e2bec5e..424bbdbca9 100644 --- a/internal/k8s/validation.go +++ b/internal/k8s/validation.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/nginxinc/kubernetes-ingress/internal/configs" + configvalidation "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/validation" networking "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" @@ -214,6 +215,7 @@ var ( }, jwtTokenAnnotation: { validatePlusOnlyAnnotation, + validateJWTTokenAnnotation, }, jwtLoginURLAnnotation: { validatePlusOnlyAnnotation, @@ -288,6 +290,10 @@ var ( annotationNames = sortedAnnotationNames(annotationValidations) ) +func validateJWTTokenAnnotation(context *annotationValidationContext) field.ErrorList { + return configvalidation.ValidateJWTToken(context.value, context.fieldPath) +} + func validateJWTLoginURLAnnotation(context *annotationValidationContext) field.ErrorList { allErrs := field.ErrorList{} diff --git a/internal/k8s/validation_test.go b/internal/k8s/validation_test.go index f8048eb734..0aa054b2c4 100644 --- a/internal/k8s/validation_test.go +++ b/internal/k8s/validation_test.go @@ -1515,27 +1515,45 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { }, { annotations: map[string]string{ - "nginx.com/jwt-token": "true", + "nginx.com/jwt-token": "$cookie", }, specServices: map[string]bool{}, isPlus: true, appProtectEnabled: false, appProtectDosEnabled: false, internalRoutesEnabled: false, - expectedErrors: nil, - msg: "Invalid nginx.com/jwt-token annotation", + expectedErrors: []string{ + "annotations.nginx.com/jwt-token: Invalid value: \"$cookie\": must only have special vars", + }, + msg: "invalid nginx.com/jwt-token annotation", }, { annotations: map[string]string{ - "nginx.com/jwt-token": "true", + "nginx.com/jwt-token": "$cookie_", }, specServices: map[string]bool{}, isPlus: true, appProtectEnabled: false, appProtectDosEnabled: false, internalRoutesEnabled: false, - expectedErrors: nil, - msg: "INvalid nginx.com/jwt-token annotation", + expectedErrors: []string{ + "annotations.nginx.com/jwt-token: Invalid value: \"cookie_\": a valid cookie name must consist of alphanumeric characters or '_' (e.g. 'my_cookie_123', regex used for validation is '[_A-Za-z0-9]+')", + }, + msg: "invalid nginx.com/jwt-token annotation", + }, + { + annotations: map[string]string{ + "nginx.com/jwt-token": "$cookie_{", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.nginx.com/jwt-token: Invalid value: \"cookie_{\": a valid cookie name must consist of alphanumeric characters or '_' (e.g. 'my_cookie_123', regex used for validation is '[_A-Za-z0-9]+')", + }, + msg: "invalid nginx.com/jwt-token annotation", }, { diff --git a/pkg/apis/configuration/validation/policy.go b/pkg/apis/configuration/validation/policy.go index 600dc2189e..e1d8fecac7 100644 --- a/pkg/apis/configuration/validation/policy.go +++ b/pkg/apis/configuration/validation/policy.go @@ -155,7 +155,7 @@ func validateJWT(jwt *v1.JWTAuth, fieldPath *field.Path) field.ErrorList { } allErrs = append(allErrs, validateSecretName(jwt.Secret, fieldPath.Child("secret"))...) - allErrs = append(allErrs, validateJWTToken(jwt.Token, fieldPath.Child("token"))...) + allErrs = append(allErrs, ValidateJWTToken(jwt.Token, fieldPath.Child("token"))...) return allErrs } @@ -450,7 +450,7 @@ func validateRateLimitKey(key string, fieldPath *field.Path, isPlus bool) field. var jwtTokenSpecialVariables = []string{"arg_", "http_", "cookie_"} -func validateJWTToken(token string, fieldPath *field.Path) field.ErrorList { +func ValidateJWTToken(token string, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if token == "" { @@ -472,7 +472,7 @@ func validateJWTToken(token string, fieldPath *field.Path) field.ErrorList { } if special { - // validateJWTToken is called only when NGINX Plus is running + // ValidateJWTToken is called only when NGINX Plus is running isPlus := true allErrs = append(allErrs, validateSpecialVariable(nVar, fieldPath, isPlus)...) } else { diff --git a/pkg/apis/configuration/validation/policy_test.go b/pkg/apis/configuration/validation/policy_test.go index 47b063a76a..8e556a3e0a 100644 --- a/pkg/apis/configuration/validation/policy_test.go +++ b/pkg/apis/configuration/validation/policy_test.go @@ -640,9 +640,9 @@ func TestValidateJWTToken(t *testing.T) { }, } for _, test := range validTests { - allErrs := validateJWTToken(test.token, field.NewPath("token")) + allErrs := ValidateJWTToken(test.token, field.NewPath("token")) if len(allErrs) != 0 { - t.Errorf("validateJWTToken(%v) returned an error for valid input for the case of %v", test.token, test.msg) + t.Errorf("ValidateJWTToken(%v) returned an error for valid input for the case of %v", test.token, test.msg) } } @@ -672,9 +672,9 @@ func TestValidateJWTToken(t *testing.T) { }, } for _, test := range invalidTests { - allErrs := validateJWTToken(test.token, field.NewPath("token")) + allErrs := ValidateJWTToken(test.token, field.NewPath("token")) if len(allErrs) == 0 { - t.Errorf("validateJWTToken(%v) didn't return error for invalid input for the case of %v", test.token, test.msg) + t.Errorf("ValidateJWTToken(%v) didn't return error for invalid input for the case of %v", test.token, test.msg) } } } From 17bdf8d4332d4ce0ad49ddfa5d7e6d2bc4eb71b7 Mon Sep 17 00:00:00 2001 From: Haywood Shannon <5781935+haywoodsh@users.noreply.github.com> Date: Fri, 17 Jun 2022 18:20:04 +0100 Subject: [PATCH 5/7] validate jwt token with regex --- internal/k8s/validation.go | 18 ++++++++++++++++-- internal/k8s/validation_test.go | 31 ++++++++++++++++++++++--------- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/internal/k8s/validation.go b/internal/k8s/validation.go index 424bbdbca9..52d31e7ae3 100644 --- a/internal/k8s/validation.go +++ b/internal/k8s/validation.go @@ -9,7 +9,6 @@ import ( "strings" "github.com/nginxinc/kubernetes-ingress/internal/configs" - configvalidation "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/validation" networking "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" @@ -68,6 +67,7 @@ const ( commaDelimiter = "," annotationValueFmt = `([^"$\\]|\\[^$])*` annotationValueFmtErrMsg = `a valid annotation value must have all '"' escaped and must not contain any '$' or end with an unescaped '\'` + jwtTokenValueFmtErrMsg = `a valid JWT token variable must have all '"' escaped and must not end with an unescaped '\'` ) type annotationValidationContext struct { @@ -291,7 +291,21 @@ var ( ) func validateJWTTokenAnnotation(context *annotationValidationContext) field.ErrorList { - return configvalidation.ValidateJWTToken(context.value, context.fieldPath) + allErrs := field.ErrorList{} + + nginxVars := strings.Split(context.value, "$") + if len(nginxVars) != 2 { + return append(allErrs, field.Invalid(context.fieldPath, context.value, "must have 1 var")) + } + nVar := context.value[1:] + + if !validAnnotationValueRegex.MatchString(nVar) { + msg := validation.RegexError(jwtTokenValueFmtErrMsg, annotationValueFmt, "$http_token", "$cookie_auth_token") + allErrs = append(allErrs, field.Invalid(context.fieldPath, context.value, msg)) + } + + return allErrs + } func validateJWTLoginURLAnnotation(context *annotationValidationContext) field.ErrorList { diff --git a/internal/k8s/validation_test.go b/internal/k8s/validation_test.go index 0aa054b2c4..347a1676c8 100644 --- a/internal/k8s/validation_test.go +++ b/internal/k8s/validation_test.go @@ -1515,7 +1515,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { }, { annotations: map[string]string{ - "nginx.com/jwt-token": "$cookie", + "nginx.com/jwt-token": "cookie_auth_token", }, specServices: map[string]bool{}, isPlus: true, @@ -1523,13 +1523,26 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - "annotations.nginx.com/jwt-token: Invalid value: \"$cookie\": must only have special vars", + "annotations.nginx.com/jwt-token: Invalid value: \"cookie_auth_token\": must have 1 var", + }, msg: "invalid nginx.com/jwt-token annotation, '$' missing", + }, + { + annotations: map[string]string{ + "nginx.com/jwt-token": `$cookie_auth_token"`, + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.nginx.com/jwt-token: Invalid value: \"$cookie_auth_token\\\"\": a valid JWT token variable must have all '\"' escaped and must not end with an unescaped '\\' (e.g. '$http_token', or '$cookie_auth_token', regex used for validation is '([^\"$\\\\]|\\\\[^$])*')", }, - msg: "invalid nginx.com/jwt-token annotation", + msg: "invalid nginx.com/jwt-token annotation, containing unescaped '\"'", }, { annotations: map[string]string{ - "nginx.com/jwt-token": "$cookie_", + "nginx.com/jwt-token": `$cookie_auth_token\`, }, specServices: map[string]bool{}, isPlus: true, @@ -1537,13 +1550,13 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - "annotations.nginx.com/jwt-token: Invalid value: \"cookie_\": a valid cookie name must consist of alphanumeric characters or '_' (e.g. 'my_cookie_123', regex used for validation is '[_A-Za-z0-9]+')", + "annotations.nginx.com/jwt-token: Invalid value: \"$cookie_auth_token\\\\\": a valid JWT token variable must have all '\"' escaped and must not end with an unescaped '\\' (e.g. '$http_token', or '$cookie_auth_token', regex used for validation is '([^\"$\\\\]|\\\\[^$])*')", }, - msg: "invalid nginx.com/jwt-token annotation", + msg: "invalid nginx.com/jwt-token annotation, containing escape characters", }, { annotations: map[string]string{ - "nginx.com/jwt-token": "$cookie_{", + "nginx.com/jwt-token": "$cookie_auth$token", }, specServices: map[string]bool{}, isPlus: true, @@ -1551,9 +1564,9 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - "annotations.nginx.com/jwt-token: Invalid value: \"cookie_{\": a valid cookie name must consist of alphanumeric characters or '_' (e.g. 'my_cookie_123', regex used for validation is '[_A-Za-z0-9]+')", + "annotations.nginx.com/jwt-token: Invalid value: \"$cookie_auth$token\": must have 1 var", }, - msg: "invalid nginx.com/jwt-token annotation", + msg: "invalid nginx.com/jwt-token annotation, containing more than 1 variables", }, { From cc802ecd45ea0b9861ad5ee4e17d809e1f310379 Mon Sep 17 00:00:00 2001 From: Haywood Shannon <5781935+haywoodsh@users.noreply.github.com> Date: Fri, 17 Jun 2022 19:57:42 +0100 Subject: [PATCH 6/7] simplify regex --- internal/k8s/validation.go | 33 ++++++++++++++------------------- internal/k8s/validation_test.go | 26 ++++++++++++++++++++------ 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/internal/k8s/validation.go b/internal/k8s/validation.go index 52d31e7ae3..6a37e433f5 100644 --- a/internal/k8s/validation.go +++ b/internal/k8s/validation.go @@ -66,8 +66,9 @@ const ( const ( commaDelimiter = "," annotationValueFmt = `([^"$\\]|\\[^$])*` + jwtTokenValueFmt = "\\$" + annotationValueFmt annotationValueFmtErrMsg = `a valid annotation value must have all '"' escaped and must not contain any '$' or end with an unescaped '\'` - jwtTokenValueFmtErrMsg = `a valid JWT token variable must have all '"' escaped and must not end with an unescaped '\'` + jwtTokenValueFmtErrMsg = `a valid annotation value must start with '$', have all '"' escaped, and must not contain any '$' or end with an unescaped '\'` ) type annotationValidationContext struct { @@ -90,6 +91,7 @@ type ( ) var validAnnotationValueRegex = regexp.MustCompile("^" + annotationValueFmt + "$") +var validJWTTokenAnnotationValueRegex = regexp.MustCompile("^" + jwtTokenValueFmt + "$") var ( // annotationValidations defines the various validations which will be applied in order to each ingress annotation. @@ -290,24 +292,6 @@ var ( annotationNames = sortedAnnotationNames(annotationValidations) ) -func validateJWTTokenAnnotation(context *annotationValidationContext) field.ErrorList { - allErrs := field.ErrorList{} - - nginxVars := strings.Split(context.value, "$") - if len(nginxVars) != 2 { - return append(allErrs, field.Invalid(context.fieldPath, context.value, "must have 1 var")) - } - nVar := context.value[1:] - - if !validAnnotationValueRegex.MatchString(nVar) { - msg := validation.RegexError(jwtTokenValueFmtErrMsg, annotationValueFmt, "$http_token", "$cookie_auth_token") - allErrs = append(allErrs, field.Invalid(context.fieldPath, context.value, msg)) - } - - return allErrs - -} - func validateJWTLoginURLAnnotation(context *annotationValidationContext) field.ErrorList { allErrs := field.ErrorList{} @@ -351,6 +335,17 @@ func validateJWTRealm(context *annotationValidationContext) field.ErrorList { return allErrs } +func validateJWTTokenAnnotation(context *annotationValidationContext) field.ErrorList { + allErrs := field.ErrorList{} + + if !validJWTTokenAnnotationValueRegex.MatchString(context.value) { + msg := validation.RegexError(jwtTokenValueFmtErrMsg, jwtTokenValueFmt, "$http_token", "$cookie_auth_token") + allErrs = append(allErrs, field.Invalid(context.fieldPath, context.value, msg)) + } + + return allErrs +} + func validateHTTPHeadersAnnotation(context *annotationValidationContext) field.ErrorList { var allErrs field.ErrorList headers := strings.Split(context.value, commaDelimiter) diff --git a/internal/k8s/validation_test.go b/internal/k8s/validation_test.go index 347a1676c8..387cae4188 100644 --- a/internal/k8s/validation_test.go +++ b/internal/k8s/validation_test.go @@ -1523,7 +1523,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - "annotations.nginx.com/jwt-token: Invalid value: \"cookie_auth_token\": must have 1 var", + "annotations.nginx.com/jwt-token: Invalid value: \"cookie_auth_token\": a valid annotation value must start with '$', have all '\"' escaped, and must not contain any '$' or end with an unescaped '\\' (e.g. '$http_token', or '$cookie_auth_token', regex used for validation is '\\$([^\"$\\\\]|\\\\[^$])*')", }, msg: "invalid nginx.com/jwt-token annotation, '$' missing", }, { @@ -1536,7 +1536,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - "annotations.nginx.com/jwt-token: Invalid value: \"$cookie_auth_token\\\"\": a valid JWT token variable must have all '\"' escaped and must not end with an unescaped '\\' (e.g. '$http_token', or '$cookie_auth_token', regex used for validation is '([^\"$\\\\]|\\\\[^$])*')", + "annotations.nginx.com/jwt-token: Invalid value: \"$cookie_auth_token\\\"\": a valid annotation value must start with '$', have all '\"' escaped, and must not contain any '$' or end with an unescaped '\\' (e.g. '$http_token', or '$cookie_auth_token', regex used for validation is '\\$([^\"$\\\\]|\\\\[^$])*')", }, msg: "invalid nginx.com/jwt-token annotation, containing unescaped '\"'", }, @@ -1550,13 +1550,13 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - "annotations.nginx.com/jwt-token: Invalid value: \"$cookie_auth_token\\\\\": a valid JWT token variable must have all '\"' escaped and must not end with an unescaped '\\' (e.g. '$http_token', or '$cookie_auth_token', regex used for validation is '([^\"$\\\\]|\\\\[^$])*')", + "annotations.nginx.com/jwt-token: Invalid value: \"$cookie_auth_token\\\\\": a valid annotation value must start with '$', have all '\"' escaped, and must not contain any '$' or end with an unescaped '\\' (e.g. '$http_token', or '$cookie_auth_token', regex used for validation is '\\$([^\"$\\\\]|\\\\[^$])*')", }, msg: "invalid nginx.com/jwt-token annotation, containing escape characters", }, { annotations: map[string]string{ - "nginx.com/jwt-token": "$cookie_auth$token", + "nginx.com/jwt-token": "cookie_auth$token", }, specServices: map[string]bool{}, isPlus: true, @@ -1564,9 +1564,23 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - "annotations.nginx.com/jwt-token: Invalid value: \"$cookie_auth$token\": must have 1 var", + "annotations.nginx.com/jwt-token: Invalid value: \"cookie_auth$token\": a valid annotation value must start with '$', have all '\"' escaped, and must not contain any '$' or end with an unescaped '\\' (e.g. '$http_token', or '$cookie_auth_token', regex used for validation is '\\$([^\"$\\\\]|\\\\[^$])*')", }, - msg: "invalid nginx.com/jwt-token annotation, containing more than 1 variables", + msg: "invalid nginx.com/jwt-token annotation, containing incorrect variable", + }, + { + annotations: map[string]string{ + "nginx.com/jwt-token": "$cookie_auth_token$http_token", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.nginx.com/jwt-token: Invalid value: \"$cookie_auth_token$http_token\": a valid annotation value must start with '$', have all '\"' escaped, and must not contain any '$' or end with an unescaped '\\' (e.g. '$http_token', or '$cookie_auth_token', regex used for validation is '\\$([^\"$\\\\]|\\\\[^$])*')", + }, + msg: "invalid nginx.com/jwt-token annotation, containing more than 1 variable", }, { From 9d128a0172d5c60acceaca145e1f1ee66251029e Mon Sep 17 00:00:00 2001 From: Haywood Shannon <5781935+haywoodsh@users.noreply.github.com> Date: Fri, 17 Jun 2022 20:29:22 +0100 Subject: [PATCH 7/7] fix rebase and revert policy --- internal/k8s/validation_test.go | 42 ------------------- pkg/apis/configuration/validation/policy.go | 6 +-- .../configuration/validation/policy_test.go | 8 ++-- 3 files changed, 7 insertions(+), 49 deletions(-) diff --git a/internal/k8s/validation_test.go b/internal/k8s/validation_test.go index 387cae4188..b8899ef7a3 100644 --- a/internal/k8s/validation_test.go +++ b/internal/k8s/validation_test.go @@ -1473,20 +1473,6 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { msg: "invalid nginx.com/jwt-key annotation, containing '_", }, - { - annotations: map[string]string{ - "nginx.com/jwt-key": "my_jwk", - }, - specServices: map[string]bool{}, - isPlus: true, - appProtectEnabled: false, - appProtectDosEnabled: false, - internalRoutesEnabled: false, - expectedErrors: []string{ - "annotations.nginx.com/jwt-key: Invalid value: \"my_jwk\": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')", - }, - msg: "invalid nginx.com/jwt-key annotation, containing '_", - }, { annotations: map[string]string{ "nginx.com/jwt-token": "true", @@ -1665,34 +1651,6 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { }, msg: "invalid nginx.com/jwt-login-url annotation, hostname missing", }, - { - annotations: map[string]string{ - "nginx.com/jwt-login-url": "login.example.com", - }, - specServices: map[string]bool{}, - isPlus: true, - appProtectEnabled: false, - appProtectDosEnabled: false, - internalRoutesEnabled: false, - expectedErrors: []string{ - "annotations.nginx.com/jwt-login-url: Invalid value: \"login.example.com\": scheme required, please use the prefix http(s)://", - }, - msg: "invalid nginx.com/jwt-login-url annotation, scheme missing", - }, - { - annotations: map[string]string{ - "nginx.com/jwt-login-url": "http:", - }, - specServices: map[string]bool{}, - isPlus: true, - appProtectEnabled: false, - appProtectDosEnabled: false, - internalRoutesEnabled: false, - expectedErrors: []string{ - "annotations.nginx.com/jwt-login-url: Invalid value: \"http:\": hostname required", - }, - msg: "invalid nginx.com/jwt-login-url annotation, hostname missing", - }, { annotations: map[string]string{ diff --git a/pkg/apis/configuration/validation/policy.go b/pkg/apis/configuration/validation/policy.go index e1d8fecac7..600dc2189e 100644 --- a/pkg/apis/configuration/validation/policy.go +++ b/pkg/apis/configuration/validation/policy.go @@ -155,7 +155,7 @@ func validateJWT(jwt *v1.JWTAuth, fieldPath *field.Path) field.ErrorList { } allErrs = append(allErrs, validateSecretName(jwt.Secret, fieldPath.Child("secret"))...) - allErrs = append(allErrs, ValidateJWTToken(jwt.Token, fieldPath.Child("token"))...) + allErrs = append(allErrs, validateJWTToken(jwt.Token, fieldPath.Child("token"))...) return allErrs } @@ -450,7 +450,7 @@ func validateRateLimitKey(key string, fieldPath *field.Path, isPlus bool) field. var jwtTokenSpecialVariables = []string{"arg_", "http_", "cookie_"} -func ValidateJWTToken(token string, fieldPath *field.Path) field.ErrorList { +func validateJWTToken(token string, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if token == "" { @@ -472,7 +472,7 @@ func ValidateJWTToken(token string, fieldPath *field.Path) field.ErrorList { } if special { - // ValidateJWTToken is called only when NGINX Plus is running + // validateJWTToken is called only when NGINX Plus is running isPlus := true allErrs = append(allErrs, validateSpecialVariable(nVar, fieldPath, isPlus)...) } else { diff --git a/pkg/apis/configuration/validation/policy_test.go b/pkg/apis/configuration/validation/policy_test.go index 8e556a3e0a..47b063a76a 100644 --- a/pkg/apis/configuration/validation/policy_test.go +++ b/pkg/apis/configuration/validation/policy_test.go @@ -640,9 +640,9 @@ func TestValidateJWTToken(t *testing.T) { }, } for _, test := range validTests { - allErrs := ValidateJWTToken(test.token, field.NewPath("token")) + allErrs := validateJWTToken(test.token, field.NewPath("token")) if len(allErrs) != 0 { - t.Errorf("ValidateJWTToken(%v) returned an error for valid input for the case of %v", test.token, test.msg) + t.Errorf("validateJWTToken(%v) returned an error for valid input for the case of %v", test.token, test.msg) } } @@ -672,9 +672,9 @@ func TestValidateJWTToken(t *testing.T) { }, } for _, test := range invalidTests { - allErrs := ValidateJWTToken(test.token, field.NewPath("token")) + allErrs := validateJWTToken(test.token, field.NewPath("token")) if len(allErrs) == 0 { - t.Errorf("ValidateJWTToken(%v) didn't return error for invalid input for the case of %v", test.token, test.msg) + t.Errorf("validateJWTToken(%v) didn't return error for invalid input for the case of %v", test.token, test.msg) } } }