Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanitize Central config request URI and headers in logs #1471

Merged
merged 8 commits into from
Sep 3, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 4 additions & 88 deletions src/Elastic.Apm/Api/Http.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System;
using System.Net.Http;
using Elastic.Apm.Api.Constraints;
using Elastic.Apm.Helpers;
using Elastic.Apm.Report.Serialization;
using Elastic.Apm.Libraries.Newtonsoft.Json;

Expand All @@ -23,7 +24,7 @@ public class Http
public string Method { get; set; }

/// <summary>
/// The Url in its original form as it was passed to the Agent, without sanitization or any other trimming.
/// The Url in its original form as it was passed to the Agent, without sanitization or trimming.
/// </summary>
internal Uri OriginalUrl => _originalUrl;

Expand All @@ -39,7 +40,7 @@ public class Http
public string Url
{
get => _url;
set => _url = Sanitize(value, out var newValue, out _originalUrl) ? newValue : value;
set => _url = Sanitization.TrySanitizeUrl(value, out var newValue, out _originalUrl) ? newValue : value;
}

/// <summary>
Expand All @@ -53,97 +54,12 @@ internal void SetUrl(Uri uri)
_originalUrl = uri;
try
{
_url = string.IsNullOrEmpty(uri.UserInfo) ? uri.ToString() : SanitizeUserNameAndPassword(uri).ToString();
_url = uri.Sanitize().ToString();
}
catch
{
_url = string.Empty;
}
}

/// <summary>
/// Removes the username and password from the <paramref name="uri" /> and returns it as a <see cref="string" />.
/// If there is no username and password in the <paramref name="uri" />, the simple string representation is returned.
/// </summary>
/// <param name="uri">The URI that you'd like to sanitize.</param>
/// <returns>The string representation of <paramref name="uri" /> without username and password.</returns>
internal static string Sanitize(Uri uri)
{
Sanitize(uri, out var result);
return result;
}

/// <summary>
/// Returns the .ToString representation of an <see cref="HttpRequestMessage" />
/// but makes sure that the username and the password is sanitized.
/// </summary>
/// <param name="msg"></param>
/// <returns></returns>
internal static string HttpRequestSanitizedToString(HttpRequestMessage msg)
{
if (!string.IsNullOrEmpty(msg.RequestUri?.UserInfo))
msg.RequestUri = new Uri(Sanitize(msg.RequestUri));

return msg.ToString();
}

private static bool Sanitize(string uriString, out string result, out Uri originalUrl)
{
try
{
originalUrl = new Uri(uriString, UriKind.RelativeOrAbsolute);
return Sanitize(originalUrl, out result);
}
catch
{
result = null;
originalUrl = null;
return false;
}
}

/// <summary>
/// Returns <code>true</code> if sanitization was applied, <code>false</code> otherwise.
/// In some cases turning a string into a URL and then turning it back to a string adds a trailing `/`.
/// To avoid this problem, in the <paramref name="result" /> parameter the input is returned if there is nothing to change
/// on the input.
/// </summary>
/// <param name="uriString">The Uri to sanitize.</param>
/// <param name="result">
/// The result, which is the sanitized string. If no sanitization was needed
/// (because there was no username and password in the URL) then this contains the <paramref name="result" /> parameter.
/// </param>
/// <returns></returns>
internal static bool Sanitize(string uriString, out string result) =>
Sanitize(uriString, out result, out _);

internal static bool Sanitize(Uri uri, out string result)
{
try
{
if (string.IsNullOrEmpty(uri.UserInfo))
{
result = uri.ToString();
return false;
}
result = SanitizeUserNameAndPassword(uri).ToString();
return true;
}
catch
{
result = null;
return false;
}
}

private static Uri SanitizeUserNameAndPassword(Uri uri)
{
var builder = new UriBuilder(uri)
{
UserName = "[REDACTED]",
Password = "[REDACTED]"
};
return builder.Uri;
}
}
}
4 changes: 2 additions & 2 deletions src/Elastic.Apm/Api/Request.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public class Url
public string Full
{
get => _full;
set => _full = Http.Sanitize(value, out var newValue) ? newValue : value;
set => _full = Sanitization.TrySanitizeUrl(value, out var newValue, out _) ? newValue : value;
}

[MaxLength]
Expand All @@ -95,7 +95,7 @@ public string Full
public string Raw
{
get => _raw;
set => _raw = Http.Sanitize(value, out var newValue) ? newValue : value;
set => _raw = Sanitization.TrySanitizeUrl(value, out var newValue, out _) ? newValue : value;
}

