Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add a spec for autonat #362

Merged
merged 5 commits into from Jan 14, 2022
Merged

add a spec for autonat #362

merged 5 commits into from Jan 14, 2022

Conversation

marten-seemann
Copy link
Contributor

No description provided.

autonat/README.md Outdated Show resolved Hide resolved
autonat/README.md Outdated Show resolved Hide resolved
Comment on lines +52 to +53
dial all of them in parallel. The peer MAY use a different IP and peer ID than
it uses for its regular libp2p connection to perform these dial backs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly the fact that a server might use a different peer ID is due to an implementation detail in the Golang implementation. Given that AutoNAT is already widely deployed we won't be able to change that in this iteration. Still, would you mind documenting this, to ease discussions on this in future versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we'd even want to change this in a future iteration, but I'm ok with documenting the reasoning here. Can you suggest some text?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we'd even want to change this in a future iteration

I might be missing something here. What is the benefit of this amendment other than not having to fix the shortcomings within a single implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree that this is a shortcoming to begin with.
All we're interested in is if an address is dialable - who dials it (and from what address) is completely irrelevant.

autonat/README.md Show resolved Hide resolved
autonat/README.md Show resolved Hide resolved
Note that in the current iteration of this protocol, a node doesn't check if
a peer's report of a successful dial is accurate. This might be solved in a
future iteration of this protocol, see
https://github.com/libp2p/go-libp2p-autonat/issues/10 for a detailed discussion.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏

Co-authored-by: Max Inden <mail@max-inden.de>
@mxinden mxinden linked an issue Sep 6, 2021 that may be closed by this pull request
@mxinden mxinden mentioned this pull request Sep 6, 2021
10 tasks
@mxinden
Copy link
Member

mxinden commented Sep 7, 2021

On address prioritization

I read quite a bunch of NAT traversal related RFCs (4787, 5128, 4614, 5382, 7604, 7857, 5389, 8445)

At the current state of Project Flare, I think the only thing useful would be RFC8445, more specifically section 5 on (address) candidate selection.

https://datatracker.ietf.org/doc/html/rfc8445#section-5

They differentiate in 4 candidate types, depending on the mechanism the address was discovered through:

  • host candidates
  • server-reflexive candidates
  • peer-reflexive candidates
  • relayed candidates

Section 5.1.2.1 then follows with a formula to prioritize among the many addresses depending on their type.

Would (a) the candidate type differentiation and (b) the proposed prioritization formula be something worth adopting in this specification or our implementations? What do you think @marten-seemann?

Document that peers MUST NOT dial addresses that are not based on the IP
addresses the requesting node is observed as.

Corresponding logic in Golang implementation:

https://github.com/libp2p/go-libp2p-autonat/blob/1247ac6d9fa798e7032127878a6f3d0b9eb487c6/svc.go#L133-L147

The AutoNAT Protocol uses the Protocol ID `/libp2p/autonat/1.0.0`. The node
wishing to determine its NAT status opens a stream using this protocol ID, and
then sends a `Dial` message. The `Dial` message contains a list of multiaddresses.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PeerInfo that is send in the Dial message also (may?) contain the target's peer ID. Should it be enforced that this Peer ID matches the sender's Peer ID?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect addresses in PeerInfo::addrs not to contain peer IDs (/p2p/QmXXX), given that the peer ID is already carried in PeerInfo::id.

That said, I would suggest to take the liberal approach of allowing addresses in PeerInfo::addrs to contain peer IDs anyways. Though in that case, I do agree that implementations should enforce the peer IDs to match.

Copy link
Contributor

@elenaf9 elenaf9 Oct 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect addresses in PeerInfo::addrs not to contain peer IDs (/p2p/QmXXX), given that the peer ID is already carried in PeerInfo::id.

Sorry I probably placed my comment wrong.
I was referring to PeerInfo::id, not PeerInfo::addrs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry, my bad.

The Golang implementation does the validation you suggest, namely validate that the peer ID in PeerInfo matches the remote's peer ID.

https://github.com/libp2p/go-libp2p-autonat/blob/005802887af9a40607fad1283f0f16dbf31e93a9/svc.go#L90-L100

To be honest, I don't know why we send the peer ID in the first place. Maybe @marten-seemann knows more.

@elenaf9
Copy link
Contributor

elenaf9 commented Nov 25, 2021

