diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index 6f87422f2cc..b152a4be4b8 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -3484,14 +3484,12 @@ void NodeDB::addFromContact(meshtastic_SharedContact contact) } } info->num = contact.node_num; - // CopyUserToNodeInfoLite assigns public_key unconditionally, and clients send add_contact before every - // DM - often from an entry that carries no key at all. A contact may still supply or update a full - // 32-byte key (that's what add_contact is for), but it must never *erase* a key we already hold, which - // would be persisted below and break subsequent DMs with PKI_SEND_FAIL_PUBLIC_KEY. + // CopyUserToNodeInfoLite assigns public_key unconditionally. A contact may supply or update a + // full key, but must never erase one we already hold - clients send add_contact before every DM. const meshtastic_NodeInfoLite_public_key_t storedKey = info->public_key; TypeConversions::CopyUserToNodeInfoLite(info, contact.user); if (storedKey.size == 32 && info->public_key.size != 32) { - LOG_INFO("Contact 0x%08x has no key, keep the stored one", contact.node_num); + LOG_DEBUG("Contact 0x%08x has no key, keep the stored one", contact.node_num); info->public_key = storedKey; } if (contact.should_ignore) { diff --git a/test/test_nodedb_blocked/test_main.cpp b/test/test_nodedb_blocked/test_main.cpp index 96d392cd854..85ff8eb28a7 100644 --- a/test/test_nodedb_blocked/test_main.cpp +++ b/test/test_nodedb_blocked/test_main.cpp @@ -1,5 +1,5 @@ -// Tests for the NodeDB hot-store migration and favourite/ignored (blocked) -// retention paths - src/mesh/NodeDB.cpp. +// Tests for the NodeDB hot-store migration, favourite/ignored (blocked) +// retention, and addFromContact() public-key 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 @@ -14,6 +14,7 @@ #if WARM_NODE_COUNT > 0 #include "mesh/NodeDB.h" +#include "mesh/generated/meshtastic/admin.pb.h" // meshtastic_SharedContact #include // Subclass shim: exposes the private maintenance paths (via the friend @@ -73,6 +74,30 @@ bool warmHasKey(NodeNum n) return db->copyPublicKey(n, k) && k.size == 32; } +// Seed a hot-store node holding a full 32-byte key of repeated `fill`, copied out to `out`. +void seedKeyedNode(NodeNum num, uint8_t fill, uint8_t out[32]) +{ + db->push(num, /*last_heard=*/100, /*favorite=*/false, /*ignored=*/false, /*withUser=*/true, /*withKey=*/false); + meshtastic_NodeInfoLite *n = db->getMeshNode(num); + TEST_ASSERT_NOT_NULL(n); + n->public_key.size = 32; + memset(n->public_key.bytes, fill, 32); + memcpy(out, n->public_key.bytes, 32); +} + +// An add_contact payload for `num` whose key is `size` bytes of repeated `fill`. +meshtastic_SharedContact makeContact(NodeNum num, pb_size_t size, uint8_t fill) +{ + meshtastic_SharedContact c = meshtastic_SharedContact_init_zero; + c.node_num = num; + c.has_user = true; + strncpy(c.user.long_name, "Contact", sizeof(c.user.long_name) - 1); + strncpy(c.user.short_name, "ct", sizeof(c.user.short_name) - 1); + c.user.public_key.size = size; + memset(c.user.public_key.bytes, fill, size); + return c; +} + } // namespace void setUp(void) @@ -280,6 +305,75 @@ static void test_removeNodeByNum_presentNodeOnFullDb(void) TEST_ASSERT_NOT_NULL(db->getMeshNode(8000 + MAX_NUM_NODES - 1)); // survivors kept } +// addFromContact() runs CopyUserToNodeInfoLite, which assigns public_key unconditionally. +// A keyless contact (clients send add_contact before every DM) must not erase a stored key. +static void test_addFromContact_emptyKeyKeepsStoredKey(void) +{ + db->seedSelf(); + const NodeNum num = 0xC0FFEE01; + uint8_t stored[32]; + seedKeyedNode(num, 0xA5, stored); + + db->addFromContact(makeContact(num, /*size=*/0, /*fill=*/0x00)); + + const meshtastic_NodeInfoLite *n = db->getMeshNode(num); + TEST_ASSERT_NOT_NULL(n); + TEST_ASSERT_EQUAL_UINT(32, n->public_key.size); + TEST_ASSERT_EQUAL_MEMORY(stored, n->public_key.bytes, 32); +} + +// Same guard for a truncated key: anything short of 32 bytes is unusable for PKI, +// so it must not displace a full stored key either. +static void test_addFromContact_shortKeyKeepsStoredKey(void) +{ + db->seedSelf(); + const NodeNum num = 0xC0FFEE02; + uint8_t stored[32]; + seedKeyedNode(num, 0x5A, stored); + + db->addFromContact(makeContact(num, /*size=*/4, /*fill=*/0x11)); + + const meshtastic_NodeInfoLite *n = db->getMeshNode(num); + TEST_ASSERT_NOT_NULL(n); + TEST_ASSERT_EQUAL_UINT(32, n->public_key.size); + TEST_ASSERT_EQUAL_MEMORY(stored, n->public_key.bytes, 32); +} + +// Regression guard on the guard: a well-formed 32-byte contact key still replaces the +// stored one. Refusing erasure must not harden into a blanket first-key-wins pin. +static void test_addFromContact_fullKeyReplacesStoredKey(void) +{ + db->seedSelf(); + const NodeNum num = 0xC0FFEE03; + uint8_t stored[32]; + seedKeyedNode(num, 0x11, stored); + uint8_t incoming[32]; + memset(incoming, 0x22, sizeof(incoming)); + + db->addFromContact(makeContact(num, /*size=*/32, /*fill=*/0x22)); + + const meshtastic_NodeInfoLite *n = db->getMeshNode(num); + TEST_ASSERT_NOT_NULL(n); + TEST_ASSERT_EQUAL_UINT(32, n->public_key.size); + TEST_ASSERT_EQUAL_MEMORY(incoming, n->public_key.bytes, 32); +} + +// The ordinary add_contact case: a full key is stored on an entry that held none. +static void test_addFromContact_fullKeyStoredWhenNoneHeld(void) +{ + db->seedSelf(); + const NodeNum num = 0xC0FFEE04; + uint8_t incoming[32]; + memset(incoming, 0x33, sizeof(incoming)); + + db->addFromContact(makeContact(num, /*size=*/32, /*fill=*/0x33)); + + const meshtastic_NodeInfoLite *n = db->getMeshNode(num); + TEST_ASSERT_NOT_NULL(n); + TEST_ASSERT_EQUAL_UINT(32, n->public_key.size); + TEST_ASSERT_EQUAL_MEMORY(incoming, n->public_key.bytes, 32); +} + NDB_TEST_ENTRY void setup() { initializeTestEnvironment(); @@ -296,6 +390,10 @@ NDB_TEST_ENTRY void setup() RUN_TEST(test_protectedCap_refusesBeyondLimit); RUN_TEST(test_removeNodeByNum_absentNodeOnFullDb); RUN_TEST(test_removeNodeByNum_presentNodeOnFullDb); + RUN_TEST(test_addFromContact_emptyKeyKeepsStoredKey); + RUN_TEST(test_addFromContact_shortKeyKeepsStoredKey); + RUN_TEST(test_addFromContact_fullKeyReplacesStoredKey); + RUN_TEST(test_addFromContact_fullKeyStoredWhenNoneHeld); exit(UNITY_END()); } NDB_TEST_ENTRY void loop() {}