Skip to content

Commit

Permalink
[ASM] Fix bug RC empty key (#6058)
Browse files Browse the repository at this point in the history
## Summary of changes

Fixed a regression bug.
The key variable for the process update wasn't the right one.

# Bug explanation

The Path of the file wasn't get from the right variable.
`result.TypedFile` is a `JToken`, getting the `Path` from this object
would get the path of the JSON token. The path is an empty string and
totally not the info we wanted to get.

## Test coverage

- Add a way to set custom headers instead of the default `Mistake
Not...` to trigger a blocking event from the default ruleset
- Added an integration test where it tests deletion and switch back to
default rules but keep updated blocking action.
  • Loading branch information
e-n-0 authored Sep 24, 2024
1 parent 73ca3f3 commit 9dc9b77
Show file tree
Hide file tree
Showing 6 changed files with 220 additions and 6 deletions.
2 changes: 1 addition & 1 deletion tracer/src/Datadog.Trace/AppSec/Rcm/AsmDdProduct.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void ProcessUpdates(ConfigurationStatus configurationStatus, List<RemoteC
ruleSet = RuleSet.From(result.TypedFile);
}

configurationStatus.RulesByFile[result.TypedFile.Path] = ruleSet;
configurationStatus.RulesByFile[firstFile.Path.Path] = ruleSet;
configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafRulesKey);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ public void AddCookies(Dictionary<string, string> cookiesValues)
}
}

public void ResetDefaultUserAgent()
{
_httpClient.DefaultRequestHeaders.Remove("user-agent");
}

