From 684e8164f5f176f286a55d8497a454e8397dc9a9 Mon Sep 17 00:00:00 2001 From: Ignotus Peverell Date: Wed, 9 Jan 2019 19:14:50 +0000 Subject: [PATCH 1/6] Break out of main peer loop on error --- p2p/src/serv.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/p2p/src/serv.rs b/p2p/src/serv.rs index 853bf6ad3..29d14affe 100644 --- a/p2p/src/serv.rs +++ b/p2p/src/serv.rs @@ -87,6 +87,7 @@ impl Server { let sc = stream.try_clone(); if let Err(e) = self.handle_new_peer(stream) { warn!("Error accepting peer {}: {:?}", peer_addr.to_string(), e); + break; } else { if let Ok(s) = sc { connected_sockets.insert(peer_addr, s); @@ -101,6 +102,7 @@ impl Server { } Err(e) => { warn!("Couldn't establish new client connection: {:?}", e); + break; } } if self.stop_state.lock().is_stopped() { From 7a44ee9aa076325b1086663de766a404559884da Mon Sep 17 00:00:00 2001 From: Ignotus Peverell Date: Wed, 9 Jan 2019 19:24:59 +0000 Subject: [PATCH 2/6] Force client conn shutdown on error --- p2p/src/serv.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/p2p/src/serv.rs b/p2p/src/serv.rs index 29d14affe..99a1ab828 100644 --- a/p2p/src/serv.rs +++ b/p2p/src/serv.rs @@ -86,12 +86,12 @@ impl Server { if !self.check_banned(&stream) { let sc = stream.try_clone(); if let Err(e) = self.handle_new_peer(stream) { + // in theory, should be shutdown when stream gets dropped, + // practically doesn't seem to be happening + stream.shutdown(Shutdown::Both); warn!("Error accepting peer {}: {:?}", peer_addr.to_string(), e); - break; - } else { - if let Ok(s) = sc { - connected_sockets.insert(peer_addr, s); - } + } else if let Ok(s) = sc { + connected_sockets.insert(peer_addr, s); } } // if any active socket not in our peers list, close it @@ -102,7 +102,6 @@ impl Server { } Err(e) => { warn!("Couldn't establish new client connection: {:?}", e); - break; } } if self.stop_state.lock().is_stopped() { From 26d250bbead9758f386df717dc652e3043344ee8 Mon Sep 17 00:00:00 2001 From: Ignotus Peverell Date: Wed, 9 Jan 2019 20:24:57 +0000 Subject: [PATCH 3/6] Fix borrow error --- p2p/src/serv.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/p2p/src/serv.rs b/p2p/src/serv.rs index 99a1ab828..a0cb73d3b 100644 --- a/p2p/src/serv.rs +++ b/p2p/src/serv.rs @@ -86,9 +86,6 @@ impl Server { if !self.check_banned(&stream) { let sc = stream.try_clone(); if let Err(e) = self.handle_new_peer(stream) { - // in theory, should be shutdown when stream gets dropped, - // practically doesn't seem to be happening - stream.shutdown(Shutdown::Both); warn!("Error accepting peer {}: {:?}", peer_addr.to_string(), e); } else if let Ok(s) = sc { connected_sockets.insert(peer_addr, s); @@ -176,13 +173,21 @@ impl Server { let total_diff = self.peers.total_difficulty(); // accept the peer and add it to the server map - let mut peer = Peer::accept( + let mut peer = match Peer::accept( &mut stream, self.capabilities, total_diff, &self.handshake, self.peers.clone(), - )?; + ) { + Ok(p) => p, + Err(e) => { + // in theory, should be shutdown when stream gets dropped, + // practically doesn't seem to be happening + let _ = stream.shutdown(Shutdown::Both); + return Err(e); + } + }; peer.start(stream); self.peers.add_connected(Arc::new(peer))?; Ok(()) From cf088f6f51f2b0ff61c1b944931cc91ff26579a8 Mon Sep 17 00:00:00 2001 From: Ignotus Peverell Date: Wed, 9 Jan 2019 20:31:09 +0000 Subject: [PATCH 4/6] Ban peers that fail handshake --- p2p/src/serv.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/p2p/src/serv.rs b/p2p/src/serv.rs index a0cb73d3b..8e8f0ec8b 100644 --- a/p2p/src/serv.rs +++ b/p2p/src/serv.rs @@ -28,7 +28,7 @@ use crate::core::pow::Difficulty; use crate::handshake::Handshake; use crate::peer::Peer; use crate::peers::Peers; -use crate::store::PeerStore; +use crate::store::{PeerStore, State}; use crate::types::{Capabilities, ChainAdapter, Error, NetAdapter, P2PConfig, TxHashSetRead}; use crate::util::{Mutex, StopState}; use chrono::prelude::{DateTime, Utc}; @@ -87,6 +87,7 @@ impl Server { let sc = stream.try_clone(); if let Err(e) = self.handle_new_peer(stream) { warn!("Error accepting peer {}: {:?}", peer_addr.to_string(), e); + let _ = self.peers.update_state(peer_addr, State::Banned); } else if let Ok(s) = sc { connected_sockets.insert(peer_addr, s); } From 6a244e904a6464ec15596e0599e0bd667e4caab3 Mon Sep 17 00:00:00 2001 From: Ignotus Peverell Date: Thu, 10 Jan 2019 01:22:48 +0000 Subject: [PATCH 5/6] Fix add_peer for ban, remove useless disconnect --- p2p/src/peers.rs | 16 ++++++++++++++++ p2p/src/serv.rs | 18 +++++------------- p2p/src/types.rs | 1 + 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/p2p/src/peers.rs b/p2p/src/peers.rs index 13a12e9f6..1dcd0d90b 100644 --- a/p2p/src/peers.rs +++ b/p2p/src/peers.rs @@ -80,6 +80,22 @@ impl Peers { Ok(()) } + /// Add a peer as banned to block future connections, usually due to failed + /// handshake + pub fn add_banned(&self, addr: SocketAddr, ban_reason: ReasonForBan) -> Result<(), Error> { + let peer_data = PeerData { + addr, + capabilities: Capabilities::UNKNOWN, + user_agent: "".to_string(), + flags: State::Banned, + last_banned: Utc::now().timestamp(), + ban_reason, + last_connected: Utc::now().timestamp(), + }; + debug!("Banning peer {}.", addr); + self.save_peer(&peer_data) + } + // Update the dandelion relay pub fn update_dandelion_relay(&self) { let peers = self.outgoing_connected_peers(); diff --git a/p2p/src/serv.rs b/p2p/src/serv.rs index 8e8f0ec8b..17644888d 100644 --- a/p2p/src/serv.rs +++ b/p2p/src/serv.rs @@ -28,8 +28,8 @@ use crate::core::pow::Difficulty; use crate::handshake::Handshake; use crate::peer::Peer; use crate::peers::Peers; -use crate::store::{PeerStore, State}; -use crate::types::{Capabilities, ChainAdapter, Error, NetAdapter, P2PConfig, TxHashSetRead}; +use crate::store::PeerStore; +use crate::types::{Capabilities, ChainAdapter, Error, NetAdapter, P2PConfig, TxHashSetRead, ReasonForBan}; use crate::util::{Mutex, StopState}; use chrono::prelude::{DateTime, Utc}; @@ -87,7 +87,7 @@ impl Server { let sc = stream.try_clone(); if let Err(e) = self.handle_new_peer(stream) { warn!("Error accepting peer {}: {:?}", peer_addr.to_string(), e); - let _ = self.peers.update_state(peer_addr, State::Banned); + let _ = self.peers.add_banned(peer_addr, ReasonForBan::BadHandshake); } else if let Ok(s) = sc { connected_sockets.insert(peer_addr, s); } @@ -174,21 +174,13 @@ impl Server { let total_diff = self.peers.total_difficulty(); // accept the peer and add it to the server map - let mut peer = match Peer::accept( + let mut peer = Peer::accept( &mut stream, self.capabilities, total_diff, &self.handshake, self.peers.clone(), - ) { - Ok(p) => p, - Err(e) => { - // in theory, should be shutdown when stream gets dropped, - // practically doesn't seem to be happening - let _ = stream.shutdown(Shutdown::Both); - return Err(e); - } - }; + )?; peer.start(stream); self.peers.add_connected(Arc::new(peer))?; Ok(()) diff --git a/p2p/src/types.rs b/p2p/src/types.rs index 36a674bd3..09bb3d73b 100644 --- a/p2p/src/types.rs +++ b/p2p/src/types.rs @@ -244,6 +244,7 @@ enum_from_primitive! { BadTxHashSet = 4, ManualBan = 5, FraudHeight = 6, + BadHandshake = 7, } } From f86c90ade59bb857ea247763651ddaa6a993a307 Mon Sep 17 00:00:00 2001 From: Ignotus Peverell Date: Thu, 10 Jan 2019 01:23:02 +0000 Subject: [PATCH 6/6] rustfmt --- p2p/src/serv.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/p2p/src/serv.rs b/p2p/src/serv.rs index 17644888d..baaf333e5 100644 --- a/p2p/src/serv.rs +++ b/p2p/src/serv.rs @@ -29,7 +29,9 @@ use crate::handshake::Handshake; use crate::peer::Peer; use crate::peers::Peers; use crate::store::PeerStore; -use crate::types::{Capabilities, ChainAdapter, Error, NetAdapter, P2PConfig, TxHashSetRead, ReasonForBan}; +use crate::types::{ + Capabilities, ChainAdapter, Error, NetAdapter, P2PConfig, ReasonForBan, TxHashSetRead, +}; use crate::util::{Mutex, StopState}; use chrono::prelude::{DateTime, Utc};