Harden against finalized block spam

beacon_node/beacon_chain/src/block_verification.rs

@@ -169,6 +169,7 @@ pub enum BlockError<T: EthSpec> {
     WouldRevertFinalizedSlot {
         block_slot: Slot,
         finalized_slot: Slot,
+        block_root: Hash256,
     },
     /// The block conflicts with finalization, no need to propagate.
     ///
@@ -412,6 +413,19 @@ impl<T: EthSpec> std::fmt::Display for BlockError<T> {
     }
 }
 
+impl<T: EthSpec> BlockError<T> {
+    /// Return the block root of the block that failed processing if it was already computed.
+    ///
+    /// This is used to avoid some double block root computations, which represent a small DoS
+    /// vector.
+    pub fn failed_block_root(&self) -> Option<Hash256> {
+        match self {
+            Self::WouldRevertFinalizedSlot { block_root, .. } => Some(*block_root),
+            _ => None,
+        }
+    }
+}
+
 impl<T: EthSpec> From<BlockSignatureVerifierError> for BlockError<T> {
     fn from(e: BlockSignatureVerifierError) -> Self {
         match e {
@@ -708,9 +722,6 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
 
         let block_root = get_block_root(&block);
 
-        // Disallow blocks that conflict with the anchor (weak subjectivity checkpoint), if any.
-        check_block_against_anchor_slot(block.message(), chain)?;
-
         // Do not gossip a block from a finalized slot.
         check_block_against_finalized_slot(block.message(), block_root, chain)?;
 
@@ -937,8 +948,8 @@ impl<T: BeaconChainTypes> SignatureVerifiedBlock<T> {
             .fork_name(&chain.spec)
             .map_err(BlockError::InconsistentFork)?;
 
-        // Check the anchor slot before loading the parent, to avoid spurious lookups.
-        check_block_against_anchor_slot(block.message(), chain)?;
+        // Check the block against the finalized slot to prevent spurious lookups.
+        check_block_against_finalized_slot(block.message(), block_root, chain)?;
 
         let (mut parent, block) = load_parent(block_root, block, chain)?;
 
@@ -1515,19 +1526,6 @@ fn check_block_skip_slots<T: BeaconChainTypes>(
     Ok(())
 }
 
-/// Returns `Ok(())` if the block's slot is greater than the anchor block's slot (if any).
-fn check_block_against_anchor_slot<T: BeaconChainTypes>(
-    block: BeaconBlockRef<'_, T::EthSpec>,
-    chain: &BeaconChain<T>,
-) -> Result<(), BlockError<T::EthSpec>> {
-    if let Some(anchor_slot) = chain.store.get_anchor_slot() {
-        if block.slot() <= anchor_slot {
-            return Err(BlockError::WeakSubjectivityConflict);
-        }
-    }
-    Ok(())
-}
-
 /// Returns `Ok(())` if the block is later than the finalized slot on `chain`.
 ///
 /// Returns an error if the block is earlier or equal to the finalized slot, or there was an error
@@ -1553,6 +1551,7 @@ fn check_block_against_finalized_slot<T: BeaconChainTypes>(
         Err(BlockError::WouldRevertFinalizedSlot {
             block_slot: block.slot(),
             finalized_slot,
+            block_root,
         })
     } else {
         Ok(())

beacon_node/network/src/beacon_processor/worker/gossip_methods.rs

@@ -729,10 +729,11 @@ impl<T: BeaconChainTypes> Worker<T> {
             .verify_block_for_gossip(block.clone())
             .await;
 
-        let block_root = if let Ok(verified_block) = &verification_result {
-            verified_block.block_root
-        } else {
-            block.canonical_root()
+        let block_root = match &verification_result {
+            Ok(verified_block) => verified_block.block_root,
+            Err(e) => e
+                .failed_block_root()
+                .unwrap_or_else(|| block.canonical_root()),
         };
 
         // Write the time the block was observed into delay cache.
@@ -799,10 +800,8 @@ impl<T: BeaconChainTypes> Worker<T> {
                 return None;
             }
             Err(e @ BlockError::FutureSlot { .. })
-            | Err(e @ BlockError::WouldRevertFinalizedSlot { .. })
             | Err(e @ BlockError::BlockIsAlreadyKnown)
-            | Err(e @ BlockError::RepeatProposal { .. })
-            | Err(e @ BlockError::NotFinalizedDescendant { .. }) => {
+            | Err(e @ BlockError::RepeatProposal { .. }) => {
                 debug!(self.log, "Could not verify block for gossip. Ignoring the block";
                             "error" => %e);
                 // Prevent recurring behaviour by penalizing the peer slightly.
@@ -820,6 +819,23 @@ impl<T: BeaconChainTypes> Worker<T> {
                 self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);
                 return None;
             }
+            Err(e @ BlockError::WouldRevertFinalizedSlot { .. }) => {
+                // Ignore the block (per the spec), but apply a low-tolerance penalty. Peers that
+                // send us blocks prior to finalization on gossip are either severely faulty or
+                // malicious.
+                debug!(
+                    self.log,
+                    "Could not verify block for gossip. Ignoring the block";
+                    "error" => %e
+                );
+                self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);
+                self.gossip_penalize_peer(
+                    peer_id,
+                    PeerAction::LowToleranceError,
+                    "gossip_block_low",
+                );
+                return None;
+            }
             Err(e @ BlockError::StateRootMismatch { .. })
             | Err(e @ BlockError::IncorrectBlockProposer { .. })
             | Err(e @ BlockError::BlockSlotLimitReached)
@@ -835,7 +851,8 @@ impl<T: BeaconChainTypes> Worker<T> {
             | Err(e @ BlockError::InconsistentFork(_))
             | Err(e @ BlockError::ExecutionPayloadError(_))
             | Err(e @ BlockError::ParentExecutionPayloadInvalid { .. })
-            | Err(e @ BlockError::GenesisBlock) => {
+            | Err(e @ BlockError::GenesisBlock)
+            | Err(e @ BlockError::NotFinalizedDescendant { .. }) => {
                 warn!(self.log, "Could not verify block for gossip. Rejecting the block";
                             "error" => %e);
                 self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject);