Merge pull request #3400 from diazjf/more-modsecurity

Add Snippet for ModSecurity

docs/user-guide/nginx-configuration/annotations.md

@@ -98,7 +98,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
 |[nginx.ingress.kubernetes.io/enable-modsecurity](#modsecurity)|bool|
 |[nginx.ingress.kubernetes.io/enable-owasp-core-rules](#modsecurity)|bool|
 |[nginx.ingress.kubernetes.io/modsecurity-transaction-id](#modsecurity)|string|
-
+|[nginx.ingress.kubernetes.io/modsecurity-snippet](#modsecurity)|string|
 
 ### Canary
 
@@ -649,6 +649,7 @@ It can be enabled using the following annotation:
 ```yaml
 nginx.ingress.kubernetes.io/enable-modsecurity: "true"
 ```
+ModSecurity will run in "Detection-Only" mode using the [recommended configuration](https://github.com/SpiderLabs/ModSecurity/blob/v3/master/modsecurity.conf-recommended).
 
 You can enable the [OWASP Core Rule Set](https://www.modsecurity.org/CRS/Documentation/) by
 setting the following annotation:
@@ -661,6 +662,23 @@ You can pass transactionIDs from nginx by setting up the following:
 nginx.ingress.kubernetes.io/modsecurity-transaction-id: "$request_id"
 ```
 
+You can also add your own set of modsecurity rules via a snippet:
+```yaml
+nginx.ingress.kubernetes.io/modsecurity-snippet: |
+SecRuleEngine On
+SecDebugLog /tmp/modsec_debug.log
+```
+
+Note: If you use both `enable-owasp-core-rules` and `modsecurity-snippet` annotations together, only the
+`modsecurity-snippet` will take effect. If you wish to include the [OWASP Core Rule Set](https://www.modsecurity.org/CRS/Documentation/) or 
+[recommended configuration](https://github.com/SpiderLabs/ModSecurity/blob/v3/master/modsecurity.conf-recommended) simply use the include
+statement:
+```yaml
+nginx.ingress.kubernetes.io/modsecurity-snippet: |
+Include /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf
+Include /etc/nginx/modsecurity/modsecurity.conf
+```
+
 ### InfluxDB
 
 Using `influxdb-*` annotations we can monitor requests passing through a Location by sending them to an InfluxDB backend exposing the UDP socket

internal/ingress/annotations/modsecurity/main.go

@@ -18,17 +18,16 @@ package modsecurity
 
 import (
 	extensions "k8s.io/api/extensions/v1beta1"
-
 	"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
 	"k8s.io/ingress-nginx/internal/ingress/resolver"
 )
 
-// Config contains the AuthSSLCert used for mutual authentication
+// Config contains ModSecurity Configuration items
-// and the configured ValidationDepth
 type Config struct {
 	Enable        bool   `json:"enable-modsecurity"`
 	OWASPRules    bool   `json:"enable-owasp-core-rules"`
 	TransactionID string `json:"modsecurity-transaction-id"`
+	Snippet       string `json:"modsecurity-snippet"`
 }
 
 // Equal tests for equality between two Config types
@@ -48,6 +47,9 @@ func (modsec1 *Config) Equal(modsec2 *Config) bool {
 	if modsec1.TransactionID != modsec2.TransactionID {
 		return false
 	}
+	if modsec1.Snippet != modsec2.Snippet {
+		return false
+	}
 
 	return true
 }
@@ -80,9 +82,15 @@ func (a modSecurity) Parse(ing *extensions.Ingress) (interface{}, error) {
 		transactionID = ""
 	}
 
+	snippet, err := parser.GetStringAnnotation("modsecurity-snippet", ing)
+	if err != nil {
+		snippet = ""
+	}
+
 	return Config{
 		Enable:        enableModSecurity,
 		OWASPRules:    owaspRules,
 		TransactionID: transactionID,
+		Snippet:       snippet,
 	}, nil
 }

internal/ingress/annotations/modsecurity/main_test.go

@@ -30,6 +30,7 @@ func TestParse(t *testing.T) {
 	enable := parser.GetAnnotationWithPrefix("enable-modsecurity")
 	owasp := parser.GetAnnotationWithPrefix("enable-owasp-core-rules")
 	transID := parser.GetAnnotationWithPrefix("modsecurity-transaction-id")
+	snippet := parser.GetAnnotationWithPrefix("modsecurity-snippet")
 
 	ap := NewParser(&resolver.Mock{})
 	if ap == nil {
@@ -40,19 +41,22 @@ func TestParse(t *testing.T) {
 		annotations map[string]string
 		expected    Config
 	}{
-		{map[string]string{enable: "true"}, Config{true, false, ""}},
+		{map[string]string{enable: "true"}, Config{true, false, "", ""}},
-		{map[string]string{enable: "false"}, Config{false, false, ""}},
+		{map[string]string{enable: "false"}, Config{false, false, "", ""}},
-		{map[string]string{enable: ""}, Config{false, false, ""}},
+		{map[string]string{enable: ""}, Config{false, false, "", ""}},
 
-		{map[string]string{owasp: "true"}, Config{false, true, ""}},
+		{map[string]string{owasp: "true"}, Config{false, true, "", ""}},
-		{map[string]string{owasp: "false"}, Config{false, false, ""}},
+		{map[string]string{owasp: "false"}, Config{false, false, "", ""}},
-		{map[string]string{owasp: ""}, Config{false, false, ""}},
+		{map[string]string{owasp: ""}, Config{false, false, "", ""}},
 
-		{map[string]string{transID: "ok"}, Config{false, false, "ok"}},
+		{map[string]string{transID: "ok"}, Config{false, false, "ok", ""}},
-		{map[string]string{transID: ""}, Config{false, false, ""}},
+		{map[string]string{transID: ""}, Config{false, false, "", ""}},
 
-		{map[string]string{}, Config{false, false, ""}},
+		{map[string]string{snippet: "ModSecurity Rule"}, Config{false, false, "", "ModSecurity Rule"}},
-		{nil, Config{false, false, ""}},
+		{map[string]string{snippet: ""}, Config{false, false, "", ""}},
+
+		{map[string]string{}, Config{false, false, "", ""}},
+		{nil, Config{false, false, "", ""}},
 	}
 
 	ing := &extensions.Ingress{

rootfs/etc/nginx/template/nginx.tmpl

@@ -1024,9 +1024,14 @@ stream {
             {{ if (or $location.ModSecurity.Enable $all.Cfg.EnableModsecurity) }}
             modsecurity on;
 
-            modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;
+            {{ if $location.ModSecurity.Snippet }}
-            {{ if (or $location.ModSecurity.OWASPRules $all.Cfg.EnableOWASPCoreRules) }}
+            modsecurity_rules '
+                {{ $location.ModSecurity.Snippet }}
+            ';
+            {{ else if (or $location.ModSecurity.OWASPRules $all.Cfg.EnableOWASPCoreRules) }}
             modsecurity_rules_file /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf;
+            {{ else }}
+            modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;
             {{ end }}
 
             {{ if (not (empty $location.ModSecurity.TransactionID)) }}

test/e2e/annotations/modsecurity.go

@@ -31,7 +31,6 @@ var _ = framework.IngressNginxDescribe("Annotations - ModSecurityLocation", func
 	})
 
 	AfterEach(func() {
-		f.UpdateNginxConfigMapData("enable-modsecurity", "false")
 	})
 
 	It("should enable modsecurity", func() {
@@ -42,8 +41,6 @@ var _ = framework.IngressNginxDescribe("Annotations - ModSecurityLocation", func
 			"nginx.ingress.kubernetes.io/enable-modsecurity": "true",
 		}
 
-		f.UpdateNginxConfigMapData("enable-modsecurity", "true")
-
 		ing := framework.NewSingleIngress(host, "/", host, nameSpace, "http-svc", 80, &annotations)
 		f.EnsureIngress(ing)
 
@@ -64,8 +61,6 @@ var _ = framework.IngressNginxDescribe("Annotations - ModSecurityLocation", func
 			"nginx.ingress.kubernetes.io/modsecurity-transaction-id": "modsecurity-$request_id",
 		}
 
-		f.UpdateNginxConfigMapData("enable-modsecurity", "true")
-
 		ing := framework.NewSingleIngress(host, "/", host, nameSpace, "http-svc", 80, &annotations)
 		f.EnsureIngress(ing)
 
@@ -85,8 +80,6 @@ var _ = framework.IngressNginxDescribe("Annotations - ModSecurityLocation", func
 			"nginx.ingress.kubernetes.io/enable-modsecurity": "false",
 		}
 
-		f.UpdateNginxConfigMapData("enable-modsecurity", "false")
-
 		ing := framework.NewSingleIngress(host, "/", host, nameSpace, "http-svc", 80, &annotations)
 		f.EnsureIngress(ing)
 
@@ -95,4 +88,23 @@ var _ = framework.IngressNginxDescribe("Annotations - ModSecurityLocation", func
 				return !strings.Contains(server, "modsecurity on;")
 			})
 	})
+
+	It("should enable modsecurity with snippet", func() {
+		host := "modsecurity.foo.com"
+		nameSpace := f.IngressController.Namespace
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/enable-modsecurity":  "true",
+			"nginx.ingress.kubernetes.io/modsecurity-snippet": "SecRuleEngine On",
+		}
+
+		ing := framework.NewSingleIngress(host, "/", host, nameSpace, "http-svc", 80, &annotations)
+		f.EnsureIngress(ing)
+
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return strings.Contains(server, "modsecurity on;") &&
+					strings.Contains(server, "SecRuleEngine On")
+			})
+	})
 })