Avoid throwing Win32Exception from HTTP authentication

When server sends malformed NTLM challenge the NT authentication processing
would throw an unexpected Win32Exception from HttpClientHandler.Send[Async]
calls. This aligns the behavior to WinHTTP handler where the Unauthorized
reply with challenge token is returned back to the client.

Similarly, failure to validate the last MIC token in Negotiate scheme could
result in Win32Exception. Handle it by throwing HttpRequestException instead.

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

@@ -689,5 +689,40 @@ await LoopbackServerFactory.CreateClientAndServerAsync(
                     _output.WriteLine(authHeaderValue);
                });
         }
+
+        [ConditionalFact(nameof(IsNtlmInstalled))]
+        public async Task Credentials_BrokenNtlmFromServer()
+        {
+            if (IsWinHttpHandler && UseVersion >= HttpVersion20.Value)
+            {
+                return;
+            }
+
+            await LoopbackServerFactory.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"));
+
+                    // Incorrect NTLMv1 challenge from server (generated by Cyrus HTTP)
+                    responseHeader = new HttpHeaderData[] { new HttpHeaderData("WWW-Authenticate", "NTLM TlRMTVNTUAACAAAAHAAcADAAAACV/wIAUwCrhitz1vsAAAAAAAAAAAAAAAAAAAAASgAuAEUATQBDAEwASQBFAE4AVAAuAEMATwBNAA==") };
+                    requestData = await server.HandleRequestAsync(HttpStatusCode.Unauthorized, responseHeader);
+                    string authHeaderValue = requestData.GetSingleHeaderValue("Authorization");
+                    Assert.Contains("NTLM", authHeaderValue);
+                    _output.WriteLine(authHeaderValue);
+                });
+        }
     }
 }

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