diff --git a/src/configuration.h b/src/configuration.h index a06c2e75b..124c45029 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -530,8 +530,9 @@ along with this program. If not, see . // ----------------------------------------------------------------------------- // MESHTASTIC_LOCKDOWN — combined hardened build flag -// Derives MESHTASTIC_PHONEAPI_ACCESS_CONTROL (PhoneAPI access control) on all platforms, -// and MESHTASTIC_ENCRYPTED_STORAGE (CC310 flash encryption) on nRF52 only. +// On nRF52 builds, derives MESHTASTIC_PHONEAPI_ACCESS_CONTROL (PhoneAPI access control), +// MESHTASTIC_ENCRYPTED_STORAGE (CC310 flash encryption), MESHTASTIC_ENABLE_APPROTECT, +// and HARDENED_DEFAULTS. On non-nRF52 platforms this flag is ignored (requires CC310). // ----------------------------------------------------------------------------- #ifdef MESHTASTIC_LOCKDOWN #ifdef ARCH_NRF52 diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 9df9cadad..7aad3fbb5 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -98,12 +98,13 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta if (config.security.is_managed && EncryptedStorage::isUnlocked()) { // TAK security commands carry a non-empty private_key — always let them through. // The passphrase HMAC in EncryptedStorage is the security gate for those packets. + // Only the passphrase delivery command itself is whitelisted — it is its own auth gate. + // Destructive commands (factory reset, nodedb reset) require an already-authorized + // connection (passphrase verified or PKC admin key). bool isTakSecurityCmd = (r->which_payload_variant == meshtastic_AdminMessage_set_config_tag && r->set_config.which_payload_variant == meshtastic_Config_security_tag && - r->set_config.payload_variant.security.private_key.size >= 1) || - r->which_payload_variant == meshtastic_AdminMessage_factory_reset_config_tag || - r->which_payload_variant == meshtastic_AdminMessage_nodedb_reset_tag; + r->set_config.payload_variant.security.private_key.size >= 1); #ifdef MESHTASTIC_PHONEAPI_ACCESS_CONTROL if (!isTakSecurityCmd && !PhoneAPI::isLocalAdminAuthorized()) { #else @@ -136,8 +137,12 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta LOG_INFO("PKC admin payload with authorized sender key"); #ifdef MESHTASTIC_PHONEAPI_ACCESS_CONTROL - // Authorize local PhoneAPI connections to dump full config - PhoneAPI::authorizeLocalAdmin(); + // Only authorize local PhoneAPI connections when the PKC admin message + // came from a local client (from=0). A remote PKC admin (from!=0) has no + // business unlocking local BLE/USB config dumps. + if (mp.from == 0) { + PhoneAPI::authorizeLocalAdmin(); + } #endif // Automatically favorite the node that is using the admin key diff --git a/src/security/EncryptedStorage.cpp b/src/security/EncryptedStorage.cpp index bf0973142..557df0e23 100644 --- a/src/security/EncryptedStorage.cpp +++ b/src/security/EncryptedStorage.cpp @@ -7,6 +7,7 @@ #include "FSCommon.h" #include "RTC.h" #include "SPILock.h" +#include "SafeFile.h" #include #include @@ -1181,6 +1182,17 @@ bool readAndDecrypt(const char *filename, uint8_t *outBuf, size_t outBufSize, si } memset(computedHmac, 0, sizeof(computedHmac)); + // plaintextLen is not covered by the HMAC, so validate it against the actual ciphertext + // length derived from the file size. For AES-CTR the two are always equal in a legitimate + // file; a mismatch means the header field was tampered independently of the ciphertext. + if (plaintextLen != ciphertextLen) { + LOG_ERROR("EncryptedStorage: plaintextLen (%d) != ciphertextLen (%d) in %s — header tampered", + plaintextLen, (uint32_t)ciphertextLen, filename); + memset(fileBuf, 0, fileSize); + delete[] fileBuf; + return false; + } + // Decrypt if (plaintextLen > outBufSize) { LOG_ERROR("EncryptedStorage: Output buffer too small for %s (%d > %d)", filename, plaintextLen, @@ -1265,30 +1277,28 @@ bool encryptAndWrite(const char *filename, const uint8_t *plaintext, size_t plai return false; } - concurrency::LockGuard g(spiLock); - FSCom.remove(filename); // prevent O_APPEND accumulation on nRF52 - auto f = FSCom.open(filename, FILE_O_WRITE); - if (!f) { - LOG_ERROR("EncryptedStorage: Can't create %s", filename); - memset(ciphertext, 0, ciphertextLen); - delete[] ciphertext; - return false; - } + // SafeFile handles remove-before-write (nRF52) and tmp+readback+rename (other platforms). + // fullAtomic controls whether the old file is kept until the rename succeeds. + SafeFile sf(filename, fullAtomic); uint32_t magic = MAGIC; - f.write((uint8_t *)&magic, 4); + sf.write((uint8_t *)&magic, 4); uint8_t ver = VERSION; - f.write(&ver, 1); - f.write(nonce, NONCE_SIZE); + sf.write(&ver, 1); + sf.write(nonce, NONCE_SIZE); uint32_t ptLen = (uint32_t)plaintextLen; - f.write((uint8_t *)&ptLen, 4); - f.write(ciphertext, ciphertextLen); - f.write(hmac, HMAC_SIZE); - f.flush(); - f.close(); + sf.write((uint8_t *)&ptLen, 4); + sf.write(ciphertext, ciphertextLen); + sf.write(hmac, HMAC_SIZE); + memset(ciphertext, 0, ciphertextLen); delete[] ciphertext; + if (!sf.close()) { + LOG_ERROR("EncryptedStorage: Write/verify failed for %s", filename); + return false; + } + LOG_INFO("EncryptedStorage: Encrypted %s (%d bytes plaintext)", filename, plaintextLen); return true; #else