From 5390ce4879a7989ad9b34285e5137705d6ab652f Mon Sep 17 00:00:00 2001 From: Manuel Alejandro de Brito Fontes Date: Wed, 1 Apr 2020 22:04:36 -0300 Subject: [PATCH] Fix definition order of modsecurity directives --- .../ingress/controller/template/template.go | 16 +++-- .../controller/template/template_test.go | 12 ++-- test/e2e/annotations/modsecurity.go | 68 +++++++++++++++++++ 3 files changed, 85 insertions(+), 11 deletions(-) diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index a423b86a29..51a9fec1d0 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -1360,12 +1360,6 @@ func buildModSecurityForLocation(cfg config.Configuration, location *ingress.Loc if !isMSEnabled { buffer.WriteString(`modsecurity on; -modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf; -`) - } - - if !cfg.EnableOWASPCoreRules && location.ModSecurity.OWASPRules { - buffer.WriteString(`modsecurity_rules_file /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf; `) } @@ -1381,6 +1375,16 @@ modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf; `, location.ModSecurity.TransactionID)) } + if !isMSEnabled { + buffer.WriteString(`modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf; +`) + } + + if !cfg.EnableOWASPCoreRules && location.ModSecurity.OWASPRules { + buffer.WriteString(`modsecurity_rules_file /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf; +`) + } + return buffer.String() } diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index 782a54d4a7..7a8599f679 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -1373,7 +1373,9 @@ func TestShouldLoadOpentracingModule(t *testing.T) { func TestModSecurityForLocation(t *testing.T) { loadModule := `modsecurity on; -modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf; +` + + modSecCfg := `modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf; ` modsecOff := "modsecurity off;" @@ -1411,12 +1413,12 @@ modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf; {"configmap enabled, configmap OWASP disabled, annotation enabled, OWASP enabled, no snippet and no transaction ID", true, false, true, true, true, "", "", owaspRules}, {"configmap enabled, configmap OWASP disabled, annotation disabled, OWASP disabled, no snippet and no transaction ID", true, false, false, true, false, "", "", modsecOff}, {"configmap enabled, configmap OWASP disabled, annotation enabled, OWASP enabled, with snippet and no transaction ID", true, false, true, true, true, "", "", owaspRules}, - {"configmap enabled, configmap OWASP disabled, annotation enabled, OWASP enabled, with snippet and transaction ID", true, false, true, true, true, "", transactionID, fmt.Sprintf("%v%v", owaspRules, transactionCfg)}, + {"configmap enabled, configmap OWASP disabled, annotation enabled, OWASP enabled, with snippet and transaction ID", true, false, true, true, true, "", transactionID, fmt.Sprintf("%v%v", transactionCfg, owaspRules)}, {"configmap enabled, configmap OWASP enabled, annotation enabled, OWASP disabled", true, true, true, true, false, "", "", ""}, - {"configmap disabled, annotation enabled, OWASP disabled", false, false, true, true, false, "", "", loadModule}, + {"configmap disabled, annotation enabled, OWASP disabled", false, false, true, true, false, "", "", fmt.Sprintf("%v%v", loadModule, modSecCfg)}, {"configmap disabled, annotation disabled, OWASP disabled", false, false, false, true, false, "", "", ""}, - {"configmap disabled, annotation enabled, OWASP disabled", false, false, true, true, false, testRule, "", fmt.Sprintf("%v%v", loadModule, modsecRule)}, - {"configmap disabled, annotation enabled, OWASP enabled", false, false, true, true, false, testRule, "", fmt.Sprintf("%v%v", loadModule, modsecRule)}, + {"configmap disabled, annotation enabled, OWASP disabled", false, false, true, true, false, testRule, "", fmt.Sprintf("%v%v%v", loadModule, modsecRule, modSecCfg)}, + {"configmap disabled, annotation enabled, OWASP enabled", false, false, true, true, false, testRule, "", fmt.Sprintf("%v%v%v", loadModule, modsecRule, modSecCfg)}, } for _, testCase := range testCases { diff --git a/test/e2e/annotations/modsecurity.go b/test/e2e/annotations/modsecurity.go index a3682243f2..1f5a2f607e 100644 --- a/test/e2e/annotations/modsecurity.go +++ b/test/e2e/annotations/modsecurity.go @@ -17,6 +17,7 @@ limitations under the License. package annotations import ( + "net/http" "strings" "github.com/onsi/ginkgo" @@ -148,4 +149,71 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() { }) }) + ginkgo.It("should enable modsecurity with snippet and block requests", func() { + host := "modsecurity.foo.com" + nameSpace := f.Namespace + + snippet := `SecRuleEngine On + SecRequestBodyAccess On + SecAuditEngine RelevantOnly + SecAuditLogParts ABIJDEFHZ + SecAuditLog /dev/stdout + SecAuditLogType Serial + SecRule REQUEST_HEADERS:User-Agent \"block-ua\" \"log,deny,id:107,status:403,msg:\'UA blocked\'\"` + + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/enable-modsecurity": "true", + "nginx.ingress.kubernetes.io/modsecurity-snippet": snippet, + } + + ing := framework.NewSingleIngress(host, "/", host, nameSpace, framework.EchoService, 80, annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "modsecurity on;") && + strings.Contains(server, "SecRuleEngine On") + }) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("User-Agent", "block-ua"). + Expect(). + Status(http.StatusForbidden) + }) + + ginkgo.It("should enable modsecurity globally and with modsecurity-snippet block requests", func() { + host := "modsecurity.foo.com" + nameSpace := f.Namespace + + snippet := `SecRuleEngine On + SecRequestBodyAccess On + SecAuditEngine RelevantOnly + SecAuditLogParts ABIJDEFHZ + SecAuditLog /dev/stdout + SecAuditLogType Serial + SecRule REQUEST_HEADERS:User-Agent \"block-ua\" \"log,deny,id:107,status:403,msg:\'UA blocked\'\"` + + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/modsecurity-snippet": snippet, + } + + ing := framework.NewSingleIngress(host, "/", host, nameSpace, framework.EchoService, 80, annotations) + f.EnsureIngress(ing) + + f.UpdateNginxConfigMapData("enable-modsecurity", "true") + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "SecRuleEngine On") + }) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("User-Agent", "block-ua"). + Expect(). + Status(http.StatusForbidden) + }) })