snyk/broker

Filtering incoming Client connections

JackuB opened this issue · 2 comments

I think I've discovered a vulnerability with the incoming Client connection.

When Client with BROKER_ID 123 is connected to the server and 2nd (evil) Client connects with the same BROKER_ID, it overtakes original Client's position and all Server calls that should go to the original Client goes only to the evil one. Could cause both data leaks and mocked responses to mess with Server.

There are a few ways to mitigate this, although they are not perfect:

  • IP filtering: not sure if we can have a stable IP/IP range from those internal networks, but if so, it sounds reasonable
  • per-client Server-Client certificate: Client will specify what cert it expects from Server and Server serves different per-client certificate - could be bypassed with a dedicated client ignoring errors IMHO
  • OAuth 1: I know, I know… but it would also come with a MAC, key rotation etc.

What prevents it now is that, Nginx can't distinguish incoming connections before handshake because path looks like this path="/primus/?_primuscb=LU9fg6t&EIO=3&transport=polling&t=LU9fg6v&b64=1" and BROKER_ID is in payload so it might have to be resolved on Server - check that BROKER_ID is allowed in ENV and its ENV value matches IP/IP range (in socket.on('identify'))

tl;dr: I'd propose a Server would only accept Client connections if they are named in the ENV and their IP/IP range matches. So Server would also have a MY-CLIENT-ID=<IP> in ENV for each allowed Client.

gjvis commented

I'm not sure its a "vulnerability" as such – token-based auth is pretty ubiquitous. A client's BROKER_ID can be rotated as often as you'd like (and using any mechanism you like), so you can tailor that to your specific threat model.

We hadn't planned on adding more auth at this stage, but thoughts on it thus far are:

Token-agnostic security
This is probably where I'd want to start. It would be pretty simple to add mutual-cert TLS, where the broker client presents a certificate that the server can validate. This gives us validation at the transport-layer that the connecting client is a real user (we wouldn't know which user though). Its standard enough that it doesn't require much explanation for clients, and it'll play well with load balancers.

Token-aware security
If we need more layers of auth, then we could extend the broker with a pluggable authentication mechanism of some sort, so you could then run token-aware authentication directly in node.

What do you think? Are you in a situation currently where token-based auth isn't sufficient?

Closing this. Things changed a bit since and comments above and unlikely to be applicable. please reopen if need be.