Limit NodeInfo discovery replies to direct neighbors

src/mesh/MeshModule.cpp

@@ -224,6 +224,11 @@ meshtastic_MeshPacket *MeshModule::allocReply()
     return r;
 }
 
+uint8_t MeshModule::getResponseHopLimit(const meshtastic_MeshPacket &req)
+{
+    return routingModule->getHopLimitForResponse(req);
+}
+
 /** Messages can be received that have the want_response bit set.  If set, this callback will be invoked
  * so that subclasses can (optionally) send a response back to the original sender.  Implementing this method
  * is optional
@@ -233,6 +238,7 @@ void MeshModule::sendResponse(const meshtastic_MeshPacket &req)
     auto r = allocReply();
     if (r) {
         setReplyTo(r, req);
+        r->hop_limit = getResponseHopLimit(req);
         currentReply = r;
     } else {
         // Ignore - this is now expected behavior for routing module (because it ignores some replies)

src/mesh/MeshModule.h

@@ -231,6 +231,10 @@ class MeshModule
 
     friend class ReliableRouter;
 
+  protected:
+    virtual uint8_t getResponseHopLimit(const meshtastic_MeshPacket &req);
+
+  private:
     /** Messages can be received that have the want_response bit set.  If set, this callback will be invoked
      * so that subclasses can (optionally) send a response back to the original sender.  This method calls allocReply()
      * to generate the reply message, and if !NULL that message will be delivered to whoever sent req

src/modules/NodeInfoModule.cpp

@@ -30,8 +30,21 @@ bool NodeInfoModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, mes
 
     auto p = *pptr;
 
+    if (p.is_licensed != owner.is_licensed) {
+        LOG_WARN("Invalid nodeInfo detected, is_licensed mismatch!");
+        return true;
+    }
+    NodeNum sourceNum = getFrom(&mp);
+    const meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(sourceNum);
+    // Broadcasts only: senders never sign unicast NodeInfo, so dropping it would break exchanges
+    // with signer nodes. Backstops ingress that skips Router's downgrade drop (e.g. decoded MQTT).
+    if (node && nodeInfoLiteHasXeddsaSigned(node) && !mp.xeddsa_signed && isBroadcast(mp.to)) {
+        LOG_WARN("Dropping unsigned NodeInfo broadcast from node 0x%08x that previously signed", sourceNum);
+        return true;
+    }
+
     // Suppress replies to senders we've replied to recently (12H window)
-    if (mp.decoded.want_response && !isFromUs(&mp)) {
+    if (mp.decoded.want_response && !isFromUs(&mp) && (!isBroadcast(mp.to) || isDirectBroadcastDiscoveryRequest(mp))) {
         const NodeNum sender = getFrom(&mp);
         const uint32_t now = mp.rx_time ? mp.rx_time : getTime();
         auto it = lastNodeInfoSeen.find(sender);
@@ -45,19 +58,6 @@ bool NodeInfoModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, mes
         pruneLastNodeInfoCache();
     }
 
-    if (p.is_licensed != owner.is_licensed) {
-        LOG_WARN("Invalid nodeInfo detected, is_licensed mismatch!");
-        return true;
-    }
-    NodeNum sourceNum = getFrom(&mp);
-    const meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(sourceNum);
-    // Broadcasts only: senders never sign unicast NodeInfo, so dropping it would break exchanges
-    // with signer nodes. Backstops ingress that skips Router's downgrade drop (e.g. decoded MQTT).
-    if (node && nodeInfoLiteHasXeddsaSigned(node) && !mp.xeddsa_signed && isBroadcast(mp.to)) {
-        LOG_WARN("Dropping unsigned NodeInfo broadcast from node 0x%08x that previously signed", sourceNum);
-        return true;
-    }
-
     // Coerce user.id to be derived from the node number
     snprintf(p.id, sizeof(p.id), "!%08x", getFrom(&mp));
 
@@ -113,6 +113,8 @@ void NodeInfoModule::sendOurNodeInfo(NodeNum dest, bool wantReplies, uint8_t cha
                                    wantReplies;
 
         p->decoded.want_response = requestWantResponse;
+        if (requestWantResponse && isBroadcast(dest))
+            p->hop_limit = 0;
         if (_shorterTimeout)
             p->priority = meshtastic_MeshPacket_Priority_DEFAULT;
         else
@@ -135,6 +137,21 @@ void NodeInfoModule::triggerImmediateNodeInfoCheck()
     setIntervalFromNow(0);
 }
 
+bool NodeInfoModule::isDirectBroadcastDiscoveryRequest(const meshtastic_MeshPacket &request)
+{
+    return request.which_payload_variant == meshtastic_MeshPacket_decoded_tag &&
+           request.decoded.portnum == meshtastic_PortNum_NODEINFO_APP && request.decoded.want_response &&
+           isBroadcast(request.to) && getHopsAway(request) == 0;
+}
+
+uint8_t NodeInfoModule::getResponseHopLimit(const meshtastic_MeshPacket &req)
+{
+    if (isDirectBroadcastDiscoveryRequest(req))
+        return 0;
+
+    return MeshModule::getResponseHopLimit(req);
+}
+
 meshtastic_MeshPacket *NodeInfoModule::allocReply()
 {
     // Only apply suppression when actually replying to someone else's request, not for periodic broadcasts.
@@ -143,6 +160,13 @@ meshtastic_MeshPacket *NodeInfoModule::allocReply()
                                              currentRequest->decoded.portnum == meshtastic_PortNum_NODEINFO_APP &&
                                              currentRequest->decoded.want_response && !isFromUs(currentRequest);
 
+    const bool isBroadcastRequest = isReplyingToExternalRequest && isBroadcast(currentRequest->to);
+    if (isBroadcastRequest && !isDirectBroadcastDiscoveryRequest(*currentRequest)) {
+        LOG_DEBUG("Skip NodeInfo response to non-direct broadcast discovery");
+        ignoreRequest = true;
+        return NULL;
+    }
+
     if (suppressReplyForCurrentRequest && isReplyingToExternalRequest) {
         LOG_DEBUG("Skip send NodeInfo since we heard the requester <12h ago");
         ignoreRequest = true;
@@ -234,4 +258,4 @@ int32_t NodeInfoModule::runOnce()
         sendOurNodeInfo(NODENUM_BROADCAST, requestReplies); // Send our info (don't request replies)
     }
     return Default::getConfiguredOrDefaultMs(config.device.node_info_broadcast_secs, default_node_info_broadcast_secs);
-}
+}

src/modules/NodeInfoModule.h

@@ -31,6 +31,10 @@ class NodeInfoModule : public ProtobufModule<meshtastic_User>, private concurren
     void triggerImmediateNodeInfoCheck();
 
   protected:
+    static bool isDirectBroadcastDiscoveryRequest(const meshtastic_MeshPacket &request);
+
+    virtual uint8_t getResponseHopLimit(const meshtastic_MeshPacket &req) override;
+
     /** Called to handle a particular incoming message
 
     @return true if you've guaranteed you've handled this message and no other handlers should be considered for it

test/test_mesh_module/test_main.cpp

@@ -3,13 +3,15 @@
 #include "TestUtil.h"
 #include <unity.h>
 
+#include "airtime.h"
 #include "configuration.h"
 #include "mesh/CryptoEngine.h"
 #include "mesh/MeshService.h"
 #include "mesh/NodeDB.h"
 #include "mesh/RadioInterface.h"
 #include "mesh/Router.h"
 #include "modules/NeighborInfoModule.h"
+#include "modules/NodeInfoModule.h"
 #include "modules/RoutingModule.h"
 #include "support/MockMeshService.h"
 #include <memory>
@@ -126,6 +128,34 @@ class SyntheticReplyModule : public MeshModule
     bool acceptsEveryPort;
 };
 
+class ZeroHopReplyModule : public SyntheticReplyModule
+{
+  public:
+    ZeroHopReplyModule()
+        : SyntheticReplyModule("zero-hop-reply", meshtastic_PortNum_TELEMETRY_APP, meshtastic_PortNum_TELEMETRY_APP)
+    {
+    }
+
+  protected:
+    uint8_t getResponseHopLimit(const meshtastic_MeshPacket &req) override
+    {
+        (void)req;
+        return 0;
+    }
+};
+
+class NodeInfoPolicyShim : public NodeInfoModule
+{
+  public:
+    using NodeInfoModule::allocReply;
+    using NodeInfoModule::getResponseHopLimit;
+    using NodeInfoModule::handleReceivedProtobuf;
+    using NodeInfoModule::isDirectBroadcastDiscoveryRequest;
+
+    MeshModule *asMeshModule() { return this; }
+    void setCurrentRequest(const meshtastic_MeshPacket *request) { currentRequest = request; }
+};
+
 class ObservingIgnoreModule : public MeshModule
 {
   public:
@@ -175,6 +205,7 @@ static MockMeshService *mockService;
 static MockRouter *mockRouter;
 static MockRoutingModule *mockRoutingModule;
 static NeighborInfoModule *realNeighborInfoModule;
+static AirTime *testAirTime;
 static std::vector<MeshModule *> dispatchModules;
 
 template <typename T> static T *registerDispatchModule(T *module)
@@ -204,6 +235,11 @@ static void dispatch(meshtastic_PortNum port)
     MeshModule::callModules(request);
 }
 
+static void dispatch(meshtastic_MeshPacket request)
+{
+    MeshModule::callModules(request);
+}
+
 } // namespace
 
 void setUp(void)
@@ -214,6 +250,9 @@ void setUp(void)
     owner = meshtastic_User_init_zero;
     myNodeInfo.my_node_num = LOCAL_NODE;
 
+    testAirTime = new AirTime();
+    airTime = testAirTime;
+
     mockNodeDB = new MockNodeDB();
     nodeDB = mockNodeDB;
     myNodeInfo.my_node_num = LOCAL_NODE;
@@ -267,6 +306,10 @@ void tearDown(void)
     delete mockNodeDB;
     mockNodeDB = nullptr;
     nodeDB = nullptr;
+
+    airTime = nullptr;
+    delete testAirTime;
+    testAirTime = nullptr;
 }
 
 // Zero-hop broadcast (hop_limit == hop_start): should be allowed
@@ -418,6 +461,109 @@ static void test_dispatch_crossPortReplyUsesRequestOwner()
     TEST_ASSERT_EQUAL_UINT32(0, mockRoutingModule->ackNaks.size());
 }
 
+static void test_dispatch_moduleCanConstrainReplyHopLimit()
+{
+    registerDispatchModule(new ZeroHopReplyModule());
+
+    dispatch(meshtastic_PortNum_TELEMETRY_APP);
+
+    TEST_ASSERT_EQUAL_UINT32(1, mockRouter->sentPackets.size());
+    TEST_ASSERT_EQUAL_UINT8(0, mockRouter->sentPackets[0].hop_limit);
+}
+
+static void test_nodeInfo_directBroadcastDiscoveryUsesZeroHopReply()
+{
+    NodeInfoPolicyShim nodeInfo;
+    meshtastic_MeshPacket request = makeRequest(meshtastic_PortNum_NODEINFO_APP);
+    request.to = NODENUM_BROADCAST;
+    request.hop_start = 3;
+    request.hop_limit = 3;
+    request.decoded.has_bitfield = true;
+
+    TEST_ASSERT_TRUE(NodeInfoPolicyShim::isDirectBroadcastDiscoveryRequest(request));
+    TEST_ASSERT_EQUAL_UINT8(0, nodeInfo.getResponseHopLimit(request));
+}
+
+static void test_nodeInfo_relayedAndUnknownBroadcastDiscoveryDoNotQualify()
+{
+    meshtastic_MeshPacket request = makeRequest(meshtastic_PortNum_NODEINFO_APP);
+    request.to = NODENUM_BROADCAST;
+    request.hop_start = 3;
+    request.hop_limit = 2;
+    request.decoded.has_bitfield = true;
+
+    TEST_ASSERT_FALSE(NodeInfoPolicyShim::isDirectBroadcastDiscoveryRequest(request));
+
+    request.hop_start = 0;
+    request.hop_limit = 0;
+    request.decoded.has_bitfield = false;
+
+    TEST_ASSERT_FALSE(NodeInfoPolicyShim::isDirectBroadcastDiscoveryRequest(request));
+}
+
+static void test_nodeInfo_unicastRequestRetainsRoutingHopLimit()
+{
+    NodeInfoPolicyShim nodeInfo;
+    meshtastic_MeshPacket request = makeRequest(meshtastic_PortNum_NODEINFO_APP);
+
+    TEST_ASSERT_FALSE(NodeInfoPolicyShim::isDirectBroadcastDiscoveryRequest(request));
+    TEST_ASSERT_EQUAL_UINT8(mockRoutingModule->getHopLimitForResponse(request), nodeInfo.getResponseHopLimit(request));
+}
+
+static void test_nodeInfo_rejectedBroadcastDoesNotSuppressDirectDiscovery()
+{
+    auto *nodeInfo = new NodeInfoPolicyShim();
+    dispatchModules.push_back(nodeInfo->asMeshModule());
+    meshtastic_MeshPacket request = makeRequest(meshtastic_PortNum_NODEINFO_APP);
+    request.to = NODENUM_BROADCAST;
+    request.decoded.has_bitfield = true;
+    request.hop_start = 3;
+    request.hop_limit = 2;
+
+    dispatch(request);
+    TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size());
+    TEST_ASSERT_EQUAL_UINT32(0, mockRoutingModule->ackNaks.size());
+
+    request.hop_start = 0;
+    request.hop_limit = 0;
+    request.decoded.has_bitfield = false;
+    dispatch(request);
+    TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size());
+    TEST_ASSERT_EQUAL_UINT32(0, mockRoutingModule->ackNaks.size());
+
+    request.decoded.has_bitfield = true;
+    dispatch(request);
+    TEST_ASSERT_EQUAL_UINT32(1, mockRouter->sentPackets.size());
+    TEST_ASSERT_EQUAL_UINT8(0, mockRouter->sentPackets[0].hop_limit);
+    TEST_ASSERT_EQUAL_UINT32(0, mockRoutingModule->ackNaks.size());
+}
+
+static void test_nodeInfo_rejectedDirectRequestDoesNotSuppressDiscovery()
+{
+    NodeInfoPolicyShim nodeInfo;
+    meshtastic_MeshPacket request = makeRequest(meshtastic_PortNum_NODEINFO_APP);
+    request.to = NODENUM_BROADCAST;
+    request.hop_start = 0;
+    request.hop_limit = 0;
+    request.decoded.has_bitfield = true;
+
+    TEST_ASSERT_NOT_NULL(mockNodeDB->getOrCreateMeshNode(REMOTE_NODE));
+
+    meshtastic_User rejectedUser = meshtastic_User_init_zero;
+    rejectedUser.is_licensed = true;
+    TEST_ASSERT_TRUE(nodeInfo.handleReceivedProtobuf(request, &rejectedUser));
+
+    meshtastic_User validUser = meshtastic_User_init_zero;
+    TEST_ASSERT_FALSE(nodeInfo.handleReceivedProtobuf(request, &validUser));
+
+    nodeInfo.setCurrentRequest(&request);
+    meshtastic_MeshPacket *reply = nodeInfo.allocReply();
+    nodeInfo.setCurrentRequest(nullptr);
+
+    TEST_ASSERT_NOT_NULL(reply);
+    packetPool.release(reply);
+}
+
 static void test_dispatch_foreignPortObserverCanSuppressNak()
 {
     auto *observer = registerDispatchModule(new ObservingIgnoreModule());
@@ -491,6 +637,12 @@ void setup()
     RUN_TEST(test_dispatch_foreignPortOffenderCannotShadowOwner);
     RUN_TEST(test_dispatch_ownerPortStillReplies);
     RUN_TEST(test_dispatch_crossPortReplyUsesRequestOwner);
+    RUN_TEST(test_dispatch_moduleCanConstrainReplyHopLimit);
+    RUN_TEST(test_nodeInfo_directBroadcastDiscoveryUsesZeroHopReply);
+    RUN_TEST(test_nodeInfo_relayedAndUnknownBroadcastDiscoveryDoNotQualify);
+    RUN_TEST(test_nodeInfo_unicastRequestRetainsRoutingHopLimit);
+    RUN_TEST(test_nodeInfo_rejectedBroadcastDoesNotSuppressDirectDiscovery);
+    RUN_TEST(test_nodeInfo_rejectedDirectRequestDoesNotSuppressDiscovery);
     RUN_TEST(test_dispatch_foreignPortObserverCanSuppressNak);
     RUN_TEST(test_dispatch_noResponderSendsNak);
     RUN_TEST(test_dispatch_ignoreRequestIsClearedPerPacket);