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: Prevent inv and AddressBook message denial of service #3271

Closed
Tracked by #2311
teor2345 opened this issue Dec 20, 2021 · 2 comments
Closed
Tracked by #2311

Security: Prevent inv and AddressBook message denial of service #3271

teor2345 opened this issue Dec 20, 2021 · 2 comments
Labels
A-network Area: Network protocol updates or fixes C-security Category: Security issues I-hang A Zebra component stops responding to requests I-heavy Problems with excessive memory, disk, or CPU usage I-slow Problems with performance or responsiveness

Comments

@teor2345
Copy link
Contributor

teor2345 commented Dec 20, 2021

Motivation

If peers send a lot of messages, they can overload the InventoryRegistry or AddressBook.

This is a memory or CPU denial of service risk.

Scheduling

This risk is acceptable for the stable release, but it probably needs to be fixed to support lightwalletd users.

Suggested Solutions

AddressBook:

  • make the sender task yield after sending each update on the channel
  • to reduce the volume of messages from honest peers, only send message timestamps for ping and pong messages
  • to avoid peers flushing or delaying other peers' updates, choose one or more of these alternatives:
    • rate-limit the number of messages per peer, and disconnect peers that go over the limit
    • use one channel per peer for timestamp updates, or for all updates
    • combine all ready timestamp updates for the same peer before sending them

InventoryRegistry:

  • make the sender task yield after sending each update on the channel
  • limit the number of inv entries per peer, and disconnect peers that go over the limit
  • also limit the number of peers?
  • to avoid peers flushing or delaying other peers' updates, choose one or more of these alternatives:
    • rate-limit the number of messages per peer, and disconnect peers that go over the limit
    • use one channel per peer for inventory updates, or for all updates
    • combine all ready inventory updates for the same peer before sending them
    • split notfound and inv updates into separate channels:
      • inv are a low priority and can be dropped if there are excess
      • notfound are a higher priority, but can still be faked by peers, so they should be dropped if there are a lot
      • synthesised notfound should never be dropped, because they represent failed requests that Zebra does not want to retry

Related Work

@teor2345 teor2345 added S-needs-triage Status: A bug report needs triage P-Medium C-security Category: Security issues I-hang A Zebra component stops responding to requests I-heavy Problems with excessive memory, disk, or CPU usage I-slow Problems with performance or responsiveness A-network Area: Network protocol updates or fixes labels Dec 20, 2021
@mpguerra
Copy link
Contributor

@mpguerra mpguerra mentioned this issue Jan 27, 2022
40 tasks
@teor2345
Copy link
Contributor Author

teor2345 commented Mar 1, 2022

I think our current broadcast channel implementation does this well enough for now.

@teor2345 teor2345 closed this as completed Mar 1, 2022
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Sep 28, 2022
@teor2345 teor2345 reopened this May 7, 2023
@teor2345 teor2345 closed this as not planned Won't fix, can't repro, duplicate, stale May 7, 2023
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 C-security Category: Security issues I-hang A Zebra component stops responding to requests I-heavy Problems with excessive memory, disk, or CPU usage I-slow Problems with performance or responsiveness
Projects
None yet
Development

No branches or pull requests

2 participants