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

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;
 

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 {

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;

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;
 

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;

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! */

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) {

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;

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);

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 */

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");