Repository navigation
Fix CN fallback handling in name constraints checking - #3108
Merged
samuel40791765 merged 1 commit intoMar 19, 2026
Merged
Conversation
samuel40791765
had a problem deploying
to
auto-approve
March 19, 2026 15:38 — with 
GitHub Actions
Error
GitHub Actions
Error
samuel40791765
had a problem deploying
to
auto-approve
March 19, 2026 15:38 — with
GitHub Actions
Error
GitHub Actions
Error
samuel40791765
had a problem deploying
to
auto-approve
March 19, 2026 15:38 — with
GitHub Actions
Error
GitHub Actions
Error
samuel40791765
had a problem deploying
to
auto-approve
March 19, 2026 15:38 — with
GitHub Actions
Failure
GitHub Actions
Failure
samuel40791765
had a problem deploying
to
auto-approve
March 19, 2026 15:38 — with
GitHub Actions
Error
GitHub Actions
Error
samuel40791765
had a problem deploying
to
auto-approve
March 19, 2026 15:38 — with
GitHub Actions
Error
GitHub Actions
Error
samuel40791765
enabled auto-merge (squash)
March 19, 2026 15:45
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| // Normalize absolute DNS names by removing the trailing dot, if any. | ||
| if (ends_with_byte(&dns_cbs, '.')) { | ||
| uint8_t unused; |
Contributor
There was a problem hiding this comment.
warning: variable 'unused' is not initialized [cppcoreguidelines-init-variables]
Suggested change
| uint8_t unused; | |
| = 0 |
| CBS_get_last_u8(&dns_cbs, &unused); | ||
| } | ||
| if (ends_with_byte(&base_cbs, '.')) { | ||
| uint8_t unused; |
Contributor
There was a problem hiding this comment.
warning: variable 'unused' is not initialized [cppcoreguidelines-init-variables]
Suggested change
| uint8_t unused; | |
| = 0 |
WillChilds-Klein
approved these changes
Mar 19, 2026
justsmth
approved these changes
Mar 19, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## fips-2025-09-12 #3108 +/- ##
===================================================
+ Coverage 78.73% 78.75% +0.02%
===================================================
Files 667 667
Lines 113124 113213 +89
Branches 16057 16072 +15
===================================================
+ Hits 89072 89165 +93
+ Misses 23277 23265 -12
- Partials 775 783 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
abelianbee
added a commit
to abelianbee/aws-lc
that referenced
this pull request
Oct 1, 2026
…only check_name_constraints() ran NAME_CONSTRAINTS_check_CN() on every certificate in the chain that has no dNSName SAN, including intermediate CAs. The CN fallback exists because X509_check_host() may match a leaf's commonName as a hostname when there is no SAN; a CA's commonName is never used that way. OpenSSL 1.1.1, which this code was reintroduced from in aws#2376, restricts the check to the leaf (i == 0). Since aws#3108 made a multi-label CN with spaces an error instead of a non-DNS name, the extra check rejects chains through ordinary intermediates such as "CN=ACME Corp. Issuing CA G2" under a name-constrained root with X509_V_ERR_UNSUPPORTED_NAME_SYNTAX at depth 1, where OpenSSL verifies them. Restore the leaf-only condition. The leaf behavior from aws#3108 is unchanged.
justsmth
pushed a commit
to abelianbee/aws-lc
that referenced
this pull request
Oct 14, 2026
…only check_name_constraints() ran NAME_CONSTRAINTS_check_CN() on every certificate in the chain that has no dNSName SAN, including intermediate CAs. The CN fallback exists because X509_check_host() may match a leaf's commonName as a hostname when there is no SAN; a CA's commonName is never used that way. OpenSSL 1.1.1, which this code was reintroduced from in aws#2376, restricts the check to the leaf (i == 0). Since aws#3108 made a multi-label CN with spaces an error instead of a non-DNS name, the extra check rejects chains through ordinary intermediates such as "CN=ACME Corp. Issuing CA G2" under a name-constrained root with X509_V_ERR_UNSUPPORTED_NAME_SYNTAX at depth 1, where OpenSSL verifies them. Restore the leaf-only condition. The leaf behavior from aws#3108 is unchanged.
justsmth
pushed a commit
to abelianbee/aws-lc
that referenced
this pull request
Oct 28, 2026
…only check_name_constraints() ran NAME_CONSTRAINTS_check_CN() on every certificate in the chain that has no dNSName SAN, including intermediate CAs. The CN fallback exists because X509_check_host() may match a leaf's commonName as a hostname when there is no SAN; a CA's commonName is never used that way. OpenSSL 1.1.1, which this code was reintroduced from in aws#2376, restricts the check to the leaf (i == 0). Since aws#3108 made a multi-label CN with spaces an error instead of a non-DNS name, the extra check rejects chains through ordinary intermediates such as "CN=ACME Corp. Issuing CA G2" under a name-constrained root with X509_V_ERR_UNSUPPORTED_NAME_SYNTAX at depth 1, where OpenSSL verifies them. Restore the leaf-only condition. The leaf behavior from aws#3108 is unchanged.
justsmth
pushed a commit
that referenced
this pull request
Nov 2, 2026
…only (#3603) ### Related issues Addresses #3521 ### Context and motivation `check_name_constraints()` runs `NAME_CONSTRAINTS_check_CN()` on every chain certificate without a dNSName SAN, intermediates included. The fallback exists because `X509_check_host()` may match a leaf's commonName as a hostname when there is no SAN. A CA's commonName is never used that way, and OpenSSL 1.1.1, which #2376 reintroduced this code from, applies it to the leaf only (`i == 0`). That was harmless until #3108: a CN like `ACME Corp. Issuing CA G2` has spaces, so `cn2dnsid()` skipped it as a non-DNS name. #3108 made a multi-label CN containing a space an error instead, so a chain through an intermediate with an ordinary organization name under a name-constrained root now fails with `X509_V_ERR_UNSUPPORTED_NAME_SYNTAX` at depth 1. OpenSSL verifies the same chain. ### Description of changes Restore `i == 0` so the fallback applies to the end-entity certificate only. That drops the CN check on every intermediate, including ones whose CN does parse as a DNS name: an intermediate with `CN=issuing.evil.com` under a root permitting only `.example.com` used to fail with `X509_V_ERR_PERMITTED_VIOLATION` and now verifies. Intermediate CNs are never matched as hostnames, so a name constraint on them has nothing to protect, and OpenSSL and pre-#2376 AWS-LC behave the same way. `NAME_CONSTRAINTS_check()` still runs against every certificate, so directoryName and SAN constraints on intermediates are unaffected, and the leaf behavior from #3108 is unchanged. This does not decide whether a space should make a leaf CN an error or a non-DNS name, which is the other half of #3521. ### Testing - `X509Test.NameConstraintsIntermediateCN`: name-constrained root, intermediate with `CN=ACME Corp. Issuing CA G2` and no SAN, three leaves. One inside the permitted subtree verifies, one outside still gets `X509_V_ERR_PERMITTED_VIOLATION`, one with `CN=foo .evil.com` still gets `X509_V_ERR_UNSUPPORTED_NAME_SYNTAX`. On main the first two fail with error 53 at the intermediate. A fourth leaf goes through an intermediate with `CN=issuing.evil.com`, which fails with error 47 on main. - Reproduced #3521 with an OpenSSL CLI chain and a verifier linked against this tree and against OpenSSL 3.6.4: `verify=0 err=53 depth=1` before, `ok` on OpenSSL, both `ok` after. - Full `crypto_test` passes on macOS arm64. 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.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes:
This change fixes two issues in name constraints enforcement:
cn2dnsidnow recognizes wildcard CNs (leading*.) as DNS identifiers by skipping the prefix during DNS syntax validation. The full wildcard string is returned sonc_dnscan perform suffix matching against permitted/excluded subtrees.nc_dnsnow handles wildcard DNS names against excluded subtrees. An exclusion offoo.example.comcorrectly matches*.example.comby comparing parent domains when the DNS name starts with*. This is based on upstream commit: google/boringssl@5774ecaCall-outs:
This change makes name constraints enforcement stricter. Certificates that were previously accepted (due to wildcard CNs being skipped) will now be checked against their CA's name constraints. Only certificates that were already in violation of their CA's constraints will be newly rejected.
Testing:
NameConstraintsinx509_test.ccto test both permitted and excluded subtrees. Also taken from upstream commit referenced above.CommonNameToDNSinx509_compat_test.cc.NameConstraintsWildcardCNinx509_test.ccto test the end-to-end CN fallback path with wildcard CNs against both permitted and excluded subtrees.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.