From 7d69dd6bf54224ed688c7c04cb5821dfbf802b17 Mon Sep 17 00:00:00 2001 From: ksmith3036 <44052735+ksmith3036@users.noreply.github.com> Date: Wed, 23 Dec 2020 12:15:03 +0100 Subject: [PATCH 1/4] Make bootloaders check for the MCU Security Bit. If Security Bit is set, it will not let a client read from flash memory, and will always erase the full sketch flash memory, when flashing, to avoid anyone trying to perform a partial flash. If BOOTPROT is set to 2 and Security Bit is set, the bootloader will be fully protected, and it should not be trivial to read the sketch out of the MCU. Without these changes, the bootloader ignores the Security Bit, and let clients like bossac.exe read the entire flash memory. --- bootloaders/zero/sam_ba_monitor.c | 124 ++++++++++++++++++++++-------- 1 file changed, 94 insertions(+), 30 deletions(-) diff --git a/bootloaders/zero/sam_ba_monitor.c b/bootloaders/zero/sam_ba_monitor.c index 4d1620c1b..fe96df898 100644 --- a/bootloaders/zero/sam_ba_monitor.c +++ b/bootloaders/zero/sam_ba_monitor.c @@ -30,7 +30,8 @@ #include const char RomBOOT_Version[] = SAM_BA_VERSION; -const char RomBOOT_ExtendedCapabilities[] = "[Arduino:XYZ]"; +// X = Chip Erase, Y = Write Buffer, Z = Checksum Buffer, P = Secure Bit Aware +const char RomBOOT_ExtendedCapabilities[] = "[Arduino:XYZP]"; /* Provides one common interface to handle both USART and USB-CDC */ typedef struct @@ -83,6 +84,8 @@ const t_monitor_if usbcdc_if = /* The pointer to the interface object use by the monitor */ t_monitor_if * ptr_monitor_if; +bool b_security_enabled = false; + /* b_terminal_mode mode (ascii) or hex mode */ volatile bool b_terminal_mode = false; volatile bool b_sam_ba_interface_usart = false; @@ -225,6 +228,10 @@ void sam_ba_putdata_term(uint8_t* data, uint32_t length) volatile uint32_t sp; void call_applet(uint32_t address) { + if (b_security_enabled) { + return; + } + uint32_t app_start_address; __disable_irq(); @@ -242,6 +249,7 @@ void call_applet(uint32_t address) } uint32_t current_number; +uint32_t erased_from = 0; uint32_t i, length; uint8_t command, *ptr_data, *ptr, data[SIZEBUFMAX]; uint8_t j; @@ -264,6 +272,20 @@ static void put_uint32(uint32_t n) sam_ba_putdata( ptr_monitor_if, buff, 8); } +static void eraseFlash(uint32_t dst_addr) +{ + erased_from = dst_addr; + while (dst_addr < MAX_FLASH) + { + // Execute "ER" Erase Row + NVMCTRL->ADDR.reg = dst_addr / 2; + NVMCTRL->CTRLA.reg = NVMCTRL_CTRLA_CMDEX_KEY | NVMCTRL_CTRLA_CMD_ER; + while (NVMCTRL->INTFLAG.bit.READY == 0) + ; + dst_addr += PAGE_SIZE * 4; // Skip a ROW + } +} + #ifdef ENABLE_JTAG_LOAD static uint32_t offset = __UINT32_MAX__; static bool flashNeeded = false; @@ -284,7 +306,7 @@ static void sam_ba_monitor_loop(void) { sam_ba_putdata(ptr_monitor_if, "\n\r", 2); } - if (command == 'S') + if (command == 'S') // Write memory (normally RAM, but might be flash, if client handles the Flash MCU commands?) { //Check if some data are remaining in the "data" buffer if(length>i) @@ -318,37 +340,80 @@ static void sam_ba_monitor_loop(void) __asm("nop"); } - else if (command == 'R') + else if (command == 'R') // Read memory (flash or RAM) { + // Flash memory starts at address 0 and runs to flash size 0x40000 (256 KByte) + + // Internal RWW section is at adress 0x400000. RWW is flash used for EEPROM emulation. Will not let anyone read that, when in secure mode, either. + // Bootloader ends at 0x1FFF, so user programs start at 0x2000 + // RAM starts at 0x20000000, so redirect FLASH reads into RAM reads, when in secure mode + if (b_security_enabled && ((uint32_t)ptr_data >= 0x0000 && (uint32_t)ptr_data < 0x20000000)) + { + ptr_data = (uint8_t *)0x20005000; + } + sam_ba_putdata_xmd(ptr_monitor_if, ptr_data, current_number); } - else if (command == 'O') + else if (command == 'O') // write byte { *ptr_data = (char) current_number; } - else if (command == 'H') + else if (command == 'H') // Write half word { *((uint16_t *) ptr_data) = (uint16_t) current_number; } - else if (command == 'W') + else if (command == 'W') // Write word { *((int *) ptr_data) = current_number; } - else if (command == 'o') + else if (command == 'o') // Read byte { + // Flash memory starts at address 0 and runs to flash size 0x40000 (256 KByte). RAM starts at 0x20000000. + // Intern RWW section is at adress 0x400000. RWW is flash used for EEPROM emulation. Will not let anyone read that, when in secure mode, either. + // BOSSA reads address 0 to check something, but using read word instead of read byte, but in any case allow reading first byte + // Bootloader ends at 0x1FFF, so user programs start at 0x2000 + if (b_security_enabled && ((uint32_t)ptr_data > 0x0003 && (uint32_t)ptr_data < 0x20000000)) + { + ptr_data = (uint8_t*) ¤t_number; + } + sam_ba_putdata_term(ptr_data, 1); } - else if (command == 'h') + else if (command == 'h') // Read half word { - current_number = *((uint16_t *) ptr_data); + // Flash memory starts at address 0 and runs to flash size 0x40000 (256 KByte). RAM starts at 0x20000000. + // Intern RWW section is at adress 0x400000. RWW is flash used for EEPROM emulation. Will not let anyone read that, when in secure mode, either. + // BOSSA reads address 0 to check something, but using read word instead of read byte, but in any case allow reading first byte + // Bootloader ends at 0x1FFF, so user programs start at 0x2000 + if (b_security_enabled && ((uint32_t)ptr_data > 0x0003 && (uint32_t)ptr_data < 0x20000000)) + { + current_number = 0; + } + else + { + current_number = *((uint16_t *) ptr_data); + } + sam_ba_putdata_term((uint8_t*) ¤t_number, 2); } - else if (command == 'w') + else if (command == 'w') // Read word { - current_number = *((uint32_t *) ptr_data); + // Flash memory starts at address 0 and runs to flash size 0x40000 (256 KByte). RAM starts at 0x20000000. + // Intern RWW section is at adress 0x400000. RWW is flash used for EEPROM emulation. Will not let anyone read that, when in secure mode, either. + // BOSSA reads address 0 to check something, but using read word instead of read byte, but in any case allow reading first byte + // Bootloader ends at 0x1FFF, so user programs start at 0x2000 + if (b_security_enabled && ((uint32_t)ptr_data > 0x0003 && (uint32_t)ptr_data < 0x20000000)) + { + current_number = 0; + } + else + { + current_number = *((uint32_t *) ptr_data); + } + sam_ba_putdata_term((uint8_t*) ¤t_number, 4); } - else if (command == 'G') + else if (!b_security_enabled && command == 'G') // Execute code. Will not allow when security is enabled. { call_applet(current_number); /* Rebase the Stack Pointer */ @@ -358,12 +423,12 @@ static void sam_ba_monitor_loop(void) ptr_monitor_if->put_c(0x6); } } - else if (command == 'T') + else if (command == 'T') // Turn on terminal mode { b_terminal_mode = 1; sam_ba_putdata(ptr_monitor_if, "\n\r", 2); } - else if (command == 'N') + else if (command == 'N') // Turn off terminal mode { if (b_terminal_mode == 0) { @@ -371,7 +436,7 @@ static void sam_ba_monitor_loop(void) } b_terminal_mode = 0; } - else if (command == 'V') + else if (command == 'V') // Read version information { sam_ba_putdata( ptr_monitor_if, "v", 1); sam_ba_putdata( ptr_monitor_if, (uint8_t *) RomBOOT_Version, strlen(RomBOOT_Version)); @@ -391,7 +456,7 @@ static void sam_ba_monitor_loop(void) sam_ba_putdata( ptr_monitor_if, (uint8_t *) &(__TIME__), i); sam_ba_putdata( ptr_monitor_if, "\n\r", 2); } - else if (command == 'X') + else if (command == 'X') // Erase flash { // Syntax: X[ADDR]# // Erase the flash memory starting from ADDR to the end of flash. @@ -400,22 +465,13 @@ static void sam_ba_monitor_loop(void) // Even if the starting address is the last byte of a ROW the entire // ROW is erased anyway. - uint32_t dst_addr = current_number; // starting address - - while (dst_addr < MAX_FLASH) - { - // Execute "ER" Erase Row - NVMCTRL->ADDR.reg = dst_addr / 2; - NVMCTRL->CTRLA.reg = NVMCTRL_CTRLA_CMDEX_KEY | NVMCTRL_CTRLA_CMD_ER; - while (NVMCTRL->INTFLAG.bit.READY == 0) - ; - dst_addr += PAGE_SIZE * 4; // Skip a ROW - } - + // BOSSAC.exe always erase with 0x2000 as argument, but an attacker might try to erase just parts of the flash, to be able to copy or analyze the untouched parts. + // To mitigate this, always erase all sketch flash, that is, starting from address 0x2000. This butloader always assume 8 KByte for itself, and sketch starting at 0x2000. + eraseFlash(b_security_enabled ? 0x2000 : current_number); // Notify command completed sam_ba_putdata( ptr_monitor_if, "X\n\r", 3); } - else if (command == 'Y') + else if (command == 'Y') // Write buffer to flash { // This command writes the content of a buffer in SRAM into flash memory. @@ -435,6 +491,13 @@ static void sam_ba_monitor_loop(void) } else { + if (b_security_enabled && erased_from != 0x2000) + { + // To mitigate that an attacker might not use the ordinary BOSSA method of erasing flash before programming, + // always erase flash, if it hasn't been done already. + eraseFlash(0x2000); + } + // Write to flash uint32_t size = current_number/4; uint32_t *src_addr = src_buff_addr; @@ -546,7 +609,7 @@ static void sam_ba_monitor_loop(void) // Notify command completed sam_ba_putdata( ptr_monitor_if, "Y\n\r", 3); } - else if (command == 'Z') + else if (command == 'Z') // Calculate CRC16 { // This command calculate CRC for a given area of memory. // It's useful to quickly check if a transfer has been done @@ -648,6 +711,7 @@ void sam_ba_monitor_run(void) PAGES = NVMCTRL->PARAM.bit.NVMP; MAX_FLASH = PAGE_SIZE * PAGES; + b_security_enabled = NVMCTRL->STATUS.bit.SB != 0; ptr_data = NULL; command = 'z'; while (1) From 9cb10ad181f5c0e2419fbe5302036349741d0b10 Mon Sep 17 00:00:00 2001 From: ksmith3036 <44052735+ksmith3036@users.noreply.github.com> Date: Wed, 23 Dec 2020 12:24:56 +0100 Subject: [PATCH 2/4] Bootloader protection of flash memory may also be turned on by setting the SECURE_BY_DEFAULT compile flag, either through an argument to make, or by setting flag in sam_ba_monitor.h. --- bootloaders/zero/Makefile | 5 ++++- bootloaders/zero/sam_ba_monitor.c | 13 +++++++++++++ bootloaders/zero/sam_ba_monitor.h | 3 +++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/bootloaders/zero/Makefile b/bootloaders/zero/Makefile index 65243c52b..61fca6eb5 100644 --- a/bootloaders/zero/Makefile +++ b/bootloaders/zero/Makefile @@ -71,11 +71,14 @@ else CFLAGS+=-Os -DDEBUG=0 -flto endif +ifdef SECURE_BY_DEFAULT + CFLAGS+=-DSECURE_BY_DEFAULT=1 +endif + ELF=$(NAME).elf BIN=$(NAME).bin HEX=$(NAME).hex - INCLUDES=-I"$(MODULE_PATH)/tools/CMSIS/4.5.0/CMSIS/Include/" -I"$(MODULE_PATH)/tools/CMSIS-Atmel/1.2.0/CMSIS/Device/ATMEL/" # ----------------------------------------------------------------------------- diff --git a/bootloaders/zero/sam_ba_monitor.c b/bootloaders/zero/sam_ba_monitor.c index fe96df898..134e54fd7 100644 --- a/bootloaders/zero/sam_ba_monitor.c +++ b/bootloaders/zero/sam_ba_monitor.c @@ -84,7 +84,11 @@ const t_monitor_if usbcdc_if = /* The pointer to the interface object use by the monitor */ t_monitor_if * ptr_monitor_if; +#ifdef SECURE_BY_DEFAULT +bool b_security_enabled = true; +#else bool b_security_enabled = false; +#endif /* b_terminal_mode mode (ascii) or hex mode */ volatile bool b_terminal_mode = false; @@ -225,6 +229,7 @@ void sam_ba_putdata_term(uint8_t* data, uint32_t length) return; } +#ifndef SECURE_BY_DEFAULT volatile uint32_t sp; void call_applet(uint32_t address) { @@ -247,6 +252,7 @@ void call_applet(uint32_t address) /* Jump to application Reset Handler in the application */ asm("bx %0"::"r"(app_start_address)); } +#endif uint32_t current_number; uint32_t erased_from = 0; @@ -413,6 +419,7 @@ static void sam_ba_monitor_loop(void) sam_ba_putdata_term((uint8_t*) ¤t_number, 4); } +#ifndef SECURE_BY_DEFAULT else if (!b_security_enabled && command == 'G') // Execute code. Will not allow when security is enabled. { call_applet(current_number); @@ -423,6 +430,7 @@ static void sam_ba_monitor_loop(void) ptr_monitor_if->put_c(0x6); } } +#endif else if (command == 'T') // Turn on terminal mode { b_terminal_mode = 1; @@ -711,7 +719,12 @@ void sam_ba_monitor_run(void) PAGES = NVMCTRL->PARAM.bit.NVMP; MAX_FLASH = PAGE_SIZE * PAGES; +#ifdef SECURE_BY_DEFAULT + b_security_enabled = true; +#else b_security_enabled = NVMCTRL->STATUS.bit.SB != 0; +#endif + ptr_data = NULL; command = 'z'; while (1) diff --git a/bootloaders/zero/sam_ba_monitor.h b/bootloaders/zero/sam_ba_monitor.h index 9ba72343c..122b22a6c 100644 --- a/bootloaders/zero/sam_ba_monitor.h +++ b/bootloaders/zero/sam_ba_monitor.h @@ -36,6 +36,9 @@ /* Selects USB as the communication interface of the monitor */ #define SIZEBUFMAX 64 +// Set this flag to let the bootloader enforce read restrictions of flash memory, even if security bit is not set +//#define SECURE_BY_DEFAULT + /** * \brief Initialize the monitor * From efcc5c9fc0fa730530770ac10747f9a167dce6f0 Mon Sep 17 00:00:00 2001 From: ksmith3036 <44052735+ksmith3036@users.noreply.github.com> Date: Wed, 23 Dec 2020 12:29:32 +0100 Subject: [PATCH 3/4] To allow the MKR VIDOR 4000 loader to build to less than 8 KByte, the string handling of compile time DATE and TIME had to be concatinated in source code. The side effect is that the code is much more concise and readable. --- bootloaders/zero/sam_ba_monitor.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/bootloaders/zero/sam_ba_monitor.c b/bootloaders/zero/sam_ba_monitor.c index 134e54fd7..619d1334b 100644 --- a/bootloaders/zero/sam_ba_monitor.c +++ b/bootloaders/zero/sam_ba_monitor.c @@ -450,19 +450,8 @@ static void sam_ba_monitor_loop(void) sam_ba_putdata( ptr_monitor_if, (uint8_t *) RomBOOT_Version, strlen(RomBOOT_Version)); sam_ba_putdata( ptr_monitor_if, " ", 1); sam_ba_putdata( ptr_monitor_if, (uint8_t *) RomBOOT_ExtendedCapabilities, strlen(RomBOOT_ExtendedCapabilities)); - sam_ba_putdata( ptr_monitor_if, " ", 1); - ptr = (uint8_t*) &(__DATE__); - i = 0; - while (*ptr++ != '\0') - i++; - sam_ba_putdata( ptr_monitor_if, (uint8_t *) &(__DATE__), i); - sam_ba_putdata( ptr_monitor_if, " ", 1); - i = 0; - ptr = (uint8_t*) &(__TIME__); - while (*ptr++ != '\0') - i++; - sam_ba_putdata( ptr_monitor_if, (uint8_t *) &(__TIME__), i); - sam_ba_putdata( ptr_monitor_if, "\n\r", 2); + ptr = (uint8_t*) &(" " __DATE__ " " __TIME__ "\n\r"); + sam_ba_putdata( ptr_monitor_if, ptr, strlen(ptr)); } else if (command == 'X') // Erase flash { From aafa688a3f3323734ad2a152627498ed0de999e3 Mon Sep 17 00:00:00 2001 From: ksmith3036 <44052735+ksmith3036@users.noreply.github.com> Date: Wed, 23 Dec 2020 12:39:08 +0100 Subject: [PATCH 4/4] Sometimes an Arduino sketch needs to reset the MCU, but still wants to have some variables to be able to survive. The bootloader itself uses very little RAM and excludes by default the last 4 bytes of RAM from use by the stack. To allow sketches using a modified linker script to take the same approach, the changed bootloader linker script excludes the last 1 KByte of RAM from stack. --- bootloaders/zero/bootloader_samd21x18.ld | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bootloaders/zero/bootloader_samd21x18.ld b/bootloaders/zero/bootloader_samd21x18.ld index 2a8b056d3..91d308d47 100644 --- a/bootloaders/zero/bootloader_samd21x18.ld +++ b/bootloaders/zero/bootloader_samd21x18.ld @@ -27,7 +27,7 @@ MEMORY { FLASH (rx) : ORIGIN = 0x00000000, LENGTH = 0x2000 /* First 8KB used by bootloader */ - RAM (rwx) : ORIGIN = 0x20000000, LENGTH = 0x00008000-0x0004 /* 4 bytes used by bootloader to keep data between resets */ + RAM (rwx) : ORIGIN = 0x20000000, LENGTH = 0x00008000-0x0400 /* last 4 bytes used by bootloader to keep data between resets, but reserves 1024 bytes for sketches to have same possibility */ } /* Linker script to place sections and symbol values. Should be used together