Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security: Spawn a separate task for each initial handshake, Credit: Equilibrium #2181

Closed
wants to merge 7 commits into from
30 changes: 19 additions & 11 deletions zebra-network/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,22 @@ use zebra_chain::parameters::NetworkUpgrade;
/// buffer adds up to 6 seconds worth of blocks to the queue.
pub const PEERSET_BUFFER_SIZE: usize = 3;

/// The timeout for requests made to a remote peer.
pub const REQUEST_TIMEOUT: Duration = Duration::from_secs(20);
/// The timeout for DNS lookups.
///
/// [6.1.3.3 Efficient Resource Usage] from [RFC 1123: Requirements for Internet Hosts]
/// suggest no less than 5 seconds for resolving timeout.
///
/// [RFC 1123: Requirements for Internet Hosts] https://tools.ietf.org/rfcmarkup?doc=1123
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
/// [6.1.3.3 Efficient Resource Usage] https://tools.ietf.org/rfcmarkup?doc=1123#page-77
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
pub const DNS_LOOKUP_TIMEOUT: Duration = Duration::from_secs(5);

/// The minimum time between connections to initial or candidate peers.
///
/// ## Security
///
/// Zebra resists distributed denial of service attacks by making sure that new peer connections
/// are initiated at least `MIN_PEER_CONNECTION_INTERVAL` apart.
pub const MIN_PEER_CONNECTION_INTERVAL: Duration = Duration::from_millis(100);

/// The timeout for handshakes when connecting to new peers.
///
Expand All @@ -32,6 +46,9 @@ pub const REQUEST_TIMEOUT: Duration = Duration::from_secs(20);
/// nodes, and on testnet.
pub const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(4);

/// The timeout for requests made to a remote peer.
pub const REQUEST_TIMEOUT: Duration = Duration::from_secs(20);

/// We expect to receive a message from a live peer at least once in this time duration.
///
/// This is the sum of:
Expand Down Expand Up @@ -123,15 +140,6 @@ lazy_static! {
}.expect("regex is valid");
}

/// The timeout for DNS lookups.
///
/// [6.1.3.3 Efficient Resource Usage] from [RFC 1123: Requirements for Internet Hosts]
/// suggest no less than 5 seconds for resolving timeout.
///
/// [RFC 1123: Requirements for Internet Hosts] https://tools.ietf.org/rfcmarkup?doc=1123
/// [6.1.3.3 Efficient Resource Usage] https://tools.ietf.org/rfcmarkup?doc=1123#page-77
pub const DNS_LOOKUP_TIMEOUT: Duration = Duration::from_secs(5);

/// Magic numbers used to identify different Zcash networks.
pub mod magics {
use super::*;
Expand Down
14 changes: 14 additions & 0 deletions zebra-network/src/meta_addr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,20 @@ pub struct MetaAddr {
}

impl MetaAddr {
/// Create a new seed [`MetaAddr`], based on the configured seed addresses.
pub fn new_seed_meta_addr(addr: SocketAddr) -> MetaAddr {
MetaAddr {
addr,
// TODO: stop guessing this field
services: PeerServices::NODE_NETWORK,
// TODO: remove this time, it's far too optimistic. Using `now` can
// keep invalid peers in the consensus peer set for a long time.
last_seen: Utc::now(),
// TODO: replace with NeverAttemptedSeed
last_connection_state: NeverAttemptedGossiped,
}
}

/// Create a new gossiped [`MetaAddr`], based on the deserialized fields from
/// a gossiped peer [`Addr`][crate::protocol::external::Message::Addr] message.
pub fn new_gossiped_meta_addr(
Expand Down
37 changes: 19 additions & 18 deletions zebra-network/src/peer_set/candidate_set.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use std::{cmp::min, mem, sync::Arc, time::Duration};
use std::{
cmp::{max, min},
sync::Arc,
};

use chrono::{DateTime, Utc};
use futures::stream::{FuturesUnordered, StreamExt};
use tokio::time::{sleep, sleep_until, timeout, Sleep};
use tokio::time::{sleep_until, timeout, Instant};
use tower::{Service, ServiceExt};

use crate::{constants, types::MetaAddr, AddressBook, BoxError, Request, Response};
Expand Down Expand Up @@ -104,22 +107,14 @@ use crate::{constants, types::MetaAddr, AddressBook, BoxError, Request, Response
pub(super) struct CandidateSet<S> {
pub(super) address_book: Arc<std::sync::Mutex<AddressBook>>,
pub(super) peer_service: S,
next_peer_min_wait: Sleep,
next_peer_sleep_until: Instant,
}

impl<S> CandidateSet<S>
where
S: Service<Request, Response = Response, Error = BoxError>,
S::Future: Send + 'static,
{
/// The minimum time between successive calls to `CandidateSet::next()`.
///
/// ## Security
///
/// Zebra resists distributed denial of service attacks by making sure that new peer connections
/// are initiated at least `MIN_PEER_CONNECTION_INTERVAL` apart.
const MIN_PEER_CONNECTION_INTERVAL: Duration = Duration::from_millis(100);

/// Uses `address_book` and `peer_service` to manage a [`CandidateSet`] of peers.
pub fn new(
address_book: Arc<std::sync::Mutex<AddressBook>>,
Expand All @@ -128,7 +123,7 @@ where
CandidateSet {
address_book,
peer_service,
next_peer_min_wait: sleep(Duration::from_secs(0)),
next_peer_sleep_until: Instant::now(),
}
}

Expand Down Expand Up @@ -278,10 +273,6 @@ where
/// new peer connections are initiated at least
/// `MIN_PEER_CONNECTION_INTERVAL` apart.
pub async fn next(&mut self) -> Option<MetaAddr> {
let current_deadline = self.next_peer_min_wait.deadline();
let mut sleep = sleep_until(current_deadline + Self::MIN_PEER_CONNECTION_INTERVAL);
mem::swap(&mut self.next_peer_min_wait, &mut sleep);

// # Correctness
//
// In this critical section, we hold the address mutex, blocking the
Expand All @@ -304,8 +295,18 @@ where
reconnect
};

// SECURITY: rate-limit new candidate connections
sleep.await;
// # Security
//
// Rate-limit new candidate connections to avoid denial of service.
//
// Update the deadline before sleeping, so we handle concurrent requests
// correctly.
let current_deadline = self.next_peer_sleep_until;
// If we recently had a connection, and we're about to sleep, base the
// next time off our sleep time. Otherwise, use the current time.
self.next_peer_sleep_until = max(self.next_peer_sleep_until, Instant::now());
self.next_peer_sleep_until += constants::MIN_PEER_CONNECTION_INTERVAL;
sleep_until(current_deadline).await;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security: Correctly use MIN_PEER_CONNECTION_INTERVAL in CandidateSet

  • Make sure the next deadline is at least MIN_PEER_CONNECTION_INTERVAL in the future
  • Replace the Sleep future with an Instant


Some(reconnect)
}
Expand Down
Loading