[release/3.1] Client certificate authentication on HTTP/2 in WinHttpHandler

[alnikola]

This is port of dotnet/runtime#33158.

Description
Pre-release WinHTTP's version supports client certificate-based 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.

Customer impact
Without this change, it's not possible to use client certificate-based authentication on HTTP/2 in .Net Framework 4.7.2 and 4.8.

Regression?
No

Packaging review
Change affects System.Net.Http.WinHttpHandler.dll which is distributed in System.Net.Http.WinHttpHandler package.

Risk
Low, covered by functional tests in master branch, but only manually tested for 3.1

[davidsh]

@danmosemsft

[danmoseley]

Thanks @alnikola . Do we have customer reports? Generally, at this point we don't want to service and add risk, without evidence of sufficient customer impact to justify the churn.

@davidsh thoughts? comfortable about risk?

cc @karelz as well

[danmoseley]

If this only affects .NET Framework, is there a way to have zero code churn for .NET Core?

[alnikola]

It affects .Net Core as well because WinHttpHandler implementation is the same for both of the platforms. That also means it will work the same way on .Net Core.

[davidsh]

@davidsh David Shulman FTE thoughts? comfortable about risk?

Yes, I'm comfortable with LOW risk assessment.

Do we have customer reports?

Yes, 2 large customers need .NET Framework HTTP/2 support with TLS client certificates as their endpoints (third-party servers) are removing HTTP/1.1 support and forcing HTTP/2 with TLS client certs for authentication.

[danmoseley]

Sounds good. I"ll take it to tactics. thanks

[danmoseley]

@ericstj @Anipik for packaging review

[danmoseley]

@alnikola could you share why you didn't port the tests from master? (missing dependencies in our test infrastructure?).

normally shipping a fix in servicing that we can only test manually is not as ideal as having a test. when you say tested manually, do you mean you wrote code to use it, or walked through in the debugger, or found some way to run the unit tests locally?

there is particular interest in reducing churn in this branch going forward - so if there's any reasonable step we can take to increase confidence even further that this doesn't break anything, it would be worth taking. if you've already done all the reasonable things, then we're good.

[danmoseley]

setting no-merge for a few days while we complete the validation discussion..

[alnikola]

@danmosemsft I didn't port tests from master to 3.1 because our Common test infastructure changed significantly since 3.1 release and now I cannot easily port any changes I made to WinHttpHandler tests including new tests for this feature (client cert authentication on HTTP/2). To do it would require a huge extra effort.

Manual testing consists of 2 steps:

1. I implemented a test client and validated the client cert authentication against a simple Asp.Net Core application. SslStream got mutually authenticated and everything worked.
2. I asked the customer who requested this feature to validate on their environment. Currently, I'm still waiting for results.

[danmoseley]

@alnikola makes sense. when we get the customer acknowledgement, please feel free to remove no-merge and @Anipik can merge it.

[danmoseley]

@alnikola ping on this one, anything from customer?

[karelz]

@danmosemsft we are pinging them, but no response / ETA yet :(

[danmoseley]

OK. If we don't get an ack in a few days, you/we will have to make a call on whether to slip this one to the next month.

[karelz]

Yep, I view it that the end-to-end validation is blocking this servicing request. So, we will keep slipping, until we have it.

[karelz]

We just got confirmation from the partner team -- they validated the end-to-end scenario works on their side with our private build.
@danmosemsft we can proceed with merge once the branch is opened. Thanks!

[danmoseley]

As far as I know it's open -- @Anipik ?

[karelz]

Updating version to 3.1.5 -- that's where the fix will ship.

The 3.1.4 package was unlisted by @ericstj due to other problems (e.g. failed attempt to disable package build in #42908 + interaction with other changes by @joperezr).

[jensolsson]

I am affected by this issue. Understand that this will ship with .NET Core 3.1.5?
However we are using .NET 4.8 and not .NET Core and hence we rely on the NuGet package for WinHttpHandler.
Is there any plan on which NuGet package version that will receive this fix?

[karelz]

This is fix in WinHttpHandler package, shipped with .NET Core 3.1.5 update - that translates to version 4.7.2 of this package I believe. It contains also .NET Framework targeting bits, so you should be covered.
Note that it requires fairly new OS version - it may not be yet released beyond Insiders builds. I will dig the build version from tests later on.

[jensolsson]

Thanks @karelz if you can find out the version it would be very interesting to know so I can investigate when it may be released.

[karelz]

[UPDATED to avoid confusion -- thanks @alnikola for correcting me!]

Here's the OS version: Version 2004, build 19573+
https://github.com/dotnet/runtime/blob/v5.0.0-preview.7.20364.11/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/ClientCertificateTest.cs#L58

https://github.com/dotnet/runtime/blob/6072e4d3a7a2a1493f514cdf4be75a3d56580e84/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.Windows.cs#L63

[alnikola]

To clarify this feature is supported Windows 10 Version 2004 build 19573 or newer.

[rjalbert]

What about server OS versions? Are they affected too, or is it only Windows 10? And if they are affected, what versions are affected?

[antonfirsov]

@rjalbert Windows Server 2016 and 2019 share the versions and the update schedule with the desktop Windows 10. All you need is to make sure that [Environment]::OSVersion.Version.Build >= 19573.

[GordonEilen]

Any chance this functionality will be available on Windows Server 2012 R2?

[karelz]

@GordonEilen you will need to talk to OS servicing directly (I assume via your MS support contract).
My guess is that it was a new feature, so backporting will need strong business case.

[GordonEilen]

@karelz Thanks!