From 963752974343389416f6281b7c3a6736398c3063 Mon Sep 17 00:00:00 2001 From: Aleksei Zakharov Date: Wed, 9 Apr 2025 17:17:36 +0300 Subject: [PATCH 1/3] Add RuleFilter for dynamic rule skipping Introduces the `types.RuleFilter` interface to allow custom, per-transaction logic for skipping rules based on their metadata. Adds `Transaction.UseRuleFilter` to apply a filter instance and integrates the filter check at the beginning of the rule evaluation loop in `RuleGroup.Eval`. Includes associated unit tests. --- internal/corazawaf/rulegroup.go | 13 +++- internal/corazawaf/rulegroup_test.go | 92 ++++++++++++++++++++++++++++ internal/corazawaf/transaction.go | 11 ++++ internal/corazawaf/waf.go | 1 + types/rules.go | 9 +++ types/transaction.go | 5 ++ 6 files changed, 129 insertions(+), 2 deletions(-) diff --git a/internal/corazawaf/rulegroup.go b/internal/corazawaf/rulegroup.go index c03da9afa..d82d9bc04 100644 --- a/internal/corazawaf/rulegroup.go +++ b/internal/corazawaf/rulegroup.go @@ -133,6 +133,16 @@ func (rg *RuleGroup) Eval(phase types.RulePhase, tx *Transaction) bool { RulesLoop: for i := range rg.rules { r := &rg.rules[i] + // Check if a specific rule filter is applied to this transaction + // and if the current rule should be ignored according to the filter. + if tx.ruleFilter != nil { + if tx.ruleFilter.ShouldIgnore(r) { + tx.DebugLogger().Debug(). + Int("rule_id", r.ID_). + Msg("Skipping rule due to RulesFilter") + continue RulesLoop + } + } // if there is already an interruption and the phase isn't logging // we break the loop if tx.interruption != nil && phase != types.PhaseLogging { @@ -159,8 +169,7 @@ RulesLoop: if trb == r.ID_ { tx.DebugLogger().Debug(). Int("rule_id", r.ID_). - Msg("Skipping rule") - + Msg("Skipping rule due to ruleRemoveByID") continue RulesLoop } } diff --git a/internal/corazawaf/rulegroup_test.go b/internal/corazawaf/rulegroup_test.go index f4bdfa063..822b01251 100644 --- a/internal/corazawaf/rulegroup_test.go +++ b/internal/corazawaf/rulegroup_test.go @@ -4,9 +4,11 @@ package corazawaf import ( + "fmt" "testing" "github.com/corazawaf/coraza/v3/experimental/plugins/macro" + "github.com/corazawaf/coraza/v3/types" ) func newTestRule(id int) *Rule { @@ -85,3 +87,93 @@ func TestRuleGroupDeleteByID(t *testing.T) { t.Fatal("Unexpected remaining rule in the rulegroup") } } + +// RuleFilterWrapper provides a flexible way to define rule filtering logic for tests. +type RuleFilterWrapper struct { + shouldIgnore func(rule types.RuleMetadata) bool +} + +func (fw *RuleFilterWrapper) ShouldIgnore(rule types.RuleMetadata) bool { + if fw.shouldIgnore == nil { + return false // Default behavior: don't ignore if no function is provided + } + return fw.shouldIgnore(rule) +} + +// TestRuleFilterInteraction confirms filter is checked first in Eval loop for all phases. +func TestRuleFilterInteraction(t *testing.T) { + // --- Define Rule (Phase 0 to run in all phases) --- + rule := NewRule() + rule.ID_ = 1 + rule.Phase_ = 0 // Phase 0: Always evaluate + rule.operator = nil // No operator means it always matches + if err := rule.AddAction("deny", &dummyDenyAction{}); err != nil { + t.Fatalf("Setup: Failed to add deny action: %v", err) + } + + // --- Phases to Test --- + phasesToTest := []types.RulePhase{ + types.PhaseRequestHeaders, + types.PhaseRequestBody, + types.PhaseResponseHeaders, + types.PhaseResponseBody, + types.PhaseLogging, + } + + // --- Filter Actions --- + filterActions := []struct { + name string + filterShouldIgnore bool + expectInterruption bool // Expect interruption only if filter *allows* the deny rule + }{ + { + name: "Rule Filtered", + filterShouldIgnore: true, + expectInterruption: false, + }, + { + name: "Rule Allowed", + filterShouldIgnore: false, + expectInterruption: true, + }, + } + + // --- Iterate through Phases --- + for _, currentPhase := range phasesToTest { + phaseTestName := fmt.Sprintf("Phase_%d", currentPhase) + + t.Run(phaseTestName, func(t *testing.T) { + // --- Iterate through Filter Actions --- + for _, fa := range filterActions { + filterActionTestName := fa.name + + t.Run(filterActionTestName, func(t *testing.T) { + waf := NewWAF() + if err := waf.Rules.Add(rule); err != nil { + t.Fatalf("Setup: Failed to add rule for %s/%s: %v", phaseTestName, filterActionTestName, err) + } + tx := waf.NewTransaction() + + var filterCalled bool + testFilter := &RuleFilterWrapper{ + shouldIgnore: func(r types.RuleMetadata) bool { + filterCalled = true + return fa.filterShouldIgnore + }, + } + tx.UseRuleFilter(testFilter) + + interrupted := waf.Rules.Eval(currentPhase, tx) + if interrupted != fa.expectInterruption { + t.Fatalf("[%s/%s] ShouldFilter is '%t', expecting interruption '%t', but Eval returned '%t'", + phaseTestName, filterActionTestName, fa.filterShouldIgnore, fa.expectInterruption, interrupted, + ) + } + if !filterCalled { + t.Fatalf("[%s/%s] ShouldIgnore was *not* called", phaseTestName, filterActionTestName) + } + }) + } + }) + } +} diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index c17baa933..cd1a78873 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -123,6 +123,10 @@ type Transaction struct { variables TransactionVariables transformationCache map[transformationKey]*transformationValue + + // ruleFilter allows applying custom rule filtering logic per transaction. + // If set, it's used during rule evaluation to determine if a rule should be skipped. + ruleFilter types.RuleFilter } func (tx *Transaction) ID() string { @@ -1598,6 +1602,13 @@ func (tx *Transaction) Close() error { return fmt.Errorf("transaction close failed: %v", errors.Join(errs...)) } +// UseRuleFilter applies a RuleFilter to the transaction. +// This filter will be consulted during rule evaluation in each phase +// to determine if specific rules should be skipped for this transaction. +func (tx *Transaction) UseRuleFilter(filter types.RuleFilter) { + tx.ruleFilter = filter +} + // String will return a string with the transaction debug information func (tx *Transaction) String() string { res := strings.Builder{} diff --git a/internal/corazawaf/waf.go b/internal/corazawaf/waf.go index 55db79942..87292796d 100644 --- a/internal/corazawaf/waf.go +++ b/internal/corazawaf/waf.go @@ -197,6 +197,7 @@ func (w *WAF) newTransaction(opts Options) *Transaction { tx.debugLogger = w.Logger.With(debuglog.Str("tx_id", tx.id)) tx.Timestamp = time.Now().UnixNano() tx.audit = false + tx.ruleFilter = nil // Always non-nil if buffers / collections were already initialized so we don't do any of them // based on the presence of RequestBodyBuffer. diff --git a/types/rules.go b/types/rules.go index 719b60c59..c21403754 100644 --- a/types/rules.go +++ b/types/rules.go @@ -20,3 +20,12 @@ type RuleMetadata interface { Raw() string SecMark() string } + +// RuleFilter provides an interface for filtering rules during transaction processing. +// Implementations can define custom logic to determine if a specific rule +// should be ignored for a given transaction based on its metadata. +type RuleFilter interface { + // ShouldIgnore evaluates the provided RuleMetadata and returns true if the rule + // should be skipped for the current transaction, false otherwise. + ShouldIgnore(RuleMetadata) bool +} diff --git a/types/transaction.go b/types/transaction.go index b16448a20..84d4d30a3 100644 --- a/types/transaction.go +++ b/types/transaction.go @@ -196,6 +196,11 @@ type Transaction interface { // ID returns the transaction ID. ID() string + // UseRuleFilter applies a RuleFilter to the transaction. + // This filter will be consulted during rule evaluation in each phase + // to determine if specific rules should be skipped for this transaction. + UseRuleFilter(RuleFilter) + // Closer closes the transaction and releases any resources associated with it such as request/response bodies. io.Closer } From 6d79d4760930a1a578d877221cce925505ac615a Mon Sep 17 00:00:00 2001 From: Aleksei Zakharov Date: Mon, 28 Apr 2025 12:49:36 +0300 Subject: [PATCH 2/3] Moved rulefilter to experimental package --- experimental/rulefilter/rftypes/rftypes.go | 17 ++++++ experimental/rulefilter/rulefilter.go | 28 +++++++++ experimental/rulefilter/rulefilter_test.go | 70 ++++++++++++++++++++++ internal/corazawaf/rulegroup_test.go | 2 +- internal/corazawaf/transaction.go | 7 ++- types/rules.go | 9 --- types/transaction.go | 5 -- 7 files changed, 120 insertions(+), 18 deletions(-) create mode 100644 experimental/rulefilter/rftypes/rftypes.go create mode 100644 experimental/rulefilter/rulefilter.go create mode 100644 experimental/rulefilter/rulefilter_test.go diff --git a/experimental/rulefilter/rftypes/rftypes.go b/experimental/rulefilter/rftypes/rftypes.go new file mode 100644 index 000000000..105d88f57 --- /dev/null +++ b/experimental/rulefilter/rftypes/rftypes.go @@ -0,0 +1,17 @@ +// Copyright 2024 Juan Pablo Tosso and the OWASP Coraza contributors +// SPDX-License-Identifier: Apache-2.0 + +// This package defines shared types for the rulefilter package. + +package rftypes + +import "github.com/corazawaf/coraza/v3/types" + +// RuleFilter provides an interface for filtering rules during transaction processing. +// Implementations can define custom logic to determine if a specific rule +// should be ignored for a given transaction based on its metadata. +type RuleFilter interface { + // ShouldIgnore evaluates the provided RuleMetadata and returns true if the rule + // should be skipped for the current transaction, false otherwise. + ShouldIgnore(types.RuleMetadata) bool +} diff --git a/experimental/rulefilter/rulefilter.go b/experimental/rulefilter/rulefilter.go new file mode 100644 index 000000000..2ad1d2422 --- /dev/null +++ b/experimental/rulefilter/rulefilter.go @@ -0,0 +1,28 @@ +// Copyright 2024 Juan Pablo Tosso and the OWASP Coraza contributors +// SPDX-License-Identifier: Apache-2.0 + +// This package provides experimental way to filter rule evaluation +// during transaction processing. + +package rulefilter + +import ( + "fmt" + + "github.com/corazawaf/coraza/v3/experimental/rulefilter/rftypes" + "github.com/corazawaf/coraza/v3/internal/corazawaf" + "github.com/corazawaf/coraza/v3/types" +) + +// SetRuleFilter applies a RuleFilter to the transaction. +// This filter will be consulted during rule evaluation in each phase +// to determine if specific rules should be skipped for this transaction. +// It returns an error if the provided transaction is not of the expected internal type. +func SetRuleFilter(tx types.Transaction, filter rftypes.RuleFilter) error { + internalTx, ok := tx.(*corazawaf.Transaction) + if !ok { + return fmt.Errorf("transaction type assertion failed, expected *corazawaf.Transaction but got %T", tx) + } + internalTx.SetRuleFilter(filter) + return nil +} diff --git a/experimental/rulefilter/rulefilter_test.go b/experimental/rulefilter/rulefilter_test.go new file mode 100644 index 000000000..dff79c0d5 --- /dev/null +++ b/experimental/rulefilter/rulefilter_test.go @@ -0,0 +1,70 @@ +// Copyright 2024 Juan Pablo Tosso and the OWASP Coraza contributors +// SPDX-License-Identifier: Apache-2.0 +package rulefilter + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/corazawaf/coraza/v3" + "github.com/corazawaf/coraza/v3/types" +) + +// Simple implementation for testing. +type mockRuleFilter struct{} + +func (m *mockRuleFilter) ShouldIgnore(types.RuleMetadata) bool { + return true +} + +// Embed the interface to avoid implementing all methods initially. +// We only need this struct to *not* be a *corazawaf.Transaction. +type mockTransaction struct { + types.Transaction +} + +func TestSetRuleFilter(t *testing.T) { + // Note: Verification that the filter *works* is covered by internal tests. + // This test specifically checks whether SetRuleFilter returns the expected error. + + t.Run("set success", func(t *testing.T) { + conf := coraza.NewWAFConfig() + waf, err := coraza.NewWAF(conf) + require.NoError(t, err) + tx := waf.NewTransaction() + require.NotNil(t, tx) + + filter := &mockRuleFilter{} + + err = SetRuleFilter(tx, filter) + require.NoError(t, err, "Setting filter on standard tx should succeed") + }) + + t.Run("set success for nil", func(t *testing.T) { + conf := coraza.NewWAFConfig() + waf, err := coraza.NewWAF(conf) + require.NoError(t, err) + tx := waf.NewTransaction() + require.NotNil(t, tx) + + // First set a filter + initialFilter := &mockRuleFilter{} + err = SetRuleFilter(tx, initialFilter) + require.NoError(t, err) + + // Now clear it by setting nil + err = SetRuleFilter(tx, nil) + require.NoError(t, err, "Setting nil filter should succeed") + }) + + t.Run("fail wrong transaction type", func(t *testing.T) { + // Use our mockTransaction which fulfills the interface but isn't the internal type + mockTx := &mockTransaction{} + filter := &mockRuleFilter{} + + err := SetRuleFilter(mockTx, filter) + require.Error(t, err, "Setting filter on incorrect tx type should fail") + }) + +} diff --git a/internal/corazawaf/rulegroup_test.go b/internal/corazawaf/rulegroup_test.go index 822b01251..f4d51e48a 100644 --- a/internal/corazawaf/rulegroup_test.go +++ b/internal/corazawaf/rulegroup_test.go @@ -161,7 +161,7 @@ func TestRuleFilterInteraction(t *testing.T) { return fa.filterShouldIgnore }, } - tx.UseRuleFilter(testFilter) + tx.SetRuleFilter(testFilter) interrupted := waf.Rules.Eval(currentPhase, tx) if interrupted != fa.expectInterruption { diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index cd1a78873..0481ac85c 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -21,6 +21,7 @@ import ( "github.com/corazawaf/coraza/v3/collection" "github.com/corazawaf/coraza/v3/debuglog" "github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes" + "github.com/corazawaf/coraza/v3/experimental/rulefilter/rftypes" "github.com/corazawaf/coraza/v3/internal/auditlog" "github.com/corazawaf/coraza/v3/internal/bodyprocessors" "github.com/corazawaf/coraza/v3/internal/collections" @@ -126,7 +127,7 @@ type Transaction struct { // ruleFilter allows applying custom rule filtering logic per transaction. // If set, it's used during rule evaluation to determine if a rule should be skipped. - ruleFilter types.RuleFilter + ruleFilter rftypes.RuleFilter } func (tx *Transaction) ID() string { @@ -1602,10 +1603,10 @@ func (tx *Transaction) Close() error { return fmt.Errorf("transaction close failed: %v", errors.Join(errs...)) } -// UseRuleFilter applies a RuleFilter to the transaction. +// SetRuleFilter applies a RuleFilter to the transaction. // This filter will be consulted during rule evaluation in each phase // to determine if specific rules should be skipped for this transaction. -func (tx *Transaction) UseRuleFilter(filter types.RuleFilter) { +func (tx *Transaction) SetRuleFilter(filter rftypes.RuleFilter) { tx.ruleFilter = filter } diff --git a/types/rules.go b/types/rules.go index c21403754..719b60c59 100644 --- a/types/rules.go +++ b/types/rules.go @@ -20,12 +20,3 @@ type RuleMetadata interface { Raw() string SecMark() string } - -// RuleFilter provides an interface for filtering rules during transaction processing. -// Implementations can define custom logic to determine if a specific rule -// should be ignored for a given transaction based on its metadata. -type RuleFilter interface { - // ShouldIgnore evaluates the provided RuleMetadata and returns true if the rule - // should be skipped for the current transaction, false otherwise. - ShouldIgnore(RuleMetadata) bool -} diff --git a/types/transaction.go b/types/transaction.go index 84d4d30a3..b16448a20 100644 --- a/types/transaction.go +++ b/types/transaction.go @@ -196,11 +196,6 @@ type Transaction interface { // ID returns the transaction ID. ID() string - // UseRuleFilter applies a RuleFilter to the transaction. - // This filter will be consulted during rule evaluation in each phase - // to determine if specific rules should be skipped for this transaction. - UseRuleFilter(RuleFilter) - // Closer closes the transaction and releases any resources associated with it such as request/response bodies. io.Closer } From 395799dd850acdeff7c5f157c480284a208a97e1 Mon Sep 17 00:00:00 2001 From: Aleksei Zakharov Date: Mon, 28 Apr 2025 13:13:21 +0300 Subject: [PATCH 3/3] Not using stretchr/testify for tests --- experimental/rulefilter/rulefilter_test.go | 38 +++++++++++++--------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/experimental/rulefilter/rulefilter_test.go b/experimental/rulefilter/rulefilter_test.go index dff79c0d5..91ea75741 100644 --- a/experimental/rulefilter/rulefilter_test.go +++ b/experimental/rulefilter/rulefilter_test.go @@ -5,8 +5,6 @@ package rulefilter import ( "testing" - "github.com/stretchr/testify/require" - "github.com/corazawaf/coraza/v3" "github.com/corazawaf/coraza/v3/types" ) @@ -31,31 +29,38 @@ func TestSetRuleFilter(t *testing.T) { t.Run("set success", func(t *testing.T) { conf := coraza.NewWAFConfig() waf, err := coraza.NewWAF(conf) - require.NoError(t, err) + if err != nil { + t.Fatalf("Failed to create WAF: %v", err) + } tx := waf.NewTransaction() - require.NotNil(t, tx) + if tx == nil { + t.Fatal("Expected non-nil transaction, but got nil") + } filter := &mockRuleFilter{} err = SetRuleFilter(tx, filter) - require.NoError(t, err, "Setting filter on standard tx should succeed") + if err != nil { + t.Fatalf("Setting filter should succeed, but got error: %v", err) + } }) t.Run("set success for nil", func(t *testing.T) { conf := coraza.NewWAFConfig() waf, err := coraza.NewWAF(conf) - require.NoError(t, err) + if err != nil { + t.Fatalf("Failed to create WAF: %v", err) + } tx := waf.NewTransaction() - require.NotNil(t, tx) - - // First set a filter - initialFilter := &mockRuleFilter{} - err = SetRuleFilter(tx, initialFilter) - require.NoError(t, err) + if tx == nil { + t.Fatal("Expected non-nil transaction, but got nil") + } - // Now clear it by setting nil + // resetting the filter should not fail err = SetRuleFilter(tx, nil) - require.NoError(t, err, "Setting nil filter should succeed") + if err != nil { + t.Fatalf("Setting nil filter should succeed, but got error: %v", err) + } }) t.Run("fail wrong transaction type", func(t *testing.T) { @@ -64,7 +69,8 @@ func TestSetRuleFilter(t *testing.T) { filter := &mockRuleFilter{} err := SetRuleFilter(mockTx, filter) - require.Error(t, err, "Setting filter on incorrect tx type should fail") + if err == nil { + t.Fatal("Setting filter on incorrect tx type should fail") + } }) - }