From 6c148b2e9e1b5959ac70eb54b210f683d4b57ccc Mon Sep 17 00:00:00 2001 From: Brett Nicholas <7547222+bigbrett@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:15:10 -0600 Subject: [PATCH 01/10] increase custom TLV size limit to UINT16_MAX --- docs/Signing.md | 10 ++- tools/keytools/sign.c | 141 +++++++++++++++++++++++------------------- 2 files changed, 85 insertions(+), 66 deletions(-) diff --git a/docs/Signing.md b/docs/Signing.md index af21fa6855..8a5fe461a5 100644 --- a/docs/Signing.md +++ b/docs/Signing.md @@ -255,17 +255,23 @@ Provides a value to be set with a custom tag * `--custom-tlv-buffer tag value`: Adds a TLV entry with arbitrary length to the manifest header, corresponding to the type identified by `tag`, and assigns the value `value`. The tag is a 16-bit number. Valid tags are in the range between 0x0030 and 0xFEFE. The length - is implicit, and is the length of the value. + is implicit, and is the length of the value. The maximum length is 65535 bytes. Value argument is in the form of a hex string, e.g. `--custom-tlv-buffer 0x0030 AABBCCDDEE` will add a TLV entry with tag 0x0030, length 5 and value 0xAABBCCDDEE. * `--custom-tlv-string tag ascii-string`: Adds a TLV entry with arbitrary length to the manifest header, corresponding to the type identified by `tag`, and assigns the value of `ascii-string`. The tag is a 16-bit number. Valid tags are in the range between 0x0030 and 0xFEFE. The length - is implicit, and is the length of the `ascii-string`. `ascii-string` argument is in the form of a string, + is implicit, and is the length of the `ascii-string`. The maximum length is 65535 bytes. + `ascii-string` argument is in the form of a string, e.g. `--custom-tlv-string 0x0030 "Version-1"` will add a TLV entry with tag 0x0030, length 9 and value Version-1. + If the custom TLVs do not fit in the configured header size, the sign tool automatically + increases the size of the manifest header, rounding up to the next power of two. wolfBoot + must be built with a matching `IMAGE_HEADER_SIZE`, or it will fail to locate the firmware + image at boot. + #### Three-steps signing using external provisioning tools If the private key is not accessible, while it's possible to sign payloads using diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index 77b3c5389b..227f9b4949 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -1377,42 +1377,46 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, /* Check certificate chain file size before allocating header, and adjust * header size if needed */ - if (CMD.cert_chain_file != NULL) { - struct stat file_stat; - - /* Get the file size */ - if (stat(CMD.cert_chain_file, &file_stat) == 0) { - off_t chain_file_sz = file_stat.st_size; - uint32_t required_space; - - if ((chain_file_sz < 0) || - ((uintmax_t)chain_file_sz > (uintmax_t)UINT32_MAX)) { - printf("Warning: certificate chain file size is invalid (%jd)\n", - (intmax_t)chain_file_sz); + if ((CMD.cert_chain_file != NULL) || (CMD.custom_tlvs > 0)) { + uint32_t cert_chain_sz = 0; + uint32_t required_space; + + /* Get the certificate chain file size */ + if (CMD.cert_chain_file != NULL) { + struct stat file_stat; + if (stat(CMD.cert_chain_file, &file_stat) == 0) { + off_t chain_file_sz = file_stat.st_size; + if ((chain_file_sz < 0) || + ((uintmax_t)chain_file_sz > (uintmax_t)UINT32_MAX)) { + printf("Warning: certificate chain file size is invalid " + "(%jd)\n", (intmax_t)chain_file_sz); + } + else { + cert_chain_sz = (uint32_t)chain_file_sz; + } } else { - required_space = header_required_size(is_diff, - (uint32_t)chain_file_sz, secondary_key_sz); - - /* If the current header size is too small, increase it */ - if (CMD.header_sz < required_space) { - /* Round up to nearest power of 2 that can hold the chain */ - const uint32_t min_header_size = 256; - uint32_t new_size = min_header_size; - while (new_size < required_space) { - new_size *= 2; - } - - printf("Increasing header size from %u to %u bytes to fit " - "certificate chain\n", - CMD.header_sz, new_size); - CMD.header_sz = new_size; - } + printf("Warning: Could not stat certificate chain file %s: %s\n", + CMD.cert_chain_file, strerror(errno)); } } - else { - printf("Warning: Could not stat certificate chain file %s: %s\n", - CMD.cert_chain_file, strerror(errno)); + + required_space = header_required_size(is_diff, cert_chain_sz, + secondary_key_sz); + + /* If the current header size is too small, increase it */ + if (CMD.header_sz < required_space) { + /* Round up to nearest power of 2 that can hold all fields */ + const uint32_t min_header_size = 256; + uint32_t new_size = min_header_size; + while (new_size < required_space) { + new_size *= 2; + } + + printf("Increasing header size from %u to %u bytes to fit " + "manifest header fields\n", + CMD.header_sz, new_size); + CMD.header_sz = new_size; } } @@ -2530,35 +2534,37 @@ static int base_diff(const char *f_base, uint8_t *pubkey, uint32_t pubkey_sz, in len3++; } /* make_header_delta() below calls make_header_ex(is_diff=1), which may grow - * CMD.header_sz to fit the delta TLVs plus certificate chain. Resolve that - * expansion here, using the same logic, so patch_inv_off reflects the header - * size actually written; otherwise HDR_IMG_DELTA_INVERSE would encode a - * stale, too-small offset and break inverse-patch rollback. */ - if (CMD.cert_chain_file != NULL) { - struct stat cc_stat; - if ((stat(CMD.cert_chain_file, &cc_stat) == 0) && - (cc_stat.st_size >= 0)) { - if ((uintmax_t)cc_stat.st_size > (uintmax_t)UINT16_MAX) { - printf("Error: Certificate chain too large for TLV encoding " - "(%ju > %u)\n", (uintmax_t)cc_stat.st_size, UINT16_MAX); - goto cleanup; + * CMD.header_sz to fit the delta TLVs, custom TLVs and certificate chain. + * Resolve that expansion here, using the same logic, so patch_inv_off + * reflects the header size actually written; otherwise HDR_IMG_DELTA_INVERSE + * would encode a stale, too-small offset and break inverse-patch rollback. */ + if ((CMD.cert_chain_file != NULL) || (CMD.custom_tlvs > 0)) { + uint32_t cert_chain_sz = 0; + uint32_t required_space; + if (CMD.cert_chain_file != NULL) { + struct stat cc_stat; + if ((stat(CMD.cert_chain_file, &cc_stat) == 0) && + (cc_stat.st_size >= 0)) { + if ((uintmax_t)cc_stat.st_size > (uintmax_t)UINT16_MAX) { + printf("Error: Certificate chain too large for TLV encoding " + "(%ju > %u)\n", (uintmax_t)cc_stat.st_size, UINT16_MAX); + goto cleanup; + } + cert_chain_sz = (uint32_t)cc_stat.st_size; } - else { - uint32_t required_space = header_required_size(1, - (uint32_t)cc_stat.st_size, 0); - if (CMD.header_sz < required_space) { - uint32_t new_size = 256; - while (new_size < required_space) { - if (new_size > (UINT32_MAX / 2U)) { - printf("Error: Header size overflow while sizing " - "certificate chain\n"); - goto cleanup; - } - new_size *= 2; - } - CMD.header_sz = new_size; + } + required_space = header_required_size(1, cert_chain_sz, 0); + if (CMD.header_sz < required_space) { + uint32_t new_size = 256; + while (new_size < required_space) { + if (new_size > (UINT32_MAX / 2U)) { + printf("Error: Header size overflow while sizing " + "manifest header\n"); + goto cleanup; } + new_size *= 2; } + CMD.header_sz = new_size; } } patch_inv_off = (uint32_t)len3 + CMD.header_sz; @@ -3202,6 +3208,7 @@ int main(int argc, char** argv) } else if (strcmp(argv[i], "--custom-tlv-buffer") == 0) { int p = CMD.custom_tlvs; uint16_t tag, len; + size_t slen; uint32_t j; if (p >= MAX_CUSTOM_TLVS) { fprintf(stderr, "Too many custom TLVs.\n"); @@ -3212,7 +3219,7 @@ int main(int argc, char** argv) exit(16); } tag = (uint16_t)arg2num(argv[i + 1], 2); - len = (uint16_t)strlen(argv[i + 2]) / 2; + slen = strlen(argv[i + 2]); if (tag < 0x0030) { fprintf(stderr, "Invalid custom tag: %s\n", argv[i + 1]); exit(16); @@ -3221,10 +3228,13 @@ int main(int argc, char** argv) fprintf(stderr, "Invalid custom tag: %s\n", argv[i + 1]); exit(16); } - if (len > 255) { - fprintf(stderr, "custom tlv buffer size too big: %s\n", argv[i + 2]); + if ((slen / 2) > UINT16_MAX) { + fprintf(stderr, "custom tlv buffer size too big: " + "%lu bytes (max %u)\n", (unsigned long)(slen / 2), + UINT16_MAX); exit(16); } + len = (uint16_t)(slen / 2); CMD.custom_tlv[p].tag = tag; CMD.custom_tlv[p].len = len; CMD.custom_tlv[p].buffer = malloc(len); @@ -3241,6 +3251,7 @@ int main(int argc, char** argv) } else if (strcmp(argv[i], "--custom-tlv-string") == 0) { int p = CMD.custom_tlvs; uint16_t tag, len; + size_t slen; uint32_t j; if (p >= MAX_CUSTOM_TLVS) { fprintf(stderr, "Too many custom TLVs.\n"); @@ -3251,7 +3262,7 @@ int main(int argc, char** argv) exit(16); } tag = (uint16_t)arg2num(argv[i + 1], 2); - len = (uint16_t)strlen(argv[i + 2]); + slen = strlen(argv[i + 2]); if (tag < 0x0030) { fprintf(stderr, "Invalid custom tag: %s\n", argv[i + 1]); exit(16); @@ -3260,10 +3271,12 @@ int main(int argc, char** argv) fprintf(stderr, "Invalid custom tag: %s\n", argv[i + 1]); exit(16); } - if (len > 255) { - fprintf(stderr, "custom tlv buffer size too big: %s\n", argv[i + 2]); + if (slen > UINT16_MAX) { + fprintf(stderr, "custom tlv string size too big: " + "%lu bytes (max %u)\n", (unsigned long)slen, UINT16_MAX); exit(16); } + len = (uint16_t)slen; CMD.custom_tlv[p].tag = tag; CMD.custom_tlv[p].len = len; CMD.custom_tlv[p].buffer = malloc(len); From 22b2fe5dc99c804bcfe6b154d7ba43b7c7e0514c Mon Sep 17 00:00:00 2001 From: Brett Nicholas <7547222+bigbrett@users.noreply.github.com> Date: Tue, 21 Jul 2026 16:19:04 -0600 Subject: [PATCH 02/10] add custom file-backed TLV sign option --- docs/Signing.md | 7 +++++ tools/keytools/sign.c | 73 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/docs/Signing.md b/docs/Signing.md index 8a5fe461a5..7cf3219834 100644 --- a/docs/Signing.md +++ b/docs/Signing.md @@ -267,6 +267,13 @@ Provides a value to be set with a custom tag e.g. `--custom-tlv-string 0x0030 "Version-1"` will add a TLV entry with tag 0x0030, length 9 and value Version-1. + * `--custom-tlv-file tag filename`: Adds a TLV entry with arbitrary length to the manifest + header, corresponding to the type identified by `tag`, with the value read as raw bytes + from the file `filename`. The tag is a 16-bit number. Valid tags are in the range between + 0x0030 and 0xFEFE. The length is implicit, and is the size of the file. The maximum length + is 65535 bytes. Unlike `--custom-tlv-buffer`, the value is not passed on the command line, + so large binary values are not subject to the OS argument length limits. + If the custom TLVs do not fit in the configured header size, the sign tool automatically increases the size of the manifest header, rounding up to the next power of two. wolfBoot must be built with a matching `IMAGE_HEADER_SIZE`, or it will fail to locate the firmware diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index 227f9b4949..e73919b212 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -3289,6 +3289,68 @@ int main(int argc, char** argv) } CMD.custom_tlvs++; i += 2; + } else if (strcmp(argv[i], "--custom-tlv-file") == 0) { + int p = CMD.custom_tlvs; + uint16_t tag; + FILE *f; + long fsz; + size_t rd; + if (p >= MAX_CUSTOM_TLVS) { + fprintf(stderr, "Too many custom TLVs.\n"); + exit(16); + } + if (argc < (i + 2)) { + fprintf(stderr, "Invalid custom TLV fields. \n"); + exit(16); + } + tag = (uint16_t)arg2num(argv[i + 1], 2); + if (tag < 0x0030) { + fprintf(stderr, "Invalid custom tag: %s\n", argv[i + 1]); + exit(16); + } + if ( ((tag & 0xFF00) == 0xFF00) || ((tag & 0xFF) == 0xFF) ) { + fprintf(stderr, "Invalid custom tag: %s\n", argv[i + 1]); + exit(16); + } + f = fopen(argv[i + 2], "rb"); + if (f == NULL) { + fprintf(stderr, "Cannot open custom tlv file %s: %s\n", + argv[i + 2], strerror(errno)); + exit(16); + } + fseek(f, 0, SEEK_END); + fsz = ftell(f); + fseek(f, 0, SEEK_SET); + if (fsz <= 0) { + fprintf(stderr, "custom tlv file is empty or unreadable: %s\n", + argv[i + 2]); + fclose(f); + exit(16); + } + if (fsz > UINT16_MAX) { + fprintf(stderr, "custom tlv file too big: %ld bytes " + "(max %u): %s\n", fsz, UINT16_MAX, argv[i + 2]); + fclose(f); + exit(16); + } + CMD.custom_tlv[p].tag = tag; + CMD.custom_tlv[p].len = (uint16_t)fsz; + CMD.custom_tlv[p].buffer = malloc((size_t)fsz); + if (CMD.custom_tlv[p].buffer == NULL) { + fprintf(stderr, "Error malloc for custom tlv buffer %ld\n", + fsz); + fclose(f); + exit(16); + } + rd = fread(CMD.custom_tlv[p].buffer, 1, (size_t)fsz, f); + fclose(f); + if (rd != (size_t)fsz) { + fprintf(stderr, "Error reading custom tlv file %s\n", + argv[i + 2]); + exit(16); + } + CMD.custom_tlvs++; + i += 2; } else if (strcmp(argv[i], "--cert-chain") == 0) { if (argc <= (i + 1)) { @@ -3408,11 +3470,20 @@ int main(int argc, char** argv) printf("TLV %u\n", i); printf("----\n"); if (CMD.custom_tlv[i].buffer) { + /* only print the first 256 bytes of large values */ + uint16_t print_len = CMD.custom_tlv[i].len; + if (print_len > 256) { + print_len = 256; + } printf("Tag: %04X Len: %hu Val: ", CMD.custom_tlv[i].tag, CMD.custom_tlv[i].len); - for (j = 0; j < CMD.custom_tlv[i].len; j++) { + for (j = 0; j < print_len; j++) { printf("%02X", CMD.custom_tlv[i].buffer[j]); } + if (print_len < CMD.custom_tlv[i].len) { + printf("... (truncated, %hu bytes total)", + CMD.custom_tlv[i].len); + } printf("\n"); } else { From 0d5eec0d6f30b952458d65ac071272fabc02f5e8 Mon Sep 17 00:00:00 2001 From: Brett Nicholas <7547222+bigbrett@users.noreply.github.com> Date: Tue, 21 Jul 2026 16:32:56 -0600 Subject: [PATCH 03/10] add test coverage for large and file-backed tlvs --- .../workflows/test-custom-tlv-simulator.yml | 48 +++ tools/unit-tests/Makefile | 1 + .../unit-tests/unit-sign-custom-tlv-large.py | 392 ++++++++++++++++++ 3 files changed, 441 insertions(+) create mode 100644 tools/unit-tests/unit-sign-custom-tlv-large.py diff --git a/.github/workflows/test-custom-tlv-simulator.yml b/.github/workflows/test-custom-tlv-simulator.yml index be94d6d000..712d498479 100644 --- a/.github/workflows/test-custom-tlv-simulator.yml +++ b/.github/workflows/test-custom-tlv-simulator.yml @@ -48,3 +48,51 @@ jobs: - name: Run get_tlv simulator test run: | [ x`./wolfboot.elf get_tlv 2>/dev/null| tail -1` = xAABBCCDDEEFF0011223344 ] + + custom_tlv_large_simulator_tests: + runs-on: ubuntu-latest + container: + image: ghcr.io/wolfssl/wolfboot-ci-sim:v1.0 + timeout-minutes: 15 + + steps: + - uses: actions/checkout@v4 + with: + submodules: true + + - name: Trust workspace + run: git config --global --add safe.directory "$GITHUB_WORKSPACE" + + - 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 with a 2KB header + run: | + make clean && make IMAGE_HEADER_SIZE=2048 + + - name: Sign the image with 300-byte buffer and file custom TLVs + run: | + VAL=`printf 'A5%.0s' $(seq 1 300)` + head -c 300 /dev/zero | tr '\0' '\245' > large_tlv.bin + IMAGE_HEADER_SIZE=2048 tools/keytools/sign --ed25519 --custom-tlv-buffer 0x0034 $VAL --custom-tlv-file 0x0035 large_tlv.bin 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 IMAGE_HEADER_SIZE=2048 + + - name: Run get_tlv simulator test on the 300-byte buffer TLV + run: | + [ x`./wolfboot.elf get_tlv 2>/dev/null| tail -1` = x`printf 'A5%.0s' $(seq 1 300)` ] + + - name: Run get_tlv simulator test on the 300-byte file TLV (tag 0x35) + run: | + [ x`./wolfboot.elf get_tlv=53 2>/dev/null| tail -1` = x`printf 'A5%.0s' $(seq 1 300)` ] diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 3a1b466086..5374d28a80 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -129,6 +129,7 @@ run: $(TESTS) python3 unit-sign-delta-tlv.py || exit 1 python3 unit-sign-delta-cert-inv-off.py || exit 1 python3 unit-sign-custom-tlv-le.py || exit 1 + python3 unit-sign-custom-tlv-large.py || exit 1 WOLFCRYPT_SRC:=$(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha.c \ diff --git a/tools/unit-tests/unit-sign-custom-tlv-large.py b/tools/unit-tests/unit-sign-custom-tlv-large.py new file mode 100644 index 0000000000..57938dfb72 --- /dev/null +++ b/tools/unit-tests/unit-sign-custom-tlv-large.py @@ -0,0 +1,392 @@ +#!/usr/bin/env python3 +# unit-sign-custom-tlv-large.py +# +# Coverage for large custom TLVs in the C sign tool (tools/keytools/sign.c). +# +# Custom TLV values passed with --custom-tlv-buffer and --custom-tlv-string +# were historically capped at 255 bytes. The cap is now the TLV wire-format +# limit of 65535 bytes (16-bit length field), --custom-tlv-file loads a value +# from a raw binary file, and make_header_ex() grows the manifest header to +# the next power of two when the custom TLVs do not fit. The delta signing +# path (base_diff()) pre-computes the same growth before capturing +# patch_inv_off, so HDR_IMG_DELTA_INVERSE matches the header actually written. +# +# This test drives the C sign binary and asserts: +# - a >255-byte --custom-tlv-buffer, --custom-tlv-string and +# --custom-tlv-file value each land byte-exact in the manifest header, +# parsed the same way wolfBoot_find_header() walks it +# - the header grows to a power of two and the firmware payload starts +# exactly at the grown header size (header size derived from +# filesize - payload size, not from the tool's stdout) +# - the 65535-byte format maximum is accepted via --custom-tlv-file +# - a 65536-byte file, a 65536-char string, an empty file and a missing +# file are all rejected without producing a signed image +# - a delta image signed with a large custom TLV keeps +# HDR_IMG_DELTA_INVERSE consistent with the grown header: the inverse +# patch must be the trailing HDR_IMG_DELTA_INVERSE_SIZE bytes of the +# file, which fails if base_diff() sizes the header differently from +# make_header_ex() +# +# Copyright (C) 2026 wolfSSL Inc. +# +# This file is part of wolfBoot. +# +# wolfBoot is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# wolfBoot is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +import os +import struct +import subprocess +import sys +import tempfile + +HDR_PADDING = 0xFF +HDR_IMG_DELTA_SIZE = 0x06 +HDR_IMG_DELTA_INVERSE = 0x15 +HDR_IMG_DELTA_INVERSE_SIZE = 0x16 + +TAG_BUFFER = 0x0030 +TAG_STRING = 0x0031 +TAG_FILE = 0x0032 + +SECTOR_SIZE = 0x1000 + +THIS_DIR = os.path.dirname(os.path.abspath(__file__)) +ROOT = os.path.abspath(os.path.join(THIS_DIR, "..", "..")) +SIGN = os.path.join(ROOT, "tools", "keytools", "sign") + +failures = [] + + +def skip(msg): + print("SKIP unit-sign-custom-tlv-large: " + msg) + sys.exit(0) + + +def fail(msg): + failures.append(msg) + + +def parse_tlvs(data, scan_end): + """Walk the header like wolfBoot_find_header(): {tag: value bytes}.""" + tlvs = {} + p = 8 # skip 4-byte magic + 4-byte image size + while p + 4 <= scan_end: + htype = data[p] | (data[p + 1] << 8) + if htype == 0: + break + if data[p] == HDR_PADDING or (p & 1) != 0: + p += 1 + continue + length = data[p + 2] | (data[p + 3] << 8) + if p + 4 + length > scan_end: + break + if htype not in tlvs: + tlvs[htype] = bytes(data[p + 4:p + 4 + length]) + p += 4 + length + return tlvs + + +def tlv_u32(tlvs, tag): + val = tlvs.get(tag) + if val is None or len(val) != 4: + return None + return struct.unpack(" Date: Tue, 21 Jul 2026 17:34:26 -0600 Subject: [PATCH 04/10] fix sign tool arg index error --- tools/keytools/sign.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index e73919b212..6f4da9dd39 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -1351,7 +1351,7 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, uint8_t *base_hash, uint32_t base_hash_sz) { uint32_t header_idx; - uint8_t *header; + uint8_t *header = NULL; FILE *f = NULL, *f2 = NULL, *fek = NULL, *fef = NULL; uint32_t fw_version32; struct stat attrib; @@ -1386,11 +1386,15 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, struct stat file_stat; if (stat(CMD.cert_chain_file, &file_stat) == 0) { off_t chain_file_sz = file_stat.st_size; - if ((chain_file_sz < 0) || - ((uintmax_t)chain_file_sz > (uintmax_t)UINT32_MAX)) { + if (chain_file_sz < 0) { printf("Warning: certificate chain file size is invalid " "(%jd)\n", (intmax_t)chain_file_sz); } + else if ((uintmax_t)chain_file_sz > (uintmax_t)UINT16_MAX) { + printf("Error: Certificate chain too large for TLV encoding " + "(%ju > %u)\n", (uintmax_t)chain_file_sz, UINT16_MAX); + goto failure; + } else { cert_chain_sz = (uint32_t)chain_file_sz; } @@ -1410,6 +1414,11 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, const uint32_t min_header_size = 256; uint32_t new_size = min_header_size; while (new_size < required_space) { + if (new_size > (UINT32_MAX / 2U)) { + printf("Error: Header size overflow while sizing " + "manifest header\n"); + goto failure; + } new_size *= 2; } @@ -3177,7 +3186,7 @@ int main(int argc, char** argv) fprintf(stderr, "Too many custom TLVs.\n"); exit(16); } - if (argc < (i + 3)) { + if (argc < (i + 4)) { fprintf(stderr, "Invalid custom TLV fields. \n"); exit(16); } @@ -3214,7 +3223,7 @@ int main(int argc, char** argv) fprintf(stderr, "Too many custom TLVs.\n"); exit(16); } - if (argc < (i + 2)) { + if (argc < (i + 3)) { fprintf(stderr, "Invalid custom TLV fields. \n"); exit(16); } @@ -3234,6 +3243,11 @@ int main(int argc, char** argv) UINT16_MAX); exit(16); } + if ((slen % 2) != 0) { + fprintf(stderr, "custom tlv buffer hex string must have an " + "even number of digits: %s\n", argv[i + 2]); + exit(16); + } len = (uint16_t)(slen / 2); CMD.custom_tlv[p].tag = tag; CMD.custom_tlv[p].len = len; @@ -3257,7 +3271,7 @@ int main(int argc, char** argv) fprintf(stderr, "Too many custom TLVs.\n"); exit(16); } - if (argc < (i + 2)) { + if (argc < (i + 3)) { fprintf(stderr, "Invalid custom TLV fields. \n"); exit(16); } @@ -3299,7 +3313,7 @@ int main(int argc, char** argv) fprintf(stderr, "Too many custom TLVs.\n"); exit(16); } - if (argc < (i + 2)) { + if (argc < (i + 3)) { fprintf(stderr, "Invalid custom TLV fields. \n"); exit(16); } From 8c476ae3a0a32d37dca6a98a9c2c7851d2d8a5ec Mon Sep 17 00:00:00 2001 From: Brett Nicholas <7547222+bigbrett@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:05:22 -0600 Subject: [PATCH 05/10] fixed aliased variable --- tools/keytools/sign.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index 6f4da9dd39..f61323f348 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -1378,7 +1378,7 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, /* Check certificate chain file size before allocating header, and adjust * header size if needed */ if ((CMD.cert_chain_file != NULL) || (CMD.custom_tlvs > 0)) { - uint32_t cert_chain_sz = 0; + uint32_t hdr_cert_chain_sz = 0; uint32_t required_space; /* Get the certificate chain file size */ @@ -1396,7 +1396,7 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, goto failure; } else { - cert_chain_sz = (uint32_t)chain_file_sz; + hdr_cert_chain_sz = (uint32_t)chain_file_sz; } } else { @@ -1405,8 +1405,8 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, } } - required_space = header_required_size(is_diff, cert_chain_sz, - secondary_key_sz); + required_space = + header_required_size(is_diff, hdr_cert_chain_sz, secondary_key_sz); /* If the current header size is too small, increase it */ if (CMD.header_sz < required_space) { From 95e90f2e916c3998cacd063faac651c052ae8df1 Mon Sep 17 00:00:00 2001 From: Brett Nicholas <7547222+bigbrett@users.noreply.github.com> Date: Wed, 22 Jul 2026 09:42:45 -0600 Subject: [PATCH 06/10] revined comment --- docs/Signing.md | 9 ++-- tools/keytools/sign.c | 44 ++++++++++++------ ...unit-sign-custom-tlv-large.cpython-310.pyc | Bin 0 -> 10202 bytes .../unit-tests/unit-sign-custom-tlv-large.py | 36 ++++++++------ 4 files changed, 57 insertions(+), 32 deletions(-) create mode 100644 tools/unit-tests/__pycache__/unit-sign-custom-tlv-large.cpython-310.pyc diff --git a/docs/Signing.md b/docs/Signing.md index 7cf3219834..3e3377b6ae 100644 --- a/docs/Signing.md +++ b/docs/Signing.md @@ -255,14 +255,14 @@ Provides a value to be set with a custom tag * `--custom-tlv-buffer tag value`: Adds a TLV entry with arbitrary length to the manifest header, corresponding to the type identified by `tag`, and assigns the value `value`. The tag is a 16-bit number. Valid tags are in the range between 0x0030 and 0xFEFE. The length - is implicit, and is the length of the value. The maximum length is 65535 bytes. + is implicit, and is the length of the value. The maximum length is 65524 bytes. Value argument is in the form of a hex string, e.g. `--custom-tlv-buffer 0x0030 AABBCCDDEE` will add a TLV entry with tag 0x0030, length 5 and value 0xAABBCCDDEE. * `--custom-tlv-string tag ascii-string`: Adds a TLV entry with arbitrary length to the manifest header, corresponding to the type identified by `tag`, and assigns the value of `ascii-string`. The tag is a 16-bit number. Valid tags are in the range between 0x0030 and 0xFEFE. The length - is implicit, and is the length of the `ascii-string`. The maximum length is 65535 bytes. + is implicit, and is the length of the `ascii-string`. The maximum length is 65524 bytes. `ascii-string` argument is in the form of a string, e.g. `--custom-tlv-string 0x0030 "Version-1"` will add a TLV entry with tag 0x0030, length 9 and value Version-1. @@ -271,9 +271,12 @@ Provides a value to be set with a custom tag header, corresponding to the type identified by `tag`, with the value read as raw bytes from the file `filename`. The tag is a 16-bit number. Valid tags are in the range between 0x0030 and 0xFEFE. The length is implicit, and is the size of the file. The maximum length - is 65535 bytes. Unlike `--custom-tlv-buffer`, the value is not passed on the command line, + is 65524 bytes. Unlike `--custom-tlv-buffer`, the value is not passed on the command line, so large binary values are not subject to the OS argument length limits. + The 65524-byte maximum is the largest TLV value the wolfBoot header parser can walk + past when locating the fields that follow it, such as the signature. + If the custom TLVs do not fit in the configured header size, the sign tool automatically increases the size of the manifest header, rounding up to the next power of two. wolfBoot must be built with a matching `IMAGE_HEADER_SIZE`, or it will fail to locate the firmware diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index f61323f348..84a168b219 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -78,6 +78,13 @@ static inline int fp_truncate(FILE *f, size_t len) #define MAX_CUSTOM_TLVS (16) #endif +/* The header parsers in wolfBoot and this tool bound each field to + * (uint16_t)(header size - IMAGE_HEADER_OFFSET), at most 65528 including the + * 4-byte tag and length, so the largest value they can walk past is 65524. + * A longer field ends the walk and hides the fields after it, including the + * signature. */ +#define MAX_TLV_LEN (65524) + #include #include #include @@ -1390,9 +1397,9 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, printf("Warning: certificate chain file size is invalid " "(%jd)\n", (intmax_t)chain_file_sz); } - else if ((uintmax_t)chain_file_sz > (uintmax_t)UINT16_MAX) { + else if ((uintmax_t)chain_file_sz > (uintmax_t)MAX_TLV_LEN) { printf("Error: Certificate chain too large for TLV encoding " - "(%ju > %u)\n", (uintmax_t)chain_file_sz, UINT16_MAX); + "(%ju > %u)\n", (uintmax_t)chain_file_sz, MAX_TLV_LEN); goto failure; } else { @@ -1422,9 +1429,11 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, new_size *= 2; } - printf("Increasing header size from %u to %u bytes to fit " - "manifest header fields\n", - CMD.header_sz, new_size); + fprintf(stderr, "Warning: increasing header size from %u to %u " + "bytes to fit manifest header fields.\n" + "Warning: wolfBoot must be built with IMAGE_HEADER_SIZE=%u " + "or it will not find the firmware image.\n", + CMD.header_sz, new_size, new_size); CMD.header_sz = new_size; } } @@ -1585,10 +1594,10 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, } cert_chain_sz = (uint32_t)file_stat.st_size; - if (cert_chain_sz > (uint32_t)UINT16_MAX) { + if (cert_chain_sz > (uint32_t)MAX_TLV_LEN) { printf("Error: Certificate chain too large for TLV encoding " "(%u > %u)\n", - cert_chain_sz, (unsigned int)UINT16_MAX); + cert_chain_sz, (unsigned int)MAX_TLV_LEN); fclose(f); f = NULL; goto failure; @@ -2554,9 +2563,9 @@ static int base_diff(const char *f_base, uint8_t *pubkey, uint32_t pubkey_sz, in struct stat cc_stat; if ((stat(CMD.cert_chain_file, &cc_stat) == 0) && (cc_stat.st_size >= 0)) { - if ((uintmax_t)cc_stat.st_size > (uintmax_t)UINT16_MAX) { + if ((uintmax_t)cc_stat.st_size > (uintmax_t)MAX_TLV_LEN) { printf("Error: Certificate chain too large for TLV encoding " - "(%ju > %u)\n", (uintmax_t)cc_stat.st_size, UINT16_MAX); + "(%ju > %u)\n", (uintmax_t)cc_stat.st_size, MAX_TLV_LEN); goto cleanup; } cert_chain_sz = (uint32_t)cc_stat.st_size; @@ -2573,6 +2582,11 @@ static int base_diff(const char *f_base, uint8_t *pubkey, uint32_t pubkey_sz, in } new_size *= 2; } + fprintf(stderr, "Warning: increasing header size from %u to %u " + "bytes to fit manifest header fields.\n" + "Warning: wolfBoot must be built with IMAGE_HEADER_SIZE=%u " + "or it will not find the firmware image.\n", + CMD.header_sz, new_size, new_size); CMD.header_sz = new_size; } } @@ -3237,10 +3251,10 @@ int main(int argc, char** argv) fprintf(stderr, "Invalid custom tag: %s\n", argv[i + 1]); exit(16); } - if ((slen / 2) > UINT16_MAX) { + if ((slen / 2) > MAX_TLV_LEN) { fprintf(stderr, "custom tlv buffer size too big: " "%lu bytes (max %u)\n", (unsigned long)(slen / 2), - UINT16_MAX); + MAX_TLV_LEN); exit(16); } if ((slen % 2) != 0) { @@ -3285,9 +3299,9 @@ int main(int argc, char** argv) fprintf(stderr, "Invalid custom tag: %s\n", argv[i + 1]); exit(16); } - if (slen > UINT16_MAX) { + if (slen > MAX_TLV_LEN) { fprintf(stderr, "custom tlv string size too big: " - "%lu bytes (max %u)\n", (unsigned long)slen, UINT16_MAX); + "%lu bytes (max %u)\n", (unsigned long)slen, MAX_TLV_LEN); exit(16); } len = (uint16_t)slen; @@ -3341,9 +3355,9 @@ int main(int argc, char** argv) fclose(f); exit(16); } - if (fsz > UINT16_MAX) { + if (fsz > (long)MAX_TLV_LEN) { fprintf(stderr, "custom tlv file too big: %ld bytes " - "(max %u): %s\n", fsz, UINT16_MAX, argv[i + 2]); + "(max %u): %s\n", fsz, MAX_TLV_LEN, argv[i + 2]); fclose(f); exit(16); } diff --git a/tools/unit-tests/__pycache__/unit-sign-custom-tlv-large.cpython-310.pyc b/tools/unit-tests/__pycache__/unit-sign-custom-tlv-large.cpython-310.pyc new file mode 100644 index 0000000000000000000000000000000000000000..f55b8ba788bf56f1b76371a002dbbf85a3d1b2f7 GIT binary patch literal 10202 zcmcIqTW=gkcJABE^bCjhJ4Mm03oVYMp)R(3ktJCdQZm z_mE<5GrJ3VA2xwuB|s7*2omzqPXY3pKalG~5+n~DAPEp82!Q|r;_N^}R#dEiPCU8Od&H&Ly?vB>Nh5ZBFzLgVUbq3q8F|6^;RR&)>Xb7fHiemn{YqVhM_Kz78zGW zA7Lg6>Tp<-pCO8KW_bOfU0$_u@bB`2FXxkofc#pK$-G_NVTy1YnJt*A2F*P+adnZnap}Ayi zFgvhg(=XbU0z?=aP+LD%t8rpE*sMBnvg}lrg5|?f%~WmfI|Ty1k2(CaC=AU|v+9s) zs+wwa4*fU1N7X(9U&*dtp)|}dEkI=d%jn_b7|@~xL@F&oXaSjQr0hTzyXuw}!MD{E zzY~1cwjmuzw401C(00J@NZZvIbOXFJX8ABiexY5NktS!v`jP{K!y@9OAMjc+h?BJn ztZFr<^J9RGb?`IR!Oz1>Okr__+DjK*K>5VsjM}FfYM09M_$cCO`K9qCX#W6}Kv{vC z2;e)pN!}{#3yc-$7t@zC(U%NT%wUOKonbxrOMaYCm4`~D-B18&2I-QvnN#8GROS27 z#W`i;08Y}H+4&S?+HVa3G_7AK0!PH{EvDB9HkCR*@nLA4e@y3V8E zb}kcZ9z0Oh4wmC2{1BW*Y|PEvnT<1kZL!L|qT~Cq&TEy}Sn*tx5ANNYk9%&}<+8)> z%l>K3Jszj-&56nZ?vX3w!uH#oichnXFRG%|C(oP(6E!shzmrrmU-jroRol;M{3^Oy zd=PDV#CE~d-$f=fG-AP0c1i6Zx zU;X4kq&REsjV%aU`EZeu7 zM^&d7pmlN6BKAy5g;nG3X*uvdch~K}dE0>=#O=PrUAydtcHnxI+r9;M-29ebtZgjL z&ga_W3q`(J4ZJ08SC=;@m+f%P4kob^t+|1_?)a0ozqz*N1l%o7)`{lVfIlAl=5hQ@ z0!mLmDmqo7D()$9Z><2tM0Y_rm^dSsTmTp1Y`rdf%;`$eV{Qe;_rTtWv-bsWZhIWm z#fFFdBu;D)Uvnw`GG-Dq$vF*TXRP_Fu@026S~{S7LhriBG79y@(A*Ia_{ULbT2}4C zLrQ08QV9*HUzeJ}ch^^j#>t-*X+LWGND=7~ia3P|g#%1m(kUc}$j9qT*pr}0YM`y? z+bYz^;6qR)s2r(LkXT7Zi69jv8TKJ&e4Kym-8Iy2E|QioZh`^x_(eQ?PP){`JE=G^w35^J9Se6ax&wXbn76CinUKXln7`wBr!p z1fLLY2@g)3iqrvRm-ae+_6uWj(XE7?h4pb?V0RcbK*dvtYRZUqavFaRWBK=}5U%8a zKOuS?R;bu(4tWF-Qoteq8tN^i=xAvA4&p`tVwegW@1X4ju?#jAVc)Rsx_x5Srd;%RMO2&YQZ5f472H-UcUm^|g_W++nc#|k;PhbLLC&5w( zy-b!SNVx&axiMx5V=|HQ1fFn~NCDbH_kn{AsbWn~+;O%Z@jw$oCue!wn)NEq8;v+< z4S}?^NRGAYZ6Gf4N>;GpWi0%by)yX@5jE9?U~zY8Ik4;!Ou=%4$ynoyv%;%#is8dI z$NkGz$t^qZx1nQ=Gb?X3hT`KYwYa`j@qj~4nDcc}U z#E!DhDS9lDc{hG!=1r#kHa=kLWQ^KYGEi3#(MCx`ja@iq8dRg*L+&wvF}6}sY9)yX zSmP%VL4tjCBO|sZ&}ZU2@;Z93H_7jv^e!053^LP_E};+1a|f_+2T!mFrYEJ{gf9|T zNxdw;RxD!Eb=cV_kqdWL5W-+L2~QGSgIU(8asL#F#t(3q;b$%I?y@!R@BJATD-A)8 zgxyHofm<$Hh&oxVNVZh5WV9;nC6^KIUF>->5V!<96vda_%Bt9f;tUSVa4Lu=93fAN z^=a-nftcT|V)GP|eHR1e=JJqgw8O;73YJxNIHM2sYZlWkS5=S{|0#g{Q4|^21O*TD zrY+G>2VgN2MnL2r0)Xag{>UCFOuvQ4{}c)kDBFskBHIud1qgjtd2D<#v_-*xVh8>f z83t`jMaTdMe-x0g4XA=ihlY=oAdPrcor2Fsu%R`_q%cMgGKgQbZA8UE+Y*kR*hfq# z4nmQYEK9HI{#ydRJ?Owu)ZB)(`tJaL2K}8oUAv7ehlx~=oGII!DZ$!zl1Tc<_DC6FSwTs|R$ML3d=bUhHV?Tj?mh(-Wl*@4#YD`=n)0YjC3v7&2HHj?8jp ztUu@t`f$Kb?DRsx2k_O8uOu5pZ2&cMDGlgDYv+F%46>odN?{hmWEL{7Q+YKQVk6)R z5@g%7i|Si#QQKpcYf;+|w5{w`d(_U3AQka%z%d%N*G*|S%0}%l-i}~|jYJ(mUzFH3 zz$$xOLFpsCeJ#XJhQOkF7T|Yg=d{#<5~KG@>umxKb338 zzQ(l%ce)U~evLa=>-$*S@n`U71pGPi6#mp2{K1+}1a*D{15u|~WkZ4|57E+SA4hvP z*4ZuA*(r5{(tdr76Ie4stH`zPw5)ZMO*CcR-B=aNZ6_gvQFgka8SD&UJk;Ny;VfwA z0S!oBZfiS7cX7VM*nAUjPjlR49q+kDUsGP^cj-VNc*-si6uTIV1(2+(ZuK^yUcyX$ z(Br;_9>YTWNK2z$_VUMxCsz7cIJj8oYg&Dr~REOv!m1)JEu z`Z;vuP0Z8J-XI%#CPmV=H2%w|ja?sBw%Wn{0d_;gyW|~?A>JKCyxS^!&Huv|*$+p9 zP20N(+iO_Xanw)1CWiv5okVR|)W%U8iH0aL!8-mr8gAA`WR3qV)^>|LZRKP%6b%n5 z!Hd!0HWE&d$G@X*idtn`V>3lddo(hrtO6(hC19pyOQRpY@pj$L?=s>;72F`WyN?^W0jc^Shw%JI|2b-!!Ru`~RR-qtB&Py8jyov`Y6s1zmTa zLD$cqRo{JXtupx0x>g;1F0Io2KX2hGv?@5oW}~Bhb?I+o&k_Fpp3t!)!mm9=$LgA; z`@g7bmd?Mxs_qMYI)UDI2q*N!;Ayl!fSw#}#Kojn2eb`q0K^=%wrbngY{Kv-TBx4@ z_8Y;(l=8#$)>x2Z^U;{x-yRzuUJOp}oMG2RP?=-ziim<*($pdXzxQUt6Gg9vqX`{@ z7HckQ@_GT;16iSvEi@_1Gl>u@oTzRF%U;Dgm@Bf@5X#}4C5}?-NdMc5Wv3;#N2vmx zq9J-Hp!zA!=b#wpa;EetOueqW2QT~~Zdxd9utxzir$MtK3ja)twaN3b>K=~C(^r<9 z3PLu1?e7RP9aglTj40vYn(bC5uav!_UG}d{HlbWPb%$+5uM%*tOqqySy&%qvVSl|* z#;A0O;jqirGgtAsg}KBk+McXp5OG{{B|27O?6m`Ij}A@;mcxk+hnB+E(e_VxK9`Fa zQ{4Xm$Kuu;X(Qz#_-FyTK><=!B7-m>qMH_8-T>Ghorq_Vb)*f>Np{~_Ml`+c(M*wzkXq^RR1OJZ6I|klLfgr zVf#hb{WdOe(q@rxcpfhNs>y}_*(WZn*^k7k+fXUEYfIu|93S;#m|TNZLZ;-EOP5}{ zB-73Fci+2g;Udv~6e5eZA_kcrQhikW94mwbe?j=WM^=Mb-_PLeOx z%4I28o6yLq^MoNhEryBuWlN%fP<*S-4~sJ6ja0?rL0GzzoB8gY!qoKL`5T3q+4rU& z%uQcKw&DzE2py^&yDG9)bv=Ya1~z%6h>om2%;ogL9x2SzkxO zP^t<@Nju7R)hAWjusN%DPSnu~0+1y6Hf`S%w@*UlpTp+A4^@;lZ^4y?sl|#&{)QRJ z5-A5+f2y$CWS@|Bc%%U++1;uZbx0{Px)KZAO zaaP%@EFt9x!?77s3BF~wNN15!RW`>hP+dF_>RF4FsSbwk+#42m@YfI=g&7S zTj)VpY?=T6No$VXzA3Z9Q&q!bGY|*?yR`S6aP>Un*p2yxth3V9yO1H2~#dd z&dB4N2l6?ugTdm~g^!^YC%8@g{D4~Dr=m;+C4FQfXpL$WDw_FtBs-l)ac2I;ox;s` zZ{MDNAnSAU59qGjr1sl0cc*2pkJ6ap`dQ@WaQ`XuMwGR|wQ1S&aaRIa#%%|j68|9$ zTBL$5f8+kAN+2G2YO`=_aZhU>xjK>7h2O@uNB_6Nvs6%k!0Ear_d;yY|A26jtSX=- zPSs-)UKL;14GTy=#wn01DkgDT*68w@zGOz2Zd7oe>-b!3(hff5?KkZ>asX9?u+rok zWHlqayTtPjp)kk=$2u;Y=q9REiW5xy|3Ip)+HsO9ei$c98;ss$;}Snr1pef5L3mP1 zmmUgw=t|qI`~^Dvb@+-T5~*3O1Er}BXdMP++(waD9ngo6Z|gwrEsKnG2lCdqzQdpP zRZ=&;%ow8>g?~5LPiiz$X6mSB?00Abx*Ru#`jhKFy4E-M=`Nbmw}i2SE(+B#WWlpE z@+-pt1l}D+2eN&QHo~s$o0{=uHbvRM9-;_ikBeEUC;3%YA3>X8^x)Q8+Xt0M3nz6+ z*Vujr>m%AT>KKjJfLl*~*_z-@6aN1I6A7&?z)s(qnJY}qJmA73(6ybo^pwWY zOP_;Oj8j2oD1FU+stHFy*MQP*2pgx=yGX`1S41;oMcvf#r0BtCY+jLvmTMF)`BNy& xNzxgiH{hE1lW(mLcO*Tkp_#gwFb#9qOqtnCx0yygVfNzPsr=ZyjjZtg{{kDXw8H=Z literal 0 HcmV?d00001 diff --git a/tools/unit-tests/unit-sign-custom-tlv-large.py b/tools/unit-tests/unit-sign-custom-tlv-large.py index 57938dfb72..a05243c6c3 100644 --- a/tools/unit-tests/unit-sign-custom-tlv-large.py +++ b/tools/unit-tests/unit-sign-custom-tlv-large.py @@ -4,12 +4,14 @@ # Coverage for large custom TLVs in the C sign tool (tools/keytools/sign.c). # # Custom TLV values passed with --custom-tlv-buffer and --custom-tlv-string -# were historically capped at 255 bytes. The cap is now the TLV wire-format -# limit of 65535 bytes (16-bit length field), --custom-tlv-file loads a value -# from a raw binary file, and make_header_ex() grows the manifest header to -# the next power of two when the custom TLVs do not fit. The delta signing -# path (base_diff()) pre-computes the same growth before capturing -# patch_inv_off, so HDR_IMG_DELTA_INVERSE matches the header actually written. +# were historically capped at 255 bytes. The cap is now 65524 bytes, the +# largest value the header parsers can walk past; a longer value hides +# every field after it, including the signature, from wolfBoot. +# --custom-tlv-file loads a value from a raw binary file, and +# make_header_ex() grows the manifest header to the next power of two when +# the custom TLVs do not fit. The delta signing path (base_diff()) +# pre-computes the same growth before capturing patch_inv_off, so +# HDR_IMG_DELTA_INVERSE matches the header actually written. # # This test drives the C sign binary and asserts: # - a >255-byte --custom-tlv-buffer, --custom-tlv-string and @@ -18,8 +20,9 @@ # - the header grows to a power of two and the firmware payload starts # exactly at the grown header size (header size derived from # filesize - payload size, not from the tool's stdout) -# - the 65535-byte format maximum is accepted via --custom-tlv-file -# - a 65536-byte file, a 65536-char string, an empty file and a missing +# - the 65524-byte maximum is accepted via --custom-tlv-file and every +# header field is still reachable by the parser walk +# - a 65525-byte file, a 65525-char string, an empty file and a missing # file are all rejected without producing a signed image # - a delta image signed with a large custom TLV keeps # HDR_IMG_DELTA_INVERSE consistent with the grown header: the inverse @@ -86,6 +89,11 @@ def parse_tlvs(data, scan_end): p += 1 continue length = data[p + 2] | (data[p + 3] << 8) + # wolfBoot_find_header() stops at any field larger than the header + # capacity truncated to uint16_t, so this walk must stop too or the + # test would accept images wolfBoot cannot read + if 4 + length > (scan_end - 8) & 0xFFFF: + break if p + 4 + length > scan_end: break if htype not in tlvs: @@ -239,8 +247,8 @@ def main(): str_val.encode("ascii")) check_value("large", tlvs, TAG_FILE, file_val) - # Format maximum: 65535 bytes via file. - max_val = bytes((i * 3 + 1) & 0xFF for i in range(65535)) + # Largest value the header parsers can walk past (see parse_tlvs). + max_val = bytes((i * 3 + 1) & 0xFF for i in range(65524)) max_file = os.path.join(work, "max.bin") with open(max_file, "wb") as f: f.write(max_val) @@ -263,13 +271,13 @@ def main(): over_file = os.path.join(work, "over.bin") with open(over_file, "wb") as f: - f.write(b"\x00" * 65536) - expect_reject("reject-file-65536", + f.write(b"\x00" * 65525) + expect_reject("reject-file-65525", ["--custom-tlv-file", hex(TAG_FILE), over_file], image, key, "1", "too big") - expect_reject("reject-string-65536", - ["--custom-tlv-string", hex(TAG_STRING), "X" * 65536], + expect_reject("reject-string-65525", + ["--custom-tlv-string", hex(TAG_STRING), "X" * 65525], image, key, "1", "too big") empty_file = os.path.join(work, "empty.bin") From 1db68ec858bbfc06c02d7bd806c65985c2b2e121 Mon Sep 17 00:00:00 2001 From: Brett Nicholas <7547222+bigbrett@users.noreply.github.com> Date: Wed, 22 Jul 2026 09:50:35 -0600 Subject: [PATCH 07/10] remove erroneously committed pycache object --- .../unit-sign-custom-tlv-large.cpython-310.pyc | Bin 10202 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 tools/unit-tests/__pycache__/unit-sign-custom-tlv-large.cpython-310.pyc diff --git a/tools/unit-tests/__pycache__/unit-sign-custom-tlv-large.cpython-310.pyc b/tools/unit-tests/__pycache__/unit-sign-custom-tlv-large.cpython-310.pyc deleted file mode 100644 index f55b8ba788bf56f1b76371a002dbbf85a3d1b2f7..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 10202 zcmcIqTW=gkcJABE^bCjhJ4Mm03oVYMp)R(3ktJCdQZm z_mE<5GrJ3VA2xwuB|s7*2omzqPXY3pKalG~5+n~DAPEp82!Q|r;_N^}R#dEiPCU8Od&H&Ly?vB>Nh5ZBFzLgVUbq3q8F|6^;RR&)>Xb7fHiemn{YqVhM_Kz78zGW zA7Lg6>Tp<-pCO8KW_bOfU0$_u@bB`2FXxkofc#pK$-G_NVTy1YnJt*A2F*P+adnZnap}Ayi zFgvhg(=XbU0z?=aP+LD%t8rpE*sMBnvg}lrg5|?f%~WmfI|Ty1k2(CaC=AU|v+9s) zs+wwa4*fU1N7X(9U&*dtp)|}dEkI=d%jn_b7|@~xL@F&oXaSjQr0hTzyXuw}!MD{E zzY~1cwjmuzw401C(00J@NZZvIbOXFJX8ABiexY5NktS!v`jP{K!y@9OAMjc+h?BJn ztZFr<^J9RGb?`IR!Oz1>Okr__+DjK*K>5VsjM}FfYM09M_$cCO`K9qCX#W6}Kv{vC z2;e)pN!}{#3yc-$7t@zC(U%NT%wUOKonbxrOMaYCm4`~D-B18&2I-QvnN#8GROS27 z#W`i;08Y}H+4&S?+HVa3G_7AK0!PH{EvDB9HkCR*@nLA4e@y3V8E zb}kcZ9z0Oh4wmC2{1BW*Y|PEvnT<1kZL!L|qT~Cq&TEy}Sn*tx5ANNYk9%&}<+8)> z%l>K3Jszj-&56nZ?vX3w!uH#oichnXFRG%|C(oP(6E!shzmrrmU-jroRol;M{3^Oy zd=PDV#CE~d-$f=fG-AP0c1i6Zx zU;X4kq&REsjV%aU`EZeu7 zM^&d7pmlN6BKAy5g;nG3X*uvdch~K}dE0>=#O=PrUAydtcHnxI+r9;M-29ebtZgjL z&ga_W3q`(J4ZJ08SC=;@m+f%P4kob^t+|1_?)a0ozqz*N1l%o7)`{lVfIlAl=5hQ@ z0!mLmDmqo7D()$9Z><2tM0Y_rm^dSsTmTp1Y`rdf%;`$eV{Qe;_rTtWv-bsWZhIWm z#fFFdBu;D)Uvnw`GG-Dq$vF*TXRP_Fu@026S~{S7LhriBG79y@(A*Ia_{ULbT2}4C zLrQ08QV9*HUzeJ}ch^^j#>t-*X+LWGND=7~ia3P|g#%1m(kUc}$j9qT*pr}0YM`y? z+bYz^;6qR)s2r(LkXT7Zi69jv8TKJ&e4Kym-8Iy2E|QioZh`^x_(eQ?PP){`JE=G^w35^J9Se6ax&wXbn76CinUKXln7`wBr!p z1fLLY2@g)3iqrvRm-ae+_6uWj(XE7?h4pb?V0RcbK*dvtYRZUqavFaRWBK=}5U%8a zKOuS?R;bu(4tWF-Qoteq8tN^i=xAvA4&p`tVwegW@1X4ju?#jAVc)Rsx_x5Srd;%RMO2&YQZ5f472H-UcUm^|g_W++nc#|k;PhbLLC&5w( zy-b!SNVx&axiMx5V=|HQ1fFn~NCDbH_kn{AsbWn~+;O%Z@jw$oCue!wn)NEq8;v+< z4S}?^NRGAYZ6Gf4N>;GpWi0%by)yX@5jE9?U~zY8Ik4;!Ou=%4$ynoyv%;%#is8dI z$NkGz$t^qZx1nQ=Gb?X3hT`KYwYa`j@qj~4nDcc}U z#E!DhDS9lDc{hG!=1r#kHa=kLWQ^KYGEi3#(MCx`ja@iq8dRg*L+&wvF}6}sY9)yX zSmP%VL4tjCBO|sZ&}ZU2@;Z93H_7jv^e!053^LP_E};+1a|f_+2T!mFrYEJ{gf9|T zNxdw;RxD!Eb=cV_kqdWL5W-+L2~QGSgIU(8asL#F#t(3q;b$%I?y@!R@BJATD-A)8 zgxyHofm<$Hh&oxVNVZh5WV9;nC6^KIUF>->5V!<96vda_%Bt9f;tUSVa4Lu=93fAN z^=a-nftcT|V)GP|eHR1e=JJqgw8O;73YJxNIHM2sYZlWkS5=S{|0#g{Q4|^21O*TD zrY+G>2VgN2MnL2r0)Xag{>UCFOuvQ4{}c)kDBFskBHIud1qgjtd2D<#v_-*xVh8>f z83t`jMaTdMe-x0g4XA=ihlY=oAdPrcor2Fsu%R`_q%cMgGKgQbZA8UE+Y*kR*hfq# z4nmQYEK9HI{#ydRJ?Owu)ZB)(`tJaL2K}8oUAv7ehlx~=oGII!DZ$!zl1Tc<_DC6FSwTs|R$ML3d=bUhHV?Tj?mh(-Wl*@4#YD`=n)0YjC3v7&2HHj?8jp ztUu@t`f$Kb?DRsx2k_O8uOu5pZ2&cMDGlgDYv+F%46>odN?{hmWEL{7Q+YKQVk6)R z5@g%7i|Si#QQKpcYf;+|w5{w`d(_U3AQka%z%d%N*G*|S%0}%l-i}~|jYJ(mUzFH3 zz$$xOLFpsCeJ#XJhQOkF7T|Yg=d{#<5~KG@>umxKb338 zzQ(l%ce)U~evLa=>-$*S@n`U71pGPi6#mp2{K1+}1a*D{15u|~WkZ4|57E+SA4hvP z*4ZuA*(r5{(tdr76Ie4stH`zPw5)ZMO*CcR-B=aNZ6_gvQFgka8SD&UJk;Ny;VfwA z0S!oBZfiS7cX7VM*nAUjPjlR49q+kDUsGP^cj-VNc*-si6uTIV1(2+(ZuK^yUcyX$ z(Br;_9>YTWNK2z$_VUMxCsz7cIJj8oYg&Dr~REOv!m1)JEu z`Z;vuP0Z8J-XI%#CPmV=H2%w|ja?sBw%Wn{0d_;gyW|~?A>JKCyxS^!&Huv|*$+p9 zP20N(+iO_Xanw)1CWiv5okVR|)W%U8iH0aL!8-mr8gAA`WR3qV)^>|LZRKP%6b%n5 z!Hd!0HWE&d$G@X*idtn`V>3lddo(hrtO6(hC19pyOQRpY@pj$L?=s>;72F`WyN?^W0jc^Shw%JI|2b-!!Ru`~RR-qtB&Py8jyov`Y6s1zmTa zLD$cqRo{JXtupx0x>g;1F0Io2KX2hGv?@5oW}~Bhb?I+o&k_Fpp3t!)!mm9=$LgA; z`@g7bmd?Mxs_qMYI)UDI2q*N!;Ayl!fSw#}#Kojn2eb`q0K^=%wrbngY{Kv-TBx4@ z_8Y;(l=8#$)>x2Z^U;{x-yRzuUJOp}oMG2RP?=-ziim<*($pdXzxQUt6Gg9vqX`{@ z7HckQ@_GT;16iSvEi@_1Gl>u@oTzRF%U;Dgm@Bf@5X#}4C5}?-NdMc5Wv3;#N2vmx zq9J-Hp!zA!=b#wpa;EetOueqW2QT~~Zdxd9utxzir$MtK3ja)twaN3b>K=~C(^r<9 z3PLu1?e7RP9aglTj40vYn(bC5uav!_UG}d{HlbWPb%$+5uM%*tOqqySy&%qvVSl|* z#;A0O;jqirGgtAsg}KBk+McXp5OG{{B|27O?6m`Ij}A@;mcxk+hnB+E(e_VxK9`Fa zQ{4Xm$Kuu;X(Qz#_-FyTK><=!B7-m>qMH_8-T>Ghorq_Vb)*f>Np{~_Ml`+c(M*wzkXq^RR1OJZ6I|klLfgr zVf#hb{WdOe(q@rxcpfhNs>y}_*(WZn*^k7k+fXUEYfIu|93S;#m|TNZLZ;-EOP5}{ zB-73Fci+2g;Udv~6e5eZA_kcrQhikW94mwbe?j=WM^=Mb-_PLeOx z%4I28o6yLq^MoNhEryBuWlN%fP<*S-4~sJ6ja0?rL0GzzoB8gY!qoKL`5T3q+4rU& z%uQcKw&DzE2py^&yDG9)bv=Ya1~z%6h>om2%;ogL9x2SzkxO zP^t<@Nju7R)hAWjusN%DPSnu~0+1y6Hf`S%w@*UlpTp+A4^@;lZ^4y?sl|#&{)QRJ z5-A5+f2y$CWS@|Bc%%U++1;uZbx0{Px)KZAO zaaP%@EFt9x!?77s3BF~wNN15!RW`>hP+dF_>RF4FsSbwk+#42m@YfI=g&7S zTj)VpY?=T6No$VXzA3Z9Q&q!bGY|*?yR`S6aP>Un*p2yxth3V9yO1H2~#dd z&dB4N2l6?ugTdm~g^!^YC%8@g{D4~Dr=m;+C4FQfXpL$WDw_FtBs-l)ac2I;ox;s` zZ{MDNAnSAU59qGjr1sl0cc*2pkJ6ap`dQ@WaQ`XuMwGR|wQ1S&aaRIa#%%|j68|9$ zTBL$5f8+kAN+2G2YO`=_aZhU>xjK>7h2O@uNB_6Nvs6%k!0Ear_d;yY|A26jtSX=- zPSs-)UKL;14GTy=#wn01DkgDT*68w@zGOz2Zd7oe>-b!3(hff5?KkZ>asX9?u+rok zWHlqayTtPjp)kk=$2u;Y=q9REiW5xy|3Ip)+HsO9ei$c98;ss$;}Snr1pef5L3mP1 zmmUgw=t|qI`~^Dvb@+-T5~*3O1Er}BXdMP++(waD9ngo6Z|gwrEsKnG2lCdqzQdpP zRZ=&;%ow8>g?~5LPiiz$X6mSB?00Abx*Ru#`jhKFy4E-M=`Nbmw}i2SE(+B#WWlpE z@+-pt1l}D+2eN&QHo~s$o0{=uHbvRM9-;_ikBeEUC;3%YA3>X8^x)Q8+Xt0M3nz6+ z*Vujr>m%AT>KKjJfLl*~*_z-@6aN1I6A7&?z)s(qnJY}qJmA73(6ybo^pwWY zOP_;Oj8j2oD1FU+stHFy*MQP*2pgx=yGX`1S41;oMcvf#r0BtCY+jLvmTMF)`BNy& xNzxgiH{hE1lW(mLcO*Tkp_#gwFb#9qOqtnCx0yygVfNzPsr=ZyjjZtg{{kDXw8H=Z From 3df56e136ec2dac31c8dfd9a30eee0a7ec19777f Mon Sep 17 00:00:00 2001 From: Brett Nicholas <7547222+bigbrett@users.noreply.github.com> Date: Wed, 22 Jul 2026 09:50:49 -0600 Subject: [PATCH 08/10] add __pycache__ to gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index b1bc7fb6ad..3139c130af 100644 --- a/.gitignore +++ b/.gitignore @@ -225,6 +225,7 @@ tools/unit-tests/unit-update-ram-enc tools/unit-tests/unit-update-ram-enc-nopart tools/unit-tests/unit-va416x0-fram tools/unit-tests/unit-wolfhsm_flash_hal +tools/unit-tests/__pycache__/* From f5877b4de69754de8a26db36a8d3d908e4b93a2e Mon Sep 17 00:00:00 2001 From: Brett Nicholas <7547222+bigbrett@users.noreply.github.com> Date: Wed, 22 Jul 2026 09:51:39 -0600 Subject: [PATCH 09/10] simplify comments --- tools/keytools/sign.c | 9 ++-- .../unit-tests/unit-sign-custom-tlv-large.py | 44 +++++-------------- 2 files changed, 13 insertions(+), 40 deletions(-) diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index 84a168b219..cae6099b64 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -78,11 +78,10 @@ static inline int fp_truncate(FILE *f, size_t len) #define MAX_CUSTOM_TLVS (16) #endif -/* The header parsers in wolfBoot and this tool bound each field to +/* wolfBoot and this tool stop parsing at any header field larger than * (uint16_t)(header size - IMAGE_HEADER_OFFSET), at most 65528 including the - * 4-byte tag and length, so the largest value they can walk past is 65524. - * A longer field ends the walk and hides the fields after it, including the - * signature. */ + * 4-byte tag and length, so the largest usable value is 65524. A longer + * field would hide every field after it, including the signature. */ #define MAX_TLV_LEN (65524) #include @@ -1388,7 +1387,6 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, uint32_t hdr_cert_chain_sz = 0; uint32_t required_space; - /* Get the certificate chain file size */ if (CMD.cert_chain_file != NULL) { struct stat file_stat; if (stat(CMD.cert_chain_file, &file_stat) == 0) { @@ -3498,7 +3496,6 @@ int main(int argc, char** argv) printf("TLV %u\n", i); printf("----\n"); if (CMD.custom_tlv[i].buffer) { - /* only print the first 256 bytes of large values */ uint16_t print_len = CMD.custom_tlv[i].len; if (print_len > 256) { print_len = 256; diff --git a/tools/unit-tests/unit-sign-custom-tlv-large.py b/tools/unit-tests/unit-sign-custom-tlv-large.py index a05243c6c3..e26dfee577 100644 --- a/tools/unit-tests/unit-sign-custom-tlv-large.py +++ b/tools/unit-tests/unit-sign-custom-tlv-large.py @@ -1,34 +1,12 @@ #!/usr/bin/env python3 # unit-sign-custom-tlv-large.py # -# Coverage for large custom TLVs in the C sign tool (tools/keytools/sign.c). -# -# Custom TLV values passed with --custom-tlv-buffer and --custom-tlv-string -# were historically capped at 255 bytes. The cap is now 65524 bytes, the -# largest value the header parsers can walk past; a longer value hides -# every field after it, including the signature, from wolfBoot. -# --custom-tlv-file loads a value from a raw binary file, and -# make_header_ex() grows the manifest header to the next power of two when -# the custom TLVs do not fit. The delta signing path (base_diff()) -# pre-computes the same growth before capturing patch_inv_off, so -# HDR_IMG_DELTA_INVERSE matches the header actually written. -# -# This test drives the C sign binary and asserts: -# - a >255-byte --custom-tlv-buffer, --custom-tlv-string and -# --custom-tlv-file value each land byte-exact in the manifest header, -# parsed the same way wolfBoot_find_header() walks it -# - the header grows to a power of two and the firmware payload starts -# exactly at the grown header size (header size derived from -# filesize - payload size, not from the tool's stdout) -# - the 65524-byte maximum is accepted via --custom-tlv-file and every -# header field is still reachable by the parser walk -# - a 65525-byte file, a 65525-char string, an empty file and a missing -# file are all rejected without producing a signed image -# - a delta image signed with a large custom TLV keeps -# HDR_IMG_DELTA_INVERSE consistent with the grown header: the inverse -# patch must be the trailing HDR_IMG_DELTA_INVERSE_SIZE bytes of the -# file, which fails if base_diff() sizes the header differently from -# make_header_ex() +# Tests large custom TLVs in the C sign tool (tools/keytools/sign.c): +# values up to the 65524-byte maximum via --custom-tlv-buffer, +# --custom-tlv-string and --custom-tlv-file, automatic power-of-two growth +# of the manifest header, rejection of oversized, empty or missing values, +# and delta images keeping HDR_IMG_DELTA_INVERSE consistent with the grown +# header. # # Copyright (C) 2026 wolfSSL Inc. # @@ -336,12 +314,10 @@ def main(): "forward patch (header %d + %d)" % (inv_off, hdr, fwd_sz)) - # Boundary delta: size the TLV so the non-delta header fits in a - # power of two but the extra delta TLVs push past it. The full v2 - # image is signed first, so base_diff() starts from the non-delta - # header size and must pre-grow it before capturing patch_inv_off; - # without the pre-grow, HDR_IMG_DELTA_INVERSE is stale by a full - # header step and no longer points at the trailing inverse patch. + # Boundary delta: size the TLV so the header fits a power of two + # without the delta TLVs but not with them. base_diff() must grow + # the header before capturing patch_inv_off, or HDR_IMG_DELTA_INVERSE + # points a full header step short of the trailing inverse patch. bnd_val = bytes((i * 11 + 3) & 0xFF for i in range(800)) bnd_file = os.path.join(work, "bnd.bin") with open(bnd_file, "wb") as f: From 7f96408b7ac8585275187cd3cb68a567bc31dbcf Mon Sep 17 00:00:00 2001 From: Brett Nicholas <7547222+bigbrett@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:24:59 -0600 Subject: [PATCH 10/10] fix bad max value in test print --- tools/unit-tests/unit-sign-custom-tlv-large.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/unit-tests/unit-sign-custom-tlv-large.py b/tools/unit-tests/unit-sign-custom-tlv-large.py index e26dfee577..e3b3536eba 100644 --- a/tools/unit-tests/unit-sign-custom-tlv-large.py +++ b/tools/unit-tests/unit-sign-custom-tlv-large.py @@ -235,7 +235,7 @@ def main(): r = run_sign(["--custom-tlv-file", hex(TAG_FILE), max_file], image, key, "1") if r.returncode != 0: - fail("max: sign failed for 65535-byte TLV: " + + fail("max: sign failed for 65524-byte TLV: " + r.stderr.strip()[:200]) else: res = check_signed_layout("max", signed_name(image, "1"), payload)