Skip to content

Fix CN fallback handling in name constraints checking#3107

Merged
samuel40791765 merged 1 commit intoaws:mainfrom
samuel40791765:fix-nm-constraints
Mar 19, 2026
Merged

Fix CN fallback handling in name constraints checking#3107
samuel40791765 merged 1 commit intoaws:mainfrom
samuel40791765:fix-nm-constraints

Conversation

@samuel40791765
Copy link
Copy Markdown
Contributor

Description of changes:

This change fixes two issues in name constraints enforcement:

  1. cn2dnsid now recognizes wildcard CNs (leading *.) as DNS identifiers by skipping the prefix during DNS syntax validation. The full wildcard string is returned so nc_dns can perform suffix matching against permitted/excluded subtrees.

  2. nc_dns now handles wildcard DNS names against excluded subtrees. An exclusion of foo.example.com correctly matches *.example.com by comparing parent domains when the DNS name starts with *. This is based on upstream commit: google/boringssl@5774eca

Call-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:

  • Expanded NameConstraints in x509_test.cc to test both permitted and excluded subtrees. Also taken from upstream commit referenced above.
  • Added wildcard CN test cases to CommonNameToDNS in x509_compat_test.cc.
  • Added NameConstraintsWildcardCN in x509_test.cc to 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.

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions


// Normalize absolute DNS names by removing the trailing dot, if any.
if (ends_with_byte(&dns_cbs, '.')) {
uint8_t unused;
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.

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;
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.

warning: variable 'unused' is not initialized [cppcoreguidelines-init-variables]

Suggested change
uint8_t unused;
= 0

@samuel40791765 samuel40791765 enabled auto-merge (squash) March 19, 2026 15:44
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.20%. Comparing base (a9e26fe) to head (5411d52).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crypto/x509/x509_test.cc 80.64% 7 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3107      +/-   ##
==========================================
+ Coverage   78.18%   78.20%   +0.02%     
==========================================
  Files         689      689              
  Lines      121825   121914      +89     
  Branches    16994    17009      +15     
==========================================
+ Hits        95247    95344      +97     
+ Misses      25694    25681      -13     
- Partials      884      889       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@samuel40791765 samuel40791765 merged commit 2aa5224 into aws:main Mar 19, 2026
448 of 456 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants