Reject certificates with trailing data. Avoid copying trailing data to WOLFSSL_X509 structs.

[kareem-wolfssl]

Description

Fixes zd#22010

Testing

Provided reproducer, built-in tests

Checklist

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

[Copilot]

Pull request overview

This PR tightens X.509 parsing behavior to prevent accepting and re-emitting certificates that have extra trailing bytes beyond the certificate’s outer ASN.1 SEQUENCE, while preserving support for OpenSSL/RFC “TRUSTED CERTIFICATE” blobs that legitimately carry auxiliary trust data after the cert.

Changes:

• Add strict-mode detection/rejection of trailing bytes after the certificate’s outer SEQUENCE (with an internal opt-out for TRUSTED_CERT_TYPE).
• Ensure WOLFSSL_X509 DER storage never includes trailing bytes by truncating copies to the parsed outer-SEQUENCE end.
• Adjust TRUSTED_CERT_TYPE handling to be recognized centrally in ParseCertRelative and fix an API test to use the actual DER length.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
wolfssl/wolfcrypt/asn.h	Adds DecodedCert.allowTrailing bit to permit trusted-cert auxiliary trailing data internally.
wolfcrypt/src/asn.c	Rejects trailing bytes in strict builds; centralizes TRUSTED_CERT_TYPE handling to allow aux trailing data.
tests/api.c	Fixes a test to use the returned DER length (avoids unintended trailing bytes).
src/x509.c	Uses ParseCertRelative’s TRUSTED_CERT_TYPE handling and clarifies trusted-cert truncation behavior.
src/internal.c	Truncates stored DER in X509/ACERT objects to the end of the outer SEQUENCE (prevents re-emitting trailing bytes).

---

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

[github-actions]

MemBrowse Memory Report

gcc-arm-cortex-m0plus

• FLASH: .text +96 B (+0.1%, 64,279 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m3

• FLASH: .text +40 B (+0.0%, 122,559 B / 262,144 B, total: 47% used)

gcc-arm-cortex-m4

• FLASH: .rodata.CSWTCH.1 +28 B, .rodata.str1.1 +332 B, .rodata.wolfSSL_ERR_reason_error_string.str1.1 +31 B (+0.2%, 200,789 B / 262,144 B, total: 77% used)

gcc-arm-cortex-m4-baremetal

• FLASH: .text +64 B (+0.1%, 66,891 B / 262,144 B, total: 26% used)

gcc-arm-cortex-m4-crypto-only

• FLASH: .rodata.CSWTCH.1 +28 B, .rodata.str1.1 +332 B, .text +64 B (+0.2%, 174,992 B / 262,144 B, total: 67% used)

gcc-arm-cortex-m4-dtls13

• FLASH: .rodata +12 B, .text +192 B (+0.1%, 181,092 B / 1,048,576 B, total: 17% used)

gcc-arm-cortex-m4-min-ecc

• FLASH: .text +64 B (+0.1%, 61,805 B / 262,144 B, total: 24% used)

gcc-arm-cortex-m4-openssl-compat

• FLASH: .rodata +496 B, .text -2,432 B (-0.3%, 769,276 B / 1,048,576 B, total: 73% used)
• RAM: .bss +192 B (+0.1%, 136,936 B / 262,144 B, total: 52% used)

gcc-arm-cortex-m4-pkcs7

• FLASH: .rodata.CSWTCH.1 +28 B, .rodata.str1.1 +332 B, .text +256 B (+0.3%, 213,396 B / 262,144 B, total: 81% used)

gcc-arm-cortex-m4-pq

• FLASH: .rodata +412 B, .text +192 B (+0.2%, 280,064 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m4-rsa-only

• FLASH: .rodata +408 B, .text +512 B (+0.3%, 325,976 B / 1,048,576 B, total: 31% used)

gcc-arm-cortex-m4-sp-math

• FLASH: .text +64 B (+0.1%, 61,805 B / 262,144 B, total: 24% used)

gcc-arm-cortex-m4-tls12

• FLASH: .text +64 B (+0.1%, 123,355 B / 262,144 B, total: 47% used)

gcc-arm-cortex-m4-tls13

• FLASH: .rodata.CSWTCH.1 +28 B, .rodata.str1.1 +332 B, .rodata.wolfSSL_ERR_reason_error_string.str1.1 +31 B, .text +128 B (+0.2%, 236,679 B / 262,144 B, total: 90% used)

gcc-arm-cortex-m7

• FLASH: .rodata.CSWTCH.1 +28 B, .rodata.str1.1 +332 B, .rodata.wolfSSL_ERR_reason_error_string.str1.1 +31 B, .text +64 B (+0.2%, 200,789 B / 262,144 B, total: 77% used)

gcc-arm-cortex-m7-pq

• FLASH: .rodata +412 B, .text +192 B (+0.2%, 280,640 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m7-tls13

• FLASH: .rodata.CSWTCH.1 +28 B, .rodata.str1.1 +332 B, .rodata.wolfSSL_ERR_reason_error_string.str1.1 +31 B, .text +192 B (+0.3%, 236,743 B / 262,144 B, total: 90% used)

linuxkm-pie

• Data: __patchable_function_entries +400 B (+1.6%, 25,960 B)

linuxkm-standard

• Data: __patchable_function_entries +368 B (+0.8%, 48,616 B)

stm32-sim-stm32h753

• FLASH: .text +496 B (+0.3%, 183,892 B / 2,097,152 B, total: 9% used)

[kareem-wolfssl]

Retest this please

[dgarske]

Skoll Multi-Scan Review

Modes: review + review-securityOverall recommendation: COMMENT
Findings: 7 total — 6 posted, 1 skipped
4 finding(s) posted as inline comments (see file-level comments below)
2 finding(s) not tied to a diff line (full detail below)

Posted findings

• [Medium] [review] No positive test that a certificate with trailing data is rejected — tests/api.c:11897
• [Low] [review+review-security] Strict trailing-data rejection absent in non-WOLFSSL_ASN_TEMPLATE builds (incomplete hardening) — wolfcrypt/src/asn.c:22232-22259
• [Low] [review] Redundant stopAtPubKey/stopAfterPubKey conditions in trailing-data guard — wolfcrypt/src/asn.c:22249
• [Low] [review] New bitfield inserted mid-struct in DecodedCert rather than appended — wolfssl/wolfcrypt/asn.h:2075

Findings not tied to a diff line

OCSP responses embedding a certificate chain (1 cert) are now rejected, breaking OCSP validation

File: wolfcrypt/src/asn.c:22249 (manifests in OcspCheckCert, asn.c:35008-35014)
Function: DecodeCertInternal / OcspCheckCert
Severity: Medium
Category: Regression

The new trailing-data check if ((ret == 0) && (!done) && (!stopAtPubKey) && (!stopAfterPubKey) && (!cert->allowTrailing) && (cert->srcIdx != cert->maxIdx)...) ret = ASN_PARSE_E; runs on every full ParseCertRelative(CERT_TYPE). In OcspCheckCert the embedded responder certificate is parsed via InitDecodedCert(cert, resp->cert, resp->certSz, heap), where resp->cert points at the first Certificate but resp->certSz spans the ENTIRE SEQUENCE OF Certificate (from GetASN_GetRef on OCSPBASICRESPASN_IDX_CERTS_SEQ). RFC 6960 permits the certs field to carry a chain, and the code comment at asn.c:35225 confirms wolfSSL intentionally parses only the first embedded cert ('TODO: support more than one certificate'). BEFORE this PR the trailing certs were ignored and OCSP validation succeeded; AFTER, cert->srcIdx (end of first cert) != cert->maxIdx (end of all certs) -> ASN_PARSE_E, so OcspCheckCert/OcspResponseDecode fail. This is a functional/availability regression for delegated OCSP responders that include a cert chain, and OCSP hard-fail configs will drop otherwise-valid handshakes. allowTrailing is only set for TRUSTED_CERT_TYPE, so this CERT_TYPE parse is not exempted.

Recommendation: Exempt the OCSP responder-cert parse from the trailing-data check (e.g. set cert->allowTrailing = 1 before ParseCertRelative in OcspCheckCert, since only the first cert is intentionally parsed), OR size the DecodedCert to exactly the first Certificate (parse the leading SEQUENCE length and pass that as inSz). Add a regression test with a multi-certificate OCSP certs field.

Referenced code: wolfcrypt/src/asn.c:22249 (manifests in OcspCheckCert, asn.c:35008-35014) (3 lines)

---

wolfSSL_d2i_X509 / d2i_X509_REQ now reject DER buffers with trailing data (OpenSSL d2i contract change)

File: wolfcrypt/src/asn.c:22249 (d2i path: src/x509.c:4228)
Function: DecodeCertInternal / d2i_X509orX509REQ
Severity: Medium
Category: Regression

The new strict trailing-data rejection in DecodeCertInternal fires whenever a full parse ends with cert->srcIdx != cert->maxIdx, i.e. whenever the buffer is longer than one encoded certificate. This reaches the OpenSSL-compat path wolfSSL_d2i_X509 -> d2i_X509orX509REQ, which calls ParseCertRelative(cert, CERT_TYPE, ...) with len = the full remaining buffer. OpenSSL's d2i_X509 is a streaming decoder: it parses one object and advances *pp past the consumed bytes; the wrapper even does *in += newX509->derCert->length; (src/x509.c:4201-4203), clearly intended to allow walking a concatenation of DER objects. With this PR any multi-object or over-sized buffer now returns ASN_PARSE_E and d2i_X509/d2i_X509_REQ return NULL, so the pointer-advance iteration path is effectively unreachable in a default (strict) build. Severity views differ (review: Medium/api-compat; review-security: Low/regression); the stricter Medium is retained. Note the paired CopyDecodedToX509 bounding would make the pointer-advance idiom correct under WOLFSSL_NO_ASN_STRICT, but in strict builds the parse fails first. This appears intended by the security fix (zd#22010), but its interaction with the *in += derCert->length iteration idiom should be a conscious, documented decision.

Recommendation: Decide whether strict rejection is intended for the OpenSSL-compat d2i surface (wolfSSL_d2i_X509 / wolfSSL_X509_d2i / wolfSSL_X509_load_certificate_buffer DER). If OpenSSL streaming semantics must be preserved, trim the DecodedCert to the leading SEQUENCE length before parsing (or set cert->allowTrailing) so a single cert is parsed and the pointer advanced by its exact length, while still rejecting trailing data on trust-load paths. If strict rejection is deliberate here, document the behavior change in the API/ChangeLog for OPENSSL_EXTRA users and add a test for a cert-plus-trailing-bytes buffer.

Referenced code: wolfcrypt/src/asn.c:22249 (d2i path: src/x509.c:4228) (5 lines)

---

Skipped findings

• [Info] New trailing-data rejection changes wc_ParseCert public-API contract for over-long buffers

Review generated by Skoll

[dgarske]

#10755 (review)

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #10755

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

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.