From 84e2918b063000eb3585f8d9c637fd0e25685cc9 Mon Sep 17 00:00:00 2001 From: dudik <8845578+dudikeleti@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:08:23 +0200 Subject: [PATCH] Protect API keys on agentless HTTP transports Require factory-owned API-key headers, reject unsafe redirects/proxies/endpoints, and disable affected uploaders after terminal transport failures. --- .../ApiKeyHttpTransportException.cs | 23 +++ .../Transports/ApiKeyHttpTransportGuard.cs | 51 ++++++ .../Agent/Transports/ApiWebRequest.cs | 54 +++++- .../Agent/Transports/ApiWebRequestFactory.cs | 7 +- .../Agent/Transports/HttpClientRequest.cs | 41 ++++- .../Transports/HttpClientRequestFactory.cs | 45 ++++- .../Agent/Transports/HttpStreamRequest.cs | 1 + .../Ci/Agent/CIWriterHttpSender.cs | 32 +++- .../Ci/Net/TestOptimizationClient.cs | 22 ++- .../Ci/TestOptimizationTracerManagement.cs | 27 ++- .../Debugger/DebuggerTransportStrategy.cs | 9 +- .../ExceptionReplayTransportFactory.cs | 20 ++- .../Debugger/Sink/BatchUploader.cs | 15 ++ .../Debugger/Upload/SymbolUploadApi.cs | 37 +++- .../DirectLogSubmissionManager.cs | 2 +- .../DirectSubmission/LogsTransportStrategy.cs | 10 +- .../Logging/DirectSubmission/Sink/LogsApi.cs | 35 +++- .../Transports/JsonTelemetryTransport.cs | 17 ++ .../ApiKeyHttpTransportGuardTests.cs | 167 ++++++++++++++++++ .../Transports/ApiWebRequestFactoryTests.cs | 70 ++++++++ .../Ci/CIWriterHttpSenderTests.cs | 51 ++++++ .../Ci/TestOptimizationClientTests.cs | 40 +++++ .../Debugger/BatchUploaderTests.cs | 24 +++ .../ExceptionReplayTransportFactoryTests.cs | 15 ++ .../SymbolsTests/SymbolUploadApiTests.cs | 43 ++++- .../DirectSubmission/Sink/LogsApiTests.cs | 105 +++++++++-- .../Transports/JsonTelemetryTransportTests.cs | 32 ++++ 27 files changed, 922 insertions(+), 73 deletions(-) create mode 100644 tracer/src/Datadog.Trace/Agent/Transports/ApiKeyHttpTransportException.cs create mode 100644 tracer/src/Datadog.Trace/Agent/Transports/ApiKeyHttpTransportGuard.cs create mode 100644 tracer/test/Datadog.Trace.Tests/Agent/Transports/ApiKeyHttpTransportGuardTests.cs create mode 100644 tracer/test/Datadog.Trace.Tests/Ci/CIWriterHttpSenderTests.cs diff --git a/tracer/src/Datadog.Trace/Agent/Transports/ApiKeyHttpTransportException.cs b/tracer/src/Datadog.Trace/Agent/Transports/ApiKeyHttpTransportException.cs new file mode 100644 index 000000000000..782f1b5080bb --- /dev/null +++ b/tracer/src/Datadog.Trace/Agent/Transports/ApiKeyHttpTransportException.cs @@ -0,0 +1,23 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +#nullable enable + +using System; + +namespace Datadog.Trace.Agent.Transports; + +internal sealed class ApiKeyHttpTransportException : InvalidOperationException +{ + public ApiKeyHttpTransportException(string message) + : base(message) + { + } + + public ApiKeyHttpTransportException(string message, Exception innerException) + : base(message, innerException) + { + } +} diff --git a/tracer/src/Datadog.Trace/Agent/Transports/ApiKeyHttpTransportGuard.cs b/tracer/src/Datadog.Trace/Agent/Transports/ApiKeyHttpTransportGuard.cs new file mode 100644 index 000000000000..79368cf9a835 --- /dev/null +++ b/tracer/src/Datadog.Trace/Agent/Transports/ApiKeyHttpTransportGuard.cs @@ -0,0 +1,51 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +#nullable enable + +using System; + +namespace Datadog.Trace.Agent.Transports; + +internal static class ApiKeyHttpTransportGuard +{ + internal const string ApiKeyHeaderName = "DD-API-KEY"; + + public static bool IsPlaintextLoopback(Uri endpoint) + => string.Equals(endpoint.Scheme, Uri.UriSchemeHttp, StringComparison.OrdinalIgnoreCase) && endpoint.IsLoopback; + + public static void RejectLateApiKeyHeader(string headerName) + { + if (string.Equals(headerName, ApiKeyHeaderName, StringComparison.OrdinalIgnoreCase)) + { + throw new ApiKeyHttpTransportException("DD-API-KEY must be configured when constructing the request factory."); + } + } + + public static void EnsureSafeEndpoint(Uri endpoint) + { + if (string.Equals(endpoint.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase) || + IsPlaintextLoopback(endpoint)) + { + return; + } + + throw new ApiKeyHttpTransportException( + "Refusing to send DD-API-KEY unless the endpoint uses HTTPS or loopback HTTP."); + } + + public static void EnsureSafe(Uri endpoint, bool isProxyDisabled, bool redirectsDisabled) + { + EnsureSafeEndpoint(endpoint); + + if (redirectsDisabled && (!IsPlaintextLoopback(endpoint) || isProxyDisabled)) + { + return; + } + + throw new ApiKeyHttpTransportException( + "Refusing to send DD-API-KEY unless automatic redirects are disabled and the endpoint uses HTTPS or direct loopback HTTP."); + } +} diff --git a/tracer/src/Datadog.Trace/Agent/Transports/ApiWebRequest.cs b/tracer/src/Datadog.Trace/Agent/Transports/ApiWebRequest.cs index 2b0b11fdefbc..0e214fbabf64 100644 --- a/tracer/src/Datadog.Trace/Agent/Transports/ApiWebRequest.cs +++ b/tracer/src/Datadog.Trace/Agent/Transports/ApiWebRequest.cs @@ -24,17 +24,24 @@ internal sealed class ApiWebRequest : IApiRequest private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(); private readonly HttpWebRequest _request; + private readonly bool _hasApiKeyHeader; private byte[] _boundarySeparatorInBytes; private byte[] _boundaryTrailerInBytes; - public ApiWebRequest(HttpWebRequest request) + public ApiWebRequest(HttpWebRequest request, bool hasApiKeyHeader) { _request = request; + _hasApiKeyHeader = hasApiKeyHeader; + if (hasApiKeyHeader) + { + ConfigureApiKeyTransport(); + } } public void AddHeader(string name, string value) { + ApiKeyHttpTransportGuard.RejectLateApiKeyHeader(name); _request.Headers.Add(name, value); } @@ -194,6 +201,51 @@ private void ResetRequest(string method, string contentType, string contentEncod { _request.Headers.Set(HttpRequestHeader.ContentEncoding, contentEncoding); } + + ValidateApiKeyTransport(); + } + + private void ConfigureApiKeyTransport() + { + try + { + _request.AllowAutoRedirect = false; + var endpoint = _request.RequestUri; + if (ApiKeyHttpTransportGuard.IsPlaintextLoopback(endpoint)) + { + // Proxy bypass checks are mutable and racy. Enforce a direct connection instead. + _request.Proxy = null; + } + } + catch (Exception ex) + { + throw new ApiKeyHttpTransportException("Unable to configure a safe HTTP transport for DD-API-KEY.", ex); + } + } + + private void ValidateApiKeyTransport() + { + if (!_hasApiKeyHeader) + { + return; + } + + try + { + var endpoint = _request.RequestUri; + ApiKeyHttpTransportGuard.EnsureSafe( + endpoint, + isProxyDisabled: !ApiKeyHttpTransportGuard.IsPlaintextLoopback(endpoint) || _request.Proxy is null, + redirectsDisabled: !_request.AllowAutoRedirect); + } + catch (ApiKeyHttpTransportException) + { + throw; + } + catch (Exception ex) + { + throw new ApiKeyHttpTransportException("Unable to verify a safe HTTP transport for DD-API-KEY.", ex); + } } private async Task FinishAndGetResponse() diff --git a/tracer/src/Datadog.Trace/Agent/Transports/ApiWebRequestFactory.cs b/tracer/src/Datadog.Trace/Agent/Transports/ApiWebRequestFactory.cs index 46160bba2f3e..7785f1574b9f 100644 --- a/tracer/src/Datadog.Trace/Agent/Transports/ApiWebRequestFactory.cs +++ b/tracer/src/Datadog.Trace/Agent/Transports/ApiWebRequestFactory.cs @@ -53,12 +53,17 @@ public IApiRequest Create(Uri endpoint) request.Timeout = (int)_timeout.Value.TotalMilliseconds; } + var hasApiKeyHeader = false; foreach (var pair in _defaultHeaders) { request.Headers.Add(pair.Key, pair.Value); + if (string.Equals(pair.Key, ApiKeyHttpTransportGuard.ApiKeyHeaderName, StringComparison.OrdinalIgnoreCase)) + { + hasApiKeyHeader = true; + } } - return new ApiWebRequest(request); + return new ApiWebRequest(request, hasApiKeyHeader); } public void SetProxy(WebProxy proxy, NetworkCredential credential) diff --git a/tracer/src/Datadog.Trace/Agent/Transports/HttpClientRequest.cs b/tracer/src/Datadog.Trace/Agent/Transports/HttpClientRequest.cs index 8bae11a038cc..2642888769a0 100644 --- a/tracer/src/Datadog.Trace/Agent/Transports/HttpClientRequest.cs +++ b/tracer/src/Datadog.Trace/Agent/Transports/HttpClientRequest.cs @@ -24,13 +24,15 @@ internal sealed class HttpClientRequest : IApiRequest private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(); private readonly HttpClient _client; + private readonly HttpClientHandler _apiKeyProtectedHandler; private readonly HttpRequestMessage _postRequest; private readonly HttpRequestMessage _getRequest; private readonly Uri _uri; - public HttpClientRequest(HttpClient client, Uri endpoint) + public HttpClientRequest(HttpClient client, HttpClientHandler apiKeyProtectedHandler, Uri endpoint) { _client = client; + _apiKeyProtectedHandler = apiKeyProtectedHandler; _postRequest = new HttpRequestMessage(HttpMethod.Post, endpoint); _getRequest = new HttpRequestMessage(HttpMethod.Get, endpoint); _uri = endpoint; @@ -38,6 +40,7 @@ public HttpClientRequest(HttpClient client, Uri endpoint) public void AddHeader(string name, string value) { + ApiKeyHttpTransportGuard.RejectLateApiKeyHeader(name); _postRequest.Headers.Add(name, value); _getRequest.Headers.Add(name, value); } @@ -46,7 +49,7 @@ public async Task GetAsync() { _getRequest.Content = null; - return new HttpClientResponse(await _client.SendAsync(_getRequest).ConfigureAwait(false)); + return new HttpClientResponse(await SendAsync(_getRequest).ConfigureAwait(false)); } public Task PostAsync(ArraySegment bytes, string contentType) @@ -65,7 +68,7 @@ public async Task PostAsync(ArraySegment bytes, string conte _postRequest.Content = content; - var response = await _client.SendAsync(_postRequest).ConfigureAwait(false); + var response = await SendAsync(_postRequest).ConfigureAwait(false); return new HttpClientResponse(response); } @@ -91,7 +94,7 @@ public async Task PostAsJsonAsync(T payload, MultipartCompressi _postRequest.Content = content; - var response = await _client.SendAsync(_postRequest).ConfigureAwait(false); + var response = await SendAsync(_postRequest).ConfigureAwait(false); return new HttpClientResponse(response); } @@ -114,7 +117,7 @@ public async Task PostAsync(Func writeToRequestStrea } _postRequest.Content = content; - var response = await _client.SendAsync(_postRequest).ConfigureAwait(false); + var response = await SendAsync(_postRequest).ConfigureAwait(false); return new HttpClientResponse(response); } @@ -175,9 +178,35 @@ public async Task PostAsync(MultipartFormItem[] items, MultipartCo _postRequest.Content = formDataContent; } - var response = await _client.SendAsync(_postRequest).ConfigureAwait(false); + var response = await SendAsync(_postRequest).ConfigureAwait(false); return new HttpClientResponse(response); } + + private Task SendAsync(HttpRequestMessage request) + { + if (_apiKeyProtectedHandler is not null) + { + ApiKeyHttpTransportGuard.EnsureSafe( + _uri, + isProxyDisabled: IsProxyDisabledForEndpoint(), + redirectsDisabled: AreRedirectsDisabled()); + } + + return _client.SendAsync(request); + } + + private bool AreRedirectsDisabled() + => !_apiKeyProtectedHandler.AllowAutoRedirect; + + private bool IsProxyDisabledForEndpoint() + { + if (!ApiKeyHttpTransportGuard.IsPlaintextLoopback(_uri)) + { + return true; + } + + return !_apiKeyProtectedHandler.UseProxy; + } } } #endif diff --git a/tracer/src/Datadog.Trace/Agent/Transports/HttpClientRequestFactory.cs b/tracer/src/Datadog.Trace/Agent/Transports/HttpClientRequestFactory.cs index a6b5b8360e89..7844a51f8ac9 100644 --- a/tracer/src/Datadog.Trace/Agent/Transports/HttpClientRequestFactory.cs +++ b/tracer/src/Datadog.Trace/Agent/Transports/HttpClientRequestFactory.cs @@ -20,13 +20,45 @@ internal sealed class HttpClientRequestFactory : IApiRequestFactory { private readonly HttpClient _client; private readonly HttpMessageHandler _handler; + private readonly HttpClientHandler _apiKeyProtectedHandler; + private readonly bool _disableProxyForPlaintextLoopback; + private readonly bool _hasApiKeyHeader; private readonly Uri _baseEndpoint; - public HttpClientRequestFactory(Uri baseEndpoint, KeyValuePair[] defaultHeaders, HttpMessageHandler handler = null, TimeSpan? timeout = null) + public HttpClientRequestFactory( + Uri baseEndpoint, + KeyValuePair[] defaultHeaders, + HttpMessageHandler handler = null, + TimeSpan? timeout = null, + DecompressionMethods automaticDecompression = DecompressionMethods.None) { - _handler = handler ?? new HttpClientHandler(); - _client = new HttpClient(_handler); _baseEndpoint = baseEndpoint; + foreach (var pair in defaultHeaders) + { + if (string.Equals(pair.Key, ApiKeyHttpTransportGuard.ApiKeyHeaderName, StringComparison.OrdinalIgnoreCase)) + { + _hasApiKeyHeader = true; + } + } + + if (_hasApiKeyHeader && handler is not null) + { + throw new ApiKeyHttpTransportException("Caller-provided HTTP handlers are not supported for protected DD-API-KEY transport."); + } + + _handler = handler ?? new HttpClientHandler { AutomaticDecompression = automaticDecompression }; + _disableProxyForPlaintextLoopback = _hasApiKeyHeader && ApiKeyHttpTransportGuard.IsPlaintextLoopback(baseEndpoint); + if (_hasApiKeyHeader) + { + _apiKeyProtectedHandler = (HttpClientHandler)_handler; + _apiKeyProtectedHandler.AllowAutoRedirect = false; + if (_disableProxyForPlaintextLoopback) + { + _apiKeyProtectedHandler.UseProxy = false; + } + } + + _client = new HttpClient(_handler); if (timeout.HasValue) { _client.Timeout = timeout.Value; @@ -54,11 +86,16 @@ public string Info(Uri endpoint) public IApiRequest Create(Uri endpoint) { - return new HttpClientRequest(_client, endpoint); + return new HttpClientRequest(_client, _apiKeyProtectedHandler, endpoint); } public void SetProxy(WebProxy proxy, NetworkCredential credential) { + if (_disableProxyForPlaintextLoopback) + { + return; + } + if (_handler is HttpClientHandler handler) { handler.Proxy = proxy; diff --git a/tracer/src/Datadog.Trace/Agent/Transports/HttpStreamRequest.cs b/tracer/src/Datadog.Trace/Agent/Transports/HttpStreamRequest.cs index 5eb6ce181ee9..6503ac058b3c 100644 --- a/tracer/src/Datadog.Trace/Agent/Transports/HttpStreamRequest.cs +++ b/tracer/src/Datadog.Trace/Agent/Transports/HttpStreamRequest.cs @@ -35,6 +35,7 @@ public HttpStreamRequest(DatadogHttpClient client, Uri uri, IStreamFactory strea public void AddHeader(string name, string value) { + ApiKeyHttpTransportGuard.RejectLateApiKeyHeader(name); _headers.Add(name, value); } diff --git a/tracer/src/Datadog.Trace/Ci/Agent/CIWriterHttpSender.cs b/tracer/src/Datadog.Trace/Ci/Agent/CIWriterHttpSender.cs index 724e0ad72398..5a0ef3f83e5c 100644 --- a/tracer/src/Datadog.Trace/Ci/Agent/CIWriterHttpSender.cs +++ b/tracer/src/Datadog.Trace/Ci/Agent/CIWriterHttpSender.cs @@ -8,6 +8,7 @@ using System.Diagnostics; using System.IO; using System.IO.Compression; +using System.Threading; using System.Threading.Tasks; using Datadog.Trace.Agent; using Datadog.Trace.Agent.Transports; @@ -24,12 +25,12 @@ namespace Datadog.Trace.Ci.Agent; internal sealed class CIWriterHttpSender : ICIVisibilityProtocolWriterSender { - private const string ApiKeyHeader = "dd-api-key"; private const string EvpSubdomainHeader = "X-Datadog-EVP-Subdomain"; private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(); private readonly IApiRequestFactory _apiRequestFactory; private readonly bool _isDebugEnabled; + private int _apiKeyTransportRejected; public CIWriterHttpSender(IApiRequestFactory apiRequestFactory) { @@ -91,6 +92,11 @@ private static async Task SendPayloadAsync(Func(EventPlatformPayload payload, Func> senderFunc, T state) { + if (Volatile.Read(ref _apiKeyTransportRejected) != 0) + { + return; + } + // retry up to 5 times with exponential back-off const int retryLimit = 5; var retryCount = 1; @@ -111,10 +117,12 @@ private async Task SendPayloadAsync(EventPlatformPayload payload, Func(EventPlatformPayload payload, Func(EventPlatformPayload payload, Func payloadArraySegment; diff --git a/tracer/src/Datadog.Trace/Ci/Net/TestOptimizationClient.cs b/tracer/src/Datadog.Trace/Ci/Net/TestOptimizationClient.cs index 6f0b8724ecd0..215a3b91d752 100644 --- a/tracer/src/Datadog.Trace/Ci/Net/TestOptimizationClient.cs +++ b/tracer/src/Datadog.Trace/Ci/Net/TestOptimizationClient.cs @@ -15,6 +15,7 @@ using System.Runtime.ExceptionServices; using System.Text; using System.Text.RegularExpressions; +using System.Threading; using System.Threading.Tasks; using Datadog.Trace.Agent; using Datadog.Trace.Agent.Transports; @@ -35,7 +36,6 @@ namespace Datadog.Trace.Ci.Net; internal sealed partial class TestOptimizationClient : ITestOptimizationClient { - private const string ApiKeyHeader = "DD-API-KEY"; private const string EvpSubdomainHeader = "X-Datadog-EVP-Subdomain"; private const int MaxRetries = 5; @@ -57,6 +57,7 @@ internal sealed partial class TestOptimizationClient : ITestOptimizationClient private readonly string _repositoryUrl; private readonly string _branchName; private readonly string _commitSha; + private int _apiKeyTransportRejected; static TestOptimizationClient() { @@ -414,10 +415,6 @@ private void SetRequestHeader(IApiRequest request) { request.AddHeader(EvpSubdomainHeader, "api"); } - else - { - request.AddHeader(ApiKeyHeader, _testOptimization.Settings.ApiKey); - } } private void CheckResponseStatusCode(IApiResponse response, byte[]? responseContent, bool finalTry) @@ -450,6 +447,11 @@ private void CheckResponseStatusCode(IApiResponse response, byte[]? responseCont private async Task WithRetries(Func> sendDelegate, TState state, int numOfRetries) { + if (Volatile.Read(ref _apiKeyTransportRejected) != 0) + { + throw new ApiKeyHttpTransportException("Test Optimization agentless requests are disabled because the API-key transport is unsafe."); + } + var retryCount = 1; var sleepDuration = 100; // in milliseconds @@ -473,6 +475,16 @@ private async Task WithRetries(Func> sendDel { var sourceException = exceptionDispatchInfo.SourceException; + if (sourceException is ApiKeyHttpTransportException) + { + if (Interlocked.Exchange(ref _apiKeyTransportRejected, 1) == 0) + { + Log.Error(sourceException, "Disabling Test Optimization agentless requests because the API-key transport is unsafe."); + } + + exceptionDispatchInfo.Throw(); + } + if (isFinalTry || sourceException is RateLimitException { DelayTimeInSeconds: null } || sourceException is DirectoryNotFoundException || diff --git a/tracer/src/Datadog.Trace/Ci/TestOptimizationTracerManagement.cs b/tracer/src/Datadog.Trace/Ci/TestOptimizationTracerManagement.cs index fdd0024a65da..81e973244369 100644 --- a/tracer/src/Datadog.Trace/Ci/TestOptimizationTracerManagement.cs +++ b/tracer/src/Datadog.Trace/Ci/TestOptimizationTracerManagement.cs @@ -5,6 +5,7 @@ #nullable enable using System; +using System.Collections.Generic; using System.Net; using System.Text.RegularExpressions; using System.Threading; @@ -160,7 +161,7 @@ public IApiRequestFactory GetRequestFactory(TracerSettings tracerSettings, TimeS { IApiRequestFactory? factory; var exporterSettings = tracerSettings.Manager.InitialExporterSettings; - if (exporterSettings.TracesTransport != TracesTransportType.Default) + if (!_settings.Agentless && exporterSettings.TracesTransport != TracesTransportType.Default) { factory = AgentTransportStrategy.Get( exporterSettings, @@ -170,16 +171,30 @@ public IApiRequestFactory GetRequestFactory(TracerSettings tracerSettings, TimeS } else { + var baseEndpoint = exporterSettings.AgentUri; + var defaultHeaders = AgentHttpHeaderNames.DefaultHeaders; + if (_settings.Agentless) + { + baseEndpoint = string.IsNullOrWhiteSpace(_settings.AgentlessUrl) + ? new Uri($"https://api.{_settings.Site}") + : new Uri(_settings.AgentlessUrl); + defaultHeaders = + [ + ..AgentHttpHeaderNames.DefaultHeaders, + new KeyValuePair(ApiKeyHttpTransportGuard.ApiKeyHeaderName, _settings.ApiKey!) + ]; + } + #if NETCOREAPP Log.Information("TestOptimizationTracerManagement: Using {FactoryType} for trace transport.", nameof(HttpClientRequestFactory)); factory = new HttpClientRequestFactory( - exporterSettings.AgentUri, - AgentHttpHeaderNames.DefaultHeaders, - handler: new System.Net.Http.HttpClientHandler { AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate, }, - timeout: timeout); + baseEndpoint, + defaultHeaders, + timeout: timeout, + automaticDecompression: DecompressionMethods.GZip | DecompressionMethods.Deflate); #else Log.Information("TestOptimizationTracerManagement: Using {FactoryType} for trace transport.", nameof(ApiWebRequestFactory)); - factory = new ApiWebRequestFactory(exporterSettings.AgentUri, AgentHttpHeaderNames.DefaultHeaders, timeout: timeout); + factory = new ApiWebRequestFactory(baseEndpoint, defaultHeaders, timeout: timeout); #endif if (!string.IsNullOrWhiteSpace(_settings.ProxyHttps)) { diff --git a/tracer/src/Datadog.Trace/Debugger/DebuggerTransportStrategy.cs b/tracer/src/Datadog.Trace/Debugger/DebuggerTransportStrategy.cs index 8c078428b3c5..aeee0a3ddcb6 100644 --- a/tracer/src/Datadog.Trace/Debugger/DebuggerTransportStrategy.cs +++ b/tracer/src/Datadog.Trace/Debugger/DebuggerTransportStrategy.cs @@ -16,8 +16,15 @@ internal static class DebuggerTransportStrategy { private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(DebuggerTransportStrategy)); - public static IApiRequestFactory Get(Uri baseEndpoint, KeyValuePair[] defaultHeaders) + public static IApiRequestFactory Get(Uri baseEndpoint, string apiKey) { + KeyValuePair[] defaultHeaders = + [ + ..AgentHttpHeaderNames.DefaultHeaders, + new(ApiKeyHttpTransportGuard.ApiKeyHeaderName, apiKey), + new("DD-EVP-ORIGIN", "dd-trace-dotnet") + ]; + #if NETCOREAPP Log.Information("Using {FactoryType} for debugger transport.", nameof(HttpClientRequestFactory)); return new HttpClientRequestFactory(baseEndpoint, defaultHeaders); diff --git a/tracer/src/Datadog.Trace/Debugger/ExceptionAutoInstrumentation/ExceptionReplayTransportFactory.cs b/tracer/src/Datadog.Trace/Debugger/ExceptionAutoInstrumentation/ExceptionReplayTransportFactory.cs index d6b1a8fed34f..0273c4863617 100644 --- a/tracer/src/Datadog.Trace/Debugger/ExceptionAutoInstrumentation/ExceptionReplayTransportFactory.cs +++ b/tracer/src/Datadog.Trace/Debugger/ExceptionAutoInstrumentation/ExceptionReplayTransportFactory.cs @@ -6,9 +6,9 @@ #nullable enable using System; -using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using Datadog.Trace.Agent.DiscoveryService; +using Datadog.Trace.Agent.Transports; using Datadog.Trace.Configuration; using Datadog.Trace.HttpOverStreams; using Datadog.Trace.Logging; @@ -39,13 +39,17 @@ internal static class ExceptionReplayTransportFactory return null; } - var apiFactory = DebuggerTransportStrategy.Get( - baseUri, - [ - ..AgentHttpHeaderNames.DefaultHeaders, - new KeyValuePair("DD-API-KEY", settings.AgentlessApiKey), - new KeyValuePair("DD-EVP-ORIGIN", "dd-trace-dotnet") - ]); + try + { + ApiKeyHttpTransportGuard.EnsureSafeEndpoint(baseUri); + } + catch (ApiKeyHttpTransportException ex) + { + Log.ErrorSkipTelemetry(ex, "Exception Replay agentless uploads enabled but the intake URL does not support safe API-key transport. Disabling Exception Replay."); + return null; + } + + var apiFactory = DebuggerTransportStrategy.Get(baseUri, settings.AgentlessApiKey); return new ExceptionReplayTransportInfo(apiFactory, null, relativePath, isAgentless: true); } diff --git a/tracer/src/Datadog.Trace/Debugger/Sink/BatchUploader.cs b/tracer/src/Datadog.Trace/Debugger/Sink/BatchUploader.cs index e507f6010f28..dcf37f1225da 100644 --- a/tracer/src/Datadog.Trace/Debugger/Sink/BatchUploader.cs +++ b/tracer/src/Datadog.Trace/Debugger/Sink/BatchUploader.cs @@ -7,7 +7,9 @@ using System; using System.Collections.Generic; using System.Text; +using System.Threading; using System.Threading.Tasks; +using Datadog.Trace.Agent.Transports; using Datadog.Trace.Debugger.Upload; using Datadog.Trace.Logging; @@ -29,6 +31,7 @@ internal sealed class BatchUploader : IBatchUploader private readonly StringBuilder _sb; private byte[] _serializedPayloads = new byte[InitialPayloadSizeBytes]; + private int _apiKeyTransportRejected; private BatchUploader(IBatchUploadApi api) { @@ -43,6 +46,11 @@ public static BatchUploader Create(IBatchUploadApi api) public async Task Upload(IEnumerable payloads) { + if (Volatile.Read(ref _apiKeyTransportRejected) != 0) + { + return; + } + try { foreach (var batch in GetBatches(payloads)) @@ -50,6 +58,13 @@ public async Task Upload(IEnumerable payloads) await _api.SendBatchAsync(batch).ConfigureAwait(false); } } + catch (ApiKeyHttpTransportException e) + { + if (Interlocked.Exchange(ref _apiKeyTransportRejected, 1) == 0) + { + Log.Error(e, "Disabling debugger batch uploads because the API-key transport is unsafe."); + } + } catch (Exception e) { Log.Error(e, "Failed to upload batch"); diff --git a/tracer/src/Datadog.Trace/Debugger/Upload/SymbolUploadApi.cs b/tracer/src/Datadog.Trace/Debugger/Upload/SymbolUploadApi.cs index a2eb8bc49e85..727fcc520b2e 100644 --- a/tracer/src/Datadog.Trace/Debugger/Upload/SymbolUploadApi.cs +++ b/tracer/src/Datadog.Trace/Debugger/Upload/SymbolUploadApi.cs @@ -35,6 +35,7 @@ internal sealed class SymbolUploadApi : DebuggerUploadApiBase, ISymbolUploadApi private readonly string _runtimeId; private readonly bool _enableCompression; private readonly Func _delayAsync; + private int _apiKeyTransportRejected; private int _uploadFailureCount; private SymbolUploadApi( @@ -131,6 +132,11 @@ public async Task SendBatchAsync(Func writeS ThrowHelper.ThrowArgumentNullException(nameof(writeSymbols)); } + if (Volatile.Read(ref _apiKeyTransportRejected) != 0) + { + return false; + } + var uri = BuildUri(); if (string.IsNullOrEmpty(uri)) { @@ -143,14 +149,29 @@ public async Task SendBatchAsync(Func writeS while (retries < MaxRetries) { - var request = _apiRequestFactory.Create(endpoint); - using var response = await request - .PostAsync( - stream => WriteMultipartFormData(stream, writeSymbols, state, metadata), - MimeTypes.MultipartFormData, - contentEncoding: null, - DatadogHttpValues.Boundary) - .ConfigureAwait(false); + IApiResponse response; + try + { + var request = _apiRequestFactory.Create(endpoint); + response = await request + .PostAsync( + stream => WriteMultipartFormData(stream, writeSymbols, state, metadata), + MimeTypes.MultipartFormData, + contentEncoding: null, + DatadogHttpValues.Boundary) + .ConfigureAwait(false); + } + catch (ApiKeyHttpTransportException e) + { + if (Interlocked.Exchange(ref _apiKeyTransportRejected, 1) == 0) + { + Log.Error(e, "Disabling symbol database uploads because the API-key transport is unsafe."); + } + + return false; + } + + using var responseToDispose = response; if (response.StatusCode is >= 200 and <= 299) { diff --git a/tracer/src/Datadog.Trace/Logging/DirectSubmission/DirectLogSubmissionManager.cs b/tracer/src/Datadog.Trace/Logging/DirectSubmission/DirectLogSubmissionManager.cs index c65a1ce2662f..088ffc934210 100644 --- a/tracer/src/Datadog.Trace/Logging/DirectSubmission/DirectLogSubmissionManager.cs +++ b/tracer/src/Datadog.Trace/Logging/DirectSubmission/DirectLogSubmissionManager.cs @@ -50,7 +50,7 @@ public static DirectLogSubmissionManager Create( } var apiFactory = LogsTransportStrategy.Get(directLogSettings); - var logsApi = new LogsApi(directLogSettings.ApiKey, apiFactory); + var logsApi = new LogsApi(apiFactory); return new DirectLogSubmissionManager(directLogSettings, new DirectSubmissionLogSink(logsApi, formatter, directLogSettings.CreateBatchingSinkOptions()), formatter); } diff --git a/tracer/src/Datadog.Trace/Logging/DirectSubmission/LogsTransportStrategy.cs b/tracer/src/Datadog.Trace/Logging/DirectSubmission/LogsTransportStrategy.cs index dece2330a165..95e2cf287b88 100644 --- a/tracer/src/Datadog.Trace/Logging/DirectSubmission/LogsTransportStrategy.cs +++ b/tracer/src/Datadog.Trace/Logging/DirectSubmission/LogsTransportStrategy.cs @@ -5,6 +5,7 @@ #nullable enable using System; +using System.Collections.Generic; using Datadog.Trace.Agent; using Datadog.Trace.Agent.Transports; using Datadog.Trace.Logging.DirectSubmission.Sink; @@ -19,13 +20,18 @@ public static IApiRequestFactory Get(DirectLogSubmissionSettings settings) { // Still quite a long time, but we could be sending a lot of data var timeout = TimeSpan.FromSeconds(15); + KeyValuePair[] defaultHeaders = + [ + ..LogsApiHeaderNames.DefaultHeaders, + new(ApiKeyHttpTransportGuard.ApiKeyHeaderName, settings.ApiKey) + ]; #if NETCOREAPP Log.Information("Using {FactoryType} for log submission transport.", nameof(HttpClientRequestFactory)); - return new HttpClientRequestFactory(settings.IntakeUrl, LogsApiHeaderNames.DefaultHeaders, timeout: timeout); + return new HttpClientRequestFactory(settings.IntakeUrl, defaultHeaders, timeout: timeout); #else Log.Information("Using {FactoryType} for log submission transport.", nameof(ApiWebRequestFactory)); - return new ApiWebRequestFactory(settings.IntakeUrl, LogsApiHeaderNames.DefaultHeaders, timeout: timeout); + return new ApiWebRequestFactory(settings.IntakeUrl, defaultHeaders, timeout: timeout); #endif } } diff --git a/tracer/src/Datadog.Trace/Logging/DirectSubmission/Sink/LogsApi.cs b/tracer/src/Datadog.Trace/Logging/DirectSubmission/Sink/LogsApi.cs index 414c94b7a009..420cb347f85a 100644 --- a/tracer/src/Datadog.Trace/Logging/DirectSubmission/Sink/LogsApi.cs +++ b/tracer/src/Datadog.Trace/Logging/DirectSubmission/Sink/LogsApi.cs @@ -6,8 +6,10 @@ using System; using System.Net.Sockets; +using System.Threading; using System.Threading.Tasks; using Datadog.Trace.Agent; +using Datadog.Trace.Agent.Transports; using Datadog.Trace.Telemetry; using Datadog.Trace.Telemetry.Metrics; using Datadog.Trace.Util.Http; @@ -17,7 +19,6 @@ namespace Datadog.Trace.Logging.DirectSubmission.Sink internal sealed class LogsApi : ILogsApi { internal const string LogIntakePath = "/api/v2/logs"; - internal const string IntakeHeaderNameApiKey = "DD-API-KEY"; private const string MimeType = "application/json"; @@ -26,13 +27,12 @@ internal sealed class LogsApi : ILogsApi private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(); - private readonly string _apiKey; private readonly IApiRequestFactory _apiRequestFactory; private readonly Uri _logsIntakeEndpoint; + private int _apiKeyTransportRejected; - public LogsApi(string apiKey, IApiRequestFactory apiRequestFactory) + public LogsApi(IApiRequestFactory apiRequestFactory) { - _apiKey = apiKey; _apiRequestFactory = apiRequestFactory; _logsIntakeEndpoint = _apiRequestFactory.GetEndpoint(LogIntakePath); Log.Debug("Using logs intake endpoint {LogsIntakeEndpoint}", _logsIntakeEndpoint.ToString()); @@ -44,6 +44,11 @@ public void Dispose() public async Task SendLogsAsync(ArraySegment logs, int numberOfLogs) { + if (Volatile.Read(ref _apiKeyTransportRejected) != 0) + { + return false; + } + var retriesRemaining = MaxNumberRetries - 1; var nextSleepDuration = InitialSleepDurationMs; @@ -57,15 +62,17 @@ public async Task SendLogsAsync(ArraySegment logs, int numberOfLogs) { request = _apiRequestFactory.Create(_logsIntakeEndpoint); } + catch (ApiKeyHttpTransportException ex) + { + DisableForUnsafeApiKeyTransport(ex); + return false; + } catch (Exception ex) { Log.Error(ex, "An error occurred while generating request to send logs to the intake at {IntakeEndpoint}", _apiRequestFactory.Info(_logsIntakeEndpoint)); return false; } - // Set additional headers - request.AddHeader(IntakeHeaderNameApiKey, _apiKey); - Exception? exception = null; var isFinalTry = retriesRemaining <= 0; var shouldRetry = true; @@ -109,6 +116,11 @@ public async Task SendLogsAsync(ArraySegment logs, int numberOfLogs) response?.Dispose(); } } + catch (ApiKeyHttpTransportException ex) + { + DisableForUnsafeApiKeyTransport(ex); + return false; + } catch (Exception ex) { var tag = ex is TimeoutException ? MetricTags.ApiError.Timeout : MetricTags.ApiError.NetworkError; @@ -144,5 +156,14 @@ public async Task SendLogsAsync(ArraySegment logs, int numberOfLogs) nextSleepDuration *= 2; } } + + private void DisableForUnsafeApiKeyTransport(ApiKeyHttpTransportException exception) + { + if (Interlocked.Exchange(ref _apiKeyTransportRejected, 1) == 0) + { + TelemetryFactory.Metrics.RecordCountDirectLogApiErrors(MetricTags.ApiError.NetworkError); + Log.Error(exception, "Disabling direct log submission because the API-key transport is unsafe."); + } + } } } diff --git a/tracer/src/Datadog.Trace/Telemetry/Transports/JsonTelemetryTransport.cs b/tracer/src/Datadog.Trace/Telemetry/Transports/JsonTelemetryTransport.cs index 75b48f90ec2b..cd3d9f536c88 100644 --- a/tracer/src/Datadog.Trace/Telemetry/Transports/JsonTelemetryTransport.cs +++ b/tracer/src/Datadog.Trace/Telemetry/Transports/JsonTelemetryTransport.cs @@ -7,6 +7,7 @@ using System; using System.Net; +using System.Threading; using System.Threading.Tasks; using Datadog.Trace.Agent; using Datadog.Trace.Agent.Transports; @@ -29,6 +30,7 @@ internal abstract class JsonTelemetryTransport : ITelemetryTransport private readonly ContainerMetadata _containerMetadata; private readonly bool _enableDebug; private readonly bool _telemetryGzipCompressionEnabled; + private int _apiKeyTransportRejected; protected JsonTelemetryTransport(IApiRequestFactory requestFactory, bool enableDebug, string telemetryCompressionMethod, ContainerMetadata containerMetadata) { @@ -43,6 +45,11 @@ protected JsonTelemetryTransport(IApiRequestFactory requestFactory, bool enableD public async Task PushTelemetry(TelemetryData data) { + if (Volatile.Read(ref _apiKeyTransportRejected) != 0) + { + return TelemetryPushResult.FatalError; + } + var endpointMetricTag = GetEndpointMetricTag(); try @@ -79,6 +86,16 @@ public async Task PushTelemetry(TelemetryData data) Log.Debug("Error sending telemetry to '{Endpoint}' {StatusCode} . CompressionEnabled {Compression}", GetEndpointInfo(), response.StatusCode, _telemetryGzipCompressionEnabled); return TelemetryPushResult.TransientFailure; } + catch (ApiKeyHttpTransportException ex) + { + if (Interlocked.Exchange(ref _apiKeyTransportRejected, 1) == 0) + { + Log.Error(ex, "Disabling direct telemetry because the API-key transport is unsafe."); + } + + TelemetryFactory.Metrics.RecordCountTelemetryApiErrors(endpointMetricTag, MetricTags.ApiError.NetworkError); + return TelemetryPushResult.FatalError; + } catch (Exception ex) when (IsFatalException(ex)) { Log.Information(ex, "Error sending telemetry data, unable to communicate with '{Endpoint}'. CompressionEnabled {Compression}", GetEndpointInfo(), _telemetryGzipCompressionEnabled); diff --git a/tracer/test/Datadog.Trace.Tests/Agent/Transports/ApiKeyHttpTransportGuardTests.cs b/tracer/test/Datadog.Trace.Tests/Agent/Transports/ApiKeyHttpTransportGuardTests.cs new file mode 100644 index 000000000000..308ef8be5e64 --- /dev/null +++ b/tracer/test/Datadog.Trace.Tests/Agent/Transports/ApiKeyHttpTransportGuardTests.cs @@ -0,0 +1,167 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +#nullable enable + +using System; +#if NETCOREAPP3_1_OR_GREATER +using System.Collections.Generic; +using System.Collections.Specialized; +using System.Net; +using System.Net.Http; +using System.Reflection; +using System.Threading.Tasks; +#endif +using Datadog.Trace.Agent.Transports; +#if NETCOREAPP3_1_OR_GREATER +using Datadog.Trace.Ci; +using Datadog.Trace.Ci.Configuration; +using Datadog.Trace.Configuration; +using Datadog.Trace.Configuration.Telemetry; +#endif +using FluentAssertions; +using Xunit; + +namespace Datadog.Trace.Tests.Agent.Transports; + +public class ApiKeyHttpTransportGuardTests +{ + [Theory] + [InlineData("https://example.com")] + [InlineData("http://localhost")] + [InlineData("http://127.0.0.1")] + [InlineData("http://[::1]")] + public void AllowsSecureOrLoopbackEndpointWithApiKey(string endpoint) + { + var action = () => ApiKeyHttpTransportGuard.EnsureSafeEndpoint(new Uri(endpoint)); + + action.Should().NotThrow(); + } + + [Theory] + [InlineData("http://example.com")] + [InlineData("ftp://example.com")] + public void RejectsUnsafeEndpointWithApiKey(string endpoint) + { + var action = () => ApiKeyHttpTransportGuard.EnsureSafeEndpoint(new Uri(endpoint)); + + action.Should() + .Throw() + .WithMessage("*DD-API-KEY*"); + } + + [Theory] + [InlineData("https://example.com", true, false)] + [InlineData("http://localhost", false, true)] + public void RejectsUnsafeTransport(string endpoint, bool isProxyDisabled, bool redirectsDisabled) + { + var action = () => ApiKeyHttpTransportGuard.EnsureSafe( + new Uri(endpoint), + isProxyDisabled, + redirectsDisabled); + + action.Should().Throw(); + } + +#if NETCOREAPP3_1_OR_GREATER + [Fact] + public async Task HttpClientRequestRejectsUnsafeDefaultApiKeyHeader() + { + var factory = new HttpClientRequestFactory( + new Uri("http://example.com"), + [new KeyValuePair(ApiKeyHttpTransportGuard.ApiKeyHeaderName, "test-key")]); + var request = factory.Create(factory.GetEndpoint("/intake")); + + Func action = async () => { await request.GetAsync(); }; + + await action.Should().ThrowAsync(); + } + + [Fact] + public void HttpClientRequestFactoryConfiguresOwnedProtectedHandler() + { + var factory = new HttpClientRequestFactory( + new Uri("https://example.com"), + [new KeyValuePair(ApiKeyHttpTransportGuard.ApiKeyHeaderName, "test-key")], + automaticDecompression: DecompressionMethods.GZip | DecompressionMethods.Deflate); + + var handler = GetHandler(factory); + handler.AllowAutoRedirect.Should().BeFalse(); + handler.AutomaticDecompression.Should().Be(DecompressionMethods.GZip | DecompressionMethods.Deflate); + } + + [Fact] + public void HttpClientRequestFactoryDisablesProxyForPlaintextLoopback() + { + var factory = new HttpClientRequestFactory( + new Uri("http://localhost"), + [new KeyValuePair(ApiKeyHttpTransportGuard.ApiKeyHeaderName, "test-key")]); + factory.SetProxy(new WebProxy("http://example.com"), credential: null); + + var handler = GetHandler(factory); + + handler.UseProxy.Should().BeFalse(); + } + + [Fact] + public void HttpClientRequestFactoryRejectsProtectedApiKeyWithCallerOwnedHandler() + { + var handler = new HttpClientHandler { AllowAutoRedirect = true }; + + var action = () => new HttpClientRequestFactory( + new Uri("https://example.com"), + [new KeyValuePair(ApiKeyHttpTransportGuard.ApiKeyHeaderName, "test-key")], + handler); + + action.Should().Throw(); + } + + [Fact] + public void HttpClientRequestRejectsApiKeyAddedToKeylessFactoryRequest() + { + var factory = new HttpClientRequestFactory( + new Uri("https://example.com"), + []); + var request = factory.Create(factory.GetEndpoint("/intake")); + + var action = () => request.AddHeader(ApiKeyHttpTransportGuard.ApiKeyHeaderName.ToLowerInvariant(), "test-key"); + + action.Should().Throw(); + } + + [Theory] + [InlineData(true, "unix:///tmp/apm.socket")] + [InlineData(false, "http://localhost:8126")] + public void TestOptimizationFactoryUsesProtectedHttpTransportOnlyInAgentlessMode(bool agentless, string agentUri) + { + const string apiKey = "test-key"; + var values = new NameValueCollection(); + values.Add(ConfigurationKeys.AgentUri, agentUri); + var source = new NameValueConfigurationSource(values); + var settings = new TestOptimizationSettings(source, NullConfigurationTelemetry.Instance); + settings.SetAgentlessConfiguration(agentless, apiKey, "https://example.com"); + var management = new TestOptimizationTracerManagement(settings); + var tracerSettings = settings.InitializeTracerSettings(source); + + var factory = management.GetRequestFactory(tracerSettings).Should().BeOfType().Subject; + + GetClient(factory).DefaultRequestHeaders.Contains(ApiKeyHttpTransportGuard.ApiKeyHeaderName).Should().Be(agentless); + } + + private static HttpClientHandler GetHandler(HttpClientRequestFactory factory) + { + var field = typeof(HttpClientRequestFactory).GetField("_handler", BindingFlags.Instance | BindingFlags.NonPublic); + field.Should().NotBeNull(); + return field!.GetValue(factory).Should().BeOfType().Subject; + } + + private static HttpClient GetClient(HttpClientRequestFactory factory) + { + var field = typeof(HttpClientRequestFactory).GetField("_client", BindingFlags.Instance | BindingFlags.NonPublic); + field.Should().NotBeNull(); + return field!.GetValue(factory).Should().BeOfType().Subject; + } +#endif +} diff --git a/tracer/test/Datadog.Trace.Tests/Agent/Transports/ApiWebRequestFactoryTests.cs b/tracer/test/Datadog.Trace.Tests/Agent/Transports/ApiWebRequestFactoryTests.cs index 3a24b758fc73..181a7a523723 100644 --- a/tracer/test/Datadog.Trace.Tests/Agent/Transports/ApiWebRequestFactoryTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Agent/Transports/ApiWebRequestFactoryTests.cs @@ -4,9 +4,13 @@ // using System; +using System.Collections.Generic; using System.Net; using System.Reflection; +using System.Threading.Tasks; +using Datadog.Trace.Agent; using Datadog.Trace.Agent.Transports; +using FluentAssertions; using Xunit; namespace Datadog.Trace.Tests.Agent.Transports @@ -48,6 +52,72 @@ public void OverrideHttpPrefix() Assert.IsType(WebRequest.Create("http://localhost/")); } + [Fact] + public async Task RejectsUnsafeDefaultApiKeyHeader() + { + var factory = new ApiWebRequestFactory( + new Uri("http://example.com"), + [new KeyValuePair(ApiKeyHttpTransportGuard.ApiKeyHeaderName, "test-key")]); + var request = factory.Create(factory.GetEndpoint("/intake")); + + GetHttpWebRequest(request).AllowAutoRedirect.Should().BeFalse(); + await Assert.ThrowsAsync(() => request.GetAsync()); + } + + [Fact] + public void RejectsApiKeyAddedToKeylessFactoryRequest() + { + var factory = new ApiWebRequestFactory( + new Uri("https://example.com"), + []); + var request = factory.Create(factory.GetEndpoint("/intake")); + + var action = () => request.AddHeader(ApiKeyHttpTransportGuard.ApiKeyHeaderName.ToLowerInvariant(), "test-key"); + + action.Should().Throw(); + } + + [Fact] + public void DisablesProxyForPlaintextLoopbackWithApiKey() + { + var factory = new ApiWebRequestFactory( + new Uri("http://localhost"), + [new KeyValuePair(ApiKeyHttpTransportGuard.ApiKeyHeaderName, "test-key")]); + factory.SetProxy(new WebProxy("http://example.com"), credential: null); + + var request = factory.Create(factory.GetEndpoint("/intake")); + + GetHttpWebRequest(request).Proxy.Should().BeNull(); + } + + [Fact] + public async Task RejectsPlaintextLoopbackIfProxyIsReenabled() + { + var factory = new ApiWebRequestFactory( + new Uri("http://localhost"), + [new KeyValuePair(ApiKeyHttpTransportGuard.ApiKeyHeaderName, "test-key")]); + var request = factory.Create(factory.GetEndpoint("/intake")); + GetHttpWebRequest(request).Proxy = new NeverBypassProxy(); + + await Assert.ThrowsAsync(() => request.GetAsync()); + } + + private static HttpWebRequest GetHttpWebRequest(IApiRequest request) + { + var field = typeof(ApiWebRequest).GetField("_request", BindingFlags.Instance | BindingFlags.NonPublic); + field.Should().NotBeNull(); + return field!.GetValue(request).Should().BeOfType().Subject; + } + + private sealed class NeverBypassProxy : IWebProxy + { + public ICredentials Credentials { get; set; } + + public Uri GetProxy(Uri destination) => new("http://example.com"); + + public bool IsBypassed(Uri host) => false; + } + private class CustomWebRequestCreator : IWebRequestCreate { public WebRequest Create(Uri uri) diff --git a/tracer/test/Datadog.Trace.Tests/Ci/CIWriterHttpSenderTests.cs b/tracer/test/Datadog.Trace.Tests/Ci/CIWriterHttpSenderTests.cs new file mode 100644 index 000000000000..0801ad1bf77b --- /dev/null +++ b/tracer/test/Datadog.Trace.Tests/Ci/CIWriterHttpSenderTests.cs @@ -0,0 +1,51 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +#nullable enable + +using System; +using System.Threading.Tasks; +using Datadog.Trace.Agent; +using Datadog.Trace.Agent.Transports; +using Datadog.Trace.Ci.Agent; +using Datadog.Trace.Ci.Agent.MessagePack; +using Datadog.Trace.Ci.Agent.Payloads; +using Datadog.Trace.Ci.Configuration; +using Datadog.Trace.Configuration; +using Datadog.Trace.Configuration.Telemetry; +using Datadog.Trace.TestHelpers.TransportHelpers; +using FluentAssertions; +using Xunit; + +namespace Datadog.Trace.Tests.Ci; + +public class CIWriterHttpSenderTests +{ + [Fact] + public async Task DoesNotRetryAfterApiKeyTransportRejection() + { + var settings = new TestOptimizationSettings(NullConfigurationSource.Instance, NullConfigurationTelemetry.Instance); + settings.SetAgentlessConfiguration(enabled: true, apiKey: "test-key", agentlessUrl: "https://example.com"); + var payload = new CITestCyclePayload(settings, CIFormatterResolver.Instance); + var requestFactory = new TestRequestFactory(new Uri("https://example.com"), x => new UnsafeApiKeyTransportRequest(x)); + var sender = new CIWriterHttpSender(requestFactory); + + await sender.SendPayloadAsync(payload); + await sender.SendPayloadAsync(payload); + + requestFactory.RequestsSent.Should().ContainSingle(); + } + + private sealed class UnsafeApiKeyTransportRequest : TestApiRequest + { + public UnsafeApiKeyTransportRequest(Uri endpoint) + : base(endpoint) + { + } + + public override Task PostAsync(ArraySegment bytes, string contentType, string contentEncoding) + => Task.FromException(new ApiKeyHttpTransportException("Unsafe API-key transport.")); + } +} diff --git a/tracer/test/Datadog.Trace.Tests/Ci/TestOptimizationClientTests.cs b/tracer/test/Datadog.Trace.Tests/Ci/TestOptimizationClientTests.cs index 08652cf96578..e2b5b1a7e865 100644 --- a/tracer/test/Datadog.Trace.Tests/Ci/TestOptimizationClientTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Ci/TestOptimizationClientTests.cs @@ -9,6 +9,8 @@ using System.Collections.Generic; using System.IO; using System.Threading.Tasks; +using Datadog.Trace.Agent; +using Datadog.Trace.Agent.Transports; using Datadog.Trace.Ci; using Datadog.Trace.Ci.CiEnvironment; using Datadog.Trace.Ci.Configuration; @@ -19,6 +21,7 @@ using Datadog.Trace.Configuration.Telemetry; using Datadog.Trace.Logging; using Datadog.Trace.TestHelpers; +using Datadog.Trace.TestHelpers.TransportHelpers; using Datadog.Trace.Util.Json; using FluentAssertions; using Moq; @@ -507,6 +510,27 @@ public async Task FileClientCachesSkippableTestsByFullScopeFingerprintAndBypasse } } + [Fact] + public async Task ApiKeyTransportRejectionStopsRetriesAndFutureRequests() + { + var workspacePath = Path.Combine(Path.GetTempPath(), $"dd-trace-dotnet-api-key-transport-{Guid.NewGuid():N}"); + var settings = CreateSettings(); + settings.SetAgentlessConfiguration(enabled: true, apiKey: "test-key", agentlessUrl: "https://example.com"); + var requestFactory = new TestRequestFactory(new Uri("https://example.com"), x => new UnsafeApiKeyTransportRequest(x)); + var tracerManagement = new Mock(); + tracerManagement.Setup(x => x.GetRequestFactory(It.IsAny(), It.IsAny())).Returns(requestFactory); + var testOptimization = CreateTestOptimization(settings, workspacePath); + testOptimization.Setup(x => x.TracerManagement).Returns(tracerManagement.Object); + var client = TestOptimizationClient.Create(workspacePath, testOptimization.Object); + + Func firstRequest = async () => await client.GetSettingsAsync(); + Func secondRequest = async () => await client.GetSettingsAsync(); + + await firstRequest.Should().ThrowAsync(); + await secondRequest.Should().ThrowAsync(); + requestFactory.RequestsSent.Should().ContainSingle(); + } + [Fact] public void UnscopedSkippableCandidateKeepsLegacyMatching() { @@ -525,10 +549,14 @@ private static TestOptimizationSettings CreateSettings(params (string Key, strin private static Mock CreateTestOptimization(TestOptimizationSettings settings, string workspacePath) { + var hostInfo = new Mock(); + hostInfo.Setup(x => x.GetOperatingSystemVersion()).Returns("test-os-version"); + var testOptimization = new Mock(); testOptimization.Setup(x => x.RunId).Returns("test-run"); testOptimization.Setup(x => x.Settings).Returns(settings); testOptimization.Setup(x => x.CIValues).Returns(new TestCIEnvironmentValues(workspacePath)); + testOptimization.Setup(x => x.HostInfo).Returns(hostInfo.Object); testOptimization.Setup(x => x.Log).Returns(DatadogLogging.GetLoggerFor(typeof(TestOptimizationClientTests))); return testOptimization; } @@ -572,6 +600,7 @@ private sealed class TestCIEnvironmentValues : CIEnvironmentValues public TestCIEnvironmentValues(string workspacePath) { WorkspacePath = workspacePath; + Repository = "https://github.com/DataDog/dd-trace-dotnet"; Branch = "main"; Commit = "abcdef123456"; } @@ -580,4 +609,15 @@ protected override void Setup(IGitInfo gitInfo) { } } + + private sealed class UnsafeApiKeyTransportRequest : TestApiRequest + { + public UnsafeApiKeyTransportRequest(Uri endpoint) + : base(endpoint) + { + } + + public override Task PostAsync(ArraySegment bytes, string contentType, string contentEncoding) + => Task.FromException(new ApiKeyHttpTransportException("Unsafe API-key transport.")); + } } diff --git a/tracer/test/Datadog.Trace.Tests/Debugger/BatchUploaderTests.cs b/tracer/test/Datadog.Trace.Tests/Debugger/BatchUploaderTests.cs index ffcda2da470b..4e8e2c5858d6 100644 --- a/tracer/test/Datadog.Trace.Tests/Debugger/BatchUploaderTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Debugger/BatchUploaderTests.cs @@ -9,6 +9,7 @@ using System.Net.Http; using System.Text; using System.Threading.Tasks; +using Datadog.Trace.Agent.Transports; using Datadog.Trace.Debugger.Sink; using Datadog.Trace.Debugger.Upload; using FluentAssertions; @@ -129,6 +130,18 @@ public async Task CheckTwoSmallOutputs() Encoding.UTF8.GetString(_api.Segments[1]).Should().Be("[Test3]"); } + [Fact] + public async Task ApiKeyTransportFailureDisablesSubsequentUploads() + { + var api = new UnsafeApiKeyTransport(); + var uploader = BatchUploader.Create(api); + + await uploader.Upload(new[] { "Test1" }); + await uploader.Upload(new[] { "Test2" }); + + api.SendCount.Should().Be(1); + } + [Fact] public async Task CheckMediumOutput() { @@ -167,4 +180,15 @@ public Task SendBatchAsync(ArraySegment symbols) return Task.FromResult(true); } } + + private sealed class UnsafeApiKeyTransport : IBatchUploadApi + { + public int SendCount { get; private set; } + + public Task SendBatchAsync(ArraySegment symbols) + { + SendCount++; + return Task.FromException(new ApiKeyHttpTransportException("unsafe endpoint")); + } + } } diff --git a/tracer/test/Datadog.Trace.Tests/Debugger/ExceptionReplayTransportFactoryTests.cs b/tracer/test/Datadog.Trace.Tests/Debugger/ExceptionReplayTransportFactoryTests.cs index 3e1ca4869ba1..efa929354f44 100644 --- a/tracer/test/Datadog.Trace.Tests/Debugger/ExceptionReplayTransportFactoryTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Debugger/ExceptionReplayTransportFactoryTests.cs @@ -71,5 +71,20 @@ public void AgentlessTransport_UsesOverrideUrl() .ToString() .Should().Be("https://custom-host.example.com/api/v2/debugger"); } + + [Fact] + public void AgentlessTransport_RejectsUnsafeOverrideUrlDuringCreation() + { + var tracerSettings = new TracerSettings(new NameValueConfigurationSource(new NameValueCollection())); + var collection = new NameValueCollection + { + { ConfigurationKeys.Debugger.ExceptionReplayAgentlessEnabled, "true" }, + { ConfigurationKeys.ApiKey, "test-key" }, + { ConfigurationKeys.Debugger.ExceptionReplayAgentlessUrl, "http://example.com" } + }; + var erSettings = new ExceptionReplaySettings(new NameValueConfigurationSource(collection), NullConfigurationTelemetry.Instance); + + ExceptionReplayTransportFactory.Create(tracerSettings, erSettings, NullDiscoveryService.Instance).Should().BeNull(); + } } } diff --git a/tracer/test/Datadog.Trace.Tests/Debugger/SymbolsTests/SymbolUploadApiTests.cs b/tracer/test/Datadog.Trace.Tests/Debugger/SymbolsTests/SymbolUploadApiTests.cs index da6453cb4bef..c6a7832bb750 100644 --- a/tracer/test/Datadog.Trace.Tests/Debugger/SymbolsTests/SymbolUploadApiTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Debugger/SymbolsTests/SymbolUploadApiTests.cs @@ -151,6 +151,28 @@ static async (stream, state) => .BeEquivalentTo(ParseRequestForVerify(requestFactory.Requests[0], enableCompression)); } + [Fact] + public async Task SendBatchAsync_ApiKeyTransportFailureDisablesSubsequentUploads() + { + var metadata = new SymDbUploadMetadata( + Service: "benchmark-service", + Version: "1.0.0", + UploadId: Guid.Parse("11111111-2222-3333-4444-555555555555"), + BatchNum: 7, + Final: false); + var requestFactory = CapturingRequestFactory.RejectUnsafeApiKeyTransport(); + var discoveryService = new DiscoveryServiceMock(); + var api = SymbolUploadApi.Create(requestFactory, discoveryService, new NullGitMetadataProvider(), enableCompression: false); + discoveryService.TriggerChange(symbolDbEndpoint: "symdb/v1/input"); + + var firstResult = await api.SendBatchAsync(static (_, _) => Task.CompletedTask, state: 0, metadata: metadata); + var secondResult = await api.SendBatchAsync(static (_, _) => Task.CompletedTask, state: 0, metadata: metadata); + + firstResult.Should().BeFalse(); + secondResult.Should().BeFalse(); + requestFactory.Requests.Should().ContainSingle(); + } + [Fact] public async Task SendBatchAsync_DoesNotDelayAfterFinalRetry() { @@ -293,10 +315,17 @@ private sealed class MutableInt private sealed class CapturingRequestFactory : IApiRequestFactory { private readonly Uri _baseEndpoint = new("http://localhost:8126/"); + private readonly bool _rejectUnsafeApiKeyTransport; private readonly Queue _statusCodes; public CapturingRequestFactory(params int[] statusCodes) + : this(rejectUnsafeApiKeyTransport: false, statusCodes: statusCodes) + { + } + + private CapturingRequestFactory(bool rejectUnsafeApiKeyTransport, params int[] statusCodes) { + _rejectUnsafeApiKeyTransport = rejectUnsafeApiKeyTransport; _statusCodes = new Queue(statusCodes.Length == 0 ? new[] { 200 } : statusCodes); } @@ -304,6 +333,9 @@ public CapturingRequestFactory(params int[] statusCodes) public CapturingRequest Request => Requests[0]; + public static CapturingRequestFactory RejectUnsafeApiKeyTransport() + => new(rejectUnsafeApiKeyTransport: true); + public string Info(Uri endpoint) => endpoint.ToString(); @@ -313,7 +345,7 @@ public Uri GetEndpoint(string relativePath) public IApiRequest Create(Uri endpoint) { var statusCode = _statusCodes.Count > 0 ? _statusCodes.Dequeue() : 200; - var request = new CapturingRequest(statusCode); + var request = new CapturingRequest(statusCode, _rejectUnsafeApiKeyTransport); Requests.Add(request); return request; } @@ -326,10 +358,12 @@ public void SetProxy(WebProxy proxy, NetworkCredential credential) private sealed class CapturingRequest : IApiRequest { private readonly int _statusCode; + private readonly bool _rejectUnsafeApiKeyTransport; - public CapturingRequest(int statusCode) + public CapturingRequest(int statusCode, bool rejectUnsafeApiKeyTransport) { _statusCode = statusCode; + _rejectUnsafeApiKeyTransport = rejectUnsafeApiKeyTransport; } public byte[] Body { get; private set; } = []; @@ -365,6 +399,11 @@ public Task PostAsJsonAsync(T payload, MultipartCompression com public async Task PostAsync(Func writeToRequestStream, string contentType, string contentEncoding, string multipartBoundary) { + if (_rejectUnsafeApiKeyTransport) + { + throw new ApiKeyHttpTransportException("unsafe endpoint"); + } + using var stream = new MemoryStream(); await writeToRequestStream(stream).ConfigureAwait(false); Body = stream.ToArray(); diff --git a/tracer/test/Datadog.Trace.Tests/Logging/DirectSubmission/Sink/LogsApiTests.cs b/tracer/test/Datadog.Trace.Tests/Logging/DirectSubmission/Sink/LogsApiTests.cs index d311d9a9be1c..0b0925818d4c 100644 --- a/tracer/test/Datadog.Trace.Tests/Logging/DirectSubmission/Sink/LogsApiTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Logging/DirectSubmission/Sink/LogsApiTests.cs @@ -5,10 +5,17 @@ using System; using System.Collections.Generic; +using System.Collections.Specialized; using System.IO; using System.Linq; +using System.Net; using System.Text; using System.Threading.Tasks; +using Datadog.Trace.Agent; +using Datadog.Trace.Agent.Transports; +using Datadog.Trace.Configuration; +using Datadog.Trace.Configuration.Telemetry; +using Datadog.Trace.Logging.DirectSubmission; using Datadog.Trace.Logging.DirectSubmission.Sink; using Datadog.Trace.TestHelpers.TransportHelpers; using FluentAssertions; @@ -29,8 +36,8 @@ private static readonly Func SingleFaultyRequest = x => new FaultyApiRequest(x); [Theory] - [InlineData("http://http-intake.logs.datadoghq.com", "http://http-intake.logs.datadoghq.com/api/v2/logs")] - [InlineData("http://http-intake.logs.datadoghq.com/", "http://http-intake.logs.datadoghq.com/api/v2/logs")] + [InlineData("https://http-intake.logs.datadoghq.com", "https://http-intake.logs.datadoghq.com/api/v2/logs")] + [InlineData("https://http-intake.logs.datadoghq.com/", "https://http-intake.logs.datadoghq.com/api/v2/logs")] [InlineData("https://http-intake.logs.datadoghq.com:443", "https://http-intake.logs.datadoghq.com:443/api/v2/logs")] [InlineData("http://localhost:8080", "http://localhost:8080/api/v2/logs")] [InlineData("http://localhost:8080/sub-path", "http://localhost:8080/sub-path/api/v2/logs")] @@ -40,7 +47,7 @@ public async Task SendsRequestToCorrectUrl(string baseUri, string expected) var baseEndpoint = new Uri(baseUri); var requestFactory = new TestRequestFactory(baseEndpoint); - var api = new LogsApi("SECR3TZ", requestFactory); + var api = new LogsApi(requestFactory); var result = await api.SendLogsAsync(Logs, NumberOfLogs); requestFactory.RequestsSent.Should() @@ -49,14 +56,30 @@ public async Task SendsRequestToCorrectUrl(string baseUri, string expected) result.Should().BeTrue(); } + [Fact] + public async Task RejectsUnsafeUrlWithProductionTransport() + { + var source = new NameValueConfigurationSource( + new NameValueCollection + { + { ConfigurationKeys.ApiKey, "test-key" }, + { ConfigurationKeys.DirectLogSubmission.Url, "http://example.com" } + }); + var settings = new DirectLogSubmissionSettings(source, NullConfigurationTelemetry.Instance); + var api = new LogsApi(LogsTransportStrategy.Get(settings)); + + var result = await api.SendLogsAsync(Logs, NumberOfLogs); + + result.Should().BeFalse(); + } + [Fact] public async Task ShouldRetryRequestsWhenTheyFail() { // two faults, then success var requestFactory = new TestRequestFactory(new Uri(DefaultIntake), SingleFaultyRequest, SingleFaultyRequest); - var apiKey = "SECR3TZ"; - var api = new LogsApi(apiKey, requestFactory); + var api = new LogsApi(requestFactory); var result = await api.SendLogsAsync(Logs, NumberOfLogs); requestFactory.RequestsSent @@ -72,18 +95,31 @@ public async Task ShouldRetryRequestsWhenTheyFail() } [Fact] - public async Task ShouldAddApiKeyToAllRequests() + public async Task ShouldNotRetryAfterApiKeyTransportRejection() { - var requestFactory = new TestRequestFactory(new Uri(DefaultIntake), SingleFaultyRequest); + var requestFactory = new TestRequestFactory(new Uri(DefaultIntake), x => new UnsafeApiKeyTransportRequest(x)); + var api = new LogsApi(requestFactory); - var apiKey = "SECR3TZ"; - var api = new LogsApi(apiKey, requestFactory); - await api.SendLogsAsync(Logs, NumberOfLogs); + var firstResult = await api.SendLogsAsync(Logs, NumberOfLogs); + var secondResult = await api.SendLogsAsync(Logs, NumberOfLogs); - requestFactory.RequestsSent.Should() - .NotBeEmpty() - .And.OnlyContain(x => x.ExtraHeaders.ContainsKey(LogsApi.IntakeHeaderNameApiKey)) - .And.OnlyContain(x => x.ExtraHeaders[LogsApi.IntakeHeaderNameApiKey] == apiKey); + firstResult.Should().BeFalse(); + secondResult.Should().BeFalse(); + requestFactory.RequestsSent.Should().ContainSingle(); + } + + [Fact] + public async Task ShouldNotRetryAfterApiKeyTransportRejectionDuringRequestCreation() + { + var requestFactory = new RejectingRequestFactory(new Uri(DefaultIntake)); + var api = new LogsApi(requestFactory); + + var firstResult = await api.SendLogsAsync(Logs, NumberOfLogs); + var secondResult = await api.SendLogsAsync(Logs, NumberOfLogs); + + firstResult.Should().BeFalse(); + secondResult.Should().BeFalse(); + requestFactory.CreationAttempts.Should().Be(1); } [Fact] @@ -91,7 +127,7 @@ public async Task ShouldSetContentTypeForAllRequests() { var requestFactory = new TestRequestFactory(new Uri(DefaultIntake), SingleFaultyRequest); - var api = new LogsApi("SECR3TZ", requestFactory); + var api = new LogsApi(requestFactory); await api.SendLogsAsync(Logs, NumberOfLogs); using var scope = new AssertionScope(); @@ -109,7 +145,7 @@ public async Task ShouldNotRetryWhenClientError() { var requestFactory = new TestRequestFactory(new Uri(DefaultIntake), x => new FaultyApiRequest(x, statusCode: 400)); - var api = new LogsApi("SECR3TZ", requestFactory); + var api = new LogsApi(requestFactory); var result = await api.SendLogsAsync(Logs, NumberOfLogs); using var scope = new AssertionScope(); @@ -118,5 +154,42 @@ public async Task ShouldNotRetryWhenClientError() result.Should().BeFalse(); } + + private sealed class UnsafeApiKeyTransportRequest : TestApiRequest + { + public UnsafeApiKeyTransportRequest(Uri endpoint) + : base(endpoint) + { + } + + public override Task PostAsync(ArraySegment bytes, string contentType, string contentEncoding) + => Task.FromException(new ApiKeyHttpTransportException("Unsafe API-key transport.")); + } + + private sealed class RejectingRequestFactory : IApiRequestFactory + { + private readonly Uri _baseEndpoint; + + public RejectingRequestFactory(Uri baseEndpoint) + { + _baseEndpoint = baseEndpoint; + } + + public int CreationAttempts { get; private set; } + + public Uri GetEndpoint(string relativePath) => new(_baseEndpoint, relativePath); + + public string Info(Uri endpoint) => endpoint.ToString(); + + public IApiRequest Create(Uri endpoint) + { + CreationAttempts++; + throw new ApiKeyHttpTransportException("Unsafe API-key transport."); + } + + public void SetProxy(WebProxy proxy, NetworkCredential credential) + { + } + } } } diff --git a/tracer/test/Datadog.Trace.Tests/Telemetry/Transports/JsonTelemetryTransportTests.cs b/tracer/test/Datadog.Trace.Tests/Telemetry/Transports/JsonTelemetryTransportTests.cs index 5813c1d32aa3..a2ab55cc68a9 100644 --- a/tracer/test/Datadog.Trace.Tests/Telemetry/Transports/JsonTelemetryTransportTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Telemetry/Transports/JsonTelemetryTransportTests.cs @@ -86,6 +86,38 @@ public async Task ShouldContainRequiredHeaders(bool debugEnabled, [Combinatorial #endif + [Fact] + public async Task ApiKeyTransportExceptionIsFatalAndDisablesSubsequentPushes() + { + var requestMock = new Mock(); + requestMock.Setup(x => x.PostAsJsonAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ThrowsAsync(new ApiKeyHttpTransportException("unsafe endpoint")); + var requestFactoryMock = new Mock(); + requestFactoryMock.Setup(x => x.Create(It.IsAny())).Returns(requestMock.Object); + var transport = new AgentlessTelemetryTransport( + requestFactoryMock.Object, + debugEnabled: false, + telemetryCompressionMethod: string.Empty, + containerMetadata: new ContainerMetadata(containerId: null, entityId: null)); + var data = new TelemetryData( + "request-type", + tracerTime: 0, + string.Empty, + seqId: 0, + new ApplicationTelemetryData(string.Empty, string.Empty, string.Empty, string.Empty, string.Empty, string.Empty, string.Empty, string.Empty, string.Empty, string.Empty, string.Empty), + new HostTelemetryData(string.Empty, string.Empty, string.Empty), + payload: null); + + var firstResult = await transport.PushTelemetry(data); + var secondResult = await transport.PushTelemetry(data); + + firstResult.Should().Be(TelemetryPushResult.FatalError); + secondResult.Should().Be(TelemetryPushResult.FatalError); + requestMock.Verify( + x => x.PostAsJsonAsync(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once); + } + [Fact] public void SerializedAppStartedShouldProduceJsonWithExpectedFormat() {