Free pending_query_count slot when DNS proof build fails#4591
Conversation
`OMDomainResolver` rate-limits in-flight DNSSEC proof builds via a `pending_query_count` counter capped at `MAX_PENDING_RESPONSES` (1024). The counter was only released when the proof build succeeded, so any failure mode -- NXDOMAIN, insecure zones, unreachable resolvers, I/O timeouts, malformed names -- permanently consumed a slot. Because the queried name is attacker-controlled (it travels in over a `DNSSECQuery` onion message from any LN peer, given DNS resolution is an opt-in network-advertised feature), an adversary could exhaust the counter with ~1025 failing queries and persistently DoS the resolver for any subsequent legitimate BIP-353 lookups, until the process is restarted. Always release the slot once the proof build completes, regardless of outcome, and add a regression test which points the resolver at a TCP-refusing local port and asserts the counter returns to zero. Co-Authored-By: HAL 9000
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
|
The fix is correct and the PR looks clean. The core change correctly moves the No issues found. |
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4591 +/- ##
==========================================
- Coverage 86.84% 86.25% -0.59%
==========================================
Files 161 159 -2
Lines 109260 109242 -18
Branches 109260 109242 -18
==========================================
- Hits 94882 94226 -656
- Misses 11797 12405 +608
- Partials 2581 2611 +30
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4 participants
OMDomainResolverrate-limits in-flight DNSSEC proof builds via apending_query_countcounter capped atMAX_PENDING_RESPONSES(1024). The counter was only released when the proof build succeeded, so any failure mode -- NXDOMAIN, insecure zones, unreachable resolvers, I/O timeouts, malformed names -- permanently consumed a slot.Because the queried name is attacker-controlled (it travels in over a
DNSSECQueryonion message from any LN peer, given DNS resolution is an opt-in network-advertised feature), an adversary could exhaust the counter with ~1025 failing queries and persistently DoS the resolver for any subsequent legitimate BIP-353 lookups, until the process is restarted.Always release the slot once the proof build completes, regardless of outcome, and add a regression test which points the resolver at a TCP-refusing local port and asserts the counter returns to zero.
Co-Authored-By: HAL 9000