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

support tls on redis url #1268

Closed
dcharbonnier opened this issue Aug 22, 2017 · 8 comments
Closed

support tls on redis url #1268

dcharbonnier opened this issue Aug 22, 2017 · 8 comments

Comments

@dcharbonnier
Copy link
Contributor

Example :
rediss://:password@rediss:6400/1?rejectUnauthorized=false

https://www.iana.org/assignments/uri-schemes/prov/redis

Apparently named by analogy to HTTPS (RFC 2818), the rediss: URI scheme (yes, two "s"es,
not a typo) has been used by some clients to designate RESP over TLS.
Other than the usage of TLS, the rediss: URI scheme is not known to
have any differences from the redis: URI scheme.

@BridgeAR
Copy link
Contributor

I am against adding this. The reason is that it is not possible to add all tls options with the url. Therefore more options would have to be passed to the client anyway in most cases. It is also difficult to distinguish the tls options from regular options. Thanks for bringing this to my attention though!

@dcharbonnier
Copy link
Contributor Author

Mongodb for example manage it... https://docs.mongodb.com/manual/reference/connection-string/
More over, considering the fact that you already accept to not cover all options through the url I don't see the point of not adding more support.

@svengerlach
Copy link

svengerlach commented Nov 14, 2017

I don't see why it's necessary "to add all tls options with the url". From an operations perspective it would be good enough to just enable TLS encryption for the connection – establishing the trust relationship should be handled by the operation system eitherway (installed root/intermediate certificate authorities; see /etc/ssl on Linux). I really don't want to take a deep-dive into each and every programming language's settings in order to "just enable TLS". It basically should be a yes/no thing. This is what the rediss uri scheme provides.

Other client libraries (in other languages) already support this:

By not providing this feature it makes it necessary to keep a separate configuration for node.js.

@BridgeAR BridgeAR removed the wontfix label Jan 9, 2018
@BridgeAR BridgeAR reopened this Jan 9, 2018
@BridgeAR
Copy link
Contributor

BridgeAR commented Jan 9, 2018

Seems like there was enough interest in this feature for me to reconsider this.

@dfontenot
Copy link

Is an NPM release for this feature planned soon? 2.8.0 is the latest, which doesn't have this feature.

@rmg
Copy link

rmg commented Mar 8, 2018

What about a lesser form of this that does something like set default tls options it can set from the URL without extending the format? Setting servername to the hostname portion of the URL, for example, might be a reasonably safe thing to do.

I don't know about others, but I know that at least compose.io uses SNI for Redis over SSL.

@calebboyd
Copy link
Contributor

calebboyd commented Jul 9, 2018

This was partially solved via #1282

@BridgeAR It remains unreleased, any hope for a release soon?

epatters added a commit to epatters/datascienceontology-backend that referenced this issue Oct 18, 2018
Use master version of Node Redis client for now, because TLS support is
not yet officially released:

redis/node-redis#1268
redis/node-redis#1282
epatters added a commit to epatters/datascienceontology-backend that referenced this issue Dec 1, 2019
Use master version of Node Redis client for now, because TLS support is
not yet officially released:

redis/node-redis#1268
redis/node-redis#1282
epatters added a commit to epatters/datascienceontology-backend that referenced this issue Dec 1, 2019
Use master version of Node Redis client for now, because TLS support is
not yet officially released:

redis/node-redis#1268
redis/node-redis#1282
@Salakar Salakar added this to the v3.0.0 milestone Feb 7, 2020
@Salakar
Copy link
Contributor

Salakar commented Feb 9, 2020

I've just published v3.0.0 to NPM; https://github.com/NodeRedis/node-redis/releases/tag/v3.0.0 - which includes the change @calebboyd mentioned, can this issue be closed now?

@Salakar Salakar removed this from the v3.0.0 milestone Feb 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants