Reduce key duplication by enabling hardware RNG #8803

Komzpa · 2025-11-29T19:00:48Z

Summary

add portable HardwareRNG helper that taps NRF52 crypto RNG, ESP32/TRNG, RP2040 hwrand, Portduino host RNG, and modem-provided entropy as available
stir hardware bytes into CryptRNG during key generation to avoid repeated keys on NRF devices
route NRF52, Portduino, and RP2040 platform setup through the new helper so runtime random seeds also benefit
expose RadioLibInterface::randomBytes so modem entropy can be mixed in when radios are initialized

🤝 Attestations

I have tested that my proposed changes behave as described.
I have tested that my proposed changes do not cause any obvious regressions on the following devices:
- Heltec (Lora32) V3
- LilyGo T-Deck
- LilyGo T-Beam
- RAK WisBlock 4631
- Seeed Studio T-1000E tracker card
- Other (please specify below)

Copilot

Pull request overview

This PR introduces a portable HardwareRNG helper to improve random number generation across platforms by tapping into hardware RNG sources (NRF52 crypto RNG, ESP32 TRNG, RP2040 hwrand, Portduino host RNG) and optionally mixing in modem-provided entropy. The goal is to reduce key duplication issues on NRF devices by stirring hardware bytes into CryptRNG during key generation.

Key changes:

Added HardwareRNG namespace with fill() and seed() functions for platform-agnostic hardware entropy access
Integrated hardware entropy into CryptoEngine::generateKeyPair() to strengthen key generation
Exposed RadioLibInterface::randomBytes() to allow modem entropy mixing when radios are initialized
Updated NRF52, RP2040, and Portduino platform initialization to use the new HardwareRNG::seed() helper

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`src/mesh/HardwareRNG.h`	New header defining the HardwareRNG interface with `fill()` and `seed()` functions
`src/mesh/HardwareRNG.cpp`	Implementation providing platform-specific hardware RNG access with optional radio entropy mixing
`src/mesh/RadioLibInterface.h`	Added public `randomBytes()` method declaration for exposing modem entropy
`src/mesh/RadioLibInterface.cpp`	Implemented `randomBytes()` to fetch entropy from LoRa modem via RadioLib API
`src/mesh/CryptoEngine.cpp`	Integrated HardwareRNG to stir 64 bytes of hardware entropy into CryptRNG during key generation
`src/platform/nrf52/main-nrf52.cpp`	Replaced direct nRFCrypto RNG calls with `HardwareRNG::seed()`
`src/platform/rp2xx0/main-rp2xx0.cpp`	Added `HardwareRNG::seed()` call alongside existing hwrand32 usage
`src/platform/portduino/PortduinoGlue.cpp`	Added `HardwareRNG::seed()` calls in both simulated and native modes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-29T21:43:45Z

src/mesh/RadioLibInterface.h


