Skip to content
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

[QUIC] Certificate Validation with OpenSSL on Windows #55544

Closed
wegylexy opened this issue Jul 13, 2021 · 7 comments
Closed

[QUIC] Certificate Validation with OpenSSL on Windows #55544

wegylexy opened this issue Jul 13, 2021 · 7 comments

Comments

@wegylexy
Copy link
Contributor

wegylexy commented Jul 13, 2021

Description

To support QUIC on older versions of Windows, the OpenSSL build of msquic may substitute. However, the raw certificate needs to be converted to PCERT_CONTEXT before loading into X509Certificate2. #51015 breaks this scenario due to loading the certificate in the wrong format.

Moreover, there should be better a way to disable certificate validation altogether and not attempt to load the certificate, than using a lambda that always returns true.

Configuration

Windows with OpenSSL build of msquic.dll, self-signed cert without chain.

Regression?

Skip this line https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicConnection.cs#L368 and return success.

Other information

SChannel on Windows 11 works, OpenSSL on Linux works. QUIC could otherwise be supported on older versions of Windows too.

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Security untriaged New issue has not been triaged by the area owner labels Jul 13, 2021
@ghost
Copy link

ghost commented Jul 13, 2021

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

#51015 breaks the OpenSSL alternative of msquic.dll on Windows where the newer SChannel is unavailable. When the cert validation callback always returns true (intended to bypass validation), .NET still tries to load the cert anyway, but assuming SChannel causes a crash. Skipping the Windows conditional branch results in a TLS alert of bad cert too.

Configuration

Windows with OpenSSL build of msquic.dll, self-signed cert without chain.

Regression?

Bypass certificate loading in HandleEventPeerCertificateReceived() in https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicConnection.cs#L368 and return success.

Other information

SChannel on Windows 11 works, OpenSSL on Linux works. QUIC could otherwise be supported on older versions of Windows too.

Author: wegylexy
Assignees: -
Labels:

area-System.Security, untriaged

Milestone: -

@ghost
Copy link

ghost commented Jul 13, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

#51015 breaks the OpenSSL alternative of msquic.dll on Windows where the newer SChannel is unavailable. When the cert validation callback always returns true (intended to bypass validation), .NET still tries to load the cert anyway, but assuming SChannel causes a crash. Skipping the Windows conditional branch results in a TLS alert of bad cert too.

Configuration

Windows with OpenSSL build of msquic.dll, self-signed cert without chain.

Regression?

Bypass certificate loading in HandleEventPeerCertificateReceived() in https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicConnection.cs#L368 and return success.

Other information

SChannel on Windows 11 works, OpenSSL on Linux works. QUIC could otherwise be supported on older versions of Windows too.

Author: wegylexy
Assignees: -
Labels:

area-System.Net.Quic, untriaged

Milestone: -

@wegylexy
Copy link
Contributor Author

@bartonjs Could be security-related too as the exception is thrown from X509Certificate2Collection and X509Certificate2, given a raw cert from the OpenSSL build of msquic. Do they only work with SChannel on Windows?

@wfurt
Copy link
Member

wfurt commented Jul 13, 2021

I'm not sure I understand the comment @wegylexy. On Windows with channel the Handle is basically raw pointer to memory. When this is called with openssl backend you basically give it pointer to random memory. It is not surprising that it would crash. If you build MsQuic yourself you could fix up the crypto to cover to PCERT_CONTEXT or set it to null.

@wegylexy
Copy link
Contributor Author

@wfurt I see. So either msquic or .NET needs to convert the raw cert to PCERT_CONTEXT on Windows, and .NET QUIC should support OpenSSL build of msquic on older versions of Windows, because at least Windows 8.1 and some LTS servers have no ways to enable TLS 1.3 in SChannel. It's also risky to ask Windows 10 users to change the registry to just to enable QUIC for our .NET 6 apps.

@wfurt
Copy link
Member

wfurt commented Jul 13, 2021

avoiding crash is easy. The real question is how you are going to do real validation and certificate management.
I added flag to MsQuic to specific if the certificates are expected in "portable" e.g. PKCS format or as a native platform pointer. microsoft/msquic#1453 tracks deferred work to support "portable" option on Windows/Schannel. When I look at it back then, Windows has all the needed functions to deal with the formats so can could also submit fix to upstream msquic to convert OpenSSL structures to native PCERT_CONTEXT.
Furher more, microsoft/msquic#1258 added option to do OpenSSL validation with Windows store. That should get you really close.

cc: @nibanks @ThadHouse

@wfurt wfurt removed the untriaged New issue has not been triaged by the area owner label Jul 14, 2021
@wfurt wfurt added this to the Future milestone Jul 14, 2021
@wegylexy
Copy link
Contributor Author

Finally found it!

if (!OperatingSystem.IsWindows())
{
// Use certificate handles on Windows, fall-back to ASN1 otherwise.
flags |= QUIC_CREDENTIAL_FLAGS.USE_PORTABLE_CERTIFICATES;
}

@karelz karelz modified the milestones: Future, 6.0.0 Jul 29, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants