[Bug] Malicious peers can still send spam connection attempts #3311
Comments
|
The BFT module also has the same problem: snarkOS/node/bft/src/gateway.rs Lines 1262 to 1282 in cd73c74 |
|
I checked the source code and found that the limit is only checked for actively connected peers, not for passively connected peers. The experiment is as follows:
// Send a challenge request to the peer.
debug!("Send challenge request, port: {}, type: {}, address: {}, nonce: {}", self.local_ip().port(), self.node_type, self.address(), our_nonce);
let our_request = ChallengeRequest::new(self.local_ip().port(), self.node_type, self.address(), our_nonce);
sleep(Duration::from_millis(3100)).await;
send(&mut framed, peer_addr, Message::ChallengeRequest(our_request)).await?;
|
|
I've taken a look at this and I agree, this isn't ideal. However, finding a good solution isn't straightforward and isn't only a technical decision. For now, here are some thoughts and observations (cc @vicsn). On the gateway side, we don't have any notion of a restricted peer. Designing one raises questions about how to handle malicious behaviour in the validator set (aka slashing) and this is something we don't have a fleshed out solution for yet. On the router side, we do have a restricted peer set. Unfortunately, as you point out in your description, we check that set after the Overall, I'd be in favour of integrating these cases into the design for slashing so we have a consistent solution over both network stacks. Edit: it was brought to my attention that this may be a non-attributable attack and so slashing for it may not be ideal. |
|
Thank you for the thorough review @niklaslong ! Yeah this won't be fixable with slashing. We could tackle the Router issue first, assume good intent from validators for now.
I may be missing networking knowledge, but if your peers are listening on the same IP as a malicious spamming peer, I would assume its their fault? And we should just block the whole IP? Could you briefly look into whether Bitcoin/Ethereum block entire IP addresses? |
|
Ethereum seems to ban the ip address, feasible in our case if we're fine with the implications this has for proxied pools of nodes. |
|
Sounds good @niklaslong @elderhammer unfortunately without demonstration that 1-2 attackers can actually fully DoS a validator, this is not a valid P1/P2, but I can imagine a small reward is appropriate for the great bug find and discussion. |
@vicsn What does '1-2 attackers' mean? Can you elaborate? The num_connecting function counts the number of IP+Port, not just the number of IPs. Lines 72 to 73 in 09aa62b Lines 147 to 150 in 09aa62b Lines 409 to 425 in 09aa62b |
In other words: my understanding is P1/P2 rewards are reserved if a single attacker would be able to bring down a validator. But given that validators have a hardcoded set of friendly peers, if they additionally connect with a single maliciour peer or even a few malicious peers, it would cost them some resources and hurt connectivity, but its not clear it would actually halt the network. |
Ok, I get it.
Do you mean the peer address set by the startup command? Let me share my experiment:
This experiment shows that it is possible for the network to be disrupted by spam attacks.
Please correct me if I missed anything in my experiment. |
|
Thank you for correcting me @elderhammer . Reformulating my understanding of the assumptions for a valid attack on validators:
However, its also very observable and preventable if the validator in question is e.g. hiding behind a well calibrated firewall or has good alerts on their number of connections. Given that the most vulnerable moment is during a restart, the main impact is delay of reconnection. Updating my personal recommendation to P2 / High |
馃悰 Bug Report
Malicious peers can still send spam connection attempts.
Steps to Reproduce
snarkOS/node/router/src/handshake.rs
Lines 208 to 223 in cd73c74
Test log:
Expected Behavior
Modify the restriction logic to prevent malicious peers from spamming connection attempts.
Your Environment
snarkOS Version: cd73c74
The text was updated successfully, but these errors were encountered: