diff --git a/src/graphics/draw/MenuHandler.cpp b/src/graphics/draw/MenuHandler.cpp index 6471d70c6..446f29a21 100644 --- a/src/graphics/draw/MenuHandler.cpp +++ b/src/graphics/draw/MenuHandler.cpp @@ -245,8 +245,9 @@ static void applyLoraRegion(meshtastic_Config_LoRaConfig_RegionCode region, bool } auto changes = SEGMENT_CONFIG; #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - if (crypto) { - crypto->ensurePkiKeys(config.security, owner); + // Minting the key moves our node num with it, and nothing reboots on this path to repair it later. + if (nodeDB->ensurePkiIdentity()) { + changes |= SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; } #endif initRegion(); diff --git a/src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp b/src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp index 5e8a08e75..863c1e85d 100644 --- a/src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp +++ b/src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp @@ -324,8 +324,9 @@ static void applyLoRaRegion(meshtastic_Config_LoRaConfig_RegionCode region) auto changes = SEGMENT_CONFIG; #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - if (crypto) { - crypto->ensurePkiKeys(config.security, owner); + // Minting the key moves our node num with it, and the reboot below only re-derives after the save. + if (nodeDB->ensurePkiIdentity()) { + changes |= SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; } #endif diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index 040777857..e2557e8d0 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -4446,14 +4446,32 @@ bool NodeDB::createNewIdentity() myNodeInfo.my_node_num = newNodeNum; + // The number has moved, so the caller must persist it whatever happens next. Returning false here + // would leave the new key saved against the old number, which is the break this exists to prevent. meshtastic_NodeInfoLite *info = getOrCreateMeshNode(getNodeNum()); - if (!info) - return false; - TypeConversions::CopyUserToNodeInfoLite(info, owner); + if (info) + TypeConversions::CopyUserToNodeInfoLite(info, owner); + else + LOG_ERROR("No room for our own node 0x%08x, identity moved without a self record", newNodeNum); return true; } +bool NodeDB::ensurePkiIdentity() +{ +#if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) + // A failed or declined keygen leaves the existing key, and so the existing node num, untouched. + if (!crypto || !crypto->ensurePkiKeys(config.security, owner)) + return false; + + // ensurePkiKeys() writes key material only, so my_node_num is still the stale MAC-derived value. + // createNewIdentity() early-returns when the key, and so the node num, did not actually change. + return createNewIdentity(); +#else + return false; +#endif +} + bool NodeDB::backupPreferences(meshtastic_AdminMessage_BackupLocation location) { bool success = false; diff --git a/src/mesh/NodeDB.h b/src/mesh/NodeDB.h index ca0acf171..0e669cca5 100644 --- a/src/mesh/NodeDB.h +++ b/src/mesh/NodeDB.h @@ -596,6 +596,10 @@ class NodeDB bool createNewIdentity(); + /// Mint the identity keypair outside the boot path and re-seat my_node_num == crc32(public_key). + /// @return true if my_node_num moved; the caller must then also persist SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE. + bool ensurePkiIdentity(); + bool backupPreferences(meshtastic_AdminMessage_BackupLocation location); bool restorePreferences(meshtastic_AdminMessage_BackupLocation location, int restoreWhat = SEGMENT_CONFIG | SEGMENT_MODULECONFIG | SEGMENT_DEVICESTATE | SEGMENT_CHANNELS); diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 80bb79903..55b029f03 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -1034,8 +1034,10 @@ 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 && !owner.is_licensed) { - crypto->ensurePkiKeys(config.security, owner); + // Minting the key moves our node num with it (my_node_num == crc32(public_key)), so + // persist devicestate + the node DB too - exactly as the licensed branch below does. + if (!owner.is_licensed && nodeDB->ensurePkiIdentity()) { + changes |= SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; } #endif // new region is valid and we're coming from an unset region, so enable tx diff --git a/src/platform/portduino/wasm/portduino_glue_wasm.cpp b/src/platform/portduino/wasm/portduino_glue_wasm.cpp index a4b4a31e9..65d632084 100644 --- a/src/platform/portduino/wasm/portduino_glue_wasm.cpp +++ b/src/platform/portduino/wasm/portduino_glue_wasm.cpp @@ -12,10 +12,9 @@ // - exec() short-circuits to "" (no popen/shell in the browser). // Downstream is unchanged: Ch341Hal -> libpinedio_webusb.c -> WebUSB. -#include "CryptoEngine.h" // crypto->ensurePkiKeys() #include "MeshRadio.h" // initRegion() #include "MeshService.h" // service->reloadConfig() -#include "NodeDB.h" // config, owner globals + SEGMENT_CONFIG +#include "NodeDB.h" // config globals, SEGMENT_*, nodeDB->ensurePkiIdentity() #include "PhoneAPI.h" // the transport-agnostic client API seam #include "PortduinoFS.h" // portduinoVFS #include "PortduinoGlue.h" // declares `portduino_config` + Ch341Hal @@ -260,11 +259,13 @@ extern "C" EMSCRIPTEN_KEEPALIVE int wasm_set_region(int region) if (!(RadioInterface::validateConfigRegion(validated) && RadioInterface::validateConfigLora(validated))) return -1; + int changes = SEGMENT_CONFIG; bool wasUnset = (config.lora.region == meshtastic_Config_LoRaConfig_RegionCode_UNSET); if (wasUnset && newRegion > meshtastic_Config_LoRaConfig_RegionCode_UNSET) { #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - if (crypto) - crypto->ensurePkiKeys(config.security, owner); // first real region -> generate keys + // Minting the key moves our node num with it, so persist devicestate + the node DB too. + if (nodeDB && nodeDB->ensurePkiIdentity()) + changes |= SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; #endif validated.tx_enabled = true; } @@ -274,8 +275,8 @@ extern "C" EMSCRIPTEN_KEEPALIVE int wasm_set_region(int region) config.lora = validated; initRegion(); // repoint myRegion at the new region table if (service) - service->reloadConfig(SEGMENT_CONFIG); // reconfigure radio (new freq) + persist - wasm_fs_sync(); // browser: flush config.proto to IndexedDB + service->reloadConfig(changes); // reconfigure radio (new freq) + persist + wasm_fs_sync(); // browser: flush config.proto to IndexedDB return 0; } diff --git a/test/test_admin_radio/test_main.cpp b/test/test_admin_radio/test_main.cpp index ebdad8827..004037c9a 100644 --- a/test/test_admin_radio/test_main.cpp +++ b/test/test_admin_radio/test_main.cpp @@ -23,6 +23,7 @@ #include "mesh/Channels.h" #include "modules/AdminModule.h" #include "modules/NodeInfoModule.h" +#include // crc32Buffer(), for the my_node_num == crc32(public_key) invariant #include #include #include @@ -1153,6 +1154,32 @@ static void test_handleSetConfig_persistsLicensedFirstRegionIdentity() TEST_ASSERT_EQUAL(32, owner.public_key.size); } +// Unlicensed twin of the test above. Without the re-derivation the node signs broadcasts every receiver +// drops (verifyFirstContactNodeInfo: crc32(user.public_key) != from). +static void test_handleSetConfig_persistsUnlicensedFirstRegionIdentity() +{ + owner = meshtastic_User_init_zero; + owner.is_licensed = false; + 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, config.security.public_key.size); + TEST_ASSERT_EQUAL(32, owner.public_key.size); + // The invariant: a node's mesh address is derived from its identity key. + TEST_ASSERT_EQUAL_UINT32(crc32Buffer(config.security.public_key.bytes, config.security.public_key.size), + nodeDB->getNodeNum()); +} + static void test_handleSetConfig_fromOthers_invalidPresetRejected() { // Set up a known-good baseline in the global config @@ -1975,6 +2002,7 @@ void setup() // getRegion() RUN_TEST(test_handleSetOwner_persistsLicensedChannelSanitation); RUN_TEST(test_handleSetConfig_persistsLicensedFirstRegionIdentity); + RUN_TEST(test_handleSetConfig_persistsUnlicensedFirstRegionIdentity); RUN_TEST(test_bootDefense_sanitizesStaleLicensedChannelsOnce); RUN_TEST(test_restorePreferences_sanitizesLicensedBackupBeforeReturn); RUN_TEST(test_getRegion_returnsCorrectRegion_US);