fix(net): Rate-limit MetaAddrChange::Responded from peers (#6738)

* Rate-limit MetaAddrChange::Responded from peers

* Document rate-limits on the address book updater channel
This commit is contained in:
teor 2023-05-24 06:50:29 +10:00 committed by GitHub
parent 0b8e73206f
commit 0918663e3e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 46 additions and 12 deletions

View File

@ -914,6 +914,10 @@ where
// addresses. Otherwise, malicious peers could interfere with the
// address book state of other peers by providing their addresses in
// `Version` messages.
//
// New alternate peer address and peer responded updates are rate-limited because:
// - opening connections is rate-limited
// - we only send these messages once per handshake
let alternate_addrs = connected_addr.get_alternate_addrs(remote_canonical_addr);
for alt_addr in alternate_addrs {
let alt_addr = MetaAddr::new_alternate(alt_addr, &remote_services);
@ -1010,18 +1014,10 @@ where
"addr" => connected_addr.get_transient_addr_label(),
);
if let Some(book_addr) = connected_addr.get_address_book_addr() {
if matches!(msg, Message::Ping(_) | Message::Pong(_)) {
// the collector doesn't depend on network activity,
// so this await should not hang
let _ = inbound_ts_collector
.send(MetaAddr::new_responded(
book_addr,
&remote_services,
))
.await;
}
}
// # Security
//
// Peer messages are not rate-limited, so we can't send anything
// to a shared channel or do anything expensive here.
}
Err(err) => {
metrics::counter!(
@ -1031,6 +1027,12 @@ where
"addr" => connected_addr.get_transient_addr_label(),
);
// # Security
//
// Peer errors are rate-limited because:
// - opening connections is rate-limited
// - the number of connections is limited
// - after the first error, the peer is disconnected
if let Some(book_addr) = connected_addr.get_address_book_addr() {
let _ = inbound_ts_collector
.send(MetaAddr::new_errored(book_addr, remote_services))
@ -1295,6 +1297,20 @@ async fn send_periodic_heartbeats_run_loop(
&remote_services,
)
.await?;
// # Security
//
// Peer heartbeats are rate-limited because:
// - opening connections is rate-limited
// - the number of connections is limited
// - Zebra initiates each heartbeat using a timer
if let Some(book_addr) = connected_addr.get_address_book_addr() {
// the collector doesn't depend on network activity,
// so this await should not hang
let _ = heartbeat_ts_collector
.send(MetaAddr::new_responded(book_addr, &remote_services))
.await;
}
}
unreachable!("unexpected IntervalStream termination")
@ -1399,6 +1415,12 @@ where
Err(err) => {
tracing::debug!(?err, "heartbeat error, shutting down");
// # Security
//
// Peer errors and shutdowns are rate-limited because:
// - opening connections is rate-limited
// - the number of connections is limited
// - after the first error or shutdown, the peer is disconnected
if let Some(book_addr) = connected_addr.get_address_book_addr() {
let _ = address_book_updater
.send(MetaAddr::new_errored(book_addr, *remote_services))

View File

@ -316,6 +316,12 @@ where
/// Add new `addrs` to the address book.
async fn send_addrs(&self, addrs: impl IntoIterator<Item = MetaAddr>) {
// # Security
//
// New gossiped peers are rate-limited because:
// - Zebra initiates requests for new gossiped peers
// - the fanout is limited
// - the number of addresses per peer is limited
let addrs: Vec<MetaAddrChange> = addrs
.into_iter()
.map(MetaAddr::new_gossiped_change)

View File

@ -433,6 +433,12 @@ async fn limit_initial_peers(
// Send every initial peer to the address book, in preferred order.
// (This treats initial peers the same way we treat gossiped peers.)
//
// # Security
//
// Initial peers are limited because:
// - the number of initial peers is limited
// - this code only runs once at startup
for peer in preferred_peers.values().flatten() {
let peer_addr = MetaAddr::new_initial_peer(*peer);
// `send` only waits when the channel is full.