Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/graphics/draw/MenuHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
5 changes: 3 additions & 2 deletions src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
24 changes: 21 additions & 3 deletions src/mesh/NodeDB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

bool NodeDB::backupPreferences(meshtastic_AdminMessage_BackupLocation location)
{
bool success = false;
Expand Down
4 changes: 4 additions & 0 deletions src/mesh/NodeDB.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions src/modules/AdminModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 7 additions & 6 deletions src/platform/portduino/wasm/portduino_glue_wasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Comment thread
h3lix1 marked this conversation as resolved.
#endif
validated.tx_enabled = true;
}
Expand All @@ -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;
}

Expand Down
28 changes: 28 additions & 0 deletions test/test_admin_radio/test_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "mesh/Channels.h"
#include "modules/AdminModule.h"
#include "modules/NodeInfoModule.h"
#include <ErriezCRC32.h> // crc32Buffer(), for the my_node_num == crc32(public_key) invariant
#include <pb_decode.h>
#include <pb_encode.h>
#include <string>
Expand Down Expand Up @@ -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());
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

static void test_handleSetConfig_fromOthers_invalidPresetRejected()
{
// Set up a known-good baseline in the global config
Expand Down Expand Up @@ -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);
Expand Down
Loading