Files
90a6dec3f3 fix(NodeDB): require a full 32-byte key when demoting to the warm tier (#11431)
* fix(NodeDB): require a full 32-byte key when demoting to the warm tier

meshtastic_User.public_key is a wire `bytes` field with max_size 32, so any
size in 0..32 decodes off the air, and nothing validates it on ingress:
NodeInfoModule hands the decoded User straight to NodeDB::updateUser, whose
PKI gates are all `== 32` and so fall through for a partial key, and
TypeConversions::CopyUserToNodeInfoLite then stores it with the short size.

demoteOldestHotNodesToWarm() admitted that partial key into the warm tier on
a `size > 0` gate. WarmNodeEntry has no length field - it distinguishes "has
a key" from "no key" purely by all-zero - so N real bytes plus 32-N zeros
become indistinguishable from a genuine key. copyPublicKeyAuthoritative()
then hands that fabricated key back with size = 32 and reports it
AUTHORITATIVE, and re-admission writes size = 32 into the hot store. From
then on updateUser's key pin permanently rejects the node's real NodeInfo,
and DMs to it are encrypted to a key nobody holds.

Require a full 32-byte key, so a partial one is absorbed as "no key"
(nullptr) rather than as a truncated one. WarmNodeStore::place() already
treats a null key as keyless and clears the slot's stale key when
repurposing it. This aligns the site with its two siblings, which both
already gate on `size == 32` (the purge path in cleanupMeshDB and the
runtime eviction in getOrCreateMeshNode).

The ingress gap - updateUser accepting a 1..31-byte key at all - is a
separate, larger change and is left for its own review.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(NodeDB): shorten warm-demotion comment to two lines

Repo guideline (AGENTS.md): keep code comments to one or two lines. Retains the
non-obvious invariant - warm entries have no key length field - and drops the
restated detail.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(NodeDB): cover short-key demotion into the warm tier

A warm record stores 32 raw key bytes with no length field, so a partial
hot-store key is indistinguishable from a real one once demoted. The
public_key.size == 32 gate in demoteOldestHotNodesToWarm() is what keeps a
truncated key from being laundered into a full-looking warm key, but nothing
exercised it.

test_migration_dropsShortKeyOnDemotion overflows the hot store with one node
carrying a 31-byte key and asserts it lands as a keyless placeholder while a
genuine 32-byte key still survives. push() grows a keySize parameter to seed
the partial key, and clearWarm() gives the test an empty warm tier, which it
needs because the warm store outlives setUp() and a prior run's warm.dat.

Verified to discriminate: with the size gate reverted to size > 0 the new
test fails on "a 31-byte key must not be demoted as if it were a full key",
and passes again once restored.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(NodeDB): assert the keyless placeholder carries last_heard

The test only proved a warm metadata row survived the demotion, not that the
placeholder does the job the nullptr is there for, which is preserving
last_heard when the key is dropped.

Asserting the value needed the seeds fixing first. Warm entries pack role,
protected category and the xeddsa flag into the low 7 bits of last_heard
(WARM_TIME_MASK is 0xFFFFFF80), so warm time has 128 second granularity and the
old seeds of 1, 2, 3 all quantised to 0. They are now multiples of 128, which
keeps the demotion ordering identical and makes the values survive the round
trip. Real last_heard is epoch seconds, so this is closer to production than
the old counter was.

Reads the entry through WarmNodeStore::take() rather than getOrCreateMeshNode(),
which does not restore last_heard from the warm tier and would have been
asserting a path that does not exist.

Reported by CodeRabbit on #11431.

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
2026-08-20 12:19:57 +00:00

354 lines
16 KiB
C++

// Tests for the NodeDB hot-store migration and favourite/ignored (blocked)
// retention paths - src/mesh/NodeDB.cpp.
#include "MeshTypes.h" // BEFORE TestUtil.h - provides WARM_NODE_COUNT / MAX_NUM_NODES via mesh-pb-constants.h
#include "TestUtil.h"
#include <unity.h>
#if defined(ARCH_PORTDUINO)
#define NDB_TEST_ENTRY extern "C"
#else
#define NDB_TEST_ENTRY
#endif
// The migration demotes overflow into the warm tier, so these tests need it.
#if WARM_NODE_COUNT > 0
#include "mesh/NodeDB.h"
#include <cstring>
// Subclass shim: exposes the private maintenance paths (via the friend
// declaration in NodeDB.h) and lets a test own the hot store directly
// (meshNodes/numMeshNodes are public). Declared at global scope so it matches
// `friend class NodeDBTestShim` - an anonymous-namespace class would not.
class NodeDBTestShim : public NodeDB
{
public:
void runDemote() { demoteOldestHotNodesToWarm(); }
void runCleanup() { cleanupMeshDB(); }
void stampUntrusted(NodeNum num, uint32_t uptimeSecs) { recordHeardWhileClockUntrusted(num, uptimeSecs); }
// Read back the role + protected category the warm tier cached for a node.
bool warmMeta(NodeNum n, uint8_t &role, uint8_t &prot) { return warmStore.lookupMeta(n, role, prot); }
bool warmTake(NodeNum n, WarmNodeEntry &out) { return warmStore.take(n, out); }
void clearHot()
{
meshNodes->clear();
numMeshNodes = 0;
}
// The warm tier outlives setUp() (and a prior run's warm.dat), so a test that
// asserts on a warm row has to start from an empty one.
void clearWarm() { warmStore.clear(); }
// keySize < 32 seeds a partial key, as a truncated/short NodeInfo would leave behind.
void push(NodeNum num, uint32_t lastHeard, bool favorite, bool ignored, bool withUser, bool withKey,
meshtastic_Config_DeviceConfig_Role role = meshtastic_Config_DeviceConfig_Role_CLIENT, pb_size_t keySize = 32)
{
meshtastic_NodeInfoLite n = meshtastic_NodeInfoLite_init_zero;
n.num = num;
n.last_heard = lastHeard;
n.role = role;
if (favorite)
nodeInfoLiteSetBit(&n, NODEINFO_BITFIELD_IS_FAVORITE_MASK, true);
if (ignored)
nodeInfoLiteSetBit(&n, NODEINFO_BITFIELD_IS_IGNORED_MASK, true);
if (withUser)
nodeInfoLiteSetBit(&n, NODEINFO_BITFIELD_HAS_USER_MASK, true);
if (withKey) {
n.public_key.size = keySize;
memset(n.public_key.bytes, static_cast<uint8_t>(num & 0xff), keySize);
n.public_key.bytes[0] = 0x01; // ensure non-zero (all-zero == "no key")
}
meshNodes->push_back(n);
numMeshNodes = meshNodes->size();
}
// Index 0 is our own node; the eviction/migration scans treat it as self.
void seedSelf() { push(0x0BADF00D, 0xFFFFFFFFu, false, false, /*withUser=*/true, /*withKey=*/false); }
};
namespace
{
NodeDBTestShim *db = nullptr;
bool warmHasKey(NodeNum n)
{
meshtastic_NodeInfoLite_public_key_t k = {0, {0}};
return db->copyPublicKey(n, k) && k.size == 32;
}
} // namespace
void setUp(void)
{
db->clearHot();
}
void tearDown(void) {}
// Migration: a database from a larger-cap build trims to MAX_NUM_NODES; the
// oldest non-protected nodes are demoted into the warm tier (keys preserved),
// while self, favourites and ignored survive even when they are the oldest.
static void test_migration_demotesOldestKeepsKeepersAndSelf(void)
{
db->seedSelf();
const int extra = MAX_NUM_NODES + 30; // overflow well past the MAX-2 cap
for (int i = 1; i <= extra; i++) {
const bool fav = (i == 1); // oldest, but a favourite
const bool ign = (i == 2); // 2nd-oldest, but blocked
db->push(2000 + i, /*last_heard=*/i, fav, ign, /*withUser=*/true, /*withKey=*/true);
}
db->runDemote();
TEST_ASSERT_EQUAL_INT(MAX_NUM_NODES, (int)db->getNumMeshNodes());
TEST_ASSERT_NOT_NULL(db->getMeshNode(0x0BADF00D)); // self retained
TEST_ASSERT_NOT_NULL(db->getMeshNode(2000 + 1)); // oldest favourite retained
TEST_ASSERT_NOT_NULL(db->getMeshNode(2000 + 2)); // oldest ignored retained
TEST_ASSERT_NOT_NULL(db->getMeshNode(2000 + extra)); // freshest retained
TEST_ASSERT_NULL(db->getMeshNode(2000 + 3)); // oldest non-protected demoted out of hot
TEST_ASSERT_TRUE(warmHasKey(2000 + 3)); // ...but its key kept in the warm tier
}
// Eviction carries the device role + protected category into the warm tier. A TRACKER is
// hop-protected but NOT eviction-protected, so it gets demoted with its key; the warm
// record must report role=TRACKER / category=Role. A plain CLIENT carries role=CLIENT/None.
static void test_migration_carriesRoleAndProtectedIntoWarm(void)
{
db->seedSelf();
const int extra = MAX_NUM_NODES + 30; // overflow so the oldest non-protected are demoted
for (int i = 1; i <= extra; i++) {
const auto role = (i == 3) ? meshtastic_Config_DeviceConfig_Role_TRACKER : meshtastic_Config_DeviceConfig_Role_CLIENT;
db->push(2000 + i, /*last_heard=*/i, /*favorite=*/false, /*ignored=*/false, /*withUser=*/true,
/*withKey=*/true, role);
}
db->runDemote();
uint8_t role = 0xFF, prot = 0xFF;
// TRACKER (i=3): demoted out of hot, key kept, role + protected carried into warm.
TEST_ASSERT_NULL(db->getMeshNode(2000 + 3));
TEST_ASSERT_TRUE(warmHasKey(2000 + 3));
TEST_ASSERT_TRUE(db->warmMeta(2000 + 3, role, prot));
TEST_ASSERT_EQUAL(meshtastic_Config_DeviceConfig_Role_TRACKER, role);
TEST_ASSERT_EQUAL((uint8_t)WarmProtected::Role, prot);
// CLIENT (i=4): also demoted, carries role=CLIENT / category=None.
TEST_ASSERT_TRUE(db->warmMeta(2000 + 4, role, prot));
TEST_ASSERT_EQUAL(meshtastic_Config_DeviceConfig_Role_CLIENT, role);
TEST_ASSERT_EQUAL((uint8_t)WarmProtected::None, prot);
}
// The signer bit is learned from verified traffic, not NodeInfo, so it must survive a warm
// round trip. The plain node is the control: re-admission restores it, it doesn't invent it.
static void test_migration_carriesSignerBitThroughWarm(void)
{
db->seedSelf();
const NodeNum signerNum = 2000 + 3;
const NodeNum plainNum = 2000 + 4;
const int extra = MAX_NUM_NODES + 30; // overflow so the oldest non-protected are demoted
for (int i = 1; i <= extra; i++)
db->push(2000 + i, /*last_heard=*/i, /*favorite=*/false, /*ignored=*/false, /*withUser=*/true, /*withKey=*/true);
nodeInfoLiteSetBit(db->getMeshNode(signerNum), NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK, true);
TEST_ASSERT_TRUE(nodeInfoLiteHasXeddsaSigned(db->getMeshNode(signerNum)));
db->runDemote();
// Both are out of the hot store and held in the warm tier.
TEST_ASSERT_NULL(db->getMeshNode(signerNum));
TEST_ASSERT_NULL(db->getMeshNode(plainNum));
const meshtastic_NodeInfoLite *back = db->getOrCreateMeshNode(signerNum);
TEST_ASSERT_NOT_NULL(back);
TEST_ASSERT_TRUE_MESSAGE(nodeInfoLiteHasXeddsaSigned(back), "signer bit must survive a warm-tier round trip");
const meshtastic_NodeInfoLite *plainBack = db->getOrCreateMeshNode(plainNum);
TEST_ASSERT_NOT_NULL(plainBack);
TEST_ASSERT_FALSE_MESSAGE(nodeInfoLiteHasXeddsaSigned(plainBack), "re-admission must not invent the signer bit");
}
// A warm record stores 32 raw key bytes with no length, so a partial hot-store key would be
// indistinguishable from a real one once demoted. It must land as a keyless placeholder instead.
static void test_migration_dropsShortKeyOnDemotion(void)
{
db->clearWarm();
db->seedSelf();
const NodeNum shortKeyNum = 2000 + 3;
const NodeNum fullKeyNum = 2000 + 4;
const int extra = MAX_NUM_NODES + 30; // overflow so the oldest non-protected are demoted
// Warm entries steal the low 7 bits of last_heard for role and protected-category metadata
// (WARM_TIME_MASK), so seed multiples of 128 to keep the values representable once demoted.
for (int i = 1; i <= extra; i++)
db->push(2000 + i, /*last_heard=*/(uint32_t)i * 128, /*favorite=*/false, /*ignored=*/false, /*withUser=*/true,
/*withKey=*/true, meshtastic_Config_DeviceConfig_Role_CLIENT,
/*keySize=*/(NodeNum)(2000 + i) == shortKeyNum ? 31 : 32);
db->runDemote();
// Both left the hot store; only the full key is allowed through to the warm tier.
TEST_ASSERT_NULL(db->getMeshNode(shortKeyNum));
TEST_ASSERT_NULL(db->getMeshNode(fullKeyNum));
TEST_ASSERT_FALSE_MESSAGE(warmHasKey(shortKeyNum), "a 31-byte key must not be demoted as if it were a full key");
TEST_ASSERT_TRUE_MESSAGE(warmHasKey(fullKeyNum), "a full 32-byte key still survives demotion");
// The short-key node is still held, just keyless, so re-admission restores its last_heard.
uint8_t role = 0xFF, prot = 0xFF;
TEST_ASSERT_TRUE_MESSAGE(db->warmMeta(shortKeyNum, role, prot), "keyless placeholder row must still be present");
WarmNodeEntry placeholder = {};
TEST_ASSERT_TRUE_MESSAGE(db->warmTake(shortKeyNum, placeholder), "placeholder must be readable from the warm tier");
TEST_ASSERT_EQUAL_UINT32_MESSAGE(3u * 128, warmTimeOf(placeholder), "the keyless placeholder must carry last_heard");
}
// Favourite handling: a favourite is never the eviction victim, even when it is
// the oldest node in a full hot store.
static void test_eviction_preservesFavorite(void)
{
db->seedSelf();
for (int i = 1; i < MAX_NUM_NODES; i++) { // fill to MAX_NUM_NODES total (incl. self)
const bool fav = (i == 1); // oldest non-self, favourite
db->push(3000 + i, /*last_heard=*/i, fav, false, /*withUser=*/true, /*withKey=*/true);
}
TEST_ASSERT_EQUAL_INT(MAX_NUM_NODES, (int)db->getNumMeshNodes()); // full
TEST_ASSERT_NOT_NULL(db->getOrCreateMeshNode(0x99990000)); // forces an eviction
TEST_ASSERT_NOT_NULL(db->getMeshNode(3000 + 1)); // favourite survived despite being oldest
TEST_ASSERT_NULL(db->getMeshNode(3000 + 2)); // oldest non-favourite evicted
TEST_ASSERT_NOT_NULL(db->getMeshNode(0x99990000));
}
// A node heard during this boot is newer than every persisted epoch, including valid epochs after
// 2038. Ranking both domains in one uint32_t incorrectly evicts the current-boot node first.
static void test_eviction_prefersCurrentBootStampOverPost2038Epoch(void)
{
constexpr NodeNum futureDated = 0x70000001;
constexpr NodeNum heardThisBoot = 0x70000002;
db->seedSelf();
db->push(futureDated, 0xB5000000u, false, false, /*withUser=*/true, /*withKey=*/true);
db->push(heardThisBoot, 0, false, false, /*withUser=*/true, /*withKey=*/true);
db->stampUntrusted(heardThisBoot, 10);
for (int i = 3; i < MAX_NUM_NODES; i++)
db->push(0x70000000u + i, UINT32_MAX, false, false, /*withUser=*/true, /*withKey=*/true);
TEST_ASSERT_EQUAL_INT(MAX_NUM_NODES, (int)db->getNumMeshNodes());
TEST_ASSERT_NOT_NULL(db->getOrCreateMeshNode(0x79999999));
TEST_ASSERT_NULL(db->getMeshNode(futureDated));
TEST_ASSERT_NOT_NULL(db->getMeshNode(heardThisBoot));
}
// Ignored handling: an ignored node survives eviction (like a favourite), and is
// never purged by cleanupMeshDB even with no user info (a block set by bare ID).
static void test_ignored_survivesEvictionAndCleanup(void)
{
// (a) eviction protection
db->clearHot();
db->seedSelf();
for (int i = 1; i < MAX_NUM_NODES; i++) {
const bool ign = (i == 1); // oldest non-self, blocked
db->push(4000 + i, /*last_heard=*/i, false, ign, /*withUser=*/true, /*withKey=*/true);
}
TEST_ASSERT_NOT_NULL(db->getOrCreateMeshNode(0x88880000));
TEST_ASSERT_NOT_NULL(db->getMeshNode(4000 + 1)); // blocked node survived
TEST_ASSERT_NULL(db->getMeshNode(4000 + 2)); // oldest non-blocked evicted
// (b) cleanup protection - ignored kept without user info, plain no-user purged
db->clearHot();
db->seedSelf();
db->push(5000, 100, false, /*ignored=*/true, /*withUser=*/false, false);
db->push(5001, 100, false, false, /*withUser=*/false, false);
db->runCleanup();
TEST_ASSERT_NOT_NULL(db->getMeshNode(5000)); // blocked-by-ID kept despite no user info
TEST_ASSERT_NULL(db->getMeshNode(5001)); // ordinary no-user node purged
}
// Protected-node cap: at most MAX_NUM_NODES-2 nodes may be protected, so >=2
// evictable slots always remain. setProtectedFlag refuses once the cap is hit.
static void test_protectedCap_refusesBeyondLimit(void)
{
db->seedSelf();
for (int i = 0; i < MAX_NUM_NODES - 2; i++)
db->push(6000 + i, 100, /*favorite=*/true, false, /*withUser=*/true, false);
TEST_ASSERT_EQUAL_INT(MAX_NUM_NODES - 2, db->numProtectedNodes());
db->push(7000, 100, false, false, /*withUser=*/true, false);
meshtastic_NodeInfoLite *fresh = db->getMeshNode(7000);
TEST_ASSERT_NOT_NULL(fresh);
TEST_ASSERT_FALSE(db->setProtectedFlag(fresh, NODEINFO_BITFIELD_IS_IGNORED_MASK, true)); // refused at cap
TEST_ASSERT_FALSE(nodeInfoLiteIsIgnored(fresh)); // unchanged
TEST_ASSERT_EQUAL_INT(MAX_NUM_NODES - 2, db->numProtectedNodes());
// Adding another flag to an already-protected node doesn't grow the set, so
// it's still allowed at the cap.
meshtastic_NodeInfoLite *already = db->getMeshNode(6000);
TEST_ASSERT_TRUE(db->setProtectedFlag(already, NODEINFO_BITFIELD_IS_IGNORED_MASK, true));
}
// removeNodeByNum() compacts survivors down and clears the slots that leaves free. A full
// store with no matching node frees none, so there is nothing past the last node to clear.
static void test_removeNodeByNum_absentNodeOnFullDb(void)
{
db->seedSelf();
for (int i = 1; i < MAX_NUM_NODES; i++) // fill to MAX_NUM_NODES total (incl. self)
db->push(8000 + i, /*last_heard=*/i, false, false, /*withUser=*/true, /*withKey=*/true);
TEST_ASSERT_EQUAL_INT(MAX_NUM_NODES, (int)db->getNumMeshNodes());
db->removeNodeByNum(0xDEADBEEF); // absent; ASan flags a write past the last slot
TEST_ASSERT_EQUAL_INT(MAX_NUM_NODES, (int)db->getNumMeshNodes()); // nothing removed
TEST_ASSERT_NOT_NULL(db->getMeshNode(0x0BADF00D)); // self intact
TEST_ASSERT_NOT_NULL(db->getMeshNode(8000 + 1));
TEST_ASSERT_NOT_NULL(db->getMeshNode(8000 + MAX_NUM_NODES - 1)); // last slot intact
}
// Control for the above: a matching node on a full store is still removed, the survivors
// compact down, and the freed tail slot is cleared.
static void test_removeNodeByNum_presentNodeOnFullDb(void)
{
db->seedSelf();
for (int i = 1; i < MAX_NUM_NODES; i++)
db->push(8000 + i, /*last_heard=*/i, false, false, /*withUser=*/true, /*withKey=*/true);
db->removeNodeByNum(8000 + 5);
TEST_ASSERT_EQUAL_INT(MAX_NUM_NODES - 1, (int)db->getNumMeshNodes());
TEST_ASSERT_NULL(db->getMeshNode(8000 + 5));
TEST_ASSERT_NOT_NULL(db->getMeshNode(8000 + 4));
TEST_ASSERT_NOT_NULL(db->getMeshNode(8000 + MAX_NUM_NODES - 1)); // survivors kept
}
NDB_TEST_ENTRY void setup()
{
initializeTestEnvironment();
db = new NodeDBTestShim();
nodeDB = db;
UNITY_BEGIN();
RUN_TEST(test_migration_demotesOldestKeepsKeepersAndSelf);
RUN_TEST(test_migration_carriesRoleAndProtectedIntoWarm);
RUN_TEST(test_migration_carriesSignerBitThroughWarm);
RUN_TEST(test_migration_dropsShortKeyOnDemotion);
RUN_TEST(test_eviction_preservesFavorite);
RUN_TEST(test_eviction_prefersCurrentBootStampOverPost2038Epoch);
RUN_TEST(test_ignored_survivesEvictionAndCleanup);
RUN_TEST(test_protectedCap_refusesBeyondLimit);
RUN_TEST(test_removeNodeByNum_absentNodeOnFullDb);
RUN_TEST(test_removeNodeByNum_presentNodeOnFullDb);
exit(UNITY_END());
}
NDB_TEST_ENTRY void loop() {}
#else // WARM_NODE_COUNT == 0 - nothing to exercise here
void setUp(void) {}
void tearDown(void) {}
NDB_TEST_ENTRY void setup()
{
UNITY_BEGIN();
exit(UNITY_END());
}
NDB_TEST_ENTRY void loop() {}
#endif