Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC - #117472

Merged
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match
Jul 11, 2025
Merged

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC#117472
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match

Conversation

@vcsjones

Copy link
Copy Markdown
Member

Over at #117465, I am trying to get rid of S.S.C.Native.OpenSsl on macOS, but System.Net.Quic current uses parts of it for matching a certificate to a hostname. This makes sense since Apple's native APIs don't offer a similar API. But good news, we have an entirely managed implementation we can use on macOS.

This pull request replaces the use of the native PAL for hostname matching with our managed implementation, X509Certificate2.MatchesHostname, on macOS.

@vcsjonesvcsjones self-assigned this Jul 9, 2025
CopilotAI review requested due to automatic review settings July 9, 2025 18:54

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the dependency on System.Security.Cryptography.Native.OpenSsl for hostname matching in the QUIC library on macOS by replacing native OpenSSL certificate hostname validation with the managed X509Certificate2.MatchesHostname implementation.

  • Restructures project file to exclude OpenSSL cryptography interop files from macOS builds
  • Replaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname API
  • Simplifies certificate validation logic by removing unsafe native code handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
System.Net.Quic.csprojReorganizes ItemGroup conditions to exclude OpenSSL cryptography interop files from macOS builds while maintaining them for Linux/FreeBSD
CertificateValidation.OSX.csReplaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname implementation

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@vcsjones

Copy link
Copy Markdown
MemberAuthor

One of the tests is failing with the hostname "\u017C\u00F3\u0142\u0107 g\u0119\u015Bl\u0105 ja\u017A\u0144. \u7EA2\u70E7. \u7167\u308A\u713C\u304D". The presence of spaces causes Uri.CheckHostName to return Unknown instead of Dns. That in turn causes X509Certificate2 to reject it.

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

It should probably also be said: using libmsquic on macOS with .NET on Apple Silicon machines is very difficult. I see the tests failing on x64 macOS because libmsquic is available, but the hardened runtime on Apple Silicon makes it very difficult to enable.

@rzikm

Copy link
Copy Markdown
Member

Does this change mean that we can get rid of

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContextsecurityContext,
X509Chainchain,
X509Certificate2?remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
SslPolicyErrorserrors=SslPolicyErrors.None;
if(remoteCertificate==null)
{
errors|=SslPolicyErrors.RemoteCertificateNotAvailable;
}
else
{
if(!chain.Build(remoteCertificate))
{
errors|=SslPolicyErrors.RemoteCertificateChainErrors;
}
if(!isServer&&checkCertName)
{
SafeDeleteSslContextsslContext=(SafeDeleteSslContext)securityContext;
if(!Interop.AppleCrypto.SslCheckHostnameMatch(sslContext.SslContext,hostName!,remoteCertificate.NotBefore,outintosStatus))
{
errors|=SslPolicyErrors.RemoteCertificateNameMismatch;
if(NetEventSource.Log.IsEnabled())
NetEventSource.Error(sslContext,$"Cert name validation for '{hostName}' failed with status '{osStatus}'");
}
}
}
returnerrors;
}

And do the same thing as we do on Unix?

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContext?_/*securityContext*/,
X509Chainchain,
X509Certificate2remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
returnCertificateValidation.BuildChainAndVerifyProperties(chain,remoteCertificate,checkCertName,isServer,hostName);
}

I looked at the code recently in the TLS 1.3 support PR and it seems to me that Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work (basically rebuilding the chain again?), so this change could possibly bring perf improvement.

@rzikm

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

@ManickaP
ManickaP requested review from rzikm and wfurtJuly 10, 2025 06:46
@krwq

krwq commented Jul 10, 2025

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

I don't remember - that was 7 years ago... but I think per this comment: dotnet/corefx#28278 (comment)

all ASCII are legal and UTF-8 used to be legal at one point.

Looking at RFC 4366 3.1:

"HostName" contains the fully qualified DNS hostname of the server,
as understood by the client. The hostname is represented as a byte
string using UTF-8 encoding [[UTF8](https://datatracker.ietf.org/doc/html/rfc4366#ref-UTF8)], without a trailing dot.
If the hostname labels contain only US-ASCII characters, then the
client MUST ensure that labels are separated only by the byte 0x2E,
representing the dot character U+002E (requirement 1 in Section 3.1
of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)] notwithstanding). If the server needs to match the
HostName against names that contain non-US-ASCII characters, it MUST
perform the conversion operation described in Section 4 of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)],
treating the HostName as a "query string" (i.e., the AllowUnassigned
flag MUST be set). Note that IDNA allows labels to be separated by
any of the Unicode characters U+002E, U+3002, U+FF0E, and U+FF61;
therefore, servers MUST accept any of these characters as a label
separator. If the server only needs to match the HostName against
names containing exclusively ASCII characters, it MUST compare ASCII
names case-insensitively.

"non US-ASCII" characters are a bit of a moot term for things like space so let's look at the IDNA ToASCII definition...

IDNA is defined in RFC3490 4.1:

(with my comments)

 1. If the sequence contains any code points outside the ASCII range
(0..7F) then proceed to step 2, otherwise skip to step 3.
(we skip 2 for space)
3. If the UseSTD3ASCIIRules flag is set, then perform these checks:
(a) Verify the absence of non-LDH ASCII code points; that is, the
absence of 0..2C, 2E..2F, 3A..40, 5B..60, and 7B..7F.
(b) Verify the absence of leading and trailing hyphen-minus; that
is, the absence of U+002D at the beginning and end of the
sequence.
(only if UseSTD3ASCIIRules is defined we consider space (20) as invalid)

There is also RFC6066 Appendix A:

 The Server Name extension now specifies only ASCII representation,
eliminating UTF-8.

so IMO all ASCII (0x00-0x7F) are legal for decoding given it's not guaranteed that UseSTD3ASCIIRules is used for encoding. But in theory we could block them with some sort of "strict validation" setting given it's possible to encode them.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

I think we are concerned more with how hostnames match against certificates, not what is permitted in the SNI extension.

RFC 1035 does not permit spaces in DNS labels. https://datatracker.ietf.org/doc/html/rfc1035#section-2.3.1

It also doesn't permit underscores, and we know OpenSSL enforces that.

Does this change mean that we can get rid of

Maybe, assuming we can get this change though :-).

Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work

Yeah Ideally we could get rid of the AppleCrypto hostname check... they don't expose a straight forward API for "Is this certificate okay for this host". The only way we could get something to work was to just build a whole chain and see if the chain had a host mismatch name error.

We should keep using the native hostname checks for OpenSSL and Windows though.

@wfurt

Copy link
Copy Markdown
Member

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

@vcsjones

Copy link
Copy Markdown
MemberAuthor

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

OpenSSL is going to do it itself in other places, so it should be consistent with itself. Apple is already using two implementations, Security.framework and OpenSSL, so we are just changing it to a... different... two implementations.

@rzikmrzikm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@krwqkrwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I already mentioned offline, it should be ok to break this until we see someone actually demonstrating space is needed for their scenario (which sounds very unlikely even if technically allowed). My recommendation would be to only allow same characters UseSTD3ASCIIRules talks about but I'm ok with this as is.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

Quic failures are unrelated and should be addressed by #117495.

@vcsjones
vcsjones merged commit d9e8e3f into dotnet:mainJul 11, 2025
@vcsjones
vcsjones deleted the macos-quic-no-openssl-hostname-match branch July 11, 2025 13:47
@vcsjonesvcsjones added this to the 10.0.0 milestone Jul 24, 2025
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 23, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@vcsjones@rzikm@krwq@wfurt
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC - #117472

Merged
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match
Jul 11, 2025
Merged

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC#117472
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match

Conversation

@vcsjones

Copy link
Copy Markdown
Member

Over at #117465, I am trying to get rid of S.S.C.Native.OpenSsl on macOS, but System.Net.Quic current uses parts of it for matching a certificate to a hostname. This makes sense since Apple's native APIs don't offer a similar API. But good news, we have an entirely managed implementation we can use on macOS.

This pull request replaces the use of the native PAL for hostname matching with our managed implementation, X509Certificate2.MatchesHostname, on macOS.

@vcsjonesvcsjones self-assigned this Jul 9, 2025
CopilotAI review requested due to automatic review settings July 9, 2025 18:54

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the dependency on System.Security.Cryptography.Native.OpenSsl for hostname matching in the QUIC library on macOS by replacing native OpenSSL certificate hostname validation with the managed X509Certificate2.MatchesHostname implementation.

  • Restructures project file to exclude OpenSSL cryptography interop files from macOS builds
  • Replaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname API
  • Simplifies certificate validation logic by removing unsafe native code handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
System.Net.Quic.csprojReorganizes ItemGroup conditions to exclude OpenSSL cryptography interop files from macOS builds while maintaining them for Linux/FreeBSD
CertificateValidation.OSX.csReplaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname implementation

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@vcsjones

Copy link
Copy Markdown
MemberAuthor

One of the tests is failing with the hostname "\u017C\u00F3\u0142\u0107 g\u0119\u015Bl\u0105 ja\u017A\u0144. \u7EA2\u70E7. \u7167\u308A\u713C\u304D". The presence of spaces causes Uri.CheckHostName to return Unknown instead of Dns. That in turn causes X509Certificate2 to reject it.

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

It should probably also be said: using libmsquic on macOS with .NET on Apple Silicon machines is very difficult. I see the tests failing on x64 macOS because libmsquic is available, but the hardened runtime on Apple Silicon makes it very difficult to enable.

@rzikm

Copy link
Copy Markdown
Member

Does this change mean that we can get rid of

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContextsecurityContext,
X509Chainchain,
X509Certificate2?remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
SslPolicyErrorserrors=SslPolicyErrors.None;
if(remoteCertificate==null)
{
errors|=SslPolicyErrors.RemoteCertificateNotAvailable;
}
else
{
if(!chain.Build(remoteCertificate))
{
errors|=SslPolicyErrors.RemoteCertificateChainErrors;
}
if(!isServer&&checkCertName)
{
SafeDeleteSslContextsslContext=(SafeDeleteSslContext)securityContext;
if(!Interop.AppleCrypto.SslCheckHostnameMatch(sslContext.SslContext,hostName!,remoteCertificate.NotBefore,outintosStatus))
{
errors|=SslPolicyErrors.RemoteCertificateNameMismatch;
if(NetEventSource.Log.IsEnabled())
NetEventSource.Error(sslContext,$"Cert name validation for '{hostName}' failed with status '{osStatus}'");
}
}
}
returnerrors;
}

And do the same thing as we do on Unix?

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContext?_/*securityContext*/,
X509Chainchain,
X509Certificate2remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
returnCertificateValidation.BuildChainAndVerifyProperties(chain,remoteCertificate,checkCertName,isServer,hostName);
}

I looked at the code recently in the TLS 1.3 support PR and it seems to me that Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work (basically rebuilding the chain again?), so this change could possibly bring perf improvement.

@rzikm

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

@ManickaP
ManickaP requested review from rzikm and wfurtJuly 10, 2025 06:46
@krwq

krwq commented Jul 10, 2025

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

I don't remember - that was 7 years ago... but I think per this comment: dotnet/corefx#28278 (comment)

all ASCII are legal and UTF-8 used to be legal at one point.

Looking at RFC 4366 3.1:

"HostName" contains the fully qualified DNS hostname of the server,
as understood by the client. The hostname is represented as a byte
string using UTF-8 encoding [[UTF8](https://datatracker.ietf.org/doc/html/rfc4366#ref-UTF8)], without a trailing dot.
If the hostname labels contain only US-ASCII characters, then the
client MUST ensure that labels are separated only by the byte 0x2E,
representing the dot character U+002E (requirement 1 in Section 3.1
of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)] notwithstanding). If the server needs to match the
HostName against names that contain non-US-ASCII characters, it MUST
perform the conversion operation described in Section 4 of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)],
treating the HostName as a "query string" (i.e., the AllowUnassigned
flag MUST be set). Note that IDNA allows labels to be separated by
any of the Unicode characters U+002E, U+3002, U+FF0E, and U+FF61;
therefore, servers MUST accept any of these characters as a label
separator. If the server only needs to match the HostName against
names containing exclusively ASCII characters, it MUST compare ASCII
names case-insensitively.

"non US-ASCII" characters are a bit of a moot term for things like space so let's look at the IDNA ToASCII definition...

IDNA is defined in RFC3490 4.1:

(with my comments)

 1. If the sequence contains any code points outside the ASCII range
(0..7F) then proceed to step 2, otherwise skip to step 3.
(we skip 2 for space)
3. If the UseSTD3ASCIIRules flag is set, then perform these checks:
(a) Verify the absence of non-LDH ASCII code points; that is, the
absence of 0..2C, 2E..2F, 3A..40, 5B..60, and 7B..7F.
(b) Verify the absence of leading and trailing hyphen-minus; that
is, the absence of U+002D at the beginning and end of the
sequence.
(only if UseSTD3ASCIIRules is defined we consider space (20) as invalid)

There is also RFC6066 Appendix A:

 The Server Name extension now specifies only ASCII representation,
eliminating UTF-8.

so IMO all ASCII (0x00-0x7F) are legal for decoding given it's not guaranteed that UseSTD3ASCIIRules is used for encoding. But in theory we could block them with some sort of "strict validation" setting given it's possible to encode them.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

I think we are concerned more with how hostnames match against certificates, not what is permitted in the SNI extension.

RFC 1035 does not permit spaces in DNS labels. https://datatracker.ietf.org/doc/html/rfc1035#section-2.3.1

It also doesn't permit underscores, and we know OpenSSL enforces that.

Does this change mean that we can get rid of

Maybe, assuming we can get this change though :-).

Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work

Yeah Ideally we could get rid of the AppleCrypto hostname check... they don't expose a straight forward API for "Is this certificate okay for this host". The only way we could get something to work was to just build a whole chain and see if the chain had a host mismatch name error.

We should keep using the native hostname checks for OpenSSL and Windows though.

@wfurt

Copy link
Copy Markdown
Member

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

@vcsjones

Copy link
Copy Markdown
MemberAuthor

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

OpenSSL is going to do it itself in other places, so it should be consistent with itself. Apple is already using two implementations, Security.framework and OpenSSL, so we are just changing it to a... different... two implementations.

@rzikmrzikm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@krwqkrwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I already mentioned offline, it should be ok to break this until we see someone actually demonstrating space is needed for their scenario (which sounds very unlikely even if technically allowed). My recommendation would be to only allow same characters UseSTD3ASCIIRules talks about but I'm ok with this as is.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

Quic failures are unrelated and should be addressed by #117495.

@vcsjones
vcsjones merged commit d9e8e3f into dotnet:mainJul 11, 2025
@vcsjones
vcsjones deleted the macos-quic-no-openssl-hostname-match branch July 11, 2025 13:47
@vcsjonesvcsjones added this to the 10.0.0 milestone Jul 24, 2025
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 23, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@vcsjones@rzikm@krwq@wfurt
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC - #117472

Merged
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match
Jul 11, 2025
Merged

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC#117472
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match

Conversation

@vcsjones

Copy link
Copy Markdown
Member

Over at #117465, I am trying to get rid of S.S.C.Native.OpenSsl on macOS, but System.Net.Quic current uses parts of it for matching a certificate to a hostname. This makes sense since Apple's native APIs don't offer a similar API. But good news, we have an entirely managed implementation we can use on macOS.

This pull request replaces the use of the native PAL for hostname matching with our managed implementation, X509Certificate2.MatchesHostname, on macOS.

@vcsjonesvcsjones self-assigned this Jul 9, 2025
CopilotAI review requested due to automatic review settings July 9, 2025 18:54

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the dependency on System.Security.Cryptography.Native.OpenSsl for hostname matching in the QUIC library on macOS by replacing native OpenSSL certificate hostname validation with the managed X509Certificate2.MatchesHostname implementation.

  • Restructures project file to exclude OpenSSL cryptography interop files from macOS builds
  • Replaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname API
  • Simplifies certificate validation logic by removing unsafe native code handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
System.Net.Quic.csprojReorganizes ItemGroup conditions to exclude OpenSSL cryptography interop files from macOS builds while maintaining them for Linux/FreeBSD
CertificateValidation.OSX.csReplaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname implementation

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@vcsjones

Copy link
Copy Markdown
MemberAuthor

One of the tests is failing with the hostname "\u017C\u00F3\u0142\u0107 g\u0119\u015Bl\u0105 ja\u017A\u0144. \u7EA2\u70E7. \u7167\u308A\u713C\u304D". The presence of spaces causes Uri.CheckHostName to return Unknown instead of Dns. That in turn causes X509Certificate2 to reject it.

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

It should probably also be said: using libmsquic on macOS with .NET on Apple Silicon machines is very difficult. I see the tests failing on x64 macOS because libmsquic is available, but the hardened runtime on Apple Silicon makes it very difficult to enable.

@rzikm

Copy link
Copy Markdown
Member

Does this change mean that we can get rid of

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContextsecurityContext,
X509Chainchain,
X509Certificate2?remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
SslPolicyErrorserrors=SslPolicyErrors.None;
if(remoteCertificate==null)
{
errors|=SslPolicyErrors.RemoteCertificateNotAvailable;
}
else
{
if(!chain.Build(remoteCertificate))
{
errors|=SslPolicyErrors.RemoteCertificateChainErrors;
}
if(!isServer&&checkCertName)
{
SafeDeleteSslContextsslContext=(SafeDeleteSslContext)securityContext;
if(!Interop.AppleCrypto.SslCheckHostnameMatch(sslContext.SslContext,hostName!,remoteCertificate.NotBefore,outintosStatus))
{
errors|=SslPolicyErrors.RemoteCertificateNameMismatch;
if(NetEventSource.Log.IsEnabled())
NetEventSource.Error(sslContext,$"Cert name validation for '{hostName}' failed with status '{osStatus}'");
}
}
}
returnerrors;
}

And do the same thing as we do on Unix?

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContext?_/*securityContext*/,
X509Chainchain,
X509Certificate2remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
returnCertificateValidation.BuildChainAndVerifyProperties(chain,remoteCertificate,checkCertName,isServer,hostName);
}

I looked at the code recently in the TLS 1.3 support PR and it seems to me that Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work (basically rebuilding the chain again?), so this change could possibly bring perf improvement.

@rzikm

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

@ManickaP
ManickaP requested review from rzikm and wfurtJuly 10, 2025 06:46
@krwq

krwq commented Jul 10, 2025

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

I don't remember - that was 7 years ago... but I think per this comment: dotnet/corefx#28278 (comment)

all ASCII are legal and UTF-8 used to be legal at one point.

Looking at RFC 4366 3.1:

"HostName" contains the fully qualified DNS hostname of the server,
as understood by the client. The hostname is represented as a byte
string using UTF-8 encoding [[UTF8](https://datatracker.ietf.org/doc/html/rfc4366#ref-UTF8)], without a trailing dot.
If the hostname labels contain only US-ASCII characters, then the
client MUST ensure that labels are separated only by the byte 0x2E,
representing the dot character U+002E (requirement 1 in Section 3.1
of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)] notwithstanding). If the server needs to match the
HostName against names that contain non-US-ASCII characters, it MUST
perform the conversion operation described in Section 4 of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)],
treating the HostName as a "query string" (i.e., the AllowUnassigned
flag MUST be set). Note that IDNA allows labels to be separated by
any of the Unicode characters U+002E, U+3002, U+FF0E, and U+FF61;
therefore, servers MUST accept any of these characters as a label
separator. If the server only needs to match the HostName against
names containing exclusively ASCII characters, it MUST compare ASCII
names case-insensitively.

"non US-ASCII" characters are a bit of a moot term for things like space so let's look at the IDNA ToASCII definition...

IDNA is defined in RFC3490 4.1:

(with my comments)

 1. If the sequence contains any code points outside the ASCII range
(0..7F) then proceed to step 2, otherwise skip to step 3.
(we skip 2 for space)
3. If the UseSTD3ASCIIRules flag is set, then perform these checks:
(a) Verify the absence of non-LDH ASCII code points; that is, the
absence of 0..2C, 2E..2F, 3A..40, 5B..60, and 7B..7F.
(b) Verify the absence of leading and trailing hyphen-minus; that
is, the absence of U+002D at the beginning and end of the
sequence.
(only if UseSTD3ASCIIRules is defined we consider space (20) as invalid)

There is also RFC6066 Appendix A:

 The Server Name extension now specifies only ASCII representation,
eliminating UTF-8.

so IMO all ASCII (0x00-0x7F) are legal for decoding given it's not guaranteed that UseSTD3ASCIIRules is used for encoding. But in theory we could block them with some sort of "strict validation" setting given it's possible to encode them.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

I think we are concerned more with how hostnames match against certificates, not what is permitted in the SNI extension.

RFC 1035 does not permit spaces in DNS labels. https://datatracker.ietf.org/doc/html/rfc1035#section-2.3.1

It also doesn't permit underscores, and we know OpenSSL enforces that.

Does this change mean that we can get rid of

Maybe, assuming we can get this change though :-).

Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work

Yeah Ideally we could get rid of the AppleCrypto hostname check... they don't expose a straight forward API for "Is this certificate okay for this host". The only way we could get something to work was to just build a whole chain and see if the chain had a host mismatch name error.

We should keep using the native hostname checks for OpenSSL and Windows though.

@wfurt

Copy link
Copy Markdown
Member

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

@vcsjones

Copy link
Copy Markdown
MemberAuthor

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

OpenSSL is going to do it itself in other places, so it should be consistent with itself. Apple is already using two implementations, Security.framework and OpenSSL, so we are just changing it to a... different... two implementations.

@rzikmrzikm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@krwqkrwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I already mentioned offline, it should be ok to break this until we see someone actually demonstrating space is needed for their scenario (which sounds very unlikely even if technically allowed). My recommendation would be to only allow same characters UseSTD3ASCIIRules talks about but I'm ok with this as is.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

Quic failures are unrelated and should be addressed by #117495.

@vcsjones
vcsjones merged commit d9e8e3f into dotnet:mainJul 11, 2025
@vcsjones
vcsjones deleted the macos-quic-no-openssl-hostname-match branch July 11, 2025 13:47
@vcsjonesvcsjones added this to the 10.0.0 milestone Jul 24, 2025
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 23, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@vcsjones@rzikm@krwq@wfurt
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC - #117472

Merged
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match
Jul 11, 2025
Merged

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC#117472
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match

Conversation

@vcsjones

Copy link
Copy Markdown
Member

Over at #117465, I am trying to get rid of S.S.C.Native.OpenSsl on macOS, but System.Net.Quic current uses parts of it for matching a certificate to a hostname. This makes sense since Apple's native APIs don't offer a similar API. But good news, we have an entirely managed implementation we can use on macOS.

This pull request replaces the use of the native PAL for hostname matching with our managed implementation, X509Certificate2.MatchesHostname, on macOS.

@vcsjonesvcsjones self-assigned this Jul 9, 2025
CopilotAI review requested due to automatic review settings July 9, 2025 18:54

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the dependency on System.Security.Cryptography.Native.OpenSsl for hostname matching in the QUIC library on macOS by replacing native OpenSSL certificate hostname validation with the managed X509Certificate2.MatchesHostname implementation.

  • Restructures project file to exclude OpenSSL cryptography interop files from macOS builds
  • Replaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname API
  • Simplifies certificate validation logic by removing unsafe native code handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
System.Net.Quic.csprojReorganizes ItemGroup conditions to exclude OpenSSL cryptography interop files from macOS builds while maintaining them for Linux/FreeBSD
CertificateValidation.OSX.csReplaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname implementation

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@vcsjones

Copy link
Copy Markdown
MemberAuthor

One of the tests is failing with the hostname "\u017C\u00F3\u0142\u0107 g\u0119\u015Bl\u0105 ja\u017A\u0144. \u7EA2\u70E7. \u7167\u308A\u713C\u304D". The presence of spaces causes Uri.CheckHostName to return Unknown instead of Dns. That in turn causes X509Certificate2 to reject it.

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

It should probably also be said: using libmsquic on macOS with .NET on Apple Silicon machines is very difficult. I see the tests failing on x64 macOS because libmsquic is available, but the hardened runtime on Apple Silicon makes it very difficult to enable.

@rzikm

Copy link
Copy Markdown
Member

Does this change mean that we can get rid of

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContextsecurityContext,
X509Chainchain,
X509Certificate2?remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
SslPolicyErrorserrors=SslPolicyErrors.None;
if(remoteCertificate==null)
{
errors|=SslPolicyErrors.RemoteCertificateNotAvailable;
}
else
{
if(!chain.Build(remoteCertificate))
{
errors|=SslPolicyErrors.RemoteCertificateChainErrors;
}
if(!isServer&&checkCertName)
{
SafeDeleteSslContextsslContext=(SafeDeleteSslContext)securityContext;
if(!Interop.AppleCrypto.SslCheckHostnameMatch(sslContext.SslContext,hostName!,remoteCertificate.NotBefore,outintosStatus))
{
errors|=SslPolicyErrors.RemoteCertificateNameMismatch;
if(NetEventSource.Log.IsEnabled())
NetEventSource.Error(sslContext,$"Cert name validation for '{hostName}' failed with status '{osStatus}'");
}
}
}
returnerrors;
}

And do the same thing as we do on Unix?

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContext?_/*securityContext*/,
X509Chainchain,
X509Certificate2remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
returnCertificateValidation.BuildChainAndVerifyProperties(chain,remoteCertificate,checkCertName,isServer,hostName);
}

I looked at the code recently in the TLS 1.3 support PR and it seems to me that Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work (basically rebuilding the chain again?), so this change could possibly bring perf improvement.

@rzikm

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

@ManickaP
ManickaP requested review from rzikm and wfurtJuly 10, 2025 06:46
@krwq

krwq commented Jul 10, 2025

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

I don't remember - that was 7 years ago... but I think per this comment: dotnet/corefx#28278 (comment)

all ASCII are legal and UTF-8 used to be legal at one point.

Looking at RFC 4366 3.1:

"HostName" contains the fully qualified DNS hostname of the server,
as understood by the client. The hostname is represented as a byte
string using UTF-8 encoding [[UTF8](https://datatracker.ietf.org/doc/html/rfc4366#ref-UTF8)], without a trailing dot.
If the hostname labels contain only US-ASCII characters, then the
client MUST ensure that labels are separated only by the byte 0x2E,
representing the dot character U+002E (requirement 1 in Section 3.1
of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)] notwithstanding). If the server needs to match the
HostName against names that contain non-US-ASCII characters, it MUST
perform the conversion operation described in Section 4 of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)],
treating the HostName as a "query string" (i.e., the AllowUnassigned
flag MUST be set). Note that IDNA allows labels to be separated by
any of the Unicode characters U+002E, U+3002, U+FF0E, and U+FF61;
therefore, servers MUST accept any of these characters as a label
separator. If the server only needs to match the HostName against
names containing exclusively ASCII characters, it MUST compare ASCII
names case-insensitively.

"non US-ASCII" characters are a bit of a moot term for things like space so let's look at the IDNA ToASCII definition...

IDNA is defined in RFC3490 4.1:

(with my comments)

 1. If the sequence contains any code points outside the ASCII range
(0..7F) then proceed to step 2, otherwise skip to step 3.
(we skip 2 for space)
3. If the UseSTD3ASCIIRules flag is set, then perform these checks:
(a) Verify the absence of non-LDH ASCII code points; that is, the
absence of 0..2C, 2E..2F, 3A..40, 5B..60, and 7B..7F.
(b) Verify the absence of leading and trailing hyphen-minus; that
is, the absence of U+002D at the beginning and end of the
sequence.
(only if UseSTD3ASCIIRules is defined we consider space (20) as invalid)

There is also RFC6066 Appendix A:

 The Server Name extension now specifies only ASCII representation,
eliminating UTF-8.

so IMO all ASCII (0x00-0x7F) are legal for decoding given it's not guaranteed that UseSTD3ASCIIRules is used for encoding. But in theory we could block them with some sort of "strict validation" setting given it's possible to encode them.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

I think we are concerned more with how hostnames match against certificates, not what is permitted in the SNI extension.

RFC 1035 does not permit spaces in DNS labels. https://datatracker.ietf.org/doc/html/rfc1035#section-2.3.1

It also doesn't permit underscores, and we know OpenSSL enforces that.

Does this change mean that we can get rid of

Maybe, assuming we can get this change though :-).

Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work

Yeah Ideally we could get rid of the AppleCrypto hostname check... they don't expose a straight forward API for "Is this certificate okay for this host". The only way we could get something to work was to just build a whole chain and see if the chain had a host mismatch name error.

We should keep using the native hostname checks for OpenSSL and Windows though.

@wfurt

Copy link
Copy Markdown
Member

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

@vcsjones

Copy link
Copy Markdown
MemberAuthor

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

OpenSSL is going to do it itself in other places, so it should be consistent with itself. Apple is already using two implementations, Security.framework and OpenSSL, so we are just changing it to a... different... two implementations.

@rzikmrzikm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@krwqkrwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I already mentioned offline, it should be ok to break this until we see someone actually demonstrating space is needed for their scenario (which sounds very unlikely even if technically allowed). My recommendation would be to only allow same characters UseSTD3ASCIIRules talks about but I'm ok with this as is.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

Quic failures are unrelated and should be addressed by #117495.

@vcsjones
vcsjones merged commit d9e8e3f into dotnet:mainJul 11, 2025
@vcsjones
vcsjones deleted the macos-quic-no-openssl-hostname-match branch July 11, 2025 13:47
@vcsjonesvcsjones added this to the 10.0.0 milestone Jul 24, 2025
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 23, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@vcsjones@rzikm@krwq@wfurt
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC - #117472

Merged
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match
Jul 11, 2025
Merged

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC#117472
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match

Conversation

@vcsjones

Copy link
Copy Markdown
Member

Over at #117465, I am trying to get rid of S.S.C.Native.OpenSsl on macOS, but System.Net.Quic current uses parts of it for matching a certificate to a hostname. This makes sense since Apple's native APIs don't offer a similar API. But good news, we have an entirely managed implementation we can use on macOS.

This pull request replaces the use of the native PAL for hostname matching with our managed implementation, X509Certificate2.MatchesHostname, on macOS.

@vcsjonesvcsjones self-assigned this Jul 9, 2025
CopilotAI review requested due to automatic review settings July 9, 2025 18:54

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the dependency on System.Security.Cryptography.Native.OpenSsl for hostname matching in the QUIC library on macOS by replacing native OpenSSL certificate hostname validation with the managed X509Certificate2.MatchesHostname implementation.

  • Restructures project file to exclude OpenSSL cryptography interop files from macOS builds
  • Replaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname API
  • Simplifies certificate validation logic by removing unsafe native code handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
System.Net.Quic.csprojReorganizes ItemGroup conditions to exclude OpenSSL cryptography interop files from macOS builds while maintaining them for Linux/FreeBSD
CertificateValidation.OSX.csReplaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname implementation

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@vcsjones

Copy link
Copy Markdown
MemberAuthor

One of the tests is failing with the hostname "\u017C\u00F3\u0142\u0107 g\u0119\u015Bl\u0105 ja\u017A\u0144. \u7EA2\u70E7. \u7167\u308A\u713C\u304D". The presence of spaces causes Uri.CheckHostName to return Unknown instead of Dns. That in turn causes X509Certificate2 to reject it.

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

It should probably also be said: using libmsquic on macOS with .NET on Apple Silicon machines is very difficult. I see the tests failing on x64 macOS because libmsquic is available, but the hardened runtime on Apple Silicon makes it very difficult to enable.

@rzikm

Copy link
Copy Markdown
Member

Does this change mean that we can get rid of

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContextsecurityContext,
X509Chainchain,
X509Certificate2?remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
SslPolicyErrorserrors=SslPolicyErrors.None;
if(remoteCertificate==null)
{
errors|=SslPolicyErrors.RemoteCertificateNotAvailable;
}
else
{
if(!chain.Build(remoteCertificate))
{
errors|=SslPolicyErrors.RemoteCertificateChainErrors;
}
if(!isServer&&checkCertName)
{
SafeDeleteSslContextsslContext=(SafeDeleteSslContext)securityContext;
if(!Interop.AppleCrypto.SslCheckHostnameMatch(sslContext.SslContext,hostName!,remoteCertificate.NotBefore,outintosStatus))
{
errors|=SslPolicyErrors.RemoteCertificateNameMismatch;
if(NetEventSource.Log.IsEnabled())
NetEventSource.Error(sslContext,$"Cert name validation for '{hostName}' failed with status '{osStatus}'");
}
}
}
returnerrors;
}

And do the same thing as we do on Unix?

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContext?_/*securityContext*/,
X509Chainchain,
X509Certificate2remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
returnCertificateValidation.BuildChainAndVerifyProperties(chain,remoteCertificate,checkCertName,isServer,hostName);
}

I looked at the code recently in the TLS 1.3 support PR and it seems to me that Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work (basically rebuilding the chain again?), so this change could possibly bring perf improvement.

@rzikm

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

@ManickaP
ManickaP requested review from rzikm and wfurtJuly 10, 2025 06:46
@krwq

krwq commented Jul 10, 2025

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

I don't remember - that was 7 years ago... but I think per this comment: dotnet/corefx#28278 (comment)

all ASCII are legal and UTF-8 used to be legal at one point.

Looking at RFC 4366 3.1:

"HostName" contains the fully qualified DNS hostname of the server,
as understood by the client. The hostname is represented as a byte
string using UTF-8 encoding [[UTF8](https://datatracker.ietf.org/doc/html/rfc4366#ref-UTF8)], without a trailing dot.
If the hostname labels contain only US-ASCII characters, then the
client MUST ensure that labels are separated only by the byte 0x2E,
representing the dot character U+002E (requirement 1 in Section 3.1
of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)] notwithstanding). If the server needs to match the
HostName against names that contain non-US-ASCII characters, it MUST
perform the conversion operation described in Section 4 of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)],
treating the HostName as a "query string" (i.e., the AllowUnassigned
flag MUST be set). Note that IDNA allows labels to be separated by
any of the Unicode characters U+002E, U+3002, U+FF0E, and U+FF61;
therefore, servers MUST accept any of these characters as a label
separator. If the server only needs to match the HostName against
names containing exclusively ASCII characters, it MUST compare ASCII
names case-insensitively.

"non US-ASCII" characters are a bit of a moot term for things like space so let's look at the IDNA ToASCII definition...

IDNA is defined in RFC3490 4.1:

(with my comments)

 1. If the sequence contains any code points outside the ASCII range
(0..7F) then proceed to step 2, otherwise skip to step 3.
(we skip 2 for space)
3. If the UseSTD3ASCIIRules flag is set, then perform these checks:
(a) Verify the absence of non-LDH ASCII code points; that is, the
absence of 0..2C, 2E..2F, 3A..40, 5B..60, and 7B..7F.
(b) Verify the absence of leading and trailing hyphen-minus; that
is, the absence of U+002D at the beginning and end of the
sequence.
(only if UseSTD3ASCIIRules is defined we consider space (20) as invalid)

There is also RFC6066 Appendix A:

 The Server Name extension now specifies only ASCII representation,
eliminating UTF-8.

so IMO all ASCII (0x00-0x7F) are legal for decoding given it's not guaranteed that UseSTD3ASCIIRules is used for encoding. But in theory we could block them with some sort of "strict validation" setting given it's possible to encode them.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

I think we are concerned more with how hostnames match against certificates, not what is permitted in the SNI extension.

RFC 1035 does not permit spaces in DNS labels. https://datatracker.ietf.org/doc/html/rfc1035#section-2.3.1

It also doesn't permit underscores, and we know OpenSSL enforces that.

Does this change mean that we can get rid of

Maybe, assuming we can get this change though :-).

Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work

Yeah Ideally we could get rid of the AppleCrypto hostname check... they don't expose a straight forward API for "Is this certificate okay for this host". The only way we could get something to work was to just build a whole chain and see if the chain had a host mismatch name error.

We should keep using the native hostname checks for OpenSSL and Windows though.

@wfurt

Copy link
Copy Markdown
Member

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

@vcsjones

Copy link
Copy Markdown
MemberAuthor

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

OpenSSL is going to do it itself in other places, so it should be consistent with itself. Apple is already using two implementations, Security.framework and OpenSSL, so we are just changing it to a... different... two implementations.

@rzikmrzikm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@krwqkrwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I already mentioned offline, it should be ok to break this until we see someone actually demonstrating space is needed for their scenario (which sounds very unlikely even if technically allowed). My recommendation would be to only allow same characters UseSTD3ASCIIRules talks about but I'm ok with this as is.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

Quic failures are unrelated and should be addressed by #117495.

@vcsjones
vcsjones merged commit d9e8e3f into dotnet:mainJul 11, 2025
@vcsjones
vcsjones deleted the macos-quic-no-openssl-hostname-match branch July 11, 2025 13:47
@vcsjonesvcsjones added this to the 10.0.0 milestone Jul 24, 2025
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 23, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@vcsjones@rzikm@krwq@wfurt
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC - #117472

Merged
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match
Jul 11, 2025
Merged

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC#117472
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match

Conversation

@vcsjones

Copy link
Copy Markdown
Member

Over at #117465, I am trying to get rid of S.S.C.Native.OpenSsl on macOS, but System.Net.Quic current uses parts of it for matching a certificate to a hostname. This makes sense since Apple's native APIs don't offer a similar API. But good news, we have an entirely managed implementation we can use on macOS.

This pull request replaces the use of the native PAL for hostname matching with our managed implementation, X509Certificate2.MatchesHostname, on macOS.

@vcsjonesvcsjones self-assigned this Jul 9, 2025
CopilotAI review requested due to automatic review settings July 9, 2025 18:54

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the dependency on System.Security.Cryptography.Native.OpenSsl for hostname matching in the QUIC library on macOS by replacing native OpenSSL certificate hostname validation with the managed X509Certificate2.MatchesHostname implementation.

  • Restructures project file to exclude OpenSSL cryptography interop files from macOS builds
  • Replaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname API
  • Simplifies certificate validation logic by removing unsafe native code handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
System.Net.Quic.csprojReorganizes ItemGroup conditions to exclude OpenSSL cryptography interop files from macOS builds while maintaining them for Linux/FreeBSD
CertificateValidation.OSX.csReplaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname implementation

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@vcsjones

Copy link
Copy Markdown
MemberAuthor

One of the tests is failing with the hostname "\u017C\u00F3\u0142\u0107 g\u0119\u015Bl\u0105 ja\u017A\u0144. \u7EA2\u70E7. \u7167\u308A\u713C\u304D". The presence of spaces causes Uri.CheckHostName to return Unknown instead of Dns. That in turn causes X509Certificate2 to reject it.

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

It should probably also be said: using libmsquic on macOS with .NET on Apple Silicon machines is very difficult. I see the tests failing on x64 macOS because libmsquic is available, but the hardened runtime on Apple Silicon makes it very difficult to enable.

@rzikm

Copy link
Copy Markdown
Member

Does this change mean that we can get rid of

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContextsecurityContext,
X509Chainchain,
X509Certificate2?remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
SslPolicyErrorserrors=SslPolicyErrors.None;
if(remoteCertificate==null)
{
errors|=SslPolicyErrors.RemoteCertificateNotAvailable;
}
else
{
if(!chain.Build(remoteCertificate))
{
errors|=SslPolicyErrors.RemoteCertificateChainErrors;
}
if(!isServer&&checkCertName)
{
SafeDeleteSslContextsslContext=(SafeDeleteSslContext)securityContext;
if(!Interop.AppleCrypto.SslCheckHostnameMatch(sslContext.SslContext,hostName!,remoteCertificate.NotBefore,outintosStatus))
{
errors|=SslPolicyErrors.RemoteCertificateNameMismatch;
if(NetEventSource.Log.IsEnabled())
NetEventSource.Error(sslContext,$"Cert name validation for '{hostName}' failed with status '{osStatus}'");
}
}
}
returnerrors;
}

And do the same thing as we do on Unix?

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContext?_/*securityContext*/,
X509Chainchain,
X509Certificate2remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
returnCertificateValidation.BuildChainAndVerifyProperties(chain,remoteCertificate,checkCertName,isServer,hostName);
}

I looked at the code recently in the TLS 1.3 support PR and it seems to me that Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work (basically rebuilding the chain again?), so this change could possibly bring perf improvement.

@rzikm

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

@ManickaP
ManickaP requested review from rzikm and wfurtJuly 10, 2025 06:46
@krwq

krwq commented Jul 10, 2025

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

I don't remember - that was 7 years ago... but I think per this comment: dotnet/corefx#28278 (comment)

all ASCII are legal and UTF-8 used to be legal at one point.

Looking at RFC 4366 3.1:

"HostName" contains the fully qualified DNS hostname of the server,
as understood by the client. The hostname is represented as a byte
string using UTF-8 encoding [[UTF8](https://datatracker.ietf.org/doc/html/rfc4366#ref-UTF8)], without a trailing dot.
If the hostname labels contain only US-ASCII characters, then the
client MUST ensure that labels are separated only by the byte 0x2E,
representing the dot character U+002E (requirement 1 in Section 3.1
of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)] notwithstanding). If the server needs to match the
HostName against names that contain non-US-ASCII characters, it MUST
perform the conversion operation described in Section 4 of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)],
treating the HostName as a "query string" (i.e., the AllowUnassigned
flag MUST be set). Note that IDNA allows labels to be separated by
any of the Unicode characters U+002E, U+3002, U+FF0E, and U+FF61;
therefore, servers MUST accept any of these characters as a label
separator. If the server only needs to match the HostName against
names containing exclusively ASCII characters, it MUST compare ASCII
names case-insensitively.

"non US-ASCII" characters are a bit of a moot term for things like space so let's look at the IDNA ToASCII definition...

IDNA is defined in RFC3490 4.1:

(with my comments)

 1. If the sequence contains any code points outside the ASCII range
(0..7F) then proceed to step 2, otherwise skip to step 3.
(we skip 2 for space)
3. If the UseSTD3ASCIIRules flag is set, then perform these checks:
(a) Verify the absence of non-LDH ASCII code points; that is, the
absence of 0..2C, 2E..2F, 3A..40, 5B..60, and 7B..7F.
(b) Verify the absence of leading and trailing hyphen-minus; that
is, the absence of U+002D at the beginning and end of the
sequence.
(only if UseSTD3ASCIIRules is defined we consider space (20) as invalid)

There is also RFC6066 Appendix A:

 The Server Name extension now specifies only ASCII representation,
eliminating UTF-8.

so IMO all ASCII (0x00-0x7F) are legal for decoding given it's not guaranteed that UseSTD3ASCIIRules is used for encoding. But in theory we could block them with some sort of "strict validation" setting given it's possible to encode them.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

I think we are concerned more with how hostnames match against certificates, not what is permitted in the SNI extension.

RFC 1035 does not permit spaces in DNS labels. https://datatracker.ietf.org/doc/html/rfc1035#section-2.3.1

It also doesn't permit underscores, and we know OpenSSL enforces that.

Does this change mean that we can get rid of

Maybe, assuming we can get this change though :-).

Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work

Yeah Ideally we could get rid of the AppleCrypto hostname check... they don't expose a straight forward API for "Is this certificate okay for this host". The only way we could get something to work was to just build a whole chain and see if the chain had a host mismatch name error.

We should keep using the native hostname checks for OpenSSL and Windows though.

@wfurt

Copy link
Copy Markdown
Member

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

@vcsjones

Copy link
Copy Markdown
MemberAuthor

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

OpenSSL is going to do it itself in other places, so it should be consistent with itself. Apple is already using two implementations, Security.framework and OpenSSL, so we are just changing it to a... different... two implementations.

@rzikmrzikm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@krwqkrwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I already mentioned offline, it should be ok to break this until we see someone actually demonstrating space is needed for their scenario (which sounds very unlikely even if technically allowed). My recommendation would be to only allow same characters UseSTD3ASCIIRules talks about but I'm ok with this as is.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

Quic failures are unrelated and should be addressed by #117495.

@vcsjones
vcsjones merged commit d9e8e3f into dotnet:mainJul 11, 2025
@vcsjones
vcsjones deleted the macos-quic-no-openssl-hostname-match branch July 11, 2025 13:47
@vcsjonesvcsjones added this to the 10.0.0 milestone Jul 24, 2025
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 23, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@vcsjones@rzikm@krwq@wfurt
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC - #117472

Merged
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match
Jul 11, 2025
Merged

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC#117472
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match

Conversation

@vcsjones

Copy link
Copy Markdown
Member

Over at #117465, I am trying to get rid of S.S.C.Native.OpenSsl on macOS, but System.Net.Quic current uses parts of it for matching a certificate to a hostname. This makes sense since Apple's native APIs don't offer a similar API. But good news, we have an entirely managed implementation we can use on macOS.

This pull request replaces the use of the native PAL for hostname matching with our managed implementation, X509Certificate2.MatchesHostname, on macOS.

@vcsjonesvcsjones self-assigned this Jul 9, 2025
CopilotAI review requested due to automatic review settings July 9, 2025 18:54

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the dependency on System.Security.Cryptography.Native.OpenSsl for hostname matching in the QUIC library on macOS by replacing native OpenSSL certificate hostname validation with the managed X509Certificate2.MatchesHostname implementation.

  • Restructures project file to exclude OpenSSL cryptography interop files from macOS builds
  • Replaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname API
  • Simplifies certificate validation logic by removing unsafe native code handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
