From 9d061e732263659d60bc114fcbe659f8fe9807d1 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 1/7] 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 | 14 +- src/modules/AdminModule.cpp | 16 ++- src/modules/NodeInfoModule.cpp | 12 +- test/test_packet_signing/test_main.cpp | 175 ++++++++++++++++++++++++- 5 files changed, 199 insertions(+), 25 deletions(-) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index e05f8b8f0a2..2c676c7078b 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -3863,15 +3863,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 67d724433f2..5b15d5f48f7 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -528,9 +528,9 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) // store or the warm tier (nodes evicted from the hot store keep their key // there), so DMs from long-tail nodes still decrypt. meshtastic_NodeInfoLite_public_key_t fromKey = {0, {0}}; - 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) { + if (!owner.is_licensed && 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) { LOG_DEBUG("Attempt PKI decryption"); if (crypto->decryptCurve25519(p->from, fromKey, p->id, rawSize, p->encrypted.bytes, bytes)) { @@ -698,12 +698,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 a3ee10b121a..256345f1258 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -731,6 +731,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 @@ -757,6 +758,12 @@ void AdminModule::handleSetOwner(const meshtastic_User &o) if (channels.ensureLicensedOperation()) { sendWarning(licensedModeMessage); } +#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)) { @@ -767,7 +774,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)); } } @@ -934,7 +941,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 @@ -947,6 +954,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 c86c54aff34..6c68168a793 100644 --- a/src/modules/NodeInfoModule.cpp +++ b/src/modules/NodeInfoModule.cpp @@ -167,14 +167,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'; @@ -232,4 +226,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/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 707d716195b..ecdd26bff40 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -12,7 +12,9 @@ // Group E decoded-ingress policy (checkXeddsaReceivePolicy, the plaintext-MQTT trust boundary) #include "MeshTypes.h" // include BEFORE TestUtil.h +#include "NodeStatus.h" #include "TestUtil.h" +#include "airtime.h" #include // The whole suite exercises XEdDSA sign/verify and checkXeddsaReceivePolicy, all of which are @@ -27,6 +29,7 @@ #include #include #include +#include #include #include @@ -343,7 +346,7 @@ void test_B1_local_broadcast_is_signed(void) TEST_ASSERT_TRUE(p.xeddsa_signed); } -// B2: our own unicast is NOT signed. +// B2: our own normal-mode unicast is NOT signed. void test_B2_local_unicast_not_signed(void) { mockNodeDB->addNode(REMOTE_NODE); @@ -351,7 +354,7 @@ void test_B2_local_unicast_not_signed(void) meshtastic_MeshPacket p = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_PRIVATE_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, "normal unicast must not be signed"); } // B3: our own oversized broadcast is NOT signed (signature wouldn't fit). @@ -454,6 +457,115 @@ void test_B6_rich_shape_sweep_no_deadband(void) TEST_ASSERT_TRUE_MESSAGE(sawUnsigned, "rich sweep never crossed the fit boundary"); } +// B7: licensed operation signs both plaintext broadcasts and direct messages. +void test_B7_licensed_broadcast_and_unicast_are_signed(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(); + + meshtastic_MeshPacket broadcast = + makeDecoded(LOCAL_NODE, NODENUM_BROADCAST, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD); + TEST_ASSERT_EQUAL(DECODE_SUCCESS, roundTrip(&broadcast)); + TEST_ASSERT_EQUAL(XEDDSA_SIGNATURE_SIZE, broadcast.decoded.xeddsa_signature.size); + TEST_ASSERT_TRUE(broadcast.xeddsa_signed); + + meshtastic_MeshPacket direct = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD); + TEST_ASSERT_EQUAL(DECODE_SUCCESS, roundTrip(&direct)); + TEST_ASSERT_EQUAL(XEDDSA_SIGNATURE_SIZE, direct.decoded.xeddsa_signature.size); + TEST_ASSERT_TRUE(direct.xeddsa_signed); +} + +// B8: even with both identity keys present, licensed direct messages use the plaintext channel path. +void test_B8_licensed_unicast_never_uses_pki_encryption(void) +{ + uint8_t localPub[32], localPriv[32], remotePub[32], remotePriv[32]; + crypto->generateKeyPair(localPub, localPriv); + memcpy(config.security.private_key.bytes, localPriv, sizeof(localPriv)); + config.security.private_key.size = sizeof(localPriv); + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->setPublicKey(LOCAL_NODE, localPub); + mockNodeDB->addNode(REMOTE_NODE); + crypto->generateKeyPair(remotePub, remotePriv); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePub); + crypto->setDHPrivateKey(localPriv); + + owner.is_licensed = true; + channels.ensureLicensedOperation(); + meshtastic_MeshPacket p = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD); + + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_NONE, perhapsEncode(&p)); + TEST_ASSERT_FALSE(p.pki_encrypted); + meshtastic_Data plaintext = meshtastic_Data_init_zero; + TEST_ASSERT_TRUE_MESSAGE(pb_decode_from_bytes(p.encrypted.bytes, p.encrypted.size, &meshtastic_Data_msg, &plaintext), + "licensed channel payload must remain plaintext"); + TEST_ASSERT_EQUAL(XEDDSA_SIGNATURE_SIZE, plaintext.xeddsa_signature.size); +} + +// B9: licensed direct messages remain deliverable unsigned when the existing signature will not fit. +void test_B9_licensed_oversized_unicast_remains_unsigned(void) +{ + owner.is_licensed = true; + channels.ensureLicensedOperation(); + meshtastic_MeshPacket p = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, OVERSIZED_PAYLOAD); + + TEST_ASSERT_EQUAL(DECODE_SUCCESS, roundTrip(&p)); + TEST_ASSERT_EQUAL(0, p.decoded.xeddsa_signature.size); +} + +// B10: normal-mode direct messages retain their existing PKI encryption behavior. +void test_B10_normal_unicast_still_uses_pki(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); + memcpy(config.security.private_key.bytes, localPriv, sizeof(localPriv)); + config.security.private_key.size = sizeof(localPriv); + crypto->setDHPrivateKey(localPriv); + + meshtastic_MeshPacket p = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_NONE, perhapsEncode(&p)); + TEST_ASSERT_TRUE(p.pki_encrypted); + + myNodeInfo.my_node_num = REMOTE_NODE; + crypto->setDHPrivateKey(remotePriv); + TEST_ASSERT_EQUAL(DECODE_SUCCESS, perhapsDecode(&p)); + TEST_ASSERT_TRUE(p.pki_encrypted); + TEST_ASSERT_EQUAL(0, p.decoded.xeddsa_signature.size); +} + +// B11: publishing a licensed node's key must not make inbound PKI decryption reachable. +void test_B11_licensed_receiver_does_not_decrypt_pki(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); + memcpy(config.security.private_key.bytes, localPriv, sizeof(localPriv)); + config.security.private_key.size = sizeof(localPriv); + crypto->setDHPrivateKey(localPriv); + + meshtastic_MeshPacket p = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_NONE, perhapsEncode(&p)); + TEST_ASSERT_TRUE(p.pki_encrypted); + + owner.is_licensed = true; + channels.ensureLicensedOperation(); + myNodeInfo.my_node_num = REMOTE_NODE; + crypto->setDHPrivateKey(remotePriv); + TEST_ASSERT_EQUAL(DECODE_FAILURE, perhapsDecode(&p)); +} + // =========================================================================== // 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) @@ -461,9 +573,52 @@ void test_B6_rich_shape_sweep_no_deadband(void) class NodeInfoTestShim : public NodeInfoModule { public: + using NodeInfoModule::allocReply; using NodeInfoModule::handleReceivedProtobuf; // protected virtual -> exposed for direct call }; +// C0: licensed NodeInfo publishes the same public identity key used to verify plaintext signatures. +void test_C0_licensed_nodeinfo_publishes_public_key(void) +{ + owner.is_licensed = true; + owner.public_key.size = 32; + memset(owner.public_key.bytes, 0x5A, owner.public_key.size); + + NodeInfoTestShim shim; + meshtastic_MeshPacket *reply = shim.allocReply(); + TEST_ASSERT_NOT_NULL(reply); + meshtastic_User published = meshtastic_User_init_zero; + TEST_ASSERT_TRUE( + pb_decode_from_bytes(reply->decoded.payload.bytes, reply->decoded.payload.size, &meshtastic_User_msg, &published)); + TEST_ASSERT_TRUE(published.is_licensed); + TEST_ASSERT_EQUAL(32, published.public_key.size); + TEST_ASSERT_EQUAL_UINT8_ARRAY(owner.public_key.bytes, published.public_key.bytes, 32); + packetPool.release(reply); +} + +// C00: a pre-signing licensed install creates one identity and derives the same public key after reload. +void test_C00_licensed_identity_key_is_generated_and_preserved(void) +{ + owner.is_licensed = true; + config.lora.region = meshtastic_Config_LoRaConfig_RegionCode_US; + config.security = meshtastic_Config_SecurityConfig_init_zero; + + TEST_ASSERT_TRUE(mockNodeDB->generateCryptoKeyPair()); + uint8_t privateKey[32], publicKey[32]; + memcpy(privateKey, config.security.private_key.bytes, sizeof(privateKey)); + memcpy(publicKey, config.security.public_key.bytes, sizeof(publicKey)); + const NodeNum migratedNodeNum = myNodeInfo.my_node_num; + TEST_ASSERT_NOT_EQUAL(LOCAL_NODE, migratedNodeNum); + + config.security.public_key.size = 0; + owner.public_key.size = 0; + TEST_ASSERT_TRUE(mockNodeDB->generateCryptoKeyPair()); + TEST_ASSERT_EQUAL(migratedNodeNum, myNodeInfo.my_node_num); + TEST_ASSERT_EQUAL_UINT8_ARRAY(privateKey, config.security.private_key.bytes, sizeof(privateKey)); + TEST_ASSERT_EQUAL_UINT8_ARRAY(publicKey, config.security.public_key.bytes, sizeof(publicKey)); + TEST_ASSERT_EQUAL_UINT8_ARRAY(publicKey, owner.public_key.bytes, sizeof(publicKey)); +} + static meshtastic_MeshPacket makeNodeInfoPacket(bool signed_) { // Broadcast so the module's phone-forward path (which needs `service`) is skipped. @@ -688,6 +843,10 @@ void test_E9_decoded_partial_signature_from_nonsigner_dropped(void) void setup() { initializeTestEnvironment(); + AirTime testAirTime; + meshtastic::NodeStatus testNodeStatus; + airTime = &testAirTime; + nodeStatus = &testNodeStatus; UNITY_BEGIN(); printf("\n=== Group A: receive-side accept/reject ===\n"); @@ -708,8 +867,15 @@ 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_licensed_broadcast_and_unicast_are_signed); + RUN_TEST(test_B8_licensed_unicast_never_uses_pki_encryption); + RUN_TEST(test_B9_licensed_oversized_unicast_remains_unsigned); + RUN_TEST(test_B10_normal_unicast_still_uses_pki); + RUN_TEST(test_B11_licensed_receiver_does_not_decrypt_pki); printf("\n=== Group C: NodeInfoModule downgrade drop ===\n"); + RUN_TEST(test_C0_licensed_nodeinfo_publishes_public_key); + RUN_TEST(test_C00_licensed_identity_key_is_generated_and_preserved); 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); @@ -729,7 +895,10 @@ void setup() RUN_TEST(test_E8_decoded_partial_signature_from_signer_dropped); RUN_TEST(test_E9_decoded_partial_signature_from_nonsigner_dropped); - exit(UNITY_END()); + const int result = UNITY_END(); + airTime = nullptr; + nodeStatus = nullptr; + exit(result); } void loop() {} From 449f1726b06c050608b44d7fe30eff5bb52947ce 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 2/7] 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 | 72 ++++++++++++++++++++++++++ 8 files changed, 204 insertions(+), 16 deletions(-) diff --git a/src/mesh/Channels.cpp b/src/mesh/Channels.cpp index b086f5e6b3e..1cebcce1732 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 2c676c7078b..7360db118cd 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -860,7 +860,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); } @@ -2599,6 +2599,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 8aa32dcc75a..a54f5b9d201 100644 --- a/src/mesh/NodeDB.h +++ b/src/mesh/NodeDB.h @@ -545,6 +545,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 256345f1258..7288f4689e8 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -8,6 +8,7 @@ #include "SPILock.h" #include "input/InputBroker.h" #include "meshUtils.h" +#include #include #include // for better whitespace handling #if defined(ARCH_ESP32) && !MESHTASTIC_EXCLUDE_WIFI @@ -62,6 +63,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. @@ -731,7 +743,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 @@ -750,21 +763,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()) { sendWarning(licensedModeMessage); + 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; @@ -774,7 +791,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)); } } @@ -956,6 +974,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 @@ -1694,6 +1714,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 @@ -1747,6 +1770,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()) { sendWarning(licensedModeMessage); } diff --git a/src/modules/AdminModule.h b/src/modules/AdminModule.h index 468e020eaf0..611d696d472 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}; uint session_time = 0; @@ -94,9 +97,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 7dba0677e9b..71fa268bfe4 100644 --- a/test/support/AdminModuleTestShim.h +++ b/test/support/AdminModuleTestShim.h @@ -10,9 +10,11 @@ class AdminModuleTestShim : public AdminModule using AdminModule::handleReceivedProtobuf; using AdminModule::handleSetConfig; using AdminModule::handleSetModuleConfig; + using AdminModule::handleSetOwner; // 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 8a839f9345c..4dc3d49d48c 100644 --- a/test/test_admin_radio/test_main.cpp +++ b/test/test_admin_radio/test_main.cpp @@ -11,6 +11,7 @@ * 6. Channel spacing calculation (placeholder for future protobuf changes) */ +#include "Channels.h" #include "DisplayFormatters.h" #include "MeshRadio.h" #include "MeshService.h" @@ -18,6 +19,9 @@ #include "RadioInterface.h" #include "TestUtil.h" #include "modules/AdminModule.h" +#include "modules/NodeInfoModule.h" +#include +#include #include #include "meshtastic/config.pb.h" @@ -928,6 +932,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); // reloadOwner(false) only checks presence + 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) { @@ -1269,6 +1336,8 @@ void setup() UNITY_BEGIN(); // getRegion() + RUN_TEST(test_handleSetOwner_persistsLicensedChannelSanitation); + RUN_TEST(test_bootDefense_sanitizesStaleLicensedChannelsOnce); RUN_TEST(test_getRegion_returnsCorrectRegion_US); RUN_TEST(test_getRegion_returnsCorrectRegion_EU868); RUN_TEST(test_getRegion_returnsCorrectRegion_LORA24); diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index ecdd26bff40..afd9af6666a 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -51,6 +51,8 @@ static constexpr size_t OVERSIZED_PAYLOAD = 180; class MockNodeDB : public NodeDB { public: + void installDefaultsPreservingIdentity() { installDefaultConfig(true); } + void clearTestNodes() { testNodes.clear(); @@ -566,6 +568,31 @@ void test_B11_licensed_receiver_does_not_decrypt_pki(void) TEST_ASSERT_EQUAL(DECODE_FAILURE, perhapsDecode(&p)); } +void test_B12_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 - 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) @@ -619,6 +646,48 @@ void test_C00_licensed_identity_key_is_generated_and_preserved(void) TEST_ASSERT_EQUAL_UINT8_ARRAY(publicKey, owner.public_key.bytes, sizeof(publicKey)); } +void test_C01_factory_config_reset_preserves_valid_identity_private_key(void) +{ + uint8_t publicKey[32], privateKey[32]; + crypto->generateKeyPair(publicKey, privateKey); + config.has_security = true; + config.security.private_key.size = 32; + memcpy(config.security.private_key.bytes, privateKey, sizeof(privateKey)); + + mockNodeDB->installDefaultsPreservingIdentity(); + TEST_ASSERT_EQUAL(32, config.security.private_key.size); + TEST_ASSERT_EQUAL_UINT8_ARRAY(privateKey, config.security.private_key.bytes, sizeof(privateKey)); + TEST_ASSERT_EQUAL(0, config.security.public_key.size); + + owner.is_licensed = true; + config.lora.region = meshtastic_Config_LoRaConfig_RegionCode_US; + TEST_ASSERT_TRUE(mockNodeDB->generateCryptoKeyPair()); + TEST_ASSERT_EQUAL_UINT8_ARRAY(privateKey, config.security.private_key.bytes, sizeof(privateKey)); + TEST_ASSERT_EQUAL_UINT8_ARRAY(publicKey, config.security.public_key.bytes, sizeof(publicKey)); +} + +void test_C02_licensed_low_entropy_identity_is_regenerated(void) +{ + static const uint8_t compromisedPublicKey[32] = { + 0xac, 0xaf, 0x8c, 0x1c, 0x3c, 0x1c, 0x37, 0xac, 0x4f, 0x03, 0xa1, 0xe9, 0xfc, 0x37, 0x23, 0x29, + 0xc8, 0xa3, 0x5d, 0x7f, 0x05, 0x26, 0xeb, 0x00, 0xbd, 0x26, 0xb8, 0x2e, 0xb1, 0x94, 0x7d, 0x24, + }; + owner.is_licensed = true; + config.lora.region = meshtastic_Config_LoRaConfig_RegionCode_US; + config.security.private_key.size = 32; + memset(config.security.private_key.bytes, 0xA5, 32); + config.security.public_key.size = 32; + memcpy(config.security.public_key.bytes, compromisedPublicKey, sizeof(compromisedPublicKey)); + TEST_ASSERT_TRUE(mockNodeDB->checkLowEntropyPublicKey(config.security.public_key)); + + uint8_t oldPrivateKey[32]; + memcpy(oldPrivateKey, config.security.private_key.bytes, sizeof(oldPrivateKey)); + TEST_ASSERT_TRUE(mockNodeDB->generateCryptoKeyPair()); + TEST_ASSERT_TRUE(mockNodeDB->keyIsLowEntropy); + TEST_ASSERT_FALSE(mockNodeDB->checkLowEntropyPublicKey(config.security.public_key)); + TEST_ASSERT_FALSE(memcmp(oldPrivateKey, config.security.private_key.bytes, sizeof(oldPrivateKey)) == 0); +} + static meshtastic_MeshPacket makeNodeInfoPacket(bool signed_) { // Broadcast so the module's phone-forward path (which needs `service`) is skipped. @@ -872,10 +941,13 @@ void setup() RUN_TEST(test_B9_licensed_oversized_unicast_remains_unsigned); RUN_TEST(test_B10_normal_unicast_still_uses_pki); RUN_TEST(test_B11_licensed_receiver_does_not_decrypt_pki); + RUN_TEST(test_B12_licensed_port_and_destination_signing_matrix); printf("\n=== Group C: NodeInfoModule downgrade drop ===\n"); RUN_TEST(test_C0_licensed_nodeinfo_publishes_public_key); RUN_TEST(test_C00_licensed_identity_key_is_generated_and_preserved); + RUN_TEST(test_C01_factory_config_reset_preserves_valid_identity_private_key); + RUN_TEST(test_C02_licensed_low_entropy_identity_is_regenerated); 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); From 8009692d48d1df39d94f27831fef1a0b665686d1 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 3/7] 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 +++++- test/test_admin_radio/test_main.cpp | 40 ++++++++++++++++++++++++++ test/test_packet_signing/test_main.cpp | 11 +++++++ 7 files changed, 100 insertions(+), 6 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 63cbf35ab07..91633fbdb42 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1062,6 +1062,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 7360db118cd..577e7349348 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -3925,8 +3925,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 @@ -3934,6 +3934,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(); @@ -4037,6 +4052,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 a54f5b9d201..08840c323f8 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. @@ -218,6 +220,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 @@ -471,6 +474,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 7288f4689e8..f5c4d8b5b36 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -777,8 +777,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()); @@ -974,9 +977,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(); @@ -1772,9 +1778,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; }; diff --git a/test/test_admin_radio/test_main.cpp b/test/test_admin_radio/test_main.cpp index 4dc3d49d48c..28422343a88 100644 --- a/test/test_admin_radio/test_main.cpp +++ b/test/test_admin_radio/test_main.cpp @@ -13,6 +13,7 @@ #include "Channels.h" #include "DisplayFormatters.h" +#include "FSCommon.h" #include "MeshRadio.h" #include "MeshService.h" #include "NodeDB.h" @@ -943,6 +944,15 @@ static void installEncryptedAndAdminChannels() admin.settings.psk.size = 16; memset(admin.settings.psk.bytes, 0xA5, admin.settings.psk.size); channels.setChannel(admin); + + meshtastic_Channel secondary = meshtastic_Channel_init_zero; + secondary.index = 2; + secondary.role = meshtastic_Channel_Role_SECONDARY; + secondary.has_settings = true; + strncpy(secondary.settings.name, "private", sizeof(secondary.settings.name)); + secondary.settings.psk.size = 32; + memset(secondary.settings.psk.bytes, 0x5A, secondary.settings.psk.size); + channels.setChannel(secondary); } static void assertLicensedChannelsSanitized() @@ -950,6 +960,7 @@ 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); + TEST_ASSERT_EQUAL(0, channels.getByIndex(2).settings.psk.size); } static void test_handleSetOwner_persistsLicensedChannelSanitation() @@ -995,6 +1006,34 @@ static void test_bootDefense_sanitizesStaleLicensedChannelsOnce() TEST_ASSERT_FALSE_MESSAGE(channels.ensureLicensedOperation(), "boot sanitation must be idempotent"); } +static void test_restorePreferences_sanitizesLicensedBackupBeforeReturn() +{ + NodeDB *savedNodeDB = nodeDB; + nodeDB = new NodeDB(); + const meshtastic_DeviceState savedDeviceState = devicestate; + const meshtastic_ChannelFile savedChannelFile = channelFile; + + owner = meshtastic_User_init_zero; + owner.is_licensed = true; + installEncryptedAndAdminChannels(); + TEST_ASSERT_TRUE(nodeDB->backupPreferences(meshtastic_AdminMessage_BackupLocation_FLASH)); + + owner.is_licensed = false; + channels.initDefaults(); + TEST_ASSERT_TRUE( + nodeDB->restorePreferences(meshtastic_AdminMessage_BackupLocation_FLASH, SEGMENT_DEVICESTATE | SEGMENT_CHANNELS)); + TEST_ASSERT_TRUE(owner.is_licensed); + assertLicensedChannelsSanitized(); + TEST_ASSERT_FALSE_MESSAGE(channels.ensureLicensedOperation(), "restored licensed channels must remain sanitized"); + + devicestate = savedDeviceState; + channelFile = savedChannelFile; + nodeDB->saveToDisk(SEGMENT_DEVICESTATE | SEGMENT_CHANNELS); + FSCom.remove(backupFileName); + delete nodeDB; + nodeDB = savedNodeDB; +} + static meshtastic_Config makeLoraSetConfig(meshtastic_Config_LoRaConfig_RegionCode region, bool usePreset, meshtastic_Config_LoRaConfig_ModemPreset preset) { @@ -1338,6 +1377,7 @@ void setup() // getRegion() RUN_TEST(test_handleSetOwner_persistsLicensedChannelSanitation); RUN_TEST(test_bootDefense_sanitizesStaleLicensedChannelsOnce); + RUN_TEST(test_restorePreferences_sanitizesLicensedBackupBeforeReturn); RUN_TEST(test_getRegion_returnsCorrectRegion_US); RUN_TEST(test_getRegion_returnsCorrectRegion_EU868); RUN_TEST(test_getRegion_returnsCorrectRegion_LORA24); diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index afd9af6666a..713c359caba 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -15,6 +15,7 @@ #include "NodeStatus.h" #include "TestUtil.h" #include "airtime.h" +#include "support/MockMeshService.h" #include // The whole suite exercises XEdDSA sign/verify and checkXeddsaReceivePolicy, all of which are @@ -636,6 +637,16 @@ void test_C00_licensed_identity_key_is_generated_and_preserved(void) memcpy(publicKey, config.security.public_key.bytes, sizeof(publicKey)); const NodeNum migratedNodeNum = myNodeInfo.my_node_num; TEST_ASSERT_NOT_EQUAL(LOCAL_NODE, migratedNodeNum); + TEST_ASSERT_TRUE(mockNodeDB->licensedIdentityMigrationPending); + + MockMeshService mockService; + service = &mockService; + TEST_ASSERT_TRUE(mockNodeDB->notifyPendingLicensedIdentityMigration()); + TEST_ASSERT_EQUAL(1, mockService.notificationCount); + TEST_ASSERT_FALSE(mockNodeDB->licensedIdentityMigrationPending); + TEST_ASSERT_FALSE(mockNodeDB->notifyPendingLicensedIdentityMigration()); + TEST_ASSERT_EQUAL(1, mockService.notificationCount); + service = nullptr; config.security.public_key.size = 0; owner.public_key.size = 0; From 0dd414d2069e280485d2ef6bca977dd5b4305589 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 4/7] fix(security): address licensed signing review feedback --- src/modules/AdminModule.cpp | 3 +- test/test_admin_radio/test_main.cpp | 60 +++++++++++++++++++++++--- test/test_packet_signing/test_main.cpp | 30 ++++++++++--- 3 files changed, 79 insertions(+), 14 deletions(-) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index f5c4d8b5b36..d4f1617d867 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -981,6 +981,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; } @@ -993,7 +994,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; diff --git a/test/test_admin_radio/test_main.cpp b/test/test_admin_radio/test_main.cpp index 28422343a88..283a5266c07 100644 --- a/test/test_admin_radio/test_main.cpp +++ b/test/test_admin_radio/test_main.cpp @@ -933,6 +933,36 @@ static void test_channelSpacingCalculation_placeholder() // AdminModuleTestShim comes from test/support - the friend seam AdminModule.h declares. static AdminModuleTestShim *testAdmin; +class ScopedAdminRadioGlobals +{ + public: + ScopedAdminRadioGlobals() + : savedNodeDB(nodeDB), savedNodeInfoModule(nodeInfoModule), savedDeviceState(devicestate), savedConfig(config), + savedChannelFile(channelFile), testNodeDB(new NodeDB()) + { + nodeDB = testNodeDB; + } + + ~ScopedAdminRadioGlobals() + { + nodeInfoModule = savedNodeInfoModule; + nodeDB = savedNodeDB; + delete testNodeDB; + devicestate = savedDeviceState; + config = savedConfig; + channelFile = savedChannelFile; + initRegion(); + } + + private: + NodeDB *savedNodeDB; + NodeInfoModule *savedNodeInfoModule; + meshtastic_DeviceState savedDeviceState; + meshtastic_LocalConfig savedConfig; + meshtastic_ChannelFile savedChannelFile; + NodeDB *testNodeDB; +}; + static void installEncryptedAndAdminChannels() { channels.initDefaults(); @@ -965,8 +995,7 @@ static void assertLicensedChannelsSanitized() static void test_handleSetOwner_persistsLicensedChannelSanitation() { - NodeDB *savedNodeDB = nodeDB; - nodeDB = new NodeDB(); + ScopedAdminRadioGlobals globals; owner = meshtastic_User_init_zero; config.lora.region = meshtastic_Config_LoRaConfig_RegionCode_UNSET; installEncryptedAndAdminChannels(); @@ -974,10 +1003,8 @@ static void test_handleSetOwner_persistsLicensedChannelSanitation() meshtastic_User licensed = meshtastic_User_init_zero; licensed.is_licensed = true; testAdmin->deferSaves(); - NodeInfoModule *savedNodeInfoModule = nodeInfoModule; nodeInfoModule = reinterpret_cast(1); // reloadOwner(false) only checks presence testAdmin->handleSetOwner(licensed); - nodeInfoModule = savedNodeInfoModule; TEST_ASSERT_TRUE(testAdmin->savedSegments() & SEGMENT_CHANNELS); assertLicensedChannelsSanitized(); @@ -990,9 +1017,6 @@ static void test_handleSetOwner_persistsLicensedChannelSanitation() 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() @@ -1045,6 +1069,27 @@ static meshtastic_Config makeLoraSetConfig(meshtastic_Config_LoRaConfig_RegionCo return c; } +static void test_handleSetConfig_persistsLicensedFirstRegionIdentity() +{ + ScopedAdminRadioGlobals globals; + owner = meshtastic_User_init_zero; + owner.is_licensed = true; + config.security = meshtastic_Config_SecurityConfig_init_zero; + config.lora = meshtastic_Config_LoRaConfig_init_zero; + config.lora.region = meshtastic_Config_LoRaConfig_RegionCode_UNSET; + initRegion(); + + testAdmin->deferSaves(); + const meshtastic_Config c = + makeLoraSetConfig(meshtastic_Config_LoRaConfig_RegionCode_US, true, meshtastic_Config_LoRaConfig_ModemPreset_LONG_FAST); + testAdmin->handleSetConfig(c, false); + + const int expectedSegments = SEGMENT_CONFIG | SEGMENT_MODULECONFIG | SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; + TEST_ASSERT_EQUAL_INT(expectedSegments, testAdmin->savedSegments()); + TEST_ASSERT_EQUAL(32, config.security.private_key.size); + TEST_ASSERT_EQUAL(32, owner.public_key.size); +} + static void test_handleSetConfig_fromOthers_invalidPresetRejected() { // Set up a known-good baseline in the global config @@ -1376,6 +1421,7 @@ void setup() // getRegion() RUN_TEST(test_handleSetOwner_persistsLicensedChannelSanitation); + RUN_TEST(test_handleSetConfig_persistsLicensedFirstRegionIdentity); RUN_TEST(test_bootDefense_sanitizesStaleLicensedChannelsOnce); RUN_TEST(test_restorePreferences_sanitizesLicensedBackupBeforeReturn); RUN_TEST(test_getRegion_returnsCorrectRegion_US); diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 713c359caba..0e29ffec3cb 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -92,6 +92,18 @@ class MockNodeDB : public NodeDB static MockNodeDB *mockNodeDB = nullptr; +#ifdef ARCH_PORTDUINO +class ScopedPkiRoutingForTest +{ + public: + ScopedPkiRoutingForTest() : savedForceSimRadio(portduino_config.force_simradio) { portduino_config.force_simradio = false; } + ~ScopedPkiRoutingForTest() { portduino_config.force_simradio = savedForceSimRadio; } + + private: + bool savedForceSimRadio; +}; +#endif + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- @@ -522,6 +534,9 @@ void test_B9_licensed_oversized_unicast_remains_unsigned(void) // B10: normal-mode direct messages retain their existing PKI encryption behavior. void test_B10_normal_unicast_still_uses_pki(void) { +#ifdef ARCH_PORTDUINO + ScopedPkiRoutingForTest pkiRouting; +#endif uint8_t localPub[32], localPriv[32], remotePub[32], remotePriv[32]; crypto->generateKeyPair(localPub, localPriv); crypto->generateKeyPair(remotePub, remotePriv); @@ -547,6 +562,9 @@ void test_B10_normal_unicast_still_uses_pki(void) // B11: publishing a licensed node's key must not make inbound PKI decryption reachable. void test_B11_licensed_receiver_does_not_decrypt_pki(void) { +#ifdef ARCH_PORTDUINO + ScopedPkiRoutingForTest pkiRouting; +#endif uint8_t localPub[32], localPriv[32], remotePub[32], remotePriv[32]; crypto->generateKeyPair(localPub, localPriv); crypto->generateKeyPair(remotePub, remotePriv); @@ -594,10 +612,8 @@ void test_B12_licensed_port_and_destination_signing_matrix(void) } } -// =========================================================================== -// 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 - NodeInfoModule downgrade drops for broadcast ingress paths; +// unicast NodeInfo is exempt because senders never sign it. class NodeInfoTestShim : public NodeInfoModule { public: @@ -923,6 +939,8 @@ void test_E9_decoded_partial_signature_from_nonsigner_dropped(void) void setup() { initializeTestEnvironment(); + AirTime *savedAirTime = airTime; + meshtastic::NodeStatus *savedNodeStatus = nodeStatus; AirTime testAirTime; meshtastic::NodeStatus testNodeStatus; airTime = &testAirTime; @@ -979,8 +997,8 @@ void setup() RUN_TEST(test_E9_decoded_partial_signature_from_nonsigner_dropped); const int result = UNITY_END(); - airTime = nullptr; - nodeStatus = nullptr; + airTime = savedAirTime; + nodeStatus = savedNodeStatus; exit(result); } From 155c4f76289e760bd789d36f3254105f6b8e1702 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Thu, 9 Jul 2026 21:10:48 -0700 Subject: [PATCH 5/7] test: restore signing globals from Unity teardown --- test/test_admin_radio/test_main.cpp | 72 ++++++++++++++------------ test/test_packet_signing/test_main.cpp | 23 +++----- 2 files changed, 48 insertions(+), 47 deletions(-) diff --git a/test/test_admin_radio/test_main.cpp b/test/test_admin_radio/test_main.cpp index 283a5266c07..e16c990c505 100644 --- a/test/test_admin_radio/test_main.cpp +++ b/test/test_admin_radio/test_main.cpp @@ -932,36 +932,43 @@ static void test_channelSpacingCalculation_placeholder() // AdminModuleTestShim comes from test/support - the friend seam AdminModule.h declares. static AdminModuleTestShim *testAdmin; - -class ScopedAdminRadioGlobals -{ - public: - ScopedAdminRadioGlobals() - : savedNodeDB(nodeDB), savedNodeInfoModule(nodeInfoModule), savedDeviceState(devicestate), savedConfig(config), - savedChannelFile(channelFile), testNodeDB(new NodeDB()) - { - nodeDB = testNodeDB; - } - - ~ScopedAdminRadioGlobals() - { - nodeInfoModule = savedNodeInfoModule; - nodeDB = savedNodeDB; - delete testNodeDB; - devicestate = savedDeviceState; - config = savedConfig; - channelFile = savedChannelFile; - initRegion(); - } - - private: - NodeDB *savedNodeDB; - NodeInfoModule *savedNodeInfoModule; - meshtastic_DeviceState savedDeviceState; - meshtastic_LocalConfig savedConfig; - meshtastic_ChannelFile savedChannelFile; - NodeDB *testNodeDB; -}; +static bool adminRadioGlobalsActive; +static NodeDB *savedNodeDB; +static NodeDB *replacementNodeDB; +static NodeInfoModule *savedNodeInfoModule; +static meshtastic_DeviceState savedDeviceState; +static meshtastic_User savedOwner; +static meshtastic_LocalConfig savedConfig; +static meshtastic_ChannelFile savedChannelFile; + +static void replaceAdminRadioGlobals() +{ + savedNodeDB = nodeDB; + savedNodeInfoModule = nodeInfoModule; + savedDeviceState = devicestate; + savedOwner = owner; + savedConfig = config; + savedChannelFile = channelFile; + replacementNodeDB = new NodeDB(); + nodeDB = replacementNodeDB; + adminRadioGlobalsActive = true; +} + +static void restoreAdminRadioGlobals() +{ + if (!adminRadioGlobalsActive) + return; + nodeInfoModule = savedNodeInfoModule; + nodeDB = savedNodeDB; + delete replacementNodeDB; + replacementNodeDB = nullptr; + devicestate = savedDeviceState; + owner = savedOwner; + config = savedConfig; + channelFile = savedChannelFile; + initRegion(); + adminRadioGlobalsActive = false; +} static void installEncryptedAndAdminChannels() { @@ -995,7 +1002,7 @@ static void assertLicensedChannelsSanitized() static void test_handleSetOwner_persistsLicensedChannelSanitation() { - ScopedAdminRadioGlobals globals; + replaceAdminRadioGlobals(); owner = meshtastic_User_init_zero; config.lora.region = meshtastic_Config_LoRaConfig_RegionCode_UNSET; installEncryptedAndAdminChannels(); @@ -1071,7 +1078,7 @@ static meshtastic_Config makeLoraSetConfig(meshtastic_Config_LoRaConfig_RegionCo static void test_handleSetConfig_persistsLicensedFirstRegionIdentity() { - ScopedAdminRadioGlobals globals; + replaceAdminRadioGlobals(); owner = meshtastic_User_init_zero; owner.is_licensed = true; config.security = meshtastic_Config_SecurityConfig_init_zero; @@ -1403,6 +1410,7 @@ void setUp(void) } void tearDown(void) { + restoreAdminRadioGlobals(); service = nullptr; delete mockMeshService; mockMeshService = nullptr; diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 0e29ffec3cb..a2f703a8274 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -93,15 +93,7 @@ class MockNodeDB : public NodeDB static MockNodeDB *mockNodeDB = nullptr; #ifdef ARCH_PORTDUINO -class ScopedPkiRoutingForTest -{ - public: - ScopedPkiRoutingForTest() : savedForceSimRadio(portduino_config.force_simradio) { portduino_config.force_simradio = false; } - ~ScopedPkiRoutingForTest() { portduino_config.force_simradio = savedForceSimRadio; } - - private: - bool savedForceSimRadio; -}; +static bool savedForceSimRadio; #endif // --------------------------------------------------------------------------- @@ -171,6 +163,10 @@ static bool signedEncodingFits(const meshtastic_Data *d) // --------------------------------------------------------------------------- void setUp(void) { +#ifdef ARCH_PORTDUINO + savedForceSimRadio = portduino_config.force_simradio; + portduino_config.force_simradio = false; +#endif // Construct the mock FIRST: the NodeDB constructor can reload persisted state from the // host filesystem (portduino VFS) and repopulate the globals - a saved private key // re-enables the PKI encrypt path and fails the unicast tests on hosts with leftover prefs. @@ -194,6 +190,9 @@ void tearDown(void) delete mockNodeDB; mockNodeDB = nullptr; nodeDB = nullptr; +#ifdef ARCH_PORTDUINO + portduino_config.force_simradio = savedForceSimRadio; +#endif } // =========================================================================== @@ -534,9 +533,6 @@ void test_B9_licensed_oversized_unicast_remains_unsigned(void) // B10: normal-mode direct messages retain their existing PKI encryption behavior. void test_B10_normal_unicast_still_uses_pki(void) { -#ifdef ARCH_PORTDUINO - ScopedPkiRoutingForTest pkiRouting; -#endif uint8_t localPub[32], localPriv[32], remotePub[32], remotePriv[32]; crypto->generateKeyPair(localPub, localPriv); crypto->generateKeyPair(remotePub, remotePriv); @@ -562,9 +558,6 @@ void test_B10_normal_unicast_still_uses_pki(void) // B11: publishing a licensed node's key must not make inbound PKI decryption reachable. void test_B11_licensed_receiver_does_not_decrypt_pki(void) { -#ifdef ARCH_PORTDUINO - ScopedPkiRoutingForTest pkiRouting; -#endif uint8_t localPub[32], localPriv[32], remotePub[32], remotePriv[32]; crypto->generateKeyPair(localPub, localPriv); crypto->generateKeyPair(remotePub, remotePriv); From 446e146aff90a381b85db83b6a5373623794b639 Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 19 Jul 2026 16:32:59 -0700 Subject: [PATCH 6/7] fix(baseui): allow confirmed ham region selection --- src/graphics/draw/MenuHandler.cpp | 15 +++++++-------- src/mesh/RadioInterface.cpp | 5 +++-- src/mesh/RadioInterface.h | 6 ++++-- test/test_admin_radio/test_main.cpp | 11 +++++++++++ 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/graphics/draw/MenuHandler.cpp b/src/graphics/draw/MenuHandler.cpp index ccb447399bd..8155510b2c3 100644 --- a/src/graphics/draw/MenuHandler.cpp +++ b/src/graphics/draw/MenuHandler.cpp @@ -285,21 +285,20 @@ void menuHandler::LoraRegionPicker(uint32_t duration) return; } - // Guard: without a reboot, reconfigure() applies the region directly, so reject - // regions this node can't use up front: unrecognized codes, licensed-only regions, - // and radio hardware mismatches (2.4 GHz vs sub-GHz) - the same checks the admin - // set-config path applies, but side-effect-free: ignoring a menu selection should - // not record a critical error or notify clients. getRadio() used to catch hardware - // mismatches post-reboot only. + const RegionInfo *selectedRegionInfo = getRegion(selectedRegion); + bool hamMode = selectedRegionInfo->profile->licensedOnly; + + // Guard: without a reboot, reconfigure() applies the region directly. A Ham choice + // becomes licensed only after its confirmation, so validate its radio compatibility + // against that prospective owner state without bypassing hardware checks. auto candidateLora = config.lora; candidateLora.region = selectedRegion; char regionErr[160]; - if (!RadioInterface::checkConfigRegion(candidateLora, regionErr, sizeof(regionErr))) { + if (!RadioInterface::checkConfigRegion(candidateLora, regionErr, sizeof(regionErr), hamMode)) { LOG_WARN("Ignoring region selection: %s", regionErr); return; } - bool hamMode = getRegion(selectedRegion)->profile->licensedOnly; if (hamMode) { LOG_INFO("User chose an amateur radio mode region"); pendingRegion = selectedRegion; diff --git a/src/mesh/RadioInterface.cpp b/src/mesh/RadioInterface.cpp index f616e866a42..043bd59eff7 100644 --- a/src/mesh/RadioInterface.cpp +++ b/src/mesh/RadioInterface.cpp @@ -1022,7 +1022,8 @@ const RegionInfo *RadioInterface::regionSwapForPreset(meshtastic_Config_LoRaConf * receives the human-readable failure reason. * Returns false if not compatible. */ -bool RadioInterface::checkConfigRegion(const meshtastic_Config_LoRaConfig &loraConfig, char *errBuf, size_t errLen) +bool RadioInterface::checkConfigRegion(const meshtastic_Config_LoRaConfig &loraConfig, char *errBuf, size_t errLen, + bool prospectiveLicensedOwner) { const RegionInfo *newRegion = getRegion(loraConfig.region); @@ -1034,7 +1035,7 @@ bool RadioInterface::checkConfigRegion(const meshtastic_Config_LoRaConfig &loraC } // If you are not licensed, you can't use ham regions. - if (newRegion->profile->licensedOnly && !devicestate.owner.is_licensed) { + if (newRegion->profile->licensedOnly && !devicestate.owner.is_licensed && !prospectiveLicensedOwner) { if (errBuf) snprintf(errBuf, errLen, "Region %s requires licensed mode", newRegion->name); return false; diff --git a/src/mesh/RadioInterface.h b/src/mesh/RadioInterface.h index 59542674658..eb9315ac122 100644 --- a/src/mesh/RadioInterface.h +++ b/src/mesh/RadioInterface.h @@ -256,8 +256,10 @@ class RadioInterface static bool checkOrClampConfigLora(meshtastic_Config_LoRaConfig &loraConfig, bool clamp); // Check if a candidate region is compatible and valid, with no side effects (safe for - // speculative UI checks). errBuf, if given, receives the failure reason. - static bool checkConfigRegion(const meshtastic_Config_LoRaConfig &loraConfig, char *errBuf = nullptr, size_t errLen = 0); + // speculative UI checks). prospectiveLicensedOwner is for a UI flow that requires + // confirmation before it sets the owner licensed. errBuf, if given, receives the failure reason. + static bool checkConfigRegion(const meshtastic_Config_LoRaConfig &loraConfig, char *errBuf = nullptr, size_t errLen = 0, + bool prospectiveLicensedOwner = false); // Check if a candidate region is compatible and valid. On failure, logs at ERROR, // records a critical error, and sends a client notification. diff --git a/test/test_admin_radio/test_main.cpp b/test/test_admin_radio/test_main.cpp index c35d6d48bd3..c2f3f6759ed 100644 --- a/test/test_admin_radio/test_main.cpp +++ b/test/test_admin_radio/test_main.cpp @@ -1379,6 +1379,16 @@ static void test_checkConfigRegion_quietCheckReportsReason() TEST_ASSERT_TRUE_MESSAGE(strlen(err) > 0, "Expected a failure reason in errBuf"); } +static void test_checkConfigRegion_allowsProspectiveLicensedOwner() +{ + meshtastic_Config_LoRaConfig cfg = meshtastic_Config_LoRaConfig_init_zero; + cfg.region = meshtastic_Config_LoRaConfig_RegionCode_ITU1_2M; + devicestate.owner.is_licensed = false; + + TEST_ASSERT_FALSE(RadioInterface::checkConfigRegion(cfg)); + TEST_ASSERT_TRUE(RadioInterface::checkConfigRegion(cfg, nullptr, 0, true)); +} + static void test_handleSetConfig_fromOthers_siblingLockedPresetSwapsRegion() { // Baseline: EU_866 (LITE profile) @@ -1705,6 +1715,7 @@ void setup() RUN_TEST(test_handleSetConfig_security_acceptsSuppliedKeypair); RUN_TEST(test_regionInfo_supportsPreset); RUN_TEST(test_checkConfigRegion_quietCheckReportsReason); + RUN_TEST(test_checkConfigRegion_allowsProspectiveLicensedOwner); RUN_TEST(test_handleSetConfig_fromOthers_siblingLockedPresetSwapsRegion); RUN_TEST(test_handleSetConfig_fromOthers_lockedPresetFromNonTrioRegionRejected); From 6584e1e8d5b9ece848681b8efe9367e39fc71eed Mon Sep 17 00:00:00 2001 From: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Date: Sun, 19 Jul 2026 16:52:59 -0700 Subject: [PATCH 7/7] fix(baseui): guard region picker validation --- src/graphics/draw/MenuHandler.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/graphics/draw/MenuHandler.cpp b/src/graphics/draw/MenuHandler.cpp index 8155510b2c3..b65917dad0f 100644 --- a/src/graphics/draw/MenuHandler.cpp +++ b/src/graphics/draw/MenuHandler.cpp @@ -286,14 +286,13 @@ void menuHandler::LoraRegionPicker(uint32_t duration) } const RegionInfo *selectedRegionInfo = getRegion(selectedRegion); - bool hamMode = selectedRegionInfo->profile->licensedOnly; + bool hamMode = selectedRegionInfo->code == selectedRegion && selectedRegionInfo->profile && + selectedRegionInfo->profile->licensedOnly; - // Guard: without a reboot, reconfigure() applies the region directly. A Ham choice - // becomes licensed only after its confirmation, so validate its radio compatibility - // against that prospective owner state without bypassing hardware checks. + // Validate radio compatibility for a prospective Ham region before confirmation. auto candidateLora = config.lora; candidateLora.region = selectedRegion; - char regionErr[160]; + char regionErr[160] = {}; if (!RadioInterface::checkConfigRegion(candidateLora, regionErr, sizeof(regionErr), hamMode)) { LOG_WARN("Ignoring region selection: %s", regionErr); return;