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

Disable Peer Exchange (PX) in Gossipsub #3500

Open
diegomrsantos opened this issue Sep 12, 2023 · 7 comments
Open

Disable Peer Exchange (PX) in Gossipsub #3500

diegomrsantos opened this issue Sep 12, 2023 · 7 comments

Comments

@diegomrsantos
Copy link

diegomrsantos commented Sep 12, 2023

After reviewing the Gossipsub v1.1 specification in the context of the Eth consensus network, we'd like to propose explicitly disabling the Peer Exchange (PX) feature.

PX was introduced in the Gossipsub v1.1 specification as a potential solution to a couple of issues #215 and #233. The primary motivation was to address the challenge posed by a "star" mesh topology where a central node might continuously attempt to prune, while peripheral nodes continually attempt to graft. Although PX does offer a way to potentially alleviate this problem by suggesting alternate peers to the pruned nodes, we aren't sure this is an actual issue for the network, and enabling this feature can increase network traffic and broaden the attack vector surface.

We elaborate more on this below:

Concerns:

  • Unverifiable Data: PX introduces data that can't be validated within the protocol. This allows a malicious entity to create and inject fake or malicious identities without any means of verification.

  • Message Size Concerns: We've observed prune messages with sizes reaching up to 9KB and encompassing 210 peerIDs. Should signed records be populated, the sizes could escalate significantly, leading to both efficiency and potential DoS concerns.

  • Privacy and Security Risks: PX publicizes all connected peers in a global topic, inadvertently marking them as potential targets for malicious attacks.

Ethereum's Bootstrap Mechanism:

Ethereum's consensus network has its established bootstrap and discovery mechanisms, using a combination of hardcoded bootnodes and discv5. These mechanisms are designed to ensure robust peer discovery without needing to lean on Gossipsub's PX feature. Given the inherent robustness of Ethereum's own discovery methods, PX might not be relevant, as mentioned here libp2p/specs#570 (comment).

Additionally, it appears that PX is currently enabled on Teku, and it might be worth reviewing its utility and potential implications on that client.

We welcome feedback and insights for further elaboration and discussion.

@diegomrsantos diegomrsantos changed the title Disable Peer Exchange (PX) in Gossipsub for Eth Consensus Specification Disable Peer Exchange (PX) in Gossipsub Sep 12, 2023
@Nashatyrev
Copy link
Member

The proposal looks reasonable to me.

I can confirm that Teku doesn't utilize this functionality. However it's enabled (by default) and indeed an (unbounded) list of peers is attached to the prune message.
I'm not sure whether any other clients utilize that mechanism.

The PX mechanism itself doesn't looks quite critical to me. It looks more like optimization effort for the Ethereum use case.

@dapplion
Copy link
Collaborator

Neither the Javascript nor the Rust libp2p implementations support PX, nor intend to do so (from some old conversations)

@wemeetagain
Copy link
Contributor

js-libp2p supports PX but we have it turned off in lodestar.
The behavior, when turned on, is to attempt connection to signed peer records, assuming we have stored multiaddrs for those peers already. But it stands, without hooking into peer discovery, the feature is kinda useless.

@diegomrsantos
Copy link
Author

Neither the Javascript nor the Rust libp2p implementations support PX, nor intend to do so (from some old conversations)

@dapplion could you please share relevant issues and PRs?

@dapplion
Copy link
Collaborator

dapplion commented Dec 1, 2023

Neither the Javascript nor the Rust libp2p implementations support PX, nor intend to do so (from some old conversations)

@dapplion could you please share relevant issues and PRs?

rs-libp2p, non implementation https://github.com/libp2p/rust-libp2p/blob/6d21e6ed7f208c1ed2ba840a739f2445faaa5e5a/protocols/gossipsub/src/protocol.rs#L458

Lodestar not enabling doPx flag https://github.com/ChainSafe/lodestar/blob/bc2ea44a5bb410c560e1c30b0f2d4290ec333d7d/packages/beacon-node/src/network/gossip/gossipsub.ts#L91

@ppopth
Copy link
Member

ppopth commented Dec 19, 2023

I thought it was disabled already, but yeah disabling it makes sense and there is no reason we want to use it.

@ppopth
Copy link
Member

ppopth commented Dec 19, 2023

  • Privacy and Security Risks: PX publicizes all connected peers in a global topic, inadvertently marking them as potential targets for malicious attacks.

Discv5 also publicizes all the peers, so I think this may not be an additional risk.

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

8 participants
@ppopth @wemeetagain @diegomrsantos @Nashatyrev @hwwhww @dapplion and others