From 3de986daed4a2582afca268d4ee7f351f55cdc33 Mon Sep 17 00:00:00 2001 From: Torlando <281092095+torlando-agent[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 17:05:14 +0000 Subject: [PATCH] fix(lxmf): bound automatic path requests to 30 min per identity Greptile P2 on PR #92: with 64 tracked sources, evicting the oldest entry discarded its open cooldown, so a flood of distinct bogus identities reset other sources' windows and forced unbounded path requests (each answered by every peer holding the announce). - Per-identity cooldown 5 min -> 30 min. - The table no longer evicts an open window: while all 64 slots hold unexpired windows, never-before-seen identities are deferred until a slot frees (at most one 30-minute window) instead of dropping someone else's cooldown. Open windows are only ever pruned after they expire. - Aggregate bound: at most kMaxTrackedSources automatic path requests per rolling 30-minute window, capping the worst-case network cost of a rotating-identity flood. Host tests extended: saturated-table deferral, 512-identity flood bound (one window and across consecutive windows), lazy expiry/prune, boundary checks at the 30-minute cooldown. --- .../UI/LXMF/UnknownSourceKeyRequest.h | 75 ++++++++-- .../test_unknown_source_key_request.cpp | 134 +++++++++++++++--- 2 files changed, 177 insertions(+), 32 deletions(-) diff --git a/lib/tdeck_ui/UI/LXMF/UnknownSourceKeyRequest.h b/lib/tdeck_ui/UI/LXMF/UnknownSourceKeyRequest.h index 9ec2e754..eee43022 100644 --- a/lib/tdeck_ui/UI/LXMF/UnknownSourceKeyRequest.h +++ b/lib/tdeck_ui/UI/LXMF/UnknownSourceKeyRequest.h @@ -22,6 +22,23 @@ // Only SOURCE_UNKNOWN should ever be passed here: an invalid signature // from a KNOWN identity is not recoverable by asking the network for the // same key it already has. +// +// Rate limit, two independent bounds: +// 1. Per-identity: at most one automatic path request per unknown +// identity per 30 minutes. Once the first request fires, the source's +// window is tracked and never dropped before it expires, so no +// identity can force a second request within its window — an +// eviction of one source cannot reset another's cooldown. +// 2. Aggregate: at most kMaxTrackedSources automatic requests per +// rolling 30-minute window in total (a request budget that expires +// with the requests it counts). While the budget is exhausted, +// never-before-seen identities are declined even though their own +// per-identity window is empty; once their budget slots free up they +// may fire. This caps the worst-case network cost of a flood of +// rotating bogus identities at kMaxTrackedSources path requests per +// window, instead of leaving the answer bandwidth unbounded. +// Cost: a 65th honest new peer in a congested window is deferred by +// at most one 30-minute window. // --------------------------------------------------------------------------- #include @@ -34,34 +51,66 @@ namespace UI { namespace LXMF { struct UnknownSourceKeyRequestPolicy { - static constexpr unsigned long long kCooldownMillis = 5U * 60U * 1000U; + static constexpr unsigned long long kCooldownMillis = 30U * 60U * 1000U; + // Aggregate bound: maximum number of automatic path requests in any + // rolling 30-minute window. Also bounds the per-source tracking table + // (one open window per fired request). static constexpr unsigned kMaxTrackedSources = 64U; - std::vector> last_requested; + struct Entry { + std::string source_hex; + unsigned long long last_requested_millis = 0; + }; - bool should_request(const std::string& source_hex, - unsigned long long now_millis) const { - for (const auto& entry : last_requested) { - if (entry.first == source_hex) { - return now_millis >= - entry.second + kCooldownMillis; + // One entry per fired request whose window is still open. + std::vector last_requested; + + // Drop entries whose window has fully elapsed. O(n) with n <= + // kMaxTrackedSources; called lazily on every decision. + void prune_expired(unsigned long long now_millis) { + for (unsigned i = 0; i < last_requested.size();) { + if (now_millis >= + last_requested[i].last_requested_millis + kCooldownMillis) { + last_requested.erase(last_requested.begin() + i); + } else { + ++i; } } - return true; + } + + bool should_request(const std::string& source_hex, + unsigned long long now_millis) { + for (const auto& entry : last_requested) { + if (entry.source_hex == source_hex) { + return now_millis >= + entry.last_requested_millis + kCooldownMillis; + } + } + // Not seen since its own window expired: the request is allowed + // only if the aggregate budget has room, so a flood of distinct + // identities cannot force more than kMaxTrackedSources requests + // per window. + prune_expired(now_millis); + return last_requested.size() < kMaxTrackedSources; } void record_request(const std::string& source_hex, unsigned long long now_millis) { for (auto& entry : last_requested) { - if (entry.first == source_hex) { - entry.second = now_millis; + if (entry.source_hex == source_hex) { + entry.last_requested_millis = now_millis; return; } } + prune_expired(now_millis); if (last_requested.size() >= kMaxTrackedSources) { - last_requested.erase(last_requested.begin()); + // Defensive: should_request() gates this; if the budget is + // full the request was not authorized and must not be + // recorded (recording would extend the budget for an + // unauthorized request). + return; } - last_requested.emplace_back(source_hex, now_millis); + last_requested.push_back(Entry{source_hex, now_millis}); } }; diff --git a/tests/native/test_unknown_source_key_request.cpp b/tests/native/test_unknown_source_key_request.cpp index 4541a006..bcde96b3 100644 --- a/tests/native/test_unknown_source_key_request.cpp +++ b/tests/native/test_unknown_source_key_request.cpp @@ -4,9 +4,11 @@ // The policy decides when the firmware fires an RNS path request for an // LXMF message from an identity it has not learned (Sideband's "Query // Network For Keys" equivalent). The decision must be pure and bounded: -// one request per source per cooldown, independent per source, and a -// capped tracking table so an opportunistic flood of unknown senders can -// neither hammer the transport nor starve the table forever. +// one request per source per 30-minute window, independent per source, +// and an aggregate bound — a flood of distinct bogus identities must +// never force more than kMaxTrackedSources requests per window, and a +// source whose window is open must stay suppressed for the whole window +// no matter what. #include #include #include @@ -32,8 +34,8 @@ int failures = 0; using UI::LXMF::UnknownSourceKeyRequestPolicy; std::string sourceHex(unsigned seed) { - // Distinct 8-hex string per seed up to 0xffffff (the tests use <= 65), - // so the capacity-cap test has no hash collisions. + // Distinct 8-hex string per seed up to 0xffffff (the tests use <= + // 1512), so the flood tests have no hash collisions. const char* digits = "0123456789abcdef"; std::string hex; for (unsigned shift : {28U, 24U, 20U, 16U, 12U, 8U, 4U, 0U}) { @@ -53,7 +55,7 @@ void testCooldownSuppressesRepeat() { UnknownSourceKeyRequestPolicy policy; const std::string src = sourceHex(2); policy.record_request(src, 1000); - // Within the 5-minute cooldown: suppressed. + // Within the 30-minute cooldown: suppressed. CHECK(!policy.should_request(src, 1001)); CHECK(!policy.should_request(src, 1000 + UnknownSourceKeyRequestPolicy::kCooldownMillis - 1)); // At and after the cooldown: requested again. @@ -85,22 +87,32 @@ void testSourcesTrackedIndependently() { CHECK(!policy.should_request(b, 300)); } -void testCapacityCapEvictsOldest() { +void testSaturatedTableDeclinesNewSources() { + // While every tracking slot holds an open window, a never-before-seen + // source is DECLINED (deferred), not tracked by dropping someone + // else's window. Existing sources stay suppressed throughout. UnknownSourceKeyRequestPolicy policy; const unsigned cap = UnknownSourceKeyRequestPolicy::kMaxTrackedSources; - for (unsigned i = 0; i < cap; ++i) { - policy.record_request(sourceHex(i + 1), 1000 + static_cast(i)); + for (unsigned i = 1; i <= cap; ++i) { + policy.record_request(sourceHex(i), + 1000 + static_cast(i) * 1000); } CHECK(policy.last_requested.size() == cap); - // New source pushes the oldest (seed 1) out of the table. - const std::string evicted = sourceHex(1); - const std::string newest = sourceHex(cap + 1); - policy.record_request(newest, 5000); + const std::string stranger = sourceHex(1000); + CHECK(!policy.should_request(stranger, 2000)); + // Nobody's open window was dropped to make room: existing sources are + // still suppressed. + CHECK(!policy.should_request(sourceHex(1), 2000)); + CHECK(!policy.should_request(sourceHex(cap), 2000)); + CHECK(policy.last_requested.size() == cap); + // The deferred source stays declined while the budget is full, and + // becomes eligible as soon as the first recorded window expires + // (src 1, recorded at 2000, expires at 2000 + cooldown). + CHECK(!policy.should_request(stranger, 1801000 - 1)); // all still open + const unsigned long long after_windows = 2000 + UnknownSourceKeyRequestPolicy::kCooldownMillis; + CHECK(policy.should_request(stranger, after_windows)); + policy.record_request(stranger, after_windows); CHECK(policy.last_requested.size() == cap); - // The evicted source is no longer rate-limited (its entry is gone). - CHECK(policy.should_request(evicted, 1010)); - // The most recent source IS rate-limited. - CHECK(!policy.should_request(newest, 5001)); } void testSteadyStateCostIsZero() { @@ -119,8 +131,89 @@ void testSteadyStateCostIsZero() { CHECK(policy.should_request(learned, UnknownSourceKeyRequestPolicy::kCooldownMillis)); } +void testFloodBoundWorstCaseAirtime() { + // Worst-case airtime an attacker can force: flood more distinct bogus + // identities than the table can track, all inside one 30-minute + // window. The total number of requests fired must be bounded by the + // table cap, and every fired source must stay suppressed for the rest + // of the window. + UnknownSourceKeyRequestPolicy policy; + const unsigned cap = UnknownSourceKeyRequestPolicy::kMaxTrackedSources; + const unsigned long long t0 = 100000; + const unsigned flood = 512; + unsigned requests_fired = 0; + for (unsigned i = 1; i <= flood; ++i) { + const std::string src = sourceHex(1000 + i); + const unsigned long long now = t0 + static_cast(i) * 1000; + if (policy.should_request(src, now)) { + ++requests_fired; + policy.record_request(src, now); + } + } + CHECK(requests_fired == cap); + CHECK(policy.last_requested.size() == cap); + // No identity whose window is open can fire a second time. + unsigned violations = 0; + for (unsigned i = 1; i <= flood; ++i) { + const std::string src = sourceHex(1000 + i); + const unsigned long long now = t0 + 29U * 60U * 1000U; + if (policy.should_request(src, now)) { + ++violations; + } + } + CHECK(violations == 0); +} + +void testFloodBoundAcrossConsecutiveWindows() { + // Aggregate bound: the same 512 identities re-ask in a second + // 30-minute window (each exactly one window after its first request, + // staggered one per second). The first window's budget only frees one + // slot per second as the first requests expire, so the 65th identity's + // second request is still declined — at most cap requests may fire in + // the second window too. + UnknownSourceKeyRequestPolicy policy; + const unsigned cap = UnknownSourceKeyRequestPolicy::kMaxTrackedSources; + const unsigned long long t0 = 100000; + const unsigned flood = 512; + unsigned first_window = 0; + unsigned second_window = 0; + for (unsigned i = 1; i <= flood; ++i) { + const std::string src = sourceHex(1000 + i); + const unsigned long long w1 = t0 + static_cast(i) * 1000; + if (policy.should_request(src, w1)) { + ++first_window; + policy.record_request(src, w1); + } + const unsigned long long w2 = w1 + UnknownSourceKeyRequestPolicy::kCooldownMillis; + if (policy.should_request(src, w2)) { + ++second_window; + policy.record_request(src, w2); + } + } + CHECK(first_window == cap); + CHECK(second_window == cap); +} + +void testWindowExpiryPrunesAndAllows() { + UnknownSourceKeyRequestPolicy policy; + const std::string a = sourceHex(20); + const std::string b = sourceHex(21); + policy.record_request(a, 0); + policy.record_request(b, 5000); + CHECK(!policy.should_request(a, 6000)); + CHECK(!policy.should_request(b, 6000)); + // a's window expires before b's; the table prunes a lazily and makes + // room for a new source while b stays suppressed. + CHECK(policy.should_request(a, UnknownSourceKeyRequestPolicy::kCooldownMillis)); + const std::string c = sourceHex(22); + CHECK(policy.should_request(c, UnknownSourceKeyRequestPolicy::kCooldownMillis)); + policy.record_request(c, UnknownSourceKeyRequestPolicy::kCooldownMillis); + CHECK(!policy.should_request(b, UnknownSourceKeyRequestPolicy::kCooldownMillis)); + CHECK(!policy.should_request(c, UnknownSourceKeyRequestPolicy::kCooldownMillis + 1)); +} + void testConstants() { - CHECK(UnknownSourceKeyRequestPolicy::kCooldownMillis == 5U * 60U * 1000U); + CHECK(UnknownSourceKeyRequestPolicy::kCooldownMillis == 30U * 60U * 1000U); CHECK(UnknownSourceKeyRequestPolicy::kMaxTrackedSources == 64U); } @@ -132,8 +225,11 @@ int main() { testCooldownSuppressesRepeat(); testReRecordResetsCooldown(); testSourcesTrackedIndependently(); - testCapacityCapEvictsOldest(); + testSaturatedTableDeclinesNewSources(); testSteadyStateCostIsZero(); + testFloodBoundWorstCaseAirtime(); + testFloodBoundAcrossConsecutiveWindows(); + testWindowExpiryPrunesAndAllows(); std::cout << "unknown source key request policy: " << passed << " passed, " << failures << " failed" << std::endl; return failures == 0 ? 0 : 1;