System.Net.Quic.csprojReorganizes ItemGroup conditions to exclude OpenSSL cryptography interop files from macOS builds while maintaining them for Linux/FreeBSD
CertificateValidation.OSX.csReplaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname implementation

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@vcsjones

Copy link
Copy Markdown
MemberAuthor

One of the tests is failing with the hostname "\u017C\u00F3\u0142\u0107 g\u0119\u015Bl\u0105 ja\u017A\u0144. \u7EA2\u70E7. \u7167\u308A\u713C\u304D". The presence of spaces causes Uri.CheckHostName to return Unknown instead of Dns. That in turn causes X509Certificate2 to reject it.

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

It should probably also be said: using libmsquic on macOS with .NET on Apple Silicon machines is very difficult. I see the tests failing on x64 macOS because libmsquic is available, but the hardened runtime on Apple Silicon makes it very difficult to enable.

@rzikm

Copy link
Copy Markdown
Member

Does this change mean that we can get rid of

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContextsecurityContext,
X509Chainchain,
X509Certificate2?remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
SslPolicyErrorserrors=SslPolicyErrors.None;
if(remoteCertificate==null)
{
errors|=SslPolicyErrors.RemoteCertificateNotAvailable;
}
else
{
if(!chain.Build(remoteCertificate))
{
errors|=SslPolicyErrors.RemoteCertificateChainErrors;
}
if(!isServer&&checkCertName)
{
SafeDeleteSslContextsslContext=(SafeDeleteSslContext)securityContext;
if(!Interop.AppleCrypto.SslCheckHostnameMatch(sslContext.SslContext,hostName!,remoteCertificate.NotBefore,outintosStatus))
{
errors|=SslPolicyErrors.RemoteCertificateNameMismatch;
if(NetEventSource.Log.IsEnabled())
NetEventSource.Error(sslContext,$"Cert name validation for '{hostName}' failed with status '{osStatus}'");
}
}
}
returnerrors;
}

And do the same thing as we do on Unix?

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContext?_/*securityContext*/,
X509Chainchain,
X509Certificate2remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
returnCertificateValidation.BuildChainAndVerifyProperties(chain,remoteCertificate,checkCertName,isServer,hostName);
}

I looked at the code recently in the TLS 1.3 support PR and it seems to me that Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work (basically rebuilding the chain again?), so this change could possibly bring perf improvement.

@rzikm

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

@ManickaP
ManickaP requested review from rzikm and wfurtJuly 10, 2025 06:46
@krwq

krwq commented Jul 10, 2025

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

I don't remember - that was 7 years ago... but I think per this comment: dotnet/corefx#28278 (comment)

all ASCII are legal and UTF-8 used to be legal at one point.

Looking at RFC 4366 3.1:

"HostName" contains the fully qualified DNS hostname of the server,
as understood by the client. The hostname is represented as a byte
string using UTF-8 encoding [[UTF8](https://datatracker.ietf.org/doc/html/rfc4366#ref-UTF8)], without a trailing dot.
If the hostname labels contain only US-ASCII characters, then the
client MUST ensure that labels are separated only by the byte 0x2E,
representing the dot character U+002E (requirement 1 in Section 3.1
of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)] notwithstanding). If the server needs to match the
HostName against names that contain non-US-ASCII characters, it MUST
perform the conversion operation described in Section 4 of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)],
treating the HostName as a "query string" (i.e., the AllowUnassigned
flag MUST be set). Note that IDNA allows labels to be separated by
any of the Unicode characters U+002E, U+3002, U+FF0E, and U+FF61;
therefore, servers MUST accept any of these characters as a label
separator. If the server only needs to match the HostName against
names containing exclusively ASCII characters, it MUST compare ASCII
names case-insensitively.

"non US-ASCII" characters are a bit of a moot term for things like space so let's look at the IDNA ToASCII definition...

IDNA is defined in RFC3490 4.1:

(with my comments)

 1. If the sequence contains any code points outside the ASCII range
(0..7F) then proceed to step 2, otherwise skip to step 3.
(we skip 2 for space)
3. If the UseSTD3ASCIIRules flag is set, then perform these checks:
(a) Verify the absence of non-LDH ASCII code points; that is, the
absence of 0..2C, 2E..2F, 3A..40, 5B..60, and 7B..7F.
(b) Verify the absence of leading and trailing hyphen-minus; that
is, the absence of U+002D at the beginning and end of the
sequence.
(only if UseSTD3ASCIIRules is defined we consider space (20) as invalid)

There is also RFC6066 Appendix A:

 The Server Name extension now specifies only ASCII representation,
eliminating UTF-8.

so IMO all ASCII (0x00-0x7F) are legal for decoding given it's not guaranteed that UseSTD3ASCIIRules is used for encoding. But in theory we could block them with some sort of "strict validation" setting given it's possible to encode them.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

I think we are concerned more with how hostnames match against certificates, not what is permitted in the SNI extension.

RFC 1035 does not permit spaces in DNS labels. https://datatracker.ietf.org/doc/html/rfc1035#section-2.3.1

It also doesn't permit underscores, and we know OpenSSL enforces that.

Does this change mean that we can get rid of

Maybe, assuming we can get this change though :-).

Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work

Yeah Ideally we could get rid of the AppleCrypto hostname check... they don't expose a straight forward API for "Is this certificate okay for this host". The only way we could get something to work was to just build a whole chain and see if the chain had a host mismatch name error.

We should keep using the native hostname checks for OpenSSL and Windows though.

@wfurt

Copy link
Copy Markdown
Member

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

@vcsjones

Copy link
Copy Markdown
MemberAuthor

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

OpenSSL is going to do it itself in other places, so it should be consistent with itself. Apple is already using two implementations, Security.framework and OpenSSL, so we are just changing it to a... different... two implementations.

@rzikmrzikm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@krwqkrwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I already mentioned offline, it should be ok to break this until we see someone actually demonstrating space is needed for their scenario (which sounds very unlikely even if technically allowed). My recommendation would be to only allow same characters UseSTD3ASCIIRules talks about but I'm ok with this as is.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

Quic failures are unrelated and should be addressed by #117495.

@vcsjones
vcsjones merged commit d9e8e3f into dotnet:mainJul 11, 2025
@vcsjones
vcsjones deleted the macos-quic-no-openssl-hostname-match branch July 11, 2025 13:47
@vcsjonesvcsjones added this to the 10.0.0 milestone Jul 24, 2025
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 23, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@vcsjones@rzikm@krwq@wfurt
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC - #117472

Merged
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match
Jul 11, 2025
Merged

Remove dependency on System.Security.Cryptography.Native.OpenSsl in QUIC#117472
vcsjones merged 2 commits into
dotnet:mainfrom
vcsjones:macos-quic-no-openssl-hostname-match

Conversation

@vcsjones

Copy link
Copy Markdown
Member

Over at #117465, I am trying to get rid of S.S.C.Native.OpenSsl on macOS, but System.Net.Quic current uses parts of it for matching a certificate to a hostname. This makes sense since Apple's native APIs don't offer a similar API. But good news, we have an entirely managed implementation we can use on macOS.

This pull request replaces the use of the native PAL for hostname matching with our managed implementation, X509Certificate2.MatchesHostname, on macOS.

