From 799d6949148b60abf4406fb3034bfc65b8e81e5e Mon Sep 17 00:00:00 2001 From: liquidraver <504870+liquidraver@users.noreply.github.com> Date: Fri, 29 May 2026 07:58:54 +0200 Subject: [PATCH] crypto: simplify entropy path after audit review - Lift duplicated identity-gen block from main_companion.cpp + main_repeater.cpp into ZephyrRNG::generateFirstBootIdentity(). Both mains shrink from ~40 lines to a 3-line helper call. - Add LocalIdentity::fromSeed() so seed-derived keygen doesn't need a one-shot RNG wrapper; delete SeededRNG. - Drop the per-byte ADC sampling loop: getBattMilliVolts() does an 8-sample average + 10ms regulator settle internally, costing 300-480ms of real wall-time and actively destroying the LSB jitter it was meant to harvest. Jitter mixer already dwarfs it. - Centralize the printk + sys_reboot pattern as Utils::cryptoPanicReboot(); drop the 2000ms pre-reboot k_msleep (printk is synchronous, sleep just blocked the mesh thread on the ZephyrRNG::random() retry-failure path). - Inline sample_cpu_jitter health check via online scalars instead of a 512-byte deltas[] array. Saves 1.5KB stack churn across boot and tracks every sample instead of only the first 128. - extract_via_aes_ctr now uses Utils::sha256 instead of open-coding psa_hash_compute. --- zephcore/adapters/rng/ZephyrRNG.cpp | 121 +++++++++++++++------------- zephcore/adapters/rng/ZephyrRNG.h | 41 ++++------ zephcore/include/mesh/Identity.h | 7 ++ zephcore/include/mesh/Utils.h | 7 ++ zephcore/src/Identity.cpp | 6 ++ zephcore/src/Utils.cpp | 13 +++ zephcore/src/main_companion.cpp | 46 +---------- zephcore/src/main_repeater.cpp | 48 +---------- 8 files changed, 119 insertions(+), 170 deletions(-) diff --git a/zephcore/adapters/rng/ZephyrRNG.cpp b/zephcore/adapters/rng/ZephyrRNG.cpp index f95851a..81fc19d 100644 --- a/zephcore/adapters/rng/ZephyrRNG.cpp +++ b/zephcore/adapters/rng/ZephyrRNG.cpp @@ -27,12 +27,10 @@ void ZephyrRNG::random(uint8_t *dest, size_t sz) if (sys_csrand_get(dest, sz) == 0) return; k_msleep(10); } - printk("ZephyrRNG: CSPRNG unavailable after retries — rebooting\n"); - k_msleep(2000); - sys_reboot(SYS_REBOOT_COLD); + Utils::cryptoPanicReboot("CSPRNG unavailable after retries"); } -/* ===== Jitter sampling + health check ===================================== +/* ===== Jitter sampling + online health check ============================= * * Stephan Müller "CPU Time Jitter Based Non-Physical True Random Number * Generator" — Linux kernel's jitterentropy_rng. NIST SP 800-90B has @@ -44,55 +42,28 @@ void ZephyrRNG::random(uint8_t *dest, size_t sz) * × 0.1 bits = 3,200 estimated bits. 256 needed for Ed25519 → 12× * margin even pessimistically. * - * Health check is the NIST SP 800-90B "repetition count" plus a - * variance check on the first 128 samples. Detects stuck-source + * Health check (NIST SP 800-90B style): online repetition count + a + * distinct-value check tracked across all samples in the window with + * scalar state — no per-sample buffer needed. Detects stuck-source * catastrophic failure (e.g. cycle counter not advancing). Does not * statistically prove entropy quality — that's what the literature is * for. */ -#define JITTER_TRACKED 128 - -static bool jitter_health_check(const uint32_t *deltas, size_t n) -{ - if (n < 16) return false; - - /* Repetition count: 32+ consecutive identical samples is failure. */ - int consec = 1, max_consec = 1; - for (size_t i = 1; i < n; i++) { - if (deltas[i] == deltas[i - 1]) { - consec++; - if (consec > max_consec) max_consec = consec; - } else { - consec = 1; - } - } - if (max_consec >= 32) return false; - - /* Variance: require at least 5 distinct delta values across the - * tracked window. A perfectly deterministic CPU would produce one - * value; even minimal jitter produces several. */ - uint32_t distinct[8] = {0}; - int n_distinct = 0; - for (size_t i = 0; i < n && n_distinct < 8; i++) { - bool found = false; - for (int j = 0; j < n_distinct; j++) { - if (distinct[j] == deltas[i]) { found = true; break; } - } - if (!found) distinct[n_distinct++] = deltas[i]; - } - return n_distinct >= 5; -} - static bool sample_cpu_jitter(uint8_t *pool, size_t pool_size, size_t pool_offset, uint32_t duration_ms) { - uint32_t deltas[JITTER_TRACKED]; - size_t tracked_idx = 0; - uint32_t accum = k_cycle_get_32(); int64_t deadline = k_uptime_get() + duration_ms; size_t idx = pool_offset; + /* Online health stats: 32 bytes total vs. the previous 512-byte + * deltas[] array. Tracks every sample, not just the first 128. */ + uint32_t prev_delta = 0; + int cur_consec = 0, max_consec = 0; + uint32_t distinct[8] = {0}; + int n_distinct = 0; + int n_samples = 0; + while (k_uptime_get() < deadline) { uint32_t t1 = k_cycle_get_32(); /* Variable-time work — number of iterations depends on the @@ -113,10 +84,29 @@ static bool sample_cpu_jitter(uint8_t *pool, size_t pool_size, pool[idx++ % pool_size] ^= (uint8_t)accum; pool[idx++ % pool_size] ^= (uint8_t)(accum >> 8); - if (tracked_idx < JITTER_TRACKED) deltas[tracked_idx++] = delta; + /* Online repetition count */ + if (n_samples > 0 && delta == prev_delta) { + if (++cur_consec > max_consec) max_consec = cur_consec; + } else { + cur_consec = 1; + } + prev_delta = delta; + + /* Track first 8 distinct delta values */ + if (n_distinct < 8) { + bool found = false; + for (int j = 0; j < n_distinct; j++) { + if (distinct[j] == delta) { found = true; break; } + } + if (!found) distinct[n_distinct++] = delta; + } + + n_samples++; } - return jitter_health_check(deltas, tracked_idx); + if (n_samples < 16) return false; + if (max_consec >= 32) return false; /* stuck source */ + return n_distinct >= 5; /* minimal variance */ } /* ===== Entropy extraction via AES-256-CTR ================================ @@ -142,18 +132,14 @@ static int extract_via_aes_ctr(const uint8_t *pool, size_t pool_len, { psa_status_t status; uint8_t key[32]; - size_t key_len = 0; /* PSA is idempotent — already initialized via mbedTLS but a defensive * call here costs nothing if it returns PSA_ERROR_ALREADY_EXISTS. */ (void)psa_crypto_init(); - /* Extract: SHA-256(pool) → AES key */ - status = psa_hash_compute(PSA_ALG_SHA_256, pool, pool_len, - key, sizeof(key), &key_len); - if (status != PSA_SUCCESS || key_len != sizeof(key)) { - return -1; - } + /* Extract: SHA-256(pool) → AES key. Reuses the codebase's PSA-backed + * SHA-256 wrapper instead of open-coding psa_hash_compute here. */ + Utils::sha256(key, sizeof(key), pool, (int)pool_len); /* Import key for AES-256-ECB */ psa_key_attributes_t attr = PSA_KEY_ATTRIBUTES_INIT; @@ -246,22 +232,18 @@ void ZephyrRNG::mixIdentitySeed(uint8_t *out, size_t out_len, * AES-ECB on a 128-bit counter. Per crypto consultant guidance — * see extract_via_aes_ctr() for full rationale. */ if (extract_via_aes_ctr(pool, sizeof(pool), out, out_len) != 0) { - printk("ZephyrRNG: AES-CTR extraction failed — rebooting\n"); - k_msleep(2000); - sys_reboot(SYS_REBOOT_COLD); + Utils::cryptoPanicReboot("AES-CTR seed extraction failed"); } /* Output sanity check — reject all-zero / all-0xFF (catastrophic - * failure of every source). Reboot to retry. */ + * failure of every source). */ bool all_zero = true, all_ff = true; for (size_t i = 0; i < out_len; i++) { if (out[i] != 0x00) all_zero = false; if (out[i] != 0xFF) all_ff = false; } if (all_zero || all_ff) { - printk("ZephyrRNG: degenerate seed output — rebooting\n"); - k_msleep(2000); - sys_reboot(SYS_REBOOT_COLD); + Utils::cryptoPanicReboot("degenerate seed output (all-zero / all-FF)"); } /* Wipe sensitive intermediate buffers — secureZeroize survives the @@ -271,4 +253,27 @@ void ZephyrRNG::mixIdentitySeed(uint8_t *out, size_t out_len, Utils::secureZeroize(devid, sizeof(devid)); } +void ZephyrRNG::generateFirstBootIdentity(LocalIdentity &out_identity) +{ + uint8_t seed[32]; + + mixIdentitySeed(seed, sizeof(seed)); + out_identity.fromSeed(seed); + + /* Reserved-prefix guard — MeshCore protocol treats pub_key[0] of + * 0x00/0xFF as reserved markers. With a working CSPRNG the first + * attempt almost always passes (P(reserved) = 2/256); the cap + + * panic-reboot is a stuck-source backstop. */ + int attempt = 0; + while (out_identity.pub_key[0] == 0x00 || out_identity.pub_key[0] == 0xFF) { + if (++attempt > 100) { + Utils::cryptoPanicReboot("identity gen stuck on reserved prefix"); + } + mixIdentitySeed(seed, sizeof(seed)); + out_identity.fromSeed(seed); + } + + Utils::secureZeroize(seed, sizeof(seed)); +} + } /* namespace mesh */ diff --git a/zephcore/adapters/rng/ZephyrRNG.h b/zephcore/adapters/rng/ZephyrRNG.h index 9e84f92..cb9b20e 100644 --- a/zephcore/adapters/rng/ZephyrRNG.h +++ b/zephcore/adapters/rng/ZephyrRNG.h @@ -6,8 +6,8 @@ #pragma once #include +#include #include -#include namespace mesh { @@ -19,42 +19,31 @@ public: * generation. Combines: * 1. sys_csrand_get (early) — CSPRNG, strong on nRF/MG24 * 2. HWINFO unique device ID — per-device uniqueness - * 3. Optional caller-supplied data — e.g. ADC LSB noise samples + * 3. Optional caller-supplied data — e.g. external noise samples * 4. CPU cycle-counter jitter — main entropy source * (NIST SP 800-90B class) * 5. sys_csrand_get (late) — catches mid-boot radio init * 6. CPU cycle-counter jitter #2 — independent timing window - * Conditioned via SHA-512. NIST-style health checks on jitter - * samples; reboots on degenerate output. - * Blocks for ~250-300ms — only called at first-boot identity gen. + * Conditioned via AES-256-CTR (SHA-256(pool) → key, ECB on counter). + * NIST-style health checks on jitter samples; reboots on degenerate + * output. Blocks for ~250ms — only called at first-boot identity gen. * * Output is suitable as an Ed25519 seed regardless of platform * TRNG state at boot. */ static void mixIdentitySeed(uint8_t *out, size_t out_len, const uint8_t *extra = nullptr, size_t extra_len = 0); -}; -/* Thin wrapper that returns pre-supplied bytes from the mesh::RNG - * interface. Used to feed a mixed seed into mesh::LocalIdentity(RNG*) - * without changing the LocalIdentity API. One-shot — bytes are - * consumed sequentially across calls. */ -class SeededRNG : public RNG { -public: - SeededRNG(const uint8_t *seed_data, size_t len) - : _data(seed_data), _avail(len) {} - - void random(uint8_t *dest, size_t sz) override { - size_t n = (sz <= _avail) ? sz : _avail; - if (n > 0) memcpy(dest, _data, n); - _data += n; - _avail -= n; - if (n < sz) memset(dest + n, 0, sz - n); - } - -private: - const uint8_t *_data; - size_t _avail; + /* End-to-end first-boot identity generation. Mixes a fresh seed, + * derives the Ed25519 keypair, and retries (up to 100×) if the + * MeshCore protocol-reserved 0x00/0xFF public-key prefix happens + * to land. Panics-and-reboots on cap exhaustion (essentially + * impossible with a working CSPRNG: P(100 reserved in a row) ≈ 10⁻²¹¹). + * Wipes the intermediate seed before return. + * + * Use this from main()'s `loadIdentity` fall-through path instead + * of open-coding the mix+derive+retry+zeroize sequence. */ + static void generateFirstBootIdentity(LocalIdentity &out_identity); }; } /* namespace mesh */ diff --git a/zephcore/include/mesh/Identity.h b/zephcore/include/mesh/Identity.h index ee6f181..0270260 100644 --- a/zephcore/include/mesh/Identity.h +++ b/zephcore/include/mesh/Identity.h @@ -49,6 +49,13 @@ public: LocalIdentity(const char *prv_hex, const char *pub_hex); LocalIdentity(RNG *rng); + /* Derive Ed25519 keypair from a 32-byte seed. Use this when you've + * already produced a high-quality seed externally (e.g. via the + * layered ZephyrRNG::mixIdentitySeed entropy mixer) — avoids the + * one-shot-RNG-wrapper dance otherwise needed to feed bytes + * through the LocalIdentity(RNG*) constructor. */ + void fromSeed(const uint8_t seed[SEED_SIZE]); + void sign(uint8_t *sig, const uint8_t *message, int msg_len) const; void calcSharedSecret(uint8_t *secret, const Identity &other) const { calcSharedSecret(secret, other.pub_key); } void calcSharedSecret(uint8_t *secret, const uint8_t *other_pub_key) const; diff --git a/zephcore/include/mesh/Utils.h b/zephcore/include/mesh/Utils.h index c4e50af..bd37fa3 100644 --- a/zephcore/include/mesh/Utils.h +++ b/zephcore/include/mesh/Utils.h @@ -34,6 +34,13 @@ public: * secrets after their last use. */ static void secureZeroize(void *buf, size_t n); + /* Log a crypto-invariant failure to printk and cold-reboot. + * Used when an entropy source, KDF, or other primitive cannot + * produce a safe result — proceeding would risk weak keys or + * bypassed authentication, so we restart rather than continue. + * Does not return. */ + [[noreturn]] static void cryptoPanicReboot(const char *msg); + static void toHex(char *dest, const uint8_t *src, size_t len); static bool fromHex(uint8_t *dest, int dest_size, const char *src_hex); static bool isHexChar(char c); diff --git a/zephcore/src/Identity.cpp b/zephcore/src/Identity.cpp index 4156413..306b6f5 100644 --- a/zephcore/src/Identity.cpp +++ b/zephcore/src/Identity.cpp @@ -54,6 +54,12 @@ LocalIdentity::LocalIdentity(RNG *rng) uint8_t seed[SEED_SIZE]; rng->random(seed, SEED_SIZE); ed25519_create_keypair(pub_key, prv_key, seed); + Utils::secureZeroize(seed, sizeof(seed)); +} + +void LocalIdentity::fromSeed(const uint8_t seed[SEED_SIZE]) +{ + ed25519_create_keypair(pub_key, prv_key, seed); } bool LocalIdentity::validatePrivateKey(const uint8_t prv[64]) diff --git a/zephcore/src/Utils.cpp b/zephcore/src/Utils.cpp index 6b7c8bc..a85144b 100644 --- a/zephcore/src/Utils.cpp +++ b/zephcore/src/Utils.cpp @@ -6,6 +6,8 @@ #include #include #include +#include +#include #include LOG_MODULE_REGISTER(zephcore_utils, CONFIG_ZEPHCORE_MAIN_LOG_LEVEL); @@ -205,6 +207,17 @@ bool Utils::constantTimeEqual(const void *a, const void *b, size_t n) return result == 0; } +void Utils::cryptoPanicReboot(const char *msg) +{ + /* No pre-reboot k_msleep: printk is synchronous on RTT/UART so the + * line is already on the wire by the time sys_reboot fires, and the + * 2-second delay we used to do here just blocked the mesh thread on + * the rare-but-realistic ZephyrRNG::random() retry failure path. */ + printk("crypto panic: %s — rebooting\n", msg ? msg : "(no detail)"); + sys_reboot(SYS_REBOOT_COLD); + for (;;) { /* sys_reboot is FUNC_NORETURN, but satisfy [[noreturn]] */ } +} + void Utils::secureZeroize(void *buf, size_t n) { /* Volatile pointer prevents the compiler from eliminating the diff --git a/zephcore/src/main_companion.cpp b/zephcore/src/main_companion.cpp index e98d9b2..201d72f 100644 --- a/zephcore/src/main_companion.cpp +++ b/zephcore/src/main_companion.cpp @@ -660,51 +660,13 @@ int main(void) LOG_INF("Added default Public channel"); } - /* Load or generate identity. - * - * First-boot keygen uses ZephyrRNG::mixIdentitySeed() — a layered - * entropy mixer combining sys_csrand_get + HWINFO unique ID + ADC - * LSB noise + CPU cycle-counter jitter, conditioned via SHA-512. - * This compensates for ESP32's hardware TRNG being only seeded - * after WiFi/BT radio init (identity gen happens before bt_enable). */ + /* Load or generate identity. First-boot keygen runs the layered + * entropy mixer + Ed25519 derive + reserved-prefix guard inside + * ZephyrRNG::generateFirstBootIdentity. */ mesh::LocalIdentity self_identity; if (!data_store.loadMainIdentity(self_identity)) { - /* Sample ADC LSB noise — best-effort independent physical - * source. Boards without battery ADC return 0; jitter remains - * the primary entropy source either way. */ - uint8_t adc_noise[32] = {0}; - for (size_t i = 0; i < sizeof(adc_noise); i++) { - adc_noise[i] = (uint8_t)zephyr_board.getBattMilliVolts(); - k_msleep(1); - } - - uint8_t seed[32]; - mesh::ZephyrRNG::mixIdentitySeed(seed, sizeof(seed), - adc_noise, sizeof(adc_noise)); - { - mesh::SeededRNG seed_rng(seed, sizeof(seed)); - self_identity = mesh::LocalIdentity(&seed_rng); - } - - /* Ensure pub_key[0] is not reserved (0x00 or 0xFF in MeshCore protocol). - * With a properly mixed seed this almost never triggers; the - * cap+reboot is a safety net against pathological entropy failure. */ - int attempt = 0; - while (self_identity.pub_key[0] == 0x00 || self_identity.pub_key[0] == 0xFF) { - if (++attempt > 100) { - LOG_ERR("Identity gen stuck on reserved prefix; rebooting"); - k_msleep(2000); - sys_reboot(SYS_REBOOT_COLD); - } - mesh::ZephyrRNG::mixIdentitySeed(seed, sizeof(seed)); - mesh::SeededRNG retry_rng(seed, sizeof(seed)); - self_identity = mesh::LocalIdentity(&retry_rng); - } - + mesh::ZephyrRNG::generateFirstBootIdentity(self_identity); data_store.saveMainIdentity(self_identity); - - mesh::Utils::secureZeroize(seed, sizeof(seed)); - mesh::Utils::secureZeroize(adc_noise, sizeof(adc_noise)); } companion_mesh.self_id = self_identity; diff --git a/zephcore/src/main_repeater.cpp b/zephcore/src/main_repeater.cpp index 2e5f716..2478240 100644 --- a/zephcore/src/main_repeater.cpp +++ b/zephcore/src/main_repeater.cpp @@ -454,55 +454,15 @@ int main(void) lora_radio.setTxDoneCallback(lora_tx_done_callback, nullptr); repeater_mesh.setTxQueuedCallback(tx_queued_callback, nullptr); - /* Load or generate identity BEFORE begin(). - * - * First-boot keygen uses ZephyrRNG::mixIdentitySeed() — a layered - * entropy mixer combining sys_csrand_get + HWINFO unique ID + ADC - * LSB noise + CPU cycle-counter jitter, conditioned via SHA-512. - * This compensates for ESP32's hardware TRNG being only seeded - * after WiFi/BT radio init (which on a repeater is on-demand for - * WiFi OTA — there's no guaranteed radio activity at boot). */ + /* Load or generate identity BEFORE begin(). First-boot keygen runs + * the layered entropy mixer + Ed25519 derive + reserved-prefix + * guard inside ZephyrRNG::generateFirstBootIdentity. */ mesh::LocalIdentity self_identity; if (!data_store.loadIdentity(self_identity)) { LOG_INF("No identity found, generating new keypair..."); - - /* Sample ADC LSB noise — best-effort independent physical - * source. Boards without battery ADC return 0; jitter remains - * the primary entropy source either way. */ - uint8_t adc_noise[32] = {0}; - for (size_t i = 0; i < sizeof(adc_noise); i++) { - adc_noise[i] = (uint8_t)zephyr_board.getBattMilliVolts(); - k_msleep(1); - } - - uint8_t seed[32]; - mesh::ZephyrRNG::mixIdentitySeed(seed, sizeof(seed), - adc_noise, sizeof(adc_noise)); - { - mesh::SeededRNG seed_rng(seed, sizeof(seed)); - self_identity = mesh::LocalIdentity(&seed_rng); - } - - /* Ensure pub_key[0] is not reserved (0x00 or 0xFF in MeshCore protocol). - * With a properly mixed seed this almost never triggers; the - * cap+reboot is a safety net against pathological entropy failure. */ - int attempt = 0; - while (self_identity.pub_key[0] == 0x00 || self_identity.pub_key[0] == 0xFF) { - if (++attempt > 100) { - LOG_ERR("Identity gen stuck on reserved prefix; rebooting"); - k_msleep(2000); - sys_reboot(SYS_REBOOT_COLD); - } - mesh::ZephyrRNG::mixIdentitySeed(seed, sizeof(seed)); - mesh::SeededRNG retry_rng(seed, sizeof(seed)); - self_identity = mesh::LocalIdentity(&retry_rng); - } - + mesh::ZephyrRNG::generateFirstBootIdentity(self_identity); data_store.saveIdentity(self_identity); LOG_INF("New identity saved"); - - mesh::Utils::secureZeroize(seed, sizeof(seed)); - mesh::Utils::secureZeroize(adc_noise, sizeof(adc_noise)); } repeater_mesh.self_id = self_identity;