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

Allow clients that don't trust the SSL certificate to opt-in to connecting insecurely #602

Closed
3 tasks done
JorgenPhi opened this issue Jul 16, 2016 · 10 comments
Closed
3 tasks done

Comments

@JorgenPhi
Copy link

JorgenPhi commented Jul 16, 2016

Server Problem

Please confirm whether you've tried the following debugging steps:

  • Run npm run build-server to regenerate lib/ from src/
  • Run rm -rf node_modules && npm install to get a fresh install of dependencies
  • Restarted the server

Description of the Problem

  • What triggers the problem? - Connecting to the HTTP version of the site.
  • What happens? - The HTTPS socket.io connection is given.
  • What do you expect to happen instead? - The HTTP socket.io connection to be given.

System Information

  • Operating System: Ubuntu 14.04.4 LTS
  • Node Version: (run node -v) v4.4.7
  • CyTube Version: (displayed at startup) v3.18.7
  • Error Messages Displayed: N/A

This should also take advantage of upstream proxies passing x-forwarded-proto for determining which version to pass on to the client.

@calzoneman
Copy link
Owner

This is by design. The only reason that the site does not auto-redirect to HTTPS when it is available is because of cross-origin content security policy which breaks certain video types that do not support HTTPS yet (such as Hitbox). I can't think of any reason why you would want the socket to connect to an insecure endpoint when a secure one is available; what is your use case?

@JorgenPhi
Copy link
Author

Some users have issues getting their browser to trust Let's Encrypt. Notably android clients. Should probably just suck up the money for a real certificate. Wasn't aware that this was intentional.

@calzoneman
Copy link
Owner

Yes, there is an issue with older clients and LetsEncrypt certificates, which is unfortunate. This was an issue for cytu.be as well; I had to buy a cert for the main server but now I've added a secondary one using LetsEncrypt so there will need to be a solution for that.

I think a better solution than defaulting to insecure would be to programmatically detect when the connection fails and prompt the user to accept that they are using a browser that does not support the CA, but if they check the box then they can proceed with an insecure connection.

@calzoneman
Copy link
Owner

To make the issue more clear, I'm updating the title.

@calzoneman calzoneman changed the title HTTPS socket.io url is given to HTTP connecting clients Allow clients that don't trust the SSL certificate to opt-in to connecting insecurely Jul 16, 2016
@JorgenPhi
Copy link
Author

On another note, the ACP seems to attempt to choose the corresponding protocol's socket.io URL, but if the server is configured to use a non-standard port it breaks.
https://github.com/calzoneman/sync/blob/3.0/src/web/acp.js#L29

@calzoneman
Copy link
Owner

Oh that actually brings up another point which is that the socket.io library is loaded over the default endpoint which is HTTPS, so that will be tricky to handle for clients where HTTPS is broken.

With regard to the ACP, "configured to use a non-standard port it breaks" -- to be a bit pedantic, if you set "https.default-port" to a port that is not actually bound to any HTTPS listener then it breaks. The ACP should be updated to share the same endpoint selection logic as the channel page, but it is still currently possible to use the ACP on non-standard HTTPS ports as long as it's configured correctly.

@JorgenPhi
Copy link
Author

JorgenPhi commented Jul 16, 2016

So I use nginx as a cache for static elements hosted on my cytube instance, and i was always under the impression that "default-port" was only for generating links. The socket.io URLs are still correctly generated for all pages except the acp. I'd also like to mention that I would prefer to not have to proxy websockets through nginx.

I also had to override how ipv4-ssl was generated in sync/src/config.js as it would ignore the different subdomain passed in cfg.io["domain"].

@calzoneman
Copy link
Owner

The configuration I'm using:

listeners:
  - io: true
    https: true
    port: 10443

https:
  default-port: 10443
  full-address: 'https://cytu.be'

I'm not proxying any socket.io connections (only HTTPS), so I've never run into this particular issue with the ACP, which I believe is off topic for this issue anyways and should probably be reported separately.

Anyways, the real solution for that is to untangle what is probably one of the biggest and longest lasting hacks in CyTube (the HTTP/HTTPS/IO listener configuration logic) and implement a more sane configuration that will likely involve server administrators explicitly specifying base URLs instead of trying to autogenerate them with complicated and confusing logic.

It's worth noting that proxying socket.io was not officially supported until recently (there are very few reasons you would want to do so, and it carries a performance loss for higher volume sites) so the ACP is just one issue that slipped by because it was never tested and no one complained.

@calzoneman
Copy link
Owner

Please try the changes from the related commit (6e416fe) and edit callbacks.js for your server to set USING_LETS_ENCRYPT = true to enable it.

@JorgenPhi
Copy link
Author

I'll deploy this on the server and get back to you later tonight. Thanks for looking into this.

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

2 participants