diff --git a/internal/appsec/remoteconfig.go b/internal/appsec/remoteconfig.go index 16981421fc..79e7873760 100644 --- a/internal/appsec/remoteconfig.go +++ b/internal/appsec/remoteconfig.go @@ -50,21 +50,13 @@ func mergeMaps[K comparable, V any](m1 map[K]V, m2 map[K]V) map[K]V { return m1 } -// onRCUpdate is called when one or several remote configuration updates are available -func (a *appsec) onRCUpdate(updates map[string]remoteconfig.ProductUpdate) map[string]rc.ApplyStatus { - r := a.cfg.rulesManager.clone() +// combineRCRulesUpdates updates the state of the given rulesManager with the combination of all the provided rules updates +func combineRCRulesUpdates(r *rulesManager, updates map[string]remoteconfig.ProductUpdate) (map[string]rc.ApplyStatus, error) { statuses := map[string]rc.ApplyStatus{} // Set the default statuses for all updates to unacknowledged for _, u := range updates { statuses = mergeMaps(statuses, statusesFromUpdate(u, false, nil)) } - // Process remote activation update separately - // This makes it easier to short circuit the following loop that focuses on rules related updates - if u, ok := updates[rc.ProductASMFeatures]; ok { - statuses = mergeMaps(statuses, a.handleASMFeatures(u)) - delete(updates, rc.ProductASMFeatures) - } - var err error updateLoop: // Process rules related updates @@ -112,36 +104,60 @@ updateLoop: } var f rulesFragment if err = json.Unmarshal(data, &f); err != nil || !f.validate() { - if err == nil { // + if err == nil { err = errors.New("invalid configuration payload") } log.Debug("appsec: Remote config: error processing ASM config %s: %v", path, err) statuses[path] = genApplyStatus(true, err) break updateLoop - } else { - r.addEdit(path, f) } + r.addEdit(path, f) } default: log.Debug("appsec: Remote config: ignoring unsubscribed product %s", p) } } + // Set all statuses to ack if no error occured + if err == nil { + for _, u := range updates { + statuses = mergeMaps(statuses, statusesFromUpdate(u, true, nil)) + } + } + + return statuses, err + +} + +// onRemoteActivation is the RC callback called when an update is received for ASM_FEATURES +func (a *appsec) onRemoteActivation(updates map[string]remoteconfig.ProductUpdate) map[string]rc.ApplyStatus { + statuses := map[string]rc.ApplyStatus{} + if u, ok := updates[rc.ProductASMFeatures]; ok { + statuses = a.handleASMFeatures(u) + } + return statuses + +} + +// onRCRulesUpdate is the RC callback called when security rules related RC updates are available +func (a *appsec) onRCRulesUpdate(updates map[string]remoteconfig.ProductUpdate) map[string]rc.ApplyStatus { + // If appsec was deactivated through RC, stop here + if !a.started { + return map[string]rc.ApplyStatus{} + } + + r := a.cfg.rulesManager.clone() + statuses, err := combineRCRulesUpdates(r, updates) if err != nil { log.Debug("appsec: Remote config: not applying any updates because of error: %v", err) return statuses } - // Set all statuses to ack - for _, u := range updates { - statuses = mergeMaps(statuses, statusesFromUpdate(u, true, nil)) - } - - // Compile the final rules once all updates have been processed + // Compile the final rules once all updates have been processed and no error occurred r.compile() data := r.raw() log.Debug("appsec: Remote config: final compiled rules: %s", data) - // If an error occurs updating the WAF handle, don't swap the rulesManager and propagate the error + // If an error occurs while updating the WAF handle, don't swap the rulesManager and propagate the error // to all config statuses since we can't know which config is the faulty one if err = a.swapWAF(data); err != nil { log.Error("appsec: Remote config: could not apply the new security rules: %v", err) @@ -272,7 +288,8 @@ func mergeRulesDataEntries(entries1, entries2 []rc.ASMDataRuleDataEntry) []rc.AS func (a *appsec) startRC() { if a.rc != nil { - a.rc.RegisterCallback(a.onRCUpdate) + a.rc.RegisterCallback(a.onRCRulesUpdate) + a.rc.RegisterCallback(a.onRemoteActivation) a.rc.Start() } } @@ -344,10 +361,10 @@ func (a *appsec) enableRCBlocking() { a.registerRCProduct(rc.ProductASM) a.registerRCProduct(rc.ProductASMDD) a.registerRCProduct(rc.ProductASMData) - a.registerRCCapability(remoteconfig.ASMUserBlocking) - a.registerRCCapability(remoteconfig.ASMRequestBlocking) if _, isSet := os.LookupEnv(rulesEnvVar); !isSet { + a.registerRCCapability(remoteconfig.ASMUserBlocking) + a.registerRCCapability(remoteconfig.ASMRequestBlocking) a.registerRCCapability(remoteconfig.ASMIPBlocking) a.registerRCCapability(remoteconfig.ASMDDRules) a.registerRCCapability(remoteconfig.ASMExclusions) diff --git a/internal/appsec/remoteconfig_test.go b/internal/appsec/remoteconfig_test.go index a216ef6bd2..e318358ca6 100644 --- a/internal/appsec/remoteconfig_test.go +++ b/internal/appsec/remoteconfig_test.go @@ -381,7 +381,7 @@ func TestCapabilities(t *testing.T) { { name: "appsec-enabled/rulesManager-from-env", env: map[string]string{enabledEnvVar: "1", rulesEnvVar: "testdata/blocking.json"}, - expected: []remoteconfig.Capability{remoteconfig.ASMRequestBlocking, remoteconfig.ASMUserBlocking}, + expected: []remoteconfig.Capability{}, }, } { @@ -531,7 +531,7 @@ func TestOnRCUpdate(t *testing.T) { tc.ruleset.compile() // Craft and process the RC updates updates := craftRCUpdates(tc.ruleset.edits) - activeAppSec.onRCUpdate(updates) + activeAppSec.onRCRulesUpdate(updates) // Compare rulesets require.ElementsMatch(t, tc.ruleset.latest.Rules, activeAppSec.cfg.rulesManager.latest.Rules) require.ElementsMatch(t, tc.ruleset.latest.Overrides, activeAppSec.cfg.rulesManager.latest.Overrides) @@ -624,7 +624,7 @@ func TestOnRCUpdateStatuses(t *testing.T) { t.Skip("AppSec needs to be enabled for this test") } - statuses := activeAppSec.onRCUpdate(tc.updates) + statuses := activeAppSec.onRCRulesUpdate(tc.updates) if tc.updateError { for _, status := range statuses { require.NotEmpty(t, status.Error) @@ -651,15 +651,13 @@ func TestWafRCUpdate(t *testing.T) { }, } - Start() - defer Stop() - - if !Enabled() { - t.Skip("AppSec needs to be enabled for this test") + if waf.Health() != nil { + t.Skip("WAF needs to be available for this test") } t.Run("toggle-blocking", func(t *testing.T) { - cfg := activeAppSec.cfg + cfg, err := newConfig() + require.NoError(t, err) wafHandle, err := waf.NewHandle(cfg.rulesManager.raw(), cfg.obfuscator.KeyRegex, cfg.obfuscator.ValueRegex) require.NoError(t, err) defer wafHandle.Close() @@ -673,12 +671,11 @@ func TestWafRCUpdate(t *testing.T) { require.Contains(t, string(matches), "crs-913-120") require.Empty(t, actions) // Simulate an RC update that disables the rule - statuses := activeAppSec.onRCUpdate(craftRCUpdates(map[string]rulesFragment{"override": override})) + statuses, err := combineRCRulesUpdates(cfg.rulesManager, craftRCUpdates(map[string]rulesFragment{"override": override})) for _, status := range statuses { require.Equal(t, status.State, rc.ApplyStateAcknowledged) } - // Retrieve the new appsec cfg and instantiate a new waf handle form it to prepare the next run - cfg = activeAppSec.cfg + cfg.rulesManager.compile() newWafHandle, err := waf.NewHandle(cfg.rulesManager.raw(), cfg.obfuscator.KeyRegex, cfg.obfuscator.ValueRegex) require.NoError(t, err) defer newWafHandle.Close() diff --git a/internal/appsec/ruleset_builder.go b/internal/appsec/ruleset_builder.go index bd895ba7c6..919da4ad32 100644 --- a/internal/appsec/ruleset_builder.go +++ b/internal/appsec/ruleset_builder.go @@ -116,10 +116,11 @@ func (r_ *rulesFragment) clone() rulesFragment { // newRulesManager initializes and returns a new rulesManager using the provided rules. // If no rules are provided (nil), or the provided rules are invalid, the default rules are used instead func newRulesManager(rules []byte) *rulesManager { - f := defaultRulesFragment() + var f rulesFragment buf := new(bytes.Buffer) json.Compact(buf, rules) if err := json.Unmarshal(buf.Bytes(), &f); err != nil { + f = defaultRulesFragment() log.Debug("appsec: cannot create rulesManager from specified rules. Using default rules instead") } return &rulesManager{