From 4ff212f8ed328fb95261e11add8f4d2f0616895a Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Tue, 19 May 2026 10:57:53 +0200 Subject: [PATCH] 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 --- docs/libcurl/libcurl-env-dbg.md | 5 +++++ lib/ftp.c | 24 ++++++++++++++++-------- lib/url.c | 9 +++------ src/config2setopts.c | 7 +++++++ tests/http/test_31_vsftpds.py | 24 ++++++++++++++++++++++++ 5 files changed, 55 insertions(+), 14 deletions(-) diff --git a/docs/libcurl/libcurl-env-dbg.md b/docs/libcurl/libcurl-env-dbg.md index 9fa9c069d1..70d0f12c88 100644 --- 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 3723e7e968..5a37856fd6 100644 --- 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 b7ba30fe2b..868767a772 100644 --- 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 9138b3b147..06c5dc6bb9 100644 --- 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 f36bb1715a..688d157f8f 100644 --- 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):