Skip to content

Commit 8476563

Browse files
authored
Reject URIs containing '@' in name constraint checking (#3202)
### Issues: Addresses P422947832 ### Description of changes: `nc_uri()` extracts the host from a URI by scanning for `:` (port) then `/` (path start). It does not account for the `@` delimiter that separates optional userinfo from the host in RFC 3986 §3.2 (authority = `[userinfo "@"] host [":" port]`). Since `:` is permitted inside userinfo, a URI like `spiffe://x.team-a.corp:x@team-b.corp/admin` causes nc_uri() to extract x`.team-a.corp` as the host instead of the actual host `team-b.corp` This change makes three improvements to nc_uri(): 1. Reject `@` in authority — URIs containing userinfo are rejected with `X509_V_ERR_UNSUPPORTED_NAME_SYNTAX`. Certificate SAN URIs have no standards-defined use for userinfo, so rejecting is the correct fail-closed behavior — consistent with the IPv6 literal rejection added in `fed51c342`. The @ scan stops at /, ?, or # (the characters that terminate the authority per RFC 3986), so @ appearing in the path (e.g., `foo://example.com/@user`) is not affected. 2. Trailing dot normalization — Strip trailing dots from both the host and constraint before comparison, so `team-a.corp.` and `team-a.corp` are treated as the same FQDN (RFC 1034 §3.1). This matches the existing normalization in nc_dns(). 3. FQDN validation — Validate that the extracted host contains only characters valid in a DNS name (`a-zA-Z0-9-.`) per RFC 1034 §3.5. RFC 5280 §4.2.1.10 requires the host to be a fully qualified domain name, so any host containing percent-encoding, `?`, `#`, or other non-FQDN characters is rejected. This prevents equivalence bypasses (e.g., `b%61d.com` evading an exclusion for .bad.com). ### Call-outs: This intentionally rejects rather than normalizes percent-encoded hosts. RFC 5280 requires the host to be a FQDN, and `%` is not a valid FQDN character — a conformant CA should never issue such a cert. Rejecting is stricter than the RFC 3986 §6.2.2.2 SHOULD-level normalization guidance, but avoids being more permissive than the standard. ### Testing: Nine new cases in `X509Test.NameConstraints`: - Basic `user@host` rejection - Colon-in-userinfo bypass (`spiffe://x.team-a.corp:x@team-b.corp/admin`) tested against both the fake and real host constraints - Userinfo with path, query, and fragment variants - `@` in path (after `/`) is correctly not rejected All 102 existing X509 tests pass. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
1 parent 64d6586 commit 8476563

2 files changed

Lines changed: 106 additions & 0 deletions

File tree

crypto/x509/v3_ncons.c

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,21 @@ static int nc_uri(const ASN1_IA5STRING *uri, const ASN1_IA5STRING *base) {
686686
return X509_V_ERR_UNSUPPORTED_NAME_SYNTAX;
687687
}
688688

689+
// RFC 3986 §3.2 defines authority = [userinfo "@"] host [":" port].
690+
// Certificate SAN URIs have no standards-defined use for userinfo. If present,
691+
// the '@' delimiter would cause the host extraction below to parse the
692+
// userinfo as the host, matching against attacker-chosen bytes instead of
693+
// the URI's actual host.
694+
for (size_t i = 0; i < CBS_len(&uri_cbs); i++) {
695+
uint8_t c = CBS_data(&uri_cbs)[i];
696+
if (c == '/' || c == '?' || c == '#') {
697+
break;
698+
}
699+
if (c == '@') {
700+
return X509_V_ERR_UNSUPPORTED_NAME_SYNTAX;
701+
}
702+
}
703+
689704
// Look for a port indicator as end of hostname first. Otherwise look for
690705
// trailing slash, or the end of the string.
691706
CBS host;
@@ -698,6 +713,35 @@ static int nc_uri(const ASN1_IA5STRING *uri, const ASN1_IA5STRING *base) {
698713
return X509_V_ERR_UNSUPPORTED_NAME_SYNTAX;
699714
}
700715

716+
// Normalize absolute DNS names by removing the trailing dot, if any.
717+
// RFC 1034 §3.1 defines that a trailing dot denotes the DNS root; names with
718+
// and without it refer to the same host. This matches the normalization in
719+
// nc_dns().
720+
if (ends_with_byte(&host, '.')) {
721+
uint8_t unused;
722+
CBS_get_last_u8(&host, &unused);
723+
}
724+
if (ends_with_byte(&base_cbs, '.')) {
725+
uint8_t unused;
726+
CBS_get_last_u8(&base_cbs, &unused);
727+
}
728+
729+
// RFC 5280 §4.2.1.10 requires the host to be a fully qualified domain name.
730+
// Validate that the host contains only characters valid in a DNS name
731+
// (RFC 1034 §3.5): letters, digits, hyphens, and dots. This rejects
732+
// percent-encoded hosts and any other non-FQDN syntax, preventing
733+
// equivalence bypasses (e.g., "b%61d.com" evading ".bad.com").
734+
if (CBS_len(&host) == 0) {
735+
return X509_V_ERR_UNSUPPORTED_NAME_SYNTAX;
736+
}
737+
for (size_t i = 0; i < CBS_len(&host); i++) {
738+
uint8_t c = CBS_data(&host)[i];
739+
if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') ||
740+
(c >= '0' && c <= '9') || c == '-' || c == '.')) {
741+
return X509_V_ERR_UNSUPPORTED_NAME_SYNTAX;
742+
}
743+
}
744+
701745
// Special case: initial '.' is RHS match
702746
if (starts_with(&base_cbs, '.')) {
703747
if (has_suffix_case(&host, &base_cbs)) {

crypto/x509/x509_test.cc

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2640,6 +2640,68 @@ TEST(X509Test, NameConstraints) {
26402640
// An incomplete IPv6 literal is also rejected.
26412641
{GEN_URI, "foo://[2001:db8::1", "[2001:db8::1]",
26422642
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
2643+
2644+
// RFC 3986 §3.2 defines authority = [userinfo "@"] host [":" port].
2645+
// URIs with userinfo are rejected to prevent host confusion: without
2646+
// this, "spiffe://x.team-a.corp:x@team-b.corp/admin" would be matched
2647+
// against "x.team-a.corp" instead of the actual host "team-b.corp",
2648+
// bypassing permittedSubtrees or evading excludedSubtrees.
2649+
//
2650+
// Basic userinfo before host.
2651+
{GEN_URI, "foo://user@example.com", "example.com",
2652+
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
2653+
// Userinfo with colon (user:password style) — the colon in userinfo
2654+
// would previously be mistaken for a port delimiter, extracting the
2655+
// userinfo prefix as the host.
2656+
{GEN_URI, "spiffe://x.team-a.corp:x@team-b.corp/admin", "team-a.corp",
2657+
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
2658+
{GEN_URI, "spiffe://x.team-a.corp:x@team-b.corp/admin", "team-b.corp",
2659+
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
2660+
// Userinfo with path, query, and fragment components.
2661+
{GEN_URI, "foo://user@example.com/path", "example.com",
2662+
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
2663+
{GEN_URI, "foo://user@example.com?query", "example.com",
2664+
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
2665+
{GEN_URI, "foo://user@example.com#fragment", "example.com",
2666+
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
2667+
// '@' in path (after '/') is not userinfo and should not be rejected.
2668+
{GEN_URI, "foo://example.com/@user", "example.com", X509_V_OK,
2669+
X509_V_ERR_EXCLUDED_VIOLATION},
2670+
{GEN_URI, "foo://example.com/path@thing", ".example.com",
2671+
X509_V_ERR_PERMITTED_VIOLATION, X509_V_OK},
2672+
// '@' after '?' or '#' is not in the authority and is not rejected.
2673+
// However, since the host parser doesn't treat '?' or '#' as authority
2674+
// terminators, the extracted host contains invalid FQDN characters and
2675+
// is rejected by FQDN validation.
2676+
{GEN_URI, "foo://example.com?x@y", "example.com",
2677+
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
2678+
{GEN_URI, "foo://example.com#x@y", "example.com",
2679+
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
2680+
// Empty userinfo and user:pass userinfo are both rejected.
2681+
{GEN_URI, "foo://@example.com", "example.com",
2682+
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
2683+
{GEN_URI, "foo://user:pass@example.com", "example.com",
2684+
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
2685+
2686+
// Trailing dot normalization: "host." and "host" are the same FQDN.
2687+
// This matches the normalization in nc_dns().
2688+
{GEN_URI, "spiffe://admin.team-a.corp./admin", ".team-a.corp",
2689+
X509_V_OK, X509_V_ERR_EXCLUDED_VIOLATION},
2690+
{GEN_URI, "spiffe://admin.team-a.corp/admin", ".team-a.corp.",
2691+
X509_V_OK, X509_V_ERR_EXCLUDED_VIOLATION},
2692+
{GEN_URI, "spiffe://team-a.corp./admin", "team-a.corp",
2693+
X509_V_OK, X509_V_ERR_EXCLUDED_VIOLATION},
2694+
2695+
// FQDN validation: hosts must contain only a-z, A-Z, 0-9, '-', '.'.
2696+
// Percent-encoded hosts are rejected, preventing equivalence bypasses
2697+
// (e.g., "b%61d.com" should not evade an exclusion for ".bad.com").
2698+
{GEN_URI, "spiffe://evil.b%61d.com/resource", ".bad.com",
2699+
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
2700+
{GEN_URI, "foo://ex%61mple.com/path", "example.com",
2701+
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
2702+
// Underscores are not valid in FQDNs.
2703+
{GEN_URI, "foo://my_host.example.com/path", ".example.com",
2704+
X509_V_ERR_UNSUPPORTED_NAME_SYNTAX, X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
26432705
};
26442706
for (const auto &t : kTests) {
26452707
SCOPED_TRACE(t.type);

0 commit comments

Comments
 (0)