# Let OpenSSL decide which TLS protocol version is used by default

I use Gem in a Box, a Ruby project that allows to create personal self-hosted gem repositories. Gem in a Box uses httpclient to connect to the gem repository. After reviewing my server's TLS setup in the aftermath of the OpenSSL heartbleed bug, I decided only to allow protocol versions of TLSv1 and higher on my server. A decision quite a few fellow administrators made too, because of security vulnerabilities in SSLv3 and below.

Now after I did this change, Gem in a Box refused to work with the following message:

OpenSSL::SSL::SSLError: SSL_connect returned=1 errno=0 state=SSLv3 read server hello A: sslv3 alert handshake failure


I traced this problem back until I found the source in httpclient. Even though OpenSSL can find the best protocol version to use by itself, you set it only to use SSLv3 by default. I think this is a very bad idea and created a patch which lets OpenSSL do its own decision again by default.

As I am not so familiar with OpenSSL itself and the documentation of the Ruby binding is severely lacking I want to prevent introducing more problems than there were originally. Please review my changes and verify that it is doing what I intended.

It is quite hard to post the actual changes here because they are only meaningful if one knows the context. So now I will try to describe what I did:

httpclient has a class called SSLConfig which seems to produce helper objects to configure OpenSSL sockets. It sets some default configurations in its #initialize method. Originally it had the @ssl_version instance variable set to "SSLv3", When using the library an instance of SSLConfig is created and its #set_context method is called which applies the aforementioned defaults onto an instance of OpenSSL::SSL::SSLContext, which afterwards seems to be used to create OpenSSL sockets.

I now changed to initial value of @ssl_version to the symbol :auto and modified the #set_context method to only set the actual value of @ssl_version to the SSLContext object, if its value is not :auto.

Manual testing so far resulted in the exact effect I wanted to achieve.

Here are some relevant snippets from lib/httpclient/ssl_config.rb:

class SSLConfig

...

# Which TLS protocol version (also called method) will be used. Defaults
# to :auto which means that OpenSSL decides (In my tests this resulted
# with always the highest available protocol being used).
#
# See the content of OpenSSL::SSL::SSLContext::METHODS for a list of
# available versions in your specific Ruby environment.

...

# Creates a SSLConfig.
def initialize(client)
return unless SSLEnabled
@client = client
@cert_store = X509::Store.new
@client_cert = @client_key = @client_ca = nil
@verify_mode = SSL::VERIFY_PEER | SSL::VERIFY_FAIL_IF_NO_PEER_CERT
@verify_depth = nil
@verify_callback = nil
@dest = nil
@timeout = nil
@ssl_version = :auto
@options = defined?(SSL::OP_ALL) ? SSL::OP_ALL | SSL::OP_NO_SSLv2 : nil
# OpenSSL 0.9.8 default: "ALL:!ADH:!LOW:!EXP:!MD5:+SSLv2:@STRENGTH"
@ciphers = "ALL:!aNULL:!eNULL:!SSLv2" # OpenSSL >1.0.0 default
end

...

# interfaces for SSLSocketWrap.
def set_context(ctx) # :nodoc:
# Verification: Use Store#verify_callback instead of SSLContext#verify*?
ctx.cert_store = @cert_store
ctx.verify_mode = @verify_mode
ctx.verify_depth = @verify_depth if @verify_depth
ctx.verify_callback = @verify_callback || method(:default_verify_callback)
# SSL config
ctx.cert = @client_cert
ctx.key = @client_key
ctx.client_ca = @client_ca
ctx.timeout = @timeout
ctx.options = @options
ctx.ciphers = @ciphers
ctx.ssl_version = @ssl_version unless @ssl_version == :auto
end

...

end


Here's what is using the #set_context method in lib/httpclient/session.rb:

...

def create_openssl_socket(socket)
ssl_socket = nil
if OpenSSL::SSL.const_defined?("SSLContext")
ctx = OpenSSL::SSL::SSLContext.new
@context.set_context(ctx)
ssl_socket = OpenSSL::SSL::SSLSocket.new(socket, ctx)
else
ssl_socket = OpenSSL::SSL::SSLSocket.new(socket)
@context.set_context(ssl_socket)
end
ssl_socket
end

...


I've got a few things to say about the code, but having looked at the diff:

-      @ssl_version = "SSLv3"
+      @ssl_version = :auto


and:

-      ctx.ssl_version = @ssl_version
+      ctx.ssl_version = @ssl_version unless @ssl_version == :auto


your code changes seem minimal, and to the point doing exactly what they are supposed to do, and nothing more.

What I think you should add are the tests to show your changes, and make sure no-one will break your code in the future.