openssl: make the requested TLS version the *minimum* wanted #2694
Conversation
The code treated the set version as the *exact* version to require in the TLS handshake, which is not what other TLS backends do and probably not what most people expect either. Reported-by: Andreas Olsson Fixes #2691
lib/vtls/openssl.c
Outdated
| @@ -2113,6 +2109,7 @@ set_ssl_version_min_max(long *ctx_options, struct connectdata *conn, | |||
| #endif | |||
| /* FALLTHROUGH */ | |||
| case CURL_SSLVERSION_TLSv1_0: | |||
| case CURL_SSLVERSION_TLSv1: | |||
There was a problem hiding this comment.
Adding this case here has the same effect as the existing code at line number 2336. Should that line be removed?
There was a problem hiding this comment.
Thanks, good catch. It seems wrong to have this duplicated, I'll see what I should cleanup here...
|
It looks like setting CURLOPT_SSLVERSION to CURL_SSLVERSION_SSLv3 or CURL_SSLVERSION_SSLv2 will result in using that exact version of the protocol and not set that as a minimum. Is that intentional? |
|
@malhotrag I suppose not, but since they are so old and not used by default I feel less sure about changing how they work... A default OpenSSL build doesn't even support them at all these days. |
|
It's not directly related to this change, but I happened to notice while reviewing that the ssl_authtype == CURL_TLSAUTH_SRP check at line 2318 will never be satisfied because of the earlier check at line 2223. |
|
With this change, we seem to have lost the difference in behavior between CURL_SSLVERSION_TLSv1 and CURL_SSLVERSION_TLSv1_0. They appear to be equivalent now. Before this change, specifying CURL_SSLVERSION_TLSv1 without a *VERSION_MAX implied 1.0 or 1.1 or 1.2, while, specifying CURL_SSLVERSION_TLSv1_0 without a *VERSION_MAX implied 1.0 only. I think the CURLOPT_SSLVERSION documentation can be improved to convey the current behavior more accurately. |
Can you submit this as a separate issue so that we don't lose it? |
Feedback-by: Gaurav Malhotra
The code treated the set version as the exact version to require in
the TLS handshake, which is not what other TLS backends do and probably
not what most people expect either.
Reported-by: Andreas Olsson
Fixes #2691