[ASM] upgrade WAF to 1.7.0 and WAF rules to 1.5.0 (#3742)

Author: robertpi

tracer/build/_build/Build.Steps.cs

@@ -58,7 +58,7 @@ partial class Build
 
     AbsolutePath NativeBuildDirectory => RootDirectory / "obj";
 
-    const string LibDdwafVersion = "1.5.1";
+    const string LibDdwafVersion = "1.7.0";
 
     const string OlderLibDdwafVersion = "1.4.0";
 
@@ -474,7 +474,8 @@ async Task DownloadWafVersion(string libddwafVersion = null, string uncompressFo
                         var (arch, _) = GetUnixArchitectureAndExtension();
                         var (archWaf, ext) = GetLibDdWafUnixArchitectureAndExtension();
                         var source = MonitoringHomeDirectory / (IsOsx ? "osx" : arch);
-                        var oldVersionPath = oldVersionTempPath / "runtimes" / archWaf / "native" / $"libddwaf.{ext}";
+                        var patchedArchWaf = (IsOsx ? archWaf + "-x64" : archWaf);
+                        var oldVersionPath = oldVersionTempPath / "runtimes" / patchedArchWaf / "native" / $"libddwaf.{ext}";
                         foreach (var fmk in frameworks)
                         {
                             // We have to copy into the _root_ test bin folder here, not the arch sub-folder.
@@ -2019,9 +2020,8 @@ private void MakeGrpcToolsExecutable()
     private (string Arch, string Ext) GetLibDdWafUnixArchitectureAndExtension() =>
         (IsOsx) switch
         {
-            // (true) => ($"osx-{UnixArchitectureIdentifier}", "dylib"), //LibDdWaf doesn't support osx-arm64 yet.
-            (true) => ($"osx-x64", "dylib"),
-            (false) => ($"linux-{UnixArchitectureIdentifier}", "so"), // LibDdWaf doesn't
+            (true) => ($"osx", "dylib"), // LibDdWaf is universal binary on osx
+            (false) => ($"linux-{UnixArchitectureIdentifier}", "so"),
         };
 
     private (string Arch, string Ext) GetUnixArchitectureAndExtension() =>

tracer/build/smoke_test_snapshots/smoke_test_snapshots.json

@@ -37,7 +37,7 @@
       "parent_id": 1,
       "type": "web",
       "meta": {
-        "_dd.appsec.waf.version": "1.5.1",
+        "_dd.appsec.waf.version": "1.7.0",
         "_dd.runtime_family": "dotnet",
         "aspnet_core.endpoint": "AspNetCoreSmokeTest.ValuesController.Get (AspNetCoreSmokeTest)",
         "aspnet_core.route": "api/values",

tracer/build/smoke_test_snapshots/smoke_test_snapshots_2_1.json

@@ -37,7 +37,7 @@
       "parent_id": 1,
       "type": "web",
       "meta": {
-        "_dd.appsec.waf.version": "1.5.1",
+        "_dd.appsec.waf.version": "1.7.0",
         "_dd.runtime_family": "dotnet",
         "aspnet_core.route": "api/values",
         "component": "aspnet_core",

tracer/src/Datadog.Trace/AppSec/Concurrency/ReaderWriterLock.Core.cs

@@ -30,7 +30,6 @@ internal class Security : IDatadogSecurity, IDisposable
         private static Security _instance;
         private static bool _globalInstanceInitialized;
         private static object _globalInstanceLock = new();
-        private readonly Concurrency.ReaderWriterLock _wafLocker = new();
         private readonly SecuritySettings _settings;
         private LibraryInitializationResult _libraryInitializationResult;
         private IWaf _waf;
@@ -328,28 +327,17 @@ private void UpdateStatus(bool fromRemoteConfig = false)
                 _wafInitializationResult = Waf.Waf.Create(_wafLibraryInvoker, _settings.ObfuscationParameterKeyRegex, _settings.ObfuscationParameterValueRegex, _settings.Rules, _remoteRulesJson);
                 if (_wafInitializationResult.Success)
                 {
-                    if (_wafLocker.EnterWriteLock())
-                    {
-                        var oldWaf = _waf;
-                        _waf = _wafInitializationResult.Waf;
-                        oldWaf?.Dispose();
-                        _wafLocker.ExitWriteLock();
-                        Log.Debug("Disposed old waf and affected new waf");
-                        UpdateRulesData();
-                        AddInstrumentationsAndProducts(fromRemoteConfig);
-                    }
-                    else
-                    {
-                        _wafInitializationResult.Waf?.Dispose();
-                        Log.Warning("Could not replace waf because the writer lock couldn't be acquired within the specified timeout {timeout}", Concurrency.ReaderWriterLock.TimeoutInMs.ToString());
-                    }
+                    var oldWaf = _waf;
+                    _waf = _wafInitializationResult.Waf;
+                    oldWaf?.Dispose();
+                    Log.Debug("Disposed old waf and affected new waf");
+                    UpdateRulesData();
+                    AddInstrumentationsAndProducts(fromRemoteConfig);
                 }
                 else
                 {
-                    _wafLocker.EnterWriteLock();
                     _waf?.Dispose();
                     _wafInitializationResult.Waf?.Dispose();
-                    _wafLocker.ExitWriteLock();
                     _settings.Enabled = false;
                 }
             }
@@ -404,15 +392,14 @@ internal void SetTraceSamplingPriority(Span span)
             }
         }
 
-        internal IContext CreateAdditiveContext() => _waf?.CreateContext(_wafLocker);
+        internal IContext CreateAdditiveContext() => _waf?.CreateContext();
 
         private void RunShutdown()
         {
             AsmRemoteConfigurationProducts.AsmDataProduct.ConfigChanged -= AsmDataProductConfigChanged;
             AsmRemoteConfigurationProducts.AsmProduct.ConfigChanged -= AsmProductConfigChanged;
             AsmRemoteConfigurationProducts.AsmFeaturesProduct.ConfigChanged -= FeaturesProductConfigChanged;
             AsmRemoteConfigurationProducts.AsmDDProduct.ConfigChanged -= AsmDDProductConfigChanged;
-            _wafLocker.Dispose();
             Dispose();
         }
     }

tracer/src/Datadog.Trace/AppSec/Concurrency/ReaderWriterLock.Framework.cs

@@ -7,7 +7,6 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
-using Datadog.Trace.AppSec.Concurrency;
 using Datadog.Trace.AppSec.Waf.NativeBindings;
 using Datadog.Trace.Logging;
 using Datadog.Trace.Util;
@@ -18,12 +17,14 @@ namespace Datadog.Trace.AppSec.Waf
     internal class Context : IContext
     {
         private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor<Context>();
+
+        // the context handle should be locked, it is not safe for concurrent access and two
+        // waf events may be processed at the same time due to code being run asynchronously
+        private readonly object _sync = new object();
         private readonly IntPtr _contextHandle;
 
         private readonly Waf _waf;
 
-        // this lock is to avoid accessing the waf object from the Context while it's being disposed. Other events involving the waf arent concurrent (rcm is not) and thread safe (other methods on native waf)
-        private readonly ReaderWriterLock _wafLocker;
         private readonly List<Obj> _argCache = new();
         private readonly Stopwatch _stopwatch;
         private readonly WafLibraryInvoker _wafLibraryInvoker;
@@ -32,9 +33,8 @@ internal class Context : IContext
         private ulong _totalRuntimeOverRuns;
 
         // Beware this class is created on a thread but can be disposed on another so don't trust the lock is not going to be held
-        private Context(IntPtr contextHandle, Waf waf, ReaderWriterLock wafLocker, WafLibraryInvoker wafLibraryInvoker)
+        private Context(IntPtr contextHandle, Waf waf, WafLibraryInvoker wafLibraryInvoker)
         {
-            _wafLocker = wafLocker;
             _contextHandle = contextHandle;
             _waf = waf;
             _wafLibraryInvoker = wafLibraryInvoker;
@@ -43,7 +43,7 @@ private Context(IntPtr contextHandle, Waf waf, ReaderWriterLock wafLocker, WafLi
 
         ~Context() => Dispose(false);
 
-        public static IContext? GetContext(IntPtr contextHandle, Waf waf, ReaderWriterLock wafLocker, WafLibraryInvoker wafLibraryInvoker)
+        public static IContext? GetContext(IntPtr contextHandle, Waf waf, WafLibraryInvoker wafLibraryInvoker)
         {
             // in high concurrency, the waf passed as argument here could have been disposed just above in between creation / waf update so last test here
             if (waf.Disposed)
@@ -52,8 +52,7 @@ private Context(IntPtr contextHandle, Waf waf, ReaderWriterLock wafLocker, WafLi
                 return null;
             }
 
-            var hasLock = wafLocker.IsReadLockHeld || wafLocker.EnterReadLock();
-            return hasLock ? new Context(contextHandle, waf, wafLocker, wafLibraryInvoker) : null;
+            return new Context(contextHandle, waf, wafLibraryInvoker);
         }
 
         public IResult? Run(IDictionary<string, object> addresses, ulong timeoutMicroSeconds)
@@ -81,7 +80,12 @@ private Context(IntPtr contextHandle, Waf waf, ReaderWriterLock wafLocker, WafLi
             _stopwatch.Start();
             using var pwArgs = Encoder.Encode(addresses, _wafLibraryInvoker, _argCache, applySafetyLimits: true);
             var rawArgs = pwArgs.RawPtr;
-            var code = _waf.Run(_contextHandle, rawArgs, ref retNative, timeoutMicroSeconds);
+
+            DDWAF_RET_CODE code;
+            lock (_sync)
+            {
+                code = _waf.Run(_contextHandle, rawArgs, ref retNative, timeoutMicroSeconds);
+            }
 
             _stopwatch.Stop();
             _totalRuntimeOverRuns += retNative.TotalRuntime / 1000;
@@ -113,9 +117,10 @@ public void Dispose(bool disposing)
                 arg.Dispose();
             }
 
-            _wafLibraryInvoker.ContextDestroy(_contextHandle);
-
-            _wafLocker.ExitReadLock();
+            lock (_sync)
+            {
+                _wafLibraryInvoker.ContextDestroy(_contextHandle);
+            }
         }
 
         public void Dispose()

tracer/src/Datadog.Trace/AppSec/Security.cs

@@ -5,7 +5,6 @@
 
 using System;
 using System.Collections.Generic;
-using Datadog.Trace.AppSec.Concurrency;
 using Datadog.Trace.AppSec.RcmModels.AsmData;
 using Datadog.Trace.AppSec.Waf.NativeBindings;
 
@@ -15,7 +14,7 @@ internal interface IWaf : IDisposable
     {
         public string Version { get; }
 
-        public IContext CreateContext(ReaderWriterLock wafLocker);
+        public IContext CreateContext();
 
         public bool UpdateRulesData(IEnumerable<RuleData> res);
 

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

@@ -180,7 +180,7 @@ internal static string GetLibName(FrameworkDescription fwk, string libVersion =
         internal static string[] GetRuntimeIds(FrameworkDescription fwk)
             => fwk.OSPlatform switch
             {
-                OSPlatformName.MacOS => new[] { $"osx-x64" },
+                OSPlatformName.MacOS => new[] { $"osx" },
                 OSPlatformName.Windows => new[] { $"win-{fwk.ProcessArchitecture}" },
                 OSPlatformName.Linux => fwk.ProcessArchitecture == ProcessArchitecture.Arm64
                                             ? new[] { "linux-arm64" }

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

@@ -72,7 +72,7 @@ internal static InitializationResult Create(WafLibraryInvoker wafLibraryInvoker,
         /// </summary>
         /// <returns>Context object to perform matching using the provided WAF instance</returns>
         /// <exception cref="Exception">Exception</exception>
-        public IContext? CreateContext(Concurrency.ReaderWriterLock wafLocker)
+        public IContext? CreateContext()
         {
             if (Disposed)
             {
@@ -88,7 +88,7 @@ internal static InitializationResult Create(WafLibraryInvoker wafLibraryInvoker,
                 throw new Exception(InitContextError);
             }
 
-            return Context.GetContext(contextHandle, this, wafLocker, _wafLibraryInvoker);
+            return Context.GetContext(contextHandle, this, _wafLibraryInvoker);
         }
 
         // Requires a non disposed waf handle