Add addr-entries spammer detection to the alerts tool - #459
Conversation
Any reason for picking this number? Bitcoin Core treats everything with a rate higher than 1 entry per 10 seconds as "spammer" and rate-limits it. |
|
I anchored it to the per-message cap, not the rate limiter. Since an addr message can hold up to MAX_ADDR_TO_SEND (1000) entries and the message-count rule only trips at 6 messages, I set the threshold at ~2× the per-message cap so it catches a peer sending a couple of maxed-out messages that slip under that rule. |
|
I see - however, I don't think that someone spamming addresses would send multiple 1000 addresses at a time. Normally, addr message with up to 1000 addresses are only send in response to an GETADDR message. Sending them without us having send a GETADDR before will get most of them rate-limited, so it won't have any effect on the node. Additionally, if a goal is to get us to propagate these addr messages, you'll want to send batches smaller than 11 addresses (and not older than 10min): https://github.com/bitcoin/bitcoin/blob/bcbf5bae1645605939f0700c438f0cafb47056dc/src/net_processing.cpp#L5711-L5714 So my recommendation would be to:
Another approach could be to implement a token-bucket similar to Bitcoin Core (bitcoin/bitcoin#22387) and seed it with a higher starting token number. This is probably a bit more work, but could mean that we rule out false-positive alerts better. |
| impl SpammerKind { | ||
| /// The single source of truth for the set of kinds and their order. The | ||
| /// per-peer state array is sized and indexed off this, and `index()` is |
There was a problem hiding this comment.
I think this could happen in it's own commit - it's not directly related to the AddrSpammer?
|
|
||
| ``` | ||
| PeerDisconnected | peer_id=<id> addr=<addr> | active=<duration>s | ||
| PeerDisconnected | peer_id=<id> addr=<addr> | active=<duration>s | flags=[PingSpammer, AddrSpammer] |
There was a problem hiding this comment.
This also seems to be an unrelated addition?
|
I like the token-bucket approach with higher starting token number. Sorry about that, I was just trying to replicate the solution from the first addrSpammer. |
88dba15 to
a1c5e5d
Compare
|
I found some trade-offs working on the token-bucket: The heuristic counts rate-limited entries via a token bucket modeled on Core's limiter, so an alert means "this peer sent addr entries faster than Core would accept" rather than some arbitrary number. That keeps false positives low and can't be dodged by splitting a flood across many small messages. The main limitation: the count is cumulative over the connection (not windowed), and it intentionally doesn't catch relay abuse, which stays under the rate limit on purpose. A follow-up could be a separate relay-focused signal (fresh, small Next (I wanted to share the addr-entries heuristic first): improving the I'd also still like to tighten up the tests a bit more on this one before it's final. |
a1c5e5d to
77a2b3a
Compare
|
I modified the implementation and compared it more closely with Core's behavior. The scope for this PR is therefore an EBPF-only, close-effort estimate of Core's standard addr rate limiting:
This intentionally won't account for peers with the addr permission, since that requires RPC correlation. The alert threshold (default 20) is separate from Core's rate limit. It is configurable headroom so a few excess entries do no t immediately label a a peer, while an unsolicited oversized message still triggers quickly. It keeps the heuristic useful without pulling RPC state management into it. |
|
nice! let me know when this is ready for review! |
18673d8 to
be46383
Compare
Adds an
AddrEntriesSpammerheuristic that flags peers whoseaddr/addrv2messages carry too many entries - catching peers that stay under the message-count threshold by sending a few oversized messages.Detection uses a per-peer token bucket modeled on Core's addr rate limiting.
Part of #185 (the addr-spammer / addr-entries item).
Note: overlaps with anyhow refactor, happy to rebase on top of whichever lands first.