From c69583cd068c3a50b23da021c37eb73438c34dab Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:42:47 -0700 Subject: [PATCH 01/15] fix: defer direct messages while learning peer keys --- protobufs | 2 +- src/mesh/MeshService.cpp | 21 ++-- src/mesh/MeshService.h | 5 +- src/mesh/ReliableRouter.cpp | 9 +- src/mesh/Router.cpp | 91 +++++++++++++- src/mesh/Router.h | 36 ++++++ src/mesh/generated/meshtastic/mesh.pb.cpp | 1 - src/mesh/generated/meshtastic/mesh.pb.h | 27 ++++- src/modules/NodeInfoModule.cpp | 23 ++-- src/modules/NodeInfoModule.h | 7 +- test/test_mesh_module/test_main.cpp | 16 +++ test/test_packet_signing/test_main.cpp | 140 +++++++++++++++++++++- 12 files changed, 350 insertions(+), 28 deletions(-) diff --git a/protobufs b/protobufs index bfd718fa1dc..9395d423d67 160000 --- a/protobufs +++ b/protobufs @@ -1 +1 @@ -Subproject commit bfd718fa1dcb019ed11b7b7185f37318abebdafc +Subproject commit 9395d423d679acb61b4780b1b029720c31941b4d diff --git a/src/mesh/MeshService.cpp b/src/mesh/MeshService.cpp index 0efd23c8141..5e501694dbb 100644 --- a/src/mesh/MeshService.cpp +++ b/src/mesh/MeshService.cpp @@ -295,7 +295,8 @@ bool MeshService::cancelSending(PacketId id) return router->cancelSending(nodeDB->getNodeNum(), id); } -ErrorCode MeshService::sendQueueStatusToPhone(const meshtastic_QueueStatus &qs, ErrorCode res, uint32_t mesh_packet_id) +ErrorCode MeshService::sendQueueStatusToPhone(const meshtastic_QueueStatus &qs, ErrorCode res, uint32_t mesh_packet_id, + meshtastic_QueueStatus_State state) { meshtastic_QueueStatus *copied = queueStatusPool.allocCopy(qs); if (!copied) @@ -303,6 +304,7 @@ ErrorCode MeshService::sendQueueStatusToPhone(const meshtastic_QueueStatus &qs, copied->res = res; copied->mesh_packet_id = mesh_packet_id; + copied->state = state; if (toPhoneQueueStatusQueue.numFree() == 0) { LOG_INFO("tophone queue status queue is full, discard oldest"); @@ -319,7 +321,7 @@ ErrorCode MeshService::sendQueueStatusToPhone(const meshtastic_QueueStatus &qs, return res ? ERRNO_OK : ERRNO_UNKNOWN; } -void MeshService::sendToMesh(meshtastic_MeshPacket *p, RxSource src, bool ccToPhone) +void MeshService::sendToMesh(meshtastic_MeshPacket *p, RxSource src, bool ccToPhone, bool reportQueueStatus) { uint32_t mesh_packet_id = p->id; nodeDB->updateFrom(*p); // update our local DB for this packet (because phone might have sent position packets etc...) @@ -335,11 +337,16 @@ void MeshService::sendToMesh(meshtastic_MeshPacket *p, RxSource src, bool ccToPh /* NOTE(pboldin): Prepare and send QueueStatus message to the phone as a * high-priority message. */ - meshtastic_QueueStatus qs = router->getQueueStatus(); - // SHOULD_RELEASE means "caller frees", not a send failure, so don't report it as one. - ErrorCode r = sendQueueStatusToPhone(qs, (res == ERRNO_SHOULD_RELEASE && localDelivery) ? ERRNO_OK : res, mesh_packet_id); - if (r != ERRNO_OK) { - LOG_DEBUG("Can't send status to phone"); + if (reportQueueStatus) { + meshtastic_QueueStatus qs = router->getQueueStatus(); + const auto state = router->isDeferredDm(mesh_packet_id) ? meshtastic_QueueStatus_State_KEY_EXCHANGE + : meshtastic_QueueStatus_State_STATE_UNSPECIFIED; + // SHOULD_RELEASE means "caller frees", not a send failure, so don't report it as one. + ErrorCode r = + sendQueueStatusToPhone(qs, (res == ERRNO_SHOULD_RELEASE && localDelivery) ? ERRNO_OK : res, mesh_packet_id, state); + if (r != ERRNO_OK) { + LOG_DEBUG("Can't send status to phone"); + } } if ((res == ERRNO_OK || res == ERRNO_SHOULD_RELEASE) && ccToPhone) { // Check if p is not released in case it couldn't be sent diff --git a/src/mesh/MeshService.h b/src/mesh/MeshService.h index b529d283619..de92b6f8ef8 100644 --- a/src/mesh/MeshService.h +++ b/src/mesh/MeshService.h @@ -181,7 +181,7 @@ class MeshService /// Send a packet into the mesh - note p must have been allocated from packetPool. We will return it to that pool after /// sending. This is the ONLY function you should use for sending messages into the mesh, because it also updates the nodedb /// cache - void sendToMesh(meshtastic_MeshPacket *p, RxSource src = RX_SRC_LOCAL, bool ccToPhone = false); + void sendToMesh(meshtastic_MeshPacket *p, RxSource src = RX_SRC_LOCAL, bool ccToPhone = false, bool reportQueueStatus = true); /** Attempt to cancel a previously sent packet from this _local_ node. Returns true if a packet was found we could cancel */ bool cancelSending(PacketId id); @@ -203,7 +203,8 @@ class MeshService bool isToPhoneQueueEmpty(); - ErrorCode sendQueueStatusToPhone(const meshtastic_QueueStatus &qs, ErrorCode res, uint32_t mesh_packet_id); + ErrorCode sendQueueStatusToPhone(const meshtastic_QueueStatus &qs, ErrorCode res, uint32_t mesh_packet_id, + meshtastic_QueueStatus_State state = meshtastic_QueueStatus_State_STATE_UNSPECIFIED); uint32_t GetTimeSinceMeshPacket(const meshtastic_MeshPacket *mp); diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp index 7ec6bb4b72d..22f7a7e73e0 100644 --- a/src/mesh/ReliableRouter.cpp +++ b/src/mesh/ReliableRouter.cpp @@ -16,6 +16,13 @@ */ ErrorCode ReliableRouter::send(meshtastic_MeshPacket *p) { +#if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO + // Router owns the delayed packet while it asks for the destination's NodeInfo. Do this before + // allocating a retransmission copy, otherwise the stale copy can later emit MAX_RETRANSMIT. + if (deferMissingKeyDm(p)) + return ERRNO_OK; +#endif + if (p->want_ack) { DEBUG_HEAP_BEFORE; auto copy = packetPool.allocCopy(*p); @@ -196,4 +203,4 @@ bool ReliableRouter::shouldSuccessAckWithWantAck(const meshtastic_MeshPacket *p) } return false; -} \ No newline at end of file +} diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 66bc1d3b3b8..5bdc6830582 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -5,6 +5,7 @@ #include "MeshService.h" #include "NodeDB.h" #include "PositionPrecision.h" +#include "Throttle.h" #include "gps/RTC.h" #include "configuration.h" @@ -12,6 +13,9 @@ #include "mesh-pb-constants.h" #include "meshUtils.h" #include "modules/RoutingModule.h" +#if !MESHTASTIC_EXCLUDE_NODEINFO +#include "modules/NodeInfoModule.h" +#endif #include #include #include @@ -59,7 +63,7 @@ Allocator &packetPool = dynamicPool; // Embedded targets use static memory pools with compile-time constants #define MAX_PACKETS_STATIC \ (MAX_RX_TOPHONE + MAX_RX_FROMRADIO + 2 * MAX_TX_QUEUE + \ - 2) // max number of packets which can be in flight (either queued from reception or queued for sending) + 4) // max number of packets in flight plus two deferred direct messages waiting for a peer key // Static pool RAM is BSS, not heap; "pktpool(live)" still shows in-flight packet bytes static MemoryPool staticPool("pktpool(live)"); @@ -222,6 +226,12 @@ int32_t Router::runOnce() perhapsHandleReceived(mp); } +#if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO + processDeferredDms(); + if (deferredDmCount() > 0) + return 1000; +#endif + // LOG_DEBUG("Sleep forever!"); return INT32_MAX; // Wait a long time - until we get woken for the message queue } @@ -319,6 +329,19 @@ meshtastic_QueueStatus Router::getQueueStatus() return iface->getQueueStatus(); } +bool Router::isDeferredDm(PacketId id) const +{ +#if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO + for (const auto &deferred : deferredDms) { + if (deferred.p && deferred.p->id == id) + return true; + } +#else + (void)id; +#endif + return false; +} + ErrorCode Router::sendLocal(meshtastic_MeshPacket *p, RxSource src) { if (p->to == 0) { @@ -476,6 +499,10 @@ ErrorCode Router::send(meshtastic_MeshPacket *p) auto encodeResult = perhapsEncode(p); if (encodeResult != meshtastic_Routing_Error_NONE) { packetPool.release(p_decoded); +#if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO + if (encodeResult == meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY && deferMissingKeyDm(p)) + return ERRNO_OK; +#endif p->channel = 0; // Reset the channel to 0, so we don't use the failing hash again abortSendAndNak(encodeResult, p); return encodeResult; // FIXME - this isn't a valid ErrorCode @@ -1223,6 +1250,68 @@ bool Router::dequeueDeferredLocal(DeferredLocal &out) return true; } +#if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO +uint8_t Router::deferredDmCount() const +{ + uint8_t count = 0; + for (const auto &deferred : deferredDms) { + if (deferred.p) + count++; + } + return count; +} + +bool Router::deferMissingKeyDm(meshtastic_MeshPacket *p) +{ + if (!nodeInfoModule || p->which_payload_variant != meshtastic_MeshPacket_decoded_tag || + !IS_ONE_OF(p->decoded.portnum, meshtastic_PortNum_TEXT_MESSAGE_APP, meshtastic_PortNum_TEXT_MESSAGE_COMPRESSED_APP)) + return false; + + meshtastic_NodeInfoLite_public_key_t remoteKey = {0, {0}}; + if (nodeDB->copyPublicKey(p->to, remoteKey) || !wouldEncryptWithPKC(p, p->channel, false)) + return false; + + for (auto &deferred : deferredDms) { + if (deferred.p) + continue; + + deferred.p = p; + deferred.queuedAtMs = millis(); + LOG_INFO("Deferring DM id=0x%08x to 0x%08x while requesting NodeInfo", p->id, p->to); + nodeInfoModule->requestNodeInfo(p->to, p->channel); + setInterval(0); + runASAP = true; + return true; + } + + LOG_WARN("Deferred DM queue is full; cannot wait for public key of 0x%08x", p->to); + return false; +} + +void Router::processDeferredDms() +{ + for (auto &deferred : deferredDms) { + meshtastic_MeshPacket *p = deferred.p; + if (!p) + continue; + + meshtastic_NodeInfoLite_public_key_t remoteKey = {0, {0}}; + if (nodeDB->copyPublicKey(p->to, remoteKey)) { + deferred.p = nullptr; + deferred.queuedAtMs = 0; + LOG_INFO("Peer key learned for 0x%08x; retrying deferred DM id=0x%08x", p->to, p->id); + service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, p->id, meshtastic_QueueStatus_State_STATE_UNSPECIFIED); + send(p); + } else if (!Throttle::isWithinTimespanMs(deferred.queuedAtMs, deferredDmKeyWaitMs)) { + deferred.p = nullptr; + deferred.queuedAtMs = 0; + LOG_WARN("No public key learned for 0x%08x before deferred DM id=0x%08x timed out", p->to, p->id); + abortSendAndNak(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, p); + } + } +} +#endif + void Router::deliverLocal(meshtastic_MeshPacket *p, RxSource src) { // Top level: handle synchronously, exactly as before the depth guard existed. diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 4a6356cb585..f2894c15cc5 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -71,6 +71,9 @@ class Router : protected concurrency::OSThread, protected PacketHistory /** Return Underlying interface's TX queue status */ [[nodiscard]] meshtastic_QueueStatus getQueueStatus(); + /// True while a direct message with this ID is waiting for a peer public key. + bool isDeferredDm(PacketId id) const; + /** * @return our local nodenum */ [[nodiscard]] NodeNum getNodeNum(); @@ -102,6 +105,12 @@ class Router : protected concurrency::OSThread, protected PacketHistory protected: friend class RoutingModule; +#if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO + /// Takes ownership when a local text DM needs a public-key exchange before it can be sent. + /// Derived routers must call this before creating retransmission state for the packet. + bool deferMissingKeyDm(meshtastic_MeshPacket *p); +#endif + /** * Should this incoming filter be dropped? * @@ -203,6 +212,22 @@ class Router : protected concurrency::OSThread, protected PacketHistory /// Pop the oldest deferred local packet into out. Returns false when empty. bool dequeueDeferredLocal(DeferredLocal &out); +#if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO + /// A missing peer key is recoverable: ask the peer for NodeInfo, then retry the original DM + /// after its public key is learned. The fixed queue bounds RAM held for unavailable peers. + struct DeferredDm { + meshtastic_MeshPacket *p = nullptr; + uint32_t queuedAtMs = 0; + }; + + static constexpr uint8_t deferredDmCapacity = 2; + static constexpr uint32_t deferredDmKeyWaitMs = 30 * 1000UL; + DeferredDm deferredDms[deferredDmCapacity]; + + void processDeferredDms(); + uint8_t deferredDmCount() const; +#endif + /** Frees the provided packet, and generates a NAK indicating the specifed error while sending */ void abortSendAndNak(meshtastic_Routing_Error err, meshtastic_MeshPacket *p); @@ -215,6 +240,17 @@ class Router : protected concurrency::OSThread, protected PacketHistory uint32_t deferredLocalDropped = 0; /// Number of deferred local packets currently queued. uint8_t deferredLocalPending() const { return deferredLocalCount; } +#if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO + uint8_t deferredDmPending() const { return deferredDmCount(); } + void processDeferredDmsForTest() { processDeferredDms(); } + void expireDeferredDmsForTest() + { + for (auto &deferred : deferredDms) { + if (deferred.p) + deferred.queuedAtMs = millis() - deferredDmKeyWaitMs; + } + } +#endif #endif }; diff --git a/src/mesh/generated/meshtastic/mesh.pb.cpp b/src/mesh/generated/meshtastic/mesh.pb.cpp index 4cf8e980cf2..3a089ffd140 100644 --- a/src/mesh/generated/meshtastic/mesh.pb.cpp +++ b/src/mesh/generated/meshtastic/mesh.pb.cpp @@ -151,6 +151,5 @@ PB_BIND(meshtastic_ChunkedPayloadResponse, meshtastic_ChunkedPayloadResponse, AU - diff --git a/src/mesh/generated/meshtastic/mesh.pb.h b/src/mesh/generated/meshtastic/mesh.pb.h index 7ce8115baee..47da7dd1c09 100644 --- a/src/mesh/generated/meshtastic/mesh.pb.h +++ b/src/mesh/generated/meshtastic/mesh.pb.h @@ -660,6 +660,16 @@ typedef enum _meshtastic_LogRecord_Level { meshtastic_LogRecord_Level_TRACE = 5 } meshtastic_LogRecord_Level; +/* Lifecycle state for the mesh packet identified by mesh_packet_id. + Clients that do not recognize this field should retain their existing + queue-status behavior. */ +typedef enum _meshtastic_QueueStatus_State { + meshtastic_QueueStatus_State_STATE_UNSPECIFIED = 0, + /* The device is holding a direct message while it requests the + recipient's NodeInfo to obtain a public encryption key. */ + meshtastic_QueueStatus_State_KEY_EXCHANGE = 1 +} meshtastic_QueueStatus_State; + typedef enum _meshtastic_LockdownStatus_State { /* Default; should not be sent. */ meshtastic_LockdownStatus_State_STATE_UNSPECIFIED = 0, @@ -1242,6 +1252,8 @@ typedef struct _meshtastic_QueueStatus { uint8_t maxlen; /* What was mesh packet id that generated this response? */ uint32_t mesh_packet_id; + /* Current lifecycle state for mesh_packet_id */ + meshtastic_QueueStatus_State state; } meshtastic_QueueStatus; /* Lockdown state report from firmware to client (for hardened builds @@ -1652,6 +1664,10 @@ extern "C" { #define _meshtastic_LogRecord_Level_MAX meshtastic_LogRecord_Level_CRITICAL #define _meshtastic_LogRecord_Level_ARRAYSIZE ((meshtastic_LogRecord_Level)(meshtastic_LogRecord_Level_CRITICAL+1)) +#define _meshtastic_QueueStatus_State_MIN meshtastic_QueueStatus_State_STATE_UNSPECIFIED +#define _meshtastic_QueueStatus_State_MAX meshtastic_QueueStatus_State_KEY_EXCHANGE +#define _meshtastic_QueueStatus_State_ARRAYSIZE ((meshtastic_QueueStatus_State)(meshtastic_QueueStatus_State_KEY_EXCHANGE+1)) + #define _meshtastic_LockdownStatus_State_MIN meshtastic_LockdownStatus_State_STATE_UNSPECIFIED #define _meshtastic_LockdownStatus_State_MAX meshtastic_LockdownStatus_State_DISABLED #define _meshtastic_LockdownStatus_State_ARRAYSIZE ((meshtastic_LockdownStatus_State)(meshtastic_LockdownStatus_State_DISABLED+1)) @@ -1685,6 +1701,7 @@ extern "C" { #define meshtastic_LogRecord_level_ENUMTYPE meshtastic_LogRecord_Level +#define meshtastic_QueueStatus_state_ENUMTYPE meshtastic_QueueStatus_State #define meshtastic_LockdownStatus_state_ENUMTYPE meshtastic_LockdownStatus_State @@ -1734,7 +1751,7 @@ extern "C" { #define meshtastic_NodeInfo_init_default {0, false, meshtastic_User_init_default, false, meshtastic_Position_init_default, 0, 0, false, meshtastic_DeviceMetrics_init_default, 0, 0, false, 0, 0, 0, 0, 0, 0} #define meshtastic_MyNodeInfo_init_default {0, 0, 0, {0, {0}}, "", _meshtastic_FirmwareEdition_MIN, 0} #define meshtastic_LogRecord_init_default {"", 0, "", _meshtastic_LogRecord_Level_MIN} -#define meshtastic_QueueStatus_init_default {0, 0, 0, 0} +#define meshtastic_QueueStatus_init_default {0, 0, 0, 0, _meshtastic_QueueStatus_State_MIN} #define meshtastic_FromRadio_init_default {0, 0, {meshtastic_MeshPacket_init_default}} #define meshtastic_LockdownStatus_init_default {_meshtastic_LockdownStatus_State_MIN, "", 0, 0, 0} #define meshtastic_ClientNotification_init_default {false, 0, 0, _meshtastic_LogRecord_Level_MIN, "", 0, {meshtastic_KeyVerificationNumberInform_init_default}} @@ -1773,7 +1790,7 @@ extern "C" { #define meshtastic_NodeInfo_init_zero {0, false, meshtastic_User_init_zero, false, meshtastic_Position_init_zero, 0, 0, false, meshtastic_DeviceMetrics_init_zero, 0, 0, false, 0, 0, 0, 0, 0, 0} #define meshtastic_MyNodeInfo_init_zero {0, 0, 0, {0, {0}}, "", _meshtastic_FirmwareEdition_MIN, 0} #define meshtastic_LogRecord_init_zero {"", 0, "", _meshtastic_LogRecord_Level_MIN} -#define meshtastic_QueueStatus_init_zero {0, 0, 0, 0} +#define meshtastic_QueueStatus_init_zero {0, 0, 0, 0, _meshtastic_QueueStatus_State_MIN} #define meshtastic_FromRadio_init_zero {0, 0, {meshtastic_MeshPacket_init_zero}} #define meshtastic_LockdownStatus_init_zero {_meshtastic_LockdownStatus_State_MIN, "", 0, 0, 0} #define meshtastic_ClientNotification_init_zero {false, 0, 0, _meshtastic_LogRecord_Level_MIN, "", 0, {meshtastic_KeyVerificationNumberInform_init_zero}} @@ -1943,6 +1960,7 @@ extern "C" { #define meshtastic_QueueStatus_free_tag 2 #define meshtastic_QueueStatus_maxlen_tag 3 #define meshtastic_QueueStatus_mesh_packet_id_tag 4 +#define meshtastic_QueueStatus_state_tag 5 #define meshtastic_LockdownStatus_state_tag 1 #define meshtastic_LockdownStatus_lock_reason_tag 2 #define meshtastic_LockdownStatus_boots_remaining_tag 3 @@ -2253,7 +2271,8 @@ X(a, STATIC, SINGULAR, UENUM, level, 4) X(a, STATIC, SINGULAR, INT32, res, 1) \ X(a, STATIC, SINGULAR, UINT32, free, 2) \ X(a, STATIC, SINGULAR, UINT32, maxlen, 3) \ -X(a, STATIC, SINGULAR, UINT32, mesh_packet_id, 4) +X(a, STATIC, SINGULAR, UINT32, mesh_packet_id, 4) \ +X(a, STATIC, SINGULAR, UENUM, state, 5) #define meshtastic_QueueStatus_CALLBACK NULL #define meshtastic_QueueStatus_DEFAULT NULL @@ -2582,7 +2601,7 @@ extern const pb_msgdesc_t meshtastic_ChunkedPayloadResponse_msg; #define meshtastic_NodeInfo_size 327 #define meshtastic_NodeRemoteHardwarePin_size 29 #define meshtastic_Position_size 144 -#define meshtastic_QueueStatus_size 23 +#define meshtastic_QueueStatus_size 25 #define meshtastic_RemoteShell_size 253 #define meshtastic_RouteDiscovery_size 256 #define meshtastic_Routing_size 259 diff --git a/src/modules/NodeInfoModule.cpp b/src/modules/NodeInfoModule.cpp index 01168114dcf..9b63c667453 100644 --- a/src/modules/NodeInfoModule.cpp +++ b/src/modules/NodeInfoModule.cpp @@ -96,21 +96,24 @@ void NodeInfoModule::alterReceivedProtobuf(meshtastic_MeshPacket &mp, meshtastic pb_encode_to_bytes(mp.decoded.payload.bytes, sizeof(mp.decoded.payload.bytes), &meshtastic_User_msg, p); } -void NodeInfoModule::sendOurNodeInfo(NodeNum dest, bool wantReplies, uint8_t channel, bool _shorterTimeout) +void NodeInfoModule::sendOurNodeInfo(NodeNum dest, bool wantReplies, uint8_t channel, bool _shorterTimeout, bool _force) { // cancel any not yet sent (now stale) position packets if (prevPacketId) // if we wrap around to zero, we'll simply fail to cancel in that rare case (no big deal) service->cancelSending(prevPacketId); shorterTimeout = _shorterTimeout; + forceSend = _force; DEBUG_HEAP_BEFORE; meshtastic_MeshPacket *p = allocReply(); DEBUG_HEAP_AFTER("NodeInfoModule::sendOurNodeInfo", p); + shorterTimeout = false; + forceSend = false; if (p) { // Check whether we didn't ignore it p->to = dest; - bool requestWantResponse = (config.device.role != meshtastic_Config_DeviceConfig_Role_TRACKER && - config.device.role != meshtastic_Config_DeviceConfig_Role_SENSOR) && - wantReplies; + bool requestWantResponse = _force || ((config.device.role != meshtastic_Config_DeviceConfig_Role_TRACKER && + config.device.role != meshtastic_Config_DeviceConfig_Role_SENSOR) && + wantReplies); p->decoded.want_response = requestWantResponse; if (_shorterTimeout) @@ -124,11 +127,15 @@ void NodeInfoModule::sendOurNodeInfo(NodeNum dest, bool wantReplies, uint8_t cha prevPacketId = p->id; - service->sendToMesh(p); - shorterTimeout = false; + service->sendToMesh(p, RX_SRC_LOCAL, false, false); } } +void NodeInfoModule::requestNodeInfo(NodeNum dest, uint8_t channel) +{ + sendOurNodeInfo(dest, true, channel, true, true); +} + void NodeInfoModule::triggerImmediateNodeInfoCheck() { LOG_DEBUG("NodeInfo: scheduling immediate periodic check"); @@ -159,11 +166,11 @@ meshtastic_MeshPacket *NodeInfoModule::allocReply() // Use graduated scaling based on active mesh size (10 minute base, scales with congestion coefficient) uint32_t timeoutMs = Default::getConfiguredOrDefaultMsScaled(0, 10 * 60, nodeStatus->getNumOnline()); uint32_t lastNodeInfo = transmitHistory ? transmitHistory->getLastSentToMeshMillis(meshtastic_PortNum_NODEINFO_APP) : 0; - if (!shorterTimeout && lastNodeInfo && Throttle::isWithinTimespanMs(lastNodeInfo, timeoutMs)) { + if (!forceSend && !shorterTimeout && lastNodeInfo && Throttle::isWithinTimespanMs(lastNodeInfo, timeoutMs)) { LOG_DEBUG("Skip send NodeInfo since we sent it <%us ago", timeoutMs / 1000); ignoreRequest = true; // Mark it as ignored for MeshModule return NULL; - } else if (shorterTimeout && lastNodeInfo && Throttle::isWithinTimespanMs(lastNodeInfo, 60 * 1000)) { + } else if (!forceSend && shorterTimeout && lastNodeInfo && Throttle::isWithinTimespanMs(lastNodeInfo, 60 * 1000)) { // For interactive/urgent requests (e.g., user-triggered or implicit requests), use a shorter 60s timeout LOG_DEBUG("Skip send NodeInfo since we sent it <60s ago"); ignoreRequest = true; diff --git a/src/modules/NodeInfoModule.h b/src/modules/NodeInfoModule.h index 9b3b66caed4..1b99a68fd60 100644 --- a/src/modules/NodeInfoModule.h +++ b/src/modules/NodeInfoModule.h @@ -22,7 +22,11 @@ class NodeInfoModule : public ProtobufModule, private concurren * Send our NodeInfo into the mesh */ void sendOurNodeInfo(NodeNum dest = NODENUM_BROADCAST, bool wantReplies = false, uint8_t channel = 0, - bool _shorterTimeout = false); + bool _shorterTimeout = false, bool _force = false); + + /// Send a directed NodeInfo request even when the regular announcement throttle is active. + /// Router uses this to recover a missing direct-message public key without exposing the DM. + void requestNodeInfo(NodeNum dest, uint8_t channel); /** * Schedule an immediate NodeInfo periodic check. @@ -49,6 +53,7 @@ class NodeInfoModule : public ProtobufModule, private concurren private: bool shorterTimeout = false; + bool forceSend = false; bool suppressReplyForCurrentRequest = false; std::map lastNodeInfoSeen; diff --git a/test/test_mesh_module/test_main.cpp b/test/test_mesh_module/test_main.cpp index a3998806449..9ec7220d65a 100644 --- a/test/test_mesh_module/test_main.cpp +++ b/test/test_mesh_module/test_main.cpp @@ -606,6 +606,21 @@ static void test_localReplyToSelf_isDeliveredToPhone() TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size()); // nothing went toward the radio } +static void test_queueStatus_keyExchangeKeepsOriginalMessageId() +{ + constexpr PacketId messageId = 0xD00DFEED; + meshtastic_QueueStatus queueStatus = meshtastic_QueueStatus_init_zero; + + TEST_ASSERT_EQUAL(ERRNO_OK, mockService->sendQueueStatusToPhone(queueStatus, ERRNO_OK, messageId, + meshtastic_QueueStatus_State_KEY_EXCHANGE)); + + meshtastic_QueueStatus *reported = mockService->getQueueStatusForPhone(); + TEST_ASSERT_NOT_NULL(reported); + TEST_ASSERT_EQUAL_UINT32(messageId, reported->mesh_packet_id); + TEST_ASSERT_EQUAL(meshtastic_QueueStatus_State_KEY_EXCHANGE, reported->state); + mockService->releaseQueueStatusToPool(reported); +} + // Full loop: a phone-originated want_response request (from == 0, RX_SRC_USER) dispatched // through the real router must produce a module reply that reaches the phone queue. static void test_phoneRequest_replyReachesPhone() @@ -736,6 +751,7 @@ void setup() RUN_TEST(test_dispatch_ignoreRequestIsClearedPerPacket); RUN_TEST(test_dispatch_realNeighborInfoCannotShadowTelemetryOwner); RUN_TEST(test_localReplyToSelf_isDeliveredToPhone); + RUN_TEST(test_queueStatus_keyExchangeKeepsOriginalMessageId); RUN_TEST(test_phoneRequest_replyReachesPhone); RUN_TEST(test_nestedLocalSend_isDeferred_notReentrant); RUN_TEST(test_deferredChain_drainsBreadthFirst); diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index f5eaa44bbdf..44819f17ecd 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -19,6 +19,7 @@ // compiled out unless both PKI and XEdDSA are enabled (e.g. stm32 sets MESHTASTIC_EXCLUDE_XEDDSA). #if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA) +#include "airtime.h" #include "mesh/Channels.h" #include "mesh/CryptoEngine.h" #include "mesh/MeshRadio.h" @@ -36,6 +37,9 @@ #include #include #include +#if ARCH_PORTDUINO +#include "platform/portduino/PortduinoGlue.h" +#endif // --------------------------------------------------------------------------- // Test fixture identifiers @@ -114,6 +118,7 @@ class AuthPipelineRadio : public RadioInterface ErrorCode send(meshtastic_MeshPacket *p) override { sendCalls++; + sentPackets.push_back(*p); packetPool.release(p); return ERRNO_OK; } @@ -133,12 +138,17 @@ class AuthPipelineRadio : public RadioInterface return true; } uint32_t getPacketTime(uint32_t, bool = false) override { return 7; } - void reset() { sendCalls = cancelCalls = findCalls = removeCalls = 0; } + void reset() + { + sendCalls = cancelCalls = findCalls = removeCalls = 0; + sentPackets.clear(); + } uint32_t sendCalls = 0; uint32_t cancelCalls = 0; uint32_t findCalls = 0; uint32_t removeCalls = 0; + std::vector sentPackets; }; class AuthPipelineRouter : public ReliableRouter @@ -174,8 +184,15 @@ class AuthPipelineRouter : public ReliableRouter class AuthPipelineRoutingModule : public RoutingModule { public: - void sendAckNak(meshtastic_Routing_Error, NodeNum, PacketId, ChannelIndex, uint8_t = 0, bool = false) override { ackCalls++; } + void sendAckNak(meshtastic_Routing_Error err, NodeNum, PacketId id, ChannelIndex, uint8_t = 0, bool = false) override + { + ackCalls++; + lastAckError = err; + lastAckId = id; + } uint32_t ackCalls = 0; + meshtastic_Routing_Error lastAckError = meshtastic_Routing_Error_NONE; + PacketId lastAckId = 0; }; class AuthPipelineModule : public SinglePortModule @@ -207,6 +224,12 @@ static AuthPipelineRoutingModule *pipelineRouting = nullptr; static AuthPipelineModule *pipelineModule = nullptr; static AuthPipelineMqtt *pipelineMqtt = nullptr; static MeshService *pipelineService = nullptr; +static NodeInfoModule *dmKeyWaitNodeInfo = nullptr; +static AirTime *dmKeyWaitAirTime = nullptr; +#if ARCH_PORTDUINO +static bool dmKeyWaitOriginalForceSimRadio = false; +static bool dmKeyWaitChangedForceSimRadio = false; +#endif // --------------------------------------------------------------------------- // Helpers @@ -394,6 +417,8 @@ void setUp(void) pipelineRouter->txRelayCanceled = 0; pipelineRadio->reset(); pipelineRouting->ackCalls = 0; + pipelineRouting->lastAckError = meshtastic_Routing_Error_NONE; + pipelineRouting->lastAckId = 0; pipelineModule->calls = 0; pipelineMqtt->clearQueue(); while (meshtastic_MeshPacket *queued = pipelineService->getForPhone()) @@ -1304,6 +1329,114 @@ void test_C12_exact_authenticated_replay_reuses_verdict_without_collision_bypass TEST_ASSERT_EQUAL_MESSAGE(3, routingAuthEvaluationCount(), "same packet ID with different bytes must be reevaluated"); } +// =========================================================================== +// Group M - deferred direct messages while a peer key is being learned +// =========================================================================== + +static void enablePkiForLocalNode() +{ +#if ARCH_PORTDUINO + if (!dmKeyWaitChangedForceSimRadio) { + dmKeyWaitOriginalForceSimRadio = portduino_config.force_simradio; + dmKeyWaitChangedForceSimRadio = true; + } + portduino_config.force_simradio = false; +#endif + uint8_t localPublic[32], localPrivate[32]; + crypto->generateKeyPair(localPublic, localPrivate); + crypto->setDHPrivateKey(localPrivate); + config.security.private_key.size = sizeof(localPrivate); + memcpy(config.security.private_key.bytes, localPrivate, sizeof(localPrivate)); + owner.public_key.size = sizeof(localPublic); + memcpy(owner.public_key.bytes, localPublic, sizeof(localPublic)); + + meshtastic_Channel channel = channels.getByIndex(0); + strncpy(channel.settings.name, "DMKeyWait", sizeof(channel.settings.name) - 1); + channel.settings.name[sizeof(channel.settings.name) - 1] = '\0'; + channels.setChannel(channel); + channels.onConfigChanged(); +} + +static void enableNodeInfoForDmKeyWait() +{ + if (!dmKeyWaitNodeInfo) + dmKeyWaitNodeInfo = new NodeInfoModule(); + if (!dmKeyWaitAirTime) + dmKeyWaitAirTime = new AirTime(); + nodeInfoModule = dmKeyWaitNodeInfo; + airTime = dmKeyWaitAirTime; +} + +void test_M1_unknown_dm_waits_for_nodeinfo_key_exchange_then_retries(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + TEST_ASSERT_EQUAL_HEX32(LOCAL_NODE, nodeDB->getNodeNum()); + TEST_ASSERT_EQUAL(32, config.security.private_key.size); + TEST_ASSERT_FALSE(owner.is_licensed); + TEST_ASSERT_FALSE(isBroadcast(dm->to)); + TEST_ASSERT_TRUE_MESSAGE(wouldEncryptWithPKC(dm, dm->channel, false), "fixture must exercise the missing-key PKI path"); + dm->want_ack = true; + const PacketId originalDmId = dm->id; + + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + TEST_ASSERT_EQUAL_MESSAGE(1, pipelineRouter->deferredDmPending(), "DM must stay queued until a key arrives"); + TEST_ASSERT_EQUAL_MESSAGE(0, pipelineRouter->pendingCount(), "deferred DM must not create a stale retry record"); + TEST_ASSERT_EQUAL_MESSAGE(1, pipelineRadio->sendCalls, "request a directed NodeInfo exchange before retrying the DM"); + TEST_ASSERT_EQUAL(0, pipelineRouting->ackCalls); + + meshtastic_MeshPacket nodeInfoRequest = pipelineRadio->sentPackets.front(); + TEST_ASSERT_EQUAL(DECODE_SUCCESS, perhapsDecode(&nodeInfoRequest)); + TEST_ASSERT_EQUAL(meshtastic_PortNum_NODEINFO_APP, nodeInfoRequest.decoded.portnum); + TEST_ASSERT_TRUE(nodeInfoRequest.decoded.want_response); + TEST_ASSERT_EQUAL(REMOTE_NODE, nodeInfoRequest.to); + TEST_ASSERT_NOT_EQUAL(originalDmId, nodeInfoRequest.id); + + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePublic); + + pipelineRouter->processDeferredDmsForTest(); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); + TEST_ASSERT_TRUE_MESSAGE(pipelineRadio->sentPackets.back().pki_encrypted, + "the delayed DM must retry as PKI, never as channel-encrypted plaintext"); + TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRadio->sentPackets.back().id); + TEST_ASSERT_EQUAL(0, pipelineRouting->ackCalls); +} + +void test_M2_unknown_dm_fails_only_after_key_exchange_timeout(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + TEST_ASSERT_TRUE_MESSAGE(wouldEncryptWithPKC(dm, dm->channel, false), "fixture must exercise the missing-key PKI path"); + dm->want_ack = true; + const PacketId originalDmId = dm->id; + + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(0, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(0, pipelineRouting->ackCalls); + + pipelineRouter->expireDeferredDmsForTest(); + pipelineRouter->processDeferredDmsForTest(); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRouting->ackCalls); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, pipelineRouting->lastAckError); + TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRouting->lastAckId); +#if ARCH_PORTDUINO + portduino_config.force_simradio = dmKeyWaitOriginalForceSimRadio; +#endif +} + // 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_N5_unsigned_unicast_nodeinfo_from_signer_does_not_change_name(void) @@ -1666,6 +1799,9 @@ void setup() RUN_TEST(test_C10_legacy_channel_dm_failure_has_no_pipeline_effects); RUN_TEST(test_C11_malformed_pki_plaintext_has_no_pipeline_effects); RUN_TEST(test_C12_exact_authenticated_replay_reuses_verdict_without_collision_bypass); + printf("\n=== Group M: deferred DM key exchange ===\n"); + RUN_TEST(test_M1_unknown_dm_waits_for_nodeinfo_key_exchange_then_retries); + RUN_TEST(test_M2_unknown_dm_fails_only_after_key_exchange_timeout); printf("\n=== Group N: NodeInfoModule authentication ===\n"); RUN_TEST(test_N1_unsigned_nodeinfo_from_signer_dropped); RUN_TEST(test_N2_signed_nodeinfo_from_signer_not_dropped); From 4195ea7db82ebf27c1d243b0f2a6d6bfa9bcbc34 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sat, 25 Jul 2026 20:19:58 -0700 Subject: [PATCH 02/15] fix: recover DMs when peers lack sender keys --- src/mesh/ReliableRouter.cpp | 37 +++++-- src/mesh/Router.cpp | 93 +++++++++++++++++ src/mesh/Router.h | 36 ++++++- src/modules/RoutingModule.cpp | 5 +- test/test_packet_signing/test_main.cpp | 134 +++++++++++++++++++++++++ 5 files changed, 295 insertions(+), 10 deletions(-) diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp index 22f7a7e73e0..9cd36ad0e59 100644 --- a/src/mesh/ReliableRouter.cpp +++ b/src/mesh/ReliableRouter.cpp @@ -100,6 +100,28 @@ bool ReliableRouter::shouldFilterReceived(const meshtastic_MeshPacket *p) void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtastic_Routing *c) { if (isToUs(p)) { // ignore ack/nak/want_ack packets that are not address to us (we only handle 0 hop reliability) + bool deferredForPeerKey = false; + bool alreadyRetriedForPeerKey = false; + if (c && c->error_reason == meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY && + p->which_payload_variant == meshtastic_MeshPacket_decoded_tag && p->decoded.request_id) { +#if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO + if (isWaitingForPeerKeyDm(p->from, p->decoded.request_id)) { + suppressRoutingDelivery(*p); + deferredForPeerKey = true; + } else if (!(alreadyRetriedForPeerKey = hasRetriedPeerKeyDm(p->from, p->decoded.request_id))) { + if (PendingPacket *pendingPacket = findPendingPacket(GlobalPacketId(p->to, p->decoded.request_id))) { + meshtastic_MeshPacket *retry = packetPool.allocCopy(*pendingPacket->packet); + if (retry && deferPeerKeyDm(retry)) { + stopRetransmission(p->to, p->decoded.request_id); + suppressRoutingDelivery(*p); + deferredForPeerKey = true; + } else if (retry) { + packetPool.release(retry); + } + } + } +#endif + } if (!MeshModule::currentReply) { if (p->want_ack) { if (p->which_payload_variant == meshtastic_MeshPacket_decoded_tag) { @@ -122,11 +144,12 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas // stop the immediate relayer's retransmissions. sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, p->channel, 0); } - } else if (p->which_payload_variant == meshtastic_MeshPacket_encrypted_tag && p->channel == 0 && - (nodeDB->getMeshNode(p->from) == nullptr || nodeDB->getMeshNode(p->from)->public_key.size == 0)) { - LOG_INFO("PKI packet from unknown node, send PKI_UNKNOWN_PUBKEY"); - sendAckNak(meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY, getFrom(p), p->id, channels.getPrimaryIndex(), - routingModule->getHopLimitForResponse(*p)); + } else if (p->which_payload_variant == meshtastic_MeshPacket_encrypted_tag && p->channel == 0) { + const meshtastic_NodeInfoLite *sender = nodeDB->getMeshNode(p->from); + const auto error = (!sender || sender->public_key.size == 0) ? meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY + : meshtastic_Routing_Error_PKI_FAILED; + LOG_INFO("Undecryptable PKI packet from 0x%08x, send error %d", p->from, error); + sendAckNak(error, getFrom(p), p->id, channels.getPrimaryIndex(), routingModule->getHopLimitForResponse(*p)); } else { // Send a 'NO_CHANNEL' error on the primary channel if want_ack packet destined for us cannot be decoded sendAckNak(meshtastic_Routing_Error_NO_CHANNEL, getFrom(p), p->id, channels.getPrimaryIndex(), @@ -139,8 +162,8 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas } else { LOG_DEBUG("Another module replied to this message, no need for 2nd ack"); } - if (p->which_payload_variant == meshtastic_MeshPacket_decoded_tag && c && - c->error_reason == meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY) { + if (!deferredForPeerKey && !alreadyRetriedForPeerKey && p->which_payload_variant == meshtastic_MeshPacket_decoded_tag && + c && c->error_reason == meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY) { if (owner.public_key.size == 32) { LOG_INFO("PKI decrypt failure, send a NodeInfo"); nodeInfoModule->sendOurNodeInfo(p->from, false, p->channel, true); diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 5bdc6830582..4410257ebf1 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -342,6 +342,20 @@ bool Router::isDeferredDm(PacketId id) const return false; } +bool Router::shouldSuppressRoutingDelivery(const meshtastic_MeshPacket &p) +{ +#if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO + if (suppressedRoutingDelivery.from == p.from && suppressedRoutingDelivery.id == p.id && + suppressedRoutingDelivery.requestId == p.decoded.request_id) { + suppressedRoutingDelivery = {}; + return true; + } +#else + (void)p; +#endif + return false; +} + ErrorCode Router::sendLocal(meshtastic_MeshPacket *p, RxSource src) { if (p->to == 0) { @@ -1277,6 +1291,7 @@ bool Router::deferMissingKeyDm(meshtastic_MeshPacket *p) deferred.p = p; deferred.queuedAtMs = millis(); + deferred.reason = DeferredDm::Reason::DESTINATION_KEY; LOG_INFO("Deferring DM id=0x%08x to 0x%08x while requesting NodeInfo", p->id, p->to); nodeInfoModule->requestNodeInfo(p->to, p->channel); setInterval(0); @@ -1288,6 +1303,71 @@ bool Router::deferMissingKeyDm(meshtastic_MeshPacket *p) return false; } +bool Router::deferPeerKeyDm(meshtastic_MeshPacket *p) +{ + if (!nodeInfoModule || p->which_payload_variant != meshtastic_MeshPacket_decoded_tag || + !IS_ONE_OF(p->decoded.portnum, meshtastic_PortNum_TEXT_MESSAGE_APP, meshtastic_PortNum_TEXT_MESSAGE_COMPRESSED_APP)) + return false; + + for (auto &deferred : deferredDms) { + if (deferred.p) + continue; + + deferred.p = p; + deferred.queuedAtMs = millis(); + deferred.reason = DeferredDm::Reason::PEER_KEY; + LOG_INFO("Deferring DM id=0x%08x while peer 0x%08x learns our NodeInfo", p->id, p->to); + nodeInfoModule->sendOurNodeInfo(p->to, false, p->channel, true, true); + service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, p->id, meshtastic_QueueStatus_State_KEY_EXCHANGE); + setInterval(0); + runASAP = true; + return true; + } + + LOG_WARN("Deferred DM queue is full; cannot wait for peer 0x%08x to learn our key", p->to); + return false; +} + +bool Router::isWaitingForPeerKeyDm(NodeNum peer, PacketId id) const +{ + for (const auto &deferred : deferredDms) { + if (deferred.p && deferred.reason == DeferredDm::Reason::PEER_KEY && deferred.p->to == peer && deferred.p->id == id) + return true; + } + return false; +} + +bool Router::hasRetriedPeerKeyDm(NodeNum peer, PacketId id) +{ + for (auto &retry : peerKeyRetries) { + if (retry.peer != peer || retry.id != id) + continue; + if (Throttle::isWithinTimespanMs(retry.retriedAtMs, peerKeyRetryMemoryMs)) + return true; + retry = {}; + return false; + } + return false; +} + +void Router::rememberPeerKeyRetry(NodeNum peer, PacketId id) +{ + PeerKeyRetry *slot = &peerKeyRetries[0]; + for (auto &retry : peerKeyRetries) { + if ((retry.peer == peer && retry.id == id) || retry.peer == 0 || + !Throttle::isWithinTimespanMs(retry.retriedAtMs, peerKeyRetryMemoryMs)) { + slot = &retry; + break; + } + } + *slot = {peer, id, static_cast(millis())}; +} + +void Router::suppressRoutingDelivery(const meshtastic_MeshPacket &p) +{ + suppressedRoutingDelivery = {p.from, p.id, p.decoded.request_id}; +} + void Router::processDeferredDms() { for (auto &deferred : deferredDms) { @@ -1295,6 +1375,19 @@ void Router::processDeferredDms() if (!p) continue; + if (deferred.reason == DeferredDm::Reason::PEER_KEY) { + if (!Throttle::isWithinTimespanMs(deferred.queuedAtMs, deferredDmPeerKeyWaitMs)) { + deferred.p = nullptr; + deferred.queuedAtMs = 0; + LOG_INFO("Retrying deferred DM id=0x%08x after sharing our NodeInfo with 0x%08x", p->id, p->to); + rememberPeerKeyRetry(p->to, p->id); + service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, p->id, + meshtastic_QueueStatus_State_STATE_UNSPECIFIED); + send(p); + } + continue; + } + meshtastic_NodeInfoLite_public_key_t remoteKey = {0, {0}}; if (nodeDB->copyPublicKey(p->to, remoteKey)) { deferred.p = nullptr; diff --git a/src/mesh/Router.h b/src/mesh/Router.h index f2894c15cc5..967703b0a4e 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -74,6 +74,9 @@ class Router : protected concurrency::OSThread, protected PacketHistory /// True while a direct message with this ID is waiting for a peer public key. bool isDeferredDm(PacketId id) const; + /// Consume a routing packet that firmware handled as an internal DM key exchange step. + bool shouldSuppressRoutingDelivery(const meshtastic_MeshPacket &p); + /** * @return our local nodenum */ [[nodiscard]] NodeNum getNodeNum(); @@ -109,6 +112,11 @@ class Router : protected concurrency::OSThread, protected PacketHistory /// Takes ownership when a local text DM needs a public-key exchange before it can be sent. /// Derived routers must call this before creating retransmission state for the packet. bool deferMissingKeyDm(meshtastic_MeshPacket *p); + bool deferPeerKeyDm(meshtastic_MeshPacket *p); + bool isWaitingForPeerKeyDm(NodeNum peer, PacketId id) const; + bool hasRetriedPeerKeyDm(NodeNum peer, PacketId id); + void rememberPeerKeyRetry(NodeNum peer, PacketId id); + void suppressRoutingDelivery(const meshtastic_MeshPacket &p); #endif /** @@ -213,19 +221,36 @@ class Router : protected concurrency::OSThread, protected PacketHistory bool dequeueDeferredLocal(DeferredLocal &out); #if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO - /// A missing peer key is recoverable: ask the peer for NodeInfo, then retry the original DM - /// after its public key is learned. The fixed queue bounds RAM held for unavailable peers. + /// Key-exchange DM recovery holds the original packet while it learns or shares public keys. + /// The fixed queue bounds RAM held for unavailable peers. struct DeferredDm { + enum class Reason : uint8_t { DESTINATION_KEY, PEER_KEY }; + meshtastic_MeshPacket *p = nullptr; uint32_t queuedAtMs = 0; + Reason reason = Reason::DESTINATION_KEY; }; static constexpr uint8_t deferredDmCapacity = 2; static constexpr uint32_t deferredDmKeyWaitMs = 30 * 1000UL; + static constexpr uint32_t deferredDmPeerKeyWaitMs = 10 * 1000UL; + static constexpr uint32_t peerKeyRetryMemoryMs = 30 * 1000UL; DeferredDm deferredDms[deferredDmCapacity]; + struct PeerKeyRetry { + NodeNum peer = 0; + PacketId id = 0; + uint32_t retriedAtMs = 0; + } peerKeyRetries[deferredDmCapacity]; + void processDeferredDms(); uint8_t deferredDmCount() const; + + struct SuppressedRoutingDelivery { + NodeNum from = 0; + PacketId id = 0; + PacketId requestId = 0; + } suppressedRoutingDelivery; #endif /** Frees the provided packet, and generates a NAK indicating the specifed error while sending */ @@ -250,6 +275,13 @@ class Router : protected concurrency::OSThread, protected PacketHistory deferred.queuedAtMs = millis() - deferredDmKeyWaitMs; } } + void retryDeferredDmsForTest() + { + for (auto &deferred : deferredDms) { + if (deferred.p && deferred.reason == DeferredDm::Reason::PEER_KEY) + deferred.queuedAtMs = millis() - deferredDmPeerKeyWaitMs; + } + } #endif #endif }; diff --git a/src/modules/RoutingModule.cpp b/src/modules/RoutingModule.cpp index 1ce7c450254..501a74faa26 100644 --- a/src/modules/RoutingModule.cpp +++ b/src/modules/RoutingModule.cpp @@ -30,6 +30,9 @@ bool RoutingModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, mesh printPacket("Routing sniffing", &mp); router->sniffReceived(&mp, r); + if (router->shouldSuppressRoutingDelivery(mp)) + return false; + // FIXME - move this to a non promsicious PhoneAPI module? // Note: we are careful not to send back packets that started with the phone back to the phone if ((isBroadcast(mp.to) || isToUs(&mp)) && (mp.from != 0)) { @@ -93,4 +96,4 @@ RoutingModule::RoutingModule() : ProtobufModule("routing", meshtastic_PortNum_RO // LocalOnly requires either the from or to to be a known node // knownOnly specifically requires the from to be a known node. encryptedOk = true; -} \ No newline at end of file +} diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 44819f17ecd..907a95eccdd 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -159,6 +159,7 @@ class AuthPipelineRouter : public ReliableRouter void remember(const meshtastic_MeshPacket *p) { wasSeenRecently(p, true); } void forgetRelayer(uint8_t relay, PacketId id, NodeNum from) { removeRelayer(relay, id, from); } bool handleUpgrade(meshtastic_MeshPacket *p) { return perhapsHandleUpgradedPacket(p); } + void sniff(const meshtastic_MeshPacket *p, const meshtastic_Routing *c) { ReliableRouter::sniffReceived(p, c); } void addPending(const meshtastic_MeshPacket &p, uint32_t nextTx) { auto *copy = packetPool.allocCopy(p); @@ -1437,6 +1438,136 @@ void test_M2_unknown_dm_fails_only_after_key_exchange_timeout(void) #endif } +void test_M3_undecryptable_dm_reports_key_state_not_no_channel(void) +{ + meshtastic_MeshPacket dm = meshtastic_MeshPacket_init_zero; + dm.from = REMOTE_NODE; + dm.to = LOCAL_NODE; + dm.id = 0xD00D0003; + dm.channel = 0; + dm.want_ack = true; + dm.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; + dm.encrypted.size = MESHTASTIC_PKC_OVERHEAD + 1; + + pipelineRouter->sniff(&dm, nullptr); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY, pipelineRouting->lastAckError); + TEST_ASSERT_EQUAL_HEX32(dm.id, pipelineRouting->lastAckId); + + uint8_t staleKey[32], unusedPrivate[32]; + crypto->generateKeyPair(staleKey, unusedPrivate); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, staleKey); + + pipelineRouter->sniff(&dm, nullptr); + TEST_ASSERT_EQUAL_MESSAGE(meshtastic_Routing_Error_PKI_FAILED, pipelineRouting->lastAckError, + "a stored-but-wrong key is not a channel failure"); + TEST_ASSERT_EQUAL_HEX32(dm.id, pipelineRouting->lastAckId); + crypto->setDHPrivateKey(config.security.private_key.bytes); +} + +void test_M4_peer_missing_our_key_retries_original_dm_after_nodeinfo(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePublic); + + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + dm->want_ack = true; + const PacketId originalDmId = dm->id; + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + + meshtastic_MeshPacket nak = meshtastic_MeshPacket_init_zero; + nak.from = REMOTE_NODE; + nak.to = LOCAL_NODE; + nak.id = 0xD00D0004; + nak.channel = 0; + nak.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + nak.decoded.portnum = meshtastic_PortNum_ROUTING_APP; + nak.decoded.request_id = originalDmId; + meshtastic_Routing routing = meshtastic_Routing_init_zero; + routing.error_reason = meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY; + + pipelineRouter->sniff(&nak, &routing); + TEST_ASSERT_EQUAL(0, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_TRUE_MESSAGE(pipelineRouter->shouldSuppressRoutingDelivery(nak), "interim key-exchange NAK stays off the client"); + TEST_ASSERT_FALSE(pipelineRouter->shouldSuppressRoutingDelivery(nak)); + TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); + meshtastic_MeshPacket nodeInfo = pipelineRadio->sentPackets.back(); + TEST_ASSERT_EQUAL(DECODE_SUCCESS, perhapsDecode(&nodeInfo)); + TEST_ASSERT_EQUAL(meshtastic_PortNum_NODEINFO_APP, nodeInfo.decoded.portnum); + + bool sawKeyExchange = false; + while (meshtastic_QueueStatus *status = pipelineService->getQueueStatusForPhone()) { + sawKeyExchange |= status->mesh_packet_id == originalDmId && status->state == meshtastic_QueueStatus_State_KEY_EXCHANGE; + pipelineService->releaseQueueStatusToPool(status); + } + TEST_ASSERT_TRUE_MESSAGE(sawKeyExchange, "client receives key exchange state for the original DM"); + + pipelineRouter->sniff(&nak, &routing); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_TRUE_MESSAGE(pipelineRouter->shouldSuppressRoutingDelivery(nak), + "duplicate key-exchange NAK stays off the client while the exchange is pending"); + TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); + + pipelineRouter->retryDeferredDmsForTest(); + pipelineRouter->processDeferredDmsForTest(); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRadio->sentPackets.back().id); + TEST_ASSERT_TRUE(pipelineRadio->sentPackets.back().pki_encrypted); + + pipelineRouter->sniff(&nak, &routing); + TEST_ASSERT_EQUAL(0, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_FALSE_MESSAGE(pipelineRouter->shouldSuppressRoutingDelivery(nak), + "a second key-missing NAK is surfaced after the one retry"); + TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); +} + +void test_M5_peer_key_mismatch_does_not_auto_retry(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePublic); + + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + dm->want_ack = true; + const PacketId originalDmId = dm->id; + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); + + meshtastic_MeshPacket nak = meshtastic_MeshPacket_init_zero; + nak.from = REMOTE_NODE; + nak.to = LOCAL_NODE; + nak.id = 0xD00D0005; + nak.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + nak.decoded.portnum = meshtastic_PortNum_ROUTING_APP; + nak.decoded.request_id = originalDmId; + meshtastic_Routing routing = meshtastic_Routing_init_zero; + routing.error_reason = meshtastic_Routing_Error_PKI_FAILED; + + pipelineRouter->sniff(&nak, &routing); + TEST_ASSERT_EQUAL(0, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_FALSE(pipelineRouter->shouldSuppressRoutingDelivery(nak)); +} + // 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_N5_unsigned_unicast_nodeinfo_from_signer_does_not_change_name(void) @@ -1802,6 +1933,9 @@ void setup() printf("\n=== Group M: deferred DM key exchange ===\n"); RUN_TEST(test_M1_unknown_dm_waits_for_nodeinfo_key_exchange_then_retries); RUN_TEST(test_M2_unknown_dm_fails_only_after_key_exchange_timeout); + RUN_TEST(test_M3_undecryptable_dm_reports_key_state_not_no_channel); + RUN_TEST(test_M4_peer_missing_our_key_retries_original_dm_after_nodeinfo); + RUN_TEST(test_M5_peer_key_mismatch_does_not_auto_retry); printf("\n=== Group N: NodeInfoModule authentication ===\n"); RUN_TEST(test_N1_unsigned_nodeinfo_from_signer_dropped); RUN_TEST(test_N2_signed_nodeinfo_from_signer_not_dropped); From d70f26757a2f0d18417f16dd46fda47d1726b3af Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sat, 25 Jul 2026 20:45:22 -0700 Subject: [PATCH 03/15] fix: retry deferred DMs on NodeInfo response --- src/mesh/Router.cpp | 48 +++++++++- src/mesh/Router.h | 9 ++ src/modules/NodeInfoModule.cpp | 12 ++- src/modules/NodeInfoModule.h | 6 +- test/test_packet_signing/test_main.cpp | 128 +++++++++++++++++++++++-- 5 files changed, 187 insertions(+), 16 deletions(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 4410257ebf1..f4c8dcc7813 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -356,6 +356,39 @@ bool Router::shouldSuppressRoutingDelivery(const meshtastic_MeshPacket &p) return false; } +bool Router::retryDeferredDmOnNodeInfo(const meshtastic_MeshPacket &p) +{ +#if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO + if (!isToUs(&p) || p.which_payload_variant != meshtastic_MeshPacket_decoded_tag || + p.decoded.portnum != meshtastic_PortNum_NODEINFO_APP || !p.decoded.request_id) + return false; + + for (auto &deferred : deferredDms) { + if (!deferred.p || deferred.p->to != p.from || deferred.keyExchangeId != p.decoded.request_id) + continue; + + meshtastic_MeshPacket *dm = deferred.p; + if (deferred.reason == DeferredDm::Reason::DESTINATION_KEY) { + meshtastic_NodeInfoLite_public_key_t remoteKey = {0, {0}}; + if (!nodeDB->copyPublicKey(dm->to, remoteKey)) + return false; + } + deferred.p = nullptr; + deferred.queuedAtMs = 0; + deferred.keyExchangeId = 0; + LOG_INFO("NodeInfo exchange with 0x%08x completed; retrying deferred DM id=0x%08x", p.from, dm->id); + if (deferred.reason == DeferredDm::Reason::PEER_KEY) + rememberPeerKeyRetry(dm->to, dm->id); + service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, dm->id, meshtastic_QueueStatus_State_STATE_UNSPECIFIED); + send(dm); + return true; + } +#else + (void)p; +#endif + return false; +} + ErrorCode Router::sendLocal(meshtastic_MeshPacket *p, RxSource src) { if (p->to == 0) { @@ -1289,11 +1322,15 @@ bool Router::deferMissingKeyDm(meshtastic_MeshPacket *p) if (deferred.p) continue; + const PacketId keyExchangeId = nodeInfoModule->requestNodeInfo(p->to, p->channel); + if (!keyExchangeId) + return false; + deferred.p = p; deferred.queuedAtMs = millis(); + deferred.keyExchangeId = keyExchangeId; deferred.reason = DeferredDm::Reason::DESTINATION_KEY; LOG_INFO("Deferring DM id=0x%08x to 0x%08x while requesting NodeInfo", p->id, p->to); - nodeInfoModule->requestNodeInfo(p->to, p->channel); setInterval(0); runASAP = true; return true; @@ -1313,11 +1350,15 @@ bool Router::deferPeerKeyDm(meshtastic_MeshPacket *p) if (deferred.p) continue; + const PacketId keyExchangeId = nodeInfoModule->sendOurNodeInfo(p->to, false, p->channel, true, true); + if (!keyExchangeId) + return false; + deferred.p = p; deferred.queuedAtMs = millis(); + deferred.keyExchangeId = keyExchangeId; deferred.reason = DeferredDm::Reason::PEER_KEY; LOG_INFO("Deferring DM id=0x%08x while peer 0x%08x learns our NodeInfo", p->id, p->to); - nodeInfoModule->sendOurNodeInfo(p->to, false, p->channel, true, true); service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, p->id, meshtastic_QueueStatus_State_KEY_EXCHANGE); setInterval(0); runASAP = true; @@ -1379,6 +1420,7 @@ void Router::processDeferredDms() if (!Throttle::isWithinTimespanMs(deferred.queuedAtMs, deferredDmPeerKeyWaitMs)) { deferred.p = nullptr; deferred.queuedAtMs = 0; + deferred.keyExchangeId = 0; LOG_INFO("Retrying deferred DM id=0x%08x after sharing our NodeInfo with 0x%08x", p->id, p->to); rememberPeerKeyRetry(p->to, p->id); service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, p->id, @@ -1392,12 +1434,14 @@ void Router::processDeferredDms() if (nodeDB->copyPublicKey(p->to, remoteKey)) { deferred.p = nullptr; deferred.queuedAtMs = 0; + deferred.keyExchangeId = 0; LOG_INFO("Peer key learned for 0x%08x; retrying deferred DM id=0x%08x", p->to, p->id); service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, p->id, meshtastic_QueueStatus_State_STATE_UNSPECIFIED); send(p); } else if (!Throttle::isWithinTimespanMs(deferred.queuedAtMs, deferredDmKeyWaitMs)) { deferred.p = nullptr; deferred.queuedAtMs = 0; + deferred.keyExchangeId = 0; LOG_WARN("No public key learned for 0x%08x before deferred DM id=0x%08x timed out", p->to, p->id); abortSendAndNak(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, p); } diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 967703b0a4e..5824959f092 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -77,6 +77,9 @@ class Router : protected concurrency::OSThread, protected PacketHistory /// Consume a routing packet that firmware handled as an internal DM key exchange step. bool shouldSuppressRoutingDelivery(const meshtastic_MeshPacket &p); + /// Retry a deferred DM early when its directed NodeInfo exchange completes. + bool retryDeferredDmOnNodeInfo(const meshtastic_MeshPacket &p); + /** * @return our local nodenum */ [[nodiscard]] NodeNum getNodeNum(); @@ -228,6 +231,7 @@ class Router : protected concurrency::OSThread, protected PacketHistory meshtastic_MeshPacket *p = nullptr; uint32_t queuedAtMs = 0; + PacketId keyExchangeId = 0; Reason reason = Reason::DESTINATION_KEY; }; @@ -282,6 +286,11 @@ class Router : protected concurrency::OSThread, protected PacketHistory deferred.queuedAtMs = millis() - deferredDmPeerKeyWaitMs; } } + void resetPeerKeyRetriesForTest() + { + for (auto &retry : peerKeyRetries) + retry = {}; + } #endif #endif }; diff --git a/src/modules/NodeInfoModule.cpp b/src/modules/NodeInfoModule.cpp index 9b63c667453..a8a74046c4e 100644 --- a/src/modules/NodeInfoModule.cpp +++ b/src/modules/NodeInfoModule.cpp @@ -65,6 +65,9 @@ bool NodeInfoModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, mes // 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); + if (router) + router->retryDeferredDmOnNodeInfo(mp); + bool wasBroadcast = isBroadcast(mp.to); // LOG_DEBUG("did encode"); @@ -96,7 +99,7 @@ void NodeInfoModule::alterReceivedProtobuf(meshtastic_MeshPacket &mp, meshtastic pb_encode_to_bytes(mp.decoded.payload.bytes, sizeof(mp.decoded.payload.bytes), &meshtastic_User_msg, p); } -void NodeInfoModule::sendOurNodeInfo(NodeNum dest, bool wantReplies, uint8_t channel, bool _shorterTimeout, bool _force) +PacketId NodeInfoModule::sendOurNodeInfo(NodeNum dest, bool wantReplies, uint8_t channel, bool _shorterTimeout, bool _force) { // cancel any not yet sent (now stale) position packets if (prevPacketId) // if we wrap around to zero, we'll simply fail to cancel in that rare case (no big deal) @@ -128,12 +131,15 @@ void NodeInfoModule::sendOurNodeInfo(NodeNum dest, bool wantReplies, uint8_t cha prevPacketId = p->id; service->sendToMesh(p, RX_SRC_LOCAL, false, false); + return prevPacketId; } + + return 0; } -void NodeInfoModule::requestNodeInfo(NodeNum dest, uint8_t channel) +PacketId NodeInfoModule::requestNodeInfo(NodeNum dest, uint8_t channel) { - sendOurNodeInfo(dest, true, channel, true, true); + return sendOurNodeInfo(dest, true, channel, true, true); } void NodeInfoModule::triggerImmediateNodeInfoCheck() diff --git a/src/modules/NodeInfoModule.h b/src/modules/NodeInfoModule.h index 1b99a68fd60..cc67fc5cc57 100644 --- a/src/modules/NodeInfoModule.h +++ b/src/modules/NodeInfoModule.h @@ -21,12 +21,12 @@ class NodeInfoModule : public ProtobufModule, private concurren /** * Send our NodeInfo into the mesh */ - void sendOurNodeInfo(NodeNum dest = NODENUM_BROADCAST, bool wantReplies = false, uint8_t channel = 0, - bool _shorterTimeout = false, bool _force = false); + PacketId sendOurNodeInfo(NodeNum dest = NODENUM_BROADCAST, bool wantReplies = false, uint8_t channel = 0, + bool _shorterTimeout = false, bool _force = false); /// Send a directed NodeInfo request even when the regular announcement throttle is active. /// Router uses this to recover a missing direct-message public key without exposing the DM. - void requestNodeInfo(NodeNum dest, uint8_t channel); + PacketId requestNodeInfo(NodeNum dest, uint8_t channel); /** * Schedule an immediate NodeInfo periodic check. diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 907a95eccdd..ec62ca0b9d5 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -225,7 +225,8 @@ static AuthPipelineRoutingModule *pipelineRouting = nullptr; static AuthPipelineModule *pipelineModule = nullptr; static AuthPipelineMqtt *pipelineMqtt = nullptr; static MeshService *pipelineService = nullptr; -static NodeInfoModule *dmKeyWaitNodeInfo = nullptr; +class NodeInfoTestShim; +static NodeInfoTestShim *dmKeyWaitNodeInfo = nullptr; static AirTime *dmKeyWaitAirTime = nullptr; #if ARCH_PORTDUINO static bool dmKeyWaitOriginalForceSimRadio = false; @@ -414,6 +415,7 @@ void setUp(void) channels.onConfigChanged(); pipelineRouter->clearPending(); + pipelineRouter->resetPeerKeyRetriesForTest(); pipelineRouter->rxDupe = 0; pipelineRouter->txRelayCanceled = 0; pipelineRadio->reset(); @@ -957,6 +959,11 @@ class NodeInfoTestShim : public NodeInfoModule { public: using NodeInfoModule::handleReceivedProtobuf; + + bool rejectReplies = false; + + protected: + meshtastic_MeshPacket *allocReply() override { return rejectReplies ? nullptr : NodeInfoModule::allocReply(); } }; static meshtastic_MeshPacket makeNodeInfoPacket(bool signed_) @@ -1361,7 +1368,8 @@ static void enablePkiForLocalNode() static void enableNodeInfoForDmKeyWait() { if (!dmKeyWaitNodeInfo) - dmKeyWaitNodeInfo = new NodeInfoModule(); + dmKeyWaitNodeInfo = new NodeInfoTestShim(); + dmKeyWaitNodeInfo->rejectReplies = false; if (!dmKeyWaitAirTime) dmKeyWaitAirTime = new AirTime(); nodeInfoModule = dmKeyWaitNodeInfo; @@ -1399,10 +1407,13 @@ void test_M1_unknown_dm_waits_for_nodeinfo_key_exchange_then_retries(void) uint8_t remotePublic[32], remotePrivate[32]; crypto->generateKeyPair(remotePublic, remotePrivate); crypto->setDHPrivateKey(config.security.private_key.bytes); - mockNodeDB->addNode(REMOTE_NODE); - mockNodeDB->setPublicKey(REMOTE_NODE, remotePublic); - - pipelineRouter->processDeferredDmsForTest(); + meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + nodeInfoResponse.decoded.request_id = nodeInfoRequest.id; + meshtastic_User responseUser = meshtastic_User_init_zero; + responseUser.is_licensed = owner.is_licensed; + responseUser.public_key.size = sizeof(remotePublic); + memcpy(responseUser.public_key.bytes, remotePublic, sizeof(remotePublic)); + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); TEST_ASSERT_TRUE_MESSAGE(pipelineRadio->sentPackets.back().pki_encrypted, @@ -1518,8 +1529,18 @@ void test_M4_peer_missing_our_key_retries_original_dm_after_nodeinfo(void) "duplicate key-exchange NAK stays off the client while the exchange is pending"); TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); - pipelineRouter->retryDeferredDmsForTest(); - pipelineRouter->processDeferredDmsForTest(); + meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + nodeInfoResponse.decoded.request_id = nodeInfo.id + 1; + meshtastic_User responseUser = meshtastic_User_init_zero; + responseUser.is_licensed = owner.is_licensed; + responseUser.public_key.size = sizeof(remotePublic); + memcpy(responseUser.public_key.bytes, remotePublic, sizeof(remotePublic)); + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); + + nodeInfoResponse.decoded.request_id = nodeInfo.id; + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); @@ -1568,6 +1589,94 @@ void test_M5_peer_key_mismatch_does_not_auto_retry(void) TEST_ASSERT_FALSE(pipelineRouter->shouldSuppressRoutingDelivery(nak)); } +void test_M6_peer_missing_our_key_retries_after_nodeinfo_response_timeout(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePublic); + + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + dm->want_ack = true; + const PacketId originalDmId = dm->id; + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + + meshtastic_MeshPacket nak = meshtastic_MeshPacket_init_zero; + nak.from = REMOTE_NODE; + nak.to = LOCAL_NODE; + nak.id = 0xD00D0006; + nak.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + nak.decoded.portnum = meshtastic_PortNum_ROUTING_APP; + nak.decoded.request_id = originalDmId; + meshtastic_Routing routing = meshtastic_Routing_init_zero; + routing.error_reason = meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY; + + pipelineRouter->sniff(&nak, &routing); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); + + pipelineRouter->retryDeferredDmsForTest(); + pipelineRouter->processDeferredDmsForTest(); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRadio->sentPackets.back().id); +} + +void test_M7_missing_recipient_key_fails_when_nodeinfo_cannot_queue(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + dmKeyWaitNodeInfo->rejectReplies = true; + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + dm->want_ack = true; + const PacketId originalDmId = dm->id; + + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(0, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL(1, pipelineRouting->ackCalls); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, pipelineRouting->lastAckError); + TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRouting->lastAckId); +} + +void test_M8_nodeinfo_reply_without_key_keeps_dm_deferred(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + dm->want_ack = true; + const PacketId originalDmId = dm->id; + + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + meshtastic_MeshPacket nodeInfoRequest = pipelineRadio->sentPackets.back(); + + meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + nodeInfoResponse.decoded.request_id = nodeInfoRequest.id; + meshtastic_User responseUser = meshtastic_User_init_zero; + responseUser.is_licensed = owner.is_licensed; + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + + pipelineRouter->expireDeferredDmsForTest(); + pipelineRouter->processDeferredDmsForTest(); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRouting->ackCalls); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, pipelineRouting->lastAckError); + TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRouting->lastAckId); +} + // 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_N5_unsigned_unicast_nodeinfo_from_signer_does_not_change_name(void) @@ -1936,6 +2045,9 @@ void setup() RUN_TEST(test_M3_undecryptable_dm_reports_key_state_not_no_channel); RUN_TEST(test_M4_peer_missing_our_key_retries_original_dm_after_nodeinfo); RUN_TEST(test_M5_peer_key_mismatch_does_not_auto_retry); + RUN_TEST(test_M6_peer_missing_our_key_retries_after_nodeinfo_response_timeout); + RUN_TEST(test_M7_missing_recipient_key_fails_when_nodeinfo_cannot_queue); + RUN_TEST(test_M8_nodeinfo_reply_without_key_keeps_dm_deferred); printf("\n=== Group N: NodeInfoModule authentication ===\n"); RUN_TEST(test_N1_unsigned_nodeinfo_from_signer_dropped); RUN_TEST(test_N2_signed_nodeinfo_from_signer_not_dropped); From ea09bbd5c4babfc4e39dba7692babc78ad4ee3b1 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:11:54 -0700 Subject: [PATCH 04/15] fix: harden deferred DM key exchanges --- src/mesh/MeshService.cpp | 4 +- src/mesh/MeshService.h | 3 +- src/mesh/ReliableRouter.cpp | 7 +- src/mesh/Router.cpp | 16 ++-- src/mesh/Router.h | 10 +- src/modules/NodeInfoModule.cpp | 12 ++- test/test_packet_signing/test_main.cpp | 121 ++++++++++++++++++++++++- 7 files changed, 151 insertions(+), 22 deletions(-) diff --git a/src/mesh/MeshService.cpp b/src/mesh/MeshService.cpp index 5e501694dbb..b00ef78e881 100644 --- a/src/mesh/MeshService.cpp +++ b/src/mesh/MeshService.cpp @@ -321,7 +321,7 @@ ErrorCode MeshService::sendQueueStatusToPhone(const meshtastic_QueueStatus &qs, return res ? ERRNO_OK : ERRNO_UNKNOWN; } -void MeshService::sendToMesh(meshtastic_MeshPacket *p, RxSource src, bool ccToPhone, bool reportQueueStatus) +ErrorCode MeshService::sendToMesh(meshtastic_MeshPacket *p, RxSource src, bool ccToPhone, bool reportQueueStatus) { uint32_t mesh_packet_id = p->id; nodeDB->updateFrom(*p); // update our local DB for this packet (because phone might have sent position packets etc...) @@ -362,6 +362,8 @@ void MeshService::sendToMesh(meshtastic_MeshPacket *p, RxSource src, bool ccToPh if (res == ERRNO_SHOULD_RELEASE) { releaseToPool(p); } + + return res; } bool MeshService::trySendPosition(NodeNum dest, bool wantReplies) diff --git a/src/mesh/MeshService.h b/src/mesh/MeshService.h index de92b6f8ef8..895e5715a3c 100644 --- a/src/mesh/MeshService.h +++ b/src/mesh/MeshService.h @@ -181,7 +181,8 @@ class MeshService /// Send a packet into the mesh - note p must have been allocated from packetPool. We will return it to that pool after /// sending. This is the ONLY function you should use for sending messages into the mesh, because it also updates the nodedb /// cache - void sendToMesh(meshtastic_MeshPacket *p, RxSource src = RX_SRC_LOCAL, bool ccToPhone = false, bool reportQueueStatus = true); + ErrorCode sendToMesh(meshtastic_MeshPacket *p, RxSource src = RX_SRC_LOCAL, bool ccToPhone = false, + bool reportQueueStatus = true); /** Attempt to cancel a previously sent packet from this _local_ node. Returns true if a packet was found we could cancel */ bool cancelSending(PacketId id); diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp index 9cd36ad0e59..4cc7a4b801e 100644 --- a/src/mesh/ReliableRouter.cpp +++ b/src/mesh/ReliableRouter.cpp @@ -19,8 +19,13 @@ ErrorCode ReliableRouter::send(meshtastic_MeshPacket *p) #if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO // Router owns the delayed packet while it asks for the destination's NodeInfo. Do this before // allocating a retransmission copy, otherwise the stale copy can later emit MAX_RETRANSMIT. - if (deferMissingKeyDm(p)) + const auto deferredDm = deferMissingKeyDm(p); + if (deferredDm == DeferredDmResult::DEFERRED) return ERRNO_OK; + if (deferredDm == DeferredDmResult::FAILED) { + abortSendAndNak(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, p); + return meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY; + } #endif if (p->want_ack) { diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index f4c8dcc7813..eb6ced834f8 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -547,7 +547,8 @@ ErrorCode Router::send(meshtastic_MeshPacket *p) if (encodeResult != meshtastic_Routing_Error_NONE) { packetPool.release(p_decoded); #if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO - if (encodeResult == meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY && deferMissingKeyDm(p)) + if (encodeResult == meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY && + deferMissingKeyDm(p) == DeferredDmResult::DEFERRED) return ERRNO_OK; #endif p->channel = 0; // Reset the channel to 0, so we don't use the failing hash again @@ -1308,15 +1309,15 @@ uint8_t Router::deferredDmCount() const return count; } -bool Router::deferMissingKeyDm(meshtastic_MeshPacket *p) +Router::DeferredDmResult Router::deferMissingKeyDm(meshtastic_MeshPacket *p) { if (!nodeInfoModule || p->which_payload_variant != meshtastic_MeshPacket_decoded_tag || !IS_ONE_OF(p->decoded.portnum, meshtastic_PortNum_TEXT_MESSAGE_APP, meshtastic_PortNum_TEXT_MESSAGE_COMPRESSED_APP)) - return false; + return DeferredDmResult::NOT_APPLICABLE; meshtastic_NodeInfoLite_public_key_t remoteKey = {0, {0}}; if (nodeDB->copyPublicKey(p->to, remoteKey) || !wouldEncryptWithPKC(p, p->channel, false)) - return false; + return DeferredDmResult::NOT_APPLICABLE; for (auto &deferred : deferredDms) { if (deferred.p) @@ -1324,7 +1325,7 @@ bool Router::deferMissingKeyDm(meshtastic_MeshPacket *p) const PacketId keyExchangeId = nodeInfoModule->requestNodeInfo(p->to, p->channel); if (!keyExchangeId) - return false; + return DeferredDmResult::FAILED; deferred.p = p; deferred.queuedAtMs = millis(); @@ -1333,11 +1334,11 @@ bool Router::deferMissingKeyDm(meshtastic_MeshPacket *p) LOG_INFO("Deferring DM id=0x%08x to 0x%08x while requesting NodeInfo", p->id, p->to); setInterval(0); runASAP = true; - return true; + return DeferredDmResult::DEFERRED; } LOG_WARN("Deferred DM queue is full; cannot wait for public key of 0x%08x", p->to); - return false; + return DeferredDmResult::FAILED; } bool Router::deferPeerKeyDm(meshtastic_MeshPacket *p) @@ -1717,7 +1718,6 @@ void Router::perhapsHandleReceived(meshtastic_MeshPacket *p) packetPool.release(p); return; } - if (shouldFilterReceived(p)) { clearRoutingAuthCache(); LOG_DEBUG("Incoming msg was filtered from 0x%08x", p->from); diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 5824959f092..3a2b5bf491d 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -112,9 +112,11 @@ class Router : protected concurrency::OSThread, protected PacketHistory friend class RoutingModule; #if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO + enum class DeferredDmResult : uint8_t { NOT_APPLICABLE, DEFERRED, FAILED }; + /// Takes ownership when a local text DM needs a public-key exchange before it can be sent. /// Derived routers must call this before creating retransmission state for the packet. - bool deferMissingKeyDm(meshtastic_MeshPacket *p); + DeferredDmResult deferMissingKeyDm(meshtastic_MeshPacket *p); bool deferPeerKeyDm(meshtastic_MeshPacket *p); bool isWaitingForPeerKeyDm(NodeNum peer, PacketId id) const; bool hasRetriedPeerKeyDm(NodeNum peer, PacketId id); @@ -159,6 +161,9 @@ class Router : protected concurrency::OSThread, protected PacketHistory void sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopLimit = 0, bool ackWantsAck = false); + /** Frees the provided packet, and generates a NAK indicating the specifed error while sending */ + void abortSendAndNak(meshtastic_Routing_Error err, meshtastic_MeshPacket *p); + private: /** * Called from loop() @@ -257,9 +262,6 @@ class Router : protected concurrency::OSThread, protected PacketHistory } suppressedRoutingDelivery; #endif - /** Frees the provided packet, and generates a NAK indicating the specifed error while sending */ - void abortSendAndNak(meshtastic_Routing_Error err, meshtastic_MeshPacket *p); - #ifdef PIO_UNIT_TESTING public: /// High-water mark of handleDepth across this Router's life. The deferral must keep it at 1: diff --git a/src/modules/NodeInfoModule.cpp b/src/modules/NodeInfoModule.cpp index a8a74046c4e..8168293bc9d 100644 --- a/src/modules/NodeInfoModule.cpp +++ b/src/modules/NodeInfoModule.cpp @@ -101,8 +101,9 @@ void NodeInfoModule::alterReceivedProtobuf(meshtastic_MeshPacket &mp, meshtastic PacketId NodeInfoModule::sendOurNodeInfo(NodeNum dest, bool wantReplies, uint8_t channel, bool _shorterTimeout, bool _force) { - // cancel any not yet sent (now stale) position packets - if (prevPacketId) // if we wrap around to zero, we'll simply fail to cancel in that rare case (no big deal) + // Periodic broadcasts replace stale periodic broadcasts. Directed exchanges must stay independent. + const bool replacePrevious = isBroadcast(dest); + if (replacePrevious && prevPacketId) // if we wrap around to zero, we'll simply fail to cancel in that rare case (no big deal) service->cancelSending(prevPacketId); shorterTimeout = _shorterTimeout; forceSend = _force; @@ -128,10 +129,11 @@ PacketId NodeInfoModule::sendOurNodeInfo(NodeNum dest, bool wantReplies, uint8_t p->channel = channel; } - prevPacketId = p->id; + const PacketId packetId = p->id; + if (replacePrevious) + prevPacketId = packetId; - service->sendToMesh(p, RX_SRC_LOCAL, false, false); - return prevPacketId; + return service->sendToMesh(p, RX_SRC_LOCAL, false, false) == ERRNO_OK ? packetId : 0; } return 0; diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index ec62ca0b9d5..ba1df044d72 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -46,6 +46,7 @@ // --------------------------------------------------------------------------- static constexpr NodeNum LOCAL_NODE = 0x0A0A0A0A; static constexpr NodeNum REMOTE_NODE = 0x0B0B0B0B; +static constexpr NodeNum SECOND_REMOTE_NODE = 0x0C0C0C0C; // A "small" broadcast payload whose signed encoding easily fits a LoRa frame, and an "oversized" // one whose signed encoding does not, yet still encodes within a LoRa frame unsigned. @@ -92,6 +93,13 @@ class MockNodeDB : public NodeDB nodeInfoLiteSetBit(n, NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK, value); } + void setHasUser(NodeNum num) + { + meshtastic_NodeInfoLite *n = getMeshNode(num); + TEST_ASSERT_NOT_NULL(n); + nodeInfoLiteSetBit(n, NODEINFO_BITFIELD_HAS_USER_MASK, true); + } + void setLongName(NodeNum num, const char *name) { meshtastic_NodeInfoLite *n = getMeshNode(num); @@ -120,7 +128,7 @@ class AuthPipelineRadio : public RadioInterface sendCalls++; sentPackets.push_back(*p); packetPool.release(p); - return ERRNO_OK; + return sendResult; } bool cancelSending(NodeNum, PacketId) override { @@ -141,6 +149,7 @@ class AuthPipelineRadio : public RadioInterface void reset() { sendCalls = cancelCalls = findCalls = removeCalls = 0; + sendResult = ERRNO_OK; sentPackets.clear(); } @@ -148,6 +157,7 @@ class AuthPipelineRadio : public RadioInterface uint32_t cancelCalls = 0; uint32_t findCalls = 0; uint32_t removeCalls = 0; + ErrorCode sendResult = ERRNO_OK; std::vector sentPackets; }; @@ -1459,6 +1469,8 @@ void test_M3_undecryptable_dm_reports_key_state_not_no_channel(void) dm.want_ack = true; dm.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; dm.encrypted.size = MESHTASTIC_PKC_OVERHEAD + 1; + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setHasUser(REMOTE_NODE); pipelineRouter->sniff(&dm, nullptr); TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY, pipelineRouting->lastAckError); @@ -1466,7 +1478,6 @@ void test_M3_undecryptable_dm_reports_key_state_not_no_channel(void) uint8_t staleKey[32], unusedPrivate[32]; crypto->generateKeyPair(staleKey, unusedPrivate); - mockNodeDB->addNode(REMOTE_NODE); mockNodeDB->setPublicKey(REMOTE_NODE, staleKey); pipelineRouter->sniff(&dm, nullptr); @@ -1677,6 +1688,108 @@ void test_M8_nodeinfo_reply_without_key_keeps_dm_deferred(void) TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRouting->lastAckId); } +void test_M9_missing_recipient_key_fails_when_nodeinfo_send_is_rejected(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + pipelineRadio->sendResult = meshtastic_Routing_Error_NO_INTERFACE; + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + dm->want_ack = true; + const PacketId originalDmId = dm->id; + + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL(1, pipelineRouting->ackCalls); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, pipelineRouting->lastAckError); + TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRouting->lastAckId); +} + +void test_M10_two_missing_keys_keep_independent_nodeinfo_requests(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + meshtastic_MeshPacket *first = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + meshtastic_MeshPacket *second = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, SECOND_REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(first); + TEST_ASSERT_NOT_NULL(second); + first->want_ack = true; + second->want_ack = true; + + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(first, RX_SRC_USER)); + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(second, RX_SRC_USER)); + TEST_ASSERT_EQUAL(2, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL(0, pipelineRadio->cancelCalls); + TEST_ASSERT_EQUAL(REMOTE_NODE, pipelineRadio->sentPackets[0].to); + TEST_ASSERT_EQUAL(SECOND_REMOTE_NODE, pipelineRadio->sentPackets[1].to); +} + +void test_M11_unknown_sender_pki_dm_reaches_terminal_nak_from_rf_ingress(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->setPublicKey(LOCAL_NODE, owner.public_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setHasUser(REMOTE_NODE); + + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + meshtastic_NodeInfoLite_public_key_t localKey = {sizeof(owner.public_key.bytes), {0}}; + memcpy(localKey.bytes, owner.public_key.bytes, sizeof(owner.public_key.bytes)); + + meshtastic_MeshPacket dm = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD); + dm.id = 0xD00D0011; + dm.want_ack = true; + uint8_t encoded[meshtastic_Constants_DATA_PAYLOAD_LEN] = {}; + const size_t encodedSize = pb_encode_to_bytes(encoded, sizeof(encoded), &meshtastic_Data_msg, &dm.decoded); + TEST_ASSERT_GREATER_THAN(0, encodedSize); + crypto->setDHPrivateKey(remotePrivate); + TEST_ASSERT_TRUE( + crypto->encryptCurve25519(LOCAL_NODE, REMOTE_NODE, localKey, dm.id, encodedSize, encoded, dm.encrypted.bytes)); + crypto->setDHPrivateKey(config.security.private_key.bytes); + dm.encrypted.size = encodedSize + MESHTASTIC_PKC_OVERHEAD; + dm.channel = 0; + dm.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; + + runPipelineIngress(dm); + TEST_ASSERT_EQUAL(0, pipelineRouting->ackCalls); + TEST_ASSERT_FALSE(pipelineRouter->historyContains(&dm)); + TEST_ASSERT_EQUAL(0, pipelineModule->calls); + + meshtastic_MeshPacket repeated = dm; + repeated.id++; + runPipelineIngress(repeated); + TEST_ASSERT_EQUAL(0, pipelineRouting->ackCalls); +} + +void test_M12_unknown_pki_shaped_radio_packet_does_not_trigger_nak(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->setPublicKey(LOCAL_NODE, owner.public_key.bytes); + + meshtastic_MeshPacket forged = meshtastic_MeshPacket_init_zero; + forged.from = REMOTE_NODE; + forged.to = LOCAL_NODE; + forged.id = 0xD00D0012; + forged.want_ack = true; + forged.channel = 0; + forged.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; + forged.encrypted.size = MESHTASTIC_PKC_OVERHEAD + 1; + + runPipelineIngress(forged); + TEST_ASSERT_EQUAL(0, pipelineRouting->ackCalls); + TEST_ASSERT_FALSE(pipelineRouter->historyContains(&forged)); + TEST_ASSERT_EQUAL(0, pipelineModule->calls); +} + // 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_N5_unsigned_unicast_nodeinfo_from_signer_does_not_change_name(void) @@ -2048,6 +2161,10 @@ void setup() RUN_TEST(test_M6_peer_missing_our_key_retries_after_nodeinfo_response_timeout); RUN_TEST(test_M7_missing_recipient_key_fails_when_nodeinfo_cannot_queue); RUN_TEST(test_M8_nodeinfo_reply_without_key_keeps_dm_deferred); + RUN_TEST(test_M9_missing_recipient_key_fails_when_nodeinfo_send_is_rejected); + RUN_TEST(test_M10_two_missing_keys_keep_independent_nodeinfo_requests); + RUN_TEST(test_M11_unknown_sender_pki_dm_reaches_terminal_nak_from_rf_ingress); + RUN_TEST(test_M12_unknown_pki_shaped_radio_packet_does_not_trigger_nak); printf("\n=== Group N: NodeInfoModule authentication ===\n"); RUN_TEST(test_N1_unsigned_nodeinfo_from_signer_dropped); RUN_TEST(test_N2_signed_nodeinfo_from_signer_not_dropped); From 9e424422575a513d0415ec51359594e3d9cbc7cb Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sat, 25 Jul 2026 22:27:45 -0700 Subject: [PATCH 05/15] fix: share direct message key exchanges --- src/mesh/ReliableRouter.cpp | 16 +- src/mesh/Router.cpp | 92 ++++++- src/mesh/Router.h | 26 +- test/test_packet_signing/test_main.cpp | 361 +++++++++++++++++++++---- 4 files changed, 416 insertions(+), 79 deletions(-) diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp index 4cc7a4b801e..3d04a52e5be 100644 --- a/src/mesh/ReliableRouter.cpp +++ b/src/mesh/ReliableRouter.cpp @@ -17,9 +17,17 @@ ErrorCode ReliableRouter::send(meshtastic_MeshPacket *p) { #if !MESHTASTIC_EXCLUDE_PKI && !MESHTASTIC_EXCLUDE_NODEINFO - // Router owns the delayed packet while it asks for the destination's NodeInfo. Do this before - // allocating a retransmission copy, otherwise the stale copy can later emit MAX_RETRANSMIT. - const auto deferredDm = deferMissingKeyDm(p); + // Router owns delayed DMs before creating retransmission state, otherwise a stale copy can + // later emit MAX_RETRANSMIT. First request that a peer refresh our NodeInfo, then recover a + // missing destination key when necessary. + auto deferredDm = deferPeerKeyDm(p, false); + if (deferredDm == DeferredDmResult::DEFERRED) + return ERRNO_OK; + if (deferredDm == DeferredDmResult::FAILED) { + abortSendAndNak(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, p); + return meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY; + } + deferredDm = deferMissingKeyDm(p); if (deferredDm == DeferredDmResult::DEFERRED) return ERRNO_OK; if (deferredDm == DeferredDmResult::FAILED) { @@ -116,7 +124,7 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas } else if (!(alreadyRetriedForPeerKey = hasRetriedPeerKeyDm(p->from, p->decoded.request_id))) { if (PendingPacket *pendingPacket = findPendingPacket(GlobalPacketId(p->to, p->decoded.request_id))) { meshtastic_MeshPacket *retry = packetPool.allocCopy(*pendingPacket->packet); - if (retry && deferPeerKeyDm(retry)) { + if (retry && deferPeerKeyDm(retry) == DeferredDmResult::DEFERRED) { stopRetransmission(p->to, p->decoded.request_id); suppressRoutingDelivery(*p); deferredForPeerKey = true; diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index eb6ced834f8..36ca66a8bc0 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -363,6 +363,7 @@ bool Router::retryDeferredDmOnNodeInfo(const meshtastic_MeshPacket &p) p.decoded.portnum != meshtastic_PortNum_NODEINFO_APP || !p.decoded.request_id) return false; + bool retried = false; for (auto &deferred : deferredDms) { if (!deferred.p || deferred.p->to != p.from || deferred.keyExchangeId != p.decoded.request_id) continue; @@ -371,18 +372,21 @@ bool Router::retryDeferredDmOnNodeInfo(const meshtastic_MeshPacket &p) if (deferred.reason == DeferredDm::Reason::DESTINATION_KEY) { meshtastic_NodeInfoLite_public_key_t remoteKey = {0, {0}}; if (!nodeDB->copyPublicKey(dm->to, remoteKey)) - return false; + continue; } deferred.p = nullptr; deferred.queuedAtMs = 0; deferred.keyExchangeId = 0; LOG_INFO("NodeInfo exchange with 0x%08x completed; retrying deferred DM id=0x%08x", p.from, dm->id); - if (deferred.reason == DeferredDm::Reason::PEER_KEY) + rememberPeerKeyExchangeAttempt(dm->to); + if (deferred.reason == DeferredDm::Reason::PEER_KEY) { rememberPeerKeyRetry(dm->to, dm->id); + } service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, dm->id, meshtastic_QueueStatus_State_STATE_UNSPECIFIED); send(dm); - return true; + retried = true; } + return retried; #else (void)p; #endif @@ -1341,33 +1345,55 @@ Router::DeferredDmResult Router::deferMissingKeyDm(meshtastic_MeshPacket *p) return DeferredDmResult::FAILED; } -bool Router::deferPeerKeyDm(meshtastic_MeshPacket *p) +Router::DeferredDmResult Router::deferPeerKeyDm(meshtastic_MeshPacket *p, bool reportQueueStatus) { if (!nodeInfoModule || p->which_payload_variant != meshtastic_MeshPacket_decoded_tag || !IS_ONE_OF(p->decoded.portnum, meshtastic_PortNum_TEXT_MESSAGE_APP, meshtastic_PortNum_TEXT_MESSAGE_COMPRESSED_APP)) - return false; + return DeferredDmResult::NOT_APPLICABLE; + + meshtastic_NodeInfoLite_public_key_t remoteKey = {0, {0}}; + if (!nodeDB->copyPublicKey(p->to, remoteKey) || !wouldEncryptWithPKC(p, p->channel, true) || + hasRetriedPeerKeyDm(p->to, p->id)) + return DeferredDmResult::NOT_APPLICABLE; + if (p->pki_encrypted && !memfll(p->public_key.bytes, 0, sizeof(p->public_key.bytes)) && + memcmp(p->public_key.bytes, remoteKey.bytes, sizeof(remoteKey.bytes)) != 0) + return DeferredDmResult::NOT_APPLICABLE; + + PacketId keyExchangeId = 0; + for (const auto &deferred : deferredDms) { + if (deferred.p && deferred.reason == DeferredDm::Reason::PEER_KEY && deferred.p->to == p->to) { + keyExchangeId = deferred.keyExchangeId; + break; + } + } + if (!keyExchangeId && hasPeerKeyExchangeAttempt(p->to)) + return DeferredDmResult::NOT_APPLICABLE; for (auto &deferred : deferredDms) { if (deferred.p) continue; - const PacketId keyExchangeId = nodeInfoModule->sendOurNodeInfo(p->to, false, p->channel, true, true); - if (!keyExchangeId) - return false; + if (!keyExchangeId) { + keyExchangeId = nodeInfoModule->sendOurNodeInfo(p->to, false, p->channel, true, true); + if (!keyExchangeId) + return DeferredDmResult::NOT_APPLICABLE; + rememberPeerKeyExchangeAttempt(p->to); + } deferred.p = p; deferred.queuedAtMs = millis(); deferred.keyExchangeId = keyExchangeId; deferred.reason = DeferredDm::Reason::PEER_KEY; - LOG_INFO("Deferring DM id=0x%08x while peer 0x%08x learns our NodeInfo", p->id, p->to); - service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, p->id, meshtastic_QueueStatus_State_KEY_EXCHANGE); + LOG_INFO("Deferring DM id=0x%08x while requesting NodeInfo exchange with peer 0x%08x", p->id, p->to); + if (reportQueueStatus) + service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, p->id, meshtastic_QueueStatus_State_KEY_EXCHANGE); setInterval(0); runASAP = true; - return true; + return DeferredDmResult::DEFERRED; } - LOG_WARN("Deferred DM queue is full; cannot wait for peer 0x%08x to learn our key", p->to); - return false; + LOG_DEBUG("Deferred DM queue is full; send to 0x%08x without preflight", p->to); + return DeferredDmResult::NOT_APPLICABLE; } bool Router::isWaitingForPeerKeyDm(NodeNum peer, PacketId id) const @@ -1405,6 +1431,43 @@ void Router::rememberPeerKeyRetry(NodeNum peer, PacketId id) *slot = {peer, id, static_cast(millis())}; } +bool Router::hasPeerKeyExchangeAttempt(NodeNum peer) +{ + meshtastic_NodeInfoLite_public_key_t remoteKey = {0, {0}}; + if (owner.public_key.size != 32 || !nodeDB->copyPublicKey(peer, remoteKey) || remoteKey.size != 32) + return false; + const uint32_t localKeyTag = crc32Buffer(owner.public_key.bytes, owner.public_key.size); + const uint32_t peerKeyTag = crc32Buffer(remoteKey.bytes, remoteKey.size); + for (auto &attempt : peerKeyExchangeAttempts) { + if (attempt.peer != peer) + continue; + if (attempt.localKeyTag == localKeyTag && attempt.peerKeyTag == peerKeyTag && + Throttle::isWithinTimespanMs(attempt.attemptedAtMs, peerKeyExchangeAttemptMs)) + return true; + attempt = {}; + return false; + } + return false; +} + +void Router::rememberPeerKeyExchangeAttempt(NodeNum peer) +{ + meshtastic_NodeInfoLite_public_key_t remoteKey = {0, {0}}; + if (owner.public_key.size != 32 || !nodeDB->copyPublicKey(peer, remoteKey) || remoteKey.size != 32) + return; + + PeerKeyExchangeAttempt *slot = &peerKeyExchangeAttempts[0]; + for (auto &attempt : peerKeyExchangeAttempts) { + if (attempt.peer == peer || attempt.peer == 0 || + !Throttle::isWithinTimespanMs(attempt.attemptedAtMs, peerKeyExchangeAttemptMs)) { + slot = &attempt; + break; + } + } + *slot = {peer, static_cast(millis()), crc32Buffer(owner.public_key.bytes, owner.public_key.size), + crc32Buffer(remoteKey.bytes, remoteKey.size)}; +} + void Router::suppressRoutingDelivery(const meshtastic_MeshPacket &p) { suppressedRoutingDelivery = {p.from, p.id, p.decoded.request_id}; @@ -1422,7 +1485,8 @@ void Router::processDeferredDms() deferred.p = nullptr; deferred.queuedAtMs = 0; deferred.keyExchangeId = 0; - LOG_INFO("Retrying deferred DM id=0x%08x after sharing our NodeInfo with 0x%08x", p->id, p->to); + LOG_INFO("Retrying deferred DM id=0x%08x after NodeInfo response wait for 0x%08x", p->id, p->to); + rememberPeerKeyExchangeAttempt(p->to); rememberPeerKeyRetry(p->to, p->id); service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, p->id, meshtastic_QueueStatus_State_STATE_UNSPECIFIED); diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 3a2b5bf491d..82b9b4acb35 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -117,10 +117,13 @@ class Router : protected concurrency::OSThread, protected PacketHistory /// Takes ownership when a local text DM needs a public-key exchange before it can be sent. /// Derived routers must call this before creating retransmission state for the packet. DeferredDmResult deferMissingKeyDm(meshtastic_MeshPacket *p); - bool deferPeerKeyDm(meshtastic_MeshPacket *p); + /// Takes ownership while requesting a NodeInfo exchange before a PKI DM. + DeferredDmResult deferPeerKeyDm(meshtastic_MeshPacket *p, bool reportQueueStatus = true); bool isWaitingForPeerKeyDm(NodeNum peer, PacketId id) const; bool hasRetriedPeerKeyDm(NodeNum peer, PacketId id); void rememberPeerKeyRetry(NodeNum peer, PacketId id); + bool hasPeerKeyExchangeAttempt(NodeNum peer); + void rememberPeerKeyExchangeAttempt(NodeNum peer); void suppressRoutingDelivery(const meshtastic_MeshPacket &p); #endif @@ -244,6 +247,7 @@ class Router : protected concurrency::OSThread, protected PacketHistory static constexpr uint32_t deferredDmKeyWaitMs = 30 * 1000UL; static constexpr uint32_t deferredDmPeerKeyWaitMs = 10 * 1000UL; static constexpr uint32_t peerKeyRetryMemoryMs = 30 * 1000UL; + static constexpr uint32_t peerKeyExchangeAttemptMs = 30 * 60 * 1000UL; DeferredDm deferredDms[deferredDmCapacity]; struct PeerKeyRetry { @@ -252,6 +256,13 @@ class Router : protected concurrency::OSThread, protected PacketHistory uint32_t retriedAtMs = 0; } peerKeyRetries[deferredDmCapacity]; + struct PeerKeyExchangeAttempt { + NodeNum peer = 0; + uint32_t attemptedAtMs = 0; + uint32_t localKeyTag = 0; + uint32_t peerKeyTag = 0; + } peerKeyExchangeAttempts[8]; + void processDeferredDms(); uint8_t deferredDmCount() const; @@ -288,11 +299,24 @@ class Router : protected concurrency::OSThread, protected PacketHistory deferred.queuedAtMs = millis() - deferredDmPeerKeyWaitMs; } } + void clearDeferredDmsForTest() + { + for (auto &deferred : deferredDms) { + if (deferred.p) + packetPool.release(deferred.p); + deferred = {}; + } + } void resetPeerKeyRetriesForTest() { for (auto &retry : peerKeyRetries) retry = {}; } + void resetPeerKeyExchangeAttemptsForTest() + { + for (auto &attempt : peerKeyExchangeAttempts) + attempt = {}; + } #endif #endif }; diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index ba1df044d72..7e60656306e 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -47,6 +47,7 @@ static constexpr NodeNum LOCAL_NODE = 0x0A0A0A0A; static constexpr NodeNum REMOTE_NODE = 0x0B0B0B0B; static constexpr NodeNum SECOND_REMOTE_NODE = 0x0C0C0C0C; +static constexpr NodeNum THIRD_REMOTE_NODE = 0x0D0D0D0D; // A "small" broadcast payload whose signed encoding easily fits a LoRa frame, and an "oversized" // one whose signed encoding does not, yet still encodes within a LoRa frame unsigned. @@ -425,7 +426,9 @@ void setUp(void) channels.onConfigChanged(); pipelineRouter->clearPending(); + pipelineRouter->clearDeferredDmsForTest(); pipelineRouter->resetPeerKeyRetriesForTest(); + pipelineRouter->resetPeerKeyExchangeAttemptsForTest(); pipelineRouter->rxDupe = 0; pipelineRouter->txRelayCanceled = 0; pipelineRadio->reset(); @@ -1384,6 +1387,8 @@ static void enableNodeInfoForDmKeyWait() dmKeyWaitAirTime = new AirTime(); nodeInfoModule = dmKeyWaitNodeInfo; airTime = dmKeyWaitAirTime; + pipelineRouter->resetPeerKeyRetriesForTest(); + pipelineRouter->resetPeerKeyExchangeAttemptsForTest(); } void test_M1_unknown_dm_waits_for_nodeinfo_key_exchange_then_retries(void) @@ -1487,7 +1492,7 @@ void test_M3_undecryptable_dm_reports_key_state_not_no_channel(void) crypto->setDHPrivateKey(config.security.private_key.bytes); } -void test_M4_peer_missing_our_key_retries_original_dm_after_nodeinfo(void) +void test_M4_peer_key_preflight_retries_original_dm_after_nodeinfo(void) { enableNodeInfoForDmKeyWait(); enablePkiForLocalNode(); @@ -1502,43 +1507,22 @@ void test_M4_peer_missing_our_key_retries_original_dm_after_nodeinfo(void) TEST_ASSERT_NOT_NULL(dm); dm->want_ack = true; const PacketId originalDmId = dm->id; - TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(dm, RX_SRC_USER)); - TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); - TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); - - meshtastic_MeshPacket nak = meshtastic_MeshPacket_init_zero; - nak.from = REMOTE_NODE; - nak.to = LOCAL_NODE; - nak.id = 0xD00D0004; - nak.channel = 0; - nak.which_payload_variant = meshtastic_MeshPacket_decoded_tag; - nak.decoded.portnum = meshtastic_PortNum_ROUTING_APP; - nak.decoded.request_id = originalDmId; - meshtastic_Routing routing = meshtastic_Routing_init_zero; - routing.error_reason = meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY; - - pipelineRouter->sniff(&nak, &routing); + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineService->sendToMesh(dm, RX_SRC_USER, false, true)); TEST_ASSERT_EQUAL(0, pipelineRouter->pendingCount()); TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); - TEST_ASSERT_TRUE_MESSAGE(pipelineRouter->shouldSuppressRoutingDelivery(nak), "interim key-exchange NAK stays off the client"); - TEST_ASSERT_FALSE(pipelineRouter->shouldSuppressRoutingDelivery(nak)); - TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + meshtastic_MeshPacket nodeInfo = pipelineRadio->sentPackets.back(); TEST_ASSERT_EQUAL(DECODE_SUCCESS, perhapsDecode(&nodeInfo)); TEST_ASSERT_EQUAL(meshtastic_PortNum_NODEINFO_APP, nodeInfo.decoded.portnum); + TEST_ASSERT_TRUE(nodeInfo.decoded.want_response); - bool sawKeyExchange = false; + uint8_t keyExchangeCount = 0; while (meshtastic_QueueStatus *status = pipelineService->getQueueStatusForPhone()) { - sawKeyExchange |= status->mesh_packet_id == originalDmId && status->state == meshtastic_QueueStatus_State_KEY_EXCHANGE; + keyExchangeCount += status->mesh_packet_id == originalDmId && status->state == meshtastic_QueueStatus_State_KEY_EXCHANGE; pipelineService->releaseQueueStatusToPool(status); } - TEST_ASSERT_TRUE_MESSAGE(sawKeyExchange, "client receives key exchange state for the original DM"); - - pipelineRouter->sniff(&nak, &routing); - TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); - TEST_ASSERT_TRUE_MESSAGE(pipelineRouter->shouldSuppressRoutingDelivery(nak), - "duplicate key-exchange NAK stays off the client while the exchange is pending"); - TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL_MESSAGE(1, keyExchangeCount, "client receives one key exchange state for the original DM"); meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); nodeInfoResponse.decoded.request_id = nodeInfo.id + 1; @@ -1548,25 +1532,18 @@ void test_M4_peer_missing_our_key_retries_original_dm_after_nodeinfo(void) memcpy(responseUser.public_key.bytes, remotePublic, sizeof(remotePublic)); TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); - TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); nodeInfoResponse.decoded.request_id = nodeInfo.id; TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); - TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRadio->sentPackets.back().id); TEST_ASSERT_TRUE(pipelineRadio->sentPackets.back().pki_encrypted); - - pipelineRouter->sniff(&nak, &routing); - TEST_ASSERT_EQUAL(0, pipelineRouter->pendingCount()); - TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); - TEST_ASSERT_FALSE_MESSAGE(pipelineRouter->shouldSuppressRoutingDelivery(nak), - "a second key-missing NAK is surfaced after the one retry"); - TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); } -void test_M5_peer_key_mismatch_does_not_auto_retry(void) +void test_M5_peer_key_exchange_attempt_avoids_repeated_preflight_and_preserves_mismatch(void) { enableNodeInfoForDmKeyWait(); enablePkiForLocalNode(); @@ -1580,9 +1557,30 @@ void test_M5_peer_key_mismatch_does_not_auto_retry(void) packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); TEST_ASSERT_NOT_NULL(dm); dm->want_ack = true; - const PacketId originalDmId = dm->id; TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + meshtastic_MeshPacket nodeInfo = pipelineRadio->sentPackets.back(); + + meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + nodeInfoResponse.decoded.request_id = nodeInfo.id; + meshtastic_User responseUser = meshtastic_User_init_zero; + responseUser.is_licensed = owner.is_licensed; + responseUser.public_key.size = sizeof(remotePublic); + memcpy(responseUser.public_key.bytes, remotePublic, sizeof(remotePublic)); + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); + pipelineRouter->clearPending(); + + meshtastic_MeshPacket *second = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(second); + second->id = 0xD00D0005; + second->want_ack = true; + const PacketId originalDmId = second->id; + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(second, RX_SRC_USER)); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); meshtastic_MeshPacket nak = meshtastic_MeshPacket_init_zero; nak.from = REMOTE_NODE; @@ -1600,7 +1598,7 @@ void test_M5_peer_key_mismatch_does_not_auto_retry(void) TEST_ASSERT_FALSE(pipelineRouter->shouldSuppressRoutingDelivery(nak)); } -void test_M6_peer_missing_our_key_retries_after_nodeinfo_response_timeout(void) +void test_M6_peer_key_preflight_retries_after_nodeinfo_response_timeout(void) { enableNodeInfoForDmKeyWait(); enablePkiForLocalNode(); @@ -1616,27 +1614,26 @@ void test_M6_peer_missing_our_key_retries_after_nodeinfo_response_timeout(void) dm->want_ack = true; const PacketId originalDmId = dm->id; TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(dm, RX_SRC_USER)); - - meshtastic_MeshPacket nak = meshtastic_MeshPacket_init_zero; - nak.from = REMOTE_NODE; - nak.to = LOCAL_NODE; - nak.id = 0xD00D0006; - nak.which_payload_variant = meshtastic_MeshPacket_decoded_tag; - nak.decoded.portnum = meshtastic_PortNum_ROUTING_APP; - nak.decoded.request_id = originalDmId; - meshtastic_Routing routing = meshtastic_Routing_init_zero; - routing.error_reason = meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY; - - pipelineRouter->sniff(&nak, &routing); TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); - TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); pipelineRouter->retryDeferredDmsForTest(); pipelineRouter->processDeferredDmsForTest(); TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); - TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRadio->sentPackets.back().id); + + pipelineRouter->clearPending(); + meshtastic_MeshPacket *second = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(second); + second->id = 0xD00D0007; + second->want_ack = true; + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(second, RX_SRC_USER)); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); } void test_M7_missing_recipient_key_fails_when_nodeinfo_cannot_queue(void) @@ -1729,7 +1726,7 @@ void test_M10_two_missing_keys_keep_independent_nodeinfo_requests(void) TEST_ASSERT_EQUAL(SECOND_REMOTE_NODE, pipelineRadio->sentPackets[1].to); } -void test_M11_unknown_sender_pki_dm_reaches_terminal_nak_from_rf_ingress(void) +void test_M11_unknown_sender_pki_dm_is_ignored_at_rf_ingress(void) { enableNodeInfoForDmKeyWait(); enablePkiForLocalNode(); @@ -1790,6 +1787,244 @@ void test_M12_unknown_pki_shaped_radio_packet_does_not_trigger_nak(void) TEST_ASSERT_EQUAL(0, pipelineModule->calls); } +void test_M13_two_peer_key_preflights_keep_independent_nodeinfo_requests(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + uint8_t firstPublic[32], firstPrivate[32], secondPublic[32], secondPrivate[32]; + crypto->generateKeyPair(firstPublic, firstPrivate); + crypto->generateKeyPair(secondPublic, secondPrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, firstPublic); + mockNodeDB->addNode(SECOND_REMOTE_NODE); + mockNodeDB->setPublicKey(SECOND_REMOTE_NODE, secondPublic); + + meshtastic_MeshPacket *first = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + meshtastic_MeshPacket *second = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, SECOND_REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(first); + TEST_ASSERT_NOT_NULL(second); + first->id = 0xD00D0013; + second->id = 0xD00D0014; + first->want_ack = second->want_ack = true; + + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(first, RX_SRC_USER)); + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(second, RX_SRC_USER)); + TEST_ASSERT_EQUAL(2, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL(0, pipelineRadio->cancelCalls); + + meshtastic_MeshPacket firstResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + firstResponse.decoded.request_id = pipelineRadio->sentPackets[0].id; + meshtastic_User firstUser = meshtastic_User_init_zero; + firstUser.is_licensed = owner.is_licensed; + firstUser.public_key.size = sizeof(firstPublic); + memcpy(firstUser.public_key.bytes, firstPublic, sizeof(firstPublic)); + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(firstResponse, &firstUser)); + + meshtastic_MeshPacket secondResponse = + makeDecoded(SECOND_REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + secondResponse.decoded.request_id = pipelineRadio->sentPackets[1].id; + meshtastic_User secondUser = meshtastic_User_init_zero; + secondUser.is_licensed = owner.is_licensed; + secondUser.public_key.size = sizeof(secondPublic); + memcpy(secondUser.public_key.bytes, secondPublic, sizeof(secondPublic)); + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(secondResponse, &secondUser)); + + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(2, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(4, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL_HEX32(0xD00D0013, pipelineRadio->sentPackets[2].id); + TEST_ASSERT_EQUAL_HEX32(0xD00D0014, pipelineRadio->sentPackets[3].id); +} + +void test_M14_explicit_destination_key_mismatch_fails_without_preflight(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + uint8_t remotePublic[32], remotePrivate[32], conflictingPublic[32], conflictingPrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->generateKeyPair(conflictingPublic, conflictingPrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePublic); + + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + dm->id = 0xD00D0015; + dm->want_ack = true; + dm->pki_encrypted = true; + dm->public_key.size = sizeof(conflictingPublic); + memcpy(dm->public_key.bytes, conflictingPublic, sizeof(conflictingPublic)); + + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_FAILED, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(0, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL(1, pipelineRouting->ackCalls); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_FAILED, pipelineRouting->lastAckError); +} + +void test_M15_peer_key_preflight_queue_full_sends_known_key_dm(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + uint8_t firstPublic[32], firstPrivate[32], secondPublic[32], secondPrivate[32], thirdPublic[32], thirdPrivate[32]; + crypto->generateKeyPair(firstPublic, firstPrivate); + crypto->generateKeyPair(secondPublic, secondPrivate); + crypto->generateKeyPair(thirdPublic, thirdPrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, firstPublic); + mockNodeDB->addNode(SECOND_REMOTE_NODE); + mockNodeDB->setPublicKey(SECOND_REMOTE_NODE, secondPublic); + mockNodeDB->addNode(THIRD_REMOTE_NODE); + mockNodeDB->setPublicKey(THIRD_REMOTE_NODE, thirdPublic); + + meshtastic_MeshPacket *first = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + meshtastic_MeshPacket *second = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, SECOND_REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + meshtastic_MeshPacket *third = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, THIRD_REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(first); + TEST_ASSERT_NOT_NULL(second); + TEST_ASSERT_NOT_NULL(third); + first->id = 0xD00D0016; + second->id = 0xD00D0017; + third->id = 0xD00D0018; + first->want_ack = second->want_ack = third->want_ack = true; + + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(first, RX_SRC_USER)); + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(second, RX_SRC_USER)); + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(third, RX_SRC_USER)); + TEST_ASSERT_EQUAL(2, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL_HEX32(0xD00D0018, pipelineRadio->sentPackets.back().id); + TEST_ASSERT_TRUE(pipelineRadio->sentPackets.back().pki_encrypted); + pipelineRouter->clearPending(); + pipelineRouter->clearDeferredDmsForTest(); +} + +void test_M16_local_key_rotation_invalidates_peer_key_exchange_attempt(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePublic); + + meshtastic_MeshPacket *first = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(first); + first->want_ack = true; + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(first, RX_SRC_USER)); + meshtastic_MeshPacket nodeInfo = pipelineRadio->sentPackets.back(); + meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + nodeInfoResponse.decoded.request_id = nodeInfo.id; + meshtastic_User responseUser = meshtastic_User_init_zero; + responseUser.is_licensed = owner.is_licensed; + responseUser.public_key.size = sizeof(remotePublic); + memcpy(responseUser.public_key.bytes, remotePublic, sizeof(remotePublic)); + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); + pipelineRouter->clearPending(); + + enablePkiForLocalNode(); + meshtastic_MeshPacket *second = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(second); + second->id = 0xD00D0019; + second->want_ack = true; + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(second, RX_SRC_USER)); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); + pipelineRouter->clearDeferredDmsForTest(); +} + +void test_M17_peer_key_rotation_invalidates_peer_key_exchange_attempt(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + uint8_t firstPublic[32], firstPrivate[32], secondPublic[32], secondPrivate[32]; + crypto->generateKeyPair(firstPublic, firstPrivate); + crypto->generateKeyPair(secondPublic, secondPrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, firstPublic); + + meshtastic_MeshPacket *first = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(first); + first->want_ack = true; + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(first, RX_SRC_USER)); + meshtastic_MeshPacket nodeInfo = pipelineRadio->sentPackets.back(); + meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + nodeInfoResponse.decoded.request_id = nodeInfo.id; + meshtastic_User responseUser = meshtastic_User_init_zero; + responseUser.is_licensed = owner.is_licensed; + responseUser.public_key.size = sizeof(firstPublic); + memcpy(responseUser.public_key.bytes, firstPublic, sizeof(firstPublic)); + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); + pipelineRouter->clearPending(); + + mockNodeDB->setPublicKey(REMOTE_NODE, secondPublic); + meshtastic_MeshPacket *second = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(second); + second->id = 0xD00D001A; + second->want_ack = true; + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(second, RX_SRC_USER)); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); + pipelineRouter->clearDeferredDmsForTest(); +} + +void test_M18_same_peer_dms_share_nodeinfo_preflight(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePublic); + + meshtastic_MeshPacket *first = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + meshtastic_MeshPacket *second = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(first); + TEST_ASSERT_NOT_NULL(second); + first->id = 0xD00D001B; + second->id = 0xD00D001C; + first->want_ack = second->want_ack = true; + + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(first, RX_SRC_USER)); + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(second, RX_SRC_USER)); + TEST_ASSERT_EQUAL(2, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + + meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + nodeInfoResponse.decoded.request_id = pipelineRadio->sentPackets.back().id; + meshtastic_User responseUser = meshtastic_User_init_zero; + responseUser.is_licensed = owner.is_licensed; + responseUser.public_key.size = sizeof(remotePublic); + memcpy(responseUser.public_key.bytes, remotePublic, sizeof(remotePublic)); + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); + + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(2, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL_HEX32(0xD00D001B, pipelineRadio->sentPackets[1].id); + TEST_ASSERT_EQUAL_HEX32(0xD00D001C, pipelineRadio->sentPackets[2].id); + pipelineRouter->clearPending(); +} + // 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_N5_unsigned_unicast_nodeinfo_from_signer_does_not_change_name(void) @@ -2156,15 +2391,21 @@ void setup() RUN_TEST(test_M1_unknown_dm_waits_for_nodeinfo_key_exchange_then_retries); RUN_TEST(test_M2_unknown_dm_fails_only_after_key_exchange_timeout); RUN_TEST(test_M3_undecryptable_dm_reports_key_state_not_no_channel); - RUN_TEST(test_M4_peer_missing_our_key_retries_original_dm_after_nodeinfo); - RUN_TEST(test_M5_peer_key_mismatch_does_not_auto_retry); - RUN_TEST(test_M6_peer_missing_our_key_retries_after_nodeinfo_response_timeout); + RUN_TEST(test_M4_peer_key_preflight_retries_original_dm_after_nodeinfo); + RUN_TEST(test_M5_peer_key_exchange_attempt_avoids_repeated_preflight_and_preserves_mismatch); + RUN_TEST(test_M6_peer_key_preflight_retries_after_nodeinfo_response_timeout); RUN_TEST(test_M7_missing_recipient_key_fails_when_nodeinfo_cannot_queue); + RUN_TEST(test_M13_two_peer_key_preflights_keep_independent_nodeinfo_requests); + RUN_TEST(test_M14_explicit_destination_key_mismatch_fails_without_preflight); RUN_TEST(test_M8_nodeinfo_reply_without_key_keeps_dm_deferred); RUN_TEST(test_M9_missing_recipient_key_fails_when_nodeinfo_send_is_rejected); RUN_TEST(test_M10_two_missing_keys_keep_independent_nodeinfo_requests); - RUN_TEST(test_M11_unknown_sender_pki_dm_reaches_terminal_nak_from_rf_ingress); + RUN_TEST(test_M11_unknown_sender_pki_dm_is_ignored_at_rf_ingress); RUN_TEST(test_M12_unknown_pki_shaped_radio_packet_does_not_trigger_nak); + RUN_TEST(test_M15_peer_key_preflight_queue_full_sends_known_key_dm); + RUN_TEST(test_M16_local_key_rotation_invalidates_peer_key_exchange_attempt); + RUN_TEST(test_M17_peer_key_rotation_invalidates_peer_key_exchange_attempt); + RUN_TEST(test_M18_same_peer_dms_share_nodeinfo_preflight); printf("\n=== Group N: NodeInfoModule authentication ===\n"); RUN_TEST(test_N1_unsigned_nodeinfo_from_signer_dropped); RUN_TEST(test_N2_signed_nodeinfo_from_signer_not_dropped); From 4220156b3575e1f1f3d8dfc4fe4c28e0e4f1c9d0 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sat, 25 Jul 2026 22:56:12 -0700 Subject: [PATCH 06/15] fix: recover deferred direct message failures --- src/mesh/ReliableRouter.cpp | 3 +- src/mesh/Router.cpp | 40 ++++++------ src/mesh/Router.h | 4 +- test/test_packet_signing/test_main.cpp | 85 +++++++++++++++++++++++--- 4 files changed, 100 insertions(+), 32 deletions(-) diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp index 3d04a52e5be..7fbebd384c7 100644 --- a/src/mesh/ReliableRouter.cpp +++ b/src/mesh/ReliableRouter.cpp @@ -124,7 +124,8 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas } else if (!(alreadyRetriedForPeerKey = hasRetriedPeerKeyDm(p->from, p->decoded.request_id))) { if (PendingPacket *pendingPacket = findPendingPacket(GlobalPacketId(p->to, p->decoded.request_id))) { meshtastic_MeshPacket *retry = packetPool.allocCopy(*pendingPacket->packet); - if (retry && deferPeerKeyDm(retry) == DeferredDmResult::DEFERRED) { + if (retry && deferPeerKeyDm(retry, true, true) == DeferredDmResult::DEFERRED) { + rememberPeerKeyRetry(p->from, p->decoded.request_id); stopRetransmission(p->to, p->decoded.request_id); suppressRoutingDelivery(*p); deferredForPeerKey = true; diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 36ca66a8bc0..eba3d9a0c31 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -363,7 +363,8 @@ bool Router::retryDeferredDmOnNodeInfo(const meshtastic_MeshPacket &p) p.decoded.portnum != meshtastic_PortNum_NODEINFO_APP || !p.decoded.request_id) return false; - bool retried = false; + meshtastic_MeshPacket *retries[deferredDmCapacity] = {}; + uint8_t retryCount = 0; for (auto &deferred : deferredDms) { if (!deferred.p || deferred.p->to != p.from || deferred.keyExchangeId != p.decoded.request_id) continue; @@ -374,16 +375,18 @@ bool Router::retryDeferredDmOnNodeInfo(const meshtastic_MeshPacket &p) if (!nodeDB->copyPublicKey(dm->to, remoteKey)) continue; } - deferred.p = nullptr; - deferred.queuedAtMs = 0; - deferred.keyExchangeId = 0; + retries[retryCount++] = dm; + deferred = {}; + } + + bool retried = false; + for (uint8_t i = 0; i < retryCount; ++i) { + meshtastic_MeshPacket *dm = retries[i]; + const PacketId dmId = dm->id; LOG_INFO("NodeInfo exchange with 0x%08x completed; retrying deferred DM id=0x%08x", p.from, dm->id); rememberPeerKeyExchangeAttempt(dm->to); - if (deferred.reason == DeferredDm::Reason::PEER_KEY) { - rememberPeerKeyRetry(dm->to, dm->id); - } - service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, dm->id, meshtastic_QueueStatus_State_STATE_UNSPECIFIED); - send(dm); + const ErrorCode result = send(dm); + service->sendQueueStatusToPhone(getQueueStatus(), result, dmId, meshtastic_QueueStatus_State_STATE_UNSPECIFIED); retried = true; } return retried; @@ -1345,15 +1348,14 @@ Router::DeferredDmResult Router::deferMissingKeyDm(meshtastic_MeshPacket *p) return DeferredDmResult::FAILED; } -Router::DeferredDmResult Router::deferPeerKeyDm(meshtastic_MeshPacket *p, bool reportQueueStatus) +Router::DeferredDmResult Router::deferPeerKeyDm(meshtastic_MeshPacket *p, bool reportQueueStatus, bool force) { if (!nodeInfoModule || p->which_payload_variant != meshtastic_MeshPacket_decoded_tag || !IS_ONE_OF(p->decoded.portnum, meshtastic_PortNum_TEXT_MESSAGE_APP, meshtastic_PortNum_TEXT_MESSAGE_COMPRESSED_APP)) return DeferredDmResult::NOT_APPLICABLE; meshtastic_NodeInfoLite_public_key_t remoteKey = {0, {0}}; - if (!nodeDB->copyPublicKey(p->to, remoteKey) || !wouldEncryptWithPKC(p, p->channel, true) || - hasRetriedPeerKeyDm(p->to, p->id)) + if (!nodeDB->copyPublicKey(p->to, remoteKey) || !wouldEncryptWithPKC(p, p->channel, true)) return DeferredDmResult::NOT_APPLICABLE; if (p->pki_encrypted && !memfll(p->public_key.bytes, 0, sizeof(p->public_key.bytes)) && memcmp(p->public_key.bytes, remoteKey.bytes, sizeof(remoteKey.bytes)) != 0) @@ -1366,7 +1368,7 @@ Router::DeferredDmResult Router::deferPeerKeyDm(meshtastic_MeshPacket *p, bool r break; } } - if (!keyExchangeId && hasPeerKeyExchangeAttempt(p->to)) + if (!keyExchangeId && !force && hasPeerKeyExchangeAttempt(p->to)) return DeferredDmResult::NOT_APPLICABLE; for (auto &deferred : deferredDms) { @@ -1487,10 +1489,9 @@ void Router::processDeferredDms() deferred.keyExchangeId = 0; LOG_INFO("Retrying deferred DM id=0x%08x after NodeInfo response wait for 0x%08x", p->id, p->to); rememberPeerKeyExchangeAttempt(p->to); - rememberPeerKeyRetry(p->to, p->id); - service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, p->id, - meshtastic_QueueStatus_State_STATE_UNSPECIFIED); - send(p); + const PacketId dmId = p->id; + const ErrorCode result = send(p); + service->sendQueueStatusToPhone(getQueueStatus(), result, dmId, meshtastic_QueueStatus_State_STATE_UNSPECIFIED); } continue; } @@ -1501,8 +1502,9 @@ void Router::processDeferredDms() deferred.queuedAtMs = 0; deferred.keyExchangeId = 0; LOG_INFO("Peer key learned for 0x%08x; retrying deferred DM id=0x%08x", p->to, p->id); - service->sendQueueStatusToPhone(getQueueStatus(), ERRNO_OK, p->id, meshtastic_QueueStatus_State_STATE_UNSPECIFIED); - send(p); + const PacketId dmId = p->id; + const ErrorCode result = send(p); + service->sendQueueStatusToPhone(getQueueStatus(), result, dmId, meshtastic_QueueStatus_State_STATE_UNSPECIFIED); } else if (!Throttle::isWithinTimespanMs(deferred.queuedAtMs, deferredDmKeyWaitMs)) { deferred.p = nullptr; deferred.queuedAtMs = 0; diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 82b9b4acb35..207da479dd6 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -118,7 +118,7 @@ class Router : protected concurrency::OSThread, protected PacketHistory /// Derived routers must call this before creating retransmission state for the packet. DeferredDmResult deferMissingKeyDm(meshtastic_MeshPacket *p); /// Takes ownership while requesting a NodeInfo exchange before a PKI DM. - DeferredDmResult deferPeerKeyDm(meshtastic_MeshPacket *p, bool reportQueueStatus = true); + DeferredDmResult deferPeerKeyDm(meshtastic_MeshPacket *p, bool reportQueueStatus = true, bool force = false); bool isWaitingForPeerKeyDm(NodeNum peer, PacketId id) const; bool hasRetriedPeerKeyDm(NodeNum peer, PacketId id); void rememberPeerKeyRetry(NodeNum peer, PacketId id); @@ -245,7 +245,7 @@ class Router : protected concurrency::OSThread, protected PacketHistory static constexpr uint8_t deferredDmCapacity = 2; static constexpr uint32_t deferredDmKeyWaitMs = 30 * 1000UL; - static constexpr uint32_t deferredDmPeerKeyWaitMs = 10 * 1000UL; + static constexpr uint32_t deferredDmPeerKeyWaitMs = 2 * 1000UL; static constexpr uint32_t peerKeyRetryMemoryMs = 30 * 1000UL; static constexpr uint32_t peerKeyExchangeAttemptMs = 30 * 60 * 1000UL; DeferredDm deferredDms[deferredDmCapacity]; diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 7e60656306e..2c8df358796 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -1598,7 +1598,7 @@ void test_M5_peer_key_exchange_attempt_avoids_repeated_preflight_and_preserves_m TEST_ASSERT_FALSE(pipelineRouter->shouldSuppressRoutingDelivery(nak)); } -void test_M6_peer_key_preflight_retries_after_nodeinfo_response_timeout(void) +void test_M6_peer_key_preflight_recovers_unknown_key_nak_after_timeout(void) { enableNodeInfoForDmKeyWait(); enablePkiForLocalNode(); @@ -1624,16 +1624,39 @@ void test_M6_peer_key_preflight_retries_after_nodeinfo_response_timeout(void) TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRadio->sentPackets.back().id); - pipelineRouter->clearPending(); - meshtastic_MeshPacket *second = - packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); - TEST_ASSERT_NOT_NULL(second); - second->id = 0xD00D0007; - second->want_ack = true; - TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(second, RX_SRC_USER)); + meshtastic_MeshPacket nak = meshtastic_MeshPacket_init_zero; + nak.from = REMOTE_NODE; + nak.to = LOCAL_NODE; + nak.id = 0xD00D0006; + nak.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + nak.decoded.portnum = meshtastic_PortNum_ROUTING_APP; + nak.decoded.request_id = originalDmId; + meshtastic_Routing routing = meshtastic_Routing_init_zero; + routing.error_reason = meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY; + pipelineRouter->sniff(&nak, &routing); + + TEST_ASSERT_EQUAL(0, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); + TEST_ASSERT_TRUE_MESSAGE(pipelineRouter->shouldSuppressRoutingDelivery(nak), + "first unknown-key NAK starts one forced recovery"); + + meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + nodeInfoResponse.decoded.request_id = pipelineRadio->sentPackets.back().id; + meshtastic_User responseUser = meshtastic_User_init_zero; + responseUser.is_licensed = owner.is_licensed; + responseUser.public_key.size = sizeof(remotePublic); + memcpy(responseUser.public_key.bytes, remotePublic, sizeof(remotePublic)); + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); - TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL(4, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRadio->sentPackets.back().id); + + pipelineRouter->sniff(&nak, &routing); + TEST_ASSERT_EQUAL(0, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_FALSE_MESSAGE(pipelineRouter->shouldSuppressRoutingDelivery(nak), "second unknown-key NAK remains terminal"); } void test_M7_missing_recipient_key_fails_when_nodeinfo_cannot_queue(void) @@ -2025,6 +2048,47 @@ void test_M18_same_peer_dms_share_nodeinfo_preflight(void) pipelineRouter->clearPending(); } +void test_M19_deferred_dm_reports_the_resumed_send_result(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePublic); + + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + dm->id = 0xD00D001D; + dm->want_ack = true; + const PacketId originalDmId = dm->id; + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineService->sendToMesh(dm, RX_SRC_USER, false, true)); + meshtastic_MeshPacket nodeInfo = pipelineRadio->sentPackets.back(); + + while (meshtastic_QueueStatus *status = pipelineService->getQueueStatusForPhone()) + pipelineService->releaseQueueStatusToPool(status); + pipelineRadio->sendResult = meshtastic_Routing_Error_DUTY_CYCLE_LIMIT; + + meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + nodeInfoResponse.decoded.request_id = nodeInfo.id; + meshtastic_User responseUser = meshtastic_User_init_zero; + responseUser.is_licensed = owner.is_licensed; + responseUser.public_key.size = sizeof(remotePublic); + memcpy(responseUser.public_key.bytes, remotePublic, sizeof(remotePublic)); + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); + + meshtastic_QueueStatus *status = pipelineService->getQueueStatusForPhone(); + TEST_ASSERT_NOT_NULL(status); + TEST_ASSERT_EQUAL_HEX32(originalDmId, status->mesh_packet_id); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_DUTY_CYCLE_LIMIT, status->res); + TEST_ASSERT_EQUAL(meshtastic_QueueStatus_State_STATE_UNSPECIFIED, status->state); + pipelineService->releaseQueueStatusToPool(status); + TEST_ASSERT_NULL(pipelineService->getQueueStatusForPhone()); + pipelineRouter->clearPending(); +} + // 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_N5_unsigned_unicast_nodeinfo_from_signer_does_not_change_name(void) @@ -2393,7 +2457,7 @@ void setup() RUN_TEST(test_M3_undecryptable_dm_reports_key_state_not_no_channel); RUN_TEST(test_M4_peer_key_preflight_retries_original_dm_after_nodeinfo); RUN_TEST(test_M5_peer_key_exchange_attempt_avoids_repeated_preflight_and_preserves_mismatch); - RUN_TEST(test_M6_peer_key_preflight_retries_after_nodeinfo_response_timeout); + RUN_TEST(test_M6_peer_key_preflight_recovers_unknown_key_nak_after_timeout); RUN_TEST(test_M7_missing_recipient_key_fails_when_nodeinfo_cannot_queue); RUN_TEST(test_M13_two_peer_key_preflights_keep_independent_nodeinfo_requests); RUN_TEST(test_M14_explicit_destination_key_mismatch_fails_without_preflight); @@ -2406,6 +2470,7 @@ void setup() RUN_TEST(test_M16_local_key_rotation_invalidates_peer_key_exchange_attempt); RUN_TEST(test_M17_peer_key_rotation_invalidates_peer_key_exchange_attempt); RUN_TEST(test_M18_same_peer_dms_share_nodeinfo_preflight); + RUN_TEST(test_M19_deferred_dm_reports_the_resumed_send_result); printf("\n=== Group N: NodeInfoModule authentication ===\n"); RUN_TEST(test_N1_unsigned_nodeinfo_from_signer_dropped); RUN_TEST(test_N2_signed_nodeinfo_from_signer_not_dropped); From e4d19fbd3b1afd6cce0ee8611abab0fb5f82ab70 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 26 Jul 2026 08:23:55 -0700 Subject: [PATCH 07/15] fix: preserve PKI routing replies --- src/mesh/Router.cpp | 6 +- test/test_packet_signing/test_main.cpp | 95 ++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index eba3d9a0c31..56c3b307008 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -1116,7 +1116,8 @@ bool wouldEncryptWithPKC(const meshtastic_MeshPacket *p, ChannelIndex chIndex, b config.security.private_key.size == 32 && !isBroadcast(p->to) && // Some portnums either make no sense to send with PKC p->decoded.portnum != meshtastic_PortNum_TRACEROUTE_APP && p->decoded.portnum != meshtastic_PortNum_NODEINFO_APP && - p->decoded.portnum != meshtastic_PortNum_ROUTING_APP && p->decoded.portnum != meshtastic_PortNum_POSITION_APP && + (p->decoded.portnum != meshtastic_PortNum_ROUTING_APP || p->pki_encrypted) && + p->decoded.portnum != meshtastic_PortNum_POSITION_APP && // We allow Key Verification messages to be sent without a known destination key, since the point of those messages is // to exchange keys. The first exchange (no usable key yet) falls through to channel encryption; the follow-on packet // uses the pending key resolved into haveDestKey/destKey above. @@ -1234,7 +1235,8 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p) *destKey.bytes); return meshtastic_Routing_Error_PKI_FAILED; } - crypto->encryptCurve25519(p->to, getFrom(p), destKey, p->id, numbytes, bytes, p->encrypted.bytes); + if (!crypto->encryptCurve25519(p->to, getFrom(p), destKey, p->id, numbytes, bytes, p->encrypted.bytes)) + return meshtastic_Routing_Error_PKI_FAILED; numbytes += MESHTASTIC_PKC_OVERHEAD; p->channel = 0; p->pki_encrypted = true; diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 2c8df358796..44b2a646bc1 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -28,9 +28,11 @@ #include "mesh/ReliableRouter.h" #include "mesh/Router.h" #include "mesh/SinglePortModule.h" +#include "modules/AdminModule.h" #include "modules/NodeInfoModule.h" #include "modules/RoutingModule.h" #include "mqtt/MQTT.h" +#include "support/AdminModuleTestShim.h" #include #include #include @@ -1437,6 +1439,39 @@ void test_M1_unknown_dm_waits_for_nodeinfo_key_exchange_then_retries(void) TEST_ASSERT_EQUAL(0, pipelineRouting->ackCalls); } +void test_M1a_phone_origin_unknown_dm_waits_for_nodeinfo_key_exchange(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(0, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + dm->want_ack = true; + const PacketId originalDmId = dm->id; + + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + meshtastic_MeshPacket nodeInfoRequest = pipelineRadio->sentPackets.back(); + TEST_ASSERT_EQUAL(DECODE_SUCCESS, perhapsDecode(&nodeInfoRequest)); + TEST_ASSERT_EQUAL(meshtastic_PortNum_NODEINFO_APP, nodeInfoRequest.decoded.portnum); + + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + nodeInfoResponse.decoded.request_id = nodeInfoRequest.id; + meshtastic_User responseUser = meshtastic_User_init_zero; + responseUser.is_licensed = owner.is_licensed; + responseUser.public_key.size = sizeof(remotePublic); + memcpy(responseUser.public_key.bytes, remotePublic, sizeof(remotePublic)); + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); + TEST_ASSERT_TRUE(pipelineRadio->sentPackets.back().pki_encrypted); + TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRadio->sentPackets.back().id); +} + void test_M2_unknown_dm_fails_only_after_key_exchange_timeout(void) { enableNodeInfoForDmKeyWait(); @@ -2089,6 +2124,63 @@ void test_M19_deferred_dm_reports_the_resumed_send_result(void) pipelineRouter->clearPending(); } +void test_M20_weak_destination_key_fails_closed(void) +{ + enablePkiForLocalNode(); + + uint8_t weakPublic[32] = {0}; + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, weakPublic); + + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_FAILED, perhapsEncode(dm)); + packetPool.release(dm); +} + +void test_M21_pki_admin_routing_reply_remains_pki_encrypted(void) +{ + enablePkiForLocalNode(); + uint8_t localPublic[32]; + memcpy(localPublic, owner.public_key.bytes, sizeof(localPublic)); + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePublic); + config.security.admin_key[0].size = sizeof(remotePublic); + memcpy(config.security.admin_key[0].bytes, remotePublic, sizeof(remotePublic)); + + AdminModuleTestShim admin; + meshtastic_MeshPacket request = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_ADMIN_APP, 0); + request.pki_encrypted = true; + request.public_key.size = sizeof(remotePublic); + memcpy(request.public_key.bytes, remotePublic, sizeof(remotePublic)); + request.decoded.want_response = true; + + meshtastic_AdminMessage message = meshtastic_AdminMessage_init_zero; + message.which_payload_variant = meshtastic_AdminMessage_get_channel_request_tag; + message.get_channel_request = 0; + + admin.handleReceivedProtobuf(request, &message); + meshtastic_MeshPacket *reply = admin.reply(); + TEST_ASSERT_NOT_NULL(reply); + TEST_ASSERT_EQUAL(meshtastic_PortNum_ROUTING_APP, reply->decoded.portnum); + TEST_ASSERT_TRUE(reply->pki_encrypted); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_NONE, perhapsEncode(reply)); + TEST_ASSERT_TRUE(reply->pki_encrypted); + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->setPublicKey(LOCAL_NODE, localPublic); + crypto->setDHPrivateKey(remotePrivate); + myNodeInfo.my_node_num = REMOTE_NODE; + TEST_ASSERT_EQUAL(DECODE_SUCCESS, perhapsDecode(reply)); + myNodeInfo.my_node_num = LOCAL_NODE; + crypto->setDHPrivateKey(config.security.private_key.bytes); + admin.drainReply(); +} + // 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_N5_unsigned_unicast_nodeinfo_from_signer_does_not_change_name(void) @@ -2453,6 +2545,7 @@ void setup() RUN_TEST(test_C12_exact_authenticated_replay_reuses_verdict_without_collision_bypass); printf("\n=== Group M: deferred DM key exchange ===\n"); RUN_TEST(test_M1_unknown_dm_waits_for_nodeinfo_key_exchange_then_retries); + RUN_TEST(test_M1a_phone_origin_unknown_dm_waits_for_nodeinfo_key_exchange); RUN_TEST(test_M2_unknown_dm_fails_only_after_key_exchange_timeout); RUN_TEST(test_M3_undecryptable_dm_reports_key_state_not_no_channel); RUN_TEST(test_M4_peer_key_preflight_retries_original_dm_after_nodeinfo); @@ -2471,6 +2564,8 @@ void setup() RUN_TEST(test_M17_peer_key_rotation_invalidates_peer_key_exchange_attempt); RUN_TEST(test_M18_same_peer_dms_share_nodeinfo_preflight); RUN_TEST(test_M19_deferred_dm_reports_the_resumed_send_result); + RUN_TEST(test_M20_weak_destination_key_fails_closed); + RUN_TEST(test_M21_pki_admin_routing_reply_remains_pki_encrypted); printf("\n=== Group N: NodeInfoModule authentication ===\n"); RUN_TEST(test_N1_unsigned_nodeinfo_from_signer_dropped); RUN_TEST(test_N2_signed_nodeinfo_from_signer_not_dropped); From 2971596cd93f25fd9ab6ff5c88a9db7975a9bfb0 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 26 Jul 2026 10:34:34 -0700 Subject: [PATCH 08/15] fix: report recoverable direct message failures --- src/mesh/NodeDB.cpp | 13 +++++--- src/mesh/PhoneAPI.cpp | 2 +- src/mesh/Router.cpp | 3 ++ test/test_packet_signing/test_main.cpp | 45 +++++++++++++++++++++++--- 4 files changed, 53 insertions(+), 10 deletions(-) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index a8de3cdd927..12bc3a9f627 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -3386,7 +3386,8 @@ bool NodeDB::updateUser(uint32_t nodeId, meshtastic_User &p, uint8_t channelInde return false; } } - if (info->public_key.size == 32) { // if we have a key for this user already, don't overwrite with a new one + if (info->public_key.size == 32 && !memfll(info->public_key.bytes, 0, sizeof(info->public_key.bytes))) { + // If we have a usable key for this user already, don't overwrite it with a new one. // if the key doesn't match, don't update nodeDB at all. if (p.public_key.size != 32 || (memcmp(p.public_key.bytes, info->public_key.bytes, 32) != 0)) { LOG_WARN("Public Key mismatch, dropping NodeInfo"); @@ -3760,12 +3761,12 @@ uint32_t NodeDB::hotNodeLastHeard(NodeNum n) const bool NodeDB::copyPublicKeyAuthoritative(NodeNum n, meshtastic_NodeInfoLite_public_key_t &out) { const meshtastic_NodeInfoLite *info = getMeshNode(n); - if (info && info->public_key.size == 32) { + if (info && info->public_key.size == 32 && !memfll(info->public_key.bytes, 0, sizeof(info->public_key.bytes))) { out = info->public_key; return true; } #if WARM_NODE_COUNT > 0 - if (warmStore.copyKey(n, out.bytes)) { + if (warmStore.copyKey(n, out.bytes) && !memfll(out.bytes, 0, sizeof(out.bytes))) { out.size = 32; return true; } @@ -3782,7 +3783,8 @@ bool NodeDB::copyPublicKey(NodeNum n, meshtastic_NodeInfoLite_public_key_t &out) // for a node no longer in either NodeDB tier. This extends the pool of peers we can // encrypt to. Keys here may be trust-on-first-use (see copyPublicKey's signerProven), the // same first-contact trust NodeDB itself applies via updateUser(). - if (trafficManagementModule && trafficManagementModule->copyPublicKey(n, out.bytes)) { + if (trafficManagementModule && trafficManagementModule->copyPublicKey(n, out.bytes) && + !memfll(out.bytes, 0, sizeof(out.bytes))) { out.size = 32; return true; } @@ -3798,7 +3800,8 @@ bool NodeDB::copyPublicKeyForDecrypt(NodeNum n, meshtastic_NodeInfoLite_public_k // A cold-tier cache key backs an authenticated decrypt only when signer-proven; unverified TOFU // cache keys must not. Outbound encryption still uses the opportunistic copyPublicKey(). bool signerProven = false; - if (trafficManagementModule && trafficManagementModule->copyPublicKey(n, out.bytes, &signerProven) && signerProven) { + if (trafficManagementModule && trafficManagementModule->copyPublicKey(n, out.bytes, &signerProven) && signerProven && + !memfll(out.bytes, 0, sizeof(out.bytes))) { out.size = 32; return true; } diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp index 28d047f13fb..ff4003bd37a 100644 --- a/src/mesh/PhoneAPI.cpp +++ b/src/mesh/PhoneAPI.cpp @@ -1805,7 +1805,7 @@ bool PhoneAPI::handleToRadioPacket(meshtastic_MeshPacket &p) Throttle::isWithinTimespanMs(lastPortNumToRadio[p.decoded.portnum], TWO_SECONDS_MS)) { LOG_WARN("Rate limit portnum %d", p.decoded.portnum); meshtastic_QueueStatus qs = router->getQueueStatus(); - service->sendQueueStatusToPhone(qs, 0, p.id); + service->sendQueueStatusToPhone(qs, meshtastic_Routing_Error_RATE_LIMIT_EXCEEDED, p.id); service->sendRoutingErrorResponse(meshtastic_Routing_Error_RATE_LIMIT_EXCEEDED, &p); // sendNotification(meshtastic_LogRecord_Level_WARNING, p.id, "Text messages can only be sent once every 2 seconds"); return false; diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 56c3b307008..af5ea320401 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -1512,7 +1512,10 @@ void Router::processDeferredDms() deferred.queuedAtMs = 0; deferred.keyExchangeId = 0; LOG_WARN("No public key learned for 0x%08x before deferred DM id=0x%08x timed out", p->to, p->id); + const PacketId dmId = p->id; abortSendAndNak(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, p); + service->sendQueueStatusToPhone(getQueueStatus(), meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, dmId, + meshtastic_QueueStatus_State_STATE_UNSPECIFIED); } } } diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 44b2a646bc1..da3bd5613ba 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -1494,6 +1494,20 @@ void test_M2_unknown_dm_fails_only_after_key_exchange_timeout(void) TEST_ASSERT_EQUAL(1, pipelineRouting->ackCalls); TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, pipelineRouting->lastAckError); TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRouting->lastAckId); + bool terminalStatusReceived = false; + ErrorCode finalStatus = ERRNO_OK; + meshtastic_QueueStatus_State finalState = meshtastic_QueueStatus_State_STATE_UNSPECIFIED; + while (meshtastic_QueueStatus *status = pipelineService->getQueueStatusForPhone()) { + if (status->mesh_packet_id == originalDmId) { + finalStatus = status->res; + finalState = status->state; + terminalStatusReceived = status->res == meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY; + } + pipelineService->releaseQueueStatusToPool(status); + } + TEST_ASSERT_TRUE(terminalStatusReceived); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, finalStatus); + TEST_ASSERT_EQUAL(meshtastic_QueueStatus_State_STATE_UNSPECIFIED, finalState); #if ARCH_PORTDUINO portduino_config.force_simradio = dmKeyWaitOriginalForceSimRadio; #endif @@ -2124,20 +2138,43 @@ void test_M19_deferred_dm_reports_the_resumed_send_result(void) pipelineRouter->clearPending(); } -void test_M20_weak_destination_key_fails_closed(void) +void test_M20_weak_destination_key_refreshes_before_retry(void) { + enableNodeInfoForDmKeyWait(); enablePkiForLocalNode(); uint8_t weakPublic[32] = {0}; mockNodeDB->addNode(REMOTE_NODE); mockNodeDB->setPublicKey(REMOTE_NODE, weakPublic); + meshtastic_NodeInfoLite_public_key_t storedKey = {0, {0}}; + TEST_ASSERT_FALSE(nodeDB->copyPublicKey(REMOTE_NODE, storedKey)); meshtastic_MeshPacket *dm = packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); TEST_ASSERT_NOT_NULL(dm); + dm->want_ack = true; + const PacketId originalDmId = dm->id; - TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_FAILED, perhapsEncode(dm)); - packetPool.release(dm); + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + meshtastic_MeshPacket nodeInfoRequest = pipelineRadio->sentPackets.back(); + + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + nodeInfoResponse.decoded.request_id = nodeInfoRequest.id; + meshtastic_User responseUser = meshtastic_User_init_zero; + responseUser.is_licensed = owner.is_licensed; + responseUser.public_key.size = sizeof(remotePublic); + memcpy(responseUser.public_key.bytes, remotePublic, sizeof(remotePublic)); + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); + + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); + TEST_ASSERT_TRUE(pipelineRadio->sentPackets.back().pki_encrypted); + TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRadio->sentPackets.back().id); } void test_M21_pki_admin_routing_reply_remains_pki_encrypted(void) @@ -2564,7 +2601,7 @@ void setup() RUN_TEST(test_M17_peer_key_rotation_invalidates_peer_key_exchange_attempt); RUN_TEST(test_M18_same_peer_dms_share_nodeinfo_preflight); RUN_TEST(test_M19_deferred_dm_reports_the_resumed_send_result); - RUN_TEST(test_M20_weak_destination_key_fails_closed); + RUN_TEST(test_M20_weak_destination_key_refreshes_before_retry); RUN_TEST(test_M21_pki_admin_routing_reply_remains_pki_encrypted); printf("\n=== Group N: NodeInfoModule authentication ===\n"); RUN_TEST(test_N1_unsigned_nodeinfo_from_signer_dropped); From fd5975a8c73ced221a79c4403bd016f3a924f973 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 26 Jul 2026 10:39:54 -0700 Subject: [PATCH 09/15] fix: recover malformed direct message keys --- src/mesh/NodeDB.cpp | 4 +++- src/mesh/ReliableRouter.cpp | 6 ++++-- test/test_packet_signing/test_main.cpp | 7 +++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index 12bc3a9f627..8441c43fe6f 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -3347,7 +3347,9 @@ bool NodeDB::updateUser(uint32_t nodeId, meshtastic_User &p, uint8_t channelInde // Only a signed update may change the identity of a node that has proven it signs; our own record is // exempt. Checked before getOrCreateMeshNode so a refused update cannot evict or write the warm tier. const meshtastic_NodeInfoLite *existing = getMeshNode(nodeId); - if (nodeId != getNodeNum() && existing && nodeInfoLiteHasXeddsaSigned(existing) && !xeddsaSigned) { + const bool hasWeakSignerKey = + existing && existing->public_key.size == 32 && memfll(existing->public_key.bytes, 0, sizeof(existing->public_key.bytes)); + if (nodeId != getNodeNum() && existing && nodeInfoLiteHasXeddsaSigned(existing) && !hasWeakSignerKey && !xeddsaSigned) { LOG_WARN("Refusing unsigned identity update for node 0x%08x that previously signed", nodeId); return false; } diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp index 7fbebd384c7..877d0a8eb2e 100644 --- a/src/mesh/ReliableRouter.cpp +++ b/src/mesh/ReliableRouter.cpp @@ -160,8 +160,10 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas } } else if (p->which_payload_variant == meshtastic_MeshPacket_encrypted_tag && p->channel == 0) { const meshtastic_NodeInfoLite *sender = nodeDB->getMeshNode(p->from); - const auto error = (!sender || sender->public_key.size == 0) ? meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY - : meshtastic_Routing_Error_PKI_FAILED; + const bool hasSenderKey = sender && sender->public_key.size == 32 && + !memfll(sender->public_key.bytes, 0, sizeof(sender->public_key.bytes)); + const auto error = + hasSenderKey ? meshtastic_Routing_Error_PKI_FAILED : meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY; LOG_INFO("Undecryptable PKI packet from 0x%08x, send error %d", p->from, error); sendAckNak(error, getFrom(p), p->id, channels.getPrimaryIndex(), routingModule->getHopLimitForResponse(*p)); } else { diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index da3bd5613ba..6a42d27f939 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -1538,6 +1538,12 @@ void test_M3_undecryptable_dm_reports_key_state_not_no_channel(void) TEST_ASSERT_EQUAL_MESSAGE(meshtastic_Routing_Error_PKI_FAILED, pipelineRouting->lastAckError, "a stored-but-wrong key is not a channel failure"); TEST_ASSERT_EQUAL_HEX32(dm.id, pipelineRouting->lastAckId); + + uint8_t weakKey[32] = {0}; + mockNodeDB->setPublicKey(REMOTE_NODE, weakKey); + pipelineRouter->sniff(&dm, nullptr); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY, pipelineRouting->lastAckError); + TEST_ASSERT_EQUAL_HEX32(dm.id, pipelineRouting->lastAckId); crypto->setDHPrivateKey(config.security.private_key.bytes); } @@ -2146,6 +2152,7 @@ void test_M20_weak_destination_key_refreshes_before_retry(void) uint8_t weakPublic[32] = {0}; mockNodeDB->addNode(REMOTE_NODE); mockNodeDB->setPublicKey(REMOTE_NODE, weakPublic); + mockNodeDB->setSignerBit(REMOTE_NODE, true); meshtastic_NodeInfoLite_public_key_t storedKey = {0, {0}}; TEST_ASSERT_FALSE(nodeDB->copyPublicKey(REMOTE_NODE, storedKey)); From cbd2f291e5f92384d7b2dcc1bad690bcde2c46d7 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 26 Jul 2026 12:55:03 -0700 Subject: [PATCH 10/15] fix: initialize queue status state --- src/mesh/RadioInterface.h | 3 +-- src/mesh/RadioLibInterface.cpp | 2 +- src/mesh/Router.cpp | 3 +-- src/platform/portduino/SimRadio.cpp | 2 +- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/mesh/RadioInterface.h b/src/mesh/RadioInterface.h index 59542674658..c355fd387fe 100644 --- a/src/mesh/RadioInterface.h +++ b/src/mesh/RadioInterface.h @@ -174,8 +174,7 @@ class RadioInterface /** Return TX queue status */ [[nodiscard]] virtual meshtastic_QueueStatus getQueueStatus() { - meshtastic_QueueStatus qs; - qs.res = qs.mesh_packet_id = qs.free = qs.maxlen = 0; + meshtastic_QueueStatus qs = meshtastic_QueueStatus_init_zero; return qs; } diff --git a/src/mesh/RadioLibInterface.cpp b/src/mesh/RadioLibInterface.cpp index 5a9b292cda8..f109cdb0f1f 100644 --- a/src/mesh/RadioLibInterface.cpp +++ b/src/mesh/RadioLibInterface.cpp @@ -213,7 +213,7 @@ ErrorCode RadioLibInterface::send(meshtastic_MeshPacket *p) meshtastic_QueueStatus RadioLibInterface::getQueueStatus() { - meshtastic_QueueStatus qs; + meshtastic_QueueStatus qs = meshtastic_QueueStatus_init_zero; qs.res = qs.mesh_packet_id = 0; qs.free = txQueue.getFree(); diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index af5ea320401..13a75d638ab 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -322,8 +322,7 @@ void Router::setReceivedMessage() meshtastic_QueueStatus Router::getQueueStatus() { if (!iface) { - meshtastic_QueueStatus qs; - qs.res = qs.mesh_packet_id = qs.free = qs.maxlen = 0; + meshtastic_QueueStatus qs = meshtastic_QueueStatus_init_zero; return qs; } else return iface->getQueueStatus(); diff --git a/src/platform/portduino/SimRadio.cpp b/src/platform/portduino/SimRadio.cpp index 2786903ebbb..b6712301fb5 100644 --- a/src/platform/portduino/SimRadio.cpp +++ b/src/platform/portduino/SimRadio.cpp @@ -343,7 +343,7 @@ void SimRadio::startReceive(meshtastic_MeshPacket *p) meshtastic_QueueStatus SimRadio::getQueueStatus() { - meshtastic_QueueStatus qs; + meshtastic_QueueStatus qs = meshtastic_QueueStatus_init_zero; qs.res = qs.mesh_packet_id = 0; qs.free = txQueue.getFree(); From 96e388444f7473cca10201a6b49dc3d3b7c07818 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 26 Jul 2026 13:20:34 -0700 Subject: [PATCH 11/15] fix: harden deferred direct message recovery --- src/mesh/NodeDB.cpp | 4 +- src/mesh/Router.cpp | 28 ++++++- test/test_packet_signing/test_main.cpp | 102 +++++++++++++++++++++++-- 3 files changed, 122 insertions(+), 12 deletions(-) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index 8441c43fe6f..12bc3a9f627 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -3347,9 +3347,7 @@ bool NodeDB::updateUser(uint32_t nodeId, meshtastic_User &p, uint8_t channelInde // Only a signed update may change the identity of a node that has proven it signs; our own record is // exempt. Checked before getOrCreateMeshNode so a refused update cannot evict or write the warm tier. const meshtastic_NodeInfoLite *existing = getMeshNode(nodeId); - const bool hasWeakSignerKey = - existing && existing->public_key.size == 32 && memfll(existing->public_key.bytes, 0, sizeof(existing->public_key.bytes)); - if (nodeId != getNodeNum() && existing && nodeInfoLiteHasXeddsaSigned(existing) && !hasWeakSignerKey && !xeddsaSigned) { + if (nodeId != getNodeNum() && existing && nodeInfoLiteHasXeddsaSigned(existing) && !xeddsaSigned) { LOG_WARN("Refusing unsigned identity update for node 0x%08x that previously signed", nodeId); return false; } diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 13a75d638ab..703684dfda8 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -1213,6 +1213,12 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p) // has been committed to NodeDB. meshtastic_NodeInfoLite_public_key_t destKey = {0, {0}}; bool haveDestKey = nodeDB->copyPublicKey(p->to, destKey); + if (!haveDestKey && p->pki_encrypted && p->public_key.size == 32 && + !memfll(p->public_key.bytes, 0, sizeof(p->public_key.bytes))) { + destKey.size = p->public_key.size; + memcpy(destKey.bytes, p->public_key.bytes, destKey.size); + haveDestKey = true; + } if (!haveDestKey && p->pki_encrypted && p->decoded.portnum == meshtastic_PortNum_KEY_VERIFICATION_APP && crypto->getPendingPublicKey(p->to, destKey)) { haveDestKey = true; @@ -1326,14 +1332,30 @@ Router::DeferredDmResult Router::deferMissingKeyDm(meshtastic_MeshPacket *p) meshtastic_NodeInfoLite_public_key_t remoteKey = {0, {0}}; if (nodeDB->copyPublicKey(p->to, remoteKey) || !wouldEncryptWithPKC(p, p->channel, false)) return DeferredDmResult::NOT_APPLICABLE; + if (p->pki_encrypted && p->public_key.size == 32 && !memfll(p->public_key.bytes, 0, sizeof(p->public_key.bytes))) + return DeferredDmResult::NOT_APPLICABLE; + if (nodeDB->getLicenseStatus(p->to) == UserLicenseStatus::Licensed) { + LOG_INFO("Recipient 0x%08x is licensed; encrypted DM key exchange is unavailable", p->to); + return DeferredDmResult::NOT_APPLICABLE; + } + + PacketId keyExchangeId = 0; + for (const auto &deferred : deferredDms) { + if (deferred.p && deferred.reason == DeferredDm::Reason::DESTINATION_KEY && deferred.p->to == p->to) { + keyExchangeId = deferred.keyExchangeId; + break; + } + } for (auto &deferred : deferredDms) { if (deferred.p) continue; - const PacketId keyExchangeId = nodeInfoModule->requestNodeInfo(p->to, p->channel); - if (!keyExchangeId) - return DeferredDmResult::FAILED; + if (!keyExchangeId) { + keyExchangeId = nodeInfoModule->requestNodeInfo(p->to, p->channel); + if (!keyExchangeId) + return DeferredDmResult::FAILED; + } deferred.p = p; deferred.queuedAtMs = millis(); diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 6a42d27f939..0cc5eca1e85 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -103,6 +103,13 @@ class MockNodeDB : public NodeDB nodeInfoLiteSetBit(n, NODEINFO_BITFIELD_HAS_USER_MASK, true); } + void setLicensed(NodeNum num, bool value) + { + meshtastic_NodeInfoLite *n = getMeshNode(num); + TEST_ASSERT_NOT_NULL(n); + nodeInfoLiteSetBit(n, NODEINFO_BITFIELD_IS_LICENSED_MASK, value); + } + void setLongName(NodeNum num, const char *name) { meshtastic_NodeInfoLite *n = getMeshNode(num); @@ -1513,6 +1520,63 @@ void test_M2_unknown_dm_fails_only_after_key_exchange_timeout(void) #endif } +void test_M2a_known_licensed_recipient_does_not_start_key_exchange(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setHasUser(REMOTE_NODE); + mockNodeDB->setLicensed(REMOTE_NODE, true); + + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + const PacketId originalDmId = dm->id; + + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(0, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL(1, pipelineRouting->ackCalls); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_SEND_FAIL_PUBLIC_KEY, pipelineRouting->lastAckError); + TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRouting->lastAckId); +} + +void test_M2b_same_recipient_missing_key_shares_nodeinfo_request(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + + meshtastic_MeshPacket *first = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + meshtastic_MeshPacket *second = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(first); + TEST_ASSERT_NOT_NULL(second); + first->id = 0xD00D0022; + second->id = 0xD00D0023; + + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(first, RX_SRC_USER)); + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(second, RX_SRC_USER)); + TEST_ASSERT_EQUAL(2, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + + meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + nodeInfoResponse.decoded.request_id = pipelineRadio->sentPackets.front().id; + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + meshtastic_User responseUser = meshtastic_User_init_zero; + responseUser.is_licensed = owner.is_licensed; + responseUser.public_key.size = sizeof(remotePublic); + memcpy(responseUser.public_key.bytes, remotePublic, sizeof(remotePublic)); + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); + + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL_HEX32(0xD00D0022, pipelineRadio->sentPackets[1].id); + TEST_ASSERT_EQUAL_HEX32(0xD00D0023, pipelineRadio->sentPackets[2].id); +} + void test_M3_undecryptable_dm_reports_key_state_not_no_channel(void) { meshtastic_MeshPacket dm = meshtastic_MeshPacket_init_zero; @@ -1945,6 +2009,29 @@ void test_M14_explicit_destination_key_mismatch_fails_without_preflight(void) TEST_ASSERT_EQUAL(meshtastic_Routing_Error_PKI_FAILED, pipelineRouting->lastAckError); } +void test_M14a_explicit_destination_key_sends_without_nodedb_entry(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + dm->id = 0xD00D0024; + dm->pki_encrypted = true; + dm->public_key.size = sizeof(remotePublic); + memcpy(dm->public_key.bytes, remotePublic, sizeof(remotePublic)); + + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(dm, RX_SRC_USER)); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + TEST_ASSERT_TRUE(pipelineRadio->sentPackets.back().pki_encrypted); + TEST_ASSERT_NULL(mockNodeDB->getMeshNode(REMOTE_NODE)); +} + void test_M15_peer_key_preflight_queue_full_sends_known_key_dm(void) { enableNodeInfoForDmKeyWait(); @@ -2144,7 +2231,7 @@ void test_M19_deferred_dm_reports_the_resumed_send_result(void) pipelineRouter->clearPending(); } -void test_M20_weak_destination_key_refreshes_before_retry(void) +void test_M20_weak_signed_destination_key_is_not_replaced_by_unsigned_nodeinfo(void) { enableNodeInfoForDmKeyWait(); enablePkiForLocalNode(); @@ -2178,10 +2265,10 @@ void test_M20_weak_destination_key_refreshes_before_retry(void) memcpy(responseUser.public_key.bytes, remotePublic, sizeof(remotePublic)); TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); - TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); - TEST_ASSERT_EQUAL(2, pipelineRadio->sendCalls); - TEST_ASSERT_TRUE(pipelineRadio->sentPackets.back().pki_encrypted); - TEST_ASSERT_EQUAL_HEX32(originalDmId, pipelineRadio->sentPackets.back().id); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + TEST_ASSERT_FALSE(nodeDB->copyPublicKey(REMOTE_NODE, storedKey)); + TEST_ASSERT_EQUAL_HEX32(originalDmId, dm->id); } void test_M21_pki_admin_routing_reply_remains_pki_encrypted(void) @@ -2591,6 +2678,8 @@ void setup() RUN_TEST(test_M1_unknown_dm_waits_for_nodeinfo_key_exchange_then_retries); RUN_TEST(test_M1a_phone_origin_unknown_dm_waits_for_nodeinfo_key_exchange); RUN_TEST(test_M2_unknown_dm_fails_only_after_key_exchange_timeout); + RUN_TEST(test_M2a_known_licensed_recipient_does_not_start_key_exchange); + RUN_TEST(test_M2b_same_recipient_missing_key_shares_nodeinfo_request); RUN_TEST(test_M3_undecryptable_dm_reports_key_state_not_no_channel); RUN_TEST(test_M4_peer_key_preflight_retries_original_dm_after_nodeinfo); RUN_TEST(test_M5_peer_key_exchange_attempt_avoids_repeated_preflight_and_preserves_mismatch); @@ -2598,6 +2687,7 @@ void setup() RUN_TEST(test_M7_missing_recipient_key_fails_when_nodeinfo_cannot_queue); RUN_TEST(test_M13_two_peer_key_preflights_keep_independent_nodeinfo_requests); RUN_TEST(test_M14_explicit_destination_key_mismatch_fails_without_preflight); + RUN_TEST(test_M14a_explicit_destination_key_sends_without_nodedb_entry); RUN_TEST(test_M8_nodeinfo_reply_without_key_keeps_dm_deferred); RUN_TEST(test_M9_missing_recipient_key_fails_when_nodeinfo_send_is_rejected); RUN_TEST(test_M10_two_missing_keys_keep_independent_nodeinfo_requests); @@ -2608,7 +2698,7 @@ void setup() RUN_TEST(test_M17_peer_key_rotation_invalidates_peer_key_exchange_attempt); RUN_TEST(test_M18_same_peer_dms_share_nodeinfo_preflight); RUN_TEST(test_M19_deferred_dm_reports_the_resumed_send_result); - RUN_TEST(test_M20_weak_destination_key_refreshes_before_retry); + RUN_TEST(test_M20_weak_signed_destination_key_is_not_replaced_by_unsigned_nodeinfo); RUN_TEST(test_M21_pki_admin_routing_reply_remains_pki_encrypted); printf("\n=== Group N: NodeInfoModule authentication ===\n"); RUN_TEST(test_N1_unsigned_nodeinfo_from_signer_dropped); From 647125858d6c32606626adb4801242d3e8908080 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 26 Jul 2026 13:22:29 -0700 Subject: [PATCH 12/15] fix: retain direct message key exchange state --- src/mesh/Router.cpp | 12 ++++++--- test/test_packet_signing/test_main.cpp | 37 ++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 703684dfda8..389bf421648 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -385,7 +385,9 @@ bool Router::retryDeferredDmOnNodeInfo(const meshtastic_MeshPacket &p) LOG_INFO("NodeInfo exchange with 0x%08x completed; retrying deferred DM id=0x%08x", p.from, dm->id); rememberPeerKeyExchangeAttempt(dm->to); const ErrorCode result = send(dm); - service->sendQueueStatusToPhone(getQueueStatus(), result, dmId, meshtastic_QueueStatus_State_STATE_UNSPECIFIED); + const auto state = + isDeferredDm(dmId) ? meshtastic_QueueStatus_State_KEY_EXCHANGE : meshtastic_QueueStatus_State_STATE_UNSPECIFIED; + service->sendQueueStatusToPhone(getQueueStatus(), result, dmId, state); retried = true; } return retried; @@ -1514,7 +1516,9 @@ void Router::processDeferredDms() rememberPeerKeyExchangeAttempt(p->to); const PacketId dmId = p->id; const ErrorCode result = send(p); - service->sendQueueStatusToPhone(getQueueStatus(), result, dmId, meshtastic_QueueStatus_State_STATE_UNSPECIFIED); + const auto state = isDeferredDm(dmId) ? meshtastic_QueueStatus_State_KEY_EXCHANGE + : meshtastic_QueueStatus_State_STATE_UNSPECIFIED; + service->sendQueueStatusToPhone(getQueueStatus(), result, dmId, state); } continue; } @@ -1527,7 +1531,9 @@ void Router::processDeferredDms() LOG_INFO("Peer key learned for 0x%08x; retrying deferred DM id=0x%08x", p->to, p->id); const PacketId dmId = p->id; const ErrorCode result = send(p); - service->sendQueueStatusToPhone(getQueueStatus(), result, dmId, meshtastic_QueueStatus_State_STATE_UNSPECIFIED); + const auto state = + isDeferredDm(dmId) ? meshtastic_QueueStatus_State_KEY_EXCHANGE : meshtastic_QueueStatus_State_STATE_UNSPECIFIED; + service->sendQueueStatusToPhone(getQueueStatus(), result, dmId, state); } else if (!Throttle::isWithinTimespanMs(deferred.queuedAtMs, deferredDmKeyWaitMs)) { deferred.p = nullptr; deferred.queuedAtMs = 0; diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 0cc5eca1e85..982583bb342 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -2312,6 +2312,42 @@ void test_M21_pki_admin_routing_reply_remains_pki_encrypted(void) admin.drainReply(); } +void test_M22_redeferred_dm_reports_key_exchange_state(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePublic); + + meshtastic_MeshPacket *dm = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(dm); + dm->id = 0xD00D0025; + dm->want_ack = true; + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineService->sendToMesh(dm, RX_SRC_USER, false, true)); + meshtastic_MeshPacket nodeInfoRequest = pipelineRadio->sentPackets.back(); + while (meshtastic_QueueStatus *status = pipelineService->getQueueStatusForPhone()) + pipelineService->releaseQueueStatusToPool(status); + + mockNodeDB->clearTestNodes(); + meshtastic_MeshPacket nodeInfoResponse = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); + nodeInfoResponse.decoded.request_id = nodeInfoRequest.id; + meshtastic_User responseUser = meshtastic_User_init_zero; + responseUser.is_licensed = owner.is_licensed; + TEST_ASSERT_FALSE(dmKeyWaitNodeInfo->handleReceivedProtobuf(nodeInfoResponse, &responseUser)); + + meshtastic_QueueStatus *status = pipelineService->getQueueStatusForPhone(); + TEST_ASSERT_NOT_NULL(status); + TEST_ASSERT_EQUAL_HEX32(0xD00D0025, status->mesh_packet_id); + TEST_ASSERT_EQUAL(ERRNO_OK, status->res); + TEST_ASSERT_EQUAL(meshtastic_QueueStatus_State_KEY_EXCHANGE, status->state); + pipelineService->releaseQueueStatusToPool(status); + pipelineRouter->clearDeferredDmsForTest(); +} + // 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_N5_unsigned_unicast_nodeinfo_from_signer_does_not_change_name(void) @@ -2700,6 +2736,7 @@ void setup() RUN_TEST(test_M19_deferred_dm_reports_the_resumed_send_result); RUN_TEST(test_M20_weak_signed_destination_key_is_not_replaced_by_unsigned_nodeinfo); RUN_TEST(test_M21_pki_admin_routing_reply_remains_pki_encrypted); + RUN_TEST(test_M22_redeferred_dm_reports_key_exchange_state); printf("\n=== Group N: NodeInfoModule authentication ===\n"); RUN_TEST(test_N1_unsigned_nodeinfo_from_signer_dropped); RUN_TEST(test_N2_signed_nodeinfo_from_signer_not_dropped); From 141077855ccddc1b0c84dbdecd1e9980e734fc24 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 26 Jul 2026 13:24:54 -0700 Subject: [PATCH 13/15] fix: replay pending direct message status --- src/mesh/PhoneAPI.cpp | 4 +++ test/test_packet_signing/test_main.cpp | 41 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp index ff4003bd37a..d76b17f24ac 100644 --- a/src/mesh/PhoneAPI.cpp +++ b/src/mesh/PhoneAPI.cpp @@ -1775,6 +1775,10 @@ bool PhoneAPI::handleToRadioPacket(meshtastic_MeshPacket &p) #endif if (p.id > 0 && wasSeenRecently(p.id)) { LOG_DEBUG("Ignore packet from phone, already seen recently"); + if (router->isDeferredDm(p.id)) { + meshtastic_QueueStatus qs = router->getQueueStatus(); + service->sendQueueStatusToPhone(qs, ERRNO_OK, p.id, meshtastic_QueueStatus_State_KEY_EXCHANGE); + } return false; } diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 982583bb342..633980f1113 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -25,6 +25,7 @@ #include "mesh/MeshRadio.h" #include "mesh/MeshService.h" #include "mesh/NodeDB.h" +#include "mesh/PhoneAPI.h" #include "mesh/ReliableRouter.h" #include "mesh/Router.h" #include "mesh/SinglePortModule.h" @@ -239,6 +240,12 @@ class AuthPipelineMqtt : public MQTT } }; +class DmPhoneAPITestShim : public PhoneAPI +{ + protected: + bool checkIsConnected() override { return true; } +}; + static AuthPipelineRouter *pipelineRouter = nullptr; static AuthPipelineRadio *pipelineRadio = nullptr; static AuthPipelineRoutingModule *pipelineRouting = nullptr; @@ -2348,6 +2355,39 @@ void test_M22_redeferred_dm_reports_key_exchange_state(void) pipelineRouter->clearDeferredDmsForTest(); } +void test_M23_duplicate_phone_dm_replays_key_exchange_state(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + + meshtastic_ToRadio request = meshtastic_ToRadio_init_zero; + request.which_payload_variant = meshtastic_ToRadio_packet_tag; + request.packet = makeDecoded(0, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD); + request.packet.id = 0xD00D0026; + uint8_t requestBytes[meshtastic_ToRadio_size]; + const size_t requestSize = pb_encode_to_bytes(requestBytes, sizeof(requestBytes), &meshtastic_ToRadio_msg, &request); + TEST_ASSERT_GREATER_THAN(0, requestSize); + + DmPhoneAPITestShim api; + TEST_ASSERT_TRUE(api.handleToRadio(requestBytes, requestSize)); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + while (meshtastic_QueueStatus *status = pipelineService->getQueueStatusForPhone()) + pipelineService->releaseQueueStatusToPool(status); + + TEST_ASSERT_FALSE(api.handleToRadio(requestBytes, requestSize)); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + meshtastic_QueueStatus *status = pipelineService->getQueueStatusForPhone(); + TEST_ASSERT_NOT_NULL(status); + TEST_ASSERT_EQUAL_HEX32(0xD00D0026, status->mesh_packet_id); + TEST_ASSERT_EQUAL(ERRNO_OK, status->res); + TEST_ASSERT_EQUAL(meshtastic_QueueStatus_State_KEY_EXCHANGE, status->state); + pipelineService->releaseQueueStatusToPool(status); + api.close(); + pipelineRouter->clearDeferredDmsForTest(); +} + // 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_N5_unsigned_unicast_nodeinfo_from_signer_does_not_change_name(void) @@ -2737,6 +2777,7 @@ void setup() RUN_TEST(test_M20_weak_signed_destination_key_is_not_replaced_by_unsigned_nodeinfo); RUN_TEST(test_M21_pki_admin_routing_reply_remains_pki_encrypted); RUN_TEST(test_M22_redeferred_dm_reports_key_exchange_state); + RUN_TEST(test_M23_duplicate_phone_dm_replays_key_exchange_state); printf("\n=== Group N: NodeInfoModule authentication ===\n"); RUN_TEST(test_N1_unsigned_nodeinfo_from_signer_dropped); RUN_TEST(test_N2_signed_nodeinfo_from_signer_not_dropped); From e8a1ac4069938ff910dd634831cd93e4aa8b7147 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 26 Jul 2026 13:27:05 -0700 Subject: [PATCH 14/15] chore: pin pending DM status schema PR --- protobufs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protobufs b/protobufs index 9395d423d67..4322c0a45ac 160000 --- a/protobufs +++ b/protobufs @@ -1 +1 @@ -Subproject commit 9395d423d679acb61b4780b1b029720c31941b4d +Subproject commit 4322c0a45ac1b7c94dae1758a2c7616b3ac2bd05 From ae7592b629d0affb9a367f676134714d6c06412b Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 26 Jul 2026 15:25:57 -0700 Subject: [PATCH 15/15] fix: address deferred DM retry feedback --- src/mesh/Router.cpp | 61 ++++++++++++++++++++--- src/mesh/Router.h | 15 ++++++ test/test_packet_signing/test_main.cpp | 69 +++++++++++++++++++++++++- 3 files changed, 138 insertions(+), 7 deletions(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 389bf421648..a0c58e459aa 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -378,6 +378,9 @@ bool Router::retryDeferredDmOnNodeInfo(const meshtastic_MeshPacket &p) deferred = {}; } + if (retryCount) + clearDestinationKeyExchange(p.from, p.decoded.request_id); + bool retried = false; for (uint8_t i = 0; i < retryCount; ++i) { meshtastic_MeshPacket *dm = retries[i]; @@ -1354,9 +1357,13 @@ Router::DeferredDmResult Router::deferMissingKeyDm(meshtastic_MeshPacket *p) continue; if (!keyExchangeId) { - keyExchangeId = nodeInfoModule->requestNodeInfo(p->to, p->channel); - if (!keyExchangeId) - return DeferredDmResult::FAILED; + keyExchangeId = recentDestinationKeyExchange(p->to); + if (!keyExchangeId) { + keyExchangeId = nodeInfoModule->requestNodeInfo(p->to, p->channel); + if (!keyExchangeId) + return DeferredDmResult::FAILED; + rememberDestinationKeyExchange(p->to, keyExchangeId); + } } deferred.p = p; @@ -1386,6 +1393,13 @@ Router::DeferredDmResult Router::deferPeerKeyDm(meshtastic_MeshPacket *p, bool r memcmp(p->public_key.bytes, remoteKey.bytes, sizeof(remoteKey.bytes)) != 0) return DeferredDmResult::NOT_APPLICABLE; + for (auto &deferred : deferredDms) { + if (deferred.p == p && deferred.reason == DeferredDm::Reason::PEER_KEY && deferred.retryingAfterPeerKeyWait) { + deferred.retryingAfterPeerKeyWait = false; + return DeferredDmResult::NOT_APPLICABLE; + } + } + PacketId keyExchangeId = 0; for (const auto &deferred : deferredDms) { if (deferred.p && deferred.reason == DeferredDm::Reason::PEER_KEY && deferred.p->to == p->to) { @@ -1495,6 +1509,42 @@ void Router::rememberPeerKeyExchangeAttempt(NodeNum peer) crc32Buffer(remoteKey.bytes, remoteKey.size)}; } +PacketId Router::recentDestinationKeyExchange(NodeNum peer) +{ + for (auto &attempt : destinationKeyExchangeAttempts) { + if (attempt.peer != peer) + continue; + if (Throttle::isWithinTimespanMs(attempt.attemptedAtMs, deferredDmKeyWaitMs)) + return attempt.requestId; + attempt = {}; + return 0; + } + return 0; +} + +void Router::rememberDestinationKeyExchange(NodeNum peer, PacketId requestId) +{ + DestinationKeyExchangeAttempt *slot = &destinationKeyExchangeAttempts[0]; + for (auto &attempt : destinationKeyExchangeAttempts) { + if (attempt.peer == peer || attempt.peer == 0 || + !Throttle::isWithinTimespanMs(attempt.attemptedAtMs, deferredDmKeyWaitMs)) { + slot = &attempt; + break; + } + } + *slot = {peer, requestId, static_cast(millis())}; +} + +void Router::clearDestinationKeyExchange(NodeNum peer, PacketId requestId) +{ + for (auto &attempt : destinationKeyExchangeAttempts) { + if (attempt.peer == peer && attempt.requestId == requestId) { + attempt = {}; + return; + } + } +} + void Router::suppressRoutingDelivery(const meshtastic_MeshPacket &p) { suppressedRoutingDelivery = {p.from, p.id, p.decoded.request_id}; @@ -1509,13 +1559,12 @@ void Router::processDeferredDms() if (deferred.reason == DeferredDm::Reason::PEER_KEY) { if (!Throttle::isWithinTimespanMs(deferred.queuedAtMs, deferredDmPeerKeyWaitMs)) { - deferred.p = nullptr; - deferred.queuedAtMs = 0; - deferred.keyExchangeId = 0; LOG_INFO("Retrying deferred DM id=0x%08x after NodeInfo response wait for 0x%08x", p->id, p->to); rememberPeerKeyExchangeAttempt(p->to); const PacketId dmId = p->id; + deferred.retryingAfterPeerKeyWait = true; const ErrorCode result = send(p); + deferred = {}; const auto state = isDeferredDm(dmId) ? meshtastic_QueueStatus_State_KEY_EXCHANGE : meshtastic_QueueStatus_State_STATE_UNSPECIFIED; service->sendQueueStatusToPhone(getQueueStatus(), result, dmId, state); diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 207da479dd6..5fc96aa6632 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -241,6 +241,7 @@ class Router : protected concurrency::OSThread, protected PacketHistory uint32_t queuedAtMs = 0; PacketId keyExchangeId = 0; Reason reason = Reason::DESTINATION_KEY; + bool retryingAfterPeerKeyWait = false; }; static constexpr uint8_t deferredDmCapacity = 2; @@ -263,8 +264,17 @@ class Router : protected concurrency::OSThread, protected PacketHistory uint32_t peerKeyTag = 0; } peerKeyExchangeAttempts[8]; + struct DestinationKeyExchangeAttempt { + NodeNum peer = 0; + PacketId requestId = 0; + uint32_t attemptedAtMs = 0; + } destinationKeyExchangeAttempts[8]; + void processDeferredDms(); uint8_t deferredDmCount() const; + PacketId recentDestinationKeyExchange(NodeNum peer); + void rememberDestinationKeyExchange(NodeNum peer, PacketId requestId); + void clearDestinationKeyExchange(NodeNum peer, PacketId requestId); struct SuppressedRoutingDelivery { NodeNum from = 0; @@ -317,6 +327,11 @@ class Router : protected concurrency::OSThread, protected PacketHistory for (auto &attempt : peerKeyExchangeAttempts) attempt = {}; } + void resetDestinationKeyExchangeAttemptsForTest() + { + for (auto &attempt : destinationKeyExchangeAttempts) + attempt = {}; + } #endif #endif }; diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 633980f1113..31242b1ca2a 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -445,6 +445,7 @@ void setUp(void) pipelineRouter->clearDeferredDmsForTest(); pipelineRouter->resetPeerKeyRetriesForTest(); pipelineRouter->resetPeerKeyExchangeAttemptsForTest(); + pipelineRouter->resetDestinationKeyExchangeAttemptsForTest(); pipelineRouter->rxDupe = 0; pipelineRouter->txRelayCanceled = 0; pipelineRadio->reset(); @@ -1405,6 +1406,7 @@ static void enableNodeInfoForDmKeyWait() airTime = dmKeyWaitAirTime; pipelineRouter->resetPeerKeyRetriesForTest(); pipelineRouter->resetPeerKeyExchangeAttemptsForTest(); + pipelineRouter->resetDestinationKeyExchangeAttemptsForTest(); } void test_M1_unknown_dm_waits_for_nodeinfo_key_exchange_then_retries(void) @@ -2275,7 +2277,7 @@ void test_M20_weak_signed_destination_key_is_not_replaced_by_unsigned_nodeinfo(v TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); TEST_ASSERT_FALSE(nodeDB->copyPublicKey(REMOTE_NODE, storedKey)); - TEST_ASSERT_EQUAL_HEX32(originalDmId, dm->id); + TEST_ASSERT_TRUE(pipelineRouter->isDeferredDm(originalDmId)); } void test_M21_pki_admin_routing_reply_remains_pki_encrypted(void) @@ -2388,6 +2390,69 @@ void test_M23_duplicate_phone_dm_replays_key_exchange_state(void) pipelineRouter->clearDeferredDmsForTest(); } +void test_M24_two_peer_key_waits_retry_without_redeferring(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + uint8_t remotePublic[32], remotePrivate[32]; + crypto->generateKeyPair(remotePublic, remotePrivate); + crypto->setDHPrivateKey(config.security.private_key.bytes); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePublic); + + meshtastic_MeshPacket *first = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + meshtastic_MeshPacket *second = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(first); + TEST_ASSERT_NOT_NULL(second); + first->id = 0xD00D0029; + second->id = 0xD00D002A; + first->want_ack = second->want_ack = true; + + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(first, RX_SRC_USER)); + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(second, RX_SRC_USER)); + TEST_ASSERT_EQUAL(2, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + + pipelineRouter->retryDeferredDmsForTest(); + pipelineRouter->processDeferredDmsForTest(); + + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(2, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL(3, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL_HEX32(0xD00D0029, pipelineRadio->sentPackets[1].id); + TEST_ASSERT_EQUAL_HEX32(0xD00D002A, pipelineRadio->sentPackets[2].id); + pipelineRouter->clearPending(); +} + +void test_M25_destination_key_recovery_reuses_recent_nodeinfo_request(void) +{ + enableNodeInfoForDmKeyWait(); + enablePkiForLocalNode(); + + meshtastic_MeshPacket *first = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(first); + first->id = 0xD00D002B; + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(first, RX_SRC_USER)); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + + pipelineRouter->expireDeferredDmsForTest(); + pipelineRouter->processDeferredDmsForTest(); + TEST_ASSERT_EQUAL(0, pipelineRouter->deferredDmPending()); + + meshtastic_MeshPacket *second = + packetPool.allocCopy(makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(second); + second->id = 0xD00D002C; + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(second, RX_SRC_USER)); + TEST_ASSERT_EQUAL(1, pipelineRouter->deferredDmPending()); + TEST_ASSERT_EQUAL(1, pipelineRadio->sendCalls); + pipelineRouter->clearDeferredDmsForTest(); +} + // 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_N5_unsigned_unicast_nodeinfo_from_signer_does_not_change_name(void) @@ -2778,6 +2843,8 @@ void setup() RUN_TEST(test_M21_pki_admin_routing_reply_remains_pki_encrypted); RUN_TEST(test_M22_redeferred_dm_reports_key_exchange_state); RUN_TEST(test_M23_duplicate_phone_dm_replays_key_exchange_state); + RUN_TEST(test_M24_two_peer_key_waits_retry_without_redeferring); + RUN_TEST(test_M25_destination_key_recovery_reuses_recent_nodeinfo_request); printf("\n=== Group N: NodeInfoModule authentication ===\n"); RUN_TEST(test_N1_unsigned_nodeinfo_from_signer_dropped); RUN_TEST(test_N2_signed_nodeinfo_from_signer_not_dropped);