4ff212f8ed — curl

Subject

url: connection reuse fixes for starttls

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

docs/libcurl/libcurl-env-dbg.md +5 -0 lib/ftp.c +16 -8 lib/url.c +3 -6 src/config2setopts.c +7 -0 tests/http/test_31_vsftpds.py +24 -0

docs/libcurl/libcurl-env-dbg.md+5 -0

@@ -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.

lib/ftp.c+16 -8

@@ -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;
	}

lib/url.c+3 -6

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

src/config2setopts.c+7 -0

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

tests/http/test_31_vsftpds.py+24 -0

@@ -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):