Skip to content

Commit

Permalink
Trim spaces from badword items (kubernetes#7921)
Browse files Browse the repository at this point in the history
  • Loading branch information
rikatz committed Nov 16, 2021
1 parent f9350d5 commit ec20b86
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 1 deletion.
18 changes: 18 additions & 0 deletions docs/user-guide/nginx-configuration/configmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"|
Expand Down Expand Up @@ -217,6 +218,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.
Expand Down
2 changes: 1 addition & 1 deletion internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,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)
}
}
Expand Down
4 changes: 4 additions & 0 deletions internal/ingress/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,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) {
Expand Down

0 comments on commit ec20b86

Please sign in to comment.