Security: Limit address book size to limit memory usage

zebra-network/src/address_book.rs

@@ -8,8 +8,8 @@ use ordered_map::OrderedMap;
 use tracing::Span;
 
 use crate::{
-    meta_addr::MetaAddrChange, protocol::external::canonical_socket_addr, types::MetaAddr,
-    PeerAddrState,
+    constants, meta_addr::MetaAddrChange, protocol::external::canonical_socket_addr,
+    types::MetaAddr, PeerAddrState,
 };
 
 #[cfg(test)]
@@ -59,6 +59,12 @@ pub struct AddressBook {
     /// sorts in ascending order, but [`OrderedMap`] sorts in descending order.
     by_addr: OrderedMap<SocketAddr, MetaAddr, Reverse<MetaAddr>>,
 
+    /// The maximum number of addresses in the address book.
+    ///
+    /// Always set to [`MAX_ADDRS_IN_ADDRESS_BOOK`](constants::MAX_ADDRS_IN_ADDRESS_BOOK),
+    /// in release builds. Lower values are used during testing.
+    addr_limit: usize,
+
     /// The local listener address.
     local_listener: SocketAddr,
 
@@ -107,6 +113,7 @@ impl AddressBook {
 
         let mut new_book = AddressBook {
             by_addr: OrderedMap::new(|meta_addr| Reverse(*meta_addr)),
+            addr_limit: constants::MAX_ADDRS_IN_ADDRESS_BOOK,
             local_listener: canonical_socket_addr(local_listener),
             span,
             last_address_log: None,
@@ -117,7 +124,9 @@ impl AddressBook {
     }
 
     /// Construct an [`AddressBook`] with the given `local_listener`,
-    /// [`tracing::Span`], and addresses.
+    /// `addr_limit`, [`tracing::Span`], and addresses.
+    ///
+    /// `addr_limit` is enforced by this method, and by [`AddressBook::update`].
     ///
     /// If there are multiple [`MetaAddr`]s with the same address,
     /// an arbitrary address is inserted into the address book,
@@ -128,6 +137,7 @@ impl AddressBook {
     #[cfg(any(test, feature = "proptest-impl"))]
     pub fn new_with_addrs(
         local_listener: SocketAddr,
+        addr_limit: usize,
         span: Span,
         addrs: impl IntoIterator<Item = MetaAddr>,
     ) -> AddressBook {
@@ -138,6 +148,7 @@ impl AddressBook {
         let chrono_now = Utc::now();
 
         let mut new_book = AddressBook::new(local_listener, span);
+        new_book.addr_limit = addr_limit;
 
         let addrs = addrs
             .into_iter()
@@ -146,6 +157,7 @@ impl AddressBook {
                 meta_addr
             })
             .filter(MetaAddr::address_is_valid_for_outbound)
+            .take(addr_limit)
             .map(|meta_addr| (meta_addr.addr, meta_addr));
 
         for (socket_addr, meta_addr) in addrs {
@@ -270,6 +282,25 @@ impl AddressBook {
             }
 
             self.by_addr.insert(updated.addr, updated);
+
+            // Security: Limit the number of peers in the address book.
+            //
+            // We only delete outdated peers when we have too many peers.
+            // If we deleted them as soon as they became too old,
+            // then other peers could re-insert them into the address book.
+            // And we would start connecting to those outdated peers again,
+            // ignoring the age limit in [`MetaAddr::is_probably_reachable`].
+            while self.by_addr.len() > self.addr_limit {
+                let surplus_peer = self
+                    .peers()
+                    .next_back()
+                    .expect("just checked there is at least one peer");
+
+                self.by_addr.remove(&surplus_peer.addr);
+            }
+
+            assert!(self.len() <= self.addr_limit);
+
             std::mem::drop(_guard);
             self.update_metrics(instant_now, chrono_now);
         }
@@ -320,7 +351,7 @@ impl AddressBook {
     /// Return an iterator over all peers.
     ///
     /// Returns peers in reconnection attempt order, including recently connected peers.
-    pub fn peers(&'_ self) -> impl Iterator<Item = MetaAddr> + '_ {
+    pub fn peers(&'_ self) -> impl Iterator<Item = MetaAddr> + DoubleEndedIterator + '_ {
         let _guard = self.span.enter();
         self.by_addr.descending_values().cloned()
     }
@@ -331,7 +362,7 @@ impl AddressBook {
         &'_ self,
         instant_now: Instant,
         chrono_now: chrono::DateTime<Utc>,
-    ) -> impl Iterator<Item = MetaAddr> + '_ {
+    ) -> impl Iterator<Item = MetaAddr> + DoubleEndedIterator + '_ {
         let _guard = self.span.enter();
 
         // Skip live peers, and peers pending a reconnect attempt.
@@ -344,7 +375,10 @@ impl AddressBook {
 
     /// Return an iterator over all the peers in `state`,
     /// in reconnection attempt order, including recently connected peers.
-    pub fn state_peers(&'_ self, state: PeerAddrState) -> impl Iterator<Item = MetaAddr> + '_ {
+    pub fn state_peers(
+        &'_ self,
+        state: PeerAddrState,
+    ) -> impl Iterator<Item = MetaAddr> + DoubleEndedIterator + '_ {
         let _guard = self.span.enter();
 
         self.by_addr
@@ -359,7 +393,7 @@ impl AddressBook {
         &'_ self,
         instant_now: Instant,
         chrono_now: chrono::DateTime<Utc>,
-    ) -> impl Iterator<Item = MetaAddr> + '_ {
+    ) -> impl Iterator<Item = MetaAddr> + DoubleEndedIterator + '_ {
         let _guard = self.span.enter();
 
         self.by_addr
@@ -373,7 +407,7 @@ impl AddressBook {
     pub fn recently_live_peers(
         &'_ self,
         now: chrono::DateTime<Utc>,
-    ) -> impl Iterator<Item = MetaAddr> + '_ {
+    ) -> impl Iterator<Item = MetaAddr> + DoubleEndedIterator + '_ {
         let _guard = self.span.enter();
 
         self.by_addr

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

@@ -9,13 +9,15 @@ use tracing::Span;
 use zebra_chain::serialization::Duration32;
 
 use crate::{
-    constants::MAX_PEER_ACTIVE_FOR_GOSSIP,
-    meta_addr::{arbitrary::MAX_META_ADDR, MetaAddr},
+    constants::{MAX_ADDRS_IN_ADDRESS_BOOK, MAX_PEER_ACTIVE_FOR_GOSSIP},
+    meta_addr::{arbitrary::MAX_META_ADDR, MetaAddr, MetaAddrChange},
     AddressBook,
 };
 
 const TIME_ERROR_MARGIN: Duration32 = Duration32::from_seconds(1);
 
+const MAX_ADDR_CHANGE: usize = 10;
+
 proptest! {
     #[test]
     fn only_recently_reachable_are_gossiped(
@@ -25,7 +27,12 @@ proptest! {
         zebra_test::init();
         let chrono_now = Utc::now();
 
-        let address_book = AddressBook::new_with_addrs(local_listener, Span::none(), addresses);
+        let address_book = AddressBook::new_with_addrs(
+            local_listener,
+            MAX_ADDRS_IN_ADDRESS_BOOK,
+            Span::none(),
+            addresses
+        );
 
         for gossiped_address in address_book.sanitized(chrono_now) {
             let duration_since_last_seen = gossiped_address
@@ -48,10 +55,83 @@ proptest! {
         let instant_now = Instant::now();
         let chrono_now = Utc::now();
 
-        let address_book = AddressBook::new_with_addrs(local_listener, Span::none(), addresses);
+        let address_book = AddressBook::new_with_addrs(
+            local_listener,
+            MAX_ADDRS_IN_ADDRESS_BOOK,
+            Span::none(),
+            addresses
+        );
 
         for peer in address_book.reconnection_peers(instant_now, chrono_now) {
             prop_assert!(peer.is_probably_reachable(chrono_now), "peer: {:?}", peer);
         }
     }
+
+    /// Test that the address book limit is respected for multiple peers.
+    #[test]
+    fn address_book_length_is_limited(
+        local_listener in any::<SocketAddr>(),
+        addr_changes_lists in vec(
+            MetaAddrChange::addr_changes_strategy(MAX_ADDR_CHANGE),
+            2..MAX_ADDR_CHANGE
+        ),
+        addr_limit in 0..=MAX_ADDR_CHANGE,
+        pre_fill in any::<bool>(),
+    ) {
+        zebra_test::init();
+
+        let initial_addrs = if pre_fill {
+            addr_changes_lists
+                .iter()
+                .map(|(addr, _changes)| addr)
+                .cloned()
+                .collect()
+        } else {
+            Vec::new()
+        };
+
+        // sequentially apply changes for one address, then move on to the next
+
+        let mut address_book = AddressBook::new_with_addrs(
+            local_listener,
+            addr_limit,
+            Span::none(),
+            initial_addrs.clone(),
+        );
+
+        for (_addr, changes) in addr_changes_lists.iter() {
+            for change in changes {
+                address_book.update(*change);
+
+                prop_assert!(
+                    address_book.len() <= addr_limit,
+                    "sequential test length: {} was greater than limit: {}",
+                    address_book.len(), addr_limit,
+                );
+            }
+        }
+
+        // interleave changes for different addresses
+
+        let mut address_book = AddressBook::new_with_addrs(
+            local_listener,
+            addr_limit,
+            Span::none(),
+            initial_addrs,
+        );
+
+        for index in 0..MAX_ADDR_CHANGE {
+            for (_addr, changes) in addr_changes_lists.iter() {
+                if let Some(change) = changes.get(index) {
+                    address_book.update(*change);
+
+                    prop_assert!(
+                        address_book.len() <= addr_limit,
+                        "interleave test length: {} was greater than limit: {}",
+                        address_book.len(), addr_limit,
+                    );
+                }
+            }
+        }
+    }
 }

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

@@ -7,7 +7,10 @@ use tracing::Span;
 
 use zebra_chain::serialization::{DateTime32, Duration32};
 
-use crate::{meta_addr::MetaAddr, protocol::external::types::PeerServices, AddressBook};
+use crate::{
+    constants::MAX_ADDRS_IN_ADDRESS_BOOK, meta_addr::MetaAddr,
+    protocol::external::types::PeerServices, AddressBook,
+};
 
 /// Make sure an empty address book is actually empty.
 #[test]
@@ -39,8 +42,12 @@ fn address_book_peer_order() {
 
     // Regardless of the order of insertion, the most recent address should be chosen first
     let addrs = vec![meta_addr1, meta_addr2];
-    let address_book =
-        AddressBook::new_with_addrs("0.0.0.0:0".parse().unwrap(), Span::current(), addrs);
+    let address_book = AddressBook::new_with_addrs(
+        "0.0.0.0:0".parse().unwrap(),
+        MAX_ADDRS_IN_ADDRESS_BOOK,
+        Span::current(),
+        addrs,
+    );
     assert_eq!(
         address_book
             .reconnection_peers(Instant::now(), Utc::now())
@@ -50,8 +57,12 @@ fn address_book_peer_order() {
 
     // Reverse the order, check that we get the same result
     let addrs = vec![meta_addr2, meta_addr1];
-    let address_book =
-        AddressBook::new_with_addrs("0.0.0.0:0".parse().unwrap(), Span::current(), addrs);
+    let address_book = AddressBook::new_with_addrs(
+        "0.0.0.0:0".parse().unwrap(),
+        MAX_ADDRS_IN_ADDRESS_BOOK,
+        Span::current(),
+        addrs,
+    );
     assert_eq!(
         address_book
             .reconnection_peers(Instant::now(), Utc::now())
@@ -64,8 +75,12 @@ fn address_book_peer_order() {
     meta_addr2.addr = addr1;
 
     let addrs = vec![meta_addr1, meta_addr2];
-    let address_book =
-        AddressBook::new_with_addrs("0.0.0.0:0".parse().unwrap(), Span::current(), addrs);
+    let address_book = AddressBook::new_with_addrs(
+        "0.0.0.0:0".parse().unwrap(),
+        MAX_ADDRS_IN_ADDRESS_BOOK,
+        Span::current(),
+        addrs,
+    );
     assert_eq!(
         address_book
             .reconnection_peers(Instant::now(), Utc::now())
@@ -75,8 +90,12 @@ fn address_book_peer_order() {
 
     // Reverse the order, check that we get the same result
     let addrs = vec![meta_addr2, meta_addr1];
-    let address_book =
-        AddressBook::new_with_addrs("0.0.0.0:0".parse().unwrap(), Span::current(), addrs);
+    let address_book = AddressBook::new_with_addrs(
+        "0.0.0.0:0".parse().unwrap(),
+        MAX_ADDRS_IN_ADDRESS_BOOK,
+        Span::current(),
+        addrs,
+    );
     assert_eq!(
         address_book
             .reconnection_peers(Instant::now(), Utc::now())

zebra-network/src/constants.rs

@@ -138,6 +138,27 @@ pub const GET_ADDR_FANOUT: usize = 3;
 /// https://zips.z.cash/zip-0155#specification
 pub const MAX_ADDRS_IN_MESSAGE: usize = 1000;
 
+/// The fraction of addresses Zebra sends in response to a `Peers` request.
+///
+/// Each response contains approximately:
+/// `address_book.len() / ADDR_RESPONSE_LIMIT_DENOMINATOR`
+/// addresses, selected at random from the address book.
+///
+/// # Security
+///
+/// This limit makes sure that Zebra does not reveal its entire address book
+/// in a single `Peers` response.
+pub const ADDR_RESPONSE_LIMIT_DENOMINATOR: usize = 3;
+
+/// The maximum number of addresses Zebra will keep in its address book.
+///
+/// This is a tradeoff between:
+/// - revealing the whole address book in a few requests,
+/// - sending the maximum number of peer addresses, and
+/// - making sure the limit code actually gets run.
+pub const MAX_ADDRS_IN_ADDRESS_BOOK: usize =
+    MAX_ADDRS_IN_MESSAGE * (ADDR_RESPONSE_LIMIT_DENOMINATOR + 1);
+
 /// Truncate timestamps in outbound address messages to this time interval.
 ///
 /// ## SECURITY
@@ -261,4 +282,40 @@ mod tests {
         assert!(EWMA_DECAY_TIME > REQUEST_TIMEOUT,
                 "The EWMA decay time should be higher than the request timeout, so timed out peers are penalised by the EWMA.");
     }
+
+    /// Make sure that peer age limits are consistent with each other.
+    #[test]
+    fn ensure_peer_age_limits_consistent() {
+        zebra_test::init();
+
+        assert!(
+            MAX_PEER_ACTIVE_FOR_GOSSIP <= MAX_RECENT_PEER_AGE,
+            "we should only gossip peers we are actually willing to try ourselves"
+        );
+    }
+
+    /// Make sure the address limits are consistent with each other.
+    #[test]
+    #[allow(clippy::assertions_on_constants)]
+    fn ensure_address_limits_consistent() {
+        // Zebra 1.0.0-beta.2 address book metrics in December 2021.
+        const TYPICAL_MAINNET_ADDRESS_BOOK_SIZE: usize = 4_500;
+
+        zebra_test::init();
+
+        assert!(
+            MAX_ADDRS_IN_ADDRESS_BOOK >= GET_ADDR_FANOUT * MAX_ADDRS_IN_MESSAGE,
+            "the address book should hold at least a fanout's worth of addresses"
+        );
+
+        assert!(
+            MAX_ADDRS_IN_ADDRESS_BOOK / ADDR_RESPONSE_LIMIT_DENOMINATOR > MAX_ADDRS_IN_MESSAGE,
+            "the address book should hold enough addresses for a full response"
+        );
+
+        assert!(
+            MAX_ADDRS_IN_ADDRESS_BOOK < TYPICAL_MAINNET_ADDRESS_BOOK_SIZE,
+            "the address book limit should actually be used"
+        );
+    }
 }