diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index fc093c4afb..d58ef17cd6 100755 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -30,6 +30,7 @@ The following table shows a configuration option's name, type, and the default v |[add-headers](#add-headers)|string|""| |[allow-backend-server-header](#allow-backend-server-header)|bool|"false"| |[allow-snippet-annotations](#allow-snippet-annotations)|bool|true| +|[annotation-value-word-blocklist](#annotation-value-word-blocklist)|string array|"load_module","lua_package","_by_lua","location","root","proxy_pass","serviceaccount","{","}","'","\" |[hide-headers](#hide-headers)|string array|empty| |[access-log-params](#access-log-params)|string|""| |[access-log-path](#access-log-path)|string|"/var/log/nginx/access.log"| @@ -222,6 +223,23 @@ Enables Ingress to parse and add *-snippet annotations/directives created by the Warning: We recommend enabling this option only if you TRUST users with permission to create Ingress objects, as this may allow a user to add restricted configurations to the final nginx.conf file +## annotation-value-word-blocklist + +Contains a comma-separated value of chars/words that are well known of being used to abuse Ingress configuration +and must be blocked. + +When an annotation is detected with a value that matches one of the blocked badwords, the whole Ingress wont be configured. + +_**default:**_ `"load_module,lua_package,_by_lua,location,root,proxy_pass,serviceaccount,{,},',\"` + + +Warning: The default value already contains a sane set of badwords. Some features like mod_security needs characters that are blocked, and it's up to the Ingress admin to remove this characters from the blocklist. + +When doing this, the default blocklist is overrided, which means that the Ingress admin should add all the words +that should be blocked. + +If you find some word should not be on the default list, or if you think that we should add more badwords, please +feel free to open an issue with your case! ## hide-headers Sets additional header that will not be passed from the upstream server to the client response. diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index b1dbf9cd1e..fb06a58c6e 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -253,7 +253,7 @@ func (n *NGINXController) CheckIngress(ing *networking.Ingress) error { } if strings.HasPrefix(key, fmt.Sprintf("%s/", parser.AnnotationsPrefix)) { for _, forbiddenvalue := range arraybadWords { - if strings.Contains(value, forbiddenvalue) { + if strings.Contains(value, strings.TrimSpace(forbiddenvalue)) { return fmt.Errorf("%s annotation contains invalid word %s", key, forbiddenvalue) } } diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go index 15367bc0d3..b67929b972 100644 --- a/internal/ingress/controller/controller_test.go +++ b/internal/ingress/controller/controller_test.go @@ -284,6 +284,10 @@ func TestCheckIngress(t *testing.T) { if err := nginx.CheckIngress(ing); err == nil { t.Errorf("with an invalid value in annotation the ingress should be rejected") } + ing.ObjectMeta.Annotations["nginx.ingress.kubernetes.io/custom-headers"] = "another_directive" + if err := nginx.CheckIngress(ing); err == nil { + t.Errorf("with an invalid value in annotation the ingress should be rejected") + } }) t.Run("When a new catch-all ingress is being created despite catch-alls being disabled ", func(t *testing.T) {