mirror of
https://github.com/curl/curl.git
synced 2026-06-02 05:14:16 +03:00
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
This commit is contained in:
parent
f1a6f190a6
commit
4ff212f8ed
5 changed files with 55 additions and 14 deletions
|
|
@ -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.
|
||||
|
|
|
|||
24
lib/ftp.c
24
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;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue