url: connection reuse fixes for starttls

Add test_31_13 to check connection reuse on mixed --ssl-reqd setting.
For that add debug env var CURL_DBG_NO_USE_SSL_ON_FIRST to disable
--ssl-reqd for the first url. Check that the connection without SSL
from the first url is not reused on the second URL that requires it.

Tweak special ftp: protocol check to fail a DEBUGASSERT on mismatched
`use_ssl` settings as that should have been caught before in the
connection reuse matching (imap/smtp etc. do not have this extra check
and rely on the general part doing its job).

Closes #21665

diff --git a/docs/libcurl/libcurl-env-dbg.md b/docs/libcurl/libcurl-env-dbg.md
index 9fa9c069d1f99d61c2776f2fa98211b8229a0c72..70d0f12c88bdc776af1041ee3444c49d9fd050db 100644 (file)
--- a/docs/libcurl/libcurl-env-dbg.md
+++ b/docs/libcurl/libcurl-env-dbg.md
@@ -199,3 +199,8 @@ Make `curl` use the quick exit option, even when built in debug mode.
 
 When happy eyeballing for https: wait for the HTTPS-RR resolve
 answer to arrive before starting any connect attempt.
+
+## `CURL_DBG_NO_USE_SSL_ON_FIRST`
+
+When passing `--ssl-reqd`, clear it for the first URL in a curl command.
+This allows testing of connection reuse in mixed `STARTTLS` needs.

diff --git a/lib/ftp.c b/lib/ftp.c
index 3723e7e96817e698589218fa649c2989033b4490..5a37856fd6fb74f0c20cb682d507c684557f5be5 100644 (file)
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -3115,7 +3115,7 @@ static CURLcode ftp_wait_resp(struct Curl_easy *data,
   CURLcode result = CURLE_OK;
   if(ftpcode == 230) {
     /* 230 User logged in - already! Take as 220 if TLS required. */
-    if(data->set.use_ssl <= CURLUSESSL_TRY ||
+    if(ftpc->use_ssl <= CURLUSESSL_TRY ||
        Curl_conn_is_ssl(conn, FIRSTSOCKET))
       return ftp_state_user_resp(data, ftpc, ftpcode);
   }
@@ -3125,7 +3125,7 @@ static CURLcode ftp_wait_resp(struct Curl_easy *data,
     return CURLE_WEIRD_SERVER_REPLY;
   }
 
-  if(data->set.use_ssl && !Curl_conn_is_ssl(conn, FIRSTSOCKET)) {
+  if(ftpc->use_ssl && !Curl_conn_is_ssl(conn, FIRSTSOCKET)) {
     /* We do not have an SSL/TLS control connection yet, but FTPS is
        requested. Try an FTPS connection now */
 
@@ -3218,7 +3218,7 @@ static CURLcode ftp_pp_statemachine(struct Curl_easy *data,
       /* remain in this same state */
     }
     else {
-      if(data->set.use_ssl > CURLUSESSL_TRY)
+      if(ftpc->use_ssl > CURLUSESSL_TRY)
         /* we failed and CURLUSESSL_CONTROL or CURLUSESSL_ALL is set */
         result = CURLE_USE_SSL_FAILED;
       else
@@ -3239,7 +3239,7 @@ static CURLcode ftp_pp_statemachine(struct Curl_easy *data,
   case FTP_PBSZ:
     result =
       Curl_pp_sendf(data, &ftpc->pp, "PROT %c",
-                    data->set.use_ssl == CURLUSESSL_CONTROL ? 'C' : 'P');
+                    ftpc->use_ssl == CURLUSESSL_CONTROL ? 'C' : 'P');
     if(!result)
       ftp_state(data, ftpc, FTP_PROT);
     break;
@@ -3247,10 +3247,10 @@ static CURLcode ftp_pp_statemachine(struct Curl_easy *data,
   case FTP_PROT:
     if(ftpcode / 100 == 2)
       /* We have enabled SSL for the data connection! */
-      conn->bits.ftp_use_data_ssl = (data->set.use_ssl != CURLUSESSL_CONTROL);
+      conn->bits.ftp_use_data_ssl = (ftpc->use_ssl != CURLUSESSL_CONTROL);
     /* FTP servers typically responds with 500 if they decide to reject
        our 'P' request */
-    else if(data->set.use_ssl > CURLUSESSL_CONTROL)
+    else if(ftpc->use_ssl > CURLUSESSL_CONTROL)
       /* we failed and bails out */
       return CURLE_USE_SSL_FAILED;
 
@@ -4453,14 +4453,22 @@ bool ftp_conns_match(struct connectdata *needle, struct connectdata *conn)
 {
   struct ftp_conn *nftpc = Curl_conn_meta_get(needle, CURL_META_FTP_CONN);
   struct ftp_conn *cftpc = Curl_conn_meta_get(conn, CURL_META_FTP_CONN);
-  /* Also match ACCOUNT, ALTERNATIVE-TO-USER, USE_SSL and CCC options */
+  /* Also match ACCOUNT, ALTERNATIVE-TO-USER and CCC options */
   if(!nftpc || !cftpc ||
      Curl_timestrcmp(nftpc->account, cftpc->account) ||
      Curl_timestrcmp(nftpc->alternative_to_user,
                      cftpc->alternative_to_user) ||
-     (nftpc->use_ssl != cftpc->use_ssl) ||
      (nftpc->ccc != cftpc->ccc))
     return FALSE;
+  /* A mismatch on `use_ssl` MUST have been found in connection matching
+   * before we come here. This is a check on MAYBE/MUST use of STARTTLS and
+   * it only works on ftp. But imap/smtp etc have the same `use_ssl` and
+   * no extra match like ftp. We lack tests in this area, so let ftp fail
+   * loudly here to help other cases. */
+  if(nftpc->use_ssl > cftpc->use_ssl) {
+    DEBUGASSERT(0);
+    return FALSE;
+  }
   return TRUE;
 }
 

diff --git a/lib/url.c b/lib/url.c
index b7ba30fe2bb1e49a885f6b3e4d339753a3ce4784..868767a772599e3f0172c5ac8482687b1de24957 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -974,12 +974,6 @@ static bool url_match_destination(struct connectdata *conn,
        m->needle->scheme->protocol) {
       return FALSE;
     }
-    if(!url_match_ssl_use(conn, m)) {
-      DEBUGF(infof(m->data, "Connection #%" FMT_OFF_T
-                   " has compatible protocol family, but no SSL, no match",
-                   conn->connection_id));
-      return FALSE;
-    }
   }
   /* Scheme mismatch is acceptable, just compare hostname/port */
   return Curl_peer_same_destination(m->needle->origin, conn->origin);
@@ -1141,6 +1135,9 @@ static bool url_match_conn(struct connectdata *conn, void *userdata)
   if(!url_match_multiplex_needs(conn, m))
     return FALSE;
 
+  if(!url_match_ssl_use(conn, m))
+    return FALSE;
+
   if(!url_match_proxy_use(conn, m))
     return FALSE;
   if(!url_match_ssl_config(conn, m))

diff --git a/src/config2setopts.c b/src/config2setopts.c
index 9138b3b147423c71e331a741b86338ffc644832f..06c5dc6bb960d40746f7c3c41af20b0ae91599bd 100644 (file)
--- a/src/config2setopts.c
+++ b/src/config2setopts.c
@@ -951,6 +951,13 @@ CURLcode config2setopts(struct OperationConfig *config,
       result = ssl_setopts(config, curl);
     if(setopt_bad(result))
       return result;
+#ifdef DEBUGBUILD
+    if(!per->urlnum) {
+      char *env = getenv("CURL_DBG_NO_USE_SSL_ON_FIRST");
+      if(env)
+        my_setopt_enum(curl, CURLOPT_USE_SSL, CURLUSESSL_NONE);
+    }
+#endif
   }
 
   if(config->path_as_is)

diff --git a/tests/http/test_31_vsftpds.py b/tests/http/test_31_vsftpds.py
index f36bb1715a46d9affec09981606252ebd850c288..688d157f8f0233094035802fc4e57ac3bf76ac99 100644 (file)
--- a/tests/http/test_31_vsftpds.py
+++ b/tests/http/test_31_vsftpds.py
@@ -270,6 +270,30 @@ class TestVsFTPD:
         dstfile = os.path.join(vsftpds.docs_dir, docname)
         assert os.path.exists(dstfile), f'{r.dump_logs()}'
 
+    # connection reuse with STARTTLS required
+    # 1st download without STARTTLS, 2nd with --ssl-reqd
+    @pytest.mark.skipif(condition=not Env.curl_is_debug(), reason="needs curl debug")
+    def test_31_13_starttls_reuse(self, env: Env, vsftpds: VsFTPD):
+        run_env = os.environ.copy()
+        run_env['CURL_DBG_NO_USE_SSL_ON_FIRST'] = '1'
+        curl = CurlClient(env=env, run_env=run_env)
+        url1 = f'ftp://{env.ftp_domain}:{vsftpds.port}/data-1k'
+        url2 = f'ftp://{env.ftp_domain}:{vsftpds.port}/data-10k'
+        r = curl.run_direct(with_stats=True, args=[
+            '-svv', '--resolve', f'{env.ftp_domain}:{vsftpds.port}:127.0.0.1',
+            '--cacert', env.ca.cert_file,
+            url1, '--out-null',
+            url2, '--out-null', '--ssl-reqd'
+        ])
+        r.check_exit_code(0)
+        r.check_stats(count=2, http_status=226)
+        # expect 4 connections to have been made:
+        # 1. 1st CONTROL without STARTTLS
+        # 2. 1st DATA for download
+        # 3. 2nd CONTROL with STARTTLS (not reuse of 1)
+        # 4. 2nd DATA for download
+        assert r.total_connects == 4, f'{r.dump_logs()}'
+
     def check_downloads(self, client, srcfile: str, count: int,
                         complete: bool = True):
         for i in range(count):