Avoid throwing Win32Exception from HTTP authentication

src/libraries/Common/src/System/Net/NTAuthentication.Common.cs

@@ -163,6 +163,11 @@ internal int MakeSignature(byte[] buffer, int offset, int count, [AllowNull] ref
         }
 
         internal string? GetOutgoingBlob(string? incomingBlob)
+        {
+            return GetOutgoingBlob(incomingBlob, throwOnError: true, out _);
+        }
+
+        internal string? GetOutgoingBlob(string? incomingBlob, bool throwOnError, out SecurityStatusPal statusCode)
         {
             byte[]? decodedIncomingBlob = null;
             if (incomingBlob != null && incomingBlob.Length > 0)
@@ -176,10 +181,11 @@ internal int MakeSignature(byte[] buffer, int offset, int count, [AllowNull] ref
                 // we tried auth previously, now we got a null blob, we're done. this happens
                 // with Kerberos & valid credentials on the domain but no ACLs on the resource
                 _isCompleted = true;
+                statusCode = new SecurityStatusPal(SecurityStatusPalErrorCode.OK);
             }
             else
             {
-                decodedOutgoingBlob = GetOutgoingBlob(decodedIncomingBlob, true);
+                decodedOutgoingBlob = GetOutgoingBlob(decodedIncomingBlob, throwOnError, out statusCode);
             }
 
             string? outgoingBlob = null;

src/libraries/Common/src/System/Net/NTAuthentication.Managed.cs

@@ -52,6 +52,14 @@ internal sealed partial class NTAuthentication
 
         private static readonly byte[] s_workstation = Encoding.Unicode.GetBytes(Environment.MachineName);
 
+        private static SecurityStatusPal SecurityStatusPalOk = new SecurityStatusPal(SecurityStatusPalErrorCode.OK);
+        private static SecurityStatusPal SecurityStatusPalContinueNeeded = new SecurityStatusPal(SecurityStatusPalErrorCode.ContinueNeeded);
+        private static SecurityStatusPal SecurityStatusPalInvalidToken = new SecurityStatusPal(SecurityStatusPalErrorCode.InvalidToken);
+        private static SecurityStatusPal SecurityStatusPalInternalError = new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError);
+        private static SecurityStatusPal SecurityStatusPalPackageNotFound = new SecurityStatusPal(SecurityStatusPalErrorCode.PackageNotFound);
+        private static SecurityStatusPal SecurityStatusPalMessageAltered = new SecurityStatusPal(SecurityStatusPalErrorCode.MessageAltered);
+        private static SecurityStatusPal SecurityStatusPalLogonDenied = new SecurityStatusPal(SecurityStatusPalErrorCode.LogonDenied);
+
         private const Flags s_requiredFlags =
             Flags.NegotiateNtlm2 | Flags.NegotiateNtlm | Flags.NegotiateUnicode | Flags.TargetName |
             Flags.NegotiateVersion | Flags.NegotiateKeyExchange | Flags.Negotiate128 |
@@ -291,7 +299,12 @@ internal void CloseContext()
             IsCompleted = true;
         }
 
-        internal unsafe string? GetOutgoingBlob(string? incomingBlob)
+        internal string? GetOutgoingBlob(string? incomingBlob)
+        {
+            return GetOutgoingBlob(incomingBlob, throwOnError: true, out _);
+        }
+
+        internal unsafe string? GetOutgoingBlob(string? incomingBlob, bool throwOnError, out SecurityStatusPal statusCode)
         {
             Debug.Assert(!IsCompleted);
 
@@ -311,6 +324,7 @@ internal void CloseContext()
                 CreateNtlmNegotiateMessage(_negotiateMessage);
 
                 decodedOutgoingBlob = _isSpNego ? CreateSpNegoNegotiateMessage(_negotiateMessage) : _negotiateMessage;
+                statusCode = SecurityStatusPalContinueNeeded;
             }
             else
             {
@@ -319,9 +333,12 @@ internal void CloseContext()
                 if (_isSpNego)
                 {
                     IsCompleted = true;
+                    decodedOutgoingBlob = ProcessChallenge(decodedIncomingBlob, out statusCode);
+                }
+                else
+                {
+                    decodedOutgoingBlob = ProcessSpNegoChallenge(decodedIncomingBlob, out statusCode);
                 }
-
-                decodedOutgoingBlob = _isSpNego ? ProcessSpNegoChallenge(decodedIncomingBlob) : ProcessChallenge(decodedIncomingBlob);
             }
 
             string? outgoingBlob = null;
@@ -604,7 +621,7 @@ private static byte[] DeriveKey(ReadOnlySpan<byte> exportedSessionKey, ReadOnlyS
         }
 
         // This gets decoded byte blob and returns response in binary form.
-        private unsafe byte[]? ProcessChallenge(byte[] blob)
+        private unsafe byte[]? ProcessChallenge(byte[] blob, out SecurityStatusPal statusCode)
         {
             // TODO: Validate size and offsets
 
@@ -615,6 +632,7 @@ private static byte[] DeriveKey(ReadOnlySpan<byte> exportedSessionKey, ReadOnlyS
             if (challengeMessage.Header.MessageType != MessageType.Challenge ||
                 !NtlmHeader.SequenceEqual(asBytes.Slice(0, NtlmHeader.Length)))
             {
+                statusCode = SecurityStatusPalInvalidToken;
                 return null;
             }
 
@@ -627,6 +645,7 @@ private static byte[] DeriveKey(ReadOnlySpan<byte> exportedSessionKey, ReadOnlyS
             // that is used for MIC.
             if ((flags & s_requiredFlags) != s_requiredFlags)
             {
+                statusCode = SecurityStatusPalInvalidToken;
                 return null;
             }
 
@@ -638,6 +657,7 @@ private static byte[] DeriveKey(ReadOnlySpan<byte> exportedSessionKey, ReadOnlyS
             // Confidentiality is TRUE, then return STATUS_LOGON_FAILURE ([MS-ERREF] section 2.3.1).
             if (!hasNbNames && (flags & (Flags.NegotiateSign | Flags.NegotiateSeal)) != 0)
             {
+                statusCode = SecurityStatusPalInvalidToken;
                 return null;
             }
 
@@ -733,6 +753,7 @@ private static byte[] DeriveKey(ReadOnlySpan<byte> exportedSessionKey, ReadOnlyS
 
             Debug.Assert(payloadOffset == responseBytes.Length);
 
+            statusCode = SecurityStatusPalOk;
             return responseBytes;
         }
 
@@ -834,7 +855,7 @@ private unsafe byte[] CreateSpNegoNegotiateMessage(ReadOnlySpan<byte> ntlmNegoti
             return writer.Encode();
         }
 
-        private unsafe byte[] ProcessSpNegoChallenge(byte[] challenge)
+        private unsafe byte[]? ProcessSpNegoChallenge(byte[] challenge, out SecurityStatusPal statusCode)
         {
             NegState state = NegState.Unknown;
             string? mech = null;
@@ -894,9 +915,10 @@ private unsafe byte[] ProcessSpNegoChallenge(byte[] challenge)
 
                 challengeReader.ThrowIfNotEmpty();
             }
-            catch (AsnContentException e)
+            catch (AsnContentException)
             {
-                throw new Win32Exception(NTE_FAIL, e.Message);
+                statusCode = SecurityStatusPalInvalidToken;
+                return null;
             }
 
             if (blob?.Length > 0)
@@ -905,11 +927,18 @@ private unsafe byte[] ProcessSpNegoChallenge(byte[] challenge)
                 // message with the challenge blob.
                 if (!NtlmOid.Equals(mech))
                 {
-                    throw new Win32Exception(NTE_FAIL, SR.Format(SR.net_nego_mechanism_not_supported, mech));
+                    statusCode = SecurityStatusPalPackageNotFound;
+                    return null;
                 }
 
                 // Process decoded NTLM blob.
-                byte[]? response = ProcessChallenge(blob);
+                byte[]? response = ProcessChallenge(blob, out statusCode);
+
+                if (statusCode.ErrorCode != SecurityStatusPalErrorCode.OK)
+                {
+                    return null;
+                }
+
                 if (response?.Length > 0)
                 {
                     AsnWriter writer = new AsnWriter(AsnEncodingRules.DER);
@@ -930,21 +959,35 @@ private unsafe byte[] ProcessSpNegoChallenge(byte[] challenge)
                         }
                     }
 
+                    statusCode = state == NegState.RequestMic ? SecurityStatusPalContinueNeeded : SecurityStatusPalOk;
                     return writer.Encode();
                 }
             }
 
             if (mechListMIC != null)
             {
-                if (_spnegoMechList == null || state != NegState.AcceptCompleted || !VerifyMIC(_spnegoMechList, mechListMIC))
+                if (_spnegoMechList == null || state != NegState.AcceptCompleted)
                 {
-                    throw new Win32Exception(NTE_FAIL);
+                    statusCode = SecurityStatusPalInternalError;
+                    return null;
+                }
+
+                if (!VerifyMIC(_spnegoMechList, mechListMIC))
+                {
+                    statusCode = SecurityStatusPalMessageAltered;
+                    return null;
                 }
             }
 
             IsCompleted = state == NegState.AcceptCompleted || state == NegState.Reject;
-
-            return Array.Empty<byte>();
+            statusCode = state switch {
+                NegState.AcceptCompleted => SecurityStatusPalOk,
+                NegState.AcceptIncomplete => SecurityStatusPalContinueNeeded,
+                NegState.Reject => SecurityStatusPalLogonDenied,
+                _ => SecurityStatusPalInternalError
+            };
+
+            return null;
         }
     }
 }

src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Authentication.cs

@@ -6,6 +6,7 @@
 using System.Net.Sockets;
 using System.Net.Test.Common;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 
 using Microsoft.DotNet.XUnitExtensions;
@@ -689,5 +690,64 @@ await LoopbackServerFactory.CreateClientAndServerAsync(
                     _output.WriteLine(authHeaderValue);
                });
         }
+
+        [ConditionalFact(nameof(IsNtlmInstalled))]
+        public async Task Credentials_BrokenNtlmFromServer()
+        {
+            if (IsWinHttpHandler && UseVersion >= HttpVersion20.Value)
+            {
+                return;
+            }
+
+            await LoopbackServer.CreateClientAndServerAsync(
+                async uri =>
+                {
+                    using (HttpClientHandler handler = CreateHttpClientHandler())
+                    using (HttpClient client = CreateHttpClient(handler))
+                    {
+                        handler.Credentials = new NetworkCredential("username", "password");
+                        var response = await client.GetAsync(uri);
+                        Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode);
+                    }
+                },
+                async server =>
+                {
+                    var responseHeader = new HttpHeaderData[] { new HttpHeaderData("WWW-Authenticate", "NTLM") };
+                    HttpRequestData requestData = await server.HandleRequestAsync(HttpStatusCode.Unauthorized, responseHeader);
+                    Assert.Equal(0, requestData.GetHeaderValueCount("Authorization"));
+
+                    // Establish a session connection
+                    using var connection = await server.EstablishConnectionAsync();
+                    requestData = await connection.ReadRequestDataAsync();
+                    string authHeaderValue = requestData.GetSingleHeaderValue("Authorization");
+                    Assert.Contains("NTLM", authHeaderValue);
+                    _output.WriteLine(authHeaderValue);
+
+                    // Incorrect NTLMv1 challenge from server (generated by Cyrus HTTP)
+                    responseHeader = new HttpHeaderData[] {
+                        new HttpHeaderData("WWW-Authenticate", "NTLM TlRMTVNTUAACAAAAHAAcADAAAACV/wIAUwCrhitz1vsAAAAAAAAAAAAAAAAAAAAASgAuAEUATQBDAEwASQBFAE4AVAAuAEMATwBNAA=="),
+                        new HttpHeaderData("Connection", "keep-alive")
+                    };
+                    await connection.SendResponseAsync(HttpStatusCode.Unauthorized, responseHeader);
+                    connection.CompleteRequestProcessing();
+
+                    // Wait for the client to close the connection
+                    try
+                    {
+                        CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(1000);
+                        await connection.WaitForCloseAsync(cancellationTokenSource.Token);
+                    }
+                    catch (OperationCanceledException)
+                    {
+                        // On Linux the GSSAPI NTLM provider may try to continue with the authentication, so go along with it
+                        requestData = await connection.ReadRequestDataAsync();
+                        authHeaderValue = requestData.GetSingleHeaderValue("Authorization");
+                        Assert.Contains("NTLM", authHeaderValue);
+                        _output.WriteLine(authHeaderValue);
+                        await connection.SendResponseAsync(HttpStatusCode.Unauthorized);
+                        connection.CompleteRequestProcessing();
+                    }
+                });
+        }
     }
 }

src/libraries/System.Net.Http/src/Resources/Strings.resx

@@ -441,15 +441,15 @@
   <data name="net_http_authconnectionfailure" xml:space="preserve">
     <value>Authentication failed because the connection could not be reused.</value>
   </data>
+  <data name="net_http_authvalidationfailure" xml:space="preserve">
+    <value>Authentication validation failed with error - {0}.</value>
+  </data>
   <data name="net_nego_server_not_supported" xml:space="preserve">
     <value>Server implementation is not supported</value>
   </data>
   <data name="net_nego_protection_level_not_supported" xml:space="preserve">
     <value>Requested protection level is not supported with the GSSAPI implementation currently installed.</value>
   </data>
-  <data name="net_nego_mechanism_not_supported" xml:space="preserve">
-    <value>The security package '{0}' is not supported.</value>
-  </data>
   <data name="net_context_buffer_too_small" xml:space="preserve">
     <value>Insufficient buffer space. Required: {0} Actual: {1}.</value>
   </data>

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs

@@ -172,8 +172,9 @@ private static async Task<HttpResponseMessage> SendWithNtAuthAsync(HttpRequestMe
                         {
                             while (true)
                             {
-                                string? challengeResponse = authContext.GetOutgoingBlob(challengeData);
-                                if (challengeResponse == null)
+                                SecurityStatusPal statusCode;
+                                string? challengeResponse = authContext.GetOutgoingBlob(challengeData, throwOnError: false, out statusCode);
+                                if (statusCode.ErrorCode > SecurityStatusPalErrorCode.TryAgain || challengeResponse == null)
                                 {
                                     // Response indicated denial even after login, so stop processing and return current response.
                                     break;
@@ -195,7 +196,12 @@ private static async Task<HttpResponseMessage> SendWithNtAuthAsync(HttpRequestMe
                                 if (!IsAuthenticationChallenge(response, isProxyAuth))
                                 {
                                     // Tail response for Negoatiate on successful authentication. Validate it before we proceed.
-                                    authContext.GetOutgoingBlob(challengeData);
+                                    authContext.GetOutgoingBlob(challengeData, throwOnError: false, out statusCode);
+                                    if (statusCode.ErrorCode != SecurityStatusPalErrorCode.OK)
+                                    {
+                                        await connection.DrainResponseAsync(response!, cancellationToken).ConfigureAwait(false);
+                                        throw new HttpRequestException(SR.Format(SR.net_http_authvalidationfailure, statusCode.ErrorCode), null, HttpStatusCode.Unauthorized);
+                                    }
                                     break;
                                 }