You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Confusing: Is it true that this PR cannot prevent pubsub consensus spamming?
Other than that, two minor issues uncovered. Please check it out.
This PR is designed to prevent p2p attack
As far as I see, speaking of the current code.
In consensus validation, the signature field is never checked before deliver to consensus module. It is only checked in consensus module. (consensus/consensus_v2.go:46)
The validation in consensus validation (node/node.go:531) is only to check whether the bls public key is in the committee. This public key is very easy to be obtained by attacker. So the malicious attacker can easily construct a message to pass this validation.
There is no validation on Node protocol thus attacker can easily spam attack on this.
So an attacker can:
Easily spam attack on consensus layer with a public BLS key in committee. The message will pass validation and be further delivered to consensus module. Which is CPU consuming (consensus/consensus_v2.go:46).
Spam with Node message. For example, flooding the network with transactions with random content and invalid signature. This could easily drain the memory resource on validators in transaction pool.
As far as I see, these malicious peers should be stopped in network layer, before delivering to other modules, and blacklist these peers.
I would recommend the following fix in future PRs:
Add a middleware filtering logic in validation (node/node.go:531) to
Limit the request per minute to limit access based on peerId.
Filter out the blacklisted peerId.
Do all the sanity check logic in validation, including consensus sanity check, transaction signature checks. If fails the check, add the peerId to blacklist with timeout in middleware.
All nodes has the responsibility to check the validation of message before sending it to pubsub.
But yeah, the spamming shall be completely prevented for future PRs. For this one, it looks good to me.
node messages are just transactions and they are encoded using RLP, it could be very CPU intensive to decode RLP and do validation on every p2p messages, as there are many redundant messages.
sanity check in validation for all consensus messages are very costly. I've already tried to do signature validation as in original PR, but the CPU was tripled in normal condition. So, this will hugely increase the latency. Thus, I only do basic sanity check at p2p layer. Remember there are many redundant messages at p2p gossipsub.
As far as I see, speaking of the current code.
Node
protocol thus attacker can easily spam attack on this.So an attacker can:
As far as I see, these malicious peers should be stopped in network layer, before delivering to other modules, and blacklist these peers.
I would recommend the following fix in future PRs:
But yeah, the spamming shall be completely prevented for future PRs. For this one, it looks good to me.
Originally posted by @JackyWYX in #3114 (comment)
The text was updated successfully, but these errors were encountered: