Sanitize Central config request URI and headers in logs (#1471)

This commit sanitizes the request URI and headers of
request to fetch central configuration, to redact username/password info
and sensitive headers.

- Move sanitization methods to separate Santization helper class.
- Add test to assert central config sensitive request details are redacted.
- Use Consts.Redacted wherever "[REDACTED]" is needed.

Fixes #1376

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; }
 
 		/// <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;
 
@@ -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>
@@ -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;
-		}
 	}
 }

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;
 		}
 
 		/// <summary>

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.</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>
@@ -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(

src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigFetcher.cs

@@ -84,15 +84,14 @@ 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();
 		}
 
 		protected override async Task WorkLoopIteration()
 		{
 			++_dbgIterationsCount;
-			var waitingLogSeverity = LogLevel.Trace;
 			WaitInfoS waitInfo;
 			HttpRequestMessage httpRequest = null;
 			HttpResponseMessage httpResponse = null;
@@ -118,48 +117,42 @@ 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
 			{
 				httpRequest?.Dispose();
 				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,16 +166,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);

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:

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]";
 	}
 }