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 904516822adf14ee55475bf00028e11025f9a943..7d35651d10ccb069c316b6fb8154b11442934baf 100644 (file)
--- 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 d38623b0703d62d8113a20de0d7cb828a37f5b65..74b744c7157e97ab2f0c118d1162d3fbeadd8329 100644 (file)
--- 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 04de54f0060f5886f9756ffaba76b8be7c1e357b..47c5528c29c90d1bd271be09cf5caad3262b7e27 100644 (file)
--- 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 f3917076112f2a11bef3e4e12b4c5c6c2c7056f5..dc0f77ba57e53360174f1197424dcefd664f3934 100644 (file)
--- 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 ba61c50e1317e0e7190705cc7f96c39540898af7..4471f7d79b75bca66b4171a08f5f6c5a48779c8a 100644 (file)
--- 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 0bb6f0ee9b7cd3d3129cafca9fe51afd822a6bc9..f7dc9516a0a1a3960bffea8403f15e5b1b7a3a0a 100644 (file)
--- 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 a5eed1024963f41d10f26dc1f914b32f59806e8a..f9bb2f824715ce7510c5619e2515cc9a27eeba1a 100644 (file)
--- 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 0866af04e4fd4562e8f398118d2559fb20ff316e..4868395c5313141e538a7a4eff45ab183465f657 100644 (file)
--- 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 45f9d7cab861eae0f593cc5d869fb67493792336..96e7197d4530260909c4bec1a74b098c50ec8036 100644 (file)
--- 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 ea41abdda7224a43d72ad367235b79441f60c76f..1472a0ca5cfcd94efa71616b371036b769249209 100644 (file)
--- 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 0aaf59ba0199c6cffce1398ee342767ab20c7981..01036919b238eb63534c5319892e9204e6ec183e 100644 (file)
--- 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");