From f71c5d8aa98135ecaf4414018d060126729bfa1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Thu, 16 Jul 2026 22:48:08 +0200 Subject: [PATCH] TrafficManagement: don't learn identity from a signer's unsigned NodeInfo The NodeInfo direct-response path opportunistically called updateUser() with the requester's decoded User. That path only sees unicast NodeInfo, which is never signed, so for a node already known to sign its broadcasts the identity claim is unauthenticated and would overwrite the stored name. Skip the identity update when the sender is a known signer and the packet is unsigned; non-signers are still learned as before, and the cached direct response is unchanged. --- src/modules/TrafficManagementModule.cpp | 7 ++- test/test_traffic_management/test_main.cpp | 55 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/modules/TrafficManagementModule.cpp b/src/modules/TrafficManagementModule.cpp index 89ad128ccf5..f10a9f0d47e 100644 --- a/src/modules/TrafficManagementModule.cpp +++ b/src/modules/TrafficManagementModule.cpp @@ -744,8 +744,13 @@ ProcessMessage TrafficManagementModule::handleReceived(const meshtastic_MeshPack if (cfg.nodeinfo_direct_response_max_hops > 0 && mp.decoded.portnum == meshtastic_PortNum_NODEINFO_APP && mp.decoded.want_response && !isBroadcast(mp.to) && !isToUs(&mp) && !isFromUs(&mp)) { if (shouldRespondToNodeInfo(&mp, true)) { + // Unicast NodeInfo is never signed, so a known signer's identity claim here is + // unauthenticated: don't overwrite its stored name. The cached response is unaffected. + const meshtastic_NodeInfoLite *senderNode = nodeDB->getMeshNode(getFrom(&mp)); + const bool unauthenticatedSigner = senderNode && nodeInfoLiteHasXeddsaSigned(senderNode) && !mp.xeddsa_signed; meshtastic_User requester = meshtastic_User_init_zero; - if (pb_decode_from_bytes(mp.decoded.payload.bytes, mp.decoded.payload.size, &meshtastic_User_msg, &requester)) { + if (!unauthenticatedSigner && + pb_decode_from_bytes(mp.decoded.payload.bytes, mp.decoded.payload.size, &meshtastic_User_msg, &requester)) { nodeDB->updateUser(getFrom(&mp), requester, mp.channel); } logAction("respond", &mp, "nodeinfo-cache"); diff --git a/test/test_traffic_management/test_main.cpp b/test/test_traffic_management/test_main.cpp index c203438f4bf..f64ea814f5b 100644 --- a/test/test_traffic_management/test_main.cpp +++ b/test/test_traffic_management/test_main.cpp @@ -110,6 +110,21 @@ class MockNodeDB : public NodeDB clearCachedNode(); } + // Seed a hot-store node that has been learned as an XEdDSA signer, with a known name, so the + // identity-update gate can be exercised. Distinct from setCachedNode() so a separate cached + // node (the direct-response target) can coexist. + void setSignerHotNode(NodeNum n, const char *longName) + { + if (meshNodes->size() < 2) + meshNodes->resize(2); + (*meshNodes)[1] = meshtastic_NodeInfoLite_init_zero; + (*meshNodes)[1].num = n; + nodeInfoLiteSetBit(&(*meshNodes)[1], NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK, true); + nodeInfoLiteSetBit(&(*meshNodes)[1], NODEINFO_BITFIELD_HAS_USER_MASK, true); + strncpy((*meshNodes)[1].long_name, longName, sizeof((*meshNodes)[1].long_name) - 1); + numMeshNodes = 2; + } + private: bool hasCachedNode = false; NodeNum cachedNodeNum = 0; @@ -589,6 +604,45 @@ static void test_tm_nodeinfo_directResponse_learnsRequestorNodeInfo(void) TEST_ASSERT_EQUAL_UINT8(request.channel, requestor->channel); } +/** + * A unicast NodeInfo request is never signed, so a known signer's identity claim on the + * direct-response path is unauthenticated. It must not overwrite the stored name (spoofing + * defense), while the direct response itself still goes out. The non-signer case + * (test_tm_nodeinfo_directResponse_learnsRequestorNodeInfo) is the control: it still learns. + */ +static void test_tm_nodeinfo_directResponse_ignoresUnsignedSignerIdentity(void) +{ + moduleConfig.traffic_management.nodeinfo_direct_response_max_hops = 10; + config.device.role = meshtastic_Config_DeviceConfig_Role_CLIENT; + mockNodeDB->setCachedNode(kTargetNode); // the direct-response target + mockNodeDB->setSignerHotNode(kRemoteNode, "victim-real"); // the requester is a known signer + + MockRouter mockRouter; + mockRouter.addInterface(std::unique_ptr(new MockRadioInterface())); + MeshService mockService; + router = &mockRouter; + service = &mockService; + + TrafficManagementModuleTestShim module; + meshtastic_MeshPacket request = makeNodeInfoPacket(kRemoteNode, "attacker-name", "atk"); + request.to = kTargetNode; + request.decoded.want_response = true; + request.id = 0x0A0B0C0D; + request.hop_start = 3; + request.hop_limit = 3; + request.xeddsa_signed = false; // unicast: never signed + + ProcessMessage result = module.handleReceived(request); + + // The response still went out (the request was consumed from cache)... + TEST_ASSERT_EQUAL_INT(static_cast(ProcessMessage::STOP), static_cast(result)); + TEST_ASSERT_EQUAL_UINT32(1, module.getStats().nodeinfo_cache_hits); + // ...but the known signer's stored name was not overwritten by the unauthenticated claim. + const meshtastic_NodeInfoLite *requestor = mockNodeDB->getMeshNode(kRemoteNode); + TEST_ASSERT_NOT_NULL(requestor); + TEST_ASSERT_EQUAL_STRING("victim-real", requestor->long_name); +} + /** * Verify client role only answers direct (0-hop) NodeInfo requests. * Important so clients do not answer relayed requests outside intended scope. @@ -1715,6 +1769,7 @@ TM_TEST_ENTRY void setup() RUN_TEST(test_tm_nodeinfo_routerClamp_skipsWhenTooManyHops); RUN_TEST(test_tm_nodeinfo_directResponse_respondsFromCache); RUN_TEST(test_tm_nodeinfo_directResponse_learnsRequestorNodeInfo); + RUN_TEST(test_tm_nodeinfo_directResponse_ignoresUnsignedSignerIdentity); RUN_TEST(test_tm_nodeinfo_clientClamp_skipsWhenNotDirect); #if !(defined(ARCH_ESP32) && defined(BOARD_HAS_PSRAM)) RUN_TEST(test_tm_nodeinfo_directResponse_withoutNodeDbEntry_skips);