-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable HTTP/2 client cert authentication in WinHttpHandler #33158
Enable HTTP/2 client cert authentication in WinHttpHandler #33158
Conversation
/azp run runtime-libraries outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-libraries outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM.
But as we discussed offline, we need to have some kind of test coverage to run these tests with WinHttpHandler against .NET Framework. You can either make that part of this PR or a separate PR.
…uth for older Windows and net472
HTTP/2 tests enabled for .Net Core only
/azp run runtime-libraries outerloop |
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
/azp list |
/azp run runtime-libraries outerloop |
Commenter does not have sufficient privileges for PR 33158 in repo dotnet/runtime |
/azp run runtime-libraries outerloop |
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments
src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cookies.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/Net/Http/ByteAtATimeContent.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs
Outdated
Show resolved
Hide resolved
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-libraries outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-libraries outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-libraries outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-libraries outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
|
||
sslStream.AuthenticateAsServerAsync(options, CancellationToken.None).Wait(); | ||
#else | ||
sslStream.AuthenticateAsServerAsync(cert, httpOptions.ClientCertificateRequired, httpOptions.SslProtocols, checkCertificateRevocation: false).Wait(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does WinHTTP ignore ALPN?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems - yes.
src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cancellation.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Decompression.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/Net/Http/HttpRetryProtocolTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/Net/Http/HttpProtocolTests.cs
Outdated
Show resolved
Hide resolved
/azp run runtime-libraries outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
Pre-release WinHTTP's version supports client cert authentication over HTTP/2, but the feature must be explicitly opted-in. PR sets WINHTTP_OPTION_ENABLE_HTTP2_PLUS_CLIENT_CERT to TRUE before invoking WinHttpConnect if the request's protocol is HTTP/2 and scheme is HTTPS.