Discussion: Securing token for socket.io auth

[corymsmith]

Found this article and thought I'd share to start a discussion around it:

https://facundoolano.wordpress.com/2014/10/11/better-authentication-for-socket-io-no-query-strings/

[marshallswain]

This is a cool idea and I don't see any reason we shouldn't implement it with the auth rewrite we're doing. We could map the authenticate method that he talks about to one of the token service methods.

One thing that's not addressed is checking the token on every request. I think the only reason we wanted to do that in the first place was to watch for an expired token. Rather than checking it on every request, maybe we could grab the token expiration and set it up on the socket the same way he's doing socket.auth = true;. We do socket.expiresAt = timestamp; and only do an

if(new Date().getTime() > socket.expiresAt){
  // Do we boot them? 
  socket.disconnect('unauthorized');
  // or let them stay connected as a public user?
  socket.feathers.user = false;
}

on every request. That should spare the server from having to crunch away at the token on every request. We would still want to re-auth on reconnects after a temporary disconnection. I don't think we have to disconnect the socket if no authentication data is received, though. (but that's a good thing to show in the docs if people want to only allow auth'ed sockets.) Actually, why not make it a configuration option for the socket-io auth plugin? options.expireDisconnect = true We would still want to send a 'unauthorized' message back to the client, so the app could hide the private data or refresh if that's what they want. Maybe I'm overthinking that part.

@ekryski @daffl Am I overlooking something?

[daffl]

Is it really that expensive to do the token check? In a REST API we do the same thing all the time no? It might make sense to benchmark it before we make any workarounds for that.

[marshallswain]

https://www.cryptopp.com/benchmarks.html
Looks like on that machine you would have to have over 100 MB/sec traffic before you'd hit a bottleneck, assuming the server is ONLY performing hashing. We're running HMAC over SHA-256, by default, which there's not a benchmark for.

So really, we have a while before we need to concern ourselves with the hashing stuff. What do you think about getting rid of query string auth / the rest of what he says?

[ekryski]

I don't think we need to worry too much about checking the token on each request at this stage. Possibly in the future. The thing I'm more concerned about is do we want to support both methods of authentication via sockets:

1. passing a token via query string params for auth during handshake? (which is less secure)
2. passing the token in a socket message body or as part of the headers?

Looking at Auth0's implementation they support both. https://github.com/auth0/socketio-jwt

It's pretty much the same as that guy's implementation but how they treat namespaces for events is a little different.

I'm inclined to adopt best practices by default and not support passing the token by query string, or at least that not being the default behaviour.

[daffl]

They should probably only be passed via the headers or a socket call. We're promoting JWT as a more secure and compatible way to deal with APIs. Passing it as a query string goes a little against that. I think by now all web clients support setting custom headers.

I do think though that we have to check the token on each request (which can be easily done cross-provider as a service hook or as provider specific middleware). Imagine you have a long running connection e.g. with a mobile app and someone cancels their account or exceeds certain limits. You want to invalidate the token immediately and prevent subsequent requests not just throw an error when the user tries to reconnect.

[ekryski]

@daffl ya I agree. Right now I have it as provider specific middleware. Although I have a hunch it may turn into a hook.

[marshallswain]

Good point. I'd want to be able to boot a troublesome user immediately, not just when the token expires.

If we can auth on a header, let's do that. That's consistent with the Rest provider.

[marshallswain]

I vote we get rid of query string auth completely. I think the server gets to dictate the client, not the other way around. If there is some client library that won't work with it, it shouldn't be too hard for them to fix it or find a new one.

[ekryski]

Alright so on the decoupling branch I've implemented it so that it doesn't support authenticating via the socket handshake (aka. the query string method). As part of those changes it now also supports sending email/password or token for authenticating over sockets (#32).

[marshallswain]

👍 Awesome! I'm sure that was hard work. Thank you.

[ekryski]

So it turns out that we don't need to disconnect a socket if it doesn't connect within a certain time period like that article suggests. We might actually want sockets that don't need to be authenticated for certain endpoints.

We also don't need to do the namespace hacking because we can .filter() service events. So you can filter events from a service so that only auth'd users see them.

[marshallswain]

Makes sense to me.