1
0
mirror of https://github.com/wolfSSL/wolfssl.git synced 2026-10-24 04:09:07 +03:00

Merge pull request #10755 from kareem-wolfssl/zd22010

Reject certificates with trailing data.  Avoid copying trailing data to WOLFSSL_X509 structs.
This commit is contained in:
JacobBarthelmeh
2026-08-12 15:03:24 -06:00
committed by GitHub
5 changed files with 297 additions and 26 deletions

View File

@@ -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;
}
}

View File

@@ -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) {

View File

@@ -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),

View File

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

View File

@@ -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;