[ddp-client] Insufficient Sockjs Session-ID Length

[winstoncse]

Current ddp-client package implement Sockjs connection uses 8 characters session ID, as "Session identifiers should be at least 128 bits long to prevent brute-force session guessing attacks."

For Example:

https://www.****.com/sockjs/831/svp1a0l2/xhr

Where svp1a0l2 is the 8 characters long session id that the client send to the server to distinguish connections.

Would be great to increase the Session ID to 32 characters

Sockjs updates with option to customized session ID
sockjs/sockjs-client#250

These would reduce the likelihood of guessing a valid session ID

[abernix]

Ultimately, this comes down to upgrading SockJS. Please see #4114. I'll try to take a look at that this week since SockJS 1.x has been out for a while now.

I do think your concern is valid, but for what it's worth, I did a quick investigation several months back and determined that, assuming SSL was used (as it should be), the attack surface was very, very small. The sessionId here is not a typical session ID that would be used in a cookie. It was not possible to connect two clients to the same socket at the same time and once a socket was closed, it could not be re-used. This means that the attacker would have to win a race with the browser requesting the session. Since the possible combinations are roughly 3.5 trillion and the timeline to execute that attack very short, the chances are slim. I think it's worth pointing out that SockJS still hasn't raised their default length from 8 characters either.

Obviously, if you're not using SSL there are some legit concerns here as the SockJS sessionId could be obtained directly from the URL and various techniques could be used to force the client to reconnect, but this risk exists in any non-SSL implementation.

Again, this isn't to say that this shouldn't be addressed and I'll try to look soon – it'd be nice if SockJS just wasn't out of date and was easier to upgrade (currently there are some "Meteor" exceptions which may no longer be necessary).

Thanks for bringing this up.

[hwillson]

Let's track this issue under FR meteor/meteor-feature-requests#216. Closing here - thanks!