websockets-rs / rust-websocket

A WebSocket (RFC6455) library written in Rust
http://websockets-rs.github.io/rust-websocket/
MIT License
1.55k stars 223 forks source link

Also assume websockets over HTTPS are secure #209

Closed timvisee closed 5 years ago

timvisee commented 5 years ago

This PR changes connect() on ClientBuilder to also assume the https scheme is secure, instead of just wss.

This change is required for an application I'm developing, in which a https URL is upgraded to a secure web socket (for Firefox Send v3 to be exact, using https://host/api/upload for web sockets).

It would be awesome if this change could be accepted and published to crates.io, as the crate I'm working on is dependent on this to work.

timvisee commented 5 years ago

Note: The CI build seems to be failing. I don't know what is causing this, it does not seem to be related to this change.

vi commented 5 years ago

Is it to avoid substituting ^http to ws before using URL for websockets?

I don't remember WebSockets using http/https instead of ws/wss.

The CI build seems to be failing. I don't know what is causing this

cargo clippy's errors fail CI and clippy is getting more features, requiring the (semi-unmaintained) project to keep pace.

vi commented 5 years ago

Applied the same modification to async_connect.

Published version 0.22.3 with this patch to crates.io.

timvisee commented 5 years ago

@vi no, not to avoid substituting. For example, directly connecting over wss? to Firefox Send v3 doesn't work, as it isn't properly routed. It does over https? though.

I'm sorry I missed the async_connect function.

Thanks a lot for publishing the update! Will be able to publish my crate as well now.

vi commented 5 years ago

For example, directly connecting over wss? to Firefox Send v3 doesn't work, as it isn't properly routed. It does over https? though.

How does the server know what's inside the URL scheme?

$ websocat --ws-c-uri=wss://lol/123 -t - ws-c:open-fd:2
GET /123 HTTP/1.1
Host: lol
Connection: Upgrade
Upgrade: websocket
Sec-WebSocket-Version: 13
Sec-WebSocket-Key: xoronNYSyFb9dS42g+V+LA==

$ websocat --ws-c-uri=https://lol/123 -t - ws-c:open-fd:2
GET /123 HTTP/1.1
Host: lol
Connection: Upgrade
Upgrade: websocket
Sec-WebSocket-Version: 13
Sec-WebSocket-Key: ZFGrwrAIZr799EoXYMwJ6w==

Or is is some kind of proxying where there is GET https://... HTTP/1.1?

timvisee commented 5 years ago

I'm not sure to be honest. The Firefox Send project isn't worked on by me, so I don't know the ins and outs that well.

When directly connecting to the upload endpoint with the wss scheme instead of https I get the following error (when calling .connect(None) on a ClientBuilder): ResponseError("Status code must be Switching Protocols")

I did test what the server responded with using the commands you've shown, with these parameters it looks the same:

$ websocat --ws-c-uri=https://send2.dev.lcip.org/api/upload -t - ws-c:open-fd:2
GET /api/upload HTTP/1.1
Host: send2.dev.lcip.org
Connection: Upgrade
Upgrade: websocket
Sec-WebSocket-Version: 13
Sec-WebSocket-Key: oU7T3dwsG2q3tW3Eg9IlcQ==

$ websocat --ws-c-uri=https://send2.dev.lcip.org/api/upload -t - ws-c:open-fd:2
GET /api/upload HTTP/1.1
Host: send2.dev.lcip.org
Connection: Upgrade
Upgrade: websocket
Sec-WebSocket-Version: 13
Sec-WebSocket-Key: QPVTuUiuYE2Po4d50g4EoA==

In case you'd be interested in debugging it, a public instance is available with the endpoint https://send2.dev.lcip.org/api/upload. The source for the route is available here, and the route implementation is available here

timvisee commented 5 years ago

I'm very sorry, apparently my implementation was incorrect, and I was testing an old URL. Substituting https to wss seems to be working all right now. I needed to test the /api/ws endpoint. Long work days cause me to miss simple things like this.

In that case, the change introduced by this pull request may be reverted (although I think having it this way doesn't do any harm).

Sorry for possibly wasting some of your time.

vi commented 5 years ago

Going TLS when facing https still seems reasonable, even though both http and https is non-standard for WebSockets.

Rejecting any scheme except of ws and wss may be even more reasonable although...

vi commented 5 years ago

I did test what the server responded with using the commands you've shown ... websocat --ws-c-uri=https://send2.dev.lcip.org/api/upload -t - ws-c:open-fd:2

This command does not send request to any server. It just dumps it to terminal instead (and hangs waiting for response which will never come). It was just to illustrate that URL scheme is missing in the data that is sent to server.