boostorg / redis

An async redis client designed for performance and scalability
https://www.boost.org/doc/libs/develop/libs/redis/doc/html/index.html
Boost Software License 1.0
212 stars 38 forks source link

No way to specify CA/client certificate for SSL connections. #168

Open grapland0 opened 7 months ago

grapland0 commented 7 months ago

It always constuct a system-default ssl context which uses system common CA set and no client certificate (for mutual authentation at server side).

criatura2 commented 7 months ago

You can pass the context method to the connection constructor: https://github.com/boostorg/redis/blob/7caea928affde85146b738eb456d4772a410f721/include/boost/redis/connection.hpp#L346

grapland0 commented 7 months ago

It is just the method. Can I pass the whole context in? I need to specify CA certificate, as well as client cert/key for mutual authentication.

criatura2 commented 7 months ago

At the moment you can't pass the context. I agree that passing the whole context would have been more flexible, so I might change that in a future release. But I fail to see why you can't first pass the method and then set whatever you have to set. There is only one non-trivial constructor in ssl::context, which is the one you will be using anyway https://www.boost.org/doc/libs/1_83_0/doc/html/boost_asio/reference/ssl__context/context.html

grapland0 commented 7 months ago

Is there any API I can use to directly access the SSL context of current connection from boost::redis?

The get_ssl_context doesn't exist in executor-erased connection (https://github.com/boostorg/redis/blob/7caea928affde85146b738eb456d4772a410f721/include/boost/redis/connection.hpp#L337C7-L337C17)

mzimbres commented 7 months ago

Yeah it is missing. I will add one.

grapland0 commented 7 months ago

I think just adding a constructor with ssl context passed in is sufficient, like Beast's secure websocket. Letting end users play with existing ssl context may interfere with ongoing async ops (or you'll need to document when they can/cannot make change safely)

mzimbres commented 7 months ago

Did not have the time to look at this yet. Perhaps in the forthcoming weekends.

mzimbres commented 6 months ago

@grapland0 It was not my intention to close the ticket, so please reopen if necessary. The PR adds the getters you were missing to the ssl-context. I am still unsure whether and how I will allow passing a custom ssl context to the connection. This is what Boost.MySql does (from a conversation with @anarthal)

The way mysql solves it (on the newer any_connection)

If you want a custom SSL context, you pass a referene

If you don't need a custom context but need TLS, you pass nullptr, and you receive a singleton reference with appropriate values

If you don't need a context because you're not using TLS, you pass nullptr and nothing gets ever created

If you're using a connection pool, SSL contexts are passed by value. A single context object is shared between all connections

SSL contexts are unique pointers to openssl SSL_CTX objects. It's not documented how heavyweight they are, but they have an internal refcount mechanism, so they introduce some overhead, at least.

I understand that using custom TLS connections without using a connection pool is something advanced and not too frequent

anarthal commented 6 months ago

Please note that it is UB to set properties of SSL contexts once a ssl::stream is created from them. If connection creates internally a ssl::stream when constructed, the ssl::context must not be modified after construction.

mzimbres commented 6 months ago

Fix is merged now. Please have a look.