dragonflydb / dragonfly

A modern replacement for Redis and Memcached
https://www.dragonflydb.io/
Other
26.07k stars 962 forks source link

Include TLS information into INFO command output #3292

Open moredure opened 4 months ago

moredure commented 4 months ago

Describe the solution you'd like Include server TLS certificate information such as name, expiration date, issue date into server section of INFO output

kostasrim commented 4 months ago

@moredure is this on Redis/Valkey as well ?

romange commented 4 months ago

I do not think so that Valkey displays this. @moredure can you provide more details on how you want to use this feature? is it for automated systems or for manual inspection?

moredure commented 4 months ago

I do not think so that Valkey displays this. @moredure can you provide more details on how you want to use this feature? is it for automated systems or for manual inspection?

Yeap, just for manual inspection, to check that TLS is up to date or have expected common name, etc without using openssl or any advanced tools

moredure commented 4 months ago

@moredure is this on Redis/Valkey as well ?

I've not seen it there

Lakshyadevelops commented 2 months ago

@romange @kostasrim I would like to take this on, I have understood the ServerFamily::Info(). Any leads as to where i can start to get this data ?

romange commented 2 months ago

@moredure what does it mean to have a proper common name?

moredure commented 2 months ago

@moredure what does it mean to have a proper common name?

I've meant to be able to see certificate CN/SAN, etc

romange commented 2 months ago

@Lakshyadevelops the SSL context is initialized inside CreateSslServerCntx (dragonfly_listener.cc) but I do not know how to fetch the certificate details from it - some learnings are required to complete the task.

Lakshyadevelops commented 2 months ago

So I looked into it. Using openssl I can easily get all the info.

My plan is to get the certificate location using GetFlag(FLAGS_tls_cert_file) and then read the certificate to get all the necessary info. All this code will be in server_family::Info(). Another approach could be we store this information at startup but if sometime after the certificate is refreshed then we will be displaying outdated info. I would prefer the former.

I tried first to generate a tls key and certificate for the server but I got E20240930 16:04:48.690333 1654 dragonfly_listener.cc:89] Failed to load TLS key

Is there any guide to generating tls keys correctly? I could only find this https://www.dragonflydb.io/docs/managing-dragonfly/using-tls @romange

moredure commented 2 months ago

@Lakshyadevelops I don't like first approach because of we might fetch not currently used certificates (no tls reload issued yet, but files/symlink was updated) any chance to extract from some runtime data instead of loading current files, or it's incapsulated within lib and we have no access to this data directly? Probably it make sense to record certificate used during startup and in case of tls reload.

To generate cert/key you can search for openssl entry within tests directory, there was some python helpers to generate.

romange commented 2 months ago

I would also like to understand what does not work in https://www.dragonflydb.io/docs/managing-dragonfly/using-tls

and fix it. Can you please provide exact sequence of how you create certificates and how you run dragonfly (what arguments)?

romange commented 2 months ago

actually this doc is not very good for local development. so yeah, better use locally generated certificates. This worked for me:

openssl req -x509 -newkey rsa:4096 -days 10 -nodes -keyout ca-key.pem -out ca-cert.pem --subj "/C=GR/ST=SKG/L=Thessaloniki/O=KK/OU=AcmeStudios/CN=Gr"

openssl req -newkey rsa:4096 -nodes -keyout df-key.pem -out df-req.pem --subj \
"/C=GR/ST=SKG/L=Thessaloniki/O=KK/OU=Comp/CN=Gr"

openssl x509 -req -in df-req.pem -days 10 -CA ca-cert.pem -CAkey ca-key.pem -CAcreateserial -out df-cert.pem

./dragonfly --tls --tls_key_file=./df-key.pem --tls_cert_file=./df-cert.pem --requirepass=foo

redis-cli --tls  --insecure -a foo PING
Lakshyadevelops commented 2 months ago

Thanks for this insight @moredure. Agreed we should choose the second approach btw it was an example of what not to do and we are choosing it 😂.

btw how do we issue TLS reload ? I could not find any relevant command in dragonflydb docs. Redis TLS reload We do support single value config update but not multiple simultaneously.

Also I am not familiar with how clusters in K8 manage certificates. Ik they are using dragonfly-operator but how it exactly updates certificates is still unclear. And would this approach work here I am not sure.

moredure commented 1 month ago

Thanks for this insight @moredure. Agreed we should choose the second approach btw it was an example of what not to do and we are choosing it 😂.

btw how do we issue TLS reload ? I could not find any relevant command in dragonflydb docs. Redis TLS reload We do support single value config update but not multiple simultaneously.

Also I am not familiar with how clusters in K8 manage certificates. Ik they are using dragonfly-operator but how it exactly updates certificates is still unclear. And would this approach work here I am not sure.

CONFIG SET tls true results in reload of certificate for dragonfly - new cert/key/ca data is loaded from flags, etc

Lakshyadevelops commented 1 month ago

Apologies for the long delay. @moredure Could you please look at this implementation. I still have to add better error handling and tests, but can this approach serve our purpose?

moredure commented 1 month ago

Apologies for the long delay. @moredure Could you please look at this implementation. I still have to add better error handling and tests, but can this approach serve our purpose?

@Lakshyadevelops, all good, pls check romange comments