Re: [hybi] Redesigning the Web Socket handshake

[Maciej Stachowiak <mjs at apple.com>]

On Feb 2, 2010, at 10:37 PM, Justin Erenkrantz wrote:

> On Tue, Feb 2, 2010 at 9:22 PM, Maciej Stachowiak <mjs at apple.com> wrote:
>> Let me start by laying out the security risks in the current handshake. Then I will explain my proposed change and how I believe it addresses them.
> 
> Thanks!  For the security stuff, I think it's best if I ponder that
> for a few days to see if I can grok where the risks are.  In the
> meantime, there is one point that I'd like to ask clarification on:
> 
>> To give a concrete example, consider a chat service over WebSocket. Let's say it doesn't check correctness of the handshake, and does authentication in-band. An attacker could construct an HTTP POST body which would look like an authentication message plus a couple of submitted sent messages. True, the attacker cannot read the victim's chat messages, but could send chat messages as the victim. Clearly this would be a significant security breach.
> 
> I am trying to understand how a properly implemented WS or HTTP server
> would fall into this trap.  For httpd, this would not be feasible -
> the code that parses request bodies isn't the same that handles the
> protocol.

A standalone WebSocket server could, per the WebSocket protocol draft, do no validation whatsoever of the WebSocket request handshake. I believe it would be completely conforming to skip until it sees two newlines in a row and start reading messages. Well, not quite - there seems to be a should-level requirement to abort if the third token of the first line does not contain a "/" character.

http://tools.ietf.org/html/draft-hixie-thewebsocketprotocol-74#section-5.2

A recent change also adds that "If a server reads fields for authentication purposes ... then the server should also verify that the client's handshake includes the invariant "Upgrade" and "Connection" parts of the handshake".

This is an improvement, but I think it should be a MUST not a should, and should be a requirement for all WebSocket servers. The spec also does not say that a WebSocket server MAY disconnect from the client if it finds the client handshake is invalid in any way other than lacking a "/".

A WebSocket module for an HTTP server would be less likely to fail to validate the handshake at all. However, whether the HTTP server would pass off invalid requests to a WebSocket module depends on how exactly it does dispatch. If it dispatches to the WebSocket module based on the relevant Connection and Upgrade headers, then it can't be tricked by XHR. But if it dispatches based on the path in the request, then it could pass a non-WebSocket request to the WebSocket module. In that case, if the WebSocket module doesn't do checking, it is similarly vulnerable.


>  I'm trying to think of any real server implementations that
> would attempt to process an HTTP body that also includes "fake"
> requests (but in a different protocol).  Maybe there's a bit of
> crafting that I'm just not seeing...

The HTTP body wouldn't "also" include fake requests, it would *just* include fake requests. The HTTP header of the HTTP request itself might be enough to fool a client that did nothing but check for "/" and skip until it finds two newlines. And after that, you can have well-formed WebSocket messages in the HTTP body.

> 
>> We could mitigate both of these risks, and also reduce the implementation difficulty for servers and proxies, by changing the handshake. We would remove the requirement for newlines or exact capitalization of the headers. Instead, we could have the browser generate a unique random nonce, which it includes in the handshake request. The server would be required to read it, and respond with a hash of the nonce value.
> 
> While I'm not 100% sold on the security rationale for nonce yet (still
> pondering it), at the least, the nonce is pretty straightforward to
> implement - so, I could certainly be willing to admit I have a blind
> spot there and go with the nonce if it means I can implement this in
> httpd in an otherwise-straightforward manner.

Thanks, that's useful feedback.

Regards,
Maciej