Merge pull request #385 from danielinux/test-delta

Improve delta tests + fix delta+encrypt bug

Author: dgarske

.github/workflows/test-powerfail-simulator.yml

@@ -380,3 +380,39 @@ jobs:
       - name: Run update-revert test with power failures (AES128 NVM_WRITEONCE FLAGS_HOME FLAGS_INVERT)
         run: |
           tools/scripts/sim-update-powerfail-resume.sh
+
+     # TEST with encryption (aes128) and NVM_WRITEONCE and DELTA updates
+      - name: make clean
+        run: |
+          make distclean
+      - name: Select config with encrypted updates
+        run: |
+          cp config/examples/sim-encrypt-delta-nvm-writeonce-update.config .config
+
+      - name: Build tools
+        run: |
+          make -C tools/keytools && make -C tools/bin-assemble
+
+      - name: Build wolfboot.elf
+        run: |
+          make clean && make test-sim-external-flash-with-enc-delta-update
+
+      - name: Run sunny day update test (AES128 DELTA)
+        run: |
+          tools/scripts/sim-sunnyday-update.sh
+
+      - name: Rebuild wolfboot.elf
+        run: |
+          make clean && make test-sim-external-flash-with-enc-delta-update
+
+      - name: Run update-revert test (AES128 DELTA)
+        run: |
+          tools/scripts/sim-update-fallback.sh
+
+      - name: Rebuild wolfboot.elf
+        run: |
+          make clean && make test-sim-external-flash-with-enc-delta-update
+
+      - name: Run update-revert test with power failures (AES128 DELTA)
+        run: |
+          tools/scripts/sim-update-powerfail-resume.sh

config/examples/sim-encrypt-delta-nvm-writeonce-update.config

@@ -0,0 +1,23 @@
+ARCH=sim
+TARGET=sim
+SIGN?=ED25519
+HASH?=SHA256
+WOLFBOOT_SMALL_STACK=1
+SPI_FLASH=0
+EXT_FLASH=1
+ENCRYPT=1
+ENCRYPT_WITH_AES128=1
+DEBUG=1
+DELTA_UPDATES=1
+NVM_FLASH_WRITEONCE=1
+
+# sizes should be multiple of system page size
+WOLFBOOT_PARTITION_SIZE=0x40000
+WOLFBOOT_SECTOR_SIZE=0x1000
+WOLFBOOT_PARTITION_BOOT_ADDRESS=0x20000
+# if on external flash, it should be multiple of system page size
+WOLFBOOT_PARTITION_UPDATE_ADDRESS=0x00000
+WOLFBOOT_PARTITION_SWAP_ADDRESS=0x40000
+
+# required for keytools
+WOLFBOOT_FIXED_PARTITIONS=1

include/delta.h

@@ -67,7 +67,7 @@ int wb_diff_init(WB_DIFF_CTX *ctx, uint8_t *src_a, uint32_t len_a, uint8_t *src_
 int wb_diff(WB_DIFF_CTX *ctx, uint8_t *patch, uint32_t len);
 int wb_patch_init(WB_PATCH_CTX *bm, uint8_t *src, uint32_t ssz, uint8_t *patch, uint32_t psz);
 int wb_patch(WB_PATCH_CTX *ctx, uint8_t *dst, uint32_t len);
-int wolfBoot_get_delta_info(uint8_t part, int inverse, uint32_t **img_offset, uint16_t **img_size);
+int wolfBoot_get_delta_info(uint8_t part, int inverse, uint32_t **img_offset, uint32_t **img_size);
 
 #endif
 

src/delta.c

@@ -209,7 +209,7 @@ int wb_diff(WB_DIFF_CTX *ctx, uint8_t *patch, uint32_t len)
          * base for the sectors that have already been updated.
          */
 
-        pa_start = (WOLFBOOT_SECTOR_SIZE + 1) * page_start;
+        pa_start = WOLFBOOT_SECTOR_SIZE  * page_start;
         pa = ctx->src_a + pa_start;
         while (((uintptr_t)(pa - ctx->src_a) < (uintptr_t)ctx->size_a) && (p_off < len)) {
             if ((uintptr_t)(ctx->size_a - (pa - ctx->src_a)) < BLOCK_HDR_SIZE)
@@ -273,7 +273,8 @@ int wb_diff(WB_DIFF_CTX *ctx, uint8_t *patch, uint32_t len)
                 /* Don't try matching backwards if the distance between the two
                  * blocks is smaller than one sector.
                  */
-                if (WOLFBOOT_SECTOR_SIZE > (pb - ctx->src_b) - (page_start * WOLFBOOT_SECTOR_SIZE))
+                if (WOLFBOOT_SECTOR_SIZE > (page_start * WOLFBOOT_SECTOR_SIZE)
+                        - (pb - ctx->src_b))
                     break;
 
                 if ((memcmp(pb, (ctx->src_b + ctx->off_b), BLOCK_HDR_SIZE) == 0)) {

src/libwolfboot.c

@@ -1690,7 +1690,7 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data,
     uint32_t row_address = address, row_offset;
     int sz = len, i, step;
     uint8_t part;
-    uint32_t iv_counter;
+    uint32_t iv_counter = 0;
 
     row_offset = address & (ENCRYPT_BLOCK_SIZE - 1);
     if (row_offset != 0) {
@@ -1768,7 +1768,10 @@ int RAMFUNCTION ext_flash_decrypt_read(uintptr_t address, uint8_t *data, int len
     uint8_t block[ENCRYPT_BLOCK_SIZE];
     uint8_t dec_block[ENCRYPT_BLOCK_SIZE];
     uint32_t row_address = address, row_offset, iv_counter = 0;
-    int sz = len, i, step;
+    int i;
+    int flash_read_size;
+    int read_remaining = len;
+    int unaligned_head_size, unaligned_tail_size;
     uint8_t part;
     uintptr_t base_address;
 
@@ -1778,10 +1781,6 @@ int RAMFUNCTION ext_flash_decrypt_read(uintptr_t address, uint8_t *data, int len
     row_offset = address & (ENCRYPT_BLOCK_SIZE - 1);
     if (row_offset != 0) {
         row_address = address & ~(ENCRYPT_BLOCK_SIZE - 1);
-        sz += ENCRYPT_BLOCK_SIZE - row_offset;
-    }
-    if (sz < ENCRYPT_BLOCK_SIZE) {
-        sz = ENCRYPT_BLOCK_SIZE;
     }
     if (!encrypt_initialized) {
         if (crypto_init() < 0)
@@ -1806,42 +1805,54 @@ int RAMFUNCTION ext_flash_decrypt_read(uintptr_t address, uint8_t *data, int len
         default:
             return -1;
     }
-    /* decrypt blocks */
-    if (sz > len) {
-        step = ENCRYPT_BLOCK_SIZE - row_offset;
+    /* Decrypt block. If the address does not align with the encryption block,
+     * decrypt then copy only the bytes from the requested address.
+     */
+    if (row_offset != 0) {
+        unaligned_head_size = ENCRYPT_BLOCK_SIZE - row_offset;
         if (ext_flash_read(row_address, block, ENCRYPT_BLOCK_SIZE)
                 != ENCRYPT_BLOCK_SIZE) {
             return -1;
         }
         crypto_decrypt(dec_block, block, ENCRYPT_BLOCK_SIZE);
-        XMEMCPY(data, dec_block + row_offset, step);
-        address += step;
-        data += step;
-        sz = len - step;
+        XMEMCPY(data, dec_block + row_offset, unaligned_head_size);
+        address += unaligned_head_size;
+        data += unaligned_head_size;
+        read_remaining -= unaligned_head_size;
         iv_counter++;
     }
-
-    /* decrypt remainder */
-    step = sz & ~(ENCRYPT_BLOCK_SIZE - 1);
-    if (ext_flash_read(address, data, step) != step)
+    /* Trim the read size to align with the Encryption Blocks. Read the
+     * remaining unaligned tail bytes after, since the `data` buffer won't have
+     * enough space to handle the extra bytes.
+     */
+    flash_read_size = read_remaining & ~(ENCRYPT_BLOCK_SIZE - 1);
+    if (ext_flash_read(address, data, flash_read_size) != flash_read_size)
         return -1;
-    for (i = 0; i < step / ENCRYPT_BLOCK_SIZE; i++) {
+    for (i = 0; i < flash_read_size / ENCRYPT_BLOCK_SIZE; i++)
+    {
         XMEMCPY(block, data + (ENCRYPT_BLOCK_SIZE * i), ENCRYPT_BLOCK_SIZE);
         crypto_decrypt(data + (ENCRYPT_BLOCK_SIZE * i), block,
-            ENCRYPT_BLOCK_SIZE);
+                ENCRYPT_BLOCK_SIZE);
         iv_counter++;
     }
-    sz -= step;
-    if (sz > 0) {
-        if (ext_flash_read(address + step, block, ENCRYPT_BLOCK_SIZE)
-                != ENCRYPT_BLOCK_SIZE) {
+
+    address += flash_read_size;
+    data += flash_read_size;
+    read_remaining -= flash_read_size;
+
+    /* Read the unaligned tail bytes. */
+    unaligned_tail_size = read_remaining;
+    if (unaligned_tail_size > 0)
+    {
+        uint8_t dec_block[ENCRYPT_BLOCK_SIZE];
+        if (ext_flash_read(address, block, ENCRYPT_BLOCK_SIZE)
+                != ENCRYPT_BLOCK_SIZE)
             return -1;
-        }
         crypto_decrypt(dec_block, block, ENCRYPT_BLOCK_SIZE);
-            XMEMCPY(data + step, dec_block, sz);
-        iv_counter++;
+        XMEMCPY(data, dec_block, unaligned_tail_size);
+        read_remaining -= unaligned_tail_size;
     }
-    return len;
+    return (len - read_remaining);
 }
 #endif /* EXT_FLASH */
 #endif /* __WOLFBOOT */

src/update_flash.c

@@ -212,7 +212,7 @@ static int wolfBoot_delta_update(struct wolfBoot_image *boot,
     uint32_t offset = 0;
     uint16_t ptr_len;
     uint32_t *img_offset;
-    uint16_t *img_size;
+    uint32_t *img_size;
     uint32_t total_size;
     WB_PATCH_CTX ctx;
 #ifdef EXT_ENCRYPTED

test-app/Makefile

@@ -8,7 +8,8 @@ ARCH?=ARM
 MCUXPRESSO_CMSIS?=$(MCUXPRESSO)/CMSIS
 CFLAGS+=-I.
 CFLAGS+=-I./wcs
-DEBUG=1
+DEBUG?=1
+DELTA_DATA_SIZE?=2000
 
 ifeq ($(SIGN),RSA2048)
   IMAGE_HEADER_SIZE:=512
@@ -166,7 +167,7 @@ endif
 ifeq ($(TARGET),sim)
   APP_OBJS=app_$(TARGET).o ../test-app/libwolfboot.o ../hal/$(TARGET).o
   # Override linker flags
-  LDFLAGS=
+  LDFLAGS=-Wl,-Map=image.map
 endif
 
 ifeq ($(EXT_FLASH),1)
@@ -327,6 +328,9 @@ CFLAGS+=-I../lib/wolfssl
 standalone:CFLAGS+=-D"TEST_APP_STANDALONE"
 standalone:LDFLAGS:=-T standalone.ld -Wl,-gc-sections -Wl,-Map=image.map
 
+delta-extra-data:CFLAGS+=-D"TEST_DELTA_DATA=$(DELTA_DATA_SIZE)" -ffunction-sections -fdata-sections
+delta-extra-data:LDFLAGS=-Wl,-Map=image.map
+
 image.bin: image.elf
 	@echo "\t[BIN] $@"
 	$(Q)$(OBJCOPY) --gap-fill $(FILL_BYTE) -O binary $^ $@
@@ -338,6 +342,8 @@ image.elf: $(APP_OBJS) $(LSCRIPT)
 
 standalone: image.bin
 
+delta-extra-data: image.bin
+
 ../test-app/libwolfboot.o: ../src/libwolfboot.c FORCE
 	@echo "\t[CC-$(ARCH)] $@"
 	$(Q)$(CC) $(CFLAGS) -c $(OUTPUT_FLAG) $@ ../src/libwolfboot.c

test-app/app_sim.c

@@ -40,6 +40,11 @@
 char enc_key[] = "0123456789abcdef0123456789abcdef"
 		 "0123456789abcdef";
 
+#ifdef TEST_DELTA_DATA
+static volatile char __attribute__((used)) garbage[TEST_DELTA_DATA] = {0x01, 0x02, 0x03, 0x04 };
+
+#endif
+
 void hal_init(void);
 
 int do_cmd(const char *cmd)

tools/test.mk

@@ -7,6 +7,7 @@ SPI_CHIP=SST25VF080B
 SPI_OPTIONS=SPI_FLASH=1 WOLFBOOT_PARTITION_SIZE=0x80000 WOLFBOOT_PARTITION_UPDATE_ADDRESS=0x00000 WOLFBOOT_PARTITION_SWAP_ADDRESS=0x80000
 SIGN_ARGS=
 SIGN_ENC_ARGS=
+DELTA_DATA_SIZE?=2000
 
 # python version only supported using
 # KEYGEN_TOOL="python3 $(WOLFBOOT_ROOT)/tools/keytools/keygen.py"
@@ -162,6 +163,28 @@ test-sim-external-flash-with-update: wolfboot.bin test-app/image.elf FORCE
 	$(Q)$(BINASSEMBLE) external_flash.dd 0 test-app/image_v$(TEST_UPDATE_VERSION)_signed.bin \
 		$(WOLFBOOT_PARTITION_SIZE) erased_sec.dd
 
+test-sim-external-flash-with-enc-delta-update-extradata:DELTA_UPDATE_OPTIONS=--delta test-app/image_v1_signed.bin
+test-sim-external-flash-with-enc-delta-update-extradata:SIGN_ENC_ARGS=--encrypt /tmp/enc_key.der --aes128
+test-sim-external-flash-with-enc-delta-update-extradata: wolfboot.bin test-app/image.elf FORCE
+	@printf "0123456789abcdef0123456789abcdef0123456789abcdef" > /tmp/enc_key.der
+	$(Q)$(SIGN_TOOL) $(SIGN_OPTIONS) test-app/image.elf $(PRIVATE_KEY) 1
+	$(Q)cp test-app/image_v1_signed.bin test-app/image_v1_signed.bak
+	$(Q)rm -f test-app/image.elf test-app/app_sim.o
+	$(Q)make -C test-app delta-extra-data DELTA_DATA_SIZE=$(DELTA_DATA_SIZE)
+	$(Q)cp test-app/image_v1_signed.bak test-app/image_v1_signed.bin
+	$(Q)$(SIGN_TOOL) $(SIGN_OPTIONS) $(SIGN_ENC_ARGS) test-app/image.elf $(PRIVATE_KEY) $(TEST_UPDATE_VERSION)
+	$(Q)$(SIGN_TOOL) $(SIGN_ARGS) $(DELTA_UPDATE_OPTIONS) $(SIGN_ENC_ARGS) \
+		test-app/image.elf $(PRIVATE_KEY) $(TEST_UPDATE_VERSION)
+	$(Q)dd if=/dev/zero bs=$$(($(WOLFBOOT_PARTITION_SIZE))) count=1 2>/dev/null | tr "\000" "\377" > v1_part.dd
+	$(Q)dd if=test-app/image_v1_signed.bin bs=256 of=v1_part.dd conv=notrunc
+	$(Q)$(BINASSEMBLE) internal_flash.dd \
+		0 wolfboot.bin \
+		$$(($(WOLFBOOT_PARTITION_BOOT_ADDRESS) - $(ARCH_FLASH_OFFSET))) v1_part.dd
+	$(Q)dd if=/dev/zero bs=$$(($(WOLFBOOT_SECTOR_SIZE))) count=1 2>/dev/null | tr "\000" "\377" > erased_sec.dd
+	$(Q)$(BINASSEMBLE) external_flash.dd 0 test-app/image_v$(TEST_UPDATE_VERSION)_signed_diff_encrypted.bin \
+		$(WOLFBOOT_PARTITION_SIZE) erased_sec.dd
+	$(Q)ls -l test-app/*.bin
+
 
 test-sim-external-flash-with-enc-update:SIGN_ENC_ARGS=--encrypt /tmp/enc_key.der --aes128
 test-sim-external-flash-with-enc-update: wolfboot.bin test-app/image.elf FORCE