From: Stefan Eissing Date: Mon, 8 Jul 2024 10:11:30 +0000 (+0200) Subject: vtls: replace addsessionid with set_sessionid X-Git-Tag: curl-8_9_0~82 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=46a26f122a74e24aeb144af2da47a7f1ba4cf821;p=thirdparty%2Fcurl.git vtls: replace addsessionid with set_sessionid - deduplicate the code in many tls backends that check for an existing id and delete it before adding the new one - rename ssl_primary_config's `sessionid` bool to `cache_session` Closes #14121 --- diff --git a/lib/setopt.c b/lib/setopt.c index 904516822a..7d35651d10 100644 --- a/lib/setopt.c +++ b/lib/setopt.c @@ -2537,9 +2537,10 @@ CURLcode Curl_vsetopt(struct Curl_easy *data, CURLoption option, va_list param) break; case CURLOPT_SSL_SESSIONID_CACHE: - data->set.ssl.primary.sessionid = (0 != va_arg(param, long)); + data->set.ssl.primary.cache_session = (0 != va_arg(param, long)); #ifndef CURL_DISABLE_PROXY - data->set.proxy_ssl.primary.sessionid = data->set.ssl.primary.sessionid; + data->set.proxy_ssl.primary.cache_session = + data->set.ssl.primary.cache_session; #endif break; diff --git a/lib/urldata.h b/lib/urldata.h index d38623b070..74b744c715 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -298,7 +298,7 @@ struct ssl_primary_config { BIT(verifypeer); /* set TRUE if this is desired */ BIT(verifyhost); /* set TRUE if CN/SAN must match hostname */ BIT(verifystatus); /* set TRUE if certificate status must be checked */ - BIT(sessionid); /* cache session IDs or not */ + BIT(cache_session); /* cache session or not */ }; struct ssl_config_data { diff --git a/lib/vtls/bearssl.c b/lib/vtls/bearssl.c index 04de54f006..47c5528c29 100644 --- a/lib/vtls/bearssl.c +++ b/lib/vtls/bearssl.c @@ -584,7 +584,7 @@ static CURLcode bearssl_connect_step1(struct Curl_cfilter *cf, backend->x509.verifyhost = verifyhost; br_ssl_engine_set_x509(&backend->ctx.eng, &backend->x509.vtable); - if(ssl_config->primary.sessionid) { + if(ssl_config->primary.cache_session) { void *session; CURL_TRC_CF(data, cf, "connect_step1, check session cache"); @@ -818,9 +818,7 @@ static CURLcode bearssl_connect_step3(struct Curl_cfilter *cf, proto? strlen(proto) : 0); } - if(ssl_config->primary.sessionid) { - bool incache; - void *oldsession; + if(ssl_config->primary.cache_session) { br_ssl_session_parameters *session; session = malloc(sizeof(*session)); @@ -828,13 +826,8 @@ static CURLcode bearssl_connect_step3(struct Curl_cfilter *cf, return CURLE_OUT_OF_MEMORY; br_ssl_engine_get_session_parameters(&backend->ctx.eng, session); Curl_ssl_sessionid_lock(data); - incache = !(Curl_ssl_getsessionid(cf, data, &connssl->peer, - &oldsession, NULL)); - if(incache) - Curl_ssl_delsessionid(data, oldsession); - - ret = Curl_ssl_addsessionid(cf, data, &connssl->peer, session, 0, - bearssl_session_free); + ret = Curl_ssl_set_sessionid(cf, data, &connssl->peer, session, 0, + bearssl_session_free); Curl_ssl_sessionid_unlock(data); if(ret) return ret; diff --git a/lib/vtls/gtls.c b/lib/vtls/gtls.c index f391707611..dc0f77ba57 100644 --- a/lib/vtls/gtls.c +++ b/lib/vtls/gtls.c @@ -727,7 +727,7 @@ static CURLcode gtls_update_session_id(struct Curl_cfilter *cf, struct ssl_connect_data *connssl = cf->ctx; CURLcode result = CURLE_OK; - if(ssl_config->primary.sessionid) { + if(ssl_config->primary.cache_session) { /* we always unconditionally get the session id here, as even if we already got it from the cache and asked to use it in the connection, it might've been rejected and then a new one is in use now and we need to @@ -742,27 +742,16 @@ static CURLcode gtls_update_session_id(struct Curl_cfilter *cf, return CURLE_OUT_OF_MEMORY; } else { - bool incache; - void *ssl_sessionid; - /* extract session ID to the allocated buffer */ gnutls_session_get_data(session, connect_sessionid, &connect_idsize); CURL_TRC_CF(data, cf, "get session id (len=%zu) and store in cache", connect_idsize); Curl_ssl_sessionid_lock(data); - incache = !(Curl_ssl_getsessionid(cf, data, &connssl->peer, - &ssl_sessionid, NULL)); - if(incache) { - /* there was one before in the cache, so instead of risking that the - previous one was rejected, we just kill that and store the new */ - Curl_ssl_delsessionid(data, ssl_sessionid); - } - /* store this session id, takes ownership */ - result = Curl_ssl_addsessionid(cf, data, &connssl->peer, - connect_sessionid, connect_idsize, - gtls_sessionid_free); + result = Curl_ssl_set_sessionid(cf, data, &connssl->peer, + connect_sessionid, connect_idsize, + gtls_sessionid_free); Curl_ssl_sessionid_unlock(data); } } @@ -1082,7 +1071,7 @@ CURLcode Curl_gtls_ctx_init(struct gtls_ctx *gctx, /* This might be a reconnect, so we check for a session ID in the cache to speed up things */ - if(conn_config->sessionid) { + if(conn_config->cache_session) { void *ssl_sessionid; size_t ssl_idsize; diff --git a/lib/vtls/mbedtls.c b/lib/vtls/mbedtls.c index ba61c50e13..4471f7d79b 100644 --- a/lib/vtls/mbedtls.c +++ b/lib/vtls/mbedtls.c @@ -838,7 +838,7 @@ mbed_connect_step1(struct Curl_cfilter *cf, struct Curl_easy *data) #endif /* Check if there is a cached ID we can/should use here! */ - if(ssl_config->primary.sessionid) { + if(ssl_config->primary.cache_session) { void *old_session = NULL; Curl_ssl_sessionid_lock(data); @@ -1189,10 +1189,9 @@ mbed_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) DEBUGASSERT(ssl_connect_3 == connssl->connecting_state); DEBUGASSERT(backend); - if(ssl_config->primary.sessionid) { + if(ssl_config->primary.cache_session) { int ret; mbedtls_ssl_session *our_ssl_sessionid; - void *old_ssl_sessionid = NULL; our_ssl_sessionid = malloc(sizeof(mbedtls_ssl_session)); if(!our_ssl_sessionid) @@ -1211,13 +1210,9 @@ mbed_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) /* If there is already a matching session in the cache, delete it */ Curl_ssl_sessionid_lock(data); - if(!Curl_ssl_getsessionid(cf, data, &connssl->peer, - &old_ssl_sessionid, NULL)) - Curl_ssl_delsessionid(data, old_ssl_sessionid); - - retcode = Curl_ssl_addsessionid(cf, data, &connssl->peer, - our_ssl_sessionid, 0, - mbedtls_session_free); + retcode = Curl_ssl_set_sessionid(cf, data, &connssl->peer, + our_ssl_sessionid, 0, + mbedtls_session_free); Curl_ssl_sessionid_unlock(data); if(retcode) return retcode; diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c index 0bb6f0ee9b..f7dc9516a0 100644 --- a/lib/vtls/openssl.c +++ b/lib/vtls/openssl.c @@ -2867,42 +2867,25 @@ CURLcode Curl_ossl_add_session(struct Curl_cfilter *cf, SSL_SESSION *session) { const struct ssl_config_data *config; - bool isproxy; - bool added = FALSE; + CURLcode result = CURLE_OK; if(!cf || !data) goto out; - isproxy = Curl_ssl_cf_is_proxy(cf); - config = Curl_ssl_cf_get_config(cf, data); - if(config->primary.sessionid) { - bool incache; - void *old_session = NULL; + if(config->primary.cache_session) { Curl_ssl_sessionid_lock(data); - if(isproxy) - incache = FALSE; - else - incache = !(Curl_ssl_getsessionid(cf, data, peer, - &old_session, NULL)); - if(incache && (old_session != session)) { - infof(data, "old SSL session ID is stale, removing"); - Curl_ssl_delsessionid(data, old_session); - incache = FALSE; - } - - if(!incache) { - added = TRUE; - Curl_ssl_addsessionid(cf, data, peer, session, 0, ossl_session_free); - } + result = Curl_ssl_set_sessionid(cf, data, peer, session, 0, + ossl_session_free); + session = NULL; /* call has taken ownership */ Curl_ssl_sessionid_unlock(data); } out: - if(!added) + if(session) ossl_session_free(session, 0); - return CURLE_OK; + return result; } /* The "new session" callback must return zero if the session can be removed @@ -3945,7 +3928,7 @@ CURLcode Curl_ossl_ctx_init(struct ossl_ctx *octx, #endif octx->reused_session = FALSE; - if(ssl_config->primary.sessionid && transport == TRNSPRT_TCP) { + if(ssl_config->primary.cache_session && transport == TRNSPRT_TCP) { Curl_ssl_sessionid_lock(data); if(!Curl_ssl_getsessionid(cf, data, peer, &ssl_sessionid, NULL)) { /* we got a session id, use it! */ diff --git a/lib/vtls/schannel.c b/lib/vtls/schannel.c index a5eed10249..f9bb2f8247 100644 --- a/lib/vtls/schannel.c +++ b/lib/vtls/schannel.c @@ -1127,7 +1127,7 @@ schannel_connect_step1(struct Curl_cfilter *cf, struct Curl_easy *data) backend->cred = NULL; /* check for an existing reusable credential handle */ - if(ssl_config->primary.sessionid) { + if(ssl_config->primary.cache_session) { Curl_ssl_sessionid_lock(data); if(!Curl_ssl_getsessionid(cf, data, &connssl->peer, (void **)&old_cred, NULL)) { @@ -1772,35 +1772,16 @@ schannel_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) #endif /* save the current session data for possible reuse */ - if(ssl_config->primary.sessionid) { - bool incache; - struct Curl_schannel_cred *old_cred = NULL; - + if(ssl_config->primary.cache_session) { Curl_ssl_sessionid_lock(data); - incache = !(Curl_ssl_getsessionid(cf, data, &connssl->peer, - (void **)&old_cred, NULL)); - if(incache) { - if(old_cred != backend->cred) { - DEBUGF(infof(data, - "schannel: old credential handle is stale, removing")); - /* we are not taking old_cred ownership here, no refcount++ is - needed */ - Curl_ssl_delsessionid(data, (void *)old_cred); - incache = FALSE; - } - } - 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), - schannel_session_free); - if(result) { - Curl_ssl_sessionid_unlock(data); - return result; - } - } + /* Up ref count since call takes ownership */ + backend->cred->refcount++; + result = Curl_ssl_set_sessionid(cf, data, &connssl->peer, backend->cred, + sizeof(struct Curl_schannel_cred), + schannel_session_free); Curl_ssl_sessionid_unlock(data); + if(result) + return result; } if(data->set.ssl.certinfo) { diff --git a/lib/vtls/sectransp.c b/lib/vtls/sectransp.c index 0866af04e4..4868395c53 100644 --- a/lib/vtls/sectransp.c +++ b/lib/vtls/sectransp.c @@ -1477,7 +1477,7 @@ static CURLcode sectransp_connect_step1(struct Curl_cfilter *cf, #endif /* CURL_BUILD_MAC_10_9 || CURL_BUILD_IOS_7 */ /* Check if there is a cached ID we can/should use here! */ - if(ssl_config->primary.sessionid) { + if(ssl_config->primary.cache_session) { char *ssl_sessionid; size_t ssl_sessionid_len; @@ -1511,9 +1511,9 @@ static CURLcode sectransp_connect_step1(struct Curl_cfilter *cf, return CURLE_SSL_CONNECT_ERROR; } - result = Curl_ssl_addsessionid(cf, data, &connssl->peer, ssl_sessionid, - ssl_sessionid_len, - sectransp_session_free); + result = Curl_ssl_set_sessionid(cf, data, &connssl->peer, ssl_sessionid, + ssl_sessionid_len, + sectransp_session_free); Curl_ssl_sessionid_unlock(data); if(result) return result; diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c index 45f9d7cab8..96e7197d45 100644 --- a/lib/vtls/vtls.c +++ b/lib/vtls/vtls.c @@ -168,7 +168,7 @@ void Curl_ssl_easy_config_init(struct Curl_easy *data) */ data->set.ssl.primary.verifypeer = TRUE; data->set.ssl.primary.verifyhost = TRUE; - data->set.ssl.primary.sessionid = TRUE; /* session ID caching by default */ + data->set.ssl.primary.cache_session = TRUE; /* caching by default */ #ifndef CURL_DISABLE_PROXY data->set.proxy_ssl = data->set.ssl; #endif @@ -230,7 +230,7 @@ static bool clone_ssl_primary_config(struct ssl_primary_config *source, dest->verifypeer = source->verifypeer; dest->verifyhost = source->verifyhost; dest->verifystatus = source->verifystatus; - dest->sessionid = source->sessionid; + dest->cache_session = source->cache_session; dest->ssl_options = source->ssl_options; CLONE_BLOB(cert_blob); @@ -551,9 +551,9 @@ bool Curl_ssl_getsessionid(struct Curl_cfilter *cf, if(!ssl_config) return TRUE; - DEBUGASSERT(ssl_config->primary.sessionid); + DEBUGASSERT(ssl_config->primary.cache_session); - if(!ssl_config->primary.sessionid || !data->state.session) + if(!ssl_config->primary.cache_session || !data->state.session) /* session ID reuse is disabled or the session cache has not been setup */ return TRUE; @@ -637,18 +637,12 @@ void Curl_ssl_delsessionid(struct Curl_easy *data, void *ssl_sessionid) } } -/* - * Store session id in the session cache. The ID passed on to this function - * must already have been extracted and allocated the proper way for the SSL - * layer. Curl_XXXX_session_free() will be called to free/kill the session ID - * later on. - */ -CURLcode Curl_ssl_addsessionid(struct Curl_cfilter *cf, - struct Curl_easy *data, - const struct ssl_peer *peer, - void *ssl_sessionid, - size_t idsize, - Curl_ssl_sessionid_dtor *sessionid_free_cb) +CURLcode Curl_ssl_set_sessionid(struct Curl_cfilter *cf, + struct Curl_easy *data, + const struct ssl_peer *peer, + void *ssl_sessionid, + size_t idsize, + 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); @@ -659,6 +653,8 @@ CURLcode Curl_ssl_addsessionid(struct Curl_cfilter *cf, char *clone_conn_to_host = NULL; int conn_to_port; long *general_age; + void *old_sessionid; + size_t old_size; CURLcode result = CURLE_OUT_OF_MEMORY; DEBUGASSERT(ssl_sessionid); @@ -669,9 +665,20 @@ CURLcode Curl_ssl_addsessionid(struct Curl_cfilter *cf, return CURLE_OK; } + if(!Curl_ssl_getsessionid(cf, data, peer, &old_sessionid, &old_size)) { + if((old_size == idsize) && + ((old_sessionid == ssl_sessionid) || + (idsize && !memcmp(old_sessionid, ssl_sessionid, idsize)))) { + /* the very same */ + sessionid_free_cb(ssl_sessionid, idsize); + return CURLE_OK; + } + Curl_ssl_delsessionid(data, old_sessionid); + } + store = &data->state.session[0]; oldest_age = data->state.session[0].age; /* zero if unused */ - DEBUGASSERT(ssl_config->primary.sessionid); + DEBUGASSERT(ssl_config->primary.cache_session); (void)ssl_config; clone_host = strdup(peer->hostname); diff --git a/lib/vtls/vtls_int.h b/lib/vtls/vtls_int.h index ea41abdda7..1472a0ca5c 100644 --- a/lib/vtls/vtls_int.h +++ b/lib/vtls/vtls_int.h @@ -198,19 +198,22 @@ bool Curl_ssl_getsessionid(struct Curl_cfilter *cf, const struct ssl_peer *peer, void **ssl_sessionid, size_t *idsize); /* set 0 if unknown */ -/* add a new session ID + +/* Set a TLS session ID for `peer`. Replaces an existing session ID if + * not already the very same. * Sessionid mutex must be locked (see Curl_ssl_sessionid_lock). + * Call takes ownership of `ssl_sessionid`, using `sessionid_free_cb` + * to deallocate it. Is called in all outcomes, either right away or + * later when the session cache is cleaned up. * 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, - Curl_ssl_sessionid_dtor *sessionid_free_cb); +CURLcode Curl_ssl_set_sessionid(struct Curl_cfilter *cf, + struct Curl_easy *data, + const struct ssl_peer *peer, + void *sessionid, + size_t sessionid_size, + 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 0aaf59ba01..01036919b2 100644 --- a/lib/vtls/wolfssl.c +++ b/lib/vtls/wolfssl.c @@ -891,7 +891,7 @@ wolfssl_connect_step1(struct Curl_cfilter *cf, struct Curl_easy *data) #endif /* HAVE_SECURE_RENEGOTIATION */ /* Check if there is a cached ID we can/should use here! */ - if(ssl_config->primary.sessionid) { + if(ssl_config->primary.cache_session) { void *ssl_sessionid = NULL; Curl_ssl_sessionid_lock(data); @@ -1268,24 +1268,16 @@ wolfssl_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) DEBUGASSERT(ssl_connect_3 == connssl->connecting_state); DEBUGASSERT(backend); - if(ssl_config->primary.sessionid) { + if(ssl_config->primary.cache_session) { /* 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) { - Curl_ssl_delsessionid(data, old_ssl_sessionid); - } - /* call takes ownership of `our_ssl_sessionid` */ - result = Curl_ssl_addsessionid(cf, data, &connssl->peer, - our_ssl_sessionid, 0, - wolfssl_session_free); + result = Curl_ssl_set_sessionid(cf, data, &connssl->peer, + our_ssl_sessionid, 0, + wolfssl_session_free); Curl_ssl_sessionid_unlock(data); if(result) { failf(data, "failed to store ssl session");