diff --git a/src/libraries/Common/src/System/Net/NTAuthentication.Common.cs b/src/libraries/Common/src/System/Net/NTAuthentication.Common.cs index 2b5047e108042..7d733b9f4912d 100644 --- a/src/libraries/Common/src/System/Net/NTAuthentication.Common.cs +++ b/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; diff --git a/src/libraries/Common/src/System/Net/NTAuthentication.Managed.cs b/src/libraries/Common/src/System/Net/NTAuthentication.Managed.cs index 335550d9f9c4f..9a6086d869192 100644 --- a/src/libraries/Common/src/System/Net/NTAuthentication.Managed.cs +++ b/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 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 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 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 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 exportedSessionKey, ReadOnlyS Debug.Assert(payloadOffset == responseBytes.Length); + statusCode = SecurityStatusPalOk; return responseBytes; } @@ -834,7 +855,7 @@ private unsafe byte[] CreateSpNegoNegotiateMessage(ReadOnlySpan 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(); + statusCode = state switch { + NegState.AcceptCompleted => SecurityStatusPalOk, + NegState.AcceptIncomplete => SecurityStatusPalContinueNeeded, + NegState.Reject => SecurityStatusPalLogonDenied, + _ => SecurityStatusPalInternalError + }; + + return null; } } } diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Authentication.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Authentication.cs index 7d0a476dd1a95..4c71f0cf54082 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Authentication.cs +++ b/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); + }); + } } } diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index e575e8ada9f7f..d5ff73663f820 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -441,15 +441,15 @@ Authentication failed because the connection could not be reused. + + Authentication validation failed with error - {0}. + Server implementation is not supported Requested protection level is not supported with the GSSAPI implementation currently installed. - - The security package '{0}' is not supported. - Insufficient buffer space. Required: {0} Actual: {1}. 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 58155ea59b6ca..d6b178c144c32 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 @@ -172,8 +172,9 @@ private static async Task 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 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; }