Bug 32246 – Multiple connection attempts to a WebSocket server should not be allowed

• Component: WebCore Misc.
• Status: NEW

[Alexey Proskuryakov]

We don't have the following implemented yet:

-------------------------------
   1.   If the user agent already has a Web Socket connection to the
        remote host (IP address) identified by /host/, even if known by
        another name, wait until that connection has been established or
        for that connection to have failed.

        NOTE: This makes it harder for a script to perform a denial of
        service attack by just opening a large number of Web Socket
        connections to a remote host.

        NOTE: There is no limit to the number of established Web Socket
        connections a user agent can have with a single remote host.
        Servers can refuse to connect users with an excessive number of
        connections, or disconnect resource-hogging users when suffering
        high load.
-------------------------------

[Fumitoshi Ukai]

Chromium implement this in SocketStreamHandle, but it's not clean, but it needs to sniff data to detect handshake message.

Maybe, we need to add some methods in SocketStreamHandle and SocketStreamHandleClient.

 - new SocketStreamHandle




(In reply to comment #0)
> We don't have the following implemented yet:
> 
> -------------------------------
>    1.   If the user agent already has a Web Socket connection to the
>         remote host (IP address) identified by /host/, even if known by
>         another name, wait until that connection has been established or
>         for that connection to have failed.
> 
>         NOTE: This makes it harder for a script to perform a denial of
>         service attack by just opening a large number of Web Socket
>         connections to a remote host.
> 
>         NOTE: There is no limit to the number of established Web Socket
>         connections a user agent can have with a single remote host.
>         Servers can refuse to connect users with an excessive number of
>         connections, or disconnect resource-hogging users when suffering
>         high load.
> -------------------------------

[Fumitoshi Ukai]

Chromium implement this in SocketStreamHandle, but it's not clean.
It needs to sniff data to detect handshake message.

To implement it in WebCore/websocket,  we need to add some methods in SocketStreamHandle and SocketStreamHandleClient.

 - new SocketStreamHandle
        resolve ip address from host.
        calls back client->willOpen(handle, addresslist);
 - in WebSocketChannel, maintain table of address that is running handshake.
   if handle's address is open, call handle->connect().
   otherwise, wait other handle's handshake finishes, or close.

  - in WebSocketChannel, once handshake is finished or closed, clear its addresses from table.
     pick next handle which address becomes free from the table, and call handle->connect().



(In reply to comment #0)
> We don't have the following implemented yet:
> 
> -------------------------------
>    1.   If the user agent already has a Web Socket connection to the
>         remote host (IP address) identified by /host/, even if known by
>         another name, wait until that connection has been established or
>         for that connection to have failed.
> 
>         NOTE: This makes it harder for a script to perform a denial of
>         service attack by just opening a large number of Web Socket
>         connections to a remote host.
> 
>         NOTE: There is no limit to the number of established Web Socket
>         connections a user agent can have with a single remote host.
>         Servers can refuse to connect users with an excessive number of
>         connections, or disconnect resource-hogging users when suffering
>         high load.
> -------------------------------

[Alexey Proskuryakov]

This sounds like a good plan.

Alternatively, we could try to shoot down this provision in the spec. I'm not sure if it really adds any protection.

[Alexey Proskuryakov]

Actually, I need to reverse both statements!

Yes, WebSocket is the first way to open an unlimited number of connections to a single server, so it indeed likely needs additional protection to prevent DOS attacks.

But we don't really have a way to implement this correctly. Since each DNS request can result in a new result (this is a form of load balancing), resolving the name first and re-resolving it for actual connect() won't work. We don't have a way to pass both host name and its pre-resolved IP address down to the network stack.

[Alexey Proskuryakov]

<rdar://problem/7455142>

[Alexey Proskuryakov]

*** Bug 55178 has been marked as a duplicate of this bug. ***

[Joe Mason]

(In reply to comment #4)
> Actually, I need to reverse both statements!
> 
> Yes, WebSocket is the first way to open an unlimited number of connections to a single server, so it indeed likely needs additional protection to prevent DOS attacks.
> 
> But we don't really have a way to implement this correctly. Since each DNS request can result in a new result (this is a form of load balancing), resolving the name first and re-resolving it for actual connect() won't work. We don't have a way to pass both host name and its pre-resolved IP address down to the network stack.

We could add didResolveIP(handle, ip) to SocketStreamHandleClient, and only add a host/ip pair to the "existing connections" map if this is called.  That way each platform's network backend can call this function if they can get the IP, and if they don't implement it they just don't get connection limiting.

[Lamarque V. Souza]

Created attachment 197853 [details]
Patch

Proposed patch. The patch is not ready so I am not asking for +cq just yet. I want to read comments about it before I finish it, specially regarding the RefPtr/ListHashSet usage and the two FIXME I added to it. Just as a note, I am using 50 as pending request limit because LayoutTests/http/tests/websocket/tests/hybi/multiple-connections.html starts 50 connection attempts. Using 50 as limit allows me to avoid editing multiple-connections.html to lower that value.

[Build Bot]

Comment on attachment 197853 [details]
Patch

Attachment 197853 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/131073

[Build Bot]

Comment on attachment 197853 [details]
Patch

Attachment 197853 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/35303

[Alexey Proskuryakov]

Comment on attachment 197853 [details]
Patch

I don't think that WebSocket class is the correct level for such checks. In a multi-process environment, the checks should be in networking process to account for the possibility of a client using multiple windows and/or frames.

[Lamarque V. Souza]

(In reply to comment #11)
> (From update of attachment 197853 [details])
> I don't think that WebSocket class is the correct level for such checks. In a multi-process environment, the checks should be in networking process to account for the possibility of a client using multiple windows and/or frames.

Would moving the checks to WebSocketChannel as suggested in comment #2 work in multi-process environment?

[Alexey Proskuryakov]

Unsure. I really see this as part of underlying networking code, just like HTTP connection limit enforcement.

[Lamarque V. Souza]

(In reply to comment #13)
> Unsure. I really see this as part of underlying networking code, just like HTTP connection limit enforcement.

In which file is the HTTP connection limit enforcement implemented? I have found initializeMaximumHTTPConnectionCountPerHost in Source/WebCore/platform/network/ResourceRequestBase.h but it seems not to be used anywhere.

[Joe Mason]

(In reply to comment #14)
> (In reply to comment #13)
> > Unsure. I really see this as part of underlying networking code, just like HTTP connection limit enforcement.
> 
> In which file is the HTTP connection limit enforcement implemented? I have found initializeMaximumHTTPConnectionCountPerHost in Source/WebCore/platform/network/ResourceRequestBase.h but it seems not to be used anywhere.

It's not in WebKit - it's part of the underlying platform network stack (the Windows network stack, or the Mac network stack, or libsoup for the GTK+ port, or the Qt network classes for the Qt port, etc.)

I'm not sure websockets are in the same boat as HTTP connection limits. It's assumed that the underlying stack is a complete implementation of the HTTP protocol, but for WebSockets most of the protocol is implemented in WebKit and the "underlying stack" is simply TCP. So any additional limitations that don't apply to TCP connections in general would be more properly be part of the WebSocket protocol code in WebKit than in the network stack.

(To be a completely clean parallel to HTTP, much of the WebSocket code should be removed from webkit and provided through an external library... but that's uselessly pedantic.)

[Lamarque V. Souza]

(In reply to comment #15)
> (In reply to comment #14)
> > In which file is the HTTP connection limit enforcement implemented? I have found initializeMaximumHTTPConnectionCountPerHost in Source/WebCore/platform/network/ResourceRequestBase.h but it seems not to be used anywhere.
> 
> It's not in WebKit - it's part of the underlying platform network stack (the Windows network stack, or the Mac network stack, or libsoup for the GTK+ port, or the Qt network classes for the Qt port, etc.)
> 
> I'm not sure websockets are in the same boat as HTTP connection limits. It's assumed that the underlying stack is a complete implementation of the HTTP protocol, but for WebSockets most of the protocol is implemented in WebKit and the "underlying stack" is simply TCP. So any additional limitations that don't apply to TCP connections in general would be more properly be part of the WebSocket protocol code in WebKit than in the network stack.
> 
> (To be a completely clean parallel to HTTP, much of the WebSocket code should be removed from webkit and provided through an external library... but that's uselessly pedantic.)

Actually, initializeMaximumHTTPConnectionCountPerHost is used in Source/WebCore/loader/ResourceLoadScheduler.cpp. It seems to be the right place. I am still investigating how ResourceLoadScheduler works.

[Alexey Proskuryakov]

WebCore counts HTTP connections in addition to lower level code doing this. IIRC that's a workaround for some CFNetwork issues, where trying to exceed allowed number of connections would trigger bugs. I don't remember all details though.