From d5cc65200d2c0d41100728420f773070e8148f21 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 28 Oct 2026 13:48:32 +0000 Subject: [PATCH 1/9] Reduce OpenSSL compat layer heap use and time when parsing certificates With OPENSSL_EXTRA or OPENSSL_EXTRA_X509_SMALL, every certificate parse built a WOLFSSL_X509_NAME for the issuer and subject: about 1.3 KB each plus a heap ASN1_OBJECT and ASN1_STRING per attribute. This happened for CA loads, every chain certificate and again when a verified intermediate was added as a temporary CA, but the names were only read when the certificate was copied into a WOLFSSL_X509. On a Cortex-M33 this was most of the compat layer's handshake overhead. Parsing can now skip the X509_NAMEs: DecodedCert has a noX509Name flag and keeps where each encoded name is, and GetCertX509Name() builds a name on demand. GetCertName() still checks DirectoryString lengths and fails a name with more than MAX_NAME_ENTRIES attributes when it skips building it, so the same certificates are accepted and rejected. AddCA(), the CSR2 pending signer parse, TLS peer certificate parsing, d2i_X509/X509_REQ and the X509 buffer load set the flag. The hash directory issuer and CRL lookups build the issuer name when needed. CopyDecodedToX509() adds the entries straight to the X509's names rather than building them in the DecodedCert and copying, so the leaf's names no longer exist twice at the handshake peak. A failure to build them then is reported as MEMORY_E. X509_NAME_add_entry_by_NID() sets the entry in place instead of creating a temporary entry and copying it, and RebuildFullName() uses the entry's object when it matches the NID instead of searching the OID table again for every entry on every add. X509_NAME_add_entry() now fails if the value could not be set. With OPENSSL_ALL, CopyDecodedToX509() still decoded the public key into an EVP_PKEY and created the algorithm object for every certificate, including ECC point validation, although X509CachedPubKey() and wolfSSL_X509_PUBKEY_get0_param() create them on first use. Leave them to those. STM32U585 at 160 MHz, TLS 1.3 client handshake with an ECC chain of two intermediates, overhead over a build without the compat layer: OPENSSL_EXTRA 9.0 ms -> 1.8 ms, OPENSSL_ALL 16.1 ms -> 3.0 ms, X509_SMALL 6.0 ms -> 0.2 ms. Handshake heap peak OPENSSL_EXTRA 21.4 KB -> 16.0 KB, OPENSSL_ALL 47.1 KB -> 23.2 KB. --- src/crl.c | 34 ++++++--- src/internal.c | 152 ++++++++++++++++++++++------------------ src/ssl_certman.c | 4 ++ src/x509.c | 82 +++++++++++++++------- wolfcrypt/src/asn.c | 138 +++++++++++++++++++++++++++++++----- wolfssl/wolfcrypt/asn.h | 19 +++++ 6 files changed, 309 insertions(+), 120 deletions(-) diff --git a/src/crl.c b/src/crl.c index bc3c24e2f8d..78871bc3d01 100644 --- a/src/crl.c +++ b/src/crl.c @@ -724,14 +724,31 @@ int CheckCertCRL_ex(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, extCrlInfo, extCrlInfoSz, issuerName, crl->cm); } -/* Is the cert ok with CRL, return 0 on success */ -int CheckCertCRL(WOLFSSL_CRL* crl, DecodedCert* cert) +/* Get the issuer X509_NAME of cert to pass to CheckCertCRLCm(). */ +static void* CertCrlIssuerName(DecodedCert* cert) { -#if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) - void* issuerName = cert->issuerName; +#if defined(OPENSSL_ALL) && defined(WOLFSSL_CERT_GEN) && \ + (defined(WOLFSSL_CERT_REQ) || defined(WOLFSSL_CERT_EXT)) && \ + !defined(NO_FILESYSTEM) && !defined(NO_WOLFSSL_DIR) && \ + !defined(NO_STDIO_FILESYSTEM) && \ + (!defined(NO_WOLFSSL_CLIENT) || !defined(WOLFSSL_NO_CLIENT_AUTH)) + /* Used to load a CRL from the hash directory. The cert may have been + * parsed without its X509_NAMEs. */ + (void)GetCertX509Name(cert, ASN_ISSUER, NULL); + return cert->issuerName; +#elif defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) + return cert->issuerName; #else - void* issuerName = NULL; + (void)cert; + return NULL; #endif +} + +/* Is the cert ok with CRL, return 0 on success */ +int CheckCertCRL(WOLFSSL_CRL* crl, DecodedCert* cert) +{ + void* issuerName = CertCrlIssuerName(cert); + return CheckCertCRL_ex(crl, cert->issuerHash, cert->serial, cert->serialSz, NULL, cert->extCrlInfo, cert->extCrlInfoSz, issuerName); } @@ -742,11 +759,8 @@ int CheckCertCRL(WOLFSSL_CRL* crl, DecodedCert* cert) int CheckCertCRLFromCm(WOLFSSL_CERT_MANAGER* cm, WOLFSSL_CRL* crl, DecodedCert* cert) { -#if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) - void* issuerName = cert->issuerName; -#else - void* issuerName = NULL; -#endif + void* issuerName = CertCrlIssuerName(cert); + return CheckCertCRLCm(crl, cert->issuerHash, cert->serial, cert->serialSz, NULL, cert->extCrlInfo, cert->extCrlInfoSz, issuerName, cm); } diff --git a/src/internal.c b/src/internal.c index e4585216985..a0b3b327fba 100644 --- a/src/internal.c +++ b/src/internal.c @@ -15659,6 +15659,68 @@ void CopyDecodedName(WOLFSSL_X509_NAME* name, DecodedCert* dCert, int nameType) } } +#if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) +/* Set the issuer or subject X509_NAME of the X509 from the decoded + * certificate. + * + * When parsing skipped building the X509_NAME, the entries are added straight + * to the X509's name rather than building one and copying it. + * + * @param [in, out] x509 X509 object. + * @param [in, out] name X509's issuer or subject name. + * @param [in, out] dCert Decoded certificate. + * @param [in] nameType ASN_ISSUER or ASN_SUBJECT. + * @return 0 on success. + * @return MEMORY_E when building the name fails. + */ +static int CopyDecodedX509Name(WOLFSSL_X509* x509, WOLFSSL_X509_NAME* name, + DecodedCert* dCert, int nameType) +{ + int ret = 0; + WOLFSSL_X509_NAME* decName; + const byte* der; + + if (nameType == ASN_ISSUER) { + decName = (WOLFSSL_X509_NAME*)dCert->issuerName; + der = dCert->issuerNameDer; + } + else { + decName = (WOLFSSL_X509_NAME*)dCert->subjectName; + der = dCert->subjectNameDer; + } + + if (decName != NULL) { + if (nameType == ASN_ISSUER) { + wolfSSL_X509_set_issuer_name(x509, decName); + } + else { + wolfSSL_X509_set_subject_name(x509, decName); + } + name->x509 = x509; + } + else if (der != NULL) { + /* Same reset as wolfSSL_X509_set_issuer/subject_name(). */ + FreeX509Name(name); + InitX509Name(name, 0, x509->heap); + ret = GetCertX509Name(dCert, nameType, name); + if (ret == 0) { + name->x509 = x509; + if (nameType == ASN_ISSUER) { + x509->issuerSet = 1; + } + } + else { + FreeX509Name(name); + /* The name was checked when the certificate was parsed, so not + * being able to build it now is a resource failure. */ + ret = MEMORY_E; + } + } + + return ret; +} +#endif /* OPENSSL_EXTRA || OPENSSL_EXTRA_X509_SMALL */ + #ifdef WOLFSSL_CERT_REQ static int CopyREQAttributes(WOLFSSL_X509* x509, DecodedCert* dCert) { @@ -15825,66 +15887,11 @@ static int CopyDecodedPubKey(WOLFSSL_X509* x509, DecodedCert* dCert, int ret) ret = MEMORY_E; #if defined(OPENSSL_ALL) if (ret == 0) { + /* The EVP_PKEY is decoded on first use by X509CachedPubKey() and + * the algorithm object by wolfSSL_X509_PUBKEY_get0_param(). */ x509->key.pubKeyOID = (int)dCert->keyOID; - - if (!x509->key.algor) { - x509->key.algor = wolfSSL_X509_ALGOR_new(); - } else { - wolfSSL_ASN1_OBJECT_free(x509->key.algor->algorithm); - } - if (!x509->key.algor) { - ret = MEMORY_E; - } else { - if (!(x509->key.algor->algorithm = - wolfSSL_OBJ_nid2obj(oid2nid(dCert->keyOID, oidKeyType)))) { - ret = PUBLIC_KEY_E; - WOLFSSL_ERROR_VERBOSE(ret); - } - } - - switch (dCert->keyOID) { - #ifdef HAVE_ED25519 - case ED25519k: - x509->key.pkey = wolfSSL_EVP_PKEY_new_raw_public_key( - WC_EVP_PKEY_ED25519, NULL, dCert->publicKey, - dCert->pubKeySize); - break; - #endif - #ifdef HAVE_ED448 - case ED448k: - x509->key.pkey = wolfSSL_EVP_PKEY_new_raw_public_key( - WC_EVP_PKEY_ED448, NULL, dCert->publicKey, - dCert->pubKeySize); - break; - #endif - #if defined(WOLFSSL_HAVE_MLDSA) && \ - defined(WOLFSSL_MLDSA_PUBLIC_KEY) && \ - !defined(WOLFSSL_MLDSA_NO_ASN1) && \ - defined(WC_ENABLE_ASYM_KEY_EXPORT) - #ifdef WOLFSSL_MLDSA_FIPS204_DRAFT - case DILITHIUM_LEVEL2k: - case DILITHIUM_LEVEL3k: - case DILITHIUM_LEVEL5k: - #endif - case ML_DSA_44k: - case ML_DSA_65k: - case ML_DSA_87k: - /* Raw key size does not distinguish draft from final, so - * encode with the certificate's key OID. This fills - * the x509->key.pkey cache without a new reference. */ - (void)wolfSSL_X509_get0_pubkey(x509); - break; - #endif - default: - x509->key.pkey = wolfSSL_d2i_PUBKEY(NULL, - &dCert->publicKey, dCert->pubKeySize); - break; - } - - if (x509->key.pkey == NULL) { - ret = PUBLIC_KEY_E; - WOLFSSL_ERROR_VERBOSE(ret); - } + wolfSSL_X509_ALGOR_free(x509->key.algor); + x509->key.algor = NULL; } #endif } @@ -16118,18 +16125,16 @@ int CopyDecodedToX509(WOLFSSL_X509* x509, DecodedCert* dCert) CopyDecodedName(&x509->issuer, dCert, ASN_ISSUER); #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) - if (dCert->issuerName != NULL) { - wolfSSL_X509_set_issuer_name(x509, - (WOLFSSL_X509_NAME*)dCert->issuerName); - x509->issuer.x509 = x509; + ret = CopyDecodedX509Name(x509, &x509->issuer, dCert, ASN_ISSUER); + if (ret != 0) { + return ret; } #endif /* OPENSSL_EXTRA || OPENSSL_EXTRA_X509_SMALL */ CopyDecodedName(&x509->subject, dCert, ASN_SUBJECT); #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) - if (dCert->subjectName != NULL) { - wolfSSL_X509_set_subject_name(x509, - (WOLFSSL_X509_NAME*)dCert->subjectName); - x509->subject.x509 = x509; + ret = CopyDecodedX509Name(x509, &x509->subject, dCert, ASN_SUBJECT); + if (ret != 0) { + return ret; } #endif /* OPENSSL_EXTRA || OPENSSL_EXTRA_X509_SMALL */ @@ -17504,6 +17509,11 @@ PRAGMA_GCC_DIAG_POP #endif InitDecodedCert(args->dCert, cert->buffer, cert->length, ssl->heap); + #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) + /* The X509_NAMEs are only needed when the certificate is copied into + * a WOLFSSL_X509, which adds the entries to its names directly. */ + args->dCert->noX509Name = 1; + #endif args->dCertInit = 1; args->dCert->sigCtx.devId = ssl->devId; @@ -17962,6 +17972,10 @@ static int ProcessPeerCertAddPendingCA(WOLFSSL* ssl, buffer* cert) }); InitDecodedCert(dCertAdd, cert->buffer, cert->length, ssl->heap); +#if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) + /* A signer doesn't keep the X509_NAMEs, don't build them. */ + dCertAdd->noX509Name = 1; +#endif dCertAdd_inited = 1; ret = ParseCert(dCertAdd, CA_TYPE, NO_VERIFY, SSL_CM(ssl)); @@ -19277,6 +19291,8 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, if (ret == WC_NO_ERR_TRACE(ASN_NO_SIGNER_E) || ret == WC_NO_ERR_TRACE(ASN_SELF_SIGNED_E)) { WOLFSSL_MSG("try to load certificate if hash dir is set"); + /* Peer certs are parsed without their X509_NAMEs */ + (void)GetCertX509Name(args->dCert, ASN_ISSUER, NULL); ret = LoadCertByIssuer(SSL_STORE(ssl), (WOLFSSL_X509_NAME*)args->dCert->issuerName, X509_LU_X509); @@ -19722,6 +19738,8 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, ret == WC_NO_ERR_TRACE(ASN_SELF_SIGNED_E)) { int lastErr = ret; /* save error from last time */ WOLFSSL_MSG("try to load certificate if hash dir is set"); + /* Peer certs are parsed without their X509_NAMEs */ + (void)GetCertX509Name(args->dCert, ASN_ISSUER, NULL); ret = LoadCertByIssuer(SSL_STORE(ssl), (WOLFSSL_X509_NAME*)args->dCert->issuerName, X509_LU_X509); diff --git a/src/ssl_certman.c b/src/ssl_certman.c index fb9490525bc..6815a6de8dc 100644 --- a/src/ssl_certman.c +++ b/src/ssl_certman.c @@ -3177,6 +3177,10 @@ int AddCA(WOLFSSL_CERT_MANAGER* cm, DerBuffer** pDer, int type, int verify) #endif InitDecodedCert(cert, der->buffer, der->length, cm->heap); +#if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) + /* A signer doesn't keep the X509_NAMEs, don't build them. */ + cert->noX509Name = 1; +#endif #ifdef WC_ASN_UNKNOWN_EXT_CB if (cm->unknownExtCallback32 != NULL) { diff --git a/src/x509.c b/src/x509.c index 62159e99aac..669feb0a7e0 100644 --- a/src/x509.c +++ b/src/x509.c @@ -4625,6 +4625,10 @@ static WOLFSSL_X509* d2i_X509orX509REQ(WOLFSSL_X509** x509, InitDecodedCert(cert, (byte*)in, (word32)len, heap); #ifdef WOLFSSL_CERT_REQ cert->isCSR = (byte)req; + #endif + #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) + /* CopyDecodedToX509() adds the name entries to the X509 directly. */ + cert->noX509Name = 1; #endif if (ParseCertRelative(cert, type, 0, NULL, NULL) == 0) { newX509 = wolfSSL_X509_new_ex(heap); @@ -6609,6 +6613,11 @@ static WOLFSSL_X509* loadX509orX509REQFromBuffer( if (WC_VAR_OK(cert)) { InitDecodedCert(cert, der->buffer, der->length, NULL); + #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) + /* CopyDecodedToX509() adds the name entries to the X509 + * directly. */ + cert->noX509Name = 1; + #endif /* 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 @@ -15583,6 +15592,18 @@ WOLFSSL_ASN1_OBJECT* wolfSSL_X509_NAME_ENTRY_get_object( * HAVE_STUNNEL || WOLFSSL_NGINX || HAVE_POCO_LIB || WOLFSSL_HAPROXY */ #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) + /* Get the object of an entry in a name. + * The object set with the entry is used when it is for the entry's NID, + * rather than filling it in from the OID table again. + */ + static WOLFSSL_ASN1_OBJECT* X509NameEntryObject(WOLFSSL_X509_NAME_ENTRY* e) + { + if ((e->object != NULL) && (e->object->nid == e->nid)) { + return e->object; + } + return wolfSSL_X509_NAME_ENTRY_get_object(e); + } + /* add all entry of type "nid" to the buffer "fullName" and advance "idx" * since number of entries is small, a brute force search is used here * returns the number of entries added @@ -15602,7 +15623,7 @@ WOLFSSL_ASN1_OBJECT* wolfSSL_X509_NAME_ENTRY_get_object( unsigned char* data; e = &name->entry[i]; - obj = wolfSSL_X509_NAME_ENTRY_get_object(e); + obj = X509NameEntryObject(e); if (obj == NULL) { return BAD_FUNC_ARG; } @@ -15643,7 +15664,7 @@ WOLFSSL_ASN1_OBJECT* wolfSSL_X509_NAME_ENTRY_get_object( WOLFSSL_ASN1_OBJECT* obj; e = &name->entry[i]; - obj = wolfSSL_X509_NAME_ENTRY_get_object(e); + obj = X509NameEntryObject(e); if (obj == NULL) return BAD_FUNC_ARG; @@ -15678,22 +15699,15 @@ WOLFSSL_ASN1_OBJECT* wolfSSL_X509_NAME_ENTRY_get_object( return 0; } - /* Copies entry into name. With it being copied freeing entry becomes the - * callers responsibility. + /* Set an entry in name from the NID, string type and data. + * idx is the index to place the entry at or -1 for the first unset one. * returns 1 for success and 0 for error */ - int wolfSSL_X509_NAME_add_entry(WOLFSSL_X509_NAME* name, - WOLFSSL_X509_NAME_ENTRY* entry, int idx, int set) + static int X509NameAddEntry(WOLFSSL_X509_NAME* name, int nid, int type, + const unsigned char* data, int dataSz, int idx) { WOLFSSL_X509_NAME_ENTRY* current = NULL; int ret, i; - WOLFSSL_ENTER_VERBOSE("wolfSSL_X509_NAME_add_entry"); - - if (name == NULL || entry == NULL || entry->value == NULL) { - WOLFSSL_MSG("NULL argument passed in"); - return WOLFSSL_FAILURE; - } - if (idx >= 0) { /* place in specific index */ @@ -15722,11 +15736,8 @@ WOLFSSL_ASN1_OBJECT* wolfSSL_X509_NAME_ENTRY_get_object( if (current->set == 0) name->entrySz++; - if (wolfSSL_X509_NAME_ENTRY_create_by_NID(¤t, - entry->nid, - wolfSSL_ASN1_STRING_type(entry->value), - wolfSSL_ASN1_STRING_data(entry->value), - wolfSSL_ASN1_STRING_length(entry->value)) != NULL) + if ((wolfSSL_X509_NAME_ENTRY_create_by_NID(¤t, nid, type, data, + dataSz) != NULL) && (current->value != NULL)) { ret = WOLFSSL_SUCCESS; #ifdef OPENSSL_ALL @@ -15761,10 +15772,29 @@ WOLFSSL_ASN1_OBJECT* wolfSSL_X509_NAME_ENTRY_get_object( if (RebuildFullName(name) != 0) return WOLFSSL_FAILURE; - (void)set; return WOLFSSL_SUCCESS; } + /* Copies entry into name. With it being copied freeing entry becomes the + * callers responsibility. + * returns 1 for success and 0 for error */ + int wolfSSL_X509_NAME_add_entry(WOLFSSL_X509_NAME* name, + WOLFSSL_X509_NAME_ENTRY* entry, int idx, int set) + { + WOLFSSL_ENTER_VERBOSE("wolfSSL_X509_NAME_add_entry"); + + if (name == NULL || entry == NULL || entry->value == NULL) { + WOLFSSL_MSG("NULL argument passed in"); + return WOLFSSL_FAILURE; + } + + (void)set; + return X509NameAddEntry(name, entry->nid, + wolfSSL_ASN1_STRING_type(entry->value), + wolfSSL_ASN1_STRING_data(entry->value), + wolfSSL_ASN1_STRING_length(entry->value), idx); + } + int wolfSSL_X509_NAME_add_entry_by_txt(WOLFSSL_X509_NAME *name, const char *field, int type, const unsigned char *bytes, int len, @@ -15800,16 +15830,14 @@ WOLFSSL_ASN1_OBJECT* wolfSSL_X509_NAME_ENTRY_get_object( int type, const unsigned char *bytes, int len, int loc, int set) { - int ret; - WOLFSSL_X509_NAME_ENTRY* entry; WOLFSSL_ENTER_VERBOSE("wolfSSL_X509_NAME_add_entry_by_NID"); - entry = wolfSSL_X509_NAME_ENTRY_create_by_NID(NULL, nid, type, bytes, - len); - if (entry == NULL) + if (name == NULL || bytes == NULL) { + WOLFSSL_MSG("NULL argument passed in"); return WOLFSSL_FAILURE; - ret = wolfSSL_X509_NAME_add_entry(name, entry, loc, set); - wolfSSL_X509_NAME_ENTRY_free(entry); - return ret; + } + (void)set; + /* Set the entry in place rather than copying a temporary one. */ + return X509NameAddEntry(name, nid, type, bytes, len, loc); } WOLFSSL_X509_NAME_ENTRY *wolfSSL_X509_NAME_delete_entry( diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index afec636c635..c31eb6b046b 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -16076,16 +16076,20 @@ static int GetRDN(DecodedCert* cert, char* full, word32* idx, int* nid, /* Add the attribute value last parsed into an X509_NAME. * - * @param [in, out] dName X509_NAME to add to. NULL is ignored. - * @param [in] nid OpenSSL NID of the attribute type. 0 when unknown, - * in which case nothing is added. - * @param [in] val Parsed attribute value (rdnChoice data). + * @param [in, out] dName X509_NAME to add to. NULL when the X509_NAME is + * not being built. + * @param [in] nid OpenSSL NID of the attribute type. 0 when unknown, + * in which case nothing is added. + * @param [in] val Parsed attribute value (rdnChoice data). + * @param [in, out] entries Number of entries the X509_NAME would hold. Used + * when dName is NULL. * @return 0 on success. * @return ASN_PARSE_E when the string is empty or the entry could not be * added. */ #ifdef WOLFSSL_X509_NAME_AVAILABLE -static int AddRDNToName(WOLFSSL_X509_NAME* dName, int nid, ASNGetData* val) +static int AddRDNToName(WOLFSSL_X509_NAME* dName, int nid, ASNGetData* val, + int* entries) { int ret = 0; int enc; @@ -16137,6 +16141,14 @@ static int AddRDNToName(WOLFSSL_X509_NAME* dName, int nid, ASNGetData* val) ret = ASN_PARSE_E; } } + else if (nid != 0) { + /* Fail when the X509_NAME, built later by GetCertX509Name(), can't + * hold all the entries - as when it is built now. */ + if (++(*entries) > MAX_NAME_ENTRIES) { + WOLFSSL_MSG("Too many entries for X509_NAME"); + ret = ASN_PARSE_E; + } + } return ret; } #endif /* WOLFSSL_X509_NAME_AVAILABLE */ @@ -16171,6 +16183,7 @@ static int GetCertName(DecodedCert* cert, char* full, byte* hash, int nameType, word32 srcIdx = *inOutIdx; #ifdef WOLFSSL_X509_NAME_AVAILABLE WOLFSSL_X509_NAME* dName = NULL; + int nameEntries = 0; #endif /* WOLFSSL_X509_NAME_AVAILABLE */ WOLFSSL_MSG_VERBOSE("Getting Cert Name"); @@ -16193,7 +16206,21 @@ static int GetCertName(DecodedCert* cert, char* full, byte* hash, int nameType, CALLOC_ASNGETDATA(dataASN, rdnASN_Length, ret, cert->heap); #ifdef WOLFSSL_X509_NAME_AVAILABLE - if (ret == 0) { + /* Keep where the encoded name is so GetCertX509Name() can build the + * X509_NAME later. */ + if (nameType == ASN_ISSUER) { + cert->issuerNameDer = input + srcIdx; + cert->issuerNameDerSz = maxIdx - srcIdx; + } + else { + cert->subjectNameDer = input + srcIdx; + cert->subjectNameDerSz = maxIdx - srcIdx; + } + if (cert->x509NameTarget != NULL) { + /* Fill in the X509_NAME passed in. */ + dName = (WOLFSSL_X509_NAME*)cert->x509NameTarget; + } + else if ((ret == 0) && (!cert->noX509Name)) { /* Create an X509_NAME to hold data for OpenSSL compatibility APIs. */ dName = wolfSSL_X509_NAME_new_ex(cert->heap); if (dName == NULL) { @@ -16255,10 +16282,12 @@ static int GetCertName(DecodedCert* cert, char* full, byte* hash, int nameType, dataASN); } #ifdef WOLFSSL_X509_NAME_AVAILABLE - /* TODO: push this back up to ssl.c - * (do parsing for WOLFSSL_X509_NAME on demand) */ + /* The entry is only added when the X509_NAME is being built. + * GetCertX509Name() builds it on demand when parsing skipped it, + * but the checks are always done. */ if (ret == 0) { - ret = AddRDNToName(dName, nid, &dataASN[RDNASN_IDX_ATTR_VAL]); + ret = AddRDNToName(dName, nid, &dataASN[RDNASN_IDX_ATTR_VAL], + &nameEntries); } #endif @@ -16282,7 +16311,8 @@ static int GetCertName(DecodedCert* cert, char* full, byte* hash, int nameType, #ifdef WOLFSSL_X509_NAME_AVAILABLE if (ret == 0) { ret = AddRDNToName(dName, nid, - &dataASN[RDNASN_IDX_ATTR_VAL]); + &dataASN[RDNASN_IDX_ATTR_VAL], + &nameEntries); } #endif } @@ -16295,8 +16325,9 @@ static int GetCertName(DecodedCert* cert, char* full, byte* hash, int nameType, *inOutIdx = srcIdx; #ifdef WOLFSSL_X509_NAME_AVAILABLE - /* Store X509_NAME in certificate. */ - if (nameType == ASN_ISSUER) { + /* Store X509_NAME in certificate, when it was built and not passed + * in. */ + if ((dName != NULL) && (nameType == ASN_ISSUER)) { #if (defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || \ defined(HAVE_LIGHTY)) && \ (defined(HAVE_PKCS7) || defined(WOLFSSL_CERT_EXT)) @@ -16304,18 +16335,22 @@ static int GetCertName(DecodedCert* cert, char* full, byte* hash, int nameType, WC_ASN_NAME_MAX); XMEMCPY(dName->raw, cert->issuerRaw, (size_t)dName->rawLen); #endif - cert->issuerName = dName; + if (dName != cert->x509NameTarget) { + cert->issuerName = dName; + } } - else { + else if (dName != NULL) { #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) dName->rawLen = (int)min((word32)cert->subjectRawLen, WC_ASN_NAME_MAX); XMEMCPY(dName->raw, cert->subjectRaw, (size_t)dName->rawLen); #endif - cert->subjectName = dName; + if (dName != cert->x509NameTarget) { + cert->subjectName = dName; + } } } - else { + else if (dName != cert->x509NameTarget) { /* Dispose of unused X509_NAME. */ wolfSSL_X509_NAME_free(dName); #endif @@ -16394,6 +16429,77 @@ int GetName(DecodedCert* cert, int nameType, int maxIdx) return ret; } #endif /* WOLFSSL_ASN_TEMPLATE */ + +#ifdef WOLFSSL_X509_NAME_AVAILABLE +/* Build the WOLFSSL_X509_NAME for the issuer or subject of a parsed + * certificate when parsing skipped it (noX509Name set). + * + * The encoded name is parsed again in a temporary DecodedCert so that the + * fields of cert are left as they are. + * + * @param [in, out] cert Decoded certificate object. + * @param [in] nameType ASN_ISSUER or ASN_SUBJECT. + * @param [in, out] target Empty WOLFSSL_X509_NAME to add the entries to. + * NULL to store a new one in cert. + * @return 0 on success, including when the name in cert was already built. + * @return BAD_FUNC_ARG when cert is NULL or the encoded name is not known. + * @return MEMORY_E when dynamic memory allocation fails. + * @return Other negative values when parsing the name fails. + */ +int GetCertX509Name(DecodedCert* cert, int nameType, void* target) +{ + int ret = 0; + void** name; + const byte* der; + word32 derSz; + WC_DECLARE_VAR(tmp, DecodedCert, 1, 0); + + if (cert == NULL) { + return BAD_FUNC_ARG; + } + if (nameType == ASN_ISSUER) { + name = &cert->issuerName; + der = cert->issuerNameDer; + derSz = cert->issuerNameDerSz; + } + else { + name = &cert->subjectName; + der = cert->subjectNameDer; + derSz = cert->subjectNameDerSz; + } + if ((target == NULL) && (*name != NULL)) { + return 0; + } + if ((der == NULL) || (derSz == 0)) { + return BAD_FUNC_ARG; + } + + WC_ALLOC_VAR_EX(tmp, DecodedCert, 1, cert->heap, DYNAMIC_TYPE_DCERT, + return MEMORY_E); + InitDecodedCert(tmp, der, derSz, cert->heap); + tmp->x509NameTarget = target; + ret = GetName(tmp, nameType, (int)derSz); + if ((ret == 0) && (target == NULL)) { + /* Take ownership of the X509_NAME from the temporary object. */ + if (nameType == ASN_ISSUER) { + *name = tmp->issuerName; + tmp->issuerName = NULL; + } + else { + *name = tmp->subjectName; + tmp->subjectName = NULL; + } + if (*name == NULL) { + ret = ASN_PARSE_E; + } + } + FreeDecodedCert(tmp); + WC_FREE_VAR_EX(tmp, cert->heap, DYNAMIC_TYPE_DCERT); + + return ret; +} +#endif /* WOLFSSL_X509_NAME_AVAILABLE */ + #ifndef NO_ASN_TIME /* two byte date/time, add to value */ diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 81870bba23d..f369ce20c3e 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -2170,6 +2170,14 @@ struct DecodedCert { /* WOLFSSL_X509_NAME structures (used void* to avoid including ssl.h) */ void* issuerName; void* subjectName; + /* Encoded issuer and subject Names in source, for GetCertX509Name() */ + const byte* issuerNameDer; + const byte* subjectNameDer; + word32 issuerNameDerSz; + word32 subjectNameDerSz; + /* WOLFSSL_X509_NAME to fill in when parsing a name instead of + * allocating one. Set by GetCertX509Name(). */ + void* x509NameTarget; #endif /* OPENSSL_EXTRA || OPENSSL_EXTRA_X509_SMALL */ #ifdef WOLFSSL_SEP int deviceTypeSz; @@ -2303,6 +2311,12 @@ struct DecodedCert { * TRUSTED CERTIFICATE auxiliary trust * info. */ WC_BITFIELD selfIssued:1; /* Issuer name matches subject name */ +#if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) + WC_BITFIELD noX509Name:1; /* don't build issuerName and + * subjectName when parsing. Use + * GetCertX509Name() to build them + * when needed. */ +#endif #ifdef WC_ASN_UNKNOWN_EXT_CB wc_UnknownExtCallback unknownExtCallback; wc_UnknownExtCallback32 unknownExtCallback32; @@ -2547,6 +2561,11 @@ WOLFSSL_LOCAL int CalcHashId_ex(const byte* data, word32 len, byte* hash, WOLFSSL_LOCAL int GetHashId(const byte* id, int length, byte* hash, int hashAlg); WOLFSSL_LOCAL int GetName(DecodedCert* cert, int nameType, int maxIdx); +#if (defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL)) && \ + !defined(WOLFCRYPT_ONLY) +WOLFSSL_LOCAL int GetCertX509Name(DecodedCert* cert, int nameType, + void* target); +#endif #ifdef ASN_BER_TO_DER WOLFSSL_API int wc_BerToDer(const byte* ber, word32 berSz, byte* der, From 4b5886219001cfdd0c9e150955eeb096d31909d3 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 28 Oct 2026 13:51:25 +0000 Subject: [PATCH 2/9] Test the X509_NAME entry limit with names built on demand Parsing that skips building the X509_NAMEs must still reject a name with more attributes than an X509_NAME holds. Add self-signed CAs whose subject and issuer have MAX_NAME_ENTRIES (16 with OPENSSL_EXTRA) and one more attributes, generated by gen-ext-certs.sh. Loading the first as an X509 gives 16 entries in each name and loading it as a CA succeeds; the second is rejected by both. --- certs/test/cert-max-name-entries.cfg | 25 ++++++++ certs/test/cert-max-name-entries.der | Bin 0 -> 1255 bytes certs/test/cert-max-name-entries.pem | 29 ++++++++++ certs/test/cert-over-max-name-entries.cfg | 26 +++++++++ certs/test/cert-over-max-name-entries.der | Bin 0 -> 1291 bytes certs/test/cert-over-max-name-entries.pem | 29 ++++++++++ certs/test/gen-ext-certs.sh | 66 ++++++++++++++++++++++ certs/test/include.am | 4 ++ tests/api/test_ossl_x509.c | 46 +++++++++++++++ tests/api/test_ossl_x509.h | 2 + 10 files changed, 227 insertions(+) create mode 100644 certs/test/cert-max-name-entries.cfg create mode 100644 certs/test/cert-max-name-entries.der create mode 100644 certs/test/cert-max-name-entries.pem create mode 100644 certs/test/cert-over-max-name-entries.cfg create mode 100644 certs/test/cert-over-max-name-entries.der create mode 100644 certs/test/cert-over-max-name-entries.pem diff --git a/certs/test/cert-max-name-entries.cfg b/certs/test/cert-max-name-entries.cfg new file mode 100644 index 00000000000..f526cb52af1 --- /dev/null +++ b/certs/test/cert-max-name-entries.cfg @@ -0,0 +1,25 @@ +[ req ] +distinguished_name = req_distinguished_name +prompt = no +x509_extensions = v3_ca + +[ req_distinguished_name ] +0.OU = Unit 1 +1.OU = Unit 2 +2.OU = Unit 3 +3.OU = Unit 4 +4.OU = Unit 5 +5.OU = Unit 6 +6.OU = Unit 7 +7.OU = Unit 8 +8.OU = Unit 9 +9.OU = Unit 10 +10.OU = Unit 11 +11.OU = Unit 12 +12.OU = Unit 13 +13.OU = Unit 14 +14.OU = Unit 15 +CN = www.wolfssl.com + +[ v3_ca ] +basicConstraints = critical, CA:TRUE diff --git a/certs/test/cert-max-name-entries.der b/certs/test/cert-max-name-entries.der new file mode 100644 index 0000000000000000000000000000000000000000..04f66dbb92cef4780da233f9a01d3c45a8a6b4a9 GIT binary patch literal 1255 zcmXqLVtH)P#C&=IGZP~dlZe@&Yd3}cA81D<7aFL&DwO}vYfx&y%f_kI=F#?@mywa1 zmBFBiQQnXrD99Yj!p*}LnwMFkV2CbYgf3u=E?|N#V2UnahAv=^E?|K!U}-2|zz1;{ zJH%lI7$SxkB1RY@#uy?d7$T;I5(Z)*yO??S%gf94%JXy5ii>mflJj#7?zcF22ag+$nNNU}wrK)0f(HD+L?|oF$oQXy)qojDAqN{UzXF4ekzuC!tt?kjg9AtQw9P+| zp7m^vpG05_M}(L6nTcZBn-#rc%l6z^S0{RLSc6&^P%%&Z; z+HFd{6#v?{UAt-a2FW$Xkmgb84K?#leYNN^-k`> zx@$60D(53t9gtys6W|)Qnd$2LY*jsz3C}wAeY5TP>XN@l`09#t{ae>ozuzOgiT}i| zh0ibTY|eVydQqx>)z$>X6A9`2j;`wn^|X&G?!OmfH|@qP^L(l6Yu$dc7d+<)OK7gU z_u5Y{arMX7Gk4D_+Ov;6UW`R6T=LWVGha@1CT}rby{}C3-|NlII-g3s&w4K61ppWp Bk;ec4 literal 0 HcmV?d00001 diff --git a/certs/test/cert-max-name-entries.pem b/certs/test/cert-max-name-entries.pem new file mode 100644 index 00000000000..13f79fcd21d --- /dev/null +++ b/certs/test/cert-max-name-entries.pem @@ -0,0 +1,29 @@ +-----BEGIN CERTIFICATE----- +MIIE4zCCA8ugAwIBAgIUNsLW2RNP4CtaY3EwJupxH/8NMHUwDQYJKoZIhvcNAQEL +BQAwggEfMQ8wDQYDVQQLDAZVbml0IDExDzANBgNVBAsMBlVuaXQgMjEPMA0GA1UE +CwwGVW5pdCAzMQ8wDQYDVQQLDAZVbml0IDQxDzANBgNVBAsMBlVuaXQgNTEPMA0G +A1UECwwGVW5pdCA2MQ8wDQYDVQQLDAZVbml0IDcxDzANBgNVBAsMBlVuaXQgODEP +MA0GA1UECwwGVW5pdCA5MRAwDgYDVQQLDAdVbml0IDEwMRAwDgYDVQQLDAdVbml0 +IDExMRAwDgYDVQQLDAdVbml0IDEyMRAwDgYDVQQLDAdVbml0IDEzMRAwDgYDVQQL +DAdVbml0IDE0MRAwDgYDVQQLDAdVbml0IDE1MRgwFgYDVQQDDA93d3cud29sZnNz +bC5jb20wHhcNMjYxMDI4MTM0NTQzWhcNMjkwNzI0MTM0NTQzWjCCAR8xDzANBgNV +BAsMBlVuaXQgMTEPMA0GA1UECwwGVW5pdCAyMQ8wDQYDVQQLDAZVbml0IDMxDzAN +BgNVBAsMBlVuaXQgNDEPMA0GA1UECwwGVW5pdCA1MQ8wDQYDVQQLDAZVbml0IDYx +DzANBgNVBAsMBlVuaXQgNzEPMA0GA1UECwwGVW5pdCA4MQ8wDQYDVQQLDAZVbml0 +IDkxEDAOBgNVBAsMB1VuaXQgMTAxEDAOBgNVBAsMB1VuaXQgMTExEDAOBgNVBAsM +B1VuaXQgMTIxEDAOBgNVBAsMB1VuaXQgMTMxEDAOBgNVBAsMB1VuaXQgMTQxEDAO +BgNVBAsMB1VuaXQgMTUxGDAWBgNVBAMMD3d3dy53b2xmc3NsLmNvbTCCASIwDQYJ +KoZIhvcNAQEBBQADggEPADCCAQoCggEBAMCVCOFXQfJxbbfSRUEnAWXGRa7yvCQw +uJXOL07W9hyIvHyf+6hnf/5cnFF194rKB+c1L4/hvXvAL3yrZKgX/Mpde7rgIeVy +Lm8uhtiVc9qsG1O5Xz/XGQ0lT+FjY1GLC2Q/rUO4pRxcNLOuAKBjxfZ/C1loeHOm +jBipAm2vwxkBLrgQ48bMQLRpo0YzaYduxLsXpvPo3a1zvHsvIbX9ZlEMvVSz4W1f +HLwjc9EJA4kU0hC5ZMMq0KGWSrzh1Bpbx6DAwWN4D0Q3MDKWgDIjlaF3uhPSl3Pi +XSXJag3DOWCktLBpQkIJ6dgIvDMgs1gip6rrxOHmYYPF0pbf2dBPrdcCAwEAAaMT +MBEwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAmTfaakUVMMDE +vIafwGdq5qxOGFGECFhKS8yRFiuzIUpddrzcrn4V1H9gVtD0ND2ke4sevNCV8e23 +XHgcsrjahzx09HP6vrcrgpuwGawzd1S0jGuXtE9bq7/QnJo2co4wK6/DWEIzE752 +Gzj8iu4e0K7WHBokz1mqwBwB7FBFVrMC1e9rJS40kOaIvvY9jPVEb7wT1ajOj7Wt +e++8E7IPyLqh59K5g2rthdEaj6q1YCHIYGe+xa6IVUk/XnOP3lw+ltjaN28a161G ++wdw5wxWYIN+3utOLmGr8euZu5pyvL4HXxYEKlcZ8u/M9MqJY7Qzq752Kf7rs4Ms +8nRLzUmiDQ== +-----END CERTIFICATE----- diff --git a/certs/test/cert-over-max-name-entries.cfg b/certs/test/cert-over-max-name-entries.cfg new file mode 100644 index 00000000000..d1a7e593be5 --- /dev/null +++ b/certs/test/cert-over-max-name-entries.cfg @@ -0,0 +1,26 @@ +[ req ] +distinguished_name = req_distinguished_name +prompt = no +x509_extensions = v3_ca + +[ req_distinguished_name ] +0.OU = Unit 1 +1.OU = Unit 2 +2.OU = Unit 3 +3.OU = Unit 4 +4.OU = Unit 5 +5.OU = Unit 6 +6.OU = Unit 7 +7.OU = Unit 8 +8.OU = Unit 9 +9.OU = Unit 10 +10.OU = Unit 11 +11.OU = Unit 12 +12.OU = Unit 13 +13.OU = Unit 14 +14.OU = Unit 15 +15.OU = Unit 16 +CN = www.wolfssl.com + +[ v3_ca ] +basicConstraints = critical, CA:TRUE diff --git a/certs/test/cert-over-max-name-entries.der b/certs/test/cert-over-max-name-entries.der new file mode 100644 index 0000000000000000000000000000000000000000..cc757aecf404ae76bc173aab364543b8525313cc GIT binary patch literal 1291 zcmXqLVr4gIVt&7XnTe5!Nu=~mi2bwXX$hIJRo57P{C4)db=1j#myJ`a&7?y4g`0;hG%vG60YktDU4=2afC;*QDY}3ex_~*lfCajMrJ;ZUAH-qo z5QiCHh!|ps7-5JQV~Chwh?ruCm>EhKh=FWp=HV|dFV`#2&q*sT&e2QG&oz(}=QT1j zG%&I-G&V6cF^&RqEe*_#OrYG+K!rpXX@T0rsDzw28Ce;an;7{)DU^$;iII`vz*LTh z;f|jQbGKh|byR0eJ?6UZ(;gLr9aGQg`(67c)3K*!{_hp(_5Wh#1eSj9I>r9nRKNe> z-s%JTHLFuri2peiTfOUn;?p9%e7&|CQ;Tn{kq+J&Z+~5qSJnSva&lldcZ&U5=N(IB zVoWx#V_1-U^jkf5WJX2tvL1<*Ou6e1OET*15O{p-jKh}9#csx#?RiIbi!b~9;_lkw zJ=OY(TmPm7^6U-S{4h6OW{-05MNZ~UkxK$QQx0ogSUAmV&%-NH(Z?4YIG9|)?_zFX zG_Ao%dFsOQUBZ{97e9(sJ(eAVkC51%DAAH6i~ z{>=;iYp*jgGcqtP7B&zx;0Gp4Sz$)T|17Kq%s>h`*noK&7;KCT`?_ukE|Ay}9>Dfi zx8fdC&dSYi7#{wb9C7{74eN{SZ(Dg+2C7bc%J{xJ>DNqu#Xc^Bid+9DZdzpcymieU zqeFifz3*JOq4Ma=zHM#?y%#Lov|;HFovi%y4>M9MtSwJ1I`(+t&j(%_T=^@F_`fr1 zPne>?zF}2C-u~`q#~02>Kb5(Pt;jzujO&Zsbol-Kh}bF`ne z=&H@t`62879A3}qRuG(Ax>hvH>M666ln6XDio7K(u9{fyw(%+e Dreu?{ literal 0 HcmV?d00001 diff --git a/certs/test/cert-over-max-name-entries.pem b/certs/test/cert-over-max-name-entries.pem new file mode 100644 index 00000000000..b15ed6cef21 --- /dev/null +++ b/certs/test/cert-over-max-name-entries.pem @@ -0,0 +1,29 @@ +-----BEGIN CERTIFICATE----- +MIIFBzCCA++gAwIBAgIUdexUP+aDlmBpXXrWAPj7Q0naxUIwDQYJKoZIhvcNAQEL +BQAwggExMQ8wDQYDVQQLDAZVbml0IDExDzANBgNVBAsMBlVuaXQgMjEPMA0GA1UE +CwwGVW5pdCAzMQ8wDQYDVQQLDAZVbml0IDQxDzANBgNVBAsMBlVuaXQgNTEPMA0G +A1UECwwGVW5pdCA2MQ8wDQYDVQQLDAZVbml0IDcxDzANBgNVBAsMBlVuaXQgODEP +MA0GA1UECwwGVW5pdCA5MRAwDgYDVQQLDAdVbml0IDEwMRAwDgYDVQQLDAdVbml0 +IDExMRAwDgYDVQQLDAdVbml0IDEyMRAwDgYDVQQLDAdVbml0IDEzMRAwDgYDVQQL +DAdVbml0IDE0MRAwDgYDVQQLDAdVbml0IDE1MRAwDgYDVQQLDAdVbml0IDE2MRgw +FgYDVQQDDA93d3cud29sZnNzbC5jb20wHhcNMjYxMDI4MTM0NTQzWhcNMjkwNzI0 +MTM0NTQzWjCCATExDzANBgNVBAsMBlVuaXQgMTEPMA0GA1UECwwGVW5pdCAyMQ8w +DQYDVQQLDAZVbml0IDMxDzANBgNVBAsMBlVuaXQgNDEPMA0GA1UECwwGVW5pdCA1 +MQ8wDQYDVQQLDAZVbml0IDYxDzANBgNVBAsMBlVuaXQgNzEPMA0GA1UECwwGVW5p +dCA4MQ8wDQYDVQQLDAZVbml0IDkxEDAOBgNVBAsMB1VuaXQgMTAxEDAOBgNVBAsM +B1VuaXQgMTExEDAOBgNVBAsMB1VuaXQgMTIxEDAOBgNVBAsMB1VuaXQgMTMxEDAO +BgNVBAsMB1VuaXQgMTQxEDAOBgNVBAsMB1VuaXQgMTUxEDAOBgNVBAsMB1VuaXQg +MTYxGDAWBgNVBAMMD3d3dy53b2xmc3NsLmNvbTCCASIwDQYJKoZIhvcNAQEBBQAD +ggEPADCCAQoCggEBAMCVCOFXQfJxbbfSRUEnAWXGRa7yvCQwuJXOL07W9hyIvHyf ++6hnf/5cnFF194rKB+c1L4/hvXvAL3yrZKgX/Mpde7rgIeVyLm8uhtiVc9qsG1O5 +Xz/XGQ0lT+FjY1GLC2Q/rUO4pRxcNLOuAKBjxfZ/C1loeHOmjBipAm2vwxkBLrgQ +48bMQLRpo0YzaYduxLsXpvPo3a1zvHsvIbX9ZlEMvVSz4W1fHLwjc9EJA4kU0hC5 +ZMMq0KGWSrzh1Bpbx6DAwWN4D0Q3MDKWgDIjlaF3uhPSl3PiXSXJag3DOWCktLBp +QkIJ6dgIvDMgs1gip6rrxOHmYYPF0pbf2dBPrdcCAwEAAaMTMBEwDwYDVR0TAQH/ +BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAvoraEaAYuFdQBu0teN4CbKmz7ADh ++pNY18LYO9EH7YUNqVElkeUB74ti+plPIY4KMHja/5GyojHnhay8MsL8AUvc0Ngk +4sy+tkbBS6CmsrCl+Cxqb2fwmGQ4OznKosbjkfngSigKb6kyD/cBK5CUKAewqnBu +v4vmx6GYZ8ppqgZyT19SZvUv4KvhEpwARPjicsaAe+rLs72VrqgfSZOBCFonCc0p +RLCecYCunYQItaDIc6dgQu09QEwPgcR4yp503B4jI2h4MlkfLkhmQT+aotU81Z9U +r/zDrwlGcFNjda0VajrlAyqux+23RAY5OnY8c3x0EOEyWdoTFyVhf0ax1Q== +-----END CERTIFICATE----- diff --git a/certs/test/gen-ext-certs.sh b/certs/test/gen-ext-certs.sh index 0d88a7048cd..b05a1300e5a 100755 --- a/certs/test/gen-ext-certs.sh +++ b/certs/test/gen-ext-certs.sh @@ -1541,3 +1541,69 @@ DNS.1050 = example1050.com EOF gen_cert +# Subject and issuer with 16 attributes: the most an X509_NAME holds +# with OPENSSL_EXTRA +OUT=certs/test/cert-max-name-entries +CONFIG=certs/test/cert-max-name-entries.cfg +tee >$CONFIG <$CONFIG < Date: Wed, 28 Oct 2026 14:03:40 +0000 Subject: [PATCH 3/9] Load the name entry limit test CA with a certificate manager The test created a WOLFSSL_CTX from a server or client method, which the API guard check rejects as reachable in a build with neither. Loading the CAs through a WOLFSSL_CERT_MANAGER goes to AddCA() the same way and needs no TLS method. --- tests/api/test_ossl_x509.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/api/test_ossl_x509.c b/tests/api/test_ossl_x509.c index 0264d7335d0..808ee0f1a1d 100644 --- a/tests/api/test_ossl_x509.c +++ b/tests/api/test_ossl_x509.c @@ -2125,7 +2125,7 @@ int test_wolfSSL_X509_max_name_entries(void) #if WC_MAX_NAME_ENTRIES == 16 WOLFSSL_X509* x509 = NULL; - WOLFSSL_CTX* ctx = NULL; + WOLFSSL_CERT_MANAGER* cm = NULL; /* Self-signed CAs with 16 and 17 attributes in the subject and issuer */ const char* max_entries = "./certs/test/cert-max-name-entries.pem"; const char* over_max_entries = "./certs/test/cert-over-max-name-entries.pem"; @@ -2145,16 +2145,12 @@ int test_wolfSSL_X509_max_name_entries(void) /* Loading a CA doesn't build the X509_NAMEs, but still rejects names * that they can't hold. */ -#ifndef NO_WOLFSSL_SERVER - ExpectNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_server_method())); -#else - ExpectNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_client_method())); -#endif - ExpectIntEQ(wolfSSL_CTX_load_verify_locations(ctx, max_entries, NULL), + ExpectNotNull(cm = wolfSSL_CertManagerNew()); + ExpectIntEQ(wolfSSL_CertManagerLoadCA(cm, max_entries, NULL), WOLFSSL_SUCCESS); - ExpectIntNE(wolfSSL_CTX_load_verify_locations(ctx, over_max_entries, - NULL), WOLFSSL_SUCCESS); - wolfSSL_CTX_free(ctx); + ExpectIntNE(wolfSSL_CertManagerLoadCA(cm, over_max_entries, NULL), + WOLFSSL_SUCCESS); + wolfSSL_CertManagerFree(cm); #endif #endif From 7209943415de6dc978f4c12efa2a18a5a833b3ff Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 28 Oct 2026 14:45:59 +0000 Subject: [PATCH 4/9] Distribute the DER forms of the name entry limit test certificates gen-ext-certs.sh writes both DER and PEM and both are committed, so list the DER files in EXTRA_DIST too. --- certs/test/include.am | 2 ++ 1 file changed, 2 insertions(+) diff --git a/certs/test/include.am b/certs/test/include.am index 245a6df5d9e..0857c3b5d34 100644 --- a/certs/test/include.am +++ b/certs/test/include.am @@ -49,8 +49,10 @@ EXTRA_DIST += \ certs/test/cert-over-max-nc.cfg \ certs/test/cert-over-max-nc.pem \ certs/test/cert-max-name-entries.cfg \ + certs/test/cert-max-name-entries.der \ certs/test/cert-max-name-entries.pem \ certs/test/cert-over-max-name-entries.cfg \ + certs/test/cert-over-max-name-entries.der \ certs/test/cert-over-max-name-entries.pem \ certs/test/client-ecc-cert-ski.hex From f8ec337e5358a99a0dbd83e0fa94bc16e728ec4d Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 28 Oct 2026 19:50:23 +0000 Subject: [PATCH 5/9] Decode Ed448 and other certificate public keys on first use With OPENSSL_ALL, the EVP_PKEY of a certificate used to be made when the certificate was parsed: wolfSSL_EVP_PKEY_new_raw_public_key() for Ed448 and wolfSSL_d2i_PUBKEY() for key types without a case of their own. It is now made on first use by X509DecodePubKey(), which treated those keys as EC keys, so X509_get_pubkey() returned NULL for an Ed448 certificate. Import Ed448 keys in X509DecodePubKey() and decode other non-EC key types with wolfSSL_d2i_PUBKEY(), as the parse-time code did. This also makes X509_get_pubkey() work for these keys in OPENSSL_EXTRA builds without OPENSSL_ALL. --- src/x509.c | 38 ++++++++++++++++++++++++ tests/api/test_ossl_x509_pk.c | 54 +++++++++++++++++++++++++++++++++++ tests/api/test_ossl_x509_pk.h | 2 ++ 3 files changed, 94 insertions(+) diff --git a/src/x509.c b/src/x509.c index 669feb0a7e0..ba5817f009a 100644 --- a/src/x509.c +++ b/src/x509.c @@ -6868,6 +6868,22 @@ static WOLFSSL_EVP_PKEY* X509DecodePubKey(WOLFSSL_X509* x509) else if (x509->pubKeyOID == ED25519k) { key->type = WC_EVP_PKEY_ED25519; } + #endif + #if defined(OPENSSL_EXTRA) && defined(HAVE_ED448) && \ + defined(HAVE_ED448_KEY_IMPORT) + else if (x509->pubKeyOID == ED448k) { + key->type = WC_EVP_PKEY_ED448; + } + #endif + #ifdef OPENSSL_EXTRA + else if (x509->pubKeyOID != ECDSAk && x509->pubKeyOID != SM2k) { + /* Other key types are decoded from the encoded key. */ + const unsigned char* der = x509->pubKey.buffer; + + wolfSSL_EVP_PKEY_free(key); + return wolfSSL_d2i_PUBKEY(NULL, &der, + (long)x509->pubKey.length); + } #endif else { key->type = WC_EVP_PKEY_EC; @@ -6983,6 +6999,28 @@ static WOLFSSL_EVP_PKEY* X509DecodePubKey(WOLFSSL_X509* x509) } #endif /* HAVE_ED25519 */ + /* decode Ed448 key */ + #if defined(OPENSSL_EXTRA) && defined(HAVE_ED448) && \ + defined(HAVE_ED448_KEY_IMPORT) + if (key->type == WC_EVP_PKEY_ED448) { + key->ed448 = wolfSSL_ED448_new(x509->heap, INVALID_DEVID); + if (key->ed448 == NULL) { + wolfSSL_EVP_PKEY_free(key); + return NULL; + } + key->ownEd448 = 1; + + /* The X.509 public key buffer holds the raw Ed448 key. */ + if (wc_ed448_import_public( + (const unsigned char*)key->pkey.ptr, + (word32)key->pkey_sz, key->ed448) != 0) { + WOLFSSL_MSG("wc_ed448_import_public failed"); + wolfSSL_EVP_PKEY_free(key); + return NULL; + } + } + #endif /* HAVE_ED448 */ + /* re-encode ML-DSA key */ #if defined(WOLFSSL_HAVE_MLDSA) && \ defined(WOLFSSL_MLDSA_PUBLIC_KEY) && \ diff --git a/tests/api/test_ossl_x509_pk.c b/tests/api/test_ossl_x509_pk.c index 72455ad551f..977017ef1d5 100644 --- a/tests/api/test_ossl_x509_pk.c +++ b/tests/api/test_ossl_x509_pk.c @@ -867,6 +867,60 @@ int test_wolfSSL_X509_get0_pubkey(void) return EXPECT_RESULT(); } +/* The public key of a certificate is decoded on first use for every key type, + * not only those decoded when the certificate is parsed. */ +int test_wolfSSL_X509_get_pubkey_key_types(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_FILESYSTEM) && !defined(NO_CERTS) +#if defined(HAVE_ED448) && defined(HAVE_ED448_KEY_IMPORT) && \ + defined(HAVE_ED448_KEY_EXPORT) + { + X509* x509 = NULL; + EVP_PKEY* key = NULL; + ASN1_OBJECT* obj = NULL; + const unsigned char* pk = NULL; + int pkLen = 0; + byte pub[ED448_PUB_KEY_SIZE]; + word32 pubSz = (word32)sizeof(pub); + + ExpectNotNull(x509 = X509_load_certificate_file( + "./certs/ed448/ca-ed448.pem", SSL_FILETYPE_PEM)); + ExpectNotNull(key = X509_get_pubkey(x509)); + ExpectPtrEq(X509_get0_pubkey(x509), key); + ExpectIntEQ(EVP_PKEY_id(key), EVP_PKEY_ED448); + /* The decoded key is the key in the certificate. */ + ExpectIntEQ(X509_PUBKEY_get0_param(&obj, &pk, &pkLen, NULL, + X509_get_X509_PUBKEY(x509)), 1); + ExpectIntEQ(pkLen, ED448_PUB_KEY_SIZE); + ExpectNotNull(key == NULL ? NULL : key->ed448); + ExpectIntEQ(wc_ed448_export_public(key == NULL ? NULL : key->ed448, + pub, &pubSz), 0); + ExpectIntEQ(pubSz, ED448_PUB_KEY_SIZE); + if ((pk != NULL) && (pkLen == ED448_PUB_KEY_SIZE)) { + ExpectBufEQ(pub, pk, ED448_PUB_KEY_SIZE); + } + EVP_PKEY_free(key); + X509_free(x509); + } +#endif +#if !defined(NO_RSA) && defined(WC_RSA_PSS) + { + X509* x509 = NULL; + EVP_PKEY* key = NULL; + + ExpectNotNull(x509 = X509_load_certificate_file( + "./certs/rsapss/ca-rsapss.pem", SSL_FILETYPE_PEM)); + ExpectNotNull(key = X509_get0_pubkey(x509)); + ExpectIntEQ(EVP_PKEY_id(key), EVP_PKEY_RSA); + ExpectIntEQ(EVP_PKEY_bits(key), 2048); + X509_free(x509); + } +#endif +#endif + return EXPECT_RESULT(); +} + /* A failed X509_PUBKEY_get0_param() must not leave a half built algorithm * that later calls report as success. */ int test_wolfSSL_X509_PUBKEY_get0_param_fail(void) diff --git a/tests/api/test_ossl_x509_pk.h b/tests/api/test_ossl_x509_pk.h index a095da75780..27024b83fff 100644 --- a/tests/api/test_ossl_x509_pk.h +++ b/tests/api/test_ossl_x509_pk.h @@ -32,6 +32,7 @@ int test_wolfSSL_X509_PUBKEY_MLDSA(void); int test_wolfSSL_X509_PUBKEY_get(void); int test_wolfSSL_X509_set_pubkey(void); int test_wolfSSL_X509_get0_pubkey(void); +int test_wolfSSL_X509_get_pubkey_key_types(void); int test_wolfSSL_X509_PUBKEY_get0_param_fail(void); #define TEST_OSSL_X509_PK_DECLS \ @@ -43,6 +44,7 @@ int test_wolfSSL_X509_PUBKEY_get0_param_fail(void); TEST_DECL_GROUP("ossl_x509_pk", test_wolfSSL_X509_PUBKEY_get), \ TEST_DECL_GROUP("ossl_x509_pk", test_wolfSSL_X509_set_pubkey), \ TEST_DECL_GROUP("ossl_x509_pk", test_wolfSSL_X509_get0_pubkey), \ + TEST_DECL_GROUP("ossl_x509_pk", test_wolfSSL_X509_get_pubkey_key_types), \ TEST_DECL_GROUP("ossl_x509_pk", test_wolfSSL_X509_PUBKEY_get0_param_fail) #endif /* WOLFCRYPT_TEST_OSSL_X509_PK_H */ From 941beb3b4c1678d72ea6eb579d51af8d7b9a0ec6 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 28 Oct 2026 21:32:41 +0000 Subject: [PATCH 6/9] Use the certificate's signature hash when building a name on demand GetCertX509Name() parses the name again in a temporary DecodedCert, and GetCertName() hashes the name with the hash of the signature algorithm. The temporary object had no signature algorithm, so the default hash was used. With SM2 and SM3 but without the default hash, building the name failed and loading the certificate through the X509 APIs failed. Copy the signature algorithm into the temporary object. --- wolfcrypt/src/asn.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index c31eb6b046b..bd835d2b817 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -16477,6 +16477,8 @@ int GetCertX509Name(DecodedCert* cert, int nameType, void* target) WC_ALLOC_VAR_EX(tmp, DecodedCert, 1, cert->heap, DYNAMIC_TYPE_DCERT, return MEMORY_E); InitDecodedCert(tmp, der, derSz, cert->heap); + /* The name hash uses the hash of the signature algorithm (SM3 for SM2). */ + tmp->signatureOID = cert->signatureOID; tmp->x509NameTarget = target; ret = GetName(tmp, nameType, (int)derSz); if ((ret == 0) && (target == NULL)) { From 20e777ede76c8402c951ef1fecbddc04854d336d Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 28 Oct 2026 21:32:41 +0000 Subject: [PATCH 7/9] Make the lazy X509 public key and algorithm caches thread safe With OPENSSL_ALL, the EVP_PKEY and the X509_ALGOR of a certificate were made when it was parsed. They are now made on first use, so threads reading a shared X509 could each make one and store it, replacing a key already handed out and leaking the others. Store the cached key and algorithm with an atomic compare and exchange, freeing the object made by a thread that loses, and read them with an atomic load. Set the key OID of the X509_PUBKEY when parsing so the getters never write it. --- src/internal.c | 6 ++- src/x509.c | 84 +++++++++++++++++++++++++++++------ tests/api/test_ossl_x509_pk.c | 66 +++++++++++++++++++++++++++ tests/api/test_ossl_x509_pk.h | 2 + 4 files changed, 144 insertions(+), 14 deletions(-) diff --git a/src/internal.c b/src/internal.c index a0b3b327fba..0ed278d29f6 100644 --- a/src/internal.c +++ b/src/internal.c @@ -15880,6 +15880,11 @@ static int CopyDecodedPubKey(WOLFSSL_X509* x509, DecodedCert* dCert, int ret) dCert->pubKeySize, x509->heap, DYNAMIC_TYPE_PUBLIC_KEY); if (x509->pubKey.buffer != NULL) { x509->pubKeyOID = (int)dCert->keyOID; + #ifdef OPENSSL_EXTRA_X509_SMALL + /* Set now as the getters that cache objects made from the key + * may run in parallel. */ + x509->key.pubKeyOID = (int)dCert->keyOID; + #endif x509->pubKey.length = dCert->pubKeySize; XMEMCPY(x509->pubKey.buffer, dCert->publicKey, dCert->pubKeySize); } @@ -15889,7 +15894,6 @@ static int CopyDecodedPubKey(WOLFSSL_X509* x509, DecodedCert* dCert, int ret) if (ret == 0) { /* The EVP_PKEY is decoded on first use by X509CachedPubKey() and * the algorithm object by wolfSSL_X509_PUBKEY_get0_param(). */ - x509->key.pubKeyOID = (int)dCert->keyOID; wolfSSL_X509_ALGOR_free(x509->key.algor); x509->key.algor = NULL; } diff --git a/src/x509.c b/src/x509.c index ba5817f009a..7fde980ffe1 100644 --- a/src/x509.c +++ b/src/x509.c @@ -7050,6 +7050,26 @@ static WOLFSSL_EVP_PKEY* X509DecodePubKey(WOLFSSL_X509* x509) return key; } +#ifndef SINGLE_THREADED +/* Reads a pointer that another thread may store with + * wolfSSL_Atomic_Ptr_CompareExchange(). */ +static void* X509CacheLoad(void* volatile* ptr) +{ +#if defined(__GNUC__) && defined(__ATOMIC_ACQUIRE) + return __atomic_load_n(ptr, __ATOMIC_ACQUIRE); +#else + void* current = NULL; + + /* Only replaces NULL with NULL, so nothing is changed. */ + (void)wolfSSL_Atomic_Ptr_CompareExchange(ptr, ¤t, NULL); + return current; +#endif +} +#define X509_CACHE_LOAD(ptr) X509CacheLoad((void* volatile*)&(ptr)) +#else +#define X509_CACHE_LOAD(ptr) (ptr) +#endif + /* Returns the public key cached in x509, decoding it on first use. * The key is owned by x509 and freed with it. */ static WOLFSSL_EVP_PKEY* X509CachedPubKey(WOLFSSL_X509* x509) @@ -7058,12 +7078,24 @@ static WOLFSSL_EVP_PKEY* X509CachedPubKey(WOLFSSL_X509* x509) if (x509 == NULL) return NULL; - key = x509->key.pkey; + key = (WOLFSSL_EVP_PKEY*)X509_CACHE_LOAD(x509->key.pkey); if (key == NULL) { key = X509DecodePubKey(x509); if (key != NULL) { - x509->key.pubKeyOID = x509->pubKeyOID; + #ifndef SINGLE_THREADED + void* current = NULL; + + /* Threads may decode the key of a shared X509 at the same time. + * Only the first key is cached, so a key handed out is never + * replaced. */ + if (!wolfSSL_Atomic_Ptr_CompareExchange( + (void* volatile*)&x509->key.pkey, ¤t, key)) { + wolfSSL_EVP_PKEY_free(key); + key = (WOLFSSL_EVP_PKEY*)current; + } + #else x509->key.pkey = key; + #endif } } return key; @@ -12416,6 +12448,8 @@ int wolfSSL_X509_PUBKEY_get0_param(WOLFSSL_ASN1_OBJECT **ppkalg, { const unsigned char* bitStr; int bitStrSz; + WOLFSSL_EVP_PKEY* pkey; + WOLFSSL_X509_ALGOR* algor; WOLFSSL_ENTER("wolfSSL_X509_PUBKEY_get0_param"); @@ -12423,15 +12457,23 @@ int wolfSSL_X509_PUBKEY_get0_param(WOLFSSL_ASN1_OBJECT **ppkalg, WOLFSSL_MSG("X509_PUBKEY struct not populated"); return WOLFSSL_FAILURE; } - if ((pk || ppklen) && !pub->pkey) { + /* The key and algorithm are cached and read once, as other threads may + * be storing them. */ + pkey = (WOLFSSL_EVP_PKEY*)X509_CACHE_LOAD(pub->pkey); + if ((pk || ppklen) && (pkey == NULL)) { WOLFSSL_MSG("X509_PUBKEY has no decoded key"); return WOLFSSL_FAILURE; } - if (!pub->algor) { + algor = (WOLFSSL_X509_ALGOR*)X509_CACHE_LOAD(pub->algor); + if (algor == NULL) { + #ifndef SINGLE_THREADED + void* current = NULL; + #endif + /* Build the algorithm fully before storing it, so a failure * never leaves a half built one behind. */ - WOLFSSL_X509_ALGOR* algor = wolfSSL_X509_ALGOR_new(); + algor = wolfSSL_X509_ALGOR_new(); if (algor == NULL) { return WOLFSSL_FAILURE; } @@ -12442,17 +12484,27 @@ int wolfSSL_X509_PUBKEY_get0_param(WOLFSSL_ASN1_OBJECT **ppkalg, wolfSSL_X509_ALGOR_free(algor); return WOLFSSL_FAILURE; } + #ifndef SINGLE_THREADED + /* Threads may build the algorithm of a shared X509 at the same + * time. Only the first is stored. */ + if (!wolfSSL_Atomic_Ptr_CompareExchange( + (void* volatile*)&pub->algor, ¤t, algor)) { + wolfSSL_X509_ALGOR_free(algor); + algor = (WOLFSSL_X509_ALGOR*)current; + } + #else pub->algor = algor; + #endif } if (pa) - *pa = pub->algor; + *pa = algor; if (ppkalg) - *ppkalg = pub->algor->algorithm; + *ppkalg = algor->algorithm; if ((pk != NULL) || (ppklen != NULL)) { - bitStr = (const unsigned char*)pub->pkey->pkey.ptr; - bitStrSz = pub->pkey->pkey_sz; + bitStr = (const unsigned char*)pkey->pkey.ptr; + bitStrSz = pkey->pkey_sz; /* try to parse BIT STRING from SPKI, otherwise return the * pointer as is */ @@ -12472,17 +12524,22 @@ int wolfSSL_X509_PUBKEY_get0_param(WOLFSSL_ASN1_OBJECT **ppkalg, /* Returns a pointer to the pkey when passed a key */ WOLFSSL_EVP_PKEY* wolfSSL_X509_PUBKEY_get(WOLFSSL_X509_PUBKEY* key) { + WOLFSSL_EVP_PKEY* pkey = NULL; + WOLFSSL_ENTER("wolfSSL_X509_PUBKEY_get"); - if (key == NULL || key->pkey == NULL) { + if (key != NULL) { + pkey = (WOLFSSL_EVP_PKEY*)X509_CACHE_LOAD(key->pkey); + } + if (pkey == NULL) { WOLFSSL_LEAVE("wolfSSL_X509_PUBKEY_get", BAD_FUNC_ARG); return NULL; } - if (wolfSSL_EVP_PKEY_up_ref(key->pkey) != WOLFSSL_SUCCESS) { + if (wolfSSL_EVP_PKEY_up_ref(pkey) != WOLFSSL_SUCCESS) { WOLFSSL_LEAVE("wolfSSL_X509_PUBKEY_get", BAD_MUTEX_E); return NULL; } WOLFSSL_LEAVE("wolfSSL_X509_PUBKEY_get", WOLFSSL_SUCCESS); - return key->pkey; + return pkey; } /* Returns the pkey without a new reference. */ @@ -12491,7 +12548,8 @@ WOLFSSL_EVP_PKEY* wolfSSL_X509_PUBKEY_get0(const WOLFSSL_X509_PUBKEY* key) WOLFSSL_ENTER("wolfSSL_X509_PUBKEY_get0"); if (key == NULL) return NULL; - return key->pkey; + return (WOLFSSL_EVP_PKEY*)X509_CACHE_LOAD( + ((WOLFSSL_X509_PUBKEY*)key)->pkey); } int wolfSSL_X509_PUBKEY_set(WOLFSSL_X509_PUBKEY **x, WOLFSSL_EVP_PKEY *key) diff --git a/tests/api/test_ossl_x509_pk.c b/tests/api/test_ossl_x509_pk.c index 977017ef1d5..e4b7302d7c2 100644 --- a/tests/api/test_ossl_x509_pk.c +++ b/tests/api/test_ossl_x509_pk.c @@ -921,6 +921,72 @@ int test_wolfSSL_X509_get_pubkey_key_types(void) return EXPECT_RESULT(); } +#if defined(OPENSSL_EXTRA) && !defined(NO_RSA) && !defined(NO_FILESYSTEM) && \ + !defined(NO_CERTS) && !defined(SINGLE_THREADED) +#define X509_PUBKEY_THREADS 4 + +typedef struct X509PubKeyThreadData { + X509* x509; + EVP_PKEY* key; + X509_ALGOR* algor; +} X509PubKeyThreadData; + +static THREAD_RETURN WOLFSSL_THREAD test_X509_pubkey_thread(void* arg) +{ + X509PubKeyThreadData* data = (X509PubKeyThreadData*)arg; + + data->key = X509_get0_pubkey(data->x509); + if (X509_PUBKEY_get0_param(NULL, NULL, NULL, &data->algor, + X509_get_X509_PUBKEY(data->x509)) != 1) { + data->algor = NULL; + } + WOLFSSL_RETURN_FROM_THREAD(0); +} +#endif + +/* Threads getting the public key of a shared X509 for the first time all get + * the one key and algorithm that are cached. */ +int test_wolfSSL_X509_get0_pubkey_threads(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_RSA) && !defined(NO_FILESYSTEM) && \ + !defined(NO_CERTS) && !defined(SINGLE_THREADED) + X509PubKeyThreadData data[X509_PUBKEY_THREADS]; + THREAD_TYPE threads[X509_PUBKEY_THREADS]; + X509* x509 = NULL; + int started; + int round; + int i; + + for (round = 0; (round < 16) && EXPECT_SUCCESS(); round++) { + ExpectNotNull(x509 = X509_load_certificate_file(caCertFile, + SSL_FILETYPE_PEM)); + XMEMSET(data, 0, sizeof(data)); + started = 0; + while ((started < X509_PUBKEY_THREADS) && EXPECT_SUCCESS()) { + data[started].x509 = x509; + ExpectIntEQ(wolfSSL_NewThread(&threads[started], + test_X509_pubkey_thread, &data[started]), 0); + if (EXPECT_SUCCESS()) { + started++; + } + } + for (i = 0; i < started; i++) { + ExpectIntEQ(wolfSSL_JoinThread(threads[i]), 0); + } + for (i = 0; i < started; i++) { + ExpectNotNull(data[i].key); + ExpectPtrEq(data[i].key, X509_get0_pubkey(x509)); + ExpectNotNull(data[i].algor); + ExpectPtrEq(data[i].algor, data[0].algor); + } + X509_free(x509); + x509 = NULL; + } +#endif + return EXPECT_RESULT(); +} + /* A failed X509_PUBKEY_get0_param() must not leave a half built algorithm * that later calls report as success. */ int test_wolfSSL_X509_PUBKEY_get0_param_fail(void) diff --git a/tests/api/test_ossl_x509_pk.h b/tests/api/test_ossl_x509_pk.h index 27024b83fff..c7afb032848 100644 --- a/tests/api/test_ossl_x509_pk.h +++ b/tests/api/test_ossl_x509_pk.h @@ -33,6 +33,7 @@ int test_wolfSSL_X509_PUBKEY_get(void); int test_wolfSSL_X509_set_pubkey(void); int test_wolfSSL_X509_get0_pubkey(void); int test_wolfSSL_X509_get_pubkey_key_types(void); +int test_wolfSSL_X509_get0_pubkey_threads(void); int test_wolfSSL_X509_PUBKEY_get0_param_fail(void); #define TEST_OSSL_X509_PK_DECLS \ @@ -45,6 +46,7 @@ int test_wolfSSL_X509_PUBKEY_get0_param_fail(void); TEST_DECL_GROUP("ossl_x509_pk", test_wolfSSL_X509_set_pubkey), \ TEST_DECL_GROUP("ossl_x509_pk", test_wolfSSL_X509_get0_pubkey), \ TEST_DECL_GROUP("ossl_x509_pk", test_wolfSSL_X509_get_pubkey_key_types), \ + TEST_DECL_GROUP("ossl_x509_pk", test_wolfSSL_X509_get0_pubkey_threads), \ TEST_DECL_GROUP("ossl_x509_pk", test_wolfSSL_X509_PUBKEY_get0_param_fail) #endif /* WOLFCRYPT_TEST_OSSL_X509_PK_H */ From 9c559739b3fc389e5158074039c8e9962faf42f9 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Thu, 29 Oct 2026 10:11:16 +0000 Subject: [PATCH 8/9] Use a known macro to detect GCC atomic builtins in the X509 cache load check-source-text rejects __ATOMIC_ACQUIRE in a preprocessor condition as an unrecognized macro. Test __ATOMIC_SEQ_CST instead, as wc_port.h does for the same check. --- src/x509.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/x509.c b/src/x509.c index 7fde980ffe1..91efa6ca311 100644 --- a/src/x509.c +++ b/src/x509.c @@ -7055,7 +7055,7 @@ static WOLFSSL_EVP_PKEY* X509DecodePubKey(WOLFSSL_X509* x509) * wolfSSL_Atomic_Ptr_CompareExchange(). */ static void* X509CacheLoad(void* volatile* ptr) { -#if defined(__GNUC__) && defined(__ATOMIC_ACQUIRE) +#if defined(__GNUC__) && defined(__ATOMIC_SEQ_CST) return __atomic_load_n(ptr, __ATOMIC_ACQUIRE); #else void* current = NULL; From 946e3a62322272fc7adcc564962d6923549b8238 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Thu, 29 Oct 2026 11:34:33 +0000 Subject: [PATCH 9/9] Keep the TLS peer certificate as a reference-counted heap object With KEEP_PEER_CERT, which OPENSSL_EXTRA turns on, every WOLFSSL embedded a whole WOLFSSL_X509 for the peer's certificate (3.4 KB on Cortex-M33, 4.3 KB with OPENSSL_ALL) and InitSSL() zeroed it. It was paid for when there is no peer certificate: servers that do not ask for client authentication, PSK connections, and objects before the handshake. wolfSSL_get_peer_certificate() returned wolfSSL_X509_dup() of it, which parses the certificate DER again on every call. ssl->peerCert is now a pointer, NULL until a certificate is kept, and the WOLFSSL holds one reference to it. KeepPeerCert() replaces it with a new object each time instead of reusing it, so a reference handed out stays valid across renegotiation, wolfSSL_clear() and a certificate restored from a ticket, and after wolfSSL_free(). A copy that fails part way is still kept and its error returned, as before. wolfSSL_get_peer_certificate() returns the kept object with a new reference when the X509 has a reference count (OPENSSL_EXTRA or OPENSSL_EXTRA_X509_SMALL), as OpenSSL does. Without one it still returns a copy. ReinitX509() only existed for the embedded object and is removed. --- doc/dox_comments/header_files/ssl.h | 10 ++- src/internal.c | 89 ++++++++++---------- src/ssl.c | 7 +- src/ssl_api_cert.c | 63 +++++++------- src/tls13.c | 21 +++-- tests/api/test_ssl_cert.c | 122 ++++++++++++++++++++++++++++ tests/api/test_ssl_cert.h | 2 + tests/api/test_tls13.c | 2 +- wolfssl/internal.h | 13 ++- 9 files changed, 235 insertions(+), 94 deletions(-) diff --git a/doc/dox_comments/header_files/ssl.h b/doc/dox_comments/header_files/ssl.h index 4328cbe7411..46ecf10d3a7 100644 --- a/doc/dox_comments/header_files/ssl.h +++ b/doc/dox_comments/header_files/ssl.h @@ -7492,9 +7492,12 @@ int wolfSSL_state(WOLFSSL* ssl); \brief This function gets the peer’s certificate. - \return pointer a pointer to the peerCert member of the WOLFSSL_X509 - structure if it exists. - \return 0 returned if the peer certificate issuer size is not defined. + \return pointer to the peer's WOLFSSL_X509 certificate if one was kept. + With OPENSSL_EXTRA or OPENSSL_EXTRA_X509_SMALL this is the certificate + object kept by the WOLFSSL with its reference count incremented, as + OpenSSL does; otherwise it is a copy. Either way the caller must free it + with wolfSSL_X509_free(), and it stays valid after wolfSSL_free(). + \return NULL if there is no peer certificate or memory allocation fails. \param ssl a pointer to a WOLFSSL structure, created using wolfSSL_new(). @@ -7507,6 +7510,7 @@ int wolfSSL_state(WOLFSSL* ssl); if(peerCert){ // You have a pointer peerCert to the peer certification + wolfSSL_X509_free(peerCert); } \endcode diff --git a/src/internal.c b/src/internal.c index 0ed278d29f6..f27da15a286 100644 --- a/src/internal.c +++ b/src/internal.c @@ -6072,31 +6072,38 @@ void FreeX509(WOLFSSL_X509* x509) } -/* Free an X509's contents and re-initialize it for reuse. Unlike FreeX509() - * followed by InitX509(), the object's heap and reference state carry across, - * so references held elsewhere stay counted. */ -void ReinitX509(WOLFSSL_X509* x509) +#ifdef KEEP_PEER_CERT +/* Drop the SSL's reference to the kept peer certificate. References handed + * out by wolfSSL_get_peer_certificate() keep the object alive. */ +void FreePeerCert(WOLFSSL* ssl) { -#if defined(OPENSSL_EXTRA_X509_SMALL) || defined(OPENSSL_EXTRA) - wolfSSL_Ref ref; -#endif - void* heap; - byte dynamic; - - if (x509 == NULL) - return; + if (ssl->peerCert != NULL) { + wolfSSL_X509_free(ssl->peerCert); + ssl->peerCert = NULL; + } +} - heap = x509->heap; - dynamic = x509->dynamicMemory; -#if defined(OPENSSL_EXTRA_X509_SMALL) || defined(OPENSSL_EXTRA) - ref = x509->ref; -#endif - FreeX509Contents(x509); - InitX509Fields(x509, dynamic, heap); -#if defined(OPENSSL_EXTRA_X509_SMALL) || defined(OPENSSL_EXTRA) - x509->ref = ref; -#endif +#ifndef NO_ASN +/* Replace the kept peer certificate with a new object copied from dCert. + * + * The previous certificate is released, not overwritten, so references to it + * stay valid. A copy that fails part way is still kept, as it was when the + * certificate was embedded in the WOLFSSL, and the error is returned. + * + * @return 0 on success. + * @return MEMORY_E when the object cannot be allocated; no cert is kept. + * @return Other error from CopyDecodedToX509(). + */ +int KeepPeerCert(WOLFSSL* ssl, DecodedCert* dCert) +{ + FreePeerCert(ssl); + ssl->peerCert = wolfSSL_X509_new_ex(ssl->heap); + if (ssl->peerCert == NULL) + return MEMORY_E; + return CopyDecodedToX509(ssl->peerCert, dCert); } +#endif /* !NO_ASN */ +#endif /* KEEP_PEER_CERT */ #if !defined(NO_WOLFSSL_SERVER) || (!defined(NO_WOLFSSL_CLIENT) && \ @@ -9199,10 +9206,6 @@ int InitSSL(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup) } #endif -#ifdef KEEP_PEER_CERT - InitX509(&ssl->peerCert, 0, ssl->heap); -#endif - ssl->rfd = -1; /* set to invalid descriptor */ ssl->wfd = -1; ssl->devId = ctx->devId; /* device for async HW (from wolfAsync_DevOpen) */ @@ -10551,7 +10554,7 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl) wolfSSL_X509_STORE_free(ssl->x509_store_pt); #endif #ifdef KEEP_PEER_CERT - FreeX509(&ssl->peerCert); + FreePeerCert(ssl); #endif if (ssl->session != NULL) @@ -16843,11 +16846,11 @@ int SetupStoreCtxCallback(WOLFSSL_X509_STORE_CTX** store_pt, #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) #ifdef KEEP_PEER_CERT if (args->certIdx == 0) { - FreeX509(&ssl->peerCert); - InitX509(&ssl->peerCert, 0, ssl->heap); - if (CopyDecodedToX509(&ssl->peerCert, args->dCert) != 0) + if (KeepPeerCert(ssl, args->dCert) != 0) WOLFSSL_MSG("Unable to copy to ssl->peerCert"); - store->current_cert = &ssl->peerCert; /* use existing X509 */ + if (ssl->peerCert == NULL) + goto mem_error; + store->current_cert = ssl->peerCert; /* owned by ssl */ } else #endif @@ -20040,10 +20043,8 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, if (args->fatal == 0) { int copyRet = 0; - /* free old peer cert */ - FreeX509(&ssl->peerCert); - InitX509(&ssl->peerCert, 0, ssl->heap); - copyRet = CopyDecodedToX509(&ssl->peerCert, args->dCert); + /* replace old peer cert */ + copyRet = KeepPeerCert(ssl, args->dCert); if (copyRet == WC_NO_ERR_TRACE(MEMORY_E)) { args->fatal = 1; } @@ -44551,11 +44552,11 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) const byte* certDer = NULL; word32 certDerSz = 0; - if (ssl->peerCert.derCert != NULL && - ssl->peerCert.derCert->length > 0) { + if (ssl->peerCert != NULL && ssl->peerCert->derCert != NULL && + ssl->peerCert->derCert->length > 0) { /* Use current peer certificate */ - certDer = ssl->peerCert.derCert->buffer; - certDerSz = ssl->peerCert.derCert->length; + certDer = ssl->peerCert->derCert->buffer; + certDerSz = ssl->peerCert->derCert->length; } #ifdef SESSION_CERTS else if (ssl->session->chain.count > 0) { @@ -44929,8 +44930,7 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) /* Clear any peer cert state that may have been copied from the session * cache by wolfSSL_DupSession before we got here. */ - FreeX509(&ssl->peerCert); - InitX509(&ssl->peerCert, 0, ssl->heap); + FreePeerCert(ssl); #ifdef SESSION_CERTS ssl->session->chain.count = 0; #endif @@ -44967,12 +44967,9 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) AddSessionCertToChain(&ssl->session->chain, it->peerCert, peerCertLen); #endif - FreeX509(&ssl->peerCert); - InitX509(&ssl->peerCert, 0, ssl->heap); - ret = CopyDecodedToX509(&ssl->peerCert, dCert); + ret = KeepPeerCert(ssl, dCert); if (ret != 0) { - FreeX509(&ssl->peerCert); - InitX509(&ssl->peerCert, 0, ssl->heap); + FreePeerCert(ssl); } } FreeDecodedCert(dCert); diff --git a/src/ssl.c b/src/ssl.c index d4dfe1afee7..e9723c91502 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -4017,8 +4017,8 @@ const char *wolfSSL_get0_peername(WOLFSSL *ssl) { else if (ssl->session && ssl->session->peer) return ssl->session->peer->subjectCN; #ifdef KEEP_PEER_CERT - else if (ssl->peerCert.subjectCN[0]) - return ssl->peerCert.subjectCN; + else if (ssl->peerCert != NULL && ssl->peerCert->subjectCN[0]) + return ssl->peerCert->subjectCN; #endif else { ssl->error = NO_PEER_CERT; @@ -6110,8 +6110,7 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, return WOLFSSL_FAILURE; #ifdef KEEP_PEER_CERT - FreeX509(&ssl->peerCert); - InitX509(&ssl->peerCert, 0, ssl->heap); + FreePeerCert(ssl); #endif #ifdef WOLFSSL_QUIC diff --git a/src/ssl_api_cert.c b/src/ssl_api_cert.c index 62e8f653d56..fb6dee9b1ed 100644 --- a/src/ssl_api_cert.c +++ b/src/ssl_api_cert.c @@ -2212,12 +2212,12 @@ int wolfSSL_cmp_peer_cert_to_file(WOLFSSL* ssl, const char *fname) XFILE file = XBADFILE; long sz = 0; void* heap = ssl->ctx->heap; - WOLFSSL_X509* peer_cert = &ssl->peerCert; + WOLFSSL_X509* peer_cert = ssl->peerCert; DerBuffer* fileDer = NULL; /* handle gracefully if called before handshake, or if peer did not * present a cert. */ - if (peer_cert->derCert == NULL) { + if ((peer_cert == NULL) || (peer_cert->derCert == NULL)) { WOLFSSL_MSG("wolfSSL_cmp_peer_cert_to_file without peer cert"); ret = WOLFSSL_FATAL_ERROR; } @@ -3010,14 +3010,6 @@ static int DecodeToX509(WOLFSSL_X509* x509, const byte* in, int len) InitDecodedCert(cert, (byte*)in, (word32)len, NULL); ret = ParseCertRelative(cert, CERT_TYPE, 0, NULL, NULL); if (ret == 0) { - /* Check if x509 was not previously initialized by wolfSSL_X509_new() */ - if (x509->dynamicMemory != TRUE) { - /* A non-dynamic x509 (e.g. ssl->peerCert) may already hold - * decoded contents; free them before re-populating. ReinitX509 - * keeps the object's heap and reference state, as the object - * may be referenced elsewhere. */ - ReinitX509(x509); - } ret = CopyDecodedToX509(x509, cert); } FreeDecodedCert(cert); @@ -3030,11 +3022,14 @@ static int DecodeToX509(WOLFSSL_X509* x509, const byte* in, int len) #ifdef KEEP_PEER_CERT -/* Get a copy of the peer's certificate. +/* Get the peer's certificate. * - * The certificate is decoded from the session chain when not already - * available on the object. Caller must free the returned certificate with - * wolfSSL_X509_free(). + * The certificate is decoded from the session chain, and kept, when not + * already available on the object. With X509 reference counting + * (OPENSSL_EXTRA or OPENSSL_EXTRA_X509_SMALL) the kept object is returned + * with a new reference, as OpenSSL does; otherwise a copy is returned. Either + * way the caller must free the returned certificate with wolfSSL_X509_free() + * and it stays valid after the SSL/TLS object is freed. * * @param [in, out] ssl SSL/TLS object. * @return Peer's X509 certificate on success. @@ -3049,18 +3044,30 @@ WOLFSSL_X509* wolfSSL_get_peer_certificate(WOLFSSL* ssl) WOLFSSL_ENTER("wolfSSL_get_peer_certificate"); if (ssl != NULL) { - if (ssl->peerCert.issuer.sz > 0) { - ret = wolfSSL_X509_dup(&ssl->peerCert); - } #ifdef SESSION_CERTS - else if (ssl->session->chain.count > 0) { - if (DecodeToX509(&ssl->peerCert, + if (((ssl->peerCert == NULL) || (ssl->peerCert->issuer.sz == 0)) && + (ssl->session->chain.count > 0)) { + WOLFSSL_X509* x509 = wolfSSL_X509_new_ex(ssl->heap); + if ((x509 != NULL) && (DecodeToX509(x509, ssl->session->chain.certs[0].buffer, - ssl->session->chain.certs[0].length) == 0) { - ret = wolfSSL_X509_dup(&ssl->peerCert); + ssl->session->chain.certs[0].length) == 0)) { + FreePeerCert(ssl); + ssl->peerCert = x509; + } + else { + wolfSSL_X509_free(x509); } } #endif + if ((ssl->peerCert != NULL) && (ssl->peerCert->issuer.sz > 0)) { + #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) + if (wolfSSL_X509_up_ref(ssl->peerCert) == WOLFSSL_SUCCESS) { + ret = ssl->peerCert; + } + #else + ret = wolfSSL_X509_dup(ssl->peerCert); + #endif + } } WOLFSSL_LEAVE("wolfSSL_get_peer_certificate", ret != NULL); return ret; @@ -3164,8 +3171,9 @@ static int PushPeerCertToChain(const WOLFSSL* ssl, int idx, WOLFSSL_STACK* sk, if (ssl->session->chain.certs[idx].length == 0) { #ifdef KEEP_PEER_CERT - if ((idx == 0) && (ssl->peerCert.issuer.sz > 0)) { - x509 = wolfSSL_X509_dup((WOLFSSL_X509*)&ssl->peerCert); + if ((idx == 0) && (ssl->peerCert != NULL) && + (ssl->peerCert->issuer.sz > 0)) { + x509 = wolfSSL_X509_dup(ssl->peerCert); if (x509 == NULL) { WOLFSSL_MSG("Error duplicating peer certificate"); ret = MEMORY_E; @@ -3357,12 +3365,11 @@ WOLF_STACK_OF(WOLFSSL_X509) *wolfSSL_get0_verified_chain(const WOLFSSL *ssl) err = 1; } else { - /* wolfSSL_get_peer_certificate returns a copy. We want the - * internal member so that we don't have to worry about free'ing - * it. We call wolfSSL_get_peer_certificate so that we don't have - * to worry about setting up the internal pointer. */ + /* wolfSSL_get_peer_certificate sets up ssl->peerCert and returns + * a reference to it. Drop the reference and use the object kept + * by ssl so that we don't have to worry about free'ing it. */ wolfSSL_X509_free(peerCert); - peerCert = (WOLFSSL_X509*)&ssl->peerCert; + peerCert = ssl->peerCert; } } diff --git a/src/tls13.c b/src/tls13.c index 40946c8e0e6..e2bf581389b 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -12647,9 +12647,9 @@ static int decodeRsaKey(WOLFSSL* ssl) return PEER_KEY_ERROR; ssl->peerRsaKeyPresent = 1; - keyRet = wc_RsaPublicKeyDecode(ssl->peerCert.sapkiDer, &tmpIdx, + keyRet = wc_RsaPublicKeyDecode(ssl->peerCert->sapkiDer, &tmpIdx, ssl->peerRsaKey, - ssl->peerCert.sapkiLen); + ssl->peerCert->sapkiLen); if (keyRet != 0) return PEER_KEY_ERROR; @@ -12673,9 +12673,9 @@ static int decodeEccKey(WOLFSSL* ssl) return PEER_KEY_ERROR; ssl->peerEccDsaKeyPresent = 1; - keyRet = wc_EccPublicKeyDecode(ssl->peerCert.sapkiDer, &tmpIdx, + keyRet = wc_EccPublicKeyDecode(ssl->peerCert->sapkiDer, &tmpIdx, ssl->peerEccDsaKey, - ssl->peerCert.sapkiLen); + ssl->peerCert->sapkiLen); if (keyRet != 0) return PEER_KEY_ERROR; @@ -12705,8 +12705,8 @@ static int decodeMlDsaKey(WOLFSSL* ssl, int level) return PEER_KEY_ERROR; keyRet = wc_MlDsaKey_PublicKeyDecode(ssl->peerMlDsaKey, - ssl->peerCert.sapkiDer, - ssl->peerCert.sapkiLen, &tmpIdx); + ssl->peerCert->sapkiDer, + ssl->peerCert->sapkiLen, &tmpIdx); if (keyRet != 0) return PEER_KEY_ERROR; @@ -12734,9 +12734,9 @@ static int decodeFalconKey(WOLFSSL* ssl, int level) if (keyRet != 0) return PEER_KEY_ERROR; - keyRet = wc_Falcon_PublicKeyDecode(ssl->peerCert.sapkiDer, &tmpIdx, + keyRet = wc_Falcon_PublicKeyDecode(ssl->peerCert->sapkiDer, &tmpIdx, ssl->peerFalconKey, - ssl->peerCert.sapkiLen); + ssl->peerCert->sapkiLen); if (keyRet != 0) return PEER_KEY_ERROR; @@ -12902,6 +12902,11 @@ static int DoTls13CertificateVerify(WOLFSSL* ssl, byte* input, (*ssl->sigSpec != WOLFSSL_CKS_SIGSPEC_NATIVE)) { word16 sa; + /* The alternative public key comes from the kept peer + * certificate. */ + if (ssl->peerCert == NULL) { + ERROR_OUT(PEER_KEY_ERROR, exit_dcv); + } if (args->altSigAlgo == 0) sa = ssl->options.peerSigAlgo; else diff --git a/tests/api/test_ssl_cert.c b/tests/api/test_ssl_cert.c index 68dcedca73a..19ddd85be0d 100644 --- a/tests/api/test_ssl_cert.c +++ b/tests/api/test_ssl_cert.c @@ -474,6 +474,128 @@ int test_wolfSSL_cmp_peer_cert_to_file(void) return EXPECT_RESULT(); } +/* Test that getting the peer's certificate returns the object kept by the + * SSL/TLS object with a new reference rather than a copy, that a server + * without client authentication keeps no certificate, and that a reference + * stays valid when the kept certificate is replaced and after the SSL/TLS + * object is freed. + * + * @return TEST_SUCCESS on success. + */ +int test_wolfSSL_get_peer_certificate_ref(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(OPENSSL_EXTRA) && \ + defined(KEEP_PEER_CERT) && !defined(WOLFSSL_NO_TLS12) && \ + !defined(NO_RSA) && !defined(NO_TLS) + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + struct test_memio_ctx test_ctx; + WOLFSSL_X509* srv = NULL; + WOLFSSL_X509* peer = NULL; + WOLFSSL_X509* again = NULL; + WOLFSSL_X509* old = NULL; + const unsigned char* srvDer = NULL; + const unsigned char* der = NULL; + int srvDerSz = 0; + int derSz = 0; + + ExpectNull(wolfSSL_get_peer_certificate(NULL)); + ExpectNotNull(srv = wolfSSL_X509_load_certificate_file(svrCertFile, + WOLFSSL_FILETYPE_PEM)); + ExpectNotNull(srvDer = wolfSSL_X509_get_der(srv, &srvDerSz)); + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + + /* Nothing is kept before the handshake. */ + if (EXPECT_SUCCESS()) { + ExpectNull(ssl_c->peerCert); + } + ExpectNull(wolfSSL_get_peer_certificate(ssl_c)); + + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + /* The server did not ask for a client certificate, so keeps none. */ + if (EXPECT_SUCCESS()) { + ExpectNull(ssl_s->peerCert); + } + ExpectNull(wolfSSL_get_peer_certificate(ssl_s)); + + /* The client is handed the kept object with a reference. */ + ExpectNotNull(peer = wolfSSL_get_peer_certificate(ssl_c)); + if (EXPECT_SUCCESS()) { + ExpectPtrEq(peer, ssl_c->peerCert); + ExpectIntEQ((int)wolfSSL_RefCur(peer->ref), 2); + } + ExpectNotNull(again = wolfSSL_get_peer_certificate(ssl_c)); + ExpectPtrEq(again, peer); + if (EXPECT_SUCCESS()) { + ExpectIntEQ((int)wolfSSL_RefCur(peer->ref), 3); + } + wolfSSL_X509_free(again); + again = NULL; + if (EXPECT_SUCCESS()) { + ExpectIntEQ((int)wolfSSL_RefCur(peer->ref), 2); + } + /* It is the certificate the server sent. */ + ExpectNotNull(der = wolfSSL_X509_get_der(peer, &derSz)); + ExpectIntEQ(derSz, srvDerSz); + ExpectBufEQ(der, srvDer, srvDerSz); + + /* Reusing the object for a new connection releases the kept certificate + * instead of overwriting it, so the reference held stays valid. */ + old = peer; + peer = NULL; + ExpectIntEQ(wolfSSL_clear(ssl_c), WOLFSSL_SUCCESS); + if (EXPECT_SUCCESS()) { + ExpectNull(ssl_c->peerCert); + ExpectIntEQ((int)wolfSSL_RefCur(old->ref), 1); + } + wolfSSL_free(ssl_s); + ssl_s = NULL; + ExpectNotNull(ssl_s = wolfSSL_new(ctx_s)); + wolfSSL_SetIOWriteCtx(ssl_s, &test_ctx); + wolfSSL_SetIOReadCtx(ssl_s, &test_ctx); +#ifndef NO_DH + SetDH(ssl_s); +#endif + test_memio_clear_buffer(&test_ctx, 0); + test_memio_clear_buffer(&test_ctx, 1); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + ExpectNotNull(peer = wolfSSL_get_peer_certificate(ssl_c)); + ExpectPtrNE(peer, old); + ExpectNotNull(der = wolfSSL_X509_get_der(old, &derSz)); + ExpectIntEQ(derSz, srvDerSz); + ExpectBufEQ(der, srvDer, srvDerSz); + wolfSSL_X509_free(old); + old = NULL; + + /* The reference outlives the SSL/TLS object. */ + wolfSSL_free(ssl_c); + ssl_c = NULL; + if (EXPECT_SUCCESS()) { + ExpectIntEQ((int)wolfSSL_RefCur(peer->ref), 1); + } + ExpectNotNull(der = wolfSSL_X509_get_der(peer, &derSz)); + ExpectIntEQ(derSz, srvDerSz); + ExpectBufEQ(der, srvDer, srvDerSz); + ExpectIntEQ(wolfSSL_X509_NAME_cmp(wolfSSL_X509_get_subject_name(peer), + wolfSSL_X509_get_subject_name(srv)), 0); + + wolfSSL_X509_free(old); + wolfSSL_X509_free(peer); + wolfSSL_X509_free(srv); + wolfSSL_free(ssl_s); + wolfSSL_free(ssl_c); + wolfSSL_CTX_free(ctx_s); + wolfSSL_CTX_free(ctx_c); +#endif + return EXPECT_RESULT(); +} + /* Guarded to match its only caller, test_wolfSSL_CTX_set_client_cert_cb(), * which needs OPENSSL_EXTRA for the ctx->CBClientCert field. */ #if defined(WOLFSSL_CERT_SETUP_CB) && defined(OPENSSL_EXTRA) && \ diff --git a/tests/api/test_ssl_cert.h b/tests/api/test_ssl_cert.h index bb6978ea123..5aae614a925 100644 --- a/tests/api/test_ssl_cert.h +++ b/tests/api/test_ssl_cert.h @@ -41,6 +41,7 @@ int test_wolfSSL_get_peer_chain(void); int test_wolfSSL_get_chain_X509(void); int test_wolfSSL_get_chain_cert_pem(void); int test_wolfSSL_cmp_peer_cert_to_file(void); +int test_wolfSSL_get_peer_certificate_ref(void); int test_wolfSSL_CTX_set_client_cert_cb(void); int test_wolfSSL_CTX_set_cert_cb(void); int test_wolfSSL_cert_setup_cb_ret(void); @@ -85,6 +86,7 @@ int test_wolfSSL_SESSION_get0_peer_full_chain(void); TEST_DECL_GROUP("ssl_cert", test_wolfSSL_get_chain_X509), \ TEST_DECL_GROUP("ssl_cert", test_wolfSSL_get_chain_cert_pem), \ TEST_DECL_GROUP("ssl_cert", test_wolfSSL_cmp_peer_cert_to_file), \ + TEST_DECL_GROUP("ssl_cert", test_wolfSSL_get_peer_certificate_ref), \ TEST_DECL_GROUP("ssl_cert", test_wolfSSL_CTX_set_client_cert_cb), \ TEST_DECL_GROUP("ssl_cert", test_wolfSSL_CTX_set_cert_cb), \ TEST_DECL_GROUP("ssl_cert", test_wolfSSL_cert_setup_cb_ret), \ diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index 18730cb07f5..ebe82ec999f 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -12706,7 +12706,7 @@ int test_tls13_ticket_peer_cert_reverify(void) * no longer in the trust store. Check the peerCert directly rather * than wolfSSL_get_peer_certificate which has a session-chain * fallback that may see stale cache state. */ - ExpectIntEQ(ssl_s->peerCert.issuer.sz, 0); + ExpectNull(ssl_s->peerCert); wolfSSL_SESSION_free(sess); wolfSSL_free(ssl_c); diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 423fa27b074..092b53ddfcd 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -7212,9 +7212,9 @@ struct WOLFSSL { WOLFSSL_X509_STORE* x509_store_pt; /* take ownership of external store */ #endif #ifdef KEEP_PEER_CERT - /* TODO put this on the heap so we can properly use the - * reference counter and not have to duplicate it. */ - WOLFSSL_X509 peerCert; /* X509 peer cert */ + WOLFSSL_X509* peerCert; /* X509 peer cert, NULL when none. + Holds one reference; replaced, + never reused, by a new cert */ #endif #ifdef KEEP_OUR_CERT WOLFSSL_X509* ourCert; /* keep alive a X509 struct of cert. @@ -8067,11 +8067,16 @@ WOLFSSL_LOCAL enum wc_HashType HashAlgoToType(int hashAlgo); WOLFSSL_LOCAL void InitX509(WOLFSSL_X509* x509, int dynamicFlag, void* heap); WOLFSSL_LOCAL void FreeX509(WOLFSSL_X509* x509); - WOLFSSL_LOCAL void ReinitX509(WOLFSSL_X509* x509); #ifndef NO_ASN WOLFSSL_LOCAL int CopyDecodedToX509(WOLFSSL_X509* x509, DecodedCert* dCert); #endif + #ifdef KEEP_PEER_CERT + WOLFSSL_LOCAL void FreePeerCert(WOLFSSL* ssl); + #ifndef NO_ASN + WOLFSSL_LOCAL int KeepPeerCert(WOLFSSL* ssl, DecodedCert* dCert); + #endif + #endif #endif #if defined(WOLFSSL_ACERT)