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>
This commit is contained in:
co-authored by
GitHub
Copilot Autofix powered by AI
parent
358e4e2fcd
commit
5ffd30c4a8
+9
-2
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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!");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user