diff --git a/src/internal.c b/src/internal.c index 004943c4e7..fb6c54b1bb 100644 --- a/src/internal.c +++ b/src/internal.c @@ -14770,6 +14770,27 @@ static void CopyDateToASN1_TIME(const byte* srcDate, int srcDateLen, dst->length = 0; } } + +/* Store the DER object at the start of source in a new DerBuffer, bounded by + * the end of the object rather than maxIdx (the full input length). With + * WOLFSSL_ASN_TEMPLATE, srcIdx is written only when the object's outer + * SEQUENCE parses and so marks its end. The original parser instead rebounds + * maxIdx to the outer SEQUENCE itself, while its srcIdx advances step by step + * and can rest inside the object, so there it is not a valid bound. */ +static int AllocCopyDecodedDer(DerBuffer** der, const byte* source, + word32 maxIdx, word32 srcIdx, void* heap) +{ + word32 derSz = maxIdx; + +#ifdef WOLFSSL_ASN_TEMPLATE + if ((srcIdx > 0) && (srcIdx < derSz)) { + derSz = srcIdx; + } +#else + (void)srcIdx; +#endif + return AllocCopyDer(der, source, derSz, CERT_TYPE, heap); +} #endif /* KEEP_PEER_CERT || SESSION_CERTS || OPENSSL_EXTRA || * OPENSSL_EXTRA_X509_SMALL || WOLFSSL_ACERT */ @@ -15297,13 +15318,11 @@ int CopyDecodedToX509(WOLFSSL_X509* x509, DecodedCert* dCert) ret = copyRet; /* if der contains original source buffer then store for potential - * retrieval */ + * retrieval. Store only the certificate itself, never trailing bytes that + * may follow it in the source buffer. */ 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); - } - else { + if (AllocCopyDecodedDer(&x509->derCert, dCert->source, dCert->maxIdx, + dCert->srcIdx, x509->heap) != 0) { ret = MEMORY_E; } } @@ -15504,13 +15523,11 @@ int CopyDecodedAcertToX509(WOLFSSL_X509_ACERT* x509, DecodedAcert* dAcert) } /* if der contains original source buffer then store for potential - * retrieval */ + * retrieval. Store only the attribute certificate itself, never trailing + * bytes that may follow it in the source buffer. */ 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); - } - else { + if (AllocCopyDecodedDer(&x509->derCert, dAcert->source, dAcert->maxIdx, + dAcert->srcIdx, x509->heap) != 0) { ret = MEMORY_E; } } diff --git a/src/x509.c b/src/x509.c index a8f0690b80..f39e276c2f 100644 --- a/src/x509.c +++ b/src/x509.c @@ -6181,25 +6181,19 @@ 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. + * CopyDecodedToX509() bounds the stored DER to the certificate. */ + 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 - * end of the certificate, so we adjust cert->maxIdx accordingly. */ - if (type == TRUSTED_CERT_TYPE && cert->srcIdx < cert->maxIdx) { - cert->maxIdx = cert->srcIdx; - } - x509 = (WOLFSSL_X509*)XMALLOC(sizeof(WOLFSSL_X509), NULL, DYNAMIC_TYPE_X509); if (x509 != NULL) { diff --git a/tests/api.c b/tests/api.c index f0aba071f3..25992ee13c 100644 --- a/tests/api.c +++ b/tests/api.c @@ -12824,8 +12824,12 @@ static int test_wc_CheckCertSigPubKey(void) ExpectIntEQ(load_file(ca_cert, &cert_buf, &cert_sz), 0); cert_dersz = (word32)cert_sz; /* DER will be smaller than PEM */ ExpectNotNull(cert_der = (byte*)malloc(cert_dersz)); - ExpectIntGE(ret = wc_CertPemToDer(cert_buf, (int)cert_sz, cert_der, + ExpectIntGT(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); @@ -12867,6 +12871,175 @@ static int test_wc_CheckCertSigPubKey(void) return EXPECT_RESULT(); } +/* Trailing data after a certificate's outer SEQUENCE must be rejected at parse + * time on a default (strict) build and tolerated when WOLFSSL_NO_ASN_STRICT is + * defined. In the latter case the stored/canonical DER (wolfSSL_X509_get_der / + * wolfSSL_i2d_X509) must still be bounded to the certificate itself and never + * re-emit the trailing bytes. TRUSTED CERTIFICATE blobs are the exception: + * their auxiliary trust data after the certificate parses on every build, with + * the stored DER again bounded to the certificate itself. */ +static int test_wc_ParseCert_trailing_data(void) +{ + EXPECT_DECLS; +#if !defined(NO_CERTS) && !defined(NO_RSA) && !defined(NO_FILESYSTEM) && \ + defined(WOLFSSL_PEM_TO_DER) + const char* ca_cert = "./certs/ca-cert.pem"; + const char* trusted_cert = "./certs/test/ossl-trusted-cert.pem"; + byte* pem = NULL; + size_t pemSz = 0; + byte* der = NULL; + word32 derBufSz = 0; + int certSz = 0; + byte* certPlusJunk = NULL; + word32 certPlusJunkSz = 0; + const word32 junkSz = 24; + DerBuffer* trustedDer = NULL; + word32 trustedCertSz = 0; + EncryptedInfo info; + int keyFormat = 0; + DecodedCert decoded; + + /* Load a known-good certificate and convert it to DER. */ + ExpectIntEQ(load_file(ca_cert, &pem, &pemSz), 0); + derBufSz = (word32)pemSz; /* DER is always smaller than its PEM. */ + ExpectNotNull(der = (byte*)XMALLOC(derBufSz, NULL, DYNAMIC_TYPE_TMP_BUFFER)); + ExpectIntGT(certSz = wc_CertPemToDer(pem, (int)pemSz, der, (int)derBufSz, + CERT_TYPE), 0); + + /* Build a buffer holding the certificate followed by N junk bytes. */ + if (certSz > 0) { + certPlusJunkSz = (word32)certSz + junkSz; + ExpectNotNull(certPlusJunk = (byte*)XMALLOC(certPlusJunkSz, NULL, + DYNAMIC_TYPE_TMP_BUFFER)); + } + if (certPlusJunk != NULL) { + XMEMCPY(certPlusJunk, der, (size_t)certSz); + XMEMSET(certPlusJunk + certSz, 0xA5, junkSz); + } + + /* Sanity: the clean certificate parses successfully. */ + if (certSz > 0) { + wc_InitDecodedCert(&decoded, der, (word32)certSz, NULL); + ExpectIntEQ(wc_ParseCert(&decoded, CERT_TYPE, NO_VERIFY, NULL), 0); + wc_FreeDecodedCert(&decoded); + } + + /* Certificate with appended junk. */ + if (certPlusJunk != NULL) { + wc_InitDecodedCert(&decoded, certPlusJunk, certPlusJunkSz, NULL); +#ifdef WOLFSSL_NO_ASN_STRICT + /* Opt-out build: trailing data is tolerated at parse time, but srcIdx + * must stop at the end of the certificate (not the junk). */ + ExpectIntEQ(wc_ParseCert(&decoded, CERT_TYPE, NO_VERIFY, NULL), 0); + ExpectIntEQ((int)decoded.srcIdx, certSz); +#else + /* Default (strict) build: trailing data is rejected. */ + ExpectIntEQ(wc_ParseCert(&decoded, CERT_TYPE, NO_VERIFY, NULL), + WC_NO_ERR_TRACE(ASN_PARSE_E)); +#endif + wc_FreeDecodedCert(&decoded); + } + + /* TRUSTED CERTIFICATE: the certificate is followed by auxiliary trust + * data. TRUSTED_CERT_TYPE permits it on every build and srcIdx must still + * stop at the end of the certificate. */ + XMEMSET(&info, 0, sizeof(info)); + if (pem != NULL) { + XFREE(pem, NULL, DYNAMIC_TYPE_TMP_BUFFER); + pem = NULL; + } + ExpectIntEQ(load_file(trusted_cert, &pem, &pemSz), 0); + ExpectIntEQ(wc_PemToDer(pem, (long)pemSz, TRUSTED_CERT_TYPE, &trustedDer, + NULL, &info, &keyFormat), 0); + if (trustedDer != NULL) { + wc_InitDecodedCert(&decoded, trustedDer->buffer, trustedDer->length, + NULL); + ExpectIntEQ(wc_ParseCert(&decoded, TRUSTED_CERT_TYPE, NO_VERIFY, NULL), + 0); + trustedCertSz = decoded.srcIdx; + ExpectIntGT((int)trustedCertSz, 0); + /* The fixture must actually carry auxiliary data after the certificate + * for this test to prove anything. */ + ExpectIntLT((int)trustedCertSz, (int)trustedDer->length); + wc_FreeDecodedCert(&decoded); + +#ifndef WOLFSSL_NO_ASN_STRICT + /* The same blob parsed as a plain certificate is rejected. */ + wc_InitDecodedCert(&decoded, trustedDer->buffer, trustedDer->length, + NULL); + ExpectIntEQ(wc_ParseCert(&decoded, CERT_TYPE, NO_VERIFY, NULL), + WC_NO_ERR_TRACE(ASN_PARSE_E)); + wc_FreeDecodedCert(&decoded); +#endif + } + +#ifdef OPENSSL_EXTRA + /* derCert bounding: wolfSSL_X509_get_der / wolfSSL_i2d_X509 must re-emit + * only the certificate bytes, proving data after the certificate is never + * stored or re-serialized. */ +#ifdef WOLFSSL_NO_ASN_STRICT + if (certPlusJunk != NULL) { + WOLFSSL_X509* x509 = NULL; + const byte* getDer = NULL; + int getDerSz = 0; + byte* i2dDer = NULL; + int i2dSz = 0; + + x509 = wolfSSL_X509_load_certificate_buffer(certPlusJunk, + (int)certPlusJunkSz, WOLFSSL_FILETYPE_ASN1); + ExpectNotNull(x509); + + getDer = wolfSSL_X509_get_der(x509, &getDerSz); + ExpectNotNull(getDer); + ExpectIntEQ(getDerSz, certSz); + + ExpectIntEQ((i2dSz = wolfSSL_i2d_X509(x509, &i2dDer)), certSz); + if (i2dDer != NULL) + XFREE(i2dDer, HEAP_HINT, DYNAMIC_TYPE_OPENSSL); + + wolfSSL_X509_free(x509); + } +#else + /* Default (strict) build: a certificate with trailing junk does not load + * as DER at all. */ + if (certPlusJunk != NULL) { + ExpectNull(wolfSSL_X509_load_certificate_buffer(certPlusJunk, + (int)certPlusJunkSz, WOLFSSL_FILETYPE_ASN1)); + } +#endif /* WOLFSSL_NO_ASN_STRICT */ + +#ifndef NO_BIO + /* PEM_read_bio_X509_AUX consumes the TRUSTED CERTIFICATE format; the + * X509's DER must hold only the certificate, not the auxiliary data. */ + if ((pem != NULL) && (trustedCertSz > 0)) { + WOLFSSL_BIO* bio = NULL; + WOLFSSL_X509* x509 = NULL; + const byte* auxDer = NULL; + int auxDerSz = 0; + + ExpectNotNull(bio = wolfSSL_BIO_new_mem_buf(pem, (int)pemSz)); + ExpectNotNull(x509 = wolfSSL_PEM_read_bio_X509_AUX(bio, NULL, NULL, + NULL)); + auxDer = wolfSSL_X509_get_der(x509, &auxDerSz); + ExpectNotNull(auxDer); + ExpectIntEQ(auxDerSz, (int)trustedCertSz); + wolfSSL_X509_free(x509); + wolfSSL_BIO_free(bio); + } +#endif /* !NO_BIO */ +#endif /* OPENSSL_EXTRA */ + + wc_FreeDer(&trustedDer); + if (certPlusJunk != NULL) + XFREE(certPlusJunk, NULL, DYNAMIC_TYPE_TMP_BUFFER); + if (der != NULL) + XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER); + if (pem != NULL) + XFREE(pem, NULL, DYNAMIC_TYPE_TMP_BUFFER); +#endif + return EXPECT_RESULT(); +} + static int test_wolfSSL_X509_ext_d2i(void) { EXPECT_DECLS; @@ -38936,6 +39109,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_wc_GetPubKeyDerFromCert), TEST_DECL(test_wc_GetSubjectPubKeyInfoDerFromCert), TEST_DECL(test_wc_CheckCertSigPubKey), + TEST_DECL(test_wc_ParseCert_trailing_data), /* wolfCrypt ASN tests */ TEST_DECL(test_ToTraditional), diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 8e6c7560ee..934a98cb6d 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -22557,6 +22557,7 @@ static int DecodeCertInternal(DecodedCert* cert, int verify, int* criticalExt, #if defined(HAVE_RPK) /* try to parse the cert as Raw Public Key cert */ + word32 rpkStartIdx = cert->srcIdx; DECL_ASNGETDATA(RPKdataASN, RPKCertASN_Length); CALLOC_ASNGETDATA(RPKdataASN, RPKCertASN_Length, ret, cert->heap); GetASN_OID(&RPKdataASN[RPKCERTASN_IDX_SPUBKEYINFO_ALGO_OID], @@ -22596,10 +22597,22 @@ static int DecodeCertInternal(DecodedCert* cert, int verify, int* criticalExt, FREE_ASNGETDATA(RPKdataASN, cert->heap); if (ret == 0) { +#ifndef WOLFSSL_NO_ASN_STRICT + /* Data after the SubjectPublicKeyInfo is rejected the same way as + * data after a certificate's outer SEQUENCE below. */ + if ((!stopAtPubKey) && (!stopAfterPubKey) && (!cert->allowTrailing) && + (cert->srcIdx != cert->maxIdx)) { + WOLFSSL_MSG("Trailing data after certificate"); + WOLFSSL_ERROR_VERBOSE(ASN_PARSE_E); + return ASN_PARSE_E; + } +#endif /* !WOLFSSL_NO_ASN_STRICT */ return ret; } else { - ret = 0; /* proceed to the original x509 parsing */ + /* Restore the index and proceed to the original x509 parsing. */ + cert->srcIdx = rpkStartIdx; + ret = 0; } #endif /* HAVE_RPK */ @@ -22630,6 +22643,19 @@ static int DecodeCertInternal(DecodedCert* cert, int verify, int* criticalExt, } #endif } +#ifndef WOLFSSL_NO_ASN_STRICT + /* cert->srcIdx is now just past the certificate's outer SEQUENCE. Reject + * any data after it, with two exceptions: the pubkey-only paths + * (stopAtPubKey/stopAfterPubKey), whose callers may pass larger buffers, + * and the TRUSTED CERTIFICATE format (cert->allowTrailing), which carries + * auxiliary trust data after the certificate. */ + if ((ret == 0) && (!stopAtPubKey) && (!stopAfterPubKey) && + (!cert->allowTrailing) && (cert->srcIdx != cert->maxIdx)) { + WOLFSSL_MSG("Trailing data after certificate"); + WOLFSSL_ERROR_VERBOSE(ASN_PARSE_E); + ret = ASN_PARSE_E; + } +#endif /* !WOLFSSL_NO_ASN_STRICT */ /* Check version is valid/supported - can't be negative. */ if ((ret == 0) && (version > MAX_X509_VERSION)) { WOLFSSL_MSG("Unexpected certificate version"); @@ -24127,6 +24153,11 @@ int ParseCertRelative(DecodedCert* cert, int type, int verify, void* cm, int ret = 0; #ifndef WOLFSSL_ASN_TEMPLATE word32 confirmOID = 0; +#ifndef WOLFSSL_NO_ASN_STRICT + /* Full input length before GetCertHeader() rebounds cert->maxIdx to the + * certificate's outer SEQUENCE; used to detect trailing data. */ + word32 origMaxIdx = 0; +#endif #ifdef WOLFSSL_CERT_REQ int len = 0; #endif @@ -24147,6 +24178,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; @@ -24154,6 +24196,12 @@ int ParseCertRelative(DecodedCert* cert, int type, int verify, void* cm, if (cert->sigCtx.state == SIG_STATE_BEGIN) { #ifndef WOLFSSL_ASN_TEMPLATE +#ifndef WOLFSSL_NO_ASN_STRICT + /* Capture the full input length now: DecodeToKey()->GetCertHeader() + * rebounds cert->maxIdx to the certificate's outer SEQUENCE, after + * which trailing bytes would otherwise be invisible. */ + origMaxIdx = cert->maxIdx; +#endif cert->badDate = 0; cert->criticalExt = 0; if ((ret = DecodeToKey(cert, verify)) < 0) { @@ -24170,6 +24218,15 @@ int ParseCertRelative(DecodedCert* cert, int type, int verify, void* cm, WOLFSSL_MSG("Parsed Past Key"); #if defined(HAVE_RPK) if (cert->isRPK) { +#ifndef WOLFSSL_NO_ASN_STRICT + /* A Raw Public Key cert is only the SubjectPublicKeyInfo; reject + * data after it like data after a certificate's outer SEQUENCE. */ + if ((!cert->allowTrailing) && (cert->srcIdx != cert->maxIdx)) { + WOLFSSL_MSG("Trailing data after certificate"); + WOLFSSL_ERROR_VERBOSE(ASN_PARSE_E); + return ASN_PARSE_E; + } +#endif /* !WOLFSSL_NO_ASN_STRICT */ return ret; } #endif /* HAVE_RPK */ @@ -24396,6 +24453,27 @@ int ParseCertRelative(DecodedCert* cert, int type, int verify, void* cm, WOLFSSL_ERROR_VERBOSE(ASN_SIG_OID_E); return ASN_SIG_OID_E; } + +#ifndef WOLFSSL_NO_ASN_STRICT + /* Reject trailing data after the certificate's outer SEQUENCE. + * + * This is the non-template counterpart to the trailing-data check in + * DecodeCertInternal(). GetCertHeader() rebounded cert->maxIdx to the + * end of the certificate's outer SEQUENCE, so comparing it against the + * original input length (origMaxIdx) detects any appended bytes. The + * TRUSTED CERTIFICATE format legitimately carries auxiliary trust data + * after the certificate (cert->allowTrailing), and CSRs carry their own + * trailing structure, so both are exempt. */ + if ((cert->maxIdx != origMaxIdx) && (!cert->allowTrailing) +#ifdef WOLFSSL_CERT_REQ + && (!cert->isCSR) +#endif + ) { + WOLFSSL_MSG("Trailing data after certificate"); + WOLFSSL_ERROR_VERBOSE(ASN_PARSE_E); + return ASN_PARSE_E; + } +#endif /* !WOLFSSL_NO_ASN_STRICT */ #else #ifdef WOLFSSL_CERT_REQ if (cert->isCSR) { @@ -35781,6 +35859,10 @@ static int OcspCheckCert(OcspResponse *resp, int noVerify, #endif InitDecodedCert(cert, resp->cert, resp->certSz, heap); + /* The response's certs field may hold a chain (RFC 6960: SEQUENCE OF + * Certificate) and certSz spans all of it; only the first certificate is + * parsed and used, so permit the data that follows it. */ + cert->allowTrailing = 1; ret = ParseCertRelative(cert, CERT_TYPE, noVerify ? NO_VERIFY : VERIFY_OCSP_CERT, cm, resp->pendingCAs); diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 42c94e0940..308acc14d5 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -2229,6 +2229,10 @@ struct DecodedCert { #ifdef HAVE_RPK WC_BITFIELD isRPK:1; /* indicate the cert is Raw-Public-Key cert in RFC7250 */ #endif + WC_BITFIELD allowTrailing:1; /* permit data after the cert's outer + * SEQUENCE. Used internally for the + * TRUSTED CERTIFICATE auxiliary trust + * info. */ #ifdef WC_ASN_UNKNOWN_EXT_CB wc_UnknownExtCallback unknownExtCallback; wc_UnknownExtCallbackEx unknownExtCallbackEx;