Merge branch 'bugfix/secure_boot_padding' into 'master'

secure boot: Fix padding issue allowing unverified data to be mapped into the address space

See merge request idf/esp-idf!2778

components/app_update/esp_ota_ops.c

@@ -240,14 +240,6 @@ esp_err_t esp_ota_end(esp_ota_handle_t handle)
         goto cleanup;
     }
 
-#ifdef CONFIG_SECURE_BOOT_ENABLED
-    ret = esp_secure_boot_verify_signature(it->part->address, data.image_len);
-    if (ret != ESP_OK) {
-        ret = ESP_ERR_OTA_VALIDATE_FAILED;
-        goto cleanup;
-    }
-#endif
-
  cleanup:
     LIST_REMOVE(it, entries);
     free(it);

components/bootloader/Kconfig.projbuild

@@ -275,6 +275,13 @@ config SECURE_BOOT_ALLOW_JTAG
 
           Only set this option in testing environments.
 
+config SECURE_BOOT_ALLOW_SHORT_APP_PARTITION
+       bool "Allow app partition length not 64KB aligned"
+       depends on SECURE_BOOT_INSECURE
+       help
+           If not set (default), app partition size must be a multiple of 64KB. App images are padded to 64KB length, and the bootloader checks any trailing bytes after the signature (before the next 64KB boundary) have not been written. This is because flash cache maps entire 64KB pages into the address space. This prevents an attacker from appending unverified data after the app image in the flash, causing it to be mapped into the address space.
+
+           Setting this option allows the app partition length to be unaligned, and disables padding of the app image to this length. It is generally not recommended to set this option, unless you have a legacy partitioning scheme which doesn't support 64KB aligned partition lengths.
 
 config FLASH_ENCRYPTION_UART_BOOTLOADER_ALLOW_ENCRYPT
     bool "Leave UART bootloader encryption enabled"

components/bootloader_support/src/secure_boot_signatures.c

@@ -84,10 +84,13 @@ esp_err_t esp_secure_boot_verify_signature_block(const esp_secure_boot_sig_block
         return ESP_FAIL;
     }
 
+    ESP_LOGD(TAG, "Verifying secure boot signature");
+
     is_valid = uECC_verify(signature_verification_key_start,
                                 image_digest,
                                 DIGEST_LEN,
                                 sig_block->signature,
                                 uECC_secp256r1());
+    ESP_LOGD(TAG, "Verification result %d", is_valid);
     return is_valid ? ESP_OK : ESP_ERR_IMAGE_INVALID;
 }

components/esptool_py/Makefile.projbuild

@@ -31,6 +31,14 @@ endif
 
 ESPTOOL_ELF2IMAGE_OPTIONS :=
 
+ifdef CONFIG_SECURE_BOOT_ENABLED
+ifndef CONFIG_SECURE_BOOT_ALLOW_SHORT_APP_PARTITION
+ifndef IS_BOOTLOADER_BUILD
+ESPTOOL_ELF2IMAGE_OPTIONS += --secure-pad
+endif
+endif
+endif
+
 ESPTOOLPY_WRITE_FLASH=$(ESPTOOLPY_SERIAL) write_flash $(if $(CONFIG_ESPTOOLPY_COMPRESSED),-z,-u) $(ESPTOOL_WRITE_FLASH_OPTIONS)
 
 ESPTOOL_ALL_FLASH_ARGS += $(APP_OFFSET) $(APP_BIN)

components/esptool_py/esptool

@@ -1 +1 @@
-Subproject commit da31d9d7a1bb496995f8e30a6be259689948e43e
+Subproject commit fd8c25d2160505fb9d5abbe56f85116a136afb05

components/partition_table/Makefile.projbuild

@@ -9,7 +9,7 @@
 .PHONY: partition_table partition_table-flash partition_table-clean partition_table_get_info
 
 PARTITION_MD5_OPT :=
-ifneq ("$(CONFIG_PARTITION_TABLE_MD5)", "y")
+ifndef CONFIG_PARTITION_TABLE_MD5
 PARTITION_MD5_OPT := "--disable-md5sum"
 endif
 
@@ -18,11 +18,18 @@ ifneq ("$(CONFIG_ESPTOOLPY_FLASHSIZE)", "")
 PARTITION_FLASHSIZE_OPT := --flash-size $(CONFIG_ESPTOOLPY_FLASHSIZE)
 endif
 
+PARTITION_SECURE_OPT :=
+ifdef CONFIG_SECURE_BOOT_ENABLED
+ifndef CONFIG_SECURE_BOOT_ALLOW_SHORT_APP_PARTITION
+PARTITION_SECURE_OPT += --secure
+endif
+endif
+
 # Address of partition table
 PARTITION_TABLE_OFFSET := $(CONFIG_PARTITION_TABLE_OFFSET)
 PARTITION_TABLE_OFFSET_ARG := --offset $(PARTITION_TABLE_OFFSET)
 
-GEN_ESP32PART := $(PYTHON) $(COMPONENT_PATH)/gen_esp32part.py -q $(PARTITION_MD5_OPT) $(PARTITION_FLASHSIZE_OPT) $(PARTITION_TABLE_OFFSET_ARG)
+GEN_ESP32PART := $(PYTHON) $(COMPONENT_PATH)/gen_esp32part.py -q $(PARTITION_MD5_OPT) $(PARTITION_FLASHSIZE_OPT) $(PARTITION_TABLE_OFFSET_ARG) $(PARTITION_SECURE_OPT)
 GET_PART_INFO := $(COMPONENT_PATH)/parttool.py -q
 
 # if CONFIG_PARTITION_TABLE_FILENAME is unset, means we haven't re-generated config yet...

components/partition_table/gen_esp32part.py

@@ -62,6 +62,7 @@ SUBTYPES = {
 
 quiet = False
 md5sum = True
+secure = False
 offset_part_table = 0
 
 def status(msg):
@@ -322,6 +323,8 @@ class PartitionDefinition(object):
         align = self.ALIGNMENT.get(self.type, 4)
         if self.offset % align:
             raise ValidationError(self, "Offset 0x%x is not aligned to 0x%x" % (self.offset, align))
+        if self.size % align and secure:
+            raise ValidationError(self, "Size 0x%x is not aligned to 0x%x" % (self.size, align))
         if self.size is None:
             raise ValidationError(self, "Size field is not set")
 
@@ -414,6 +417,7 @@ def main():
     global quiet
     global md5sum
     global offset_part_table
+    global secure
     parser = argparse.ArgumentParser(description='ESP32 partition table utility')
 
     parser.add_argument('--flash-size', help='Optional flash size limit, checks partition table fits in flash',
@@ -423,7 +427,7 @@ def main():
     parser.add_argument('--verify', '-v', help="Verify partition table fields (deprecated, this behaviour is enabled by default and this flag does nothing.", action='store_true')
     parser.add_argument('--quiet', '-q', help="Don't print non-critical status messages to stderr", action='store_true')
     parser.add_argument('--offset', '-o', help='Set offset partition table', default='0x8000')
-    
+    parser.add_argument('--secure', help="Require app partitions to be suitable for secure boot", action='store_true')
     parser.add_argument('input', help='Path to CSV or binary file to parse.', type=argparse.FileType('rb'))
     parser.add_argument('output', help='Path to output converted binary or CSV file. Will use stdout if omitted.',
                         nargs='?', default='-')
@@ -432,6 +436,7 @@ def main():
 
     quiet = args.quiet
     md5sum = not args.disable_md5sum
+    secure = args.secure
     offset_part_table = int(args.offset, 0)
     input = args.input.read()
     input_is_binary = input[0:2] == PartitionDefinition.MAGIC_BYTES