diff --git a/src/Elastic.Apm/Api/Http.cs b/src/Elastic.Apm/Api/Http.cs index 03c079ae0..871a7d83b 100644 --- a/src/Elastic.Apm/Api/Http.cs +++ b/src/Elastic.Apm/Api/Http.cs @@ -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; @@ -23,7 +24,7 @@ public class Http public string Method { get; set; } /// - /// 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. /// internal Uri OriginalUrl => _originalUrl; @@ -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; } /// @@ -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; } } - - /// - /// Removes the username and password from the and returns it as a . - /// If there is no username and password in the , the simple string representation is returned. - /// - /// The URI that you'd like to sanitize. - /// The string representation of without username and password. - internal static string Sanitize(Uri uri) - { - Sanitize(uri, out var result); - return result; - } - - /// - /// Returns the .ToString representation of an - /// but makes sure that the username and the password is sanitized. - /// - /// - /// - 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; - } - } - - /// - /// Returns true if sanitization was applied, false 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 parameter the input is returned if there is nothing to change - /// on the input. - /// - /// The Uri to sanitize. - /// - /// 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 parameter. - /// - /// - 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; - } } } diff --git a/src/Elastic.Apm/Api/Request.cs b/src/Elastic.Apm/Api/Request.cs index 733353e19..283fbcf23 100644 --- a/src/Elastic.Apm/Api/Request.cs +++ b/src/Elastic.Apm/Api/Request.cs @@ -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] @@ -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; } /// diff --git a/src/Elastic.Apm/BackendComm/BackendCommUtils.cs b/src/Elastic.Apm/BackendComm/BackendCommUtils.cs index a3a85fd4a..1abcba1ed 100644 --- a/src/Elastic.Apm/BackendComm/BackendCommUtils.cs +++ b/src/Elastic.Apm/BackendComm/BackendCommUtils.cs @@ -46,19 +46,19 @@ internal static Uri BuildIntakeV2EventsAbsoluteUrl(Uri baseUrl) => /// service.name and service.environment are URL encoded in the returned URL. 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()); } /// @@ -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( diff --git a/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigFetcher.cs b/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigFetcher.cs index 27784e01a..4fda03d85 100644 --- a/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigFetcher.cs +++ b/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigFetcher.cs @@ -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(); } @@ -92,7 +92,6 @@ private CentralConfigFetcher(IApmLogger logger, IConfigStore configStore, IConfi protected override async Task WorkLoopIteration() { ++_dbgIterationsCount; - var waitingLogSeverity = LogLevel.Trace; WaitInfoS waitInfo; HttpRequestMessage httpRequest = null; HttpResponseMessage httpResponse = null; @@ -118,39 +117,34 @@ protected override async Task WorkLoopIteration() } catch (Exception ex) { - var severity = LogLevel.Error; + var level = LogLevel.Error; - if (ex is FailedToFetchConfigException fEx) + if (ex is FailedToFetchConfigException fetchConfigException) { - severity = fEx.Severity; - waitInfo = fEx.WaitInfo; + waitInfo = fetchConfigException.WaitInfo; + level = fetchConfigException.Severity; } else { waitInfo = new WaitInfoS(WaitTimeIfAnyError, "Default wait time is used because exception was thrown" - + " while fetching configuration from APM Server and parsing it."); + + " while fetching configuration from APM Server and parsing it."); } - if (severity == LogLevel.Error) waitingLogSeverity = LogLevel.Information; - - _logger.IfLevel(severity) - ?.LogException(ex, "Exception was thrown while fetching configuration from APM Server and parsing it." + _logger.IfLevel(level)?.LogException(ex, "Exception was thrown while fetching configuration from APM Server and parsing it." + " ETag: `{ETag}'. URL: `{Url}'. Apm Server base URL: `{ApmServerUrl}'. WaitInterval: {WaitInterval}." + " dbgIterationsCount: {dbgIterationsCount}." + Environment.NewLine + "+-> Request:{HttpRequest}" + 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 { @@ -158,8 +152,7 @@ protected override async Task WorkLoopIteration() httpResponse?.Dispose(); } - _logger.IfLevel(waitingLogSeverity) - ?.Log("Waiting {WaitInterval}... {WaitReason}. dbgIterationsCount: {dbgIterationsCount}." + _logger.Info()?.Log("Waiting {WaitInterval}... {WaitReason}. dbgIterationsCount: {dbgIterationsCount}." , waitInfo.Interval.ToHms(), waitInfo.Reason, _dbgIterationsCount); await _agentTimer.Delay(_agentTimer.Now + waitInfo.Interval, CancellationTokenSource.Token).ConfigureAwait(false); } @@ -173,7 +166,7 @@ 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); @@ -181,8 +174,8 @@ private HttpRequestMessage BuildHttpRequest(EntityTagHeaderValue eTag) 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); diff --git a/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigResponseParser.cs b/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigResponseParser.cs index 355eb837a..8881af0af 100644 --- a/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigResponseParser.cs +++ b/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigResponseParser.cs @@ -86,12 +86,10 @@ private bool InterpretResponseStatusCode(HttpResponseMessage httpResponse, Centr { if (httpResponse.IsSuccessStatusCode) return true; - var statusCode = (int)httpResponse.StatusCode; - var severity = 400 <= statusCode && statusCode < 500 ? LogLevel.Debug : LogLevel.Error; - - string message; + var severity = LogLevel.Error; var statusAsString = $"HTTP status code is {httpResponse.ReasonPhrase} ({(int)httpResponse.StatusCode})"; - var msgPrefix = $"{statusAsString} which most likely means that "; + string message; + // ReSharper disable once SwitchStatementMissingSomeCases switch (httpResponse.StatusCode) { @@ -107,21 +105,24 @@ private bool InterpretResponseStatusCode(HttpResponseMessage httpResponse, Centr return false; case HttpStatusCode.BadRequest: // 400 - severity = LogLevel.Error; message = $"{statusAsString} which is unexpected"; break; case HttpStatusCode.Forbidden: // 403 - message = msgPrefix + "APM Server supports the central configuration endpoint but Kibana connection is not enabled"; + severity = LogLevel.Debug; + message = $"{statusAsString} which most likely means that APM Server supports the central configuration " + + "endpoint but Kibana connection is not enabled"; break; case HttpStatusCode.NotFound: // 404 - message = msgPrefix + "APM Server is an old (pre 7.3) version which doesn't support the central configuration endpoint"; + severity = LogLevel.Debug; + message = $"{statusAsString} which most likely means that APM Server is an old (pre 7.3) version which " + + "doesn't support the central configuration endpoint"; break; case HttpStatusCode.ServiceUnavailable: // 503 - message = msgPrefix + "APM Server supports the central configuration endpoint and Kibana connection is enabled" - + ", but Kibana connection is unavailable"; + message = $"{statusAsString} which most likely means that APM Server supports the central configuration " + + "endpoint and Kibana connection is enabled, but Kibana connection is unavailable"; break; default: diff --git a/src/Elastic.Apm/Consts.cs b/src/Elastic.Apm/Consts.cs index 686368919..88764e8a1 100644 --- a/src/Elastic.Apm/Consts.cs +++ b/src/Elastic.Apm/Consts.cs @@ -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]"; } } diff --git a/src/Elastic.Apm/DiagnosticListeners/HttpDiagnosticListenerImplBase.cs b/src/Elastic.Apm/DiagnosticListeners/HttpDiagnosticListenerImplBase.cs index 450630798..1836a894d 100644 --- a/src/Elastic.Apm/DiagnosticListeners/HttpDiagnosticListenerImplBase.cs +++ b/src/Elastic.Apm/DiagnosticListeners/HttpDiagnosticListenerImplBase.cs @@ -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; @@ -103,7 +104,7 @@ protected override void HandleOnNext(KeyValuePair 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; } @@ -124,7 +125,7 @@ protected override void HandleOnNext(KeyValuePair 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) @@ -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; } } @@ -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)) { @@ -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; @@ -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)) { diff --git a/src/Elastic.Apm/Helpers/Sanitization.cs b/src/Elastic.Apm/Helpers/Sanitization.cs new file mode 100644 index 000000000..c0e1b0661 --- /dev/null +++ b/src/Elastic.Apm/Helpers/Sanitization.cs @@ -0,0 +1,89 @@ +// 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.Generic; +using System.Linq; +using System.Net.Http; + +namespace Elastic.Apm.Helpers +{ + internal static class Sanitization + { + /// + /// Sanitizes a , redacting user info from the request URI and HTTP headers that + /// match any of the matchers. + /// + /// + /// The instance is mutated, so this method should not be used for logging + /// **before** using the instance in a request. + /// + /// The message to sanitize + /// The matchers used to redact HTTP headers + /// A sanitized + internal static HttpRequestMessage Sanitize(this HttpRequestMessage message, IReadOnlyList matchers) + { + if (message is null) + return null; + + message.RequestUri = message.RequestUri.Sanitize(); + + var headers = message.Headers.Select(h => h.Key).ToList(); + foreach (var header in headers) + { + if (WildcardMatcher.IsAnyMatch(matchers, header) && message.Headers.Remove(header)) + message.Headers.TryAddWithoutValidation(header, Consts.Redacted); + } + + return message; + } + + /// + /// Redacts username and password, if present + /// + internal static Uri Sanitize(this Uri uri) + { + if (string.IsNullOrEmpty(uri.UserInfo)) + return uri; + + var builder = new UriBuilder(uri) + { + UserName = Consts.Redacted, + Password = Consts.Redacted + }; + return builder.Uri; + } + + internal static bool Sanitize(Uri uri, out string result) + { + try + { + var sanitized = uri.Sanitize(); + result = sanitized.ToString(); + return sanitized != uri; + } + catch + { + result = null; + return false; + } + } + + internal static bool TrySanitizeUrl(string uri, out string sanitizedUri, out Uri originalUri) + { + try + { + originalUri = new Uri(uri, UriKind.RelativeOrAbsolute); + return Sanitize(originalUri, out sanitizedUri); + } + catch + { + sanitizedUri = null; + originalUri = null; + return false; + } + } + } +} diff --git a/src/Elastic.Apm/Report/PayloadSenderV2.cs b/src/Elastic.Apm/Report/PayloadSenderV2.cs index 6b6309ddb..2937cc925 100644 --- a/src/Elastic.Apm/Report/PayloadSenderV2.cs +++ b/src/Elastic.Apm/Report/PayloadSenderV2.cs @@ -104,7 +104,7 @@ public PayloadSenderV2( + ", FlushInterval: {FlushInterval}" + ", MaxBatchEventCount: {MaxBatchEventCount}" + ", MaxQueueEventCount: {MaxQueueEventCount}" - , _intakeV2EventsAbsoluteUrl, _flushInterval.ToHms(), config.MaxBatchEventCount, _maxQueueEventCount); + , _intakeV2EventsAbsoluteUrl.Sanitize(), _flushInterval.ToHms(), config.MaxBatchEventCount, _maxQueueEventCount); _eventQueue = new BatchBlock(config.MaxBatchEventCount); @@ -323,9 +323,7 @@ private async Task ProcessQueueItems(object[] queueItems) + " Events intake API absolute URL: {EventsIntakeAbsoluteUrl}." + " APM Server response: status code: {ApmServerResponseStatusCode}" + ", content: \n{ApmServerResponseContent}" - , Http.Sanitize(_intakeV2EventsAbsoluteUrl, out var sanitizedServerUrl) - ? sanitizedServerUrl - : _intakeV2EventsAbsoluteUrl.ToString() + , _intakeV2EventsAbsoluteUrl.Sanitize() , response?.StatusCode, response is null ? null : await response.Content.ReadAsStringAsync().ConfigureAwait(false)); } @@ -345,9 +343,7 @@ private async Task ProcessQueueItems(object[] queueItems) ?.Log( "Cancellation requested. Following events were not transferred successfully to the server ({ApmServerUrl}):" + $"{Environment.NewLine}{TextUtils.Indentation}{{SerializedItems}}" - , Http.Sanitize(HttpClient.BaseAddress, out var sanitizedServerUrl) - ? sanitizedServerUrl - : HttpClient.BaseAddress.ToString() + , HttpClient.BaseAddress.Sanitize() , string.Join($",{Environment.NewLine}{TextUtils.Indentation}", queueItems)); // throw to allow Workloop to handle @@ -360,9 +356,7 @@ private async Task ProcessQueueItems(object[] queueItems) e, "Failed sending events. Following events were not transferred successfully to the server ({ApmServerUrl}):" + $"{Environment.NewLine}{TextUtils.Indentation}{{SerializedItems}}" - , Http.Sanitize(HttpClient.BaseAddress, out var sanitizedServerUrl) - ? sanitizedServerUrl - : HttpClient.BaseAddress.ToString() + , HttpClient.BaseAddress.Sanitize() , string.Join($",{Environment.NewLine}{TextUtils.Indentation}", queueItems) ); } diff --git a/test/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs b/test/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs index e89901241..3354a8d7e 100644 --- a/test/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs +++ b/test/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs @@ -190,7 +190,7 @@ public async Task CustomSanitizeFieldNameSettingWithHeaders(string sanitizeField _capturedPayload.FirstTransaction.Context.Request.Headers.Should().NotBeNull(); foreach (var header in headerNames) - _capturedPayload.FirstTransaction.Context.Request.Headers[header].Should().Be("[REDACTED]"); + _capturedPayload.FirstTransaction.Context.Request.Headers[header].Should().Be(Apm.Consts.Redacted); } /// @@ -245,7 +245,7 @@ public async Task ChangeSanitizeFieldNamesAfterStart(bool useDiagnosticSourceOnl _capturedPayload.FirstTransaction.Context.Request.Should().NotBeNull(); _capturedPayload.FirstTransaction.Context.Request.Headers.Should().NotBeNull(); - _capturedPayload.FirstTransaction.Context.Request.Headers["foo"].Should().Be("[REDACTED]"); + _capturedPayload.FirstTransaction.Context.Request.Headers["foo"].Should().Be(Apm.Consts.Redacted); } ///// @@ -273,7 +273,7 @@ public async Task CustomSanitizeFieldNameSettingWithCaseSensitivityWithHeaders(s _capturedPayload.FirstTransaction.Context.Should().NotBeNull(); _capturedPayload.FirstTransaction.Context.Request.Should().NotBeNull(); _capturedPayload.FirstTransaction.Context.Request.Headers.Should().NotBeNull(); - _capturedPayload.FirstTransaction.Context.Request.Headers[headerName].Should().Be(shouldBeSanitized ? "[REDACTED]" : headerValue); + _capturedPayload.FirstTransaction.Context.Request.Headers[headerName].Should().Be(shouldBeSanitized ? Apm.Consts.Redacted : headerValue); } ///// @@ -296,7 +296,7 @@ public async Task DefaultsWithHeaders(string headerName, bool useOnlyDiagnosticS _capturedPayload.FirstTransaction.Context.Should().NotBeNull(); _capturedPayload.FirstTransaction.Context.Request.Should().NotBeNull(); _capturedPayload.FirstTransaction.Context.Request.Headers.Should().NotBeNull(); - _capturedPayload.FirstTransaction.Context.Request.Headers[headerName].Should().Be("[REDACTED]"); + _capturedPayload.FirstTransaction.Context.Request.Headers[headerName].Should().Be(Apm.Consts.Redacted); } /// @@ -317,14 +317,14 @@ public async Task SanitizeHeadersOnError(string headerName, bool useOnlyDiagnost _capturedPayload.FirstTransaction.Context.Should().NotBeNull(); _capturedPayload.FirstTransaction.Context.Request.Should().NotBeNull(); _capturedPayload.FirstTransaction.Context.Request.Headers.Should().NotBeNull(); - _capturedPayload.FirstTransaction.Context.Request.Headers[headerName].Should().Be("[REDACTED]"); + _capturedPayload.FirstTransaction.Context.Request.Headers[headerName].Should().Be(Apm.Consts.Redacted); _capturedPayload.WaitForErrors(); _capturedPayload.Errors.Should().NotBeEmpty(); _capturedPayload.FirstError.Context.Should().NotBeNull(); _capturedPayload.FirstError.Context.Request.Should().NotBeNull(); _capturedPayload.FirstError.Context.Request.Headers.Should().NotBeNull(); - _capturedPayload.FirstError.Context.Request.Headers[headerName].Should().Be("[REDACTED]"); + _capturedPayload.FirstError.Context.Request.Headers[headerName].Should().Be(Apm.Consts.Redacted); } ///// @@ -351,7 +351,7 @@ public async Task DefaultsWithKnownHeaders(string headerName, string returnedHea _capturedPayload.FirstTransaction.Context.Should().NotBeNull(); _capturedPayload.FirstTransaction.Context.Request.Should().NotBeNull(); _capturedPayload.FirstTransaction.Context.Request.Headers.Should().NotBeNull(); - _capturedPayload.FirstTransaction.Context.Request.Headers[returnedHeaderName].Should().Be("[REDACTED]"); + _capturedPayload.FirstTransaction.Context.Request.Headers[returnedHeaderName].Should().Be(Apm.Consts.Redacted); } diff --git a/test/Elastic.Apm.Tests.Utilities/TestLogger.cs b/test/Elastic.Apm.Tests.Utilities/TestLogger.cs index 4060e9c20..590966915 100644 --- a/test/Elastic.Apm.Tests.Utilities/TestLogger.cs +++ b/test/Elastic.Apm.Tests.Utilities/TestLogger.cs @@ -21,6 +21,9 @@ internal class TestLogger : ConsoleLogger private TestLogger(LogLevel level, SynchronizedStringWriter writer) : base(level, writer, writer) => _writer = writer; + /// + /// Gets the log lines + /// public IReadOnlyList Lines { get @@ -34,6 +37,18 @@ public IReadOnlyList Lines } } } + + /// + /// Gets the log + /// + public string Log + { + get + { + lock (_writer.Lock) + return _writer.GetStringBuilder().ToString(); + } + } } public class SynchronizedStringWriter : StringWriter diff --git a/test/Elastic.Apm.Tests/BackendCommTests/CentralConfig/CentralConfigFetcherTests.cs b/test/Elastic.Apm.Tests/BackendCommTests/CentralConfig/CentralConfigFetcherTests.cs index c7df9289a..22d864e02 100644 --- a/test/Elastic.Apm.Tests/BackendCommTests/CentralConfig/CentralConfigFetcherTests.cs +++ b/test/Elastic.Apm.Tests/BackendCommTests/CentralConfig/CentralConfigFetcherTests.cs @@ -32,6 +32,46 @@ public class CentralConfigFetcherTests : LoggingTestBase { public CentralConfigFetcherTests(ITestOutputHelper xUnitOutputHelper) : base(xUnitOutputHelper) { } + [Fact] + public void Should_Sanitize_HttpRequestMessage_In_Log() + { + var testLogger = new TestLogger(LogLevel.Trace); + var secretToken = "secretToken"; + var serverUrl = "http://username:password@localhost:8200"; + + var configSnapshotFromReader = new MockConfigSnapshot(testLogger, logLevel: "Trace", serverUrl: serverUrl, secretToken: secretToken); + var configStore = new ConfigStore(configSnapshotFromReader, testLogger); + var service = Service.GetDefaultService(configSnapshotFromReader, testLogger); + + var waitHandle = new ManualResetEvent(false); + var handler = new MockHttpMessageHandler(); + var configUrl = BackendCommUtils.ApmServerEndpoints + .BuildGetConfigAbsoluteUrl(configSnapshotFromReader.ServerUrl, service); + + handler.When(configUrl.AbsoluteUri) + .Respond(_ => + { + waitHandle.Set(); + return new HttpResponseMessage(HttpStatusCode.ServiceUnavailable); + }); + + var centralConfigFetcher = new CentralConfigFetcher(testLogger, configStore, service, handler); + waitHandle.WaitOne(); + + var count = 0; + + while (!testLogger.Log.Contains("Exception was thrown while fetching configuration from APM Server and parsing it.") + && count < 10) + { + Thread.Sleep(500); + count++; + } + + testLogger.Log + .Should().Contain($"Authorization: {Consts.Redacted}").And.NotContain(secretToken) + .And.NotContain(serverUrl); + } + [Fact] public void Should_Update_Logger_That_Is_ILogLevelSwitchable() { diff --git a/test/Elastic.Apm.Tests/BackendCommTests/PayloadSenderTests.cs b/test/Elastic.Apm.Tests/BackendCommTests/PayloadSenderTests.cs index fc8f0e9f9..6c0bd61a7 100644 --- a/test/Elastic.Apm.Tests/BackendCommTests/PayloadSenderTests.cs +++ b/test/Elastic.Apm.Tests/BackendCommTests/PayloadSenderTests.cs @@ -11,6 +11,7 @@ using System.Threading; using System.Threading.Tasks; using Elastic.Apm.Api; +using Elastic.Apm.BackendComm; using Elastic.Apm.Config; using Elastic.Apm.Helpers; using Elastic.Apm.Logging; @@ -22,6 +23,9 @@ using Xunit; using Xunit.Abstractions; using static Elastic.Apm.Tests.Utilities.FluentAssertionsUtils; +using MockHttpMessageHandler = Elastic.Apm.Tests.Utilities.MockHttpMessageHandler; +using RichardSzalay.MockHttp; +using System = Elastic.Apm.Api.System; namespace Elastic.Apm.Tests.BackendCommTests { @@ -45,6 +49,45 @@ public PayloadSenderTests(ITestOutputHelper xUnitOutputHelper) : base(xUnitOutpu public static IEnumerable TestArgsVariantsWithVeryLongFlushInterval => TestArgsVariants(args => args.FlushInterval.HasValue && args.FlushInterval >= VeryLongFlushInterval).Select(t => new object[] { t }); + + [Fact] + public void Should_Sanitize_HttpRequestMessage_In_Log() + { + var testLogger = new TestLogger(LogLevel.Trace); + var secretToken = "secretToken"; + var serverUrl = "http://username:password@localhost:8200"; + + var config = new MockConfigSnapshot(testLogger, logLevel: "Trace", serverUrl: serverUrl, secretToken: secretToken, flushInterval: "0"); + var service = Service.GetDefaultService(config, testLogger); + var waitHandle = new ManualResetEvent(false); + var handler = new RichardSzalay.MockHttp.MockHttpMessageHandler(); + var configUrl = BackendCommUtils.ApmServerEndpoints + .BuildIntakeV2EventsAbsoluteUrl(config.ServerUrl); + + handler.When(configUrl.AbsoluteUri) + .Respond(_ => + { + waitHandle.Set(); + return new HttpResponseMessage(HttpStatusCode.ServiceUnavailable); + }); + + var payloadSender = new PayloadSenderV2(testLogger, config, service, new Api.System(), MockApmServerInfo.Version710, handler); + using var agent = new ApmAgent(new TestAgentComponents(LoggerBase, config, payloadSender)); + agent.PayloadSender.QueueTransaction(new Transaction(agent, "TestName", "TestType")); + + waitHandle.WaitOne(); + + var count = 0; + while (!testLogger.Log.Contains("Failed sending event.") + && count < 10) + { + Thread.Sleep(500); + count++; + } + + testLogger.Log.Should().NotContain(secretToken) + .And.Contain("http://[REDACTED]:[REDACTED]@localhost:8200").And.NotContain(serverUrl); + } [Fact] public async Task SecretToken_ShouldBeSent_WhenApiKeyIsNotSpecified() { @@ -138,19 +181,21 @@ public async Task UserAgent_test() await isRequestFinished.Task; } - userAgentHeader + var headerValues = userAgentHeader.ToList(); + + headerValues .Should() .NotBeEmpty() .And.HaveCount(3); - userAgentHeader.First().Product.Name.Should().Be($"elasticapm-{Consts.AgentName}"); - userAgentHeader.First().Product.Version.Should().NotBeEmpty(); + headerValues[0].Product.Name.Should().Be($"elasticapm-{Consts.AgentName}"); + headerValues[0].Product.Version.Should().NotBeEmpty(); - userAgentHeader.Skip(1).First().Product.Name.Should().Be("System.Net.Http"); - userAgentHeader.Skip(1).First().Product.Version.Should().NotBeEmpty(); + headerValues[1].Product.Name.Should().Be("System.Net.Http"); + headerValues[1].Product.Version.Should().NotBeEmpty(); - userAgentHeader.Skip(2).First().Product.Name.Should().NotBeEmpty(); - userAgentHeader.Skip(2).First().Product.Version.Should().NotBeEmpty(); + headerValues[2].Product.Name.Should().NotBeEmpty(); + headerValues[2].Product.Version.Should().NotBeEmpty(); } private static IEnumerable TestArgsVariantsWithoutIndex() diff --git a/test/Elastic.Apm.Tests/HttpDiagnosticListenerTests.cs b/test/Elastic.Apm.Tests/HttpDiagnosticListenerTests.cs index 212572d9e..6f85a0564 100644 --- a/test/Elastic.Apm.Tests/HttpDiagnosticListenerTests.cs +++ b/test/Elastic.Apm.Tests/HttpDiagnosticListenerTests.cs @@ -302,8 +302,8 @@ public async Task TestUrlSanitization() firstSpan.Should().NotBeNull(); firstSpan.Context.Http.Url.Should() .Be(uriBuilder.Uri.ToString() - .Replace("TestUser", "[REDACTED]") - .Replace("TestPassword", "[REDACTED]")); + .Replace("TestUser", Consts.Redacted) + .Replace("TestPassword", Consts.Redacted)); firstSpan.Context.Http.StatusCode.Should().Be(200); firstSpan.Context.Http.Method.Should().Be(HttpMethod.Get.Method); firstSpan.Context.Destination.Address.Should().Be(new Uri(localServer.Uri).Host); @@ -615,7 +615,7 @@ public async Task NoUserNameAndPasswordInLogsForHttp() // looking for lines with "localhost:8082" and asserting that those contain [REDACTED]. foreach (var lineWithHttpLog in logger.Lines.Where(n => n.Contains($"{uriBuilder.Host}:{uriBuilder.Port}"))) - lineWithHttpLog.Should().Contain("[REDACTED]"); + lineWithHttpLog.Should().Contain(Consts.Redacted); } }