Add handling of mutiple log destinations

nginx · Mar 11, 2022 · e93d010 · e93d010
1 parent 8791d41
commit e93d010
Show file tree

Hide file tree

Showing 16 changed files with 434 additions and 39 deletions.
diff --git a/deployments/common/crds/k8s.nginx.org_policies.yaml b/deployments/common/crds/k8s.nginx.org_policies.yaml
@@ -156,6 +156,18 @@ spec:
                           type: boolean
                         logDest:
                           type: string
+                    securityLogs:
+                      type: array
+                      items:
+                        description: SecurityLog defines the security log of a WAF policy.
+                        type: object
+                        properties:
+                          apLogConf:
+                            type: string
+                          enable:
+                            type: boolean
+                          logDest:
+                            type: string
             status:
               description: PolicyStatus is the status of the policy resource
               type: object

diff --git a/deployments/helm-chart/crds/k8s.nginx.org_policies.yaml b/deployments/helm-chart/crds/k8s.nginx.org_policies.yaml
@@ -156,6 +156,18 @@ spec:
                           type: boolean
                         logDest:
                           type: string
+                    securityLogs:
+                      type: array
+                      items:
+                        description: SecurityLog defines the security log of a WAF policy.
+                        type: object
+                        properties:
+                          apLogConf:
+                            type: string
+                          enable:
+                            type: boolean
+                          logDest:
+                            type: string
             status:
               description: PolicyStatus is the status of the policy resource
               type: object

diff --git a/docs/content/configuration/policy-resource.md b/docs/content/configuration/policy-resource.md
@@ -362,17 +362,20 @@ For `kubectl get` and similar commands, you can also use the short name `pol` in
 
 The WAF policy configures NGINX Plus to secure client requests using App Protect policies.
 
-For example, the following policy will enable the referenced APPolicy and APLogConf with the configured log destination:
+For example, the following policy will enable the referenced APPolicy. You can configure multiple APLogConfs with log destinations:
 ```yaml
 waf:
   enable: true
   apPolicy: "default/dataguard-alarm"
-  securityLog:
-    enable: true
+  securityLogs:
+  - enable: true
     apLogConf: "default/logconf"
     logDest: "syslog:server=syslog-svc.default:514"
+  - enable: true
+    apLogConf: "default/logconf"
+    logDest: "syslog:server=syslog-svc-secondary.default:514"    
 ```
-
+> Note: The field `waf.securityLog` is supported but will be ignored if `waf.securityLogs` is populated.
 > Note: The feature is implemented using the NGINX Plus [NGINX App Protect Module](https://docs.nginx.com/nginx-app-protect/configuration/).
 
 {{% table %}}

diff --git a/examples/custom-resources/waf/waf.yaml b/examples/custom-resources/waf/waf.yaml
@@ -6,7 +6,7 @@ spec:
   waf:
     enable: true
     apPolicy: "default/dataguard-alarm"
-    securityLog:
-      enable: true
+    securityLogs:
+    - enable: true
       apLogConf: "default/logconf"
       logDest: "syslog:server=syslog-svc.default:514"
diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go
@@ -123,7 +123,7 @@ type WAF struct {
 	Enable              string
 	ApPolicy            string
 	ApSecurityLogEnable bool
-	ApLogConf           string
+	ApLogConf           []string
 }
 
 // Dos defines Dos configuration.

diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl
@@ -189,7 +189,9 @@ server {
 
         {{ if .ApSecurityLogEnable }}
     app_protect_security_log_enable on;
-    app_protect_security_log {{ .ApLogConf }};
+        {{ range $logconf := .ApLogConf }}
+    app_protect_security_log {{ $logconf }};
+        {{ end }}
         {{ end }}
     {{ end }}
 

diff --git a/internal/configs/version2/templates_test.go b/internal/configs/version2/templates_test.go
@@ -150,7 +150,7 @@ var virtualServerCfg = VirtualServerConfig{
 		WAF: &WAF{
 			ApPolicy:            "/etc/nginx/waf/nac-policies/default-dataguard-alarm",
 			ApSecurityLogEnable: true,
-			ApLogConf:           "/etc/nginx/waf/nac-logconfs/default-logconf",
+			ApLogConf:           []string{"/etc/nginx/waf/nac-logconfs/default-logconf"},
 		},
 		Snippets: []string{"# server snippet"},
 		InternalRedirectLocations: []InternalRedirectLocation{

diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go
@@ -1041,7 +1041,7 @@ func (p *policiesCfg) addWAFConfig(
 		}
 	}
 
-	if waf.SecurityLog != nil {
+	if waf.SecurityLog != nil && waf.SecurityLogs == nil {
 		p.WAF.ApSecurityLogEnable = true
 
 		logConfKey := waf.SecurityLog.ApLogConf
@@ -1052,13 +1052,31 @@ func (p *policiesCfg) addWAFConfig(
 
 		if logConfPath, ok := apResources.LogConfs[logConfKey]; ok {
 			logDest := generateString(waf.SecurityLog.LogDest, "syslog:server=localhost:514")
-			p.WAF.ApLogConf = fmt.Sprintf("%s %s", logConfPath, logDest)
+			p.WAF.ApLogConf = []string{fmt.Sprintf("%s %s", logConfPath, logDest)}
 		} else {
 			res.addWarningf("WAF policy %s references an invalid or non-existing log config %s", polKey, logConfKey)
 			res.isError = true
 		}
 	}
 
+	if waf.SecurityLogs != nil {
+		p.WAF.ApSecurityLogEnable = true
+		p.WAF.ApLogConf = []string{}
+		for _, loco := range waf.SecurityLogs {
+			logConfKey := loco.ApLogConf
+			hasNamepace := strings.Contains(logConfKey, "/")
+			if !hasNamepace {
+				logConfKey = fmt.Sprintf("%v/%v", polNamespace, logConfKey)
+			}
+			if logConfPath, ok := apResources.LogConfs[logConfKey]; ok {
+				logDest := generateString(loco.LogDest, "syslog:server=localhost:514")
+				p.WAF.ApLogConf = append(p.WAF.ApLogConf, fmt.Sprintf("%s %s", logConfPath, logDest))
+			} else {
+				res.addWarningf("WAF policy %s references an invalid or non-existing log config %s", polKey, logConfKey)
+				res.isError = true
+			}
+		}
+	}
 	return res
 }
 

diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go
@@ -2924,7 +2924,7 @@ func TestGeneratePolicies(t *testing.T) {
 					Enable:              "on",
 					ApPolicy:            "/etc/nginx/waf/nac-policies/default-dataguard-alarm",
 					ApSecurityLogEnable: true,
-					ApLogConf:           "/etc/nginx/waf/nac-logconfs/default-logconf syslog:server=127.0.0.1:514",
+					ApLogConf:           []string{"/etc/nginx/waf/nac-logconfs/default-logconf syslog:server=127.0.0.1:514"},
 				},
 			},
 			msg: "WAF reference",
@@ -6673,7 +6673,6 @@ func TestGenerateHealthCheck(t *testing.T) {
 		msg          string
 	}{
 		{
-
 			upstream: conf_v1.Upstream{
 				HealthCheck: &conf_v1.HealthCheck{
 					Enable:         true,
@@ -6854,7 +6853,6 @@ func TestGenerateGrpcHealthCheck(t *testing.T) {
 		msg          string
 	}{
 		{
-
 			upstream: conf_v1.Upstream{
 				HealthCheck: &conf_v1.HealthCheck{
 					Enable:         true,
@@ -8174,7 +8172,6 @@ func TestAddWafConfig(t *testing.T) {
 		msg          string
 	}{
 		{
-
 			wafInput: &conf_v1.WAF{
 				Enable: true,
 			},
@@ -8191,7 +8188,6 @@ func TestAddWafConfig(t *testing.T) {
 			msg:      "valid waf config, default App Protect config",
 		},
 		{
-
 			wafInput: &conf_v1.WAF{
 				Enable:   true,
 				ApPolicy: "dataguard-alarm",
@@ -8214,13 +8210,42 @@ func TestAddWafConfig(t *testing.T) {
 			wafConfig: &version2.WAF{
 				ApPolicy:            "/etc/nginx/waf/nac-policies/default-dataguard-alarm",
 				ApSecurityLogEnable: true,
-				ApLogConf:           "/etc/nginx/waf/nac-logconfs/default-logconf",
+				ApLogConf:           []string{"/etc/nginx/waf/nac-logconfs/default-logconf"},
+			},
+			expected: &validationResults{isError: false},
+			msg:      "valid waf config",
+		},
+		{
+			wafInput: &conf_v1.WAF{
+				Enable:   true,
+				ApPolicy: "dataguard-alarm",
+				SecurityLogs: []*conf_v1.SecurityLog{
+					{
+						Enable:    true,
+						ApLogConf: "logconf",
+						LogDest:   "syslog:server=127.0.0.1:514",
+					},
+				},
+			},
+			polKey:       "default/waf-policy",
+			polNamespace: "default",
+			apResources: &appProtectResourcesForVS{
+				Policies: map[string]string{
+					"default/dataguard-alarm": "/etc/nginx/waf/nac-policies/default-dataguard-alarm",
+				},
+				LogConfs: map[string]string{
+					"default/logconf": "/etc/nginx/waf/nac-logconfs/default-logconf",
+				},
+			},
+			wafConfig: &version2.WAF{
+				ApPolicy:            "/etc/nginx/waf/nac-policies/default-dataguard-alarm",
+				ApSecurityLogEnable: true,
+				ApLogConf:           []string{"/etc/nginx/waf/nac-logconfs/default-logconf"},
 			},
 			expected: &validationResults{isError: false},
 			msg:      "valid waf config",
 		},
 		{
-
 			wafInput: &conf_v1.WAF{
 				Enable:   true,
 				ApPolicy: "default/dataguard-alarm",
@@ -8241,7 +8266,7 @@ func TestAddWafConfig(t *testing.T) {
 			wafConfig: &version2.WAF{
 				ApPolicy:            "/etc/nginx/waf/nac-policies/default-dataguard-alarm",
 				ApSecurityLogEnable: true,
-				ApLogConf:           "/etc/nginx/waf/nac-logconfs/default-logconf",
+				ApLogConf:           []string{"/etc/nginx/waf/nac-logconfs/default-logconf"},
 			},
 			expected: &validationResults{
 				isError: true,
@@ -8252,7 +8277,6 @@ func TestAddWafConfig(t *testing.T) {
 			msg: "invalid waf config, apLogConf references non-existing log conf",
 		},
 		{
-
 			wafInput: &conf_v1.WAF{
 				Enable:   true,
 				ApPolicy: "default/dataguard-alarm",
@@ -8272,7 +8296,7 @@ func TestAddWafConfig(t *testing.T) {
 			wafConfig: &version2.WAF{
 				ApPolicy:            "/etc/nginx/waf/nac-policies/default-dataguard-alarm",
 				ApSecurityLogEnable: true,
-				ApLogConf:           "/etc/nginx/waf/nac-logconfs/default-logconf",
+				ApLogConf:           []string{"/etc/nginx/waf/nac-logconfs/default-logconf"},
 			},
 			expected: &validationResults{
 				isError: true,
@@ -8283,7 +8307,6 @@ func TestAddWafConfig(t *testing.T) {
 			msg: "invalid waf config, apLogConf references non-existing ap conf",
 		},
 		{
-
 			wafInput: &conf_v1.WAF{
 				Enable:   true,
 				ApPolicy: "ns1/dataguard-alarm",
@@ -8306,13 +8329,12 @@ func TestAddWafConfig(t *testing.T) {
 			wafConfig: &version2.WAF{
 				ApPolicy:            "/etc/nginx/waf/nac-policies/ns1-dataguard-alarm",
 				ApSecurityLogEnable: true,
-				ApLogConf:           "/etc/nginx/waf/nac-logconfs/ns2-logconf",
+				ApLogConf:           []string{"/etc/nginx/waf/nac-logconfs/ns2-logconf"},
 			},
 			expected: &validationResults{},
 			msg:      "valid waf config, cross ns reference",
 		},
 		{
-
 			wafInput: &conf_v1.WAF{
 				Enable:   false,
 				ApPolicy: "dataguard-alarm",

diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go
@@ -2786,19 +2786,37 @@ func (lbc *LoadBalancerController) addWAFPolicyRefs(
 			apPolRef[apPolKey] = apPolicy
 		}
 
-		if pol.Spec.WAF.SecurityLog != nil && pol.Spec.WAF.SecurityLog.ApLogConf != "" {
-			logConfKey := pol.Spec.WAF.SecurityLog.ApLogConf
-			if !strings.Contains(pol.Spec.WAF.SecurityLog.ApLogConf, "/") {
-				logConfKey = fmt.Sprintf("%v/%v", pol.Namespace, logConfKey)
-			}
+		if pol.Spec.WAF.SecurityLog != nil && pol.Spec.WAF.SecurityLogs == nil {
+			if pol.Spec.WAF.SecurityLog.ApLogConf != "" {
+				logConfKey := pol.Spec.WAF.SecurityLog.ApLogConf
+				if !strings.Contains(pol.Spec.WAF.SecurityLog.ApLogConf, "/") {
+					logConfKey = fmt.Sprintf("%v/%v", pol.Namespace, logConfKey)
+				}
 
-			logConf, err := lbc.appProtectConfiguration.GetAppResource(appprotect.LogConfGVK.Kind, logConfKey)
-			if err != nil {
-				return fmt.Errorf("WAF policy %q is invalid: %w", logConfKey, err)
+				logConf, err := lbc.appProtectConfiguration.GetAppResource(appprotect.LogConfGVK.Kind, logConfKey)
+				if err != nil {
+					return fmt.Errorf("WAF policy %q is invalid: %w", logConfKey, err)
+				}
+				logConfRef[logConfKey] = logConf
 			}
-			logConfRef[logConfKey] = logConf
 		}
 
+		if pol.Spec.WAF.SecurityLogs != nil {
+			for _, SecLog := range pol.Spec.WAF.SecurityLogs {
+				if SecLog.ApLogConf != "" {
+					logConfKey := SecLog.ApLogConf
+					if !strings.Contains(SecLog.ApLogConf, "/") {
+						logConfKey = fmt.Sprintf("%v/%v", pol.Namespace, logConfKey)
+					}
+
+					logConf, err := lbc.appProtectConfiguration.GetAppResource(appprotect.LogConfGVK.Kind, logConfKey)
+					if err != nil {
+						return fmt.Errorf("WAF policy %q is invalid: %w", logConfKey, err)
+					}
+					logConfRef[logConfKey] = logConf
+				}
+			}
+		}
 	}
 	return nil
 }
@@ -2846,6 +2864,13 @@ func getWAFPoliciesForAppProtectLogConf(pols []*conf_v1.Policy, key string) []*c
 		if pol.Spec.WAF != nil && pol.Spec.WAF.SecurityLog != nil && isMatchingResourceRef(pol.Namespace, pol.Spec.WAF.SecurityLog.ApLogConf, key) {
 			policies = append(policies, pol)
 		}
+		if pol.Spec.WAF != nil && pol.Spec.WAF.SecurityLogs != nil {
+			for _, logConf := range pol.Spec.WAF.SecurityLogs {
+				if isMatchingResourceRef(pol.Namespace, logConf.ApLogConf, key) {
+					policies = append(policies, pol)
+				}
+			}
+		}
 	}
 
 	return policies