Skip to content

Address translation doesn't work correctly with non-reused ephemeral ports #5820

@dmitry-markin

Description

@dmitry-markin

Summary

In libp2p-0.54.1 when handle_established_outbound_connection() of Identify protocol is called it seems port_use argument is not set to PortUse::New at least in some cases when ephemeral port was actually used.

This is what comment in ConnectedPoint::Dialer says:

/// The port use is implemented on a best-effort basis. It is not guaranteed
/// that [`PortUse::Reuse`] actually reused a port. A good example is the case
/// where there is no listener available to reuse a port from.

This leads to Identify mistakenly thinking that we reused the port, not taking this branch in emit_new_external_addr_candidate_event() and emitting such ephemeral address without translation in ToSwarm::NewExternalAddrCandidate(observed.clone()).

This issue was originally reported in paritytech/polkadot-sdk#7207. It leads to ephemeral addresses being discovered as external in polkadot. For the context, polkadot does not use AutoNAT protocol and eagerly confirms all external address candidates provided by Identify behavior.

To obtain the logs below additional logging was added to Identify: dmitry-markin@4205675

Expected behavior

Only listen addresses reported by Identify for connections with actually reused port are emitted without address translation.

Actual behavior

Ephemeral addresses with not reused ports are emitted without address translation.

Relevant log output

>>> skipping translation for port-reuse address "/ip4/79.142.83.14/tcp/47936"
🔍 Discovered new external address for our node: /ip4/79.142.83.14/tcp/47936/p2p/12D3KooW9tGYAQmDAVUxsHMK9Xur3bkyNNiy7Mmv8uUh7UCzRFw2
>>> skipping translation for port-reuse address "/ip4/79.142.83.14/tcp/40506"
🔍 Discovered new external address for our node: /ip4/79.142.83.14/tcp/40506/p2p/12D3KooW9tGYAQmDAVUxsHMK9Xur3bkyNNiy7Mmv8uUh7UCzRFw2
>>> skipping translation for port-reuse address "/ip4/79.142.83.14/tcp/45810"
🔍 Discovered new external address for our node: /ip4/79.142.83.14/tcp/45810/p2p/12D3KooW9tGYAQmDAVUxsHMK9Xur3bkyNNiy7Mmv8uUh7UCzRFw2
>>> skipping translation for port-reuse address "/ip4/79.142.83.14/tcp/36244"
🔍 Discovered new external address for our node: /ip4/79.142.83.14/tcp/36244/p2p/12D3KooW9tGYAQmDAVUxsHMK9Xur3bkyNNiy7Mmv8uUh7UCzRFw2
...
(and so on)

Possible Solution

  1. Update Transport::Dial to yield both Transport::Output and PortUse.
  2. Update all transports' dial() to return the actual PortUse of the outbound socket created.
  3. Update pool::task::PendingConnectionEvent::ConnectionEstablished to include PortUse; update pool::PoolEvent to yield correct PortUse in ConnectedPoint.
  4. Update all affected types. This includes transport composition types like AndThenFuture and the such.

Version

0.54.1

Would you like to work on fixing this bug?

Maybe

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions