From 9d6541f2299ac99dd73b8d61dfd2d0154e2b9658 Mon Sep 17 00:00:00 2001 From: Kareem Date: Mon, 22 Jun 2026 17:14:11 -0700 Subject: [PATCH 1/3] Fix DER buffer size in test_wc_CheckCertSigPubKey. --- tests/api.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/api.c b/tests/api.c index 32b90b9a079..4bca8a8c6a0 100644 --- a/tests/api.c +++ b/tests/api.c @@ -11792,6 +11792,10 @@ static int test_wc_CheckCertSigPubKey(void) ExpectNotNull(cert_der = (byte*)malloc(cert_dersz)); ExpectIntGE(ret = wc_CertPemToDer(cert_buf, (int)cert_sz, cert_der, (int)cert_dersz, CERT_TYPE), 0); + /* Use the actual DER length, not the (larger) PEM buffer size, otherwise + * the decoded cert would have trailing bytes after its outer SEQUENCE. */ + if (ret > 0) + cert_dersz = (word32)ret; wc_InitDecodedCert(&decoded, cert_der, cert_dersz, NULL); ExpectIntEQ(wc_ParseCert(&decoded, CERT_TYPE, NO_VERIFY, NULL), 0); From 354e3dcf37dc907e76ac5dd876e6e1229e24d9cb Mon Sep 17 00:00:00 2001 From: Kareem Date: Mon, 22 Jun 2026 17:17:41 -0700 Subject: [PATCH 2/3] Reject certificates with trailing data, except for trusted certificates. In addition, only copy the certificate itself to WOLFSSL_X509, omitting trailing bytes. Thanks to Lucca Hirschi (Inria, France) and the tlspuffin team for the report. --- src/internal.c | 27 +++++++++++++++++++++------ src/x509.c | 11 ++++++----- wolfcrypt/src/asn.c | 40 ++++++++++++++++++++++++++++++++++++++++ wolfssl/wolfcrypt/asn.h | 4 ++++ 4 files changed, 71 insertions(+), 11 deletions(-) diff --git a/src/internal.c b/src/internal.c index fed9d370dea..39c7889132f 100644 --- a/src/internal.c +++ b/src/internal.c @@ -14280,9 +14280,19 @@ int CopyDecodedToX509(WOLFSSL_X509* x509, DecodedCert* dCert) /* if der contains original source buffer then store for potential * retrieval */ if (dCert->source != NULL && dCert->maxIdx > 0) { - if (AllocDer(&x509->derCert, dCert->maxIdx, CERT_TYPE, x509->heap) - == 0) { - XMEMCPY(x509->derCert->buffer, dCert->source, dCert->maxIdx); + /* Store only the certificate itself, bounded by the end of its outer + * SEQUENCE (dCert->srcIdx), never any trailing bytes that may follow in + * the source buffer. This keeps wolfSSL_i2d_X509 / wolfSSL_X509_get_der + * / wolfSSL_X509_digest canonical - they operate on derCert - even on + * builds/paths that do not reject trailing data (e.g. + * WOLFSSL_NO_ASN_STRICT). It removes only bytes after the certificate, + * so the signed certificate bytes are preserved verbatim. */ + word32 derCertSz = dCert->maxIdx; + if ((dCert->srcIdx > 0) && (dCert->srcIdx < derCertSz)) { + derCertSz = dCert->srcIdx; + } + if (AllocDer(&x509->derCert, derCertSz, CERT_TYPE, x509->heap) == 0) { + XMEMCPY(x509->derCert->buffer, dCert->source, derCertSz); } else { ret = MEMORY_E; @@ -14618,9 +14628,14 @@ int CopyDecodedAcertToX509(WOLFSSL_X509_ACERT* x509, DecodedAcert* dAcert) /* if der contains original source buffer then store for potential * retrieval */ if (dAcert->source != NULL && dAcert->maxIdx > 0) { - if (AllocDer(&x509->derCert, dAcert->maxIdx, CERT_TYPE, x509->heap) - == 0) { - XMEMCPY(x509->derCert->buffer, dAcert->source, dAcert->maxIdx); + /* Bound to the end of the attribute certificate's outer SEQUENCE so no + * trailing bytes after it are ever re-emitted by i2d / get_der. */ + word32 derCertSz = dAcert->maxIdx; + if ((dAcert->srcIdx > 0) && (dAcert->srcIdx < derCertSz)) { + derCertSz = dAcert->srcIdx; + } + if (AllocDer(&x509->derCert, derCertSz, CERT_TYPE, x509->heap) == 0) { + XMEMCPY(x509->derCert->buffer, dAcert->source, derCertSz); } else { ret = MEMORY_E; diff --git a/src/x509.c b/src/x509.c index fd87b1da4aa..6059557d98b 100644 --- a/src/x509.c +++ b/src/x509.c @@ -6231,17 +6231,18 @@ static WOLFSSL_X509* loadX509orX509REQFromBuffer( /* ready to be decoded. */ if (der != NULL && der->buffer != NULL) { WC_DECLARE_VAR(cert, DecodedCert, 1, 0); - /* For TRUSTED_CERT_TYPE, the DER buffer contains the certificate - * followed by auxiliary trust info. ParseCertRelative expects CERT_TYPE - * and will parse only the certificate portion, ignoring the rest. */ - int parseType = (type == TRUSTED_CERT_TYPE) ? CERT_TYPE : type; WC_ALLOC_VAR_EX(cert, DecodedCert, 1, NULL, DYNAMIC_TYPE_DCERT, ret=MEMORY_ERROR); if (WC_VAR_OK(cert)) { InitDecodedCert(cert, der->buffer, der->length, NULL); - ret = ParseCertRelative(cert, parseType, 0, NULL, NULL); + /* For TRUSTED_CERT_TYPE the DER buffer holds the certificate + * followed by auxiliary trust info. ParseCertRelative() recognizes + * the type: it parses only the certificate, permits the trailing + * aux data, and treats it as CERT_TYPE for verification. The DER is + * trimmed to the certificate below. */ + ret = ParseCertRelative(cert, type, 0, NULL, NULL); if (ret == 0) { /* For TRUSTED_CERT_TYPE, truncate the DER buffer to exclude * auxiliary trust data. ParseCertRelative sets srcIdx to the diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 7e855986f67..9feb3656b36 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -22020,6 +22020,35 @@ static int DecodeCertInternal(DecodedCert* cert, int verify, int* criticalExt, } } +#ifndef WOLFSSL_NO_ASN_STRICT + /* Reject trailing data after the certificate's outer SEQUENCE. + * + * The template parser (GetASN_Items) only verifies that constructed items + * nested under the top-level item are fully consumed - it never checks that + * the outermost SEQUENCE spans all the way to maxIdx. Without this check, + * arbitrary bytes appended after a certificate are silently accepted and + * then returned/hashed verbatim by wolfSSL_i2d_X509 / wolfSSL_X509_get_der / + * wolfSSL_X509_digest (which operate on the stored source buffer of length + * maxIdx). + * + * cert->srcIdx points just past the certificate's outer SEQUENCE after the + * x509CertASN parse above. Only enforce this on a full parse; the + * pubkey-only paths (stopAtPubKey/stopAfterPubKey) intentionally parse the + * whole template but their callers may pass larger buffers. The TRUSTED + * CERTIFICATE format legitimately carries auxiliary trust data after the + * certificate, so allow it when cert->allowTrailing is set. */ + if ((ret == 0) && (!done) && (!stopAtPubKey) && (!stopAfterPubKey) && + (!cert->allowTrailing) && (cert->srcIdx != cert->maxIdx) +#ifdef WOLFSSL_CERT_REQ + && (!cert->isCSR) +#endif + ) { + WOLFSSL_MSG("Trailing data after certificate"); + WOLFSSL_ERROR_VERBOSE(ASN_PARSE_E); + ret = ASN_PARSE_E; + } +#endif /* !WOLFSSL_NO_ASN_STRICT */ + if ((ret == 0) && (!done) && (badDate != 0)) { /* Parsed whole certificate fine but return any date errors. */ ret = badDate; @@ -23169,6 +23198,17 @@ int ParseCertRelative(DecodedCert* cert, int type, int verify, void* cm, return BAD_FUNC_ARG; } + /* TRUSTED CERTIFICATE blobs (RFC/OpenSSL "TRUSTED CERTIFICATE") carry + * auxiliary trust data after the certificate. Permit that trailing data and + * parse only the certificate prefix; treat it as a normal certificate for + * all verification/path-length logic below. Doing this here (rather than in + * a single caller) means any caller of wc_ParseCert()/ParseCertRelative() + * that passes TRUSTED_CERT_TYPE gets the correct, consistent behavior. */ + if (type == TRUSTED_CERT_TYPE) { + cert->allowTrailing = 1; + type = CERT_TYPE; + } + #ifdef WOLFSSL_CERT_REQ if (type == CERTREQ_TYPE) cert->isCSR = 1; diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 45993f0638a..5eec7d42d35 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -2067,6 +2067,10 @@ struct DecodedCert { /* Option Bits */ WC_BITFIELD subjectCNStored:1; /* have we saved a copy we own */ + WC_BITFIELD allowTrailing:1; /* permit data after the cert's outer + * SEQUENCE. Used internally for the + * TRUSTED CERTIFICATE auxiliary trust + * info. */ WC_BITFIELD extSubjKeyIdSet:1; /* Set when the SKID was read from cert */ WC_BITFIELD extAuthKeyIdSet:1; /* Set when the AKID was read from cert */ #ifndef IGNORE_NAME_CONSTRAINTS From 93a5caf7946a379a6f255866d0ea7df52bc7b92e Mon Sep 17 00:00:00 2001 From: Kareem Date: Thu, 25 Jun 2026 17:38:59 -0700 Subject: [PATCH 3/3] Gate new cert trailing rejection logic behind an opt-in macro to avoid breaking existing use cases. --- .wolfssl_known_macro_extras | 1 + src/internal.c | 8 ++++++-- src/x509.c | 7 ++++--- wolfcrypt/src/asn.c | 17 ++++++++++------- wolfssl/wolfcrypt/asn.h | 2 ++ 5 files changed, 23 insertions(+), 12 deletions(-) diff --git a/.wolfssl_known_macro_extras b/.wolfssl_known_macro_extras index 840bc74f7d1..9ec073e1d66 100644 --- a/.wolfssl_known_macro_extras +++ b/.wolfssl_known_macro_extras @@ -740,6 +740,7 @@ WOLFSSL_CAAM_BLACK_KEY_AESCCM WOLFSSL_CAAM_BLACK_KEY_SM WOLFSSL_CAAM_NO_BLACK_KEY WOLFSSL_CALLBACKS +WOLFSSL_CERT_REJECT_TRAILING WOLFSSL_CHECK_DESKEY WOLFSSL_CHECK_MEM_ZERO WOLFSSL_CHIBIOS diff --git a/src/internal.c b/src/internal.c index 39c7889132f..8f50145bd4b 100644 --- a/src/internal.c +++ b/src/internal.c @@ -14280,6 +14280,8 @@ int CopyDecodedToX509(WOLFSSL_X509* x509, DecodedCert* dCert) /* if der contains original source buffer then store for potential * retrieval */ if (dCert->source != NULL && dCert->maxIdx > 0) { + word32 derCertSz = dCert->maxIdx; +#ifdef WOLFSSL_CERT_REJECT_TRAILING /* Store only the certificate itself, bounded by the end of its outer * SEQUENCE (dCert->srcIdx), never any trailing bytes that may follow in * the source buffer. This keeps wolfSSL_i2d_X509 / wolfSSL_X509_get_der @@ -14287,10 +14289,10 @@ int CopyDecodedToX509(WOLFSSL_X509* x509, DecodedCert* dCert) * builds/paths that do not reject trailing data (e.g. * WOLFSSL_NO_ASN_STRICT). It removes only bytes after the certificate, * so the signed certificate bytes are preserved verbatim. */ - word32 derCertSz = dCert->maxIdx; if ((dCert->srcIdx > 0) && (dCert->srcIdx < derCertSz)) { derCertSz = dCert->srcIdx; } +#endif if (AllocDer(&x509->derCert, derCertSz, CERT_TYPE, x509->heap) == 0) { XMEMCPY(x509->derCert->buffer, dCert->source, derCertSz); } @@ -14628,12 +14630,14 @@ int CopyDecodedAcertToX509(WOLFSSL_X509_ACERT* x509, DecodedAcert* dAcert) /* if der contains original source buffer then store for potential * retrieval */ if (dAcert->source != NULL && dAcert->maxIdx > 0) { + word32 derCertSz = dAcert->maxIdx; +#ifdef WOLFSSL_CERT_REJECT_TRAILING /* Bound to the end of the attribute certificate's outer SEQUENCE so no * trailing bytes after it are ever re-emitted by i2d / get_der. */ - word32 derCertSz = dAcert->maxIdx; if ((dAcert->srcIdx > 0) && (dAcert->srcIdx < derCertSz)) { derCertSz = dAcert->srcIdx; } +#endif if (AllocDer(&x509->derCert, derCertSz, CERT_TYPE, x509->heap) == 0) { XMEMCPY(x509->derCert->buffer, dAcert->source, derCertSz); } diff --git a/src/x509.c b/src/x509.c index 6059557d98b..59d55a5d835 100644 --- a/src/x509.c +++ b/src/x509.c @@ -6239,9 +6239,10 @@ static WOLFSSL_X509* loadX509orX509REQFromBuffer( InitDecodedCert(cert, der->buffer, der->length, NULL); /* For TRUSTED_CERT_TYPE the DER buffer holds the certificate * followed by auxiliary trust info. ParseCertRelative() recognizes - * the type: it parses only the certificate, permits the trailing - * aux data, and treats it as CERT_TYPE for verification. The DER is - * trimmed to the certificate below. */ + * the type: it parses only the certificate and treats it as + * CERT_TYPE for verification (and, under WOLFSSL_CERT_REJECT_TRAILING, + * permits the trailing aux data). The DER is trimmed to the + * certificate below. */ ret = ParseCertRelative(cert, type, 0, NULL, NULL); if (ret == 0) { /* For TRUSTED_CERT_TYPE, truncate the DER buffer to exclude diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 9feb3656b36..36a8b6379ec 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -22020,7 +22020,7 @@ static int DecodeCertInternal(DecodedCert* cert, int verify, int* criticalExt, } } -#ifndef WOLFSSL_NO_ASN_STRICT +#ifdef WOLFSSL_CERT_REJECT_TRAILING /* Reject trailing data after the certificate's outer SEQUENCE. * * The template parser (GetASN_Items) only verifies that constructed items @@ -22047,7 +22047,7 @@ static int DecodeCertInternal(DecodedCert* cert, int verify, int* criticalExt, WOLFSSL_ERROR_VERBOSE(ASN_PARSE_E); ret = ASN_PARSE_E; } -#endif /* !WOLFSSL_NO_ASN_STRICT */ +#endif /* WOLFSSL_CERT_REJECT_TRAILING */ if ((ret == 0) && (!done) && (badDate != 0)) { /* Parsed whole certificate fine but return any date errors. */ @@ -23199,13 +23199,16 @@ int ParseCertRelative(DecodedCert* cert, int type, int verify, void* cm, } /* TRUSTED CERTIFICATE blobs (RFC/OpenSSL "TRUSTED CERTIFICATE") carry - * auxiliary trust data after the certificate. Permit that trailing data and - * parse only the certificate prefix; treat it as a normal certificate for - * all verification/path-length logic below. Doing this here (rather than in - * a single caller) means any caller of wc_ParseCert()/ParseCertRelative() - * that passes TRUSTED_CERT_TYPE gets the correct, consistent behavior. */ + * auxiliary trust data after the certificate. Parse only the certificate + * prefix and treat it as a normal certificate for all verification / + * path-length logic below. Doing this here (rather than in a single caller) + * means any caller of wc_ParseCert()/ParseCertRelative() that passes + * TRUSTED_CERT_TYPE gets the correct, consistent behavior. */ if (type == TRUSTED_CERT_TYPE) { +#ifdef WOLFSSL_CERT_REJECT_TRAILING + /* Opt out of the trailing-data rejection for the aux trust info. */ cert->allowTrailing = 1; +#endif type = CERT_TYPE; } diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 5eec7d42d35..ff85867f8fa 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -2067,10 +2067,12 @@ struct DecodedCert { /* Option Bits */ WC_BITFIELD subjectCNStored:1; /* have we saved a copy we own */ +#ifdef WOLFSSL_CERT_REJECT_TRAILING WC_BITFIELD allowTrailing:1; /* permit data after the cert's outer * SEQUENCE. Used internally for the * TRUSTED CERTIFICATE auxiliary trust * info. */ +#endif WC_BITFIELD extSubjKeyIdSet:1; /* Set when the SKID was read from cert */ WC_BITFIELD extAuthKeyIdSet:1; /* Set when the AKID was read from cert */ #ifndef IGNORE_NAME_CONSTRAINTS