@vcsjonesvcsjones self-assigned this Jul 9, 2025
CopilotAI review requested due to automatic review settings July 9, 2025 18:54

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the dependency on System.Security.Cryptography.Native.OpenSsl for hostname matching in the QUIC library on macOS by replacing native OpenSSL certificate hostname validation with the managed X509Certificate2.MatchesHostname implementation.

  • Restructures project file to exclude OpenSSL cryptography interop files from macOS builds
  • Replaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname API
  • Simplifies certificate validation logic by removing unsafe native code handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
System.Net.Quic.csprojReorganizes ItemGroup conditions to exclude OpenSSL cryptography interop files from macOS builds while maintaining them for Linux/FreeBSD
CertificateValidation.OSX.csReplaces native OpenSSL hostname matching with managed X509Certificate2.MatchesHostname implementation

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@vcsjones

Copy link
Copy Markdown
MemberAuthor

One of the tests is failing with the hostname "\u017C\u00F3\u0142\u0107 g\u0119\u015Bl\u0105 ja\u017A\u0144. \u7EA2\u70E7. \u7167\u308A\u713C\u304D". The presence of spaces causes Uri.CheckHostName to return Unknown instead of Dns. That in turn causes X509Certificate2 to reject it.

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

It should probably also be said: using libmsquic on macOS with .NET on Apple Silicon machines is very difficult. I see the tests failing on x64 macOS because libmsquic is available, but the hardened runtime on Apple Silicon makes it very difficult to enable.

@rzikm

Copy link
Copy Markdown
Member

Does this change mean that we can get rid of

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContextsecurityContext,
X509Chainchain,
X509Certificate2?remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
SslPolicyErrorserrors=SslPolicyErrors.None;
if(remoteCertificate==null)
{
errors|=SslPolicyErrors.RemoteCertificateNotAvailable;
}
else
{
if(!chain.Build(remoteCertificate))
{
errors|=SslPolicyErrors.RemoteCertificateChainErrors;
}
if(!isServer&&checkCertName)
{
SafeDeleteSslContextsslContext=(SafeDeleteSslContext)securityContext;
if(!Interop.AppleCrypto.SslCheckHostnameMatch(sslContext.SslContext,hostName!,remoteCertificate.NotBefore,outintosStatus))
{
errors|=SslPolicyErrors.RemoteCertificateNameMismatch;
if(NetEventSource.Log.IsEnabled())
NetEventSource.Error(sslContext,$"Cert name validation for '{hostName}' failed with status '{osStatus}'");
}
}
}
returnerrors;
}

And do the same thing as we do on Unix?

internalstaticpartialclassCertificateValidationPal
{
internalstaticSslPolicyErrorsVerifyCertificateProperties(
SafeDeleteContext?_/*securityContext*/,
X509Chainchain,
X509Certificate2remoteCertificate,
boolcheckCertName,
boolisServer,
string?hostName)
{
returnCertificateValidation.BuildChainAndVerifyProperties(chain,remoteCertificate,checkCertName,isServer,hostName);
}

I looked at the code recently in the TLS 1.3 support PR and it seems to me that Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work (basically rebuilding the chain again?), so this change could possibly bring perf improvement.

@rzikm

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

@ManickaP
ManickaP requested review from rzikm and wfurtJuly 10, 2025 06:46
@krwq

krwq commented Jul 10, 2025

Copy link
Copy Markdown
Member

It's not immediately clear to me if this is "right". My interpretation of the relevant RFCs is that a space is not permitted in host or DNS labels, so perhaps the test should simply omit the spaces.

I am also confused by the presence of spaces in the hostname, @krwq, your name shows up on git blame, do you remember why you put spaces in the test case?

I am personally ok with removing them

I don't remember - that was 7 years ago... but I think per this comment: dotnet/corefx#28278 (comment)

all ASCII are legal and UTF-8 used to be legal at one point.

Looking at RFC 4366 3.1:

"HostName" contains the fully qualified DNS hostname of the server,
as understood by the client. The hostname is represented as a byte
string using UTF-8 encoding [[UTF8](https://datatracker.ietf.org/doc/html/rfc4366#ref-UTF8)], without a trailing dot.
If the hostname labels contain only US-ASCII characters, then the
client MUST ensure that labels are separated only by the byte 0x2E,
representing the dot character U+002E (requirement 1 in Section 3.1
of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)] notwithstanding). If the server needs to match the
HostName against names that contain non-US-ASCII characters, it MUST
perform the conversion operation described in Section 4 of [[IDNA](https://datatracker.ietf.org/doc/html/rfc4366#ref-IDNA)],
treating the HostName as a "query string" (i.e., the AllowUnassigned
flag MUST be set). Note that IDNA allows labels to be separated by
any of the Unicode characters U+002E, U+3002, U+FF0E, and U+FF61;
therefore, servers MUST accept any of these characters as a label
separator. If the server only needs to match the HostName against
names containing exclusively ASCII characters, it MUST compare ASCII
names case-insensitively.

"non US-ASCII" characters are a bit of a moot term for things like space so let's look at the IDNA ToASCII definition...

IDNA is defined in RFC3490 4.1:

(with my comments)

 1. If the sequence contains any code points outside the ASCII range
(0..7F) then proceed to step 2, otherwise skip to step 3.
(we skip 2 for space)
3. If the UseSTD3ASCIIRules flag is set, then perform these checks:
(a) Verify the absence of non-LDH ASCII code points; that is, the
absence of 0..2C, 2E..2F, 3A..40, 5B..60, and 7B..7F.
(b) Verify the absence of leading and trailing hyphen-minus; that
is, the absence of U+002D at the beginning and end of the
sequence.
(only if UseSTD3ASCIIRules is defined we consider space (20) as invalid)

There is also RFC6066 Appendix A:

 The Server Name extension now specifies only ASCII representation,
eliminating UTF-8.

so IMO all ASCII (0x00-0x7F) are legal for decoding given it's not guaranteed that UseSTD3ASCIIRules is used for encoding. But in theory we could block them with some sort of "strict validation" setting given it's possible to encode them.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

I think we are concerned more with how hostnames match against certificates, not what is permitted in the SNI extension.

RFC 1035 does not permit spaces in DNS labels. https://datatracker.ietf.org/doc/html/rfc1035#section-2.3.1

It also doesn't permit underscores, and we know OpenSSL enforces that.

Does this change mean that we can get rid of

Maybe, assuming we can get this change though :-).

Interop.AppleCrypto.SslCheckHostnameMatch is doing quite a lot of work

Yeah Ideally we could get rid of the AppleCrypto hostname check... they don't expose a straight forward API for "Is this certificate okay for this host". The only way we could get something to work was to just build a whole chain and see if the chain had a host mismatch name error.

We should keep using the native hostname checks for OpenSSL and Windows though.

@wfurt

Copy link
Copy Markdown
Member

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

@vcsjones

Copy link
Copy Markdown
MemberAuthor

would it make sense to sue the managed implementation also on Linux for consistency @vcsjones ??? Or are the OpenSSL internal check unavoidable?

OpenSSL is going to do it itself in other places, so it should be consistent with itself. Apple is already using two implementations, Security.framework and OpenSSL, so we are just changing it to a... different... two implementations.

@rzikmrzikm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@krwqkrwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I already mentioned offline, it should be ok to break this until we see someone actually demonstrating space is needed for their scenario (which sounds very unlikely even if technically allowed). My recommendation would be to only allow same characters UseSTD3ASCIIRules talks about but I'm ok with this as is.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

Quic failures are unrelated and should be addressed by #117495.

@vcsjones
vcsjones merged commit d9e8e3f into dotnet:mainJul 11, 2025
@vcsjones
vcsjones deleted the macos-quic-no-openssl-hostname-match branch July 11, 2025 13:47
@vcsjonesvcsjones added this to the 10.0.0 milestone Jul 24, 2025
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 23, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@vcsjones@rzikm@krwq@wfurt