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: Handle small numbers of initial peers better, Credit: Equilibrium #2154

Merged
merged 2 commits into from
May 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions zebra-network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ impl Config {
async fn resolve_peers(peers: &HashSet<String>) -> HashSet<SocketAddr> {
use futures::stream::StreamExt;

if peers.is_empty() {
error!("no initial peers in the network config. Hint: you must configure at least one peer or DNS seeder to run Zebra");
return HashSet::new();
Comment on lines +70 to +72
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

}

loop {
// We retry each peer individually, as well as retrying if there are
// no peers in the combined list. DNS failures are correlated, so all
Expand Down
25 changes: 22 additions & 3 deletions zebra-network/src/peer_set/candidate_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,25 @@ where
}
}

/// Update the peer set from the network.
/// Update the peer set from the network, using the default fanout limit.
///
/// See `update_initial` for details.
pub async fn update(&mut self) -> Result<(), BoxError> {
self.update_inner(None).await
}

/// Update the peer set from the network, limiting the fanout to
/// `fanout_limit`.
///
/// - Ask a few live `Responded` peers to send us more peers.
/// - Process all completed peer responses, adding new peers in the
/// `NeverAttempted` state.
///
/// ## Correctness
///
/// Pass the initial peer set size as `fanout_limit` during initialization,
/// so that Zebra does not send duplicate requests to the same peer.
///
/// The crawler exits when update returns an error, so it must only return
/// errors on permanent failures.
///
Expand All @@ -148,7 +159,15 @@ where
/// `report_failed` puts peers into the `Failed` state.
///
/// `next` puts peers into the `AttemptPending` state.
pub async fn update(&mut self) -> Result<(), BoxError> {
pub async fn update_initial(&mut self, fanout_limit: usize) -> Result<(), BoxError> {
self.update_inner(Some(fanout_limit)).await
}

/// Update the peer set from the network, limiting the fanout to
/// `fanout_limit`.
///
/// See `update_initial` for details.
async fn update_inner(&mut self, fanout_limit: Option<usize>) -> Result<(), BoxError> {
// Opportunistically crawl the network on every update call to ensure
// we're actively fetching peers. Continue independently of whether we
// actually receive any peers, but always ask the network for more.
Expand All @@ -159,7 +178,7 @@ where
// called while the peer set is already loaded.
let mut responses = FuturesUnordered::new();
trace!("sending GetPeers requests");
for _ in 0..constants::GET_ADDR_FANOUT {
for _ in 0..fanout_limit.unwrap_or(constants::GET_ADDR_FANOUT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

// CORRECTNESS
//
// Use a timeout to avoid deadlocks when there are no connected
Expand Down
7 changes: 5 additions & 2 deletions zebra-network/src/peer_set/initialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,14 @@ where
);

// 2. Initial peers, specified in the config.
let (initial_peer_count_tx, initial_peer_count_rx) = tokio::sync::oneshot::channel();
let initial_peers_fut = {
let config = config.clone();
let connector = connector.clone();
let peerset_tx = peerset_tx.clone();
async move {
let initial_peers = config.initial_peers().await;
let _ = initial_peer_count_tx.send(initial_peers.len());
// Connect the tx end to the 3 peer sources:
add_initial_peers(initial_peers, connector, peerset_tx).await
}
Expand All @@ -160,10 +162,11 @@ where
// We need to await candidates.update() here, because zcashd only sends one
// `addr` message per connection, and if we only have one initial peer we
// need to ensure that its `addr` message is used by the crawler.
// XXX this should go in CandidateSet::new, but we need init() -> Result<_,_>

info!("Sending initial request for peers");
let _ = candidates.update().await;
let _ = candidates
.update_initial(initial_peer_count_rx.await.expect("value sent before drop"))
.await;

for _ in 0..config.peerset_initial_target_size {
let _ = demand_tx.try_send(());
Expand Down