chore: support SSL for redis

[mattkrick]

Description

fix #7564

similar to #7175

Supports TLS for redis

TEST

• Add env var POSTGRES_SSL_DIR which is the path to a directory on the file system where the certs live
• set REDIS_URL to start with rediss

Inside that directory, put the following files:

• root.crt
• redis.key
• redis.crt

Note: I'm not sure if this works! it's possible that we have to break up the REDIS_URL into proto/host/port. If you give me some self-signed certs I can test it locally

[rafaelromcar-parabol]

Redis docker images are built with TLS support (see this) as indicated in the Redis' documentation.

You should be able to run Redis as a Docker container with the parameters shown here. Do not forget adding --port 0 so non-TLS connections are disabled. You might need to use tls-auth-clients no as well (here).

I have found an example of someone that did a test for running Redis with TLS and generating their own TLS certs. Check it out here. It looks good, but I haven't checked it deeply.

[rafaelromcar-parabol]

Redis provides a script to help with the certificate generation in their own utils!! see here. I think is the same script you can find in the example I shared above.

And this example is very clear.

[mattkrick]

success!

notes:

• used the redis.{crt,key} instead of server/client versions
• use the latest bitnami/redis in dev testing to avoid compiling redis with TLS turned on
• make sure REDIS_SSL_DIR is an absolute path
• make sure REDIS_SSL_REJECT_UNAUTHORIZED='false' (i'm not sure it can ever be true?)
• REDIS_PASSWORD is optional

[rafaelromcar-parabol]

Looks like working in an on-demand environment (redis-tls). I would just:

• Replace REDIS_SSL by REDIS_TLS (not like it changes anything, but it makes sense).
• Add three OPTIONAL variables with default values in the application:
  • REDIS_TLS_CA_CRT_FILENAME: root.crt or ca.crt
  • REDIS_TLS_REDIS_CRT_FILENAME: redis.crt or tls.crt
  • REDIS_TLS_REDIS_KEY_FILENAME: redis.key or tls.key

In some cases, like the one I used for testing, it will be hard to force the certificates to that names and it might be way easier to have the possibility of customizing the names.

If you prefer to skip that and just use fixed names, I would keep what I proposed in the commit I did: ca.crt, tls.crt and tls.key. That would already help us in our on-demand-env case and wouldn't make any difference with other arbitrary chosen names.

[rafaelromcar-parabol]

BTW I still need to test it with a GCP Memorystore Redis with TLS enabled. Could you rebase, verify on your end and push it (generating a Docker image)? I would then redeploy Redis in our current staging and make a real-environment test 💪🏼

[rafaelromcar-parabol]

Okey then. I was validating how to connect to the Memorystore Redis with TLS enabled and, as I suspected (and shared in a comment that I deleted because I wanted to validate it again), we need to be able to pass ONLY the CA certificate and not any tls.key or tls.crt.

Memory store supports authentication through password and TLS in-transit connection but does not allows you to generate or use client certificates.

In sum up:

• We need to be able to support passing only the CA certificate.
• We need to be able to support passing all three certificates.
• It would be perfect to be able to customize the filename of the certificates (even if we have a single path where all must be). If we cannot do that, keep ca.crt, tls.key and tls.crt if possible (to simplify our life in our on-demand environment).

Once you have done that, and we can do the testing together if needed, please generate a Docker image so I can test in all situations!

Thanks!

[rafaelromcar-parabol]

More information:

In GCP Memorystore Redis we can:

• Enable TLS so we will have a ca.crt available. If anyone tries to connect to the instance without the correct CA, it will be not be valid.
• Enable AUTH so we have a password to connect to the instance.

So, we need to be able to support the following cases:

• No auth and no TLS
• Auth with password (should work with and without TLS ca.crt)
• TLS with only the ca.crt
• TLS auth (requires TLS ca.crt) and thus it uses also tls.key and tls.crt for the client.

In sum up:

• TLS can be enabled or disabled (ca.crt or not)
• Auth can be of two types:
  • Password (auth string)
  • TLS client certificates (tls.key and tls.crt)

All the combinations are required depending on the case.

• In GPC Memorystore we will be using TLS enabled and AUTH password.
• In our On Demand environment we will be using TLS enabled with AUTH TLS

[rafaelromcar-parabol]

Question: how is the application going to manage server certificate rotations? please check Google's documentation.

[mattkrick]

Gotcha, so our app has to be ready for a TLS error at some point & when that happens we just re-try reading the certs from filesystem?

Any extra reading material on the CA-only method? I always thought that got paired with client/server certs

[rafaelromcar-parabol]

Gotcha, so our app has to be ready for a TLS error at some point & when that happens we just re-try reading the certs from filesystem?

If I get correctly what Google's documentation says, what gets rotated is the server certificate (like the cert we use for action.parbol.co) but not the CA certificate (which would be, for action.parabol.co, Let's Encrypt). The CA cert that the app reads from its file system will not change (it expires only 10 years after creation - check Google's documentation). So, the application should do the same that a browser would do if a web page's certificate expires while it is navigating it. I imagine there is a retry asking the server to provide a new TLS server certificate, that then is verified using the CA certificate again.

Any extra reading material on the CA-only method? I always thought that got paired with client/server certs

Nothing else as a file. Only the password is available, but not mandatory.

TLS in-transit encryption isn't really coupled with TLS authentication. Here we are encrypting the communication, but not really adding more than that as security. Google Memorystore provides only authentication string as auth method.

Check Google's documentation on Redis Auth to better understand it.

[mattkrick]

@rafaelromcar-parabol ready for review!

to test locally:

• you can generate certs with the script in ./certs
• the official redis docker image doesn't support TLS, so you must run yarn db:start to use the bitnami one
• there are 6 combinations to test. They are here:

  parabol/packages/server/utils/getRedisOptions.ts

  Lines 42 to 49 in 2bead67

  	if (password) {
  	if (tls?.cert) return 'TLS + Password'
  	if (tls?.ca) return 'CA + Password'
  	return 'Password only'
  	}
  	if (tls?.cert) return 'TLS'
  	if (tls?.ca) return 'CA only'
  	return 'Unsecure'

[rafaelromcar-parabol]

the official redis docker image doesn't support TLS

Yes, it does! Check the Redis 6.2 Dockerfile. It is also available in 7.0 and 7.2. But those images do not have any environment variable to ease the setup of TLS options 😸 and you need to add arguments to the Docker cmd (redis-server) like here. I'll try to use the official image to test it.

Should we use this opportunity and branch to upgrade Redis to 7.0? It is available in GCP and it is what we have in Digital Ocean already. 7.2 isn't available in GCP.

[rafaelromcar-parabol]

Testing with the official Docker image redis:6.2 I can make Redis run with TLS enabled with a redis.conf correctly configured.

Testing with TLS enabled, both with CA (implies tls-auth-clients optional in Redis config) or with all certs (default for Redis, it requires tls auth certs), regardless of using a password or not, I'm always getting the following:

:38:01 4|DB Migrations     | [ioredis] Unhandled error event: Error: self-signed certificate in certificate chain
18:38:01 4|DB Migrations     |     at TLSSocket.onConnectSecure (node:_tls_wrap:1550:34)
18:38:01 4|DB Migrations     |     at TLSSocket.emit (node:events:514:28)
18:38:01 4|DB Migrations     |     at TLSSocket._finishInit (node:_tls_wrap:967:8)
18:38:01 4|DB Migrations     |     at ssl.onhandshakedone (node:_tls_wrap:743:12)

When I run Redis with password only, no TLS enabled, and the app has also configured only REDIS_PASSWORD='testing-password' I get:

19:03:49 5|Relay Compiler    | Redis mode: Password only
19:03:50 4|DB Migrations     | [ioredis] Unhandled error event: ReplyError: NOAUTH Authentication required.
19:03:50 4|DB Migrations     |     at parseError (/home/rafaelromcar-parabol/Parabol/github/ParabolInc/parabol/node_modules/redis-parser/lib/parser.js:179:12)
19:03:50 4|DB Migrations     |     at parseType (/home/rafaelromcar-parabol/Parabol/github/ParabolInc/parabol/node_modules/redis-parser/lib/parser.js:302:14)

In both cases the application works normally and I can do a retro, creating reflections and grouping them.

It is like if everything worked even if the DB Migrations were complaining 🤔 it doesn't make sense, but the app works.

I pushed my modified version of the dev.yml for Docker Compose and also the redis.conf I'm using to play with Redis. More info about how to configure Redis here. If needed, I should be available for a sync in your early morning tomorrow. Or on Monday in an easier hour for you probably. Let me know.

[rafaelromcar-parabol]

LGTM

Just:

• Document the Redis vars
• Rollback the dev.yaml to the original