-
Notifications
You must be signed in to change notification settings - Fork 16
Conversation
I think we want to track reservations regardless of whether imposing limits and untag peers when they disconnect. edit: done. |
Such a quick turnaround ;-) I'm not so sure we need a |
So, the issue here is that there's no way to tell the relay "I'm using you as a relay for inbound connections, please don't kill this connection". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
many of these concepts already have precedent in the TURN spec (RFC5766) -- i think we should strive to align where possible, there's no reason to ignore prior art.
- RFC5766 calls this action an "Allocation".
- We should evaluate if we need a "refresh" message. We're primarily connection-oriented now and it may seem overkill, but that will change down the line, so let's plan for it now.
- We need a way for the server to terminate an allocation gracefully.
- We might want to introduce the concept of time -- leasing an allocation and renewing it.
We have agreed to be spec-first. We have to discuss this change in protocol in libp2p/specs. Note that other implementations are affected.
I personally think we need to revisit this protocol in several ways, including segregation in two:
- A
/libp2p/relay/allocate
protocol that NAT'ted peers use to acquire and renew a slot in a relay, enabling them to announce a relayed address for themselves. - A
/libp2p/relay/channel
protocol, used to open streams against NAT'ted peers.
For now, I suggest we recognise peers who need tagging by identifying HOP targets. Every time we process a valid HOP message, we increment the score of the target. Or we can enforce a ceiling of 1, if we want all NAT'ted peers to be treated equally.
^^ This approach does not require a change in protocol, and has identical effect, except that it leaves the connection vulnerable between its opening, and the time it receives its first incoming HOP. I think that's fair.
I think it is better to reserve with |
But what the reservation is doing in this PR (tagging in the conn manager) is achievable without a change in protocol. Protocol change here is overkill IMO. And it requires users to adopt this change, which won’t happen immediately. I suggest we tag peers like I suggested above, and defer allocations to a v2 of circuit relay, which we can start drafting in libp2p/specs straightaway. With a protocol this important, I prefer to take heavy steps than introduce piecemeal changes. |
Actually, the demand-based variant is more secure than the explicit RESERVE message in this PR. Isn’t it straightforward to max out a relay by creating a bunch of Sybils and sending RESERVE requests? |
The idea is to extend the implementation and impose resource limits, so that we can refuse reservations if we are over capacity. I am just not sure how to do it yet so deferred to simply tagging. |
Deployment should be pretty straightforward, we can have it to the majority of relay users with the next version of ipfs (and the relays can be deployed immediately). |
That's true, yet another instance of susceptibility to sybil attacks. |
Thinking this more through, the idea of demand-based tagging is not so bad. |
extra points of not needing client-side deployment. |
Exactly! We can increment the tag value every time the peer receives a new hop, with a ceiling to not overskew scoring. We’d still need to track which peers are targets (if we want to deny new hops by quota or something), but we need to do that anyway for observability purposes. |
BTW – I’d consider adding IncrTag and DecrTag in the connection manager so we can do that atomically. Although we can emulate the atomicity here with a global lock, assuming we’re the only ones using the tag (which we are). |
Yeah, been thinking about it. I don't think we need |
I am going to close this in favor of an on-demand based tagging approach. |
@vyzo what if we had a separate reserve protocol? We've talked about having a "go away" protocol and we could bake that in. |
Yeah, I think we do need a reservation protocol regardless, as it can provide immediate feedback about relay load; but let's discuss this in specs. |
RESERVE
, which signals to a hop relay that the peer wants to reserve the connection for relay.RESERVATION_REFUSED
, which can signal refusal to reserve.Reserve
, used to request a reservation in a relay.This initial implementation handles
RESERVE
trivially: it tags the connection in the connection manager and always succeeds.