From d6b12ea3f1fc83afcff63a7fb511d54c52cc871a Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Thu, 9 Jul 2026 13:36:10 -0700 Subject: [PATCH 1/8] feat(security): enforce packet authenticity policies --- src/main.cpp | 5 +- src/mesh/FloodingRouter.cpp | 37 +- src/mesh/FloodingRouter.h | 4 +- src/mesh/NextHopRouter.cpp | 26 +- src/mesh/NextHopRouter.h | 3 +- src/mesh/NodeDB.cpp | 16 + src/mesh/NodeDB.h | 4 + src/mesh/Router.cpp | 262 +++++- src/mesh/Router.h | 27 +- src/mesh/WarmNodeStore.h | 2 +- src/mesh/udp/UdpMulticastHandler.h | 6 +- src/modules/AdminModule.cpp | 10 + src/modules/NodeInfoModule.cpp | 11 +- src/mqtt/MQTT.cpp | 15 +- test/test_mqtt/MQTT.cpp | 59 ++ test/test_packet_signing/test_main.cpp | 806 ++++++++++++++++-- variants/stm32/CDEBYTE_E77-MBL/platformio.ini | 5 +- variants/stm32/rak3172/platformio.ini | 3 + variants/stm32/stm32.ini | 4 +- 19 files changed, 1136 insertions(+), 169 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 63cbf35ab..7161ceff6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1206,7 +1206,7 @@ bool runASAP; // TODO find better home than main.cpp extern meshtastic_DeviceMetadata getDeviceMetadata() { - meshtastic_DeviceMetadata deviceMetadata; + meshtastic_DeviceMetadata deviceMetadata = meshtastic_DeviceMetadata_init_zero; strncpy(deviceMetadata.firmware_version, optstr(APP_VERSION), sizeof(deviceMetadata.firmware_version)); deviceMetadata.device_state_version = DEVICESTATE_CUR_VER; deviceMetadata.canShutdown = pmu_found || HAS_CPU_SHUTDOWN; @@ -1262,6 +1262,9 @@ extern meshtastic_DeviceMetadata getDeviceMetadata() #if !(MESHTASTIC_EXCLUDE_PKI) deviceMetadata.hasPKC = true; +#endif +#if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA) + deviceMetadata.has_xeddsa = true; #endif return deviceMetadata; } diff --git a/src/mesh/FloodingRouter.cpp b/src/mesh/FloodingRouter.cpp index 13f98299f..7048cd918 100644 --- a/src/mesh/FloodingRouter.cpp +++ b/src/mesh/FloodingRouter.cpp @@ -51,8 +51,8 @@ bool FloodingRouter::shouldFilterReceived(const meshtastic_MeshPacket *p) LOG_DEBUG("Repeated reliable tx"); // Check if it's still in the Tx queue, if not, we have to relay it again if (!findInTxQueue(p->from, p->id)) { - reprocessPacket(p); - perhapsRebroadcast(p); + if (reprocessPacket(p)) + perhapsRebroadcast(p); } } else { perhapsCancelDupe(p); @@ -68,6 +68,12 @@ bool FloodingRouter::perhapsHandleUpgradedPacket(const meshtastic_MeshPacket *p) { // isRebroadcaster() is duplicated in perhapsRebroadcast(), but this avoids confusing log messages if (isRebroadcaster() && iface && p->hop_limit > 0) { + // Verify the replacement before deleting the valid lower-hop copy waiting in the TX queue. + // This is intentionally redundant with ReliableRouter's ingress gate: it keeps this helper + // safe if another caller is introduced later. + if (passesRoutingAuthGate(const_cast(p)) != RoutingAuthVerdict::ACCEPT) + return true; + // If we overhear a duplicate copy of the packet with more hops left than the one we are waiting to // rebroadcast, then remove the packet currently sitting in the TX queue and use this one instead. uint8_t dropThreshold = p->hop_limit; // remove queued packets that have fewer hops remaining @@ -75,7 +81,8 @@ bool FloodingRouter::perhapsHandleUpgradedPacket(const meshtastic_MeshPacket *p) LOG_DEBUG("Processing upgraded packet 0x%08x for rebroadcast with hop limit %d (dropping queued < %d)", p->id, p->hop_limit, dropThreshold); - reprocessPacket(p); + if (!reprocessPacket(p)) + return true; perhapsRebroadcast(p); rxDupe++; @@ -87,32 +94,24 @@ bool FloodingRouter::perhapsHandleUpgradedPacket(const meshtastic_MeshPacket *p) return false; } -void FloodingRouter::reprocessPacket(const meshtastic_MeshPacket *p) +bool FloodingRouter::reprocessPacket(const meshtastic_MeshPacket *p) { + if (p->which_payload_variant != meshtastic_MeshPacket_decoded_tag) { + auto decodedState = perhapsDecode(const_cast(p)); + if (decodedState != DecodeState::DECODE_SUCCESS && decodedState != DecodeState::DECODE_OPAQUE) + return false; + } + if (nodeDB) nodeDB->updateFrom(*p); #if !MESHTASTIC_EXCLUDE_TRACEROUTE - if (traceRouteModule && p->which_payload_variant != meshtastic_MeshPacket_decoded_tag) { - // If we got a packet that is not decoded, try to decode it so we can check for traceroute. - auto decodedState = perhapsDecode(const_cast(p)); - if (decodedState == DecodeState::DECODE_SUCCESS) { - // parsing was successful, print for debugging - printPacket("reprocessPacket(DUP)", p); - } else { - // Fatal decoding error, we can't do anything with this packet - LOG_WARN( - "FloodingRouter::reprocessPacket: Fatal decode error (state=%d, id=0x%08x, from=%u), can't check for traceroute", - static_cast(decodedState), p->id, getFrom(p)); - return; - } - } - if (traceRouteModule && p->which_payload_variant == meshtastic_MeshPacket_decoded_tag && p->decoded.portnum == meshtastic_PortNum_TRACEROUTE_APP) { traceRouteModule->processUpgradedPacket(*p); } #endif + return true; } bool FloodingRouter::roleAllowsCancelingDupe(const meshtastic_MeshPacket *p) diff --git a/src/mesh/FloodingRouter.h b/src/mesh/FloodingRouter.h index e8a2e9685..6bd5fca41 100644 --- a/src/mesh/FloodingRouter.h +++ b/src/mesh/FloodingRouter.h @@ -64,7 +64,7 @@ class FloodingRouter : public Router bool perhapsHandleUpgradedPacket(const meshtastic_MeshPacket *p); /* Call when we receive a packet that needs some reprocessing, but afterwards should be filtered */ - void reprocessPacket(const meshtastic_MeshPacket *p); + bool reprocessPacket(const meshtastic_MeshPacket *p); // Return false for roles like ROUTER which should always rebroadcast even when we've heard another rebroadcast of // the same packet @@ -75,4 +75,4 @@ class FloodingRouter : public Router // Return true if we are a rebroadcaster bool isRebroadcaster(); -}; \ No newline at end of file +}; diff --git a/src/mesh/NextHopRouter.cpp b/src/mesh/NextHopRouter.cpp index f33de779c..3e7361f19 100644 --- a/src/mesh/NextHopRouter.cpp +++ b/src/mesh/NextHopRouter.cpp @@ -11,6 +11,25 @@ NextHopRouter::NextHopRouter() {} +bool NextHopRouter::relayOpaquePacket(const meshtastic_MeshPacket *p) +{ + // Opaque traffic is never admitted to PacketHistory, NodeDB, modules, phone, MQTT, or ACK + // handling. Relay only from the immutable outer routing header and let hop exhaustion bound it. + const auto mode = config.device.rebroadcast_mode; + if (!iface || isToUs(p) || isFromUs(p) || p->id == 0 || p->hop_limit == 0 || !isRebroadcaster() || owner.is_licensed || + !IS_ONE_OF(mode, meshtastic_Config_DeviceConfig_RebroadcastMode_ALL, + meshtastic_Config_DeviceConfig_RebroadcastMode_ALL_SKIP_DECODING) || + (p->next_hop != NO_NEXT_HOP_PREFERENCE && p->next_hop != nodeDB->getLastByteOfNodeNum(getNodeNum()))) + return false; + + meshtastic_MeshPacket *relay = packetPool.allocCopy(*p); + if (!relay) + return false; + relay->hop_limit--; + relay->relay_node = nodeDB->getLastByteOfNodeNum(getNodeNum()); + return Router::send(relay) == ERRNO_OK; +} + PendingPacket::PendingPacket(meshtastic_MeshPacket *p, uint8_t numRetransmissions) { packet = p; @@ -65,16 +84,15 @@ bool NextHopRouter::shouldFilterReceived(const meshtastic_MeshPacket *p) LOG_INFO("Fallback to flooding from relay_node=0x%x", p->relay_node); // Check if it's still in the Tx queue, if not, we have to relay it again if (!findInTxQueue(p->from, p->id)) { - reprocessPacket(p); - perhapsRebroadcast(p); + if (reprocessPacket(p)) + perhapsRebroadcast(p); } } else { bool isRepeated = getHopsAway(*p) == 0; // If repeated and not in Tx queue anymore, try relaying again, or if we are the destination, send the ACK again if (isRepeated) { if (!findInTxQueue(p->from, p->id)) { - reprocessPacket(p); - if (!perhapsRebroadcast(p) && isToUs(p) && p->want_ack) { + if (reprocessPacket(p) && !perhapsRebroadcast(p) && isToUs(p) && p->want_ack) { sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, p->channel, 0); } } diff --git a/src/mesh/NextHopRouter.h b/src/mesh/NextHopRouter.h index 467d9ca68..d5e2a9621 100644 --- a/src/mesh/NextHopRouter.h +++ b/src/mesh/NextHopRouter.h @@ -137,6 +137,7 @@ class NextHopRouter : public FloodingRouter * @return true to abandon the packet */ virtual bool shouldFilterReceived(const meshtastic_MeshPacket *p) override; + bool relayOpaquePacket(const meshtastic_MeshPacket *p) override; /** * Look for packets we need to relay @@ -209,4 +210,4 @@ class NextHopRouter : public FloodingRouter /** Check if we should be rebroadcasting this packet if so, do so. * @return true if we did rebroadcast */ bool perhapsRebroadcast(const meshtastic_MeshPacket *p) override; -}; \ No newline at end of file +}; diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index e05f8b8f0..0abc2eff7 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -1855,6 +1855,8 @@ bool NodeDB::enforceSatelliteCaps() // them if they do); otherwise tracker/sensor/tak_tracker are role-protected. static uint8_t warmProtectedCategory(const meshtastic_NodeInfoLite &n) { + if (nodeInfoLiteHasXeddsaSigned(&n)) + return static_cast(WarmProtected::XeddsaSigner); if (n.bitfield & (NODEINFO_BITFIELD_IS_FAVORITE_MASK | NODEINFO_BITFIELD_IS_IGNORED_MASK | NODEINFO_BITFIELD_IS_KEY_MANUALLY_VERIFIED_MASK)) return static_cast(WarmProtected::Flag); @@ -3719,6 +3721,18 @@ bool NodeDB::copyPublicKey(NodeNum n, meshtastic_NodeInfoLite_public_key_t &out) return false; } +bool NodeDB::hasSeenXeddsaSigner(NodeNum n) +{ + if (nodeInfoLiteHasXeddsaSigned(getMeshNode(n))) + return true; +#if WARM_NODE_COUNT > 0 + uint8_t role = 0, prot = 0; + return warmStore.lookupMeta(n, role, prot) && prot == static_cast(WarmProtected::XeddsaSigner); +#else + return false; +#endif +} + meshtastic_Config_DeviceConfig_Role NodeDB::getNodeRole(NodeNum n) { const meshtastic_NodeInfoLite *info = getMeshNode(n); @@ -3811,6 +3825,8 @@ meshtastic_NodeInfoLite *NodeDB::getOrCreateMeshNode(NodeNum n) lite->public_key.size = 32; memcpy(lite->public_key.bytes, warm.public_key, 32); } + if (warmProtOf(warm) == static_cast(WarmProtected::XeddsaSigner)) + nodeInfoLiteSetBit(lite, NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK, true); LOG_MIGRATION("Rehydrated node 0x%08x from warm tier (key=%d)", n, lite->public_key.size == 32); } #endif diff --git a/src/mesh/NodeDB.h b/src/mesh/NodeDB.h index 8aa32dcc7..34c4d33c4 100644 --- a/src/mesh/NodeDB.h +++ b/src/mesh/NodeDB.h @@ -357,6 +357,10 @@ class NodeDB /// tier. Returns false if we don't know a key for n. bool copyPublicKey(NodeNum n, meshtastic_NodeInfoLite_public_key_t &out); + /// Whether this node has produced a verified XEdDSA signature, including while its + /// identity is resident only in the warm tier. + bool hasSeenXeddsaSigner(NodeNum n); + /// Resolve a node's device role - hot store (with user) first, then the role /// cached in the warm tier, else CLIENT. Lets role-aware policy keep firing for /// nodes that have aged out of the hot store. diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 67d724433..c9688240c 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -12,6 +12,8 @@ #include "mesh-pb-constants.h" #include "meshUtils.h" #include "modules/RoutingModule.h" +#include +#include #include #if HAS_TRAFFIC_MANAGEMENT #include "modules/TrafficManagementModule.h" @@ -67,6 +69,79 @@ Allocator &packetPool = staticPool; static uint8_t bytes[MAX_LORA_PAYLOAD_LEN + 1] __attribute__((__aligned__)); +struct RoutingAuthCache { + bool valid = false; + meshtastic_Config_SecurityConfig_PacketSignaturePolicy policy = + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_BALANCED; + meshtastic_MeshPacket wire = meshtastic_MeshPacket_init_zero; + meshtastic_MeshPacket authenticated = meshtastic_MeshPacket_init_zero; +}; +static RoutingAuthCache routingAuthCache; +static concurrency::Lock *routingAuthCacheLock; +static uint32_t routingAuthEvaluations; + +static bool routingAuthCacheMatches(const meshtastic_MeshPacket &packet) +{ + if (!routingAuthCacheLock) + return false; + concurrency::LockGuard guard(routingAuthCacheLock); + if (!routingAuthCache.valid) + return false; + if (routingAuthCache.policy != config.security.packet_signature_policy || + memcmp(&routingAuthCache.wire, &packet, sizeof(packet)) != 0) { + routingAuthCache.valid = false; + return false; + } + return true; +} + +static void storeRoutingAuthCache(const meshtastic_MeshPacket &wire, const meshtastic_MeshPacket &authenticated) +{ + concurrency::LockGuard guard(routingAuthCacheLock); + routingAuthCache.wire = wire; + routingAuthCache.authenticated = authenticated; + routingAuthCache.policy = config.security.packet_signature_policy; + routingAuthCache.valid = true; +} + +static bool applyRoutingAuthCache(meshtastic_MeshPacket *packet) +{ + if (!routingAuthCacheLock) + return false; + concurrency::LockGuard guard(routingAuthCacheLock); + if (!routingAuthCache.valid || routingAuthCache.policy != config.security.packet_signature_policy || + memcmp(&routingAuthCache.wire, packet, sizeof(*packet)) != 0) { + routingAuthCache.valid = false; + return false; + } + *packet = routingAuthCache.authenticated; + routingAuthCache.valid = false; + return true; +} + +static void clearRoutingAuthCache() +{ + if (!routingAuthCacheLock) + return; + concurrency::LockGuard guard(routingAuthCacheLock); + routingAuthCache.valid = false; +} + +#ifdef PIO_UNIT_TESTING +uint32_t routingAuthEvaluationCount() +{ + return routingAuthEvaluations; +} +void resetRoutingAuthEvaluationCount() +{ + routingAuthEvaluations = 0; + if (routingAuthCacheLock) { + concurrency::LockGuard guard(routingAuthCacheLock); + routingAuthCache.valid = false; + } +} +#endif + /** * Constructor * @@ -85,6 +160,8 @@ Router::Router() : concurrency::OSThread("Router"), fromRadioQueue(MAX_RX_FROMRA // init Lockguard for crypt operations assert(!cryptLock); cryptLock = new concurrency::Lock(); + if (!routingAuthCacheLock) + routingAuthCacheLock = new concurrency::Lock(); } bool Router::shouldDecrementHopLimit(const meshtastic_MeshPacket *p) @@ -247,8 +324,10 @@ ErrorCode Router::sendLocal(meshtastic_MeshPacket *p, RxSource src) // No need to deliver externally if the destination is the local node if (isToUs(p)) { printPacket("Enqueued local", p); - enqueueReceivedMessage(p); - return ERRNO_OK; + // Preserve the trusted origin explicitly. Queueing used to erase src and make a local + // phone/module packet indistinguishable from remote already-decoded ingress. + handleReceived(p, src); + return ERRNO_SHOULD_RELEASE; } else if (!iface) { // We must be sending to remote nodes also, fail if no interface found abortSendAndNak(meshtastic_Routing_Error_NO_INTERFACE, p); @@ -452,18 +531,55 @@ void Router::sniffReceived(const meshtastic_MeshPacket *p, const meshtastic_Rout } #if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA) +enum class NodeInfoBootstrapResult { NOT_APPLICABLE, VERIFIED, INVALID }; + +static NodeInfoBootstrapResult verifyFirstContactNodeInfo(meshtastic_MeshPacket *p) +{ + if (p->decoded.portnum != meshtastic_PortNum_NODEINFO_APP) + return NodeInfoBootstrapResult::NOT_APPLICABLE; + + meshtastic_User user = meshtastic_User_init_zero; + if (!pb_decode_from_bytes(p->decoded.payload.bytes, p->decoded.payload.size, &meshtastic_User_msg, &user) || + user.public_key.size != 32 || crc32Buffer(user.public_key.bytes, user.public_key.size) != p->from || + !crypto->xeddsa_verify(user.public_key.bytes, p->from, p->id, p->decoded.portnum, p->decoded.payload.bytes, + p->decoded.payload.size, p->decoded.xeddsa_signature.bytes)) { + return NodeInfoBootstrapResult::INVALID; + } + + meshtastic_NodeInfoLite *node = nodeDB->getOrCreateMeshNode(p->from); + if (!node) + return NodeInfoBootstrapResult::INVALID; + node->public_key.size = user.public_key.size; + memcpy(node->public_key.bytes, user.public_key.bytes, user.public_key.size); + nodeInfoLiteSetBit(node, NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK, true); + p->xeddsa_signed = true; + LOG_DEBUG("Verified first-contact XEdDSA NodeInfo from 0x%08x", p->from); + return NodeInfoBootstrapResult::VERIFIED; +} + bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p, size_t encodedDataSize) { + const auto policy = config.security.packet_signature_policy; + const bool strict = policy == meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT; + const bool compatible = policy == meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_COMPATIBLE; + // Only a signature we verify below may mark this packet signed; never trust an inbound flag. p->xeddsa_signed = false; if (p->decoded.xeddsa_signature.size == XEDDSA_SIGNATURE_SIZE) { + meshtastic_NodeInfoLite_public_key_t senderKey = {0, {0}}; meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(p->from); - if (node && node->public_key.size == 32) { + if (nodeDB->copyPublicKey(p->from, senderKey)) { p->xeddsa_signed = - crypto->xeddsa_verify(node->public_key.bytes, p->from, p->id, p->decoded.portnum, p->decoded.payload.bytes, + crypto->xeddsa_verify(senderKey.bytes, p->from, p->id, p->decoded.portnum, p->decoded.payload.bytes, p->decoded.payload.size, p->decoded.xeddsa_signature.bytes); if (p->xeddsa_signed) { // Learn this node as a signer, so a later unsigned signable broadcast from it is dropped + // A warm-tier key must be re-admitted before setting the signer bit; otherwise Balanced + // forgets downgrade protection as soon as the node is evicted from the hot store. + if (!node) + node = nodeDB->getOrCreateMeshNode(p->from); + if (!node) + return false; nodeInfoLiteSetBit(node, NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK, true); LOG_DEBUG("Verified XEdDSA signature from 0x%08x", p->from); } else { @@ -471,7 +587,16 @@ bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p, size_t encodedDataSize) return false; } } else { + const auto bootstrap = verifyFirstContactNodeInfo(p); + if (bootstrap == NodeInfoBootstrapResult::INVALID) { + LOG_WARN("Invalid first-contact XEdDSA NodeInfo from 0x%08x, dropping", p->from); + return false; + } + if (bootstrap == NodeInfoBootstrapResult::VERIFIED) + return true; LOG_DEBUG("No public key for 0x%08x, cannot verify XEdDSA signature", p->from); + if (strict) + return false; } } else if (p->decoded.xeddsa_signature.size != 0) { // A signature field that is neither empty nor a full 64 bytes is malformed - honest @@ -482,15 +607,22 @@ bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p, size_t encodedDataSize) p->from); return false; } else { - // Truly unsigned (signature size 0) - only reject the class a signing node always signs: a - // non-PKI broadcast whose signed encoding would still fit the LoRa frame. encodedDataSize is + if (p->pki_encrypted) + return true; + if (strict) { + LOG_WARN("Dropping unsigned packet from 0x%08x in Strict signature mode", p->from); + return false; + } + if (compatible) + return true; + + // In Balanced, preserve legacy unsigned-unicast compatibility and only reject a signable + // unsigned broadcast from a known signer. encodedDataSize is // the size of the encoded Data exactly as the sender built it (or 0 to size p->decoded // canonically); with no signature field present it is the unsigned base, and adding // XEDDSA_SIGNATURE_FIELD_BYTES mirrors the sender-side signedDataFits() gate per packet, - // whatever fields the Data carried. Unicast/PKI packets and broadcasts too big to carry a - // signature are never signed, so they must not be hard-failed here even for a known signer. - const meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(p->from); - if (node && nodeInfoLiteHasXeddsaSigned(node) && !p->pki_encrypted && isBroadcast(p->to)) { + // whatever fields the Data carried. Oversized broadcasts remain compatible. + if (nodeDB->hasSeenXeddsaSigner(p->from) && isBroadcast(p->to)) { if (encodedDataSize == 0 && !pb_get_encoded_size(&encodedDataSize, &meshtastic_Data_msg, &p->decoded)) return true; // can't size it; never drop on a sizing failure if (encodedDataSize + XEDDSA_SIGNATURE_FIELD_BYTES + MESHTASTIC_HEADER_LENGTH <= MAX_LORA_PAYLOAD_LEN) { @@ -503,6 +635,55 @@ bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p, size_t encodedDataSize) } #endif +RoutingAuthVerdict passesRoutingAuthGate(meshtastic_MeshPacket *p) +{ + // Routing still needs the original encrypted representation for byte-for-byte relay and for + // MQTT uplink. Authenticate a copy here; handleReceived() performs the normal in-place decode + // only after stateful routing filters have completed. + if (routingAuthCacheMatches(*p)) + return RoutingAuthVerdict::ACCEPT; + + meshtastic_MeshPacket wire = *p; + meshtastic_MeshPacket authCandidate = *p; + routingAuthEvaluations++; + if (authCandidate.which_payload_variant == meshtastic_MeshPacket_decoded_tag) { + // Already-decoded remote ingress (notably Portduino SimRadio) did not pass through a + // decryptor. Never trust serialized local authentication metadata on that boundary. + authCandidate.pki_encrypted = false; + authCandidate.public_key.size = 0; +#if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA) + concurrency::LockGuard g(cryptLock); + if (!checkXeddsaReceivePolicy(&authCandidate)) { + LOG_WARN("Already-decoded packet rejected by signature policy before routing state update"); + return RoutingAuthVerdict::REJECT; + } +#endif + p->xeddsa_signed = authCandidate.xeddsa_signed; + wire = *p; + storeRoutingAuthCache(wire, authCandidate); + return RoutingAuthVerdict::ACCEPT; + } + const DecodeState state = perhapsDecode(&authCandidate); + if (state == DecodeState::DECODE_POLICY_REJECT) { + LOG_WARN("Packet rejected by signature policy before routing state update"); + return RoutingAuthVerdict::REJECT; + } + if (state == DecodeState::DECODE_FATAL) { + LOG_WARN("Fatal decode error before routing state update"); + return RoutingAuthVerdict::REJECT; + } + if (state == DecodeState::DECODE_FAILURE) { + LOG_WARN("Decryptable packet failed decoding/authentication before routing state update"); + return RoutingAuthVerdict::REJECT; + } + + // Only an explicit unknown-channel result remains eligible for opaque relay. + if (state == DecodeState::DECODE_OPAQUE) + return RoutingAuthVerdict::OPAQUE_RELAY_ONLY; + storeRoutingAuthCache(wire, authCandidate); + return RoutingAuthVerdict::ACCEPT; +} + DecodeState perhapsDecode(meshtastic_MeshPacket *p) { concurrency::LockGuard g(cryptLock); @@ -516,12 +697,18 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) if (p->which_payload_variant == meshtastic_MeshPacket_decoded_tag) return DecodeState::DECODE_SUCCESS; // If packet was already decoded just return + // Authentication metadata is local-only. Re-establish it below only after successful PKI decryption. + p->pki_encrypted = false; + p->public_key.size = 0; + size_t rawSize = p->encrypted.size; if (rawSize > sizeof(bytes)) { LOG_ERROR("Packet too large to attempt decryption! (rawSize=%d > 256)", rawSize); return DecodeState::DECODE_FATAL; } bool decrypted = false; + bool pkiAttempted = false; + bool matchedChannel = false; ChannelIndex chIndex = 0; #if !(MESHTASTIC_EXCLUDE_PKI) // Attempt PKI decryption first. The sender's key may come from the hot @@ -531,6 +718,7 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) if (p->channel == 0 && isToUs(p) && p->to > 0 && !isBroadcast(p->to) && nodeDB->copyPublicKey(p->from, fromKey) && nodeDB->getMeshNode(p->to) != nullptr && nodeDB->getMeshNode(p->to)->public_key.size > 0 && rawSize > MESHTASTIC_PKC_OVERHEAD) { + pkiAttempted = true; LOG_DEBUG("Attempt PKI decryption"); if (crypto->decryptCurve25519(p->from, fromKey, p->id, rawSize, p->encrypted.bytes, bytes)) { @@ -564,6 +752,7 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) for (chIndex = 0; chIndex < channels.getNumChannels(); chIndex++) { // Try to use this hash/channel pair if (channels.decryptForHash(chIndex, p->channel)) { + matchedChannel = true; // we have to copy into a scratch buffer, because these bytes are a union with the decoded protobuf. Create a // fresh copy for each decrypt attempt. memcpy(bytes, p->encrypted.bytes, rawSize); @@ -605,7 +794,7 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) // MESHTASTIC_PKC_OVERHEAD subtraction preserves that, and PKI packets are unicast so the // downgrade predicate ignores them anyway). if (!checkXeddsaReceivePolicy(p, rawSize)) - return DecodeState::DECODE_FAILURE; + return DecodeState::DECODE_POLICY_REJECT; #endif /* Not actually ever used. @@ -660,7 +849,7 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) return DecodeState::DECODE_SUCCESS; } else { LOG_WARN("No suitable channel found for decoding, hash was 0x%x!", p->channel); - return DecodeState::DECODE_FAILURE; + return (matchedChannel || pkiAttempted) ? DecodeState::DECODE_FAILURE : DecodeState::DECODE_OPAQUE; } } @@ -851,8 +1040,6 @@ NodeNum Router::getNodeNum() void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src) { bool skipHandle = false; - // Also, we should set the time from the ISR and it should have msec level resolution - p->rx_time = getValidTime(RTCQualityFromNet); // store the arrival timestamp for the phone // Store a copy of the encrypted packet for MQTT. // Local, not a class member: handleReceived re-enters itself when a module @@ -863,12 +1050,31 @@ void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src) meshtastic_MeshPacket *p_encrypted = packetPool.allocCopy(*p); DEBUG_HEAP_AFTER("Router::handleReceived", p_encrypted); + // Consume the decoded/authenticated handoff after preserving the exact encrypted packet and + // before mutating any packet fields that participate in the exact cache match. + if (src == RX_SRC_RADIO) + applyRoutingAuthCache(p); + + // Also, we should set the time from the ISR and it should have msec level resolution. + // Keep the decoded working packet and encrypted MQTT copy on the same local arrival timestamp. + const uint32_t rxTime = getValidTime(RTCQualityFromNet); + p->rx_time = rxTime; + if (p_encrypted) + p_encrypted->rx_time = rxTime; + // Take those raw bytes and convert them back into a well structured protobuf we can understand auto decodedState = perhapsDecode(p); - if (decodedState == DecodeState::DECODE_FATAL) { + if (decodedState == DecodeState::DECODE_FATAL || decodedState == DecodeState::DECODE_POLICY_REJECT || + decodedState == DecodeState::DECODE_FAILURE) { // Fatal decoding error, we can't do anything with this packet - LOG_WARN("Fatal decode error, dropping packet"); - cancelSending(p->from, p->id); + LOG_WARN(decodedState == DecodeState::DECODE_POLICY_REJECT + ? "Packet rejected by signature policy" + : (decodedState == DecodeState::DECODE_FATAL ? "Fatal decode error, dropping packet" + : "Decryptable packet failed decoding, dropping packet")); + // A policy rejection is attacker-controlled input and must not cancel a valid pending + // transmission with the same (from, id). Preserve the pre-existing fatal-decode behavior. + if (decodedState == DecodeState::DECODE_FATAL) + cancelSending(p->from, p->id); skipHandle = true; } else if (decodedState == DecodeState::DECODE_SUCCESS) { // parsing was successful, queue for our recipient @@ -932,7 +1138,7 @@ void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src) } else { // Mark as pki_encrypted if it is not yet decoded and MQTT encryption is also enabled, hash matches and it's a DM not // to us (because we would be able to decrypt it) - if (decodedState == DecodeState::DECODE_FAILURE && moduleConfig.mqtt.encryption_enabled && p->channel == 0x00 && + if (decodedState == DecodeState::DECODE_OPAQUE && moduleConfig.mqtt.encryption_enabled && p->channel == 0x00 && !isBroadcast(p->to) && !isToUs(p)) p_encrypted->pki_encrypted = true; // After potentially altering it, publish received message to MQTT if we're not the original transmitter of the packet @@ -981,6 +1187,7 @@ void Router::perhapsHandleReceived(meshtastic_MeshPacket *p) #endif // assert(radioConfig.has_preferences); if (is_in_repeated(config.lora.ignore_incoming, p->from)) { + clearRoutingAuthCache(); LOG_DEBUG("Ignore msg, 0x%08x is in our ignore list", p->from); packetPool.release(p); return; @@ -988,30 +1195,49 @@ void Router::perhapsHandleReceived(meshtastic_MeshPacket *p) meshtastic_NodeInfoLite const *node = nodeDB->getMeshNode(p->from); if (nodeInfoLiteIsIgnored(node)) { + clearRoutingAuthCache(); LOG_DEBUG("Ignore msg, 0x%08x is ignored", p->from); packetPool.release(p); return; } if (p->from == NODENUM_BROADCAST) { + clearRoutingAuthCache(); LOG_DEBUG("Ignore msg from broadcast address"); packetPool.release(p); return; } if (config.lora.ignore_mqtt && p->via_mqtt) { + clearRoutingAuthCache(); LOG_DEBUG("Msg came in via MQTT from 0x%08x", p->from); packetPool.release(p); return; } if (shouldDropPacketForPreHop(*p)) { + clearRoutingAuthCache(); logHopStartDrop(*p, "pre-hop drop"); packetPool.release(p); return; } + // Decrypt and authenticate before Reliable/Flooding/NextHop filters can update retry + // timers, packet history, implicit ACK state, cancellation, or relay queues. A packet for + // an unknown channel passes as opaque traffic and retains the existing relay behavior. + const auto authVerdict = passesRoutingAuthGate(p); + if (authVerdict == RoutingAuthVerdict::REJECT) { + packetPool.release(p); + return; + } + if (authVerdict == RoutingAuthVerdict::OPAQUE_RELAY_ONLY) { + relayOpaquePacket(p); + packetPool.release(p); + return; + } + if (shouldFilterReceived(p)) { + clearRoutingAuthCache(); LOG_DEBUG("Incoming msg was filtered from 0x%08x", p->from); packetPool.release(p); return; diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 954b88f24..5711ab4e5 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -113,6 +113,9 @@ class Router : protected concurrency::OSThread, protected PacketHistory */ virtual bool shouldFilterReceived(const meshtastic_MeshPacket *p) { return false; } + /** Relay an opaque packet without admitting it to local routing/history state. */ + virtual bool relayOpaquePacket(const meshtastic_MeshPacket *) { return false; } + /** * Determine if hop_limit should be decremented for a relay operation. * Returns false (preserve hop_limit) only if all conditions are met: @@ -162,7 +165,8 @@ class Router : protected concurrency::OSThread, protected PacketHistory void abortSendAndNak(meshtastic_Routing_Error err, meshtastic_MeshPacket *p); }; -enum DecodeState { DECODE_SUCCESS, DECODE_FAILURE, DECODE_FATAL }; +enum DecodeState { DECODE_SUCCESS, DECODE_FAILURE, DECODE_OPAQUE, DECODE_FATAL, DECODE_POLICY_REJECT }; +enum class RoutingAuthVerdict { ACCEPT, OPAQUE_RELAY_ONLY, REJECT }; /** FIXME - move this into a mesh packet class * Remove any encryption and decode the protobufs inside this packet (if necessary). @@ -171,17 +175,26 @@ enum DecodeState { DECODE_SUCCESS, DECODE_FAILURE, DECODE_FATAL }; */ DecodeState perhapsDecode(meshtastic_MeshPacket *p); +/** Apply the receive authentication policy before routing state is mutated. + * Decryptable packets must pass their configured authenticity policy. Packets for unknown channels + * remain eligible for opaque relay, while fatal and policy-rejected packets are filtered. + */ +RoutingAuthVerdict passesRoutingAuthGate(meshtastic_MeshPacket *p); +#ifdef PIO_UNIT_TESTING +uint32_t routingAuthEvaluationCount(); +void resetRoutingAuthEvaluationCount(); +#endif + /** Return 0 for success or a Routing_Error code for failure */ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p); #if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA) -/** XEdDSA receive-side signature policy. When the packet carries a 64-byte signature *and* the - * sender's public key is known, verify it: on success learn the sender's signer bit, on failure - * drop. If the key is unknown the signature is left unverified and the packet passes. A signature - * of any other non-zero length is treated as malformed and dropped. For unsigned packets, enforce - * downgrade protection: drop a non-PKI broadcast from a known signer whose signed encoding would - * still fit a LoRa frame (unicast, PKI, and oversized broadcasts always pass). +/** XEdDSA receive-side signature policy. Valid signatures are verified against a cached key or an + * identity-bound key in first-contact NodeInfo. Invalid and malformed signatures always fail. + * Compatible accepts unsigned traffic, Balanced rejects signable unsigned broadcasts from known + * signers, and Strict requires a verified existing signature or successful PKI authentication + * for all decoded traffic. * * encodedDataSize is the wire size of the encoded Data as the sender built it; pass 0 to size * p->decoded canonically instead (for already-decoded ingress such as plaintext-MQTT downlink, diff --git a/src/mesh/WarmNodeStore.h b/src/mesh/WarmNodeStore.h index 4cacbbc0c..eda6e777a 100644 --- a/src/mesh/WarmNodeStore.h +++ b/src/mesh/WarmNodeStore.h @@ -58,7 +58,7 @@ static constexpr uint32_t WARM_PROT_SHIFT = 4; // bits [ static constexpr uint32_t WARM_PROT_MASK = 0x03u; // Protected category cached alongside role so consumers needn't re-derive the mapping. -enum class WarmProtected : uint8_t { None = 0, Role = 1, Flag = 2 }; +enum class WarmProtected : uint8_t { None = 0, Role = 1, Flag = 2, XeddsaSigner = 3 }; inline uint32_t warmPackLastHeard(uint32_t lastHeard, uint8_t role, uint8_t prot) { diff --git a/src/mesh/udp/UdpMulticastHandler.h b/src/mesh/udp/UdpMulticastHandler.h index 625dc008a..93f5dd6ac 100644 --- a/src/mesh/udp/UdpMulticastHandler.h +++ b/src/mesh/udp/UdpMulticastHandler.h @@ -80,9 +80,9 @@ class UdpMulticastHandler final return; } mp.transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_MULTICAST_UDP; - // Preserve the whole MeshPacket as received: while payload_variant is encrypted, `channel` is a hash (and is 0 for - // PKI DMs), so it must be copied verbatim for the router to attempt PKI/channel decryption. Keep - // pki_encrypted/public_key too so downstream auth/metadata can reflect PKI usage correctly. + // Authentication metadata is local-only; Router re-establishes it after successful PKI decryption. + mp.pki_encrypted = false; + mp.public_key.size = 0; UniquePacketPoolPacket p = packetPool.allocUniqueCopy(mp); // Unset received SNR/RSSI p->rx_snr = 0; diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index a3ee10b12..4dbe22034 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -1061,6 +1061,16 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) incoming.private_key = config.security.private_key; incoming.public_key = config.security.public_key; } +#if MESHTASTIC_EXCLUDE_PKI || MESHTASTIC_EXCLUDE_XEDDSA + if (incoming.packet_signature_policy != + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_BALANCED) { + incoming.packet_signature_policy = + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_BALANCED; + const char *warning = "Packet authenticity policy is unavailable on this firmware build"; + LOG_WARN(warning); + sendWarning(warning); + } +#endif config.security = incoming; #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN) && !(MESHTASTIC_EXCLUDE_PKI) // First provisioning (no key) generates one; a private key supplied without its public key derives it. diff --git a/src/modules/NodeInfoModule.cpp b/src/modules/NodeInfoModule.cpp index c86c54aff..66b4c6c57 100644 --- a/src/modules/NodeInfoModule.cpp +++ b/src/modules/NodeInfoModule.cpp @@ -49,15 +49,6 @@ bool NodeInfoModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, mes LOG_WARN("Invalid nodeInfo detected, is_licensed mismatch!"); return true; } - NodeNum sourceNum = getFrom(&mp); - const meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(sourceNum); - // Broadcasts only: senders never sign unicast NodeInfo, so dropping it would break exchanges - // with signer nodes. Backstops ingress that skips Router's downgrade drop (e.g. decoded MQTT). - if (node && nodeInfoLiteHasXeddsaSigned(node) && !mp.xeddsa_signed && isBroadcast(mp.to)) { - LOG_WARN("Dropping unsigned NodeInfo broadcast from node 0x%08x that previously signed", sourceNum); - return true; - } - // Coerce user.id to be derived from the node number snprintf(p.id, sizeof(p.id), "!%08x", getFrom(&mp)); @@ -232,4 +223,4 @@ int32_t NodeInfoModule::runOnce() sendOurNodeInfo(NODENUM_BROADCAST, requestReplies); // Send our info (don't request replies) } return Default::getConfiguredOrDefaultMs(config.device.node_info_broadcast_secs, default_node_info_broadcast_secs); -} \ No newline at end of file +} diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index b6f0ce24d..7051285e2 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -117,7 +117,8 @@ inline void onReceiveProto(char *topic, byte *payload, size_t length) p->hop_limit = e.packet->hop_limit; p->hop_start = e.packet->hop_start; p->want_ack = e.packet->want_ack; - p->via_mqtt = true; // Mark that the packet was received via MQTT + p->via_mqtt = true; // Mark that the packet was received via MQTT + p->pki_encrypted = false; // Only local AES-CCM decryption may establish PKI authentication. p->transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_MQTT; p->which_payload_variant = e.packet->which_payload_variant; memcpy(&p->decoded, &e.packet->decoded, std::max(sizeof(p->decoded), sizeof(p->encrypted))); @@ -139,12 +140,9 @@ inline void onReceiveProto(char *topic, byte *payload, size_t length) // impersonate a signing node with unsigned broadcasts. Hold cryptLock like the RF path // (perhapsDecode) does - checkXeddsaReceivePolicy -> xeddsa_verify mutates shared // CryptoEngine cache state, and MQTT ingress can run on a different task. - { - concurrency::LockGuard g(cryptLock); - if (!checkXeddsaReceivePolicy(p.get())) { - LOG_INFO("Ignore decoded message failing XEdDSA policy"); - return; - } + if (passesRoutingAuthGate(p.get()) != RoutingAuthVerdict::ACCEPT) { + LOG_INFO("Ignore decoded message failing XEdDSA policy"); + return; } #endif } @@ -157,8 +155,7 @@ inline void onReceiveProto(char *topic, byte *payload, size_t length) // likely they discovered each other via a channel we have downlink enabled for if (isToUs(p.get()) || (nodeInfoLiteHasUser(tx) && nodeInfoLiteHasUser(rx))) router->enqueueReceivedMessage(p.release()); - } else if (router && - perhapsDecode(p.get()) == DecodeState::DECODE_SUCCESS) // ignore messages if we don't have the channel key + } else if (router && passesRoutingAuthGate(p.get()) == RoutingAuthVerdict::ACCEPT) router->enqueueReceivedMessage(p.release()); } diff --git a/test/test_mqtt/MQTT.cpp b/test/test_mqtt/MQTT.cpp index 3716e0a20..270e8abe3 100644 --- a/test/test_mqtt/MQTT.cpp +++ b/test/test_mqtt/MQTT.cpp @@ -334,6 +334,7 @@ const meshtastic_MeshPacket encrypted = { // Initialize mocks and configuration before running each test. void setUp(void) { + config = meshtastic_LocalConfig_init_zero; moduleConfig.mqtt = meshtastic_ModuleConfig_MQTTConfig{.enabled = true, .map_reporting_enabled = true, .has_map_report_settings = true}; moduleConfig.mqtt.map_report_settings = meshtastic_ModuleConfig_MapReportSettings{ @@ -596,6 +597,8 @@ void test_receiveWithoutChannelDownlink(void) // Test receiving an encrypted MeshPacket on the PKI topic. void test_receiveEncryptedPKITopicToUs(void) { + config.security.packet_signature_policy = + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT; meshtastic_MeshPacket e = encrypted; e.to = myNodeInfo.my_node_num; @@ -687,6 +690,8 @@ static meshtastic_MeshPacket makeDecodedBroadcast() // signing node (audit F3). void test_receiveDropsUnsignedBroadcastFromSigner(void) { + config.security.packet_signature_policy = + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_BALANCED; mockNodeDB->emptyNode.bitfield |= NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK; const meshtastic_MeshPacket p = makeDecodedBroadcast(); @@ -698,6 +703,8 @@ void test_receiveDropsUnsignedBroadcastFromSigner(void) // The same unsigned broadcast from a node never seen signing is accepted. void test_receiveAcceptsUnsignedBroadcastFromNonSigner(void) { + config.security.packet_signature_policy = + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_BALANCED; const meshtastic_MeshPacket p = makeDecodedBroadcast(); unitTest->publish(&p); @@ -729,6 +736,8 @@ void test_receiveVerifiesSignedDecodedDownlink(void) // A decoded downlink carrying a signature that fails verification is dropped. void test_receiveDropsBadSignatureOnDecodedDownlink(void) { + config.security.packet_signature_policy = + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_COMPATIBLE; uint8_t pub[32], priv[32]; crypto->generateKeyPair(pub, priv); mockNodeDB->emptyNode.public_key.size = 32; @@ -744,6 +753,53 @@ void test_receiveDropsBadSignatureOnDecodedDownlink(void) TEST_ASSERT_TRUE(mockRouter->packets_.empty()); } + +void test_receiveCompatibleAcceptsUnsignedBroadcastFromSigner(void) +{ + config.security.packet_signature_policy = + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_COMPATIBLE; + mockNodeDB->emptyNode.bitfield |= NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK; + + const meshtastic_MeshPacket p = makeDecodedBroadcast(); + unitTest->publish(&p); + + TEST_ASSERT_EQUAL(1, mockRouter->packets_.size()); +} + +void test_receiveStrictDropsUnsignedPortnumsAndUnicast(void) +{ + config.security.packet_signature_policy = + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT; + const meshtastic_PortNum ports[] = { + meshtastic_PortNum_TEXT_MESSAGE_APP, meshtastic_PortNum_POSITION_APP, meshtastic_PortNum_TELEMETRY_APP, + meshtastic_PortNum_NODEINFO_APP, meshtastic_PortNum_WAYPOINT_APP, + }; + for (const auto port : ports) { + meshtastic_MeshPacket p = makeDecodedBroadcast(); + p.decoded.portnum = port; + unitTest->publish(&p); + } + + meshtastic_MeshPacket unicast = makeDecodedBroadcast(); + unicast.to = myNodeInfo.my_node_num; + unicast.decoded.portnum = meshtastic_PortNum_POSITION_APP; + unitTest->publish(&unicast); + + TEST_ASSERT_TRUE(mockRouter->packets_.empty()); +} + +// A plaintext broker assertion is not evidence that AES-CCM authentication succeeded locally. +void test_receiveStrictDoesNotTrustDecodedPkiFlag(void) +{ + config.security.packet_signature_policy = + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT; + meshtastic_MeshPacket p = makeDecodedBroadcast(); + p.to = myNodeInfo.my_node_num; + p.pki_encrypted = true; + unitTest->publish(&p); + + TEST_ASSERT_TRUE(mockRouter->packets_.empty()); +} #endif // !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA) // Only the same fields that are transmitted over LoRa should be set in MQTT messages. @@ -1063,6 +1119,9 @@ void setup() RUN_TEST(test_receiveAcceptsUnsignedBroadcastFromNonSigner); RUN_TEST(test_receiveVerifiesSignedDecodedDownlink); RUN_TEST(test_receiveDropsBadSignatureOnDecodedDownlink); + RUN_TEST(test_receiveCompatibleAcceptsUnsignedBroadcastFromSigner); + RUN_TEST(test_receiveStrictDropsUnsignedPortnumsAndUnicast); + RUN_TEST(test_receiveStrictDoesNotTrustDecodedPkiFlag); #endif RUN_TEST(test_receiveIgnoresUnexpectedFields); RUN_TEST(test_receiveIgnoresInvalidHopLimit); diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 707d71619..814657891 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -3,11 +3,11 @@ // // The decision logic under test lives in Router.cpp free functions. Groups A/B drive a real // encode -> decode round-trip through the default channel (perhapsEncode/perhapsDecode, black-box, -// no production changes); Groups C-E exercise the policy helpers directly. +// no production changes); later groups exercise routing order and policy helpers directly. // // Group A receive-side accept/reject matrix (verify, downgrade protection, signer-bit learning) // Group B send-side signing policy (which outgoing packets perhapsEncode signs) -// Group C NodeInfoModule's broadcast-only "drop unsigned NodeInfo from a known signer" rule +// Group C routing pipeline ordering (authenticate before duplicate/retry/relay state) // Group D encoding invariants the routing gates depend on // Group E decoded-ingress policy (checkXeddsaReceivePolicy, the plaintext-MQTT trust boundary) @@ -21,9 +21,15 @@ #include "mesh/Channels.h" #include "mesh/CryptoEngine.h" +#include "mesh/MeshRadio.h" +#include "mesh/MeshService.h" #include "mesh/NodeDB.h" +#include "mesh/ReliableRouter.h" #include "mesh/Router.h" -#include "modules/NodeInfoModule.h" +#include "mesh/SinglePortModule.h" +#include "modules/RoutingModule.h" +#include "mqtt/MQTT.h" +#include #include #include #include @@ -86,6 +92,106 @@ class MockNodeDB : public NodeDB static MockNodeDB *mockNodeDB = nullptr; +class AuthPipelineRadio : public RadioInterface +{ + public: + ErrorCode send(meshtastic_MeshPacket *p) override + { + sendCalls++; + packetPool.release(p); + return ERRNO_OK; + } + bool cancelSending(NodeNum, PacketId) override + { + cancelCalls++; + return true; + } + bool findInTxQueue(NodeNum, PacketId) override + { + findCalls++; + return false; + } + bool removePendingTXPacket(NodeNum, PacketId, uint32_t) override + { + removeCalls++; + return true; + } + uint32_t getPacketTime(uint32_t, bool = false) override { return 7; } + void reset() { sendCalls = cancelCalls = findCalls = removeCalls = 0; } + + uint32_t sendCalls = 0; + uint32_t cancelCalls = 0; + uint32_t findCalls = 0; + uint32_t removeCalls = 0; +}; + +class AuthPipelineRouter : public ReliableRouter +{ + public: + bool filter(meshtastic_MeshPacket *p) { return ReliableRouter::shouldFilterReceived(p); } + bool historyContains(const meshtastic_MeshPacket *p) { return wasSeenRecently(p, false); } + 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 addPending(const meshtastic_MeshPacket &p, uint32_t nextTx) + { + auto *copy = packetPool.allocCopy(p); + TEST_ASSERT_NOT_NULL(copy); + const GlobalPacketId key(copy); + pending.emplace(key, PendingPacket(copy, NUM_INTERMEDIATE_RETX)); + pending.at(key).nextTxMsec = nextTx; + } + uint32_t pendingNextTx(NodeNum from, PacketId id) + { + PendingPacket *entry = findPendingPacket(from, id); + return entry ? entry->nextTxMsec : 0; + } + size_t pendingCount() const { return pending.size(); } + void clearPending() + { + for (auto &entry : pending) + packetPool.release(entry.second.packet); + pending.clear(); + } +}; + +class AuthPipelineRoutingModule : public RoutingModule +{ + public: + void sendAckNak(meshtastic_Routing_Error, NodeNum, PacketId, ChannelIndex, uint8_t = 0, bool = false) override { ackCalls++; } + uint32_t ackCalls = 0; +}; + +class AuthPipelineModule : public SinglePortModule +{ + public: + AuthPipelineModule() : SinglePortModule("authPipeline", meshtastic_PortNum_POSITION_APP) {} + ProcessMessage handleReceived(const meshtastic_MeshPacket &) override + { + calls++; + return ProcessMessage::CONTINUE; + } + uint32_t calls = 0; +}; + +class AuthPipelineMqtt : public MQTT +{ + public: + int queueSize() { return mqttQueue.numUsed(); } + void clearQueue() + { + while (QueueEntry *entry = mqttQueue.dequeuePtr(0)) + delete entry; + } +}; + +static AuthPipelineRouter *pipelineRouter = nullptr; +static AuthPipelineRadio *pipelineRadio = nullptr; +static AuthPipelineRoutingModule *pipelineRouting = nullptr; +static AuthPipelineModule *pipelineModule = nullptr; +static AuthPipelineMqtt *pipelineMqtt = nullptr; +static MeshService *pipelineService = nullptr; + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- @@ -126,11 +232,48 @@ static DecodeState roundTrip(meshtastic_MeshPacket *p) return perhapsDecode(p); } +static meshtastic_MeshPacket channelEncode(meshtastic_MeshPacket p) +{ + uint8_t encoded[MAX_LORA_PAYLOAD_LEN + 1] = {}; + const size_t encodedSize = pb_encode_to_bytes(encoded, sizeof(encoded), &meshtastic_Data_msg, &p.decoded); + TEST_ASSERT_GREATER_THAN(0, encodedSize); + const int16_t hash = channels.setActiveByIndex(p.channel); + TEST_ASSERT_GREATER_OR_EQUAL(0, hash); + crypto->encryptPacket(p.from, p.id, encodedSize, encoded); + memcpy(p.encrypted.bytes, encoded, encodedSize); + p.encrypted.size = encodedSize; + p.channel = hash; + p.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; + return p; +} + +static meshtastic_MeshPacket makeSignedWirePacket(NodeNum from, NodeNum to, PacketId id, uint8_t hopLimit = 1, + uint8_t hopStart = 2, uint8_t nextHop = NO_NEXT_HOP_PREFERENCE, + uint8_t relayNode = 0x33, bool valid = true) +{ + meshtastic_MeshPacket p = makeDecoded(from, to, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); + p.id = id; + p.hop_limit = hopLimit; + p.hop_start = hopStart; + p.next_hop = nextHop; + p.relay_node = relayNode; + p.transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA; + signWithCurrentKey(&p); + if (!valid) + p.decoded.xeddsa_signature.bytes[0] ^= 0x80; + return channelEncode(p); +} + static bool remoteSignerBit() { return nodeInfoLiteHasXeddsaSigned(mockNodeDB->getMeshNode(REMOTE_NODE)); } +static void setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy policy) +{ + config.security.packet_signature_policy = policy; +} + // Size a Data message exactly as the wire encoder would. static size_t encodedDataSize(const meshtastic_Data *d) { @@ -158,17 +301,32 @@ void setUp(void) // re-enables the PKI encrypt path and fails the unicast tests on hosts with leftover prefs. mockNodeDB = new MockNodeDB(); mockNodeDB->clearTestNodes(); +#if WARM_NODE_COUNT > 0 + mockNodeDB->warmStore.clear(); +#endif nodeDB = mockNodeDB; // Clean global config/owner AFTER the ctor; zeroed config => rebroadcast ALL (no KNOWN_ONLY // drop) and security.private_key.size == 0 (PKI encrypt path skipped => simple channel crypto). config = meshtastic_LocalConfig_init_zero; + moduleConfig = meshtastic_LocalModuleConfig_init_zero; owner = meshtastic_User_init_zero; myNodeInfo.my_node_num = LOCAL_NODE; // drives isFromUs()/getFrom()/isToUs() // Working primary channel with the default PSK so encrypt/decrypt round-trips. channels.initDefaults(); channels.onConfigChanged(); + + pipelineRouter->clearPending(); + pipelineRouter->rxDupe = 0; + pipelineRouter->txRelayCanceled = 0; + pipelineRadio->reset(); + pipelineRouting->ackCalls = 0; + pipelineModule->calls = 0; + pipelineMqtt->clearQueue(); + while (meshtastic_MeshPacket *queued = pipelineService->getForPhone()) + packetPool.release(queued); + resetRoutingAuthEvaluationCount(); } void tearDown(void) @@ -185,6 +343,7 @@ void tearDown(void) // A1: valid signature from a node whose key we know -> accepted, marked signed, signer bit learned. void test_A1_valid_signature_accepted_and_learns_signer(void) { + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); uint8_t pub[32], priv[32]; crypto->generateKeyPair(pub, priv); // engine now holds REMOTE's key mockNodeDB->addNode(REMOTE_NODE); @@ -203,6 +362,7 @@ void test_A1_valid_signature_accepted_and_learns_signer(void) // A2: a tampered signature from a known key -> dropped. void test_A2_bad_signature_dropped(void) { + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_COMPATIBLE); uint8_t pub[32], priv[32]; crypto->generateKeyPair(pub, priv); mockNodeDB->addNode(REMOTE_NODE); @@ -212,12 +372,13 @@ void test_A2_bad_signature_dropped(void) signWithCurrentKey(&p); p.decoded.xeddsa_signature.bytes[0] ^= 0xFF; // corrupt the signature - TEST_ASSERT_EQUAL(DECODE_FAILURE, roundTrip(&p)); + TEST_ASSERT_EQUAL(DECODE_POLICY_REJECT, roundTrip(&p)); } // A3: signed packet but we have no key for the sender -> accepted unverified, signer bit NOT set. void test_A3_signed_no_pubkey_accepted_unverified(void) { + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_BALANCED); uint8_t pub[32], priv[32]; crypto->generateKeyPair(pub, priv); mockNodeDB->addNode(REMOTE_NODE); // node exists, but no public key stored @@ -239,7 +400,7 @@ void test_A4_downgrade_unsigned_broadcast_from_signer_dropped(void) meshtastic_MeshPacket p = makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD); // from != us, so perhapsEncode leaves it unsigned. - TEST_ASSERT_EQUAL(DECODE_FAILURE, roundTrip(&p)); + TEST_ASSERT_EQUAL(DECODE_POLICY_REJECT, roundTrip(&p)); } // A5: no prior knowledge - unsigned small broadcast from a non-signer -> accepted. @@ -321,9 +482,192 @@ void test_A9_unsigned_boundary_broadcast_from_signer_still_dropped(void) TEST_ASSERT_EQUAL_MESSAGE(MAX_LORA_PAYLOAD_LEN, encodedDataSize(&signedCopy) + MESHTASTIC_HEADER_LENGTH, "payload no longer sits exactly on the fit boundary - retune it"); - TEST_ASSERT_EQUAL(DECODE_FAILURE, roundTrip(&p)); + TEST_ASSERT_EQUAL(DECODE_POLICY_REJECT, roundTrip(&p)); } +void test_A10_compatible_accepts_unsigned_broadcast_from_signer(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_COMPATIBLE); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setSignerBit(REMOTE_NODE, true); + + meshtastic_MeshPacket p = makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); + TEST_ASSERT_EQUAL(DECODE_SUCCESS, roundTrip(&p)); +} + +void test_A11_strict_rejects_unsigned_all_portnums_destinations_and_sizes(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + const meshtastic_PortNum ports[] = { + meshtastic_PortNum_TEXT_MESSAGE_APP, meshtastic_PortNum_POSITION_APP, meshtastic_PortNum_TELEMETRY_APP, + meshtastic_PortNum_NODEINFO_APP, meshtastic_PortNum_WAYPOINT_APP, + }; + for (const auto port : ports) { + meshtastic_MeshPacket p = makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, port, SMALL_PAYLOAD); + TEST_ASSERT_EQUAL(DECODE_POLICY_REJECT, roundTrip(&p)); + } + + meshtastic_MeshPacket unicast = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); + TEST_ASSERT_EQUAL(DECODE_POLICY_REJECT, roundTrip(&unicast)); + + meshtastic_MeshPacket oversized = + makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, meshtastic_PortNum_POSITION_APP, OVERSIZED_PAYLOAD); + TEST_ASSERT_EQUAL(DECODE_POLICY_REJECT, roundTrip(&oversized)); +} + +void test_A12_strict_rejects_signed_packet_without_key(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + uint8_t pub[32], priv[32]; + crypto->generateKeyPair(pub, priv); + + meshtastic_MeshPacket p = makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); + signWithCurrentKey(&p); + TEST_ASSERT_EQUAL(DECODE_POLICY_REJECT, roundTrip(&p)); +} + +void test_A13_strict_accepts_locally_authenticated_pki_packet(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + uint8_t localPub[32], localPriv[32], remotePub[32], remotePriv[32]; + crypto->generateKeyPair(localPub, localPriv); + crypto->generateKeyPair(remotePub, remotePriv); + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->setPublicKey(LOCAL_NODE, localPub); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePub); + + meshtastic_Data data = meshtastic_Data_init_zero; + data.portnum = meshtastic_PortNum_PRIVATE_APP; + data.payload.size = SMALL_PAYLOAD; + memset(data.payload.bytes, 0x5A, data.payload.size); + uint8_t plaintext[MAX_LORA_PAYLOAD_LEN + 1] = {}; + const size_t plaintextSize = pb_encode_to_bytes(plaintext, sizeof(plaintext), &meshtastic_Data_msg, &data); + TEST_ASSERT_GREATER_THAN(0, plaintextSize); + + meshtastic_NodeInfoLite_public_key_t localKey = {32, {0}}; + memcpy(localKey.bytes, localPub, sizeof(localPub)); + meshtastic_MeshPacket p = meshtastic_MeshPacket_init_zero; + p.from = REMOTE_NODE; + p.to = LOCAL_NODE; + p.id = 0x0CC01234; + p.channel = 0; + p.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; + crypto->setDHPrivateKey(remotePriv); + TEST_ASSERT_TRUE(crypto->encryptCurve25519(p.to, p.from, localKey, p.id, plaintextSize, plaintext, p.encrypted.bytes)); + p.encrypted.size = plaintextSize + MESHTASTIC_PKC_OVERHEAD; + + // Only the receiver's private key can establish the local pki_encrypted authentication marker. + crypto->setDHPrivateKey(localPriv); + TEST_ASSERT_EQUAL(DECODE_SUCCESS, perhapsDecode(&p)); + TEST_ASSERT_TRUE(p.pki_encrypted); + TEST_ASSERT_EQUAL(meshtastic_PortNum_PRIVATE_APP, p.decoded.portnum); +} + +void test_A13b_strict_rejects_spoofed_pki_flag_on_encrypted_ingress(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + meshtastic_MeshPacket p = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_NONE, perhapsEncode(&p)); + p.pki_encrypted = true; + p.public_key.size = 32; + memset(p.public_key.bytes, 0xAB, p.public_key.size); + + TEST_ASSERT_EQUAL(DECODE_POLICY_REJECT, perhapsDecode(&p)); + TEST_ASSERT_FALSE(p.pki_encrypted); + TEST_ASSERT_EQUAL(0, p.public_key.size); +} + +void test_A14_strict_bootstraps_identity_bound_signed_nodeinfo(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + uint8_t pub[32], priv[32]; + crypto->generateKeyPair(pub, priv); + const NodeNum signer = crc32Buffer(pub, sizeof(pub)); + + meshtastic_User user = meshtastic_User_init_zero; + user.public_key.size = sizeof(pub); + memcpy(user.public_key.bytes, pub, sizeof(pub)); + meshtastic_MeshPacket p = makeDecoded(signer, NODENUM_BROADCAST, meshtastic_PortNum_NODEINFO_APP, 0); + p.decoded.payload.size = + pb_encode_to_bytes(p.decoded.payload.bytes, sizeof(p.decoded.payload.bytes), &meshtastic_User_msg, &user); + signWithCurrentKey(&p); + + TEST_ASSERT_EQUAL(DECODE_SUCCESS, roundTrip(&p)); + const meshtastic_NodeInfoLite *node = mockNodeDB->getMeshNode(signer); + TEST_ASSERT_NOT_NULL(node); + TEST_ASSERT_EQUAL_UINT8_ARRAY(pub, node->public_key.bytes, sizeof(pub)); + TEST_ASSERT_TRUE(p.xeddsa_signed); +} + +void test_A15_strict_rejects_nodeinfo_key_without_identity_binding(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + uint8_t pub[32], priv[32]; + crypto->generateKeyPair(pub, priv); + + meshtastic_User user = meshtastic_User_init_zero; + user.public_key.size = sizeof(pub); + memcpy(user.public_key.bytes, pub, sizeof(pub)); + meshtastic_MeshPacket p = + makeDecoded(crc32Buffer(pub, sizeof(pub)) ^ 1, NODENUM_BROADCAST, meshtastic_PortNum_NODEINFO_APP, 0); + p.decoded.payload.size = + pb_encode_to_bytes(p.decoded.payload.bytes, sizeof(p.decoded.payload.bytes), &meshtastic_User_msg, &user); + signWithCurrentKey(&p); + + TEST_ASSERT_EQUAL(DECODE_POLICY_REJECT, roundTrip(&p)); + TEST_ASSERT_NULL(mockNodeDB->getMeshNode(p.from)); +} + +void test_A16_compatible_rejects_invalid_first_contact_nodeinfo(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_COMPATIBLE); + uint8_t pub[32], priv[32]; + crypto->generateKeyPair(pub, priv); + + meshtastic_User user = meshtastic_User_init_zero; + user.public_key.size = sizeof(pub); + memcpy(user.public_key.bytes, pub, sizeof(pub)); + meshtastic_MeshPacket p = + makeDecoded(crc32Buffer(pub, sizeof(pub)) ^ 1, NODENUM_BROADCAST, meshtastic_PortNum_NODEINFO_APP, 0); + p.decoded.payload.size = + pb_encode_to_bytes(p.decoded.payload.bytes, sizeof(p.decoded.payload.bytes), &meshtastic_User_msg, &user); + signWithCurrentKey(&p); + + TEST_ASSERT_EQUAL(DECODE_POLICY_REJECT, roundTrip(&p)); +} + +#if WARM_NODE_COUNT > 0 +void test_A17_strict_verifies_signer_from_warm_key_store(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + uint8_t pub[32], priv[32]; + crypto->generateKeyPair(pub, priv); + TEST_ASSERT_TRUE(mockNodeDB->warmStore.absorb(REMOTE_NODE, 1, pub)); + TEST_ASSERT_NULL(mockNodeDB->getMeshNode(REMOTE_NODE)); + + meshtastic_MeshPacket p = makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); + signWithCurrentKey(&p); + TEST_ASSERT_EQUAL(DECODE_SUCCESS, roundTrip(&p)); + TEST_ASSERT_TRUE(p.xeddsa_signed); + const meshtastic_NodeInfoLite *rehydrated = mockNodeDB->getMeshNode(REMOTE_NODE); + TEST_ASSERT_NOT_NULL_MESSAGE(rehydrated, "verified warm signer must be re-admitted to the hot store"); + TEST_ASSERT_EQUAL_UINT8_ARRAY(pub, rehydrated->public_key.bytes, sizeof(pub)); + TEST_ASSERT_TRUE_MESSAGE(nodeInfoLiteHasXeddsaSigned(rehydrated), "re-admitted signer must retain Balanced downgrade memory"); + + // Model its next hot-store eviction and prove Balanced still remembers the signer without + // allocating a hot node merely to evaluate an unsigned packet. + TEST_ASSERT_TRUE(mockNodeDB->warmStore.absorb(REMOTE_NODE, 2, pub, meshtastic_Config_DeviceConfig_Role_CLIENT, + static_cast(WarmProtected::XeddsaSigner))); + mockNodeDB->clearTestNodes(); + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_BALANCED); + meshtastic_MeshPacket unsignedPacket = + makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); + TEST_ASSERT_FALSE_MESSAGE(checkXeddsaReceivePolicy(&unsignedPacket), + "Balanced downgrade memory must survive repeated hot-store eviction"); +} +#endif + // =========================================================================== // Group B - send-side signing policy (perhapsEncode) // =========================================================================== @@ -343,15 +687,15 @@ void test_B1_local_broadcast_is_signed(void) TEST_ASSERT_TRUE(p.xeddsa_signed); } -// B2: our own unicast is NOT signed. +// B2: preserve the existing wire behavior: non-PKI unicast is not signed. void test_B2_local_unicast_not_signed(void) { mockNodeDB->addNode(REMOTE_NODE); - meshtastic_MeshPacket p = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_PRIVATE_APP, SMALL_PAYLOAD); + meshtastic_MeshPacket p = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); TEST_ASSERT_EQUAL(DECODE_SUCCESS, roundTrip(&p)); - TEST_ASSERT_EQUAL_MESSAGE(0, p.decoded.xeddsa_signature.size, "unicast must not be signed"); + TEST_ASSERT_EQUAL_MESSAGE(0, p.decoded.xeddsa_signature.size, "unicast must remain unsigned"); } // B3: our own oversized broadcast is NOT signed (signature wouldn't fit). @@ -400,14 +744,12 @@ void test_B4_all_broadcast_sizes_deliverable_no_deadband(void) TEST_ASSERT_TRUE_MESSAGE(sawUnsigned, "sweep never crossed the fit boundary"); } -// B5: a client-preset signature on a packet we originate is discarded, not transmitted. -// perhapsEncode owns signing for our packets; a stale/garbage signature from a phone app on a -// packet we don't sign (here: unicast) would otherwise fail verification at every receiver. +// B5: a client-preset signature on a packet outside the existing broadcast sign class is discarded. void test_B5_preset_signature_on_local_packet_cleared(void) { mockNodeDB->addNode(REMOTE_NODE); - meshtastic_MeshPacket p = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_PRIVATE_APP, SMALL_PAYLOAD); + meshtastic_MeshPacket p = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); p.decoded.xeddsa_signature.size = XEDDSA_SIGNATURE_SIZE; memset(p.decoded.xeddsa_signature.bytes, 0xAB, XEDDSA_SIGNATURE_SIZE); @@ -454,81 +796,345 @@ void test_B6_rich_shape_sweep_no_deadband(void) TEST_ASSERT_TRUE_MESSAGE(sawUnsigned, "rich sweep never crossed the fit boundary"); } +void test_B7_infrastructure_port_signing_matrix(void) +{ + uint8_t pub[32], priv[32]; + crypto->generateKeyPair(pub, priv); + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->setPublicKey(LOCAL_NODE, pub); + + const meshtastic_PortNum ports[] = { + meshtastic_PortNum_NODEINFO_APP, + meshtastic_PortNum_ROUTING_APP, + meshtastic_PortNum_TRACEROUTE_APP, + meshtastic_PortNum_POSITION_APP, + }; + for (const auto port : ports) { + meshtastic_MeshPacket broadcast = makeDecoded(LOCAL_NODE, NODENUM_BROADCAST, port, SMALL_PAYLOAD); + TEST_ASSERT_EQUAL(DECODE_SUCCESS, roundTrip(&broadcast)); + TEST_ASSERT_EQUAL_MESSAGE(XEDDSA_SIGNATURE_SIZE, broadcast.decoded.xeddsa_signature.size, + "signable infrastructure broadcast must be signed"); + + meshtastic_MeshPacket unicast = makeDecoded(LOCAL_NODE, REMOTE_NODE, port, SMALL_PAYLOAD); + TEST_ASSERT_EQUAL(DECODE_SUCCESS, roundTrip(&unicast)); + TEST_ASSERT_EQUAL_MESSAGE(0, unicast.decoded.xeddsa_signature.size, + "infrastructure unicast must preserve existing unsigned behavior"); + } +} + // =========================================================================== -// Group C - NodeInfoModule downgrade drop (broadcast-only backstop for ingress paths that skip -// Router's check; unicast NodeInfo is never signed by senders, so it is exempt - see C4) +// Group C - routing pipeline authentication ordering // =========================================================================== -class NodeInfoTestShim : public NodeInfoModule -{ - public: - using NodeInfoModule::handleReceivedProtobuf; // protected virtual -> exposed for direct call -}; -static meshtastic_MeshPacket makeNodeInfoPacket(bool signed_) +static void preparePipelineSigner(NodeNum sender) { - // Broadcast so the module's phone-forward path (which needs `service`) is skipped. - meshtastic_MeshPacket mp = makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); - mp.xeddsa_signed = signed_; - return mp; + uint8_t pub[32], priv[32]; + crypto->generateKeyPair(pub, priv); + mockNodeDB->addNode(sender); + mockNodeDB->setPublicKey(sender, pub); } -// C1: unsigned NodeInfo from a node that previously signed -> dropped. -void test_C1_unsigned_nodeinfo_from_signer_dropped(void) +static void runPipelineIngress(const meshtastic_MeshPacket &p) { + meshtastic_MeshPacket *copy = packetPool.allocCopy(p); + TEST_ASSERT_NOT_NULL(copy); + pipelineRouter->enqueueReceivedMessage(copy); + pipelineRouter->runOnce(); +} + +static void assertNoRejectedPipelineEffects(NodeNum sender, uint32_t lastHeardBefore) +{ + TEST_ASSERT_EQUAL(0, pipelineRadio->sendCalls); + TEST_ASSERT_EQUAL(0, pipelineRadio->cancelCalls); + TEST_ASSERT_EQUAL(0, pipelineRadio->findCalls); + TEST_ASSERT_EQUAL(0, pipelineRadio->removeCalls); + TEST_ASSERT_EQUAL(0, pipelineRouting->ackCalls); + TEST_ASSERT_EQUAL(0, pipelineRouter->rxDupe); + TEST_ASSERT_EQUAL(0, pipelineRouter->txRelayCanceled); + TEST_ASSERT_EQUAL(0, pipelineModule->calls); + TEST_ASSERT_EQUAL(0, pipelineMqtt->queueSize()); + TEST_ASSERT_NULL(pipelineService->getForPhone()); + const meshtastic_NodeInfoLite *node = mockNodeDB->getMeshNode(sender); + TEST_ASSERT_NOT_NULL(node); + TEST_ASSERT_EQUAL_UINT32(lastHeardBefore, node->last_heard); +} + +void test_C1_invalid_first_copy_does_not_poison_valid_same_id(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + preparePipelineSigner(REMOTE_NODE); + const PacketId id = 0xC1000001; + const uint32_t lastHeard = mockNodeDB->getMeshNode(REMOTE_NODE)->last_heard; + + meshtastic_MeshPacket invalid = makeSignedWirePacket(REMOTE_NODE, NODENUM_BROADCAST, id, 1, 2, 0, 0x31, false); + moduleConfig.mqtt.enabled = true; + runPipelineIngress(invalid); + assertNoRejectedPipelineEffects(REMOTE_NODE, lastHeard); + TEST_ASSERT_FALSE(pipelineRouter->historyContains(&invalid)); + + meshtastic_MeshPacket valid = makeSignedWirePacket(REMOTE_NODE, NODENUM_BROADCAST, id); + TEST_ASSERT_EQUAL(static_cast(RoutingAuthVerdict::ACCEPT), static_cast(passesRoutingAuthGate(&valid))); + TEST_ASSERT_EQUAL_MESSAGE(meshtastic_MeshPacket_encrypted_tag, valid.which_payload_variant, + "routing auth gate must preserve encrypted relay/MQTT bytes"); + TEST_ASSERT_FALSE_MESSAGE(pipelineRouter->filter(&valid), "valid same-ID packet was poisoned by rejected first copy"); + TEST_ASSERT_TRUE(pipelineRouter->historyContains(&valid)); +} + +void test_C2_invalid_ordinary_duplicate_has_no_cancel_or_delivery_effects(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + preparePipelineSigner(REMOTE_NODE); + const PacketId id = 0xC2000002; + meshtastic_MeshPacket prior = makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); + prior.id = id; + prior.hop_limit = 1; + prior.hop_start = 2; + prior.transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA; + pipelineRouter->remember(&prior); + const uint32_t lastHeard = mockNodeDB->getMeshNode(REMOTE_NODE)->last_heard; + + meshtastic_MeshPacket invalid = makeSignedWirePacket(REMOTE_NODE, NODENUM_BROADCAST, id, 1, 2, 0, 0x32, false); + runPipelineIngress(invalid); + assertNoRejectedPipelineEffects(REMOTE_NODE, lastHeard); + TEST_ASSERT_TRUE(pipelineRouter->historyContains(&prior)); +} + +void test_C3_invalid_repeated_packet_cannot_ack_or_change_retry_state(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + preparePipelineSigner(LOCAL_NODE); + const PacketId id = 0xC3000003; + meshtastic_MeshPacket prior = makeDecoded(LOCAL_NODE, NODENUM_BROADCAST, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); + prior.id = id; + prior.hop_limit = 2; + prior.hop_start = 2; + prior.transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA; + pipelineRouter->remember(&prior); + pipelineRouter->addPending(prior, UINT32_MAX); + const uint32_t lastHeard = mockNodeDB->getMeshNode(LOCAL_NODE)->last_heard; + + meshtastic_MeshPacket invalid = makeSignedWirePacket(LOCAL_NODE, NODENUM_BROADCAST, id, 2, 2, 0, 0x34, false); + runPipelineIngress(invalid); + assertNoRejectedPipelineEffects(LOCAL_NODE, lastHeard); + TEST_ASSERT_EQUAL(1, pipelineRouter->pendingCount()); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, pipelineRouter->pendingNextTx(LOCAL_NODE, id)); +} + +void test_C4_invalid_fallback_packet_cannot_relay(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + preparePipelineSigner(REMOTE_NODE); + const PacketId id = 0xC4000004; + const uint8_t ourRelay = mockNodeDB->getLastByteOfNodeNum(LOCAL_NODE); + meshtastic_MeshPacket prior = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); + prior.id = id; + prior.next_hop = 0x22; + prior.relay_node = ourRelay; + prior.hop_limit = 1; + prior.hop_start = 2; + pipelineRouter->remember(&prior); + meshtastic_MeshPacket relayed = prior; + relayed.relay_node = 0x35; + pipelineRouter->remember(&relayed); + pipelineRouter->forgetRelayer(ourRelay, id, REMOTE_NODE); + const uint32_t lastHeard = mockNodeDB->getMeshNode(REMOTE_NODE)->last_heard; + + meshtastic_MeshPacket invalid = makeSignedWirePacket(REMOTE_NODE, LOCAL_NODE, id, 1, 2, 0, 0x35, false); + runPipelineIngress(invalid); + assertNoRejectedPipelineEffects(REMOTE_NODE, lastHeard); +} + +void test_C5_invalid_upgrade_cannot_remove_pending_valid_send(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + preparePipelineSigner(REMOTE_NODE); + const PacketId id = 0xC5000005; + meshtastic_MeshPacket prior = makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); + prior.id = id; + prior.hop_limit = 1; + prior.hop_start = 2; + pipelineRouter->remember(&prior); + const uint32_t lastHeard = mockNodeDB->getMeshNode(REMOTE_NODE)->last_heard; + + meshtastic_MeshPacket directInvalid = makeSignedWirePacket(REMOTE_NODE, NODENUM_BROADCAST, id, 2, 2, 0, 0x36, false); + TEST_ASSERT_TRUE(pipelineRouter->handleUpgrade(&directInvalid)); + TEST_ASSERT_EQUAL(0, pipelineRadio->removeCalls); + + meshtastic_MeshPacket invalid = makeSignedWirePacket(REMOTE_NODE, NODENUM_BROADCAST, id, 2, 2, 0, 0x36, false); + runPipelineIngress(invalid); + assertNoRejectedPipelineEffects(REMOTE_NODE, lastHeard); + + // The rejected upgrade did not raise the history watermark or remove the queued valid copy; + // a later authenticated replacement still performs the intended upgrade. + meshtastic_MeshPacket valid = makeSignedWirePacket(REMOTE_NODE, NODENUM_BROADCAST, id, 2, 2, 0, 0x36, true); + TEST_ASSERT_EQUAL(static_cast(RoutingAuthVerdict::ACCEPT), static_cast(passesRoutingAuthGate(&valid))); + TEST_ASSERT_TRUE(pipelineRouter->filter(&valid)); + TEST_ASSERT_EQUAL(1, pipelineRadio->removeCalls); +} + +void test_C6_opaque_unknown_channel_is_relay_only(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + meshtastic_MeshPacket opaque = meshtastic_MeshPacket_init_zero; + opaque.from = REMOTE_NODE; + opaque.to = NODENUM_BROADCAST; + opaque.id = 0xC6000006; + opaque.channel = 0xFE; + opaque.hop_limit = 1; + opaque.hop_start = 2; + opaque.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; + opaque.encrypted.size = 16; + memset(opaque.encrypted.bytes, 0xA5, opaque.encrypted.size); + + TEST_ASSERT_EQUAL(static_cast(RoutingAuthVerdict::OPAQUE_RELAY_ONLY), static_cast(passesRoutingAuthGate(&opaque))); + moduleConfig.mqtt.enabled = true; + runPipelineIngress(opaque); + TEST_ASSERT_EQUAL_MESSAGE(1, pipelineRadio->sendCalls, "opaque broadcast should take only the safety-controlled relay path"); + TEST_ASSERT_EQUAL(0, pipelineRouting->ackCalls); + TEST_ASSERT_EQUAL(0, pipelineModule->calls); + TEST_ASSERT_EQUAL(0, pipelineMqtt->queueSize()); + TEST_ASSERT_NULL(pipelineService->getForPhone()); + TEST_ASSERT_FALSE(pipelineRouter->historyContains(&opaque)); + TEST_ASSERT_NULL(mockNodeDB->getMeshNode(REMOTE_NODE)); + + pipelineRadio->reset(); + meshtastic_MeshPacket addressed = opaque; + addressed.to = LOCAL_NODE; + addressed.id++; + runPipelineIngress(addressed); + TEST_ASSERT_EQUAL_MESSAGE(0, pipelineRadio->sendCalls, "opaque packet addressed to us must not be relayed"); + TEST_ASSERT_EQUAL(0, pipelineRouting->ackCalls); + TEST_ASSERT_EQUAL(0, pipelineModule->calls); + TEST_ASSERT_EQUAL(0, pipelineMqtt->queueSize()); + TEST_ASSERT_NULL(pipelineService->getForPhone()); + TEST_ASSERT_FALSE(pipelineRouter->historyContains(&addressed)); + + const meshtastic_Config_DeviceConfig_RebroadcastMode blockedModes[] = { + meshtastic_Config_DeviceConfig_RebroadcastMode_LOCAL_ONLY, + meshtastic_Config_DeviceConfig_RebroadcastMode_CORE_PORTNUMS_ONLY, + meshtastic_Config_DeviceConfig_RebroadcastMode_NONE, + }; + for (const auto mode : blockedModes) { + pipelineRadio->reset(); + config.device.rebroadcast_mode = mode; + meshtastic_MeshPacket blocked = opaque; + blocked.id++; + blocked.id += static_cast(mode); + blocked.transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_MULTICAST_UDP; + runPipelineIngress(blocked); + TEST_ASSERT_EQUAL_MESSAGE(0, pipelineRadio->sendCalls, "restricted rebroadcast mode must suppress opaque relay"); + TEST_ASSERT_EQUAL(0, pipelineRouting->ackCalls); + TEST_ASSERT_EQUAL(0, pipelineModule->calls); + TEST_ASSERT_EQUAL(0, pipelineMqtt->queueSize()); + TEST_ASSERT_NULL(pipelineService->getForPhone()); + TEST_ASSERT_FALSE(pipelineRouter->historyContains(&blocked)); + } +} + +void test_C7_strict_rejects_unsigned_decoded_simradio_ingress(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); mockNodeDB->addNode(REMOTE_NODE); - mockNodeDB->setSignerBit(REMOTE_NODE, true); - - NodeInfoTestShim shim; - meshtastic_MeshPacket mp = makeNodeInfoPacket(/*signed_=*/false); - meshtastic_User user = meshtastic_User_init_zero; - user.is_licensed = owner.is_licensed; - - TEST_ASSERT_TRUE_MESSAGE(shim.handleReceivedProtobuf(mp, &user), "unsigned NodeInfo from signer must be dropped"); + const uint32_t lastHeard = mockNodeDB->getMeshNode(REMOTE_NODE)->last_heard; + meshtastic_MeshPacket injected = makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD); + injected.transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA; + runPipelineIngress(injected); + assertNoRejectedPipelineEffects(REMOTE_NODE, lastHeard); + TEST_ASSERT_FALSE(pipelineRouter->historyContains(&injected)); } -// C2: signed NodeInfo from a known signer -> not dropped by this rule. -void test_C2_signed_nodeinfo_from_signer_not_dropped(void) +void test_C8_trusted_local_decoded_delivery_is_not_filtered(void) { + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + meshtastic_MeshPacket *local = + packetPool.allocCopy(makeDecoded(0, LOCAL_NODE, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD)); + TEST_ASSERT_NOT_NULL(local); + TEST_ASSERT_EQUAL(ERRNO_SHOULD_RELEASE, pipelineRouter->sendLocal(local, RX_SRC_USER)); + TEST_ASSERT_EQUAL_MESSAGE(1, pipelineModule->calls, "trusted phone-origin packet must reach local modules"); + packetPool.release(local); +} + +void test_C9_known_channel_malformed_plaintext_is_not_relayed_as_opaque(void) +{ + meshtastic_MeshPacket malformed = meshtastic_MeshPacket_init_zero; + malformed.from = REMOTE_NODE; + malformed.to = NODENUM_BROADCAST; + malformed.id = 0xC9000009; + malformed.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; + malformed.encrypted.size = 3; + malformed.encrypted.bytes[0] = 0xFF; + malformed.encrypted.bytes[1] = 0xFF; + malformed.encrypted.bytes[2] = 0xFF; + malformed.channel = channels.setActiveByIndex(0); + crypto->encryptPacket(malformed.from, malformed.id, malformed.encrypted.size, malformed.encrypted.bytes); mockNodeDB->addNode(REMOTE_NODE); - mockNodeDB->setSignerBit(REMOTE_NODE, true); - - NodeInfoTestShim shim; - meshtastic_MeshPacket mp = makeNodeInfoPacket(/*signed_=*/true); - meshtastic_User user = meshtastic_User_init_zero; - user.is_licensed = owner.is_licensed; - - TEST_ASSERT_FALSE(shim.handleReceivedProtobuf(mp, &user)); + const uint32_t lastHeard = mockNodeDB->getMeshNode(REMOTE_NODE)->last_heard; + runPipelineIngress(malformed); + assertNoRejectedPipelineEffects(REMOTE_NODE, lastHeard); + TEST_ASSERT_FALSE(pipelineRouter->historyContains(&malformed)); } -// C3: unsigned NodeInfo from a node we've never seen sign -> not dropped. -void test_C3_unsigned_nodeinfo_from_nonsigner_not_dropped(void) -{ - mockNodeDB->addNode(REMOTE_NODE); // signer bit clear - - NodeInfoTestShim shim; - meshtastic_MeshPacket mp = makeNodeInfoPacket(/*signed_=*/false); - meshtastic_User user = meshtastic_User_init_zero; - user.is_licensed = owner.is_licensed; - - TEST_ASSERT_FALSE(shim.handleReceivedProtobuf(mp, &user)); -} - -// C4: F1 regression - unsigned UNICAST NodeInfo from a known signer -> NOT dropped. Unicast -// NodeInfo (want_response replies, phone-initiated exchanges) is never signed by the sender, -// so treating it as a downgrade broke NodeInfo exchange with signer nodes. -void test_C4_unsigned_unicast_nodeinfo_from_signer_accepted(void) +void test_C10_legacy_channel_dm_failure_has_no_pipeline_effects(void) { + meshtastic_MeshPacket legacyDm = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD); + legacyDm = channelEncode(legacyDm); mockNodeDB->addNode(REMOTE_NODE); - mockNodeDB->setSignerBit(REMOTE_NODE, true); + const uint32_t lastHeard = mockNodeDB->getMeshNode(REMOTE_NODE)->last_heard; + moduleConfig.mqtt.enabled = true; + runPipelineIngress(legacyDm); + assertNoRejectedPipelineEffects(REMOTE_NODE, lastHeard); + TEST_ASSERT_FALSE(pipelineRouter->historyContains(&legacyDm)); +} - NodeInfoTestShim shim; - meshtastic_MeshPacket mp = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, SMALL_PAYLOAD); - mp.xeddsa_signed = false; - meshtastic_User user = meshtastic_User_init_zero; - user.is_licensed = owner.is_licensed; +void test_C11_malformed_pki_plaintext_has_no_pipeline_effects(void) +{ + uint8_t localPub[32], localPriv[32], remotePub[32], remotePriv[32]; + crypto->generateKeyPair(localPub, localPriv); + crypto->generateKeyPair(remotePub, remotePriv); + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->setPublicKey(LOCAL_NODE, localPub); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePub); - TEST_ASSERT_FALSE_MESSAGE(shim.handleReceivedProtobuf(mp, &user), - "unsigned unicast NodeInfo from a signer must not be dropped"); + const uint8_t malformedPlaintext[] = {0xFF, 0xFF, 0xFF}; + meshtastic_NodeInfoLite_public_key_t localKey = {32, {0}}; + memcpy(localKey.bytes, localPub, sizeof(localPub)); + meshtastic_MeshPacket malformed = meshtastic_MeshPacket_init_zero; + malformed.from = REMOTE_NODE; + malformed.to = LOCAL_NODE; + malformed.id = 0xCB00000B; + malformed.channel = 0; + malformed.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; + crypto->setDHPrivateKey(remotePriv); + TEST_ASSERT_TRUE(crypto->encryptCurve25519(malformed.to, malformed.from, localKey, malformed.id, sizeof(malformedPlaintext), + malformedPlaintext, malformed.encrypted.bytes)); + malformed.encrypted.size = sizeof(malformedPlaintext) + MESHTASTIC_PKC_OVERHEAD; + crypto->setDHPrivateKey(localPriv); + + const uint32_t lastHeard = mockNodeDB->getMeshNode(REMOTE_NODE)->last_heard; + moduleConfig.mqtt.enabled = true; + runPipelineIngress(malformed); + assertNoRejectedPipelineEffects(REMOTE_NODE, lastHeard); + TEST_ASSERT_FALSE(pipelineRouter->historyContains(&malformed)); +} + +void test_C12_exact_authenticated_replay_reuses_verdict_without_collision_bypass(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + preparePipelineSigner(REMOTE_NODE); + meshtastic_MeshPacket valid = makeSignedWirePacket(REMOTE_NODE, NODENUM_BROADCAST, 0xCC00000C); + // Full ingress replaces this nonzero wire timestamp with the local arrival time. The exact + // authentication handoff must be consumed before that mutation, avoiding a second evaluation. + valid.rx_time = 0x12345678; + runPipelineIngress(valid); + TEST_ASSERT_EQUAL_MESSAGE(1, routingAuthEvaluationCount(), "full ingress must consume the primed verdict exactly once"); + runPipelineIngress(valid); + TEST_ASSERT_EQUAL_MESSAGE(2, routingAuthEvaluationCount(), "consumed verdict must not authenticate a later replay"); + + meshtastic_MeshPacket collision = valid; + collision.encrypted.bytes[0] ^= 0x80; + TEST_ASSERT_EQUAL(static_cast(RoutingAuthVerdict::REJECT), static_cast(passesRoutingAuthGate(&collision))); + TEST_ASSERT_EQUAL_MESSAGE(3, routingAuthEvaluationCount(), "same packet ID with different bytes must be reevaluated"); } // =========================================================================== @@ -628,7 +1234,7 @@ void test_E5_decoded_unsigned_oversized_broadcast_from_signer_accepted(void) TEST_ASSERT_TRUE(checkXeddsaReceivePolicy(&p)); } -// E6: unsigned unicast from a signer -> accepted (unicast is never signed). +// E6: Balanced accepts unsigned unicast from a signer for legacy compatibility. void test_E6_decoded_unsigned_unicast_from_signer_accepted(void) { mockNodeDB->addNode(REMOTE_NODE); @@ -639,20 +1245,6 @@ void test_E6_decoded_unsigned_unicast_from_signer_accepted(void) TEST_ASSERT_TRUE(checkXeddsaReceivePolicy(&p)); } -// E7: unsigned PKI-flagged packet from a signer -> accepted. Senders never sign PKI traffic, -// so the predicate's !pki_encrypted guard must exempt it (pins the assumption that the -// downgrade drop can never fire on PKI packets, whatever their addressing). -void test_E7_decoded_unsigned_pki_from_signer_accepted(void) -{ - mockNodeDB->addNode(REMOTE_NODE); - mockNodeDB->setSignerBit(REMOTE_NODE, true); - - meshtastic_MeshPacket p = makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD); - p.pki_encrypted = true; - - TEST_ASSERT_TRUE(checkXeddsaReceivePolicy(&p)); -} - // E8: a crafted partial (non-0, non-64) signature must not let a forged broadcast dodge the // downgrade drop. A 63-byte junk signature inflates the encoded size past the fit threshold, so // a size-only predicate would treat the packet as "too big to sign" and accept it as an @@ -688,6 +1280,19 @@ void test_E9_decoded_partial_signature_from_nonsigner_dropped(void) void setup() { initializeTestEnvironment(); + + config.lora.region = meshtastic_Config_LoRaConfig_RegionCode_US; + initRegion(); + pipelineRouter = new AuthPipelineRouter(); + auto pipelineRadioOwner = std::make_unique(); + pipelineRadio = pipelineRadioOwner.get(); + pipelineRouter->addInterface(std::move(pipelineRadioOwner)); + router = pipelineRouter; + routingModule = pipelineRouting = new AuthPipelineRoutingModule(); + pipelineModule = new AuthPipelineModule(); + service = pipelineService = new MeshService(); + mqtt = pipelineMqtt = new AuthPipelineMqtt(); + UNITY_BEGIN(); printf("\n=== Group A: receive-side accept/reject ===\n"); @@ -700,6 +1305,17 @@ void setup() RUN_TEST(test_A7_unsigned_oversized_broadcast_from_signer_accepted); RUN_TEST(test_A8_unsigned_deadband_broadcast_from_signer_accepted); RUN_TEST(test_A9_unsigned_boundary_broadcast_from_signer_still_dropped); + RUN_TEST(test_A10_compatible_accepts_unsigned_broadcast_from_signer); + RUN_TEST(test_A11_strict_rejects_unsigned_all_portnums_destinations_and_sizes); + RUN_TEST(test_A12_strict_rejects_signed_packet_without_key); + RUN_TEST(test_A13_strict_accepts_locally_authenticated_pki_packet); + RUN_TEST(test_A13b_strict_rejects_spoofed_pki_flag_on_encrypted_ingress); + RUN_TEST(test_A14_strict_bootstraps_identity_bound_signed_nodeinfo); + RUN_TEST(test_A15_strict_rejects_nodeinfo_key_without_identity_binding); + RUN_TEST(test_A16_compatible_rejects_invalid_first_contact_nodeinfo); +#if WARM_NODE_COUNT > 0 + RUN_TEST(test_A17_strict_verifies_signer_from_warm_key_store); +#endif printf("\n=== Group B: send-side signing policy ===\n"); RUN_TEST(test_B1_local_broadcast_is_signed); @@ -708,12 +1324,21 @@ void setup() RUN_TEST(test_B4_all_broadcast_sizes_deliverable_no_deadband); RUN_TEST(test_B5_preset_signature_on_local_packet_cleared); RUN_TEST(test_B6_rich_shape_sweep_no_deadband); + RUN_TEST(test_B7_infrastructure_port_signing_matrix); - printf("\n=== Group C: NodeInfoModule downgrade drop ===\n"); - RUN_TEST(test_C1_unsigned_nodeinfo_from_signer_dropped); - RUN_TEST(test_C2_signed_nodeinfo_from_signer_not_dropped); - RUN_TEST(test_C3_unsigned_nodeinfo_from_nonsigner_not_dropped); - RUN_TEST(test_C4_unsigned_unicast_nodeinfo_from_signer_accepted); + printf("\n=== Group C: routing pipeline authentication ordering ===\n"); + RUN_TEST(test_C1_invalid_first_copy_does_not_poison_valid_same_id); + RUN_TEST(test_C2_invalid_ordinary_duplicate_has_no_cancel_or_delivery_effects); + RUN_TEST(test_C3_invalid_repeated_packet_cannot_ack_or_change_retry_state); + RUN_TEST(test_C4_invalid_fallback_packet_cannot_relay); + RUN_TEST(test_C5_invalid_upgrade_cannot_remove_pending_valid_send); + RUN_TEST(test_C6_opaque_unknown_channel_is_relay_only); + RUN_TEST(test_C7_strict_rejects_unsigned_decoded_simradio_ingress); + RUN_TEST(test_C8_trusted_local_decoded_delivery_is_not_filtered); + RUN_TEST(test_C9_known_channel_malformed_plaintext_is_not_relayed_as_opaque); + 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 D: encoding invariants ===\n"); RUN_TEST(test_D1_signature_field_overhead_exact); @@ -725,7 +1350,6 @@ void setup() RUN_TEST(test_E4_decoded_bad_signature_dropped); RUN_TEST(test_E5_decoded_unsigned_oversized_broadcast_from_signer_accepted); RUN_TEST(test_E6_decoded_unsigned_unicast_from_signer_accepted); - RUN_TEST(test_E7_decoded_unsigned_pki_from_signer_accepted); RUN_TEST(test_E8_decoded_partial_signature_from_signer_dropped); RUN_TEST(test_E9_decoded_partial_signature_from_nonsigner_dropped); diff --git a/variants/stm32/CDEBYTE_E77-MBL/platformio.ini b/variants/stm32/CDEBYTE_E77-MBL/platformio.ini index cb980db10..f0f5d7cdc 100644 --- a/variants/stm32/CDEBYTE_E77-MBL/platformio.ini +++ b/variants/stm32/CDEBYTE_E77-MBL/platformio.ini @@ -16,5 +16,8 @@ build_flags = -DMESHTASTIC_EXCLUDE_AIR_QUALITY_SENSOR=1 -DMESHTASTIC_EXCLUDE_I2C=1 -DMESHTASTIC_EXCLUDE_GPS=1 +build_unflags = + ${arduino_base.build_unflags} + -DMESHTASTIC_EXCLUDE_XEDDSA=1 -upload_port = stlink \ No newline at end of file +upload_port = stlink diff --git a/variants/stm32/rak3172/platformio.ini b/variants/stm32/rak3172/platformio.ini index de8f2b74b..1b63eaada 100644 --- a/variants/stm32/rak3172/platformio.ini +++ b/variants/stm32/rak3172/platformio.ini @@ -15,5 +15,8 @@ build_flags = -DMESHTASTIC_EXCLUDE_AIR_QUALITY_SENSOR=1 -DMESHTASTIC_EXCLUDE_I2C=1 -DMESHTASTIC_EXCLUDE_GPS=1 +build_unflags = + ${arduino_base.build_unflags} + -DMESHTASTIC_EXCLUDE_XEDDSA=1 upload_port = stlink diff --git a/variants/stm32/stm32.ini b/variants/stm32/stm32.ini index 5726fd369..be4638e68 100644 --- a/variants/stm32/stm32.ini +++ b/variants/stm32/stm32.ini @@ -25,7 +25,7 @@ build_flags = -DMESHTASTIC_EXCLUDE_BLUETOOTH=1 -DMESHTASTIC_EXCLUDE_WIFI=1 -DMESHTASTIC_EXCLUDE_TZ=1 ; Exclude TZ to save some flash space. - -DMESHTASTIC_EXCLUDE_XEDDSA=1 ; The Ed25519 signing code does not fit in the 256KB flash. Packets are sent unsigned, like pre-XEdDSA firmware. + -DMESHTASTIC_EXCLUDE_XEDDSA=1 ; Individual STM32WL variants opt in after size validation. -DSERIAL_RX_BUFFER_SIZE=256 ; For GPS - the default of 64 is too small. -DHAS_SCREEN=0 ; Always disable screen for STM32, it is not supported. ;-DPIO_FRAMEWORK_ARDUINO_NANOLIB_FLOAT_PRINTF ; Enable this if enabling debugg logging. It is REQUIRED for at least traceroute debug prints - without it the length returned by printf ends up uninitialized. @@ -61,4 +61,4 @@ lib_ignore = OneButton ; Set a custom linker script with a higher MinStackSize value, to match NRF52. -board_build.ldscript = $PROJECT_DIR/variants/stm32/stm32wle5xx.ld \ No newline at end of file +board_build.ldscript = $PROJECT_DIR/variants/stm32/stm32wle5xx.ld From 82ad09aaee89b22c462391acda7835fcbdd81328 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:08:30 -0700 Subject: [PATCH 2/8] docs: clarify routing auth gate contract --- src/mesh/Router.h | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 5711ab4e5..74824be0a 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -175,10 +175,7 @@ enum class RoutingAuthVerdict { ACCEPT, OPAQUE_RELAY_ONLY, REJECT }; */ DecodeState perhapsDecode(meshtastic_MeshPacket *p); -/** Apply the receive authentication policy before routing state is mutated. - * Decryptable packets must pass their configured authenticity policy. Packets for unknown channels - * remain eligible for opaque relay, while fatal and policy-rejected packets are filtered. - */ +/** Apply receive authentication before routing state mutation; unknown-channel packets may remain opaque relay-only. */ RoutingAuthVerdict passesRoutingAuthGate(meshtastic_MeshPacket *p); #ifdef PIO_UNIT_TESTING uint32_t routingAuthEvaluationCount(); @@ -190,21 +187,8 @@ void resetRoutingAuthEvaluationCount(); meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p); #if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA) -/** XEdDSA receive-side signature policy. Valid signatures are verified against a cached key or an - * identity-bound key in first-contact NodeInfo. Invalid and malformed signatures always fail. - * Compatible accepts unsigned traffic, Balanced rejects signable unsigned broadcasts from known - * signers, and Strict requires a verified existing signature or successful PKI authentication - * for all decoded traffic. - * - * encodedDataSize is the wire size of the encoded Data as the sender built it; pass 0 to size - * p->decoded canonically instead (for already-decoded ingress such as plaintext-MQTT downlink, - * which bypasses perhapsDecode's crypto path). - * - * The caller MUST hold cryptLock: verification runs through the shared CryptoEngine key cache. - * (perhapsDecode already holds it; other call sites must take it themselves.) - * - * @return false if the packet must be dropped. - */ +/** Enforce the configured XEdDSA receive policy; zero encodedDataSize derives it canonically. + * The caller must hold cryptLock. Returns false when the packet must be dropped. */ bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p, size_t encodedDataSize = 0); #endif From 1317176f7845c05cac239cc64475085eb6ea664e Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Mon, 20 Jul 2026 19:12:28 -0500 Subject: [PATCH 3/8] test: accept DECODE_OPAQUE/DECODE_POLICY_REJECT in perhapsDecode fuzz assertion The packet-auth-policy change extends the DecodeState enum with DECODE_OPAQUE and DECODE_POLICY_REJECT. test_E1_perhaps_decode_fuzz drives arbitrary ciphertext through perhapsDecode and asserted the verdict was one of the original three states; random ciphertext that matches no channel with no PKI attempt now returns DECODE_OPAQUE, tripping the assertion. Broaden the check to accept all five valid verdicts, matching the test's stated 'any verdict is fine' contract. --- test/test_fuzz_packets/test_main.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_fuzz_packets/test_main.cpp b/test/test_fuzz_packets/test_main.cpp index c65b5460e..c9edcabb1 100644 --- a/test/test_fuzz_packets/test_main.cpp +++ b/test/test_fuzz_packets/test_main.cpp @@ -159,7 +159,8 @@ void test_E1_perhaps_decode_fuzz(void) DecodeState st = perhapsDecode(&p); // Any verdict is fine; the contract is that arbitrary ciphertext never crashes the pipeline. - TEST_ASSERT_TRUE(st == DECODE_SUCCESS || st == DECODE_FAILURE || st == DECODE_FATAL); + TEST_ASSERT_TRUE(st == DECODE_SUCCESS || st == DECODE_FAILURE || st == DECODE_OPAQUE || st == DECODE_FATAL || + st == DECODE_POLICY_REJECT); } } From e247f70c6dc48bd328969e5542b1e3de9da34b36 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Tue, 21 Jul 2026 05:27:23 -0500 Subject: [PATCH 4/8] test: drain toPhoneQueue in MQTT test to fix ASan leak sendLocal() now dispatches local packets through handleReceived() directly instead of the mock-overridden enqueueReceivedMessage(). The MQTT implicit ACK-to-self therefore runs the full receive pipeline (RoutingModule -> MeshService::handleFromRadio -> sendToPhone), enqueuing pooled MeshPacket copies into toPhoneQueue. Production drains that queue via PhoneAPI, but the test has no phone reader, so the copies leaked at teardown (LeakSanitizer: 42824 bytes / 101 objects across test_receiveFuzzServiceEnvelope and test_receiveAcksOwnSentMessages). Give MockMeshService a destructor that drains toPhoneQueue like the phone would. Test-only; the real firmware does not leak here. --- test/test_mqtt/MQTT.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/test_mqtt/MQTT.cpp b/test/test_mqtt/MQTT.cpp index 270e8abe3..78f8b32f5 100644 --- a/test/test_mqtt/MQTT.cpp +++ b/test/test_mqtt/MQTT.cpp @@ -53,6 +53,15 @@ class MockRouter : public Router class MockMeshService : public MeshService { public: + // No PhoneAPI reader exists in these tests, so packets the receive pipeline forwards to the phone + // (MeshService::sendToPhone enqueues pooled copies into toPhoneQueue) would leak at teardown. Drain + // the queue like the phone would. This surfaced once sendLocal() began dispatching local packets + // through handleReceived() directly rather than via the (mock-overridden) enqueueReceivedMessage(). + ~MockMeshService() + { + while (meshtastic_MeshPacket *p = getForPhone()) + releaseToPool(p); + } void sendMqttMessageToClientProxy(meshtastic_MqttClientProxyMessage *m) override { messages_.emplace_back(*m); From d587f084811d7af9917aeedbb05dac11823224ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Tue, 21 Jul 2026 14:16:22 +0200 Subject: [PATCH 5/8] Pin admin responses to the stored peer key and request id (#11092) * Pin admin responses to the stored peer key and request id noteOutgoingAdminRequest derived its PKC pin from p.public_key, which nothing populates on the outgoing path, so keyValid was false for every client request and the pin never engaged. The accepted-response predicate reduced to an unauthenticated from plus variant and subtype. Resolve the destination key from NodeDB the way perhapsEncode does, and pin it only when the request would actually be PKC-encrypted. Extract that condition from perhapsEncode as wouldEncryptWithPKC so both use one predicate. Also record the request's packet id and require the response to echo it. * Treat a zero request id as no pairing token --- src/mesh/Router.cpp | 51 ++++--- src/mesh/Router.h | 12 ++ src/modules/AdminModule.cpp | 18 ++- src/modules/AdminModule.h | 3 +- test/test_admin_session_repro/test_main.cpp | 150 +++++++++++++++++--- 5 files changed, 190 insertions(+), 44 deletions(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index f8cd05b51..779f52c1c 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -822,6 +822,34 @@ static bool signedDataFits(meshtastic_Data *d) } #endif +#if !(MESHTASTIC_EXCLUDE_PKI) +bool wouldEncryptWithPKC(const meshtastic_MeshPacket *p, ChannelIndex chIndex, bool haveDestKey) +{ + // First, only PKC encrypt packets we are originating + return isFromUs(p) && +#if ARCH_PORTDUINO + // Sim radio via the cli flag skips PKC + !portduino_config.force_simradio && +#endif + // Don't use PKC with Ham mode + !owner.is_licensed && + // Don't use PKC on 'serial' or 'gpio' channels unless explicitly requested + !(p->pki_encrypted != true && (strcasecmp(channels.getName(chIndex), Channels::serialChannel) == 0 || + strcasecmp(channels.getName(chIndex), Channels::gpioChannel) == 0)) && + // Check for valid keys and single node destination + 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 && + // 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. + // Though possible the first packet each direction should go non-pkc + // to handle the case where the remote node has our key, but we don't have theirs. + !(p->decoded.portnum == meshtastic_PortNum_KEY_VERIFICATION_APP && !haveDestKey); +} +#endif + /** Return 0 for success or a Routing_Error code for failure */ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p) @@ -915,28 +943,7 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p) } // We may want to retool things so we can send a PKC packet when the client specifies a key and nodenum, even if the node // is not in the local nodedb - // First, only PKC encrypt packets we are originating - if (isFromUs(p) && -#if ARCH_PORTDUINO - // Sim radio via the cli flag skips PKC - !portduino_config.force_simradio && -#endif - // Don't use PKC with Ham mode - !owner.is_licensed && - // Don't use PKC on 'serial' or 'gpio' channels unless explicitly requested - !(p->pki_encrypted != true && (strcasecmp(channels.getName(chIndex), Channels::serialChannel) == 0 || - strcasecmp(channels.getName(chIndex), Channels::gpioChannel) == 0)) && - // Check for valid keys and single node destination - 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 && - // 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. - // Though possible the first packet each direction should go non-pkc - // to handle the case where the remote node has our key, but we don't have theirs. - !(p->decoded.portnum == meshtastic_PortNum_KEY_VERIFICATION_APP && !haveDestKey)) { + if (wouldEncryptWithPKC(p, chIndex, haveDestKey)) { LOG_DEBUG("Use PKI!"); if (numbytes + MESHTASTIC_HEADER_LENGTH + MESHTASTIC_PKC_OVERHEAD > MAX_LORA_PAYLOAD_LEN) return meshtastic_Routing_Error_TOO_LARGE; diff --git a/src/mesh/Router.h b/src/mesh/Router.h index fed0a4500..df1448925 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -193,6 +193,18 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p); bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p); #endif +#if !(MESHTASTIC_EXCLUDE_PKI) +/** + * Would perhapsEncode() PKC-encrypt this outgoing packet? Callers that must know the encryption a + * packet will get before it is encoded (e.g. pinning a peer key at request time) have to ask this + * rather than inspect p, whose pki_encrypted/public_key fields are only populated on the RX path. + * + * @param chIndex the channel index p carries before encoding rewrites it to a hash. + * @param haveDestKey whether a public key for p->to was resolvable. + */ +bool wouldEncryptWithPKC(const meshtastic_MeshPacket *p, ChannelIndex chIndex, bool haveDestKey); +#endif + extern Router *router; /// Generate a unique packet id diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 5f4d62251..4df4ab474 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -1975,7 +1975,15 @@ void AdminModule::noteOutgoingAdminRequest(const meshtastic_MeshPacket &p) if (!responseVariant) return; // not a getter whose response we can pair - const bool keyValid = p.pki_encrypted && p.public_key.size == 32; + // Pin the key perhapsEncode will actually encrypt to, resolved the same way it resolves it. + // p.public_key is NOT it: nothing populates that field on the outgoing path (only perhapsDecode + // sets it, on RX), so reading it here pinned nothing and left `from` as the sole check. + bool keyValid = false; + meshtastic_NodeInfoLite_public_key_t destKey = {0, {0}}; +#if !(MESHTASTIC_EXCLUDE_PKI) + const bool haveDestKey = nodeDB->copyPublicKey(p.to, destKey); + keyValid = haveDestKey && wouldEncryptWithPKC(&p, p.channel, haveDestKey); +#endif // One entry per request (a client sends N indexed get_channel requests, each answered once, so // entries must not merge). Free slot, else evict the oldest by rollover-safe elapsed time. @@ -1994,6 +2002,7 @@ void AdminModule::noteOutgoingAdminRequest(const meshtastic_MeshPacket &p) } slot->to = p.to; + slot->requestId = p.id; slot->sentAtMs = millis(); slot->expectedResponse = responseVariant; slot->moduleConfigType = admin.which_payload_variant == meshtastic_AdminMessage_get_module_config_request_tag @@ -2001,7 +2010,7 @@ void AdminModule::noteOutgoingAdminRequest(const meshtastic_MeshPacket &p) : 0; slot->keyValid = keyValid; if (keyValid) - memcpy(slot->key, p.public_key.bytes, 32); + memcpy(slot->key, destKey.bytes, 32); else memset(slot->key, 0, 32); LOG_DEBUG("Admin request sent to 0x%08x, expecting its response", p.to); @@ -2014,6 +2023,11 @@ bool AdminModule::responseIsSolicited(const meshtastic_MeshPacket &mp, pb_size_t for (auto &o : outstandingAdminRequests) { if (o.to != mp.from || o.expectedResponse != responseVariant) continue; + // mp.from is unauthenticated, so also require the response to echo our request's packet id + // (setReplyTo puts it in decoded.request_id). A blind injector must now guess it. Id 0 is + // no token at all - an omitted request_id decodes to 0 - so such a slot never matches. + if (o.requestId == 0 || mp.decoded.request_id != o.requestId) + continue; if (!Throttle::isWithinTimespanMs(o.sentAtMs, kOutstandingAdminRequestMs)) { o.to = 0; // lapsed; free the slot and keep looking for another live match continue; diff --git a/src/modules/AdminModule.h b/src/modules/AdminModule.h index 5c1a6ef58..1bf98be10 100644 --- a/src/modules/AdminModule.h +++ b/src/modules/AdminModule.h @@ -90,10 +90,11 @@ class AdminModule : public ProtobufModule, public Obser static constexpr uint32_t kOutstandingAdminRequestMs = 300 * 1000; // same window as the session passkey struct OutstandingAdminRequest { NodeNum to; // 0 = free slot + uint32_t requestId; // our request's packet id; the response must echo it as request_id uint32_t sentAtMs; // millis() when this request went out pb_size_t expectedResponse; // the one response variant this request authorizes uint8_t moduleConfigType; // for get_module_config_request: which ModuleConfigType we asked - uint8_t key[32]; // pinned destination key when the request went out over PKC + uint8_t key[32]; // pinned destination key when the request goes out over PKC bool keyValid; }; OutstandingAdminRequest outstandingAdminRequests[kOutstandingAdminRequests] = {}; diff --git a/test/test_admin_session_repro/test_main.cpp b/test/test_admin_session_repro/test_main.cpp index 890a9079e..00c90e381 100644 --- a/test/test_admin_session_repro/test_main.cpp +++ b/test/test_admin_session_repro/test_main.cpp @@ -29,8 +29,43 @@ static const uint8_t ADMIN_KEY[32] = {0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0xcc, 0xdd, 0xee, 0xff, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x20}; +// MeshService assigns every outgoing packet an id before noteOutgoingAdminRequest sees it, and +// setReplyTo echoes it back as decoded.request_id, so the pairing is keyed on it. +static constexpr uint32_t REQUEST_ID = 0x5EED0001; +static const uint8_t QUERIED_KEY[32] = {0xC1, 0xC2, 0xC3, 0xC4, 0xC5, 0xC6, 0xC7, 0xC8, 0xC9, 0xCa, 0xCb, + 0xCc, 0xCd, 0xCe, 0xCf, 0xD0, 0xD1, 0xD2, 0xD3, 0xD4, 0xD5, 0xD6, + 0xD7, 0xD8, 0xD9, 0xDa, 0xDb, 0xDc, 0xDd, 0xDe, 0xDf, 0xE0}; + +// NodeDB with injectable nodes, so a destination can have a stored public key to pin. +class MockNodeDB : public NodeDB +{ + public: + void clearTestNodes() + { + testNodes.clear(); + meshNodes = &testNodes; + numMeshNodes = 0; + } + + void addNodeWithKey(NodeNum num, const uint8_t *key) + { + meshtastic_NodeInfoLite n = meshtastic_NodeInfoLite_init_zero; + n.num = num; + if (key) { + n.public_key.size = 32; + memcpy(n.public_key.bytes, key, 32); + } + testNodes.push_back(n); + meshNodes = &testNodes; + numMeshNodes = testNodes.size(); + } + + std::vector testNodes; +}; + static MockMeshService *mockService = nullptr; static AdminModuleTestShim *admin = nullptr; +static MockNodeDB *mockNodeDB = nullptr; // A remote, PKC-authorized set_owner. `session` (if non-empty) is the session_passkey the client presents. static meshtastic_MeshPacket makeRemoteSetOwner(const char *newLongName, const uint8_t *session, size_t sessionLen, @@ -71,6 +106,7 @@ static meshtastic_MeshPacket makeModuleConfigResponse(NodeNum from, meshtastic_A mp.to = LOCAL_NODE; mp.channel = 0; mp.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + mp.decoded.request_id = REQUEST_ID; // setReplyTo echoes the request's packet id return mp; } @@ -93,6 +129,7 @@ static meshtastic_MeshPacket makeOutgoingRequest(NodeNum to, const meshtastic_Ad p.to = to; p.which_payload_variant = meshtastic_MeshPacket_decoded_tag; p.decoded.portnum = meshtastic_PortNum_ADMIN_APP; + p.id = REQUEST_ID; p.decoded.payload.size = pb_encode_to_bytes(p.decoded.payload.bytes, sizeof(p.decoded.payload.bytes), &meshtastic_AdminMessage_msg, &req); return p; @@ -114,11 +151,16 @@ void setUp(void) admin = new AdminModuleTestShim(); admin->deferSaves(); // no disk/reboot side effects when a setter is accepted - if (!nodeDB) - nodeDB = new NodeDB(); + if (!mockNodeDB) + mockNodeDB = new MockNodeDB(); + mockNodeDB->clearTestNodes(); + nodeDB = mockNodeDB; myNodeInfo.my_node_num = LOCAL_NODE; config = meshtastic_LocalConfig_init_zero; + // A real device always holds a private key; without one perhapsEncode never picks PKC. + config.security.private_key.size = 32; + memset(config.security.private_key.bytes, 0xA5, 32); // Authorize ADMIN_NODE's key as an admin key so the PKC path accepts it and we reach the session gate. config.security.admin_key[0].size = 32; memcpy(config.security.admin_key[0].bytes, ADMIN_KEY, 32); @@ -411,38 +453,104 @@ void test_response_variant_must_match_request(void) // must not relax the PKC pin of an earlier one (the old shared-slot model cleared it). void test_pinned_request_keeps_its_key_after_an_unpinned_request(void) { - uint8_t key[32]; - memset(key, 0xC1, 32); + // QUERIED has a stored key, so a request to it will be PKC-encrypted and pins that key. + // STRANGER has none, so a request to it cannot be pinned. + mockNodeDB->addNodeWithKey(QUERIED_NODE, QUERIED_KEY); + mockNodeDB->addNodeWithKey(STRANGER_NODE, nullptr); - // A PKC-pinned get_config request to STRANGER (pins `key`). meshtastic_AdminMessage cfg = meshtastic_AdminMessage_init_zero; cfg.which_payload_variant = meshtastic_AdminMessage_get_config_request_tag; - meshtastic_MeshPacket pinned = makeOutgoingRequest(STRANGER_NODE, cfg); - pinned.pki_encrypted = true; - pinned.public_key.size = 32; - memcpy(pinned.public_key.bytes, key, 32); - admin->noteOutgoingAdminRequest(pinned); + admin->noteOutgoingAdminRequest(makeOutgoingRequest(QUERIED_NODE, cfg)); - // A later, unpinned get_owner request to the same node. meshtastic_AdminMessage own = meshtastic_AdminMessage_init_zero; own.which_payload_variant = meshtastic_AdminMessage_get_owner_request_tag; admin->noteOutgoingAdminRequest(makeOutgoingRequest(STRANGER_NODE, own)); meshtastic_AdminMessage m; - meshtastic_MeshPacket resp = makeModuleConfigResponse(STRANGER_NODE, m); // from set; pki off by default + meshtastic_MeshPacket resp = makeModuleConfigResponse(QUERIED_NODE, m); // pki off by default - // A plaintext get_config_response is still rejected: the config request was pinned. TEST_ASSERT_FALSE_MESSAGE(admin->responseIsSolicited(resp, meshtastic_AdminMessage_get_config_response_tag, 0), "an unpinned request must not relax an earlier request's key pin"); - // Over PKC with the pinned key it is accepted. + resp.pki_encrypted = true; resp.public_key.size = 32; - memcpy(resp.public_key.bytes, key, 32); + memcpy(resp.public_key.bytes, QUERIED_KEY, 32); TEST_ASSERT_TRUE(admin->responseIsSolicited(resp, meshtastic_AdminMessage_get_config_response_tag, 0)); - // The unpinned get_owner_response is accepted in plaintext (its request carried no pin). - resp.pki_encrypted = false; - resp.public_key.size = 0; - TEST_ASSERT_TRUE(admin->responseIsSolicited(resp, meshtastic_AdminMessage_get_owner_response_tag, 0)); +} + +// The pin is taken from the destination's stored NodeDB key - the key perhapsEncode will encrypt +// to. Reading the outgoing packet's public_key instead pinned nothing: nothing populates that +// field before encryption, so every real request was unpinned and `from` alone admitted responses. +void test_request_to_keyed_node_pins_the_stored_key(void) +{ + mockNodeDB->addNodeWithKey(QUERIED_NODE, QUERIED_KEY); + admin->noteOutgoingAdminRequest(makeOutgoingModuleConfigRequest(QUERIED_NODE)); + + meshtastic_AdminMessage m; + meshtastic_MeshPacket plain = makeModuleConfigResponse(QUERIED_NODE, m); + TEST_ASSERT_FALSE_MESSAGE(admin->responseIsSolicited(plain, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG), + "a plaintext response must not answer a PKC-pinned request"); + + meshtastic_MeshPacket wrongKey = makeModuleConfigResponse(QUERIED_NODE, m); + wrongKey.pki_encrypted = true; + wrongKey.public_key.size = 32; + memset(wrongKey.public_key.bytes, 0x77, 32); + TEST_ASSERT_FALSE_MESSAGE(admin->responseIsSolicited(wrongKey, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG), + "a response under a different key must not be accepted"); + + meshtastic_MeshPacket good = makeModuleConfigResponse(QUERIED_NODE, m); + good.pki_encrypted = true; + good.public_key.size = 32; + memcpy(good.public_key.bytes, QUERIED_KEY, 32); + TEST_ASSERT_TRUE_MESSAGE(admin->responseIsSolicited(good, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG), + "the pinned key must still admit the genuine response"); +} + +// Ham mode never uses PKC, so pinning a key there would reject the legitimate plaintext response. +void test_ham_mode_request_is_not_pinned(void) +{ + owner.is_licensed = true; + mockNodeDB->addNodeWithKey(QUERIED_NODE, QUERIED_KEY); + admin->noteOutgoingAdminRequest(makeOutgoingModuleConfigRequest(QUERIED_NODE)); + + meshtastic_AdminMessage m; + meshtastic_MeshPacket mp = makeModuleConfigResponse(QUERIED_NODE, m); + TEST_ASSERT_TRUE_MESSAGE(admin->responseIsSolicited(mp, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG), + "a request that could not have gone out over PKC must not be pinned"); +} + +// The response must echo our request's packet id, so an injector cannot answer a request it did +// not see just by naming the right node and variant. +void test_response_with_wrong_request_id_is_rejected(void) +{ + admin->noteOutgoingAdminRequest(makeOutgoingModuleConfigRequest(STRANGER_NODE)); + + meshtastic_AdminMessage m; + meshtastic_MeshPacket mp = makeModuleConfigResponse(STRANGER_NODE, m); + mp.decoded.request_id = REQUEST_ID ^ 0xFFFF; // answers some other request + + TEST_ASSERT_FALSE_MESSAGE(admin->responseIsSolicited(mp, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG), + "a response that does not echo our request id must be rejected"); + + mp.decoded.request_id = REQUEST_ID; + TEST_ASSERT_TRUE_MESSAGE(admin->responseIsSolicited(mp, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG), + "the matching request id must still be accepted"); +} + +// A request we could not bind to an id must not be answerable by a response that simply omits +// request_id, which decodes to 0. +void test_request_without_an_id_admits_nothing(void) +{ + meshtastic_MeshPacket req = makeOutgoingModuleConfigRequest(STRANGER_NODE); + req.id = 0; + admin->noteOutgoingAdminRequest(req); + + meshtastic_AdminMessage m; + meshtastic_MeshPacket mp = makeModuleConfigResponse(STRANGER_NODE, m); + mp.decoded.request_id = 0; + + TEST_ASSERT_FALSE_MESSAGE(admin->responseIsSolicited(mp, MODULE_CONFIG_RESPONSE, REMOTE_HW_TAG), + "a zero request id must not act as a matching token"); } // A remote_hardware response must answer a request for that exact subtype, not just any module @@ -542,6 +650,10 @@ void setup() RUN_TEST(test_request_to_one_node_does_not_admit_another); RUN_TEST(test_response_variant_must_match_request); RUN_TEST(test_pinned_request_keeps_its_key_after_an_unpinned_request); + RUN_TEST(test_request_to_keyed_node_pins_the_stored_key); + RUN_TEST(test_ham_mode_request_is_not_pinned); + RUN_TEST(test_response_with_wrong_request_id_is_rejected); + RUN_TEST(test_request_without_an_id_admits_nothing); RUN_TEST(test_module_config_subtype_must_match); RUN_TEST(test_response_is_consumed_no_replay); RUN_TEST(test_outgoing_setter_does_not_admit_responses); From a58daf015d52ee808b4e0c1d88ef032d05e59141 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Tue, 21 Jul 2026 07:24:01 -0500 Subject: [PATCH 6/8] Don't misdetect the ES7210 audio codec as an INA219 (#11120) The T-Deck's ES7210 audio ADC straps to 0x40, which is INA_ADDR. It answers none of the INA226/INA260/SHT2X checks, so it fell into the bare "assume INA219" fallback and PowerTelemetry then drove an audio codec as a power monitor, panicking the board into a boot loop. The INA219 has no manufacturer or die ID register, so it can only ever be inferred. Positively identify the ES7210 instead, via its chip ID registers 0x3D/0x3E (0x72/0x10), and leave the device unclaimed. Also guard the INA219 fallback on the type rather than chaining it to the SHT2X check: that else bound to the SHT2X if, so a positively identified INA226/INA260 was overwritten with INA219 right after being found. Fixes #11115 --- src/detect/ScanI2CTwoWire.cpp | 50 ++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/src/detect/ScanI2CTwoWire.cpp b/src/detect/ScanI2CTwoWire.cpp index 151e559f1..429928d57 100644 --- a/src/detect/ScanI2CTwoWire.cpp +++ b/src/detect/ScanI2CTwoWire.cpp @@ -220,6 +220,44 @@ bool detectSHT21SerialNumber(TwoWire *i2cBus, uint8_t address) } #endif +// Everest Semiconductor ES7210 4-channel audio ADC, as found on the T-Deck. Its AD1/AD0 straps +// select 0x40..0x43, so it lands squarely on the INA/SHT2X addresses. Chip ID registers and their +// reset values, per the ES7210 datasheet rev 2.0. +static constexpr uint8_t ES7210_CHIP_ID1_REG = 0x3D; +static constexpr uint8_t ES7210_CHIP_ID1 = 0x72; +static constexpr uint8_t ES7210_CHIP_ID0_REG = 0x3E; +static constexpr uint8_t ES7210_CHIP_ID0 = 0x10; + +static bool readByteRegister(TwoWire *i2cBus, uint8_t address, uint8_t reg, uint8_t &value) +{ + i2cBus->beginTransmission(address); + i2cBus->write(reg); + + if (i2cBus->endTransmission() != 0) + return false; + + if (i2cBus->requestFrom(address, (uint8_t)1) != 1 || !i2cBus->available()) + return false; + + value = i2cBus->read(); + return true; +} + +// The INA219 has no manufacturer or die ID register to check, so it is inferred from "something +// answered here and it wasn't anything else we know". That makes positively identifying the other +// occupants of this address the only way to keep them out of the fallback. +static bool detectES7210(TwoWire *i2cBus, uint8_t address) +{ + uint8_t id = 0; + + // Read the high ID byte first so anything that clearly isn't an ES7210 costs a single + // transaction, then confirm with the low byte. + if (!readByteRegister(i2cBus, address, ES7210_CHIP_ID1_REG, id) || id != ES7210_CHIP_ID1) + return false; + + return readByteRegister(i2cBus, address, ES7210_CHIP_ID0_REG, id) && id == ES7210_CHIP_ID0; +} + #define SCAN_SIMPLE_CASE(ADDR, T, ...) \ case ADDR: \ logFoundDevice(__VA_ARGS__); \ @@ -472,13 +510,23 @@ void ScanI2CTwoWire::scanPort(I2CPort port, uint8_t *address, uint8_t asize) type = INA260; } } + + // The ES7210 audio ADC shares this address on some boards (T-Deck). It answers + // none of the checks above, so identify it here and leave the type unset - driving + // an audio codec as if it were a power monitor crashes the device. See #11115. + if (type == NONE && detectES7210(i2cBus, (uint8_t)addr.address)) { + LOG_INFO("ES7210 audio codec at 0x%x, not a power sensor", (uint8_t)addr.address); + break; + } #if HAS_TELEMETRY && !MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR if (type == NONE && detectSHT21SerialNumber(i2cBus, (uint8_t)addr.address)) { logFoundDevice("SHTXX (SHT2X)", (uint8_t)addr.address); type = SHTXX; } #endif - else { // Assume INA219 if none of the above ones are found + // Guarded on the type rather than chained to the SHT2X check above, so that a + // positively identified INA226/INA260 isn't overwritten right after being found. + if (type == NONE) { // Assume INA219 if none of the above ones are found logFoundDevice("INA219", (uint8_t)addr.address); type = INA219; } From 6fc7c1b1db796f1ff3cacd71be2313d8fed3c969 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Tue, 21 Jul 2026 10:49:35 -0500 Subject: [PATCH 7/8] test: unset force_simradio so admin request pinning is exercised The native test harness boots Portduino in simulated mode, and wouldEncryptWithPKC() short-circuits to false whenever portduino_config.force_simradio is set. noteOutgoingAdminRequest() derives its pin from that predicate: keyValid = haveDestKey && wouldEncryptWithPKC(&p, p.channel, haveDestKey); so under the harness no outgoing admin request is ever key-pinned, and responseIsSolicited() admits a plaintext response to a PKC-pinned request. test_pinned_request_keeps_its_key_after_an_unpinned_request and test_request_to_keyed_node_pins_the_stored_key have therefore failed since #11092 added them, on develop and on every branch that merges it. Instrumenting the predicate shows every other term already satisfied (haveDestKey=1, isFromUs=1, private_key=32, unicast, ADMIN_APP, channel LongFast) with wouldEncryptWithPKC=0, leaving only the simradio guard. Clear the flag in setUp so the fixture models a real device. Skipping PKC under force_simradio is correct for a simulated radio - there is no PKC to pin - so the predicate is left alone rather than relaxed to make a test pass. The only sim-mode path in this suite is AdminModule's exit_simulator intercept, which no test here exercises. Native suite: 38 suites, 723/723 cases, no sanitizer findings. --- test/test_admin_session_repro/test_main.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/test_admin_session_repro/test_main.cpp b/test/test_admin_session_repro/test_main.cpp index 00c90e381..801eeec96 100644 --- a/test/test_admin_session_repro/test_main.cpp +++ b/test/test_admin_session_repro/test_main.cpp @@ -21,6 +21,10 @@ #include "support/MockMeshService.h" #include +#ifdef ARCH_PORTDUINO +#include "platform/portduino/PortduinoGlue.h" +#endif + static constexpr NodeNum LOCAL_NODE = 0x0A0A0A0A; static constexpr NodeNum ADMIN_NODE = 0x0B0B0B0B; // authorized admin, sends remote admin to us static constexpr NodeNum QUERIED_NODE = 0x0C0C0C0C; // a remote we send admin requests to @@ -157,6 +161,14 @@ void setUp(void) nodeDB = mockNodeDB; myNodeInfo.my_node_num = LOCAL_NODE; +#ifdef ARCH_PORTDUINO + // The native test harness boots Portduino in simulated mode, and wouldEncryptWithPKC() + // hard-disables PKC whenever force_simradio is set. Left true, no outgoing admin request is + // ever key-pinned, so the pinning tests below cannot exercise what they are asserting. + // Model a real (non-sim) device instead. + portduino_config.force_simradio = false; +#endif + config = meshtastic_LocalConfig_init_zero; // A real device always holds a private key; without one perhapsEncode never picks PKC. config.security.private_key.size = 32; From ef386afee501a78e4d7e2c5f6545534759b3c9a7 Mon Sep 17 00:00:00 2001 From: Jason P Date: Tue, 21 Jul 2026 11:30:31 -0500 Subject: [PATCH 8/8] Fixes SEEED Indicator when booting to BaseUI --- src/main.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/main.cpp b/src/main.cpp index 37ed61e56..4763c1f0b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1111,6 +1111,23 @@ void setup() #endif #endif +#if defined(SENSECAP_INDICATOR) + // The ST7701 panel shares SCK/MOSI/MISO (41/48/47) with the SX1262, and its host is SPI2_HOST, + // which on the S3 is the same peripheral as the Arduino `SPI` object (FSPI == SPI2). + // LovyanGFX bit-bangs the ST7701 init sequence on those pins, and because this variant builds + // with USE_ARDUINO_HAL_GPIO it does so via Arduino pinMode()/digitalWrite(). pinMode() calls + // perimanSetPinBus(.., ESP32_BUS_TYPE_GPIO, ..), whose deinit callback (spiDetachBus_SCK) ends + // up in spiStopBus() and gates the SPI2 clock. RadioLib then spins forever in spiTransferByte() + // waiting on cmd.update, which never clears on a stopped peripheral, and the watchdog fires. + // + // Restart the bus here, after the panel is up and before the radio is touched. Note that + // SPIClass::begin() early-returns when _spi is already non-NULL, so end() first is required. + SPI.end(); + SPI.begin(LORA_SCK, LORA_MISO, LORA_MOSI, -1); // CS is an IO-expander pin, driven by RadioLib + SPI.setFrequency(4000000); + LOG_DEBUG("SPI2 restarted after ST7701 init (SCK=%d, MISO=%d, MOSI=%d)", LORA_SCK, LORA_MISO, LORA_MOSI); +#endif + auto rIf = initLoRa(); lateInitVariant(); // Do board specific init (see extra_variants/README.md for documentation)