[ASM][ATO] Collect session id at all times

tracer/src/Datadog.Trace/AppSec/Coordinator/SecurityCoordinator.Framework.cs

@@ -278,7 +278,8 @@ private SecurityCoordinator(Security security, Span span, HttpTransport transpor
     /// </summary>
     internal void BlockAndReport(Dictionary<string, object> args, bool lastWafCall = false, bool isInHttpTracingModule = false)
     {
-        var result = RunWaf(args, lastWafCall);
+        var sessionId = _httpTransport.Context.Session?.SessionID;
+        var result = RunWaf(args, lastWafCall, sessionId: sessionId);
         if (result is not null)
         {
             var reporting = Reporter.MakeReportingFunction(result);

tracer/src/Datadog.Trace/AppSec/Coordinator/SecurityCoordinator.cs

@@ -49,7 +49,7 @@ internal readonly partial struct SecurityCoordinator
 
     public bool IsAdditiveContextDisposed() => _httpTransport.IsAdditiveContextDisposed();
 
-    public IResult? RunWaf(Dictionary<string, object> args, bool lastWafCall = false, bool runWithEphemeral = false, bool isRasp = false)
+    public IResult? RunWaf(Dictionary<string, object> args, bool lastWafCall = false, bool runWithEphemeral = false, bool isRasp = false, string? sessionId = null)
     {
         SecurityReporter.LogAddressIfDebugEnabled(args);
         IResult? result = null;
@@ -63,6 +63,12 @@ internal readonly partial struct SecurityCoordinator
                 return null;
             }
 
+            var shouldAddSessionId = additiveContext.ShouldRunWithSession(_security, sessionId);
+            if (shouldAddSessionId)
+            {
+                args[AddressesConstants.UserSessionId] = sessionId!;
+            }
+
             _security.ApiSecurity.ShouldAnalyzeSchema(lastWafCall, _localRootSpan, args, _httpTransport.StatusCode?.ToString(), _httpTransport.RouteData);
 
             // run the WAF and execute the results

tracer/src/Datadog.Trace/AppSec/Coordinator/SecurityCoordinatorHelpers.Core.cs

@@ -7,8 +7,10 @@
 #if !NETFRAMEWORK
 using System;
 using System.Collections.Generic;
+using Datadog.Trace.AppSec.Waf;
 using Datadog.Trace.Logging;
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Mvc.Abstractions;
 using Microsoft.AspNetCore.Routing;
 
@@ -69,7 +71,7 @@ internal static void CheckReturnedHeaders(this Security security, Span span, IHe
         }
     }
 
-    internal static void CheckPathParams(this Security security, HttpContext context, Span span, IDictionary<string, object> pathParams)
+    internal static void CheckPathParamsAndSessionId(this Security security, HttpContext context, Span span, IDictionary<string, object> pathParams)
     {
         if (security.AppsecEnabled)
         {
@@ -78,7 +80,17 @@ internal static void CheckPathParams(this Security security, HttpContext context
             {
                 var securityCoordinator = SecurityCoordinator.Get(security, span, transport);
                 var args = new Dictionary<string, object> { { AddressesConstants.RequestPathParams, pathParams } };
-                var result = securityCoordinator.RunWaf(args);
+                IResult? result;
+                // we need these tests for netcoreapp3.1 and lower, as otherwise it throws
+                if (context.Features.Get<ISessionFeature>() is not null && (context.Session?.IsAvailable ?? false))
+                {
+                    result = securityCoordinator.RunWaf(args, sessionId: context.Session.Id);
+                }
+                else
+                {
+                    result = securityCoordinator.RunWaf(args);
+                }
+
                 securityCoordinator.BlockAndReport(result);
             }
         }

tracer/src/Datadog.Trace/AppSec/Waf/Context.cs

@@ -66,32 +66,26 @@ private Context(IntPtr contextHandle, IWaf waf, IWafLibraryInvoker wafLibraryInv
     public Dictionary<string, object> ShouldRunWith(IDatadogSecurity security, string? userId = null, string? userLogin = null, string? userSessionId = null, bool fromSdk = false)
     {
         var addresses = new Dictionary<string, object>();
-        ShouldRun(_userEventsState.Id, userId, AddressesConstants.UserId);
-        ShouldRun(_userEventsState.Login, userLogin, AddressesConstants.UserLogin);
-        ShouldRun(_userEventsState.SessionId, userSessionId, AddressesConstants.UserSessionId);
-        return addresses;
+        if (ShouldRunWith(security, _userEventsState.Id, userId, AddressesConstants.UserId, fromSdk))
+        {
+            addresses[AddressesConstants.UserId] = userId!;
+        }
 
-        void ShouldRun(UserEventsState.UserRecord? userRecord, string? value, string address)
+        if (ShouldRunWith(security, _userEventsState.Login, userLogin, AddressesConstants.UserLogin, fromSdk))
         {
-            string? previousValue = null;
-            var cameFromSdk = false;
-            if (userRecord.HasValue)
-            {
-                previousValue = userRecord.Value.Value;
-                cameFromSdk = userRecord.Value.FromSdk;
-            }
+            addresses[AddressesConstants.UserLogin] = userLogin!;
+        }
 
-            if (value is not null && security.AddressEnabled(address))
-            {
-                var differentValue = string.Compare(previousValue, value, StringComparison.OrdinalIgnoreCase) is not 0;
-                if (differentValue && (fromSdk || !cameFromSdk))
-                {
-                    addresses[address] = value;
-                }
-            }
+        if (ShouldRunWith(security, _userEventsState.SessionId, userSessionId, AddressesConstants.UserSessionId, fromSdk))
+        {
+            addresses[AddressesConstants.UserSessionId] = userSessionId!;
         }
+
+        return addresses;
     }
 
+    public bool ShouldRunWithSession(IDatadogSecurity security, string? userSessionId = null, bool fromSdk = false) => ShouldRunWith(security, _userEventsState.SessionId, userSessionId, AddressesConstants.UserSessionId, fromSdk);
+
     public void CommitUserRuns(Dictionary<string, object> addresses, bool fromSdk)
     {
         if (addresses.TryGetValue(AddressesConstants.UserId, out var address))
@@ -110,6 +104,25 @@ public void CommitUserRuns(Dictionary<string, object> addresses, bool fromSdk)
         }
     }
 
+    private bool ShouldRunWith(IDatadogSecurity security, UserEventsState.UserRecord? userRecord, string? value, string address, bool fromSdk)
+    {
+        if (value is null || !security.AddressEnabled(address))
+        {
+            return false;
+        }
+
+        string? previousValue = null;
+        var previousValueFromSdk = false;
+        if (userRecord.HasValue)
+        {
+            previousValue = userRecord.Value.Value;
+            previousValueFromSdk = userRecord.Value.FromSdk;
+        }
+
+        var differentValue = string.Compare(previousValue, value, StringComparison.OrdinalIgnoreCase) is not 0;
+        return differentValue && (fromSdk || !previousValueFromSdk);
+    }
+
     private unsafe IResult? RunInternal(IDictionary<string, object>? persistentAddressData, IDictionary<string, object>? ephemeralAddressData, ulong timeoutMicroSeconds, bool isRasp = false)
     {
         DdwafResultStruct retNative = default;

tracer/src/Datadog.Trace/AppSec/Waf/IContext.cs

@@ -18,5 +18,7 @@ internal interface IContext : IDisposable
 
     Dictionary<string, object> ShouldRunWith(IDatadogSecurity security, string? userId = null, string? userLogin = null, string? userSessionId = null, bool fromSdk = false);
 
+    bool ShouldRunWithSession(IDatadogSecurity security, string? userSessionId = null, bool fromSdk = false);
+
     void CommitUserRuns(Dictionary<string, object> addresses, bool fromSdk);
 }

tracer/src/Datadog.Trace/DiagnosticListeners/AspNetCoreDiagnosticObserver.cs

@@ -564,7 +564,7 @@ private void OnRoutingEndpointMatched(object arg)
                     tags.HttpRoute = normalizedRoute;
                 }
 
-                CurrentSecurity.CheckPathParams(httpContext, rootSpan, routeValues);
+                CurrentSecurity.CheckPathParamsAndSessionId(httpContext, rootSpan, routeValues);
 
                 if (CurrentIast.Settings.Enabled)
                 {

tracer/src/Datadog.Trace/SpanExtensions.Core.cs

@@ -17,23 +17,23 @@ namespace Datadog.Trace
     /// </summary>
     public static partial class SpanExtensions
     {
-        private static void RunBlockingCheck(Span span, string userId)
+        private static void RunBlockingCheck(Span span, string userId, string userSession)
         {
             var security = Security.Instance;
 
             if (security.AppsecEnabled && AspNetCoreAvailabilityChecker.IsAspNetCoreAvailable())
             {
-                RunBlockingCheckUnsafe(security, span, userId);
+                RunBlockingCheckUnsafe(security, span, userId, userSession);
             }
 
             // Don't inline this, so we don't load the aspnetcore types if they're not available
             [MethodImpl(MethodImplOptions.NoInlining)]
-            static void RunBlockingCheckUnsafe(Security security, Span span, string userId)
+            static void RunBlockingCheckUnsafe(Security security, Span span, string userId, string userSession)
             {
                 if (CoreHttpContextStore.Instance.Get() is { } httpContext)
                 {
                     var securityCoordinator = SecurityCoordinator.Get(security, span, httpContext);
-                    var result = securityCoordinator.RunWafForUser(userId: userId, fromSdk: true);
+                    var result = securityCoordinator.RunWafForUser(userId: userId, userSessionId: userSession, fromSdk: true);
                     securityCoordinator.BlockAndReport(result);
                 }
             }

tracer/src/Datadog.Trace/SpanExtensions.Framework.cs

@@ -20,7 +20,7 @@ namespace Datadog.Trace
     /// </summary>
     public static partial class SpanExtensions
     {
-        private static void RunBlockingCheck(Span span, string userId)
+        private static void RunBlockingCheck(Span span, string userId, string userSessionId)
         {
             var security = Security.Instance;
 
@@ -32,7 +32,7 @@ private static void RunBlockingCheck(Span span, string userId)
                     return;
                 }
 
-                var result = securityCoordinator.Value.RunWafForUser(userId, fromSdk: true);
+                var result = securityCoordinator.Value.RunWafForUser(userId, userSessionId, fromSdk: true);
                 securityCoordinator.Value.BlockAndReport(result);
             }
         }

tracer/src/Datadog.Trace/SpanExtensions.cs

@@ -86,7 +86,7 @@ internal static void SetUserInternal(this ISpan span, UserDetails userDetails)
 
             if (spanClass != null)
             {
-                RunBlockingCheck(spanClass, userDetails.Id);
+                RunBlockingCheck(spanClass, userDetails.Id, userDetails.SessionId);
             }
         }
 

tracer/test/Datadog.Trace.Security.IntegrationTests/ApiSecurity/AspNetCoreApiSecurity.cs

@@ -33,7 +33,6 @@ protected AspNetCoreApiSecurity(AspNetCoreTestFixture fixture, ITestOutputHelper
         Directory.CreateDirectory(LogDirectory);
         EnvironmentHelper.CustomEnvironmentVariables.Add(ConfigurationKeys.AppSec.Rules, Path.Combine("ApiSecurity", "ruleset-with-block.json"));
         // necessary as the developer middleware prevents the right blocking response
-        EnvironmentHelper.CustomEnvironmentVariables.Add("ASPNETCORE_ENVIRONMENT", "Production");
         SetEnvironmentVariable(ConfigurationKeys.LogDirectory, LogDirectory);
         SetEnvironmentVariable(ConfigurationKeys.AppSec.ApiSecurityEnabled, enableApiSecurity.ToString());
     }

tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetBase.cs

@@ -83,6 +83,8 @@ public AspNetBase(string sampleName, ITestOutputHelper outputHelper, string shut
 
             _clearMetaStruct = clearMetaStruct;
             SetEnvironmentVariable(ConfigurationKeys.AppSec.ApiSecurityEnabled, "false");
+            // without this, the developer exception page intercepts our blocking middleware and doesn't let us write the proper response
+            SetEnvironmentVariable("ASPNETCORE_ENVIRONMENT", "Production");
         }
 
         protected bool IncludeAllHttpSpans { get; set; } = false;
@@ -114,10 +116,26 @@ 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)
+        /// <summary>
+        /// Will call verify for this type of request and add the right scrubbers and right serialization methods
+        /// </summary>
+        /// <param name="agent">agent</param>
+        /// <param name="url">url</param>
+        /// <param name="body">body</param>
+        /// <param name="expectedSpans">expected spans</param>
+        /// <param name="spansPerRequest">spans per request</param>
+        /// <param name="settings">settings</param>
+        /// <param name="contentType">content type</param>
+        /// <param name="testInit">are we testing first spans at app start</param>
+        /// <param name="userAgent">user agent</param>
+        /// <param name="methodNameOverride">override the method name</param>
+        /// <param name="fileNameOverride">override the file name</param>
+        /// <param name="scrubCookiesFingerprint">only scrub session fingerprint part that changes every request. in some conditions we might want to scrub cookies fields and values, as an authenticated user/login might generate changing values at each request</param>
+        /// <returns>when it's finished</returns>
+        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, bool scrubCookiesFingerprint = false)
         {
             var spans = await SendRequestsAsync(agent, url, body, expectedSpans, expectedSpans * spansPerRequest, string.Empty, contentType, userAgent);
-            await VerifySpans(spans, settings, testInit, methodNameOverride, fileNameOverride: fileNameOverride);
+            await VerifySpans(spans, settings, testInit, methodNameOverride, fileNameOverride: fileNameOverride, scrubCookiesFingerprint: scrubCookiesFingerprint);
         }
 
         public void ScrubFingerprintHeaders(VerifySettings settings)
@@ -126,7 +144,7 @@ public void ScrubFingerprintHeaders(VerifySettings settings)
             settings.AddRegexScrubber(AppSecFingerPrintNetwork, "_dd.appsec.fp.http.network: <NetworkPrint>");
         }
 
-        public async Task VerifySpans(IImmutableList<MockSpan> spans, VerifySettings settings, bool testInit = false, string methodNameOverride = null, string testName = null, bool forceMetaStruct = false, string fileNameOverride = null, bool showRulesVersion = false)
+        public async Task VerifySpans(IImmutableList<MockSpan> spans, VerifySettings settings, bool testInit = false, string methodNameOverride = null, string testName = null, bool forceMetaStruct = false, string fileNameOverride = null, bool showRulesVersion = false, bool scrubCookiesFingerprint = false)
         {
             settings.ModifySerialization(
                 serializationSettings =>
@@ -189,6 +207,8 @@ public async Task VerifySpans(IImmutableList<MockSpan> spans, VerifySettings set
             settings.AddRegexScrubber(AppSecRaspWafDuration, "_dd.appsec.rasp.duration: 0.0");
             settings.AddRegexScrubber(AppSecRaspWafDurationWithBindings, "_dd.appsec.rasp.duration_ext: 0.0");
             settings.AddRegexScrubber(AppSecSpanIdRegex, "\"span_id\": XXX");
+            settings.ScrubSessionFingerprint(scrubCookiesFingerprint);
+
             if (!testInit)
             {
                 settings.AddRegexScrubber(AppSecWafVersion, string.Empty);

tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetCore5AutoUserEvents.cs

@@ -8,13 +8,11 @@
 #pragma warning disable SA1649 // File name must match first type name
 
 using System.Net;
-using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 using Datadog.Trace.AppSec.Rcm.Models.AsmData;
 using Datadog.Trace.RemoteConfigurationManagement;
 using Datadog.Trace.TestHelpers;
 using FluentAssertions;
-using VerifyTests;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -32,8 +30,6 @@ protected AspNetCore5AutoUserEvents(AspNetCoreTestFixture fixture, ITestOutputHe
             _enableSecurity = enableSecurity;
             _fixture.SetOutput(outputHelper);
             EnableRasp(false);
-            // without this, the developer exception page intercepts our blocking middleware and doesn't let us write the proper response
-            EnvironmentHelper.CustomEnvironmentVariables.Add("ASPNETCORE_ENVIRONMENT", "Production");
 
             if (userTrackingMode != null)
             {
@@ -70,7 +66,7 @@ protected async Task TestUserLoginEvent(string eventName, string bodyString)
             await TryStartApp();
             var agent = _fixture.Agent;
             var settings = VerifyHelper.GetSpanVerifierSettings(eventName, bodyString);
-            VerifyScrubber.ScrubAuthenticatedTags(settings);
+            settings.ScrubAuthenticationCollectionMode();
             await TestAppSecRequestWithVerifyAsync(
                 agent,
                 "/Account/Index",
@@ -81,7 +77,6 @@ await TestAppSecRequestWithVerifyAsync(
                 contentType: "application/x-www-form-urlencoded",
                 methodNameOverride: nameof(TestUserLoginEvent),
                 fileNameOverride: GetTestFileName(eventName));
-
             // reset memory database (useless for net7 as it runs with EF7 on app.db
             await SendRequestsAsync(_fixture.Agent, "/account/reset-memory-db");
             await SendRequestsAsync(_fixture.Agent, "/account/logout");
@@ -93,7 +88,6 @@ protected async Task TestAuthenticatedRequest()
         {
             await TryStartApp();
             var settings = VerifyHelper.GetSpanVerifierSettings();
-            VerifyScrubber.ScrubSessionFingerprint(settings);
             var request = await SubmitRequest("/Account/Index", "Input.UserName=TestUser2&Input.Password=test", contentType: "application/x-www-form-urlencoded");
             request.StatusCode.Should().Be(HttpStatusCode.OK);
             // this is for testuser2 in the in memory user store and appdb
@@ -120,7 +114,7 @@ protected async Task TestAuthenticatedRequest()
                                ]);
             request2.Should().NotBeNull();
             request2.CachedTargetFiles.Should().HaveCount(_enableSecurity ? 1 : 0);
-            await TestAppSecRequestWithVerifyAsync(_fixture.Agent, "/Account/SomeAuthenticatedAction", null, 1, 1, settings, fileNameOverride: GetTestFileName(nameof(TestAuthenticatedRequest)));
+            await TestAppSecRequestWithVerifyAsync(_fixture.Agent, "/Account/SomeAuthenticatedAction", null, 1, 1, settings, fileNameOverride: GetTestFileName(nameof(TestAuthenticatedRequest)), scrubCookiesFingerprint: true);
             // reset memory database (useless for net7 as it runs with EF7 on app.db
             await SendRequestsAsync(_fixture.Agent, "/account/reset-memory-db");
             await SendRequestsAsync(_fixture.Agent, "/account/logout");
@@ -135,7 +129,7 @@ protected async Task TestLoginWithSdk(string userIdSdk)
             await TryStartApp();
             var agent = _fixture.Agent;
             var settings = VerifyHelper.GetSpanVerifierSettings(nameof(TestLoginWithSdk), userIdSdk);
-            VerifyScrubber.ScrubAuthenticatedTags(settings);
+            VerifyScrubber.ScrubAuthenticationCollectionMode(settings);
             await TestAppSecRequestWithVerifyAsync(
                 agent,
                 $"/Account/Index?userIdSdk={userIdSdk}",
@@ -145,7 +139,8 @@ await TestAppSecRequestWithVerifyAsync(
                 settings,
                 contentType: "application/x-www-form-urlencoded",
                 methodNameOverride: nameof(TestUserLoginEvent),
-                fileNameOverride: GetTestFileName($"{nameof(TestLoginWithSdk)}.{userIdSdk}"));
+                fileNameOverride: GetTestFileName($"{nameof(TestLoginWithSdk)}.{userIdSdk}"),
+                scrubCookiesFingerprint: true);
             // reset memory database (useless for net7 as it runs with EF7 on app.db
             await SendRequestsAsync(_fixture.Agent, "/account/reset-memory-db");
             await SendRequestsAsync(_fixture.Agent, "/account/logout");