redis-rb/redis-client

`ssl_params:` doesn't appear to support `min_version`

geoffharcourt opened this issue · 6 comments

While trying to programmatically enforce a minimum TLS version for our Sidekiq Redis connections, I ran into an issue that I think would need to be handled in redis-client.

If you pass a min_version parameter into ssl_params, an attempt to connect responds with an `unexpected eof while reading (Redis::CannotConnectError).

Here's what my code I used to check against a TLS-supporting (v1.2 or 1.3) Redis instance looked like:

# works
client = RedisClient.new(
  url: url, 
  ssl_params: { verify_mode: OpenSSL::SSL::VERIFY_PEER },
)
client.call("EXISTS", "hi") # works

# blows up
client = RedisClient.new(
  url: url, 
  ssl_params: { verify_mode: OpenSSL::SSL::VERIFY_PEER, min_version: OpenSSL::SSL::TLS1_3_VERSION },
)
client.call("EXISTS", "hi") # raises

The exception looks like this:

main:0> client.call("EXISTS", "hi")
/bundle/ruby/3.2.0/gems/redis-client-0.19.1/lib/redis_client/ruby_connection.rb:131:in `connect_nonblock': SSL_connect returned=1 errno=0 peeraddr=aa.bb.cc.dd:6379 state=error: unexpected eof while reading (RedisClient::CannotConnectError)

Passing this parameter works successfully with redis-rb's SSL setup. It wasn't at all clear from the code where the SSL context is built and then later passed on to OpenSSL::SSL::SSLSocket why this wouldn't be working, but unless I'm missing something I think this prevents setting a minimum SSL/TLS version for connections.

Thanks for all your work on the gem.

byroot commented

The handling of ssl_params happens here:

def ssl_context(ssl_params)
params = ssl_params.dup || {}
cert = params[:cert]
if cert.is_a?(String)
cert = File.read(cert) if File.exist?(cert)
params[:cert] = OpenSSL::X509::Certificate.new(cert)
end
key = params[:key]
if key.is_a?(String)
key = File.read(key) if File.exist?(key)
params[:key] = OpenSSL::PKey.read(key)
end
context = OpenSSL::SSL::SSLContext.new
context.set_params(params)
if context.verify_mode != OpenSSL::SSL::VERIFY_NONE
if context.respond_to?(:verify_hostname) # Missing on JRuby
context.verify_hostname
end
end
context
end

Passing this parameter works successfully with redis-rb's SSL setup.

By that you mean redis-rb 4.x? If so the code is pretty much the same:

https://github.com/redis/redis-rb/blob/0afcf1cad6f4fc74213f2b4c7a20819581c36897/lib/redis/connection/ruby.rb#L249

byroot commented

Also unexpected eof while reading sounds like redis/redis-rb#1106

What's the redis-server version? Older Redis would close the socket without shutting down the SSL session which would cause this issue (not saying it's your issue, but this may be hiding the real root cause)

Hi @byroot we're connecting to AWS Elasticache, it's configured as Redis 7.0.7.

This is interesting, I didn't realize the redis gem now uses redis-client! In my testing last night I used the same ssl_params: arguments with RedisClient and Redis v5.0.8 and was able to make calls with Redis 5.0.8 successfully but not with RedisClient 0.19.1.

byroot commented

was able to make calls with Redis 5.0.8 successfully but not with RedisClient 0.19.1.

That very surprising. I'd recommend editing redis-client to see what ssl params it receive from redis-rb, but there is little to no transformation IIRC.

I got something crossed up, apologies for using your time here.

byroot commented

Happens to the best of us 😄