redis-rb / redis-client

Simple low level client for Redis 6+
MIT License
124 stars 60 forks source link

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

Closed geoffharcourt closed 9 months ago

geoffharcourt commented 9 months ago

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 9 months ago

The handling of ssl_params happens here: https://github.com/redis-rb/redis-client/blob/d1b9284e8baafcea4647e5cabaf3d11e948874ba/lib/redis_client/ruby_connection.rb#L14-L38

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 9 months ago

Also unexpected eof while reading sounds like https://github.com/redis/redis-rb/issues/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)

geoffharcourt commented 9 months ago

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 9 months ago

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.

geoffharcourt commented 9 months ago

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

byroot commented 9 months ago

Happens to the best of us 😄