WinHttpHandler: Check available credentials when choosing authentication scheme #28105
Conversation
| { | ||
| if (IsCurlHandler && authenticateHeader.Contains("Digest")) | ||
| { | ||
| // TODO: #27113: Fix failing authentication test cases on different httpclienthandlers. |
There was a problem hiding this comment.
Change issue number to #28065. #27113 is closed.
|
The fix LGTM. |
rmkerr
force-pushed
the
WinHttp_Auth_ChecksSchemeAvailability
branch
from
a463cfe to
dfa416b
Compare
|
The new test is consistently failing on CurlHandler. Looking into it now. |
|
I think this test is failing because of the aggressive approach to pre-authentication that Curl takes. Looking at the network traces, It seems like this is the default behavior for Curl. Once authentication has been set up using the option CURLOPT_HTTPAUTH, Curl will immediately start including the authorization header. Since we set up CURLOPT_HTTPAUTH in the I think that the issue with Curl is substantially different from the issue being fixed by this pr, so I will open a separate issue to track it. |
libcurl will send credentials with a request if those credentials have been set with only one auth scheme. So if you set Credentials to a NetworkCredential, then it won't send the credentials with the first request, because those credentials will be tagged with all auth schemes. And if you set Credentials to a CredentialCache that has the same credentials stored for multiple auth schemes, it similarly won't send the credentials on the first request. But if you set Credentials to a CredentialCache that has credentials for just one auth scheme, libcurl may send those with the first request. |
|
Thanks for the info! That makes a lot of sense, since setting Credentials to a CredentialCache with only one auth scheme is exactly what the test here does. Do you think that the behavior difference is something we should look into? Otherwise I can just update the test here. |
I don't know of a good way to influence libcurl to do exactly what other handlers do for PreAuthenticate. Honestly, I think libcurl's behavior makes more sense: if you were given a credential to use and that credential only works with a single auth scheme, why not send it? |
|
That sounds reasonable to me. I'll update the test. |
|
@dotnet-bot test Outerloop Windows x64 Debug Build |
|
Like the rest of the auth tests using the loopback server, this fails on windows nano server. That's something I'd like to look into, but I don't think it's high priority right now. For now I'm going to skip the test in nano server and merge. |
…et#28105) When choosing what authentication scheme to use, WinHttpHandler picks the most secure scheme supported by the server. When the client does not support this scheme it closes the connection. This fix updates the authentication logic to instead pick the most secure scheme offered by the server that is also supported by the client. This change also adds a test of the relevant behavior. Fixes: #27672
|
Hi! You guys have tracked down and fixed a very nasty and subtle bug that didn't allow people to work with some server calls having their code deployed on linux-based machines! Kudos! |
|
The package is updated as part of shipping .NET Core -- version 2.1 is the current .NET Core wave. We do not have plans to update it on different ship cycle. Shipping standalone packages creates huge complexity and cost not worth it (we've tried it in the past). Why do you need the NuGet package without ties on .NET Core 2.1? Do you need to use it in your .NET Core 1.0 / 1.1 / 2.0 application/library? Or potentially in your .NET Framework application/library? |
|
We are developing a netcore 2.0 application that is hosted in linux amazon docker image as a self-contained application. The server we are calling from our code supports Negotiate and NTLM, and since this is linux, Negotiate is not supported, so our code provides Credential Cache with only NTLM scheme specified. |
This particular bug fix is only in WinHttpHandler which is used only on Windows. Your scenario is for a Linux client. So, perhaps you are describing a different bug? |
|
Well, the behavior is exactly the same... we have two server calls, one server supports NTLM only and it returns 401 with WWW-Authenticate: NTLM, and everything works with this call. Second server along with NTLM supports Negotiate, and this server call fails with Unauthorized eventually. What will be the component we want to look at? |
|
@Lakmus85 what OS is your client on? Linux or Windows? |
|
Sorry, I probably described it not clear enough |
|
And I believe that current Preview 1 of 2.1 doesn't include this commit, is that correct? |
|
Just to be super-clear: You say that your client application is Linux app, right? Daily builds are fairly stable - see link on our main page on how to use them. It would be best to take the discussion into separate issue. There is no evidence at this point it is related to this PR. |
|
One last question, which package will contain WinHttpAuthHelper class? It's neither in the System.Net.Http nor in System.Net.Http.WinHttpHandler |
|
That's internal class - see the sources. |
|
This might be a stupid question. But how can I test this new version inside my UWP app? I have updated my .NET Core version to preview 2, but doesn't seem to work. Still getting the 401. I want to use the |
WinHttpHandler is not available for a UWP app. The binary implementation doesn't get used for UWP apps. And .NET Core 2.1 Preview 2 doesn't have any new fixes for UWPs. Also, UWP apps use different rules for when DefaultCredentials are used and transmitted to server. Your UWP apps must have the EnterpriseAuthentication capability. And the server endpoint must be considered to be in the "Intranet" zone. |
This was a regression from .NET Core 2.0 due to PR #28105. Proxy authentication using system default proxy settings that involve a PAC file (either autodiscovery or explicit PAC file) cause a NullReferenceException in WinHttpHandler in the CheckResponseForAuthentication() method. This problem is only discovered when using an authenticating proxy server (any auth scheme) that is discovered using PAC files. This is considered the "system default" proxy. When this occurs, the handler's Proxy property is null and dereferencing it caused the exception. Due to the problems described in #6997, the uri of the proxy can't be determined yet since it is only known to WinHTTP. Fixing #6997 is complicated and impacts performance. However, in most cases, as long as the credentials are a NetworkCredential object, knowing the uri of the proxy is needed. I tested this manually using the steps I described in #30191. I did not add any tests to this PR since they can't be run in CI. However, I am working on a task that will eventually add Enterprise-Scenario testing like this (PAC files, authenticating proxies, etc.) to our systems. Fixes #30191 Contributes to #6997
This was a regression from .NET Core 2.0 due to PR #28105. Proxy authentication using system default proxy settings that involve a PAC file (either autodiscovery or explicit PAC file) cause a NullReferenceException in WinHttpHandler in the CheckResponseForAuthentication() method. This problem is only discovered when using an authenticating proxy server (any auth scheme) that is discovered using PAC files. This is considered the "system default" proxy. When this occurs, the handler's Proxy property is null and dereferencing it caused the exception. Due to the problems described in #6997, the uri of the proxy can't be determined yet since it is only known to WinHTTP. Fixing #6997 is complicated and impacts performance. However, in most cases, as long as the credentials are a NetworkCredential object, knowing the uri of the proxy is needed. I tested this manually using the steps I described in #30191. I did not add any tests to this PR since they can't be run in CI. However, I am working on a task that will eventually add Enterprise-Scenario testing like this (PAC files, authenticating proxies, etc.) to our systems. Fixes #30191 Contributes to #6997
…et/corefx#28105) When choosing what authentication scheme to use, WinHttpHandler picks the most secure scheme supported by the server. When the client does not support this scheme it closes the connection. This fix updates the authentication logic to instead pick the most secure scheme offered by the server that is also supported by the client. This change also adds a test of the relevant behavior. Fixes: dotnet/corefx#27672 Commit migrated from dotnet/corefx@6d031a0
…0196) This was a regression from .NET Core 2.0 due to PR dotnet/corefx#28105. Proxy authentication using system default proxy settings that involve a PAC file (either autodiscovery or explicit PAC file) cause a NullReferenceException in WinHttpHandler in the CheckResponseForAuthentication() method. This problem is only discovered when using an authenticating proxy server (any auth scheme) that is discovered using PAC files. This is considered the "system default" proxy. When this occurs, the handler's Proxy property is null and dereferencing it caused the exception. Due to the problems described in dotnet/corefx#6997, the uri of the proxy can't be determined yet since it is only known to WinHTTP. Fixing dotnet/corefx#6997 is complicated and impacts performance. However, in most cases, as long as the credentials are a NetworkCredential object, knowing the uri of the proxy is needed. I tested this manually using the steps I described in dotnet/corefx#30191. I did not add any tests to this PR since they can't be run in CI. However, I am working on a task that will eventually add Enterprise-Scenario testing like this (PAC files, authenticating proxies, etc.) to our systems. Fixes dotnet/corefx#30191 Contributes to dotnet/corefx#6997 Commit migrated from dotnet/corefx@69c2e20
When choosing what authentication scheme to use, WinHttpHandler picks the most secure scheme supported by the server. When the client does not support this scheme it closes the connection. We need to instead pick the most secure scheme offered by the server and supported by the client.
The incorrect behavior occurs under the following conditions:
In the conditions described in the original report, the user provides NTLM credentials, but the server supports both NTLM and Negotiate (which is considered more secure). WinHttpHandler erroneously chooses to attempt authentication with Negotiate. When we later detect that there are no credentials in the cache that support Negotiate, we close the connection.
The fix is a simple change to
WinHttpAuthHelper, and an additional test. There is an open issue tracking auth problems with Windows Nano, so depending on the results in CI I may disable the test there.Fixes: #27672