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

Implementation considerations for dialing interface #562

Closed
thomaseizinger opened this issue Jul 20, 2023 · 14 comments
Closed

Implementation considerations for dialing interface #562

thomaseizinger opened this issue Jul 20, 2023 · 14 comments

Comments

@thomaseizinger
Copy link
Contributor

thomaseizinger commented Jul 20, 2023

Inspired by #559 (comment) and libp2p/rust-libp2p#4226, I'd be interested in having a discussion between the various implementations around what a "good" dialing interface looks like.

At the moment, rust-libp2p has:

  • Transport::dial
  • Transport::dial_as_listener (for hole-punching)

For AutoNATv2, we'd need something like Transport::dial_with_new_port to avoid accidental hole-punching during the dial-back. Additionally, we currently have a transport-level setting for TCP to enable port-reuse. That applies to all dials.

After thinking about how to implement this, I concluded that a better interface would be to remove the transport-level setting for port-reuse and instead allow protocols to choose on a per-connection basis, whether the connection should reuse a port.

That allows modules like dcutr to say that they definitely want port-reuse and the autonat module would do the opposite and always request a connection with an ephemeral port.
Knowing whether a particular connection reuses a port or not also allows identify to decide whether to report the observed address as an address candidate. (If we used an ephemeral port for the dial, there is no point in reporting the address observed by the remote because the port will be wrong.)

How are other implementations solving these issues? Did you come up with a similar abstraction?

cc @Menduist @marten-seemann

@Menduist
Copy link
Contributor

Menduist commented Aug 1, 2023

Currently, in nim-libp2p, we have a transport-level flag for port-reuse which is set to true if we detect that we are private

I didn't follow the evolution of AutoNATv2, but I think we assumed that when you are private, you won't be used as a AutoNAT server, since the peer would have to hole-punch with you to reach you anyway

@thomaseizinger
Copy link
Contributor Author

thomaseizinger commented Aug 1, 2023

I didn't follow the evolution of AutoNATv2, but I think we assumed that when you are private, you won't be used as a AutoNAT server, since the peer would have to hole-punch with you to reach you anyway

The problem arises that when you are using port-reuse and you try to detect whether or not you are private, you might be accidentally setting up a hole-punch with the AutoNAT server and thus wrongly assume that you are public when you are not.

@Menduist
Copy link
Contributor

Menduist commented Aug 1, 2023

Wouldn't it be the AutoNat server responsibility to use a random source port for the dial-back?
ie to HP both peer need to bind a port, so if public servers never bind, and we only use public servers as autonat servers, seems we are safe?

@sukunrt
Copy link
Member

sukunrt commented Aug 1, 2023

@Menduist, to elaborate on @thomaseizinger's point.
What you're suggesting works in most cases. But if the AutoNAT Client is behind a Address dependent NAT/firewall, the client will accidentally setup bindings to allow inbound packets from any port on the servers ip address.

From RFC 4787

Address-Dependent Mapping:

     The NAT reuses the port mapping for subsequent packets sent
     from the same internal IP address and port (X:x) to the same
     external IP address, regardless of the external port.
     Specifically, X1':x1' equals X2':x2' if and only if, Y2 equals
     Y1.

And on the filtering behaviour

Address-Dependent Filtering:

     The NAT filters out packets not destined to the internal
     address X:x.  Additionally, the NAT will filter out packets
     from Y:y destined for the internal endpoint X:x if X:x has not
     sent packets to Y:any previously (independently of the port
     used by Y).  In other words, for receiving packets from a
     specific external endpoint, it is necessary for the internal
     endpoint to send packets first to that specific external
     endpoint's IP address.

@thomaseizinger
Copy link
Contributor Author

Wouldn't it be the AutoNat server responsibility to use a random source port for the dial-back?

This still results in the same conclusion I wrote in the PR description, right? That reusing ports is a per-connection setting and not a transport-level setting.

@Menduist
Copy link
Contributor

Menduist commented Aug 1, 2023

This still results in the same conclusion I wrote in the PR description, right? That reusing ports is a per-connection setting and not a transport-level setting.

No, because if you are private, you disable AutoNat server, and enable port binding globally
And if you are public, you enable AutoNat server, and disable port binding globally
So you don't need per connection settings

But if the AutoNAT Client is behind a Address dependent NAT/firewall, the client will accidentally setup bindings to allow inbound packets from any port on the servers ip address.

I see, I wasn't aware of this NAT configuration. But this seem impractical, it means we have to setup special connections to autonat servers? (Sorry, maybe I need to look more into autonatv2 before commenting, please let me know if I'm asking dumb questions)

Not sure how it works in other libp2ps, but in nim we connect to random nodes, not knowing what their capabilities are, and then we might ask them to dial-back if we find that they support autonat. We currently don't know at time of dialing if we are going to use them as AutoNat server.

@sukunrt
Copy link
Member

sukunrt commented Aug 1, 2023

But this seem impractical, it means we have to setup special connections to autonat servers?

Yeah, I realised this 10 minutes after writing the comment 😅. What you've described for nim is the same behaviour in go. I think we need to change the recommendation to not reuse port on the server side as opposed to the current recommendation of not reusing port on client side.

@thomaseizinger
Copy link
Contributor Author

This still results in the same conclusion I wrote in the PR description, right? That reusing ports is a per-connection setting and not a transport-level setting.

No, because if you are private, you disable AutoNat server, and enable port binding globally
And if you are public, you enable AutoNat server, and disable port binding globally
So you don't need per connection settings

Does nim-libp2p support hole-punching? If you are trying to hole punch to a node behind NAT, you want to reuse your port, even if you are publicly reachable, or am I missing something?

@thomaseizinger
Copy link
Contributor Author

But this seem impractical, it means we have to setup special connections to autonat servers?

Yeah, I realised this 10 minutes after writing the comment 😅. What you've described for nim is the same behaviour in go. I think we need to change the recommendation to not reuse port on the server side as opposed to the current recommendation of not reusing port on client side.

I second this, I'd actually say we have to change the spec to say the dial back request MUST use a new port binding.

In rust-libp2p we were planning to reuse ports by default for new connections. This ensures that the observed address by identify has a stable port and we can use that information for hole punching.

For a dial back request, we would then explicitly make a connection with a new port binding.

@Menduist
Copy link
Contributor

Menduist commented Aug 1, 2023

Does nim-libp2p support hole-punching?

Yes

If you are trying to hole punch to a node behind NAT, you want to reuse your port, even if you are publicly reachable, or am I missing something?

The private node can just dial back, no need for hole-punching (even if he just tries to hole-punch, we don't have to create an outgoing connection for it to succeed, it will just work)

@thomaseizinger
Copy link
Contributor Author

Does nim-libp2p support hole-punching?

Yes

If you are trying to hole punch to a node behind NAT, you want to reuse your port, even if you are publicly reachable, or am I missing something?

The private node can just dial back, no need for hole-punching (even if he just tries to hole-punch, we don't have to create an outgoing connection for it to succeed, it will just work)

But for the hole-punch to succeed, the outgoing connection of the private node needs to have the public node's port as the destination right?

If the incoming connection from the public node has a different source port (i.e. not reusing the binding), that won't work, will it?

What am I missing?

@Menduist
Copy link
Contributor

Menduist commented Aug 1, 2023

If public node P wants to connect to private node N

He connects to N via relay, sends his public address via dcutr or identify, and N can connect to P directly, no need for P to establish a direct connection towards N

@thomaseizinger
Copy link
Contributor Author

Thanks! This conversation was very insightful. I've updated the issue in rust-libp2p that we should be smart about our default in regards to the port binding based on our external address.

@thomaseizinger
Copy link
Contributor Author

I'll close this as resolved, thanks everyone for the fruitful conversation :)

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

No branches or pull requests

3 participants