One case just popped up in my mind where I am not sure how to handle it for rust-autonat:
Let's assume Peer A wants to know from Peer B whether they (A) can be dialed directly. At the moment A is listening via a relay and A and B have already established a connection where B is the Dialer, therefore B observes A with a relay address.
If I understand the Golang implementation correctly, B would reject the DialRequest, and A would have to establish a new one to B where A is the dialer. Would it make sense that a DialRequest in general should only be sent on connections where the sender is also the dialer, so that the receiver observes the correct address?
And what should happen if B can not be dialed directly either?

@mxinden
Copy link
Member

mxinden commented Nov 26, 2021

Would it make sense that a DialRequest in general should only be sent on connections where the sender is also the dialer

In my eyes one should be able to send DialRequests both via a connection where one is the dialer and via a connection where one is the listener. Given that connection establishment is expensive, reusing existing connections, whether one is a dialer or listener, sounds reasonable.

so that the receiver observes the correct address?

I am not sure I follow. Both as a dialer and as a listener of a connection one knows the address of the remote peer. One might not observe the right port, though the AutoNAT protocol does not discriminate on the port, only on the IP. Am I missing something?

And what should happen if B can not be dialed directly either?

Off the top of my head, I don't think the overhead of establishing a new relayed connection is worth it, i.e. in case B can not be dialed directly, but only via a relay, I don't think they are a valid AutoNAT partner. I do think it is worth trying on an already established relayed connection.

@mxinden
Copy link
Member

mxinden commented Nov 26, 2021

And what should happen if B can not be dialed directly either?

Off the top of my head, I don't think the overhead of establishing a new relayed connection is worth it, i.e. in case B can not be dialed directly, but only via a relay, I don't think they are a valid AutoNAT partner. I do think it is worth trying on an already established relayed connection.

Sorry. This obviously conflicts with 048bdbf. My bad. One should not send nor accept DialRequests via relayed connections.

@elenaf9
Copy link
Contributor

elenaf9 commented Nov 28, 2021

I am not sure I follow. Both as a dialer and as a listener of a connection one knows the address of the remote peer. One might not observe the right port, though the AutoNAT protocol does not discriminate on the port, only on the IP. Am I missing something?

Maybe I understand the relay protocol wrong.
I assumed that if a peer A is listening via a relay, a remote observe them:

  • at the relayed address if A is the Listener -> remote would reject the autonat request
  • at a "direct"/ non-relayed address if A is a Dialer, because then the connection is not relayed -> remote would do the autonat request

Is this the case, or does the remote observe them at a relayed address in both cases?

@mxinden
Copy link
Member

mxinden commented Nov 29, 2021

I assumed that if a peer A is listening via a relay, a remote observe them:

  • at the relayed address if A is the Listener -> remote would reject the autonat request

Correct. The remote would observe A at /<relay-address>/p2p-circuit/p2p/<peer-id-of-A>.

  • at a "direct"/ non-relayed address if A is a Dialer, because then the connection is not relayed -> remote would do the autonat request

In the case where the remote is public and A can dial the remote directly, the remote would observe A as /<address-of-A>. The remote should then not reject the autonat request.

In the case where the remote is not publicly reachable and is as well listening via some relay, A would dial the remote via that relay. I don't think we defined how the remote would observe A. The remote would only learn the peer ID and not the address of A through the circuit relay v2 protocol:

the peer field contains a Peer struct with the peer ID of the connection initiator.

https://github.com/libp2p/specs/blob/master/relay/circuit-v2.md#stop-protocol

The remote could infer the address of A through some other means, e.g. retrieved via an identify protocol exchange. I would argue that the remote can not trust such address and thus should again reject the autonat request via the relayed address.

Does the above answer the question? Unfortunately this is not clearly defined. Thanks for raising it.

* autonat/README: Add table-of-content

* autonat/README: Describe stream lifecycle

* autonat/README: Wrap lines

* autonat/README.md: Remove stream closing item
In order to uphold the defense mechanism introduced in
048bdbf one should not accept dial requests via
relayed connections.
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would argue that this document is helpful as is and thus worth merging. Any objections @marten-seemann @elenaf9?

@mxinden mxinden marked this pull request as ready for review January 14, 2022 09:15
@mxinden
Copy link
Member

mxinden commented Jan 14, 2022

Discussed out of band that this is ready to merge.

@mxinden mxinden merged commit 8895aa0 into master Jan 14, 2022
@mxinden mxinden deleted the autonat branch January 14, 2022 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add spec for AutoNAT
3 participants