From 98e617d8740b85ae01d7d6e0dd3f49e66057a210 Mon Sep 17 00:00:00 2001 From: Jochen Hoenicke Date: Tue, 8 Aug 2017 12:59:39 +0200 Subject: [PATCH] startup: use custom reset_handler + group confidential data in one place + zero all SRAM where needed --- Makefile | 1 + Makefile.include | 17 +++++++++++------ bootloader/bootloader.c | 3 +++ bootloader/bootloader.h | 4 ++-- firmware/Makefile | 9 ++++----- firmware/ethereum.c | 2 +- firmware/fastflash.c | 20 +++++++++++--------- firmware/fastflash.h | 1 - firmware/fsm.c | 2 +- firmware/signing.c | 5 +++-- firmware/storage.c | 6 +++--- firmware/transaction.c | 2 +- firmware/trezor.c | 1 - firmware/u2f.c | 2 +- memory.ld | 13 +++++++++++++ memory_app_1.0.0.ld | 13 +++++++++++++ memory_app_2.0.0.ld | 21 --------------------- memory_app_fastflash.ld | 13 +++++++++++++ setup.c | 9 +++++++++ startup.s | 39 +++++++++++++++++++++++++++++++++++++++ util.h | 25 ++++++++++++++++--------- 21 files changed, 145 insertions(+), 63 deletions(-) delete mode 100644 memory_app_2.0.0.ld create mode 100644 startup.s diff --git a/Makefile b/Makefile index 747da6d..27d58a0 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,4 @@ +OBJS += startup.o OBJS += buttons.o OBJS += layout.o OBJS += oled.o diff --git a/Makefile.include b/Makefile.include index 8fba930..7bbcb3d 100644 --- a/Makefile.include +++ b/Makefile.include @@ -7,11 +7,14 @@ LD = $(PREFIX)gcc OBJCOPY = $(PREFIX)objcopy OBJDUMP = $(PREFIX)objdump AR = $(PREFIX)ar +AS = $(PREFIX)as FLASH = st-flash OPENOCD = openocd OPTFLAGS ?= -O3 DBGFLAGS ?= -g -DNDEBUG +CPUFLAGS ?= -mcpu=cortex-m3 -mthumb +FPUFLAGS ?= -msoft-float CFLAGS += $(OPTFLAGS) \ $(DBGFLAGS) \ @@ -40,10 +43,10 @@ CFLAGS += $(OPTFLAGS) \ -ffunction-sections \ -fdata-sections \ -fstack-protector-all \ - -mcpu=cortex-m3 \ - -mthumb \ - -msoft-float \ + $(CPUFLAGS) \ + $(FPUFLAGS) \ -DSTM32F2 \ + -DCONFIDENTIAL='__attribute__((section("confidential")))' \ -I$(TOOLCHAIN_DIR)/include \ -I$(TOP_DIR) \ -I$(TOP_DIR)gen \ @@ -83,9 +86,8 @@ LDFLAGS += --static \ -T$(LDSCRIPT) \ -nostartfiles \ -Wl,--gc-sections \ - -mcpu=cortex-m3 \ - -mthumb \ - -msoft-float + $(CPUFLAGS) \ + $(FPUFLAGS) all: $(NAME).bin @@ -128,6 +130,9 @@ $(NAME).list: $(NAME).elf $(NAME).elf: $(OBJS) $(LDSCRIPT) $(TOOLCHAIN_DIR)/lib/libopencm3_stm32f2.a $(TOP_DIR)/libtrezor.a $(LD) -o $(NAME).elf $(OBJS) -ltrezor -lopencm3_stm32f2 $(LDFLAGS) +%.o: %.s Makefile + $(AS) $(CPUFLAGS) -o $@ $< + %.o: %.c Makefile $(CC) $(CFLAGS) -MMD -o $@ -c $< diff --git a/bootloader/bootloader.c b/bootloader/bootloader.c index 1a3c0c2..6a7ba9e 100644 --- a/bootloader/bootloader.c +++ b/bootloader/bootloader.c @@ -78,6 +78,9 @@ void show_unofficial_warning(const uint8_t *hash) void __attribute__((noreturn)) load_app(void) { + // zero out SRAM + memset_reg(_ram_start, _ram_end, 0); + load_vector_table((const vector_table_t *) FLASH_APP_START); } diff --git a/bootloader/bootloader.h b/bootloader/bootloader.h index 723b9ba..7324909 100644 --- a/bootloader/bootloader.h +++ b/bootloader/bootloader.h @@ -22,14 +22,14 @@ #define VERSION_MAJOR 1 #define VERSION_MINOR 3 -#define VERSION_PATCH 2 +#define VERSION_PATCH 3 #define STR(X) #X #define VERSTR(X) STR(X) #define VERSION_MAJOR_CHAR "\x01" #define VERSION_MINOR_CHAR "\x03" -#define VERSION_PATCH_CHAR "\x02" +#define VERSION_PATCH_CHAR "\x03" #include #include "memory.h" diff --git a/firmware/Makefile b/firmware/Makefile index 95f43e2..4599627 100644 --- a/firmware/Makefile +++ b/firmware/Makefile @@ -1,10 +1,8 @@ -ifeq ($(FASTFLASH),1) -APPVER = 2.0.0 +APPVER = 1.0.0 +ifeq ($(FASTFLASH),1) OBJS += fastflash.o OBJS += bootloader.o -else -APPVER = 1.0.0 endif NAME = trezor @@ -93,6 +91,7 @@ CFLAGS += -DUSE_ETHEREUM=1 bootloader.o: ../fastflash/bootloader.bin $(OBJCOPY) -I binary -O elf32-littlearm -B arm \ + --redefine-sym _binary_$(shell echo -n "$<" | tr -c "[:alnum:]" "_")_start=__bootloader_start__ \ --redefine-sym _binary_$(shell echo -n "$<" | tr -c "[:alnum:]" "_")_size=__bootloader_size__ \ - --rename-section .data=.bootloader \ + --rename-section .data=.rodata \ $< $@ diff --git a/firmware/ethereum.c b/firmware/ethereum.c index cf8e9a7..0b63cd6 100644 --- a/firmware/ethereum.c +++ b/firmware/ethereum.c @@ -37,7 +37,7 @@ static bool ethereum_signing = false; static uint32_t data_total, data_left; static EthereumTxRequest msg_tx_request; -static uint8_t privkey[32]; +static CONFIDENTIAL uint8_t privkey[32]; static uint8_t chain_id; struct SHA3_CTX keccak_ctx; diff --git a/firmware/fastflash.c b/firmware/fastflash.c index 76f31cf..25f33e5 100644 --- a/firmware/fastflash.c +++ b/firmware/fastflash.c @@ -24,16 +24,18 @@ #include #include -extern uint8_t __bootloader_loadaddr__[]; -extern uint8_t __bootloader_runaddr__[]; -extern uint8_t __bootloader_size__[]; - -void load_bootloader(void) -{ - memcpy(__bootloader_runaddr__, __bootloader_loadaddr__, (size_t) __bootloader_size__); -} +#define bootloader_vec ((vector_table_t *) 0x20000000) void __attribute__((noreturn)) run_bootloader(void) { - load_vector_table((const vector_table_t *) __bootloader_runaddr__); + extern uint8_t __bootloader_start__[]; + extern uint8_t __bootloader_size__[]; + + // zero out SRAM + memset_reg(_ram_start, _ram_end, 0); + + // copy bootloader + memcpy(bootloader_vec, __bootloader_start__, (size_t) __bootloader_size__); + + load_vector_table(bootloader_vec); } diff --git a/firmware/fastflash.h b/firmware/fastflash.h index 20caf1e..e35b443 100644 --- a/firmware/fastflash.h +++ b/firmware/fastflash.h @@ -20,7 +20,6 @@ #ifndef __FASTFLASH_H__ #define __FASTFLASH_H__ -void load_bootloader(void); void __attribute__((noreturn)) run_bootloader(void); #endif diff --git a/firmware/fsm.c b/firmware/fsm.c index 08bc5be..e793c37 100644 --- a/firmware/fsm.c +++ b/firmware/fsm.c @@ -177,7 +177,7 @@ const CoinType *fsm_getCoin(bool has_name, const char *name) HDNode *fsm_getDerivedNode(const char *curve, uint32_t *address_n, size_t address_n_count) { - static HDNode node; + static CONFIDENTIAL HDNode node; if (!storage_getRootNode(&node, curve, true)) { fsm_sendFailure(FailureType_Failure_NotInitialized, _("Device not initialized or passphrase request cancelled or unsupported curve")); layoutHome(); diff --git a/firmware/signing.c b/firmware/signing.c index dfca6e4..b8aaaa0 100644 --- a/firmware/signing.c +++ b/firmware/signing.c @@ -32,7 +32,7 @@ static uint32_t inputs_count; static uint32_t outputs_count; static const CoinType *coin; static const HDNode *root; -static HDNode node; +static CONFIDENTIAL HDNode node; static bool signing = false; enum { STAGE_REQUEST_1_INPUT, @@ -54,7 +54,8 @@ static TxInputType input; static TxOutputBinType bin_output; static TxStruct to, tp, ti; static SHA256_CTX hashers[3]; -static uint8_t privkey[32], pubkey[33], sig[64]; +static uint8_t CONFIDENTIAL privkey[32]; +static uint8_t pubkey[33], sig[64]; static uint8_t hash_prevouts[32], hash_sequence[32],hash_outputs[32]; static uint8_t hash_check[32]; static uint64_t to_spend, authorized_amount, spending, change_spend; diff --git a/firmware/storage.c b/firmware/storage.c index e222928..a7a4ee3 100644 --- a/firmware/storage.c +++ b/firmware/storage.c @@ -42,7 +42,7 @@ #include "usb.h" #include "gettext.h" -Storage storage; +Storage CONFIDENTIAL storage; uint32_t storage_uuid[12/sizeof(uint32_t)]; char storage_uuid_str[25]; @@ -95,12 +95,12 @@ static const uint32_t storage_magic = 0x726f7473; // 'stor' as uint32_t static bool sessionSeedCached, sessionSeedUsesPassphrase; -static uint8_t sessionSeed[64]; +static uint8_t CONFIDENTIAL sessionSeed[64]; static bool sessionPinCached; static bool sessionPassphraseCached; -static char sessionPassphrase[51]; +static char CONFIDENTIAL sessionPassphrase[51]; #define STORAGE_VERSION 8 diff --git a/firmware/transaction.c b/firmware/transaction.c index c282e67..199045f 100644 --- a/firmware/transaction.c +++ b/firmware/transaction.c @@ -152,7 +152,7 @@ int compile_output(const CoinType *coin, const HDNode *root, TxOutputType *in, T } if (in->address_n_count > 0) { - HDNode node; + static CONFIDENTIAL HDNode node; InputScriptType input_script_type; switch (in->script_type) { diff --git a/firmware/trezor.c b/firmware/trezor.c index 5560421..15ce9fb 100644 --- a/firmware/trezor.c +++ b/firmware/trezor.c @@ -95,7 +95,6 @@ int main(void) #if FASTFLASH uint16_t state = gpio_port_read(BTN_PORT); if ((state & BTN_PIN_NO) == 0) { - load_bootloader(); run_bootloader(); } #endif diff --git a/firmware/u2f.c b/firmware/u2f.c index cc47b6f..41c1f33 100644 --- a/firmware/u2f.c +++ b/firmware/u2f.c @@ -450,7 +450,7 @@ void getReadableAppId(const uint8_t appid[U2F_APPID_SIZE], const char **appname, const HDNode *getDerivedNode(uint32_t *address_n, size_t address_n_count) { - static HDNode node; + static CONFIDENTIAL HDNode node; if (!storage_getRootNode(&node, NIST256P1_NAME, false)) { layoutHome(); debugLog(0, "", "ERR: Device not init"); diff --git a/memory.ld b/memory.ld index 5398378..94667b4 100644 --- a/memory.ld +++ b/memory.ld @@ -6,4 +6,17 @@ MEMORY ram (rwx) : ORIGIN = 0x20000000, LENGTH = 128K } +SECTIONS +{ + .confidential (NOLOAD) : { + *(confidential) + ASSERT ((SIZEOF(.confidential) <= 32K), "Error: Confidential section too big!"); + } >ram +} + INCLUDE libopencm3_stm32f2.ld + +_ram_start = ORIGIN(ram); +_ram_end = ORIGIN(ram) + LENGTH(ram); + +_data_size = SIZEOF(.data); diff --git a/memory_app_1.0.0.ld b/memory_app_1.0.0.ld index a8188fb..7a09b78 100644 --- a/memory_app_1.0.0.ld +++ b/memory_app_1.0.0.ld @@ -6,4 +6,17 @@ MEMORY ram (rwx) : ORIGIN = 0x20000000, LENGTH = 128K } +SECTIONS +{ + .confidential (NOLOAD) : { + *(confidential) + ASSERT ((SIZEOF(.confidential) <= 32K), "Error: Confidential section too big!"); + } >ram +} + INCLUDE libopencm3_stm32f2.ld + +_ram_start = ORIGIN(ram); +_ram_end = ORIGIN(ram) + LENGTH(ram); + +_data_size = SIZEOF(.data); diff --git a/memory_app_2.0.0.ld b/memory_app_2.0.0.ld deleted file mode 100644 index bfd773a..0000000 --- a/memory_app_2.0.0.ld +++ /dev/null @@ -1,21 +0,0 @@ -/* STM32F205RE - 512K Flash, 128K RAM */ -/* program starts at 0x08010000 */ -MEMORY -{ - rom (rx) : ORIGIN = 0x08010000, LENGTH = 512K - 64K - bootloader (rx) : ORIGIN = 0x20000000, LENGTH = 32K - ram (rwx) : ORIGIN = 0x20000000 + LENGTH(bootloader), - LENGTH = 128K - LENGTH(bootloader) -} - -INCLUDE libopencm3_stm32f2.ld - -SECTIONS -{ - .bootloader : { - __bootloader_runaddr__ = .; - KEEP (*(.bootloader*)) - } >bootloader AT >rom - - __bootloader_loadaddr__ = LOADADDR(.bootloader); -} diff --git a/memory_app_fastflash.ld b/memory_app_fastflash.ld index 2521584..55b0fb6 100644 --- a/memory_app_fastflash.ld +++ b/memory_app_fastflash.ld @@ -6,4 +6,17 @@ MEMORY LENGTH = 128K - LENGTH(rom) } +SECTIONS +{ + .confidential (NOLOAD) : { + *(confidential) + ASSERT ((SIZEOF(.confidential) <= 32K), "Error: Confidential section too big!"); + } >ram +} + INCLUDE libopencm3_stm32f2.ld + +_ram_start = ORIGIN(ram); +_ram_end = ORIGIN(ram) + LENGTH(ram); + +_data_size = SIZEOF(.data); diff --git a/setup.c b/setup.c index cf2726a..b84bdd3 100644 --- a/setup.c +++ b/setup.c @@ -17,6 +17,7 @@ * along with this library. If not, see . */ +#include #include #include #include @@ -44,6 +45,14 @@ void nmi_handler(void) void setup(void) { + // set SCB_CCR STKALIGN bit to make sure 8-byte stack alignment on exception entry is in effect. + // This is not strictly necessary for the current TREZOR system. + // This is here to comply with guidance from section 3.3.3 "Binary compatibility with other Cortex processors" + // of the ARM Cortex-M3 Processor Technical Reference Manual. + // According to section 4.4.2 and 4.4.7 of the "STM32F10xxx/20xxx/21xxx/L1xxxx Cortex-M3 programming manual", + // STM32F2 series MCUs are r2p0 and always have this bit set on reset already. + SCB_CCR |= SCB_CCR_STKALIGN; + // setup clock struct rcc_clock_scale clock = rcc_hse_8mhz_3v3[RCC_CLOCK_3V3_120MHZ]; rcc_clock_setup_hse_3v3(&clock); diff --git a/startup.s b/startup.s new file mode 100644 index 0000000..58aec19 --- /dev/null +++ b/startup.s @@ -0,0 +1,39 @@ + .syntax unified + + .text + + .global memset_reg + .type memset_reg, STT_FUNC +memset_reg: + // call with the following (note that the arguments are not validated prior to use): + // r0 - address of first word to write (inclusive) + // r1 - address of first word following the address in r0 to NOT write (exclusive) + // r2 - word value to be written + // both addresses in r0 and r1 needs to be divisible by 4! + .L_loop_begin: + str r2, [r0], 4 // store the word in r2 to the address in r0, post-indexed + cmp r0, r1 + bne .L_loop_begin + bx lr + + .global reset_handler + .type reset_handler, STT_FUNC +reset_handler: + ldr r0, =_ram_start // r0 - point to beginning of SRAM + ldr r1, =_ram_end // r1 - point to byte after the end of SRAM + ldr r2, =0 // r2 - the byte-sized value to be written + bl memset_reg + + // copy .data section from flash to SRAM + ldr r0, =_data // dst addr + ldr r1, =_data_loadaddr // src addr + ldr r2, =_data_size // length in bytes + bl memcpy + + // enter the application code + bl main + + // loop forever if the application code returns + b . + + .end diff --git a/util.h b/util.h index ed6aaad..8e4c6ce 100644 --- a/util.h +++ b/util.h @@ -41,18 +41,25 @@ void __attribute__((noreturn)) system_halt(void); // reset system void __attribute__((noreturn)) system_reset(void); -static inline void __attribute__((noreturn)) load_vector_table(const vector_table_t *vector_table) { - // Relocate vector table - SCB_VTOR = (uint32_t) vector_table; +// defined in memory.ld +extern uint8_t _ram_start[], _ram_end[]; - // Set stack pointer - __asm__ volatile("msr msp, %0" :: "r" (vector_table->initial_sp_value)); +// defined in startup.s +extern void memset_reg(void *start, void *stop, uint32_t val); - // Jump to address - vector_table->reset(); +static inline void __attribute__((noreturn)) load_vector_table(const vector_table_t *vector_table) +{ + // Relocate vector table + SCB_VTOR = (uint32_t)vector_table; - // Prevent compiler from generating stack protector code (which causes CPU fault because the stack is moved) - for (;;); + // Set stack pointer + __asm__ volatile("msr msp, %0" :: "r" (vector_table->initial_sp_value)); + + // Jump to address + vector_table->reset(); + + // Prevent compiler from generating stack protector code (which causes CPU fault because the stack is moved) + for (;;); } #endif