Skip to content

Commit

Permalink
Merge branch 'main' into gh-oblt/replace-token-with-app
Browse files Browse the repository at this point in the history
  • Loading branch information
v1v authored Sep 18, 2024
2 parents 1b98c59 + 31cb8cb commit e35f89d
Show file tree
Hide file tree
Showing 15 changed files with 526 additions and 256 deletions.
22 changes: 22 additions & 0 deletions .github/workflows/test-reporter.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
on:
workflow_run:
workflows:
- test-windows
types:
- completed

permissions:
contents: read
actions: read
checks: write

jobs:
report:
runs-on: ubuntu-latest
steps:
- uses: elastic/oblt-actions/test-report@v1
with:
artifact: /test-results(.*)/
name: 'Test Repot $1'
path: "**/*.xml"
reporter: java-junit
2 changes: 1 addition & 1 deletion .github/workflows/test-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ jobs:
- name: Store test results
if: success() || failure()
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4
with:
name: test-results-iis
path: build/output/junit-*.xml
31 changes: 31 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,37 @@ endif::[]
[[release-notes-1.x]]
=== .NET Agent version 1.x
[[release-notes-1.29.0]]
==== 1.29.0 - 2024/09/18
This release includes a breaking change in how we parse and send transaction cookies. In 1.26.0, we
introduced improved cookie redaction based on the `SanitizeFieldNames` configuration. To implement this,
we extracted each cookie from the `Cookie` header, storing them in a cookie dictionary on the transaction request data.
We have identified a problem with the storage of cookies that include period characters due to the mapping of such data
when stored in the APM data stream. This behaviour can lead to lost transactions on requests which include such cookies.
This is common in ASP.NET Core due to the default cookie names used for sessions, authentication, etc.
In this release, we no longer parse out individual cookies, and the cookie `Dictionary` has been removed from the
data model. This means that cookies will no longer be indexed individually. However, we have ensured that we
retain the primary reason for the earlier change, which was to redact the values of sensitive cookies. Any cookies with
a name matching the `SanitizeFieldNames` patterns will be redacted in the value of the `Cookie` header
we store.
For most consumers, we expect the impact to be minimal. However, if you were relying on the parsed cookie
fields, adjustments will be necessary to work with the `Cookie` header value instead.
===== Breaking changes
{pull}2444[#2444] No longer parse request cookies, but ensure they are still redacted in the `Cookie` header string
[[release-notes-1.28.6]]
==== 1.28.6 - 2024/09/11
===== Bug fixes
{pull}2431[#2431] Hard exclude several system processes from being auto instrumented.
{pull}2436[#2436] Disabling the agent should not try to enqueue events, now a NOOP.
[[release-notes-1.28.5]]
==== 1.28.5 - 2024/08/28
Expand Down
3 changes: 1 addition & 2 deletions docs/configuration.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
7 changes: 0 additions & 7 deletions src/Elastic.Apm/Api/Request.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@ public class Request
/// </summary>
public Dictionary<string, string> Headers { get; set; }

/// <summary>
/// This field is sanitized by a filter
/// </summary>
public Dictionary<string, string> Cookies { get; set; }

[JsonProperty("http_version")]
[MaxLength]
public string HttpVersion { get; set; }
Expand All @@ -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();
Expand Down
91 changes: 91 additions & 0 deletions src/Elastic.Apm/Filters/CookieHeaderRedactionFilter.cs
Original file line number Diff line number Diff line change
@@ -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
{
/// <summary>
/// Redacts items from the cookie list of the Cookie request header.
/// </summary>
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<string, string> headers, IReadOnlyList<WildcardMatcher> 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<string>.Shared.Rent(maxKeys);
var matchedValues = ArrayPool<string>.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<string>.Shared.Return(matchedKeys);
ArrayPool<string>.Shared.Return(matchedValues);
#endif
}
}
}
}
53 changes: 0 additions & 53 deletions src/Elastic.Apm/Filters/RequestCookieExtractionFilter.cs

This file was deleted.

3 changes: 0 additions & 3 deletions src/Elastic.Apm/Filters/Sanitization.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
108 changes: 0 additions & 108 deletions src/Elastic.Apm/Helpers/CookieHeaderParser.cs

This file was deleted.

Loading

0 comments on commit e35f89d

Please sign in to comment.