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

Something wrong with TLS connection in 5.1.{0, 1} #1505

Closed
1 task done
mokcrimea opened this issue Feb 14, 2019 · 9 comments
Closed
1 task done

Something wrong with TLS connection in 5.1.{0, 1} #1505

mokcrimea opened this issue Feb 14, 2019 · 9 comments

Comments

@mokcrimea
Copy link

  • I've searched for any related issues and avoided creating a duplicate
    issue.

Description

Reproducible in:

  • version: 5.1.{0,1}
  • Node.js version(s): 6-11
  • OS version(s): macOS 10.14.3

Steps to reproduce:

Hello.

Trying to reproduce WS connection with some server (uncontrolled by me) and after few hours of researching I found, that this - 15bdb5f commit breaks something. I really do not know what is going wrong, just because do not have access to server.

Some details:

In 5.0.0 (or without commit mentioned above) when I send stringified JSON payload, that contains auth information - servers answers that's everything ok.
But in version with this commit - servers answers that's something went wrong (unfortunately without any details).

Tried change node versions - nothing helps.

Do you know anything about that ?

@lpinca
Copy link
Member

lpinca commented Feb 14, 2019

Can you try running the following code? This is basically what ws does under the hood:

const crypto = require('crypto');
const https = require('https');
const tls = require('tls');

const request = https.get({
  headers: {
    'Sec-WebSocket-Key': crypto.randomBytes(16).toString('base64'),
    'Sec-WebSocket-Version': 13,
    'Connection': 'Upgrade',
    'Upgrade': 'websocket'
  },
  createConnection: tls.connect,
  hostname: 'replace',
  path: '/replace',
  port: 'replace'
});

request.on('upgrade', function () {
  console.log('It works.');
});

@lpinca
Copy link
Member

lpinca commented Feb 14, 2019

Also do not use 5.x. It is affected by this bug #1493 and the fix has not been backported.

@mokcrimea
Copy link
Author

Thank you for the answer.

I've tried to run the suggested code before, to understand how it works under hood (I looked thought WS source code)
So in given example the connection was established and "It works." appended on the screen.

In WS "open" event I'm trying to send auth credential to the server. The credentials depend on access and secret key, and some signature in HmacSHA256 format, but I think it doesn't matter in this case.

So in previous versions of WS after credentials sent - I'm got success auth message from server, and in 5.1.0 and above I got something like "Verification failure".

I should better show you code example to reproduce – https://github.com/mokcrimea/ws-example-5-1.0
Access and secret key, I'll send you by email. Do not want bother you with registration process details.

@lpinca
Copy link
Member

lpinca commented Feb 15, 2019

Can reproduce and using an agent fixes the issue:

const { globalAgent } = require('https');

const ws = new WebSocket('wss://api.huobi.pro/ws/v1', { agent: globalAgent });

but I'm not sure why, I need to investigate.

@lpinca
Copy link
Member

lpinca commented Feb 16, 2019

It's a bug with how the Host header is computed when no agent is used.

defaultPort is undefined so this condition is always true and the port is always added to the Host header even when it is the default port.

There are two more workaround in addition to the one suggested above. The first is setting the undocumented defaultPort option, the second is setting the Host header explicitly.

new WebSocket('wss://api.huobi.pro/ws/v1', { defaultPort: 443 });
new WebSocket('wss://api.huobi.pro/ws/v1', {
  headers: { Host: 'api.huobi.pro' }
});

lpinca added a commit that referenced this issue Feb 16, 2019
Prevent the default port from being added to the `Host` header when no
`Agent` is used.

Fixes #1505
lpinca added a commit that referenced this issue Feb 16, 2019
Prevent the default port from being added to the `Host` header when no
`Agent` is used.

Fixes #1505
@taoabc
Copy link

taoabc commented Mar 20, 2019

Thanks a lot, This bug happen to me. server response 403 when I use 6.1.3 to connect at some machine. When I upgrade to 6.2.0, bug gone. Bug I'm very curious about why host can not include default port. And why most of machine can't reproduce this bug even host include port.

@fas3r
Copy link

fas3r commented Mar 29, 2019

Hello,

Is there a way to provide a certificat to the ws client ? I'm using SSL/TLS with an authentification by certificat.

I'm able to make it work via browser, but I'm not sure how I could do that when the connection comes from a ws client only ( not via a broswer+ws ).

Thanks.

@lpinca
Copy link
Member

lpinca commented Mar 30, 2019

@fas3r see https://nodejs.org/api/tls.html#tls_tls_createsecurecontext_options, you can use those options in the WebSocket constructor.

@lpinca
Copy link
Member

lpinca commented Mar 30, 2019

@taoabc I guess it depends on how the Host header is handled on the server. The port is optional so it is not wrong to specify it.

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

4 participants