mirror of
https://github.com/jmcorgan/fips.git
synced 2026-08-09 08:14:42 +00:00
Validate bloom filter fill ratio on FilterAnnounce ingress
A malformed FilterAnnounce whose fill ratio produces an implausibly high false-positive rate is mostly useless for routing and, once merged into our outgoing filter via bitwise OR, propagates the saturated state to tree peers one hop per announce tick. A saturated filter also made estimated_count() return f64::INFINITY, which compute_mesh_size summed into its cached estimate. handle_filter_announce now rejects inbound FilterAnnounce whose derived FPR exceeds `node.bloom.max_inbound_fpr` (new config field, default 0.05 ≈ fill 0.549 at k=5). Rejection is silent on the wire, logs at WARN, and increments a new `bloom.fill_exceeded` counter. The peer's prior stored filter and filter_sequence are left unchanged so a single rejected announce does not wipe the peer's existing contribution to aggregation. After a successful outgoing FilterAnnounce send, a rate-limited WARN fires if our own filter's FPR exceeds the same cap, surfacing aggregation drift. Limited to once per 60 seconds via a new Node.last_self_warn field. BloomFilter::estimated_count() now takes max_fpr and returns Option<f64>. Returns None for saturated filters (regardless of cap) or when the filter's FPR exceeds max_fpr. Callers updated: debug logs render None as "—", the Debug impl uses f64::INFINITY as "no cap" and prints "saturated" instead of inf, control-socket JSON emits null, and compute_mesh_size propagates None into the already- Option<u64> estimated_mesh_size field.
This commit is contained in:
+58
-3
@@ -9,7 +9,7 @@ use crate::protocol::FilterAnnounce;
|
||||
|
||||
use super::{Node, NodeError};
|
||||
use std::collections::HashMap;
|
||||
use tracing::debug;
|
||||
use tracing::{debug, warn};
|
||||
|
||||
impl Node {
|
||||
/// Collect inbound filters from all peers for outgoing filter computation.
|
||||
@@ -78,11 +78,41 @@ impl Node {
|
||||
|
||||
self.stats_mut().bloom.sent += 1;
|
||||
|
||||
// Self-plausibility check: WARN if our own outgoing filter is
|
||||
// above the antipoison cap. Independent detection signal if
|
||||
// aggregation drift or an ingress-check bypass pushes us over
|
||||
// despite M1. Rate-limited to once per 60s globally — outgoing
|
||||
// cadence can be per-tick during churn, and we want the
|
||||
// operator to see one clear message, not spam.
|
||||
let max_fpr = self.config.node.bloom.max_inbound_fpr;
|
||||
let out_fill = sent_filter.fill_ratio();
|
||||
let out_fpr = out_fill.powi(sent_filter.hash_count() as i32);
|
||||
if out_fpr > max_fpr {
|
||||
let now = std::time::Instant::now();
|
||||
let should_warn = self
|
||||
.last_self_warn
|
||||
.map(|t| now.duration_since(t) >= std::time::Duration::from_secs(60))
|
||||
.unwrap_or(true);
|
||||
if should_warn {
|
||||
self.last_self_warn = Some(now);
|
||||
warn!(
|
||||
to = %self.peer_display_name(peer_addr),
|
||||
fill = format_args!("{:.3}", out_fill),
|
||||
fpr = format_args!("{:.4}", out_fpr),
|
||||
cap = format_args!("{:.4}", max_fpr),
|
||||
"Outgoing filter above FPR cap — aggregation drift or missed ingress?"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Record send and store the filter for change detection
|
||||
debug!(
|
||||
peer = %self.peer_display_name(peer_addr),
|
||||
seq = announce.sequence,
|
||||
est_entries = format_args!("{:.0}", sent_filter.estimated_count()),
|
||||
est_entries = match sent_filter.estimated_count(max_fpr) {
|
||||
Some(n) => format!("{:.0}", n),
|
||||
None => "—".to_string(),
|
||||
},
|
||||
set_bits = sent_filter.count_ones(),
|
||||
fill = format_args!("{:.1}%", sent_filter.fill_ratio() * 100.0),
|
||||
tree_peer = self.is_tree_peer(peer_addr),
|
||||
@@ -174,6 +204,28 @@ impl Node {
|
||||
return;
|
||||
}
|
||||
|
||||
// Antipoison FPR cap. Reject announces whose FPR exceeds
|
||||
// node.bloom.max_inbound_fpr. Silent on the wire (no NACK) —
|
||||
// the peer's prior accepted filter and filter_sequence stay
|
||||
// untouched so the peer is not permanently silenced and an
|
||||
// on-path attacker cannot weaponize a single corrupted frame
|
||||
// to wipe a victim's contribution to aggregation.
|
||||
let max_fpr = self.config.node.bloom.max_inbound_fpr;
|
||||
let fill = announce.filter.fill_ratio();
|
||||
let fpr = fill.powi(announce.filter.hash_count() as i32);
|
||||
if fpr > max_fpr {
|
||||
self.stats_mut().bloom.fill_exceeded += 1;
|
||||
warn!(
|
||||
from = %self.peer_display_name(from),
|
||||
seq = announce.sequence,
|
||||
fill = format_args!("{:.3}", fill),
|
||||
fpr = format_args!("{:.4}", fpr),
|
||||
cap = format_args!("{:.4}", max_fpr),
|
||||
"FilterAnnounce above FPR cap — rejected"
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
self.stats_mut().bloom.accepted += 1;
|
||||
|
||||
let now_ms = std::time::SystemTime::now()
|
||||
@@ -184,7 +236,10 @@ impl Node {
|
||||
debug!(
|
||||
from = %self.peer_display_name(from),
|
||||
seq = announce.sequence,
|
||||
est_entries = format_args!("{:.0}", announce.filter.estimated_count()),
|
||||
est_entries = match announce.filter.estimated_count(max_fpr) {
|
||||
Some(n) => format!("{:.0}", n),
|
||||
None => "—".to_string(),
|
||||
},
|
||||
set_bits = announce.filter.count_ones(),
|
||||
fill = format_args!("{:.1}%", announce.filter.fill_ratio() * 100.0),
|
||||
tree_peer = self.is_tree_peer(from),
|
||||
|
||||
+35
-5
@@ -430,6 +430,13 @@ pub struct Node {
|
||||
/// Timestamp of last mesh size log emission.
|
||||
last_mesh_size_log: Option<std::time::Instant>,
|
||||
|
||||
// === Bloom Self-Plausibility ===
|
||||
/// Rate-limit state for the self-plausibility WARN. Fires at most
|
||||
/// once per 60s globally when our own outgoing FilterAnnounce has
|
||||
/// an FPR above `node.bloom.max_inbound_fpr`, signalling either
|
||||
/// aggregation drift or an ingress bypass.
|
||||
last_self_warn: Option<std::time::Instant>,
|
||||
|
||||
// === Display Names ===
|
||||
/// Human-readable names for configured peers (alias or short npub).
|
||||
/// Populated at startup from peer config.
|
||||
@@ -558,6 +565,7 @@ impl Node {
|
||||
last_congestion_log: None,
|
||||
estimated_mesh_size: None,
|
||||
last_mesh_size_log: None,
|
||||
last_self_warn: None,
|
||||
peer_aliases: HashMap::new(),
|
||||
host_map,
|
||||
})
|
||||
@@ -663,6 +671,7 @@ impl Node {
|
||||
last_congestion_log: None,
|
||||
estimated_mesh_size: None,
|
||||
last_mesh_size_log: None,
|
||||
last_self_warn: None,
|
||||
peer_aliases: HashMap::new(),
|
||||
host_map,
|
||||
}
|
||||
@@ -952,17 +961,30 @@ impl Node {
|
||||
let parent_id = *self.tree_state.my_declaration().parent_id();
|
||||
let is_root = self.tree_state.is_root();
|
||||
|
||||
let max_fpr = self.config.node.bloom.max_inbound_fpr;
|
||||
let mut total: f64 = 1.0; // count self
|
||||
let mut child_count: u32 = 0;
|
||||
let mut has_data = false;
|
||||
|
||||
// Parent's filter: nodes reachable upward through the tree
|
||||
// Parent's filter: nodes reachable upward through the tree.
|
||||
// If any contributing filter is above the FPR cap, we refuse to
|
||||
// estimate rather than substitute a partial/biased aggregate —
|
||||
// Node.estimated_mesh_size is already Option<u64> and consumers
|
||||
// (control socket, fipstop, periodic debug log) handle None.
|
||||
if !is_root
|
||||
&& let Some(parent) = self.peers.get(&parent_id)
|
||||
&& let Some(filter) = parent.inbound_filter()
|
||||
{
|
||||
total += filter.estimated_count();
|
||||
has_data = true;
|
||||
match filter.estimated_count(max_fpr) {
|
||||
Some(n) => {
|
||||
total += n;
|
||||
has_data = true;
|
||||
}
|
||||
None => {
|
||||
self.estimated_mesh_size = None;
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Children's filters: each child's subtree is disjoint
|
||||
@@ -972,8 +994,16 @@ impl Node {
|
||||
{
|
||||
child_count += 1;
|
||||
if let Some(filter) = peer.inbound_filter() {
|
||||
total += filter.estimated_count();
|
||||
has_data = true;
|
||||
match filter.estimated_count(max_fpr) {
|
||||
Some(n) => {
|
||||
total += n;
|
||||
has_data = true;
|
||||
}
|
||||
None => {
|
||||
self.estimated_mesh_size = None;
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -211,6 +211,7 @@ pub struct BloomStats {
|
||||
pub non_v1: u64,
|
||||
pub unknown_peer: u64,
|
||||
pub stale: u64,
|
||||
pub fill_exceeded: u64,
|
||||
pub accepted: u64,
|
||||
// Outbound announce sending
|
||||
pub sent: u64,
|
||||
@@ -227,6 +228,7 @@ impl BloomStats {
|
||||
non_v1: self.non_v1,
|
||||
unknown_peer: self.unknown_peer,
|
||||
stale: self.stale,
|
||||
fill_exceeded: self.fill_exceeded,
|
||||
accepted: self.accepted,
|
||||
sent: self.sent,
|
||||
debounce_suppressed: self.debounce_suppressed,
|
||||
@@ -397,6 +399,7 @@ pub struct BloomStatsSnapshot {
|
||||
pub non_v1: u64,
|
||||
pub unknown_peer: u64,
|
||||
pub stale: u64,
|
||||
pub fill_exceeded: u64,
|
||||
pub accepted: u64,
|
||||
pub sent: u64,
|
||||
pub debounce_suppressed: u64,
|
||||
|
||||
+11
-4
@@ -270,10 +270,13 @@ fn print_filter_cardinality(nodes: &[TestNode]) {
|
||||
{
|
||||
let is_tree = tn.node.is_tree_peer(&addr);
|
||||
println!(
|
||||
" n{} <- n{}: est={:.1} set_bits={} fill={:.1}% tree={}",
|
||||
" n{} <- n{}: est={} set_bits={} fill={:.1}% tree={}",
|
||||
i,
|
||||
j,
|
||||
filter.estimated_count(),
|
||||
match filter.estimated_count(f64::INFINITY) {
|
||||
Some(n) => format!("{:.1}", n),
|
||||
None => "saturated".to_string(),
|
||||
},
|
||||
filter.count_ones(),
|
||||
filter.fill_ratio() * 100.0,
|
||||
is_tree,
|
||||
@@ -369,7 +372,9 @@ async fn test_bloom_filter_split_horizon() {
|
||||
}
|
||||
|
||||
// Cardinality should match subtree size
|
||||
let up_est = filter_up.estimated_count();
|
||||
let up_est = filter_up
|
||||
.estimated_count(f64::INFINITY)
|
||||
.expect("upward filter should not be saturated in tree convergence test");
|
||||
assert!(
|
||||
(up_est - child_subtree.len() as f64).abs() < 1.5,
|
||||
"Upward filter (n{}→n{}): expected ~{} entries, got {:.1}",
|
||||
@@ -414,7 +419,9 @@ async fn test_bloom_filter_split_horizon() {
|
||||
}
|
||||
|
||||
// Cardinality should match complement size
|
||||
let down_est = filter_down.estimated_count();
|
||||
let down_est = filter_down
|
||||
.estimated_count(f64::INFINITY)
|
||||
.expect("downward filter should not be saturated in tree convergence test");
|
||||
assert!(
|
||||
(down_est - complement.len() as f64).abs() < 1.5,
|
||||
"Downward filter (n{}→n{}): expected ~{} entries, got {:.1}",
|
||||
|
||||
@@ -0,0 +1,164 @@
|
||||
//! Direct tests for the M1 antipoison FPR cap in handle_filter_announce.
|
||||
//!
|
||||
//! These tests construct a minimal Node with a single synthetic peer,
|
||||
//! then call handle_filter_announce directly with crafted FilterAnnounce
|
||||
//! payloads. Focused on the ingress check semantics; broader
|
||||
//! filter-exchange behavior is covered by the multi-node tests in
|
||||
//! bloom.rs.
|
||||
|
||||
use super::*;
|
||||
use crate::bloom::{BloomFilter, DEFAULT_FILTER_SIZE_BITS, DEFAULT_HASH_COUNT};
|
||||
use crate::peer::ActivePeer;
|
||||
use crate::protocol::FilterAnnounce;
|
||||
|
||||
/// Inject a synthetic active peer into the node with a known NodeAddr.
|
||||
/// Returns the peer's NodeAddr.
|
||||
fn inject_peer(node: &mut Node) -> NodeAddr {
|
||||
let peer_identity = make_peer_identity();
|
||||
let peer_addr = *peer_identity.node_addr();
|
||||
let peer = ActivePeer::new(peer_identity, LinkId::new(1), 0);
|
||||
node.peers.insert(peer_addr, peer);
|
||||
peer_addr
|
||||
}
|
||||
|
||||
/// Encode a FilterAnnounce to the payload format handle_filter_announce
|
||||
/// expects (msg_type byte stripped).
|
||||
fn encode_payload(announce: &FilterAnnounce) -> Vec<u8> {
|
||||
let mut full = announce.encode().unwrap();
|
||||
full.remove(0); // strip msg_type byte
|
||||
full
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_m1_rejects_all_ones_filter_announce() {
|
||||
let mut node = make_node();
|
||||
let peer_addr = inject_peer(&mut node);
|
||||
|
||||
// Craft an all-ones FilterAnnounce (the observed-in-the-wild attack).
|
||||
let all_ones = BloomFilter::from_bytes(
|
||||
vec![0xFFu8; DEFAULT_FILTER_SIZE_BITS / 8],
|
||||
DEFAULT_HASH_COUNT,
|
||||
)
|
||||
.unwrap();
|
||||
let announce = FilterAnnounce::new(all_ones, 1);
|
||||
let payload = encode_payload(&announce);
|
||||
|
||||
let before_fill_exceeded = node.stats().bloom.fill_exceeded;
|
||||
let before_accepted = node.stats().bloom.accepted;
|
||||
|
||||
node.handle_filter_announce(&peer_addr, &payload).await;
|
||||
|
||||
let after = &node.stats().bloom;
|
||||
assert_eq!(
|
||||
after.fill_exceeded,
|
||||
before_fill_exceeded + 1,
|
||||
"fill_exceeded counter must increment on all-ones rejection"
|
||||
);
|
||||
assert_eq!(
|
||||
after.accepted, before_accepted,
|
||||
"accepted counter must NOT increment on rejection"
|
||||
);
|
||||
|
||||
// Peer state unchanged: no filter stored, sequence not advanced.
|
||||
let peer = node.get_peer(&peer_addr).expect("peer still present");
|
||||
assert!(
|
||||
peer.inbound_filter().is_none(),
|
||||
"peer must NOT have a stored filter after rejection"
|
||||
);
|
||||
assert_eq!(
|
||||
peer.filter_sequence(),
|
||||
0,
|
||||
"peer filter_sequence must NOT advance on rejection"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_m1_accepts_sub_cap_filter() {
|
||||
let mut node = make_node();
|
||||
let peer_addr = inject_peer(&mut node);
|
||||
|
||||
// A legitimate filter with 50 entries — fill ~0.03, FPR ~2e-8,
|
||||
// far below the 0.05 cap. Represents normal mesh traffic.
|
||||
let mut filter = BloomFilter::new();
|
||||
for i in 0..50u8 {
|
||||
let mut bytes = [0u8; 16];
|
||||
bytes[0] = i;
|
||||
filter.insert(&NodeAddr::from_bytes(bytes));
|
||||
}
|
||||
let announce = FilterAnnounce::new(filter, 1);
|
||||
let payload = encode_payload(&announce);
|
||||
|
||||
let before_fill_exceeded = node.stats().bloom.fill_exceeded;
|
||||
let before_accepted = node.stats().bloom.accepted;
|
||||
|
||||
node.handle_filter_announce(&peer_addr, &payload).await;
|
||||
|
||||
let after = &node.stats().bloom;
|
||||
assert_eq!(
|
||||
after.fill_exceeded, before_fill_exceeded,
|
||||
"fill_exceeded must NOT increment on legitimate sub-cap filter"
|
||||
);
|
||||
assert_eq!(
|
||||
after.accepted,
|
||||
before_accepted + 1,
|
||||
"accepted must increment on legitimate filter"
|
||||
);
|
||||
|
||||
// Peer state updated: filter stored, sequence advanced.
|
||||
let peer = node.get_peer(&peer_addr).expect("peer still present");
|
||||
assert!(
|
||||
peer.inbound_filter().is_some(),
|
||||
"peer must have a stored filter after acceptance"
|
||||
);
|
||||
assert_eq!(
|
||||
peer.filter_sequence(),
|
||||
1,
|
||||
"peer filter_sequence must advance to announce's sequence"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_m1_sequence_not_advanced_allows_recovery() {
|
||||
// Confirms the "keep prior filter, don't advance seq" rejection
|
||||
// semantics: a compliant announce after a rejected one still
|
||||
// succeeds at seq=1, because the rejected announce (also seq=1)
|
||||
// did not advance the peer's recorded sequence.
|
||||
let mut node = make_node();
|
||||
let peer_addr = inject_peer(&mut node);
|
||||
|
||||
// First announce: all-ones, rejected.
|
||||
let bad = BloomFilter::from_bytes(
|
||||
vec![0xFFu8; DEFAULT_FILTER_SIZE_BITS / 8],
|
||||
DEFAULT_HASH_COUNT,
|
||||
)
|
||||
.unwrap();
|
||||
let bad_announce = FilterAnnounce::new(bad, 1);
|
||||
node.handle_filter_announce(&peer_addr, &encode_payload(&bad_announce))
|
||||
.await;
|
||||
assert_eq!(
|
||||
node.get_peer(&peer_addr).unwrap().filter_sequence(),
|
||||
0,
|
||||
"rejected announce must not advance sequence"
|
||||
);
|
||||
|
||||
// Second announce: legitimate, seq=1 (would be stale if rejection
|
||||
// had advanced the recorded sequence). Must be accepted.
|
||||
let mut good = BloomFilter::new();
|
||||
for i in 0..10u8 {
|
||||
let mut bytes = [0u8; 16];
|
||||
bytes[0] = i;
|
||||
good.insert(&NodeAddr::from_bytes(bytes));
|
||||
}
|
||||
let good_announce = FilterAnnounce::new(good, 1);
|
||||
node.handle_filter_announce(&peer_addr, &encode_payload(&good_announce))
|
||||
.await;
|
||||
|
||||
let peer = node.get_peer(&peer_addr).unwrap();
|
||||
assert!(
|
||||
peer.inbound_filter().is_some(),
|
||||
"compliant announce at same seq must be accepted after rejection"
|
||||
);
|
||||
assert_eq!(peer.filter_sequence(), 1);
|
||||
assert_eq!(node.stats().bloom.fill_exceeded, 1);
|
||||
assert_eq!(node.stats().bloom.accepted, 1);
|
||||
}
|
||||
@@ -5,6 +5,7 @@ use crate::utils::index::SessionIndex;
|
||||
use std::time::Duration;
|
||||
|
||||
mod bloom;
|
||||
mod bloom_poison;
|
||||
mod disconnect;
|
||||
mod discovery;
|
||||
#[cfg(target_os = "linux")]
|
||||
|
||||
Reference in New Issue
Block a user