vtls: Fix a memory leak if an SSL session cannot be added to the cache

On connection shutdown, a new TLS session ticket may arrive after the
SSL session cache has already been destructed. In this case, the new
SSL session cannot be added to the SSL session cache.

The callers of Curl_ssl_addsessionid() need to know whether the SSL
session has been added to the cache. If it has not been added, the
reference counter of the SSL session must not be incremented, or memory
used by the SSL session must be freed. This is now possible with the new
output parameter "added" of Curl_ssl_addsessionid().

Fixes #7683
Closes #7752
This commit is contained in:
Michael Kaufmann 2021-09-22 12:04:25 +02:00
parent f4a3ae8ea8
commit 60738f398c
10 changed files with 36 additions and 16 deletions

View File

@ -608,6 +608,7 @@ static CURLcode bearssl_connect_step3(struct Curl_easy *data,
if(SSL_SET_OPTION(primary.sessionid)) {
bool incache;
bool added = FALSE;
void *oldsession;
br_ssl_session_parameters *session;
@ -623,10 +624,11 @@ static CURLcode bearssl_connect_step3(struct Curl_easy *data,
Curl_ssl_delsessionid(data, oldsession);
ret = Curl_ssl_addsessionid(data, conn,
SSL_IS_PROXY() ? TRUE : FALSE,
session, 0, sockindex);
session, 0, sockindex, &added);
Curl_ssl_sessionid_unlock(data);
if(ret) {
if(!added)
free(session);
if(ret) {
return CURLE_OUT_OF_MEMORY;
}
}

View File

@ -1287,6 +1287,7 @@ gtls_connect_step3(struct Curl_easy *data,
if(connect_sessionid) {
bool incache;
bool added = FALSE;
void *ssl_sessionid;
/* extract session ID to the allocated buffer */
@ -1306,10 +1307,11 @@ gtls_connect_step3(struct Curl_easy *data,
result = Curl_ssl_addsessionid(data, conn,
SSL_IS_PROXY() ? TRUE : FALSE,
connect_sessionid, connect_idsize,
sockindex);
sockindex, &added);
Curl_ssl_sessionid_unlock(data);
if(result) {
if(!added)
free(connect_sessionid);
if(result) {
result = CURLE_OUT_OF_MEMORY;
}
}

View File

@ -784,6 +784,7 @@ mbed_connect_step3(struct Curl_easy *data, struct connectdata *conn,
mbedtls_ssl_session *our_ssl_sessionid;
void *old_ssl_sessionid = NULL;
bool isproxy = SSL_IS_PROXY() ? TRUE : FALSE;
bool added = FALSE;
our_ssl_sessionid = malloc(sizeof(mbedtls_ssl_session));
if(!our_ssl_sessionid)
@ -807,11 +808,13 @@ mbed_connect_step3(struct Curl_easy *data, struct connectdata *conn,
Curl_ssl_delsessionid(data, old_ssl_sessionid);
retcode = Curl_ssl_addsessionid(data, conn, isproxy, our_ssl_sessionid,
0, sockindex);
0, sockindex, &added);
Curl_ssl_sessionid_unlock(data);
if(retcode) {
if(!added) {
mbedtls_ssl_session_free(our_ssl_sessionid);
free(our_ssl_sessionid);
}
if(retcode) {
failf(data, "failed to store ssl session");
return retcode;
}

View File

@ -365,7 +365,7 @@ mesalink_connect_step3(struct connectdata *conn, int sockindex)
if(!incache) {
result =
Curl_ssl_addsessionid(data, conn, isproxy, our_ssl_sessionid, 0,
sockindex);
sockindex, NULL);
if(result) {
Curl_ssl_sessionid_unlock(data);
failf(data, "failed to store ssl session");

View File

@ -2493,6 +2493,7 @@ static int ossl_new_session_cb(SSL *ssl, SSL_SESSION *ssl_sessionid)
if(SSL_SET_OPTION(primary.sessionid)) {
bool incache;
bool added = FALSE;
void *old_ssl_sessionid = NULL;
Curl_ssl_sessionid_lock(data);
@ -2511,9 +2512,11 @@ static int ossl_new_session_cb(SSL *ssl, SSL_SESSION *ssl_sessionid)
if(!incache) {
if(!Curl_ssl_addsessionid(data, conn, isproxy, ssl_sessionid,
0 /* unknown size */, sockindex)) {
/* the session has been put into the session cache */
res = 1;
0 /* unknown size */, sockindex, &added)) {
if(added) {
/* the session has been put into the session cache */
res = 1;
}
}
else
failf(data, "failed to store ssl session");

View File

@ -1436,6 +1436,7 @@ schannel_connect_step3(struct Curl_easy *data, struct connectdata *conn,
/* save the current session data for possible re-use */
if(SSL_SET_OPTION(primary.sessionid)) {
bool incache;
bool added = FALSE;
struct Curl_schannel_cred *old_cred = NULL;
Curl_ssl_sessionid_lock(data);
@ -1453,13 +1454,13 @@ schannel_connect_step3(struct Curl_easy *data, struct connectdata *conn,
if(!incache) {
result = Curl_ssl_addsessionid(data, conn, isproxy, BACKEND->cred,
sizeof(struct Curl_schannel_cred),
sockindex);
sockindex, &added);
if(result) {
Curl_ssl_sessionid_unlock(data);
failf(data, "schannel: failed to store credential handle");
return result;
}
else {
else if(added) {
/* this cred session is now also referenced by sessionid cache */
BACKEND->cred->refcount++;
DEBUGF(infof(data,

View File

@ -2109,7 +2109,7 @@ static CURLcode sectransp_connect_step1(struct Curl_easy *data,
}
result = Curl_ssl_addsessionid(data, conn, isproxy, ssl_sessionid,
ssl_sessionid_len, sockindex);
ssl_sessionid_len, sockindex, NULL);
Curl_ssl_sessionid_unlock(data);
if(result) {
failf(data, "failed to store ssl session");

View File

@ -516,7 +516,8 @@ CURLcode Curl_ssl_addsessionid(struct Curl_easy *data,
const bool isProxy,
void *ssl_sessionid,
size_t idsize,
int sockindex)
int sockindex,
bool *added)
{
size_t i;
struct Curl_ssl_session *store;
@ -536,6 +537,10 @@ CURLcode Curl_ssl_addsessionid(struct Curl_easy *data,
const char *hostname = conn->host.name;
#endif
(void)sockindex;
if(added)
*added = FALSE;
if(!data->state.session)
return CURLE_OK;
@ -609,6 +614,9 @@ CURLcode Curl_ssl_addsessionid(struct Curl_easy *data,
return CURLE_OUT_OF_MEMORY;
}
if(added)
*added = TRUE;
DEBUGF(infof(data, "Added Session ID to cache for %s://%s:%d [%s]",
store->scheme, store->name, store->remote_port,
isProxy ? "PROXY" : "server"));

View File

@ -261,7 +261,8 @@ CURLcode Curl_ssl_addsessionid(struct Curl_easy *data,
const bool isProxy,
void *ssl_sessionid,
size_t idsize,
int sockindex);
int sockindex,
bool *added);
/* Kill a single session ID entry in the cache
* Sessionid mutex must be locked (see Curl_ssl_sessionid_lock).
* This will call engine-specific curlssl_session_free function, which must

View File

@ -749,7 +749,7 @@ wolfssl_connect_step3(struct Curl_easy *data, struct connectdata *conn,
if(!incache) {
result = Curl_ssl_addsessionid(data, conn, isproxy, our_ssl_sessionid,
0, sockindex);
0, sockindex, NULL);
if(result) {
Curl_ssl_sessionid_unlock(data);
failf(data, "failed to store ssl session");