mirror of
https://github.com/alexhopeoconnor/firmware.git
synced 2026-10-04 03:18:10 +10:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
54d6ce833e
commit
db3eb91015
+2
-1
@@ -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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user