fix(security): Rate-limit and size-limit peer transaction ID messages

[teor2345]

Motivation

Zebra can send a transaction gossip to half its peers for every transaction it verifies. This could put a lot of load on the network if transaction rates increase.

This helps with the following tickets, but doesn't completely resolve any of them:

• peer set load: Multiple tests fail in CI with "inbound service overloaded" warnings #6506
• rate-limit some messages: Security: rate-limit outbound messages, Credit: Equilibrium #2153
• inv denial of service: Document consensus rules from Zcash spec: 4.5 Output Descriptions #3217
• more transactions on the network after the deployment of feat(mempool): add ZIP-317 rules to mempool #6556

Specifications

Zcash network messages are limited to 2MB.

Gossip behaviour is implementation-specific.

The larger wide transaction ID inventory gossip message size of 68 bytes is documented in:
https://zips.z.cash/zip-0239#deployment

Complex Code or Requirements

This code modifies a channel shared between concurrent async tasks.

Solution

Fixes - network message size:

• Split MAX_INV_IN_RECEIVED_MESSAGE and MAX_TX_INV_IN_SENT_MESSAGE
  • After ZIP-239 in NU5, inventory IDs can now be around twice the size, so I halved this limit for sent messages
  • Received messages are still limited by the protocol message limit
  • I also submitted the same change to zcashd: Reduce MAX_INV_SZ so wtx invs fit within a network message zcash/zcash#6643
  • This limit shouldn't be reached with the default config, because the mempool is limited to 8,000 transactions
• Enforce MAX_TX_INV_IN_SENT_MESSAGE in the network layer for gossips and responses
  • The parser already enforces it for requests from peers)

Fixes - network message rate-limits:

• Combine multiple transaction updates into a single gossip & rate-limit gossips
  • At the moment, peers can trigger lots of broadcast gossips, by sending transactions that are quick to verify
  • This also fixes some bugs:
    • Some transaction gossips could be dropped if they verify close together
    • Zebra logs each transaction gossip, which is a usability problem
    • In rare cases, Zebra could produce invalid gossips with too many transactions, and generate a network error (which was ignored)
• Rate-limit block gossips
  • This rate limit is unlikely to be reached, because blocks are usually 75 seconds apart
  • Zebra logs each block gossip, which could be a usability problem if blocks arrive close together

Docs:

• Fix usage documentation for Message::Tx

Testing:

• Fix mempool_transaction_expiration gossip test timings

Review

This is a routine security fix. It doesn't have to go into the next release, but if we have time to review it, it would be nice to get it in.

Reviewer Checklist

• Will the PR name make sense to users?
  • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
• Are the PR labels correct?
• Does the code do what the ticket and PR says?
  • Does it change concurrent code, unsafe code, or consensus rules?
• How do you know it works? Does it have tests?

Follow Up Work

Rate-limit other kinds of network messages if they become an issue.

[teor2345]

After this change, new blocks and transactions are gossiped immediately, but then the next gossip is delayed for 6 seconds, and any pending changes are combined:

2023-05-07T23:44:14.798961Z  INFO {zebrad="e426f63" net="Main"}: zebrad::components::mempool::gossip: sending mempool transaction broadcast request=AdvertiseTransactionIds(4) changes=1
2023-05-07T23:44:15.038166Z  INFO {zebrad="e426f63" net="Main"}:{peer=Out("162.19.139.183:8233")}:msg_as_req{msg="inv"}:inbound:download_and_verify{hash=000000000067b02b015bccab67b49a0979145b9b685076c2897343706cd8e9e1}: zebrad::components::inbound::downloads: downloaded and verified gossiped block height=Height(2078266)
2023-05-07T23:44:25.046703Z  INFO {zebrad="e426f63" net="Main"}: zebrad::components::mempool::gossip: sending mempool transaction broadcast request=AdvertiseTransactionIds(7) changes=4
2023-05-07T23:44:31.048226Z  INFO {zebrad="e426f63" net="Main"}: zebrad::components::mempool::gossip: sending mempool transaction broadcast request=AdvertiseTransactionIds(3) changes=3
2023-05-07T23:44:37.049697Z  INFO {zebrad="e426f63" net="Main"}: zebrad::components::mempool::gossip: sending mempool transaction broadcast request=AdvertiseTransactionIds(3) changes=2
2023-05-07T23:44:43.050632Z  INFO {zebrad="e426f63" net="Main"}: zebrad::components::mempool::gossip: sending mempool transaction broadcast request=AdvertiseTransactionIds(1) changes=1
2023-05-07T23:44:49.052259Z  INFO {zebrad="e426f63" net="Main"}: zebrad::components::mempool::gossip: sending mempool transaction broadcast request=AdvertiseTransactionIds(1) changes=1
2023-05-07T23:45:09.622233Z  INFO {zebrad="e426f63" net="Main"}: zebrad::components::mempool::gossip: sending mempool transaction broadcast request=AdvertiseTransactionIds(1) changes=1

[codecov]

Codecov Report

Merging #6625 (07d89b5) into main (0190882) will increase coverage by 0.12%.
The diff coverage is 52.85%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6625      +/-   ##
==========================================
+ Coverage   77.87%   78.00%   +0.12%     
==========================================
  Files         309      310       +1     
  Lines       40665    40740      +75     
==========================================
+ Hits        31669    31779     +110     
+ Misses       8996     8961      -35     

[teor2345]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated

[teor2345]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated

[teor2345]

We're seeing an increased volume of transactions on the network after the ZIP-317 deployment, so this seems like a high priority to get fixed.

[teor2345]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated