Feature/8 sensitive data filter (#19)

* add SensitiveDataFilter, so that no user, login, or personal data will be leaked by logs * sensitiveDataFilter: add regex search * add sample creditCard data filter --------- Co-authored-by: Tomasz Dłuski <[email protected]> Co-authored-by: Matthias Güntert <[email protected]>
matthiasguentert · Feb 20, 2024 · f247f60 · f247f60
1 parent 3d5faeb
commit f247f60
Show file tree

Hide file tree

Showing 9 changed files with 303 additions and 6 deletions.
diff --git a/src/ApplicationInsightsRequestLogging/ApplicationInsightsRequestLogging.csproj b/src/ApplicationInsightsRequestLogging/ApplicationInsightsRequestLogging.csproj
@@ -36,6 +36,8 @@
     <PackageReference Include="Microsoft.ApplicationInsights" Version="2.18.0" />
     <PackageReference Include="Microsoft.AspNetCore.Http" Version="2.2.2" />
     <PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="5.0.0" />
+    <PackageReference Include="System.Text.Json" Version="6.0.0" />
+    <PackageReference Include="System.Text.RegularExpressions" Version="4.3.1" />
   </ItemGroup>
 
 </Project>
diff --git a/src/ApplicationInsightsRequestLogging/BodyLoggerMiddleware.cs b/src/ApplicationInsightsRequestLogging/BodyLoggerMiddleware.cs
@@ -10,12 +10,14 @@ public class BodyLoggerMiddleware : IMiddleware
         private readonly BodyLoggerOptions _options;
         private readonly IBodyReader _bodyReader;
         private readonly ITelemetryWriter _telemetryWriter;
+        private readonly ISensitiveDataFilter _sensitiveDataFilter;
 
-        public BodyLoggerMiddleware(IOptions<BodyLoggerOptions> options, IBodyReader bodyReader, ITelemetryWriter telemetryWriter)
+        public BodyLoggerMiddleware(IOptions<BodyLoggerOptions> options, IBodyReader bodyReader, ITelemetryWriter telemetryWriter, ISensitiveDataFilter sensitiveDataFilter)
         {
             _options = options.Value ?? throw new ArgumentNullException(nameof(options));
             _bodyReader = bodyReader ?? throw new ArgumentNullException(nameof(bodyReader));
             _telemetryWriter = telemetryWriter ?? throw new ArgumentNullException(nameof(telemetryWriter));
+            _sensitiveDataFilter = sensitiveDataFilter ?? throw new ArgumentNullException(nameof(telemetryWriter));
         }
 
         public async Task InvokeAsync(HttpContext context, RequestDelegate next)
@@ -38,8 +40,8 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next)
                 {
                     var responseBody = await _bodyReader.ReadResponseBodyAsync(context, _options.MaxBytes, _options.Appendix);
 
-                    _telemetryWriter.Write(context, _options.RequestBodyPropertyKey, requestBody);
-                    _telemetryWriter.Write(context, _options.ResponseBodyPropertyKey, responseBody);
+                    _telemetryWriter.Write(context, _options.RequestBodyPropertyKey, _sensitiveDataFilter.RemoveSensitiveData(requestBody));
+                    _telemetryWriter.Write(context, _options.ResponseBodyPropertyKey, _sensitiveDataFilter.RemoveSensitiveData(responseBody));
                 }
 
                 // Copy back so response body is available for the user agent