/// <summary>
Expand Down
10 changes: 5 additions & 5 deletions src/Elastic.Apm/BackendComm/BackendCommUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,19 @@ internal static Uri BuildIntakeV2EventsAbsoluteUrl(Uri baseUrl) =>
/// service.name and service.environment are URL encoded in the returned URL.</param>
internal static Uri BuildGetConfigAbsoluteUrl(Uri baseUrl, Service service)
{
var strBuilder = new StringBuilder("config/v1/agents");
var builder = new StringBuilder("config/v1/agents");
var prefix = '?';

if (service.Name != null)
{
strBuilder.Append(prefix).Append($"service.name={UrlEncode(service.Name)}");
builder.Append(prefix).Append("service.name=").Append(UrlEncode(service.Name));
prefix = '&';
}

if (service.Environment != null)
strBuilder.Append(prefix).Append($"service.environment={UrlEncode(service.Environment)}");
builder.Append(prefix).Append("service.environment=").Append(UrlEncode(service.Environment));

return CombineAbsoluteAndRelativeUrls(baseUrl, /* relativeUri: */ strBuilder.ToString());
return CombineAbsoluteAndRelativeUrls(baseUrl, builder.ToString());
}

/// <summary>
Expand Down Expand Up @@ -190,7 +190,7 @@ internal static HttpClient BuildHttpClient(IApmLogger loggerArg, IConfigSnapshot

logger.Debug()
?.Log("Building HTTP client with BaseAddress: {ApmServerUrl} for {dbgCallerDesc}..."
, serverUrlBase, dbgCallerDesc);
, serverUrlBase.Sanitize(), dbgCallerDesc);
var httpClient =
new HttpClient(httpMessageHandler ?? CreateHttpClientHandler(config, loggerArg)) { BaseAddress = serverUrlBase };
httpClient.DefaultRequestHeaders.UserAgent.Add(
Expand Down
20 changes: 9 additions & 11 deletions src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigFetcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ private CentralConfigFetcher(IApmLogger logger, IConfigStore configStore, IConfi
_getConfigAbsoluteUrl = BackendCommUtils.ApmServerEndpoints.BuildGetConfigAbsoluteUrl(initialConfigSnapshot.ServerUrl, service);
_logger.Debug()
?.Log("Combined absolute URL for APM Server get central configuration endpoint: `{Url}'. Service: {Service}."
, _getConfigAbsoluteUrl, service);
, _getConfigAbsoluteUrl.Sanitize(), service);

StartWorkLoop();
}
Expand Down Expand Up @@ -141,16 +141,14 @@ protected override async Task WorkLoopIteration()
+ Environment.NewLine + "+-> Response:{HttpResponse}"
+ Environment.NewLine + "+-> Response body [length: {HttpResponseBodyLength}]:{HttpResponseBody}"
, _eTag.AsNullableToString()
, Http.Sanitize(_getConfigAbsoluteUrl, out var sanitizedAbsoluteUrl) ? sanitizedAbsoluteUrl : _getConfigAbsoluteUrl.ToString()
, Http.Sanitize(HttpClient.BaseAddress, out var sanitizedBaseAddress)
? sanitizedBaseAddress
: HttpClient.BaseAddress.ToString()
, _getConfigAbsoluteUrl.Sanitize().ToString()
, HttpClient.BaseAddress.Sanitize().ToString()
, waitInfo.Interval.ToHms(),
_dbgIterationsCount
, httpRequest == null ? " N/A" : Environment.NewLine + TextUtils.Indent(Http.HttpRequestSanitizedToString(httpRequest))
, httpResponse == null ? " N/A" : Environment.NewLine + TextUtils.Indent(httpResponse.ToString())
, httpRequest == null ? " N/A" : Environment.NewLine + httpRequest.Sanitize(_configStore.CurrentSnapshot.SanitizeFieldNames).ToString().Indent()
, httpResponse == null ? " N/A" : Environment.NewLine + httpResponse.ToString().Indent()
, httpResponseBody == null ? "N/A" : httpResponseBody.Length.ToString()
, httpResponseBody == null ? " N/A" : Environment.NewLine + TextUtils.Indent(httpResponseBody));
, httpResponseBody == null ? " N/A" : Environment.NewLine + httpResponseBody.Indent());
}
finally
{
Expand All @@ -173,16 +171,16 @@ private HttpRequestMessage BuildHttpRequest(EntityTagHeaderValue eTag)

private async Task<(HttpResponseMessage, string)> FetchConfigHttpResponseImplAsync(HttpRequestMessage httpRequest)
{
_logger.Trace()?.Log("Making HTTP request to APM Server... Request: {HttpRequest}.", httpRequest);
_logger.Trace()?.Log("Making HTTP request to APM Server... Request: {HttpRequest}.", httpRequest.RequestUri.Sanitize());

var httpResponse = await HttpClient.SendAsync(httpRequest, HttpCompletionOption.ResponseContentRead, CancellationTokenSource.Token)
.ConfigureAwait(false);
// ReSharper disable once InvertIf
if (httpResponse == null)
{
throw new FailedToFetchConfigException("HTTP client API call for request to APM Server returned null."
+ $" Request:{Environment.NewLine}{TextUtils.Indent(httpRequest.ToString())}",
new WaitInfoS(WaitTimeIfAnyError, "HttpResponseMessage from APM Server is equal to null"));
+ $" Request:{Environment.NewLine}{httpRequest.Sanitize(_configStore.CurrentSnapshot.SanitizeFieldNames).ToString().Indent()}",
new WaitInfoS(WaitTimeIfAnyError, "HttpResponseMessage from APM Server is null"));
}

_logger.Trace()?.Log("Reading HTTP response body... Response: {HttpResponse}.", httpResponse);
Expand Down
12 changes: 6 additions & 6 deletions src/Elastic.Apm/Consts.cs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
// 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

// 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
namespace Elastic.Apm
{
internal static class Consts
{
internal const int PropertyMaxLength = 1024;

internal static string AgentName => "dotnet";
internal const string AgentName = "dotnet";

internal static string Redacted = "[REDACTED]";
internal const string Redacted = "[REDACTED]";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Reflection;
using Elastic.Apm.Api;
using Elastic.Apm.DistributedTracing;
using Elastic.Apm.Helpers;
using Elastic.Apm.Logging;
using Elastic.Apm.Model;

Expand Down Expand Up @@ -103,7 +104,7 @@ protected override void HandleOnNext(KeyValuePair<string, object> kv)

if (IsRequestFilteredOut(requestUrl))
{
Logger.Trace()?.Log("Request URL ({RequestUrl}) is filtered out - exiting", Http.Sanitize(requestUrl));
Logger.Trace()?.Log("Request URL ({RequestUrl}) is filtered out - exiting", requestUrl.Sanitize());
return;
}

Expand All @@ -124,7 +125,7 @@ protected override void HandleOnNext(KeyValuePair<string, object> kv)

private void ProcessStartEvent(TRequest request, Uri requestUrl)
{
Logger.Trace()?.Log("Processing start event... Request URL: {RequestUrl}", Http.Sanitize(requestUrl));
Logger.Trace()?.Log("Processing start event... Request URL: {RequestUrl}", requestUrl.Sanitize());

var transaction = ApmAgent.Tracer.CurrentTransaction;
if (transaction is null)
Expand Down Expand Up @@ -174,13 +175,13 @@ private void ProcessStartEvent(TRequest request, Uri requestUrl)

if (span is null)
{
Logger.Trace()?.Log("Could not create span for outgoing HTTP request to {RequestUrl}", Http.Sanitize(requestUrl));
Logger.Trace()?.Log("Could not create span for outgoing HTTP request to {RequestUrl}", requestUrl.Sanitize());
return;
}
}
else
{
Logger.Trace()?.Log("Skip creating span for outgoing HTTP request to {RequestUrl} as not to known service", Http.Sanitize(requestUrl));
Logger.Trace()?.Log("Skip creating span for outgoing HTTP request to {RequestUrl} as not to known service", requestUrl.Sanitize());
return;
}
}
Expand Down Expand Up @@ -231,7 +232,7 @@ private void PropagateTraceContext(TRequest request, ITransaction transaction, I

private void ProcessStopEvent(object eventValue, TRequest request, Uri requestUrl)
{
Logger.Trace()?.Log("Processing stop event... Request URL: {RequestUrl}", Http.Sanitize(requestUrl));
Logger.Trace()?.Log("Processing stop event... Request URL: {RequestUrl}", requestUrl.Sanitize().ToString());

if (!ProcessingRequests.TryRemove(request, out var span))
{
Expand All @@ -241,13 +242,13 @@ private void ProcessStopEvent(object eventValue, TRequest request, Uri requestUr
{
Logger.Debug()
?.Log("{eventName} called with no active current transaction, url: {url} - skipping event", nameof(ProcessStopEvent),
Http.Sanitize(requestUrl));
requestUrl.Sanitize().ToString());
}
else
{
Logger.Debug()
?.Log("Could not remove request from processing requests. This likely means it was not captured to begin with." +
"Request: method: {HttpMethod}, URL: {RequestUrl}", RequestGetMethod(request), Http.Sanitize(requestUrl));
"Request: method: {HttpMethod}, URL: {RequestUrl}", RequestGetMethod(request), requestUrl.Sanitize().ToString());
}

return;
Expand Down Expand Up @@ -285,7 +286,7 @@ internal static void SetOutcome(ISpan span, int statusCode) =>

protected virtual void ProcessExceptionEvent(object eventValue, Uri requestUrl)
{
Logger.Trace()?.Log("Processing exception event... Request URL: {RequestUrl}", Http.Sanitize(requestUrl));
Logger.Trace()?.Log("Processing exception event... Request URL: {RequestUrl}", requestUrl.Sanitize().ToString());

if (!(eventValue.GetType().GetTypeInfo().GetDeclaredProperty(EventExceptionPropertyName)?.GetValue(eventValue) is Exception exception))
{
Expand Down
Loading