From 4b5f83cd4dacb214a6e0e7d7e7552e9cfef996b5 Mon Sep 17 00:00:00 2001 From: Anna Date: Tue, 24 Sep 2024 17:05:17 +0200 Subject: [PATCH 1/6] change mechanism --- .../Datadog.Trace/AppSec/Rcm/AsmProduct.cs | 21 +++++++------------ .../AppSec/Rcm/ConfigurationStatus.cs | 7 ++++--- .../AppSec/Rcm/Models/Asm/Action.cs | 2 +- .../ActionChangeTests.cs | 4 ++-- .../RASP/RaspWafTests.cs | 2 +- 5 files changed, 16 insertions(+), 20 deletions(-) diff --git a/tracer/src/Datadog.Trace/AppSec/Rcm/AsmProduct.cs b/tracer/src/Datadog.Trace/AppSec/Rcm/AsmProduct.cs index d8e47fd22183..1da14fab3190 100644 --- a/tracer/src/Datadog.Trace/AppSec/Rcm/AsmProduct.cs +++ b/tracer/src/Datadog.Trace/AppSec/Rcm/AsmProduct.cs @@ -46,20 +46,8 @@ public void ProcessUpdates(ConfigurationStatus configurationStatus, List CustomRulesByFile { get; } = new(); - internal IncomingUpdateStatus IncomingUpdateState { get; } = new(); + internal Dictionary ActionsByFile { get; set; } = new(); - internal IDictionary Actions { get; set; } = new Dictionary(); + internal IncomingUpdateStatus IncomingUpdateState { get; } = new(); internal static List MergeRuleData(IEnumerable res) { @@ -125,7 +125,8 @@ internal Dictionary BuildDictionaryForWafAccordingToIncomingUpda if (IncomingUpdateState.WafKeysToApply.Contains(WafActionsKey)) { - dictionary.Add(WafActionsKey, Actions.Select(a => a.Value.ToKeyValuePair()).ToArray()); + var actions = ActionsByFile.SelectMany(x => x.Value).ToList(); + dictionary.Add(WafActionsKey, actions.Select(a => a.ToKeyValuePair()).ToArray()); } if (IncomingUpdateState.WafKeysToApply.Contains(WafCustomRulesKey)) diff --git a/tracer/src/Datadog.Trace/AppSec/Rcm/Models/Asm/Action.cs b/tracer/src/Datadog.Trace/AppSec/Rcm/Models/Asm/Action.cs index edbfa3a09819..609440f04bee 100644 --- a/tracer/src/Datadog.Trace/AppSec/Rcm/Models/Asm/Action.cs +++ b/tracer/src/Datadog.Trace/AppSec/Rcm/Models/Asm/Action.cs @@ -19,5 +19,5 @@ internal class Action [JsonProperty("parameters")] public Parameter? Parameters { get; set; } - public List> ToKeyValuePair() => new() { new("type", Type), new("id", Id), new("parameters", Parameters?.ToKeyValuePair()) }; + public List> ToKeyValuePair() => [new("type", Type), new("id", Id), new("parameters", Parameters?.ToKeyValuePair())]; } diff --git a/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs b/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs index 7749acb2597d..720c23f66405 100644 --- a/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs +++ b/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs @@ -47,7 +47,7 @@ public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied( ConfigurationStatus configurationStatus = new(string.Empty); var newAction = CreateNewStatusAction(action, actionType, newStatus); - configurationStatus.Actions[action] = newAction; + configurationStatus.ActionsByFile[action] = newAction; configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafActionsKey); var res = waf.UpdateWafFromConfigurationStatus(configurationStatus); res.Success.Should().BeTrue(); @@ -107,7 +107,7 @@ private void UpdateAction(string action, string actionType, Waf waf) newAction.Parameters = new AppSec.Rcm.Models.Asm.Parameter(); newAction.Parameters.StatusCode = 500; newAction.Parameters.Type = "auto"; - configurationStatus.Actions[action] = newAction; + configurationStatus.ActionsByFile[action] = newAction; configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafActionsKey); var res = waf.UpdateWafFromConfigurationStatus(configurationStatus); res.Success.Should().BeTrue(); diff --git a/tracer/test/Datadog.Trace.Security.Unit.Tests/RASP/RaspWafTests.cs b/tracer/test/Datadog.Trace.Security.Unit.Tests/RASP/RaspWafTests.cs index b6bb1ef30a7b..e879fd662375 100644 --- a/tracer/test/Datadog.Trace.Security.Unit.Tests/RASP/RaspWafTests.cs +++ b/tracer/test/Datadog.Trace.Security.Unit.Tests/RASP/RaspWafTests.cs @@ -64,7 +64,7 @@ public void GivenALfiRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied(st ConfigurationStatus configurationStatus = new(string.Empty); var newAction = CreateNewStatusAction(action, actionType, 500); - configurationStatus.Actions[action] = newAction; + configurationStatus.ActionsByFile[action] = newAction; configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafActionsKey); var res = waf.UpdateWafFromConfigurationStatus(configurationStatus); res.Success.Should().BeTrue(); From 29218d24b9e186c9b0bc3377d9ec513aa9b88d6f Mon Sep 17 00:00:00 2001 From: Anna Date: Tue, 24 Sep 2024 18:17:57 +0200 Subject: [PATCH 2/6] fix unit tests --- .../ActionChangeTests.cs | 11 +++-------- .../RASP/RaspWafTests.cs | 2 +- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs b/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs index 720c23f66405..db20aec6c84b 100644 --- a/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs +++ b/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs @@ -47,7 +47,7 @@ public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied( ConfigurationStatus configurationStatus = new(string.Empty); var newAction = CreateNewStatusAction(action, actionType, newStatus); - configurationStatus.ActionsByFile[action] = newAction; + configurationStatus.ActionsByFile[action] = [newAction]; configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafActionsKey); var res = waf.UpdateWafFromConfigurationStatus(configurationStatus); res.Success.Should().BeTrue(); @@ -101,13 +101,8 @@ private Dictionary CreateArgs(string requestParam) private void UpdateAction(string action, string actionType, Waf waf) { ConfigurationStatus configurationStatus = new(string.Empty); - var newAction = new Action(); - newAction.Id = action; - newAction.Type = actionType; - newAction.Parameters = new AppSec.Rcm.Models.Asm.Parameter(); - newAction.Parameters.StatusCode = 500; - newAction.Parameters.Type = "auto"; - configurationStatus.ActionsByFile[action] = newAction; + var newAction = new Action { Id = action, Type = actionType, Parameters = new AppSec.Rcm.Models.Asm.Parameter { StatusCode = 500, Type = "auto" } }; + configurationStatus.ActionsByFile[action] = [newAction]; configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafActionsKey); var res = waf.UpdateWafFromConfigurationStatus(configurationStatus); res.Success.Should().BeTrue(); diff --git a/tracer/test/Datadog.Trace.Security.Unit.Tests/RASP/RaspWafTests.cs b/tracer/test/Datadog.Trace.Security.Unit.Tests/RASP/RaspWafTests.cs index e879fd662375..f30da4b0835f 100644 --- a/tracer/test/Datadog.Trace.Security.Unit.Tests/RASP/RaspWafTests.cs +++ b/tracer/test/Datadog.Trace.Security.Unit.Tests/RASP/RaspWafTests.cs @@ -64,7 +64,7 @@ public void GivenALfiRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied(st ConfigurationStatus configurationStatus = new(string.Empty); var newAction = CreateNewStatusAction(action, actionType, 500); - configurationStatus.ActionsByFile[action] = newAction; + configurationStatus.ActionsByFile[action] = [newAction]; configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafActionsKey); var res = waf.UpdateWafFromConfigurationStatus(configurationStatus); res.Success.Should().BeTrue(); From a4100ba1272bc1b3f39192f17a210d6442195f38 Mon Sep 17 00:00:00 2001 From: Anna Date: Mon, 30 Sep 2024 13:56:24 +0200 Subject: [PATCH 3/6] Change the way we respond to action duplicates and store them by files --- .../AppSec/Rcm/ConfigurationStatus.cs | 14 +++- tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs | 7 ++ .../ActionChangeTests.cs | 82 ++++++++++--------- 3 files changed, 61 insertions(+), 42 deletions(-) diff --git a/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs b/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs index a94fe225221a..8aff986f40c0 100644 --- a/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs +++ b/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs @@ -67,7 +67,7 @@ internal record ConfigurationStatus internal Dictionary CustomRulesByFile { get; } = new(); - internal Dictionary ActionsByFile { get; set; } = new(); + internal Dictionary ActionsByFile { get; init; } = new(); internal IncomingUpdateStatus IncomingUpdateState { get; } = new(); @@ -126,7 +126,17 @@ internal Dictionary BuildDictionaryForWafAccordingToIncomingUpda if (IncomingUpdateState.WafKeysToApply.Contains(WafActionsKey)) { var actions = ActionsByFile.SelectMany(x => x.Value).ToList(); - dictionary.Add(WafActionsKey, actions.Select(a => a.ToKeyValuePair()).ToArray()); + var dupes = actions.GroupBy(a => a.Id).Where(g => g.Count() > 1).Select(a => a.Key).ToList(); + var actionsDic = actions.Where(a => !dupes.Contains(a.Id)).Select(a => a.ToKeyValuePair()).ToList(); + if (actionsDic.Count > 0) + { + dictionary.Add(WafActionsKey, actionsDic); + } + + foreach (var dupe in dupes) + { + Log.Warning("Duplicate action found with id: {ActionId}, this action will be discarded, default waf action, if any, will apply", dupe); + } } if (IncomingUpdateState.WafKeysToApply.Contains(WafCustomRulesKey)) diff --git a/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs b/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs index 3f6ee8115eae..1013836e6dd7 100644 --- a/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs +++ b/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs @@ -15,6 +15,7 @@ using Datadog.Trace.AppSec.Waf.NativeBindings; using Datadog.Trace.AppSec.Waf.ReturnTypes.Managed; using Datadog.Trace.AppSec.WafEncoding; +using Datadog.Trace.ExtensionMethods; using Datadog.Trace.Logging; using Datadog.Trace.Telemetry; @@ -169,6 +170,12 @@ private unsafe UpdateResult UpdateWafAndDispose(IEncodeResult updateData) public UpdateResult UpdateWafFromConfigurationStatus(ConfigurationStatus configurationStatus) { var dic = configurationStatus.BuildDictionaryForWafAccordingToIncomingUpdate(); + if (dic.IsEmpty()) + { + Log.Warning("A waf update came from remote configuration but final merged dictionary for waf is empty, no update will be performed."); + return new(null, false); + } + return Update(dic); } diff --git a/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs b/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs index db20aec6c84b..701377e033b8 100644 --- a/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs +++ b/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs @@ -20,12 +20,12 @@ namespace Datadog.Trace.Security.Unit.Tests; public class ActionChangeTests : WafLibraryRequiredTest { [Theory] - [InlineData("dummy_rule", "test-dummy-rule", "rasp-rule-set.json", "block", BlockingAction.BlockRequestType, 500)] - [InlineData("dummyrule2", "test-dummy-rule2", "rasp-rule-set.json", "customblock", BlockingAction.BlockRequestType, 500)] - // Redirect status code is restricted in newer versions of the WAF to 301, 302, 303, 307 - [InlineData("dummyrule2", "test-dummy-rule2", "rasp-rule-set.json", "customblock", BlockingAction.RedirectRequestType, 303)] - [InlineData("dummy_rule", "test-dummy-rule", "rasp-rule-set.json", "block", BlockingAction.RedirectRequestType, 303)] - public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied(string paramValue, string rule, string ruleFile, string action, string actionType, int newStatus) + [InlineData("dummy_rule", "test-dummy-rule", "block", BlockingAction.BlockRequestType, 500)] + [InlineData("dummyrule2", "test-dummy-rule2", "customblock", BlockingAction.BlockRequestType, 500)] + // Redirect status code is restricted in newer versions of the waf to 301, 302, 303, 307 + [InlineData("dummyrule2", "test-dummy-rule2", "customblock", BlockingAction.RedirectRequestType, 303)] + [InlineData("dummy_rule", "test-dummy-rule", "block", BlockingAction.RedirectRequestType, 303)] + public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied(string paramValue, string rule, string action, string actionType, int newStatus) { var args = CreateArgs(paramValue); var initResult = Waf.Create( @@ -33,7 +33,7 @@ public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied( string.Empty, string.Empty, useUnsafeEncoder: true, - embeddedRulesetPath: ruleFile); + embeddedRulesetPath: "rasp-rule-set.json"); var waf = initResult.Waf; waf.Should().NotBeNull(); @@ -44,13 +44,9 @@ public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied( var jsonString = JsonConvert.SerializeObject(result.Data); var resultData = JsonConvert.DeserializeObject(jsonString).FirstOrDefault(); resultData.Rule.Id.Should().Be(rule); - - ConfigurationStatus configurationStatus = new(string.Empty); var newAction = CreateNewStatusAction(action, actionType, newStatus); - configurationStatus.ActionsByFile[action] = [newAction]; - configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafActionsKey); - var res = waf.UpdateWafFromConfigurationStatus(configurationStatus); - res.Success.Should().BeTrue(); + UpdateWafWithActions([newAction], waf); + using var contextNew = waf.CreateContext(); result = contextNew.Run(args, TimeoutMicroSeconds); result.Timeout.Should().BeFalse("Timeout should be false"); @@ -66,43 +62,54 @@ public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied( } [Theory] - [InlineData("dummyrule2", "rasp-rule-set.json", "customblock", BlockingAction.BlockRequestType)] - public void GivenADummyRule_WhenActionReturnCodeIsChangedAfterInit_ThenChangesAreApplied(string paramValue, string ruleFile, string action, string actionType) + [InlineData("dummy_rule", "block", BlockingAction.BlockRequestType, 401)] + [InlineData("dummy_rule", "block", BlockingAction.BlockRequestType, 401, true)] + public void GivenADummyRule_WhenDuplicateActionIsReceived_ThenBackToDefaultWafActions(string paramValue, string action, string actionType, int newStatus, bool placeInDifferentFiles = false) { var args = CreateArgs(paramValue); var initResult = Waf.Create( - WafLibraryInvoker, + WafLibraryInvoker!, string.Empty, string.Empty, useUnsafeEncoder: true, - embeddedRulesetPath: ruleFile); + embeddedRulesetPath: "rasp-rule-set.json"); var waf = initResult.Waf; waf.Should().NotBeNull(); + Action[] newActions = + [ + CreateNewStatusAction(action, actionType, newStatus), CreateNewStatusAction(action, actionType, newStatus), CreateNewStatusAction(action, actionType, newStatus), + CreateNewStatusAction("dummy_rule", BlockingAction.BlockRequestType, 500) // add a dummy one, otherwise nothing will be updated + ]; - UpdateAction(action, actionType, waf); + UpdateWafWithActions(newActions, waf, placeInDifferentFiles); - using var context = waf.CreateContext(); - var result = context.Run(args, TimeoutMicroSeconds); - result.Timeout.Should().BeFalse("Timeout should be false"); - result.BlockInfo["status_code"].Should().Be("500"); + using var context = waf!.CreateContext(); + var result = context!.Run(args, TimeoutMicroSeconds); + result.Should().NotBeNull(); + result!.Timeout.Should().BeFalse("Timeout should be false"); + // default waf action block + result.BlockInfo!["status_code"].Should().Be("403"); + result.BlockInfo["grpc_status_code"].Should().Be("10"); } - private Dictionary CreateArgs(string requestParam) + private Dictionary CreateArgs(string requestParam) => new() { { AddressesConstants.RequestUriRaw, "http://localhost:54587/" }, { AddressesConstants.RequestBody, new[] { "param", requestParam } }, { AddressesConstants.RequestMethod, "GET" } }; + + private void UpdateWafWithActions(Action[] actions, Waf waf, bool placeInDifferentFiles = false) { - return new Dictionary + ConfigurationStatus configurationStatus; + if (placeInDifferentFiles) { - { AddressesConstants.RequestUriRaw, "http://localhost:54587/" }, - { AddressesConstants.RequestBody, new[] { "param", requestParam } }, - { AddressesConstants.RequestMethod, "GET" } - }; - } + var i = 0; + var dic = actions.ToDictionary(_ => $"file{i++}", action => [action]); + + configurationStatus = new(string.Empty) { ActionsByFile = dic }; + } + else + { + configurationStatus = new(string.Empty) { ActionsByFile = { ["file"] = actions } }; + } - private void UpdateAction(string action, string actionType, Waf waf) - { - ConfigurationStatus configurationStatus = new(string.Empty); - var newAction = new Action { Id = action, Type = actionType, Parameters = new AppSec.Rcm.Models.Asm.Parameter { StatusCode = 500, Type = "auto" } }; - configurationStatus.ActionsByFile[action] = [newAction]; configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafActionsKey); var res = waf.UpdateWafFromConfigurationStatus(configurationStatus); res.Success.Should().BeTrue(); @@ -110,12 +117,7 @@ private void UpdateAction(string action, string actionType, Waf waf) private Action CreateNewStatusAction(string action, string actionType, int newStatus) { - var newAction = new Action(); - newAction.Id = action; - newAction.Type = actionType; - newAction.Parameters = new AppSec.Rcm.Models.Asm.Parameter(); - newAction.Parameters.StatusCode = newStatus; - newAction.Parameters.Type = "auto"; + var newAction = new Action { Id = action, Type = actionType, Parameters = new AppSec.Rcm.Models.Asm.Parameter { StatusCode = newStatus, Type = "auto" } }; if (newAction.Type == BlockingAction.RedirectRequestType) { From a37bb29d4dccab852d3124085577c906722d465e Mon Sep 17 00:00:00 2001 From: Anna Date: Mon, 30 Sep 2024 14:18:30 +0200 Subject: [PATCH 4/6] Improve update result fix:: must be an array to waf --- .../Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs | 7 ++----- .../AppSec/Waf/ReturnTypes.Managed/UpdateResult.cs | 11 ++++++++++- tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs | 6 +++--- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs b/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs index 8aff986f40c0..6436b17e8dee 100644 --- a/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs +++ b/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs @@ -127,11 +127,8 @@ internal Dictionary BuildDictionaryForWafAccordingToIncomingUpda { var actions = ActionsByFile.SelectMany(x => x.Value).ToList(); var dupes = actions.GroupBy(a => a.Id).Where(g => g.Count() > 1).Select(a => a.Key).ToList(); - var actionsDic = actions.Where(a => !dupes.Contains(a.Id)).Select(a => a.ToKeyValuePair()).ToList(); - if (actionsDic.Count > 0) - { - dictionary.Add(WafActionsKey, actionsDic); - } + var actionsDic = actions.Where(a => !dupes.Contains(a.Id)).Select(a => a.ToKeyValuePair()).ToArray(); + dictionary.Add(WafActionsKey, actionsDic); foreach (var dupe in dupes) { diff --git a/tracer/src/Datadog.Trace/AppSec/Waf/ReturnTypes.Managed/UpdateResult.cs b/tracer/src/Datadog.Trace/AppSec/Waf/ReturnTypes.Managed/UpdateResult.cs index af78d9259d12..3069ea9587e6 100644 --- a/tracer/src/Datadog.Trace/AppSec/Waf/ReturnTypes.Managed/UpdateResult.cs +++ b/tracer/src/Datadog.Trace/AppSec/Waf/ReturnTypes.Managed/UpdateResult.cs @@ -13,7 +13,7 @@ namespace Datadog.Trace.AppSec.Waf.ReturnTypes.Managed { internal class UpdateResult { - internal UpdateResult(DdwafObjectStruct? diagObject, bool success, bool unusableRules = false) + private UpdateResult(DdwafObjectStruct? diagObject, bool success, bool unusableRules = false, bool nothingToUpdate = false) { if (diagObject != null) { @@ -33,12 +33,15 @@ internal UpdateResult(DdwafObjectStruct? diagObject, bool success, bool unusable Success = success; UnusableRules = unusableRules; + NothingToUpdate = nothingToUpdate; } internal bool Success { get; } internal bool UnusableRules { get; } + public bool NothingToUpdate { get; } + internal ushort? FailedToLoadRules { get; } /// @@ -56,6 +59,12 @@ internal UpdateResult(DdwafObjectStruct? diagObject, bool success, bool unusable public static UpdateResult FromUnusableRules() => new(null, false, true); + public static UpdateResult FromNothingToUpdate() => new(null, true, nothingToUpdate: true); + public static UpdateResult FromFailed() => new(null, false); + + public static UpdateResult FromFailed(DdwafObjectStruct diagObj) => new(diagObj, false); + + public static UpdateResult FromSuccess(DdwafObjectStruct diagObj) => new(diagObj, true); } } diff --git a/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs b/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs index 1013836e6dd7..45ba8519b81f 100644 --- a/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs +++ b/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs @@ -147,7 +147,7 @@ private unsafe UpdateResult UpdateWafAndDispose(IEncodeResult updateData) _wafHandle = newHandle; _wafLocker.ExitWriteLock(); _wafLibraryInvoker.Destroy(oldHandle); - return new(diagnosticsValue, true); + return UpdateResult.FromSuccess(diagnosticsValue); } _wafLibraryInvoker.Destroy(newHandle); @@ -159,7 +159,7 @@ private unsafe UpdateResult UpdateWafAndDispose(IEncodeResult updateData) } finally { - res ??= new(diagnosticsValue, false); + res ??= UpdateResult.FromFailed(diagnosticsValue); _wafLibraryInvoker.ObjectFree(ref diagnosticsValue); updateData.Dispose(); } @@ -173,7 +173,7 @@ public UpdateResult UpdateWafFromConfigurationStatus(ConfigurationStatus configu if (dic.IsEmpty()) { Log.Warning("A waf update came from remote configuration but final merged dictionary for waf is empty, no update will be performed."); - return new(null, false); + return UpdateResult.FromNothingToUpdate(); } return Update(dic); From 8098a61fd96f66f5ddd2a91fc73e9d54971a2dd9 Mon Sep 17 00:00:00 2001 From: Anna Date: Tue, 1 Oct 2024 11:02:15 +0200 Subject: [PATCH 5/6] in the end dont handle duplicates in tracer --- .../AppSec/Rcm/ConfigurationStatus.cs | 9 +--- .../ActionChangeTests.cs | 48 +------------------ 2 files changed, 3 insertions(+), 54 deletions(-) diff --git a/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs b/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs index 6436b17e8dee..78cd40181b5e 100644 --- a/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs +++ b/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs @@ -126,14 +126,7 @@ internal Dictionary BuildDictionaryForWafAccordingToIncomingUpda if (IncomingUpdateState.WafKeysToApply.Contains(WafActionsKey)) { var actions = ActionsByFile.SelectMany(x => x.Value).ToList(); - var dupes = actions.GroupBy(a => a.Id).Where(g => g.Count() > 1).Select(a => a.Key).ToList(); - var actionsDic = actions.Where(a => !dupes.Contains(a.Id)).Select(a => a.ToKeyValuePair()).ToArray(); - dictionary.Add(WafActionsKey, actionsDic); - - foreach (var dupe in dupes) - { - Log.Warning("Duplicate action found with id: {ActionId}, this action will be discarded, default waf action, if any, will apply", dupe); - } + dictionary.Add(WafActionsKey, actions.Select(r => r.ToKeyValuePair()).ToArray()); } if (IncomingUpdateState.WafKeysToApply.Contains(WafCustomRulesKey)) diff --git a/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs b/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs index 701377e033b8..0632ba9d72ef 100644 --- a/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs +++ b/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs @@ -61,55 +61,11 @@ public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied( } } - [Theory] - [InlineData("dummy_rule", "block", BlockingAction.BlockRequestType, 401)] - [InlineData("dummy_rule", "block", BlockingAction.BlockRequestType, 401, true)] - public void GivenADummyRule_WhenDuplicateActionIsReceived_ThenBackToDefaultWafActions(string paramValue, string action, string actionType, int newStatus, bool placeInDifferentFiles = false) - { - var args = CreateArgs(paramValue); - var initResult = Waf.Create( - WafLibraryInvoker!, - string.Empty, - string.Empty, - useUnsafeEncoder: true, - embeddedRulesetPath: "rasp-rule-set.json"); - - var waf = initResult.Waf; - waf.Should().NotBeNull(); - Action[] newActions = - [ - CreateNewStatusAction(action, actionType, newStatus), CreateNewStatusAction(action, actionType, newStatus), CreateNewStatusAction(action, actionType, newStatus), - CreateNewStatusAction("dummy_rule", BlockingAction.BlockRequestType, 500) // add a dummy one, otherwise nothing will be updated - ]; - - UpdateWafWithActions(newActions, waf, placeInDifferentFiles); - - using var context = waf!.CreateContext(); - var result = context!.Run(args, TimeoutMicroSeconds); - result.Should().NotBeNull(); - result!.Timeout.Should().BeFalse("Timeout should be false"); - // default waf action block - result.BlockInfo!["status_code"].Should().Be("403"); - result.BlockInfo["grpc_status_code"].Should().Be("10"); - } - private Dictionary CreateArgs(string requestParam) => new() { { AddressesConstants.RequestUriRaw, "http://localhost:54587/" }, { AddressesConstants.RequestBody, new[] { "param", requestParam } }, { AddressesConstants.RequestMethod, "GET" } }; - private void UpdateWafWithActions(Action[] actions, Waf waf, bool placeInDifferentFiles = false) + private void UpdateWafWithActions(Action[] actions, Waf waf) { - ConfigurationStatus configurationStatus; - if (placeInDifferentFiles) - { - var i = 0; - var dic = actions.ToDictionary(_ => $"file{i++}", action => [action]); - - configurationStatus = new(string.Empty) { ActionsByFile = dic }; - } - else - { - configurationStatus = new(string.Empty) { ActionsByFile = { ["file"] = actions } }; - } - + ConfigurationStatus configurationStatus = new(string.Empty) { ActionsByFile = { ["file"] = actions } }; configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafActionsKey); var res = waf.UpdateWafFromConfigurationStatus(configurationStatus); res.Success.Should().BeTrue(); From e0807d50fdd7b94c31f816acd46f25de81424a45 Mon Sep 17 00:00:00 2001 From: Anna Date: Tue, 1 Oct 2024 11:09:49 +0200 Subject: [PATCH 6/6] it's a fact --- .../ActionChangeTests.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs b/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs index 0632ba9d72ef..7d60c91dfcd3 100644 --- a/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs +++ b/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs @@ -61,6 +61,28 @@ public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied( } } + [Fact] + public void GivenADummyRule_WhenActionReturnCodeIsChangedAfterInit_ThenChangesAreApplied() + { + var args = CreateArgs("dummyrule2"); + var initResult = Waf.Create( + WafLibraryInvoker, + string.Empty, + string.Empty, + useUnsafeEncoder: true, + embeddedRulesetPath: "rasp-rule-set.json"); + + var waf = initResult.Waf; + waf.Should().NotBeNull(); + + UpdateWafWithActions([CreateNewStatusAction("customblock", BlockingAction.BlockRequestType, 500)], waf); + + using var context = waf.CreateContext(); + var result = context.Run(args, TimeoutMicroSeconds); + result.Timeout.Should().BeFalse("Timeout should be false"); + result.BlockInfo["status_code"].Should().Be("500"); + } + private Dictionary CreateArgs(string requestParam) => new() { { AddressesConstants.RequestUriRaw, "http://localhost:54587/" }, { AddressesConstants.RequestBody, new[] { "param", requestParam } }, { AddressesConstants.RequestMethod, "GET" } }; private void UpdateWafWithActions(Action[] actions, Waf waf)