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

Remove naughty peers from the DHT #629

Closed
AgeManning opened this issue Nov 25, 2019 · 5 comments
Closed

Remove naughty peers from the DHT #629

AgeManning opened this issue Nov 25, 2019 · 5 comments
Assignees
Milestone

Comments

@AgeManning
Copy link
Member

If a peer has performed a malicious action, they become banned for a period of time.

We should also remove them from the DHT if they exist in a particular bucket so that other nodes also don't find the malicious peer.

@AgeManning AgeManning self-assigned this Nov 25, 2019
@AgeManning
Copy link
Member Author

This is a two part issue.

Firstly, our peer management is quite basic. We ban and kick peers. See here: https://github.com/sigp/lighthouse/blob/enr-v0.2.0/beacon_node/eth2-libp2p/src/service.rs#L168
and here:
https://github.com/sigp/lighthouse/blob/enr-v0.2.0/beacon_node/eth2-libp2p/src/service.rs#L168

We should implement a method on our behaviour that tells discovery to remove the peer from our local routing table.

This is fine, but we would also need to modify our discv5 protocol itself to allow the application to remove peers from the local routing table. To do this, we'd need to add a public function to the behaviour not too dis-similar to the add_enr function, but instead of adding it we would need to remove it. The add function can be found here: https://github.com/sigp/rust-libp2p/blob/discv5/misc/discv5/src/behaviour.rs#L181

@AgeManning AgeManning added this to the v0.2.0 milestone May 19, 2020
@AgeManning
Copy link
Member Author

Discv5 now supports removing nodes from the routing table and adding peers to a ban list.

When a peer gets banned, it should be removed from the local routing table and added to the ban list.

When a peer becomes unbanned, it should be removed from the discv5 ban list.

See the discv5 struct in sigp/discv5 for public functions

@ghost ghost added A0 Networking labels Sep 9, 2020
@paulhauner
Copy link
Member

Hey @AgeManning do you think we need this before the sec review in Oct?

@AgeManning
Copy link
Member Author

Yeah, lets add it in. Its a small change

bors bot pushed a commit that referenced this issue Sep 25, 2020
## Issue Addressed

#629 

## Proposed Changes

This removes banned peers from the DHT and informs discovery to block the node_id and the known source IP's associated with this node. It has the capabilities of un banning this peer after a period of time. 

This also corrects the logic about banning specific IP addresses. We now use seen_ip addresses from libp2p rather than those sent to us via identify (which also include local addresses).
@divagant-martian
Copy link
Collaborator

closed by #1656

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants