From 6fd4739c5c24ce93afdbe39af33f4a2bfae8ce45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Vejpustek?= Date: Thu, 25 Mar 2021 19:33:21 +0100 Subject: [PATCH] feat(core): make random delays use chacha_drbg --- core/.changelog.d/1554.changed | 1 + core/SConscript.bootloader | 3 +- core/SConscript.bootloader_ci | 3 +- core/SConscript.prodtest | 3 +- core/embed/bootloader/main.c | 4 +- core/embed/bootloader_ci/main.c | 3 +- core/embed/firmware/main.c | 4 +- core/embed/prodtest/main.c | 3 +- core/embed/trezorhal/common.c | 23 ---- core/embed/trezorhal/common.h | 5 - core/embed/trezorhal/random_delays.c | 179 +++++++++++++++++---------- core/embed/trezorhal/random_delays.h | 2 + core/embed/unix/common.c | 2 - 13 files changed, 128 insertions(+), 107 deletions(-) create mode 100644 core/.changelog.d/1554.changed diff --git a/core/.changelog.d/1554.changed b/core/.changelog.d/1554.changed new file mode 100644 index 000000000..7a2b2ac82 --- /dev/null +++ b/core/.changelog.d/1554.changed @@ -0,0 +1 @@ +Random delays use ChaCha-based DRBG instead of HMAC-DRBG. diff --git a/core/SConscript.bootloader b/core/SConscript.bootloader index 4ca5236c2..bd1ed6238 100644 --- a/core/SConscript.bootloader +++ b/core/SConscript.bootloader @@ -22,13 +22,14 @@ CPPDEFINES_MOD += [ ] SOURCE_MOD += [ 'vendor/trezor-crypto/blake2s.c', + 'vendor/trezor-crypto/chacha_drbg.c', + 'vendor/trezor-crypto/chacha20poly1305/chacha_merged.c', 'vendor/trezor-crypto/ed25519-donna/curve25519-donna-32bit.c', 'vendor/trezor-crypto/ed25519-donna/curve25519-donna-helpers.c', 'vendor/trezor-crypto/ed25519-donna/ed25519.c', 'vendor/trezor-crypto/ed25519-donna/ed25519-donna-32bit-tables.c', 'vendor/trezor-crypto/ed25519-donna/ed25519-donna-impl-base.c', 'vendor/trezor-crypto/ed25519-donna/modm-donna-32bit.c', - 'vendor/trezor-crypto/hmac_drbg.c', 'vendor/trezor-crypto/memzero.c', 'vendor/trezor-crypto/rand.c', 'vendor/trezor-crypto/sha2.c', diff --git a/core/SConscript.bootloader_ci b/core/SConscript.bootloader_ci index 2be760bd4..ce986e3d0 100644 --- a/core/SConscript.bootloader_ci +++ b/core/SConscript.bootloader_ci @@ -22,13 +22,14 @@ CPPDEFINES_MOD += [ ] SOURCE_MOD += [ 'vendor/trezor-crypto/blake2s.c', + 'vendor/trezor-crypto/chacha_drbg.c', + 'vendor/trezor-crypto/chacha20poly1305/chacha_merged.c', 'vendor/trezor-crypto/ed25519-donna/curve25519-donna-32bit.c', 'vendor/trezor-crypto/ed25519-donna/curve25519-donna-helpers.c', 'vendor/trezor-crypto/ed25519-donna/ed25519.c', 'vendor/trezor-crypto/ed25519-donna/ed25519-donna-32bit-tables.c', 'vendor/trezor-crypto/ed25519-donna/ed25519-donna-impl-base.c', 'vendor/trezor-crypto/ed25519-donna/modm-donna-32bit.c', - 'vendor/trezor-crypto/hmac_drbg.c', 'vendor/trezor-crypto/memzero.c', 'vendor/trezor-crypto/rand.c', 'vendor/trezor-crypto/sha2.c', diff --git a/core/SConscript.prodtest b/core/SConscript.prodtest index c92c3b685..f906567b6 100644 --- a/core/SConscript.prodtest +++ b/core/SConscript.prodtest @@ -14,7 +14,8 @@ CPPPATH_MOD += [ 'vendor/trezor-crypto', ] SOURCE_MOD += [ - 'vendor/trezor-crypto/hmac_drbg.c', + 'vendor/trezor-crypto/chacha_drbg.c', + 'vendor/trezor-crypto/chacha20poly1305/chacha_merged.c', 'vendor/trezor-crypto/memzero.c', 'vendor/trezor-crypto/rand.c', 'vendor/trezor-crypto/sha2.c', diff --git a/core/embed/bootloader/main.c b/core/embed/bootloader/main.c index 3c944e3c8..19360837f 100644 --- a/core/embed/bootloader/main.c +++ b/core/embed/bootloader/main.c @@ -26,7 +26,7 @@ #include "image.h" #include "mini_printf.h" #include "mpu.h" -#include "rng.h" +#include "random_delays.h" #include "secbool.h" #include "touch.h" #include "usb.h" @@ -236,7 +236,7 @@ static void check_bootloader_version(void) { #endif int main(void) { - drbg_init(); + random_delays_init(); // display_init_seq(); touch_init(); touch_power_on(); diff --git a/core/embed/bootloader_ci/main.c b/core/embed/bootloader_ci/main.c index c7bfba16d..b6ddfcef7 100644 --- a/core/embed/bootloader_ci/main.c +++ b/core/embed/bootloader_ci/main.c @@ -26,6 +26,7 @@ #include "image.h" #include "mini_printf.h" #include "mpu.h" +#include "random_delays.h" #include "rng.h" #include "secbool.h" #include "touch.h" @@ -214,7 +215,7 @@ static void check_bootloader_version(void) { #endif int main(void) { - drbg_init(); + random_delays_init(); touch_init(); touch_power_on(); diff --git a/core/embed/firmware/main.c b/core/embed/firmware/main.c index e10d165a9..e4c808dbe 100644 --- a/core/embed/firmware/main.c +++ b/core/embed/firmware/main.c @@ -52,8 +52,8 @@ #include "touch.h" int main(void) { - // initialize pseudo-random number generator - drbg_init(); + random_delays_init(); + #ifdef RDI rdi_start(); #endif diff --git a/core/embed/prodtest/main.c b/core/embed/prodtest/main.c index 90ec3a609..d7350864e 100644 --- a/core/embed/prodtest/main.c +++ b/core/embed/prodtest/main.c @@ -27,6 +27,7 @@ #include "display.h" #include "flash.h" #include "mini_printf.h" +#include "random_delays.h" #include "rng.h" #include "sbu.h" #include "sdcard.h" @@ -371,7 +372,7 @@ static secbool startswith(const char *s, const char *prefix) { int main(void) { display_orientation(0); - drbg_init(); + random_delays_init(); sdcard_init(); touch_init(); sbu_init(); diff --git a/core/embed/trezorhal/common.c b/core/embed/trezorhal/common.c index 72999854d..cf20f07cc 100644 --- a/core/embed/trezorhal/common.c +++ b/core/embed/trezorhal/common.c @@ -24,7 +24,6 @@ #include "common.h" #include "display.h" #include "flash.h" -#include "hmac_drbg.h" #include "rand.h" #include "stm32f4xx_ll_utils.h" @@ -32,8 +31,6 @@ // from util.s extern void shutdown(void); -static HMAC_DRBG_CTX drbg_ctx; - #define COLOR_FATAL_ERROR RGB16(0x7F, 0x00, 0x00) void __attribute__((noreturn)) @@ -170,23 +167,3 @@ void collect_hw_entropy(void) { FLASH_OTP_BLOCK_SIZE), NULL); } - -void drbg_init(void) { - uint8_t entropy[48]; - random_buffer(entropy, sizeof(entropy)); - hmac_drbg_init(&drbg_ctx, entropy, sizeof(entropy), NULL, 0); -} - -void drbg_reseed(const uint8_t *entropy, size_t len) { - hmac_drbg_reseed(&drbg_ctx, entropy, len, NULL, 0); -} - -void drbg_generate(uint8_t *buf, size_t len) { - hmac_drbg_generate(&drbg_ctx, buf, len); -} - -uint32_t drbg_random32(void) { - uint32_t value; - drbg_generate((uint8_t *)&value, sizeof(value)); - return value; -} diff --git a/core/embed/trezorhal/common.h b/core/embed/trezorhal/common.h index ab5cdaa12..d46bfb6e2 100644 --- a/core/embed/trezorhal/common.h +++ b/core/embed/trezorhal/common.h @@ -74,11 +74,6 @@ void collect_hw_entropy(void); #define HW_ENTROPY_LEN (12 + 32) extern uint8_t HW_ENTROPY_DATA[HW_ENTROPY_LEN]; -void drbg_init(void); -void drbg_reseed(const uint8_t *entropy, size_t len); -void drbg_generate(uint8_t *buf, size_t len); -uint32_t drbg_random32(void); - // the following functions are defined in util.s void memset_reg(volatile void *start, volatile void *stop, uint32_t val); diff --git a/core/embed/trezorhal/random_delays.c b/core/embed/trezorhal/random_delays.c index d8994b591..d5df09f23 100644 --- a/core/embed/trezorhal/random_delays.c +++ b/core/embed/trezorhal/random_delays.c @@ -36,6 +36,7 @@ https://link.springer.com/content/pdf/10.1007%2F978-3-540-72354-7_3.pdf #include "random_delays.h" +#include #include #include "chacha_drbg.h" @@ -46,35 +47,125 @@ https://link.springer.com/content/pdf/10.1007%2F978-3-540-72354-7_3.pdf // from util.s extern void shutdown(void); +#define DRBG_RESEED_INTERVAL_CALLS 1000 +#define DRBG_TRNG_ENTROPY_LENGTH 50 +_Static_assert(CHACHA_DRBG_OPTIMAL_RESEED_LENGTH(1) == DRBG_TRNG_ENTROPY_LENGTH, + ""); #define BUFFER_LENGTH 64 -#define RESEED_INTERVAL 65536 static CHACHA_DRBG_CTX drbg_ctx; -static uint8_t buffer[BUFFER_LENGTH]; -static size_t buffer_index; +static secbool drbg_initialized = secfalse; static uint8_t session_delay; static bool refresh_session_delay; static secbool rdi_disabled = sectrue; -static void rdi_reseed(void) { - uint8_t entropy[CHACHA_DRBG_SEED_LENGTH]; - random_buffer(entropy, CHACHA_DRBG_SEED_LENGTH); +static void drbg_init() { + uint8_t entropy[DRBG_TRNG_ENTROPY_LENGTH] = {0}; + random_buffer(entropy, sizeof(entropy)); + chacha_drbg_init(&drbg_ctx, entropy, sizeof(entropy), NULL, 0); + memzero(entropy, sizeof(entropy)); + + drbg_initialized = sectrue; +} + +static void drbg_reseed() { + ensure(drbg_initialized, NULL); + + uint8_t entropy[DRBG_TRNG_ENTROPY_LENGTH] = {0}; + random_buffer(entropy, sizeof(entropy)); chacha_drbg_reseed(&drbg_ctx, entropy, sizeof(entropy), NULL, 0); + memzero(entropy, sizeof(entropy)); } -static void buffer_refill(void) { - chacha_drbg_generate(&drbg_ctx, buffer, BUFFER_LENGTH); -} +static void drbg_generate(uint8_t *buffer, size_t length) { + ensure(drbg_initialized, NULL); -static uint32_t random8(void) { - buffer_index += 1; - if (buffer_index >= BUFFER_LENGTH) { - buffer_refill(); - if (RESEED_INTERVAL != 0 && drbg_ctx.reseed_counter > RESEED_INTERVAL) - rdi_reseed(); - buffer_index = 0; + if (drbg_ctx.reseed_counter > DRBG_RESEED_INTERVAL_CALLS) { + drbg_reseed(); + } + chacha_drbg_generate(&drbg_ctx, buffer, length); +} + +// WARNING: Returns a constant if the function's critical section is locked +static uint32_t drbg_random8(void) { + // Since the function is called both from an interrupt (rdi_handler, + // wait_random) and the main thread (wait_random), we use a lock to + // synchronise access to global variables + static volatile atomic_flag locked = ATOMIC_FLAG_INIT; + + if (atomic_flag_test_and_set(&locked)) + // locked_old = locked; locked = true; locked_old + { + // If the critical section is locked we return a non-random value, which + // should be ok for our purposes + return 128; + } + + static size_t buffer_index = 0; + static uint8_t buffer[BUFFER_LENGTH] = {0}; + + if (buffer_index == 0) { + drbg_generate(buffer, sizeof(buffer)); + } + + // To be extra sure there is no buffer overflow, we use a local copy of + // buffer_index + size_t buffer_index_local = buffer_index % sizeof(buffer); + uint8_t value = buffer[buffer_index_local]; + memzero(&buffer[buffer_index_local], 1); + buffer_index = (buffer_index_local + 1) % sizeof(buffer); + + atomic_flag_clear(&locked); // locked = false + return value; +} + +static void wait(uint32_t delay) { + // wait (30 + delay) ticks + asm volatile( + "ldr r0, %0;" // r0 = delay + "loop:" + "subs r0, $3;" // r0 -= 3 + "bhs loop;" // if (r0 >= 3): goto loop + // loop (delay // 3) times + // every loop takes 3 ticks + // r0 == (delay % 3) - 3 + "add r0, $3;" // r0 += 3 + // r0 == delay % 3 + "and r0, r0, $3;" // r0 %= 4, make sure that 0 <= r0 < 4 + "ldr r1, =table;" // r1 = &table + "tbb [r1, r0];" // jump 2*r1[r0] bytes forward, that is goto wait_r0 + "base:" + "table:" // table of branch lengths + ".byte (wait_0 - base)/2;" + ".byte (wait_1 - base)/2;" + ".byte (wait_2 - base)/2;" + ".byte (wait_2 - base)/2;" // next instruction must be 2-byte aligned + "wait_2:" + "add r0, $1;" // wait one tick + "wait_1:" + "add r0, $1;" // wait one tick + "wait_0:" + : + : "m"(delay) + : "r0", "r1"); +} + +void random_delays_init() { drbg_init(); } + +void rdi_start(void) { + ensure(drbg_initialized, NULL); + + if (rdi_disabled == sectrue) { // if rdi disabled + refresh_session_delay = true; + rdi_disabled = secfalse; + } +} + +void rdi_stop(void) { + if (rdi_disabled == secfalse) { // if rdi enabled + rdi_disabled = sectrue; + session_delay = 0; } - return buffer[buffer_index]; } void rdi_refresh_session_delay(void) { @@ -85,71 +176,23 @@ void rdi_refresh_session_delay(void) { void rdi_handler(uint32_t uw_tick) { if (rdi_disabled == secfalse) { // if rdi enabled if (refresh_session_delay) { - session_delay = random8(); + session_delay = drbg_random8(); refresh_session_delay = false; } - uint32_t delay = random8() + session_delay; + wait(drbg_random8() + session_delay); - // wait (30 + delay) ticks - asm volatile( - "ldr r0, %0;" // r0 = delay - "loop:" - "subs r0, $3;" // r0 -= 3 - "bhs loop;" // if (r0 >= 3): goto loop - // loop (delay // 3) times - // every loop takes 3 ticks - // r0 == (delay % 3) - 3 - "add r0, $3;" // r0 += 3 - // r0 == delay % 3 - "and r0, r0, $3;" // r0 %= 4, make sure that 0 <= r0 < 4 - "ldr r1, =table;" // r1 = &table - "tbb [r1, r0];" // jump 2*r1[r0] bytes forward, that is goto wait_r0 - "base:" - "table:" // table of branch lengths - ".byte (wait_0 - base)/2;" - ".byte (wait_1 - base)/2;" - ".byte (wait_2 - base)/2;" - ".byte (wait_2 - base)/2;" // next instruction must be 2-byte aligned - "wait_2:" - "add r0, $1;" // wait one tick - "wait_1:" - "add r0, $1;" // wait one tick - "wait_0:" - : - : "m"(delay) - : "r0", "r1"); } else { // if rdi disabled or rdi_disabled corrupted ensure(rdi_disabled, "Fault detected"); } } -void rdi_start(void) { - if (rdi_disabled == sectrue) { // if rdi disabled - uint8_t entropy[CHACHA_DRBG_SEED_LENGTH]; - random_buffer(entropy, CHACHA_DRBG_SEED_LENGTH); - chacha_drbg_init(&drbg_ctx, entropy, sizeof(entropy), NULL, 0); - buffer_refill(); - buffer_index = 0; - refresh_session_delay = true; - rdi_disabled = secfalse; - } -} - -void rdi_stop(void) { - if (rdi_disabled == secfalse) { // if rdi enabled - rdi_disabled = sectrue; - session_delay = 0; - memzero(&drbg_ctx, sizeof(drbg_ctx)); - } -} - /* * Generates a delay of random length. Use this to protect sensitive code * against fault injection. */ void wait_random(void) { - int wait = drbg_random32() & 0xff; + int wait = drbg_random8(); volatile int i = 0; volatile int j = wait; while (i < wait) { diff --git a/core/embed/trezorhal/random_delays.h b/core/embed/trezorhal/random_delays.h index b7a0ae1c3..ae8e0073f 100644 --- a/core/embed/trezorhal/random_delays.h +++ b/core/embed/trezorhal/random_delays.h @@ -22,6 +22,8 @@ #include +void random_delays_init(void); + void rdi_start(void); void rdi_stop(void); void rdi_refresh_session_delay(void); diff --git a/core/embed/unix/common.c b/core/embed/unix/common.c index ef9434d4e..370245232 100644 --- a/core/embed/unix/common.c +++ b/core/embed/unix/common.c @@ -107,8 +107,6 @@ error_shutdown(const char *line1, const char *line2, const char *line3, void hal_delay(uint32_t ms) { usleep(1000 * ms); } -void wait_random(void) {} - uint8_t HW_ENTROPY_DATA[HW_ENTROPY_LEN]; void collect_hw_entropy(void) { memzero(HW_ENTROPY_DATA, HW_ENTROPY_LEN); }