heroku / roadmap

This is the public roadmap for Salesforce Heroku services.
193 stars 11 forks source link

Redis basic plans are not SSL whereas production plan are. #172

Open ombr opened 1 year ago

ombr commented 1 year ago

Required Terms

What service(s) is this request for?

redis/platform

Tell us about what you're trying to solve. What challenges are you facing?

We realized while upgrading our redis instances that basic redis plans are not using self signed certificates whereas premium plans are all using SSL and thus requiring in ruby verify_mode: none

I think this is breaking dev/prod parity on review apps. Mini plans should also only provide rediss secured URLS.

Thanks.

Luc

jbrown-heroku commented 1 year ago

@ombr Thank you for logging this! Basic plans on Heroku Data for Redis allows for SSL connection if you are using REDIS_TLS_URL as documented here: https://devcenter.heroku.com/articles/heroku-redis#security-and-compliance I hope this helps.

ombr commented 1 year ago

Hi @jbrown-heroku yes I got this, but it should be the default for redis_url as it is the default in premium plans! If I am upgrading to a premium plan my app will crash if I don't change the code to support SSL. (This break dev/prod parity)

stof commented 3 months ago

@jbrown-heroku the fact that REDIS_TLS_URL is not defined at all for production plans (instead of defining it with the TLS URL, which would be the same than REDIS_URL) makes it hard to use it though, as an app using REDIS_TLS_URL to get TLS in review apps would then crash when being promoted to production because the env variable it uses does not exist.

jbrown-heroku commented 3 months ago

@stof Thank you for clarifying the issue here. That's definitely something we can look into improving. I will take this to discuss internally on how we want to approach this and get back here.

jbrown-heroku commented 3 months ago

Here is the proposed idea: Make REDIS_URL always TLS, and prepare REDIS_INSECURE_URL as an interim solution for customers that may require non-TLS for now. After some weeks, we want to remove this to uniformly offer TLS across our offerings. This should resolve the issue as mentioned @stof ; please let us know your thoughts.

stof commented 3 months ago

Switching to TLS everywhere would make sense to provide uniformity. But then, it would be great to work on https://github.com/heroku/roadmap/issues/148 to make the TLS work out of the box (right now, using a TLS REDIS_URL without changing the code would lead to connection errors because (almost?) all libraries validate the certificate by default)

jbrown-heroku commented 2 months ago

Thank you @stof @ombr for contributing to this discussion.

We will perform REDIS_URL unification in an effort to close this gap. Upcoming change announcement is here. This is a two-step process for mini plans in a "musical chair" fashion where we introduced REDIS_TEMPORARY_URL as insecure connection (non-TLS) and we will switch REDIS_URL to TLS on the specified date. At completion, we will remove both REDIS_TEMPORARY_URL and REDIS_TLS_URL. If the library is able to use both redis:// and rediss://, then no change should be necessary.