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

feat: secure web sockets #1468

Merged
merged 1 commit into from
Sep 22, 2022
Merged

feat: secure web sockets #1468

merged 1 commit into from
Sep 22, 2022

Conversation

d10-cc
Copy link
Contributor

@d10-cc d10-cc commented Jun 2, 2022

Description

Add support for secure web sockets by using the TLS options when creating a server for the websockets.

Motivation and Context

When adding the httpsProtocol configuration in the serverless.yml file

custom:
  serverless-offline:
    httpsProtocol: "dev-certs"

serverless-offline will show the endpoints as https://localhost:3000/{default*} for HTTP and wss://localhost:3001 for WebSockets, as opposed to http:// and ws://
While https works with the certificates, the websocket protocol is not upgraded and uses http rather than https

How Has This Been Tested?

To test:

  • Add a key and a certificate in dev-certs/key.pem and dev-certs/cert.pem respectively.
  • Add websocket events to the serverless.yml file
  • Enable https in serverless.yml to point to the dev-certs folder
  • Connect to the websocket with wscat -c wss://localhost:3001

wscat will complain because it can't establish a TLS/SSL connection, however using wscat -c ws://localhost:3001 will work because it's not secure. Although this is unintended (since the protocol is listed as wss) and unusable if you're using serverless-offline with https and want to communicate with the websocket from a front-end because any browser will refuse to allow a connection to a non-secure server if using https.

I've tested this with the version 8.8.0 since I couldn't figure out how to build master right now, but the code for the websocket server hasn't changed.

Also the code is the same as for the http event's http server since hapi's API is the same for both.

Cheers!

@psiservices-uwidmark
Copy link

Had the same problem and independently did the same change (more or less). Also on 8.8 as we are stuck due to some dependency not being updated.

@dnalborczyk
Copy link
Collaborator

thank you for the PR @d10-cc. I would like to pull this in. that said, it would be great if we could a simple test case. I also tried to add a test for the TLS options for Api Gateway (with hapi), but couldn't quite get it working. I'll try to find that stash and put it up in a PR if you could have a look.

// HTTPS support
if (typeof httpsProtocol === 'string' && httpsProtocol.length > 0) {
serverOptions.tls = {
cert: readFileSync(resolve(httpsProtocol, 'cert.pem'), 'ascii'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I refactored applying the TLS options with the non-blocking node:fs/promise versions if you could base it on that if possible?

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

Successfully merging this pull request may close these issues.

3 participants