search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.98k stars 4.66k forks source link

HTTP2: SocketsHttpHandler doesn't downgrade to HTTP/1.1 during Windows authentication #1582

Open davidsh opened 5 years ago

davidsh commented 5 years ago

When SocketsHttpHandler connects to a server using HTTP/2, it is unable to downgrade to HTTP/1.1 when the server requests Windows authentication (Negotiate or NTLM). The net result is that 401 is returned to the client without even trying to reconnect (with HTTP/1.1) and send credentials.

// Repro will be posted later
geoffkizer commented 5 years ago

See also dotnet/corefx#31304

karelz commented 5 years ago

@wfurt can you please chat with @davidsh about design / ideas here?

karelz commented 5 years ago

Given we scoped HTTP/2 to gRPC scenarios, this can wait for post-3.0.

davidsh commented 5 years ago

Given we scoped HTTP/2 to gRPC scenarios, this can wait for post-3.0.

Waiting for post-3.0 is ok as long as we don't opt-in by default for HTTP/2.0.

Tratcher commented 5 years ago

I noticed this while doing some auth testing. IE, Chrome, and Edge all have this fallback behavior. Also, the downgrade isn't just for the auth handshake, all subsequent requests are made on the HTTP/1.1 connection so they share the cached auth context.

WinHttpHandler is a bit different. The first HTTP/2 request challenges, falls back to HTTP/1.1, authenticates, and completes successfully. However, the next request is still made over HTTP/2, the client attempts the fallback again, and then it re-authenticates on the already authenticated connection. That could be a major performance hole to fall into.

halter73 commented 1 year ago

This came up again. This time from a SignalR client user. See https://github.com/dotnet/aspnetcore/issues/45371. Prior to .NET 7, the SignalR client forced HTTP/1.1 but we started allowing HTTP/2 in .NET 7. We're considering backporting a fix for SignalR to force HTTP/1.1 if the customer sets HttpConnectionOptions.UseDefaultCredentials to true.

@karelz Even though we have a fix for SignalR, we can't help but think HttpClient should have avoided this for us. Do you think we could do something similar in .NET 8 at the SocketsHttpHandler level when UseDefaultCredentials is set? Of course, the handler could be smarter than SignalR and wait for a challenge before downgrading,

karelz commented 1 year ago

@wfurt @ManickaP @CarnaViire any thoughts on this? (marking for re-triage given new scenario being impacted)

wfurt commented 1 year ago

There are (at least) two distance parts IMHO. 1) get parity with WinHttp - The existing retry logic will be problematic as this needs new connection (and possibly check for version policy). This can probably be done by refactoring the connection pool logic. 2) Browsers do have global cache and therefore they can remember state. HttpClientHandler is stateless and each request is processed independently. To fix that, we would need whole new concept in SocketsHttpHandler. That may be useful for other reasons but it would be fundamental change IMHO.

While we should do something I would keep it in Future for now given complexity and no up votes e.g. no real customer pain. The workaround is somewhat easy -> just set version to 1.1 for problematic sites.

karelz commented 1 year ago

Agreed with @wfurt. It seems it is still rather rare with a complex solution, so leaving it in Future sounds like the right option now. @halter73 let us know if you disagree.