-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Fix authenticated proxy for dotnet restore #123328
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -17,6 +17,38 @@ internal static partial class AuthenticationHelper | |||
| private const string NtlmScheme = "NTLM"; | ||||
| private const string NegotiateScheme = "Negotiate"; | ||||
|
|
||||
| private const string EnableProactiveProxyAuthCtxSwitch = "System.Net.Http.EnableProactiveProxyAuth"; | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: I'd call it |
||||
| private const string EnableProactiveProxyAuthEnvironmentVariable = "DOTNET_SYSTEM_NET_HTTP_ENABLEPROACTIVEPROXYAUTH"; | ||||
|
|
||||
| private static volatile int s_enableProactiveProxyAuth = -1; | ||||
|
|
||||
| private static bool EnableProactiveProxyAuth | ||||
| { | ||||
| get | ||||
| { | ||||
| int enableProactiveProxyAuth = s_enableProactiveProxyAuth; | ||||
| if (enableProactiveProxyAuth != -1) | ||||
| { | ||||
| return enableProactiveProxyAuth != 0; | ||||
| } | ||||
|
|
||||
| // First check for the AppContext switch, giving it priority over the environment variable. | ||||
| if (AppContext.TryGetSwitch(EnableProactiveProxyAuthCtxSwitch, out bool value)) | ||||
| { | ||||
| s_enableProactiveProxyAuth = value ? 1 : 0; | ||||
| } | ||||
| else | ||||
| { | ||||
| // AppContext switch wasn't used. Check the environment variable. | ||||
| s_enableProactiveProxyAuth = | ||||
| Environment.GetEnvironmentVariable(EnableProactiveProxyAuthEnvironmentVariable) is string envVar && | ||||
| (envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase)) ? 1 : 0; | ||||
| } | ||||
|
|
||||
| return s_enableProactiveProxyAuth != 0; | ||||
| } | ||||
| } | ||||
|
Comment on lines
+20
to
+50
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please move this to
|
||||
|
|
||||
| private enum AuthenticationType | ||||
| { | ||||
| Basic, | ||||
|
|
@@ -359,6 +391,18 @@ await TrySetDigestAuthToken(request, challenge.Credential, digestResponse, isPro | |||
|
|
||||
| public static ValueTask<HttpResponseMessage> SendWithProxyAuthAsync(HttpRequestMessage request, Uri proxyUri, bool async, ICredentials proxyCredentials, bool doRequestAuth, HttpConnectionPool pool, CancellationToken cancellationToken) | ||||
| { | ||||
| // When enabled via AppContext switch or environment variable, send Basic auth proactively | ||||
| // on the first request. This is needed for proxies that don't send 407 challenges but instead | ||||
| // drop or reject unauthenticated connections (e.g., some HTTPS CONNECT tunnel proxies). | ||||
| if (EnableProactiveProxyAuth) | ||||
| { | ||||
| NetworkCredential? credential = proxyCredentials.GetCredential(proxyUri, BasicScheme); | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @wfurt, do we want this in general for proxies or only for |
||||
| if (credential != null && credential != CredentialCache.DefaultNetworkCredentials) | ||||
| { | ||||
| SetBasicAuthToken(request, credential, isProxyAuth: true); | ||||
| } | ||||
| } | ||||
|
Comment on lines
+397
to
+404
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this call |
||||
|
|
||||
| return SendWithAuthAsync(request, proxyUri, async, proxyCredentials, preAuthenticate: false, isProxyAuth: true, doRequestAuth, pool, cancellationToken); | ||||
| } | ||||
|
|
||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,192 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Collections.Generic; | ||
| using System.Diagnostics; | ||
| using System.Net.Test.Common; | ||
| using System.Text; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.DotNet.RemoteExecutor; | ||
| using Xunit; | ||
| using Xunit.Abstractions; | ||
|
|
||
| namespace System.Net.Http.Functional.Tests | ||
| { | ||
| public abstract class ProactiveProxyAuthTest : HttpClientHandlerTestBase | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to see coverage for both HTTP and HTTPS proxy as well as for HTTP and HTTPS requests (proxy request vs proxy tunnel), i.e. 4 cases. |
||
| { | ||
| public ProactiveProxyAuthTest(ITestOutputHelper helper) : base(helper) { } | ||
|
|
||
| /// <summary> | ||
| /// Tests that when proxy credentials are provided and the opt-in switch is enabled, | ||
| /// the Proxy-Authorization header is sent proactively on the first request without | ||
| /// waiting for a 407 challenge. | ||
| /// </summary> | ||
| [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] | ||
| public async Task ProxyAuth_CredentialsProvided_SentProactivelyOnFirstRequest() | ||
| { | ||
| const string ExpectedUsername = "testuser"; | ||
| const string ExpectedPassword = "testpassword"; | ||
|
|
||
| await LoopbackServer.CreateServerAsync(async (proxyServer, proxyUri) => | ||
| { | ||
| var psi = new ProcessStartInfo(); | ||
| psi.Environment.Add("http_proxy", $"http://{proxyUri.Host}:{proxyUri.Port}"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should also be test with the creds in the env var. Similar to the original issue. |
||
| psi.Environment.Add("DOTNET_SYSTEM_NET_HTTP_ENABLEPROACTIVEPROXYAUTH", "1"); | ||
|
|
||
| Task serverTask = proxyServer.AcceptConnectionAsync(async connection => | ||
| { | ||
| List<string> lines = await connection.ReadRequestHeaderAsync().ConfigureAwait(false); | ||
|
|
||
| // Verify the first request has the Proxy-Authorization header (proactive auth) | ||
| string? authHeader = null; | ||
| foreach (string line in lines) | ||
| { | ||
| if (line.StartsWith("Proxy-Authorization:", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| authHeader = line; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| Assert.NotNull(authHeader); | ||
| Assert.Contains("Basic", authHeader); | ||
|
|
||
| // Verify the credentials are correct | ||
| string expectedToken = Convert.ToBase64String(Encoding.UTF8.GetBytes($"{ExpectedUsername}:{ExpectedPassword}")); | ||
| Assert.Contains(expectedToken, authHeader); | ||
|
|
||
| await connection.SendResponseAsync(HttpStatusCode.OK).ConfigureAwait(false); | ||
| }); | ||
|
|
||
| await RemoteExecutor.Invoke(async (username, password, useVersionString) => | ||
| { | ||
| using HttpClientHandler handler = CreateHttpClientHandler(useVersionString); | ||
| handler.DefaultProxyCredentials = new NetworkCredential(username, password); | ||
|
|
||
| using HttpClient client = CreateHttpClient(handler, useVersionString); | ||
| using HttpResponseMessage response = await client.GetAsync("http://destination.test/"); | ||
|
|
||
| Assert.Equal(HttpStatusCode.OK, response.StatusCode); | ||
| }, ExpectedUsername, ExpectedPassword, UseVersion.ToString(), | ||
| new RemoteInvokeOptions { StartInfo = psi }).DisposeAsync(); | ||
|
|
||
| await serverTask; | ||
| }); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Tests that without the opt-in switch, credentials are NOT sent proactively | ||
| /// (default RFC-compliant behavior). | ||
| /// </summary> | ||
| [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] | ||
| public async Task ProxyAuth_WithoutOptIn_NotSentProactively() | ||
| { | ||
| const string ExpectedUsername = "testuser"; | ||
| const string ExpectedPassword = "testpassword"; | ||
|
|
||
| await LoopbackServer.CreateServerAsync(async (proxyServer, proxyUri) => | ||
| { | ||
| var psi = new ProcessStartInfo(); | ||
| psi.Environment.Add("http_proxy", $"http://{proxyUri.Host}:{proxyUri.Port}"); | ||
| // NOT setting DOTNET_SYSTEM_NET_HTTP_ENABLEPROACTIVEPROXYAUTH | ||
|
|
||
| Task serverTask = proxyServer.AcceptConnectionAsync(async connection => | ||
| { | ||
| List<string> lines = await connection.ReadRequestHeaderAsync().ConfigureAwait(false); | ||
|
|
||
| // Verify the first request does NOT have the Proxy-Authorization header | ||
| foreach (string line in lines) | ||
| { | ||
| Assert.False(line.StartsWith("Proxy-Authorization:", StringComparison.OrdinalIgnoreCase), | ||
| "First request should not have Proxy-Authorization header without opt-in"); | ||
| } | ||
|
|
||
| // Send 407 challenge | ||
| await connection.SendResponseAsync(HttpStatusCode.ProxyAuthenticationRequired, | ||
| "Proxy-Authenticate: Basic realm=\"Test\"\r\n").ConfigureAwait(false); | ||
|
|
||
| // Read the retry request with credentials | ||
| lines = await connection.ReadRequestHeaderAsync().ConfigureAwait(false); | ||
|
|
||
| // Now it should have credentials | ||
| string? authHeader = null; | ||
| foreach (string line in lines) | ||
| { | ||
| if (line.StartsWith("Proxy-Authorization:", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| authHeader = line; | ||
| break; | ||
| } | ||
| } | ||
| Assert.NotNull(authHeader); | ||
|
|
||
| await connection.SendResponseAsync(HttpStatusCode.OK).ConfigureAwait(false); | ||
| }); | ||
|
|
||
| await RemoteExecutor.Invoke(async (username, password, useVersionString) => | ||
| { | ||
| using HttpClientHandler handler = CreateHttpClientHandler(useVersionString); | ||
| handler.DefaultProxyCredentials = new NetworkCredential(username, password); | ||
|
|
||
| using HttpClient client = CreateHttpClient(handler, useVersionString); | ||
| using HttpResponseMessage response = await client.GetAsync("http://destination.test/"); | ||
|
|
||
| Assert.Equal(HttpStatusCode.OK, response.StatusCode); | ||
| }, ExpectedUsername, ExpectedPassword, UseVersion.ToString(), | ||
| new RemoteInvokeOptions { StartInfo = psi }).DisposeAsync(); | ||
|
|
||
| await serverTask; | ||
| }); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Tests that DefaultNetworkCredentials are NOT sent proactively even with the opt-in, | ||
| /// as they are only for NTLM/Negotiate which require challenge-response. | ||
| /// </summary> | ||
| [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] | ||
| public async Task ProxyAuth_DefaultCredentials_NotSentProactively() | ||
| { | ||
| await LoopbackServer.CreateServerAsync(async (proxyServer, proxyUri) => | ||
| { | ||
| var psi = new ProcessStartInfo(); | ||
| psi.Environment.Add("http_proxy", $"http://{proxyUri.Host}:{proxyUri.Port}"); | ||
| psi.Environment.Add("DOTNET_SYSTEM_NET_HTTP_ENABLEPROACTIVEPROXYAUTH", "1"); | ||
|
|
||
| Task serverTask = proxyServer.AcceptConnectionAsync(async connection => | ||
| { | ||
| List<string> lines = await connection.ReadRequestHeaderAsync().ConfigureAwait(false); | ||
|
|
||
| // Verify the first request does NOT have the Proxy-Authorization header | ||
| // (DefaultNetworkCredentials should not trigger proactive auth) | ||
| foreach (string line in lines) | ||
| { | ||
| Assert.False(line.StartsWith("Proxy-Authorization:", StringComparison.OrdinalIgnoreCase), | ||
| "DefaultNetworkCredentials should not trigger proactive Basic auth"); | ||
| } | ||
|
|
||
| await connection.SendResponseAsync(HttpStatusCode.OK).ConfigureAwait(false); | ||
| }); | ||
|
|
||
| await RemoteExecutor.Invoke(async (useVersionString) => | ||
| { | ||
| using HttpClientHandler handler = CreateHttpClientHandler(useVersionString); | ||
| handler.DefaultProxyCredentials = CredentialCache.DefaultNetworkCredentials; | ||
|
|
||
| using HttpClient client = CreateHttpClient(handler, useVersionString); | ||
| using HttpResponseMessage response = await client.GetAsync("http://destination.test/"); | ||
|
|
||
| Assert.Equal(HttpStatusCode.OK, response.StatusCode); | ||
| }, UseVersion.ToString(), | ||
| new RemoteInvokeOptions { StartInfo = psi }).DisposeAsync(); | ||
|
|
||
| await serverTask; | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| public sealed class ProactiveProxyAuthTest_Http11 : ProactiveProxyAuthTest | ||
| { | ||
| public ProactiveProxyAuthTest_Http11(ITestOutputHelper helper) : base(helper) { } | ||
| protected override Version UseVersion => HttpVersion.Version11; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a helper
RuntimeSettingParserexactly for parsing AppContext and Env Var switches. Can it be used here?