From 5efa2807593e5621a63dcd4f79e1bf536e59e796 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Tue, 16 Apr 2024 13:05:23 +0200 Subject: [PATCH] tls session storage - add session with destructor callback - remove vtls `session_free` method - let `Curl_ssl_addsessionid()` take ownership of session object, freeing it also on failures - change tls backend use --- lib/urldata.h | 3 ++ lib/vquic/curl_ngtcp2.c | 5 ++-- lib/vtls/bearssl.c | 24 +++++++-------- lib/vtls/gtls.c | 33 ++++++++++----------- lib/vtls/mbedtls.c | 26 +++++++--------- lib/vtls/openssl.c | 44 +++++++++++---------------- lib/vtls/openssl.h | 2 +- lib/vtls/rustls.c | 1 - lib/vtls/schannel.c | 55 ++++++++++++++++------------------ lib/vtls/sectransp.c | 31 ++++++++++--------- lib/vtls/vtls.c | 66 +++++++++++++++++++++++------------------ lib/vtls/vtls_int.h | 5 ++-- lib/vtls/wolfssl.c | 49 +++++++++++------------------- 13 files changed, 157 insertions(+), 187 deletions(-) diff --git a/lib/urldata.h b/lib/urldata.h index 8bccffb0f59b20..aa3cc44474ad5e 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -345,6 +345,8 @@ struct ssl_general_config { int ca_cache_timeout; /* Certificate store cache timeout (seconds) */ }; +typedef void Curl_ssl_sessionid_dtor(void *sessionid, size_t idsize); + /* information stored about one single SSL session */ struct Curl_ssl_session { char *name; /* host name for which this ID was used */ @@ -352,6 +354,7 @@ struct Curl_ssl_session { const char *scheme; /* protocol scheme used */ void *sessionid; /* as returned from the SSL layer */ size_t idsize; /* if known, otherwise 0 */ + Curl_ssl_sessionid_dtor *sessionid_free; /* free `sessionid` callback */ long age; /* just a number, the higher the more recent */ int remote_port; /* remote port */ int conn_to_port; /* remote port for the connection (may be -1) */ diff --git a/lib/vquic/curl_ngtcp2.c b/lib/vquic/curl_ngtcp2.c index 74006b47277cfa..5597cef9e83464 100644 --- a/lib/vquic/curl_ngtcp2.c +++ b/lib/vquic/curl_ngtcp2.c @@ -1928,9 +1928,8 @@ static int quic_ossl_new_session_cb(SSL *ssl, SSL_SESSION *ssl_sessionid) ctx = cf? cf->ctx : NULL; data = cf? CF_DATA_CURRENT(cf) : NULL; if(cf && data && ctx) { - CURLcode result = Curl_ossl_add_session(cf, data, &ctx->peer, - ssl_sessionid); - return result? 0 : 1; + Curl_ossl_add_session(cf, data, &ctx->peer, ssl_sessionid); + return 1; } return 0; } diff --git a/lib/vtls/bearssl.c b/lib/vtls/bearssl.c index e9e2ce86cc654a..1793194f4b4eca 100644 --- a/lib/vtls/bearssl.c +++ b/lib/vtls/bearssl.c @@ -873,6 +873,12 @@ static CURLcode bearssl_connect_step2(struct Curl_cfilter *cf, return ret; } +static void bearssl_session_free(void *sessionid, size_t idsize) +{ + (void)idsize; + free(sessionid); +} + static CURLcode bearssl_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) { @@ -896,7 +902,6 @@ static CURLcode bearssl_connect_step3(struct Curl_cfilter *cf, if(ssl_config->primary.sessionid) { bool incache; - bool added = FALSE; void *oldsession; br_ssl_session_parameters *session; @@ -909,13 +914,12 @@ static CURLcode bearssl_connect_step3(struct Curl_cfilter *cf, &oldsession, NULL)); if(incache) Curl_ssl_delsessionid(data, oldsession); - ret = Curl_ssl_addsessionid(cf, data, &connssl->peer, session, 0, &added); + + ret = Curl_ssl_addsessionid(cf, data, &connssl->peer, session, 0, + bearssl_session_free); Curl_ssl_sessionid_unlock(data); - if(!added) - free(session); - if(ret) { - return CURLE_OUT_OF_MEMORY; - } + if(ret) + return ret; } connssl->connecting_state = ssl_connect_done; @@ -1174,11 +1178,6 @@ static void bearssl_close(struct Curl_cfilter *cf, struct Curl_easy *data) } } -static void bearssl_session_free(void *ptr) -{ - free(ptr); -} - static CURLcode bearssl_sha256sum(const unsigned char *input, size_t inputlen, unsigned char *sha256sum, @@ -1211,7 +1210,6 @@ const struct Curl_ssl Curl_ssl_bearssl = { bearssl_get_internals, /* get_internals */ bearssl_close, /* close_one */ Curl_none_close_all, /* close_all */ - bearssl_session_free, /* session_free */ Curl_none_set_engine, /* set_engine */ Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ diff --git a/lib/vtls/gtls.c b/lib/vtls/gtls.c index a0d1fccfb891b0..aa4751744fd786 100644 --- a/lib/vtls/gtls.c +++ b/lib/vtls/gtls.c @@ -1371,6 +1371,12 @@ Curl_gtls_verifyserver(struct Curl_easy *data, return result; } +static void gtls_sessionid_free(void *sessionid, size_t idsize) +{ + (void)idsize; + free(sessionid); +} + static CURLcode gtls_verifyserver(struct Curl_cfilter *cf, struct Curl_easy *data, gnutls_session_t session) @@ -1414,10 +1420,12 @@ static CURLcode gtls_verifyserver(struct Curl_cfilter *cf, /* get the session ID data size */ gnutls_session_get_data(session, NULL, &connect_idsize); connect_sessionid = malloc(connect_idsize); /* get a buffer for it */ - - if(connect_sessionid) { + if(!connect_sessionid) { + result = CURLE_OUT_OF_MEMORY; + goto out; + } + else { bool incache; - bool added = FALSE; void *ssl_sessionid; /* extract session ID to the allocated buffer */ @@ -1432,19 +1440,14 @@ static CURLcode gtls_verifyserver(struct Curl_cfilter *cf, Curl_ssl_delsessionid(data, ssl_sessionid); } - /* store this session id */ + /* store this session id, takes ownership */ result = Curl_ssl_addsessionid(cf, data, &connssl->peer, connect_sessionid, connect_idsize, - &added); + gtls_sessionid_free); Curl_ssl_sessionid_unlock(data); - if(!added) - free(connect_sessionid); - if(result) { - result = CURLE_OUT_OF_MEMORY; - } + if(result) + return result; } - else - result = CURLE_OUT_OF_MEMORY; } out: @@ -1734,11 +1737,6 @@ static ssize_t gtls_recv(struct Curl_cfilter *cf, return ret; } -static void gtls_session_free(void *ptr) -{ - free(ptr); -} - static size_t gtls_version(char *buffer, size_t size) { return msnprintf(buffer, size, "GnuTLS/%s", gnutls_check_version(NULL)); @@ -1805,7 +1803,6 @@ const struct Curl_ssl Curl_ssl_gnutls = { gtls_get_internals, /* get_internals */ gtls_close, /* close_one */ Curl_none_close_all, /* close_all */ - gtls_session_free, /* session_free */ Curl_none_set_engine, /* set_engine */ Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ diff --git a/lib/vtls/mbedtls.c b/lib/vtls/mbedtls.c index a70a6b6fe0c7d8..507229a9a3adc1 100644 --- a/lib/vtls/mbedtls.c +++ b/lib/vtls/mbedtls.c @@ -923,6 +923,13 @@ mbed_connect_step2(struct Curl_cfilter *cf, struct Curl_easy *data) return CURLE_OK; } +static void mbedtls_session_free(void *sessionid, size_t idsize) +{ + (void)idsize; + mbedtls_ssl_session_free(sessionid); + free(sessionid); +} + static CURLcode mbed_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) { @@ -939,7 +946,6 @@ mbed_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) int ret; mbedtls_ssl_session *our_ssl_sessionid; void *old_ssl_sessionid = NULL; - bool added = FALSE; our_ssl_sessionid = malloc(sizeof(mbedtls_ssl_session)); if(!our_ssl_sessionid) @@ -963,16 +969,11 @@ mbed_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) Curl_ssl_delsessionid(data, old_ssl_sessionid); retcode = Curl_ssl_addsessionid(cf, data, &connssl->peer, - our_ssl_sessionid, 0, &added); + our_ssl_sessionid, 0, + mbedtls_session_free); Curl_ssl_sessionid_unlock(data); - if(!added) { - mbedtls_ssl_session_free(our_ssl_sessionid); - free(our_ssl_sessionid); - } - if(retcode) { - failf(data, "failed to store ssl session"); + if(retcode) return retcode; - } } connssl->connecting_state = ssl_connect_done; @@ -1065,12 +1066,6 @@ static ssize_t mbed_recv(struct Curl_cfilter *cf, struct Curl_easy *data, return len; } -static void mbedtls_session_free(void *ptr) -{ - mbedtls_ssl_session_free(ptr); - free(ptr); -} - static size_t mbedtls_version(char *buffer, size_t size) { #ifdef MBEDTLS_VERSION_C @@ -1349,7 +1344,6 @@ const struct Curl_ssl Curl_ssl_mbedtls = { mbedtls_get_internals, /* get_internals */ mbedtls_close, /* close_one */ mbedtls_close_all, /* close_all */ - mbedtls_session_free, /* session_free */ Curl_none_set_engine, /* set_engine */ Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c index 7839ab4ab60d29..8484bc24a36bca 100644 --- a/lib/vtls/openssl.c +++ b/lib/vtls/openssl.c @@ -2074,10 +2074,11 @@ static int ossl_shutdown(struct Curl_cfilter *cf, return retval; } -static void ossl_session_free(void *ptr) +static void ossl_session_free(void *sessionid, size_t idsize) { /* free the ID */ - SSL_SESSION_free(ptr); + (void)idsize; + SSL_SESSION_free(sessionid); } /* @@ -2939,17 +2940,16 @@ CURLcode Curl_ossl_add_session(struct Curl_cfilter *cf, { const struct ssl_config_data *config; bool isproxy; - CURLcode result = CURLE_WRITE_ERROR; + bool added = FALSE; if(!cf || !data) - return result; + goto out; isproxy = Curl_ssl_cf_is_proxy(cf); config = Curl_ssl_cf_get_config(cf, data); if(config->primary.sessionid) { bool incache; - bool added = FALSE; void *old_ssl_sessionid = NULL; Curl_ssl_sessionid_lock(data); @@ -2958,29 +2958,23 @@ CURLcode Curl_ossl_add_session(struct Curl_cfilter *cf, else incache = !(Curl_ssl_getsessionid(cf, data, peer, &old_ssl_sessionid, NULL)); - if(incache) { - if(old_ssl_sessionid != ssl_sessionid) { - infof(data, "old SSL session ID is stale, removing"); - Curl_ssl_delsessionid(data, old_ssl_sessionid); - incache = FALSE; - } + if(incache && (old_ssl_sessionid != ssl_sessionid)) { + infof(data, "old SSL session ID is stale, removing"); + Curl_ssl_delsessionid(data, old_ssl_sessionid); + incache = FALSE; } - if(!incache) { - if(!Curl_ssl_addsessionid(cf, data, peer, ssl_sessionid, - 0 /* unknown size */, &added)) { - if(added) { - /* the session has been put into the session cache */ - result = CURLE_OK; - } - } - else - failf(data, "failed to store ssl session"); + if(!incache && !Curl_ssl_addsessionid(cf, data, peer, ssl_sessionid, 0, + ossl_session_free)) { + added = TRUE; } Curl_ssl_sessionid_unlock(data); } - return result; +out: + if(!added) + ossl_session_free(ssl_sessionid, 0); + return CURLE_OK; } /* The "new session" callback must return zero if the session can be removed @@ -2991,13 +2985,12 @@ static int ossl_new_session_cb(SSL *ssl, SSL_SESSION *ssl_sessionid) struct Curl_cfilter *cf; struct Curl_easy *data; struct ssl_connect_data *connssl; - CURLcode result; cf = (struct Curl_cfilter*) SSL_get_app_data(ssl); connssl = cf? cf->ctx : NULL; data = connssl? CF_DATA_CURRENT(cf) : NULL; - result = Curl_ossl_add_session(cf, data, &connssl->peer, ssl_sessionid); - return result? 0 : 1; + Curl_ossl_add_session(cf, data, &connssl->peer, ssl_sessionid); + return 1; } static CURLcode load_cacert_from_memory(X509_STORE *store, @@ -5288,7 +5281,6 @@ const struct Curl_ssl Curl_ssl_openssl = { ossl_get_internals, /* get_internals */ ossl_close, /* close_one */ ossl_close_all, /* close_all */ - ossl_session_free, /* session_free */ ossl_set_engine, /* set_engine */ ossl_set_engine_default, /* set_engine_default */ ossl_engines_list, /* engines_list */ diff --git a/lib/vtls/openssl.h b/lib/vtls/openssl.h index 47bb282a9b4d5a..55e06bda440699 100644 --- a/lib/vtls/openssl.h +++ b/lib/vtls/openssl.h @@ -100,7 +100,7 @@ CURLcode Curl_ossl_ctx_configure(struct Curl_cfilter *cf, SSL_CTX *ssl_ctx); /* - * Add a new session to the cache. + * Add a new session to the cache. Takes ownership of the session. */ CURLcode Curl_ossl_add_session(struct Curl_cfilter *cf, struct Curl_easy *data, diff --git a/lib/vtls/rustls.c b/lib/vtls/rustls.c index cb25c5234ccf63..b9ff783c29b04d 100644 --- a/lib/vtls/rustls.c +++ b/lib/vtls/rustls.c @@ -743,7 +743,6 @@ const struct Curl_ssl Curl_ssl_rustls = { cr_get_internals, /* get_internals */ cr_close, /* close_one */ Curl_none_close_all, /* close_all */ - Curl_none_session_free, /* session_free */ Curl_none_set_engine, /* set_engine */ Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ diff --git a/lib/vtls/schannel.c b/lib/vtls/schannel.c index cec7fa9710278c..412acd947f1ba2 100644 --- a/lib/vtls/schannel.c +++ b/lib/vtls/schannel.c @@ -1675,6 +1675,28 @@ add_cert_to_certinfo(const CERT_CONTEXT *ccert_context, bool reverse_order, return args->result == CURLE_OK; } +static void schannel_session_free(void *sessionid, size_t idsize) +{ + /* this is expected to be called under sessionid lock */ + struct Curl_schannel_cred *cred = sessionid; + + (void)idsize + if(cred) { + cred->refcount--; + if(cred->refcount == 0) { + s_pSecFn->FreeCredentialsHandle(&cred->cred_handle); + curlx_unicodefree(cred->sni_hostname); +#ifdef HAS_CLIENT_CERT_PATH + if(cred->client_cert_store) { + CertCloseStore(cred->client_cert_store, 0); + cred->client_cert_store = NULL; + } +#endif + Curl_safefree(cred); + } + } +} + static CURLcode schannel_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) { @@ -1752,7 +1774,6 @@ schannel_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) /* save the current session data for possible reuse */ if(ssl_config->primary.sessionid) { bool incache; - bool added = FALSE; struct Curl_schannel_cred *old_cred = NULL; Curl_ssl_sessionid_lock(data); @@ -1768,20 +1789,15 @@ schannel_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) } } if(!incache) { + /* Up ref count since call takes ownership */ + backend->cred->refcount++; result = Curl_ssl_addsessionid(cf, data, &connssl->peer, backend->cred, sizeof(struct Curl_schannel_cred), - &added); + schannel_session_free); if(result) { Curl_ssl_sessionid_unlock(data); - failf(data, "schannel: failed to store credential handle"); return result; } - else if(added) { - /* this cred session is now also referenced by sessionid cache */ - backend->cred->refcount++; - DEBUGF(infof(data, - "schannel: stored credential handle in session cache")); - } } Curl_ssl_sessionid_unlock(data); } @@ -2456,27 +2472,6 @@ static bool schannel_data_pending(struct Curl_cfilter *cf, return FALSE; } -static void schannel_session_free(void *ptr) -{ - /* this is expected to be called under sessionid lock */ - struct Curl_schannel_cred *cred = ptr; - - if(cred) { - cred->refcount--; - if(cred->refcount == 0) { - s_pSecFn->FreeCredentialsHandle(&cred->cred_handle); - curlx_unicodefree(cred->sni_hostname); -#ifdef HAS_CLIENT_CERT_PATH - if(cred->client_cert_store) { - CertCloseStore(cred->client_cert_store, 0); - cred->client_cert_store = NULL; - } -#endif - Curl_safefree(cred); - } - } -} - /* shut down the SSL connection and clean up related memory. this function can be called multiple times on the same connection including if the SSL connection failed (eg connection made but failed handshake). */ diff --git a/lib/vtls/sectransp.c b/lib/vtls/sectransp.c index b7fc7c02224cae..8af3751bb5ad4b 100644 --- a/lib/vtls/sectransp.c +++ b/lib/vtls/sectransp.c @@ -1636,6 +1636,18 @@ static CURLcode sectransp_set_selected_ciphers(struct Curl_easy *data, return CURLE_OK; } +static void sectransp_session_free(void *sessionid, size_t idsize) +{ + /* ST, as of iOS 5 and Mountain Lion, has no public method of deleting a + cached session ID inside the Security framework. There is a private + function that does this, but I don't want to have to explain to you why I + got your application rejected from the App Store due to the use of a + private API, so the best we can do is free up our own char array that we + created way back in sectransp_connect_step1... */ + (void)idsize; + Curl_safefree(sessionid); +} + static CURLcode sectransp_connect_step1(struct Curl_cfilter *cf, struct Curl_easy *data) { @@ -2078,12 +2090,11 @@ static CURLcode sectransp_connect_step1(struct Curl_cfilter *cf, } result = Curl_ssl_addsessionid(cf, data, &connssl->peer, ssl_sessionid, - ssl_sessionid_len, NULL); + ssl_sessionid_len, + sectransp_session_free); Curl_ssl_sessionid_unlock(data); - if(result) { - failf(data, "failed to store ssl session"); + if(result) return result; - } } } @@ -3225,17 +3236,6 @@ static int sectransp_shutdown(struct Curl_cfilter *cf, return rc; } -static void sectransp_session_free(void *ptr) -{ - /* ST, as of iOS 5 and Mountain Lion, has no public method of deleting a - cached session ID inside the Security framework. There is a private - function that does this, but I don't want to have to explain to you why I - got your application rejected from the App Store due to the use of a - private API, so the best we can do is free up our own char array that we - created way back in sectransp_connect_step1... */ - Curl_safefree(ptr); -} - static size_t sectransp_version(char *buffer, size_t size) { return msnprintf(buffer, size, "SecureTransport"); @@ -3469,7 +3469,6 @@ const struct Curl_ssl Curl_ssl_sectransp = { sectransp_get_internals, /* get_internals */ sectransp_close, /* close_one */ Curl_none_close_all, /* close_all */ - sectransp_session_free, /* session_free */ Curl_none_set_engine, /* set_engine */ Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c index c14b398274416a..af7ebe65c5b990 100644 --- a/lib/vtls/vtls.c +++ b/lib/vtls/vtls.c @@ -605,9 +605,10 @@ void Curl_ssl_kill_session(struct Curl_ssl_session *session) /* defensive check */ /* free the ID the SSL-layer specific way */ - Curl_ssl->session_free(session->sessionid); + session->sessionid_free(session->sessionid, session->idsize); session->sessionid = NULL; + session->sessionid_free = NULL; session->age = 0; /* fresh */ Curl_free_primary_ssl_config(&session->ssl_config); @@ -645,42 +646,41 @@ CURLcode Curl_ssl_addsessionid(struct Curl_cfilter *cf, const struct ssl_peer *peer, void *ssl_sessionid, size_t idsize, - bool *added) + Curl_ssl_sessionid_dtor *sessionid_free_cb) { struct ssl_config_data *ssl_config = Curl_ssl_cf_get_config(cf, data); struct ssl_primary_config *conn_config = Curl_ssl_cf_get_primary_config(cf); size_t i; struct Curl_ssl_session *store; long oldest_age; - char *clone_host; - char *clone_conn_to_host; + char *clone_host = NULL; + char *clone_conn_to_host = NULL; int conn_to_port; long *general_age; + CURLcode result = CURLE_OUT_OF_MEMORY; - if(added) - *added = FALSE; + DEBUGASSERT(ssl_sessionid); + DEBUGASSERT(sessionid_free_cb); - if(!data->state.session) + if(!data->state.session) { + sessionid_free_cb(ssl_sessionid, idsize); return CURLE_OK; + } store = &data->state.session[0]; oldest_age = data->state.session[0].age; /* zero if unused */ - (void)ssl_config; DEBUGASSERT(ssl_config->primary.sessionid); + (void)ssl_config; clone_host = strdup(peer->hostname); if(!clone_host) - return CURLE_OUT_OF_MEMORY; /* bail out */ + goto out; if(cf->conn->bits.conn_to_host) { clone_conn_to_host = strdup(cf->conn->conn_to_host.name); - if(!clone_conn_to_host) { - free(clone_host); - return CURLE_OUT_OF_MEMORY; /* bail out */ - } + if(!clone_conn_to_host) + goto out; } - else - clone_conn_to_host = NULL; if(cf->conn->bits.conn_to_port) conn_to_port = cf->conn->conn_to_port; @@ -713,34 +713,43 @@ CURLcode Curl_ssl_addsessionid(struct Curl_cfilter *cf, store = &data->state.session[i]; /* use this slot */ /* now init the session struct wisely */ + if(!clone_ssl_primary_config(conn_config, &store->ssl_config)) { + Curl_free_primary_ssl_config(&store->ssl_config); + store->sessionid = NULL; /* let caller free sessionid */ + goto out; + } store->sessionid = ssl_sessionid; store->idsize = idsize; + store->sessionid_free = sessionid_free_cb; store->age = *general_age; /* set current age */ /* free it if there's one already present */ free(store->name); free(store->conn_to_host); store->name = clone_host; /* clone host name */ + clone_host = NULL; store->conn_to_host = clone_conn_to_host; /* clone connect to host name */ + clone_conn_to_host = NULL; store->conn_to_port = conn_to_port; /* connect to port number */ /* port number */ store->remote_port = peer->port; store->scheme = cf->conn->handler->scheme; store->transport = peer->transport; - if(!clone_ssl_primary_config(conn_config, &store->ssl_config)) { - Curl_free_primary_ssl_config(&store->ssl_config); - store->sessionid = NULL; /* let caller free sessionid */ - free(clone_host); - free(clone_conn_to_host); - return CURLE_OUT_OF_MEMORY; - } - - if(added) - *added = TRUE; + result = CURLE_OK; - DEBUGF(infof(data, "Added Session ID to cache for %s://%s:%d [%s]", - store->scheme, store->name, store->remote_port, - Curl_ssl_cf_is_proxy(cf) ? "PROXY" : "server")); +out: + free(clone_host); + free(clone_conn_to_host); + if(result) { + failf(data, "Failed to add Session ID to cache for %s://%s:%d [%s]", + store->scheme, store->name, store->remote_port, + Curl_ssl_cf_is_proxy(cf) ? "PROXY" : "server"); + sessionid_free_cb(ssl_sessionid, idsize); + return result; + } + CURL_TRC_CF(data, cf, "Added Session ID to cache for %s://%s:%d [%s]", + store->scheme, store->name, store->remote_port, + Curl_ssl_cf_is_proxy(cf) ? "PROXY" : "server"); return CURLE_OK; } @@ -1323,7 +1332,6 @@ static const struct Curl_ssl Curl_ssl_multi = { multissl_get_internals, /* get_internals */ multissl_close, /* close_one */ Curl_none_close_all, /* close_all */ - Curl_none_session_free, /* session_free */ Curl_none_set_engine, /* set_engine */ Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ diff --git a/lib/vtls/vtls_int.h b/lib/vtls/vtls_int.h index 8aca1d33311648..5259babb251a5b 100644 --- a/lib/vtls/vtls_int.h +++ b/lib/vtls/vtls_int.h @@ -123,7 +123,6 @@ struct Curl_ssl { void *(*get_internals)(struct ssl_connect_data *connssl, CURLINFO info); void (*close)(struct Curl_cfilter *cf, struct Curl_easy *data); void (*close_all)(struct Curl_easy *data); - void (*session_free)(void *ptr); CURLcode (*set_engine)(struct Curl_easy *data, const char *engine); CURLcode (*set_engine_default)(struct Curl_easy *data); @@ -186,13 +185,15 @@ bool Curl_ssl_getsessionid(struct Curl_cfilter *cf, * Sessionid mutex must be locked (see Curl_ssl_sessionid_lock). * Caller must ensure that it has properly shared ownership of this sessionid * object with cache (e.g. incrementing refcount on success) + * Call takes ownership of `ssl_sessionid`, using `sessionid_free_cb` + * to destroy it in case of failure or later removal. */ CURLcode Curl_ssl_addsessionid(struct Curl_cfilter *cf, struct Curl_easy *data, const struct ssl_peer *peer, void *ssl_sessionid, size_t idsize, - bool *added); + Curl_ssl_sessionid_dtor *sessionid_free_cb); #include "openssl.h" /* OpenSSL versions */ #include "gtls.h" /* GnuTLS versions */ diff --git a/lib/vtls/wolfssl.c b/lib/vtls/wolfssl.c index 82593f301b0bae..d7305fb7f24295 100644 --- a/lib/vtls/wolfssl.c +++ b/lib/vtls/wolfssl.c @@ -1058,6 +1058,13 @@ wolfssl_connect_step2(struct Curl_cfilter *cf, struct Curl_easy *data) } +static void wolfssl_session_free(void *sessionid, size_t idsize) +{ + (void)idsize; + wolfSSL_SESSION_free(sessionid); +} + + static CURLcode wolfssl_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) { @@ -1071,42 +1078,27 @@ wolfssl_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) DEBUGASSERT(backend); if(ssl_config->primary.sessionid) { - bool incache; - bool added = FALSE; - void *old_ssl_sessionid = NULL; /* wolfSSL_get1_session allocates memory that has to be freed. */ WOLFSSL_SESSION *our_ssl_sessionid = wolfSSL_get1_session(backend->handle); if(our_ssl_sessionid) { + void *old_ssl_sessionid = NULL; + bool incache; Curl_ssl_sessionid_lock(data); incache = !(Curl_ssl_getsessionid(cf, data, &connssl->peer, &old_ssl_sessionid, NULL)); if(incache) { - if(old_ssl_sessionid != our_ssl_sessionid) { - infof(data, "old SSL session ID is stale, removing"); - Curl_ssl_delsessionid(data, old_ssl_sessionid); - incache = FALSE; - } + Curl_ssl_delsessionid(data, old_ssl_sessionid); } - if(!incache) { - result = Curl_ssl_addsessionid(cf, data, &connssl->peer, - our_ssl_sessionid, 0, NULL); - if(result) { - Curl_ssl_sessionid_unlock(data); - wolfSSL_SESSION_free(our_ssl_sessionid); - failf(data, "failed to store ssl session"); - return result; - } - else { - added = TRUE; - } - } + /* call takes ownership of `our_ssl_sessionid` */ + result = Curl_ssl_addsessionid(cf, data, &connssl->peer, + our_ssl_sessionid, 0, + wolfssl_session_free); Curl_ssl_sessionid_unlock(data); - - if(!added) { - /* If the session info wasn't added to the cache, free our copy. */ - wolfSSL_SESSION_free(our_ssl_sessionid); + if(result) { + failf(data, "failed to store ssl session"); + return result; } } } @@ -1240,12 +1232,6 @@ static ssize_t wolfssl_recv(struct Curl_cfilter *cf, } -static void wolfssl_session_free(void *ptr) -{ - wolfSSL_SESSION_free(ptr); -} - - static size_t wolfssl_version(char *buffer, size_t size) { #if LIBWOLFSSL_VERSION_HEX >= 0x03006000 @@ -1522,7 +1508,6 @@ const struct Curl_ssl Curl_ssl_wolfssl = { wolfssl_get_internals, /* get_internals */ wolfssl_close, /* close_one */ Curl_none_close_all, /* close_all */ - wolfssl_session_free, /* session_free */ Curl_none_set_engine, /* set_engine */ Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */