chore: refactors per authority directives. (#188)

corazawaf · May 18, 2023 · 8c429fb · 8c429fb
1 parent 30eeff0
commit 8c429fb
Show file tree

Hide file tree

Showing 8 changed files with 171 additions and 171 deletions.
diff --git a/README.md b/README.md
@@ -66,7 +66,7 @@ In order to run the coraza-proxy-wasm we need to spin up an envoy configuration
                                     "SecRule REQUEST_URI \"@streq /admin\" \"id:101,phase:1,t:lowercase,deny\""
                                 ]
                             },
-                            "default_directive": "default",
+                            "default_directives": "default",
                         }
                     vm_config:
                         runtime: "envoy.wasm.runtime.v8"
@@ -95,7 +95,7 @@ configuration:
                 "Include @owasp_crs/*.conf"
             ]
         },
-        "default_directive": "default",
+        "default_directives": "default",
     }
 ```
 
@@ -114,7 +114,7 @@ configuration:
                 "Include @owasp_crs/REQUEST-901-INITIALIZATION.conf"
             ]
         },
-        "default_directive": "default",
+        "default_directives": "default",
     }
 ```
 

diff --git a/example/envoy-config.yaml b/example/envoy-config.yaml
@@ -75,7 +75,7 @@ static_resources:
                                     "SecRule REQUEST_URI \"@streq /example\" \"id:101,phase:1,t:lowercase,deny\" \nSecRule REQUEST_BODY \"@rx maliciouspayload\" \"id:102,phase:2,t:lowercase,deny\" \nSecRule RESPONSE_HEADERS::status \"@rx 406\" \"id:103,phase:3,t:lowercase,deny\" \nSecRule RESPONSE_BODY \"@contains responsebodycode\" \"id:104,phase:4,t:lowercase,deny\""
                                   ]
                               },
-                              "default_directive": "rs1",
+                              "default_directives": "rs1",
                               "metric_labels": {
                                 "owner": "coraza",
                                 "identifier": "global"

diff --git a/ftw/envoy-config.yaml b/ftw/envoy-config.yaml
@@ -44,7 +44,7 @@ static_resources:
                                     "Include @owasp_crs/*.conf"
                                   ]
                               },
-                              "default_directive": "default",
+                              "default_directives": "default",
                               "metric_labels": {},
                               "per_authority_directives": {}
                             }

diff --git a/main_test.go b/main_test.go
@@ -421,9 +421,9 @@ func TestLifecycle(t *testing.T) {
 			tt := tc
 
 			t.Run(tt.name, func(t *testing.T) {
-				conf := `{"directives_map": {"default": []}, "default_directive": "default"}`
+				conf := `{"directives_map": {"default": []}, "default_directives": "default"}`
 				if inlineRules := strings.TrimSpace(tt.inlineRules); inlineRules != "" {
-					conf = fmt.Sprintf(`{"directives_map": {"default": ["%s"]}, "default_directive": "default"}`, inlineRules)
+					conf = fmt.Sprintf(`{"directives_map": {"default": ["%s"]}, "default_directives": "default"}`, inlineRules)
 				}
 				opt := proxytest.
 					NewEmulatorOption().
@@ -587,7 +587,7 @@ func TestBadRequest(t *testing.T) {
 		for _, tc := range tests {
 			tt := tc
 			t.Run(tt.name, func(t *testing.T) {
-				conf := `{"directives_map": {"default": []}, "default_directive": "default"}`
+				conf := `{"directives_map": {"default": []}, "default_directives": "default"}`
 				opt := proxytest.
 					NewEmulatorOption().
 					WithVMContext(vm).
@@ -636,7 +636,7 @@ func TestBadResponse(t *testing.T) {
 		for _, tc := range tests {
 			tt := tc
 			t.Run(tt.name, func(t *testing.T) {
-				conf := `{"directives_map": {"default": []}, "default_directive": "default"}`
+				conf := `{"directives_map": {"default": []}, "default_directives": "default"}`
 				opt := proxytest.
 					NewEmulatorOption().
 					WithVMContext(vm).
@@ -666,7 +666,7 @@ func TestEmptyBody(t *testing.T) {
 		opt := proxytest.
 			NewEmulatorOption().
 			WithVMContext(vm).
-			WithPluginConfiguration([]byte(`{"directives_map": {"default": [ "SecRequestBodyAccess On", "SecResponseBodyAccess On" ]}, "default_directive": "default"}`))
+			WithPluginConfiguration([]byte(`{"directives_map": {"default": [ "SecRequestBodyAccess On", "SecResponseBodyAccess On" ]}, "default_directives": "default"}`))
 
 		host, reset := proxytest.NewHostEmulator(opt)
 		defer reset()
@@ -761,7 +761,7 @@ func TestLogError(t *testing.T) {
 		for _, tc := range tests {
 			tt := tc
 			t.Run(fmt.Sprintf("severity %d", tt.severity), func(t *testing.T) {
-				conf := fmt.Sprintf(`{"directives_map": {"default": ["SecRule REQUEST_HEADERS:X-CRS-Test \"@rx ^.*$\" \"id:999999,phase:1,log,severity:%d,msg:'%%{MATCHED_VAR}',pass,t:none\""]}, "default_directive": "default"}`, tt.severity)
+				conf := fmt.Sprintf(`{"directives_map": {"default": ["SecRule REQUEST_HEADERS:X-CRS-Test \"@rx ^.*$\" \"id:999999,phase:1,log,severity:%d,msg:'%%{MATCHED_VAR}',pass,t:none\""]}, "default_directives": "default"}`, tt.severity)
 
 				opt := proxytest.
 					NewEmulatorOption().
@@ -789,7 +789,7 @@ func TestParseCRS(t *testing.T) {
 		opt := proxytest.
 			NewEmulatorOption().
 			WithVMContext(vm).
-			WithPluginConfiguration([]byte(`{"directives_map": {"default": [ "Include @ftw-conf", "Include @recommended-conf", "Include @crs-setup-conf", "Include @owasp_crs/*.conf" ]}, "default_directive": "default"}`))
+			WithPluginConfiguration([]byte(`{"directives_map": {"default": [ "Include @ftw-conf", "Include @recommended-conf", "Include @crs-setup-conf", "Include @owasp_crs/*.conf" ]}, "default_directives": "default"}`))
 
 		host, reset := proxytest.NewHostEmulator(opt)
 		defer reset()
@@ -859,7 +859,7 @@ SecRuleEngine On\nSecRule REQUEST_URI \"@streq /hello\" \"id:101,phase:4,t:lower
 
 			t.Run(tt.name, func(t *testing.T) {
 				conf := fmt.Sprintf(`
-					{"directives_map": {"default": ["%s"]}, "default_directive": "default"}
+					{"directives_map": {"default": ["%s"]}, "default_directives": "default"}
 				`, strings.TrimSpace(tt.rules))
 				opt := proxytest.
 					NewEmulatorOption().
@@ -967,7 +967,7 @@ func TestRetrieveAddressInfo(t *testing.T) {
 
 				conf := `{}`
 				if inlineRules := strings.TrimSpace(inlineRules); inlineRules != "" {
-					conf = fmt.Sprintf(`{"directives_map": {"default": ["%s"]}, "default_directive": "default"}`, inlineRules)
+					conf = fmt.Sprintf(`{"directives_map": {"default": ["%s"]}, "default_directives": "default"}`, inlineRules)
 				}
 				t.Run(tt.name, func(t *testing.T) {
 					opt := proxytest.
@@ -1030,7 +1030,7 @@ func TestParseServerName(t *testing.T) {
 
 			conf := `{}`
 			if inlineRules := strings.TrimSpace(inlineRules); inlineRules != "" {
-				conf = fmt.Sprintf(`{"directives_map": {"default": ["%s"]}, "default_directive": "default"}`, inlineRules)
+				conf = fmt.Sprintf(`{"directives_map": {"default": ["%s"]}, "default_directives": "default"}`, inlineRules)
 			}
 			t.Run(name, func(t *testing.T) {
 				opt := proxytest.

diff --git a/wasmplugin/config.go b/wasmplugin/config.go
@@ -14,13 +14,13 @@ import (
 type pluginConfiguration struct {
 	directivesMap          DirectivesMap
 	metricLabels           map[string]string
-	defaultDirective       string
+	defaultDirectives      string
 	perAuthorityDirectives map[string]string
 }
 
 type DirectivesMap map[string][]string
 
-func parsePluginConfiguration(data []byte) (pluginConfiguration, error) {
+func parsePluginConfiguration(data []byte, infoLogger func(string)) (pluginConfiguration, error) {
 	config := pluginConfiguration{}
 
 	data = bytes.TrimSpace(data)
@@ -56,14 +56,14 @@ func parsePluginConfiguration(data []byte) (pluginConfiguration, error) {
 		return true
 	})
 
-	defaultDirective := jsonData.Get("default_directive")
-	if defaultDirective.Exists() {
-		defaultDirectiveName := defaultDirective.String()
-		if _, ok := config.directivesMap[defaultDirectiveName]; !ok {
-			return config, fmt.Errorf("directive map not found for default directive: %q", defaultDirectiveName)
+	defaultDirectives := jsonData.Get("default_directives")
+	if defaultDirectives.Exists() {
+		defaultDirectivesName := defaultDirectives.String()
+		if _, ok := config.directivesMap[defaultDirectivesName]; !ok {
+			return config, fmt.Errorf("directive map not found for default directive: %q", defaultDirectivesName)
 		}
 
-		config.defaultDirective = defaultDirectiveName
+		config.defaultDirectives = defaultDirectivesName
 	}
 
 	config.perAuthorityDirectives = make(map[string]string)
@@ -82,7 +82,9 @@ func parsePluginConfiguration(data []byte) (pluginConfiguration, error) {
 		rules := jsonData.Get("rules")
 
 		if rules.Exists() {
-			config.defaultDirective = "default"
+			infoLogger("Defaulting to deprecated 'rules' field")
+
+			config.defaultDirectives = "default"
 
 			var directive []string
 			rules.ForEach(func(_, value gjson.Result) bool {

diff --git a/wasmplugin/config_test.go b/wasmplugin/config_test.go
@@ -7,7 +7,9 @@ import (
 	"errors"
 	"testing"
 
+	"github.com/corazawaf/coraza/v3"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestParsePluginConfiguration(t *testing.T) {
@@ -26,7 +28,7 @@ func TestParsePluginConfiguration(t *testing.T) {
 			expectConfig: pluginConfiguration{
 				directivesMap:          DirectivesMap{},
 				metricLabels:           map[string]string{},
-				defaultDirective:       "",
+				defaultDirectives:      "",
 				perAuthorityDirectives: map[string]string{},
 			},
 		},
@@ -42,15 +44,15 @@ func TestParsePluginConfiguration(t *testing.T) {
 				"directives_map": {
 					"default": ["SecRuleEngine On"]
 				},
-				"default_directive": "default"
+				"default_directives": "default"
 			}
 			`,
 			expectConfig: pluginConfiguration{
 				directivesMap: DirectivesMap{
 					"default": []string{"SecRuleEngine On"},
 				},
 				metricLabels:           map[string]string{},
-				defaultDirective:       "default",
+				defaultDirectives:      "default",
 				perAuthorityDirectives: map[string]string{},
 			},
 		},
@@ -61,15 +63,15 @@ func TestParsePluginConfiguration(t *testing.T) {
 				"directives_map": {
 					"default": ["SecRuleEngine On", "Include @owasp_crs/*.conf\nSecRule REQUEST_URI \"@streq /admin\" \"id:101,phase:1,t:lowercase,deny\""]
 				},
-				"default_directive": "default"
+				"default_directives": "default"
 			}
 			`,
 			expectConfig: pluginConfiguration{
 				directivesMap: DirectivesMap{
 					"default": []string{"SecRuleEngine On", "Include @owasp_crs/*.conf\nSecRule REQUEST_URI \"@streq /admin\" \"id:101,phase:1,t:lowercase,deny\""},
 				},
 				metricLabels:           map[string]string{},
-				defaultDirective:       "default",
+				defaultDirectives:      "default",
 				perAuthorityDirectives: map[string]string{},
 			},
 		},
@@ -80,7 +82,7 @@ func TestParsePluginConfiguration(t *testing.T) {
 				"directives_map": {
 					"default": ["SecRuleEngine On", "Include @owasp_crs/*.conf\nSecRule REQUEST_URI \"@streq /admin\" \"id:101,phase:1,t:lowercase,deny\""]
 				},
-				"default_directive": "default",
+				"default_directives": "default",
 				"metric_labels": {"owner": "coraza","identifier": "global"}
 			}
 			`,
@@ -92,7 +94,7 @@ func TestParsePluginConfiguration(t *testing.T) {
 					"owner":      "coraza",
 					"identifier": "global",
 				},
-				defaultDirective:       "default",
+				defaultDirectives:      "default",
 				perAuthorityDirectives: map[string]string{},
 			},
 		},
@@ -105,7 +107,7 @@ func TestParsePluginConfiguration(t *testing.T) {
 					"custom-01": ["SecRuleEngine On"],
 					"custom-02": ["SecRuleEngine On"]
 				},
-				"default_directive": "default",
+				"default_directives": "default",
 				"metric_labels": {"owner": "coraza","identifier": "global"}
 			}
 			`,
@@ -119,7 +121,7 @@ func TestParsePluginConfiguration(t *testing.T) {
 					"owner":      "coraza",
 					"identifier": "global",
 				},
-				defaultDirective:       "default",
+				defaultDirectives:      "default",
 				perAuthorityDirectives: map[string]string{},
 			},
 		},
@@ -132,7 +134,7 @@ func TestParsePluginConfiguration(t *testing.T) {
 					"custom-01": ["SecRuleEngine On"],
 					"custom-02": ["SecRuleEngine On"]
 				},
-				"default_directive": "default",
+				"default_directives": "default",
 				"metric_labels": {"owner": "coraza","identifier": "global"},
 				"per_authority_directives": {
 					"mydomain.com":"custom-01",
@@ -150,7 +152,7 @@ func TestParsePluginConfiguration(t *testing.T) {
 					"owner":      "coraza",
 					"identifier": "global",
 				},
-				defaultDirective: "default",
+				defaultDirectives: "default",
 				perAuthorityDirectives: map[string]string{
 					"mydomain.com":  "custom-01",
 					"mydomain2.com": "custom-02",
@@ -164,7 +166,7 @@ func TestParsePluginConfiguration(t *testing.T) {
 				"directives_map": {
 					"default": ["SecRuleEngine On", "Include @owasp_crs/*.conf\nSecRule REQUEST_URI \"@streq /admin\" \"id:101,phase:1,t:lowercase,deny\""]
 				},
-				"default_directive": "foo"
+				"default_directives": "foo"
 			}
 			`,
 			expectErr: errors.New("directive map not found for default directive: \"foo\""),
@@ -178,7 +180,7 @@ func TestParsePluginConfiguration(t *testing.T) {
 					"custom-01": ["SecRuleEngine On"],
 					"custom-02": ["SecRuleEngine On"]
 				},
-				"default_directive": "default",
+				"default_directives": "default",
 				"metric_labels": {"owner": "coraza","identifier": "global"},
 				"per_authority_directives": {
 					"mydomain.com":"custom-01",
@@ -199,7 +201,7 @@ func TestParsePluginConfiguration(t *testing.T) {
 				directivesMap: DirectivesMap{
 					"default": []string{"SecRuleEngine On", "Include @owasp_crs/*.conf\nSecRule REQUEST_URI \"@streq /admin\" \"id:101,phase:1,t:lowercase,deny\""},
 				},
-				defaultDirective:       "default",
+				defaultDirectives:      "default",
 				metricLabels:           map[string]string{},
 				perAuthorityDirectives: map[string]string{},
 			},
@@ -212,14 +214,14 @@ func TestParsePluginConfiguration(t *testing.T) {
 				"directives_map": {
 					"foo": ["SecRuleEngine On", "Include @owasp_crs/*.conf\nSecRule REQUEST_URI \"@streq /directives\" \"id:101,phase:1,t:lowercase,deny\""]
 				},
-				"default_directive": "foo"
+				"default_directives": "foo"
 			}
 			`,
 			expectConfig: pluginConfiguration{
 				directivesMap: DirectivesMap{
 					"foo": []string{"SecRuleEngine On", "Include @owasp_crs/*.conf\nSecRule REQUEST_URI \"@streq /directives\" \"id:101,phase:1,t:lowercase,deny\""},
 				},
-				defaultDirective:       "foo",
+				defaultDirectives:      "foo",
 				metricLabels:           map[string]string{},
 				perAuthorityDirectives: map[string]string{},
 			},
@@ -228,15 +230,50 @@ func TestParsePluginConfiguration(t *testing.T) {
 
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
-			cfg, err := parsePluginConfiguration([]byte(testCase.config))
+			cfg, err := parsePluginConfiguration([]byte(testCase.config), func(string) {})
 			assert.Equal(t, testCase.expectErr, err)
 
 			if testCase.expectErr == nil {
 				assert.Equal(t, testCase.expectConfig.directivesMap, cfg.directivesMap)
 				assert.Equal(t, testCase.expectConfig.metricLabels, cfg.metricLabels)
-				assert.Equal(t, testCase.expectConfig.defaultDirective, cfg.defaultDirective)
+				assert.Equal(t, testCase.expectConfig.defaultDirectives, cfg.defaultDirectives)
 				assert.Equal(t, testCase.expectConfig.perAuthorityDirectives, cfg.perAuthorityDirectives)
 			}
 		})
 	}
 }
+
+func TestWAFMap(t *testing.T) {
+	w, _ := coraza.NewWAF(coraza.NewWAFConfig())
+
+	wm := newWAFMap(1)
+	err := wm.put("foo", w)
+	require.NoError(t, err)
+
+	t.Run("set unexisting default key", func(t *testing.T) {
+		err = wm.setDefaultKey("bar")
+		require.Error(t, err)
+	})
+
+	t.Run("get unexisting WAF with no default", func(t *testing.T) {
+		_, _, err := wm.getWAFOrDefault("bar")
+		require.Error(t, err)
+	})
+
+	err = wm.setDefaultKey("foo")
+	require.NoError(t, err)
+
+	t.Run("get existing WAF", func(t *testing.T) {
+		expecteWAF, isDefault, err := wm.getWAFOrDefault("foo")
+		require.NotNil(t, expecteWAF)
+		require.False(t, isDefault)
+		require.NoError(t, err)
+	})
+
+	t.Run("get unexisting WAF", func(t *testing.T) {
+		expecteWAF, isDefault, err := wm.getWAFOrDefault("bar")
+		require.NotNil(t, expecteWAF)
+		require.True(t, isDefault)
+		require.NoError(t, err)
+	})
+}