identify: don't report listen addresses as external addresses

[thomaseizinger]

Description

At the moment, we concatenate all listen addresses and all external addresses for the identify payload:

rust-libp2p/protocols/identify/src/behaviour.rs

Lines 225 to 231 in 95890b5

	fn all_addresses(&self) -> HashSet<Multiaddr> {
	self.listen_addresses
	.iter()
	.chain(self.external_addresses.iter())
	.cloned()
	.collect()
	}

This is quite unnecessary because it very likely contains addresses that cannot be dialed by the remote peer, for example link-local addresses such as 127.0.0.1.

To mitigate this, we should remove the listen_addresses from libp2p-identify and instead only report our external addresses. For the rare case where the listen address is actually directly reachable via the public internet, users can manually add this address via Swarm::add_external_address.

[thomaseizinger]

It seems that go only adds the listen address of the current connection to it:

libp2p/go-libp2p@1f7b0d2/p2p/protocol/identify/id.go#L606-L625

@MarcoPolo Am I understand this correctly? Does go-libp2p not append the listen addresses of all network interfaces? Does it append external addresses confirmed via AutoNAT?

@marten-seemann Can you confirm this?

[marten-seemann]

Most commonly, you won't know the external address of an interface, since you'll be listening on 0.0.0.0 or its IPv6 equivalent. The only way to learn your actual address is by observing actual connections, either via Identify, or AutoNATv2.

[thomaseizinger]

Most commonly, you won't know the external address of an interface, since you'll be listening on 0.0.0.0 or its IPv6 equivalent. The only way to learn your actual address is by observing actual connections, either via Identify, or AutoNATv2.

Thanks! Does go-libp2p only share its actual (read: external) addresses in identify.listenAddresses? Or do you also include the listen addresses reported by the interfaces you are listening on?

[thomaseizinger]

I'd suggest we:

• Provide config option to not add listen addresses to this list
• Deprecate said config option in favor of it becoming the default in the next breaking release

That is backwards-compatible but allows users to not report thede (likely useless) addresses today.

I'll open an issue on specs to clarify this.

[thomaseizinger]

I'll open an issue on specs to clarify this.

See libp2p/specs#597.

[thomaseizinger]

Deemed invalid, see libp2p/specs#597.

[b-zee]

Sorry to dig up this closed issue, but perhaps it's worth re-opening.

Deemed invalid, see libp2p/specs#597.

I think it's still a good feature to implement, as mentioned later in that discussion (libp2p/specs#597 (comment)):

One aspect I do not see mentioned is privacy/security...

I don't think this is an issue the spec needs to define. Although I could see a version of the spec making recommendations.

This sounds like an implementation decision. Maybe bring up the ability to filter the advertised listenAddrs as a feature request to your preferred libp2p implementation? It seems like a reasonable request that wouldn't be hard to implement.

Currently, in our libp2p application, we're sometimes left scratching our heads when we're seeing these local addresses being 'dialed'. We have the global IP filter enabled so these local addresses end up with MultiaddrNotSupported, but it's not the cleanest.

Adding a send_listen_addresses config value to identify and defaulting to current behavior might be an option that wouldn't disrupt current behavior in local networks.