Security: Avoid reconnecting to peers that are likely unreachable

zebra-chain/src/serialization/date_time.rs

@@ -138,6 +138,14 @@ impl Duration32 {
         Duration32::from_minutes(hours.saturating_mul(60))
     }
 
+    /// Creates a new [`Duration32`] to represent the given amount of days.
+    ///
+    /// If the resulting number of seconds does not fit in a [`u32`], [`Duration32::MAX`] is
+    /// returned.
+    pub const fn from_days(days: u32) -> Self {
+        Duration32::from_hours(days.saturating_mul(24))
+    }
+
     /// Returns the number of seconds in this duration.
     pub fn seconds(&self) -> u32 {
         self.seconds

zebra-network/src/address_book/tests/prop.rs

@@ -33,4 +33,19 @@ proptest! {
             prop_assert!(duration_since_last_seen <= MAX_PEER_ACTIVE_FOR_GOSSIP);
         }
     }
+
+    /// Test if peers that are probably unreachable are not listed for a reconnection attempt.
+    #[test]
+    fn only_reachable_addresses_are_attempted(
+        local_listener in any::<SocketAddr>(),
+        addresses in vec(any::<MetaAddr>(), 0..MAX_META_ADDR),
+    ) {
+        zebra_test::init();
+
+        let address_book = AddressBook::new_with_addrs(local_listener, Span::none(), addresses);
+
+        for peer in address_book.reconnection_peers() {
+            prop_assert!(peer.is_probably_reachable(), "peer: {:?}", peer);
+        }
+    }
 }

zebra-network/src/constants.rs

@@ -78,6 +78,15 @@ pub const MIN_PEER_RECONNECTION_DELAY: Duration = Duration::from_secs(60 + 20 +
 /// within the last three hours."
 pub const MAX_PEER_ACTIVE_FOR_GOSSIP: Duration32 = Duration32::from_hours(3);
 
+/// The maximum duration since a peer was last seen to consider reconnecting to it.
+///
+/// Peers that haven't been seen for more than three days and that had its last connection attempt
+/// fail are considered to be offline and Zebra will stop trying to connect to them.
+///
+/// This is to ensure that Zebra can't have a denial-of-service as a consequence of having too many
+/// offline peers that it constantly and uselessly retries to connect to.
+pub const MAX_RECENT_PEER_AGE: Duration32 = Duration32::from_days(3);
+
 /// Regular interval for sending keepalive `Ping` messages to each
 /// connected peer.
 pub const HEARTBEAT_INTERVAL: Duration = Duration::from_secs(60);

zebra-network/src/meta_addr.rs

@@ -503,6 +503,7 @@ impl MetaAddr {
             && !self.has_connection_recently_responded()
             && !self.was_connection_recently_attempted()
             && !self.has_connection_recently_failed()
+            && self.is_probably_reachable()
     }
 
     /// Is the [`SocketAddr`] we have for this peer valid for outbound
@@ -530,6 +531,36 @@ impl MetaAddr {
         is_node && self.address_is_valid_for_outbound()
     }
 
+    /// Should this peer considered reachable?
+    ///
+    /// A peer is considered unreachable if the last connection attempt to it failed and the last
+    /// successful connection is more than 3 days ago.
+    ///
+    /// # Security
+    ///
+    /// This is used by [`Self::is_ready_for_connection_attempt`] so that Zebra stops trying to
+    /// connect to peers that are likely unreachable.
+    ///
+    /// The `untrusted_last_seen` time is used as a fallback time if the local node has never
+    /// itself seen the peer. If the reported last seen time is a long time ago, then the local
+    /// node will attempt to connect the peer once (because the peer state won't be
+    /// [`PeerAddrState::Failed`] before an attempt is made), and if that attempt fails it won't
+    /// try to connect ever again.
+    pub fn is_probably_reachable(&self) -> bool {
+        self.last_connection_state != PeerAddrState::Failed || self.last_seen_is_recent()
+    }
+
+    /// Was this peer last seen recently?
+    ///
+    /// Returns `true` if this peer was last seen at most
+    /// [`MAX_RECENT_PEER_AGE`][constants::MAX_RECENT_PEER_AGE] ago.
+    pub fn last_seen_is_recent(&self) -> bool {
+        match self.last_seen() {
+            Some(last_seen) => last_seen.saturating_elapsed() <= constants::MAX_RECENT_PEER_AGE,
+            None => false,
+        }
+    }
+
     /// Return a sanitized version of this `MetaAddr`, for sending to a remote peer.
     ///
     /// Returns `None` if this `MetaAddr` should not be sent to remote peers.

zebra-network/src/meta_addr/tests/prop.rs

@@ -15,11 +15,13 @@ use tokio::{runtime, time::Instant};
 use tower::service_fn;
 use tracing::Span;
 
-use zebra_chain::serialization::{canonical_socket_addr, ZcashDeserialize, ZcashSerialize};
+use zebra_chain::serialization::{
+    canonical_socket_addr, DateTime32, ZcashDeserialize, ZcashSerialize,
+};
 
 use super::check;
 use crate::{
-    constants::MIN_PEER_RECONNECTION_DELAY,
+    constants::{MAX_RECENT_PEER_AGE, MIN_PEER_RECONNECTION_DELAY},
     meta_addr::{
         arbitrary::{MAX_ADDR_CHANGE, MAX_META_ADDR},
         MetaAddr, MetaAddrChange,
@@ -490,4 +492,41 @@ proptest! {
             prop_assert!(max_attempts - min_attempts <= 1);
         }
     }
+
+    /// Make sure check if a peer was recently seen is correct.
+    #[test]
+    fn last_seen_is_recent_is_correct(peer in any::<MetaAddr>()) {
+        let time_since_last_seen = peer
+            .last_seen()
+            .map(|last_seen| last_seen.saturating_elapsed());
+
+        let recently_seen = time_since_last_seen
+            .map(|elapsed| elapsed <= MAX_RECENT_PEER_AGE)
+            .unwrap_or(false);
+
+        prop_assert_eq!(
+            peer.last_seen_is_recent(),
+            recently_seen,
+            "last seen: {:?}, now: {:?}",
+            peer.last_seen(),
+            DateTime32::now(),
+        );
+    }
+
+    /// Make sure a peer is correctly determined to be probably reachable.
+    #[test]
+    fn probably_rechable_is_determined_correctly(peer in any::<MetaAddr>()) {
+        let last_attempt_failed = peer.last_connection_state == Failed;
+        let not_recently_seen = !peer.last_seen_is_recent();
+
+        let probably_unreachable = last_attempt_failed && not_recently_seen;
+
+        prop_assert_eq!(
+            peer.is_probably_reachable(),
+            !probably_unreachable,
+            "last_connection_state: {:?}, last_seen: {:?}",
+            peer.last_connection_state,
+            peer.last_seen()
+        );
+    }
 }