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

Network discovery: store the remote address of the peer after finishing a successful handshake #2900

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

Eligioo
Copy link
Member

@Eligioo Eligioo commented Sep 9, 2024

What's in this pull request?

Previously we used the remote_addr directly from the handle_pending_inbound_connection to check if it's banned. However this will never work correctly because it also contains Multiaddr protocol parts like /tcp/{random port} and others. Since a new connection always gets opened on a new port you have a different Multiaddr on each connection thus never find a banned Multiaddr.

This PR adds storing a remote address in the PeerContactMeta that only contains a ip4, ip6, dns4 or dns6 part of a Multiaddr that can be used to correctly verify if IP or hostname is banned.

Lastly this PR solves the problem of banning observed peer addresses, which could lead to banning 127.0.0.1. Fixes #2896.

@Eligioo Eligioo force-pushed the stefan/ban-remote-addr-of-peer branch from 9288c48 to b7037e0 Compare September 9, 2024 15:34
@Eligioo Eligioo linked an issue Sep 10, 2024 that may be closed by this pull request
network-libp2p/src/discovery/handler.rs Outdated Show resolved Hide resolved
network-libp2p/src/discovery/peer_contacts.rs Outdated Show resolved Hide resolved
network-libp2p/src/discovery/peer_contacts.rs Outdated Show resolved Hide resolved
network-libp2p/src/discovery/peer_contacts.rs Outdated Show resolved Hide resolved
network-libp2p/src/discovery/peer_contacts.rs Outdated Show resolved Hide resolved
network-libp2p/src/connection_pool/behaviour.rs Outdated Show resolved Hide resolved
@Eligioo Eligioo force-pushed the stefan/ban-remote-addr-of-peer branch from b7037e0 to a9867f5 Compare October 2, 2024 15:12
@Eligioo Eligioo requested a review from jsdanielh October 2, 2024 15:14
…ng a successful handshake.

This remote address can later by used for various tasks, like banning.

Network connection pool: only look at a part of a Multiaddr to determine if an address is banned.
@jsdanielh jsdanielh force-pushed the stefan/ban-remote-addr-of-peer branch from a9867f5 to c7f80f6 Compare October 3, 2024 05:03
Copy link
Member

@jsdanielh jsdanielh left a comment

Choose a reason for hiding this comment

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

LGTM

@jsdanielh jsdanielh merged commit c7f80f6 into albatross Oct 3, 2024
7 checks passed
@jsdanielh jsdanielh deleted the stefan/ban-remote-addr-of-peer branch October 3, 2024 18:50
@jsdanielh jsdanielh added this to the Nimiq PoS Mainnet milestone Oct 8, 2024
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.

Network: banning incorrect IPs
2 participants