You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Per my comment in #259, OpenSSL::SSL::SSLContext defaults the ssl_version to SSLv23. Contrary to its non-intuitive name, this setting will attempt TLS first, and will fallback to SSLv3 if TLS is not supported. It is a fairly sane default. See https://www.openssl.org/docs/ssl/SSL_CTX_new.html#item_SSLv23_method
Hardcoding the SSL/TLS version makes the code brittle, and it may require a code change if another vulnerability similar to POODLE arises for the ssl_version that is hardcoded in the gem.
Additionally, the current method of setting the ssl_version on the SSLContext object is incorrect, as each call to SSLContext#set_params overwrites any settings that have a default, such as ssl_version, see https://github.com/ruby/ruby/blob/trunk/ext/openssl/lib/openssl/ssl.rb#L86. The second call to SSLContext#set_params where verify_mode is set (
Per my comment in #259,
OpenSSL::SSL::SSLContext
defaults thessl_version
toSSLv23
. Contrary to its non-intuitive name, this setting will attempt TLS first, and will fallback to SSLv3 if TLS is not supported. It is a fairly sane default. See https://www.openssl.org/docs/ssl/SSL_CTX_new.html#item_SSLv23_methodHardcoding the SSL/TLS version makes the code brittle, and it may require a code change if another vulnerability similar to POODLE arises for the ssl_version that is hardcoded in the gem.
Additionally, the current method of setting the
ssl_version
on theSSLContext
object is incorrect, as each call toSSLContext#set_params
overwrites any settings that have a default, such asssl_version
, see https://github.com/ruby/ruby/blob/trunk/ext/openssl/lib/openssl/ssl.rb#L86. The second call toSSLContext#set_params
whereverify_mode
is set (bunny/lib/bunny/transport.rb
Line 397 in f1a12fc
ssl_version
. I have a failing test that demonstrates this.This is a followup to #258
The text was updated successfully, but these errors were encountered: