protocol: detect and drop asshole peers

defined as those who don't relay blocks for now.

Using transactions would be much faster, but more error prone,
as well as exploit prone.

The score can be reused later to store in the peer list to
affect selection probability.

src/cryptonote_basic/connection_context.h

@@ -43,7 +43,8 @@ namespace cryptonote
   {
     cryptonote_connection_context(): m_state(state_before_handshake), m_remote_blockchain_height(0), m_last_response_height(0),
         m_last_request_time(boost::date_time::not_a_date_time), m_callback_request_count(0),
-        m_last_known_hash(crypto::null_hash), m_pruning_seed(0), m_rpc_port(0), m_rpc_credits_per_hash(0),  m_anchor(false) {}
+        m_last_known_hash(crypto::null_hash), m_pruning_seed(0), m_rpc_port(0), m_rpc_credits_per_hash(0), m_anchor(false),
+        m_waiting_for_block_deadline(std::numeric_limits<time_t>::max()), m_score(0) {}
 
     enum state
     {
@@ -66,7 +67,15 @@ namespace cryptonote
     uint16_t m_rpc_port;
     uint32_t m_rpc_credits_per_hash;
     bool m_anchor;
-    //size_t m_score;  TODO: add score calculations
+
+    // when we first get a block from a peer, we pick a random other peer,
+    // and we will not relay the block to it, but instead wait to see if we
+    // do get it from that peer. If we do not get this block after a set
+    // time delay, we subtract 1 to its score. If we do, we add 1.
+    // We ban peers with a score less than a threshold
+    cryptonote::blobdata m_waiting_for_block;
+    time_t m_waiting_for_block_deadline;
+    int64_t m_score;
   };
 
   inline std::string get_protocol_state_string(cryptonote_connection_context::state s)

src/cryptonote_core/cryptonote_core.cpp

@@ -1545,7 +1545,7 @@ namespace cryptonote
       for(auto& tx:  txs)
         arg.b.txs.push_back({tx, crypto::null_hash});
 
-      m_pprotocol->relay_block(arg, exclude_context);
+      m_pprotocol->relay_block(arg, exclude_context, boost::uuids::nil_uuid());
     }
     return true;
   }

src/cryptonote_protocol/cryptonote_protocol_handler.h

@@ -129,7 +129,7 @@ namespace cryptonote
     int handle_notify_get_txpool_complement(int command, NOTIFY_GET_TXPOOL_COMPLEMENT::request& arg, cryptonote_connection_context& context);
 		
     //----------------- i_bc_protocol_layout ---------------------------------------
-    virtual bool relay_block(NOTIFY_NEW_BLOCK::request& arg, cryptonote_connection_context& exclude_context);
+    virtual bool relay_block(NOTIFY_NEW_BLOCK::request& arg, cryptonote_connection_context& exclude_context, const boost::uuids::uuid &exclude_id);
     virtual bool relay_transactions(NOTIFY_NEW_TRANSACTIONS::request& arg, const boost::uuids::uuid& source, epee::net_utils::zone zone, relay_method tx_relay);
     //----------------------------------------------------------------------------------
     //bool get_payload_sync_data(HANDSHAKE_DATA::request& hshd, cryptonote_connection_context& context);
@@ -144,10 +144,13 @@ namespace cryptonote
     bool kick_idle_peers();
     bool check_standby_peers();
     bool update_sync_search();
+    bool check_bad_peers();
     int try_add_next_blocks(cryptonote_connection_context &context);
     void notify_new_stripe(cryptonote_connection_context &context, uint32_t stripe);
     void skip_unneeded_hashes(cryptonote_connection_context& context, bool check_block_queue) const;
     bool request_txpool_complement(cryptonote_connection_context &context);
+    void select_peer_for_testing(uint64_t block_height, const cryptonote::blobdata &block, const boost::uuids::uuid &except, boost::uuids::uuid &selected);
+    void check_tested_peer(cryptonote_connection_context& context, const cryptonote::blobdata &block);
 
     t_core& m_core;
 
@@ -163,6 +166,7 @@ namespace cryptonote
     epee::math_helper::once_a_time_seconds<30> m_idle_peer_kicker;
     epee::math_helper::once_a_time_milliseconds<100> m_standby_checker;
     epee::math_helper::once_a_time_seconds<101> m_sync_search_checker;
+    epee::math_helper::once_a_time_seconds<43> m_bad_peer_checker;
     std::atomic<unsigned int> m_max_out_peers;
     tools::PerformanceTimer m_sync_timer, m_add_timer;
     uint64_t m_last_add_end_time;

src/cryptonote_protocol/cryptonote_protocol_handler.inl

@@ -72,6 +72,8 @@
 #define PASSIVE_PEER_KICK_TIME (60 * 1000000) // microseconds
 #define DROP_ON_SYNC_WEDGE_THRESHOLD (30 * 1000000000ull) // nanoseconds
 #define LAST_ACTIVITY_STALL_THRESHOLD (2.0f) // seconds
+#define WAIT_FOR_BLOCK_TIME (20) // seconds
+#define DROP_PEERS_ON_SCORE -2
 
 namespace cryptonote
 {
@@ -427,7 +429,7 @@ namespace cryptonote
     template<class t_core>
     int t_cryptonote_protocol_handler<t_core>::handle_notify_new_block(int command, NOTIFY_NEW_BLOCK::request& arg, cryptonote_connection_context& context)
   {
-    MLOGIF_P2P_MESSAGE(crypto::hash hash; cryptonote::block b; bool ret = cryptonote::parse_and_validate_block_from_blob(arg.b.block, b, &hash);, ret, "Received NOTIFY_NEW_BLOCK " << hash << " (height " << arg.current_blockchain_height << ", " << arg.b.txs.size() << " txes)");
+    MLOGIF_P2P_MESSAGE(crypto::hash hash; cryptonote::block b; bool ret = cryptonote::parse_and_validate_block_from_blob(arg.b.block, b, &hash);, ret, context << "Received NOTIFY_NEW_BLOCK " << hash << " (height " << arg.current_blockchain_height << ", " << arg.b.txs.size() << " txes)");
     if(context.m_state != cryptonote_connection_context::state_normal)
       return 1;
     if(!is_synchronized() || m_no_sync) // can happen if a peer connection goes to normal but another thread still hasn't finished adding queued blocks
@@ -475,10 +477,18 @@ namespace cryptonote
       drop_connection_with_score(context, bvc.m_bad_pow ? P2P_IP_FAILS_BEFORE_BLOCK : 1, false);
       return 1;
     }
+
+    boost::uuids::uuid no_relay_connection_id = boost::uuids::nil_uuid();
+    if(bvc.m_added_to_main_chain || !bvc.m_marked_as_orphaned)
+    {
+      check_tested_peer(context, arg.b.block);
+      if (bvc.m_added_to_main_chain)
+        select_peer_for_testing(arg.current_blockchain_height - 1, arg.b.block, context.m_connection_id, no_relay_connection_id);
+    }
     if(bvc.m_added_to_main_chain)
     {
       //TODO: Add here announce protocol usage
-      relay_block(arg, context);
+      relay_block(arg, context, no_relay_connection_id);
     }else if(bvc.m_marked_as_orphaned)
     {
       context.m_needed_objects.clear();
@@ -498,7 +508,7 @@ namespace cryptonote
   template<class t_core>
   int t_cryptonote_protocol_handler<t_core>::handle_notify_new_fluffy_block(int command, NOTIFY_NEW_FLUFFY_BLOCK::request& arg, cryptonote_connection_context& context)
   {
-    MLOGIF_P2P_MESSAGE(crypto::hash hash; cryptonote::block b; bool ret = cryptonote::parse_and_validate_block_from_blob(arg.b.block, b, &hash);, ret, "Received NOTIFY_NEW_FLUFFY_BLOCK " << hash << " (height " << arg.current_blockchain_height << ", " << arg.b.txs.size() << " txes)");
+    MLOGIF_P2P_MESSAGE(crypto::hash hash; cryptonote::block b; bool ret = cryptonote::parse_and_validate_block_from_blob(arg.b.block, b, &hash);, ret, context << "Received NOTIFY_NEW_FLUFFY_BLOCK " << hash << " (height " << arg.current_blockchain_height << ", " << arg.b.txs.size() << " txes)");
     if(context.m_state != cryptonote_connection_context::state_normal)
       return 1;
     if(!is_synchronized() || m_no_sync) // can happen if a peer connection goes to normal but another thread still hasn't finished adding queued blocks
@@ -749,13 +759,21 @@ namespace cryptonote
           drop_connection_with_score(context, bvc.m_bad_pow ? P2P_IP_FAILS_BEFORE_BLOCK : 1, false);
           return 1;
         }
+
+        boost::uuids::uuid no_relay_connection_id = boost::uuids::nil_uuid();
+        if(bvc.m_added_to_main_chain || !bvc.m_marked_as_orphaned)
+        {
+          check_tested_peer(context, arg.b.block);
+          if (bvc.m_added_to_main_chain)
+            select_peer_for_testing(arg.current_blockchain_height - 1, arg.b.block, context.m_connection_id, no_relay_connection_id);
+        }
         if( bvc.m_added_to_main_chain )
         {
           //TODO: Add here announce protocol usage
           NOTIFY_NEW_BLOCK::request reg_arg = AUTO_VAL_INIT(reg_arg);
           reg_arg.current_blockchain_height = arg.current_blockchain_height;
           reg_arg.b = b;
-          relay_block(reg_arg, context);
+          relay_block(reg_arg, context, no_relay_connection_id);
         }
         else if( bvc.m_marked_as_orphaned )
         {
@@ -1652,6 +1670,7 @@ skip:
     m_idle_peer_kicker.do_call(boost::bind(&t_cryptonote_protocol_handler<t_core>::kick_idle_peers, this));
     m_standby_checker.do_call(boost::bind(&t_cryptonote_protocol_handler<t_core>::check_standby_peers, this));
     m_sync_search_checker.do_call(boost::bind(&t_cryptonote_protocol_handler<t_core>::update_sync_search, this));
+    m_bad_peer_checker.do_call(boost::bind(&t_cryptonote_protocol_handler<t_core>::check_bad_peers, this));
     return m_core.on_idle();
   }
   //------------------------------------------------------------------------------------------------------------------------
@@ -1681,6 +1700,45 @@ skip:
   }
   //------------------------------------------------------------------------------------------------------------------------
   template<class t_core>
+  bool t_cryptonote_protocol_handler<t_core>::check_bad_peers()
+  {
+    const uint64_t target = m_core.get_target_blockchain_height();
+    const uint64_t height = m_core.get_current_blockchain_height();
+    if (target > height) // if we're not synced yet, don't do it
+      return true;
+
+    MTRACE("Checking for bad peers...");
+    std::vector<boost::uuids::uuid> bad_peers;
+    const time_t now = time(NULL);
+    m_p2p->for_each_connection([&](cryptonote_connection_context& context, nodetool::peerid_type peer_id, uint32_t support_flags)->bool
+    {
+      if (context.m_waiting_for_block != "")
+      {
+        if (now >= context.m_waiting_for_block_deadline)
+        {
+          context.m_score -= 1;
+          MINFO(context << "We did not get the block we were waiting for in time from " << context.m_connection_id << ": bad peer: score decreased to " << context.m_score);
+          context.m_waiting_for_block = "";
+          context.m_waiting_for_block_deadline = std::numeric_limits<time_t>::max();
+        }
+      }
+      if (context.m_score <= DROP_PEERS_ON_SCORE)
+        bad_peers.push_back(context.m_connection_id);
+      return true;
+    });
+    for (const auto &uuid: bad_peers)
+    {
+      if (!m_p2p->for_connection(uuid, [&](cryptonote_connection_context& ctx, nodetool::peerid_type peer_id, uint32_t f)->bool{
+        MINFO(ctx << "dropping bad peer (score " << ctx.m_score << ")");
+        drop_connection_with_score(ctx, 5, false);
+        return true;
+      }))
+        MDEBUG("Failed to find peer we wanted to drop");
+    }
+    return true;
+  }
+  //------------------------------------------------------------------------------------------------------------------------
+  template<class t_core>
   bool t_cryptonote_protocol_handler<t_core>::update_sync_search()
   {
     const uint64_t target = m_core.get_target_blockchain_height();
@@ -1738,6 +1796,48 @@ skip:
   }
   //------------------------------------------------------------------------------------------------------------------------
   template<class t_core>
+  void t_cryptonote_protocol_handler<t_core>::select_peer_for_testing(uint64_t block_height, const cryptonote::blobdata &block, const boost::uuids::uuid &except, boost::uuids::uuid &selected)
+  {
+    std::vector<boost::uuids::uuid> peers;
+    m_p2p->for_each_connection([&](cryptonote_connection_context& ctx, nodetool::peerid_type peer_id, uint32_t support_flags)->bool{
+      if (ctx.m_connection_id == except)
+        return true;
+      if (ctx.m_remote_blockchain_height < block_height)
+        return true;
+      if (ctx.m_state < cryptonote_connection_context::state_normal)
+        return true;
+      if (ctx.m_waiting_for_block != "")
+        return true;
+      peers.push_back(ctx.m_connection_id);
+      return true;
+    });
+    if (peers.size() <= 2)
+      return;
+    const size_t idx = rand() % peers.size();
+    selected = peers[idx];
+    m_p2p->for_connection(selected, [&](cryptonote_connection_context& ctx, nodetool::peerid_type peer_id, uint32_t f)->bool{
+      ctx.m_waiting_for_block = block;
+      ctx.m_waiting_for_block_deadline = time(NULL) + WAIT_FOR_BLOCK_TIME;
+      MINFO(ctx << "We will be waiting to see if we get block " << block_height << " from peer " << selected);
+      return true;
+    });
+  }
+  //------------------------------------------------------------------------------------------------------------------------
+  template<class t_core>
+  void t_cryptonote_protocol_handler<t_core>::check_tested_peer(cryptonote_connection_context& context, const cryptonote::blobdata &block)
+  {
+    if (context.m_waiting_for_block == block)
+    {
+      MINFO(context << "Peer " << context.m_connection_id << " relayed the block we were waiting on");
+      context.m_score += 1;
+      if (context.m_score > 5) // prevent a node from being all nice for a while then switching to asshole
+        context.m_score = 5;
+      context.m_waiting_for_block = "";
+      context.m_waiting_for_block_deadline = std::numeric_limits<time_t>::max();
+    }
+  }
+  //------------------------------------------------------------------------------------------------------------------------
+  template<class t_core>
   int t_cryptonote_protocol_handler<t_core>::handle_request_chain(int command, NOTIFY_REQUEST_CHAIN::request& arg, cryptonote_connection_context& context)
   {
     MLOG_P2P_MESSAGE("Received NOTIFY_REQUEST_CHAIN (" << arg.block_ids.size() << " blocks");
@@ -2485,7 +2585,7 @@ skip:
   }
   //------------------------------------------------------------------------------------------------------------------------
   template<class t_core>
-  bool t_cryptonote_protocol_handler<t_core>::relay_block(NOTIFY_NEW_BLOCK::request& arg, cryptonote_connection_context& exclude_context)
+  bool t_cryptonote_protocol_handler<t_core>::relay_block(NOTIFY_NEW_BLOCK::request& arg, cryptonote_connection_context& exclude_context, const boost::uuids::uuid &exclude_id)
   {
     NOTIFY_NEW_FLUFFY_BLOCK::request fluffy_arg = AUTO_VAL_INIT(fluffy_arg);
     fluffy_arg.current_blockchain_height = arg.current_blockchain_height;    
@@ -2495,9 +2595,9 @@ skip:
 
     // sort peers between fluffy ones and others
     std::vector<std::pair<epee::net_utils::zone, boost::uuids::uuid>> fullConnections, fluffyConnections;
-    m_p2p->for_each_connection([this, &exclude_context, &fullConnections, &fluffyConnections](connection_context& context, nodetool::peerid_type peer_id, uint32_t support_flags)
+    m_p2p->for_each_connection([this, &exclude_context, &exclude_id, &fullConnections, &fluffyConnections](connection_context& context, nodetool::peerid_type peer_id, uint32_t support_flags)
     {
-      if (peer_id && exclude_context.m_connection_id != context.m_connection_id && context.m_remote_address.get_zone() == epee::net_utils::zone::public_)
+      if (peer_id && exclude_context.m_connection_id != context.m_connection_id && exclude_id != context.m_connection_id && context.m_remote_address.get_zone() == epee::net_utils::zone::public_)
       {
         if(m_core.fluffy_blocks_enabled() && (support_flags & P2P_SUPPORT_FLAG_FLUFFY_BLOCKS))
         {

src/cryptonote_protocol/cryptonote_protocol_handler_common.h

@@ -40,7 +40,7 @@ namespace cryptonote
   /************************************************************************/
   struct i_cryptonote_protocol
   {
-    virtual bool relay_block(NOTIFY_NEW_BLOCK::request& arg, cryptonote_connection_context& exclude_context)=0;
+    virtual bool relay_block(NOTIFY_NEW_BLOCK::request& arg, cryptonote_connection_context& exclude_context, const boost::uuids::uuid &exclude_id)=0;
     virtual bool relay_transactions(NOTIFY_NEW_TRANSACTIONS::request& arg, const boost::uuids::uuid& source, epee::net_utils::zone zone, relay_method tx_relay)=0;
     //virtual bool request_objects(NOTIFY_REQUEST_GET_OBJECTS::request& arg, cryptonote_connection_context& context)=0;
   };
@@ -50,7 +50,7 @@ namespace cryptonote
   /************************************************************************/
   struct cryptonote_protocol_stub: public i_cryptonote_protocol
   {
-    virtual bool relay_block(NOTIFY_NEW_BLOCK::request& arg, cryptonote_connection_context& exclude_context)
+    virtual bool relay_block(NOTIFY_NEW_BLOCK::request& arg, cryptonote_connection_context& exclude_context, const boost::uuids::uuid &exclude_id)
     {
       return false;
     }

tests/unit_tests/levin.cpp

@@ -38,6 +38,7 @@
 
 #include "byte_slice.h"
 #include "crypto/crypto.h"
+#include "cryptonote_basic/blobdatatype.h"
 #include "cryptonote_basic/connection_context.h"
 #include "cryptonote_config.h"
 #include "cryptonote_core/cryptonote_core.h"