Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions src/internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -14730,9 +14730,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;
Expand Down Expand Up @@ -14934,9 +14944,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;
Expand Down
11 changes: 6 additions & 5 deletions src/x509.c
Original file line number Diff line number Diff line change
Expand Up @@ -6183,17 +6183,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
Expand Down
107 changes: 107 additions & 0 deletions tests/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -12223,6 +12223,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
Comment thread
kareem-wolfssl marked this conversation as resolved.
* 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);
Expand Down Expand Up @@ -12264,6 +12268,108 @@ 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. */
static int test_cert_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";
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;
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) {
XMEMSET(&decoded, 0, sizeof(decoded));
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) {
XMEMSET(&decoded, 0, sizeof(decoded));
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);
}

/* derCert bounding: on a non-strict build a certificate with trailing data
* loads into a WOLFSSL_X509, but wolfSSL_X509_get_der / wolfSSL_i2d_X509
* must re-emit only the certificate bytes (srcIdx), proving the trailing
* junk is never stored or re-serialized. */
#if defined(OPENSSL_EXTRA) && defined(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);
}
#endif /* OPENSSL_EXTRA && WOLFSSL_NO_ASN_STRICT */

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;
Expand Down Expand Up @@ -37005,6 +37111,7 @@ TEST_CASE testCases[] = {
TEST_DECL(test_wc_GetPubKeyDerFromCert),
TEST_DECL(test_wc_GetSubjectPubKeyInfoDerFromCert),
TEST_DECL(test_wc_CheckCertSigPubKey),
TEST_DECL(test_cert_trailing_data),

/* wolfCrypt ASN tests */
TEST_DECL(test_ToTraditional),
Expand Down
68 changes: 68 additions & 0 deletions wolfcrypt/src/asn.c
Original file line number Diff line number Diff line change
Expand Up @@ -22541,6 +22541,31 @@ static int DecodeCertInternal(DecodedCert* cert, int verify, int* criticalExt,
}
}

#ifndef WOLFSSL_NO_ASN_STRICT
Comment thread
kareem-wolfssl marked this conversation as resolved.
/* 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) &&
(!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 */

if ((ret == 0) && (!done) && (badDate != 0)) {
/* Parsed whole certificate fine but return any date errors. */
ret = badDate;
Expand Down Expand Up @@ -23741,6 +23766,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
Expand All @@ -23761,13 +23791,30 @@ 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;
#endif

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) {
Expand Down Expand Up @@ -24010,6 +24057,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) {
Expand Down
4 changes: 4 additions & 0 deletions wolfssl/wolfcrypt/asn.h
Original file line number Diff line number Diff line change
Expand Up @@ -2210,6 +2210,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;
Expand Down
Loading