Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forcing TLS version allows to bypass the OpenSSL OS default #4304

Closed
wants to merge 4 commits into from

Conversation

@cnotin
Copy link
Contributor

commented Sep 8, 2019

Fixes #4298

I changed a few stuffs but I tried to group every block in a single commit.
This is my first attempt to contribute to cURL, and I'm not a regular C developer, so I don't mind (and even expect) a thorough review :)

Here are a few tests:

# ./src/curl -s https://www.howsmyssl.com/a/check | jq '.tls_version'
"TLS 1.3"
# ./src/curl -s --tlsv1.0 https://www.howsmyssl.com/a/check | jq '.tls_version'
"TLS 1.3"

# ./src/curl -s --tlsv1.0 --tls-max 1.0 https://www.howsmyssl.com/a/check | jq '.tls_version'
"TLS 1.0"
# ./src/curl -s --tlsv1.0 --tls-max 1.1 https://www.howsmyssl.com/a/check | jq '.tls_version'
"TLS 1.1"
# ./src/curl -s --tlsv1.0 --tls-max 1.2 https://www.howsmyssl.com/a/check | jq '.tls_version'
"TLS 1.2"
# ./src/curl -s --tlsv1.0 --tls-max 1.3 https://www.howsmyssl.com/a/check | jq '.tls_version'
"TLS 1.3"

# ./src/curl -s --tlsv1.0 --tls-max 1.0 https://www.howsmyssl.com/a/check | jq '.tls_version'
"TLS 1.0"
# ./src/curl -s --tlsv1.1 --tls-max 1.1 https://www.howsmyssl.com/a/check | jq '.tls_version'
"TLS 1.1"
# ./src/curl -s --tlsv1.2 --tls-max 1.2 https://www.howsmyssl.com/a/check | jq '.tls_version'
"TLS 1.2"

And the fix of the original issue, with a server that only accepts TLS1.0 while the OpenSSL default on my system is TLS1.2:

  • Before:
# curl --tlsv1 https://tls-v1-0.badssl.com:1010/ 
curl: (35) error:1425F102:SSL routines:ssl_choose_client_version:unsupported protocol
  • After:
# ./src/curl -s --tlsv1 https://tls-v1-0.badssl.com:1010/  | head -n2
<!DOCTYPE html>
<html>

@cnotin cnotin force-pushed the cnotin:pr-ossl branch from 8879a2b to 979c278 Sep 8, 2019

@cnotin cnotin force-pushed the cnotin:pr-ossl branch from 979c278 to 06d3f5b Sep 8, 2019

@cnotin

This comment has been minimized.

Copy link
Contributor Author

commented Sep 8, 2019

I also would suggest to replace the implementation of set_ssl_version_min_max_legacy by the one from Ruby's openssl:
https://github.com/ruby/openssl/blob/4b43ffc1292eeb70ff886847836e21ad96ed8796/ext/openssl/ossl_ssl.c#L169-L193
It is clearer and more systematic IMO, what do you think? Should I suggest something based on it too?

openssl: modernize set_ssl_version_min_max
OpenSSL 1.1.0 adds SSL_CTX_set_<min|max>_proto_version() that we now use when available.
Existing code is preserved for older versions of OpenSSL.

@cnotin cnotin force-pushed the cnotin:pr-ossl branch from 06d3f5b to 16ec651 Sep 8, 2019

@cnotin

This comment has been minimized.

Copy link
Contributor Author

commented Sep 8, 2019

Regarding #4097:

  • Before, it fails to go down to TLSv1.0:
# curl -v --tlsv1.0 --tls-max 1.0 https://www.cloudflare.com/robots.txt 
[...]
curl: (35) error:141E70BF:SSL routines:tls_construct_client_hello:no protocols available
# curl -V
curl 7.65.3 (x86_64-pc-linux-gnu) libcurl/7.65.3 OpenSSL/1.1.1c zlib/1.2.11 libidn2/2.2.0 libpsl/0.20.2 (+libidn2/2.0.5) libssh2/1.8.0 nghttp2/1.39.2 librtmp/2.3
Release-Date: 2019-07-19
[...]
  • After, it works:
# ./src/curl -v --tlsv1.0 --tls-max 1.0 https://www.cloudflare.com/robots.txt 2>&1 | grep -i "SSL connection\|User-Agent"
* SSL connection using TLSv1.0 / ECDHE-RSA-AES128-SHA
> User-Agent: curl/7.66.0-DEV
User-agent: *
# LD_LIBRARY_PATH=/root/tools/openssl/output/lib ./src/curl -V
curl 7.66.0-DEV (x86_64-pc-linux-gnu) libcurl/7.66.0-DEV OpenSSL/1.0.2t zlib/1.2.11 libidn2/2.2.0
Release-Date: [unreleased]
[...]

With TLSv1.2 as the system default:

# tail -n3 /etc/ssl/openssl.cnf 
[system_default_sect]
MinProtocol = TLSv1.2
CipherString = DEFAULT@SECLEVEL=2

@cnotin cnotin marked this pull request as ready for review Sep 8, 2019

@cnotin

This comment has been minimized.

Copy link
Contributor Author

commented Sep 8, 2019

And with an older (<1.1.0) version of OpenSSL which doesn't have SSL_CTX_set_<min|max>_proto_version():

# LD_LIBRARY_PATH=/root/tools/openssl/output/lib ./src/curl -V
curl 7.66.0-DEV (x86_64-pc-linux-gnu) libcurl/7.66.0-DEV OpenSSL/1.0.2t zlib/1.2.11 libidn2/2.2.0
[...]
# LD_LIBRARY_PATH=/root/tools/openssl/output/lib ./src/curl  -v --tlsv1.0 --tls-max 1.0 https://www.cloudflare.com/robots.txt 2>&1 | grep -i "SSL connection\|User-Agent"
* SSL connection using TLSv1.0 / ECDHE-RSA-AES128-SHA
> User-Agent: curl/7.66.0-DEV
User-agent: *
# LD_LIBRARY_PATH=/root/tools/openssl/output/lib ./src/curl  -v --tlsv1.0 https://www.cloudflare.com/robots.txt 2>&1 | grep -i "openssl\|SSL connection\|User-Agent"
* SSL connection using TLSv1.2 / ECDHE-ECDSA-AES128-GCM-SHA256
> User-Agent: curl/7.66.0-DEV
User-agent: *
@cnotin

This comment has been minimized.

Copy link
Contributor Author

commented Sep 8, 2019

Well looks like the build is failing... I see why in some cases, and I'll address it, but I'm not sure that this PR is responsible of all errors (some I don't even see where the error is)

@bagder

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

The ngtcp2 build failure is totally not your fault.

The libressl build failure looks like it needs a better preprocessor check for when the API exists.

@cnotin

This comment has been minimized.

Copy link
Contributor Author

commented Sep 9, 2019

Thanks! I'll add a #ifdef around the whole function to prevent the "unused function" warning

@bagder bagder closed this in ffe34b7 Sep 10, 2019

@bagder

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

Thanks a lot! I squashed them into two commits before merge (primarily because I wanted to do some minor edits and couldn't really figure out to which commit!).

@cnotin

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2019

You're welcome, thanks for your quick feedback and your overall work on cURL!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.