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

Include TLS information into INFO command output #3292

Open
moredure opened this issue Jul 9, 2024 · 14 comments
Open

Include TLS information into INFO command output #3292

moredure opened this issue Jul 9, 2024 · 14 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@moredure
Copy link

moredure commented Jul 9, 2024

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
Copy link
Contributor

@moredure is this on Redis/Valkey as well ?

@romange
Copy link
Collaborator

romange commented Jul 10, 2024

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?

@romange romange added the enhancement New feature or request label Jul 10, 2024
@moredure
Copy link
Author

moredure commented Jul 10, 2024

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
Copy link
Author

@moredure is this on Redis/Valkey as well ?

I've not seen it there

@moredure moredure added the good first issue Good for newcomers label Aug 12, 2024
@Lakshyadevelops
Copy link
Contributor

@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
Copy link
Collaborator

romange commented Sep 25, 2024

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

@moredure
Copy link
Author

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

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

@romange
Copy link
Collaborator

romange commented Sep 26, 2024

@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
Copy link
Contributor

Lakshyadevelops commented Sep 30, 2024

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
Copy link
Author

moredure commented Sep 30, 2024

@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
Copy link
Collaborator

romange commented Sep 30, 2024

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
Copy link
Collaborator

romange commented Sep 30, 2024

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
Copy link
Contributor

Lakshyadevelops commented Sep 30, 2024

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
Copy link
Author

moredure commented Oct 3, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants