socketsupply / socket

A cross-platform runtime for Web developers to build desktop & mobile apps for any OS using any frontend library.
https://socketsupply.co/guides
Other
1.62k stars 78 forks source link

Subclutser peer list contains peers not in the subcluster #906

Open garth opened 9 months ago

garth commented 9 months ago

The subcluster peer list often contains peers that have not joined the subcluster. I guess that this might be because they are relaying, but if that is the case I think they should be hidden.

What OS are you using (uname -a, or Windows version)?

Darwin Garths-MacBook-Pro.local 23.2.0 Darwin Kernel Version 23.2.0: Wed Nov 15 21:53:18 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T6000 arm64

What version Socket Runtime are you using?

0.5.4 (97fa3f7c)
Installation path: /Users/garth/Projects/socket-editor/node_modules/.pnpm/@socketsupply+socket-darwin-arm64@0.5.4/node_modules/@socketsupply/socket-darwin-arm64/

I also tried this on the latest master branch but the results are the same.

0.5.4 (ef7fb555)
Installation path: /Users/garth/.local/share/socket/

What programming language are you using (C/C++/Go/Rust)?

node --version
v20.11.0

What did you expect to see and what you saw instead?

I would expect that the following would only list peers that have explicitly joined the subcluster.

console.log(
  'current peers',
  Array.from(subcluster.peers.values()).map((peer: any) => peer.peerId),
)

This list often includes additional peers that are not part of the subcluster.

garth commented 9 months ago

It's also worth noting that since we cannot currently use subcluster.emit the following will send messages to peers who should not receive it which could be a potential data leak/security issue?

for (const peer of this.subcluster.peers.values()) {
  peer.emit('message', data)
}
heapwolf commented 9 months ago

only peers with the sharedKey will be able to read the contents of the packet (of course you area able to do whatever cryptography you want to do on top of that). the peers listed on sub cluster should only be members of the sub cluster, if you're trying to create a setup where peers that are removed from the sub cluster are no longer able to read the data, that will be an application level security issue. The protocol can not realistically enforce application level security design.

garth commented 9 months ago

the peers listed on sub cluster should only be members of the sub cluster

The list of peers includes peerIds from outside the cluster.

heapwolf commented 9 months ago

They must have at one point been a member of the cluster though. When a #join event happens, the packet is checked for the corresponding subcluster, if in the case it is a match, the peer is added to the subcluster set.

https://github.com/socketsupply/socket/blob/master/api/stream-relay/sugar.js#L284