diff --git a/src/ApplicationInsightsRequestLogging/Extensions/ServiceCollectionExtensions.cs b/src/ApplicationInsightsRequestLogging/Extensions/ServiceCollectionExtensions.cs
@@ -34,6 +34,7 @@ private static void AddBodyLogger(IServiceCollection services, Action<BodyLogger
         {
             AddBodyLogger(services);
             services.Configure(setupAction);
+            services.AddTransient<ISensitiveDataFilter, SensitiveDataFilter>();
         }
 
         private static void AddBodyLogger(IServiceCollection services)

diff --git a/src/ApplicationInsightsRequestLogging/Filters/ISensitiveDataFilter.cs b/src/ApplicationInsightsRequestLogging/Filters/ISensitiveDataFilter.cs
@@ -0,0 +1,7 @@
+namespace Azureblue.ApplicationInsights.RequestLogging
+{
+    public interface ISensitiveDataFilter
+    {
+        string RemoveSensitiveData(string textOrJson);
+    }
+}
diff --git a/src/ApplicationInsightsRequestLogging/Filters/SensitiveDataFilter.cs b/src/ApplicationInsightsRequestLogging/Filters/SensitiveDataFilter.cs
@@ -0,0 +1,125 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text.Json;
+using System.Text.Json.Nodes;
+using System.Text.RegularExpressions;
+
+namespace Azureblue.ApplicationInsights.RequestLogging
+{
+    public class SensitiveDataFilter : ISensitiveDataFilter
+    {
+        private const string SensitiveValueMask = "***MASKED***";
+
+        public readonly HashSet<string> _sensitiveDataPropertyKeys;
+        private readonly IEnumerable<string> _regexesForSensitiveValues;
+
+
+        public SensitiveDataFilter(BodyLoggerOptions options) : this(options.PropertyNamesWithSensitiveData, options.SensitiveDataRegexes)
+        {
+
+        }
+
+        public SensitiveDataFilter(IEnumerable<string> sensitiveDataPropertyKeys, IEnumerable<string> regexesForSensitiveValues)
+        {
+            _sensitiveDataPropertyKeys = sensitiveDataPropertyKeys.Select(t => t.ToLowerInvariant()).ToHashSet();
+            _regexesForSensitiveValues = regexesForSensitiveValues;
+        }
+
+        public string RemoveSensitiveData(string textOrJson)
+        {
+            try
+            {
+                var json = JsonNode.Parse(textOrJson);
+                if (json == null) return string.Empty;
+
+                if (json is JsonValue jValue && TestIfContainsSensitiveData("", jValue.ToString(), _sensitiveDataPropertyKeys, _regexesForSensitiveValues))
+                {
+                    return SensitiveValueMask;
+                }
+                RemoveIds(json);
+                return json.ToJsonString();
+            }
+            catch (JsonException)
+            {
+                if (TestIfContainsSensitiveData("", textOrJson, _sensitiveDataPropertyKeys, _regexesForSensitiveValues))
+                {
+                    return SensitiveValueMask;
+                }
+                return textOrJson;
+            }
+        }
+
+        private void RemoveIds(JsonNode node)
+        {
+            if (node is JsonObject jObject)
+            {
+                RemoveIds(jObject);
+            }
+            else if (node is JsonArray jArray)
+            {
+                RemoveFromArray(jArray);
+            }
+        }
+
+        private void RemoveIds(JsonObject? jObject)
+        {
+            if (jObject == null) throw new ArgumentNullException(nameof(jObject));
+
+            foreach (var jProperty in jObject.ToList())
+            {
+                if (jProperty.Value is JsonArray array)
+                {
+                    RemoveFromArray(array);
+                }
+                else if (jProperty.Value is JsonObject obj)
+                {
+                    RemoveIds(obj);
+                }
+                else if (jProperty.Value is JsonValue val
+                    && TestIfContainsSensitiveData(jProperty.Key, val.ToString(), _sensitiveDataPropertyKeys, _regexesForSensitiveValues))
+                {
+                    jObject[jProperty.Key] = SensitiveValueMask;
+                }
+            }
+        }
+
+        private void RemoveFromArray(JsonArray jArray)
+        {
+            if (jArray == null) throw new ArgumentNullException(nameof(jArray));
+
+            foreach (var jNode in jArray.Where(v => v != null))
+            {
+                RemoveIds(jNode);
+            }
+        }
+
+        private bool TestIfContainsSensitiveData(
+            string propertyName,
+            string propertyValue,
+            HashSet<string> sensitiveDataPropertyKeys,
+            IEnumerable<string> regexesForSensitiveValues
+            )
+        {
+            var propertyNameToCompare = propertyName.ToLowerInvariant();
+            var sensitivePropertyName = sensitiveDataPropertyKeys.Contains(propertyNameToCompare)
+                || sensitiveDataPropertyKeys.Any(s => propertyNameToCompare.Contains(s));
+
+            if (sensitivePropertyName)
+            {
+                return true;
+            }
+
+            foreach (var regex in regexesForSensitiveValues)
+            {
+                if (Regex.IsMatch(propertyValue, regex))
+                {
+                    return true;
+                }
+            }
+
+
+            return false;
+        }
+    }
+}
diff --git a/src/ApplicationInsightsRequestLogging/Options/BodyLoggerOptions.cs b/src/ApplicationInsightsRequestLogging/Options/BodyLoggerOptions.cs
@@ -21,7 +21,7 @@ public BodyLoggerOptions()
         /// </summary>
         public List<string> HttpVerbs { get; set; } = new List<string>()
         {
-            HttpMethods.Post, 
+            HttpMethods.Post,
             HttpMethods.Put,
             HttpMethods.Patch
         };
@@ -44,7 +44,7 @@ public BodyLoggerOptions()
         /// <summary>
         ///     Defines the amount of bytes that should be read from HTTP context
         /// </summary>
-        public int MaxBytes { get; set; } = 80000;
+        public int MaxBytes { get; set; } = 1000;
 
         /// <summary>
         ///     Defines the text to append in case the body should be truncated <seealso cref="MaxBytes"/>
@@ -55,5 +55,23 @@ public BodyLoggerOptions()
         ///     Controls storage of client IP addresses https://learn.microsoft.com/en-us/azure/azure-monitor/app/ip-collection?tabs=net
         /// </summary>
         public bool DisableIpMasking { get; set; } = false;
+
+        public List<string> PropertyNamesWithSensitiveData { get; set; } = new List<string>()
+        {
+            "password",
+            "secret",
+            "passwd",
+            "api_key",
+            "access_token",
+            "accessToken",
+            "auth",
+            "credentials",
+            "mysql_pwd"
+        };
+
+        public List<string> SensitiveDataRegexes { get; set; } = new List<string>()
+        {
+            @"(?:4[0-9]{12}(?:[0-9]{3})?|[25][1-7][0-9]{14}|6(?:011|5[0-9][0-9])[0-9]{12}|3[47][0-9]{13}|3(?:0[0-5]|[68][0-9])[0-9]{11}|(?:2131|1800|35\d{3})\d{11})" // credit cards from https://stackoverflow.com/questions/9315647/regex-credit-card-number-tests
+        };
     }
 }
diff --git a/test/ApplicationInsightsRequestLoggingTests/BodyLoggerMiddlewareTests.cs b/test/ApplicationInsightsRequestLoggingTests/BodyLoggerMiddlewareTests.cs
@@ -14,6 +14,7 @@
 using Microsoft.ApplicationInsights.DataContracts;
 using Moq;
 using Microsoft.Extensions.DependencyInjection;
+using System.Collections.Generic;
 
 namespace ApplicationInsightsRequestLoggingTests
 {
@@ -23,7 +24,7 @@ public class BodyLoggerMiddlewareTests
         public void BodyLoggerMiddleware_Should_Throw_If_Ctor_Params_Null()
         {
             // Arrange & Act
-            var action = () => { var middleware = new BodyLoggerMiddleware(null, null, null); };
+            var action = () => { var middleware = new BodyLoggerMiddleware(null, null, null, null); };
 
             // Assert
             action.Should().Throw<NullReferenceException>();
@@ -43,6 +44,7 @@ public async void BodyLoggerMiddleware_Should_Send_Data_To_AppInsights()
                         .ConfigureServices(services =>
                         {
                             services.AddTransient<IBodyReader, BodyReader>();
+                            services.AddTransient<ISensitiveDataFilter, SensitiveDataFilter>();
                             services.AddSingleton(telemetryWriter.Object);
                             services.AddTransient<BodyLoggerMiddleware>();
                         })
@@ -87,6 +89,7 @@ public async void BodyLoggerMiddleware_Should_Not_Send_Data_To_AppInsights_When_
                         {
                             services.AddTransient<IBodyReader, BodyReader>();
                             services.AddSingleton(telemetryWriter.Object);
+                            services.AddTransient<ISensitiveDataFilter, SensitiveDataFilter>();
                             services.AddTransient<BodyLoggerMiddleware>();
                         })
                         .Configure(app =>
@@ -150,6 +153,48 @@ public async void BodyLoggerMiddleware_Should_Leave_Body_intact()
             body.Should().Be("Hello from client");
         }
 
+        [Fact]
+        public async void BodyLoggerMiddleware_Should_Redact_Password()
+        {
+            // Arrange
+            var telemetryWriter = new Mock<ITelemetryWriter>();
+
+            using var host = await new HostBuilder()
+                .ConfigureWebHost(webBuilder =>
+                {
+                    webBuilder
+                        .UseTestServer()
+                        .ConfigureServices(services =>
+                        {
+                            services.AddTransient<IBodyReader, BodyReader>();
+                            services.AddTransient<ISensitiveDataFilter>(provider =>
+                            {
+                                return new SensitiveDataFilter(new List<string>() { "password" }, new List<string>());
+                            });
+                            services.AddSingleton(telemetryWriter.Object);
+                            services.AddTransient<BodyLoggerMiddleware>();
+                        })
+                        .Configure(app =>
+                        {
+                            app.UseMiddleware<BodyLoggerMiddleware>();
+                            app.Run(async context =>
+                            {
+                                // Send request body back in response body
+                                context.Response.StatusCode = StatusCodes.Status400BadRequest;
+                                await context.Request.Body.CopyToAsync(context.Response.Body);
+                            });
+                        });
+                })
+                .StartAsync();
+
+            // Act
+            var response = await host.GetTestClient().PostAsync("/", new StringContent("{\"email\":\"[email protected]\",\"password\":\"P@ssw0rd!\"}"));
+
+            // Assert
+            telemetryWriter.Verify(x => x.Write(It.IsAny<HttpContext>(), "RequestBody", "{\"email\":\"[email protected]\",\"password\":\"***MASKED***\"}"), Times.Once);
+            telemetryWriter.Verify(x => x.Write(It.IsAny<HttpContext>(), "ResponseBody", "{\"email\":\"[email protected]\",\"password\":\"***MASKED***\"}"), Times.Once);
+        }
+
         [Fact]
         public async void BodyLoggerMiddleware_Should_Properly_Pass()
         {
@@ -162,6 +207,7 @@ public async void BodyLoggerMiddleware_Should_Properly_Pass()
                         .ConfigureServices(services =>
                         {
                             services.AddAppInsightsHttpBodyLogging();
+                            services.AddTransient<ISensitiveDataFilter, SensitiveDataFilter>();
                         })
                         .Configure(app =>
                         {