Skip to content

Fix handling of otherName in ConfirmNameConstraints#10339

Open
embhorn wants to merge 6 commits intowolfSSL:masterfrom
embhorn:zd21707
Open

Fix handling of otherName in ConfirmNameConstraints#10339
embhorn wants to merge 6 commits intowolfSSL:masterfrom
embhorn:zd21707

Conversation

@embhorn
Copy link
Copy Markdown
Member

@embhorn embhorn commented Apr 28, 2026

Description

  • ConfirmNameConstraints ignored the otherName GeneralName form (and the extNameConstraintCrit flag was never propagated to Signer)
  • Adjust default behavior of using intermediate non-CA certs in wolfSSL_X509_verify_cert with compatibility layer enabled. Added WOLFSSL_X509_STORE_ALLOW_NON_CA_INTERMEDIATE for backward compatibility.

Fixes

  • zd21707
  • zd21722

Testing

Added test_NameConstraints_OtherName

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@embhorn embhorn self-assigned this Apr 28, 2026
Copilot AI review requested due to automatic review settings April 28, 2026 16:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR fixes RFC 5280 nameConstraints enforcement for otherName SAN entries and ensures critical nameConstraints behavior is correctly propagated and enforced during verification.

Changes:

  • Add byte-exact otherName matching for permitted/excluded nameConstraints subtrees.
  • Track and fail-closed on critical nameConstraints that contain unsupported GeneralName forms.
  • Propagate extNameConstraintCrit (+ new unsupported-form flag) into Signer, and add a regression test.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
wolfssl/wolfcrypt/asn.h Adds flags to track critical nameConstraints and presence of unsupported constraint forms in DecodedCert/Signer.
wolfcrypt/src/asn.c Implements otherName constraint matching, records unsupported forms during decode, propagates flags, and enforces fail-closed behavior for critical constraints.
tests/api.c Adds a new test that builds a chain using otherName SAN + nameConstraints to verify enforcement.
Comments suppressed due to low confidence (3)

wolfcrypt/src/asn.c:1

  • New fail-closed behavior is introduced for critical nameConstraints containing unsupported GeneralName forms, but the added test suite only covers otherName. Please add a regression test that constructs a critical nameConstraints with an unsupported form (e.g., registeredID, x400Address, or ediPartyName) and asserts verification rejects; also add a non-critical variant asserting verification continues (since the fail-closed requirement is for critical constraints).
    wolfcrypt/src/asn.c:1
  • The otherName byte-exact matching logic (including the WOLFSSL_FPKI oidSum guard) is duplicated in both PermittedListOk and IsInExcludedList. Consider extracting a small helper (e.g., MatchOtherNameConstraint(name, current)) so future updates don’t accidentally diverge between permitted vs excluded handling.
    wolfcrypt/src/asn.c:1
  • The otherName byte-exact matching logic (including the WOLFSSL_FPKI oidSum guard) is duplicated in both PermittedListOk and IsInExcludedList. Consider extracting a small helper (e.g., MatchOtherNameConstraint(name, current)) so future updates don’t accidentally diverge between permitted vs excluded handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread wolfssl/wolfcrypt/asn.h Outdated
Comment thread wolfssl/wolfcrypt/asn.h Outdated
Copy link
Copy Markdown

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #10339

Scan targets checked: wolfcrypt-bugs, wolfcrypt-src

No new issues found in the changed files. ✅

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 29, 2026

MemBrowse Memory Report

gcc-arm-cortex-m4

  • FLASH: .text +192 B (+0.1%, 197,051 B / 262,144 B, total: 75% used)

gcc-arm-cortex-m4-baremetal

  • FLASH: .text +192 B (+0.3%, 64,499 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m4-min-ecc

  • FLASH: .text +192 B (+0.3%, 60,085 B / 262,144 B, total: 23% used)

gcc-arm-cortex-m4-tls12

  • FLASH: .text +192 B (+0.2%, 120,506 B / 262,144 B, total: 46% used)

@embhorn embhorn assigned wolfSSL-Bot and embhorn and unassigned embhorn and wolfSSL-Bot Apr 29, 2026
Copy link
Copy Markdown

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #10339

Scan targets checked: wolfcrypt-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

No new issues found in the changed files. ✅

@embhorn embhorn assigned wolfSSL-Bot and unassigned embhorn Apr 30, 2026
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