Connections not reused when cipher list specified

[sfc-gh-emusser]

I did this

Since 7.52.0, with libcurl built with nss, if a cipher list is specified with multiple entries on subsequent requests such as:
curl_easy_setopt(curl, CURLOPT_SSL_CIPHER_LIST, "ecdhe_ecdsa_aes_128_gcm_sha_256,ecdhe_rsa_aes_128_gcm_sha_256,dhe_rsa_aes_128_gcm_sha_256,rsa_aes_128_gcm_sha_25");

we see a new https connection created on each request as opposed to reusing the existing connection. This broke with commit
cb4e2be proxy: Support HTTPS proxy and SOCKS+HTTP(s)

The issue is that the method set_ciphers (in lib/vtls/nss.c) clobbers the cipher_list entry kept on each connection by replacing any commas with null characters. After this method is called the cipher_list on a connection will now only contain the first entry (in the above example: ecdhe_ecdsa_aes_128_gcm_sha_256) and it is not necessarily the cipher negotiated for the connection.

The above is an issue in ConnectionExists and Curl_ssl_config_matches as it expects the cipher_list of a new request to exactly match the cipher_list found on an existing connection in order for that connection to be reused.

A relatively simple fix is to call strdup on the cipher_list before passing to set_ciphers (I can push this for review if desired). But looking at the original commit that broke this it appears that we may be inadvertently sharing cipher_list pointers -- perhaps further investigation is needed.

I expected the following

I expected a subsequent request with the same cipher_list (and other properties) to reuse the existing connection.

curl/libcurl version

libcurl 7.80.0

operating system

Linux xx.xx.xx.xx 4.19.84-33.70.amzn2.x86_64 #1 SMP Sun Nov 17 03:35:39 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

[bagder]

I prefer to avoid extra memory management. See #8161