refactor(crypto): get rid of random_xor() function
What changed, and why it matters
This commit removes a helper function called random_xor() and replaces its use in the hardware wallet's PIN secret generation with a single, stronger random-number call. There is no direct evidence in the commit that this fixes an active security vulnerability; it reads as a code cleanup that simplifies how random bytes are produced before they are stored on the secure Optiga chip.
No immediate action required. Treat as routine cleanup. If auditing, verify that rng_fill_buffer_strong() is a suitable replacement for optiga_get_random() in this secure-element context and that it provides at least equivalent entropy and fault resistance.
Security signals we found
Removal of a low-value randomness-mixing helper
Switch from optiga_get_random() + random_xor() to rng_fill_buffer_strong() for PIN secret and HMAC key generation
No changelog entry and commit title explicitly calls it a refactor
Evidence from the diff
The patch deletes random_xor() from crypto/rand.c and crypto/rand.h. That function previously XORed a buffer with bytes from random_buffer() in 4-byte chunks. In core/embed/sec/optiga/optiga.c, two call sites that used optiga_get_random() followed by random_xor() now use rng_fill_buffer_strong() alone. The change reduces code surface and removes a mixing step that added no clear cryptographic benefit, but the commit message frames it only as a refactor with no changelog entry.
Changed components
core/embed/sec/optiga/optiga.ccrypto/rand.ccrypto/rand.hInspect captured patch +3 / −17
diff --git a/core/embed/sec/optiga/optiga.c b/core/embed/sec/optiga/optiga.c
index 4ae1e5d6..fad86e1c 100644
--- a/core/embed/sec/optiga/optiga.c
+++ b/core/embed/sec/optiga/optiga.c
@@ -24,6 +24,7 @@
#include <sec/optiga.h>
#include <sec/optiga_commands.h>
#include <sec/optiga_transport.h>
+#include <sec/rng.h>
#include <sec/secret_keys.h>
#include <sec/storage.h>
#include "ecdsa.h"
@@ -31,7 +32,6 @@
#include "hmac.h"
#include "memzero.h"
#include "nist256p1.h"
-#include "rand.h"
// Counter-protected PIN secret and reset key for OID_STRETCHED_PIN_CTR (OID
// 0xF1D0).
@@ -625,11 +625,10 @@ bool optiga_pin_set(optiga_ui_progress_t ui_progress,
// Generate and store the counter-protected PIN secret.
uint8_t pin_secret[OPTIGA_PIN_SECRET_SIZE] = {0};
- if (optiga_get_random(pin_secret, sizeof(pin_secret)) != OPTIGA_SUCCESS) {
+ if (!rng_fill_buffer_strong(pin_secret, sizeof(pin_secret))) {
ret = false;
goto end;
}
- random_xor(pin_secret, sizeof(pin_secret));
if (optiga_set_data_object(OID_PIN_SECRET, false, pin_secret,
sizeof(pin_secret)) != OPTIGA_SUCCESS) {
@@ -639,11 +638,10 @@ bool optiga_pin_set(optiga_ui_progress_t ui_progress,
// Generate the key for the HMAC-SHA256 PIN stretching step.
uint8_t pin_hmac[OPTIGA_PIN_SECRET_SIZE] = {0};
- if (optiga_get_random(pin_hmac, sizeof(pin_hmac)) != OPTIGA_SUCCESS) {
+ if (!rng_fill_buffer_strong(pin_hmac, sizeof(pin_hmac))) {
ret = false;
goto end;
}
- random_xor(pin_hmac, sizeof(pin_hmac));
// Authorise using OID_PIN_SECRET so that we can write to OID_STRETCHED_PIN
// and OID_STRETCHED_PIN_CTR.
diff --git a/crypto/rand.c b/crypto/rand.c
index ea0902d3..65d3ae42 100644
--- a/crypto/rand.c
+++ b/crypto/rand.c
@@ -23,16 +23,6 @@
#include "rand.h"
-void random_xor(uint8_t *buf, size_t len) {
- uint8_t r[4] = {0};
- for (size_t i = 0; i < len; i++) {
- if (i % sizeof(r) == 0) {
- random_buffer(r, sizeof(r));
- }
- buf[i] ^= r[i % sizeof(r)];
- }
-}
-
uint32_t random_uniform(uint32_t n) {
uint32_t x = 0, max = 0xFFFFFFFF - (0xFFFFFFFF % n);
while ((x = random32()) >= max)
diff --git a/crypto/rand.h b/crypto/rand.h
index b6da3fcf..3184c73d 100644
--- a/crypto/rand.h
+++ b/crypto/rand.h
@@ -39,8 +39,6 @@ static inline uint32_t random32(void) {
return r;
}
-void random_xor(uint8_t *buf, size_t len);
-
uint32_t random_uniform(uint32_t n);
void random_permute(char *buf, size_t len);
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.