mirror of
https://github.com/meshtastic/firmware.git
synced 2026-09-15 15:59:40 -04:00
Garth Vander HouwenandThomas Göttgens
83198c1cbb
* fix(pki): reject a restored pre-2.8 low-entropy key at set time, explain the swap Restoring/setting a private key is a private-key change: the public key is *generated* from it. The low-entropy blacklist check in generateCryptoKeyPair runs against the stored public_key at entry, which is empty on a bare key restore — so a known pre-2.8 weak key derived from the provided private key was never caught at set time. It was only detected on the next boot (once the weak public key had been persisted and re-checked), which looks to the user like their saved key silently "did not stick", and their node number (== crc32(public_key)) had quietly changed too. - NodeDB::generateCryptoKeyPair: in the provided-private-key branch, re-check the *derived* public key against LOW_ENTROPY_HASHES. If it matches, replace it with a fresh secure keypair and set keyIsLowEntropy so the reason is surfaced. - AdminModule set-config(security): when the restore path regenerated a rejected low-entropy key, send a client warning at set time explaining the key can't be restored and the node number changed. Scoped to that branch so a stale flag from a boot-time regeneration can't fire on unrelated security sets. No protobuf changes; reuses the existing ClientNotification warning path. Signed-off-by: Garth Vander Houwen <garthvh@yahoo.com> * fix(pki): gate low-entropy restore warning on successful keygen generateCryptoKeyPair returns false on an unset LoRa region before resetting keyIsLowEntropy, so the set-time warning could fire on a stale flag. Capture the return value and require both. Shorten the rationale comments to two lines each. * fix(pki): clear key sizes when a restored private key derives nothing The provided-private-key branch sets private_key.size and public_key.size to 32 before regeneratePublicKey() runs. On failure it returned false with both sizes still set, and AdminModule persisted that pair; every later keygen then re-derived from the same dead key. Clear both on the failure path so the next keygen mints a fresh identity. Add test_admin_radio coverage for the set-time restore path: a derived low-entropy key warns and rotates, a stale keyIsLowEntropy flag with keygen blocked does not warn, and a failed derivation clears both sizes. * fix(pki): validate a restored public key that is itself blacklisted A restore supplying both private_key and public_key reached neither keygen branch, so a whole pre-2.8 low-entropy pair was accepted and persisted at set time and only caught on the next boot. Re-derive when the supplied public key is blacklisted, which routes it through the same rejection and warning as the bare-private-key restore. A non-blacklisted keypair import is unaffected. Install the test crypto stub through a helper and drop it in restoreAdminRadioGlobals(), so a failed assertion's longjmp cannot leak a freed engine into later tests. * fix(pki): only warn about a swapped key when one was actually swapped keyIsLowEntropy is set from the stored public key at function entry, so a restore whose supplied public key is blacklisted set it even when keygen merely re-derived the public key from a private key that was kept. The warning then claimed a new key had been generated and the node number changed, which was only half true. Gate it on the private key actually being replaced. * fix(pki): re-check a freshly minted keypair against the blacklist Both mint sites called crypto->generateKeyPair() once and trusted the result, so an entropy source still producing known-weak keys could persist another blacklisted identity. Route both through a helper that re-checks and retries a bounded number of times, then logs if it cannot do better. Pass the caller's own copy of the private key to generateCryptoKeyPair() instead of config.security.private_key.bytes, which aliased the memcpy destination inside it. * fix(pki): fail keygen when every replacement stays blacklisted generateBlacklistCheckedKeyPair() logged an error after exhausting its retries but left the compromised keypair in place and its callers marked the keygen successful, persisting exactly the identity the check exists to reject. Return a flag, clear both key sizes on exhaustion, and abort both callers so the next keygen starts clean. Match the declaration guard to the definition's, and derive the expected mint count in the retry test from the configured one. * refactor(pki): drop the keygen retry loop, fail on the first weak mint Retrying cannot help: an entropy source that lands on one of the twelve blacklisted keys is broken, and a second call to it produces the same result. With real entropy the odds are ~2^-250, so the loop never runs twice in practice either. Check once and fail, which is the same guarantee in a third of the code. * fix(pki): check the derived key on the stored-private-key path too factory_reset_config keeps the private key and clears the public one, so the entry check sees no stored key, reports "not low entropy" and takes the regenerate branch, which adopted whatever it derived. A preserved pre-2.8 key was therefore accepted for a whole boot cycle before the next boot caught it - the same silent revert this PR exists to remove. Hoist the post-derive blacklist check into a helper and use it on both derive paths. * fix(pki): clear key sizes when stored-private derivation fails too The stored-private-key path set public_key.size to 32 up front and left it there when regeneratePublicKey() failed, so config claimed a pair the node never got - the same defect already fixed on the provided-key path. Both paths now derive through one helper that clears on failure and vets the derived key, replacing the separate blacklist-replace helper. --------- Signed-off-by: Garth Vander Houwen <garthvh@yahoo.com> Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>