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/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); 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