Repository navigation
Conversation
Use exact DNS SAN and CN matching without wildcards after IP SAN matching. Propagate OpenSSL matching errors and cover IPv4/IPv6 behavior with TLS regression cases. Fixes dotnet#131734 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 157c8032-dde4-46fd-aacd-8aa770fdfe74
dotnet-policy-service
Bot
added
the
community-contribution
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones |
| GENERAL_NAMES_free(san); | ||
| } | ||
|
|
||
| if (!success) | ||
| { | ||
| // This is a shared/interor pointer, do not free! | ||
| OSSL4CONST X509_NAME* subject = X509_get_subject_name(x509); | ||
|
|
||
| if (subject) | ||
| { | ||
| int i = -1; | ||
|
|
||
| while ((i = X509_NAME_get_index_by_NID(subject, subjectNid, i)) >= 0) | ||
| { | ||
| // Shared/interior pointers, do not free! | ||
| const X509_NAME_ENTRY* nameEnt = X509_NAME_get_entry(subject, i); | ||
| const ASN1_STRING* cn = X509_NAME_ENTRY_get_data(nameEnt); | ||
|
|
||
| const char* data = (const char*)ASN1_STRING_get0_data(cn); | ||
|
|
||
| if (ASN1_STRING_length(cn) == cchHostname && | ||
| data != NULL && | ||
| !strncasecmp(data, hostname, (size_t)cchHostname)) | ||
| { | ||
| success = 1; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| // Match Windows: DNS SAN entries take precedence over CN, without wildcard matching for IP literals. | ||
| success = X509_check_host(x509, hostname, (size_t)cchHostname, X509_CHECK_FLAG_NO_WILDCARDS, NULL); | ||
| } | ||
|
|
||
| return success; |
There was a problem hiding this comment.
Based on the behavior comparison/test table/matrix, I believe the better change is
if (san)
{
...
GENERAL_NAMES_free(san);
+ return success;
}
-if (!success) {
return X509_check_host(x509, hostname, (size_t)cchHostname, X509_CHECK_FLAG_NO_WILDCARDS, NULL);
-}
-return success;Really it comes down to a degenerate test case, if the SAN block contains dNSName:192.168.0.2 then I believe the proposed code will match against it, and I assume we don't want that. Though, if Windows matches IP-Address-looking-values against dNSName entries then the proposed change is actually better.
There was a problem hiding this comment.
Looking at the test cases in the PR, vs the ones described in the table, I think maybe there already is that test and it shows that windows DOES match IP-parsable strings against SAN dNSName entries (which is, of course, bonkers for IPv6 since there are many representations for each address), in which case the proposed is probably the correct change.
But that theory is sort of hard to read, so I'm not sure.
| } | ||
| } | ||
| } | ||
| // Match Windows: DNS SAN entries take precedence over CN, without wildcard matching for IP literals. |
There was a problem hiding this comment.
Whether Windows is matching IP addresses against SAN dNSName, or not, I don't think this comment is helpful as-is.
If it is, and we want that, then the comment would be more like
// Windows compat: Match the IP address as text against SAN or Subject CN
success = X509_check_host(...);if it doesn't, then the comment (matching that we already should have exited would be)
// No SAN, fall back to Subject CN matching
return X509_check_host(...);Because to me it just says "Match Windows: DNS SAN entries take precedence over CN", and I'm like "yep. they do." but the mere mention of DNS SAN entries to me makes it "obvious" (even if wrong) that we've left parsed-IP-address land and we're talking about the normal "test.example" DNS name case.
Summary
Fix Linux certificate-name validation for IP-literal targets so DNS SAN entries take precedence over CN fallback, aligning the tested behavior with Windows and addressing #131734.
cc @wfurt
Motivation
Linux previously checked for a matching
iPAddressSAN and, if none matched, searched the certificate’s CN without considering whether DNS SAN entries were present. This allowed a certificate withCN=192.0.2.1andSAN=DNS:example.testto match target192.0.2.1on Linux, while Windows reported a name mismatch.Approach
Keep the existing byte-for-byte IP SAN check as the first and fast path. If it does not match, use OpenSSL’s
X509_check_hostfor DNS SAN/CN text matching, with wildcard expansion disabled for IP targets. This lets OpenSSL apply DNS SAN precedence while retaining the legacy CN fallback observed on Windows when there are no DNS SAN entries, including certificates with only a nonmatching IP SAN.The change is intended to align with the verified Windows compatibility behavior; it does not implement strict RFC 9525 IP identity matching, which would require a matching
iPAddressSAN.Changes
CryptoNative_CheckX509IpAddresswithX509_check_host.CertificateValidation.Unix.csas cryptographic errors for IP targets as well as DNS hostnames.SslStreamTLS cases covering CN-only certificates, matching and conflicting IP SANs, DNS SAN precedence, numeric DNS SANs, and wildcard names.Compatibility and performance
The change intentionally preserves legacy fallback behavior where the tested Windows implementation accepts it: CN-only certificates and certificates with a conflicting IP-only SAN can still match via CN. A matching
iPAddressSAN continues to take precedence and match by address bytes. Exact numeric DNS SANs also matched the IP target in the Windows cases tested.A local short-run native microbenchmark showed the matching-IP-SAN path remained similar (about 636 ns before vs. 669 ns after). Fallback cases took longer: CN-only was about 181 ns vs. 380 ns, and fallback after a conflicting IP SAN was about 671 ns vs. 1,458 ns. These are microbenchmark measurements, not end-to-end TLS latency results.
Before/after behavior
For an IP-literal target, Linux previously checked for a matching
iPAddressSAN and, if none matched, compared the target against CN without considering DNS SAN entries. After this change, the fallback honors DNS SAN precedence, matching the tested Windows behavior.192.0.2.1DNS:example.test192.0.2.1other.testDNS:192.0.2.1192.0.2.1192.0.2.1IP:192.0.2.2192.0.2.1192.0.2.1IP:192.0.2.2,DNS:example.test192.0.2.2“Match” means that name validation does not report
RemoteCertificateNameMismatch; certificate-chain validation is a separate check. Equivalent IPv6 cases are covered by the regression tests.The first row is a behavioral breaking change: with default certificate validation, authentication now fails for that configuration even if the certificate chain is otherwise trusted. The recommended migration is to issue a certificate containing an
iPAddressSAN matching the target IP.A matching
iPAddressSAN still takes precedence. Legacy CN fallback remains available when there are no DNS SAN entries, including when an IP-only SAN contains a different address. Exact IP literals encoded as DNS SANs can now match, as shown in the second row; wildcard expansion remains disabled for IP targets.Validation
System.Net.Securityfunctional tests: 5,138 total, 0 failed, 8 skipped.System.Net.Securityunit tests: 146 total, 0 failed, 2 skipped.Resolves #131734
See my repro report at #131734 (comment)
Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com