libp2p / rust-libp2p

The Rust Implementation of the libp2p networking stack.
https://libp2p.io
MIT License
4.46k stars 927 forks source link

feat: gossipsub pre-validation checks on incoming message #5561

Open erhant opened 3 weeks ago

erhant commented 3 weeks ago

Description

Hi, I wanted to ask if its possible to do any type of message validation over the raw message prior to gossipsub validation (i.e. with Strict or `Permissive) takes place.

We currently have a custom app-level validation logic for message propagation (https://github.com/libp2p/rust-libp2p/blob/master/protocols/gossipsub/src/behaviour.rs#L749) but I think we should have one that takes place for incoming messages for internal protocol-level validation.

Motivation

The setting that I am in is related to discussion https://github.com/libp2p/rust-libp2p/discussions/5504, working together with @anilaltuner, we have many messages in the network that are no longer valid at "application level" but they keep being cached and validated at protocol level, increasing the memory usage & CPU usage.

This feature aims to target both, i.e. do not cache an invalid message / do not spend time checking signature if its not valid due to another reason.

CPU Issue

The issue first came to our eyes when the CPU usage skyrocketed due to old messages going around in the network. It is provable that this is the issue, using flamegraphs.

Here is a flamegraph under Permissive (also in Strict): flame-permissive

and here it is using None: flame-none

The custom message validation does not help with this due to it taking place AFTER the signature checks.

Using existing methods

I can think of two things that may alleviate the issue, but I believe both take place after sig checks:

[!NOTE]

An example for the second bullet, outbound_transform may add the timestamp at the first 8 bytes, and the inbound_transform may parse it and check that its not old, such as in an TTLDataTransform implementation. Such a method respects bijectivity as well, insofar that inbound and outbound transforms cancel eachother.

Sequence No

One possible configuration can be over sequence no as well, I believe it only matters that this is unique w.r.t client right? Currently, it uses a timestamp as seed and simply increments it. What if it used timestamp every time, and a TTL logic could be added over it within the message handler?

Perhaps a customization over how sequence no is constructed & validated can help us here as well, combined with changing the order of checks about signature & sequence no (i.e. https://github.com/libp2p/rust-libp2p/blob/master/protocols/gossipsub/src/protocol.rs#L298-L315 should happen AFTER https://github.com/libp2p/rust-libp2p/blob/master/protocols/gossipsub/src/protocol.rs#L318-L362).

Current Implementation

Signature checks take place at https://github.com/libp2p/rust-libp2p/blob/master/protocols/gossipsub/src/protocol.rs#L298, which is before other checks.

As mentioned above, sequence no check happens after siganture checks, and it seems that the returned RawMessages within sequence no checks also include message.signature although a comment says dont inform the application.

Are you planning to do it yourself in a pull request ?

Yes

erhant commented 3 weeks ago

I should mention that what we really just want in the end is to ignore historic messages all together, for our use-case, it suffices to just send and receive messages among live nodes, with propagation in mind i guess a very short TTL for messages would work