session hijacking?

[hnry]

I looked over peer server, and it appears that there is the possibility of session hijacking?

It would not be hard to randomly guess an active ID of another peer.

Once they do, they could connect with that peer randomly. (Even if user developer has a confirmation for incoming connection, it's still weird to randomly receive a connect. But mainly this serves as confirmation that the ID is active).

Once you confirm the ID is active, you can override the existing Peer by using that ID (the peer server does nothing to stop this?). So any incoming connection meant for the existing Peer now go to you.

When you consider that existing Peers could be in the thousands, the success rate is much higher.

• IDs & tokens are too short
• IDs & tokens should not be generated client side
• use tokens as a CSRF token and they should be signed

Also, server needs to ensure IDs are unique.

[UPDATE: to be more in the form of a question / possibility]

[ericz]

I agree with these issues but if an Id is active, you shouldn't be able to override it by connecting to same Id? The server checks for id already active before accepting new connections unless the token is guessed? On Mar 4, 2013 9:03 AM, "lovebear" notifications@github.com wrote:

I looked over peer server, and it appears that there is the possibility of session hijacking.

It would not be hard to randomly guess an active ID of another peer.

Once they do, they could connect with that peer randomly. (Even if user developer has a confirmation for incoming connection, it's still weird to randomly receive a connect. But mainly this serves as confirmation that the ID is active).

Once you confirm the ID is active, you can override the existing Peer by using that ID (the peer server does nothing to stop this). So any incoming connection meant for the existing Peer now go to you.

When you consider that existing Peers could be in the thousands, the success rate is much higher.

• IDs & tokens are too short
• IDs & tokens should not be generated client side
• use tokens as a CSRF token and they should be signed

— Reply to this email directly or view it on GitHubhttps://github.com/peers/peerjs/issues/23 .

[hnry]

I haven't really tested it, I just looked around the server code..

But yes tokens can be easily guessed considering how short an ID & token are. Especially if you already know an ID is active, it's more reason to try to guess the token.

Also, while checking for active IDs by getting a connection to the Peer (PeerA), you get the PeerA's IP. There are ways to get the PeerA to timeout just long enough for the socket to be closed and the id to be released or the PeerA just disconnects on their on, and you constantly keep trying to use their ID. In which case a session hijack could happen then. PeerB was connected to PeerA, and tries to connect to PeerA again thinking it was a dropped 'call' or connection, but gets you instead. Basically the other concerning issue is IDs are released right away and not held for the possibility of reconnect or just for security purposes. (Which also can be avoided if the peer client could not set it's own ID)

Though anyway, all of this is just good security practice regardless.

[ericz]

Regarding https://github.com/peers/peerjs-server/blob/master/lib/server.js#L92

It only does checks if no client with that id already (in which case there is no session to hijack). In the session hijack case you describe, it only parses the query string, and then makes sure key, id, and token is provided, and then immediately checks if token is valid.

Regarding "Also, while checking for active IDs by getting a connection to the Peer (PeerA), you get the PeerA's IP. There are ways to get the PeerA to timeout just long enough for the socket to be closed and the id to be released or the PeerA just disconnects on their on, and you constantly keep trying to use their ID. In which case a session hijack could happen then. PeerB was connected to PeerA, and tries to connect to PeerA again thinking it was a dropped 'call' or connection, but gets you instead. Basically the other concerning issue is IDs are released right away and not held for the possibility of reconnect or just for security purposes."

This is possibility. Even if the client is released after X seconds instead of immediately, the same attack would exist if the PeerA chooses not to reconnect at all. I feel that the client should take care of this case as the server can only mitigate but not eliminate it.

We should probably make the tokens generated by clients longer, as that not being guessed is important. This is the same security mechanism as any session key + cookie in a lot of web frameworks so as long as the token is long enough, I think it should be ok.

Thoughts?

On Mon, Mar 4, 2013 at 9:46 AM, lovebear notifications@github.com wrote:

I haven't really tested it, I just looked around the server code..

But yes tokens can be easily guessed considering how short an ID & token are. Especially if you already know an ID is active, it's more reason to try to guess the token.

This is also a concerning issue is https://github.com/peers/peerjs-server/blob/master/lib/server.js#L92 You do assignment, or check a lot of things before you validate the token. Token should be validated first just in case.

Also, while checking for active IDs by getting a connection to the Peer (PeerA), you get the PeerA's IP. There are ways to get the PeerA to timeout just long enough for the socket to be closed and the id to be released or the PeerA just disconnects on their on, and you constantly keep trying to use their ID. In which case a session hijack could happen then. PeerB was connected to PeerA, and tries to connect to PeerA again thinking it was a dropped 'call' or connection, but gets you instead. Basically the other concerning issue is IDs are released right away and not held for the possibility of reconnect or just for security purposes.

Though anyway, all of this is just good security practice regardless.

— Reply to this email directly or view it on GitHubhttps://github.com/peers/peerjs/issues/23#issuecomment-14393882 .

510-691-3951 http://ericzhang.com

[hnry]

Yes the code line , I misread that one.

But my thoughts are everything I said in the original issue, plus also retaining IDs and not expiring them right away so they can't be reused (they're cheap so why not hold on to them for more than just a few seconds).

I don't think just making the tokens longer is good enough.

If you compare IDs in this case to cookies, cookies are long. And they are signed or hashed in some way.

I don't understand the purpose of the peer client generating IDs and tokens. This will prevent randomly guessing IDs. (Basically I know how exactly you generate your IDs and tokens, while if it was server side with a key not api key, I'd have no way of being able to know how you generate/hash/sign your IDs or tokens).

And another thing that comes to mind is, even with encryption on the connection, you are passing IDs and tokens in the clear through the uri.

[hnry]

To elaborate further on "Basically I know how exactly you generate your IDs and tokens, while if it was server side with a key not api key, I'd have no way of being able to know how you generate/hash/sign your IDs or tokens".

When I say guess IDs & tokens run your random ID or token generator, in less than a 100 tries or so you will get a duplicate. Because it's consistent, even if it's "random".

[ericz]

IDs and tokens are not generated on the server in order to minimize the amount of time it takes before a connection can be made upon page load. Otherwise one would have to wait for a server roundtrip to get the ID/token before taking further action.

Signing a session id really only prevents attacks where malicious user gains a session ID but not the entire cookie. If they get the entire cookie, they'd be able to reuse it anyways as it contains the valid signature. If someone wanted to brute force guess the session id, they might as well guess the hash of the id also, both are essentially random strings. The security here is just that it would be computationally prohibitive to guess both.

It doesn't really matter that the id/token is in the URI, cookies are sent in plaintext also. The only way to prevent sniffing then is HTTPS, which would also hide the URI query parameters.

Imagine if the PeerJS client also accepted a secret from the user ( or used another entropy source like mouse movement or something) and then signed the token with that secret without sharing the secret with anyone. If someone were to guess the token, (by running the generator 100 times), they would not be able to guess the signature for the token because the secret is never shared. In this scheme the server would not have to participate.

Generally I try to offload computation to client as much as possible. In the PeerJS case it also helps reduces latency at startup.

On Mon, Mar 4, 2013 at 11:02 AM, lovebear notifications@github.com wrote:

To elaborate further on "Basically I know how exactly you generate your IDs and tokens, while if it was server side with a key not api key, I'd have no way of being able to know how you generate/hash/sign your IDs or tokens".

When I say guess IDs & tokens run your random ID or token generator, in less than a 100 tries or so you will get a duplicate. Because it's consistent, even if it's "random".

— Reply to this email directly or view it on GitHubhttps://github.com/peers/peerjs/issues/23#issuecomment-14398208 .

510-691-3951 http://ericzhang.com

[hnry]

I don't see how that works out latency wise. So a client has to generate the ID & token to spare the server from doing it. But before the client can initiate a connection it has to do this. So you still lose x amount of microseconds either way. It doesn't actually save you any time. Either the server takes x amount of microseconds to do it, or the client has to before it initiates the connection.

And in your purposed scheme, again it doesn't actually save time because the client is blocked from starting the initial request anyway since it has to generate the ID & token either way. And it seems more complex than it needs to be.

And it doesn't need to require a round trip, you send back the ID & token on the next response that the server was going to do anyway.

And I'm not sure what you mean about Session ID, session ids are embedded in the cookie? But yes if they get the cookie it'll be valid anyway, but you can authenticate that the server issued it. This is opposed to just using a random string as an ID and seeing if it exists on the server's session store (like it is now) to verify if an ID is valid. I guess that doesn't matter as long as it's hashed in some way. Same goes for the token.

So neither can be easily duplicated by running some function a couple of times. Though the ID is more important than the token. A token is useless without an ID to begin with.

Cookies should not be transmitted in plain text. Though I'm wrong about the URI, I forgot the URI will be encrypted.

But anyway, it's up to you however you want to do this. I just think focusing on latency over basic or extra security doesn't make sense especially when you don't get any latency gain from doing it client side.

[ericz]

Server generated id/tokens would increase latency because when a client connects to the PeerServer, before it can start sending offers/ICE candidates, it must first ask the server for, and receive an id/token. This http request would take an entire round trip to the server, which could be XXms or more. By generating id/token on client side, you can start sending offers/ICE immediately, without having to first ask server for id/token and server to respond to that http request.

Essentially the savings is a round trip http request to server

On Mon, Mar 4, 2013 at 12:22 PM, lovebear notifications@github.com wrote:

I don't see how that works out latency wise. So a client has to generate the ID & token to spare the server from doing it. But before the client can initiate a connection it has to do this. So you still lose x amount of microseconds either way. It doesn't actually save you any time. Either the server takes x amount of microseconds to do it, or the client has to before it initiates the connection.

And in your purposed scheme, again it doesn't actually save time because the client is blocked from starting the initial request anyway since it has to generate the ID & token either way. And it seems more complex than it needs to be.

And it doesn't need to require a round trip, you send back the ID & token on the next response that the server was going to do anyway.

And I'm not sure what you mean about Session ID, session ids are embedded in the cookie? But yes if they get the cookie it'll be valid anyway, but you can authenticate that the server issued it. This is opposed to just using a random string as an ID and seeing if it exists on the server's session store (like it is now) to verify if an ID is valid. I guess that doesn't matter as long as it's hashed in some way. Same goes for the token.

So neither can be easily duplicated by running some function a couple of times. Though the ID is more important than the token. A token is useless without an ID to begin with.

Cookies should not be transmitted in plain text. Though I'm wrong about the URI, I forgot the URI will be encrypted.

But anyway, it's up to you however you want to do this. I just think focusing on latency over basic or extra security doesn't make sense especially when you don't get any latency gain from doing it client side.

— Reply to this email directly or view it on GitHubhttps://github.com/peers/peerjs/issues/23#issuecomment-14403034 .

510-691-3951 http://ericzhang.com

[hnry]

Closing issue since it seems mostly things are working as intended.

[ericz]

There is definitely an issue here -- security needs to be improved. I'll recreate an issue summarizing the actions discussed here and mark it as a bug