From 51019b14f45f4f215461cd3de77268c746aa087c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Tue, 21 Jul 2026 01:36:06 +0200 Subject: [PATCH] Hold cryptLock around RNG use in admin and key verification (#11096) setPassKey drew the session passkey outside cryptLock, unlike the signing and key verification paths. Also replaces random() for the key verification nonce with the hardware RNG. --- src/modules/AdminModule.cpp | 11 +++++++++-- src/modules/KeyVerificationModule.cpp | 9 ++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index dadabc491..d3389ad13 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -1,5 +1,6 @@ #include "AdminModule.h" #include "Channels.h" +#include "CryptoEngine.h" #include "DisplayFormatters.h" #include "HardwareRNG.h" #include "MeshService.h" @@ -1849,8 +1850,14 @@ void AdminModule::setPassKey(meshtastic_AdminMessage *res) if (!sessionPasskeyValid || !Throttle::isWithinTimespanMs(session_time, 150 * 1000UL)) { // Session passkey authenticates admin replies, so it must be unpredictable: prefer the // hardware RNG, falling back to the seeded CSPRNG only when no hardware source exists. - if (!HardwareRNG::fill(session_passkey, sizeof(session_passkey))) - CryptRNG.rand(session_passkey, sizeof(session_passkey)); + // Hold cryptLock like the signing path does: this runs on the admin receive path, which on + // nRF52 is the BLE task, and the fill toggles the CC310 that packet crypto also uses while + // the CryptRNG state is shared with signing. + { + concurrency::LockGuard g(cryptLock); + if (!HardwareRNG::fill(session_passkey, sizeof(session_passkey))) + CryptRNG.rand(session_passkey, sizeof(session_passkey)); + } session_time = millis(); sessionPasskeyValid = true; } diff --git a/src/modules/KeyVerificationModule.cpp b/src/modules/KeyVerificationModule.cpp index c275ce1d9..ca57bf816 100644 --- a/src/modules/KeyVerificationModule.cpp +++ b/src/modules/KeyVerificationModule.cpp @@ -151,7 +151,14 @@ bool KeyVerificationModule::sendInitialRequest(NodeNum remoteNode) return false; } updateState(true); - currentNonce = random(); + // The nonce binds the handshake, so draw it from the hardware RNG (falling back to the CSPRNG) + // under cryptLock, as allocReply does for the security number. random() is both predictable and + // only 32 bits wide, leaving half of this nonce zero. + { + concurrency::LockGuard g(cryptLock); + if (!HardwareRNG::fill((uint8_t *)¤tNonce, sizeof(currentNonce))) + CryptRNG.rand((uint8_t *)¤tNonce, sizeof(currentNonce)); + } currentNonceTimestamp = getTime(); currentRemoteNode = remoteNode; meshtastic_KeyVerification KeyVerification = meshtastic_KeyVerification_init_zero;