Skip to content

Commit

Permalink
[ASM] Introduce SecurityReporter for all reporting functions of Secur…
Browse files Browse the repository at this point in the history 
…ityCoordinator (#6481)

## Summary of changes

This PR is just moving code around, to allow reporting security infos on
spans without needing a security coordinator.

Basically we are extracting from `SecurityCoordinator` the reporting
functions to the already existing underused `SecurityReporter`, we are
nesting core/framework classes inside of `SecurityCoordinator` and
`SecurityReporter`.
`SecurityReporter` can function without the `Security` instance, it just
needs the span and transport to add info.

This allows for better separation of concerns and prepares things for
ATO as some contexts just need to report additional headers without
running the waf necessarily, or already, within the catch
(BlockException) inside the `BlockingMiddleware` where we just want to
report the result within the exception and don't need any security
features



## Reason for change

## Implementation details

## Test coverage

## Other details
<!-- Fixes #{issue} -->

<!-- ⚠️ Note: where possible, please obtain 2 approvals prior to
merging. Unless CODEOWNERS specifies otherwise, for external teams it is
typically best to have one review from a team member, and one review
from apm-dotnet. Trivial changes do not require 2 reviews. -->
  • Loading branch information
@anna-git
anna-git authored Dec 26, 2024
1 parent 2cdde8d commit ebc42dd
Show file tree
Hide file tree
Showing 16 changed files with 286 additions and 222 deletions.
36 changes: 10 additions & 26 deletions tracer/src/Datadog.Trace/AppSec/Coordinator/SecurityCoordinator.Core.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@ private SecurityCoordinator(Security security, Span span, HttpTransport transpor
_security = security;
_localRootSpan = TryGetRoot(span);
_httpTransport = transport;
Reporter = new SecurityReporter(_localRootSpan, transport, true);
}

private static bool CanAccessHeaders => true;

internal static SecurityCoordinator? TryGet(Security security, Span span)
{
var context = CoreHttpContextStore.Instance.Get();
Expand Down Expand Up @@ -59,27 +58,6 @@ private static void GetCookieKeyValueFromIndex(IRequestCookieCollection cookies,
value = cookie.Value;
}

[MethodImpl(MethodImplOptions.NoInlining)]
internal static void CollectHeaders(Span internalSpan)
{
if (AspNetCoreAvailabilityChecker.IsAspNetCoreAvailable())
{
CollectHeadersImpl(internalSpan);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void CollectHeadersImpl(Span internalSpan)
{
var context = CoreHttpContextStore.Instance.Get();
internalSpan = TryGetRoot(internalSpan);
if (context is not null)
{
var headers = new HeadersCollectionAdapter(context.Request.Headers);
AddRequestHeaders(internalSpan, headers);
}
}
}

internal void BlockAndReport(IResult? result)
{
if (result is not null)
Expand All @@ -89,15 +67,15 @@ internal void BlockAndReport(IResult? result)
throw new BlockException(result, result.RedirectInfo ?? result.BlockInfo!);
}

TryReport(result, result.ShouldBlock);
Reporter.TryReport(result, result.ShouldBlock);
}
}

internal void ReportAndBlock(IResult? result)
{
if (result is not null)
{
TryReport(result, result.ShouldBlock);
Reporter.TryReport(result, result.ShouldBlock);

if (result.ShouldBlock)
{
Expand Down Expand Up @@ -128,7 +106,13 @@ private Dictionary<string, object> GetBasicRequestArgsForWaf()
}
}

var addressesDictionary = new Dictionary<string, object> { { AddressesConstants.RequestMethod, request.Method }, { AddressesConstants.ResponseStatus, request.HttpContext.Response.StatusCode.ToString() }, { AddressesConstants.RequestUriRaw, request.GetUrlForWaf() }, { AddressesConstants.RequestClientIp, _localRootSpan.GetTag(Tags.HttpClientIp) } };
var addressesDictionary = new Dictionary<string, object>
{
{ AddressesConstants.RequestMethod, request.Method },
{ AddressesConstants.ResponseStatus, request.HttpContext.Response.StatusCode.ToString() },
{ AddressesConstants.RequestUriRaw, request.GetUrlForWaf() },
{ AddressesConstants.RequestClientIp, _localRootSpan.GetTag(Tags.HttpClientIp) }
};

var userId = _localRootSpan.Context?.TraceContext?.Tags.GetTag(Tags.User.Id);
if (!string.IsNullOrEmpty(userId))
Expand Down
66 changes: 8 additions & 58 deletions tracer/src/Datadog.Trace/AppSec/Coordinator/SecurityCoordinator.Framework.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,33 +27,15 @@ internal readonly partial struct SecurityCoordinator
private const string WebApiControllerHandlerTypeFullname = "System.Web.Http.WebHost.HttpControllerHandler";
private static readonly Lazy<Action<IResult, HttpStatusCode, string>?> _throwHttpResponseRedirectException = new(CreateThrowHttpResponseExceptionDynMethForRedirect);
private static readonly Lazy<Action<IResult, HttpStatusCode, string, string>?> _throwHttpResponseException = new(CreateThrowHttpResponseExceptionDynMeth);
private static readonly bool? UsingIntegratedPipeline;

static SecurityCoordinator()
{
if (UsingIntegratedPipeline == null)
{
try
{
UsingIntegratedPipeline = TryGetUsingIntegratedPipelineBool();
}
catch (Exception ex)
{
UsingIntegratedPipeline = false;
Log.Error(ex, "Unable to query the IIS pipeline. Request and response information may be limited.");
}
}
}

private SecurityCoordinator(Security security, Span span, HttpTransport transport)
{
_security = security;
_localRootSpan = TryGetRoot(span);
_httpTransport = transport;
Reporter = new SecurityReporter(_localRootSpan, transport, true);
}

private bool CanAccessHeaders => UsingIntegratedPipeline is true or null;

internal static SecurityCoordinator? TryGet(Security security, Span span)
{
if (HttpContext.Current is not { } current)
Expand Down Expand Up @@ -288,7 +270,7 @@ internal void BlockAndReport(Dictionary<string, object> args, bool lastWafCall =
var result = RunWaf(args, lastWafCall);
if (result is not null)
{
var reporting = MakeReportingFunction(result);
var reporting = Reporter.MakeReportingFunction(result);

if (result.ShouldBlock)
{
Expand All @@ -304,7 +286,7 @@ internal void ReportAndBlock(IResult? result)
{
if (result is not null)
{
var reporting = MakeReportingFunction(result);
var reporting = Reporter.MakeReportingFunction(result);
reporting(null, result.ShouldBlock);

if (result.ShouldBlock)
Expand All @@ -323,20 +305,6 @@ internal void ReportAndBlock(IResult? result)
}
}

private Action<int?, bool> MakeReportingFunction(IResult result)
{
var securityCoordinator = this;
return (status, blocked) =>
{
if (result.ShouldBlock)
{
securityCoordinator._httpTransport.MarkBlocked();
}

securityCoordinator.TryReport(result, blocked, status);
};
}

private void ChooseBlockingMethodAndBlock(IResult result, Action<int?, bool> reporting, Dictionary<string, object?>? blockInfo, Dictionary<string, object?>? redirectInfo)
{
var headers = RequestDataHelper.GetHeaders(_httpTransport.Context.Request) ?? new NameValueCollection();
Expand Down Expand Up @@ -371,7 +339,7 @@ private void WriteAndEndResponse(BlockingAction blockingAction)
httpResponse.Cookies.Clear();

// cant clear headers, on some iis version we get a platform not supported exception
if (CanAccessHeaders)
if (Reporter.CanAccessHeaders)
{
var keys = httpResponse.Headers.Keys.Cast<string>().ToList();
foreach (var key in keys)
Expand Down Expand Up @@ -400,8 +368,8 @@ private void WriteAndEndResponse(BlockingAction blockingAction)
public Dictionary<string, object> GetBasicRequestArgsForWaf()
{
var request = _httpTransport.Context.Request;
var headers = RequestDataHelper.GetHeaders(request);
var headersDic = ExtractHeadersFromRequest(request.Headers);
var headers = request.Headers;
var headersDic = ExtractHeaders(headers.AllKeys, key => GetHeaderValueForWaf(headers, key));
var cookiesDic = ExtractCookiesFromRequest(request);

var queryString = RequestDataHelper.GetQueryString(request);
Expand Down Expand Up @@ -473,8 +441,6 @@ public Dictionary<string, object> GetBasicRequestArgsForWaf()
return dict;
}

internal static Dictionary<string, object> ExtractHeadersFromRequest(NameValueCollection headers) => ExtractHeaders(headers.AllKeys, key => GetHeaderValueForWaf(headers, key));

private static object GetHeaderAsArray(string[] value) => value.Length == 1 ? value[0] : value;

private static object GetHeaderValueForWaf(NameValueCollection headers, string currentKey) => GetHeaderAsArray(RequestDataHelper.GetNameValueCollectionValues(headers, currentKey) ?? []);
Expand Down Expand Up @@ -511,35 +477,19 @@ public Dictionary<string, object> GetResponseHeadersForWaf()
return headersDic;
}

internal static void CollectHeaders(Span internalSpan)
{
var context = HttpContext.Current;

if (context != null)
{
var headers = new NameValueHeadersCollection(context.Request.Headers);
AddRequestHeaders(internalSpan, headers);
}
}

internal class HttpTransport : HttpTransportBase
internal class HttpTransport(HttpContext context) : HttpTransportBase
{
private const string WafKey = "waf";

private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor<HttpTransport>();

private static bool _canReadHttpResponseHeaders = true;

public HttpTransport(HttpContext context)
{
Context = context;
}

internal override bool IsBlocked => Context.Items[BlockingAction.BlockDefaultActionName] is true;

internal override int StatusCode => Context.Response.StatusCode;

public override HttpContext Context { get; }
public override HttpContext Context { get; } = context;

internal override IDictionary<string, object>? RouteData => Context.Request.RequestContext.RouteData?.Values;

Expand Down
75 changes: 6 additions & 69 deletions tracer/src/Datadog.Trace/AppSec/Coordinator/SecurityCoordinator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,12 @@
#pragma warning disable CS0282
using System;
using System.Collections.Generic;
using System.Text;
using Datadog.Trace.AppSec.Waf;
using Datadog.Trace.Logging;
using Datadog.Trace.Telemetry;
using Datadog.Trace.Telemetry.Metrics;
using Datadog.Trace.Util;
using Datadog.Trace.Vendors.Serilog.Events;
#if !NETFRAMEWORK
using Microsoft.AspNetCore.Http;
#else
using System.Collections.Specialized;
using System.Web;
#endif

Expand All @@ -36,32 +31,9 @@ internal readonly partial struct SecurityCoordinator

public bool IsBlocked => _httpTransport.IsBlocked;

public void MarkBlocked() => _httpTransport.MarkBlocked();
public SecurityReporter Reporter { get; }

private static void LogMatchesIfDebugEnabled(IReadOnlyCollection<object>? results, bool blocked)
{
if (Log.IsEnabled(LogEventLevel.Debug) && results != null)
{
foreach (var result in results)
{
if (result is Dictionary<string, object?> match)
{
if (blocked)
{
Log.Debug("DDAS-0012-02: Blocking current transaction (rule: {RuleId})", match["rule"]);
}
else
{
Log.Debug("DDAS-0012-01: Detecting an attack from rule {RuleId}", match["rule"]);
}
}
else
{
Log.Debug("{Result} not of expected type", result);
}
}
}
}
public void MarkBlocked() => _httpTransport.MarkBlocked();

public IResult? Scan(bool lastTime = false)
{
Expand All @@ -73,7 +45,7 @@ private static void LogMatchesIfDebugEnabled(IReadOnlyCollection<object>? result

public IResult? RunWaf(Dictionary<string, object> args, bool lastWafCall = false, bool runWithEphemeral = false, bool isRasp = false)
{
LogAddressIfDebugEnabled(args);
SecurityReporter.LogAddressIfDebugEnabled(args);
IResult? result = null;
try
{
Expand Down Expand Up @@ -108,7 +80,7 @@ private static void LogMatchesIfDebugEnabled(IReadOnlyCollection<object>? result
result = additiveContext.Run(args, _security.Settings.WafTimeoutMicroSeconds);
}

RecordTelemetry(result);
SecurityReporter.RecordTelemetry(result);
}
}
catch (Exception ex) when (ex is not BlockException)
Expand All @@ -130,46 +102,13 @@ private static void LogMatchesIfDebugEnabled(IReadOnlyCollection<object>? result
return result;
}

private static void RecordTelemetry(IResult? result)
{
if (result == null)
{
return;
}

if (result.Timeout)
{
TelemetryFactory.Metrics.RecordCountWafRequests(MetricTags.WafAnalysis.WafTimeout);
}
else if (result.ShouldBlock)
{
TelemetryFactory.Metrics.RecordCountWafRequests(MetricTags.WafAnalysis.RuleTriggeredAndBlocked);
}
else if (result.ShouldReportSecurityResult)
{
TelemetryFactory.Metrics.RecordCountWafRequests(MetricTags.WafAnalysis.RuleTriggered);
}
else
{
TelemetryFactory.Metrics.RecordCountWafRequests(MetricTags.WafAnalysis.Normal);
}
}

public void AddResponseHeadersToSpanAndCleanup()
{
if (_localRootSpan.IsAppsecEvent())
{
AddResponseHeaderTags(CanAccessHeaders);
}

_httpTransport.DisposeAdditiveContext();
}
internal static Span TryGetRoot(Span span) => span.Context.TraceContext?.RootSpan ?? span;

internal static Dictionary<string, object>? ExtractCookiesFromRequest(HttpRequest request)
{
var cookies = RequestDataHelper.GetCookies(request);

if (cookies is not null && cookies.Count is > 0)
if (cookies is { Count: > 0 })
{
var cookiesCount = cookies.Count;
var cookiesDic = new Dictionary<string, object>(cookiesCount);
Expand Down Expand Up @@ -234,6 +173,4 @@ private static Dictionary<string, object> ExtractHeaders(ICollection<string> key

return headersDic;
}

private static Span TryGetRoot(Span span) => span.Context.TraceContext?.RootSpan ?? span;
}
47 changes: 47 additions & 0 deletions tracer/src/Datadog.Trace/AppSec/Coordinator/SecurityReporter.Core.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// <copyright file="SecurityReporter.Core.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

#nullable enable
#if !NETFRAMEWORK
using System.Runtime.CompilerServices;
using Datadog.Trace.Headers;

namespace Datadog.Trace.AppSec.Coordinator;

internal partial class SecurityReporter
{
private bool CanAccessHeaders => true;

/// <summary>
/// Outside of a web context this can't work and there are no web assemblies to load so without the no inlining, this would cause a load assembly exception
/// </summary>
/// <param name="span">the span to report on</param>
/// <param name="searchRootSpan">should we fetch the root span for you</param>
internal static void SafeCollectHeaders(Span span, bool searchRootSpan = true)
{
if (AspNetCoreAvailabilityChecker.IsAspNetCoreAvailable())
{
CollectHeadersSafe(searchRootSpan);
}

[MethodImpl(MethodImplOptions.NoInlining)]
void CollectHeadersSafe(bool searchRootSpanImpl)
{
var context = CoreHttpContextStore.Instance.Get();
if (context is not null)
{
var securityReporter = new SecurityReporter(span, new SecurityCoordinator.HttpTransport(context), !searchRootSpanImpl);
securityReporter.CollectHeaders();
}
}
}

internal void CollectHeaders()
{
var headers = new HeadersCollectionAdapter(_httpTransport.Context.Request.Headers);
AddRequestHeaders(headers);
}
}
#endif
Loading

0 comments on commit ebc42dd

Please sign in to comment.