From ec3c2e89a4509eaa1900e9206534eeca71781592 Mon Sep 17 00:00:00 2001 From: Quentin Le Sceller Date: Mon, 28 May 2018 22:45:31 -0400 Subject: [PATCH] Store and communicate ban reason (#1085) * Add read_i32 and write_i32 * Store and send reason for ban * Client corruption ban reason * Forgotten max message size --- api/src/handlers.rs | 7 ++++--- core/src/ser.rs | 47 +++++++++++++++++++++++++++++---------------- p2p/src/lib.rs | 8 ++++---- p2p/src/msg.rs | 33 +++++++++++++++++++++++++++++-- p2p/src/peer.rs | 26 ++++++++++++++++++++++--- p2p/src/peers.rs | 33 ++++++++++++++++--------------- p2p/src/protocol.rs | 8 +++++++- p2p/src/store.rs | 14 ++++++++++---- p2p/src/types.rs | 13 +++++++++++++ 9 files changed, 140 insertions(+), 49 deletions(-) diff --git a/api/src/handlers.rs b/api/src/handlers.rs index 846e72618..78f635b48 100644 --- a/api/src/handlers.rs +++ b/api/src/handlers.rs @@ -29,6 +29,7 @@ use core::core::hash::{Hash, Hashed}; use core::core::{OutputFeatures, OutputIdentifier, Transaction}; use core::ser; use p2p; +use p2p::types::ReasonForBan; use pool; use regex::Regex; use rest::*; @@ -403,7 +404,7 @@ impl Handler for PeerPostHandler { "ban" => { path_elems.pop(); if let Ok(addr) = path_elems.last().unwrap().parse() { - w(&self.peers).ban_peer(&addr); + w(&self.peers).ban_peer(&addr, ReasonForBan::ManualBan); Ok(Response::with((status::Ok, ""))) } else { Ok(Response::with((status::BadRequest, ""))) @@ -530,8 +531,8 @@ impl Handler for ChainCompactHandler { /// GET /v1/blocks/ /// GET /v1/blocks/ /// -/// Optionally return results as "compact blocks" by passing "?compact" query param -/// GET /v1/blocks/?compact +/// Optionally return results as "compact blocks" by passing "?compact" query +/// param GET /v1/blocks/?compact pub struct BlockHandler { pub chain: Weak, } diff --git a/core/src/ser.rs b/core/src/ser.rs index 5a8b6b497..9b4869c22 100644 --- a/core/src/ser.rs +++ b/core/src/ser.rs @@ -19,18 +19,18 @@ //! To use it simply implement `Writeable` or `Readable` and then use the //! `serialize` or `deserialize` functions on them as appropriate. -use std::{cmp, error, fmt, mem}; -use std::io::{self, Read, Write}; use byteorder::{BigEndian, ByteOrder, ReadBytesExt}; -use keychain::{BlindingFactor, Identifier, IDENTIFIER_SIZE}; use consensus; use consensus::VerifySortOrder; use core::hash::{Hash, Hashed}; -use util::secp::pedersen::Commitment; -use util::secp::pedersen::RangeProof; +use keychain::{BlindingFactor, Identifier, IDENTIFIER_SIZE}; +use std::io::{self, Read, Write}; +use std::{cmp, error, fmt, mem}; use util::secp::Signature; use util::secp::constants::{AGG_SIGNATURE_SIZE, MAX_PROOF_SIZE, PEDERSEN_COMMITMENT_SIZE, SECRET_KEY_SIZE}; +use util::secp::pedersen::Commitment; +use util::secp::pedersen::RangeProof; /// Possible errors deriving from serializing or deserializing. #[derive(Debug)] @@ -139,6 +139,13 @@ pub trait Writer { self.write_fixed_bytes(&bytes) } + /// Writes a u32 as bytes + fn write_i32(&mut self, n: i32) -> Result<(), Error> { + let mut bytes = [0; 4]; + BigEndian::write_i32(&mut bytes, n); + self.write_fixed_bytes(&bytes) + } + /// Writes a u64 as bytes fn write_u64(&mut self, n: u64) -> Result<(), Error> { let mut bytes = [0; 8]; @@ -177,6 +184,8 @@ pub trait Reader { /// Read a u64 from the underlying Read fn read_u64(&mut self) -> Result; /// Read a i32 from the underlying Read + fn read_i32(&mut self) -> Result; + /// Read a i64 from the underlying Read fn read_i64(&mut self) -> Result; /// first before the data bytes. fn read_vec(&mut self) -> Result, Error>; @@ -270,6 +279,9 @@ impl<'a> Reader for BinReader<'a> { fn read_u32(&mut self) -> Result { self.source.read_u32::().map_err(Error::IOErr) } + fn read_i32(&mut self) -> Result { + self.source.read_i32::().map_err(Error::IOErr) + } fn read_u64(&mut self) -> Result { self.source.read_u64::().map_err(Error::IOErr) } @@ -417,24 +429,25 @@ impl<'a> Writer for BinWriter<'a> { } macro_rules! impl_int { - ($int: ty, $w_fn: ident, $r_fn: ident) => { - impl Writeable for $int { - fn write(&self, writer: &mut W) -> Result<(), Error> { - writer.$w_fn(*self) - } - } + ($int:ty, $w_fn:ident, $r_fn:ident) => { + impl Writeable for $int { + fn write(&self, writer: &mut W) -> Result<(), Error> { + writer.$w_fn(*self) + } + } - impl Readable for $int { - fn read(reader: &mut Reader) -> Result<$int, Error> { - reader.$r_fn() - } - } - } + impl Readable for $int { + fn read(reader: &mut Reader) -> Result<$int, Error> { + reader.$r_fn() + } + } + }; } impl_int!(u8, write_u8, read_u8); impl_int!(u16, write_u16, read_u16); impl_int!(u32, write_u32, read_u32); +impl_int!(i32, write_i32, read_i32); impl_int!(u64, write_u64, read_u64); impl_int!(i64, write_i64, read_i64); diff --git a/p2p/src/lib.rs b/p2p/src/lib.rs index d9e184592..20280a13c 100644 --- a/p2p/src/lib.rs +++ b/p2p/src/lib.rs @@ -49,9 +49,9 @@ mod serv; mod store; pub mod types; -pub use serv::{DummyAdapter, Server}; -pub use peers::Peers; pub use peer::Peer; -pub use types::{Capabilities, ChainAdapter, Error, P2PConfig, PeerInfo, TxHashSetRead, - MAX_BLOCK_HEADERS, MAX_LOCATORS, MAX_PEER_ADDRS}; +pub use peers::Peers; +pub use serv::{DummyAdapter, Server}; pub use store::{PeerData, State}; +pub use types::{Capabilities, ChainAdapter, Error, P2PConfig, PeerInfo, ReasonForBan, + TxHashSetRead, MAX_BLOCK_HEADERS, MAX_LOCATORS, MAX_PEER_ADDRS}; diff --git a/p2p/src/msg.rs b/p2p/src/msg.rs index 7d317a1a5..91b4001b4 100644 --- a/p2p/src/msg.rs +++ b/p2p/src/msg.rs @@ -14,11 +14,11 @@ //! Message types that transit over the network and related serialization code. +use num::FromPrimitive; use std::io::{self, Read, Write}; use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6, TcpStream}; use std::thread; use std::time; -use num::FromPrimitive; use core::consensus::{MAX_MSG_LEN, MAX_TX_INPUTS, MAX_TX_KERNELS, MAX_TX_OUTPUTS}; use core::core::BlockHeader; @@ -56,6 +56,7 @@ enum_from_primitive! { Shake, Ping, Pong, + BanReason, GetPeerAddrs, PeerAddrs, GetHeaders, @@ -72,12 +73,13 @@ enum_from_primitive! { } } -const MAX_MSG_SIZES: [u64; 18] = [ +const MAX_MSG_SIZES: [u64; 19] = [ 0, // Error 128, // Hand 88, // Shake 16, // Ping 16, // Pong + 64, // BanReason 4, // GetPeerAddrs 4 + (1 + 16 + 2) * MAX_PEER_ADDRS as u64, // PeerAddrs, with all IPv6 1 + 32 * MAX_LOCATORS as u64, // GetHeaders locators @@ -676,6 +678,33 @@ impl Readable for Pong { } } +#[derive(Debug)] +pub struct BanReason { + /// the reason for the ban + pub ban_reason: ReasonForBan, +} + +impl Writeable for BanReason { + fn write(&self, writer: &mut W) -> Result<(), ser::Error> { + let ban_reason_i32 = self.ban_reason as i32; + ban_reason_i32.write(writer).unwrap(); + Ok(()) + } +} + +impl Readable for BanReason { + fn read(reader: &mut Reader) -> Result { + let ban_reason_i32 = match reader.read_i32() { + Ok(h) => h, + Err(_) => 0, + }; + + let ban_reason = ReasonForBan::from_i32(ban_reason_i32).ok_or(ser::Error::CorruptedData)?; + + Ok(BanReason { ban_reason }) + } +} + /// Request to get an archive of the full txhashset store, required to sync /// a new node. pub struct TxHashSetRequest { diff --git a/p2p/src/peer.rs b/p2p/src/peer.rs index a6965da1c..2fd982d11 100644 --- a/p2p/src/peer.rs +++ b/p2p/src/peer.rs @@ -22,8 +22,8 @@ use core::core::hash::{Hash, Hashed}; use core::core::target::Difficulty; use handshake::Handshake; use msg; -use protocol::Protocol; use msg::*; +use protocol::Protocol; use types::*; use util::LOGGER; @@ -157,6 +157,25 @@ impl Peer { .send(ping_msg, msg::Type::Ping) } + /// Send the ban reason before banning + pub fn send_ban_reason(&self, ban_reason: ReasonForBan) { + let ban_reason_msg = BanReason { ban_reason }; + match self.connection + .as_ref() + .unwrap() + .send(ban_reason_msg, msg::Type::BanReason) + { + Ok(_) => debug!( + LOGGER, + "Sent ban reason {:?} to {}", ban_reason, self.info.addr + ), + Err(e) => error!( + LOGGER, + "Could not send ban reason {:?} to {}: {:?}", ban_reason, self.info.addr, e + ), + }; + } + /// Sends the provided block to the remote peer. The request may be dropped /// if the remote peer is known to already have the block. pub fn send_block(&self, b: &core::Block) -> Result<(), Error> { @@ -235,8 +254,9 @@ impl Peer { } } - /// Sends the provided stem transaction to the remote peer. The request may be - /// dropped if the remote peer is known to already have the stem transaction. + /// Sends the provided stem transaction to the remote peer. The request may + /// be dropped if the remote peer is known to already have the stem + /// transaction. pub fn send_stem_transaction(&self, tx: &core::Transaction) -> Result<(), Error> { if !self.tracking_adapter.has(tx.hash()) { debug!(LOGGER, "Send tx {} to {}", tx.hash(), self.info.addr); diff --git a/p2p/src/peers.rs b/p2p/src/peers.rs index a75665ca5..0627b1347 100644 --- a/p2p/src/peers.rs +++ b/p2p/src/peers.rs @@ -22,8 +22,8 @@ use rand::{thread_rng, Rng}; use core::core; use core::core::hash::{Hash, Hashed}; use core::core::target::Difficulty; -use util::LOGGER; use time; +use util::LOGGER; use peer::Peer; use store::{PeerData, PeerStore, State}; @@ -59,6 +59,7 @@ impl Peers { user_agent: p.info.user_agent.clone(), flags: State::Healthy, last_banned: 0, + ban_reason: ReasonForBan::None, }; if let Err(e) = self.save_peer(&peer_data) { error!(LOGGER, "Could not save connected peer: {:?}", e); @@ -205,8 +206,8 @@ impl Peers { } } - /// Return vec of connected peers that currently have the most worked branch, - /// showing the highest total difficulty. + /// Return vec of connected peers that currently have the most worked + /// branch, showing the highest total difficulty. pub fn most_work_peers(&self) -> Vec>> { let peers = self.connected_peers(); if peers.len() == 0 { @@ -235,8 +236,8 @@ impl Peers { max_peers } - /// Returns single random peer with the most worked branch, showing the highest total - /// difficulty. + /// Returns single random peer with the most worked branch, showing the + /// highest total difficulty. pub fn most_work_peer(&self) -> Option>> { match self.most_work_peers().first() { Some(x) => Some(x.clone()), @@ -253,8 +254,8 @@ impl Peers { false } - /// Bans a peer, disconnecting it if we're currently connected - pub fn ban_peer(&self, peer_addr: &SocketAddr) { + /// Ban a peer, disconnecting it if we're currently connected + pub fn ban_peer(&self, peer_addr: &SocketAddr, ban_reason: ReasonForBan) { if let Err(e) = self.update_state(peer_addr.clone(), State::Banned) { error!(LOGGER, "Couldn't ban {}: {:?}", peer_addr, e); } @@ -272,6 +273,7 @@ impl Peers { debug!(LOGGER, "Banning peer {}", peer_addr); // setting peer status will get it removed at the next clean_peer let peer = peer.write().unwrap(); + peer.send_ban_reason(ban_reason); peer.set_banned(); peer.stop(); } @@ -399,8 +401,8 @@ impl Peers { } } - /// Broadcasts the provided transaction to PEER_PREFERRED_COUNT of our peers. - /// We may be connected to PEER_MAX_COUNT peers so we only + /// Broadcasts the provided transaction to PEER_PREFERRED_COUNT of our + /// peers. We may be connected to PEER_MAX_COUNT peers so we only /// want to broadcast to a random subset of peers. /// A peer implementation may drop the broadcast request /// if it knows the remote peer already has the transaction. @@ -416,8 +418,8 @@ impl Peers { } } - /// Ping all our connected peers. Always automatically expects a pong back or - /// disconnects. This acts as a liveness test. + /// Ping all our connected peers. Always automatically expects a pong back + /// or disconnects. This acts as a liveness test. pub fn check_all(&self, total_difficulty: Difficulty, height: u64) { let peers_map = self.peers.read().unwrap(); for p in peers_map.values() { @@ -554,7 +556,7 @@ impl ChainAdapter for Peers { LOGGER, "Received a bad block {} from {}, the peer will be banned", hash, peer_addr ); - self.ban_peer(&peer_addr); + self.ban_peer(&peer_addr, ReasonForBan::BadBlock); false } else { true @@ -572,7 +574,7 @@ impl ChainAdapter for Peers { hash, &peer_addr ); - self.ban_peer(&peer_addr); + self.ban_peer(&peer_addr, ReasonForBan::BadCompactBlock); false } else { true @@ -583,7 +585,7 @@ impl ChainAdapter for Peers { if !self.adapter.header_received(bh, peer_addr) { // if the peer sent us a block header that's intrinsically bad // they are either mistaken or manevolent, both of which require a ban - self.ban_peer(&peer_addr); + self.ban_peer(&peer_addr, ReasonForBan::BadBlockHeader); false } else { true @@ -625,7 +627,7 @@ impl ChainAdapter for Peers { LOGGER, "Received a bad txhashset data from {}, the peer will be banned", &peer_addr ); - self.ban_peer(&peer_addr); + self.ban_peer(&peer_addr, ReasonForBan::BadTxHashSet); false } else { true @@ -661,6 +663,7 @@ impl NetAdapter for Peers { user_agent: "".to_string(), flags: State::Healthy, last_banned: 0, + ban_reason: ReasonForBan::None, }; if let Err(e) = self.save_peer(&peer) { error!(LOGGER, "Could not save received peer address: {:?}", e); diff --git a/p2p/src/protocol.rs b/p2p/src/protocol.rs index 21a603c76..8a8ff9d80 100644 --- a/p2p/src/protocol.rs +++ b/p2p/src/protocol.rs @@ -17,9 +17,9 @@ use std::fs::File; use std::net::SocketAddr; use std::sync::Arc; +use conn::*; use core::core; use core::core::hash::{Hash, Hashed}; -use conn::*; use msg::*; use rand; use rand::Rng; @@ -74,6 +74,12 @@ impl MessageHandler for Protocol { Ok(None) } + Type::BanReason => { + let ban_reason: BanReason = msg.body()?; + error!(LOGGER, "handle_payload: BanReason {:?}", ban_reason); + Ok(None) + } + Type::Transaction => { let tx: core::Transaction = msg.body()?; adapter.transaction_received(tx, false); diff --git a/p2p/src/store.rs b/p2p/src/store.rs index 9039c7024..be6b4d6db 100644 --- a/p2p/src/store.rs +++ b/p2p/src/store.rs @@ -14,14 +14,14 @@ //! Storage implementation for peer data. -use std::net::SocketAddr; use num::FromPrimitive; use rand::{thread_rng, Rng}; +use std::net::SocketAddr; use core::ser::{self, Readable, Reader, Writeable, Writer}; use grin_store::{self, option_to_not_found, to_key, Error}; use msg::SockAddr; -use types::Capabilities; +use types::{Capabilities, ReasonForBan}; use util::LOGGER; const STORE_SUBPATH: &'static str = "peers"; @@ -52,6 +52,8 @@ pub struct PeerData { pub flags: State, /// The time the peer was last banned pub last_banned: i64, + /// The reason for the ban + pub ban_reason: ReasonForBan, } impl Writeable for PeerData { @@ -62,7 +64,8 @@ impl Writeable for PeerData { [write_u32, self.capabilities.bits()], [write_bytes, &self.user_agent], [write_u8, self.flags as u8], - [write_i64, self.last_banned] + [write_i64, self.last_banned], + [write_i32, self.ban_reason as i32] ); Ok(()) } @@ -71,10 +74,12 @@ impl Writeable for PeerData { impl Readable for PeerData { fn read(reader: &mut Reader) -> Result { let addr = SockAddr::read(reader)?; - let (capab, ua, fl, lb) = ser_multiread!(reader, read_u32, read_vec, read_u8, read_i64); + let (capab, ua, fl, lb, br) = + ser_multiread!(reader, read_u32, read_vec, read_u8, read_i64, read_i32); let user_agent = String::from_utf8(ua).map_err(|_| ser::Error::CorruptedData)?; let capabilities = Capabilities::from_bits(capab).ok_or(ser::Error::CorruptedData)?; let last_banned = lb; + let ban_reason = ReasonForBan::from_i32(br).ok_or(ser::Error::CorruptedData)?; match State::from_u8(fl) { Some(flags) => Ok(PeerData { addr: addr.0, @@ -82,6 +87,7 @@ impl Readable for PeerData { user_agent: user_agent, flags: flags, last_banned: last_banned, + ban_reason: ban_reason, }), None => Err(ser::Error::CorruptedData), } diff --git a/p2p/src/types.rs b/p2p/src/types.rs index 2894ba3ca..044df4279 100644 --- a/p2p/src/types.rs +++ b/p2p/src/types.rs @@ -198,6 +198,19 @@ enum_from_primitive! { } } +/// Ban reason +enum_from_primitive! { + #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)] + pub enum ReasonForBan { + None = 0, + BadBlock = 1, + BadCompactBlock = 2, + BadBlockHeader = 3, + BadTxHashSet = 4, + ManualBan = 5, + } +} + /// General information about a connected peer that's useful to other modules. #[derive(Clone, Debug, Serialize, Deserialize)] pub struct PeerInfo {