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

feat(network): Track misbehaving peer connections and ban them past a threshold #9201

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

arya2
Copy link
Contributor

@arya2 arya2 commented Feb 4, 2025

Motivation

Zebra should drop connections to and avoid reconnecting with peers which advertise blocks or mempool transactions that fail some semantic validation checks.

Will close #9111.

This is a draft PR as it's still missing a test.

Solution

Channels:

  • Adds a new mpsc channel for the mempool, inbound svc, and syncer to report misbehaviour to the address book,
  • Adds a watch channel for the address book to broadcast the latest bans to the peer set and inbound connector
  • Adds a task for batching updates to addresses' misbehavior scores

Type Updates:

  • Adds a misbehavior_score field on MetaAddr, a bans_by_ip field to AddressBook, and a UpdateMisbehavior variant to MetaAddrChange
  • Updates MetaAddrChange.apply_to_meta_addr() to apply UpdateMisbehavior changes
  • Adds the PeerSocketAddr of the peer that provided a block or transaction to the Transaction and Block variants of the peer set's response type
  • Updates error types for mempool, inbound service, and syncer Downloads components to include the peer address from which the block or transaction originated
  • Passes the new channel for reporting misbehaviour to the mempool, inbound service, and syncer

Method Updates:

  • Updates AddressBook.update() to remove any addresses that exceed a misbehavior score threshold and add them to bans_by_ip
    • Removes oldest banned IPs if bans_by_ip exceeds its maximum size
    • Removing the addresses from the by_addr field avoids attempting outbound connections to them as candidates,
  • Updates sanitized() method to avoid providing addresses with a non-zero misbehavior score in responses to GetAddr requests
  • Adds misbehavior_score() and mempool_misbehavior_score() methods on TransactionError, VerifyBlockError, and BlockError
  • Updates mempool, inbound service, and syncer to send UpdateMisbehavior messages to the address book updater when blocks or transactions fail some checks during semantic validation
  • Updates peer set's poll_ready() method to drop connections to banned IPs

Tests

This PR still needs tests.

Follow Up Work

Increment the misbehaviour score for more errors and adjust the amounts based on zcashd (though at a glance, it looks similar already).

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

arya2 added 5 commits February 3, 2025 23:40
- Add a `bans_by_ip` field to `AddressBook`
- Update the `AddressBook::update()` method to:
    - increment misbehavior scores in its entries,
    - add addr ips to bans_by_ip if the score is excessive,
    - remove any addrs at the banned ip
- Avoid responding to `GetAddr` requests with addresses of misbehaving peers (return None from `sanitized()`),
- Avoid new inbound or outbound connections to banned ips
@arya2 arya2 added C-bug Category: This is a bug C-enhancement Category: This is an improvement C-security Category: Security issues I-heavy Problems with excessive memory, disk, or CPU usage I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data A-network Area: Network protocol updates or fixes C-feature Category: New features A-mempool Area: Memory pool transactions P-Medium ⚡ labels Feb 4, 2025
@arya2 arya2 self-assigned this Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mempool Area: Memory pool transactions A-network Area: Network protocol updates or fixes C-bug Category: This is a bug C-enhancement Category: This is an improvement C-feature Category: New features C-security Category: Security issues I-heavy Problems with excessive memory, disk, or CPU usage I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data P-Medium ⚡
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Score and drop misbehaving peer connections
2 participants