bitnami / charts

Bitnami Helm Charts
https://bitnami.com
Other
9.05k stars 9.22k forks source link

[bitnami/redis] More flexibility needed for handling CA certificate #30305

Open tdg5 opened 2 weeks ago

tdg5 commented 2 weeks ago

Name and Version

bitnami/redis 20.2.1

What is the problem this feature will solve?

In order to use TLS, Redis needs

a CA certificate bundle file or path to be used as a trusted root when validating certificates [source]

redis-cli is a little more flexible in that it will:

If neither cacert nor cacertdir are specified, the default system-wide trusted root certs configuration will apply. [source (Disclaimer: these are valkey docs, but redis-cli --help says the same thing]

As such, bitnami/redis also insists upon being given a CA certificate.

This presents a problem when working with cert-manager which doesn't always include a ca.crt when issuing certificates:

Additionally, if the Certificate Authority is known, the corresponding CA certificate will be stored in the secret with key ca.crt. For example, with the ACME issuer, the CA is not known and ca.crt will not exist in acme-crt-secret. [source]

This leads to trouble with bitnami/redis because I'm left with two options:

  1. Create a custom secret from the cert-manager secret that also includes a manually retrieved copy of the appropriate root certificate.
  2. Hack a way around.

Option 1 doesn't work great for a CA like Let's Encrypt which issues certificates with a TTL of 90 days. Option 2 is the route I've been forced to go, but the path forward is brittle and hacky.

I've created tdg5/bitnami-redis-ca-cert-shenanigans to document the working hack I've found and to demonstrate that it should be easier than this.

I'm happy to help contribute a solution, but I'd need some guidance on the right way to go about simplifying this problem.

What is the feature you are proposing to solve the problem?

I think the best option is to make the source of the CA certificate more configurable (e.g. don't require it to be a part of the TLS certificate). This would allow injecting custom root certificates and/or using trusted root certs already available in the docker image.

I don't love that this still requires mounting extra volumes to the pods (if the system doesn't have an acceptable root cert), but at least its a more explicitly supported path. At a minimum, it seems like forcing the CA certificate path to start with /opt/bitnami/redis/certs severely limits options and should be made more flexible.

Another less good option (IMHO) would be to make cacert an optional argument for uses of redis-cli since redis-cli will fallback to using the default system-wide trusted root certs. I've found that redis server will work if I tell it the CA is tls.crt, but redis-cli will not cooperate. This could allow for making the CA cert more generally optional. I worry though that this is not very explicit and maybe only solves a subset of awkwardness.

What alternatives have you considered?

Maybe I'm missing something, but it seems that the only alternative would be to craft a custom secret from the cert-manager certificate secret that also includes the CA, but that makes automation very difficult so it's not an option I'm willing to accept.

juan131 commented 1 week ago

Hi @tdg5

Thanks so much for your feedback, it's true that the CA certificate could be potentially omitted. That's said, I think CertManger integration is still possible. Check this example:

apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
  name: redis-clusterissuer
spec:
  selfSigned: {}
---
apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
  name: redis-ca-crt
spec:
  secretName: redis-ca-crt
  commonName: redis-root-ca
  isCA: true
  issuerRef:
    name: redis-clusterissuer
    kind: Issuer
---
apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
  name: redis-ca-issuer
spec:
  ca:
    secretName: redis-ca-crt
---
apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
  name: redis-crt
spec:
  secretName: redis-crt
  commonName: redis.default.svc.cluster.local
  issuerRef:
    name: redis-ca-issuer
    kind: Issuer
  subject:
    organizations:
      - "Redis"
  dnsNames:
    - '127.0.0.1'
    - 'localhost'
    - '*.redis'
    - '*.redis.default'
    - '*.redis.default.svc'
    - '*.redis.default.svc.cluster.local'
    - '*.redis-headless'
    - '*.redis-headless.default'
    - '*.redis-headless.default.svc'
    - '*.redis-headless.default.svc.cluster.local'
  privateKey:
    algorithm: RSA
    size: 2048
  duration: 2160h
  renewBefore: 360h
$ kubectl get certificate
NAME           READY   SECRET         AGE
redis-ca-crt   True    redis-ca-crt   17s
redis-crt      True    redis-crt      16s
$ kubectl get secret redis-crt -o json | jq .data
{
  "ca.crt": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUMvRENDQWVTZ0F3SUJBZ0lRUnJTODZPMWk3bUdCRkRHM1pQNzIrVEFOQmdrcWhraUc5dzBCQVFzRkFEQVkKTVJZd0ZBWURWUVFERXcxeVpXUnBjeTF5YjI5MExXTmhNQjRYRFRJME1URXhOVEV5TURReE5Gb1hEVEkxTURJeApNekV5TURReE5Gb3dHREVXTUJRR0ExVUVBeE1OY21Wa2FYTXRjbTl2ZEMxallUQ0NBU0l3RFFZSktvWklodmNOCkFRRUJCUUFEZ2dFUEFEQ0NBUW9DZ2dFQkFMOUVkSG9QYkExTmdyd3k2MEVlNTJqSHMzRnNqQ2pvQUprUldkNmEKRFQyVStuQmpJMzFKS1lGUDBZbUxsc1pwR3pMMDJOeVgxRlhRbDZJamtsb3BPV3RvbXBDU2VKOENxdXRTbElTRApWWkU3am92Qkc1KytzN3EybVNRZ0Fub1JxUUJ0LytmREdhVHBJZmlOaVJyN3R4aDFnMmVxQ2k5czU0VkFiaEg0CjM1Nkx5azZpUUdXcXlUSWpQY3ZmV3dIemgyNjI4SUNJbnI3T3UxaTdkZUNBb0Y3QTBvUW1XdXZaNVZ3NmZEOXMKV3QyY1hpU2JtSllXcFYwNUxOYTJoTVNFR3RRZVYvMEdrb3ovWXpVYWtCamZSNURTT2RnNFlrNmN2K0orRnZLNgpVeDlSaWE1Ym0wdFplUVNteXBvVFVMemdUQjY3RG9vODJmS1N1NTg0OXFrU2R2OENBd0VBQWFOQ01FQXdEZ1lEClZSMFBBUUgvQkFRREFnS2tNQThHQTFVZEV3RUIvd1FGTUFNQkFmOHdIUVlEVlIwT0JCWUVGT3R0L2dUc3RmYTgKVnVvd1BkMCt3Tm1EQ1c5ek1BMEdDU3FHU0liM0RRRUJDd1VBQTRJQkFRQ0ZxWXhiK0xNU0wrMmt1NFExb05PZApLbEdNRk05bVJUamFEOFZNbkgzSGdVeTR3WUZHSXYwVGFTY3F0djFuMlErcTNhMnpLQ3dHbkc2R0FERU05Z2c5Ci85UEk0OFZYV0hkMjE1WFQ4ME14aDhRT3BMOW5YQVVVSXlReVNyUnZoOElyY2o1a0pzY2FiY2ZBS2JReEhoQzkKRDdML2hucGswWjB3WFR6TFVZdWU5cjJpOU8yeFlzVlJMdXlYUG9qNG1OaWlReHRoVEg4bmw0TmVaTmVDaXRhZgprdGNlSGhSaDRlbVloZERwL1B1V3grVWR5N09WbDNKa1RXWEREWERwcHY0SHNHL2czWWpnSHhIZWRNT2QvdkViCjJoNWdpVUJySVdJQUNoTHVBNkpQU3E4K0dPMVJGa0xMWmQyd2dYMlUwNDluaVEvV1pqZGlFcndUYkV0VUtCamgKLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo=",
  "tls.crt": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUVEVENDQXZXZ0F3SUJBZ0lRYUxOcnRPTGxpaTNiSllUcVFNc1FSekFOQmdrcWhraUc5dzBCQVFzRkFEQVkKTVJZd0ZBWURWUVFERXcxeVpXUnBjeTF5YjI5MExXTmhNQjRYRFRJME1URXhOVEV5TURReE9Wb1hEVEkxTURJeApNekV5TURReE9Wb3dPakVPTUF3R0ExVUVDaE1GVW1Wa2FYTXhLREFtQmdOVkJBTVRIM0psWkdsekxtUmxabUYxCmJIUXVjM1pqTG1Oc2RYTjBaWEl1Ykc5allXd3dnZ0VpTUEwR0NTcUdTSWIzRFFFQkFRVUFBNElCRHdBd2dnRUsKQW9JQkFRQzBtc2ZISmNPRWJBb3YrbGlubE5VTThCaHVEQ3ZoQzc3QTVmWnlQdnJaWVRFUHl3akllQzFZZVJWcgpGaDBpeVMwK1liZm9JbGt1RUF1a1l6VldzWThoZ3hlN01CTGFzWkRyNE11VlNXckx4czJQam9MWDU4YWpXTENvCnNyTUFnMkw0ZTVqK09nakFJTkhjalNpK2VrcUJVMWI3Q0xrOU1VU0FjRXRKVTRjbzRIaGs5b25Ed002NXdkVDEKVEc4WlUxSURReVlPL1ZLM291WVBBY3l6UFpDZDRRV1BCUXFXdmwrTFYwU01jMkFIcis0UGphV0JDN1Y4bG5jOQpQc2xURkt5amZxaWw2dUFrUjhGSDRZYjY5dlg4V0pmaWExaVhOU2YrdklhSkRjMGY5RzVCejI0MTZSWElrNE5LCks5azY1bGwrL3lFdGJTaW4yVHdFU09MTjN6a2ZBZ01CQUFHamdnRXZNSUlCS3pBT0JnTlZIUThCQWY4RUJBTUMKQmFBd0RBWURWUjBUQVFIL0JBSXdBREFmQmdOVkhTTUVHREFXZ0JUcmJmNEU3TFgydkZicU1EM2RQc0RaZ3dsdgpjekNCNlFZRFZSMFJCSUhoTUlIZWdna3hNamN1TUM0d0xqR0NDV3h2WTJGc2FHOXpkSUlIS2k1eVpXUnBjNElQCktpNXlaV1JwY3k1a1pXWmhkV3gwZ2hNcUxuSmxaR2x6TG1SbFptRjFiSFF1YzNaamdpRXFMbkpsWkdsekxtUmwKWm1GMWJIUXVjM1pqTG1Oc2RYTjBaWEl1Ykc5allXeUNFQ291Y21Wa2FYTXRhR1ZoWkd4bGMzT0NHQ291Y21WawphWE10YUdWaFpHeGxjM011WkdWbVlYVnNkSUljS2k1eVpXUnBjeTFvWldGa2JHVnpjeTVrWldaaGRXeDBMbk4yClk0SXFLaTV5WldScGN5MW9aV0ZrYkdWemN5NWtaV1poZFd4MExuTjJZeTVqYkhWemRHVnlMbXh2WTJGc01BMEcKQ1NxR1NJYjNEUUVCQ3dVQUE0SUJBUUJod0twaGNXMzlVempSMDBBOTBzQ2FlZ1hDUUpROS9idkRLbEdheWEyMgpDMlVnNUZ4T0RvcGcvVmdiRXNRT1Q3a0E4OE9LellQekNWMm5RMCtIZ3VkRjBSWmZCZkhncEdBL2pSYXVLS0prCjdhRFh4ZzROUHU3aURsR3VGVGdyZzlQdGVXNGFPL0NRRHJoK3BYdDExWVFDZG9pZ2tYQ3d2bjlwNTFBaXNmVVkKRlNianUvem80WUxwTTBUenltanV1TDU5YnJ0YVFqeUVjWHZhN3U5NlhBeW1Id0VRTXBNeGhTY1dOTW1JeHNPTgpXTENnVmFTUnBhRzhmZ1dpNWErOUoyN1NpZm93N1YrUWZiMGRiVXlsR2xldUF6QVFvdDJON2VBUG5NK3hGMGhqCjN6SjJVVTBxUUNhaktlTGloOElXVkNUVGpWeVFjNlVRK1FSUjJWeWxua0lmCi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K",
  "tls.key": "LS0tLS1CRUdJTiBSU0EgUFJJVkFURSBLRVktLS0tLQpNSUlFb3dJQkFBS0NBUUVBdEpySHh5WERoR3dLTC9wWXA1VFZEUEFZYmd3cjRRdSt3T1gyY2o3NjJXRXhEOHNJCnlIZ3RXSGtWYXhZZElza3RQbUczNkNKWkxoQUxwR00xVnJHUElZTVh1ekFTMnJHUTYrRExsVWxxeThiTmo0NkMKMStmR28xaXdxTEt6QUlOaStIdVkvam9Jd0NEUjNJMG92bnBLZ1ZOVyt3aTVQVEZFZ0hCTFNWT0hLT0I0WlBhSgp3OERPdWNIVTlVeHZHVk5TQTBNbUR2MVN0NkxtRHdITXN6MlFuZUVGandVS2xyNWZpMWRFakhOZ0I2L3VENDJsCmdRdTFmSlozUFQ3SlV4U3NvMzZvcGVyZ0pFZkJSK0dHK3ZiMS9GaVg0bXRZbHpVbi9yeUdpUTNOSC9SdVFjOXUKTmVrVnlKT0RTaXZaT3VaWmZ2OGhMVzBvcDlrOEJFaml6ZDg1SHdJREFRQUJBb0lCQUZOYWlFK3lieTFqeThQZQo4Q0hRQUJCcmsycmU5VFYxZlVGOElTVVRTaVVmeFV2N3NmOEw4Q25BSUpGKzhFMkl1ZmZyazd4d2RlWmJnM1FJClVhckVZa0hlUmdxZm9tUUt4YzhIdmc0VmgxT3JXbDRpTU9OQzFwdTNLRG1EMkRlcGEva3cyaWlqVWp5U0tTbDQKZ05DQWtuaGpwV1pRM1dXUG9vVUF6czUxM1pDUVdxVjNYVGduS1NSeEVHdWU5cmRoVFlaeUNVazV5b0tSeGdjSApOdVh6ZEo3UjVuMEdlZkRIYWNmYmxSYW40UkF6bWRXTDBPSHNZQ2lBVjZHMWVIQndyOXJUQnRycDd3bFNJL3JsClJwSloxdDZ5WFc2emMva2JiYTdZbTI2Y3FkM3BJU0VFNlpUd1BDbEpPeE1uaUd2WVJLRFVIRDJYRHpVNEFJWVQKVGM1by9DRUNnWUVBMW5UK09RVHVaRjNxckovRFhpcklsaDBRVlE3b0hMMHdRVlEyelF2SXIxQU5SMUt4ZjVtYwpGTGlJMWFIYVZUeEZUOTNhc2FmWStiZE9GYjNrOU1DR1RYUm9zZU1iWlhyUnhmNHU4K2kxZW4xbXpOVWFzbFNiCklRTVlzWk4zTi8wUG1NWVFRN013TUhFOUlvY2g2UDZGK0prdzhkNE1OZEtFRkV2WE9KNU9YQzhDZ1lFQTE1Y0kKcTNyVWlZOGpDemFmZDNETUhkRTZzc1pBQmJDdEgwc2ZIR3V4RmUyOElxaFFlSTJGVkVSQUJCRzc2QVJMaDlsTwp3WmZ1eTRvWnhUbGw0Wi9LenVpQ0tFN0VaR2FBNFZhSTVVR3FhYkU1blZiYlRjQkVPNmlNeEIxa1Zwb2M1WlI2CktDYUpxTTRDNmpWUmZtTmFyYkFEUGlxVGd4b3pGMVdPWWdrZUJoRUNnWUFWTVFKZUNBcTJpRUZFUlpVOG5PV3QKVDducGJjdXA2MWYxWEtqOW9naDJxbVpsZ2RwZ1RZR0sycUZLMnUrRU5LeDBzK3VFV3AycUwxN2I3eVBNdTZtSgp4ZE1Db1BwNEVLYmg5YmdXQ2d6RXlkZDZyaVF4TXdNUlJBa1FvdzNJem96WC85cGpsalZRMUIvQ1piQUlJZjZECkRrQXo1a2sybUZMTTNxR3B5cHJicVFLQmdRQ2dpNVNsUGw4djRTdlRtZVcrNkh6WVo0c3l2bmZHeFA3bkl1WVUKSUE1THFqSkVETnVBS3NOek1UQUsxN242MmRmMVhscEx5SHpIamh3S3h1ZWIwWWNSOWd1WFFMRDNvMjJ1WU9hbwo4dkdZcTZXZ0NhSmVrQ00vMmhDWnF0bWh0RHA0ck0wYlFUZWFRVy9pUWJwaVNGS3FyVHg4K3UvRzNhZldaSndYClhYckNRUUtCZ0I3aG1hQ1JEanMrMWptL241MGZwcmpaU3J0SnBOeHcva0FWYzVMQVM1YUo4TmtZK0xrOHZwNmsKMURQN0RJeVkrNzhLNFlXREFjcjBFT0RuQndnWGtmZlF4UW5LMUZnSUtnQWlvMnZ6ZHQ2ZWlmZUhjUWxzS1RLVApWa3NJT0NnVDczT2JqZTlsKytDSmRNWWpxRlFmS2dEY01PZWdjY3FDQXc4cGJsNHBUL3lECi0tLS0tRU5EIFJTQSBQUklWQVRFIEtFWS0tLS0tCg=="
}
tdg5 commented 5 days ago

Hey @juan131! Thank you for the very thorough response!

I understand that cert-manager can be used to create a self-signed certificate, but I'm specifically trying to use a certificate created by a legitimate external authority that does not include a CA certificate when issuing a certificate (because that's how ACME works).

If I were interested in using a self-signed certificate, I could just use the redis.createTlsSecret functionality built into the chart.

I may very well be missing something, but as far as I see it, using a self-signed certificate leads me down one of two roads:

  1. Tell all the clients not to validate the server's certificate, sacrificing some security and increasing operational complexity
  2. Tell all clients about the self-signed root CA by some means, probably sacrificing security and increasing operational complexity

I think both of these options are inherently less secure and both create additional operational complexity that should be unnecessary given the ubiquitous public key infrastructure of the web.

I'm open to being convinced that a self-signed certificate is as secure as a certificate signed by a well known CA, but that still wouldn't change the reality that it requires additional operational complexity to ignore or broadcast that CA.

A self-signed certificate seems like choosing a solution that is less secure and more of a hassle when there's another solution that is more secure and less of a hassle readily available.

Perhaps an option that could at least make my more minimal hack less hacky would be to make it so that if the certCAFilename value is an absolute path, then the chart wouldn't try to prepend /opt/bitnami/redis/certs/. Then I could avoid doing this silliness and feel like there was more certainty that the solution would continue to be supported in the future.

juan131 commented 5 days ago

Hi @tdg5

Please note I'm not suggesting using a self-signed certificate, I just shared it as an example. The problem here is that depending of the Issuer, ACME in you case, the CA certificate may not be included in the secret.

I created a draft PR that might address this issue, could you please give it a try?