session hijacking? about peerjs

[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" [email protected] 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//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 [email protected] 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//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 [email protected] 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//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 [email protected] 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//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