From b9e944a8a6ed53646abcdfb8e6f2276571a150ab Mon Sep 17 00:00:00 2001 From: Manuel Alejandro de Brito Fontes Date: Tue, 4 Feb 2020 14:04:11 -0300 Subject: [PATCH] Move mod-security logic from template to go code (#5009) --- .../ingress/controller/template/template.go | 41 ++++++++++++ .../controller/template/template_test.go | 65 ++++++++++++++++++- rootfs/etc/nginx/template/nginx.tmpl | 20 +----- 3 files changed, 106 insertions(+), 20 deletions(-) diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index a7f562b558..f4683a8b05 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -179,6 +179,7 @@ var ( "buildHTTPSListener": buildHTTPSListener, "buildOpentracingForLocation": buildOpentracingForLocation, "shouldLoadOpentracingModule": shouldLoadOpentracingModule, + "buildModSecurityForLocation": buildModSecurityForLocation, } ) @@ -1336,3 +1337,43 @@ func shouldLoadOpentracingModule(c interface{}, s interface{}) bool { return false } + +func buildModSecurityForLocation(cfg config.Configuration, location *ingress.Location) string { + isMSEnabledInLoc := location.ModSecurity.Enable + isMSEnabled := cfg.EnableModsecurity + + if !isMSEnabled && !isMSEnabledInLoc { + return "" + } + + if !isMSEnabledInLoc { + return "" + } + + var buffer bytes.Buffer + + 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; +`) + } + + if location.ModSecurity.Snippet != "" { + buffer.WriteString(fmt.Sprintf(`modsecurity_rules ' +%v +'; +`, location.ModSecurity.Snippet)) + } + + if location.ModSecurity.TransactionID != "" { + buffer.WriteString(fmt.Sprintf(`modsecurity_transaction_id "%v"; +`, location.ModSecurity.TransactionID)) + } + + return buffer.String() +} diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index c134196cb8..f641960ce1 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -29,7 +29,6 @@ import ( "testing" jsoniter "github.com/json-iterator/go" - apiv1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -1369,3 +1368,67 @@ func TestShouldLoadOpentracingModule(t *testing.T) { t.Errorf("Expected '%v' but returned '%v'", expected, actual) } } + +func TestModSecurityForLocation(t *testing.T) { + loadModule := `modsecurity on; +modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf; +` + + modsecRule := `modsecurity_rules ' +#RULE# +'; +` + owaspRules := `modsecurity_rules_file /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf; +` + + transactionID := "0000000" + transactionCfg := fmt.Sprintf(`modsecurity_transaction_id "%v"; +`, transactionID) + + testRule := "#RULE#" + + testCases := []struct { + description string + isEnabledInCM bool + isOwaspEnabledInCM bool + isEnabledInLoc bool + isOwaspEnabledInLoc bool + snippet string + transactionID string + expected string + }{ + {"configmap enabled, configmap OWASP disabled, without annotation, snippet or transaction ID", true, false, false, false, "", "", ""}, + {"configmap enabled, configmap OWASP disabled, without annotation, snippet and with transaction ID", true, false, false, false, "", transactionID, ""}, + {"configmap enabled, configmap OWASP enabled, without annotation, OWASP enabled", true, true, false, false, "", "", ""}, + {"configmap enabled, configmap OWASP enabled, without annotation, OWASP disabled, with snippet and no transaction ID", true, true, true, false, testRule, "", modsecRule}, + {"configmap enabled, configmap OWASP enabled, without annotation, OWASP disabled, with snippet and transaction ID", true, true, true, false, testRule, transactionID, fmt.Sprintf("%v%v", modsecRule, transactionCfg)}, + {"configmap enabled, with annotation, OWASP disabled", true, false, true, false, "", "", ""}, + {"configmap enabled, configmap OWASP disabled, with annotation, OWASP enabled, no snippet and no transaction ID", true, false, true, true, "", "", owaspRules}, + {"configmap enabled, configmap OWASP disabled, with annotation, OWASP enabled, with snippet and no transaction ID", true, false, true, true, "", "", owaspRules}, + {"configmap enabled, configmap OWASP disabled, with annotation, OWASP enabled, with snippet and transaction ID", true, false, true, true, "", transactionID, fmt.Sprintf("%v%v", owaspRules, transactionCfg)}, + {"configmap enabled, OWASP configmap enabled, with annotation, OWASP disabled", true, true, true, false, "", "", ""}, + {"configmap disabled, with annotation, OWASP disabled", false, false, true, false, "", "", loadModule}, + {"configmap disabled, with annotation, OWASP disabled", false, false, true, false, testRule, "", fmt.Sprintf("%v%v", loadModule, modsecRule)}, + {"configmap disabled, with annotation, OWASP enabled", false, false, true, false, testRule, "", fmt.Sprintf("%v%v", loadModule, modsecRule)}, + } + + for _, testCase := range testCases { + il := &ingress.Location{ + ModSecurity: modsecurity.Config{ + Enable: testCase.isEnabledInLoc, + OWASPRules: testCase.isOwaspEnabledInLoc, + Snippet: testCase.snippet, + TransactionID: testCase.transactionID, + }, + } + + actual := buildModSecurityForLocation(config.Configuration{ + EnableModsecurity: testCase.isEnabledInCM, + EnableOWASPCoreRules: testCase.isOwaspEnabledInCM, + }, il) + + if testCase.expected != actual { + t.Errorf("%v: expected '%v' but returned '%v'", testCase.description, testCase.expected, actual) + } + } +} diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 2ccdaa9216..357ac5425f 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -1029,25 +1029,7 @@ stream { set $proxy_alternative_upstream_name ""; - {{ if (or $location.ModSecurity.Enable $all.Cfg.EnableModsecurity) }} - {{ if not $all.Cfg.EnableModsecurity }} - modsecurity on; - - modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf; - {{ end }} - - {{ if $location.ModSecurity.Snippet }} - modsecurity_rules ' - {{ $location.ModSecurity.Snippet }} - '; - {{ else if (and (not $all.Cfg.EnableOWASPCoreRules) ($location.ModSecurity.OWASPRules))}} - modsecurity_rules_file /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf; - {{ end }} - - {{ if (not (empty $location.ModSecurity.TransactionID)) }} - modsecurity_transaction_id {{ $location.ModSecurity.TransactionID | quote }}; - {{ end }} - {{ end }} + {{ buildModSecurityForLocation $all.Cfg $location }} {{ if isLocationAllowed $location }} {{ if gt (len $location.Whitelist.CIDR) 0 }}