-
Notifications
You must be signed in to change notification settings - Fork 971
Fix IDNA matching #10331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Fix IDNA matching #10331
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1081,11 +1081,19 @@ int test_wolfSSL_X509_check_ip_asc(void) | |||||||||
| ExpectIntEQ(wolfSSL_X509_check_ip_asc(cn_lit, "127.0.0.1", 0), 0); | ||||||||||
| /* CN=*.0.0.1 with no SAN must NOT wildcard-match "127.0.0.1". */ | ||||||||||
| ExpectIntEQ(wolfSSL_X509_check_ip_asc(cn_wild, "127.0.0.1", 0), 0); | ||||||||||
|
|
||||||||||
| /* CN-based hostname matching must still work for hostname checks | ||||||||||
| * (sanity check that the fix didn't over-correct). */ | ||||||||||
| ExpectIntEQ(wolfSSL_X509_check_host(cn_wild, "1.0.0.1", | ||||||||||
| XSTRLEN("1.0.0.1"), 0, NULL), 1); | ||||||||||
|
|
||||||||||
| /* However, when WOLFSSL_LEFT_MOST_WILDCARD_ONLY, CN-based hostname | ||||||||||
| * matching must not apply wildcards when the supplied hostname isn't a | ||||||||||
| * well-formed FQDN. | ||||||||||
| */ | ||||||||||
| ExpectIntEQ(wolfSSL_X509_check_host(cn_wild, "1.0.0.1", | ||||||||||
| XSTRLEN("1.0.0.1"), WOLFSSL_LEFT_MOST_WILDCARD_ONLY, NULL), 0); | ||||||||||
|
|
||||||||||
| wolfSSL_X509_free(cn_wild); | ||||||||||
| wolfSSL_X509_free(cn_lit); | ||||||||||
| } | ||||||||||
|
|
@@ -1610,6 +1618,183 @@ int test_wolfSSL_X509_name_match3(void) | |||||||||
| return EXPECT_RESULT(); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| int test_wolfssl_local_IsValidFQDN(void) { | ||||||||||
| EXPECT_DECLS; | ||||||||||
| #if !defined(NO_ASN) && !defined(WOLFCRYPT_ONLY) && !defined(NO_CERTS) | ||||||||||
| static const struct { const char *str; int is_FQDN; } test_cases[] = { | ||||||||||
| {"example.com", 1}, | ||||||||||
| {"example.com.", 1}, /* trailing dot (absolute form) */ | ||||||||||
| {"sub.example.com", 1}, | ||||||||||
| {"a.b", 1}, /* minimal two-label */ | ||||||||||
| {"xn--nxasmq5b.com", 1}, /* punycode / IDN (ACE form) */ | ||||||||||
| {"test_underscore.example.com", 1}, /* underscore in non-TLD label */ | ||||||||||
| {"_leading.example.com", 1}, /* underscore at start of label */ | ||||||||||
| {"trailing_.example.com", 1},/* underscore at end of non-TLD label */ | ||||||||||
| {"123.numericlabel.example.com", 1}, /* numeric labels are fine */ | ||||||||||
| {"example.12a3", 1}, /* TLD with letters + digits */ | ||||||||||
| {"ex--ample.com", 1}, /* double hyphen inside label (allowed) */ | ||||||||||
| {"A.B.C", 1}, /* uppercase OK (case-insensitive rules) */ | ||||||||||
|
|
||||||||||
| {"example", 0}, /* single label (not fully qualified) */ | ||||||||||
| {"example.", 0}, /* becomes single label after dot strip */ | ||||||||||
| {".example.com", 0}, /* leading dot -- empty first label */ | ||||||||||
| {"example..com", 0}, /* empty label (consecutive dots) */ | ||||||||||
| {"-example.com", 0}, /* label starts with '-' */ | ||||||||||
| {"example-.com", 0}, /* label ends with '-' */ | ||||||||||
| {"example.com-", 0}, /* final label ends with '-' */ | ||||||||||
| {"example.com_", 0}, /* underscore in TLD (forbidden) */ | ||||||||||
| {"example._com", 0}, /* underscore in TLD (forbidden) */ | ||||||||||
| {"ex@mple.com", 0}, /* illegal character '@' */ | ||||||||||
| {"example com.com", 0}, /* illegal character ' ' */ | ||||||||||
| {"", 0}, /* empty string */ | ||||||||||
| {NULL, 0}, /* NULL pointer */ | ||||||||||
| {"com", 0}, /* single label */ | ||||||||||
| {"123.456", 0}, /* all-numeric final label (no alpha) */ | ||||||||||
| {"example.123", 0}, /* all-numeric TLD (no alpha) */ | ||||||||||
| {"a", 0}, /* single label, too short */ | ||||||||||
| {"example.123a", 1}, /* TLD with at least one letter -- valid */ | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| int i; | ||||||||||
| for (i = 0; i < (int)(sizeof(test_cases) / sizeof(test_cases[0])); i++) { | ||||||||||
| ExpectIntEQ(wolfssl_local_IsValidFQDN( | ||||||||||
| test_cases[i].str, | ||||||||||
| test_cases[i].str ? (word32)strlen(test_cases[i].str) : 0), | ||||||||||
| test_cases[i].is_FQDN); | ||||||||||
| if (! EXPECT_SUCCESS()) { | ||||||||||
| fprintf(stderr, "wolfssl_local_IsValidFQDN() wrong result for " | ||||||||||
| "case %d \"%s\"\n", i, test_cases[i].str); | ||||||||||
|
||||||||||
| "case %d \"%s\"\n", i, test_cases[i].str); | |
| "case %d \"%s\"\n", i, | |
| test_cases[i].str ? test_cases[i].str : "(null)"); |
Copilot
AI
Apr 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment above test_wolfSSL_MatchDomainName_idn says MatchDomainName() is WOLFSSL_LOCAL, but in this PR the declaration was changed to WOLFSSL_TEST_VIS in wolfssl/internal.h. Update the comment to reflect the current visibility/export mechanism (or remove the visibility claim) to avoid misleading future readers.
| * MatchDomainName() is WOLFSSL_LOCAL but visible to the test binary because | |
| * tests link against the in-tree library. */ | |
| * MatchDomainName() is exposed for testing via the visibility mechanism | |
| * declared in wolfssl/internal.h. */ |
Copilot
AI
Apr 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_wolfSSL_MatchDomainName_idn is currently only gated on !NO_CERTS, but it calls MatchDomainName() with WOLFSSL_LEFT_MOST_WILDCARD_ONLY, which (after this change) depends on wolfssl_local_IsValidFQDN() from ASN code. If there are configurations that build with NO_ASN (or without BUILD_ASN) but still compile this test, it will fail to build/link. Consider guarding this test with !defined(NO_ASN) (or the same feature macro that guarantees wolfssl_local_IsValidFQDN is available).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MatchDomainName()now callswolfssl_local_IsValidFQDN(), which is only declared when!NO_ASN(viawolfcrypt/asn.h) and implemented inwolfcrypt/src/asn.c(built only with ASN). IfNO_ASNis enabled without also definingNO_CERTS, this introduces a compile/link failure. Consider movingwolfssl_local_IsValidFQDN()intosrc/internal.c(or otherwise ensuring it’s available wheneverMatchDomainName()is built), or wrap the call with#ifndef NO_ASNand provide an alternate path when ASN is disabled.