Use Microsoft.Security.Utilities.Core package for the 'new' secret masker (#5169)

When a new feature flag `DistributedTask.Agent.EnableNewMaskerAndRegexes` is enabled, a new corresponding agent knob `AZP_ENABLE_NEW_MASKER_AND_REGEXES` is turned on, we replace the use of `Microsoft.TeamFoundation.DistributedTask.Logging.SecretMasker` with `Microsoft.Security.Utilities.SecurityMasker` from the https://www.nuget.org/packages/Microsoft.Security.Utilities.Core package and https://github.com/microsoft/security-utilities repo.

This flag also enables the package's `WellKnownRegexPatterns.PreciselyClassifiedSecurityKeys` regex patterns. This class of pattern has high confidence, effectively admitting no false positives. It is also strongly oriented on detecting the latest Azure provider API key formats.

The new knob/flag pair replaces `DistributedTask.Agent.UseMaskingPerformanceEnhancements`/`AZP_ENABLE_NEW_SECRET_MASKER`, which would use a built-in `SecretMasker` that was implemented in this repo, along with a subset of the aforementioned `PreciselyClassifiedSecurityKeys` patterns also re-implemented in this repo. That code is therefore deleted in this change.

Note that in the absence of feature flags, the default behavior remains unchanged.

Benchmarking shows masking with the new feature enabled outperforming masking with it disabled even though the new feature also brings the added value of 30 additional patterns.

Co-authored-by: Michael C. Fanning <[email protected]>

Author: nguerrera

src/Agent.Listener/JobDispatcher.cs

@@ -398,11 +398,11 @@ private async Task RunAsync(Pipelines.AgentJobRequestMessage message, WorkerDisp
                 {
 
                     var featureFlagProvider = HostContext.GetService<IFeatureFlagProvider>();
-                    var newSecretMaskerFeaturFlagStatus = await featureFlagProvider.GetFeatureFlagAsync(HostContext, "DistributedTask.Agent.UseMaskingPerformanceEnhancements", Trace);
+                    var newMaskerAndRegexesFeatureFlagStatus = await featureFlagProvider.GetFeatureFlagAsync(HostContext, "DistributedTask.Agent.EnableNewMaskerAndRegexes", Trace);
                     var environment = new Dictionary<string, string>();
-                    if (newSecretMaskerFeaturFlagStatus?.EffectiveState == "On")
+                    if (newMaskerAndRegexesFeatureFlagStatus?.EffectiveState == "On")
                     {
-                        environment.Add("AZP_ENABLE_NEW_SECRET_MASKER", "true");
+                        environment.Add("AZP_ENABLE_NEW_MASKER_AND_REGEXES", "true");
                     }
                     // Start the process channel.
                     // It's OK if StartServer bubbles an execption after the worker process has already started.

src/Agent.Sdk/Agent.Sdk.csproj

@@ -7,6 +7,7 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <PackageReference Include="Microsoft.Security.Utilities.Core" Version="1.17.0" />
     <PackageReference Include="Microsoft.Win32.Registry" Version="5.0.0" />
     <PackageReference Include="System.IO.FileSystem.AccessControl" Version="6.0.0-preview.5.21301.5" />
     <PackageReference Include="System.Management" Version="4.7.0" />

src/Agent.Sdk/Knob/AgentKnobs.cs

@@ -675,10 +675,10 @@ public class AgentKnobs
             new PipelineFeatureSource("UseNode20ToStartContainer"),
             new BuiltInDefaultKnobSource("false"));
 
-        public static readonly Knob EnableNewSecretMasker = new Knob(
-            nameof(EnableNewSecretMasker),
+        public static readonly Knob EnableNewMaskerAndRegexes = new Knob(
+            nameof(EnableNewMaskerAndRegexes),
             "If true, the agent will use new SecretMasker with additional filters & performance enhancements",
-            new EnvironmentKnobSource("AZP_ENABLE_NEW_SECRET_MASKER"),
+            new EnvironmentKnobSource("AZP_ENABLE_NEW_MASKER_AND_REGEXES"),
             new BuiltInDefaultKnobSource("false"));
 
         public static readonly Knob AddDockerInitOption = new Knob(

src/Agent.Sdk/SecretMasking/ILoggedSecretMasker.cs

@@ -1,15 +1,17 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-//using Microsoft.TeamFoundation.DistributedTask.Logging;
-using ValueEncoder = Microsoft.TeamFoundation.DistributedTask.Logging.ValueEncoder;
+
 using System;
 
+using Microsoft.TeamFoundation.DistributedTask.Logging;
+
 namespace Agent.Sdk.SecretMasking
 {
     /// <summary>
-    /// Extended ISecretMasker interface that is adding support of logging secret masker methods
+    /// Extended ISecretMasker interface that adds support for logging the origin of
+    /// regexes, encoders and literal secret values.
     /// </summary>
-    public interface ILoggedSecretMasker : ISecretMasker
+    public interface ILoggedSecretMasker : ISecretMasker, IDisposable
     {
         static int MinSecretLengthLimit { get; }
 

src/Agent.Sdk/SecretMasking/ISecret.cs

@@ -2,13 +2,19 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 using System;
-using ValueEncoder = Microsoft.TeamFoundation.DistributedTask.Logging.ValueEncoder;
-using ISecretMaskerVSO = Microsoft.TeamFoundation.DistributedTask.Logging.ISecretMasker;
+
+using Microsoft.TeamFoundation.DistributedTask.Logging;
 
 namespace Agent.Sdk.SecretMasking
 {
     /// <summary>
-    /// Extended secret masker service, that allows to log origins of secrets
+    /// Extended secret masker service that allows specifying the origin of any
+    /// masking operation. It works by wrapping an existing ISecretMasker
+    /// implementation and an optionally settable ITraceWriter instance for
+    /// secret origin logging operations. In the agent today, this class can be
+    /// initialized with two distinct ISecretMasker implementations, the one
+    /// that ships in VSO itself, and the official Microsoft open source secret
+    /// masker, implemented at https://github/microsoft/security-utilities.
     /// </summary>
     public class LoggedSecretMasker : ILoggedSecretMasker
     {
@@ -43,6 +49,7 @@ public void AddValue(string pattern)
         /// <param name="origin">Origin of the secret</param>
         public void AddValue(string value, string origin)
         {
+            // WARNING: Do not log the value here, it is a secret!
             this.Trace($"Setting up value for origin: {origin}");
             if (value == null)
             {
@@ -64,6 +71,7 @@ public void AddRegex(string pattern)
         /// <param name="origin"></param>
         public void AddRegex(string pattern, string origin)
         {
+            // WARNING: Do not log the pattern here, it could be very specifc and contain a secret!
             this.Trace($"Setting up regex for origin: {origin}.");
             if (pattern == null)
             {
@@ -117,7 +125,6 @@ public void AddValueEncoder(ValueEncoder encoder)
         public void AddValueEncoder(ValueEncoder encoder, string origin)
         {
             this.Trace($"Setting up value for origin: {origin}");
-            this.Trace($"Length: {encoder.ToString().Length}.");
             if (encoder == null)
             {
                 this.Trace($"Encoder is empty.");
@@ -127,7 +134,7 @@ public void AddValueEncoder(ValueEncoder encoder, string origin)
             AddValueEncoder(encoder);
         }
 
-        public ISecretMasker Clone()
+        public LoggedSecretMasker Clone()
         {
             return new LoggedSecretMasker(this._secretMasker.Clone());
         }
@@ -137,6 +144,24 @@ public string MaskSecrets(string input)
             return this._secretMasker.MaskSecrets(input);
         }
 
-        ISecretMaskerVSO ISecretMaskerVSO.Clone() => this.Clone();
+        ISecretMasker ISecretMasker.Clone() => this.Clone();
+
+        public void Dispose()
+        {
+            Dispose(true);
+            GC.SuppressFinalize(this);
+        }
+
+        protected virtual void Dispose(bool disposing)
+        {
+            if (disposing)
+            {
+                if (_secretMasker is IDisposable disposable)
+                {
+                    disposable.Dispose();
+                }
+                _secretMasker = null;
+            }
+        }
     }
 }