Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit db2a86e

Browse files
stephentoubstephentoub
committedMar 14, 2018
Fix SocketsHttpHandler.PreAuthenticate behavior
Previously SocketsHttpHandler.PreAuthenticate was causing SocketsHttpHandler to always add a basic auth header on every request if a basic credential could be retrieved from the supplied credentials. This is not how PreAuthenticate is supposed to work. Rather, the handler is supposed to track credentials that have previously been used to successfully authenticate, and then on subsequent requests use credentials from that successfully-authenticated cache.
1 parent 03496c5 commit db2a86e

File tree

5 files changed

+270
-82
lines changed

5 files changed

+270
-82
lines changed
 

‎src/System.Net.Http/src/System.Net.Http.csproj

+1-4
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@
126126
<!-- SocketsHttpHandler implementation -->
127127
<ItemGroup Condition="'$(TargetGroup)' == 'netcoreapp'">
128128
<Compile Include="System\Net\Http\SocketsHttpHandler\AuthenticationHelper.cs" />
129-
<Compile Include="System\Net\Http\SocketsHttpHandler\AuthenticationHelper.Basic.cs" />
130129
<Compile Include="System\Net\Http\SocketsHttpHandler\AuthenticationHelper.Digest.cs" />
131130
<Compile Include="System\Net\Http\SocketsHttpHandler\AuthenticationHelper.NtAuth.cs" />
132131
<Compile Include="System\Net\Http\SocketsHttpHandler\ChunkedEncodingReadStream.cs" />
@@ -634,8 +633,6 @@
634633
<Reference Include="System.Security.Cryptography.Encoding" />
635634
<Reference Include="System.Security.Cryptography.Primitives" />
636635
</ItemGroup>
637-
<ItemGroup>
638-
<Folder Include="Common\System\Threading\Tasks\" />
639-
</ItemGroup>
636+
<ItemGroup />
640637
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
641638
</Project>

‎src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.Basic.cs

-35
This file was deleted.

‎src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.cs

+82-38
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System.Diagnostics;
56
using System.Net.Http.Headers;
7+
using System.Text;
68
using System.Threading;
79
using System.Threading.Tasks;
810

@@ -79,25 +81,19 @@ private static bool TryGetValidAuthenticationChallengeForScheme(string scheme, A
7981

8082
private static bool TryGetAuthenticationChallenge(HttpResponseMessage response, bool isProxyAuth, Uri authUri, ICredentials credentials, out AuthenticationChallenge challenge)
8183
{
82-
challenge = default;
83-
8484
if (!IsAuthenticationChallenge(response, isProxyAuth))
8585
{
86+
challenge = default;
8687
return false;
8788
}
8889

89-
HttpHeaderValueCollection<AuthenticationHeaderValue> authenticationHeaderValues = GetResponseAuthenticationHeaderValues(response, isProxyAuth);
90-
9190
// Try to get a valid challenge for the schemes we support, in priority order.
92-
if (!TryGetValidAuthenticationChallengeForScheme(NegotiateScheme, AuthenticationType.Negotiate, authUri, credentials, authenticationHeaderValues, out challenge) &&
93-
!TryGetValidAuthenticationChallengeForScheme(NtlmScheme, AuthenticationType.Ntlm, authUri, credentials, authenticationHeaderValues, out challenge) &&
94-
!TryGetValidAuthenticationChallengeForScheme(DigestScheme, AuthenticationType.Digest, authUri, credentials, authenticationHeaderValues, out challenge) &&
95-
!TryGetValidAuthenticationChallengeForScheme(BasicScheme, AuthenticationType.Basic, authUri, credentials, authenticationHeaderValues, out challenge))
96-
{
97-
return false;
98-
}
99-
100-
return true;
91+
HttpHeaderValueCollection<AuthenticationHeaderValue> authenticationHeaderValues = GetResponseAuthenticationHeaderValues(response, isProxyAuth);
92+
return
93+
TryGetValidAuthenticationChallengeForScheme(NegotiateScheme, AuthenticationType.Negotiate, authUri, credentials, authenticationHeaderValues, out challenge) ||
94+
TryGetValidAuthenticationChallengeForScheme(NtlmScheme, AuthenticationType.Ntlm, authUri, credentials, authenticationHeaderValues, out challenge) ||
95+
TryGetValidAuthenticationChallengeForScheme(DigestScheme, AuthenticationType.Digest, authUri, credentials, authenticationHeaderValues, out challenge) ||
96+
TryGetValidAuthenticationChallengeForScheme(BasicScheme, AuthenticationType.Basic, authUri, credentials, authenticationHeaderValues, out challenge);
10197
}
10298

10399
private static bool TryGetRepeatedChallenge(HttpResponseMessage response, string scheme, bool isProxyAuth, out string challengeData)
@@ -147,7 +143,13 @@ private static void SetRequestAuthenticationHeaderValue(HttpRequestMessage reque
147143

148144
private static void SetBasicAuthToken(HttpRequestMessage request, NetworkCredential credential, bool isProxyAuth)
149145
{
150-
SetRequestAuthenticationHeaderValue(request, new AuthenticationHeaderValue(BasicScheme, GetBasicTokenForCredential(credential)), isProxyAuth);
146+
string authString = !string.IsNullOrEmpty(credential.Domain) ?
147+
credential.Domain + "\\" + credential.UserName + ":" + credential.Password :
148+
credential.UserName + ":" + credential.Password;
149+
150+
string base64AuthString = Convert.ToBase64String(Encoding.UTF8.GetBytes(authString));
151+
152+
SetRequestAuthenticationHeaderValue(request, new AuthenticationHeaderValue(BasicScheme, base64AuthString), isProxyAuth);
151153
}
152154

153155
private static async Task<bool> TrySetDigestAuthToken(HttpRequestMessage request, NetworkCredential credential, DigestResponse digestResponse, bool isProxyAuth)
@@ -174,49 +176,92 @@ private static Task<HttpResponseMessage> InnerSendAsync(HttpRequestMessage reque
174176

175177
private static async Task<HttpResponseMessage> SendWithAuthAsync(HttpRequestMessage request, Uri authUri, ICredentials credentials, bool preAuthenticate, bool isProxyAuth, bool doRequestAuth, HttpConnectionPool pool, CancellationToken cancellationToken)
176178
{
179+
// If preauth is enabled and this isn't proxy auth, try to get a basic credential from the
180+
// preauth credentials cache, and if successful, set an auth header for it onto the request.
181+
// Currently we only support preauth for Basic.
182+
bool performedBasicPreauth = false;
177183
if (preAuthenticate)
178184
{
179-
NetworkCredential credential = credentials.GetCredential(authUri, BasicScheme);
185+
Debug.Assert(pool.PreAuthCredentials != null);
186+
NetworkCredential credential;
187+
lock (pool.PreAuthCredentials) // TODO #28045: Get rid of this lock.
188+
{
189+
// Just look for basic credentials. If in the future we support preauth
190+
// for other schemes, this will need to search in order of precedence.
191+
Debug.Assert(pool.PreAuthCredentials.GetCredential(authUri, NegotiateScheme) == null);
192+
Debug.Assert(pool.PreAuthCredentials.GetCredential(authUri, NtlmScheme) == null);
193+
Debug.Assert(pool.PreAuthCredentials.GetCredential(authUri, DigestScheme) == null);
194+
credential = pool.PreAuthCredentials.GetCredential(authUri, BasicScheme);
195+
}
196+
180197
if (credential != null)
181198
{
182199
SetBasicAuthToken(request, credential, isProxyAuth);
200+
performedBasicPreauth = true;
183201
}
184202
}
185203

186204
HttpResponseMessage response = await InnerSendAsync(request, isProxyAuth, doRequestAuth, pool, cancellationToken).ConfigureAwait(false);
187205

188206
if (TryGetAuthenticationChallenge(response, isProxyAuth, authUri, credentials, out AuthenticationChallenge challenge))
189207
{
190-
if (challenge.AuthenticationType == AuthenticationType.Digest)
208+
switch (challenge.AuthenticationType)
191209
{
192-
var digestResponse = new DigestResponse(challenge.ChallengeData);
193-
if (await TrySetDigestAuthToken(request, challenge.Credential, digestResponse, isProxyAuth).ConfigureAwait(false))
194-
{
195-
response.Dispose();
196-
response = await InnerSendAsync(request, isProxyAuth, doRequestAuth, pool, cancellationToken).ConfigureAwait(false);
197-
198-
// Retry in case of nonce timeout in server.
199-
if (TryGetRepeatedChallenge(response, challenge.SchemeName, isProxyAuth, out string challengeData))
210+
case AuthenticationType.Digest:
211+
var digestResponse = new DigestResponse(challenge.ChallengeData);
212+
if (await TrySetDigestAuthToken(request, challenge.Credential, digestResponse, isProxyAuth).ConfigureAwait(false))
200213
{
201-
digestResponse = new DigestResponse(challengeData);
202-
if (IsServerNonceStale(digestResponse) &&
203-
await TrySetDigestAuthToken(request, challenge.Credential, digestResponse, isProxyAuth).ConfigureAwait(false))
214+
response.Dispose();
215+
response = await InnerSendAsync(request, isProxyAuth, doRequestAuth, pool, cancellationToken).ConfigureAwait(false);
216+
217+
// Retry in case of nonce timeout in server.
218+
if (TryGetRepeatedChallenge(response, challenge.SchemeName, isProxyAuth, out string challengeData))
204219
{
205-
response.Dispose();
206-
response = await InnerSendAsync(request, isProxyAuth, doRequestAuth, pool, cancellationToken).ConfigureAwait(false);
220+
digestResponse = new DigestResponse(challengeData);
221+
if (IsServerNonceStale(digestResponse) &&
222+
await TrySetDigestAuthToken(request, challenge.Credential, digestResponse, isProxyAuth).ConfigureAwait(false))
223+
{
224+
response.Dispose();
225+
response = await InnerSendAsync(request, isProxyAuth, doRequestAuth, pool, cancellationToken).ConfigureAwait(false);
226+
}
207227
}
208228
}
209-
}
210-
}
211-
else if (challenge.AuthenticationType == AuthenticationType.Basic)
212-
{
213-
if (!preAuthenticate)
214-
{
215-
SetBasicAuthToken(request, challenge.Credential, isProxyAuth);
229+
break;
230+
231+
case AuthenticationType.Basic:
232+
if (performedBasicPreauth)
233+
{
234+
break;
235+
}
216236

217237
response.Dispose();
238+
SetBasicAuthToken(request, challenge.Credential, isProxyAuth);
218239
response = await InnerSendAsync(request, isProxyAuth, doRequestAuth, pool, cancellationToken).ConfigureAwait(false);
219-
}
240+
241+
if (preAuthenticate)
242+
{
243+
switch (response.StatusCode)
244+
{
245+
case HttpStatusCode.ProxyAuthenticationRequired:
246+
case HttpStatusCode.Unauthorized:
247+
break;
248+
249+
default:
250+
lock (pool.PreAuthCredentials) // TODO #28045: Get rid of this lock.
251+
{
252+
try
253+
{
254+
pool.PreAuthCredentials.Add(authUri, BasicScheme, challenge.Credential);
255+
}
256+
catch (ArgumentException)
257+
{
258+
// The credential already existed.
259+
}
260+
}
261+
break;
262+
}
263+
}
264+
break;
220265
}
221266
}
222267

@@ -234,4 +279,3 @@ public static Task<HttpResponseMessage> SendWithRequestAuthAsync(HttpRequestMess
234279
}
235280
}
236281
}
237-

‎src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs

+7
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,12 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK
120120
_hostHeaderValueBytes = Encoding.ASCII.GetBytes(hostHeader);
121121
Debug.Assert(Encoding.ASCII.GetString(_hostHeaderValueBytes) == hostHeader);
122122
}
123+
124+
// Set up for PreAuthenticate. Access to this cache is guarded by a lock on the cache itself.
125+
if (_poolManager.Settings._preAuthenticate)
126+
{
127+
PreAuthCredentials = new CredentialCache();
128+
}
123129
}
124130

125131
private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnectionPoolManager poolManager, string sslHostName)
@@ -139,6 +145,7 @@ private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnection
139145
public Uri ProxyUri => _proxyUri;
140146
public ICredentials ProxyCredentials => _poolManager.ProxyCredentials;
141147
public byte[] HostHeaderValueBytes => _hostHeaderValueBytes;
148+
public CredentialCache PreAuthCredentials { get; }
142149

143150
/// <summary>Object used to synchronize access to state in the pool.</summary>
144151
private object SyncObj => _idleConnections;

‎src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Authentication.cs

+180-5
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ public class HttpClientHandler_Authentication_Test : HttpClientTestBase
1717
private const string Password = "testpassword";
1818
private const string Domain = "testdomain";
1919

20-
private NetworkCredential _credentials = new NetworkCredential(Username, Password, Domain);
20+
private static readonly NetworkCredential s_credentials = new NetworkCredential(Username, Password, Domain);
2121

22-
private Func<HttpClientHandler, Uri, HttpStatusCode, NetworkCredential, Task> _createAndValidateRequest = async (handler, url, expectedStatusCode, credentials) =>
22+
private static readonly Func<HttpClientHandler, Uri, HttpStatusCode, NetworkCredential, Task> s_createAndValidateRequest = async (handler, url, expectedStatusCode, credentials) =>
2323
{
2424
handler.Credentials = credentials;
2525

@@ -50,7 +50,7 @@ await LoopbackServer.CreateServerAsync(async (server, url) =>
5050
server.AcceptConnectionSendResponseAndCloseAsync(HttpStatusCode.Unauthorized, serverAuthenticateHeader);
5151

5252
await TestHelper.WhenAllCompletedOrAnyFailedWithTimeout(TestHelper.PassingTestTimeoutMilliseconds,
53-
_createAndValidateRequest(handler, url, result ? HttpStatusCode.OK : HttpStatusCode.Unauthorized, _credentials), serverTask);
53+
s_createAndValidateRequest(handler, url, result ? HttpStatusCode.OK : HttpStatusCode.Unauthorized, s_credentials), serverTask);
5454
}, options);
5555
}
5656

@@ -86,7 +86,7 @@ await LoopbackServer.CreateServerAsync(async (server, url) =>
8686
{
8787
HttpClientHandler handler = CreateHttpClientHandler();
8888
Task serverTask = server.AcceptConnectionPerformAuthenticationAndCloseAsync(authenticateHeader);
89-
await TestHelper.WhenAllCompletedOrAnyFailed(_createAndValidateRequest(handler, url, HttpStatusCode.OK, _credentials), serverTask);
89+
await TestHelper.WhenAllCompletedOrAnyFailed(s_createAndValidateRequest(handler, url, HttpStatusCode.OK, s_credentials), serverTask);
9090
}, options);
9191
}
9292

@@ -100,7 +100,7 @@ await LoopbackServer.CreateServerAsync(async (server, url) =>
100100
{
101101
HttpClientHandler handler = CreateHttpClientHandler();
102102
Task serverTask = server.AcceptConnectionPerformAuthenticationAndCloseAsync(authenticateHeader);
103-
await TestHelper.WhenAllCompletedOrAnyFailed(_createAndValidateRequest(handler, url, HttpStatusCode.Unauthorized, new NetworkCredential("wronguser", "wrongpassword")), serverTask);
103+
await TestHelper.WhenAllCompletedOrAnyFailed(s_createAndValidateRequest(handler, url, HttpStatusCode.Unauthorized, new NetworkCredential("wronguser", "wrongpassword")), serverTask);
104104
}, options);
105105
}
106106

@@ -134,5 +134,180 @@ public static IEnumerable<object[]> Authentication_TestData()
134134
$"Basic realm=\"testrealm\"", false };
135135
}
136136
}
137+
138+
[Theory]
139+
[InlineData(null)]
140+
[InlineData("Basic")]
141+
[InlineData("Digest")]
142+
[InlineData("NTLM")]
143+
[InlineData("Kerberos")]
144+
[InlineData("Negotiate")]
145+
public async Task PreAuthenticate_NoPreviousAuthenticatedRequests_NoCredentialsSent(string credCacheScheme)
146+
{
147+
const int NumRequests = 3;
148+
await LoopbackServer.CreateClientAndServerAsync(async uri =>
149+
{
150+
using (HttpClientHandler handler = CreateHttpClientHandler())
151+
using (var client = new HttpClient(handler))
152+
{
153+
client.DefaultRequestHeaders.ConnectionClose = true; // for simplicity of not needing to know every handler's pooling policy
154+
handler.PreAuthenticate = true;
155+
switch (credCacheScheme)
156+
{
157+
case null:
158+
handler.Credentials = s_credentials;
159+
break;
160+
161+
default:
162+
var cc = new CredentialCache();
163+
cc.Add(uri, credCacheScheme, s_credentials);
164+
handler.Credentials = cc;
165+
break;
166+
}
167+
168+
for (int i = 0; i < NumRequests; i++)
169+
{
170+
Assert.Equal("hello world", await client.GetStringAsync(uri));
171+
}
172+
}
173+
},
174+
async server =>
175+
{
176+
for (int i = 0; i < NumRequests; i++)
177+
{
178+
List<string> headers = await server.AcceptConnectionSendResponseAndCloseAsync(content: "hello world");
179+
Assert.All(headers, header => Assert.DoesNotContain("Authorization", header));
180+
}
181+
});
182+
}
183+
184+
[Theory]
185+
[InlineData(null, "WWW-Authenticate: Basic realm=\"hello\"\r\n")]
186+
[InlineData("Basic", "WWW-Authenticate: Basic realm=\"hello\"\r\n")]
187+
public async Task PreAuthenticate_FirstRequestNoHeaderAndAuthenticates_SecondRequestPreauthenticates(string credCacheScheme, string authResponse)
188+
{
189+
await LoopbackServer.CreateClientAndServerAsync(async uri =>
190+
{
191+
using (HttpClientHandler handler = CreateHttpClientHandler())
192+
using (var client = new HttpClient(handler))
193+
{
194+
client.DefaultRequestHeaders.ConnectionClose = true; // for simplicity of not needing to know every handler's pooling policy
195+
handler.PreAuthenticate = true;
196+
switch (credCacheScheme)
197+
{
198+
case null:
199+
handler.Credentials = s_credentials;
200+
break;
201+
202+
default:
203+
var cc = new CredentialCache();
204+
cc.Add(uri, credCacheScheme, s_credentials);
205+
handler.Credentials = cc;
206+
break;
207+
}
208+
209+
Assert.Equal("hello world", await client.GetStringAsync(uri));
210+
Assert.Equal("hello world", await client.GetStringAsync(uri));
211+
}
212+
},
213+
async server =>
214+
{
215+
List<string> headers = headers = await server.AcceptConnectionSendResponseAndCloseAsync(HttpStatusCode.Unauthorized, authResponse);
216+
Assert.All(headers, header => Assert.DoesNotContain("Authorization", header));
217+
218+
for (int i = 0; i < 2; i++)
219+
{
220+
headers = await server.AcceptConnectionSendResponseAndCloseAsync(content: "hello world");
221+
Assert.Contains(headers, header => header.Contains("Authorization"));
222+
}
223+
});
224+
}
225+
226+
[Fact]
227+
public async Task PreAuthenticate_SuccessfulBasicButThenFails_DoesntLoopInfinitely()
228+
{
229+
await LoopbackServer.CreateClientAndServerAsync(async uri =>
230+
{
231+
using (HttpClientHandler handler = CreateHttpClientHandler())
232+
using (var client = new HttpClient(handler))
233+
{
234+
client.DefaultRequestHeaders.ConnectionClose = true; // for simplicity of not needing to know every handler's pooling policy
235+
handler.PreAuthenticate = true;
236+
handler.Credentials = s_credentials;
237+
238+
// First two requests: initially without auth header, then with
239+
Assert.Equal("hello world", await client.GetStringAsync(uri));
240+
241+
// Attempt preauth, and when that fails, give up.
242+
using (HttpResponseMessage resp = await client.GetAsync(uri))
243+
{
244+
Assert.Equal(HttpStatusCode.Unauthorized, resp.StatusCode);
245+
}
246+
}
247+
},
248+
async server =>
249+
{
250+
// First request, no auth header, challenge Basic
251+
List<string> headers = headers = await server.AcceptConnectionSendResponseAndCloseAsync(HttpStatusCode.Unauthorized, "WWW-Authenticate: Basic realm=\"hello\"\r\n");
252+
Assert.All(headers, header => Assert.DoesNotContain("Authorization", header));
253+
254+
// Second request, contains Basic auth header
255+
headers = await server.AcceptConnectionSendResponseAndCloseAsync(content: "hello world");
256+
Assert.Contains(headers, header => header.Contains("Authorization"));
257+
258+
// Third request, contains Basic auth header but challenges anyway
259+
headers = headers = await server.AcceptConnectionSendResponseAndCloseAsync(HttpStatusCode.Unauthorized, "WWW-Authenticate: Basic realm=\"hello\"\r\n");
260+
Assert.Contains(headers, header => header.Contains("Authorization"));
261+
262+
if (IsNetfxHandler)
263+
{
264+
// For some reason, netfx tries one more time.
265+
headers = headers = await server.AcceptConnectionSendResponseAndCloseAsync(HttpStatusCode.Unauthorized, "WWW-Authenticate: Basic realm=\"hello\"\r\n");
266+
Assert.Contains(headers, header => header.Contains("Authorization"));
267+
}
268+
});
269+
}
270+
271+
[Fact]
272+
public async Task PreAuthenticate_SuccessfulBasic_ThenDigestChallenged()
273+
{
274+
if (IsWinHttpHandler)
275+
{
276+
// WinHttpHandler fails with Unauthorized after the basic preauth fails.
277+
return;
278+
}
279+
280+
await LoopbackServer.CreateClientAndServerAsync(async uri =>
281+
{
282+
using (HttpClientHandler handler = CreateHttpClientHandler())
283+
using (var client = new HttpClient(handler))
284+
{
285+
client.DefaultRequestHeaders.ConnectionClose = true; // for simplicity of not needing to know every handler's pooling policy
286+
handler.PreAuthenticate = true;
287+
handler.Credentials = s_credentials;
288+
289+
Assert.Equal("hello world", await client.GetStringAsync(uri));
290+
Assert.Equal("hello world", await client.GetStringAsync(uri));
291+
}
292+
},
293+
async server =>
294+
{
295+
// First request, no auth header, challenge Basic
296+
List<string> headers = headers = await server.AcceptConnectionSendResponseAndCloseAsync(HttpStatusCode.Unauthorized, "WWW-Authenticate: Basic realm=\"hello\"\r\n");
297+
Assert.All(headers, header => Assert.DoesNotContain("Authorization", header));
298+
299+
// Second request, contains Basic auth header, success
300+
headers = await server.AcceptConnectionSendResponseAndCloseAsync(content: "hello world");
301+
Assert.Contains(headers, header => header.Contains("Authorization"));
302+
303+
// Third request, contains Basic auth header, challenge digest
304+
headers = await server.AcceptConnectionSendResponseAndCloseAsync(HttpStatusCode.Unauthorized, "WWW-Authenticate: Digest realm=\"hello\", nonce=\"testnonce\"\r\n");
305+
Assert.Contains(headers, header => header.Contains("Authorization: Basic"));
306+
307+
// Fourth request, contains Digest auth header, success
308+
headers = await server.AcceptConnectionSendResponseAndCloseAsync(content: "hello world");
309+
Assert.Contains(headers, header => header.Contains("Authorization: Digest"));
310+
});
311+
}
137312
}
138313
}

0 commit comments

Comments
 (0)
This repository has been archived.