- Notifications
You must be signed in to change notification settings - Fork425
Add HTTP client options for SSL/TLS methods#530
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| always_verify_peer_(always_verify_peer) {} | ||
| void boost::network::http::impl::ssl_delegate::connect( | ||
| asio::ip::tcp::endpoint &endpoint, std::string host, | ||
| function<void(system::error_code const &)> handler) { | ||
| context_.reset( | ||
| new asio::ssl::context(service_, asio::ssl::context::sslv23_client)); | ||
| new asio::ssl::context(service_, asio::ssl::context::tlsv1_client)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please do not change the default from SSLv23 to TLSv1. This will be a silent breaking change and I'm not convinced this is the best default for users of the library anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Testing with Apple's provided OpenSSL 0.9.8 the::SSLv23_method() should use the options to decide what type of protocol it negotiates. To disable SSL3 you could use thesslv23_client method and:options.openssl_options(SSL_OP_NO_SSLv3 | SSL_OP_NO_SSLv2 | SSL_OP_ALL);. However, I'm still seeing an SSLv3 client hello. On newer versions of openssl this works as expected and thesslv23_client with theSSL_OP_NO_SSLv3 option forces the client to send a TLSv1 client hello.
I'll give this more thought, but usingtlsv1_client forces Apple's OpenSSL to TLSv1 client hello (and negotiate it's best supported TLS1.0) while other newer OpenSSLs will do the same and negotiate TLS1.1/TLS1.2 if supported by the server. This is the best possible scenario without considering supporting older clients.
Thoughts on a less-silent but still-default breaking change?
I actually liked the idea of pushing through the version explicitly down the code path instead of relying on environment variables. I'm skeptical of the value of setting TLSv1 as the default, because from what I've read so far that's less secure than using newer versions of SSL. What does ASIO do though in these cases, doesn't it need both TLS and SSL implementations available from the platform? |
Cool, so@deanberris, we can keep SSLv3 support for those in dire straights to support (god speed to them). And we don't need to change or provide control over the ssl_method. With ssl_options + ciphers we allow a modern/restricted/flexible/secure TLS1.0+ negotiation. Using: options.openssl_options(SSL_OP_NO_SSLv3 | SSL_OP_NO_SSLv2 | SSL_OP_ALL); We can force an OpenSSL 0.9.8-client (such as OS X's) to send a TLSv1 client hello only! Which is perfect, and then: options.openssl_ciphers("ECDH+AESGCM:DH+AESGCM:ECDH+AES256:DH+AES256:ECDH+AES128:DH+AES:ECDH+3DES:""DH+3DES:RSA+AESGCM:RSA+AES:RSA+3DES:!aNULL:!MD5"); We can restrict that same TLS1.0 client to an "acceptable" set of suites: ["TLS_DHE_RSA_WITH_AES_256_CBC_SHA","TLS_DHE_DSS_WITH_AES_256_CBC_SHA","TLS_DHE_RSA_WITH_AES_128_CBC_SHA","TLS_DHE_DSS_WITH_AES_128_CBC_SHA","TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA","TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA","TLS_RSA_WITH_AES_256_CBC_SHA","TLS_RSA_WITH_AES_128_CBC_SHA","TLS_RSA_WITH_3DES_EDE_CBC_SHA"] Callsites building on platforms with modern OpenSSL versions can add: #if defined(SSL_TXT_TLSV1_2) ciphers +=":!SHA";// or ":!SHA:!CBC"#endif |
| @@ -28,6 +33,12 @@ void boost::network::http::impl::ssl_delegate::connect( | |||
| function<void(system::error_code const &)> handler) { | |||
| context_.reset( | |||
| new asio::ssl::context(service_, asio::ssl::context::sslv23_client)); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Also, thissslv23_client eventually uses the OpenSSL:::SSLv23_client_method(), which combined with the recommended:SSL_OP_NO_SSLv3 | SSL_OP_NO_SSLv2 options will negotiate the maximum supported TLS client version. On a base Ubuntu14.04 with OpenSSL 1.0.1f 6 Jan 2014, this implementation, with-DSSL_TXT_TLSV1_2 negotiates TLS1.2 fine.
In our projects we add to CMakeLists.txt
# Make sure the OpenSSL/ssl library has a TLS client method.# Prefer the highest version of TLS, but accept 1.2, 1.1, or 1.0.include(CheckLibraryExists)CHECK_LIBRARY_EXISTS(${OPENSSL_SSL_LIBRARY}"TLSv1_2_client_method"""OPENSSL_TLSV12)CHECK_LIBRARY_EXISTS(${OPENSSL_SSL_LIBRARY}"TLSv1_1_client_method"""OPENSSL_TLSV11)CHECK_LIBRARY_EXISTS(${OPENSSL_SSL_LIBRARY}"TLSv1_client_method"""OPENSSL_TLSV10)# Add a define based on the highest TLS version found. Fatal if no TLS client.if(OPENSSL_TLSV12)add_definitions(-DSSL_TXT_TLSV1_2)elseif(OPENSSL_TLSV11)add_definitions(-DSSL_TXT_TLSV1_1)elseif(OPENSSL_TLSV10)add_definitions(-DSSL_TXT_TLSV1)else()message(FATAL"Cannot find any TLS client methods")endif()
leecoder commentedMay 24, 2015
In my case, some builds had been failed using a prebuilt cpp-netlib. |
LGTM @theopolis Thanks for the changes -- I hope you can send in a follow-up PR changes to the documentation to update the recommendation(s) on using this new options? Then in 0.12 maybe I can be convinced of making the default use TLS1.x where x is >0. |
Add HTTP client options for SSL/TLS methods
@leecoder -- Do you want to make that option something that can be chosen at build-time in the mainline? I'd be happy to merge something like that that's configurable in CMakeLists.txt, then we can flip it to TRUE by default in the next minor version release. |
leecoder commentedMay 25, 2015
@deanberris -- Ok~ I'll create a new PR! Thx. |
Currently the netlib HTTP client only allows SSLv23 clients without control over ciphers or protocol restrictions.
This pushes the ASIO ssl_options and OpenSSL cipher suite selection into the
client_optionsstruct. So the client API may implement something like:I'm unsure if I caught all possible default arguments in the sync/async abstractions/workflows. ;)
Edit/update: This used to push the SSL/TLS method through to ASIO, but options and ciphers are much more flexible!