From d5e717edd10d5c1e17622b72dad0aa847e30655a Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:04:38 -0700 Subject: [PATCH 01/11] feat: sign licensed plaintext packets Preserve and publish identity keys in licensed mode so existing XEdDSA signatures can authenticate plaintext traffic. Sign licensed broadcasts and direct messages when they fit, while keeping PKI encryption disabled and normal routing behavior unchanged. --- src/mesh/NodeDB.cpp | 7 +++---- src/mesh/Router.cpp | 10 +++++----- src/modules/AdminModule.cpp | 16 ++++++++++++++-- src/modules/NodeInfoModule.cpp | 10 ++-------- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index f46bbfcce48..00af42e212f 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -4029,15 +4029,14 @@ 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 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. + // Generate identity keys once a LoRa region is set. Licensed operation still needs the identity + // key for plaintext signatures, even though the key is never used for PKI encryption. 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) { + if (regionBlocksKeygen) { return false; } diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index d10e36fc2e9..24f6acc71f6 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -813,7 +813,7 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) ChannelIndex chIndex = 0; #if !(MESHTASTIC_EXCLUDE_PKI) meshtastic_NodeInfoLite *ourNode = nullptr; - if (p->channel == 0 && isToUs(p) && p->to > 0 && !isBroadcast(p->to) && rawSize > MESHTASTIC_PKC_OVERHEAD && + if (!owner.is_licensed && p->channel == 0 && isToUs(p) && p->to > 0 && !isBroadcast(p->to) && rawSize > MESHTASTIC_PKC_OVERHEAD && (ourNode = nodeDB->getMeshNode(p->to)) != nullptr && ourNode->public_key.size > 0) { pkiAttempted = true; LOG_DEBUG("Attempt PKI decryption"); @@ -1063,12 +1063,12 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p) // verification at every XEdDSA-enabled receiver that knows our key. p->decoded.xeddsa_signature.size = 0; #if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA) - // Sign broadcast packets when the Data still fits a LoRa frame with the signature - // attached. This must be the exact encoded-size criterion, not a payload-size - // heuristic: a heuristic band where we sign-then-fail-TOO_LARGE breaks packets that + // Licensed packets stay plaintext, so sign both broadcasts and unicasts. Normal mode + // continues to sign broadcasts only. Use the exact encoded size: a payload-size heuristic + // where we sign-then-fail-TOO_LARGE breaks packets that // were deliverable unsigned, and perhapsDecode() applies the mirror-image rule when // deciding whether an unsigned broadcast from a known signer is a downgrade. - if (!p->pki_encrypted && isBroadcast(p->to) && signedDataFits(&p->decoded)) { + if (!p->pki_encrypted && (owner.is_licensed || isBroadcast(p->to)) && signedDataFits(&p->decoded)) { if (crypto->xeddsa_sign(p->from, p->id, p->decoded.portnum, p->decoded.payload.bytes, p->decoded.payload.size, p->decoded.xeddsa_signature.bytes)) { p->decoded.xeddsa_signature.size = XEDDSA_SIGNATURE_SIZE; diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 945802aba5f..f0fbce0433c 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -755,6 +755,7 @@ void AdminModule::handleGetModuleConfigResponse(const meshtastic_MeshPacket &mp, void AdminModule::handleSetOwner(const meshtastic_User &o) { int changed = 0; + bool identityGenerated = false; if (*o.long_name) { // Apps built against the older 39-byte limit may send longer names; clamp @@ -781,6 +782,12 @@ void AdminModule::handleSetOwner(const meshtastic_User &o) if (channels.ensureLicensedOperation()) { warnLicensedMode(); } +#if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) + if ((config.security.private_key.size != 32 || config.security.public_key.size != 32) && + config.lora.region != meshtastic_Config_LoRaConfig_RegionCode_UNSET) { + identityGenerated = nodeDB->generateCryptoKeyPair(); + } +#endif } if (owner.has_is_unmessagable != o.has_is_unmessagable || (o.has_is_unmessagable && owner.is_unmessagable != o.is_unmessagable)) { @@ -791,7 +798,7 @@ void AdminModule::handleSetOwner(const meshtastic_User &o) if (changed) { // If nothing really changed, don't broadcast on the network or write to flash service->reloadOwner(!hasOpenEditTransaction); - saveChanges(SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE); + saveChanges(SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE | (identityGenerated ? SEGMENT_CONFIG : 0)); } } @@ -994,7 +1001,7 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) // If we're setting region for the first time, init the region and regenerate the keys if (isRegionUnset && validatedLora.region > meshtastic_Config_LoRaConfig_RegionCode_UNSET) { #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - if (crypto) { + if (crypto && !owner.is_licensed) { crypto->ensurePkiKeys(config.security, owner); } #endif @@ -1007,6 +1014,11 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) } // Ensure initRegion() uses the newly validated region config.lora.region = validatedLora.region; +#if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) + if (owner.is_licensed && isRegionUnset && validatedLora.region > meshtastic_Config_LoRaConfig_RegionCode_UNSET) { + nodeDB->generateCryptoKeyPair(); + } +#endif initRegion(); if (getEffectiveDutyCycle() < 100) { validatedLora.ignore_mqtt = true; // Ignore MQTT by default if region has a duty cycle limit diff --git a/src/modules/NodeInfoModule.cpp b/src/modules/NodeInfoModule.cpp index 01168114dcf..5b8d4622395 100644 --- a/src/modules/NodeInfoModule.cpp +++ b/src/modules/NodeInfoModule.cpp @@ -169,14 +169,8 @@ meshtastic_MeshPacket *NodeInfoModule::allocReply() ignoreRequest = true; return NULL; } else { - ignoreRequest = false; // Don't ignore requests anymore - meshtastic_User u = owner; // deliberate copy: the licensed strip below must not clobber the global owner state - - // Strip the public key if the user is licensed - if (u.is_licensed && u.public_key.size > 0) { - memset(u.public_key.bytes, 0, sizeof(u.public_key.bytes)); - u.public_key.size = 0; - } + ignoreRequest = false; // Don't ignore requests anymore + meshtastic_User u = owner; // FIXME: Clear the user.id field since it should be derived from node number on the receiving end // u.id[0] = '\0'; From 9ed866fec39e6987f11cde460229929a2fee3203 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:22:55 -0700 Subject: [PATCH 02/11] fix(security): persist licensed channel sanitation --- src/mesh/Channels.cpp | 14 +++--- src/mesh/NodeDB.cpp | 7 ++- src/mesh/NodeDB.h | 1 + src/modules/AdminModule.cpp | 47 +++++++++++++++--- src/modules/AdminModule.h | 8 ++- test/support/AdminModuleTestShim.h | 2 + test/test_admin_radio/test_main.cpp | 69 ++++++++++++++++++++++++++ test/test_packet_signing/test_main.cpp | 26 ++++++++++ 8 files changed, 158 insertions(+), 16 deletions(-) diff --git a/src/mesh/Channels.cpp b/src/mesh/Channels.cpp index f1d97ebb473..2d8d4f246ce 100644 --- a/src/mesh/Channels.cpp +++ b/src/mesh/Channels.cpp @@ -128,11 +128,13 @@ bool Channels::ensureLicensedOperation() } auto &channelSettings = channel.settings; if (strcasecmp(channelSettings.name, Channels::adminChannel) == 0) { - channel.role = meshtastic_Channel_Role_DISABLED; - channelSettings.psk.bytes[0] = 0; - channelSettings.psk.size = 0; - hasEncryptionOrAdmin = true; - channels.setChannel(channel); + if (channel.role != meshtastic_Channel_Role_DISABLED || channelSettings.psk.size > 0) { + channel.role = meshtastic_Channel_Role_DISABLED; + channelSettings.psk.bytes[0] = 0; + channelSettings.psk.size = 0; + hasEncryptionOrAdmin = true; + channels.setChannel(channel); + } } else if (channelSettings.psk.size > 0) { channelSettings.psk.bytes[0] = 0; @@ -563,4 +565,4 @@ bool Channels::setDefaultPresetCryptoForHash(ChannelHash channelHash) int16_t Channels::setActiveByIndex(ChannelIndex channelIndex) { return setCrypto(channelIndex); -} \ No newline at end of file +} diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index 00af42e212f..d78fcc0f43f 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -831,7 +831,7 @@ void NodeDB::installDefaultNodeDatabase() void NodeDB::installDefaultConfig(bool preserveKey = false) { uint8_t private_key_temp[32]; - bool shouldPreserveKey = preserveKey && config.has_security && config.security.private_key.size > 0; + bool shouldPreserveKey = preserveKey && config.has_security && config.security.private_key.size == 32; if (shouldPreserveKey) { memcpy(private_key_temp, config.security.private_key.bytes, config.security.private_key.size); } @@ -2614,6 +2614,11 @@ void NodeDB::loadFromDisk() moduleConfig.version = POSITION_TELEMETRY_OPTIN_VER; saveToDisk(SEGMENT_MODULECONFIG); } + + if (channels.ensureLicensedOperation()) { + LOG_WARN("Licensed operation removed persisted channel encryption/admin access"); + saveToDisk(SEGMENT_CHANNELS); + } #if ARCH_PORTDUINO // set any config overrides if (portduino_config.has_configDisplayMode) { diff --git a/src/mesh/NodeDB.h b/src/mesh/NodeDB.h index 00c0cc3b6f5..6b0dcae0c08 100644 --- a/src/mesh/NodeDB.h +++ b/src/mesh/NodeDB.h @@ -580,6 +580,7 @@ class NodeDB // Grant the unit-test shim access to the private maintenance paths below // (migration / cleanup / eviction) without relaxing production access. friend class NodeDBTestShim; + friend class MockNodeDB; #endif /// purge db entries without user info diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index f0fbce0433c..f1d137bf650 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -11,6 +11,7 @@ #include "gps/RTC.h" #include "input/InputBroker.h" #include "meshUtils.h" +#include #include #include #include // for better whitespace handling @@ -70,6 +71,17 @@ AdminModule *adminModule; bool hasOpenEditTransaction; +#if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) +static bool licensedIdentityWillMigrate() +{ + if (config.security.private_key.size != 32 || config.security.public_key.size != 32) + return true; + if (nodeDB->checkLowEntropyPublicKey(config.security.public_key)) + return true; + return crc32Buffer(config.security.public_key.bytes, config.security.public_key.size) != nodeDB->getNodeNum(); +} +#endif + /// A special reserved string to indicate strings we can not share with external nodes. We will use this 'reserved' word instead. /// Also, to make setting work correctly, if someone tries to set a string to this reserved value we assume they don't really want /// a change. @@ -755,7 +767,8 @@ void AdminModule::handleGetModuleConfigResponse(const meshtastic_MeshPacket &mp, void AdminModule::handleSetOwner(const meshtastic_User &o) { int changed = 0; - bool identityGenerated = false; + bool identityUpdated = false; + bool channelsSanitized = false; if (*o.long_name) { // Apps built against the older 39-byte limit may send longer names; clamp @@ -774,21 +787,25 @@ void AdminModule::handleSetOwner(const meshtastic_User &o) owner.short_name[sizeof(owner.short_name) - 1] = '\0'; sanitizeUtf8(owner.short_name, sizeof(owner.short_name)); } - snprintf(owner.id, sizeof(owner.id), "!%08x", nodeDB->getNodeNum()); - if (owner.is_licensed != o.is_licensed) { changed = 1; +#if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) + const bool identityWillMigrate = + o.is_licensed && config.lora.region != meshtastic_Config_LoRaConfig_RegionCode_UNSET && licensedIdentityWillMigrate(); + if (identityWillMigrate) + sendWarning(licensedIdentityMigrationMessage); +#endif owner.is_licensed = o.is_licensed; if (channels.ensureLicensedOperation()) { warnLicensedMode(); + channelsSanitized = true; } #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - if ((config.security.private_key.size != 32 || config.security.public_key.size != 32) && - config.lora.region != meshtastic_Config_LoRaConfig_RegionCode_UNSET) { - identityGenerated = nodeDB->generateCryptoKeyPair(); - } + if (owner.is_licensed && config.lora.region != meshtastic_Config_LoRaConfig_RegionCode_UNSET) + identityUpdated = nodeDB->generateCryptoKeyPair(); #endif } + snprintf(owner.id, sizeof(owner.id), "!%08x", nodeDB->getNodeNum()); if (owner.has_is_unmessagable != o.has_is_unmessagable || (o.has_is_unmessagable && owner.is_unmessagable != o.is_unmessagable)) { changed = 1; @@ -798,7 +815,8 @@ void AdminModule::handleSetOwner(const meshtastic_User &o) if (changed) { // If nothing really changed, don't broadcast on the network or write to flash service->reloadOwner(!hasOpenEditTransaction); - saveChanges(SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE | (identityGenerated ? SEGMENT_CONFIG : 0)); + saveChanges(SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE | (identityUpdated ? SEGMENT_CONFIG : 0) | + (channelsSanitized ? SEGMENT_CHANNELS : 0)); } } @@ -1016,6 +1034,8 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) config.lora.region = validatedLora.region; #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) if (owner.is_licensed && isRegionUnset && validatedLora.region > meshtastic_Config_LoRaConfig_RegionCode_UNSET) { + if (licensedIdentityWillMigrate()) + sendWarning(licensedIdentityMigrationMessage); nodeDB->generateCryptoKeyPair(); } #endif @@ -1830,6 +1850,9 @@ void AdminModule::reboot(int32_t seconds) void AdminModule::saveChanges(int saveWhat, bool shouldReboot) { +#ifdef PIO_UNIT_TESTING + lastSaveWhatForTest = saveWhat; +#endif if (!hasOpenEditTransaction) { LOG_INFO("Save changes to disk"); service->reloadConfig(saveWhat); // Calls saveToDisk among other things @@ -1883,6 +1906,14 @@ void AdminModule::handleSetHamMode(const meshtastic_HamParameters &p) config.device.rebroadcast_mode = meshtastic_Config_DeviceConfig_RebroadcastMode_LOCAL_ONLY; // Remove PSK of primary channel for plaintext amateur usage +#if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) + if (config.lora.region != meshtastic_Config_LoRaConfig_RegionCode_UNSET) { + if (licensedIdentityWillMigrate()) + sendWarning(licensedIdentityMigrationMessage); + nodeDB->generateCryptoKeyPair(); + } +#endif + if (channels.ensureLicensedOperation()) { warnLicensedMode(); } diff --git a/src/modules/AdminModule.h b/src/modules/AdminModule.h index 1bf98be1019..abd0c5440d4 100644 --- a/src/modules/AdminModule.h +++ b/src/modules/AdminModule.h @@ -39,6 +39,9 @@ class AdminModule : public ProtobufModule, public Obser private: bool hasOpenEditTransaction = false; +#ifdef PIO_UNIT_TESTING + int lastSaveWhatForTest = 0; +#endif uint8_t session_passkey[8] = {0}; uint32_t session_time = 0; // millis() when the current session passkey was issued @@ -139,9 +142,12 @@ class AdminModule : public ProtobufModule, public Obser static constexpr const char *licensedModeMessage = "Licensed mode activated, removing admin channel and encryption from all channels"; +static constexpr const char *licensedIdentityMigrationMessage = + "Licensed signing requires an identity key; this node identity will change after key generation"; + static constexpr const char *publicChannelPrecisionMessage = "Precise position is not allowed on a public (open / known-key) channel; reduced to coarse precision"; extern AdminModule *adminModule; -void disableBluetooth(); \ No newline at end of file +void disableBluetooth(); diff --git a/test/support/AdminModuleTestShim.h b/test/support/AdminModuleTestShim.h index 002a9cc6cb1..1b06457f1ed 100644 --- a/test/support/AdminModuleTestShim.h +++ b/test/support/AdminModuleTestShim.h @@ -13,6 +13,7 @@ class AdminModuleTestShim : public AdminModule using AdminModule::handleReceivedProtobuf; using AdminModule::handleSetConfig; using AdminModule::handleSetModuleConfig; + using AdminModule::handleSetOwner; using AdminModule::responseIsSolicited; // request/response pairing gate using AdminModule::setPassKey; @@ -21,6 +22,7 @@ class AdminModuleTestShim : public AdminModule // With an "open edit transaction" saveChanges() is a pure no-op: no reloadConfig/saveToDisk/reboot. void deferSaves() { hasOpenEditTransaction = true; } + int savedSegments() const { return lastSaveWhatForTest; } // Setters may allocate an error reply from packetPool; drain it each iteration or the pool leaks. void drainReply() diff --git a/test/test_admin_radio/test_main.cpp b/test/test_admin_radio/test_main.cpp index 673d4590fde..bce2342e869 100644 --- a/test/test_admin_radio/test_main.cpp +++ b/test/test_admin_radio/test_main.cpp @@ -19,6 +19,9 @@ #include "TestUtil.h" #include "mesh/Channels.h" #include "modules/AdminModule.h" +#include "modules/NodeInfoModule.h" +#include +#include #include #include #include @@ -950,6 +953,69 @@ static void test_channelSpacingCalculation_placeholder() // AdminModuleTestShim comes from test/support - the friend seam AdminModule.h declares. static AdminModuleTestShim *testAdmin; +static void installEncryptedAndAdminChannels() +{ + channels.initDefaults(); + meshtastic_Channel admin = meshtastic_Channel_init_zero; + admin.index = 1; + admin.role = meshtastic_Channel_Role_SECONDARY; + admin.has_settings = true; + strncpy(admin.settings.name, Channels::adminChannel, sizeof(admin.settings.name)); + admin.settings.psk.size = 16; + memset(admin.settings.psk.bytes, 0xA5, admin.settings.psk.size); + channels.setChannel(admin); +} + +static void assertLicensedChannelsSanitized() +{ + TEST_ASSERT_EQUAL(0, channels.getByIndex(0).settings.psk.size); + TEST_ASSERT_EQUAL(meshtastic_Channel_Role_DISABLED, channels.getByIndex(1).role); + TEST_ASSERT_EQUAL(0, channels.getByIndex(1).settings.psk.size); +} + +static void test_handleSetOwner_persistsLicensedChannelSanitation() +{ + NodeDB *savedNodeDB = nodeDB; + nodeDB = new NodeDB(); + owner = meshtastic_User_init_zero; + config.lora.region = meshtastic_Config_LoRaConfig_RegionCode_UNSET; + installEncryptedAndAdminChannels(); + + meshtastic_User licensed = meshtastic_User_init_zero; + licensed.is_licensed = true; + testAdmin->deferSaves(); + NodeInfoModule *savedNodeInfoModule = nodeInfoModule; + nodeInfoModule = reinterpret_cast(1); + testAdmin->handleSetOwner(licensed); + nodeInfoModule = savedNodeInfoModule; + + TEST_ASSERT_TRUE(testAdmin->savedSegments() & SEGMENT_CHANNELS); + assertLicensedChannelsSanitized(); + + uint8_t encoded[meshtastic_ChannelFile_size]; + const size_t encodedSize = pb_encode_to_bytes(encoded, sizeof(encoded), &meshtastic_ChannelFile_msg, &channelFile); + TEST_ASSERT_GREATER_THAN(0, encodedSize); + meshtastic_ChannelFile reloaded = meshtastic_ChannelFile_init_zero; + TEST_ASSERT_TRUE(pb_decode_from_bytes(encoded, encodedSize, &meshtastic_ChannelFile_msg, &reloaded)); + channelFile = reloaded; + assertLicensedChannelsSanitized(); + TEST_ASSERT_FALSE_MESSAGE(channels.ensureLicensedOperation(), "sanitized reload must not trigger another persistence write"); + + delete nodeDB; + nodeDB = savedNodeDB; +} + +static void test_bootDefense_sanitizesStaleLicensedChannelsOnce() +{ + owner = meshtastic_User_init_zero; + owner.is_licensed = true; + installEncryptedAndAdminChannels(); + + TEST_ASSERT_TRUE(channels.ensureLicensedOperation()); + assertLicensedChannelsSanitized(); + TEST_ASSERT_FALSE_MESSAGE(channels.ensureLicensedOperation(), "boot sanitation must be idempotent"); +} + static meshtastic_Config makeLoraSetConfig(meshtastic_Config_LoRaConfig_RegionCode region, bool usePreset, meshtastic_Config_LoRaConfig_ModemPreset preset) { @@ -1531,6 +1597,9 @@ void setup() UNITY_BEGIN(); + RUN_TEST(test_handleSetOwner_persistsLicensedChannelSanitation); + RUN_TEST(test_bootDefense_sanitizesStaleLicensedChannelsOnce); + // getRegion() RUN_TEST(test_getRegion_returnsCorrectRegion_US); RUN_TEST(test_getRegion_returnsCorrectRegion_EU868); diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index f5eaa44bbdf..c49031af8a7 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -923,6 +923,31 @@ void test_B7_infrastructure_port_signing_matrix(void) } } +void test_B8_licensed_port_and_destination_signing_matrix(void) +{ + uint8_t pub[32], priv[32]; + crypto->generateKeyPair(pub, priv); + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->setPublicKey(LOCAL_NODE, pub); + owner.is_licensed = true; + channels.ensureLicensedOperation(); + + const meshtastic_PortNum ports[] = { + meshtastic_PortNum_TEXT_MESSAGE_APP, meshtastic_PortNum_POSITION_APP, meshtastic_PortNum_TELEMETRY_APP, + meshtastic_PortNum_ROUTING_APP, meshtastic_PortNum_NODEINFO_APP, + }; + const NodeNum destinations[] = {NODENUM_BROADCAST, REMOTE_NODE}; + for (const auto port : ports) { + for (const auto destination : destinations) { + meshtastic_MeshPacket packet = makeDecoded(LOCAL_NODE, destination, port, SMALL_PAYLOAD); + TEST_ASSERT_EQUAL(DECODE_SUCCESS, roundTrip(&packet)); + TEST_ASSERT_EQUAL(XEDDSA_SIGNATURE_SIZE, packet.decoded.xeddsa_signature.size); + TEST_ASSERT_TRUE(packet.xeddsa_signed); + TEST_ASSERT_FALSE(packet.pki_encrypted); + } + } +} + // =========================================================================== // Group C - routing pipeline and NodeInfo authentication ordering // =========================================================================== @@ -1652,6 +1677,7 @@ void setup() 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); + RUN_TEST(test_B8_licensed_port_and_destination_signing_matrix); printf("\n=== Group C: routing pipeline authentication ordering ===\n"); RUN_TEST(test_C1_invalid_first_copy_does_not_poison_valid_same_id); From c97accd0ae29a86f91b8d8471cefe55c5e37b69f Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:31:01 -0700 Subject: [PATCH 03/11] fix(security): close licensed migration lifecycle gaps --- src/main.cpp | 1 + src/mesh/NodeDB.cpp | 26 ++++++++++++++++++++++++-- src/mesh/NodeDB.h | 5 +++++ src/modules/AdminModule.cpp | 15 ++++++++++++--- test/support/MockMeshService.h | 8 +++++++- 5 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 37ab56bc84f..af86c6c656b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1078,6 +1078,7 @@ void setup() nodeDB->hasWarned = true; } #endif + nodeDB->notifyPendingLicensedIdentityMigration(); #if !MESHTASTIC_EXCLUDE_INPUTBROKER if (inputBroker) inputBroker->Init(); diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index d78fcc0f43f..cd434418873 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -4091,8 +4091,8 @@ bool NodeDB::generateCryptoKeyPair(const uint8_t *privateKey) LOG_DEBUG("Set DH private key for crypto operations"); crypto->setDHPrivateKey(config.security.private_key.bytes); - // Conditionally create new identity based on parameter - createNewIdentity(); + if (createNewIdentity() && owner.is_licensed) + licensedIdentityMigrationPending = true; } return keygenSuccess; #else @@ -4100,6 +4100,21 @@ bool NodeDB::generateCryptoKeyPair(const uint8_t *privateKey) #endif } +bool NodeDB::notifyPendingLicensedIdentityMigration() +{ + if (!licensedIdentityMigrationPending || !service) + return false; + meshtastic_ClientNotification *notification = clientNotificationPool.allocZeroed(); + if (!notification) + return false; + notification->level = meshtastic_LogRecord_Level_WARNING; + notification->time = getValidTime(RTCQualityFromNet); + snprintf(notification->message, sizeof(notification->message), "%s", LICENSED_IDENTITY_MIGRATION_WARNING); + service->sendClientNotification(notification); + licensedIdentityMigrationPending = false; + return true; +} + bool NodeDB::createNewIdentity() { uint32_t oldNodeNum = getNodeNum(); @@ -4203,6 +4218,13 @@ bool NodeDB::restorePreferences(meshtastic_AdminMessage_BackupLocation location, LOG_DEBUG("Restored channels"); } + if (owner.is_licensed && channels.ensureLicensedOperation()) { + restoreWhat |= SEGMENT_CHANNELS; + LOG_WARN("Licensed operation sanitized restored channel encryption/admin access"); + } + if (restoreWhat & SEGMENT_CHANNELS) + channels.onConfigChanged(); + success = saveToDisk(restoreWhat); if (success) { LOG_INFO("Restored preferences from backup"); diff --git a/src/mesh/NodeDB.h b/src/mesh/NodeDB.h index 6b0dcae0c08..4744d9cc2b0 100644 --- a/src/mesh/NodeDB.h +++ b/src/mesh/NodeDB.h @@ -74,6 +74,8 @@ static const uint8_t LOW_ENTROPY_HASHES[][32] = { 0x37, 0x82, 0x8d, 0xb2, 0xcc, 0xd8, 0x97, 0x40, 0x9a, 0x5c, 0x8f, 0x40, 0x55, 0xcb, 0x4c, 0x3e}}; static const char LOW_ENTROPY_WARNING[] = "Compromised keys were detected and regenerated."; #endif +static const char LICENSED_IDENTITY_MIGRATION_WARNING[] = + "Licensed signing generated a new identity key; this node identity changed."; /* DeviceState versions used to be defined in the .proto file but really only this function cares. So changed to a #define here. @@ -221,6 +223,7 @@ class NodeDB bool keyIsLowEntropy = false; bool hasWarned = false; + bool licensedIdentityMigrationPending = false; /// don't do mesh based algorithm for node id assignment (initially) /// instead just store in flash - possibly even in the initial alpha release do this hack @@ -505,6 +508,8 @@ class NodeDB /// @param privateKey Optional 32-byte private key to use. If nullptr, generates new random keys. bool generateCryptoKeyPair(const uint8_t *privateKey = nullptr); + bool notifyPendingLicensedIdentityMigration(); + bool createNewIdentity(); bool backupPreferences(meshtastic_AdminMessage_BackupLocation location); diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index f1d137bf650..57710c0265f 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -801,8 +801,11 @@ void AdminModule::handleSetOwner(const meshtastic_User &o) channelsSanitized = true; } #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - if (owner.is_licensed && config.lora.region != meshtastic_Config_LoRaConfig_RegionCode_UNSET) + if (owner.is_licensed && config.lora.region != meshtastic_Config_LoRaConfig_RegionCode_UNSET) { identityUpdated = nodeDB->generateCryptoKeyPair(); + if (identityWillMigrate) + nodeDB->licensedIdentityMigrationPending = false; + } #endif } snprintf(owner.id, sizeof(owner.id), "!%08x", nodeDB->getNodeNum()); @@ -1034,9 +1037,12 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) config.lora.region = validatedLora.region; #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) if (owner.is_licensed && isRegionUnset && validatedLora.region > meshtastic_Config_LoRaConfig_RegionCode_UNSET) { - if (licensedIdentityWillMigrate()) + const bool identityWillMigrate = licensedIdentityWillMigrate(); + if (identityWillMigrate) sendWarning(licensedIdentityMigrationMessage); nodeDB->generateCryptoKeyPair(); + if (identityWillMigrate) + nodeDB->licensedIdentityMigrationPending = false; } #endif initRegion(); @@ -1908,9 +1914,12 @@ void AdminModule::handleSetHamMode(const meshtastic_HamParameters &p) #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) if (config.lora.region != meshtastic_Config_LoRaConfig_RegionCode_UNSET) { - if (licensedIdentityWillMigrate()) + const bool identityWillMigrate = licensedIdentityWillMigrate(); + if (identityWillMigrate) sendWarning(licensedIdentityMigrationMessage); nodeDB->generateCryptoKeyPair(); + if (identityWillMigrate) + nodeDB->licensedIdentityMigrationPending = false; } #endif diff --git a/test/support/MockMeshService.h b/test/support/MockMeshService.h index 6bfeed07792..1f863ef23de 100644 --- a/test/support/MockMeshService.h +++ b/test/support/MockMeshService.h @@ -6,5 +6,11 @@ class MockMeshService : public MeshService { public: - void sendClientNotification(meshtastic_ClientNotification *n) override { releaseClientNotificationToPool(n); } + void sendClientNotification(meshtastic_ClientNotification *n) override + { + notificationCount++; + releaseClientNotificationToPool(n); + } + + uint32_t notificationCount = 0; }; From 150a34ac7fead88b3f22c783dff80d920fa2bc0c Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Thu, 9 Jul 2026 21:07:08 -0700 Subject: [PATCH 04/11] fix(security): address licensed signing review feedback --- src/modules/AdminModule.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 57710c0265f..99691f4c79d 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -1041,6 +1041,7 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) if (identityWillMigrate) sendWarning(licensedIdentityMigrationMessage); nodeDB->generateCryptoKeyPair(); + changes |= SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; if (identityWillMigrate) nodeDB->licensedIdentityMigrationPending = false; } @@ -1053,7 +1054,7 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) // Default root is in use, so subscribe to the appropriate MQTT topic for this region snprintf(moduleConfig.mqtt.root, sizeof(moduleConfig.mqtt.root), "%s/%s", default_mqtt_root, myRegion->name); } - changes = SEGMENT_CONFIG | SEGMENT_MODULECONFIG; + changes |= SEGMENT_CONFIG | SEGMENT_MODULECONFIG; } else { // Region validation has failed, so just copy all of the old config over the new config validatedLora = oldLoraConfig; From 5090c81b1bab7eee36759bbe9de62ee7df40ee58 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Thu, 9 Jul 2026 21:38:14 -0700 Subject: [PATCH 05/11] fix(security): keep verified signer state packet-local Remove the process-global signer context and carry only the exact Router-verified key on the authenticated packet. Clear serialized authentication metadata before every untrusted decode, require both the verified-signature marker and packet-local key in licensed Admin authorization, and expand authorization/session/isolation coverage while cleaning routing test ownership. --- src/mesh/Router.cpp | 3 ++- src/modules/AdminModule.cpp | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 24f6acc71f6..6b70a2762d0 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -704,6 +704,7 @@ RoutingAuthVerdict passesRoutingAuthGate(meshtastic_MeshPacket *p) // decryptor. Never trust serialized local authentication metadata on that boundary. authCandidate.pki_encrypted = false; authCandidate.public_key.size = 0; + authCandidate.xeddsa_signed = false; #if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA) concurrency::LockGuard g(cryptLock); if (!checkXeddsaReceivePolicy(&authCandidate)) { @@ -711,7 +712,6 @@ RoutingAuthVerdict passesRoutingAuthGate(meshtastic_MeshPacket *p) return RoutingAuthVerdict::REJECT; } #endif - p->xeddsa_signed = authCandidate.xeddsa_signed; wire = *p; storeRoutingAuthCache(wire, authCandidate); return RoutingAuthVerdict::ACCEPT; @@ -801,6 +801,7 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) // Authentication metadata is local-only. Re-establish it below only after successful PKI decryption. p->pki_encrypted = false; p->public_key.size = 0; + p->xeddsa_signed = false; size_t rawSize = p->encrypted.size; if (rawSize > sizeof(bytes)) { diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 99691f4c79d..e11052f43fa 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -144,6 +144,29 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta } #endif meshtastic_Channel *ch = &channels.getByIndex(mp.channel); + const bool licensedRemote = owner.is_licensed && mp.from != 0; + bool authorizedLicensedSigner = false; + if (licensedRemote) { + const bool directedAdmin = mp.to == nodeDB->getNodeNum() && !isBroadcast(mp.to) && + mp.decoded.portnum == meshtastic_PortNum_ADMIN_APP && !mp.pki_encrypted; + if (!directedAdmin || !mp.xeddsa_signed || mp.public_key.size != 32) { + LOG_INFO("Ignore licensed admin payload without a directed Router-verified signature"); + myReply = allocErrorResponse(meshtastic_Routing_Error_NOT_AUTHORIZED, &mp); + return handled; + } + for (const auto &adminKey : config.security.admin_key) { + if (adminKey.size == 32 && memcmp(mp.public_key.bytes, adminKey.bytes, 32) == 0) { + authorizedLicensedSigner = true; + break; + } + } + if (!messageIsResponse(r) && !authorizedLicensedSigner) { + LOG_INFO("Received signed licensed admin payload from a non-allowlisted key"); + myReply = allocErrorResponse(meshtastic_Routing_Error_ADMIN_PUBLIC_KEY_UNAUTHORIZED, &mp); + return handled; + } + LOG_INFO("Signed licensed admin payload with authorized Router-verified sender"); + } if (messageIsResponse(r)) { // Only accept a response from a remote we sent the matching request to. from == 0 is a // local client, which PhoneAPI has already gated. From 185630cdf9a298ef4180120cffde025a6fbbc7d1 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Thu, 9 Jul 2026 21:46:48 -0700 Subject: [PATCH 06/11] fix: address packet auth static analysis Make the routing authentication gate input const and collapse licensed Admin authorization into a single branch so legacy response, local, channel, and PKI checks run only for non-licensed-remote traffic. --- src/mesh/FloodingRouter.cpp | 2 +- src/mesh/Router.cpp | 2 +- src/mesh/Router.h | 2 +- src/modules/AdminModule.cpp | 3 ++- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/mesh/FloodingRouter.cpp b/src/mesh/FloodingRouter.cpp index 7048cd91873..5988bacac5d 100644 --- a/src/mesh/FloodingRouter.cpp +++ b/src/mesh/FloodingRouter.cpp @@ -71,7 +71,7 @@ bool FloodingRouter::perhapsHandleUpgradedPacket(const meshtastic_MeshPacket *p) // 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) + if (passesRoutingAuthGate(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 diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 6b70a2762d0..c9f0790bba8 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -688,7 +688,7 @@ bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p) } #endif -RoutingAuthVerdict passesRoutingAuthGate(meshtastic_MeshPacket *p) +RoutingAuthVerdict passesRoutingAuthGate(const 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 diff --git a/src/mesh/Router.h b/src/mesh/Router.h index d1c36878506..0a6418de806 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -175,7 +175,7 @@ enum class RoutingAuthVerdict { ACCEPT, OPAQUE_RELAY_ONLY, REJECT }; DecodeState perhapsDecode(meshtastic_MeshPacket *p); /** Apply receive authentication before routing state mutation; unknown-channel packets may remain opaque relay-only. */ -RoutingAuthVerdict passesRoutingAuthGate(meshtastic_MeshPacket *p); +RoutingAuthVerdict passesRoutingAuthGate(const meshtastic_MeshPacket *p); #ifdef PIO_UNIT_TESTING uint32_t routingAuthEvaluationCount(); void resetRoutingAuthEvaluationCount(); diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index e11052f43fa..7b01a823f28 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -146,6 +146,7 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta meshtastic_Channel *ch = &channels.getByIndex(mp.channel); const bool licensedRemote = owner.is_licensed && mp.from != 0; bool authorizedLicensedSigner = false; + // Could tighten responses further by tracking the last public key queried. if (licensedRemote) { const bool directedAdmin = mp.to == nodeDB->getNodeNum() && !isBroadcast(mp.to) && mp.decoded.portnum == meshtastic_PortNum_ADMIN_APP && !mp.pki_encrypted; @@ -165,7 +166,7 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta myReply = allocErrorResponse(meshtastic_Routing_Error_ADMIN_PUBLIC_KEY_UNAUTHORIZED, &mp); return handled; } - LOG_INFO("Signed licensed admin payload with authorized Router-verified sender"); + LOG_INFO("Signed licensed admin payload with Router-verified sender"); } if (messageIsResponse(r)) { // Only accept a response from a remote we sent the matching request to. from == 0 is a From 81eebd655341ac1a2199680536aeffb4cdc40f0b Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Thu, 9 Jul 2026 22:03:13 -0700 Subject: [PATCH 07/11] fix(security): reverify lost routing auth handoffs --- src/mesh/FloodingRouter.cpp | 4 +-- src/mesh/Router.cpp | 66 ++++++++++++++++++++++--------------- src/mesh/Router.h | 6 +++- 3 files changed, 46 insertions(+), 30 deletions(-) diff --git a/src/mesh/FloodingRouter.cpp b/src/mesh/FloodingRouter.cpp index 5988bacac5d..1b2f05c978e 100644 --- a/src/mesh/FloodingRouter.cpp +++ b/src/mesh/FloodingRouter.cpp @@ -68,9 +68,7 @@ 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. + // Re-authenticate before replacing the queued lower-hop copy so future callers remain safe. if (passesRoutingAuthGate(p) != RoutingAuthVerdict::ACCEPT) return true; diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index c9f0790bba8..8f469ff1ef5 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -96,6 +96,8 @@ static bool routingAuthCacheMatches(const meshtastic_MeshPacket &packet) static void storeRoutingAuthCache(const meshtastic_MeshPacket &wire, const meshtastic_MeshPacket &authenticated) { + if (!routingAuthCacheLock) + return; concurrency::LockGuard guard(routingAuthCacheLock); routingAuthCache.wire = wire; routingAuthCache.authenticated = authenticated; @@ -688,55 +690,60 @@ bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p) } #endif -RoutingAuthVerdict passesRoutingAuthGate(const meshtastic_MeshPacket *p) +static RoutingAuthVerdict evaluateRoutingAuth(const meshtastic_MeshPacket &wire, meshtastic_MeshPacket &authenticated) { - // 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; + authenticated = wire; routingAuthEvaluations++; - if (authCandidate.which_payload_variant == meshtastic_MeshPacket_decoded_tag) { + if (authenticated.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; - authCandidate.xeddsa_signed = false; + authenticated.pki_encrypted = false; + authenticated.public_key.size = 0; + authenticated.xeddsa_signed = false; #if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA) concurrency::LockGuard g(cryptLock); - if (!checkXeddsaReceivePolicy(&authCandidate)) { - LOG_WARN("Already-decoded packet rejected by signature policy"); + if (!checkXeddsaReceivePolicy(&authenticated)) { + LOG_WARN("Already-decoded packet rejected by routing signature policy"); return RoutingAuthVerdict::REJECT; } #endif - wire = *p; - storeRoutingAuthCache(wire, authCandidate); return RoutingAuthVerdict::ACCEPT; } - const DecodeState state = perhapsDecode(&authCandidate); + const DecodeState state = perhapsDecode(&authenticated); if (state == DecodeState::DECODE_POLICY_REJECT) { - LOG_WARN("Packet rejected by signature policy"); + LOG_WARN("Packet rejected by routing signature policy"); return RoutingAuthVerdict::REJECT; } if (state == DecodeState::DECODE_FATAL) { - LOG_WARN("Fatal decode error, dropping packet"); + LOG_WARN("Fatal decode error during routing authentication"); return RoutingAuthVerdict::REJECT; } if (state == DecodeState::DECODE_FAILURE) { - LOG_WARN("Decryptable packet failed decoding, dropping packet"); + LOG_WARN("Decryptable packet failed routing authentication"); 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; } +RoutingAuthVerdict passesRoutingAuthGate(const 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 authenticated = meshtastic_MeshPacket_init_zero; + const RoutingAuthVerdict verdict = evaluateRoutingAuth(*p, authenticated); + if (verdict == RoutingAuthVerdict::ACCEPT) + storeRoutingAuthCache(*p, authenticated); + return verdict; +} + #if !(MESHTASTIC_EXCLUDE_PKI) // The fallback costs three X25519 ops before the AEAD tag is checked. Budget is global because p->from is // attacker-controlled; successful runs refund, and their key is then persisted for the fast path. @@ -1206,7 +1213,7 @@ NodeNum Router::getNodeNum() * Handle any packet that is received by an interface on this node. * Note: some packets may merely being passed through this node and will be forwarded elsewhere. */ -void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src) +void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src, bool routingAuthRequired) { bool skipHandle = false; @@ -1221,8 +1228,15 @@ void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src) // 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); + if (routingAuthRequired && !applyRoutingAuthCache(p)) { + meshtastic_MeshPacket authenticated = meshtastic_MeshPacket_init_zero; + if (evaluateRoutingAuth(*p, authenticated) != RoutingAuthVerdict::ACCEPT) { + LOG_WARN("Routing authentication handoff was lost and packet re-verification failed"); + packetPool.release(p_encrypted); + return; + } + *p = authenticated; + } // 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. @@ -1426,6 +1440,6 @@ void Router::perhapsHandleReceived(meshtastic_MeshPacket *p) // Note: we avoid calling shouldFilterReceived if we are supposed to ignore certain nodes - because some overrides might // cache/learn of the existence of nodes (i.e. FloodRouter) that they should not - handleReceived(p); + handleReceived(p, RX_SRC_RADIO, true); packetPool.release(p); } diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 0a6418de806..50c1557f770 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -99,6 +99,10 @@ class Router : protected concurrency::OSThread, protected PacketHistory before us */ uint32_t rxDupe = 0, txRelayCanceled = 0; +#ifdef PIO_UNIT_TESTING + void handleReceivedAfterRoutingGateForTest(meshtastic_MeshPacket *p) { handleReceived(p, RX_SRC_RADIO, true); } +#endif + protected: friend class RoutingModule; @@ -158,7 +162,7 @@ class Router : protected concurrency::OSThread, protected PacketHistory * Note: this packet will never be called for messages sent/generated by this node. * Note: this method will free the provided packet. */ - void handleReceived(meshtastic_MeshPacket *p, RxSource src = RX_SRC_RADIO); + void handleReceived(meshtastic_MeshPacket *p, RxSource src = RX_SRC_RADIO, bool routingAuthRequired = false); /** Frees the provided packet, and generates a NAK indicating the specifed error while sending */ void abortSendAndNak(meshtastic_Routing_Error err, meshtastic_MeshPacket *p); From 11555d2fc9bfd7f33b2cff59a5f44911f1322f31 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Fri, 10 Jul 2026 21:42:26 -0700 Subject: [PATCH 08/11] test: stabilize signed admin native coverage --- src/modules/AdminModule.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 7b01a823f28..91011f2058b 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -228,7 +228,7 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta // that pointer is unrelated, so the path was unsafe.) // Automatically favorite the node that is using the admin key - auto remoteNode = nodeDB->getMeshNode(mp.from); + auto remoteNode = nodeDB ? nodeDB->getMeshNode(mp.from) : nullptr; if (remoteNode && !nodeInfoLiteIsFavorite(remoteNode)) { if (config.device.role == meshtastic_Config_DeviceConfig_Role_CLIENT_BASE) { // Special case for CLIENT_BASE: is_favorite has special meaning, and we don't want to automatically set it From e8a44ee7aa8bf242f2d70d9ed9f63c06c25a4973 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 12 Jul 2026 15:24:55 -0700 Subject: [PATCH 09/11] fix(mqtt): preserve signed packet metadata --- src/mqtt/MQTT.cpp | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index 251c632a167..89617662c41 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -124,6 +124,9 @@ inline void onReceiveProto(char *topic, byte *payload, size_t length) if (!pAck) return; pAck->transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_MQTT; + // sendLocal consumes packets sent to a live interface, but returns SHOULD_RELEASE + // when it handled a local delivery synchronously. Match MeshService::sendToMesh's + // ownership contract so the MQTT acknowledgement cannot leak on the local path. if (router->sendLocal(pAck) == ERRNO_SHOULD_RELEASE) packetPool.release(pAck); } else { @@ -158,6 +161,11 @@ inline void onReceiveProto(char *topic, byte *payload, size_t length) if (shouldDropMqttDownlink(*p)) return; + // A decoded MQTT packet has already crossed the broker boundary, so authenticate it in place + // before handing it to the router. The routing-auth cache intentionally keeps an authenticated + // copy separate for encrypted packets, but a cache hit alone does not update this packet's + // xeddsa_signed marker (which is part of the downstream message metadata). + bool decodedAuthChecked = false; if (p->which_payload_variant == meshtastic_MeshPacket_decoded_tag) { if (moduleConfig.mqtt.encryption_enabled) { @@ -175,11 +183,14 @@ inline void onReceiveProto(char *topic, byte *payload, size_t length) // protection for known signers. Without this, a peer on a plaintext broker could // 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. - if (passesRoutingAuthGate(p.get()) != RoutingAuthVerdict::ACCEPT) { + // CryptoEngine cache state, and MQTT ingress can run on a different task. The in-place + // call preserves the verified xeddsa_signed marker for downstream routing/UI consumers. + concurrency::LockGuard g(cryptLock); + if (!checkXeddsaReceivePolicy(p.get())) { LOG_INFO("Ignore decoded message failing XEdDSA policy"); return; } + decodedAuthChecked = true; #endif } @@ -191,7 +202,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 && passesRoutingAuthGate(p.get()) == RoutingAuthVerdict::ACCEPT) + } else if (router && (decodedAuthChecked || passesRoutingAuthGate(p.get()) == RoutingAuthVerdict::ACCEPT)) router->enqueueReceivedMessage(p.release()); } From ba1be30779cb3bf5462991925c7bd5a97648ddeb Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Thu, 23 Jul 2026 17:16:58 -0700 Subject: [PATCH 10/11] chore: format rebased router changes --- src/mesh/Router.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 8f469ff1ef5..0418fe13975 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -821,8 +821,8 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) ChannelIndex chIndex = 0; #if !(MESHTASTIC_EXCLUDE_PKI) meshtastic_NodeInfoLite *ourNode = nullptr; - if (!owner.is_licensed && p->channel == 0 && isToUs(p) && p->to > 0 && !isBroadcast(p->to) && rawSize > MESHTASTIC_PKC_OVERHEAD && - (ourNode = nodeDB->getMeshNode(p->to)) != nullptr && ourNode->public_key.size > 0) { + if (!owner.is_licensed && p->channel == 0 && isToUs(p) && p->to > 0 && !isBroadcast(p->to) && + rawSize > MESHTASTIC_PKC_OVERHEAD && (ourNode = nodeDB->getMeshNode(p->to)) != nullptr && ourNode->public_key.size > 0) { pkiAttempted = true; LOG_DEBUG("Attempt PKI decryption"); // Resolve the sender's key only for actual PKI-decrypt candidates, not every encrypted channel From 0bee5bcfd2edc9e2aa4b2d0f0be4b31107fa857a Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Thu, 23 Jul 2026 17:30:42 -0700 Subject: [PATCH 11/11] fix: accept signed licensed admin requests --- src/modules/AdminModule.cpp | 2 ++ src/mqtt/MQTT.cpp | 9 ++---- test/test_admin_session_repro/test_main.cpp | 32 +++++++++++++++++++++ test/test_mqtt/MQTT.cpp | 5 +--- 4 files changed, 37 insertions(+), 11 deletions(-) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 91011f2058b..b387727974f 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -201,6 +201,8 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta return handled; } #endif + } else if (authorizedLicensedSigner) { + // Router verified a plaintext licensed-mode signer against the admin allowlist above. } else if (strcasecmp(ch->settings.name, Channels::adminChannel) == 0) { if (!config.security.admin_channel_enabled) { LOG_INFO("Ignore admin channel, legacy admin is disabled"); diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index 89617662c41..ee4e59d265a 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -124,9 +124,7 @@ inline void onReceiveProto(char *topic, byte *payload, size_t length) if (!pAck) return; pAck->transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_MQTT; - // sendLocal consumes packets sent to a live interface, but returns SHOULD_RELEASE - // when it handled a local delivery synchronously. Match MeshService::sendToMesh's - // ownership contract so the MQTT acknowledgement cannot leak on the local path. + // Release locally handled ACKs only when sendLocal requests it. if (router->sendLocal(pAck) == ERRNO_SHOULD_RELEASE) packetPool.release(pAck); } else { @@ -161,10 +159,7 @@ inline void onReceiveProto(char *topic, byte *payload, size_t length) if (shouldDropMqttDownlink(*p)) return; - // A decoded MQTT packet has already crossed the broker boundary, so authenticate it in place - // before handing it to the router. The routing-auth cache intentionally keeps an authenticated - // copy separate for encrypted packets, but a cache hit alone does not update this packet's - // xeddsa_signed marker (which is part of the downstream message metadata). + // Authenticate decoded MQTT packets in place so xeddsa_signed reaches consumers. bool decodedAuthChecked = false; if (p->which_payload_variant == meshtastic_MeshPacket_decoded_tag) { diff --git a/test/test_admin_session_repro/test_main.cpp b/test/test_admin_session_repro/test_main.cpp index c93c1fd9472..73c6361b6a1 100644 --- a/test/test_admin_session_repro/test_main.cpp +++ b/test/test_admin_session_repro/test_main.cpp @@ -17,6 +17,7 @@ #include "mesh/NodeDB.h" #include "mesh/mesh-pb-constants.h" #include "modules/AdminModule.h" +#include "modules/NodeInfoModule.h" #include "support/AdminModuleTestShim.h" #include "support/MockMeshService.h" #include @@ -93,6 +94,18 @@ static meshtastic_MeshPacket makeRemoteSetOwner(const char *newLongName, const u return mp; } +static meshtastic_MeshPacket makeLicensedRemoteSetOwner(const char *newLongName, const uint8_t *session, size_t sessionLen, + meshtastic_AdminMessage &out) +{ + auto mp = makeRemoteSetOwner(newLongName, session, sessionLen, out); + mp.to = LOCAL_NODE; + mp.pki_encrypted = false; + mp.xeddsa_signed = true; + mp.decoded.portnum = meshtastic_PortNum_ADMIN_APP; + out.set_owner.is_licensed = true; + return mp; +} + // A get_module_config_response carrying a remote_hardware pin list, as a remote would answer. // This is the class of message that short-circuited auth: no session passkey, sender need not // hold an admin key. handleGetModuleConfigResponse() stamps mp.from into the pin table. @@ -207,6 +220,24 @@ void test_remote_setter_without_session_is_rejected(void) TEST_ASSERT_EQUAL_STRING("Original", owner.long_name); } +void test_licensed_signed_setter_with_session_is_accepted(void) +{ + owner.is_licensed = true; + meshtastic_AdminMessage sessionResponse = meshtastic_AdminMessage_init_zero; + admin->setPassKey(&sessionResponse); + + meshtastic_AdminMessage m; + meshtastic_MeshPacket mp = makeLicensedRemoteSetOwner("LicensedAdmin", sessionResponse.session_passkey.bytes, + sessionResponse.session_passkey.size, m); + NodeInfoModule *savedNodeInfoModule = nodeInfoModule; + nodeInfoModule = reinterpret_cast(1); + admin->handleReceivedProtobuf(mp, &m); + nodeInfoModule = savedNodeInfoModule; + admin->drainReply(); + + TEST_ASSERT_EQUAL_STRING("LicensedAdmin", owner.long_name); +} + // The node's session key is minted only by setPassKey (which runs when it answers an admin GET), // so before any GET the expected key is all-zero and any presented key mismatches. void test_expected_session_key_is_zero_before_any_get(void) @@ -673,6 +704,7 @@ void setup() UNITY_BEGIN(); #if !(MESHTASTIC_EXCLUDE_PKI) RUN_TEST(test_remote_setter_without_session_is_rejected); + RUN_TEST(test_licensed_signed_setter_with_session_is_accepted); RUN_TEST(test_expected_session_key_is_zero_before_any_get); RUN_TEST(test_session_gate_accepts_key_from_a_get_response); RUN_TEST(test_remote_security_config_omits_private_key); diff --git a/test/test_mqtt/MQTT.cpp b/test/test_mqtt/MQTT.cpp index ab9a64e44a8..60c6195ad1c 100644 --- a/test/test_mqtt/MQTT.cpp +++ b/test/test_mqtt/MQTT.cpp @@ -53,10 +53,7 @@ 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(). + // These tests have no PhoneAPI reader, so drain queued packet copies before teardown. ~MockMeshService() { while (meshtastic_MeshPacket *p = getForPhone())