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

Sync observed addresses with port mapping configs #411

Closed
wants to merge 1 commit into from

Conversation

cannium
Copy link
Contributor

@cannium cannium commented Sep 7, 2018

The situation is found and described in ipfs/kubo#5411. The 4001 port on external IP address is mapped automatically when IPFS node try to connect to another node, so is observed by peers. But in-bound connections cannot use this port to connect. Turns out many NAT devices have asymmetric port mappings, we need to explicitly configure in-bound mappings even if out-bound connections have been established.

This patch

  • add observed port mappings not configured
  • removes port mappings not observed for a consecutive amount of times

depends on libp2p/go-libp2p-nat#8

Many NAT devices have asymmetric port mappings, we need to explicitly configure in-bound mappings even if out-bound connections have been established.

this patch
- add observed port mappings not configured
- removes port mappings not observed
@Stebalien
Copy link
Member

I'm pretty sure this isn't the right fix, observed addresses and explicit port mappings are pretty orthogonal features. Really, the observed address feature is a fallback for when port mappings don't work correctly. Basically, even when a router doesn't support explicit port mapping, it may predictably map internal ports to external ports and may (sometimes) allow new inbound connections through this mapping. Really, this almost never works but it's better than nothing.

Instead of adding/removing mappings, we should probably just stop advertising "observed" addresses when we notice that port mapping is working.

Additionally, we can't rely on a lack of connections to determine that the address is unreachable. For example, we support multiple transports but some of these may be rarely used. If we just say "port x isn't being used, stop advertising it", we may stop advertising a useful port.

However, there are a few things we can do to improve this situation:

Sound reasonable or am I missing something?

@cannium
Copy link
Contributor Author

cannium commented Sep 11, 2018

My patch could be a shortest path, but not necessarily a graceful path, as currently I don't know the intentions behind many features. Your logic is smooth, if I understand correctly, we need

  • modify go-nat to support explicit port mapping
  • the priority of port mapping tries: 1) map to a same port as internal listen address 2) map to a random port
  • record sources of addresses could be advertised
  • the priority of addresses to advertise: 1) public addresses if a node has; 2) public address configured by NAT port mapping; 3) observed address by most peers

There're some details I'd like to make sure, though.

  • Would libp2p/go-libp2p-autonat replace fd/go-nat? I forked fd/go-nat because it seems not active enough to process pull requests(namely Expose the ability to specify external port when adding port map fd/go-nat#5).
  • Do we have plans to support the so-called "NAT hole punching"? As you mentioned "may (sometimes) allow new inbound connections through this mapping". It might be better to implement it as a transport or something.
  • If I'd like to get involved, where to start?

@vyzo
Copy link
Contributor

vyzo commented Sep 11, 2018

  • autonat will provide an ambient service to detect the presence of NAT automatically -- see NAT Auto Discovery go-libp2p-autonat#1 for implementation.
  • we have discussed implementing a protocol with builtin NAT-hole punching, or add the latter to QUIC in some reasonable way. It's still some ways out, but this is definitely something we want long term.

@cannium
Copy link
Contributor Author

cannium commented Sep 11, 2018

@vyzo It seems go-libp2p-autonat is built on libp2p and maintains observed addresses. Originally this part of functionality is in libp2p. Am I right? But what "auto nat" stands for, it doesn't handle any port mappings.

@vyzo
Copy link
Contributor

vyzo commented Sep 11, 2018

It stands for "NAT auto detection".
The idea is that you use autonat to detect the presence of the NAT, and then decide what addresses to advertise to the network based on this information.
For instance, we want to start advertising relay addresses when NAT is detected.

@Stebalien
Copy link
Member

modify go-nat to support explicit port mapping

I don't think that's necessary (but I may be wrong). It would be sufficient to modify go-nat to simply try using the internal port as the external port before it starts trying random ports. That should be the simplest fix as it doesn't require any interface changes. That is, we'd insert a n.c.AddPortMapping(protocol, internalPort, internalPort, timoutInSeconds) line here. That way we:

  1. Try reusing an existing mapping.
  2. Fallback on trying to map (external addr, internal port) -> (internal addr, internal port). For example, 1.2.3.4:4001 -> 192.168.0.22:4001.
  3. Finally fall back on trying to map random external ports.

the priority of port mapping tries: 1) map to a same port as internal listen address 2) map to a random port

Exactly this. My point is that this doesn't even require allowing users to explicitly choose their port mappings. On the other hand, we may want to support that anyways.

Would libp2p/go-libp2p-autonat replace fd/go-nat? I forked fd/go-nat because it seems not active enough to process pull requests(namely fd/go-nat#5).

No. As you noted (#411 (comment)), it's just like our current observed address feature. However, the goal is to make it significantly more reliable by explicitly testing which observed addresses work and which ones don't.

Do we have plans to support the so-called "NAT hole punching"? As you mentioned "may (sometimes) allow new inbound connections through this mapping". It might be better to implement it as a transport or something.

As @vyzo says, we'd like to but that's a ways out. Really, the first step for better NAT traversal is getting relays working automatically. See: ipfs/kubo#4213

If I'd like to get involved, where to start?

I'd start with the go-nat change we've talked about (trying to use our internal port first, before falling back on random ports). FYI, I've now forked it to https://github.com/libp2p/go-nat. That is, fix #415.

Once that's fixed, I'd go after #416. That won't directly fix anything but will be a step in the right direction.

@vyzo thoughts? NAT stuff is really your territory.


And thanks for jumping in on this, @cannium. Reliable NAT traversal is one of the key features that's limiting the usefulness of libp2p.

@cannium
Copy link
Contributor Author

cannium commented Sep 13, 2018

It would be sufficient to modify go-nat to simply try using the internal port as the external port before it starts trying random ports. That should be the simplest fix as it doesn't require any interface changes.

My instinct was to separate mechanism(port mapping) and policy(external port assignment) to maintain a relatively stable dependency package. But as the requirements hold, future changes might be very rare, so I'm fine with it. I'll make a pull request to #415 soon.

@vyzo
Copy link
Contributor

vyzo commented Sep 13, 2018

I think we should integrate autonat asap, it's key to getting relay advertisement to work effectively.

One difficulty integrating is that the current autonat implementation depends on libp2p itself as it needs to construct a host to do the dialbacks, but perhaps we should build it in libp2p instead.
The other part is that we need autonat services, which would mean deployment to the bootstrappers.

In terms of approach for fixing our port mapping, adding some smarts to go-nat so that it tries to map the same port and then fallbacks to random ports is a good idea.

@vyzo
Copy link
Contributor

vyzo commented Sep 13, 2018

For hole punching, what I would like to see long-term is a UDP-based transport that integrates signalling through a third party to punch holes.
Perhaps we could modify QUIC to add the signalling, as this would save us a lot of work in terms of implementation.

@MikePadge
Copy link

I just ran into a weird issue that appears to be related to this. Running 'ipfs id' looks like every containerd subnet gateway that was already running on the local machine appears to be listening on 4001.

@Stebalien
Copy link
Member

Unless I'm forgetting something, this PR has been superseded.

@MikePadge, please file a new issue with more details so we can figure out what's going on.

@Stebalien Stebalien closed this Apr 24, 2019
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.

4 participants