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: avoid a single peer providing a majority of Zebra's peer addresses #2004

Merged
merged 1 commit into from
Apr 15, 2021

Conversation

teor2345
Copy link
Contributor

Motivation

We don't want a single peer to supply a majority of Zebra's addresses, because that makes takeover attacks easier.

Solution

Zebra avoids having a majority of addresses from a single peer by asking 3 peers for new addresses.
(This is a best-effort preventative measure - for a stronger guarantee, see #1869.)

Also update a bunch of security comments and related documentation.

The code in this pull request has:

  • Documentation Comments
  • Integration Tests

Review

@dconnolly and I discussed this change as part of #1850. It's not an urgent fix, but it's worth doing soon.

Follow Up Work

Limit the number of addresses we use from each peer (#1869)

Zebra avoids having a majority of addresses from a single peer by asking
3 peers for new addresses.

Also update a bunch of security comments and related documentation.
@teor2345 teor2345 added A-rust Area: Updates to Rust code P-Medium C-security Category: Security issues I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data A-network Area: Network protocol updates or fixes labels Apr 13, 2021
@teor2345 teor2345 added this to the 2021 Sprint 7 milestone Apr 13, 2021
@teor2345 teor2345 requested a review from dconnolly April 13, 2021 22:21
@teor2345 teor2345 self-assigned this Apr 13, 2021
Comment on lines +56 to +64
/// The fanout should be greater than 2, so that Zebra avoids getting a majority
/// of its initial address book entries from a single peer.
///
/// Zebra regularly crawls for new peers, initiating a new crawl every
/// [`crawl_new_peer_interval`](crate::config::Config.crawl_new_peer_interval).
///
/// TODO: limit the number of addresses that Zebra uses from a single peer
/// response (#1869)
pub const GET_ADDR_FANOUT: usize = 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

@dconnolly dconnolly merged commit 381c20b into ZcashFoundation:main Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-network Area: Network protocol updates or fixes A-rust Area: Updates to Rust code C-security Category: Security issues I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants