ssbc / ssb-conn

SSB plugin for establishing and managing peer connections
MIT License
16 stars 5 forks source link

unexpected connection of unstaged peer #8

Open mixmix opened 4 years ago

mixmix commented 4 years ago

ssb-conn is connecting to a peer I’ve not followed yet!

How to reproduce:

cc @luandro who I think saw the same problem between 2 android phones both running ssb-conn + ssb-lan - when one peer follows the other, a connection instantly happens... and they might even start replicating both ways (Please add detail Luandro)

staltz commented 4 years ago

Thanks @mixmix for the careful explanation, I wanted to be sure what's going on, and I know what you're talking about.

TL;DR: this is not ssb-conn's fault, this is actually how secret-stack works, but I agree it shouldn't work like this and we need to fix it, somewhere in the stack.

This just "looks" unexpected or undesired now because of staging. Essentially, in secret-stack and muxrpc, once you start a connection with another peer, that peer by default always accepts connections. This already happened between two (e.g.) patchbay or patchwork users using ssb-local, as soon as Alice starts a muxrpc connection with Bob in the same Wi-Fi, Bob will just by default accept that. With ssb-local, this didn't seem unexpected because there was no concept of staging, instead all Wi-Fi peers were expected to get connected no matter what.

ssb-incoming-guard is supposed to fix that for us, but I tried using that with ssb-conn and ssb-lan, and it didn't do anything. I still need to look into this carefully, because I think it's also possible that ssb-incoming-guard is bugged or needs to consider a new corner case.

Once ssb-incoming-guard is fixed (or we learn something entirely new about this situation), then that's still not the ideal situation. Suppose Alice and Bob are in the same LAN, and both of them have ssb-incoming-guard installed. Alice sees Bob in staging, and vice-versa. Alice is interested in connecting with Bob, so she clicks "connect", and Bob's muxrpc would reject that (because of ssb-incoming-guard), but more importantly Bob as a user would need some kind of visual indication that Alice was/is interesting in connecting. Essentially, connection should happen as a handshake, Alice should get some kind of notification "Bob wants to connect with you, do you allow?" and only after allowing would the connection occur. That's not the goal of ssb-incoming-guard though.

All that said, I think secret-stack already provides us good tools to build that. Essentially it should be some sbot.auth.hook, but this hook causes some UI notification to ask for the user's permission.

This issue affects not only LAN connections, but also Bluetooth and Room peers (whenever there is staging). I actually hinted about this exact same issue in the blog post announcing rooms:

Strangers can initiate connections with you. When in a room, you will see strangers and you can choose to connect with them. This also works the other way around: those strangers can choose to connect with you, and your app will suddenly show an active connection with them. Sometimes this can be undesired, given that they are strangers. So far we don't have a two-way consensual system, and in the future it will make sense to build a handshake system where the stranger requires your explicit permission before connecting to you. That said, you can always block any account to prevent them from connecting with you in the first place.


UPDATE: What happened in your example, and how it would have behaved under ssb-gossip

So, what happened in your example is that patchbay followed the android app, and this caused a connection between patchbay and the android app, allowing patchbay to fetch data from mobile.

Before, with ssb-gossip, mobile would not necessarily see patchbay as a connected peer, (here's the very important bit:) even though the connection actually existed. So in a sense ssb-gossip wasn't honest about all the connections. The gossip table only contained peers you approved or added or something like that, but it sort of ignores peers that connect to you. Specifically, these lines. In other words, with ssb-gossip installed on both sides, Alice can connect with Bob, get Bob's feed, but Bob would not even see Alice in ssb.gossip.peers() or any other gossip API. That to me is a bit worse than what ssb-conn does now.

staltz commented 4 years ago

ssb-incoming-guard is supposed to fix that for us, but I tried using that with ssb-conn and ssb-lan, and it didn't do anything. I still need to look into this carefully, because I think it's also possible that ssb-incoming-guard is bugged or needs to consider a new corner case.

About this, I just tried ssb-incoming-guard in manyverse and I figured why it wasn't working: you have to .use(require('ssb-friends')) strictly before .use(require('ssb-incoming-guard')). Then it works. And indeed in that situation the connection is correctly blocked. That's good news! And connection is allowed as soon as Alice follows Bob, then any connection from Bob to Alice will be accepted.

So I'm interested in building ssb-consensual-connection-guard (or whatever other name that makes sense) which stalls the incoming connection until the user explicitly approves this. Building the approval process in the UI will be an interesting challenge...

arj03 commented 4 years ago

@staltz great to hear you found the problem. In my browser thingy where I have tunnel chat working, I acually have an explicit accept protocol where:

1) You first explicitly accept any incoming chat connection 2) The other end connects to you explicitly by id 3) You have to accept the incoming connection before you can chat

This is all manual, would love to have this into a specific module. I have a feeling we need to untangle ssb-tunnel first, but I don't want to pile on any more stuff.

staltz commented 4 years ago

@arj03 Good to hear about your browser thingy!

I don't think we need to untangle ssb-tunnel first (we may, but not for this issue), I think it's just the matter of building a module that looks like ssb-incoming-guard, because it would be useful not just for tunnel, but also LAN and bluetooth.

arj03 commented 4 years ago

Yeah they should be orthogonal. Do you want this module to be both use hops and if outside then manual, or only manual? If it should use hops, then maybe it would be best to add the manual functionality to ssb-incoming-guard.

staltz commented 4 years ago

Interesting, yes, I think we could add the manual functionality to ssb-incoming-guard.

arj03 commented 4 years ago

Great. Maybe something with a handler and a list of ids with state, such as accepted, blocked and functionality to manipulate this list.