Merge pull request #417 from danielinux/fix-tlv-alignment

Custom TLV: enforce 8B alignment for all fields

Author: dgarske

.github/workflows/test-custom-tlv-simulator.yml

@@ -0,0 +1,44 @@
+name: Custom TLV - test with simulator target
+
+on:
+  push:
+    branches: [ 'master', 'main', 'release/**' ]
+  pull_request:
+    branches: [ '*' ]
+
+jobs:
+  custom_tlv_simulator_tests:
+    runs-on: ubuntu-20.04
+
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          submodules: true
+
+      - name: make clean
+        run: |
+          make distclean
+
+      - name: Select config
+        run: |
+          cp config/examples/sim.config .config
+
+      - name: Build tools
+        run: |
+          make -C tools/keytools && make -C tools/bin-assemble
+
+      - name: Build wolfboot.elf and test-app/image.elf
+        run: |
+          make clean && make
+
+      - name: Sign the image with a custom TLV
+        run: |
+          tools/keytools/sign --ed25519 --custom-tlv-buffer 0x0034 AABBCCDDEEFF0011223344 test-app/image.elf wolfboot_signing_private_key.der 1
+      
+      - name: Re-assemble the internal_flash.dd image file
+        run: |
+          make assemble_internal_flash.dd
+
+      - name: Run get_tlv simulator test
+        run: |
+          [ x`./wolfboot.elf get_tlv 2>/dev/null| tail -1` =  xAABBCCDDEEFF0011223344 ]

Makefile

@@ -198,15 +198,18 @@ test-app/image.elf: wolfboot.elf
 	$(Q)$(MAKE) -C test-app WOLFBOOT_ROOT="$(WOLFBOOT_ROOT)" image.elf
 	$(Q)$(SIZE) test-app/image.elf
 
-internal_flash.dd: $(BINASSEMBLE) wolfboot.bin $(BOOT_IMG) $(PRIVATE_KEY) test-app/image_v1_signed.bin
-	@echo "\t[MERGE] internal_flash.dd"
-	$(Q)dd if=/dev/zero bs=1 count=$$(($(WOLFBOOT_SECTOR_SIZE))) > /tmp/swap
-	$(Q)$(BINASSEMBLE) $@ \
+assemble_internal_flash.dd: FORCE
+	$(Q)$(BINASSEMBLE) internal_flash.dd \
 		0 wolfboot.bin \
 		$$(($(WOLFBOOT_PARTITION_BOOT_ADDRESS) - $(ARCH_FLASH_OFFSET))) test-app/image_v1_signed.bin \
 		$$(($(WOLFBOOT_PARTITION_UPDATE_ADDRESS)-$(ARCH_FLASH_OFFSET))) /tmp/swap \
 		$$(($(WOLFBOOT_PARTITION_SWAP_ADDRESS)-$(ARCH_FLASH_OFFSET))) /tmp/swap
 
+internal_flash.dd: $(BINASSEMBLE) wolfboot.bin $(BOOT_IMG) $(PRIVATE_KEY) test-app/image_v1_signed.bin
+	@echo "\t[MERGE] internal_flash.dd"
+	$(Q)dd if=/dev/zero bs=1 count=$$(($(WOLFBOOT_SECTOR_SIZE))) > /tmp/swap
+	make assemble_internal_flash.dd
+
 factory.bin: $(BINASSEMBLE) wolfboot.bin $(BOOT_IMG) $(PRIVATE_KEY) test-app/image_v1_signed.bin
 	@echo "\t[MERGE] $@"
 	$(Q)$(BINASSEMBLE) $@ \

docs/firmware_image.md

@@ -80,15 +80,16 @@ From the bootloader code, we can then retrieve the value of the custom field usi
 uint32_t value;
 uint8_t* ptr = NULL;
 uint16_t tlv = 0x34;
-uint8_t* imageHdr = (uint8_t*)WOLFBOOT_PARTITION_BOOT_ADDRESS; /* WOLFBOOT_PARTITION_UPDATE_ADDRESS */
+uint8_t* imageHdr = (uint8_t*)WOLFBOOT_PARTITION_BOOT_ADDRESS + IMAGE_HEADER_OFFSET;
 uint16_t size = wolfBoot_find_header(imageHdr, tlv, &ptr);
-if (size != sizeof(uint32_t) || ptr == NULL) {
-    /* Error: the field is not present or has the wrong size */
+if (size > 0 && ptr != NULL) {
+  /* Found field and ptr points to value 0xAABBCCDD */
+  memcpy(&value, ptr, size);
+  printf("TLV 0x%x=0x%x\n", tlv, value);
+}
+else {
+    /* Error: the field is not found */
 }
-
-/* From here, the value 0xAABBCCDD is at ptr */
-memcpy(&value, ptr, size);
-printf("TLV 0x%x=0x%x\n", tlv, value);
 ```
 
 ### Image signing tool

include/image.h

@@ -565,10 +565,6 @@ int wolfBoot_set_update_sector_flag(uint16_t sector, uint8_t newflag);
 uint8_t* wolfBoot_peek_image(struct wolfBoot_image *img, uint32_t offset,
         uint32_t* sz);
 
-
-/* Defined in libwolfboot */
-uint16_t wolfBoot_find_header(uint8_t *haystack, uint16_t type, uint8_t **ptr);
-
 /* get header type for image */
 uint16_t wolfBoot_get_header(struct wolfBoot_image *img, uint16_t type, uint8_t **ptr);
 

include/wolfboot/wolfboot.h

@@ -275,6 +275,8 @@ uint32_t wolfBoot_get_blob_version(uint8_t *blob);
 uint16_t wolfBoot_get_blob_type(uint8_t *blob);
 uint32_t wolfBoot_get_blob_diffbase_version(uint8_t *blob);
 
+uint16_t wolfBoot_find_header(uint8_t *haystack, uint16_t type, uint8_t **ptr);
+
 /* Get partition ID from manifest header */
 static inline uint8_t wolfBoot_get_blob_partition_id(uint8_t *blob) {
     return wolfBoot_get_blob_type(blob) & HDR_IMG_TYPE_PART_MASK;

test-app/app_sim.c

@@ -70,7 +70,33 @@ int do_cmd(const char *cmd)
     if (strcmp(cmd, "reset") == 0) {
         exit(0);
     }
+    if (strncmp(cmd, "get_tlv",7) == 0) {
+        /* boot partition and skip the image header offset (8 bytes) */
+        uint8_t* imageHdr = (uint8_t*)WOLFBOOT_PARTITION_BOOT_ADDRESS + IMAGE_HEADER_OFFSET;
+        uint8_t* ptr = NULL;
+        uint16_t tlv = 0x34; /* default */
+        int size;
+        int i;
 
+        const char* tlvStr = strstr(cmd, "get_tlv=");
+        if (tlvStr) {
+            tlvStr += strlen("get_tlv=");
+            tlv = (uint16_t)atoi(tlvStr);
+        }
+
+        size = wolfBoot_find_header(imageHdr, tlv, &ptr);
+        if (size > 0 && ptr != NULL) {
+            /* From here, the value 0xAABBCCDD is at ptr */
+            printf("TLV 0x%x: found (size %d):\n", tlv, size);
+            for (i=0; i<size; i++) {
+                printf("%02X", ptr[i]);
+            }
+            printf("\n");
+            return 0;
+        } else {
+            printf("TLV 0x%x: not found!\r\n", tlv);
+        }
+    }
     /* wrong command */
     return -1;
 }

tools/keytools/sign.c

@@ -1085,7 +1085,10 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz,
         uint32_t i;
         for (i = 0; i < CMD.custom_tlvs; i++) {
             /* require 8-byte alignment */
-            while ((header_idx % 8) != 0)
+            /* The offset '4' takes into account 2B Tag + 2B Len, so that the 
+             * Value starts at (addr % 8 == 0) position.
+             */
+            while ((header_idx % 8) != 4)
                 header_idx++;
 
             if (CMD.custom_tlv[i].buffer == NULL) {
@@ -1099,7 +1102,10 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz,
     }
 
     /* Add padding bytes. Sha-3 val field requires 8-byte alignment */
-    while ((header_idx % 8) != 0)
+    /* The offset '4' takes into account 2B Tag + 2B Len, so that the Value 
+     * starts at (addr % 8 == 0) position.
+     */
+    while ((header_idx % 8) != 4)
         header_idx++;
 
     /* Calculate hashes */