public async Task TestAppSecRequestWithVerifyAsync(MockTracerAgent agent, string url, string body, int expectedSpans, int spansPerRequest, VerifySettings settings, string contentType = null, bool testInit = false, string userAgent = null, string methodNameOverride = null, string fileNameOverride = null)
{
var spans = await SendRequestsAsync(agent, url, body, expectedSpans, expectedSpans * spansPerRequest, string.Empty, contentType, userAgent);
Expand Down Expand Up @@ -351,9 +356,9 @@ protected async Task TestRateLimiter(bool enableSecurity, string url, MockTracer

protected async Task<(HttpStatusCode StatusCode, string ResponseText)> SubmitRequest(string path, string body, string contentType, string userAgent = null, string accept = null, IEnumerable<KeyValuePair<string, string>> headers = null)
{
var values = _httpClient.DefaultRequestHeaders.GetValues("user-agent");
var found = _httpClient.DefaultRequestHeaders.TryGetValues("user-agent", out var values);

if (!string.IsNullOrEmpty(userAgent) && values.All(c => string.Compare(c, userAgent, StringComparison.Ordinal) != 0))
if (!string.IsNullOrEmpty(userAgent) && (!found || values.All(c => string.Compare(c, userAgent, StringComparison.Ordinal) != 0)))
{
_httpClient.DefaultRequestHeaders.Add("user-agent", userAgent);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@
<None Update="ApiSecurity\ruleset-with-block.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>

<None Update="remote-rules-override-blocking.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)'=='net8.0' OR '$(TargetFramework)'=='net7.0' OR '$(TargetFramework)'=='net6.0' OR $(TargetFramework.StartsWith('net4'))">
Expand Down Expand Up @@ -85,7 +89,7 @@
<None Update="remote-rules.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="ruleset.3.0.json">
<None Update="remote-rules-override-blocking.json">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="ruleset.3.0.json">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Collections.Immutable;
using System.IO;
using System.Threading.Tasks;
using Datadog.Trace.AppSec.Rcm.Models.Asm;
using Datadog.Trace.TestHelpers;
using Xunit;
using Xunit.Abstractions;
Expand All @@ -24,24 +25,48 @@ public AspNetCore5AsmRemoteRules(AspNetCoreTestFixture fixture, ITestOutputHelpe

[SkippableFact]
[Trait("RunOnWindows", "True")]
public async Task TestNewRemoteRules()
public async Task TestRemoteRules()
{
var url = "/Health/?[$slice]=value";
await TryStartApp();
var agent = Fixture.Agent;
var settings = VerifyHelper.GetSpanVerifierSettings();

// Test new rules
var spans1 = await SendRequestsAsync(agent, url);
var productId = nameof(TestNewRemoteRules);
var productId = nameof(TestRemoteRules);
await agent.SetupRcmAndWait(Output, new List<(object Config, string ProductName, string Id)> { (GetRules("2.22.222"), "ASM_DD", productId) });
var spans2 = await SendRequestsAsync(agent, url);
await agent.SetupRcmAndWait(Output, new List<(object Config, string ProductName, string Id)> { (GetRules("3.33.333"), "ASM_DD", productId) });
var spans3 = await SendRequestsAsync(agent, url);

// Test deletion + switch back to default rules when no RC no rules + keep updated blocking action + reset blocking actions
ResetDefaultUserAgent();
var block405Action = new Payload() { Actions = [new Action { Id = "block", Type = "block_request", Parameters = new Parameter { StatusCode = 405, Type = "json" } }] };
var block405ActionProductId = "action";

await agent.SetupRcmAndWait(Output, new List<(object Config, string ProductName, string Id)> { (block405Action, "ASM", block405ActionProductId), (GetNonBlockingRules(), "ASM_DD", productId) });
var spans4 = await SendRequestsAsync(agent, "/", null, 1, 1, null, null, "dd-test-scanner-log-block");
// Should trigger on the new applied rule "new-test-non-blocking"
// Should not block and return a 200

await agent.SetupRcmAndWait(Output, new List<(object Config, string ProductName, string Id)> { (block405Action, "ASM", block405ActionProductId) });
var spans5 = await SendRequestsAsync(agent, "/", null, 1, 1, null, null, "dd-test-scanner-log-block");
// Should fall back to the default rules and trigger "ua0-600-56x"
// Should block and return a 405 (from the defined action)

await agent.SetupRcmAndWait(Output, new List<(object Config, string ProductName, string Id)> { (new Payload { Actions = [] }, "ASM", block405ActionProductId) });
var spans6 = await SendRequestsAsync(agent, "/", null, 1, 1, null, null, "dd-test-scanner-log-block");
// Should use the default rules with no defined action and trigger "ua0-600-56x"
// Should block and return a 403

var spans = new List<MockSpan>();
spans.AddRange(spans1);
spans.AddRange(spans2);
spans.AddRange(spans3);
spans.AddRange(spans4);
spans.AddRange(spans5);
spans.AddRange(spans6);

await VerifySpans(spans.ToImmutableList(), settings);
}
Expand All @@ -52,6 +77,11 @@ private string GetRules(string version)
{
return File.ReadAllText("remote-rules.json").Replace("{VERSION}", version);
}

private string GetNonBlockingRules()
{
return File.ReadAllText("remote-rules-override-blocking.json");
}
}
}
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
{
"version": "2.2",
"metadata": {
"rules_version": "18.18.18"
},
"rules": [
{
"id": "new-test-non-blocking",
"name": "Datadog test scanner - NON blocking version: user-agent",
"tags": {
"type": "attack_tool",
"category": "attack_attempt",
"cwe": "200",
"capec": "1000/118/169",
"tool_name": "Datadog Canary Test",
"confidence": "1"
},
"conditions": [
{
"parameters": {
"inputs": [
{
"address": "server.request.headers.no_cookies",
"key_path": [
"user-agent"
]
},
{
"address": "grpc.server.request.metadata",
"key_path": [
"dd-canary"
]
}
],
"regex": "^dd-test-scanner-log-block(?:$|/|\\s)"
},
"operator": "match_regex"
}
],
"transformers": []
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -139,5 +139,137 @@
MetaStruct: {
appsec:
}
},
{
TraceId: Id_7,
SpanId: Id_8,
Name: aspnet_core.request,
Resource: GET /home/index,
Service: Samples.Security.AspNetCore5,
Type: web,
Tags: {
actor.ip: 86.242.244.246,
appsec.event: true,
aspnet_core.endpoint: Samples.Security.AspNetCore5.Controllers.HomeController.Index (Samples.Security.AspNetCore5),
aspnet_core.route: {controller=home}/{action=index}/{id?},
component: aspnet_core,
env: integration_tests,
http.client_ip: 127.0.0.1,
http.endpoint: {controller=home}/{action=index}/{id?},
http.method: GET,
http.request.headers.host: localhost:00000,
http.request.headers.user-agent: dd-test-scanner-log-block,
http.request.headers.x-forwarded-for: 86.242.244.246,
http.response.headers.content-type: text/html; charset=utf-8,
http.route: {controller=home}/{action=index}/{id?},
http.status_code: 200,
http.url: http://localhost:00000/,
http.useragent: dd-test-scanner-log-block,
language: dotnet,
network.client.ip: 127.0.0.1,
runtime-id: Guid_1,
span.kind: server,
_dd.appsec.json: {"triggers":[{"rule":{"id":"new-test-non-blocking","name":"Datadog test scanner - NON blocking version: user-agent","tags":{"category":"attack_attempt","type":"attack_tool"}},"rule_matches":[{"operator":"match_regex","operator_value":"^dd-test-scanner-log-block(?:$|/|\\s)","parameters":[{"address":"server.request.headers.no_cookies","highlight":["dd-test-scanner-log-block"],"key_path":["user-agent","0"],"value":"dd-test-scanner-log-block"}]}]}]},
_dd.origin: appsec,
_dd.runtime_family: dotnet
},
Metrics: {
process_id: 0,
_dd.appsec.enabled: 1.0,
_dd.appsec.waf.duration: 0.0,
_dd.appsec.waf.duration_ext: 0.0,
_dd.top_level: 1.0,
_dd.tracer_kr: 1.0,
_sampling_priority_v1: 2.0
},
MetaStruct: {
appsec:
}
},
{
TraceId: Id_9,
SpanId: Id_10,
Name: aspnet_core.request,
Resource: GET /,
Service: Samples.Security.AspNetCore5,
Type: web,
Tags: {
actor.ip: 86.242.244.246,
appsec.blocked: true,
appsec.event: true,
component: aspnet_core,
env: integration_tests,
http.client_ip: 127.0.0.1,
http.method: GET,
http.request.headers.host: localhost:00000,
http.request.headers.user-agent: dd-test-scanner-log-block,
http.request.headers.x-forwarded-for: 86.242.244.246,
http.response.headers.content-type: application/json,
http.status_code: 405,
http.url: http://localhost:00000/,
http.useragent: dd-test-scanner-log-block,
language: dotnet,
network.client.ip: 127.0.0.1,
runtime-id: Guid_1,
span.kind: server,
_dd.appsec.json: {"triggers":[{"rule":{"id":"ua0-600-56x","name":"Datadog test scanner - blocking version: user-agent","tags":{"category":"attack_attempt","type":"attack_tool"}},"rule_matches":[{"operator":"match_regex","operator_value":"^dd-test-scanner-log-block(?:$|/|\\s)","parameters":[{"address":"server.request.headers.no_cookies","highlight":["dd-test-scanner-log-block"],"key_path":["user-agent","0"],"value":"dd-test-scanner-log-block"}]}]}]},
_dd.origin: appsec,
_dd.runtime_family: dotnet
},
Metrics: {
process_id: 0,
_dd.appsec.enabled: 1.0,
_dd.appsec.waf.duration: 0.0,
_dd.appsec.waf.duration_ext: 0.0,
_dd.top_level: 1.0,
_dd.tracer_kr: 1.0,
_sampling_priority_v1: 2.0
},
MetaStruct: {
appsec:
}
},
{
TraceId: Id_11,
SpanId: Id_12,
Name: aspnet_core.request,
Resource: GET /,
Service: Samples.Security.AspNetCore5,
Type: web,
Tags: {
actor.ip: 86.242.244.246,
appsec.blocked: true,
appsec.event: true,
component: aspnet_core,
env: integration_tests,
http.client_ip: 127.0.0.1,
http.method: GET,
http.request.headers.host: localhost:00000,
http.request.headers.user-agent: dd-test-scanner-log-block,
http.request.headers.x-forwarded-for: 86.242.244.246,
http.response.headers.content-type: application/json,
http.status_code: 403,
http.url: http://localhost:00000/,
http.useragent: dd-test-scanner-log-block,
language: dotnet,
network.client.ip: 127.0.0.1,
runtime-id: Guid_1,
span.kind: server,
_dd.appsec.json: {"triggers":[{"rule":{"id":"ua0-600-56x","name":"Datadog test scanner - blocking version: user-agent","tags":{"category":"attack_attempt","type":"attack_tool"}},"rule_matches":[{"operator":"match_regex","operator_value":"^dd-test-scanner-log-block(?:$|/|\\s)","parameters":[{"address":"server.request.headers.no_cookies","highlight":["dd-test-scanner-log-block"],"key_path":["user-agent","0"],"value":"dd-test-scanner-log-block"}]}]}]},
_dd.origin: appsec,
_dd.runtime_family: dotnet
},
Metrics: {
process_id: 0,
_dd.appsec.enabled: 1.0,
_dd.appsec.waf.duration: 0.0,
_dd.appsec.waf.duration_ext: 0.0,
_dd.top_level: 1.0,
_dd.tracer_kr: 1.0,
_sampling_priority_v1: 2.0
},
MetaStruct: {
appsec:
}
}
]

0 comments on commit 9dc9b77

Please sign in to comment.