From d30d6bd3eba7f0db00146449391a7c89f17df5fe Mon Sep 17 00:00:00 2001 From: GUVWAF <78759985+GUVWAF@users.noreply.github.com> Date: Sat, 23 Mar 2024 13:31:58 +0100 Subject: [PATCH] Fix #3452: only alter received packet if port number matches (#3474) * Use `alterReceivedProtobuf()` for NeighborInfo and Traceroute `alterReceived()` should never return NULL Traceroute should be promiscuous * Remove extensive logging from NeighborInfo module --- src/mesh/FloodingRouter.cpp | 9 ---- src/mesh/FloodingRouter.h | 2 - src/mesh/ProtobufModule.h | 4 +- src/modules/NeighborInfoModule.cpp | 72 +++--------------------------- src/modules/NeighborInfoModule.h | 9 +--- src/modules/TraceRouteModule.cpp | 24 ++++------ src/modules/TraceRouteModule.h | 7 +-- 7 files changed, 22 insertions(+), 105 deletions(-) diff --git a/src/mesh/FloodingRouter.cpp b/src/mesh/FloodingRouter.cpp index db3f3f35e6..4cfe982d8b 100644 --- a/src/mesh/FloodingRouter.cpp +++ b/src/mesh/FloodingRouter.cpp @@ -49,15 +49,6 @@ void FloodingRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas tosend->hop_limit--; // bump down the hop count - if (p->which_payload_variant == meshtastic_MeshPacket_decoded_tag) { - // If it is a traceRoute request, update the route that it went via me - if (traceRouteModule && traceRouteModule->wantPacket(p)) - traceRouteModule->updateRoute(tosend); - // If it is a neighborInfo packet, update last_sent_by_id - if (neighborInfoModule && neighborInfoModule->wantPacket(p)) - neighborInfoModule->updateLastSentById(tosend); - } - LOG_INFO("Rebroadcasting received floodmsg to neighbors\n"); // Note: we are careful to resend using the original senders node id // We are careful not to call our hooked version of send() - because we don't want to check this again diff --git a/src/mesh/FloodingRouter.h b/src/mesh/FloodingRouter.h index 309035cb30..a3adfe70c9 100644 --- a/src/mesh/FloodingRouter.h +++ b/src/mesh/FloodingRouter.h @@ -2,8 +2,6 @@ #include "PacketHistory.h" #include "Router.h" -#include "modules/NeighborInfoModule.h" -#include "modules/TraceRouteModule.h" /** * This is a mixin that extends Router with the ability to do Naive Flooding (in the standard mesh protocol sense) diff --git a/src/mesh/ProtobufModule.h b/src/mesh/ProtobufModule.h index 1067ee01e9..a2e89e98ac 100644 --- a/src/mesh/ProtobufModule.h +++ b/src/mesh/ProtobufModule.h @@ -108,8 +108,8 @@ template class ProtobufModule : protected SinglePortModule // if we can't decode it, nobody can process it! return; } - } - return alterReceivedProtobuf(mp, decoded); + return alterReceivedProtobuf(mp, decoded); + } } }; \ No newline at end of file diff --git a/src/modules/NeighborInfoModule.cpp b/src/modules/NeighborInfoModule.cpp index 4d68b4a162..1e96524695 100644 --- a/src/modules/NeighborInfoModule.cpp +++ b/src/modules/NeighborInfoModule.cpp @@ -18,73 +18,24 @@ void NeighborInfoModule::printNeighborInfo(const char *header, const meshtastic_ { LOG_DEBUG("%s NEIGHBORINFO PACKET from Node 0x%x to Node 0x%x (last sent by 0x%x)\n", header, np->node_id, nodeDB->getNodeNum(), np->last_sent_by_id); - LOG_DEBUG("----------------\n"); LOG_DEBUG("Packet contains %d neighbors\n", np->neighbors_count); for (int i = 0; i < np->neighbors_count; i++) { LOG_DEBUG("Neighbor %d: node_id=0x%x, snr=%.2f\n", i, np->neighbors[i].node_id, np->neighbors[i].snr); } - LOG_DEBUG("----------------\n"); -} -/* -Prints the nodeDB nodes so we can see whose nodeInfo we have -NOTE: for debugging only -*/ -void NeighborInfoModule::printNodeDBNodes(const char *header) -{ - int num_nodes = nodeDB->getNumMeshNodes(); - LOG_DEBUG("%s NODEDB SELECTION from Node 0x%x:\n", header, nodeDB->getNodeNum()); - LOG_DEBUG("----------------\n"); - LOG_DEBUG("DB contains %d nodes\n", num_nodes); - for (int i = 0; i < num_nodes; i++) { - const meshtastic_NodeInfoLite *dbEntry = nodeDB->getMeshNodeByIndex(i); - LOG_DEBUG(" Node %d: node_id=0x%x, snr=%.2f\n", i, dbEntry->num, dbEntry->snr); - } - LOG_DEBUG("----------------\n"); } /* Prints the nodeDB neighbors NOTE: for debugging only */ -void NeighborInfoModule::printNodeDBNeighbors(const char *header) +void NeighborInfoModule::printNodeDBNeighbors() { int num_neighbors = getNumNeighbors(); - LOG_DEBUG("%s NODEDB SELECTION from Node 0x%x:\n", header, nodeDB->getNodeNum()); - LOG_DEBUG("----------------\n"); - LOG_DEBUG("DB contains %d neighbors\n", num_neighbors); + LOG_DEBUG("Our NodeDB contains %d neighbors\n", num_neighbors); for (int i = 0; i < num_neighbors; i++) { const meshtastic_Neighbor *dbEntry = getNeighborByIndex(i); LOG_DEBUG(" Node %d: node_id=0x%x, snr=%.2f\n", i, dbEntry->node_id, dbEntry->snr); } - LOG_DEBUG("----------------\n"); -} - -/* -Prints the nodeDB with selectors for the neighbors we've chosen to send (inefficiently) -Uses LOG_DEBUG, which equates to Console.log -NOTE: For debugging only -*/ -void NeighborInfoModule::printNodeDBSelection(const char *header, const meshtastic_NeighborInfo *np) -{ - int num_neighbors = getNumNeighbors(); - LOG_DEBUG("%s NODEDB SELECTION from Node 0x%x:\n", header, nodeDB->getNodeNum()); - LOG_DEBUG("----------------\n"); - LOG_DEBUG("Selected %d neighbors of %d DB neighbors\n", np->neighbors_count, num_neighbors); - for (int i = 0; i < num_neighbors; i++) { - meshtastic_Neighbor *dbEntry = getNeighborByIndex(i); - bool chosen = false; - for (int j = 0; j < np->neighbors_count; j++) { - if (np->neighbors[j].node_id == dbEntry->node_id) { - chosen = true; - } - } - if (!chosen) { - LOG_DEBUG(" Node %d: neighbor=0x%x, snr=%.2f\n", i, dbEntry->node_id, dbEntry->snr); - } else { - LOG_DEBUG("---> Node %d: neighbor=0x%x, snr=%.2f\n", i, dbEntry->node_id, dbEntry->snr); - } - } - LOG_DEBUG("----------------\n"); } /* Send our initial owner announcement 35 seconds after we start (to give network time to setup) */ @@ -129,9 +80,7 @@ uint32_t NeighborInfoModule::collectNeighborInfo(meshtastic_NeighborInfo *neighb neighborInfo->neighbors_count++; } } - printNodeDBNodes("DBSTATE"); - printNodeDBNeighbors("NEIGHBORS"); - printNodeDBSelection("COLLECTED", neighborInfo); + printNodeDBNeighbors(); return neighborInfo->neighbors_count; } @@ -218,20 +167,13 @@ bool NeighborInfoModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, /* Copy the content of a current NeighborInfo packet into a new one and update the last_sent_by_id to our NodeNum */ -void NeighborInfoModule::updateLastSentById(meshtastic_MeshPacket *p) +void NeighborInfoModule::alterReceivedProtobuf(meshtastic_MeshPacket &p, meshtastic_NeighborInfo *n) { - auto &incoming = p->decoded; - meshtastic_NeighborInfo scratch; - meshtastic_NeighborInfo *updated = NULL; - memset(&scratch, 0, sizeof(scratch)); - pb_decode_from_bytes(incoming.payload.bytes, incoming.payload.size, &meshtastic_NeighborInfo_msg, &scratch); - updated = &scratch; - - updated->last_sent_by_id = nodeDB->getNodeNum(); + n->last_sent_by_id = nodeDB->getNodeNum(); // Set updated last_sent_by_id to the payload of the to be flooded packet - p->decoded.payload.size = - pb_encode_to_bytes(p->decoded.payload.bytes, sizeof(p->decoded.payload.bytes), &meshtastic_NeighborInfo_msg, updated); + p.decoded.payload.size = + pb_encode_to_bytes(p.decoded.payload.bytes, sizeof(p.decoded.payload.bytes), &meshtastic_NeighborInfo_msg, n); } void NeighborInfoModule::resetNeighbors() diff --git a/src/modules/NeighborInfoModule.h b/src/modules/NeighborInfoModule.h index 820e2d0d45..b4acb0f666 100644 --- a/src/modules/NeighborInfoModule.h +++ b/src/modules/NeighborInfoModule.h @@ -20,9 +20,6 @@ class NeighborInfoModule : public ProtobufModule, priva bool saveProtoForModule(); - // Let FloodingRouter call updateLastSentById upon rebroadcasting a NeighborInfo packet - friend class FloodingRouter; - protected: // Note: this holds our local info. meshtastic_NeighborInfo neighborState; @@ -68,7 +65,7 @@ class NeighborInfoModule : public ProtobufModule, priva void updateNeighbors(const meshtastic_MeshPacket &mp, const meshtastic_NeighborInfo *np); /* update a NeighborInfo packet with our NodeNum as last_sent_by_id */ - void updateLastSentById(meshtastic_MeshPacket *p); + void alterReceivedProtobuf(meshtastic_MeshPacket &p, meshtastic_NeighborInfo *n) override; void loadProtoForModule(); @@ -81,8 +78,6 @@ class NeighborInfoModule : public ProtobufModule, priva /* These are for debugging only */ void printNeighborInfo(const char *header, const meshtastic_NeighborInfo *np); - void printNodeDBNodes(const char *header); - void printNodeDBNeighbors(const char *header); - void printNodeDBSelection(const char *header, const meshtastic_NeighborInfo *np); + void printNodeDBNeighbors(); }; extern NeighborInfoModule *neighborInfoModule; \ No newline at end of file diff --git a/src/modules/TraceRouteModule.cpp b/src/modules/TraceRouteModule.cpp index 311e211f3c..aa0b6a1ebd 100644 --- a/src/modules/TraceRouteModule.cpp +++ b/src/modules/TraceRouteModule.cpp @@ -1,5 +1,4 @@ #include "TraceRouteModule.h" -#include "FloodingRouter.h" #include "MeshService.h" TraceRouteModule *traceRouteModule; @@ -14,23 +13,17 @@ bool TraceRouteModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, m return false; // let it be handled by RoutingModule } -void TraceRouteModule::updateRoute(meshtastic_MeshPacket *p) +void TraceRouteModule::alterReceivedProtobuf(meshtastic_MeshPacket &p, meshtastic_RouteDiscovery *r) { - auto &incoming = p->decoded; - // Only append an ID for the request (one way) - if (!incoming.request_id) { - meshtastic_RouteDiscovery scratch; - meshtastic_RouteDiscovery *updated = NULL; - memset(&scratch, 0, sizeof(scratch)); - pb_decode_from_bytes(incoming.payload.bytes, incoming.payload.size, &meshtastic_RouteDiscovery_msg, &scratch); - updated = &scratch; - - appendMyID(updated); - printRoute(updated, p->from, NODENUM_BROADCAST); + auto &incoming = p.decoded; + // Only append an ID for the request (one way) and if we are not the destination (the reply will have our NodeNum already) + if (!incoming.request_id && p.to != nodeDB->getNodeNum()) { + appendMyID(r); + printRoute(r, p.from, NODENUM_BROADCAST); // Set updated route to the payload of the to be flooded packet - p->decoded.payload.size = pb_encode_to_bytes(p->decoded.payload.bytes, sizeof(p->decoded.payload.bytes), - &meshtastic_RouteDiscovery_msg, updated); + p.decoded.payload.size = + pb_encode_to_bytes(p.decoded.payload.bytes, sizeof(p.decoded.payload.bytes), &meshtastic_RouteDiscovery_msg, r); } } @@ -85,4 +78,5 @@ TraceRouteModule::TraceRouteModule() : ProtobufModule("traceroute", meshtastic_PortNum_TRACEROUTE_APP, &meshtastic_RouteDiscovery_msg) { ourPortNum = meshtastic_PortNum_TRACEROUTE_APP; + isPromiscuous = true; // We need to update the route even if it is not destined to us } \ No newline at end of file diff --git a/src/modules/TraceRouteModule.h b/src/modules/TraceRouteModule.h index 674846ef16..15e01debd8 100644 --- a/src/modules/TraceRouteModule.h +++ b/src/modules/TraceRouteModule.h @@ -9,17 +9,14 @@ class TraceRouteModule : public ProtobufModule public: TraceRouteModule(); - // Let FloodingRouter call updateRoute upon rebroadcasting a TraceRoute request - friend class FloodingRouter; - protected: bool handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshtastic_RouteDiscovery *r) override; virtual meshtastic_MeshPacket *allocReply() override; - /* Call before rebroadcasting a RouteDiscovery payload in order to update + /* Called before rebroadcasting a RouteDiscovery payload in order to update the route array containing the IDs of nodes this packet went through */ - void updateRoute(meshtastic_MeshPacket *p); + void alterReceivedProtobuf(meshtastic_MeshPacket &p, meshtastic_RouteDiscovery *r) override; private: // Call to add your ID to the route array of a RouteDiscovery message