[ASM][ATO] Collect session id at all times (#6623)

## Summary of changes

Collects session id in any case:
- .net core: on endpoint matched
- net framework: try access it through httpcontext
User doesn't need to be authenticated anymore for it to collect the
session id.

## Reason for change

Session id should always be collected, not only when user is
authenticated.

## Implementation details

there are guards for netcore as accessing Session when it's not been
setup just throws. So this way we make sure it's setup for the webapp.

## Test coverage

Change all snapshots.
Scrubbing pitfalls: 
- scrub cookies and session id for aspnet mvc because if there's a
session id it will always change the cookies
- scrub cookies and session id for aspnet core when user is
authenticated / logged in (as it will keep changing)
- other cases in aspnet core: just scrub session fingerprint
## 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. -->

Author: anna-git

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
@@ -105,7 +111,7 @@ internal readonly partial struct SecurityCoordinator
         try
         {
             var additiveContext = GetOrCreateAdditiveContext();
-            if (additiveContext?.ShouldRunWith(_security, userId, userLogin, userSessionId, fromSdk) is { Count: > 0 } userAddresses)
+            if (additiveContext?.FilterAddresses(_security, userId, userLogin, userSessionId, fromSdk) is { Count: > 0 } userAddresses)
             {
                 if (otherTags is not null)
                 {

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 to check context.Features.Get<ISessionFeature> as accessing the Session item if session has not been configured for the application is throwing InvalidOperationException
+                if (context.Features.Get<ISessionFeature>() is { Session.IsAvailable: true } feature)
+                {
+                    result = securityCoordinator.RunWaf(args, sessionId: feature.Session.Id);
+                }
+                else
+                {
+                    result = securityCoordinator.RunWaf(args);
+                }
+
                 securityCoordinator.BlockAndReport(result);
             }
         }

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

@@ -63,35 +63,29 @@ private Context(IntPtr contextHandle, IWaf waf, IWafLibraryInvoker wafLibraryInv
     public IResult? RunWithEphemeral(IDictionary<string, object> ephemeralAddressData, ulong timeoutMicroSeconds, bool isRasp)
         => RunInternal(null, ephemeralAddressData, timeoutMicroSeconds, isRasp);
 
-    public Dictionary<string, object> ShouldRunWith(IDatadogSecurity security, string? userId = null, string? userLogin = null, string? userSessionId = null, bool fromSdk = false)
+    public Dictionary<string, object> FilterAddresses(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,24 @@ public void CommitUserRuns(Dictionary<string, object> addresses, bool fromSdk)
         }
     }
 
+    private bool ShouldRunWith(IDatadogSecurity security, UserEventsState.UserRecord? previousUserRecord, string? value, string address, bool fromSdk)
+    {
+        if (value is null || !security.AddressEnabled(address))
+        {
+            return false;
+        }
+
+        if (!previousUserRecord.HasValue)
+        {
+            return true;
+        }
+
+        var previousValue = previousUserRecord.Value.Value;
+        var previousValueFromSdk = previousUserRecord.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

@@ -16,7 +16,26 @@ internal interface IContext : IDisposable
 
     IResult? RunWithEphemeral(IDictionary<string, object> ephemeralAddressData, ulong timeoutMicroSeconds, bool isRasp);
 
-    Dictionary<string, object> ShouldRunWith(IDatadogSecurity security, string? userId = null, string? userLogin = null, string? userSessionId = null, bool fromSdk = false);
+    /// <summary>
+    /// Here we compare with potential previous runs, and make sure we don't override previous values provided by the SDK
+    /// </summary>
+    /// <param name="security">security main instance</param>
+    /// <param name="userId">user id</param>
+    /// <param name="userLogin">user login</param>
+    /// <param name="userSessionId">user session id</param>
+    /// <param name="fromSdk">is this coming from sdk</param>
+    /// <returns>filtered addresses, potentially empty if nothing should precede over sdk or same values than before</returns>
+    Dictionary<string, object> FilterAddresses(IDatadogSecurity security, string? userId = null, string? userLogin = null, string? userSessionId = null, bool fromSdk = false);
+
+    /// <summary>
+    /// Special method called by RunWaf to make sure we don't override a session id provided by the sdk or run with the same value.
+    /// It's similar as FilterAddresses but only for session id, behind the scenes they both call ShouldRunWith()
+    /// </summary>
+    /// <param name="security">security main instance</param>
+    /// <param name="userSessionId">user session id</param>
+    /// <param name="fromSdk">is this coming from sdk</param>
+    /// <returns>whether we should add this address to the waf run</returns>
+    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

@@ -32,8 +32,6 @@ protected AspNetCoreApiSecurity(AspNetCoreTestFixture fixture, ITestOutputHelper
         _fixture.SetOutput(outputHelper);
         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());
     }