Skip to content

NameConstraints fixes#10638

Open
rizlik wants to merge 6 commits into
wolfSSL:masterfrom
rizlik:nc_uri_trailing_dot
Open

NameConstraints fixes#10638
rizlik wants to merge 6 commits into
wolfSSL:masterfrom
rizlik:nc_uri_trailing_dot

Conversation

@rizlik

@rizlik rizlik commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Description

  • fails closed if URI constraints are present but a presented URI's host is not a DNS name (RFC 5280 4.2.10)
  • normalize traling dot so that host.com. and host.com denote the same host
  • consolidate compat layer

Copilot AI review requested due to automatic review settings June 8, 2026 17:50

Copilot AI left a comment

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.

Pull request overview

This PR tightens X.509 nameConstraints handling for URI GeneralNames so that when URI constraints are present, URIs whose host is not a DNS reg-name (e.g., IP-literals in brackets or IPv4address hosts) are rejected (“fail closed”), aligning with RFC 5280’s URI host requirements.

Changes:

  • Add URI host extraction/type detection in wolfcrypt/src/asn.c and expose an internal helper to detect whether a URI has a DNS reg-name host.
  • Enforce “fail closed” behavior when URI constraints exist but the presented URI host is not DNS (both in core verification and the OpenSSL-compat wolfSSL_NAME_CONSTRAINTS_check_name path).
  • Update/extend unit tests to cover trailing-dot FQDN normalization and the new rejection behavior for IP-literal/IPv4 hosts under URI constraints.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
wolfssl/wolfcrypt/asn.h Declares internal helper for determining whether a URI has a DNS reg-name host.
wolfcrypt/src/asn.c Implements URI host parsing/type classification, applies DNS-host requirement when URI constraints are present, and normalizes trailing-dot behavior for exact-host URI constraints.
src/x509.c Removes local URI host extraction and routes URI name constraint matching through wolfssl_local_MatchUriNameConstraint; adds fail-closed check for non-DNS URI hosts when URI constraints exist.
tests/api/test_asn.c Expands URI name-constraint tests for trailing-dot equivalence and rejection of IP-literal/IPv4 hosts.
tests/api.c Adds integration-style verification cases to ensure non-DNS URI hosts are rejected when URI constraints are applied (including excluded-only constraints).
tests/api/test_ossl_x509_ext.c Updates test commentary to reflect DNS-host requirement for URI constraints.

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

rizlik added 3 commits June 15, 2026 09:58
Pull the URI host extraction (scheme skip, userinfo skip, IP-literal
brackets, port/path delimiters) into a GetUriHost helper so it can be
reused by other URI name-constraint checks. No functional change.
RFC 5280 4.2.1.10 defines URI name constraints in terms of a host that
is a fully qualified domain name; RFC 3986 IP-literal ([...]) and
IPv4address hosts are not DNS reg-names and cannot be meaningfully
matched against a DNS-style constraint base.

- Classify the host extracted by GetUriHost (IP-literal, IPv4address,
  reg-name) and validate that a reg-name has no empty labels.
- wolfssl_local_MatchUriNameConstraint() no longer matches URIs whose
  host is an IP address.
- ConfirmNameConstraints() fails closed: when URI constraints are
  present, a URI SAN without a DNS host is rejected. A plain non-match
  would otherwise let such names pass excluded-only constraints.
One trailing dot marks an absolute FQDN and is not part of the host:
"host.com." and "host.com" denote the same host. Strip it from the
URI host before classification (so "12.31.2.3." is still recognized
as an IPv4 address) and from the constraint base before the exact-match
comparison, mirroring what wolfssl_local_MatchBaseName() already does
for DNS name constraints. Only a single dot is the marker: an empty
last label ("host.com..") is rejected.
@rizlik rizlik force-pushed the nc_uri_trailing_dot branch from 5bdb96c to 44a22af Compare June 15, 2026 10:09
rizlik added 3 commits June 15, 2026 12:09
Add wolfssl_local_MatchDnsNameConstraint() dispatching wildcard names
to the subtree matcher and literal names to plain base-name matching,
and use it for the ASN_DNS_TYPE branches of PermittedListOk() and
IsInExcludedList().

This also drops the outer name->len >= base len byte-length guard for
literal DNS names. That guard ran before MatchBaseName() could strip
the absolute-FQDN trailing dot, so a constraint base like
DNS:example.com. never matched the SAN example.com it denotes.
Replace ExtractHostFromUri() plus DNS-style base matching in
MatchNameConstraint() with wolfssl_local_MatchUriNameConstraint(), and
make wolfSSL_NAME_CONSTRAINTS_check_name() fail closed like
ConfirmNameConstraints(): when URI subtrees are present, a URI name
without a DNS host is rejected instead of passing excluded-only
constraints as a plain non-match.

This aligns the compat layer with RFC 5280 URI constraint semantics: a
base without a leading dot now matches the host exactly instead of as a
DNS subtree, and IP hosts no longer match at all.
MatchNameConstraint() compared wildcard DNS SANs literally, so
*.example.com was not rejected by an excluded subtree covering
foo.example.com. Route WOLFSSL_GEN_DNS through
wolfssl_local_MatchDnsNameConstraint(), passing the subtree direction:
permitted subtrees require every wildcard expansion to stay inside the
subtree, excluded subtrees reject when any expansion can fall inside.
This matches what ConfirmNameConstraints() already does.
@rizlik rizlik marked this pull request as ready for review June 15, 2026 10:13
@rizlik rizlik changed the title wip: name constraint fixes NameConstraints fixes Jun 15, 2026
@github-actions

Copy link
Copy Markdown

retest this please

@github-actions

Copy link
Copy Markdown

MemBrowse Memory Report

gcc-arm-cortex-m0plus

  • FLASH: .text +380 B (+0.6%, 63,875 B / 262,144 B, total: 24% used)

gcc-arm-cortex-m3

  • FLASH: .text +420 B (+0.3%, 121,813 B / 262,144 B, total: 46% used)

gcc-arm-cortex-m4

  • FLASH: .text +448 B (+0.2%, 199,504 B / 262,144 B, total: 76% used)

gcc-arm-cortex-m4-baremetal

  • FLASH: .text +448 B (+0.7%, 66,507 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m4-crypto-only

  • FLASH: .text +448 B (+0.3%, 174,126 B / 262,144 B, total: 66% used)

gcc-arm-cortex-m4-dtls13

  • FLASH: .text +384 B (+0.2%, 180,120 B / 1,048,576 B, total: 17% used)

gcc-arm-cortex-m4-min-ecc

  • FLASH: .text +384 B (+0.6%, 61,421 B / 262,144 B, total: 23% used)

gcc-arm-cortex-m4-openssl-compat

  • FLASH: .text +384 B (+0.1%, 768,116 B / 1,048,576 B, total: 73% used)

gcc-arm-cortex-m4-pkcs7

  • FLASH: .text +448 B (+0.2%, 211,761 B / 262,144 B, total: 81% used)

gcc-arm-cortex-m4-pq

  • FLASH: .text +448 B (+0.2%, 278,328 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m4-rsa-only

  • FLASH: .text +448 B (+0.1%, 323,928 B / 1,048,576 B, total: 31% used)

gcc-arm-cortex-m4-sp-math

  • FLASH: .text +384 B (+0.6%, 61,421 B / 262,144 B, total: 23% used)

gcc-arm-cortex-m4-tls12

  • FLASH: .text +448 B (+0.4%, 122,573 B / 262,144 B, total: 47% used)

gcc-arm-cortex-m4-tls13

  • FLASH: .text +448 B (+0.2%, 235,138 B / 262,144 B, total: 90% used)

gcc-arm-cortex-m7

  • FLASH: .text +384 B (+0.2%, 199,440 B / 262,144 B, total: 76% used)

gcc-arm-cortex-m7-pq

  • FLASH: .text +448 B (+0.2%, 278,904 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m7-tls13

  • FLASH: .text +384 B (+0.2%, 235,138 B / 262,144 B, total: 90% used)

linuxkm-pie

  • Data: __patchable_function_entries +48 B (+0.2%, 24,336 B)

linuxkm-standard

  • Data: __patchable_function_entries +32 B (+0.1%, 46,040 B)

stm32-sim-stm32h753

  • FLASH: .text +576 B (+0.3%, 182,260 B / 2,097,152 B, total: 9% used)

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

@rizlik rizlik requested a review from cconlon June 15, 2026 13:16
@rizlik rizlik assigned cconlon and wolfSSL-Bot and unassigned rizlik Jun 15, 2026
@rizlik rizlik requested a review from dgarske June 18, 2026 10:10
@dgarske dgarske added the Not For This Release Not for release 5.9.2 label Jun 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Not For This Release Not for release 5.9.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants