dmitrystu / sboot_stm32

Secure USB DFU1.1 bootloader for STM32
Apache License 2.0
303 stars 63 forks source link

Timeout for inactivity. #47

Open salyzyn opened 2 years ago

salyzyn commented 2 years ago

To prevent DOSing of the Firmware by remaining locked up in the bootloader. Should something unexpected (?) result in the bootloader being activated, allow us to add an optional configurable activity-reset-able timeout for the usbd_poll loop. An option to block timeout when a firmware download is partial perhaps?

dmitrystu commented 2 years ago

Do you mean a some sort watchdog timer?

salyzyn commented 2 years ago

Software watchdog. I am thinking more like taking a timestamp after every transaction. Return a bool indicating 'something happened' from the poll functions, the mainline loop could take the timestamp on first entry, and for every 'true' response from poll. If the timestamp varies by more than the the configured timeout, then reboot, check the application image, and if it is ok, boot the App, otherwise, in effect, continue in DFU mode.

One use case is the application can request to reboot into dfu mode, and if nothing happens after, lets say, a minute, it returns to the application. This way a hacker, or some unhappy accident, could not get in and DOS the application by injecting a reboot into dfu mode.

salyzyn commented 1 year ago

Something like this should work, but lacks the ability to be consistent timebase because each mcu clocks it differently:

diff --git a/src/bootloader.c b/src/bootloader.c
index 6299daf..b412b7b 100644
--- a/src/bootloader.c
+++ b/src/bootloader.c
@@ -74,6 +74,9 @@ static struct dfu_data_s {
     uint8_t     interface;
     uint8_t     bStatus;
     uint8_t     bState;
+#if defined(DFU_WATCHDOG)
+    uint32_t    counter;
+#endif
 } dfu_data;

 /** Processing DFU_SET_IDLE request */
@@ -100,6 +103,27 @@ static usbd_respond dfu_set_idle(void) {

 extern void System_Reset(void);

+static inline void heartbeat(void) {
+#if defined(DFU_WATCHDOG)
+    dfu_data.counter = 0;
+    SysTick->CTRL = SysTick_CTRL_CLKSOURCE_Msk | SysTick_CTRL_ENABLE_Msk;
+    SysTick->VAL = SysTick->LOAD = (1ULL << 24) - 1;
+    SysTick->CTRL |= SysTick_CTRL_ENABLE_Msk;
+#endif
+}
+
+static inline void watchdog(void) {
+#if defined(DFU_WATCHDOG)
+    if (SysTick->CTRL & SysTick_CTRL_COUNTFLAG_Msk) {
+        dfu_data.counter++;
+    }
+    if ((((uint64_t)dfu_data.counter << 24)
+      | (~SysTick->VAL & ((1ULL << 24) - 1))) > DFU_WATCHDOG) {
+        System_Reset();
+    }
+#endif
+}
+
 static usbd_respond dfu_err_badreq(void) {
     dfu_data.bState  = USB_DFU_STATE_DFU_ERROR;
     dfu_data.bStatus = USB_DFU_STATUS_ERR_STALLEDPKT;
@@ -223,6 +247,7 @@ static void dfu_reset(usbd_device *dev, uint8_t ev, uint8_t ep) {
 }

 static usbd_respond dfu_control (usbd_device *dev, usbd_ctlreq *req, usbd_rqc_callback *callback) {
+    heartbeat();
     (void)callback;
     if ((req->bmRequestType  & (USB_REQ_TYPE | USB_REQ_RECIPIENT)) == (USB_REQ_STANDARD | USB_REQ_INTERFACE)) {
         switch (req->bRequest) {
@@ -287,6 +312,7 @@ static usbd_respond dfu_control (usbd_device *dev, usbd_ctlreq *req, usbd_rqc_ca

 static usbd_respond dfu_config(usbd_device *dev, uint8_t config) {
+    heartbeat();
     switch (config) {
     case 0:
         usbd_reg_event(dev, usbd_evt_reset, 0);
@@ -313,7 +339,9 @@ static void dfu_init (void) {

 int main (void) {
     dfu_init();
+    heartbeat();
     while(1) {
         usbd_poll(&dfu);
+        watchdog();
     }
 }