diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index eb3d9678b..84fe6f1f0 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -469,8 +469,7 @@ See https://www.owasp.org/index.php/Information_exposure_through_query_strings_i *`Cookie` header sanitization:* The `Cookie` header is automatically redacted for incoming HTTP request transactions. Each name-value pair from the -Cookie header is parsed by the agent and sent to the APM Server. Before the name-value pairs are recorded, they are -sanitized based on the `SanitizeFieldNames` configuration. Cookies with sensitive data in +cookie list is parsed by the agent and sanitized based on the `SanitizeFieldNames` configuration. Cookies with sensitive data in their value can be redacted by adding the cookie's name to the comma-separated list. [options="header"] diff --git a/src/Elastic.Apm/Api/Request.cs b/src/Elastic.Apm/Api/Request.cs index 608682c50..b06e2ef91 100644 --- a/src/Elastic.Apm/Api/Request.cs +++ b/src/Elastic.Apm/Api/Request.cs @@ -27,11 +27,6 @@ public class Request /// public Dictionary Headers { get; set; } - /// - /// This field is sanitized by a filter - /// - public Dictionary Cookies { get; set; } - [JsonProperty("http_version")] [MaxLength] public string HttpVersion { get; set; } @@ -47,8 +42,6 @@ internal Request DeepCopy() var newItem = (Request)MemberwiseClone(); if (Headers != null) newItem.Headers = Headers.ToDictionary(entry => entry.Key, entry => entry.Value); - if (Cookies != null) - newItem.Cookies = Cookies.ToDictionary(entry => entry.Key, entry => entry.Value); newItem.Socket = Socket?.DeepCopy(); newItem.Url = Url?.DeepCopy(); diff --git a/src/Elastic.Apm/Filters/CookieHeaderRedactionFilter.cs b/src/Elastic.Apm/Filters/CookieHeaderRedactionFilter.cs new file mode 100644 index 000000000..ed81382fb --- /dev/null +++ b/src/Elastic.Apm/Filters/CookieHeaderRedactionFilter.cs @@ -0,0 +1,91 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System; +using System.Collections; +using System.Collections.Generic; +using Elastic.Apm.Api; +using Elastic.Apm.Config; +using Elastic.Apm.Helpers; +using Elastic.Apm.Model; +#if NET6_0_OR_GREATER +using System.Buffers; +#endif + +namespace Elastic.Apm.Filters +{ + /// + /// Redacts items from the cookie list of the Cookie request header. + /// + internal class CookieHeaderRedactionFilter + { + private const string CookieHeader = "Cookie"; + + public static IError Filter(IError error) + { + if (error is Error e && e.Context is not null) + HandleCookieHeader(e.Context?.Request?.Headers, e.Configuration.SanitizeFieldNames); + return error; + } + + public static ITransaction Filter(ITransaction transaction) + { + if (transaction is Transaction { IsContextCreated: true }) + HandleCookieHeader(transaction.Context?.Request?.Headers, transaction.Configuration.SanitizeFieldNames); + return transaction; + } + + // internal for testing + internal static void HandleCookieHeader(Dictionary headers, IReadOnlyList sanitizeFieldNames) + { + if (headers is not null) + { + // Realistically, this should be more than enough for all sensible scenarios + // e.g. Cookies | cookies | COOKIES + const int maxKeys = 4; + +#if NET6_0_OR_GREATER + var matchedKeys = ArrayPool.Shared.Rent(maxKeys); + var matchedValues = ArrayPool.Shared.Rent(maxKeys); +#else + var matchedKeys = new string[maxKeys]; + var matchedValues = new string[maxKeys]; +#endif + var matchedCount = 0; + + foreach (var header in headers) + { + if (matchedCount == maxKeys) + break; + + if (header.Key.Equals(CookieHeader, StringComparison.OrdinalIgnoreCase)) + { + matchedKeys[matchedCount] = header.Key; + matchedValues[matchedCount] = CookieHeaderRedacter.Redact(header.Value, sanitizeFieldNames); + matchedCount++; + } + } + + var replacedCount = 0; + + foreach (var headerKey in matchedKeys) + { + if (replacedCount == matchedCount) + break; + + if (headerKey is not null) + { + headers[headerKey] = matchedValues[replacedCount]; + replacedCount++; + } + } + +#if NET6_0_OR_GREATER + ArrayPool.Shared.Return(matchedKeys); + ArrayPool.Shared.Return(matchedValues); +#endif + } + } + } +} diff --git a/src/Elastic.Apm/Filters/RequestCookieExtractionFilter.cs b/src/Elastic.Apm/Filters/RequestCookieExtractionFilter.cs deleted file mode 100644 index 92ec639de..000000000 --- a/src/Elastic.Apm/Filters/RequestCookieExtractionFilter.cs +++ /dev/null @@ -1,53 +0,0 @@ -// Licensed to Elasticsearch B.V under -// one or more agreements. -// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. -// See the LICENSE file in the project root for more information - -using Elastic.Apm.Api; -using Elastic.Apm.Helpers; -using Elastic.Apm.Model; - -namespace Elastic.Apm.Filters -{ - /// - /// Extracts cookies from the Cookie request header and sets the Cookie header to [REDACTED]. - /// - internal class RequestCookieExtractionFilter - { - private static readonly WildcardMatcher[] CookieMatcher = [new WildcardMatcher.VerbatimMatcher("Cookie", true)]; - - public static IError Filter(IError error) - { - if (error is Error realError) - HandleCookieHeader(realError.Context); - return error; - } - - public static ITransaction Filter(ITransaction transaction) - { - if (transaction is Transaction { IsContextCreated: true }) - HandleCookieHeader(transaction.Context); - return transaction; - } - - private static void HandleCookieHeader(Context context) - { - if (context?.Request?.Headers is not null) - { - string matchedKey = null; - foreach (var key in context.Request.Headers.Keys) - { - if (WildcardMatcher.IsAnyMatch(CookieMatcher, key)) - { - var cookies = context.Request.Headers[key]; - context.Request.Cookies = CookieHeaderParser.ParseCookies(cookies); - matchedKey = key; - } - } - - if (matchedKey is not null) - context.Request.Headers[matchedKey] = Consts.Redacted; - } - } - } -} diff --git a/src/Elastic.Apm/Filters/Sanitization.cs b/src/Elastic.Apm/Filters/Sanitization.cs index 7f7c98fed..83dec05c9 100644 --- a/src/Elastic.Apm/Filters/Sanitization.cs +++ b/src/Elastic.Apm/Filters/Sanitization.cs @@ -18,9 +18,6 @@ public static void SanitizeHeadersInContext(Context context, IConfiguration conf if (context?.Request?.Headers is not null) RedactMatches(context?.Request?.Headers, configuration); - if (context?.Request?.Cookies is not null) - RedactMatches(context?.Request?.Cookies, configuration); - if (context?.Response?.Headers is not null) RedactMatches(context?.Response?.Headers, configuration); diff --git a/src/Elastic.Apm/Helpers/CookieHeaderParser.cs b/src/Elastic.Apm/Helpers/CookieHeaderParser.cs deleted file mode 100644 index 3a4285150..000000000 --- a/src/Elastic.Apm/Helpers/CookieHeaderParser.cs +++ /dev/null @@ -1,110 +0,0 @@ -// Licensed to Elasticsearch B.V under one or more agreements. -// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. -// See the LICENSE file in the project root for more information - -#if !NETFRAMEWORK -using System; -#endif -using System.Collections.Generic; - -namespace Elastic.Apm.Helpers; - -internal static class CookieHeaderParser -{ - public static Dictionary ParseCookies(string cookieHeader) - { - // Implementation notes: - // This method handles a cookie header value for both ASP.NET (classic) and - // ASP.NET Core. As a result it must handle two possible formats. In ASP.NET - // (classic) the string is the actual Cookie value as sent over the wire, conforming - // to the HTTP standards. This uses the semicolon separator and a space between - // entries. For ASP.NET Core, when we parse the headers, we convert from the - // StringValues by calling ToString. This results in each entry being separated - // by a regular colon and no space. - - if (string.IsNullOrEmpty(cookieHeader)) - return null; - - var cookies = new Dictionary(); - -#if NETFRAMEWORK - // The use of `Span` in NETFX can cause runtime assembly loading issues due to our friend, - // binding redirects. For now, we take a slightly less allocation efficient route here, rather - // than risk introducing runtime issues for consumers. Technically, this should be "fixed" in - // NET472+, but during testing I surprisingly still reproduced an exception. Elastic.APM depends on - // `System.Diagnostics.DiagnosticSource 5.0.0` which itself depends on `System.Runtime.CompilerServices.Unsafe` - // which is where the exception occurs. 5.0.0 is marked as deprecated so we could look to prefer - // a new version but we have special handling for the ElasticApmAgentStartupHook - // zip file version. For now, we decided not to mess with this as it's hard to test all scenarios. - - var cookieValues = cookieHeader.Split(',', ';'); - - foreach (var cookieValue in cookieValues) - { - var trimmed = cookieValue.Trim(); - var parts = trimmed.Split('='); - - // Fow now, we store only the first value for a given key. This aligns to our nodeJS agent behavior. - if (parts.Length == 2 - && !string.IsNullOrEmpty(parts[0]) - && !cookies.ContainsKey(parts[0]) - && !string.IsNullOrEmpty(parts[1])) - { - cookies.Add(parts[0], parts[1]); - } - } - - return cookies; -#else - var span = cookieHeader.AsSpan(); - - while (span.Length > 0) - { - var foundComma = true; - var separatorIndex = span.IndexOfAny(',', ';'); - - if (separatorIndex == -1) - { - foundComma = false; - separatorIndex = span.Length; - } - - var entry = span.Slice(0, separatorIndex); - - var equalsIndex = entry.IndexOf('='); - - if (equalsIndex > -1) - { - var key = entry.Slice(0, equalsIndex); - var value = entry.Slice(equalsIndex + 1); - - var keyString = key.ToString(); - var valueString = value.ToString(); - - // Fow now, we store only the first value for a given key. This aligns to our nodeJS agent behavior. -#if NETSTANDARD2_0 - if (!string.IsNullOrEmpty(keyString) && !cookies.ContainsKey(keyString) && !string.IsNullOrEmpty(valueString)) - cookies.Add(keyString, valueString); -#else - if (!string.IsNullOrEmpty(keyString) && !string.IsNullOrEmpty(valueString)) - cookies.TryAdd(keyString, valueString); -#endif - } - - span = span.Slice(foundComma ? separatorIndex + 1 : span.Length); - - // skip any white space between the separator and the next entry - while (span.Length > 0) - { - if (span[0] != ' ') - break; - - span = span.Slice(1); - } - } - - return cookies; -#endif - } -} - diff --git a/src/Elastic.Apm/Helpers/CookieHeaderRedacter.cs b/src/Elastic.Apm/Helpers/CookieHeaderRedacter.cs new file mode 100644 index 000000000..76d4b6209 --- /dev/null +++ b/src/Elastic.Apm/Helpers/CookieHeaderRedacter.cs @@ -0,0 +1,206 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +#if !NETFRAMEWORK +using System; +using System.Buffers; +#endif +using System.Collections.Generic; +using System.Configuration; +using System.Text; + +namespace Elastic.Apm.Helpers; + +internal static class CookieHeaderRedacter +{ +#if !NETFRAMEWORK + private static ReadOnlySpan Redacted => Consts.Redacted.AsSpan(); + private static ReadOnlySpan SemiColon => ";".AsSpan(); + private static ReadOnlySpan Comma => ",".AsSpan(); +#endif + + public static string Redact(string cookieHeaderValue, IReadOnlyList namesToSanitize) + { + // Implementation notes: + // This method handles a cookie header value for both ASP.NET (classic) and + // ASP.NET Core. As a result it must handle two possible formats. In ASP.NET + // (classic) the string is the actual Cookie value as sent over the wire, conforming + // to the HTTP standards. This uses the semicolon separator and a space between + // entries. For ASP.NET Core, when we parse the headers, we convert from the + // StringValues by calling ToString. This results in each entry being separated + // by a regular colon and no space. + + // When the headers are stored into the `Request.Headers` on our APM data model, + // multiple headers of the same name, are concatenated into a comma-separated list. + // When redacting, we preserve this separation. + + if (string.IsNullOrEmpty(cookieHeaderValue)) + return null; + + var redactedCookieHeader = string.Empty; + +#if NETFRAMEWORK + // The use of `Span` in NETFX can cause runtime assembly loading issues due to our friend, + // binding redirects. For now, we take a slightly less allocation efficient route here, rather + // than risk introducing runtime issues for consumers. Technically, this should be "fixed" in + // NET472+, but during testing I surprisingly still reproduced an exception. Elastic.APM depends on + // `System.Diagnostics.DiagnosticSource 5.0.0` which itself depends on `System.Runtime.CompilerServices.Unsafe` + // which is where the exception occurs. 5.0.0 is marked as deprecated so we could look to prefer + // a new version but we have special handling for the ElasticApmAgentStartupHook + // zip file version. For now, we decided not to mess with this as it's hard to test all scenarios. + + var sb = new StringBuilder(cookieHeaderValue.Length); + var cookieHeaderEntries = cookieHeaderValue.Split(','); + + var requiresComma = false; + foreach (var entry in cookieHeaderEntries) + { + if (requiresComma) + sb.Append(','); + + var cookieValues = entry.Split(';'); + + var requiresSemiColon = false; + foreach (var cookieValue in cookieValues) + { + var parts = cookieValue.Split('='); + + if (requiresSemiColon) + sb.Append(';'); + + if (parts.Length == 1) + { + sb.Append(parts[0]); + } + else + { + if (WildcardMatcher.IsAnyMatch(namesToSanitize, parts[0].Trim())) + { + sb.Append(parts[0]); + sb.Append('='); + sb.Append(Consts.Redacted); + } + else + { + sb.Append(cookieValue); + } + } + + requiresSemiColon = true; + } + + requiresComma = true; + } + + redactedCookieHeader = sb.ToString(); + return redactedCookieHeader; +#else + var cookieHeaderValueSpan = cookieHeaderValue.AsSpan(); + + var written = 0; + var redactedBufferArray = ArrayPool.Shared.Rent(cookieHeaderValueSpan.Length); + var redactedBuffer = redactedBufferArray.AsSpan(); + + var whitespaceCount = 0; + while (cookieHeaderValueSpan.Length > 0) + { + // We first split on commas, to handle cases where we've combined, + // multiple headers of the same key into a concatened string. + var foundComma = true; + var commaIndex = cookieHeaderValueSpan.IndexOf(','); + + if (commaIndex == -1) // If there are no more separators, + { + foundComma = false; + commaIndex = cookieHeaderValueSpan.Length; + } + + var cookieHeaderEntrySpan = cookieHeaderValueSpan.Slice(0, commaIndex); + + if (written > 0) + { + Comma.CopyTo(redactedBuffer.Slice(written)); + written += Comma.Length; + } + + var writeSemiColon = false; + // Next handle each cookie item in the cookie header value + while (cookieHeaderEntrySpan.Length > 0) + { + var foundSemiColon = true; + var semiColonIndex = cookieHeaderEntrySpan.IndexOf(';'); + + if (semiColonIndex == -1) // If there are no more separators, + { + foundSemiColon = false; + semiColonIndex = cookieHeaderEntrySpan.Length; + } + + var cookieItem = cookieHeaderEntrySpan.Slice(0, semiColonIndex); + + var equalsIndex = cookieItem.IndexOf('='); + + if (equalsIndex > -1) + { + if (writeSemiColon) + { + SemiColon.CopyTo(redactedBuffer.Slice(written)); + written += SemiColon.Length; + + for (var i = 0; i < whitespaceCount; i++) + { + redactedBuffer[written++] = ' '; + } + } + + var key = cookieItem.Slice(0, equalsIndex); + var value = cookieItem.Slice(equalsIndex + 1); + + key.CopyTo(redactedBuffer.Slice(written)); + written += key.Length; + + redactedBuffer.Slice(written++)[0] = '='; + + if (WildcardMatcher.IsAnyMatch(namesToSanitize, key.ToString())) + { + Redacted.CopyTo(redactedBuffer.Slice(written)); + written += Redacted.Length; + } + else + { + value.CopyTo(redactedBuffer.Slice(written)); + written += value.Length; + } + } + else + { + cookieItem.CopyTo(redactedBuffer.Slice(written)); + written += cookieItem.Length; + } + + writeSemiColon = true; + cookieHeaderEntrySpan = cookieHeaderEntrySpan.Slice(foundSemiColon ? semiColonIndex + 1 : cookieItem.Length); + + whitespaceCount = 0; + + while (cookieHeaderEntrySpan.Length > 0) + { + if (cookieHeaderEntrySpan[0] != ' ') + break; + + cookieHeaderEntrySpan = cookieHeaderEntrySpan.Slice(1); + whitespaceCount++; + } + } + + cookieHeaderValueSpan = cookieHeaderValueSpan.Slice(foundComma ? commaIndex + 1 : commaIndex); + } + + redactedCookieHeader = redactedBuffer.Slice(0, written).ToString(); + ArrayPool.Shared.Return(redactedBufferArray); + return redactedCookieHeader; +#endif + } +} + diff --git a/src/Elastic.Apm/Report/PayloadSenderV2.cs b/src/Elastic.Apm/Report/PayloadSenderV2.cs index 5898fa340..691550730 100644 --- a/src/Elastic.Apm/Report/PayloadSenderV2.cs +++ b/src/Elastic.Apm/Report/PayloadSenderV2.cs @@ -141,14 +141,14 @@ IApmLogger logger ) { transactionFilters.Add(TransactionIgnoreUrlsFilter.Filter); - transactionFilters.Add(RequestCookieExtractionFilter.Filter); + transactionFilters.Add(CookieHeaderRedactionFilter.Filter); transactionFilters.Add(HeaderDictionarySanitizerFilter.Filter); // with this, stack trace demystification and conversion to the intake API model happens on a non-application thread: spanFilters.Add(new SpanStackTraceCapturingFilter(logger, apmServerInfo).Filter); errorFilters.Add(ErrorContextSanitizerFilter.Filter); - errorFilters.Add(RequestCookieExtractionFilter.Filter); + errorFilters.Add(CookieHeaderRedactionFilter.Filter); errorFilters.Add(HeaderDictionarySanitizerFilter.Filter); } diff --git a/test/Elastic.Apm.Tests/HelpersTests/CookieHeaderParserTests.cs b/test/Elastic.Apm.Tests/HelpersTests/CookieHeaderParserTests.cs deleted file mode 100644 index 588b4b2f2..000000000 --- a/test/Elastic.Apm.Tests/HelpersTests/CookieHeaderParserTests.cs +++ /dev/null @@ -1,61 +0,0 @@ -// Licensed to Elasticsearch B.V under one or more agreements. -// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. -// See the LICENSE file in the project root for more information - -using System.Collections.Generic; -using FluentAssertions; -using Xunit; - -namespace Elastic.Apm.Tests.HelpersTests; - -public class CookieHeaderParserTests -{ - [Theory] - [MemberData(nameof(TestData))] - public void ParsesHeadersCorrectly(string input, Dictionary expected) - { - var result = Helpers.CookieHeaderParser.ParseCookies(input); - - if (expected is null) - { - result.Should().BeNull(); - return; - } - - result.Count.Should().Be(expected.Count); - result.Should().Contain(expected); - } - - public static IEnumerable TestData() => [ - [null, null], - ["", null], - ["Key1=Value1", new Dictionary() { { "Key1", "Value1" }}], - [ "Key1=Value1,Key2=Value2", new Dictionary() - { - { "Key1", "Value1" }, - { "Key2", "Value2" } - }], - [ "Key1=Value1,Key2=Value2,Key3=Value3", new Dictionary() - { - { "Key1", "Value1" }, - { "Key2", "Value2" }, - { "Key3", "Value3" } - }], - [ "Key1=Value1; Key2=Value2", new Dictionary() - { - { "Key1", "Value1" }, - { "Key2", "Value2" } - }], - [ "Key1=Value1; Key2=Value2; Key3=Value3", new Dictionary() - { - { "Key1", "Value1" }, - { "Key2", "Value2" }, - { "Key3", "Value3" } - }], - [ "Key1=Value1; Key2=Value2; Key1=Value3", new Dictionary() - { - { "Key1", "Value1" }, - { "Key2", "Value2" } - }], - ]; -} diff --git a/test/Elastic.Apm.Tests/HelpersTests/CookieHeaderRedactionFilterTests.cs b/test/Elastic.Apm.Tests/HelpersTests/CookieHeaderRedactionFilterTests.cs new file mode 100644 index 000000000..ec6c6916d --- /dev/null +++ b/test/Elastic.Apm.Tests/HelpersTests/CookieHeaderRedactionFilterTests.cs @@ -0,0 +1,148 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Collections.Generic; +using Elastic.Apm.Config; +using Elastic.Apm.Filters; +using Elastic.Apm.Helpers; +using FluentAssertions; +using Xunit; + +namespace Elastic.Apm.Tests.HelpersTests; + +public class CookieHeaderRedactionFilterTests +{ + [Theory] + [MemberData(nameof(TestData))] + public void RedactsHeadersCorrectly(Dictionary headers, IReadOnlyList sanitizeFieldNames, Dictionary expectedHeaders) + { + CookieHeaderRedactionFilter.HandleCookieHeader(headers, sanitizeFieldNames); + + if (expectedHeaders is null) + { + headers.Should().BeNull(); + } + else + { + headers.Should().Contain(expectedHeaders); + } + } + + public static TheoryData, IReadOnlyList, Dictionary> TestData() => + new() + { + // When the input is null, the output should also be null + { null, ConfigConsts.DefaultValues.SanitizeFieldNames, null }, + + // Standard Cookie header, with a single cookie item whose name does not match the redaction list + { + new Dictionary() { ["Cookie"] = "Cookie1=Value1" }, + ConfigConsts.DefaultValues.SanitizeFieldNames, + new Dictionary() { ["Cookie"] = "Cookie1=Value1" } + }, + + // Standard Cookie header, with a multiple cookie items whose names do not match the redaction list + { + new Dictionary() { ["Cookie"] = "Cookie1=Value1; Cookie2=Value2" }, + ConfigConsts.DefaultValues.SanitizeFieldNames, + new Dictionary() { ["Cookie"] = "Cookie1=Value1; Cookie2=Value2" } + }, + + // In the unlikely case that a request includes multiple cookie headers, + // including some using non-standard casing, we expect each one to be + // returned. + { + new Dictionary() + { + ["Cookie"] = "Cookie1=Value1", + ["cookie"] = "Cookie2=Value2", + ["COOKIE"] = "Cookie3=Value3" + }, + ConfigConsts.DefaultValues.SanitizeFieldNames, + new Dictionary() + { + ["Cookie"] = "Cookie1=Value1", + ["cookie"] = "Cookie2=Value2", + ["COOKIE"] = "Cookie3=Value3" + } + }, + + // If the request contained multiple `Cookie` headers, our code will store each value + // on the `Context.Request.Headers` dictionary, concatenated by commas. + { + new Dictionary() + { + ["Cookie"] = "Cookie1=Value1,Cookie2=Value2" + }, + ConfigConsts.DefaultValues.SanitizeFieldNames, + new Dictionary() + { + ["Cookie"] = "Cookie1=Value1,Cookie2=Value2" + } + }, + + // If the request contained multiple `Cookie` headers, our code will store each value + // on the `Context.Request.Headers` dictionary, concatenated by commas. In this scenario, + // one of the headers includes multiple cookie items + { + new Dictionary() + { + ["Cookie"] = "Cookie1=Value1,Cookie2=Value2; Cookie3=Value3" + }, + ConfigConsts.DefaultValues.SanitizeFieldNames, + new Dictionary() + { + ["Cookie"] = "Cookie1=Value1,Cookie2=Value2; Cookie3=Value3" + } + }, + + // Standard Cookie header, with a single cookie item whose name matches the redaction list + { + new Dictionary() { ["Cookie"] = "Authorization=Bearer ABC123" }, + ConfigConsts.DefaultValues.SanitizeFieldNames, + new Dictionary() { ["Cookie"] = "Authorization=[REDACTED]" } + }, + + // Standard Cookie header, with a multiple cookie items, one of which has a name + // matching the redaction list + { + new Dictionary() { ["Cookie"] = "Cookie1=Value1; Authorization=Bearer ABC123" }, + ConfigConsts.DefaultValues.SanitizeFieldNames, + new Dictionary() { ["Cookie"] = "Cookie1=Value1; Authorization=[REDACTED]" } + }, + + // If the request contained multiple `Cookie` headers, our code will store each value + // on the `Context.Request.Headers` dictionary, concatenated by commas. In this scenario, + // one of the headers includes a cookie item that should be redacted. + { + new Dictionary() { ["Cookie"] = "Authorization=Bearer ABC123,Cookie1=Value1" }, + ConfigConsts.DefaultValues.SanitizeFieldNames, + new Dictionary() { ["Cookie"] = "Authorization=[REDACTED],Cookie1=Value1" } + }, + + // An example where we have multiple `Cookie` headers, some with multiple items, some of which + // require redaction. + { + new Dictionary() { ["Cookie"] = "Cookie1=Value1,Cookie2=Value2; Authorization=Bearer ABC123,Cookie3=Value3; password=thisissecret!" }, + ConfigConsts.DefaultValues.SanitizeFieldNames, + new Dictionary() { ["Cookie"] = "Cookie1=Value1,Cookie2=Value2; Authorization=[REDACTED],Cookie3=Value3; password=[REDACTED]" } + }, + + // MALFORMED EXAMPLES + + // This example includes an invalid cookie entry. We expect to preserve this as-is and cannot redact it. + { + new Dictionary() { ["Cookie"] = "Cookie1" }, + ConfigConsts.DefaultValues.SanitizeFieldNames, + new Dictionary() { ["Cookie"] = "Cookie1" } + }, + + // This example includes non-standard whitespace. We expect to preserve this as-is. + { + new Dictionary() { ["Cookie"] = "Cookie1=Value1; Cookie2=Value2" }, + ConfigConsts.DefaultValues.SanitizeFieldNames, + new Dictionary() { ["Cookie"] = "Cookie1=Value1; Cookie2=Value2" } + } + }; +} diff --git a/test/iis/Elastic.Apm.AspNetFullFramework.Tests/SanitizeFieldNamesTests.cs b/test/iis/Elastic.Apm.AspNetFullFramework.Tests/RedactFieldNamesTests.cs similarity index 74% rename from test/iis/Elastic.Apm.AspNetFullFramework.Tests/SanitizeFieldNamesTests.cs rename to test/iis/Elastic.Apm.AspNetFullFramework.Tests/RedactFieldNamesTests.cs index 63f167d93..603a8a939 100644 --- a/test/iis/Elastic.Apm.AspNetFullFramework.Tests/SanitizeFieldNamesTests.cs +++ b/test/iis/Elastic.Apm.AspNetFullFramework.Tests/RedactFieldNamesTests.cs @@ -10,9 +10,9 @@ namespace Elastic.Apm.AspNetFullFramework.Tests { [Collection(Consts.AspNetFullFrameworkTestsCollection)] - public class SanitizeFieldNamesTests : TestsBase + public class RedactFieldNamesTests : TestsBase { - public SanitizeFieldNamesTests(ITestOutputHelper xUnitOutputHelper) + public RedactFieldNamesTests(ITestOutputHelper xUnitOutputHelper) : base(xUnitOutputHelper) { } @@ -42,12 +42,8 @@ await WaitAndCustomVerifyReceivedData(receivedData => receivedData.Transactions.Should().ContainSingle(); receivedData.Transactions[0].Context.Should().NotBeNull(); receivedData.Transactions[0].Context.Request.Should().NotBeNull(); - receivedData.Transactions[0].Context.Request.Cookies.Should().NotBeNull(); - receivedData.Transactions[0].Context.Request.Cookies.Should().NotBeNull(); - receivedData.Transactions[0].Context.Request.Headers["Cookie"].Should().Be(Apm.Consts.Redacted); - receivedData.Transactions[0].Context.Request.Cookies["MySecureCookie"].Should().Be(Apm.Consts.Redacted); - receivedData.Transactions[0].Context.Request.Cookies["SafeCookie"].Should().Be("This is safe to record and should not be redacted."); + receivedData.Transactions[0].Context.Request.Headers["Cookie"].Should().Be($"password={Apm.Consts.Redacted}; SafeCookie=This is safe to record and should not be redacted."); }, false); } } diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs index e30113b6b..16bbff946 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs @@ -460,7 +460,7 @@ public async Task CustomWithRequestBodyNoError(string sanitizeFieldNames, string } [Fact] - public async Task SanitizesCookieHeaders() + public async Task RedactCookieHeaders() { CreateAgent("set-cookie, mysecurecookie"); @@ -483,12 +483,9 @@ public async Task SanitizesCookieHeaders() _capturedPayload.Transactions.Should().ContainSingle(); _capturedPayload.FirstTransaction.Context.Should().NotBeNull(); _capturedPayload.FirstTransaction.Context.Request.Should().NotBeNull(); - _capturedPayload.FirstTransaction.Context.Request.Cookies.Should().NotBeNull(); _capturedPayload.FirstTransaction.Context.Response.Should().NotBeNull(); - _capturedPayload.FirstTransaction.Context.Request.Headers["Cookie"].Should().Be(Apm.Consts.Redacted); - _capturedPayload.FirstTransaction.Context.Request.Cookies["MySecureCookie"].Should().Be(Apm.Consts.Redacted); - _capturedPayload.FirstTransaction.Context.Request.Cookies["SafeCookie"].Should().Be("123"); + _capturedPayload.FirstTransaction.Context.Request.Headers["Cookie"].Should().Be($"MySecureCookie={Apm.Consts.Redacted}; SafeCookie=123"); } } }