From 8f0216e3e20e95e67e9f67dd8e8aef688116fab4 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 9 Aug 2021 21:52:37 -0700 Subject: [PATCH 1/5] make port optional in SPN --- .../AuthenticationHelper.NtAuth.cs | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs index 8ee91309381721..b0c378babd4853 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs @@ -13,6 +13,28 @@ namespace System.Net.Http { internal static partial class AuthenticationHelper { + private const string UsePortInSpnCtxSwitch = "System.Net.Http.UsePortInSpn"; + private const string UsePortInSpnEnvironmentVariable = "DOTNET_SYSTEM_NET_HTTP_USEPORTINSPN"; + + private static bool UsePortInSpn() + { + // First check for the AppContext switch, giving it priority over the environment variable. + if (AppContext.TryGetSwitch(UsePortInSpnCtxSwitch, out bool disabled)) + { + return disabled; + } + + // AppContext switch wasn't used. Check the environment variable. + string? envVar = Environment.GetEnvironmentVariable(UsePortInSpnEnvironmentVariable); + + if (envVar is not null) + { + return envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase); + } + + return false; + } + private static Task InnerSendAsync(HttpRequestMessage request, bool async, bool isProxyAuth, HttpConnectionPool pool, HttpConnection connection, CancellationToken cancellationToken) { return isProxyAuth ? @@ -110,7 +132,7 @@ private static async Task SendWithNtAuthAsync(HttpRequestMe hostName = result.HostName; } - if (!isProxyAuth && !authUri.IsDefaultPort) + if (!isProxyAuth && !authUri.IsDefaultPort && UsePortInSpn) { hostName = string.Create(null, stackalloc char[128], $"{hostName}:{authUri.Port}"); } From a13095eb208aa9545d13f0fafd47333093b4dedd Mon Sep 17 00:00:00 2001 From: wfurt Date: Tue, 10 Aug 2021 11:47:02 -0700 Subject: [PATCH 2/5] fix tests --- .../EnterpriseTestConfiguration.cs | 1 + .../setup/apacheweb/apache2.conf | 12 ++++++-- .../EnterpriseTests/setup/apacheweb/run.sh | 1 + .../EnterpriseTests/setup/docker-compose.yml | 2 +- .../AuthenticationHelper.NtAuth.cs | 29 ++++++++++--------- .../HttpClientAuthenticationTest.cs | 9 ++++-- 6 files changed, 36 insertions(+), 18 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/EnterpriseTests/EnterpriseTestConfiguration.cs b/src/libraries/Common/tests/System/Net/EnterpriseTests/EnterpriseTestConfiguration.cs index e6a9cef30777bd..f6ea2e8e743f0e 100644 --- a/src/libraries/Common/tests/System/Net/EnterpriseTests/EnterpriseTestConfiguration.cs +++ b/src/libraries/Common/tests/System/Net/EnterpriseTests/EnterpriseTestConfiguration.cs @@ -7,6 +7,7 @@ public static class EnterpriseTestConfiguration { public const string Realm = "LINUX.CONTOSO.COM"; public const string NegotiateAuthWebServer = "http://apacheweb.linux.contoso.com/auth/kerberos/"; + public const string NegotiateAuthWebServerNotDefaultPort = "http://apacheweb.linux.contoso.com:8081/auth/kerberos/"; public const string AlternativeService = "http://altweb.linux.contoso.com:8080/auth/kerberos/"; public const string NtlmAuthWebServer = "http://apacheweb.linux.contoso.com:8080/auth/ntlm/"; public const string DigestAuthWebServer = "http://apacheweb.linux.contoso.com/auth/digest/"; diff --git a/src/libraries/Common/tests/System/Net/EnterpriseTests/setup/apacheweb/apache2.conf b/src/libraries/Common/tests/System/Net/EnterpriseTests/setup/apacheweb/apache2.conf index 87c20f04f2ed1f..a26a52ed8a0c9f 100644 --- a/src/libraries/Common/tests/System/Net/EnterpriseTests/setup/apacheweb/apache2.conf +++ b/src/libraries/Common/tests/System/Net/EnterpriseTests/setup/apacheweb/apache2.conf @@ -54,6 +54,7 @@ Listen 8080 Listen 80 +Listen 8081 # @@ -238,7 +239,7 @@ Group daemon # e-mailed. This address appears on some server-generated pages, such # as error documents. e.g. admin@your-domain.com # -ServerAdmin you@example.com +ServerAdmin webmaster@contoso.com # # ServerName gives the name and port that the server uses to identify itself. @@ -583,11 +584,18 @@ SSLRandomSeed startup builtin SSLRandomSeed connect builtin + - ServerAdmin webmaster@contoso.com DocumentRoot "/setup/altdocs" ServerName altservice.contoso.com:8080 + + + + + DocumentRoot "/setup/htdocs" + + diff --git a/src/libraries/Common/tests/System/Net/EnterpriseTests/setup/apacheweb/run.sh b/src/libraries/Common/tests/System/Net/EnterpriseTests/setup/apacheweb/run.sh index 6aa96948a10332..0b4a615157c0b7 100644 --- a/src/libraries/Common/tests/System/Net/EnterpriseTests/setup/apacheweb/run.sh +++ b/src/libraries/Common/tests/System/Net/EnterpriseTests/setup/apacheweb/run.sh @@ -11,6 +11,7 @@ if [ "$1" == "-debug" ]; then fi if [ "$1" == "-DNTLM" ]; then + # NTLM/Winbind is aggressive and eats Negotiate so it cannot be combined with Kerberos ./setup-pdc.sh /usr/sbin/apache2 -DALTPORT "$@" shift diff --git a/src/libraries/Common/tests/System/Net/EnterpriseTests/setup/docker-compose.yml b/src/libraries/Common/tests/System/Net/EnterpriseTests/setup/docker-compose.yml index 6aebd7ee187a4c..c54adfb7caaea3 100644 --- a/src/libraries/Common/tests/System/Net/EnterpriseTests/setup/docker-compose.yml +++ b/src/libraries/Common/tests/System/Net/EnterpriseTests/setup/docker-compose.yml @@ -41,7 +41,7 @@ services: hostname: altweb domainname: linux.contoso.com dns_search: linux.contoso.com - command: -DALTPORT + command: "-DALTPORT -DALTSPN" volumes: - shared-volume:/SHARED networks: diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs index b0c378babd4853..b6b6bdc25a06ee 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs @@ -16,24 +16,27 @@ internal static partial class AuthenticationHelper private const string UsePortInSpnCtxSwitch = "System.Net.Http.UsePortInSpn"; private const string UsePortInSpnEnvironmentVariable = "DOTNET_SYSTEM_NET_HTTP_USEPORTINSPN"; - private static bool UsePortInSpn() + private static bool UsePortInSpn { - // First check for the AppContext switch, giving it priority over the environment variable. - if (AppContext.TryGetSwitch(UsePortInSpnCtxSwitch, out bool disabled)) + get { - return disabled; - } + // First check for the AppContext switch, giving it priority over the environment variable. + if (AppContext.TryGetSwitch(UsePortInSpnCtxSwitch, out bool disabled)) + { + return disabled; + } - // AppContext switch wasn't used. Check the environment variable. - string? envVar = Environment.GetEnvironmentVariable(UsePortInSpnEnvironmentVariable); + // AppContext switch wasn't used. Check the environment variable. + string? envVar = Environment.GetEnvironmentVariable(UsePortInSpnEnvironmentVariable); - if (envVar is not null) - { - return envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase); - } + if (envVar is not null) + { + return envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase); + } - return false; - } + return false; + } + } private static Task InnerSendAsync(HttpRequestMessage request, bool async, bool isProxyAuth, HttpConnectionPool pool, HttpConnection connection, CancellationToken cancellationToken) { diff --git a/src/libraries/System.Net.Http/tests/EnterpriseTests/HttpClientAuthenticationTest.cs b/src/libraries/System.Net.Http/tests/EnterpriseTests/HttpClientAuthenticationTest.cs index 9b93d53fb8fb82..1555fb6c7a21e2 100644 --- a/src/libraries/System.Net.Http/tests/EnterpriseTests/HttpClientAuthenticationTest.cs +++ b/src/libraries/System.Net.Http/tests/EnterpriseTests/HttpClientAuthenticationTest.cs @@ -11,14 +11,19 @@ namespace System.Net.Http.Enterprise.Tests [ConditionalClass(typeof(EnterpriseTestConfiguration), nameof(EnterpriseTestConfiguration.Enabled))] public class HttpClientAuthenticationTest { + private const string AppContextSettingName = "System.Net.Http.UsePortInSpn"; + [Theory] [InlineData(EnterpriseTestConfiguration.NegotiateAuthWebServer, false)] - [InlineData(EnterpriseTestConfiguration.AlternativeService, false)] + [InlineData(EnterpriseTestConfiguration.NegotiateAuthWebServerNotDefaultPort, false)] + [InlineData(EnterpriseTestConfiguration.AlternativeService, false, true)] [InlineData(EnterpriseTestConfiguration.DigestAuthWebServer, true)] [InlineData(EnterpriseTestConfiguration.DigestAuthWebServer, false)] [InlineData(EnterpriseTestConfiguration.NtlmAuthWebServer, true)] - public async Task HttpClient_ValidAuthentication_Success(string url, bool useDomain) + public async Task HttpClient_ValidAuthentication_Success(string url, bool useDomain, bool useAltPort = false) { + // This is safe as we have no parallel tests + AppContext.SetSwitch(AppContextSettingName, useAltPort); using var handler = new HttpClientHandler(); handler.Credentials = useDomain ? EnterpriseTestConfiguration.ValidDomainNetworkCredentials : EnterpriseTestConfiguration.ValidNetworkCredentials; using var client = new HttpClient(handler); From 919cb9094fc603c30d42c165425fbbc29819a36b Mon Sep 17 00:00:00 2001 From: wfurt Date: Tue, 10 Aug 2021 15:17:38 -0700 Subject: [PATCH 3/5] feedback from review --- .../AuthenticationHelper.NtAuth.cs | 30 +++++++++---------- .../HttpClientAuthenticationTest.cs | 24 +++++++++------ .../System.Net.Http.Enterprise.Tests.csproj | 3 +- 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs index b6b6bdc25a06ee..36ee29a48eb1fe 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs @@ -16,26 +16,26 @@ internal static partial class AuthenticationHelper private const string UsePortInSpnCtxSwitch = "System.Net.Http.UsePortInSpn"; private const string UsePortInSpnEnvironmentVariable = "DOTNET_SYSTEM_NET_HTTP_USEPORTINSPN"; - private static bool UsePortInSpn + private static bool UsePortInSpn => s_UsePortInSpn.Value; + private static readonly Lazy s_UsePortInSpn = new Lazy(GetUsePortInSpn); + + private static bool GetUsePortInSpn() { - get + // First check for the AppContext switch, giving it priority over the environment variable. + if (AppContext.TryGetSwitch(UsePortInSpnCtxSwitch, out bool disabled)) { - // First check for the AppContext switch, giving it priority over the environment variable. - if (AppContext.TryGetSwitch(UsePortInSpnCtxSwitch, out bool disabled)) - { - return disabled; - } + return disabled; + } - // AppContext switch wasn't used. Check the environment variable. - string? envVar = Environment.GetEnvironmentVariable(UsePortInSpnEnvironmentVariable); + // AppContext switch wasn't used. Check the environment variable. + string? envVar = Environment.GetEnvironmentVariable(UsePortInSpnEnvironmentVariable); - if (envVar is not null) - { - return envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase); - } + if (envVar is not null) + { + return envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase); + } - return false; - } + return false; } private static Task InnerSendAsync(HttpRequestMessage request, bool async, bool isProxyAuth, HttpConnectionPool pool, HttpConnection connection, CancellationToken cancellationToken) diff --git a/src/libraries/System.Net.Http/tests/EnterpriseTests/HttpClientAuthenticationTest.cs b/src/libraries/System.Net.Http/tests/EnterpriseTests/HttpClientAuthenticationTest.cs index 1555fb6c7a21e2..c2c78d2a4fedd0 100644 --- a/src/libraries/System.Net.Http/tests/EnterpriseTests/HttpClientAuthenticationTest.cs +++ b/src/libraries/System.Net.Http/tests/EnterpriseTests/HttpClientAuthenticationTest.cs @@ -3,7 +3,7 @@ using System.Net.Test.Common; using System.Threading.Tasks; - +using Microsoft.DotNet.RemoteExecutor; using Xunit; namespace System.Net.Http.Enterprise.Tests @@ -20,16 +20,22 @@ public class HttpClientAuthenticationTest [InlineData(EnterpriseTestConfiguration.DigestAuthWebServer, true)] [InlineData(EnterpriseTestConfiguration.DigestAuthWebServer, false)] [InlineData(EnterpriseTestConfiguration.NtlmAuthWebServer, true)] - public async Task HttpClient_ValidAuthentication_Success(string url, bool useDomain, bool useAltPort = false) + public void HttpClient_ValidAuthentication_Success(string url, bool useDomain, bool useAltPort = false) { - // This is safe as we have no parallel tests - AppContext.SetSwitch(AppContextSettingName, useAltPort); - using var handler = new HttpClientHandler(); - handler.Credentials = useDomain ? EnterpriseTestConfiguration.ValidDomainNetworkCredentials : EnterpriseTestConfiguration.ValidNetworkCredentials; - using var client = new HttpClient(handler); + RemoteExecutor.Invoke((url, useAltPort, useDomain) => + { + // This is safe as we have no parallel tests + if (!string.IsNullOrEmpty(useAltPort)) + { + AppContext.SetSwitch(AppContextSettingName, true); + } + using var handler = new HttpClientHandler(); + handler.Credentials = string.IsNullOrEmpty(useDomain) ? EnterpriseTestConfiguration.ValidNetworkCredentials : EnterpriseTestConfiguration.ValidDomainNetworkCredentials; + using var client = new HttpClient(handler); - using HttpResponseMessage response = await client.GetAsync(url); - Assert.Equal(HttpStatusCode.OK, response.StatusCode); + using HttpResponseMessage response = client.GetAsync(url).GetAwaiter().GetResult(); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + }, url, useAltPort ? "true" : "" , useDomain ? "true" : "").Dispose(); } [ActiveIssue("https://github.com/dotnet/runtime/issues/416")] diff --git a/src/libraries/System.Net.Http/tests/EnterpriseTests/System.Net.Http.Enterprise.Tests.csproj b/src/libraries/System.Net.Http/tests/EnterpriseTests/System.Net.Http.Enterprise.Tests.csproj index 5d1c6c09b403ec..4af21d65d63e26 100644 --- a/src/libraries/System.Net.Http/tests/EnterpriseTests/System.Net.Http.Enterprise.Tests.csproj +++ b/src/libraries/System.Net.Http/tests/EnterpriseTests/System.Net.Http.Enterprise.Tests.csproj @@ -1,6 +1,7 @@ $(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser + true @@ -8,4 +9,4 @@ - \ No newline at end of file + From 10d9dd8898d7d8c03b4143a08416df966cef2fd3 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Fri, 13 Aug 2021 18:10:30 -0700 Subject: [PATCH 4/5] Update src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs Co-authored-by: Stephen Toub --- .../AuthenticationHelper.NtAuth.cs | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs index 36ee29a48eb1fe..ef8dbb3340a6c3 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs @@ -16,26 +16,33 @@ internal static partial class AuthenticationHelper private const string UsePortInSpnCtxSwitch = "System.Net.Http.UsePortInSpn"; private const string UsePortInSpnEnvironmentVariable = "DOTNET_SYSTEM_NET_HTTP_USEPORTINSPN"; - private static bool UsePortInSpn => s_UsePortInSpn.Value; - private static readonly Lazy s_UsePortInSpn = new Lazy(GetUsePortInSpn); + private static volatile int s_usePortInSpn = -1; - private static bool GetUsePortInSpn() + private static bool UsePortInSpn { - // First check for the AppContext switch, giving it priority over the environment variable. - if (AppContext.TryGetSwitch(UsePortInSpnCtxSwitch, out bool disabled)) + get { - return disabled; - } - - // AppContext switch wasn't used. Check the environment variable. - string? envVar = Environment.GetEnvironmentVariable(UsePortInSpnEnvironmentVariable); - - if (envVar is not null) - { - return envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase); + int usePortInSpn = s_usePortInSpn; + if (usePortInSpn != -1) + { + return usePortInSpn != 0; + } + + // First check for the AppContext switch, giving it priority over the environment variable. + if (AppContext.TryGetSwitch(UsePortInSpnCtxSwitch, out bool disabled)) + { + s_usePortInSpn = disabled; + } + else + { + // AppContext switch wasn't used. Check the environment variable. + s_usePortInSpn = + Environment.GetEnvironmentVariable(UsePortInSpnEnvironmentVariable) is string envVar && + (envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase); + } + + return s_usePortInSpn != 0; } - - return false; } private static Task InnerSendAsync(HttpRequestMessage request, bool async, bool isProxyAuth, HttpConnectionPool pool, HttpConnection connection, CancellationToken cancellationToken) From 426619af14fd20689340a3c950f5a277ea7dc935 Mon Sep 17 00:00:00 2001 From: wfurt Date: Fri, 13 Aug 2021 20:40:25 -0700 Subject: [PATCH 5/5] fix build --- .../SocketsHttpHandler/AuthenticationHelper.NtAuth.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs index ef8dbb3340a6c3..52edbb5a80cbac 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs @@ -27,20 +27,20 @@ private static bool UsePortInSpn { return usePortInSpn != 0; } - + // First check for the AppContext switch, giving it priority over the environment variable. - if (AppContext.TryGetSwitch(UsePortInSpnCtxSwitch, out bool disabled)) + if (AppContext.TryGetSwitch(UsePortInSpnCtxSwitch, out bool value)) { - s_usePortInSpn = disabled; + s_usePortInSpn = value ? 1 : 0; } else { // AppContext switch wasn't used. Check the environment variable. s_usePortInSpn = Environment.GetEnvironmentVariable(UsePortInSpnEnvironmentVariable) is string envVar && - (envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase); + (envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase)) ? 1 : 0; } - + return s_usePortInSpn != 0; } }