+    /**
+     * Request randomness sourced from the LoRa modem, if supported by the active RadioLib interface.
+     * @return true if len bytes were produced, false otherwise.


The documentation comment has a typo: "@return true if len bytes were produced" should be "@return true if length bytes were produced" to match the actual parameter name.

Suggested change

* @return true if len bytes were produced, false otherwise.

* @return true if length bytes were produced, false otherwise.

Copilot · 2025-11-29T21:43:46Z

src/platform/rp2xx0/main-rp2xx0.cpp

+    uint32_t seed = rp2040.hwrand32();
+    HardwareRNG::seed(seed);


The HardwareRNG::seed() function signature is bool seed(uint32_t &seedOut), which expects a reference to populate with a generated seed. However, this code is passing an already-generated seed value as an argument. This is a mismatch in usage - the function is designed to produce a seed, not consume one.

The correct usage should be:

uint32_t seed = 0; if (!HardwareRNG::seed(seed)) { seed = rp2040.hwrand32(); // fallback } randomSeed(seed);

This way, HardwareRNG::seed() can populate the seed variable with hardware entropy (including potential radio mixing), and if it fails, you fall back to just the RP2040 hardware RNG.

Suggested change

uint32_t seed = rp2040.hwrand32();

HardwareRNG::seed(seed);

uint32_t seed = 0;

if (!HardwareRNG::seed(seed)) {

seed = rp2040.hwrand32();

}

Copilot · 2025-11-29T21:43:46Z

src/platform/portduino/PortduinoGlue.cpp

+        uint32_t seed = TCPPort;
+        HardwareRNG::seed(seed);
+        randomSeed(seed);


Similar to the RP2040 issue, the HardwareRNG::seed() function signature is bool seed(uint32_t &seedOut), which expects a reference to populate with a generated seed. However, this code is passing an already-set seed value (TCPPort).

The correct usage should be:

uint32_t seed = 0; if (!HardwareRNG::seed(seed)) { seed = TCPPort; // fallback } randomSeed(seed);

This allows HardwareRNG::seed() to populate the seed with hardware entropy first, falling back to TCPPort only if hardware entropy is unavailable.

Copilot · 2025-11-29T21:43:47Z

src/platform/portduino/PortduinoGlue.cpp

+    uint32_t seed = static_cast<uint32_t>(time(NULL));
+    HardwareRNG::seed(seed);


Similar to the simulated mode case, the HardwareRNG::seed() function signature expects a reference to populate with a generated seed, not consume an already-set value.

The correct usage should be:

uint32_t seed = 0; if (!HardwareRNG::seed(seed)) { seed = static_cast<uint32_t>(time(NULL)); // fallback } randomSeed(seed);

This allows HardwareRNG::seed() to generate and populate the seed with hardware entropy first, falling back to time(NULL) only if hardware entropy is unavailable.

Suggested change

uint32_t seed = static_cast<uint32_t>(time(NULL));

HardwareRNG::seed(seed);

uint32_t seed = 0;

if (!HardwareRNG::seed(seed)) {

seed = static_cast<uint32_t>(time(NULL)); // fallback

}

Copilot · 2025-11-29T21:43:47Z

src/mesh/RadioLibInterface.cpp

+        if (value < 0) {
+            return false;
+        }
+        buffer[i] = static_cast<uint8_t>(value & 0xFF);


[nitpick] The bitwise AND operation value & 0xFF is redundant when casting to uint8_t. The cast to uint8_t already truncates the value to 8 bits, making the mask unnecessary.

Consider simplifying to: buffer[i] = static_cast<uint8_t>(value);

Suggested change

buffer[i] = static_cast<uint8_t>(value & 0xFF);

buffer[i] = static_cast<uint8_t>(value);

Copilot · 2025-11-29T21:43:47Z

src/mesh/HardwareRNG.cpp

+    RadioLibInterface *radio = RadioLibInterface::instance;
+    if (!radio) {
+        return false;
+    }


[nitpick] Accessing the static RadioLibInterface::instance pointer without synchronization could lead to race conditions if HardwareRNG::fill() is called from multiple threads simultaneously, or if it's called while the radio interface is being initialized or destroyed.

While the current code checks for null and handles the case gracefully, consider documenting the thread-safety expectations of this function, or adding appropriate synchronization if fill() can be called from multiple threads.

Copilot · 2025-11-29T21:43:47Z

src/platform/nrf52/main-nrf52.cpp

+    uint32_t seed = 0;
+    if (!HardwareRNG::seed(seed)) {
+        LOG_WARN("Hardware RNG seed unavailable, using PRNG fallback");
+        seed = random();


The seed variable is initialized to 0, but when HardwareRNG::seed(seed) returns false, the fallback uses random() to generate a new seed but assigns it to a local variable seed. However, at this point, randomSeed() has not yet been called, so random() will use an uninitialized or default seed state, potentially producing the same value across reboots.

Consider calling random() with a hardware-derived value first, or using a different fallback mechanism such as reading from an uninitialized memory location or system timer.

Suggested change

seed = random();

// Use a hardware timer value as a fallback seed for better entropy

seed = micros();

This actually seems like good feedback. We might consider an XOR of the deviceID, MAC address, and micros() as the fallback seed.

Copilot · 2025-11-29T21:43:48Z

src/mesh/HardwareRNG.cpp

+    if (!filled) {
+        // As a last resort, fall back to std::random_device. This should only be reached
+        // if a platform-specific source was unavailable.
+        fillWithRandomDevice(buffer, length);
+        filled = true;
+    }


The fallback logic at lines 131-136 is unreachable code. All platform-specific branches (lines 95-129) set filled = true, so the condition if (!filled) at line 131 can never be true. This fallback code will never execute.

Consider removing this dead code block, or restructuring the logic if there's an intention to handle edge cases where platform-specific sources might fail.

Suggested change

if (!filled) {

// As a last resort, fall back to std::random_device. This should only be reached

// if a platform-specific source was unavailable.

fillWithRandomDevice(buffer, length);

filled = true;

}

jp-bennett · 2025-11-30T01:51:05Z

I particularly like that this is just stirring in another potential randomness source. Will go through it in more detail as I am able.

jp-bennett · 2025-11-30T03:21:47Z

src/platform/nrf52/main-nrf52.cpp

+    uint32_t seed = 0;
+    if (!HardwareRNG::seed(seed)) {
+        LOG_WARN("Hardware RNG seed unavailable, using PRNG fallback");
+        seed = random();


This actually seems like good feedback. We might consider an XOR of the deviceID, MAC address, and micros() as the fallback seed.

jp-bennett · 2025-11-30T03:36:25Z

src/mesh/HardwareRNG.cpp

+        filled = true;
+    }
+#else
+    fillWithRandomDevice(buffer, length);


How good/bad of a source of entropy is std::random_device on various platforms? STM32 will fall over into this code, for instance. I think we want to just return false here instead.

Reduce key duplication by enabling hardware RNG

8524abf

Komzpa force-pushed the nrf-rng branch from 0a402c7 to 8524abf Compare November 29, 2025 19:57

thebentern requested review from caveman99, Copilot and jp-bennett November 29, 2025 21:40

Copilot started reviewing on behalf of thebentern November 29, 2025 21:40 View session

Copilot finished reviewing on behalf of thebentern November 29, 2025 21:42

Copilot AI reviewed Nov 29, 2025

View reviewed changes

jp-bennett added the enhancement New feature or request label Nov 30, 2025

jp-bennett reviewed Nov 30, 2025

View reviewed changes

	* @return true if len bytes were produced, false otherwise.
	* @return true if length bytes were produced, false otherwise.

		uint32_t seed = static_cast<uint32_t>(time(NULL));
		HardwareRNG::seed(seed);

	buffer[i] = static_cast<uint8_t>(value & 0xFF);
	buffer[i] = static_cast<uint8_t>(value);

	seed = random();
	// Use a hardware timer value as a fallback seed for better entropy
	seed = micros();

Uh oh!

Reduce key duplication by enabling hardware RNG #8803

Are you sure you want to change the base?

Reduce key duplication by enabling hardware RNG #8803

Conversation

Komzpa commented Nov 29, 2025

Summary

🤝 Attestations

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

jp-bennett Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

jp-bennett commented Nov 30, 2025

Uh oh!

jp-bennett Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

jp-bennett Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants