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

Peer Blacklisting #883

Closed
aarshkshah1992 opened this issue Apr 13, 2020 · 6 comments
Closed

Peer Blacklisting #883

aarshkshah1992 opened this issue Apr 13, 2020 · 6 comments
Assignees

Comments

@aarshkshah1992
Copy link
Contributor

aarshkshah1992 commented Apr 13, 2020

We have use cases where a libp2p application wants to blacklist a peer if it acts "maliciously". If one app blacklists a peer, we probably want to impose a Host wide blacklisting on the peer. This means that we should reject all future outgoing/incoming connections with the peer. We should also send a Disconnect message as part of the Control protocol we plan to implement to inform the peer that we are blacklisting it.

Design Notes

  • A host that supports blacklisting should implement the following interface:
       type Blacklister interface {
            BlacklistPeer(p peer.ID)
            IsPeerBlacklisted(p peer.ID)
       } 
    
  • Applications that want to blacklist a peer can then check if the host supports blacklisting and make a blacklist call if it does. A blacklist call should:
    • Add the peer to the set of blacklisted peers.
    • Terminate all existing connections with the peer.
  • Before dialing a peer, the Host checks if the peer has been blacklisted and returns an error if it is so.
  • Before adding an inbound connection, the Swarm should do the same check.
  • We should probably inject the BlackLister in the Upgrader/Non Upgrade Transports so we can reject connections as soon as we know the peer identify.

Questions

  1. Should the blacklist be persistent across restarts ? Should there be a GC/timeout for a blacklisted peer ?
  2. Should we have an API to remove a peer from the blacklist. If yes, things get tricky as ONLY the application that initiated the blacklist should have the right to remove the blacklist.

Related: #872.

@vyzo
Copy link
Contributor

vyzo commented Apr 13, 2020

We are using a timecache in pubsub blacklisting, to basically limit the blacklist effect to something like 1 day.
Having an unbounded blacklist is a DoS waiting to happen, the attacker can keep generating sybils and getting them blacklisted until we run out of memory.

@raulk
Copy link
Member

raulk commented Apr 13, 2020

@aarshkshah1992

If one app blacklists a peer, we probably want to impose a Host wide blacklisting on the peer.

Does app mean protocol? If so, I don't think this is the design direction we should pursue. Protocols have a local view of the world, and should not be able to take Host-wide decisions. If they want to penalise a peer, they should decrease its score in the connection manager.

If by app you mean the business logic that actually uses libp2p, they would create and inject the ConnectionGater instance. That's the component that implements the blacklisting behaviour, so given they hold a pointer to the actual implementation, an interface should not be necessary.

@vyzo
Copy link
Contributor

vyzo commented Apr 13, 2020

Note that in pubsub the protocol never proactively blacklists peers -- it has to be the application that triggers it.

@raulk
Copy link
Member

raulk commented Apr 13, 2020

The way I'd like this to work:

  • Protocols are entitled to block peers locally, insofar that protocol is concerned. If gossipsub finds that peer A behaves poorly at pubsub, it can choose to never interact again with peer A for pubsub.
  • We can add a generic EvtProtocolPeerDecision event that allows protocols to broadcast when they're taking notable local decisions about peers:
type EvtProtocolPeerDecision struct {
  protocol protocol.ID
  peer     peer.ID
  decision Decision // enum: BLACKLISTED, THROTTLED, etc.
}
  • An application that weights pubsub behaviour highly can subscribe to decisions made by the relevant protocol, and use them to drive a global blacklist.

@aarshkshah1992
Copy link
Contributor Author

aarshkshah1992 commented Apr 13, 2020

@raulk @vyzo

I think I understand what you guys mean. My original proposal was to have the protocols drive the blacklist but I now see the perils of it. The connection gater work should take care of this functionality from the application's(the business logic that uses libp2p) POV.

The idea of an EvtProtocolPeerDecision makes sense to me and I'll close this issue after getting that event in.

@Stebalien Stebalien removed the feature label Mar 27, 2021
@marten-seemann
Copy link
Contributor

Closing this, since the connection gater solves this. Protocols are free to do whatever they like.

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

No branches or pull requests

5 participants