From 5ffd30c4a8ec05dc789172f1b6dc1bdf1a4c1168 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Wed, 17 Jun 2026 13:34:29 -0500 Subject: [PATCH] Fix PKC on portduino sim by working around region blocks keygen and packet length (#10730) * Fix PKC on portduino sim by working around region blocks keygen and packet length * Reserve Compressed framing overhead in sim loopback capacity check The sim loopback re-encodes the Compressed wrapper back into decoded.payload.bytes (the same 233-byte field its data is copied from), so the carried bytes must leave room for protobuf framing. Checking against sizeof(c.data.bytes) (233) let near-max payloads overflow pb_encode_to_bytes(), which returns 0 and silently sends an empty loopback payload. Cap at sizeof(decoded.payload.bytes) minus the worst-case Compressed framing (meshtastic_Compressed_size - data size) for both the ciphertext and plaintext paths. Addresses Copilot review feedback on #10730. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Run trunk fmt --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/mesh/NodeDB.cpp | 11 ++++- src/mesh/udp/UdpMulticastHandler.h | 3 ++ src/platform/portduino/SimRadio.cpp | 77 ++++++++++++++++++++++------- 3 files changed, 71 insertions(+), 20 deletions(-) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index 01d554ab8..90eec34f8 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -3051,8 +3051,15 @@ bool NodeDB::checkLowEntropyPublicKey(const meshtastic_Config_SecurityConfig_pub bool NodeDB::generateCryptoKeyPair(const uint8_t *privateKey) { #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - // Only generate keys for non-licensed users and if LoRa region is set - if (owner.is_licensed || config.lora.region == meshtastic_Config_LoRaConfig_RegionCode_UNSET) { + // Only generate keys for non-licensed users and if the LoRa region is set. The native simulator + // boots region-UNSET but still needs a keypair so PKI-encrypted DMs work between sim nodes, so + // allow keygen there regardless of region. + bool regionBlocksKeygen = config.lora.region == meshtastic_Config_LoRaConfig_RegionCode_UNSET; +#if ARCH_PORTDUINO + if (portduino_config.lora_module == use_simradio) + regionBlocksKeygen = false; +#endif + if (owner.is_licensed || regionBlocksKeygen) { return false; } diff --git a/src/mesh/udp/UdpMulticastHandler.h b/src/mesh/udp/UdpMulticastHandler.h index eb4d90d40..93306c41d 100644 --- a/src/mesh/udp/UdpMulticastHandler.h +++ b/src/mesh/udp/UdpMulticastHandler.h @@ -78,6 +78,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. UniquePacketPoolPacket p = packetPool.allocUniqueCopy(mp); // Unset received SNR/RSSI p->rx_snr = 0; diff --git a/src/platform/portduino/SimRadio.cpp b/src/platform/portduino/SimRadio.cpp index f9fabb617..c483eca02 100644 --- a/src/platform/portduino/SimRadio.cpp +++ b/src/platform/portduino/SimRadio.cpp @@ -209,16 +209,51 @@ void SimRadio::startSend(meshtastic_MeshPacket *txp) isReceiving = false; size_t numbytes = beginSending(txp); meshtastic_MeshPacket *p = packetPool.allocCopy(*txp); - perhapsDecode(p); - meshtastic_Compressed c = meshtastic_Compressed_init_default; - c.portnum = p->decoded.portnum; - // LOG_DEBUG("Send back to simulator with portNum %d", p->decoded.portnum); - if (p->decoded.payload.size <= sizeof(c.data.bytes)) { - memcpy(&c.data.bytes, p->decoded.payload.bytes, p->decoded.payload.size); - c.data.size = p->decoded.payload.size; - } else { - LOG_WARN("Payload size larger than compressed message allows! Send empty payload"); + + // A packet we originate that's encrypted for someone else (a PKI DM, channel == 0) can't be + // decrypted here. Attempting it only logs a spurious "no suitable channel" miss, and the + // ciphertext (up to MAX_LORA_PAYLOAD_LEN + MESHTASTIC_PKC_OVERHEAD) overflows the decoded + // loopback payload. Carry such packets as ciphertext instead so the receiving sim node can + // decrypt them as if they had arrived over the air (see unpackAndReceive()). + bool carryEncrypted = p->pki_encrypted; + if (!carryEncrypted) { + perhapsDecode(p); + // Channel packets we couldn't decrypt (e.g. relaying an unknown channel) are carried too. + carryEncrypted = (p->which_payload_variant == meshtastic_MeshPacket_encrypted_tag); } + + meshtastic_Compressed c = meshtastic_Compressed_init_default; + // The Compressed wrapper is re-encoded back into decoded.payload.bytes (the same 233-byte field + // its data is copied from), so the carried bytes must leave room for the protobuf framing or + // pb_encode_to_bytes() below overflows and silently drops the loopback payload. meshtastic_Compressed_size + // is the max encoded size for a full data field, so (meshtastic_Compressed_size - sizeof(c.data.bytes)) + // is the worst-case framing overhead to reserve. + constexpr size_t loopbackCapacity = sizeof(p->decoded.payload.bytes) - (meshtastic_Compressed_size - sizeof(c.data.bytes)); + if (carryEncrypted) { + // Sentinel portnum UNKNOWN_APP marks the payload as ciphertext for unpackAndReceive(). + c.portnum = meshtastic_PortNum_UNKNOWN_APP; + if (p->encrypted.size <= loopbackCapacity) { + memcpy(&c.data.bytes, p->encrypted.bytes, p->encrypted.size); + c.data.size = p->encrypted.size; + } else { + LOG_WARN("Encrypted payload (%u) exceeds sim loopback capacity (%u)! Send empty payload", (unsigned)p->encrypted.size, + (unsigned)loopbackCapacity); + } + } else { + c.portnum = p->decoded.portnum; + // LOG_DEBUG("Send back to simulator with portNum %d", p->decoded.portnum); + if (p->decoded.payload.size <= loopbackCapacity) { + memcpy(&c.data.bytes, p->decoded.payload.bytes, p->decoded.payload.size); + c.data.size = p->decoded.payload.size; + } else { + LOG_WARN("Payload size larger than compressed message allows! Send empty payload"); + } + } + + // pb_encode_to_bytes writes into decoded.payload, which aliases `encrypted` in the union, so all + // reads of p->encrypted above must be complete before this point. + p->decoded = meshtastic_Data_init_zero; + p->which_payload_variant = meshtastic_MeshPacket_decoded_tag; p->decoded.payload.size = pb_encode_to_bytes(p->decoded.payload.bytes, sizeof(p->decoded.payload.bytes), &meshtastic_Compressed_msg, &c); p->decoded.portnum = meshtastic_PortNum_SIMULATOR_APP; @@ -233,17 +268,23 @@ void SimRadio::unpackAndReceive(meshtastic_MeshPacket &p) { // Simulator packet (=Compressed packet) is encapsulated in a MeshPacket, so need to unwrap first meshtastic_Compressed scratch; - meshtastic_Compressed *decoded = NULL; if (p.which_payload_variant == meshtastic_MeshPacket_decoded_tag) { memset(&scratch, 0, sizeof(scratch)); - p.decoded.payload.size = - pb_decode_from_bytes(p.decoded.payload.bytes, p.decoded.payload.size, &meshtastic_Compressed_msg, &scratch); - if (p.decoded.payload.size) { - decoded = &scratch; - // Extract the original payload and replace - memcpy(&p.decoded.payload, &decoded->data, sizeof(decoded->data)); - // Switch the port from PortNum_SIMULATOR_APP back to the original PortNum - p.decoded.portnum = decoded->portnum; + if (pb_decode_from_bytes(p.decoded.payload.bytes, p.decoded.payload.size, &meshtastic_Compressed_msg, &scratch)) { + if (scratch.portnum == meshtastic_PortNum_UNKNOWN_APP) { + // The sender carried ciphertext verbatim (a packet it couldn't decrypt, e.g. a PKI DM, + // see startSend()). Restore it as an encrypted packet so the router decrypts it as if + // received over the air, instead of treating the ciphertext as a plaintext payload. The + // outer MeshPacket still carries from/to/id/channel/pki_encrypted, which decrypt needs. + p.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; + memcpy(&p.encrypted.bytes, scratch.data.bytes, scratch.data.size); + p.encrypted.size = scratch.data.size; + } else { + // Extract the original payload and replace + memcpy(&p.decoded.payload, &scratch.data, sizeof(scratch.data)); + // Switch the port from PortNum_SIMULATOR_APP back to the original PortNum + p.decoded.portnum = scratch.portnum; + } } else LOG_ERROR("Error decoding proto for simulator message!"); }