Last Comment Bug 664305 - Implement websockets connection limits
: Implement websockets connection limits
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: Networking: WebSockets (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla7
Assigned To: Patrick McManus [:mcmanus]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-14 15:10 PDT by David Chan [:dchan]
Modified: 2011-10-05 08:47 PDT (History)
8 users (show)
mounir: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
websocket maxconnections v1 (9.40 KB, patch)
2011-06-23 13:51 PDT, Patrick McManus [:mcmanus]
no flags Details | Diff | Splinter Review
websocket maxconnections v2 (8.58 KB, patch)
2011-06-24 12:13 PDT, Patrick McManus [:mcmanus]
cbiesinger: review+
Details | Diff | Splinter Review

Description David Chan [:dchan] 2011-06-14 15:10:28 PDT
The current networking code restricts the maximum number of http connections
network.http.max-connections*
network.http.max-persistent-connections*

Should websockets be included in these counts or should a separate preference be created?

Websocket are kept alive longer than a normal http request which could lead to hitting the max earlier. A malicious website could open thousands of websockets on a client and consume system resources. The impact would be similar to a webpage running an infinite loop but harder to diagnosis since the machine wouldn't appear to run slower, network connections would fail.
Comment 1 Ian Melven :imelven 2011-06-14 15:30:56 PDT
fwiw, my opinion is that a separate pref makes the most sense here since websockets are different from regular http connections as outlined in dchan's comment, but are also not necessarily persistent
Comment 2 Robin 2011-06-23 09:42:46 PDT
I second Ian's opinion. A separate preference allows you to control the max number of HTTP connections and max number of WebSocket connections independently.

> A malicious website could open thousands of websockets on a client
> and consume system resources.

Bear in mind that such a malicious website would need to host a WebSocket listener capable of handling all these thousands of concurrent connections and be able to transmit enough data to impact the client. And then do that for each concurrent client as well. This is not impossible, or even hugely difficult as there is at least one WebSocket solution out there that can scale like that.

But if all you're trying to achieve is to mess with people's PCs, why go to all that hassle? It's a lot more challenging to do that than to simply write some malicious code and let it do its thing. Therefore since this kind of attack has a higher barrier to entry and little payoff, the probability of it being an issue is low. A separate preference to specify the max number of WebSocket connections will easily deal with this.
Comment 3 David Chan [:dchan] 2011-06-23 10:53:36 PDT
(In reply to comment #2)
> I second Ian's opinion. A separate preference allows you to control the max
> number of HTTP connections and max number of WebSocket connections
> independently.
> 
> > A malicious website could open thousands of websockets on a client
> > and consume system resources.
> 
> Bear in mind that such a malicious website would need to host a WebSocket
> listener capable of handling all these thousands of concurrent connections
> and be able to transmit enough data to impact the client. And then do that
> for each concurrent client as well. This is not impossible, or even hugely
> difficult as there is at least one WebSocket solution out there that can
> scale like that.
> 
> But if all you're trying to achieve is to mess with people's PCs, why go to
> all that hassle? It's a lot more challenging to do that than to simply write
> some malicious code and let it do its thing. Therefore since this kind of
> attack has a higher barrier to entry and little payoff, the probability of
> it being an issue is low. A separate preference to specify the max number of
> WebSocket connections will easily deal with this.

True there are better ways to launch a denial of service on the client machine. I agree that the impact is minimal. A more likely attack is to attack another server which is websockets capable. You visit a malicious page which then opens multiple long lived connections to this server. Ideally the target server would have implemented their own connection controls. I think you were saying that there is already an implementation which could handle this.
Comment 4 Patrick McManus [:mcmanus] 2011-06-23 13:51:27 PDT
Created attachment 541488 [details] [diff] [review]
websocket maxconnections v1

this patch prevents websockets from hogging the whole connection pool and thereby stalling http. (by default there are about 300 non-http sockets available, this limits us to 200). Given the history of servers themselves subverting low per-server limits (meant to protect those same servers), I don't really see a point in a local limit rather than the global one.

I don't see a reason to promote to aurora but if someone feels strongly then feel free to nom and provide rationale for the drivers.

there is a ridealong fix to nsPrelaodedStream in here.. I've tucked it in here because it can't really be triggered unless you are calling asyncwait(null,0,0,null) (via abortsession()) after the stream has been transferred to websockets but before any read has taken place.. that doesn't seem like it can happen without the rest of the patch.
Comment 5 Patrick McManus [:mcmanus] 2011-06-24 12:13:07 PDT
Created attachment 541756 [details] [diff] [review]
websocket maxconnections v2

the interdiff is just breaking out the nspreloadedstream fix into bug 666997
Comment 6 Christian :Biesinger (don't email me, ping me on IRC) 2011-06-25 06:59:01 PDT
Comment on attachment 541756 [details] [diff] [review]
websocket maxconnections v2

+    // from then main thread before StartWebsocketData() has completed

then -> the
Comment 7 Mounir Lamouri (:mounir) 2011-06-27 02:12:17 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/43f88c89f4cb
Comment 8 Eric Shepherd [:sheppy] 2011-07-11 12:17:50 PDT
Updated documentation:

https://developer.mozilla.org/en/WebSockets#Gecko_notes

And mentioned on Firefox 7 for developers.

Note You need to log in before you can comment on or make changes to this bug.