Skip to content

Restore error code from DecodeGeneralName#10793

Open
embhorn wants to merge 1 commit into
wolfSSL:masterfrom
embhorn:gh10790
Open

Restore error code from DecodeGeneralName#10793
embhorn wants to merge 1 commit into
wolfSSL:masterfrom
embhorn:gh10790

Conversation

@embhorn

@embhorn embhorn commented Jun 26, 2026

Copy link
Copy Markdown
Member

Description

PR #10279 added DecodeGeneralNameCheckChars, which returns ASN_PARSE_E for an embedded NUL in dNSName/rfc822Name/URI SANs, aborting the whole certificate parse before hostname matching ever runs. This broke cert testing in curl and changed the expected failure mode.

Fixes #10790

Dependency on fix in Python OSP wolfSSL/osp#347

Testing

  • tests/api/test_asn.c — updated the NUL sub-case: the cert now must parse (was ASN_PARSE_E), with the entry stored and the embedded NUL preserved (length 4, not truncated).
  • tests/api/test_ossl_x509.c — new test_wolfSSL_X509_check_host_embedded_nul_san: the exact curl-311 scenario (CN=localhost, SAN=localhost\0h). Pins that the cert parses and the host is still rejected (correct CN must not rescue the wrong SAN).

Checklist

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

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

Adjusts X.509 SubjectAltName parsing so embedded NUL bytes in IA5String-based GeneralNames (dNSName/rfc822Name/URI) do not abort certificate parsing with ASN_PARSE_E, restoring the expected verification-time failure mode (e.g., DOMAIN_NAME_MISMATCH) and fixing the curl test 311 regression described in #10790.

Changes:

  • Stop rejecting embedded-NUL IA5String SAN values at parse time by removing DecodeGeneralNameCheckChars from both ASN parsers.
  • Add/adjust tests to ensure certificates with embedded-NUL dNSName SANs still parse and store the full (non-truncated) value, while hostname verification still fails.
  • Update doxygen to warn that wolfSSL_X509_get_next_altname() returns a C string without a length and can be silently truncated by embedded NULs.

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
wolfcrypt/src/asn.c Removes parse-time NUL rejection for GeneralName IA5String SANs; documents rationale in-code.
wolfcrypt/src/asn_orig.c Mirrors the same behavior change in the legacy ASN parser.
tests/api/test_asn.c Updates SAN tests to require parse success and verify embedded NUL is preserved in stored altName data.
tests/api/test_ossl_x509.h Registers the new regression test in the ossl_x509 test group.
tests/api/test_ossl_x509.c Adds regression test reproducing curl-311 scenario (good CN, bad SAN with embedded NUL).
doc/dox_comments/header_files/ssl.h Adds API documentation warning about embedded NUL truncation with wolfSSL_X509_get_next_altname().

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

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #10793

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

No new issues found in the changed files. ✅

@embhorn embhorn marked this pull request as ready for review June 26, 2026 16:34
@github-actions

Copy link
Copy Markdown

retest this please

@github-actions

Copy link
Copy Markdown

MemBrowse Memory Report

gcc-arm-cortex-m0plus

  • FLASH: .text -56 B (-0.1%, 63,439 B / 262,144 B, total: 24% used)

gcc-arm-cortex-m3

  • FLASH: .text -56 B (-0.0%, 121,353 B / 262,144 B, total: 46% used)

gcc-arm-cortex-m4

  • FLASH: .text -64 B (-0.0%, 198,988 B / 262,144 B, total: 76% used)

gcc-arm-cortex-m4-baremetal

  • FLASH: .text -64 B (-0.1%, 65,995 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m4-crypto-only

  • FLASH: .text -64 B (-0.0%, 173,610 B / 262,144 B, total: 66% used)

gcc-arm-cortex-m4-dtls13

  • FLASH: .text -64 B (-0.0%, 179,736 B / 1,048,576 B, total: 17% used)

gcc-arm-cortex-m4-min-ecc

  • FLASH: .text -64 B (-0.1%, 60,973 B / 262,144 B, total: 23% used)

gcc-arm-cortex-m4-openssl-compat

  • FLASH: .text -64 B (-0.0%, 768,068 B / 1,048,576 B, total: 73% used)

gcc-arm-cortex-m4-pkcs7

  • FLASH: .text -64 B (-0.0%, 211,309 B / 262,144 B, total: 81% used)

gcc-arm-cortex-m4-rsa-only

  • FLASH: .text -64 B (-0.0%, 323,408 B / 1,048,576 B, total: 31% used)

gcc-arm-cortex-m4-sp-math

  • FLASH: .text -64 B (-0.1%, 60,973 B / 262,144 B, total: 23% used)

gcc-arm-cortex-m4-tls12

  • FLASH: .text -64 B (-0.1%, 122,125 B / 262,144 B, total: 47% used)

gcc-arm-cortex-m4-tls13

  • FLASH: .text -64 B (-0.0%, 234,686 B / 262,144 B, total: 90% used)

gcc-arm-cortex-m7

  • FLASH: .text -64 B (-0.0%, 198,988 B / 262,144 B, total: 76% used)

gcc-arm-cortex-m7-tls13

  • FLASH: .text -64 B (-0.0%, 234,686 B / 262,144 B, total: 90% used)

linuxkm-pie

  • Data: __patchable_function_entries -8 B (-0.0%, 24,280 B)

stm32-sim-stm32h753

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.

[Bug]: too generic return code for cert name mismatch

3 participants