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

Supports rediss:// URL #939

Merged
merged 1 commit into from
Jul 31, 2019
Merged

Supports rediss:// URL #939

merged 1 commit into from
Jul 31, 2019

Conversation

monkbroc
Copy link
Contributor

@monkbroc monkbroc commented Jul 30, 2019

Support rediss:// URL (Redis over TLS) by defaulting option tls: true for those URLs.

Fixes #846

@luin
Copy link
Collaborator

luin commented Jul 31, 2019

Thanks for the contribution! 🍺

@ioredis-robot
Copy link
Collaborator

🎉 This PR is included in version 4.14.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@monkbroc
Copy link
Contributor Author

Thanks for the quick merge and release! 🎉

@paulmelnikow
Copy link
Contributor

Hi! Unfortunately this change prevents passing in tls options, as the custom options are now replaced by the boolean true. Probably this should set tls: true only when tls is not already present in the options.

Perhaps more to the point, the reason that I need to pass in tls options is so the correct SSL certificate is used; otherwise I get this error:

Error: unable to verify the first certificate
    at TLSSocket.onConnectSecure (_tls_wrap.js:1049:34)
    at TLSSocket.emit (events.js:182:13)
    at TLSSocket.EventEmitter.emit (domain.js:442:20)
    at TLSSocket._finishInit (_tls_wrap.js:631:8) code: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE'

This is what my code looks like:

    const options =
      this.url && this.url.startsWith('rediss:')
        ? {
            //  https://www.compose.com/articles/ssl-connections-arrive-for-redis-on-compose/
            tls: { servername: new URL(this.url).hostname },
          }
        : undefined
    this.redis = new Redis(this.url, options)

Would you be open to a fix that passes this value automatically? I think it will allow for more robust out-of-the-box behavior.

I'm not sure what it is about the way Compose is set up that requires this, though I think the issue is that an SSL connection to that IP address returns more than one certificate (and without it, TLS tries to validate the wrong one).

@paulmelnikow
Copy link
Contributor

Opened new issues for these: #947 #948

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow a way to configure TLS using DSN
4 participants