search

CodelyTV / p2p-editor

Online code editor based on P2P and JavaScript. Demo:
http://p2p-editor.codely.tv
MIT License
57 stars 11 forks source link

Use UUIDs as session IDs #9

Open JavierCane opened 5 years ago

JavierCane commented 5 years ago

We have a restriction because we're using the same public key we use while communicating with all our peers.

We should refactor to have an execution flow like the following one:

  1. User A creates a new session: whatever.com/b3e2757a-d31f-41f7-ba3b-d0d18fd2fd46
  2. User B tries to connect to the user A session
  3. User A sends out his/her append only log public key to user B in order to replicate the DB
santakadev commented 5 years ago

We don't have any security layer, but I think that we should use as session ID something that is:

Right now sessions are short life living (in theory), so probably UUID is enough right now.

¿Any opinion about this?

JavierCane commented 5 years ago

Good point Mr. @santakadev 🎩🙂

UUIDs are specifically designed with these concerns in mind, so they will be more than enough 🤟

How would you suggest to approach the implementation of this issue?

I mean, it would be as simple as specifying the UUID generated while creating a new session, or received by URL while joining an existing one, as the hypercore ID while constructing the ChangeLog?

santakadev commented 5 years ago

Thinking about the possible implementation I've noticed that if we implement this we could introduce a security hole.

The thing is that right now, the session owner has to wait to hypercore being initialized to get its public key for use it as session ID.

This public key is so important, because hypercore owner will sign the database with its private key (that it's generated automatically when hypercore is initialized), and the rest of nodes will use the public key as a signature to ensure that writes comes from the owner.

If we use an UUID as session ID, we have the some advantages

But the problem is that peers need to receive the public key for BD replication / validation. The first thing that has come to my mind is: "well, when a peer is connected the first thing we should do is send to it the public key"

And here is the security hole. With the current implementation, when someone shares with you the session ID via Whatsapps for example, you are trusting that that key belongs to that person. So now, you are using that key as a signature for replication so you trust that that DB belongs to that person.

But, what happen if we wait for connection establishment for key sharing? Well, a malicious node, can modify P2P Editor in order generate its own DB for example, and share that fake key with peers is connected to. So now, if you trust in that UUID, you can't guarantee that the key/signature you are receiving belongs to session owner, meeek!

A possible solution is to have a centralised server in which all nodes trust that maps UUIDs to signatures. But, that makes architecture more complex, so I prefer not to implement this solution.

For now, I don't know another solution in order to implement this.

JavierCane commented 5 years ago

Good point. Let me describe a step by step example in order to figure out if I've understood you properly 👼

Current implementation (sharing Hypercore public ID as session ID)

Happy path

  1. User A opens a new session
  2. P2P Editor generates a Hypercore public key
  3. P2P Editor redirects user A to the URL corresponding to that Hypercore public key. That is, we're reusing the Hypercore public key as the session ID
  4. User A shares the session URL with user B
  5. User B joins the session
  6. P2P Editor connects to the Hypercore database using the session ID as the Hypercore public key

Malicious path

  1. Same from 1 to 5
  2. However, in this case we would be having a Man In The Middle (MITM) attack, so someone can intercept the Hypercore public key while B connects to A.
  3. The MITM has a modified version of P2P Editor where it connects all the received connections to a different Hypercore DB
  4. User B thinks to be seeing the user A session, but it's seeing the MITM one without noticing

Conclusion

santakadev commented 5 years ago

Am I missing something?

You get the point 👍

Just to clarify:

When you are connecting via webrtc-swarm, you can't ensure who is the session owner, so you will get the public key from any peer.

Any node, with a simple code change, can alter the session of nodes connecting to it. That nodes will also share the invalid key with new nodes connecting to the swarm.

Why the scenario you're concerned about couldn't happen with the current implementation?

Right now, this attack is not possible, because if someone generates a new hypercore DB an replicate its data, the peers recieving that data will use the public key that they own from the very begining to check the signature of the DB. They can ensure that data comming from that peer does not belongs to the public key they trust.

I think that signature checking is automatically done by hypercore. We should confirm it. :grimacing:

Is this the worst case scenario?

I think so.

santakadev commented 5 years ago

From https://www.datprotocol.com/deps/0002-hypercore/

To provide a persistent identifier for a Hypercore feed, we generate an asymmetric keypair. The public key of the keypair is used as the identifier. Any time a new root hash is generated, it is signed using the private key. This signature is distributed with the root hash to provide verification of its integrity.

JavierCane commented 5 years ago

I don't have enough knowledge about this topics neither consider this issue as a priority yet, do you?

Would you agree on removing this issue from the 1.1 milestone in order to do not be blocked by it?