paritytech
Detect fake neighbor messages in GRANDPA
Is there an existing issue?
- [X] I have searched the existing issues
Experiencing problems? Have you tried our Stack Exchange first?
- [X] This is not a support question.
Description of bug
This is a follow-up to https://github.com/paritytech/substrate/pull/12462. #12462 fixes a bug https://github.com/paritytech/substrate/issues/12191 with duplicate GRANDPA neighbor messages increasing peer's reputation. Unfortunately, it's still possible to send GRANDPA neighbor packets with increasing commit_finalized_height. Such packets will pass validation logic and increase peer's reputation.
Steps to reproduce
No response
This could be improved but doing so requires adding some complexity that I'm not sure is worth it for now. Neighbor packets can be spoofed, either by increasing the commit_finalized_height or just claiming you are in a future set_id and/or a future round. In order to detect this we'd have to look at what the other peers are reporting, maybe calculate some median set_id/round and then identify who are the peers that fall outside this range. Once identified we could ask them for proof of their claims, i.e. if someone tells us that they're on round 100 while everyone else is on round 5, then they should be able to answer a catch-up request with the votes from round 98 and 99, if they can't answer that request then they're probably faking it. If they claim they have finalized block #1000 then they should be able to provide us with a finality proof. The problem is also that while we verify these things we might actually just be doing more work for the attacker, if we are fed more invalid proofs.
The worst thing that this kind of attack can do is take over peer slots with useless peers, I think this is a form of eclipse attack, where we connect to enough attacking nodes that we can no longer have a correct view of the network. Our current approach to help with this is to randomly disconnect to peers every now and then.
I updated the issue title and labels, I think this is something we might want to improve eventually, so might make sense to keep it in the tracker. Realistically I don't see us handling this anytime soon.