]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
openssl: associate/detach the transfer from connection
authorHarry Sintonen <sintonen@iki.fi>
Wed, 5 May 2021 11:42:26 +0000 (13:42 +0200)
committerDaniel Stenberg <daniel@haxx.se>
Mon, 24 May 2021 11:15:10 +0000 (13:15 +0200)
CVE-2021-22901

Bug: https://curl.se/docs/CVE-2021-22901.html

13 files changed:
lib/multi.c
lib/vtls/gskit.c
lib/vtls/gtls.c
lib/vtls/mbedtls.c
lib/vtls/mesalink.c
lib/vtls/nss.c
lib/vtls/openssl.c
lib/vtls/rustls.c
lib/vtls/schannel.c
lib/vtls/sectransp.c
lib/vtls/vtls.c
lib/vtls/vtls.h
lib/vtls/wolfssl.c

index 54365f399e9bb61bdd3a55cfa25f7dbabdfdb8d0..1b3e261c682a47232dc2a2905ba984ea27eefc7d 100644 (file)
@@ -878,8 +878,10 @@ bool Curl_multiplex_wanted(const struct Curl_multi *multi)
 void Curl_detach_connnection(struct Curl_easy *data)
 {
   struct connectdata *conn = data->conn;
-  if(conn)
+  if(conn) {
     Curl_llist_remove(&conn->easyq, &data->conn_queue, NULL);
+    Curl_ssl_detach_conn(data, conn);
+  }
   data->conn = NULL;
 }
 
@@ -898,6 +900,7 @@ void Curl_attach_connnection(struct Curl_easy *data,
                          &data->conn_queue);
   if(conn->handler->attach)
     conn->handler->attach(data, conn);
+  Curl_ssl_associate_conn(data, conn);
 }
 
 static int waitconnect_getsock(struct connectdata *conn,
index c648f624579b2919aef8a1ef39b6db0a629e5dae..ca953769d1baf9ca23c48f4e642afd496a50bd89 100644 (file)
@@ -1304,7 +1304,9 @@ const struct Curl_ssl Curl_ssl_gskit = {
   Curl_none_set_engine_default,   /* set_engine_default */
   Curl_none_engines_list,         /* engines_list */
   Curl_none_false_start,          /* false_start */
-  NULL                            /* sha256sum */
+  NULL,                           /* sha256sum */
+  NULL,                           /* associate_connection */
+  NULL                            /* disassociate_connection */
 };
 
 #endif /* USE_GSKIT */
index a10c0dbcca05c759cbdc8bc9675d8b99111db864..ecde5c44deeb5572a03e00452cf0f592feccaaf3 100644 (file)
@@ -1656,7 +1656,9 @@ const struct Curl_ssl Curl_ssl_gnutls = {
   Curl_none_set_engine_default,  /* set_engine_default */
   Curl_none_engines_list,        /* engines_list */
   Curl_none_false_start,         /* false_start */
-  gtls_sha256sum                 /* sha256sum */
+  gtls_sha256sum,                /* sha256sum */
+  NULL,                          /* associate_connection */
+  NULL                           /* disassociate_connection */
 };
 
 #endif /* USE_GNUTLS */
index ca77de58667c8e424a267431da16dc6b2e460648..3a0be0f04b4f2740d46731e3e61bdfd785e2d843 100644 (file)
@@ -1093,7 +1093,9 @@ const struct Curl_ssl Curl_ssl_mbedtls = {
   Curl_none_set_engine_default,     /* set_engine_default */
   Curl_none_engines_list,           /* engines_list */
   Curl_none_false_start,            /* false_start */
-  mbedtls_sha256sum                 /* sha256sum */
+  mbedtls_sha256sum,                /* sha256sum */
+  NULL,                             /* associate_connection */
+  NULL                              /* disassociate_connection */
 };
 
 #endif /* USE_MBEDTLS */
index f16c77c27fe005c8cacab9c229fbd262ffce196f..bf8600d3230b1b2e46caf493342ea544aa2f2f59 100644 (file)
@@ -666,7 +666,9 @@ const struct Curl_ssl Curl_ssl_mesalink = {
   Curl_none_set_engine_default,  /* set_engine_default */
   Curl_none_engines_list,        /* engines_list */
   Curl_none_false_start,         /* false_start */
-  NULL                           /* sha256sum */
+  NULL,                          /* sha256sum */
+  NULL,                          /* associate_connection */
+  NULL                           /* disassociate_connection */
 };
 
 #endif
index 2aa4bdaa134ff30c104fac461b4f54ccd9cc2c8b..1582b1e580a9094624b1f7186f10c1e286ea08a6 100644 (file)
@@ -2465,7 +2465,9 @@ const struct Curl_ssl Curl_ssl_nss = {
   Curl_none_set_engine_default, /* set_engine_default */
   Curl_none_engines_list,       /* engines_list */
   nss_false_start,              /* false_start */
-  nss_sha256sum                 /* sha256sum */
+  nss_sha256sum,                /* sha256sum */
+  NULL,                         /* associate_connection */
+  NULL                          /* disassociate_connection */
 };
 
 #endif /* USE_NSS */
index 1521600dd5f6d53ca803d312ae220c48f10b54f7..ebd7abc3b4ac016dc6506165f9c765273beb4a1b 100644 (file)
@@ -240,6 +240,10 @@ struct ssl_backend_data {
 #endif
 };
 
+static void ossl_associate_connection(struct Curl_easy *data,
+                                      struct connectdata *conn,
+                                      int sockindex);
+
 /*
  * Number of bytes to read from the random number seed file. This must be
  * a finite value (because some entropy "files" like /dev/urandom have
@@ -2581,6 +2585,7 @@ static CURLcode ossl_connect_step1(struct Curl_easy *data,
   curl_socket_t sockfd = conn->sock[sockindex];
   struct ssl_connect_data *connssl = &conn->ssl[sockindex];
   ctx_option_t ctx_options = 0;
+  void *ssl_sessionid = NULL;
 
 #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
   bool sni;
@@ -3225,46 +3230,23 @@ static CURLcode ossl_connect_step1(struct Curl_easy *data,
   }
 #endif
 
-  /* Check if there's a cached ID we can/should use here! */
-  if(SSL_SET_OPTION(primary.sessionid)) {
-    void *ssl_sessionid = NULL;
-    int data_idx = ossl_get_ssl_data_index();
-    int connectdata_idx = ossl_get_ssl_conn_index();
-    int sockindex_idx = ossl_get_ssl_sockindex_index();
-    int proxy_idx = ossl_get_proxy_index();
-
-    if(data_idx >= 0 && connectdata_idx >= 0 && sockindex_idx >= 0 &&
-       proxy_idx >= 0) {
-      /* Store the data needed for the "new session" callback.
-       * The sockindex is stored as a pointer to an array element. */
-      SSL_set_ex_data(backend->handle, data_idx, data);
-      SSL_set_ex_data(backend->handle, connectdata_idx, conn);
-      SSL_set_ex_data(backend->handle, sockindex_idx, conn->sock + sockindex);
-#ifndef CURL_DISABLE_PROXY
-      SSL_set_ex_data(backend->handle, proxy_idx, SSL_IS_PROXY() ? (void *) 1:
-                      NULL);
-#else
-      SSL_set_ex_data(backend->handle, proxy_idx, NULL);
-#endif
-
-    }
+  ossl_associate_connection(data, conn, sockindex);
 
-    Curl_ssl_sessionid_lock(data);
-    if(!Curl_ssl_getsessionid(data, conn, SSL_IS_PROXY() ? TRUE : FALSE,
-                              &ssl_sessionid, NULL, sockindex)) {
-      /* we got a session id, use it! */
-      if(!SSL_set_session(backend->handle, ssl_sessionid)) {
-        Curl_ssl_sessionid_unlock(data);
-        failf(data, "SSL: SSL_set_session failed: %s",
-              ossl_strerror(ERR_get_error(), error_buffer,
-                            sizeof(error_buffer)));
-        return CURLE_SSL_CONNECT_ERROR;
-      }
-      /* Informational message */
-      infof(data, "SSL re-using session ID\n");
+  Curl_ssl_sessionid_lock(data);
+  if(!Curl_ssl_getsessionid(data, conn, SSL_IS_PROXY() ? TRUE : FALSE,
+                            &ssl_sessionid, NULL, sockindex)) {
+    /* we got a session id, use it! */
+    if(!SSL_set_session(backend->handle, ssl_sessionid)) {
+      Curl_ssl_sessionid_unlock(data);
+      failf(data, "SSL: SSL_set_session failed: %s",
+            ossl_strerror(ERR_get_error(), error_buffer,
+                          sizeof(error_buffer)));
+      return CURLE_SSL_CONNECT_ERROR;
     }
-    Curl_ssl_sessionid_unlock(data);
+    /* Informational message */
+    infof(data, "SSL re-using session ID\n");
   }
+  Curl_ssl_sessionid_unlock(data);
 
 #ifndef CURL_DISABLE_PROXY
   if(conn->proxy_ssl[sockindex].use) {
@@ -4498,6 +4480,90 @@ static void *ossl_get_internals(struct ssl_connect_data *connssl,
          (void *)backend->ctx : (void *)backend->handle;
 }
 
+static void ossl_associate_connection(struct Curl_easy *data,
+                                      struct connectdata *conn,
+                                      int sockindex)
+{
+  struct ssl_connect_data *connssl = &conn->ssl[sockindex];
+  struct ssl_backend_data *backend = connssl->backend;
+
+  /* If we don't have SSL context, do nothing. */
+  if(!backend->handle)
+    return;
+
+  if(SSL_SET_OPTION(primary.sessionid)) {
+    int data_idx = ossl_get_ssl_data_index();
+    int connectdata_idx = ossl_get_ssl_conn_index();
+    int sockindex_idx = ossl_get_ssl_sockindex_index();
+    int proxy_idx = ossl_get_proxy_index();
+
+    if(data_idx >= 0 && connectdata_idx >= 0 && sockindex_idx >= 0 &&
+       proxy_idx >= 0) {
+      /* Store the data needed for the "new session" callback.
+       * The sockindex is stored as a pointer to an array element. */
+      SSL_set_ex_data(backend->handle, data_idx, data);
+      SSL_set_ex_data(backend->handle, connectdata_idx, conn);
+      SSL_set_ex_data(backend->handle, sockindex_idx, conn->sock + sockindex);
+#ifndef CURL_DISABLE_PROXY
+      SSL_set_ex_data(backend->handle, proxy_idx, SSL_IS_PROXY() ? (void *) 1:
+                      NULL);
+#else
+      SSL_set_ex_data(backend->handle, proxy_idx, NULL);
+#endif
+    }
+  }
+}
+
+/*
+ * Starting with TLS 1.3, the ossl_new_session_cb callback gets called after
+ * the handshake. If the transfer that sets up the callback gets killed before
+ * this callback arrives, we must make sure to properly clear the data to
+ * avoid UAF problems. A future optimization could be to instead store another
+ * transfer that might still be using the same connection.
+ */
+
+static void ossl_disassociate_connection(struct Curl_easy *data,
+                                         int sockindex)
+{
+  struct connectdata *conn = data->conn;
+  struct ssl_connect_data *connssl = &conn->ssl[sockindex];
+  struct ssl_backend_data *backend = connssl->backend;
+
+  /* If we don't have SSL context, do nothing. */
+  if(!backend->handle)
+    return;
+
+  if(SSL_SET_OPTION(primary.sessionid)) {
+    bool isproxy = FALSE;
+    bool incache;
+    void *old_ssl_sessionid = NULL;
+    int data_idx = ossl_get_ssl_data_index();
+    int connectdata_idx = ossl_get_ssl_conn_index();
+    int sockindex_idx = ossl_get_ssl_sockindex_index();
+    int proxy_idx = ossl_get_proxy_index();
+
+    if(data_idx >= 0 && connectdata_idx >= 0 && sockindex_idx >= 0 &&
+       proxy_idx >= 0) {
+      /* Invalidate the session cache entry, if any */
+      isproxy = SSL_get_ex_data(backend->handle, proxy_idx) ? TRUE : FALSE;
+
+      /* Disable references to data in "new session" callback to avoid
+       * accessing a stale pointer. */
+      SSL_set_ex_data(backend->handle, data_idx, NULL);
+      SSL_set_ex_data(backend->handle, connectdata_idx, NULL);
+      SSL_set_ex_data(backend->handle, sockindex_idx, NULL);
+      SSL_set_ex_data(backend->handle, proxy_idx, NULL);
+    }
+
+    Curl_ssl_sessionid_lock(data);
+    incache = !(Curl_ssl_getsessionid(data, conn, isproxy,
+                                      &old_ssl_sessionid, NULL, sockindex));
+    if(incache)
+      Curl_ssl_delsessionid(data, old_ssl_sessionid);
+    Curl_ssl_sessionid_unlock(data);
+  }
+}
+
 const struct Curl_ssl Curl_ssl_openssl = {
   { CURLSSLBACKEND_OPENSSL, "openssl" }, /* info */
 
@@ -4533,10 +4599,12 @@ const struct Curl_ssl Curl_ssl_openssl = {
   ossl_engines_list,        /* engines_list */
   Curl_none_false_start,    /* false_start */
 #if (OPENSSL_VERSION_NUMBER >= 0x0090800fL) && !defined(OPENSSL_NO_SHA256)
-  ossl_sha256sum            /* sha256sum */
+  ossl_sha256sum,           /* sha256sum */
 #else
-  NULL                      /* sha256sum */
+  NULL,                     /* sha256sum */
 #endif
+  ossl_associate_connection, /* associate_connection */
+  ossl_disassociate_connection /* disassociate_connection */
 };
 
 #endif /* USE_OPENSSL */
index 9dfbd2c3c4c20c5b57357db8acfb4cd1d9ca58f9..161f3bf51d759c3be34fa4a22760d37d3f7a866e 100644 (file)
@@ -604,7 +604,9 @@ const struct Curl_ssl Curl_ssl_rustls = {
   Curl_none_set_engine_default,    /* set_engine_default */
   Curl_none_engines_list,          /* engines_list */
   Curl_none_false_start,           /* false_start */
-  NULL                             /* sha256sum */
+  NULL,                            /* sha256sum */
+  NULL,                            /* associate_connection */
+  NULL                             /* disassociate_connection */
 };
 
 #endif /* USE_RUSTLS */
index dba7072273a97e378cd4b8d35c8c8163a18d3364..2bcf11db2576748ed9502cec4de5357bc99b4449 100644 (file)
@@ -329,7 +329,7 @@ get_alg_id_by_name(char *name)
 
 static CURLcode
 set_ssl_ciphers(SCHANNEL_CRED *schannel_cred, char *ciphers,
-                int *algIds)
+                ALG_ID *algIds)
 {
   char *startCur = ciphers;
   int algCount = 0;
@@ -2433,7 +2433,9 @@ const struct Curl_ssl Curl_ssl_schannel = {
   Curl_none_set_engine_default,      /* set_engine_default */
   Curl_none_engines_list,            /* engines_list */
   Curl_none_false_start,             /* false_start */
-  schannel_sha256sum                 /* sha256sum */
+  schannel_sha256sum,                /* sha256sum */
+  NULL,                              /* associate_connection */
+  NULL                               /* disassociate_connection */
 };
 
 #endif /* USE_SCHANNEL */
index 4276b89cfb3aacdd33119745f8a94ad6731f95d8..8b1e84ed77155dfd49f3520737db4536d9cbd5c6 100644 (file)
@@ -3453,6 +3453,8 @@ const struct Curl_ssl Curl_ssl_sectransp = {
   Curl_none_engines_list,             /* engines_list */
   sectransp_false_start,              /* false_start */
   sectransp_sha256sum                 /* sha256sum */
+  NULL,                               /* associate_connection */
+  NULL                                /* disassociate_connection */
 };
 
 #ifdef __clang__
index d63fd5c76386cf7e6ef715385006af79b57b0049..65f4f773dd63b112fd1f7ec23d0fc2e1538cb244 100644 (file)
@@ -586,6 +586,25 @@ CURLcode Curl_ssl_addsessionid(struct Curl_easy *data,
   return CURLE_OK;
 }
 
+void Curl_ssl_associate_conn(struct Curl_easy *data,
+                             struct connectdata *conn)
+{
+  if(Curl_ssl->associate_connection) {
+    Curl_ssl->associate_connection(data, conn, FIRSTSOCKET);
+    if(conn->sock[SECONDARYSOCKET] && conn->bits.sock_accepted)
+      Curl_ssl->associate_connection(data, conn, SECONDARYSOCKET);
+  }
+}
+
+void Curl_ssl_detach_conn(struct Curl_easy *data,
+                          struct connectdata *conn)
+{
+  if(Curl_ssl->disassociate_connection) {
+    Curl_ssl->disassociate_connection(data, FIRSTSOCKET);
+    if(conn->sock[SECONDARYSOCKET] && conn->bits.sock_accepted)
+      Curl_ssl->disassociate_connection(data, SECONDARYSOCKET);
+  }
+}
 
 void Curl_ssl_close_all(struct Curl_easy *data)
 {
@@ -1214,7 +1233,9 @@ static const struct Curl_ssl Curl_ssl_multi = {
   Curl_none_set_engine_default,      /* set_engine_default */
   Curl_none_engines_list,            /* engines_list */
   Curl_none_false_start,             /* false_start */
-  NULL                               /* sha256sum */
+  NULL,                              /* sha256sum */
+  NULL,                              /* associate_connection */
+  NULL                               /* disassociate_connection */
 };
 
 const struct Curl_ssl *Curl_ssl =
index a22d526ca810cd11d6a3f494a01fa778ab095b9b..7f93e7aedb21d5c3dd73ab941408374902ab158c 100644 (file)
@@ -84,6 +84,11 @@ struct Curl_ssl {
   bool (*false_start)(void);
   CURLcode (*sha256sum)(const unsigned char *input, size_t inputlen,
                     unsigned char *sha256sum, size_t sha256sumlen);
+
+  void (*associate_connection)(struct Curl_easy *data,
+                               struct connectdata *conn,
+                               int sockindex);
+  void (*disassociate_connection)(struct Curl_easy *data, int sockindex);
 };
 
 #ifdef USE_SSL
@@ -283,6 +288,11 @@ bool Curl_ssl_cert_status_request(void);
 
 bool Curl_ssl_false_start(void);
 
+void Curl_ssl_associate_conn(struct Curl_easy *data,
+                             struct connectdata *conn);
+void Curl_ssl_detach_conn(struct Curl_easy *data,
+                          struct connectdata *conn);
+
 #define SSL_SHUTDOWN_TIMEOUT 10000 /* ms */
 
 #else /* if not USE_SSL */
@@ -309,6 +319,8 @@ bool Curl_ssl_false_start(void);
 #define Curl_ssl_cert_status_request() FALSE
 #define Curl_ssl_false_start() FALSE
 #define Curl_ssl_tls13_ciphersuites() FALSE
+#define Curl_ssl_associate_conn(a,b) Curl_nop_stmt
+#define Curl_ssl_detach_conn(a,b) Curl_nop_stmt
 #endif
 
 #endif /* HEADER_CURL_VTLS_H */
index 02fcd236697e4fa3357024a81b4ab46f8d8ce481..60e27e366252f1bc84b8610ab8f0e1f7bd187419 100644 (file)
@@ -1125,7 +1125,9 @@ const struct Curl_ssl Curl_ssl_wolfssl = {
   Curl_none_set_engine_default,    /* set_engine_default */
   Curl_none_engines_list,          /* engines_list */
   Curl_none_false_start,           /* false_start */
-  wolfssl_sha256sum                /* sha256sum */
+  wolfssl_sha256sum,               /* sha256sum */
+  NULL,                            /* associate_connection */
+  NULL                             /* disassociate_connection */
 };
 
 #endif