Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] apply new TLS settings after cert renewal #552

Open
lisay-yan opened this issue Mar 25, 2024 · 6 comments
Open

[FEATURE] apply new TLS settings after cert renewal #552

lisay-yan opened this issue Mar 25, 2024 · 6 comments

Comments

@lisay-yan
Copy link

Is your feature request related to a problem? Please describe.
Once the cert installed at redis server expired, it will lead to handshake failed.
Is it possible support new refreshed cert load to redis++ cluster client.

Describe the solution you'd like
Maybe set new values to ConnectionOptions , and redis++ client open a API to apply new TLS config changes.

Describe alternatives you've considered
If can't apply refreshed certs into Redis++, I have to re-instantiate redis++ client.
That maybe high cost compared with load new certs solution.

Additional context
If that is possible and doable to apply new TLS certs to Redis++, how about passwd for auth?
connection_options.password = "auth";

I prefer Redis++ can open interface to update initial settings, such as Certs and Passwd. Thanks.

@lisay-yan lisay-yan changed the title apply new TLS settings after cert renewal [Feature]apply new TLS settings after cert renewal Mar 25, 2024
@lisay-yan lisay-yan changed the title [Feature]apply new TLS settings after cert renewal [FEATURE] apply new TLS settings after cert renewal Mar 25, 2024
@sewenew
Copy link
Owner

sewenew commented Mar 25, 2024

redis-plus-plus is based on hiredis. It seems that hiredis does not support any API to update certs settings. So this feature cannot achieved so far.

We can achieve updating password without creating a new connection. However, normally, you seldom modify the password. And the cost should be ignorable. I'll consider make this feature in TODO list, although the priority is low. Thanks for your suggestion!

Regards

@lisay-yan
Copy link
Author

@sewenew

I understand hiredis doesn't support update certs for existing connection.
That is also not necessary since handshake phase has passed.

While, if TLS server decide FIN these connections, then redis++ reconnect should use latest cert rather than old ones for new connection.
So, That should be doable if redis++ can open interface to upload local save certs.
New certs only impact new connection. And keep existing connection should be fine. Agree?

@lisay-yan
Copy link
Author

@sewenew

BTW, another point, hiredis open below config for redisCreateSSLContext to gain SSL context.

opts.tls.cert = "/path/to/client/certificate"; // Optional
opts.tls.key = "/path/to/private/key/file"; // Optional
opts.tls.cacert = "/path/to/CA/certificate/file"; // You can also set opts.tls.cacertdir instead.
opts.tls.sni = "server-name-indication"; // Optional_

But, sometime, maybe need more configurable options, such as TLS protocol, maybe only TLS1.3 , or cipher list settings etc.

I do a simple trial via prepare SSL_CTX and load to hiredis, we can achieve more control for TLS.
For example:

SSL_CTX *ssl_ctx = SSL_CTX_new(SSLv23_client_method());
//Besides cert load config, we also can extra set more flags.
//TLS1.3 only
SSL_CTX_set_options(ssl_ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1 | SSL_OP_NO_TLSv1_2);
//Ciphers
SSL_CTX_set_cipher_list(ssl_ctx, "AES128-SHA:AES256-SHA:DES-CBC3-SHA")

SSL *ssl = SSL_new(ssl_ctx);

//Apply to hiredis API via SSL.
redisInitiateSSL(&(context->c), ssl);

Is it reasonable to extend Redis++ TLS ability?
Thanks for support and consider my suggestion in advance.

@sewenew
Copy link
Owner

sewenew commented Mar 31, 2024

New certs only impact new connection. And keep existing connection should be fine. Agree?

This might not be a good idea. In this case, connections in the underlying connection pool have different setting. If something bad happens, it might be hard to debug the problem, e.g. some connection works, while others not.

Is it reasonable to extend Redis++ TLS ability?

redisInitiateSSL, in fact, calls redisSSLConnect to do connect. So this API does not update SSL settings, instead, it creates a new connection with new settings. So if you need to update SSL settings, it's no better than creating a new Redis object with new settings.

Regards

@lisay-yan
Copy link
Author

@sewenew

Use redisInitiateSSL to set hiredis TLS with more SSL config is recommended by hiredis code officer.
If update hiredis local structure is not a issue.

The overall behavior can achieve new TLS connection use renewed certs, and no impact for existing established ones.

If stop existing client, and new another redis++ client, it is high cost, and all connection down in spite of TLS server maybe OK to accept.
A key point is for application in service upgrade, it is totally broken.

Appreciate Redis++ can open a general interface to allow user to update cert changes, auth passwd changes etc.
For sure, you can publish potential risk or issue together, and let user to make choice.

@sewenew
Copy link
Owner

sewenew commented Apr 8, 2024

I'm not familiar with TLS stuff. But do you mean that a Redis server can serve with multiple different certificate files?

Also, I don't think it's a frequent operation to update certificates. So you seldom need to renew all connections, and that should not be big problem. Right?

Regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants