Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/mesh/CryptoEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "NodeDB.h"
#include "aes-ccm.h"
#include "meshUtils.h"
#include "HardwareRNG.h"
#include <Crypto.h>
#include <Curve25519.h>
#include <RNG.h>
Expand All @@ -25,6 +26,15 @@ void CryptoEngine::generateKeyPair(uint8_t *pubKey, uint8_t *privKey)
{
// Mix in any randomness we can, to make key generation stronger.
CryptRNG.begin(optstr(APP_VERSION));

uint8_t hardwareEntropy[64] = {0};
if (HardwareRNG::fill(hardwareEntropy, sizeof(hardwareEntropy))) {
CryptRNG.stir(hardwareEntropy, sizeof(hardwareEntropy));
} else {
LOG_WARN("Hardware entropy unavailable, falling back to software RNG");
}
memset(hardwareEntropy, 0, sizeof(hardwareEntropy));

if (myNodeInfo.device_id.size == 16) {
CryptRNG.stir(myNodeInfo.device_id.bytes, myNodeInfo.device_id.size);
}
Expand Down
158 changes: 158 additions & 0 deletions src/mesh/HardwareRNG.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
#include "HardwareRNG.h"

#include <algorithm>
#include <cstring>
#include <random>

#include "configuration.h"

#if HAS_RADIO
#include "RadioLibInterface.h"
#endif

#if defined(ARCH_NRF52)
#include <Adafruit_nRFCrypto.h>
extern Adafruit_nRFCrypto nRFCrypto;
#elif defined(ARCH_ESP32)
#include <esp_system.h>
#elif defined(ARCH_RP2040)
#include <Arduino.h>
#elif defined(ARCH_PORTDUINO)
#include <random>
#include <sys/random.h>
#include <unistd.h>
#endif

namespace HardwareRNG
{

namespace
{
void fillWithRandomDevice(uint8_t *buffer, size_t length)
{
std::random_device rd;
size_t offset = 0;
while (offset < length) {
uint32_t value = rd();
size_t toCopy = std::min(length - offset, sizeof(value));
memcpy(buffer + offset, &value, toCopy);
offset += toCopy;
}
}

#if HAS_RADIO
bool mixWithLoRaEntropy(uint8_t *buffer, size_t length)
{
// Only attempt to pull entropy from the modem if it is initialized and exposes the helper.
// When the radio stack is disabled or has not yet been configured, we simply skip this step
// and return false so callers know no extra mixing occurred.
RadioLibInterface *radio = RadioLibInterface::instance;
if (!radio) {
return false;
}
Comment on lines +49 to +52
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 uses AI. Check for mistakes.

constexpr size_t chunkSize = 16;
uint8_t scratch[chunkSize];
size_t offset = 0;
bool mixed = false;

while (offset < length) {
size_t toCopy = std::min(length - offset, chunkSize);

// randomBytes() returns false if the modem does not support it or is not ready
// (for instance, when the radio is powered down). We break immediately to avoid
// blocking or returning partially-filled entropy and simply report failure.
if (!radio->randomBytes(scratch, toCopy)) {
break;
}

for (size_t i = 0; i < toCopy; ++i) {
buffer[offset + i] ^= scratch[i];
}

mixed = true;
offset += toCopy;
}

// Avoid leaving the modem-sourced bytes sitting on the stack longer than needed.
if (mixed) {
memset(scratch, 0, sizeof(scratch));
}

return mixed;
}
#endif
} // namespace

bool fill(uint8_t *buffer, size_t length)
{
if (!buffer || length == 0) {
return false;
}

bool filled = false;

#if defined(ARCH_NRF52)
// The Nordic SDK RNG provides cryptographic-quality randomness backed by hardware.
nRFCrypto.begin();
auto result = nRFCrypto.Random.generate(buffer, length);
nRFCrypto.end();
filled = result;
#elif defined(ARCH_ESP32)
// ESP32 exposes a true RNG via esp_fill_random().
esp_fill_random(buffer, length);
filled = true;
#elif defined(ARCH_RP2040)
// RP2040 has a hardware random number generator accessible through the Arduino core.
size_t offset = 0;
while (offset < length) {
uint32_t value = rp2040.hwrand32();
size_t toCopy = std::min(length - offset, sizeof(value));
memcpy(buffer + offset, &value, toCopy);
offset += toCopy;
}
filled = true;
#elif defined(ARCH_PORTDUINO)
// Prefer the host OS RNG first when running under Portduino.
ssize_t generated = ::getrandom(buffer, length, 0);
if (generated == static_cast<ssize_t>(length)) {
filled = true;
}

if (!filled) {
fillWithRandomDevice(buffer, length);
filled = true;
}
#else
fillWithRandomDevice(buffer, length);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

filled = true;
#endif

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;
}
Comment on lines +131 to +136
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
}

Copilot uses AI. Check for mistakes.

#if HAS_RADIO
// Best-effort: if the radio is active and can provide modem entropy, XOR it over the
// buffer to improve overall quality. We ignore failures to keep RNG usable even when
// radio hardware is powered down or uninitialized.
filled = mixWithLoRaEntropy(buffer, length) || filled;
#endif

return filled;
}

bool seed(uint32_t &seedOut)
{
uint32_t candidate = 0;
if (!fill(reinterpret_cast<uint8_t *>(&candidate), sizeof(candidate))) {
return false;
}
seedOut = candidate;
return true;
}

} // namespace HardwareRNG
27 changes: 27 additions & 0 deletions src/mesh/HardwareRNG.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#pragma once

#include <cstddef>
#include <cstdint>

namespace HardwareRNG
{

/**
* Fill the provided buffer with random bytes sourced from the most
* appropriate hardware-backed RNG available on the current platform.
*
* @param buffer Destination buffer for random bytes
* @param length Number of bytes to write
* @return true if the buffer was fully populated with entropy, false on failure
*/
bool fill(uint8_t *buffer, size_t length);

/**
* Populate a 32-bit seed value with hardware-backed randomness where possible.
*
* @param seedOut Destination for the generated seed value
* @return true if a seed was produced from a reliable entropy source
*/
bool seed(uint32_t &seedOut);

} // namespace HardwareRNG
20 changes: 19 additions & 1 deletion src/mesh/RadioLibInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,24 @@ bool RadioLibInterface::findInTxQueue(NodeNum from, PacketId id)
return txQueue.find(from, id);
}

bool RadioLibInterface::randomBytes(uint8_t *buffer, size_t length)
{
if (!buffer || length == 0 || !iface) {
return false;
}

// Older RadioLib versions only expose random(min, max), so fill the buffer byte-by-byte.
for (size_t i = 0; i < length; ++i) {
int32_t value = iface->random(0, 255);
if (value < 0) {
return false;
}
buffer[i] = static_cast<uint8_t>(value & 0xFF);
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 uses AI. Check for mistakes.
}

return true;
}

/** radio helper thread callback.
We never immediately transmit after any operation (either Rx or Tx). Instead we should wait a random multiple of
'slotTimes' (see definition in RadioInterface.h) taken from a contention window (CW) to lower the chance of collision.
Expand Down Expand Up @@ -563,4 +581,4 @@ bool RadioLibInterface::startSend(meshtastic_MeshPacket *txp)

return res == RADIOLIB_ERR_NONE;
}
}
}
6 changes: 6 additions & 0 deletions src/mesh/RadioLibInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,12 @@ class RadioLibInterface : public RadioInterface, protected concurrency::Notified
/** Attempt to find a packet in the TxQueue. Returns true if the packet was found. */
virtual bool findInTxQueue(NodeNum from, PacketId id) override;

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

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
*/
bool randomBytes(uint8_t *buffer, size_t length);

private:
/** if we have something waiting to send, start a short (random) timer so we can come check for collision before actually
* doing the transmit */
Expand Down
17 changes: 8 additions & 9 deletions src/platform/nrf52/main-nrf52.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <memory.h>
#include <stdio.h>
// #include <Adafruit_USBD_Device.h>
#include "HardwareRNG.h"
#include "NodeDB.h"
#include "PowerMon.h"
#include "error.h"
Expand Down Expand Up @@ -288,15 +289,13 @@ void nrf52Setup()
#endif

// Init random seed
union seedParts {
uint32_t seed32;
uint8_t seed8[4];
} seed;
nRFCrypto.begin();
nRFCrypto.Random.generate(seed.seed8, sizeof(seed.seed8));
LOG_DEBUG("Set random seed %u", seed.seed32);
randomSeed(seed.seed32);
nRFCrypto.end();
uint32_t seed = 0;
if (!HardwareRNG::seed(seed)) {
LOG_WARN("Hardware RNG seed unavailable, using PRNG fallback");
seed = random();
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
LOG_DEBUG("Set random seed %u", seed);
randomSeed(seed);

// Set up nrfx watchdog. Do not enable the watchdog yet (we do that
// the first time through the main loop), so that other threads can
Expand Down
9 changes: 7 additions & 2 deletions src/platform/portduino/PortduinoGlue.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "CryptoEngine.h"
#include "PortduinoGPIO.h"
#include "SPIChip.h"
#include "HardwareRNG.h"
#include "mesh/RF95Interface.h"
#include "sleep.h"
#include "target_specific.h"
Expand Down Expand Up @@ -225,7 +226,9 @@ void portduinoSetup()
std::cout << "Running in simulated mode." << std::endl;
portduino_config.MaxNodes = 200; // Default to 200 nodes
// Set the random seed equal to TCPPort to have a different seed per instance
randomSeed(TCPPort);
uint32_t seed = TCPPort;
HardwareRNG::seed(seed);
randomSeed(seed);
Comment on lines +229 to +231
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
return;
}

Expand Down Expand Up @@ -433,7 +436,9 @@ void portduinoSetup()
}
printf("MAC ADDRESS: %02X:%02X:%02X:%02X:%02X:%02X\n", dmac[0], dmac[1], dmac[2], dmac[3], dmac[4], dmac[5]);
// Rather important to set this, if not running simulated.
randomSeed(time(NULL));
uint32_t seed = static_cast<uint32_t>(time(NULL));
HardwareRNG::seed(seed);
Comment on lines +439 to +440
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
randomSeed(seed);

std::string defaultGpioChipName = gpioChipName + std::to_string(portduino_config.lora_default_gpiochip);
for (auto i : portduino_config.all_pins) {
Expand Down
9 changes: 5 additions & 4 deletions src/platform/rp2xx0/main-rp2xx0.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "HardwareRNG.h"
#include "configuration.h"
#include "hardware/xosc.h"
#include <hardware/clocks.h>
Expand Down Expand Up @@ -98,10 +99,10 @@ void getMacAddr(uint8_t *dmac)

void rp2040Setup()
{
/* Sets a random seed to make sure we get different random numbers on each boot.
Taken from CPU cycle counter and ROSC oscillator, so should be pretty random.
*/
randomSeed(rp2040.hwrand32());
/* Sets a random seed to make sure we get different random numbers on each boot. */
uint32_t seed = rp2040.hwrand32();
HardwareRNG::seed(seed);
Comment on lines +103 to +104
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
randomSeed(seed);

#ifdef RP2040_SLOW_CLOCK
uint f_pll_sys = frequency_count_khz(CLOCKS_FC0_SRC_VALUE_PLL_SYS_CLKSRC_PRIMARY);
Expand Down
Loading