Gate identity learning on signature in NodeDB::updateUser (#11084)

src/mesh/NodeDB.cpp

@@ -3310,13 +3310,20 @@ void NodeDB::addFromContact(meshtastic_SharedContact contact)
 
 /** Update user info and channel for this node based on received user data
  */
-bool NodeDB::updateUser(uint32_t nodeId, meshtastic_User &p, uint8_t channelIndex)
+bool NodeDB::updateUser(uint32_t nodeId, meshtastic_User &p, uint8_t channelIndex, bool xeddsaSigned)
 {
     meshtastic_NodeInfoLite *info = getOrCreateMeshNode(nodeId);
     if (!info) {
         return false;
     }
 
+    // Once a node has proven it signs, only a signed update may change its identity. The public-key guard
+    // below is no help - an attacker can replay the victim's real (public) key. Our own record is exempt.
+    if (nodeId != getNodeNum() && nodeInfoLiteHasXeddsaSigned(info) && !xeddsaSigned) {
+        LOG_WARN("Refusing unsigned identity update for node 0x%08x that previously signed", nodeId);
+        return false;
+    }
+
 #if !(MESHTASTIC_EXCLUDE_PKI)
     if (p.public_key.size == 32 && nodeId != nodeDB->getNodeNum()) {
         printBytes("Incoming Pubkey: ", p.public_key.bytes, 32);

src/mesh/NodeDB.h

@@ -255,9 +255,9 @@ class NodeDB
      */
     void updateTelemetry(uint32_t nodeId, const meshtastic_Telemetry &t, RxSource src = RX_SRC_RADIO);
 
-    /** Update user info and channel for this node based on received user data
-     */
-    bool updateUser(uint32_t nodeId, meshtastic_User &p, uint8_t channelIndex = 0);
+    /** Update user info and channel for this node based on received user data.
+     * A known signer's identity is only learned when xeddsaSigned; defaults false so callers fail closed. */
+    bool updateUser(uint32_t nodeId, meshtastic_User &p, uint8_t channelIndex = 0, bool xeddsaSigned = false);
 
     /*
      * Sets a node either favorite or unfavorite. Returns true if the node ends

src/modules/NodeInfoModule.cpp

@@ -61,7 +61,9 @@ bool NodeInfoModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, mes
     // Coerce user.id to be derived from the node number
     snprintf(p.id, sizeof(p.id), "!%08x", getFrom(&mp));
 
-    bool hasChanged = nodeDB->updateUser(getFrom(&mp), p, mp.channel);
+    // updateUser() refuses the identity write for a known signer sending unsigned (all unicast
+    // NodeInfo), so the exchange above still proceeds but cannot spoof the stored name.
+    bool hasChanged = nodeDB->updateUser(getFrom(&mp), p, mp.channel, mp.xeddsa_signed);
 
     bool wasBroadcast = isBroadcast(mp.to);
 

src/modules/TrafficManagementModule.cpp

@@ -734,7 +734,7 @@ ProcessMessage TrafficManagementModule::handleReceived(const meshtastic_MeshPack
             meshtastic_User requester = meshtastic_User_init_zero;
             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);
+                nodeDB->updateUser(getFrom(&mp), requester, mp.channel, mp.xeddsa_signed);
             }
             logAction("respond", &mp, "nodeinfo-cache");
             incrementStat(&stats.nodeinfo_cache_hits);

test/test_packet_signing/test_main.cpp

@@ -81,6 +81,21 @@ class MockNodeDB : public NodeDB
         nodeInfoLiteSetBit(n, NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK, value);
     }
 
+    void setLongName(NodeNum num, const char *name)
+    {
+        meshtastic_NodeInfoLite *n = getMeshNode(num);
+        TEST_ASSERT_NOT_NULL(n);
+        strncpy(n->long_name, name, sizeof(n->long_name) - 1);
+        n->long_name[sizeof(n->long_name) - 1] = '\0';
+    }
+
+    const char *longName(NodeNum num)
+    {
+        meshtastic_NodeInfoLite *n = getMeshNode(num);
+        TEST_ASSERT_NOT_NULL(n);
+        return n->long_name;
+    }
+
     std::vector<meshtastic_NodeInfoLite> testNodes;
 };
 
@@ -543,7 +558,8 @@ void test_B6_rich_shape_sweep_no_deadband(void)
 
 // ===========================================================================
 // Group C - NodeInfoModule downgrade drop (broadcast-only backstop for ingress paths that skip
-// Router's check; unicast NodeInfo is never signed by senders, so it is exempt - see C4)
+// Router's check; unicast NodeInfo is never signed by senders, so it is exempt - see C4), plus the
+// identity-learning gate in NodeDB::updateUser that keeps that exemption from being a spoofing hole
 // ===========================================================================
 class NodeInfoTestShim : public NodeInfoModule
 {
@@ -618,6 +634,67 @@ void test_C4_unsigned_unicast_nodeinfo_from_signer_accepted(void)
                               "unsigned unicast NodeInfo from a signer must not be dropped");
 }
 
+// C5: the packet survives (C4) but the identity claim inside it must not land - the pubkey guard
+// can't tell a signer from an impersonator replaying its (public) key. Only the write is refused.
+void test_C5_unsigned_unicast_nodeinfo_from_signer_does_not_change_name(void)
+{
+    mockNodeDB->addNode(REMOTE_NODE);
+    mockNodeDB->setSignerBit(REMOTE_NODE, true);
+    mockNodeDB->setLongName(REMOTE_NODE, "Genuine");
+
+    NodeInfoTestShim shim;
+    meshtastic_MeshPacket mp = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD);
+    mp.xeddsa_signed = false;
+    meshtastic_User user = meshtastic_User_init_zero;
+    user.is_licensed = owner.is_licensed;
+    strcpy(user.long_name, "Spoofed");
+    strcpy(user.short_name, "SPF");
+
+    TEST_ASSERT_FALSE_MESSAGE(shim.handleReceivedProtobuf(mp, &user), "the packet itself must still be accepted");
+    TEST_ASSERT_EQUAL_STRING_MESSAGE("Genuine", mockNodeDB->longName(REMOTE_NODE),
+                                     "unsigned unicast NodeInfo from a signer must not rewrite its stored name");
+}
+
+// C6: the same exchange signed - the update is authenticated and must land, pinning C5 as a
+// targeted refusal rather than a blanket block on unicast NodeInfo from signers.
+void test_C6_signed_unicast_nodeinfo_from_signer_changes_name(void)
+{
+    mockNodeDB->addNode(REMOTE_NODE);
+    mockNodeDB->setSignerBit(REMOTE_NODE, true);
+    mockNodeDB->setLongName(REMOTE_NODE, "Genuine");
+
+    NodeInfoTestShim shim;
+    meshtastic_MeshPacket mp = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD);
+    mp.xeddsa_signed = true;
+    meshtastic_User user = meshtastic_User_init_zero;
+    user.is_licensed = owner.is_licensed;
+    strcpy(user.long_name, "Renamed");
+    strcpy(user.short_name, "RNM");
+
+    TEST_ASSERT_FALSE(shim.handleReceivedProtobuf(mp, &user));
+    TEST_ASSERT_EQUAL_STRING_MESSAGE("Renamed", mockNodeDB->longName(REMOTE_NODE),
+                                     "a signed update from a signer must still be learned");
+}
+
+// C7: a node that has never signed is unaffected - the ordinary case for most of the mesh.
+void test_C7_unsigned_unicast_nodeinfo_from_nonsigner_changes_name(void)
+{
+    mockNodeDB->addNode(REMOTE_NODE); // signer bit clear
+    mockNodeDB->setLongName(REMOTE_NODE, "Genuine");
+
+    NodeInfoTestShim shim;
+    meshtastic_MeshPacket mp = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD);
+    mp.xeddsa_signed = false;
+    meshtastic_User user = meshtastic_User_init_zero;
+    user.is_licensed = owner.is_licensed;
+    strcpy(user.long_name, "Renamed");
+    strcpy(user.short_name, "RNM");
+
+    TEST_ASSERT_FALSE(shim.handleReceivedProtobuf(mp, &user));
+    TEST_ASSERT_EQUAL_STRING_MESSAGE("Renamed", mockNodeDB->longName(REMOTE_NODE),
+                                     "non-signer identity learning must be unaffected");
+}
+
 // ===========================================================================
 // Group D - encoding invariants the routing gates depend on
 // ===========================================================================
@@ -900,6 +977,9 @@ void setup()
     RUN_TEST(test_C2_signed_nodeinfo_from_signer_not_dropped);
     RUN_TEST(test_C3_unsigned_nodeinfo_from_nonsigner_not_dropped);
     RUN_TEST(test_C4_unsigned_unicast_nodeinfo_from_signer_accepted);
+    RUN_TEST(test_C5_unsigned_unicast_nodeinfo_from_signer_does_not_change_name);
+    RUN_TEST(test_C6_signed_unicast_nodeinfo_from_signer_changes_name);
+    RUN_TEST(test_C7_unsigned_unicast_nodeinfo_from_nonsigner_changes_name);
 
     printf("\n=== Group D: encoding invariants ===\n");
     RUN_TEST(test_D1_signature_field_overhead_exact);