Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

SocketsHttpHandler: if server sends invalid NT auth challenge, don't continue processing #28704

Merged
merged 1 commit into from
Apr 2, 2018

Conversation

geoffkizer
Copy link

Fixes #28648

@Tratcher @stephentoub @davidsh @dotnet/ncl

@geoffkizer
Copy link
Author

I tested locally using the HttpListener based loopback tests.

@geoffkizer
Copy link
Author

@dotnet-bot test outerloop Linux x64 debug build
@dotnet-bot test outerloop OSX x64 debug build
@dotnet-bot test outerloop Windows x64 debug build

@davidsh
Copy link
Contributor

davidsh commented Apr 1, 2018

I tested locally using the HttpListener based loopback tests.

Does that mean we have an existing test that failed and now passes with your PR?

Or does it mean that you used those HttpListener tests and simulated some failure in the auth to demonstrate that the PR fixes the original issue reported? Is it then possible to add a new test to validate this fix?

@geoffkizer
Copy link
Author

Does that mean we have an existing test that failed and now passes with your PR?
Or does it mean that you used those HttpListener tests and simulated some failure in the auth to demonstrate that the PR fixes the original issue reported? Is it then possible to add a new test to validate this fix?

No, I just mean I ran the local HttpListener tests to validate that I didn't break them.

I used the repro code from the linked issue to repro this and confirm the fix. Per comments there, an HttpListener repro is not possible, so I didn't try.

@stephentoub stephentoub merged commit 660dc94 into dotnet:master Apr 2, 2018
@@ -62,6 +62,11 @@ private static bool ProxySupportsConnectionAuth(HttpResponseMessage response)
while (true)
{
string challengeResponse = authContext.GetOutgoingBlob(challengeData);
if (challengeResponse == null)
{
// Server sent something invalid, so stop processing and return current response.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The server's response wasn't invalid. It indicated denial even after login. What was invalid was the following request with no data blob, which this new condition handles.

@karelz karelz added this to the 2.1.0 milestone Apr 2, 2018
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants