From 0260d0ea39d9c7d9f469a18a07f0a90ad6628cdc Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Thu, 29 Oct 2026 11:34:33 +0000 Subject: [PATCH 1/2] Allocate the TLS peer certificate on the heap with WOLFSSL_DYNAMIC_PEER_CERT With KEEP_PEER_CERT, which OPENSSL_EXTRA turns on, every WOLFSSL embeds a whole WOLFSSL_X509 for the peer's certificate (3.4 KB on Cortex-M33, 4.3 KB with OPENSSL_ALL) and InitSSL() zeroes it. It is paid for when there is no peer certificate: servers that do not ask for client authentication, PSK connections, and objects before the handshake. WOLFSSL_DYNAMIC_PEER_CERT makes ssl->peerCert a pointer instead, NULL until a certificate is kept, with the WOLFSSL holding one reference. SetPeerCert() 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(). With X509 reference counting (OPENSSL_EXTRA or OPENSSL_EXTRA_X509_SMALL), wolfSSL_get_peer_certificate() then returns the kept object with a new reference, as OpenSSL does, instead of a copy that parses the certificate DER again. WOLFSSL_DYNAMIC_PEER_CERT is on by default when KEEP_PEER_CERT and WOLFSSL_SMALL_STACK are defined, as large objects then already go on the heap. Define NO_WOLFSSL_DYNAMIC_PEER_CERT to keep the embedded object, or WOLFSSL_DYNAMIC_PEER_CERT to turn it on without WOLFSSL_SMALL_STACK. It is always off with WOLFSSL_NO_MALLOC, where the certificate could not be allocated and so would never be kept. With the flag off the certificate stays embedded and behaves as before. GetPeerCert() returns the kept object in either mode, and FreePeerCert() and SetPeerCert() release and replace it. The leaf is copied, and with the flag on allocated, once per handshake (peerCertSet). wolfSSL_get0_verified_chain() uses the kept object directly instead of taking a reference or copy and dropping it. --- .wolfssl_known_macro_extras | 2 + doc/dox_comments/header_files/ssl.h | 11 +- src/internal.c | 86 +++++++++++---- src/ssl.c | 7 +- src/ssl_api_cert.c | 100 +++++++++++++----- src/tls13.c | 21 ++-- tests/api/test_ssl_cert.c | 157 ++++++++++++++++++++++++++++ tests/api/test_ssl_cert.h | 2 + tests/api/test_tls13.c | 4 + wolfssl/internal.h | 24 ++++- wolfssl/wolfcrypt/settings.h | 14 +++ 11 files changed, 361 insertions(+), 67 deletions(-) diff --git a/.wolfssl_known_macro_extras b/.wolfssl_known_macro_extras index 88b63baf6a0..29554d91021 100644 --- a/.wolfssl_known_macro_extras +++ b/.wolfssl_known_macro_extras @@ -585,6 +585,7 @@ NO_WOLFSSL_AUTOSAR_CSM NO_WOLFSSL_BASE64_DECODE NO_WOLFSSL_BN_CTX NO_WOLFSSL_CURVE25519_BLINDING +NO_WOLFSSL_DYNAMIC_PEER_CERT NO_WOLFSSL_MSG_EX NO_WOLFSSL_RENESAS_FSPSM_AES NO_WOLFSSL_RENESAS_FSPSM_HASH @@ -980,6 +981,7 @@ WOLFSSL_DTLS_DISALLOW_FUTURE WOLFSSL_DTLS_RESEND_ONLY_TIMEOUT WOLFSSL_DUMP_MEMIO_STREAM WOLFSSL_DUP_CERTPOL +WOLFSSL_DYNAMIC_PEER_CERT WOLFSSL_EARLY_DATA_NO_ANTI_REPLAY WOLFSSL_ECC_GEN_REJECT_SAMPLING WOLFSSL_ECC_NO_SMALL_STACK diff --git a/doc/dox_comments/header_files/ssl.h b/doc/dox_comments/header_files/ssl.h index 5884a417171..67f76e82f4b 100644 --- a/doc/dox_comments/header_files/ssl.h +++ b/doc/dox_comments/header_files/ssl.h @@ -7492,9 +7492,13 @@ 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 WOLFSSL_DYNAMIC_PEER_CERT (on by default with WOLFSSL_SMALL_STACK) + and 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 +7511,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 81f89bd4c90..1c5196874d2 100644 --- a/src/internal.c +++ b/src/internal.c @@ -6168,6 +6168,52 @@ void ReinitX509(WOLFSSL_X509* x509) } +#ifdef KEEP_PEER_CERT +/* Release the kept peer certificate so that none is kept. + * + * With WOLFSSL_DYNAMIC_PEER_CERT this drops the SSL's reference; references + * handed out by wolfSSL_get_peer_certificate() keep the object alive. + * Otherwise the embedded object is emptied for reuse. + */ +void FreePeerCert(WOLFSSL* ssl) +{ +#ifdef WOLFSSL_DYNAMIC_PEER_CERT + if (ssl->peerCert != NULL) { + wolfSSL_X509_free(ssl->peerCert); + ssl->peerCert = NULL; + } +#else + FreeX509(&ssl->peerCert); + InitX509(&ssl->peerCert, 0, ssl->heap); +#endif +} + +#ifndef NO_ASN +/* Make the certificate in dCert the kept peer certificate. + * + * With WOLFSSL_DYNAMIC_PEER_CERT the previous certificate is released, not + * overwritten, and a new object allocated, so references to the previous one + * stay valid. Otherwise the embedded object is emptied and filled again. A + * copy that fails part way is still kept 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 SetPeerCert(WOLFSSL* ssl, DecodedCert* dCert) +{ + FreePeerCert(ssl); +#ifdef WOLFSSL_DYNAMIC_PEER_CERT + ssl->peerCert = wolfSSL_X509_new_ex(ssl->heap); + if (ssl->peerCert == NULL) + return MEMORY_E; +#endif + return CopyDecodedToX509(GetPeerCert(ssl), dCert); +} +#endif /* !NO_ASN */ +#endif /* KEEP_PEER_CERT */ + + #if !defined(NO_WOLFSSL_SERVER) || (!defined(NO_WOLFSSL_CLIENT) && \ !defined(WOLFSSL_NO_CLIENT_AUTH)) #if !defined(WOLFSSL_NO_TLS12) @@ -9321,7 +9367,7 @@ int InitSSL(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup) } #endif -#ifdef KEEP_PEER_CERT +#if defined(KEEP_PEER_CERT) && !defined(WOLFSSL_DYNAMIC_PEER_CERT) InitX509(&ssl->peerCert, 0, ssl->heap); #endif @@ -10736,7 +10782,9 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl) if (ssl->x509_store_pt) wolfSSL_X509_STORE_free(ssl->x509_store_pt); #endif -#ifdef KEEP_PEER_CERT +#ifdef WOLFSSL_DYNAMIC_PEER_CERT + FreePeerCert(ssl); +#elif defined(KEEP_PEER_CERT) FreeX509(&ssl->peerCert); #endif @@ -17201,14 +17249,15 @@ int SetupStoreCtxCallback(WOLFSSL_X509_STORE_CTX** store_pt, if (args->certIdx == 0) { /* Copy the leaf once, here or in ProcessPeerCerts(). */ if (!args->peerCertSet) { - FreeX509(&ssl->peerCert); - InitX509(&ssl->peerCert, 0, ssl->heap); - if (CopyDecodedToX509(&ssl->peerCert, args->dCert) == 0) + if (SetPeerCert(ssl, args->dCert) == 0) args->peerCertSet = 1; else WOLFSSL_MSG("Unable to copy to ssl->peerCert"); } - store->current_cert = &ssl->peerCert; /* use existing X509 */ + /* use the kept X509, owned by ssl */ + store->current_cert = GetPeerCert(ssl); + if (store->current_cert == NULL) + goto mem_error; } else #endif @@ -20653,10 +20702,8 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, if (args->fatal == 0 && !args->peerCertSet) { 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 = SetPeerCert(ssl, args->dCert); if (copyRet == 0) { args->peerCertSet = 1; } @@ -45422,12 +45469,13 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) if (!ssl->options.dtls) { const byte* certDer = NULL; word32 certDerSz = 0; + WOLFSSL_X509* peer = GetPeerCert(ssl); - if (ssl->peerCert.derCert != NULL && - ssl->peerCert.derCert->length > 0) { + if (peer != NULL && peer->derCert != NULL && + peer->derCert->length > 0) { /* Use current peer certificate */ - certDer = ssl->peerCert.derCert->buffer; - certDerSz = ssl->peerCert.derCert->length; + certDer = peer->derCert->buffer; + certDerSz = peer->derCert->length; } #ifdef SESSION_CERTS else if (ssl->session->chain.count > 0) { @@ -45925,8 +45973,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 @@ -45964,12 +46011,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 = SetPeerCert(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 401825e8d1a..a1aed624e11 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -4022,8 +4022,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 (GetPeerCert(ssl) != NULL && GetPeerCert(ssl)->subjectCN[0]) + return GetPeerCert(ssl)->subjectCN; #endif else { ssl->error = NO_PEER_CERT; @@ -6141,8 +6141,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 79a62abbfeb..15781db1732 100644 --- a/src/ssl_api_cert.c +++ b/src/ssl_api_cert.c @@ -2227,12 +2227,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 = GetPeerCert(ssl); 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; } @@ -3053,11 +3053,59 @@ 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 kept peer certificate, owned by the SSL/TLS object. * - * 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. + * + * @param [in, out] ssl SSL/TLS object. + * @return Kept peer certificate on success. + * @return NULL when no peer certificate is available or it cannot be + * decoded. + */ +static WOLFSSL_X509* LoadPeerCert(WOLFSSL* ssl) +{ + WOLFSSL_X509* peer = GetPeerCert(ssl); + + #ifdef SESSION_CERTS + if (((peer == NULL) || (peer->issuer.sz == 0)) && + (ssl->session->chain.count > 0)) { + #ifdef WOLFSSL_DYNAMIC_PEER_CERT + 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)) { + FreePeerCert(ssl); + ssl->peerCert = x509; + peer = x509; + } + else { + wolfSSL_X509_free(x509); + peer = NULL; + } + #else + if (DecodeToX509(peer, ssl->session->chain.certs[0].buffer, + ssl->session->chain.certs[0].length) != 0) { + peer = NULL; + } + #endif + } + #endif + if ((peer != NULL) && (peer->issuer.sz == 0)) { + peer = NULL; + } + + return peer; +} + +/* Get the peer's certificate. + * + * The certificate is decoded from the session chain, and kept, when not + * already available on the object. With WOLFSSL_DYNAMIC_PEER_CERT and 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. @@ -3068,22 +3116,22 @@ WOLFSSL_ABI WOLFSSL_X509* wolfSSL_get_peer_certificate(WOLFSSL* ssl) { WOLFSSL_X509* ret = NULL; + WOLFSSL_X509* peer = NULL; 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, - ssl->session->chain.certs[0].buffer, - ssl->session->chain.certs[0].length) == 0) { - ret = wolfSSL_X509_dup(&ssl->peerCert); - } + peer = LoadPeerCert(ssl); + } + if (peer != NULL) { + #if defined(WOLFSSL_DYNAMIC_PEER_CERT) && \ + (defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL)) + if (wolfSSL_X509_up_ref(peer) == WOLFSSL_SUCCESS) { + ret = peer; } - #endif + #else + ret = wolfSSL_X509_dup(peer); + #endif } WOLFSSL_LEAVE("wolfSSL_get_peer_certificate", ret != NULL); return ret; @@ -3187,8 +3235,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) && (GetPeerCert(ssl) != NULL) && + (GetPeerCert(ssl)->issuer.sz > 0)) { + x509 = wolfSSL_X509_dup(GetPeerCert(ssl)); if (x509 == NULL) { WOLFSSL_MSG("Error duplicating peer certificate"); ret = MEMORY_E; @@ -3374,19 +3423,12 @@ WOLF_STACK_OF(WOLFSSL_X509) *wolfSSL_get0_verified_chain(const WOLFSSL *ssl) } if (!err) { - peerCert = wolfSSL_get_peer_certificate((WOLFSSL*)ssl); + /* Owned by ssl, so not freed here. */ + peerCert = LoadPeerCert((WOLFSSL*)ssl); if (peerCert == NULL) { - WOLFSSL_MSG("wolfSSL_get_peer_certificate error"); + WOLFSSL_MSG("No peer certificate"); 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_X509_free(peerCert); - peerCert = (WOLFSSL_X509*)&ssl->peerCert; - } } if (!err) { diff --git a/src/tls13.c b/src/tls13.c index 1a4e34e73c6..68154208bcf 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -12692,9 +12692,9 @@ static int decodeRsaKey(WOLFSSL* ssl) return PEER_KEY_ERROR; ssl->peerRsaKeyPresent = 1; - keyRet = wc_RsaPublicKeyDecode(ssl->peerCert.sapkiDer, &tmpIdx, + keyRet = wc_RsaPublicKeyDecode(GetPeerCert(ssl)->sapkiDer, &tmpIdx, ssl->peerRsaKey, - ssl->peerCert.sapkiLen); + GetPeerCert(ssl)->sapkiLen); if (keyRet != 0) return PEER_KEY_ERROR; @@ -12718,9 +12718,9 @@ static int decodeEccKey(WOLFSSL* ssl) return PEER_KEY_ERROR; ssl->peerEccDsaKeyPresent = 1; - keyRet = wc_EccPublicKeyDecode(ssl->peerCert.sapkiDer, &tmpIdx, + keyRet = wc_EccPublicKeyDecode(GetPeerCert(ssl)->sapkiDer, &tmpIdx, ssl->peerEccDsaKey, - ssl->peerCert.sapkiLen); + GetPeerCert(ssl)->sapkiLen); if (keyRet != 0) return PEER_KEY_ERROR; @@ -12750,8 +12750,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); + GetPeerCert(ssl)->sapkiDer, + GetPeerCert(ssl)->sapkiLen, &tmpIdx); if (keyRet != 0) return PEER_KEY_ERROR; @@ -12779,9 +12779,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(GetPeerCert(ssl)->sapkiDer, &tmpIdx, ssl->peerFalconKey, - ssl->peerCert.sapkiLen); + GetPeerCert(ssl)->sapkiLen); if (keyRet != 0) return PEER_KEY_ERROR; @@ -12947,6 +12947,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 (GetPeerCert(ssl) == 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 628c49707e1..308937f0959 100644 --- a/tests/api/test_ssl_cert.c +++ b/tests/api/test_ssl_cert.c @@ -586,6 +586,163 @@ int test_wolfSSL_peer_cert_copy_oom(void) return EXPECT_RESULT(); } +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(OPENSSL_EXTRA) && \ + defined(KEEP_PEER_CERT) && !defined(WOLFSSL_NO_TLS12) && \ + !defined(NO_RSA) && !defined(NO_TLS) +/* Whether the SSL/TLS object keeps a peer certificate. + * + * @param [in] ssl SSL/TLS object. + * @return 1 when a peer certificate is kept, 0 otherwise. + */ +static int test_peer_cert_kept(const WOLFSSL* ssl) +{ + WOLFSSL_X509* peer = GetPeerCert(ssl); + return (peer != NULL) && (peer->issuer.sz > 0); +} +#endif + +/* Test getting the peer's certificate. + * + * With WOLFSSL_DYNAMIC_PEER_CERT the object kept by the SSL/TLS object is + * returned with a new reference; otherwise a copy is returned. Either way a + * server without client authentication keeps no certificate, and the + * returned certificate 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()) { + #ifdef WOLFSSL_DYNAMIC_PEER_CERT + ExpectNull(ssl_c->peerCert); + #endif + ExpectIntEQ(test_peer_cert_kept(ssl_c), 0); + } + 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()) { + #ifdef WOLFSSL_DYNAMIC_PEER_CERT + ExpectNull(ssl_s->peerCert); + #endif + ExpectIntEQ(test_peer_cert_kept(ssl_s), 0); + } + ExpectNull(wolfSSL_get_peer_certificate(ssl_s)); + + ExpectNotNull(peer = wolfSSL_get_peer_certificate(ssl_c)); + ExpectNotNull(again = wolfSSL_get_peer_certificate(ssl_c)); +#ifdef WOLFSSL_DYNAMIC_PEER_CERT + /* The client is handed the kept object with a reference. */ + ExpectPtrEq(again, peer); + if (EXPECT_SUCCESS()) { + ExpectPtrEq(peer, ssl_c->peerCert); + ExpectIntEQ((int)wolfSSL_RefCur(peer->ref), 3); + } + wolfSSL_X509_free(again); + again = NULL; + if (EXPECT_SUCCESS()) { + ExpectIntEQ((int)wolfSSL_RefCur(peer->ref), 2); + } +#else + /* The client is handed a copy each time. */ + ExpectPtrNE(again, peer); + if (EXPECT_SUCCESS()) { + ExpectPtrNE(peer, GetPeerCert(ssl_c)); + ExpectIntEQ((int)wolfSSL_RefCur(peer->ref), 1); + } + wolfSSL_X509_free(again); + again = NULL; +#endif + /* 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 empties the kept certificate, + * and the certificate returned stays valid. */ + old = peer; + peer = NULL; + ExpectIntEQ(wolfSSL_clear(ssl_c), WOLFSSL_SUCCESS); + if (EXPECT_SUCCESS()) { + #ifdef WOLFSSL_DYNAMIC_PEER_CERT + ExpectNull(ssl_c->peerCert); + #endif + ExpectIntEQ(test_peer_cert_kept(ssl_c), 0); + 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 certificate returned 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(again); + 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 60e67d6f89d..1f802590194 100644 --- a/tests/api/test_ssl_cert.h +++ b/tests/api/test_ssl_cert.h @@ -42,6 +42,7 @@ 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_peer_cert_copy_oom(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); @@ -90,6 +91,7 @@ int test_wolfSSL_SESSION_get0_peer_full_chain(void); 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_peer_cert_copy_oom), \ + 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 4101dfa9b3d..74982e5655e 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -12943,7 +12943,11 @@ 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. */ +#ifdef WOLFSSL_DYNAMIC_PEER_CERT + ExpectNull(ssl_s->peerCert); +#else ExpectIntEQ(ssl_s->peerCert.issuer.sz, 0); +#endif wolfSSL_SESSION_free(sess); wolfSSL_free(ssl_c); diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 3974ba87cbc..623df4f7569 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -7318,10 +7318,14 @@ 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. */ +#ifdef WOLFSSL_DYNAMIC_PEER_CERT + WOLFSSL_X509* peerCert; /* X509 peer cert, NULL when none. + Holds one reference; replaced, + never reused, by a new cert */ +#else WOLFSSL_X509 peerCert; /* X509 peer cert */ #endif +#endif #ifdef KEEP_OUR_CERT WOLFSSL_X509* ourCert; /* keep alive a X509 struct of cert. points to ctx if not owned (owned @@ -8213,6 +8217,22 @@ WOLFSSL_LOCAL enum wc_HashType HashAlgoToType(int hashAlgo); WOLFSSL_LOCAL int CopyDecodedToX509(WOLFSSL_X509* x509, DecodedCert* dCert); #endif + #ifdef KEEP_PEER_CERT + /* The kept peer certificate object. NULL only with + * WOLFSSL_DYNAMIC_PEER_CERT, when no certificate is kept. */ + static WC_INLINE WOLFSSL_X509* GetPeerCert(const WOLFSSL* ssl) + { + #ifdef WOLFSSL_DYNAMIC_PEER_CERT + return ssl->peerCert; + #else + return (WOLFSSL_X509*)&ssl->peerCert; + #endif + } + WOLFSSL_LOCAL void FreePeerCert(WOLFSSL* ssl); + #ifndef NO_ASN + WOLFSSL_LOCAL int SetPeerCert(WOLFSSL* ssl, DecodedCert* dCert); + #endif + #endif #endif #if defined(WOLFSSL_ACERT) diff --git a/wolfssl/wolfcrypt/settings.h b/wolfssl/wolfcrypt/settings.h index bd38b9d1aeb..6564f4aa52a 100644 --- a/wolfssl/wolfcrypt/settings.h +++ b/wolfssl/wolfcrypt/settings.h @@ -5221,6 +5221,20 @@ blinding by defining WC_BLINDING_NO_RNG_ACKNOWLEDGE_WEAKNESS." #define KEEP_PEER_CERT #endif +/* Allocate the kept peer certificate when it arrives instead of embedding a + * WOLFSSL_X509 in every WOLFSSL. On by default when large objects already go + * on the heap; define NO_WOLFSSL_DYNAMIC_PEER_CERT to keep it embedded. Off + * without a heap, as the certificate could then never be kept. */ +#if defined(KEEP_PEER_CERT) && defined(WOLFSSL_SMALL_STACK) && \ + !defined(WOLFSSL_NO_MALLOC) && !defined(NO_WOLFSSL_DYNAMIC_PEER_CERT) + #undef WOLFSSL_DYNAMIC_PEER_CERT + #define WOLFSSL_DYNAMIC_PEER_CERT +#endif +#if defined(WOLFSSL_DYNAMIC_PEER_CERT) && \ + (!defined(KEEP_PEER_CERT) || defined(WOLFSSL_NO_MALLOC)) + #undef WOLFSSL_DYNAMIC_PEER_CERT +#endif + /* Always copy certificate(s) from SSL CTX to each SSL object on creation, * if this is not defined then each SSL object shares a pointer to the * original certificate buffer owned by the SSL CTX. */ From 85ef82665cc38befeac3834d746c64b0e09f0300 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 4 Nov 2026 15:16:04 +0000 Subject: [PATCH 2/2] Clean up X509 ex_data only when the last reference is freed ExternalFreeX509() ran the HAVE_EX_DATA_CLEANUP_HOOKS clean-up on every wolfSSL_X509_free(), before the reference count was dropped, so freeing one reference cleared the ex_data, and ran its clean-up routine, for every other holder. With WOLFSSL_DYNAMIC_PEER_CERT this is reachable through the peer certificate: freeing one returned reference, wolfSSL_clear() or wolfSSL_free() cleared the ex_data of a certificate still in use. Run the clean-up with the final free, as WOLFSSL_RSA, WOLFSSL_BIO and WOLFSSL_X509_STORE do. The peer certificate test now checks that ex_data survives an extra reference being freed and wolfSSL_clear(), and is cleaned up once at the last free. It also loads the server certificate with CERT_FILETYPE, to match the certificate files test_memio_setup() selects. --- src/x509.c | 8 +++++--- tests/api/test_ssl_cert.c | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/x509.c b/src/x509.c index 49bab817678..39ea4847773 100644 --- a/src/x509.c +++ b/src/x509.c @@ -4268,9 +4268,6 @@ static void ExternalFreeX509(WOLFSSL_X509* x509) WOLFSSL_ENTER("ExternalFreeX509"); if (x509) { -#ifdef HAVE_EX_DATA_CLEANUP_HOOKS - wolfSSL_CRYPTO_cleanup_ex_data(&x509->ex_data); -#endif if (x509->dynamicMemory) { #if defined(OPENSSL_EXTRA_X509_SMALL) || defined(OPENSSL_EXTRA) int ret; @@ -4281,6 +4278,11 @@ static void ExternalFreeX509(WOLFSSL_X509* x509) if (doFree) #endif /* OPENSSL_EXTRA_X509_SMALL || OPENSSL_EXTRA */ { + #ifdef HAVE_EX_DATA_CLEANUP_HOOKS + /* Only with the last reference, as other holders may still + * use the ex_data. */ + wolfSSL_CRYPTO_cleanup_ex_data(&x509->ex_data); + #endif FreeX509(x509); XFREE(x509, x509->heap, DYNAMIC_TYPE_X509); } diff --git a/tests/api/test_ssl_cert.c b/tests/api/test_ssl_cert.c index 308937f0959..64c29841f44 100644 --- a/tests/api/test_ssl_cert.c +++ b/tests/api/test_ssl_cert.c @@ -599,6 +599,17 @@ static int test_peer_cert_kept(const WOLFSSL* ssl) WOLFSSL_X509* peer = GetPeerCert(ssl); return (peer != NULL) && (peer->issuer.sz > 0); } + +#if defined(HAVE_EX_DATA) && defined(HAVE_EX_DATA_CLEANUP_HOOKS) +/* Count the clean-ups of a certificate's ex_data. + * + * @param [in, out] data Count to increment. + */ +static void test_peer_cert_ex_data_cleanup(void* data) +{ + (*(int*)data)++; +} +#endif #endif /* Test getting the peer's certificate. @@ -628,10 +639,13 @@ int test_wolfSSL_get_peer_certificate_ref(void) const unsigned char* der = NULL; int srvDerSz = 0; int derSz = 0; +#if defined(HAVE_EX_DATA) && defined(HAVE_EX_DATA_CLEANUP_HOOKS) + int exFrees = 0; +#endif ExpectNull(wolfSSL_get_peer_certificate(NULL)); ExpectNotNull(srv = wolfSSL_X509_load_certificate_file(svrCertFile, - WOLFSSL_FILETYPE_PEM)); + CERT_FILETYPE)); ExpectNotNull(srvDer = wolfSSL_X509_get_der(srv, &srvDerSz)); XMEMSET(&test_ctx, 0, sizeof(test_ctx)); @@ -687,6 +701,16 @@ int test_wolfSSL_get_peer_certificate_ref(void) ExpectIntEQ(derSz, srvDerSz); ExpectBufEQ(der, srvDer, srvDerSz); +#if defined(HAVE_EX_DATA) && defined(HAVE_EX_DATA_CLEANUP_HOOKS) + /* ex_data is cleaned up only when the last reference is freed. */ + ExpectIntEQ(wolfSSL_X509_set_ex_data_with_cleanup(peer, 0, &exFrees, + test_peer_cert_ex_data_cleanup), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_X509_up_ref(peer), WOLFSSL_SUCCESS); + wolfSSL_X509_free(peer); + ExpectIntEQ(exFrees, 0); + ExpectPtrEq(wolfSSL_X509_get_ex_data(peer, 0), &exFrees); +#endif + /* Reusing the object for a new connection empties the kept certificate, * and the certificate returned stays valid. */ old = peer; @@ -699,6 +723,10 @@ int test_wolfSSL_get_peer_certificate_ref(void) ExpectIntEQ(test_peer_cert_kept(ssl_c), 0); ExpectIntEQ((int)wolfSSL_RefCur(old->ref), 1); } +#if defined(HAVE_EX_DATA) && defined(HAVE_EX_DATA_CLEANUP_HOOKS) + ExpectIntEQ(exFrees, 0); + ExpectPtrEq(wolfSSL_X509_get_ex_data(old, 0), &exFrees); +#endif wolfSSL_free(ssl_s); ssl_s = NULL; ExpectNotNull(ssl_s = wolfSSL_new(ctx_s)); @@ -718,6 +746,9 @@ int test_wolfSSL_get_peer_certificate_ref(void) ExpectBufEQ(der, srvDer, srvDerSz); wolfSSL_X509_free(old); old = NULL; +#if defined(HAVE_EX_DATA) && defined(HAVE_EX_DATA_CLEANUP_HOOKS) + ExpectIntEQ(exFrees, 1); +#endif /* The certificate returned outlives the SSL/TLS object. */ wolfSSL_free(ssl_c);