diff --git a/extensions/composer/waf/waf.go b/extensions/composer/waf/waf.go index f22d1f31..5edae97e 100644 --- a/extensions/composer/waf/waf.go +++ b/extensions/composer/waf/waf.go @@ -7,6 +7,8 @@ package waf import ( + "fmt" + "io" "net" "strconv" "strings" @@ -26,11 +28,35 @@ const ( metadataKeyBlockPhase = "block_phase" ) +// wafRegistry tracks all WAF instances created for a single Envoy config version +// (main config + per-route overrides) so they can be closed together in OnDestroy +// to release memoized regex patterns from Coraza's global cache. +type wafRegistry struct { + wafs []coraza.WAF +} + +func (r *wafRegistry) add(w coraza.WAF) { + if w != nil { + r.wafs = append(r.wafs, w) + } +} + +func (r *wafRegistry) closeAll(l *logger.Logger) { + for _, w := range r.wafs { + if c, ok := w.(io.Closer); ok { + if err := c.Close(); err != nil { + l.Error(fmt.Sprintf("waf: error closing WAF instance: %v", err)) + } + } + } +} + type wafPluginFactory struct { shared.EmptyHttpFilterFactory - config coraza.WAF - mode waf.WAFMode - metrics *metrics + config coraza.WAF + mode waf.WAFMode + metrics *metrics + registry *wafRegistry } type perRouteWafPluginConfig struct { @@ -38,6 +64,14 @@ type perRouteWafPluginConfig struct { mode waf.WAFMode } +// OnDestroy releases memoized regex caches held by all WAF instances (main + per-route) +// created by this factory. Called by Envoy when the factory is destroyed (e.g. config update). +func (f *wafPluginFactory) OnDestroy() { + l := logger.GetLogger() + l.Sugar().Infof("waf: closing %d WAF instances", len(f.registry.wafs)) + f.registry.closeAll(l) +} + func (f *wafPluginFactory) Create(handle shared.HttpFilterHandle) shared.HttpFilter { config := f.config mode := f.mode @@ -66,6 +100,9 @@ func (f *wafPluginFactory) Create(handle shared.HttpFilterHandle) shared.HttpFil type wafPluginConfigFactory struct { shared.EmptyHttpFilterConfigFactory + // registry is shared between the config factory and the plugin factory it creates, + // so per-route WAFs added after the factory is created can still be closed on destroy. + registry *wafRegistry } func (f *wafPluginConfigFactory) Create( @@ -88,10 +125,14 @@ func (f *wafPluginConfigFactory) Create( handle.Log(shared.LogLevelInfo, "waf: empty filter config") } + f.registry = &wafRegistry{} + f.registry.add(wafConfig) + return &wafPluginFactory{ - config: wafConfig, - mode: mode, - metrics: newMetrics(handle), + config: wafConfig, + mode: mode, + metrics: newMetrics(handle), + registry: f.registry, }, nil } @@ -100,6 +141,9 @@ func (f *wafPluginConfigFactory) CreatePerRoute(unparsedConfig []byte) (any, err if err != nil { return nil, err } + if f.registry != nil { + f.registry.add(wafConfig) + } return &perRouteWafPluginConfig{ config: wafConfig, mode: mode, diff --git a/extensions/composer/waf/waf_test.go b/extensions/composer/waf/waf_test.go index 0f1ebeb5..86a7339e 100644 --- a/extensions/composer/waf/waf_test.go +++ b/extensions/composer/waf/waf_test.go @@ -7,7 +7,10 @@ package waf import ( "encoding/json" + "fmt" "io" + "runtime" + "runtime/debug" "strconv" "testing" @@ -1901,3 +1904,74 @@ func Test_PerRouteConfigBlocksRequest(t *testing.T) { wafPlugin.OnStreamComplete() } + +// Verifies that OnDestroy() keeps heap growth bounded across +// config reloads by releasing Coraza's memoized regex patterns. +func Test_OnDestroyReleasesMemory(t *testing.T) { + const ( + reloads = 30 + rulesPerWAF = 50 + ) + + ruleID := 1 + makeDirectives := func() []string { + directives := []string{"SecRuleEngine On"} + for range rulesPerWAF { + directives = append(directives, fmt.Sprintf( + `SecRule REQUEST_URI "@rx ^/reload/%d/path$" "id:%d,phase:1,pass,nolog"`, ruleID, ruleID, + )) + ruleID++ + } + return directives + } + + heapAlloc := func() uint64 { + // GC twice then scavenge: the first GC collects most objects, the second + // collects anything promoted or finalized during the first pass, and + // FreeOSMemory forces the runtime to return freed pages to the OS so that + // HeapAlloc reflects only genuinely live allocations. + runtime.GC() + runtime.GC() + debug.FreeOSMemory() + var m runtime.MemStats + runtime.ReadMemStats(&m) + return m.HeapAlloc + } + + baseline := heapAlloc() + + for range reloads { + // Use a fresh controller per reload so gomock state doesn't accumulate + // and skew the heap measurement. + ctrl := gomock.NewController(t) + + mainConfig, err := json.Marshal(map[string]any{"directives": makeDirectives(), "mode": "FULL"}) + require.NoError(t, err) + perRouteConfig, err := json.Marshal(map[string]any{"directives": makeDirectives(), "mode": "FULL"}) + require.NoError(t, err) + + configHandle := mocks.NewMockHttpFilterConfigHandle(ctrl) + configHandle.EXPECT().DefineCounter("waf_tx_total").Return(shared.MetricID(1), shared.MetricsSuccess) + configHandle.EXPECT().DefineCounter("waf_tx_blocked", "authority", "phase", "rule_id").Return(shared.MetricID(2), shared.MetricsSuccess) + + configFactory := &wafPluginConfigFactory{} + factory, err := configFactory.Create(configHandle, mainConfig) + require.NoError(t, err) + _, err = configFactory.CreatePerRoute(perRouteConfig) + require.NoError(t, err) + + // Simulate Envoy destroying the old factory when a config reload arrives. + factory.OnDestroy() + ctrl.Finish() + } + + const maxGrowthKB = uint64(1024) + after := heapAlloc() + heapGrowthKB := (max(after, baseline) - baseline) / 1024 + + // Without OnDestroy releasing memoized caches: 30 reloads × 2 WAFs × 50 unique + // regexes leaks ~11+ MB. With Close(), heap growth stays under 1 MB. + assert.LessOrEqual(t, heapGrowthKB, maxGrowthKB, + "heap grew %d KB after %d reloads, expected ≤ %d KB: OnDestroy must call Close() on all WAF instances to release Coraza's memoized regex cache", + heapGrowthKB, reloads, maxGrowthKB) +}