Documentation should note that package ignores protection against untrusted client

[propertyscott]

The client does not mask sent messages or verify the the Sec-WebSocket-Accept header. Because these features are required by the protocol and related to security, the documentation should note the omission of the features.

It's unlikely that the package will be used in an application running untrusted code, but the package might be used in a websocket proxy that forwards requests from untrusted code.

[nhooyr]

Will add masking ASAP - https://tools.ietf.org/html/rfc6455#section-10.3

Wasn't aware it was still a real issue on the modern internet and I didn't expect anyone to be running untrusted code with this library.

Will generate a unique Sec-WebSocket-Key as well. Based on https://stackoverflow.com/a/37074398/4283659 I decided not to implement it but it looks like there is a strong reason to do so: https://stackoverflow.com/a/37074398/4283659

[propertyscott]

The problem scenario is a websocket proxy server that uses this package as a websocket client.

Possible attack: The missing features give code from origin evil.com significant control over data sent to good.com's backend servers. The code from evil.com uses this control to exploit bugs at good.com and make cross origin requests.

[nhooyr]

Possible attack: The missing features give code from origin evil.com significant control over data sent to good.com's backend servers. The code from evil.com uses this control to exploit bugs at good.com and make cross origin requests.

By default no cross origin requests are allowed. See the docs on Accept.

[nhooyr]

Have tagged a new release with WebSocket masking and Sec-WebSocket-Accept verification. Should be good now.

[nhooyr]

Ah I think I misinterpreted your comment.

Possible attack: The missing features give code from origin evil.com significant control over data sent to good.com's backend servers. The code from evil.com uses this control to exploit bugs at good.com and make cross origin requests.

There is no attack vector there as far as I can tell and WebSocket masking and Sec-WebSocket-Accept do not affect a WebSocket proxy.

[propertyscott]

Thank you for fixing the issue.

By default no cross origin requests are allowed

If the proxy server developer assumes that the backend server will check the origin, then the developer might disable the origin check for Accept.

WebSocket masking and Sec-WebSocket-Accept do not affect a WebSocket proxy.

These features do affect a proxy server that creates websocket.Conn's and pumps messages between them. I don't think that's the right way to write a WebSocket proxy server, but there are a few that do this.

[nhooyr]

If the proxy server developer assumes that the backend server will check the origin, then the developer might disable the origin check for Accept.

That's fine, after all the host should be the one performing origin checks. What would you want the docs to say?

These features do affect a proxy server that creates websocket.Conn's and pumps messages between them. I don't think that's the right way to write a WebSocket proxy server, but there are a few that do this.

Fair enough.

[nhooyr]

Actually could you respond in #91 on what might be better in the docs?

Thanks again for the discussion and report.

[propertyscott]

You added Sec-WebSocket-Accept verification and masking, so there's nothing to document as far as this issue is concerned.

I don't understand why the Accept function documentation is related to this issue. There's some sort of miscommunication going on.

[nhooyr]

I think so too, this is what I've understood so far:

In

Possible attack: The missing features give code from origin evil.com significant control over data sent to good.com's backend servers. The code from evil.com uses this control to exploit bugs at good.com and make cross origin requests.

I thought you were saying cross origin requests are allowed by default but they aren't.

You then stated:

If the proxy server developer assumes that the backend server will check the origin, then the developer might disable the origin check for Accept.

Which is true but I'm confused if you meant something more by it. Should something be changed regarding Accept and cross origin requests?

[propertyscott]

Yikes, I didn't say that I am talking about a reverse proxy. Does that help?

Developer writes a reverse proxy that accepts websocket connections, dials backend server and pumps messages between them.

Developer assumes that backend websocket server will verify origin, so developer sets AcceptOptions.InsecureSkipVerify to false.

Attacker sends request to reverse proxy that is forwarded to plain HTTP endpoint on some backend server, not to a Websocket endpoint as the proxy developer assumed. Without fixes in e3dc7a3, attacker can control much of the data sent to the backend server. Attacker uses this control to exploit bugs in the backend server.

The proxy scenario is just an example. The point is that some types of attack are possible when a Websocket stream is incorrectly interpreted as HTTP.

[nhooyr]

I get it now, sounds good.