From db3eb91015f1e09234a713cd68be5d41fae1c758 Mon Sep 17 00:00:00 2001 From: Clive Blackledge Date: Wed, 12 Aug 2026 06:05:20 -0700 Subject: [PATCH] fix(security): never log the X25519 identity private key (#11435) installDefaultConfig() restores a preserved identity key when the config is reset with preserveKey=true. On that path it called: printBytes("Restored key", config.security.private_key.bytes, config.security.private_key.size); printBytes() hex-dumps the buffer to LOG_DEBUG, so this emitted all 32 bytes of the raw X25519 identity private key to the serial/BLE debug log. Debug logs are not a private channel. They are routinely captured over serial or BLE and pasted verbatim into GitHub issues, Discord threads and support requests. Anyone who reads such a log recovers the node's identity private key, and can then impersonate the node and decrypt every PKI direct message addressed to it -- past messages included, since the key is long-lived and the DH shared secret is static per node pair. There is no revocation story short of generating a new identity. Replaced with a LOG_DEBUG that records that a key was restored and contains no key-derived bytes. The restore/no-restore signal is the genuinely useful diagnostic here ("did my key survive the reset?"), and it costs nothing to keep; the bytes were never what made the line useful. Log level is unchanged -- printBytes() already logged at LOG_DEBUG. Deliberately NOT a truncated prefix or a hash. A prefix is still key material: it hands an attacker free bytes and shrinks the search space. A hash is a confirmation oracle -- it lets anyone holding a candidate key verify it against the log, which is exactly the check an attacker needs. Neither is a compromise; both leak. If a log line survives at all it must carry zero key-derived bytes. Sites changed: - src/mesh/NodeDB.cpp:988 -- the only full private-key dump in src/. Audited and deliberately left alone: - NodeDB.cpp:3552,3604 ("Incoming Pubkey", "Saved Pubkey") -- public keys, published to the mesh by design; not secret. - CryptoEngine.cpp:245,285 -- nonces, not key material. - CryptoEngine.cpp:246,286 -- first 8 bytes of the derived shared_key, and AdminModule.cpp:2006,2013,2014 -- the 8-byte admin session passkey. Both are secrets rather than public values, but neither is the identity private key and both are out of scope for this fix; noted for follow-up. No unused-variable fallout: private_key_temp is still read by the memcpy above, and printBytes() is still used by the two pubkey sites, so the meshUtils.h include is still required. Co-authored-by: Claude Opus 5 --- src/mesh/NodeDB.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index d24473125..e31df4faa 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -985,7 +985,8 @@ void NodeDB::installDefaultConfig(bool preserveKey = false) if (shouldPreserveKey) { config.security.private_key.size = 32; memcpy(config.security.private_key.bytes, private_key_temp, config.security.private_key.size); - printBytes("Restored key", config.security.private_key.bytes, config.security.private_key.size); + // Never log the key bytes: debug logs get pasted into public bug reports. + LOG_DEBUG("Restored preserved private key"); } else { config.security.private_key.size = 0; }