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

Unsecure websocket connection #2

Closed
tlienart opened this issue Mar 29, 2019 · 16 comments
Closed

Unsecure websocket connection #2

tlienart opened this issue Mar 29, 2019 · 16 comments

Comments

@tlienart
Copy link
Owner

tlienart commented Mar 29, 2019

It looks like using sec-websocket-key is what we want (?) (see also this so post). It looks like it's possible to do that with HTTP.jl as per these lines.

@asprionj
Copy link
Collaborator

We're already using sec-websocket-key in the handshake: https://github.com/asprionj/LiveServer.jl/blob/57d53873c589e3cb51a0ccac5346e05d5573db5f/src/server.jl#L125-L126.
Note that I just copied this part over from HTTP.jl. What we actually want is a TLS-secured connection, indicated by wss:// instead of ws:// (similar to https:// versus http://). But for this to work, we'd have to generate a self-signed certificate, and stuff...
Also, we'd have the client JS code check whether it's on a secure connection (https://) and open an according WebSocket connection, since differences in security level are blocked by most browsers AFAIK.

@tlienart
Copy link
Owner Author

yeah that sounds a bit annoying. In the mwe.jl of HTTP.jl's tests they have a MBedTLS thing. But I mean if it works as is on most browser we should probably not bother.

@asprionj
Copy link
Collaborator

Yep, there's more important (and fun) stuff to do now. But found that Chromium (and thus Chrome) seems to forbid ws:// connections to localhost for security reasons. Did not find a way of enabling this... And I think quite some people use Chrome/-ium and/or want to test-view their page also in this browser.

@tlienart
Copy link
Owner Author

tlienart commented Mar 31, 2019

@tlienart
Copy link
Owner Author

tlienart commented Apr 5, 2019

And a post on how to generate the self-signed certificate https://letsencrypt.org/docs/certificates-for-localhost/

I guess that could just be done and ship the certificates like they do in HTTP.jl (https://github.com/JuliaWeb/HTTP.jl/blob/master/test/cert.pem, https://github.com/JuliaWeb/HTTP.jl/blob/master/test/key.pem)

Edit:

Maybe it's worth giving a shot?

Edit 2:

@tlienart tlienart added this to the Initial Annoucement milestone Apr 5, 2019
@asprionj
Copy link
Collaborator

asprionj commented Apr 5, 2019

Making the OS globally trust such certificates seems to be very specific to each OS, even different Linux distros. That means it will be quite some work... How big of an issue would it be for the user of LiveServer to add an exception due to an untrusted certificate to the browser?

@tlienart
Copy link
Owner Author

tlienart commented Apr 6, 2019

Hmm I just wonder how these libraries (ws, browser-sync etc) do it. I just think that unfortunately many people use Chrome and would be surprised if things don't work :/

@asprionj
Copy link
Collaborator

asprionj commented Apr 6, 2019

Yep, Chrome is widely used. The only way to find out is to have a look at "the others"...

@asprionj
Copy link
Collaborator

asprionj commented Apr 6, 2019

Ok, browser-sync adds this <script> tag right at the beginning of <body>:

<script id="__bs_script__">//<![CDATA[
    document.write("<script async src='/browser-sync/browser-sync-client.js?v=2.26.3'><\/script>".replace("HOST", location.hostname));
//]]></script>

That is, they asynchronously load the script file from the server, interpolating the host name into the script. I think we can stick with our solution of just pasting the few lines of code directly.

As for the certificates, they do generate a self-signed one using openssl, cf. https://github.com/BrowserSync/browser-sync/blob/master/packages/browser-sync/certs/gen.sh. They AFAIK do not take measures to get rid of the browser warnings; when I run

browser-sync --https

the browser (Firefox in this example) opens and shows me that ugly

Your connection is not secure
The owner of localhost has configured their website improperly. To protect your information from being stolen, Firefox has not connected to this website.

I can then add an exception...

@asprionj
Copy link
Collaborator

asprionj commented Apr 6, 2019

Hrm, but somehow browser-sync also works without security also in Chromium. So it IS possible to have an insecure websocket connection to localhost in Chromium... have to dig deeper into browser-sync 's client-side code!

@asprionj
Copy link
Collaborator

asprionj commented Apr 6, 2019

So #36 is closed, and it was a server-side problem after all. So users of any browsers should be able to use LiveServer now.

Do we still want to add self-signed certificates before the Initial Announcement? Could (ok, should...) be as simple as copying the shell-script from browser-sync and configuring HTTP.jl to use it when serve() is called with keyword-argument https=true.

@tlienart
Copy link
Owner Author

tlienart commented Apr 6, 2019

So if we keep it as you fixed it, they see an "insecure connection" warning and if we were to add the self-signed certificates then the warning would not show?

If so I'd be in favour of the second one of course, but if an exception shows either way then why bother. And if that's the case (exception) then it should just be explained in the doc so people know.

@asprionj
Copy link
Collaborator

asprionj commented Apr 7, 2019

There's three options:

  • Do not add certification at all. Then, one can only test the page on http and ws (insecure). This does not show any warnings, but there's no "safe" symbol left to the URL in the browser (e.g. the green lock).
  • If one wants to test his page with https and wss (secured), we'd have to add certificates and, upon keyword-based request, have HTTP.jl use those certificates to establish secured connections. We have to use self-signed certificates, because no certification authority can validate your localhost. In this case, the warnings will show. (They will not show if serve() is called in normal/insecure mode.)
  • The third option would be to automatically make the OS trust that self-signed certificate. Then, the browser would not show a warning but the "green lock" because he trusts this secured connection. But this could actually be left to the user, if he's annoyed too much by the warning or if his browser does not allow for permanent security exceptions. It would also be an immense effort work to make this work for every OS!

The question thus is whether we want to offer a dev-server which allows one to also test the behavior of his page under a secure connection. I'd say we should add it, i.e. go with the second option above (which is also what browser-sync, for example, provides).

@tlienart
Copy link
Owner Author

tlienart commented Apr 8, 2019

yeah that's great I would suggest doing (1) for the initial announcement and (2) for some further future (possibly with feedback from early users?)

@asprionj asprionj removed this from the Initial Annoucement milestone Apr 8, 2019
@asprionj
Copy link
Collaborator

asprionj commented Apr 8, 2019

Agreed; removed it from the milestone.

@fonsp
Copy link
Collaborator

fonsp commented Mar 25, 2022

Hi! I think we can close this old issue! Some comments:

  1. By running on localhost (aka 127.0.0.1 on ipv4), you are telling the OS that the connection should be local to this computer, and it is guaranteed to not be accessible from other computers on the network (LAN nor internet). The port, and WS server, is only exposed when running on 0.0.0.0 (requires sudo). This should alleviate most concerns, the only exception is a setup where many users are on the same computer, like a compute cluster at a university. Leading to my second point:
  2. Our websocket server is safe! Accessing it does not allow code execution, arbitrary file reads, etc. The code is simple enough to verify.
  3. I am not sure what this comment refers to: but on current versions, ws://localhost from http://localhost works on every browser 🎉
  4. It looks like the sec-websocket-key logic has been implemented:
    HTTP.setheader(http, "Sec-WebSocket-Accept" => HTTP.WebSockets.accept_hash(key))

@fonsp fonsp closed this as completed Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants