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

diff --git a/lib/vtls/bearssl.c b/lib/vtls/bearssl.c
index e87649e2a753fddc6ebb48dd2c1d86379bf08039..9b772d064d75848f8f56e0821d3d33e61eb5e92b 100644 (file)
--- a/lib/vtls/bearssl.c
+++ b/lib/vtls/bearssl.c
@@ -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;
     }
   }

diff --git a/lib/vtls/gtls.c b/lib/vtls/gtls.c
index 1b145d8ebb63f8d90fe1498ef3858c8a824ad649..f3a308fbcdfd1bbbf05f6fa0ac591c552a017127 100644 (file)
--- a/lib/vtls/gtls.c
+++ b/lib/vtls/gtls.c
@@ -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;
       }
     }

diff --git a/lib/vtls/mbedtls.c b/lib/vtls/mbedtls.c
index 780d13e18867c9a5bd96a1ba368146c4adb53c1a..08c79e1624c31d3a852242a7d2771dc3c3222321 100644 (file)
--- a/lib/vtls/mbedtls.c
+++ b/lib/vtls/mbedtls.c
@@ -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;
     }

diff --git a/lib/vtls/mesalink.c b/lib/vtls/mesalink.c
index 3db9184f795b7ad8446142ee11771ce5117c3cd2..0a1dea3ac02f77696d5d96c097625c5327cfa957 100644 (file)
--- a/lib/vtls/mesalink.c
+++ b/lib/vtls/mesalink.c
@@ -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");

diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
index 87f4b02b715c05da46e55a08734fe4dedb103847..aafc2ae09f130798a9392a7fa9a2ebb2536e745e 100644 (file)
--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -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");

diff --git a/lib/vtls/schannel.c b/lib/vtls/schannel.c
index 20c478533936ace28441ff0266ff8d0be9cddc4c..ef3c919bb336d175d91bb5fec2f0f81d0e6dce08 100644 (file)
--- a/lib/vtls/schannel.c
+++ b/lib/vtls/schannel.c
@@ -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,

diff --git a/lib/vtls/sectransp.c b/lib/vtls/sectransp.c
index 1e6ed5f06d6b4b9dc018276f4a0a858ccd5be930..0bf515460ddc3b0c2a2415a8de8f28bd71fa4fda 100644 (file)
--- a/lib/vtls/sectransp.c
+++ b/lib/vtls/sectransp.c
@@ -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");

diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c
index e5bbe1f5f095f4e96cb4fac57b26b34a3740f9e0..6007bbba0fd048880d129e24078e03dea0995336 100644 (file)
--- a/lib/vtls/vtls.c
+++ b/lib/vtls/vtls.c
@@ -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"));

diff --git a/lib/vtls/vtls.h b/lib/vtls/vtls.h
index beaa83d9e3d44615e085d4da87a1f8d51dfcd7a3..c7bbba082d9c88ba2d57e004a151b73866389ec0 100644 (file)
--- a/lib/vtls/vtls.h
+++ b/lib/vtls/vtls.h
@@ -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

diff --git a/lib/vtls/wolfssl.c b/lib/vtls/wolfssl.c
index 16fbb89288e6c03edd1457d1106dabb9c3765eb1..4b501b1c8c646e17f482301ec3ed8f4d0df9af2b 100644 (file)
--- a/lib/vtls/wolfssl.c
+++ b/lib/vtls/wolfssl.c
@@ -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");