Skip to content
This repository has been archived by the owner on Apr 24, 2023. It is now read-only.

Warn, or callout in connection error, when using insecure websockets #360

Open
mcclure opened this issue Jun 6, 2021 · 1 comment
Open
Labels
effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful good first issue Good issue for new contributors help wanted Seeking public contribution on this issue P2 Medium: Good to have, but can wait until someone steps up

Comments

@mcclure
Copy link

mcclure commented Jun 6, 2021

js-libp2p-webrtc-star and js-libp2p-websockets both support either ws/ or wss/ multiaddrs (WebSockets, or SSL WebSockets). Many of the examples, such as libp2p-in-the-browser or peer-and-content-discovery, use vanilla WebSockets, not Secure WebSockets, and this is appropriate, since these examples are likely tested using web servers on localhost and it is difficult to set up SSL on localhost.

However, insecure WebSockets is unpredictable. Leaving aside the question of whether insecure websockets "should" be used, attempting to use insecure WebSockets in a browser may sometimes fail just because it is insecure, for example:

  • The libp2p-in-the-browser example is deployed to an HTTPS server. The browser blocks it because accessing insecure websockets is a mixed-content violation.
  • Something I saw myself last night: I have a libp2p-in-the-browser based demo (see js-libp2p-kad-dht bug 221) that could successfully connect two peers to each other behind NAT using a personal VPS as a signaling server. Last night, this demo worked. This morning, it was failing with error "Transport (WebRTCStar) could not listen on any available address". The reason for the failure: Sometime around 10 PM last night I accessed the VPS where the signaling server in an ordinary browser tab, which activated HSTS for the entire domain, including the signaling port. That's pretty subtle.

These failures can be based on unpredictable local state (browser, HSTS state). If a user encounters one of these failures, they are unlikely to easily figure out on their own why the failure occurred: The change is something sort of nonobvious (change "/ws" in the listening list to "/wss") and making it work requires something the examples don't include (setting up SSL, libp2p has a out-of-the-box docker for a nginx proxy to do this but it isn't part of the examples directory). This is demonstrably a support issue for libp2p (searching google for the error messtingges I get when hitting insecure websockets problems I find many cases, including on https://discuss.libp2p.io/, where a libp2p core dev is having to explain that setting up SSL is needed).

A suggestion: When sending in /ws multiaddrs for listen or bootstrap in a browser, do one of the following:

  1. Print a warning to console warning that insecure websockets may not work in all browsers.
  2. Remember that an attempt was made to connect to insecure websockets. If after this an exception is thrown which could be the result of the browser cutting access to insecure resources (for example, say "Transport (WebRTCStar) could not listen on any available address" is thrown after an unsuccessful attempt to listen: for a /ws signaling server), include in the exception message "(This browser might be blocking insecure websockets)".

Since this case will mostly only come up in test/example projects, console noise is probably acceptable. If console noise is a concern, there could be a Libp2p.create() flag to disable insecure-websockets warnings.

Note: Maybe a similar issue should be filed on js-libp2p-websockets, I don't know.

@BigLep
Copy link

BigLep commented Aug 27, 2021

Thanks @mcclure for reporting. Would you be interested in making the change to print the warning in the console? This would be in libp2p/js-libp2p-websockets.

@BigLep BigLep added need/author-input Needs input from the original author and removed need/triage Needs initial labeling and prioritization labels Aug 27, 2021
@lidel lidel self-assigned this Sep 10, 2021
@lidel lidel added effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful good first issue Good issue for new contributors help wanted Seeking public contribution on this issue P2 Medium: Good to have, but can wait until someone steps up and removed need/author-input Needs input from the original author labels Sep 10, 2021
@lidel lidel removed their assignment Sep 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful good first issue Good issue for new contributors help wanted Seeking public contribution on this issue P2 Medium: Good to have, but can wait until someone steps up
Projects
Archived in project
Development

No branches or pull requests

4 participants