vtls: fix hostname handling in filters

- Copy the hostname and dispname to ssl_connect_data.

Use a copy instead of referencing the `connectdata` instance since this
may get free'ed on connection reuse.

Reported-by: Stefan Talpalaru
Reported-by: sergio-nsk@users.noreply.github.com

Fixes https://github.com/curl/curl/issues/10273
Fixes https://github.com/curl/curl/issues/10309

Closes https://github.com/curl/curl/pull/10310

3 files changed, 61 insertions, 23 deletions

diff --git a/lib/url.c b/lib/url.c
index f90427f9b..ef3b3a4fd 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -3369,6 +3369,20 @@ static void reuse_conn(struct Curl_easy *data,
   }
 #endif
 
+  /* Finding a connection for reuse in the cache matches, among other
+   * things on the "remote-relevant" hostname. This is not necessarily
+   * the authority of the URL, e.g. conn->host. For example:
+   * - we use a proxy (not tunneling). we want to send all requests
+   *   that use the same proxy on this connection.
+   * - we have a "connect-to" setting that may redirect the hostname of
+   *   a new request to the same remote endpoint of an existing conn.
+   *   We want to reuse an existing conn to the remote endpoint.
+   * Since connection reuse does not match on conn->host necessarily, we
+   * switch `existing` conn to `temp` conn's host settings.
+   * TODO: is this correct in the case of TLS connections that have
+   *       used the original hostname in SNI to negotiate? Do we send
+   *       requests for another host through the different SNI?
+   */
   Curl_free_idnconverted_hostname(&existing->host);
   Curl_free_idnconverted_hostname(&existing->conn_to_host);
   Curl_safefree(existing->host.rawalloc);
diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c
index 36ef4fd7a..dd0414ce7 100644
--- a/lib/vtls/vtls.c
+++ b/lib/vtls/vtls.c
@@ -1413,47 +1413,71 @@ CURLsslset Curl_init_sslset_nolock(curl_sslbackend id, const char *name,
 
 #ifdef USE_SSL
 
+static void free_hostname(struct ssl_connect_data *connssl)
+{
+  if(connssl->dispname != connssl->hostname)
+    free(connssl->dispname);
+  free(connssl->hostname);
+  connssl->hostname = connssl->dispname = NULL;
+}
+
 static void cf_close(struct Curl_cfilter *cf, struct Curl_easy *data)
 {
   struct ssl_connect_data *connssl = cf->ctx;
-  /* TODO: close_one closes BOTH conn->ssl AND conn->proxy_ssl for this
-   * sockindex (if in use). Gladly, it is safe to call more than once. */
   if(connssl) {
     Curl_ssl->close(cf, data);
     connssl->state = ssl_connection_none;
+    free_hostname(connssl);
   }
   cf->connected = FALSE;
 }
 
-static void reinit_hostname(struct Curl_cfilter *cf)
+static CURLcode reinit_hostname(struct Curl_cfilter *cf)
 {
   struct ssl_connect_data *connssl = cf->ctx;
+  const char *ehostname, *edispname;
+  int eport;
 
+  /* We need the hostname for SNI negotiation. Once handshaked, this
+   * remains the SNI hostname for the TLS connection. But when the
+   * connection is reused, the settings in cf->conn might change.
+   * So we keep a copy of the hostname we use for SNI.
+   */
 #ifndef CURL_DISABLE_PROXY
   if(Curl_ssl_cf_is_proxy(cf)) {
-    /* TODO: there is not definition for a proxy setup on a secondary conn */
-    connssl->hostname = cf->conn->http_proxy.host.name;
-    connssl->dispname = cf->conn->http_proxy.host.dispname;
-    connssl->port = cf->conn->http_proxy.port;
+    ehostname = cf->conn->http_proxy.host.name;
+    edispname = cf->conn->http_proxy.host.dispname;
+    eport = cf->conn->http_proxy.port;
   }
   else
 #endif
   {
-    /* TODO: secondaryhostname is set to the IP address we connect to
-     * in the FTP handler, it is assumed that host verification uses the
-     * hostname from FIRSTSOCKET */
-    if(cf->sockindex == SECONDARYSOCKET && 0) {
-      connssl->hostname = cf->conn->secondaryhostname;
-      connssl->dispname = connssl->hostname;
-      connssl->port = cf->conn->secondary_port;
+    ehostname = cf->conn->host.name;
+    edispname = cf->conn->host.dispname;
+    eport = cf->conn->remote_port;
+  }
+
+  /* change if ehostname changed */
+  if(ehostname && (!connssl->hostname
+                   || strcmp(ehostname, connssl->hostname))) {
+    free_hostname(connssl);
+    connssl->hostname = strdup(ehostname);
+    if(!connssl->hostname) {
+      free_hostname(connssl);
+      return CURLE_OUT_OF_MEMORY;
     }
+    if(!edispname || !strcmp(ehostname, edispname))
+      connssl->dispname = connssl->hostname;
     else {
-      connssl->hostname = cf->conn->host.name;
-      connssl->dispname = cf->conn->host.dispname;
-      connssl->port = cf->conn->remote_port;
+      connssl->dispname = strdup(edispname);
+      if(!connssl->dispname) {
+        free_hostname(connssl);
+        return CURLE_OUT_OF_MEMORY;
+      }
     }
   }
-  DEBUGASSERT(connssl->hostname);
+  connssl->port = eport;
+  return CURLE_OK;
 }
 
 static void ssl_cf_destroy(struct Curl_cfilter *cf, struct Curl_easy *data)
@@ -1496,10 +1520,10 @@ static CURLcode ssl_cf_connect(struct Curl_cfilter *cf,
   if(result || !*done)
     goto out;
 
-  /* TODO: right now we do not fully control when hostname is set,
-   * assign it on each connect call. */
-  reinit_hostname(cf);
   *done = FALSE;
+  result = reinit_hostname(cf);
+  if(result)
+    goto out;
 
   if(blocking) {
     result = ssl_connect(cf, data);
diff --git a/lib/vtls/vtls_int.h b/lib/vtls/vtls_int.h
index bb19a0c77..1d5f415d4 100644
--- a/lib/vtls/vtls_int.h
+++ b/lib/vtls/vtls_int.h
@@ -33,8 +33,8 @@
 struct ssl_connect_data {
   ssl_connection_state state;
   ssl_connect_state connecting_state;
-  const char *hostname;             /* hostnaem for verification */
-  const char *dispname;             /* display version of hostname */
+  char *hostname;                   /* hostname for verification */
+  char *dispname;                   /* display version of hostname */
   int port;                         /* remote port at origin */
   struct ssl_backend_data *backend; /* vtls backend specific props */
   struct Curl_easy *call_data;      /* data handle used in current call,