diff --git a/src/ApplicationInsightsRequestLogging/ApplicationInsightsRequestLogging.csproj b/src/ApplicationInsightsRequestLogging/ApplicationInsightsRequestLogging.csproj index 150d641..0ce6914 100644 --- a/src/ApplicationInsightsRequestLogging/ApplicationInsightsRequestLogging.csproj +++ b/src/ApplicationInsightsRequestLogging/ApplicationInsightsRequestLogging.csproj @@ -36,6 +36,8 @@ + + diff --git a/src/ApplicationInsightsRequestLogging/BodyLoggerMiddleware.cs b/src/ApplicationInsightsRequestLogging/BodyLoggerMiddleware.cs index 843905f..0966659 100644 --- 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 options, IBodyReader bodyReader, ITelemetryWriter telemetryWriter) + public BodyLoggerMiddleware(IOptions 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 index 8283bdc..3409970 100644 --- a/src/ApplicationInsightsRequestLogging/Extensions/ServiceCollectionExtensions.cs +++ b/src/ApplicationInsightsRequestLogging/Extensions/ServiceCollectionExtensions.cs @@ -34,6 +34,7 @@ private static void AddBodyLogger(IServiceCollection services, Action(); } private static void AddBodyLogger(IServiceCollection services) diff --git a/src/ApplicationInsightsRequestLogging/Filters/ISensitiveDataFilter.cs b/src/ApplicationInsightsRequestLogging/Filters/ISensitiveDataFilter.cs new file mode 100644 index 0000000..989b5ad --- /dev/null +++ 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 new file mode 100644 index 0000000..46908d8 --- /dev/null +++ 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 _sensitiveDataPropertyKeys; + private readonly IEnumerable _regexesForSensitiveValues; + + + public SensitiveDataFilter(BodyLoggerOptions options) : this(options.PropertyNamesWithSensitiveData, options.SensitiveDataRegexes) + { + + } + + public SensitiveDataFilter(IEnumerable sensitiveDataPropertyKeys, IEnumerable 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 sensitiveDataPropertyKeys, + IEnumerable 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 index ef5591b..88dbc87 100644 --- a/src/ApplicationInsightsRequestLogging/Options/BodyLoggerOptions.cs +++ b/src/ApplicationInsightsRequestLogging/Options/BodyLoggerOptions.cs @@ -21,7 +21,7 @@ public BodyLoggerOptions() /// public List HttpVerbs { get; set; } = new List() { - HttpMethods.Post, + HttpMethods.Post, HttpMethods.Put, HttpMethods.Patch }; @@ -44,7 +44,7 @@ public BodyLoggerOptions() /// /// Defines the amount of bytes that should be read from HTTP context /// - public int MaxBytes { get; set; } = 80000; + public int MaxBytes { get; set; } = 1000; /// /// Defines the text to append in case the body should be truncated @@ -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 /// public bool DisableIpMasking { get; set; } = false; + + public List PropertyNamesWithSensitiveData { get; set; } = new List() + { + "password", + "secret", + "passwd", + "api_key", + "access_token", + "accessToken", + "auth", + "credentials", + "mysql_pwd" + }; + + public List SensitiveDataRegexes { get; set; } = new List() + { + @"(?: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 index ac732ba..aa372a5 100644 --- 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(); @@ -43,6 +44,7 @@ public async void BodyLoggerMiddleware_Should_Send_Data_To_AppInsights() .ConfigureServices(services => { services.AddTransient(); + services.AddTransient(); services.AddSingleton(telemetryWriter.Object); services.AddTransient(); }) @@ -87,6 +89,7 @@ public async void BodyLoggerMiddleware_Should_Not_Send_Data_To_AppInsights_When_ { services.AddTransient(); services.AddSingleton(telemetryWriter.Object); + services.AddTransient(); services.AddTransient(); }) .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(); + + using var host = await new HostBuilder() + .ConfigureWebHost(webBuilder => + { + webBuilder + .UseTestServer() + .ConfigureServices(services => + { + services.AddTransient(); + services.AddTransient(provider => + { + return new SensitiveDataFilter(new List() { "password" }, new List()); + }); + services.AddSingleton(telemetryWriter.Object); + services.AddTransient(); + }) + .Configure(app => + { + app.UseMiddleware(); + 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\":\"fred@mayekawa.com\",\"password\":\"P@ssw0rd!\"}")); + + // Assert + telemetryWriter.Verify(x => x.Write(It.IsAny(), "RequestBody", "{\"email\":\"fred@mayekawa.com\",\"password\":\"***MASKED***\"}"), Times.Once); + telemetryWriter.Verify(x => x.Write(It.IsAny(), "ResponseBody", "{\"email\":\"fred@mayekawa.com\",\"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(); }) .Configure(app => { diff --git a/test/ApplicationInsightsRequestLoggingTests/SensitiveDataFilter/SensitiveDataFilterTests.cs b/test/ApplicationInsightsRequestLoggingTests/SensitiveDataFilter/SensitiveDataFilterTests.cs new file mode 100644 index 0000000..a9582d6 --- /dev/null +++ b/test/ApplicationInsightsRequestLoggingTests/SensitiveDataFilter/SensitiveDataFilterTests.cs @@ -0,0 +1,90 @@ +using Azureblue.ApplicationInsights.RequestLogging; +using FluentAssertions; +using System; +using System.Collections.Generic; +using System.Text.Json; +using Xunit; + +namespace ApplicationInsightsRequestLoggingTests.Filters +{ + public class SensitiveDataFilterTests + { + [Fact] + public void Should_mask_tokens_with_sensitive_data_from_json() + { + // Arrange + var jsonWithToken = JsonSerializer.Serialize(new { token = "some-super-secret-token" }); + var filter = new SensitiveDataFilter(new HashSet { "token" }, Array.Empty()); + + // Act + var result = filter.RemoveSensitiveData(jsonWithToken); + + // Assert + var jsonStrippedFromSensitiveData = JsonSerializer.Serialize(new { token = "***MASKED***" }); + result.Should().Be(jsonStrippedFromSensitiveData); + } + + [Fact] + public void Should_mask_tokens_with_sensitive_data_from_nested_object_json() + { + // Arrange + var jsonWithToken = JsonSerializer.Serialize(new { someObject = new { password = "some-super-secret-token" } }); + var filter = new SensitiveDataFilter(new HashSet { "password" }, Array.Empty()); + + // Act + var result = filter.RemoveSensitiveData(jsonWithToken); + + // Assert + var jsonStrippedFromSensitiveData = JsonSerializer.Serialize(new { someObject = new { password = "***MASKED***" } }); + result.Should().Be(jsonStrippedFromSensitiveData); + } + + [Fact] + public void Should_mask_tokens_with_sensitive_data_even_if_its_not_equal_match_json() + { + // Arrange + var jsonWithToken = JsonSerializer.Serialize(new { someObject = new { userPassword = "some-super-secret-token" } }); + var filter = new SensitiveDataFilter(new HashSet { "password" }, Array.Empty()); + + // Act + var result = filter.RemoveSensitiveData(jsonWithToken); + + // Assert + var jsonStrippedFromSensitiveData = JsonSerializer.Serialize(new { someObject = new { userPassword = "***MASKED***" } }); + result.Should().Be(jsonStrippedFromSensitiveData); + } + + + private const string CreditCardRegex = @"(?: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})"; + private const string SampleCreditCardNumber = "4012888888881881"; + + [Fact] + public void Should_mask_values_basedOnRegex() + { + // Arrange + var jsonWithToken = JsonSerializer.Serialize(new { someObject = new { randomTokenName = SampleCreditCardNumber } }); + var filter = new SensitiveDataFilter(new HashSet { "password" }, new List { CreditCardRegex }); + + // Act + var result = filter.RemoveSensitiveData(jsonWithToken); + + // Assert + var jsonStrippedFromSensitiveData = JsonSerializer.Serialize(new { someObject = new { randomTokenName = "***MASKED***" } }); + result.Should().Be(jsonStrippedFromSensitiveData); + } + + [Fact] + public void SensitiveDataFilter_should_remove_creditCard_number_from_plain_text_if_configured() + { + // Arrange + var plainTextWithToken = $"token: some-not-so-{SampleCreditCardNumber}secret-token-but-with-cc-inside"; + var filter = new SensitiveDataFilter(new HashSet { "token" }, new List { CreditCardRegex }); + + // Act + var result = filter.RemoveSensitiveData(plainTextWithToken); + + // Assert + result.Should().Be("***MASKED***"); + } + } +} \ No newline at end of file diff --git a/test/ManualTests/Controllers/TestController.cs b/test/ManualTests/Controllers/TestController.cs index bdc1397..4332edd 100644 --- a/test/ManualTests/Controllers/TestController.cs +++ b/test/ManualTests/Controllers/TestController.cs @@ -11,6 +11,12 @@ public ActionResult Test([FromBody] TestData data) { return Ok(data); } + + [HttpPost("maxbytes")] + public ActionResult MaxBytesTest([FromForm] string data) + { + return Ok(data); + } } }