diff --git a/src/mesh/MeshModule.cpp b/src/mesh/MeshModule.cpp index aaed129f8..5dc7fba4a 100644 --- a/src/mesh/MeshModule.cpp +++ b/src/mesh/MeshModule.cpp @@ -18,7 +18,7 @@ uint8_t MeshModule::numPeriodicModules = 0; */ meshtastic_MeshPacket *MeshModule::currentReply; -MeshModule::MeshModule(const char *_name) : name(_name) +MeshModule::MeshModule(const char *_name, meshtastic_PortNum _ourPortNum) : name(_name), ourPortNum(_ourPortNum) { // Can't trust static initializer order, so we check each time if (!modules) @@ -27,6 +27,12 @@ MeshModule::MeshModule(const char *_name) : name(_name) modules->push_back(this); } +bool MeshModule::replyPortMatches(meshtastic_PortNum modulePort, const meshtastic_MeshPacket &mp) +{ + return modulePort != meshtastic_PortNum_UNKNOWN_APP && mp.which_payload_variant == meshtastic_MeshPacket_decoded_tag && + mp.decoded.portnum == modulePort; +} + void MeshModule::setup() {} MeshModule::~MeshModule() @@ -107,6 +113,7 @@ void MeshModule::callModules(meshtastic_MeshPacket &mp, RxSource src) auto &pi = **i; pi.currentRequest = ∓ + pi.ignoreRequest = false; /// We only call modules that are interested in the packet (and the message is destined to us or we are promiscious) bool wantsPacket = (isDecoded || pi.encryptedOk) && (pi.isPromiscuous || toUs) && pi.wantPacket(&mp); @@ -157,9 +164,13 @@ void MeshModule::callModules(meshtastic_MeshPacket &mp, RxSource src) // better solution (FIXME) would be to let phones have their own distinct addresses and we 'route' to them like // any other node. if (isDecoded && mp.decoded.want_response && toUs && (!isFromUs(&mp) || isToUs(&mp)) && !currentReply) { - pi.sendResponse(mp); + if (replyPortMatches(pi.ourPortNum, mp)) { + pi.sendResponse(mp); + LOG_INFO("Asked module '%s' to send a response", pi.name); + } else { + LOG_DEBUG("Module '%s' cannot respond on portnum=%d", pi.name, mp.decoded.portnum); + } ignoreRequest = ignoreRequest || pi.ignoreRequest; // If at least one module asks it, we may ignore a request - LOG_INFO("Asked module '%s' to send a response", pi.name); } else { LOG_DEBUG("Module '%s' considered", pi.name); } @@ -313,4 +324,4 @@ bool MeshModule::isRequestingFocus() } else return false; } -#endif \ No newline at end of file +#endif diff --git a/src/mesh/MeshModule.h b/src/mesh/MeshModule.h index 9d579d4f1..3dc6414a5 100644 --- a/src/mesh/MeshModule.h +++ b/src/mesh/MeshModule.h @@ -68,10 +68,12 @@ class MeshModule /** Constructor * name is for debugging output */ - MeshModule(const char *_name); + MeshModule(const char *_name, meshtastic_PortNum _ourPortNum = meshtastic_PortNum_UNKNOWN_APP); virtual ~MeshModule(); + static bool replyPortMatches(meshtastic_PortNum modulePort, const meshtastic_MeshPacket &mp); + /** For use only by MeshService */ static void callModules(meshtastic_MeshPacket &mp, RxSource src = RX_SRC_RADIO); @@ -88,6 +90,7 @@ class MeshModule #endif protected: const char *name; + meshtastic_PortNum ourPortNum; /** Most modules only care about packets that are destined for their node (i.e. broadcasts or has their node as the specific recipient) But some plugs might want to 'sniff' packets that are merely being routed (passing through the current node). Those @@ -103,7 +106,7 @@ class MeshModule * flag */ bool encryptedOk = false; - /* We allow modules to ignore a request without sending an error if they have a specific reason for it. */ + /* Per-packet flag cleared by callModules(); modules can suppress an error response for a specific request. */ bool ignoreRequest = false; /** diff --git a/src/mesh/SinglePortModule.h b/src/mesh/SinglePortModule.h index 67444329a..d20a9b7e4 100644 --- a/src/mesh/SinglePortModule.h +++ b/src/mesh/SinglePortModule.h @@ -8,14 +8,11 @@ */ class SinglePortModule : public MeshModule { - protected: - meshtastic_PortNum ourPortNum; - public: /** Constructor * name is for debugging output */ - SinglePortModule(const char *_name, meshtastic_PortNum _ourPortNum) : MeshModule(_name), ourPortNum(_ourPortNum) {} + SinglePortModule(const char *_name, meshtastic_PortNum _ourPortNum) : MeshModule(_name, _ourPortNum) {} protected: /** @@ -38,4 +35,4 @@ class SinglePortModule : public MeshModule return p; } -}; \ No newline at end of file +}; diff --git a/src/modules/SerialModule.cpp b/src/modules/SerialModule.cpp index b3b42a37d..cb481e6a5 100644 --- a/src/modules/SerialModule.cpp +++ b/src/modules/SerialModule.cpp @@ -109,7 +109,7 @@ bool SerialModule::isValidConfig(const meshtastic_ModuleConfig_SerialConfig &con return true; } -SerialModuleRadio::SerialModuleRadio() : MeshModule("SerialModuleRadio") +SerialModuleRadio::SerialModuleRadio() : SinglePortModule("SerialModuleRadio", meshtastic_PortNum_SERIAL_APP) { switch (moduleConfig.serial.mode) { case meshtastic_ModuleConfig_SerialConfig_Serial_Mode_TEXTMSG: @@ -330,18 +330,6 @@ void SerialModule::sendTelemetry(meshtastic_Telemetry m) service->sendToMesh(p, RX_SRC_LOCAL, true); } -/** - * Allocates a new mesh packet for use as a reply to a received packet. - * - * @return A pointer to the newly allocated mesh packet. - */ -meshtastic_MeshPacket *SerialModuleRadio::allocReply() -{ - auto reply = allocDataPacket(); // Allocate a packet for sending - - return reply; -} - /** * Sends a payload to a specified destination node. * @@ -351,7 +339,7 @@ meshtastic_MeshPacket *SerialModuleRadio::allocReply() void SerialModuleRadio::sendPayload(NodeNum dest, bool wantReplies) { const meshtastic_Channel *ch = (boundChannel != NULL) ? &channels.getByName(boundChannel) : NULL; - meshtastic_MeshPacket *p = allocReply(); + meshtastic_MeshPacket *p = allocDataPacket(); if (!p) return; p->to = dest; diff --git a/src/modules/SerialModule.h b/src/modules/SerialModule.h index 768feeec6..5cbca7824 100644 --- a/src/modules/SerialModule.h +++ b/src/modules/SerialModule.h @@ -40,7 +40,7 @@ extern SerialModule *serialModule; * Radio interface for SerialModule * */ -class SerialModuleRadio : public MeshModule +class SerialModuleRadio : public SinglePortModule { uint32_t lastRxID = 0; char outbuf[90] = ""; @@ -54,31 +54,14 @@ class SerialModuleRadio : public MeshModule void sendPayload(NodeNum dest = NODENUM_BROADCAST, bool wantReplies = false); protected: - virtual meshtastic_MeshPacket *allocReply() override; - /** Called to handle a particular incoming message @return ProcessMessage::STOP if you've guaranteed you've handled this message and no other handlers should be considered for it */ virtual ProcessMessage handleReceived(const meshtastic_MeshPacket &mp) override; - - meshtastic_PortNum ourPortNum; - - virtual bool wantPacket(const meshtastic_MeshPacket *p) override { return p->decoded.portnum == ourPortNum; } - - meshtastic_MeshPacket *allocDataPacket() - { - // Update our local node info with our position (even if we don't decide to update anyone else) - meshtastic_MeshPacket *p = router->allocForSending(); - if (!p) - return nullptr; - p->decoded.portnum = ourPortNum; - - return p; - } }; extern SerialModuleRadio *serialModuleRadio; -#endif \ No newline at end of file +#endif diff --git a/test/test_mesh_module/test_main.cpp b/test/test_mesh_module/test_main.cpp index ec7b1808e..6ba038af3 100644 --- a/test/test_mesh_module/test_main.cpp +++ b/test/test_mesh_module/test_main.cpp @@ -3,6 +3,23 @@ #include "TestUtil.h" #include +#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/RoutingModule.h" +#include "support/MockMeshService.h" +#include +#include + +namespace +{ +constexpr NodeNum LOCAL_NODE = 0x11111111; +constexpr NodeNum REMOTE_NODE = 0x22222222; + // Minimal concrete subclass for testing the base class helper class TestModule : public MeshModule { @@ -13,11 +30,207 @@ class TestModule : public MeshModule using MeshModule::isMultiHopBroadcastRequest; }; +class MockNodeDB : public NodeDB +{ +}; + +class MockRadioInterface : public RadioInterface +{ + public: + ErrorCode send(meshtastic_MeshPacket *p) override + { + packetPool.release(p); + return ERRNO_OK; + } + + uint32_t getPacketTime(uint32_t totalPacketLen, bool received = false) override + { + (void)totalPacketLen; + (void)received; + return 0; + } +}; + +class MockRouter : public Router +{ + public: + ~MockRouter() + { + delete cryptLock; + cryptLock = nullptr; + } + + ErrorCode send(meshtastic_MeshPacket *p) override + { + sentPackets.push_back(*p); + packetPool.release(p); + return ERRNO_OK; + } + + std::vector sentPackets; +}; + +struct AckNak { + meshtastic_Routing_Error error; + NodeNum to; + PacketId requestId; + ChannelIndex channel; +}; + +class MockRoutingModule : public RoutingModule +{ + public: + void sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopLimit = 0, + bool ackWantsAck = false) override + { + (void)hopLimit; + (void)ackWantsAck; + ackNaks.push_back({err, to, idFrom, chIndex}); + } + + std::vector ackNaks; + + protected: + bool wantPacket(const meshtastic_MeshPacket *p) override + { + (void)p; + return false; + } +}; + +class SyntheticReplyModule : public MeshModule +{ + public: + SyntheticReplyModule(const char *name, meshtastic_PortNum modulePort, meshtastic_PortNum replyPort, + bool acceptsEveryPort = false) + : MeshModule(name, modulePort), replyPort(replyPort), acceptsEveryPort(acceptsEveryPort) + { + isPromiscuous = acceptsEveryPort; + } + + uint32_t allocReplyCalls = 0; + + protected: + bool wantPacket(const meshtastic_MeshPacket *p) override { return acceptsEveryPort || p->decoded.portnum == ourPortNum; } + + meshtastic_MeshPacket *allocReply() override + { + allocReplyCalls++; + meshtastic_MeshPacket *reply = router->allocForSending(); + reply->decoded.portnum = replyPort; + return reply; + } + + private: + meshtastic_PortNum replyPort; + bool acceptsEveryPort; +}; + +class ObservingIgnoreModule : public MeshModule +{ + public: + ObservingIgnoreModule() : MeshModule("storeforward-shaped", meshtastic_PortNum_STORE_FORWARD_APP) {} + + uint32_t allocReplyCalls = 0; + + protected: + bool wantPacket(const meshtastic_MeshPacket *p) override { return p->decoded.portnum == meshtastic_PortNum_TEXT_MESSAGE_APP; } + + ProcessMessage handleReceived(const meshtastic_MeshPacket &mp) override + { + (void)mp; + ignoreRequest = true; + return ProcessMessage::CONTINUE; + } + + meshtastic_MeshPacket *allocReply() override + { + allocReplyCalls++; + return nullptr; + } +}; + +class ReplyIgnoreModule : public MeshModule +{ + public: + ReplyIgnoreModule() : MeshModule("reply-ignore", meshtastic_PortNum_NEIGHBORINFO_APP) {} + + uint32_t allocReplyCalls = 0; + + protected: + bool wantPacket(const meshtastic_MeshPacket *p) override { return p->decoded.portnum == ourPortNum; } + + meshtastic_MeshPacket *allocReply() override + { + allocReplyCalls++; + ignoreRequest = true; + return nullptr; + } +}; + static TestModule *testModule; static meshtastic_MeshPacket testPacket; +static MockNodeDB *mockNodeDB; +static MockMeshService *mockService; +static MockRouter *mockRouter; +static MockRoutingModule *mockRoutingModule; +static NeighborInfoModule *realNeighborInfoModule; +static std::vector dispatchModules; + +template static T *registerDispatchModule(T *module) +{ + dispatchModules.push_back(module); + return module; +} + +static meshtastic_MeshPacket makeRequest(meshtastic_PortNum port) +{ + meshtastic_MeshPacket packet = meshtastic_MeshPacket_init_zero; + packet.from = REMOTE_NODE; + packet.to = LOCAL_NODE; + packet.id = 0x12345678; + packet.channel = 0; + packet.hop_start = 3; + packet.hop_limit = 3; + packet.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + packet.decoded.portnum = port; + packet.decoded.want_response = true; + return packet; +} + +static void dispatch(meshtastic_PortNum port) +{ + meshtastic_MeshPacket request = makeRequest(port); + MeshModule::callModules(request); +} + +} // namespace void setUp(void) { + config = meshtastic_LocalConfig_init_zero; + moduleConfig = meshtastic_LocalModuleConfig_init_zero; + channelFile = meshtastic_ChannelFile_init_zero; + owner = meshtastic_User_init_zero; + myNodeInfo.my_node_num = LOCAL_NODE; + + mockNodeDB = new MockNodeDB(); + nodeDB = mockNodeDB; + myNodeInfo.my_node_num = LOCAL_NODE; + + mockService = new MockMeshService(); + service = mockService; + + channels.initDefaults(); + channels.onConfigChanged(); + + mockRouter = new MockRouter(); + mockRouter->addInterface(std::unique_ptr(new MockRadioInterface())); + router = mockRouter; + + mockRoutingModule = new MockRoutingModule(); + routingModule = mockRoutingModule; + testModule = new TestModule(); memset(&testPacket, 0, sizeof(testPacket)); TestModule::currentRequest = &testPacket; @@ -26,7 +239,34 @@ void setUp(void) void tearDown(void) { TestModule::currentRequest = NULL; + + for (auto it = dispatchModules.rbegin(); it != dispatchModules.rend(); ++it) + delete *it; + dispatchModules.clear(); + + delete realNeighborInfoModule; + realNeighborInfoModule = nullptr; + delete testModule; + testModule = nullptr; + + delete mockRoutingModule; + mockRoutingModule = nullptr; + routingModule = nullptr; + + while (auto *status = mockService->getQueueStatusForPhone()) + mockService->releaseQueueStatusToPool(status); + delete mockService; + mockService = nullptr; + service = nullptr; + + delete mockRouter; + mockRouter = nullptr; + router = nullptr; + + delete mockNodeDB; + mockNodeDB = nullptr; + nodeDB = nullptr; } // Zero-hop broadcast (hop_limit == hop_start): should be allowed @@ -98,6 +338,139 @@ static void test_singleHopRelayedBroadcast_isBlocked() TEST_ASSERT_TRUE(testModule->isMultiHopBroadcastRequest()); } +static void test_replyPortMatches_ownPort() +{ + meshtastic_MeshPacket request = makeRequest(meshtastic_PortNum_TELEMETRY_APP); + + TEST_ASSERT_TRUE(MeshModule::replyPortMatches(meshtastic_PortNum_TELEMETRY_APP, request)); +} + +static void test_replyPortMatches_neighborInfoVsTelemetry() +{ + meshtastic_MeshPacket request = makeRequest(meshtastic_PortNum_TELEMETRY_APP); + + TEST_ASSERT_FALSE(MeshModule::replyPortMatches(meshtastic_PortNum_NEIGHBORINFO_APP, request)); +} + +static void test_replyPortMatches_positionRequest() +{ + meshtastic_MeshPacket request = makeRequest(meshtastic_PortNum_POSITION_APP); + + TEST_ASSERT_TRUE(MeshModule::replyPortMatches(meshtastic_PortNum_POSITION_APP, request)); +} + +static void test_replyPortMatches_unknownModulePort() +{ + meshtastic_MeshPacket request = makeRequest(meshtastic_PortNum_TELEMETRY_APP); + + TEST_ASSERT_FALSE(MeshModule::replyPortMatches(meshtastic_PortNum_UNKNOWN_APP, request)); +} + +static void test_replyPortMatches_unknownRequestPort() +{ + meshtastic_MeshPacket request = makeRequest(meshtastic_PortNum_UNKNOWN_APP); + + TEST_ASSERT_FALSE(MeshModule::replyPortMatches(meshtastic_PortNum_TELEMETRY_APP, request)); +} + +static void test_dispatch_foreignPortOffenderCannotShadowOwner() +{ + auto *offender = registerDispatchModule(new SyntheticReplyModule("neighbor-shaped", meshtastic_PortNum_NEIGHBORINFO_APP, + meshtastic_PortNum_NEIGHBORINFO_APP, true)); + auto *owner = registerDispatchModule( + new SyntheticReplyModule("telemetry-owner", meshtastic_PortNum_TELEMETRY_APP, meshtastic_PortNum_TELEMETRY_APP)); + + dispatch(meshtastic_PortNum_TELEMETRY_APP); + + TEST_ASSERT_EQUAL_UINT32(0, offender->allocReplyCalls); + TEST_ASSERT_EQUAL_UINT32(1, owner->allocReplyCalls); + TEST_ASSERT_EQUAL_UINT32(1, mockRouter->sentPackets.size()); + TEST_ASSERT_EQUAL(meshtastic_PortNum_TELEMETRY_APP, mockRouter->sentPackets[0].decoded.portnum); + TEST_ASSERT_EQUAL_UINT32(0, mockRoutingModule->ackNaks.size()); +} + +static void test_dispatch_ownerPortStillReplies() +{ + auto *offender = registerDispatchModule(new SyntheticReplyModule("neighbor-shaped", meshtastic_PortNum_NEIGHBORINFO_APP, + meshtastic_PortNum_NEIGHBORINFO_APP, true)); + auto *owner = registerDispatchModule( + new SyntheticReplyModule("telemetry-owner", meshtastic_PortNum_TELEMETRY_APP, meshtastic_PortNum_TELEMETRY_APP)); + + dispatch(meshtastic_PortNum_NEIGHBORINFO_APP); + + TEST_ASSERT_EQUAL_UINT32(1, offender->allocReplyCalls); + TEST_ASSERT_EQUAL_UINT32(0, owner->allocReplyCalls); + TEST_ASSERT_EQUAL_UINT32(1, mockRouter->sentPackets.size()); + TEST_ASSERT_EQUAL(meshtastic_PortNum_NEIGHBORINFO_APP, mockRouter->sentPackets[0].decoded.portnum); + TEST_ASSERT_EQUAL_UINT32(0, mockRoutingModule->ackNaks.size()); +} + +static void test_dispatch_crossPortReplyUsesRequestOwner() +{ + auto *position = registerDispatchModule( + new SyntheticReplyModule("position-owner", meshtastic_PortNum_POSITION_APP, meshtastic_PortNum_ATAK_PLUGIN_V2)); + + dispatch(meshtastic_PortNum_POSITION_APP); + + TEST_ASSERT_EQUAL_UINT32(1, position->allocReplyCalls); + TEST_ASSERT_EQUAL_UINT32(1, mockRouter->sentPackets.size()); + TEST_ASSERT_EQUAL(meshtastic_PortNum_ATAK_PLUGIN_V2, mockRouter->sentPackets[0].decoded.portnum); + TEST_ASSERT_EQUAL_UINT32(0, mockRoutingModule->ackNaks.size()); +} + +static void test_dispatch_foreignPortObserverCanSuppressNak() +{ + auto *observer = registerDispatchModule(new ObservingIgnoreModule()); + + dispatch(meshtastic_PortNum_TEXT_MESSAGE_APP); + + TEST_ASSERT_EQUAL_UINT32(0, observer->allocReplyCalls); + TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size()); + TEST_ASSERT_EQUAL_UINT32(0, mockRoutingModule->ackNaks.size()); +} + +static void test_dispatch_noResponderSendsNak() +{ + dispatch(meshtastic_PortNum_TELEMETRY_APP); + + TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size()); + TEST_ASSERT_EQUAL_UINT32(1, mockRoutingModule->ackNaks.size()); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_NO_RESPONSE, mockRoutingModule->ackNaks[0].error); + TEST_ASSERT_EQUAL_HEX32(REMOTE_NODE, mockRoutingModule->ackNaks[0].to); +} + +static void test_dispatch_ignoreRequestIsClearedPerPacket() +{ + auto *ignoring = registerDispatchModule(new ReplyIgnoreModule()); + + dispatch(meshtastic_PortNum_NEIGHBORINFO_APP); + + TEST_ASSERT_EQUAL_UINT32(1, ignoring->allocReplyCalls); + TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size()); + TEST_ASSERT_EQUAL_UINT32(0, mockRoutingModule->ackNaks.size()); + + dispatch(meshtastic_PortNum_TELEMETRY_APP); + + TEST_ASSERT_EQUAL_UINT32(1, ignoring->allocReplyCalls); + TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size()); + TEST_ASSERT_EQUAL_UINT32(1, mockRoutingModule->ackNaks.size()); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_NO_RESPONSE, mockRoutingModule->ackNaks[0].error); +} + +static void test_dispatch_realNeighborInfoCannotShadowTelemetryOwner() +{ + moduleConfig.neighbor_info.enabled = true; + realNeighborInfoModule = new NeighborInfoModule(); + registerDispatchModule( + new SyntheticReplyModule("telemetry-owner", meshtastic_PortNum_TELEMETRY_APP, meshtastic_PortNum_TELEMETRY_APP)); + + dispatch(meshtastic_PortNum_TELEMETRY_APP); + + TEST_ASSERT_EQUAL_UINT32(1, mockRouter->sentPackets.size()); + TEST_ASSERT_EQUAL(meshtastic_PortNum_TELEMETRY_APP, mockRouter->sentPackets[0].decoded.portnum); + TEST_ASSERT_EQUAL_UINT32(0, mockRoutingModule->ackNaks.size()); +} + void setup() { initializeTestEnvironment(); @@ -110,6 +483,18 @@ void setup() RUN_TEST(test_noCurrentRequest_isAllowed); RUN_TEST(test_legacyPacket_zeroHopStart_isAllowed); RUN_TEST(test_singleHopRelayedBroadcast_isBlocked); + RUN_TEST(test_replyPortMatches_ownPort); + RUN_TEST(test_replyPortMatches_neighborInfoVsTelemetry); + RUN_TEST(test_replyPortMatches_positionRequest); + RUN_TEST(test_replyPortMatches_unknownModulePort); + RUN_TEST(test_replyPortMatches_unknownRequestPort); + RUN_TEST(test_dispatch_foreignPortOffenderCannotShadowOwner); + RUN_TEST(test_dispatch_ownerPortStillReplies); + RUN_TEST(test_dispatch_crossPortReplyUsesRequestOwner); + RUN_TEST(test_dispatch_foreignPortObserverCanSuppressNak); + RUN_TEST(test_dispatch_noResponderSendsNak); + RUN_TEST(test_dispatch_ignoreRequestIsClearedPerPacket); + RUN_TEST(test_dispatch_realNeighborInfoCannotShadowTelemetryOwner); exit(UNITY_END()); }