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 multiple ECDH curves on older versions of Node #15245

Closed
dlundgren opened this issue Aug 25, 2019 · 5 comments
Closed

Support multiple ECDH curves on older versions of Node #15245

dlundgren opened this issue Aug 25, 2019 · 5 comments

Comments

@dlundgren
Copy link
Contributor

Description:

Snaps & Docker images that are using Node < 10.0.0 cannot connect to servers with TLS or SSL certs that use ECDH curves that are not prime256v1. With #11016 being obsolete, I'm willing to step in and make a PR to get this support added, but I need some direction on the best place to add the tls/semver checks on the server side. My initial inclination is to add it to app/settings/server, but I'm not familiar enough with how the project is laid out if there is a better location to code similar to #11016 in.

The DEFAULT_ECDH_CURVE has been set to auto in Node 10 and higher, so we'd have to target this towards nodejs < 10.0.0.

Steps to reproduce:

Launch an existing Docker or snapd image, attempt to use LDAP that has a non prime256v1 certificate issued to it.

Expected behavior:

The system should be able to connect.

Actual behavior:

I receive ECONNRESET

Server Setup Information:

  • Version of Rocket.Chat Server: 1.3.2
  • Operating System: Ubuntu
  • Deployment Method: snapd
  • Number of Running Instances: 1
  • NodeJS Version: 8.11.3
@reetp
Copy link

reetp commented Sep 12, 2019

I have no idea why the devs have not looked at this?

Free code on offer.

Perhaps @sampaiodiego might make a suggestion.

@sampaiodiego
Copy link
Member

thanks @reetp .. I'm not sure I get this though.

One thing I'd like to make clear, latest Rocket.Chat version runs only on Node 8.11.4 .. so we don't need to do test Node version as #11016 does, once we release a new version with higher NodeJS version support, we can adjust that code.

@dlundgren are you saying the current code setting DEFAULT_ECDH_CURVE to auto doesn't work for you?

CC @Hudell

@dlundgren
Copy link
Contributor Author

There is currently no code that is setting DEFAULT_ECDH_CURVE to auto, so it's using the default of prime256v1.

@sampaiodiego
Copy link
Member

you're right @dlundgren .. I'm sorry, I thought we still had that code, but it was removed on #13531 .. The we couldn't be removed because investigating a little deeper I found the fix on Node was published only on Node 10.

Looks like you can still work around this env var to run Rocket.Chat NODE_OPTIONS=--tls-default-ecdh-curve=auto

A PR is welcome too, you could add to this file as it is one of the first codes to be executed.

@sampaiodiego
Copy link
Member

Closed by #15365

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

No branches or pull requests

3 participants