Skip to content

Commit

Permalink
Avoid throwing Win32Exception from HTTP authentication
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@filipnavara
filipnavara committed Jun 9, 2022
1 parent cca6bb6 commit b06b9b7
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 20 deletions.
8 changes: 7 additions & 1 deletion src/libraries/Common/src/System/Net/NTAuthentication.Common.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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;
Expand Down
69 changes: 56 additions & 13 deletions src/libraries/Common/src/System/Net/NTAuthentication.Managed.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -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);

Expand All @@ -311,6 +324,7 @@ internal void CloseContext()
CreateNtlmNegotiateMessage(_negotiateMessage);

decodedOutgoingBlob = _isSpNego ? CreateSpNegoNegotiateMessage(_negotiateMessage) : _negotiateMessage;
statusCode = SecurityStatusPalContinueNeeded;
}
else
{
Expand All @@ -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;
Expand Down Expand Up @@ -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

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -733,6 +753,7 @@ private static byte[] DeriveKey(ReadOnlySpan<byte> exportedSessionKey, ReadOnlyS

Debug.Assert(payloadOffset == responseBytes.Length);

statusCode = SecurityStatusPalOk;
return responseBytes;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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);
Expand All @@ -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;
}
}
}
35 changes: 35 additions & 0 deletions src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Authentication.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
}
}
6 changes: 3 additions & 3 deletions src/libraries/System.Net.Http/src/Resources/Strings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
12 changes: 9 additions & 3 deletions ...ies/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down

0 comments on commit b06b9b7

Please sign in to comment.