Fixes from peer review (Thank you Daniele and Marco)

pull/772/head
David Garske 2026-05-04 12:15:39 -07:00 committed by Daniele Lacamera
parent 85fb32b1dd
commit d92053037c
8 changed files with 134 additions and 257 deletions

5
.gitignore vendored
View File

@ -187,9 +187,7 @@ tools/unit-tests/unit-update-ram-nofixed
tools/unit-tests/unit-max-space
tools/unit-tests/unit-sdhci-disk-unaligned
tools/unit-tests/unit-fwtpm-stub
tools/unit-tests/unit-gzip
# Elf preprocessing tools
@ -380,4 +378,3 @@ image.ub
system-default.dtb
test_output/
sdcard.img

View File

@ -1289,8 +1289,11 @@ mkimage -f hal/mpfs250.its fitImage
```
At boot, wolfBoot decompresses the kernel into `0x80200000` directly out of
the FIT `data` blob and verifies the FIT `hash-1` SHA-256 against the
decompressed bytes (defense-in-depth on top of the outer wolfBoot signature).
the FIT `data` blob. Image integrity is provided by the outer wolfBoot
signature over the entire FIT (which covers the compressed `data` bytes per
the FIT spec), and post-decompress integrity by gzip's CRC32 + ISIZE
trailer; per-image `hash-1` subnodes are not re-verified at runtime since
they would be redundant with the outer signature.
##### Option B - Uncompressed FIT (`GZIP=0`)
@ -3500,8 +3503,9 @@ Compressed (gzip) ramdisks are supported transparently when `GZIP=1` is set
(the same gzip path used for the kernel handles `compression = "gzip"` on
the ramdisk node). The outer wolfBoot signature already authenticates the
entire FIT, so the ramdisk inherits authentication without per-image
hashing - though if the FIT does include a `hash-1` subnode under the
ramdisk image, wolfBoot will verify it after decompression.
hashing. Per-image `hash-1` subnodes (if present) are not re-verified at
runtime - per the FIT spec they hash the in-FIT `data` bytes, which the
outer wolfBoot signature already covers.
Example FIT layout:
@ -3743,8 +3747,10 @@ sf write ${loadaddr} 0x800000 ${filesize}
The compressed FIT is roughly half the size of the uncompressed equivalent
on a typical PetaLinux ARM64 kernel, which lets a larger kernel fit in the
existing 44 MB QSPI partition. wolfBoot decompresses to `0x00200000` at boot
and verifies the FIT `hash-1` SHA-256 against the decompressed bytes.
existing 44 MB QSPI partition. wolfBoot decompresses to `0x00200000` at boot.
Integrity is provided by the outer wolfBoot signature over the entire FIT
plus gzip's CRC32 + ISIZE trailer on the decompressed payload; per-image
`hash-1` subnodes are not re-verified at runtime.
##### Option B - Uncompressed kernel (`GZIP=0`)

View File

@ -178,6 +178,14 @@ void* fit_load_image_ex(void* fdt, const char* image, int* lenp, uint32_t out_ma
* or a negative FDT_ERR_*. */
int fdt_fixup_initrd(void* fdt, uint64_t start, uint64_t size);
#ifdef WOLFBOOT_FIT_RAMDISK
/* Load a FIT ramdisk subimage (optionally relocated to
* WOLFBOOT_LOAD_RAMDISK_ADDRESS) and patch /chosen/linux,initrd-*
* in the supplied DTB. Returns 0 on success, -1 on load failure.
* Callers typically ignore the return value (log-and-continue). */
int fit_load_ramdisk(void* fit, const char* ramdisk_node, void* dts_addr);
#endif
#ifdef __cplusplus
}
#endif

View File

@ -40,15 +40,14 @@
#define WOLFBOOT_GZIP_E_ISIZE -7 /* trailer ISIZE mismatch */
#define WOLFBOOT_GZIP_E_PARAM -8 /* invalid parameter */
/* RFC 1952 gzip wrapper constants */
/* RFC 1952 gzip wrapper constants (format-defining; useful to callers
* that pre-validate the gzip magic/method before invoking the inflater).
* All other DEFLATE-internal constants live in src/gzip.c. */
#define GZIP_MAGIC_ID1 0x1FU /* first magic byte */
#define GZIP_MAGIC_ID2 0x8BU /* second magic byte */
#define GZIP_CM_DEFLATE 8 /* CM = DEFLATE */
#define GZIP_HEADER_MIN_SIZE 10 /* magic+CM+FLG+MTIME+XFL+OS */
#define GZIP_TRAILER_SIZE 8 /* CRC32 + ISIZE */
#define GZIP_CRC32_INIT 0xFFFFFFFFU
#define GZIP_CRC32_FINAL_XOR 0xFFFFFFFFU
#define GZIP_CRC32_POLY 0xEDB88320U /* IEEE 802.3 reflected */
/* RFC 1952 Sec. 2.3.1 header flag bits (FLG byte) */
#define GZIP_FLG_FTEXT 0x01
@ -58,46 +57,6 @@
#define GZIP_FLG_FCOMMENT 0x10
#define GZIP_FLG_RESERVED 0xE0
/* RFC 1951 DEFLATE - alphabet sizes */
#define GZIP_MAX_HUFF_BITS 15 /* max Huffman code length */
#define GZIP_CL_CODES 19 /* code-length alphabet */
#define GZIP_LITLEN_CODES 288 /* literal/length alphabet */
#define GZIP_DIST_CODES 32 /* distance alphabet */
/* RFC 1951 DEFLATE - fixed Huffman boundaries (Sec. 3.2.6) */
#define GZIP_FIXED_LIT_END_8BIT 144 /* 0..143 -> 8 bits */
#define GZIP_FIXED_LIT_END_9BIT 256 /* 144..255 -> 9 bits */
#define GZIP_FIXED_LIT_END_7BIT 280 /* 256..279 -> 7 bits */
#define GZIP_FIXED_LIT_END 288 /* 280..287 -> 8 bits */
#define GZIP_FIXED_DIST_COUNT 30 /* 0..29 -> 5 bits */
/* RFC 1951 DEFLATE - alphabet bounds (Sec. 3.2.4 / 3.2.5) */
#define GZIP_EOB_SYMBOL 256 /* end-of-block marker */
#define GZIP_LENGTH_CODE_BASE 257 /* first length code */
#define GZIP_LENGTH_CODE_COUNT 29 /* 257..285 */
#define GZIP_DIST_CODE_COUNT 30 /* 0..29 */
/* RFC 1951 DEFLATE - dynamic block header (Sec. 3.2.7) */
#define GZIP_HLIT_BITS 5 /* HLIT field width */
#define GZIP_HDIST_BITS 5 /* HDIST field width */
#define GZIP_HCLEN_BITS 4 /* HCLEN field width */
#define GZIP_HLIT_BASE 257 /* HLIT + 257 */
#define GZIP_HDIST_BASE 1 /* HDIST + 1 */
#define GZIP_HCLEN_BASE 4 /* HCLEN + 4 */
#define GZIP_CL_LEN_BITS 3 /* code-length code is 3 bits */
/* RFC 1951 DEFLATE - run-length repeat symbols (Sec. 3.2.7).
* sym 16: 2 extra bits, repeat previous length 3..6 times
* sym 17: 3 extra bits, repeat zero 3..10 times
* sym 18: 7 extra bits, repeat zero 11..138 times
*/
#define GZIP_REPEAT_PREV_EXTRA 2
#define GZIP_REPEAT_PREV_BASE 3
#define GZIP_REPEAT_Z3_EXTRA 3
#define GZIP_REPEAT_Z3_BASE 3
#define GZIP_REPEAT_Z7_EXTRA 7
#define GZIP_REPEAT_Z7_BASE 11
/* Decompress a gzip stream.
*
* in - pointer to gzip stream (RFC 1952 wrapper around RFC 1951 DEFLATE)

196
src/fdt.c
View File

@ -32,13 +32,7 @@
#ifdef WOLFBOOT_GZIP
#include "gzip.h"
#ifdef WOLFBOOT_HASH_SHA256
#include <wolfssl/wolfcrypt/sha256.h>
#endif
#ifdef WOLFBOOT_HASH_SHA384
#include <wolfssl/wolfcrypt/sha512.h>
#endif
#endif /* WOLFBOOT_GZIP */
/* Default upper bound on a single FIT subimage's decompressed size.
* The outer wolfBoot signature already authenticates the FIT, but a
@ -902,142 +896,68 @@ int fdt_fixup_initrd(void* fdt, uint64_t start, uint64_t size)
return 0;
}
#ifdef WOLFBOOT_GZIP
/* Verify FIT per-subimage hash-1 subnode against the loaded/decompressed
* image bytes. This is defense-in-depth: the outer wolfBoot signature
* already authenticates the entire FIT blob, but recomputing the per-image
* hash catches inflater bugs and corrupt streams that still parse.
#ifdef WOLFBOOT_FIT_RAMDISK
/* Defensive fallback: targets without a fixed relocation address
* leave WOLFBOOT_LOAD_RAMDISK_ADDRESS at 0, in which case the
* ramdisk is used in place. */
#ifndef WOLFBOOT_LOAD_RAMDISK_ADDRESS
#define WOLFBOOT_LOAD_RAMDISK_ADDRESS 0
#endif
/* Load a FIT ramdisk subimage and patch the DTB's /chosen
* linux,initrd-{start,end} to point at it. If
* WOLFBOOT_LOAD_RAMDISK_ADDRESS is nonzero, the ramdisk is relocated
* to that fixed address (overrides the FIT's `load` property);
* otherwise the address fit_load_image returned (FIT-specified or
* in-FIT pointer) is used as-is. Caller passes the DTB pointer for
* the initrd fixup, or NULL to skip the fixup.
*
* Returns 0 on success or when no usable hash node is present.
* Returns negative on hash mismatch.
*
* If hash-1.algo names an algorithm that is not compiled into this build,
* a warning is printed and 0 is returned (best-effort verification). */
static int fit_verify_hash(const void *fdt, int img_off,
const uint8_t *data, uint32_t data_len)
* Returns 0 on success, -1 if the ramdisk node was found but the
* load failed. The current callers ignore the return value
* (log-and-continue), so a missing/failed ramdisk does not abort
* the boot. */
int fit_load_ramdisk(void* fit, const char* ramdisk_node, void* dts_addr)
{
int ret = 0;
int done = 0;
int hash_off, len = 0;
const char *algo = NULL;
const uint8_t *value = NULL;
#if defined(WOLFBOOT_HASH_SHA256) || defined(WOLFBOOT_HASH_SHA384)
int did_init = 0;
#endif
#ifdef WOLFBOOT_HASH_SHA256
wc_Sha256 sha256_ctx;
uint8_t sha256_digest[WC_SHA256_DIGEST_SIZE];
#endif
#ifdef WOLFBOOT_HASH_SHA384
wc_Sha384 sha384_ctx;
uint8_t sha384_digest[WC_SHA384_DIGEST_SIZE];
#endif
int rd_size = 0;
uint8_t *rd_ptr;
uint8_t *rd_dst;
hash_off = fdt_subnode_offset_namelen(fdt, img_off, "hash-1", 6);
if (hash_off < 0) {
done = 1; /* no hash-1 subnode; nothing to verify */
if (fit == NULL || ramdisk_node == NULL) {
return -1;
}
if (!done) {
algo = (const char*)fdt_getprop(fdt, hash_off, "algo", &len);
if (algo == NULL || len <= 0) {
wolfBoot_printf("FIT hash-1: missing algo\n");
done = 1;
}
rd_ptr = (uint8_t*)fit_load_image(fit, ramdisk_node, &rd_size);
if (rd_ptr == NULL || rd_size <= 0) {
wolfBoot_printf("FIT: ramdisk node present but load failed\n");
return -1;
}
if (!done) {
value = (const uint8_t*)fdt_getprop(fdt, hash_off, "value", &len);
if (value == NULL) {
/* mkimage emits the hash node but populates 'value' only after
* signing; an empty 'value' on an unsigned tree is benign. */
done = 1;
}
}
#ifdef WOLFBOOT_HASH_SHA256
if (!done && strcmp(algo, "sha256") == 0) {
if (len != WC_SHA256_DIGEST_SIZE) {
wolfBoot_printf("FIT hash-1: bad sha256 value len %d\n", len);
ret = -1;
}
if (ret == 0) {
ret = wc_InitSha256(&sha256_ctx);
if (ret == 0) {
did_init = 1;
}
}
if (ret == 0) {
ret = wc_Sha256Update(&sha256_ctx, data, (word32)data_len);
}
if (ret == 0) {
ret = wc_Sha256Final(&sha256_ctx, sha256_digest);
}
if (did_init) {
wc_Sha256Free(&sha256_ctx);
did_init = 0;
}
if (ret != 0) {
wolfBoot_printf("FIT hash-1 (sha256): wc_Sha256 failed rc=%d\n",
ret);
ret = -1;
}
else if (memcmp(sha256_digest, value, WC_SHA256_DIGEST_SIZE) != 0) {
wolfBoot_printf("FIT hash-1 (sha256): MISMATCH\n");
ret = -1;
if (WOLFBOOT_LOAD_RAMDISK_ADDRESS != 0) {
rd_dst = (uint8_t*)WOLFBOOT_LOAD_RAMDISK_ADDRESS;
if (rd_ptr != rd_dst) {
wolfBoot_printf("Loading ramdisk: %p -> %p (%d bytes)\n",
rd_ptr, rd_dst, rd_size);
memcpy(rd_dst, rd_ptr, rd_size);
}
else {
wolfBoot_printf("FIT hash-1 (sha256): OK\n");
wolfBoot_printf("Loaded ramdisk: %p (%d bytes)\n",
rd_dst, rd_size);
}
done = 1;
}
#endif
else {
rd_dst = rd_ptr;
wolfBoot_printf("Loaded ramdisk: %p (%d bytes)\n",
rd_dst, rd_size);
}
#ifdef WOLFBOOT_HASH_SHA384
if (!done && strcmp(algo, "sha384") == 0) {
if (len != WC_SHA384_DIGEST_SIZE) {
wolfBoot_printf("FIT hash-1: bad sha384 value len %d\n", len);
ret = -1;
}
if (ret == 0) {
ret = wc_InitSha384(&sha384_ctx);
if (ret == 0) {
did_init = 1;
}
}
if (ret == 0) {
ret = wc_Sha384Update(&sha384_ctx, data, (word32)data_len);
}
if (ret == 0) {
ret = wc_Sha384Final(&sha384_ctx, sha384_digest);
}
if (did_init) {
wc_Sha384Free(&sha384_ctx);
did_init = 0;
}
if (ret != 0) {
wolfBoot_printf("FIT hash-1 (sha384): wc_Sha384 failed rc=%d\n",
ret);
ret = -1;
}
else if (memcmp(sha384_digest, value, WC_SHA384_DIGEST_SIZE) != 0) {
wolfBoot_printf("FIT hash-1 (sha384): MISMATCH\n");
ret = -1;
}
else {
wolfBoot_printf("FIT hash-1 (sha384): OK\n");
}
done = 1;
if (dts_addr != NULL) {
(void)fdt_fixup_initrd(dts_addr,
(uint64_t)(uintptr_t)rd_dst, (uint64_t)rd_size);
}
#endif
if ((ret == 0) && !done) {
wolfBoot_printf("FIT hash-1: algo '%s' not built in, skipping\n",
algo);
}
return ret;
return 0;
}
#endif /* WOLFBOOT_GZIP */
#endif /* WOLFBOOT_FIT_RAMDISK */
void* fit_load_image_ex(void* fdt, const char* image, int* lenp,
uint32_t out_max)
@ -1112,16 +1032,18 @@ void* fit_load_image_ex(void* fdt, const char* image, int* lenp,
memcpy(load, data, len);
}
#ifdef WOLFBOOT_GZIP
/* Defense-in-depth: verify FIT hash-1 against loaded
* bytes */
if (fit_verify_hash(fdt, off, (const uint8_t*)load,
(uint32_t)len) != 0) {
wolfBoot_printf("FIT hash verification failed for "
"%s\n", image);
return NULL;
}
#endif
/* No per-image hash-1 re-verification here. Per the
* FIT spec (and U-Boot's reference implementation), a
* hash-N subnode's value is computed over the image
* node's `data` property bytes verbatim - which means
* the compressed bytes when compression="gzip". The
* outer wolfBoot signature
* (wolfBoot_verify_authenticity) already authenticates
* the entire FIT, including those data bytes, so a
* runtime per-image hash check would be redundant.
* Inflater bugs on the decompressed payload are
* caught by gzip's own CRC32 + ISIZE trailer inside
* wolfBoot_gunzip. */
/* load should always have entry, but if not use load
* address */

View File

@ -38,6 +38,55 @@
#include <stddef.h>
#include <stdint.h>
/* RFC 1951/1952 implementation-detail constants. These were previously
* in include/gzip.h but are not part of the public API of this module
* (no WOLFBOOT_ prefix and no out-of-file references). */
/* RFC 1952 CRC32 (IEEE 802.3 reflected) */
#define GZIP_CRC32_INIT 0xFFFFFFFFU
#define GZIP_CRC32_FINAL_XOR 0xFFFFFFFFU
#define GZIP_CRC32_POLY 0xEDB88320U
/* RFC 1951 DEFLATE - alphabet sizes */
#define GZIP_MAX_HUFF_BITS 15 /* max Huffman code length */
#define GZIP_CL_CODES 19 /* code-length alphabet */
#define GZIP_LITLEN_CODES 288 /* literal/length alphabet */
#define GZIP_DIST_CODES 32 /* distance alphabet */
/* RFC 1951 DEFLATE - fixed Huffman boundaries (Sec. 3.2.6) */
#define GZIP_FIXED_LIT_END_8BIT 144 /* 0..143 -> 8 bits */
#define GZIP_FIXED_LIT_END_9BIT 256 /* 144..255 -> 9 bits */
#define GZIP_FIXED_LIT_END_7BIT 280 /* 256..279 -> 7 bits */
#define GZIP_FIXED_LIT_END 288 /* 280..287 -> 8 bits */
#define GZIP_FIXED_DIST_COUNT 30 /* 0..29 -> 5 bits */
/* RFC 1951 DEFLATE - alphabet bounds (Sec. 3.2.4 / 3.2.5) */
#define GZIP_EOB_SYMBOL 256 /* end-of-block marker */
#define GZIP_LENGTH_CODE_BASE 257 /* first length code */
#define GZIP_LENGTH_CODE_COUNT 29 /* 257..285 */
#define GZIP_DIST_CODE_COUNT 30 /* 0..29 */
/* RFC 1951 DEFLATE - dynamic block header (Sec. 3.2.7) */
#define GZIP_HLIT_BITS 5 /* HLIT field width */
#define GZIP_HDIST_BITS 5 /* HDIST field width */
#define GZIP_HCLEN_BITS 4 /* HCLEN field width */
#define GZIP_HLIT_BASE 257 /* HLIT + 257 */
#define GZIP_HDIST_BASE 1 /* HDIST + 1 */
#define GZIP_HCLEN_BASE 4 /* HCLEN + 4 */
#define GZIP_CL_LEN_BITS 3 /* code-length code is 3 bits */
/* RFC 1951 DEFLATE - run-length repeat symbols (Sec. 3.2.7).
* sym 16: 2 extra bits, repeat previous length 3..6 times
* sym 17: 3 extra bits, repeat zero 3..10 times
* sym 18: 7 extra bits, repeat zero 11..138 times
*/
#define GZIP_REPEAT_PREV_EXTRA 2
#define GZIP_REPEAT_PREV_BASE 3
#define GZIP_REPEAT_Z3_EXTRA 3
#define GZIP_REPEAT_Z3_BASE 3
#define GZIP_REPEAT_Z7_EXTRA 7
#define GZIP_REPEAT_Z7_BASE 11
/* RFC 1951 Sec. 3.2.5: length codes 257..285 base values and extra bits */
static const uint16_t gz_len_base[29] = {
3, 4, 5, 6, 7, 8, 9, 10,

View File

@ -562,39 +562,7 @@ void RAMFUNCTION wolfBoot_start(void)
}
#ifdef WOLFBOOT_FIT_RAMDISK
if (ramdisk != NULL) {
int rd_size = 0;
uint8_t *rd_ptr = (uint8_t*)fit_load_image(fit, ramdisk, &rd_size);
if (rd_ptr != NULL && rd_size > 0) {
uint8_t *rd_dst;
/* If WOLFBOOT_LOAD_RAMDISK_ADDRESS is set (nonzero), use it
* as the canonical destination (overrides the FIT's `load`
* property). Otherwise honor whatever fit_load_image
* returned (FIT-specified load addr or in-FIT pointer). */
if (WOLFBOOT_LOAD_RAMDISK_ADDRESS != 0) {
rd_dst = (uint8_t*)WOLFBOOT_LOAD_RAMDISK_ADDRESS;
if (rd_ptr != rd_dst) {
wolfBoot_printf("Loading ramdisk: %p -> %p (%d bytes)\n",
rd_ptr, rd_dst, rd_size);
memcpy(rd_dst, rd_ptr, rd_size);
}
else {
wolfBoot_printf("Loaded ramdisk: %p (%d bytes)\n",
rd_dst, rd_size);
}
}
else {
rd_dst = rd_ptr;
wolfBoot_printf("Loaded ramdisk: %p (%d bytes)\n",
rd_dst, rd_size);
}
if (dts_addr != NULL) {
(void)fdt_fixup_initrd((void*)dts_addr,
(uint64_t)(uintptr_t)rd_dst, (uint64_t)rd_size);
}
}
else {
wolfBoot_printf("FIT: ramdisk node present but load failed\n");
}
(void)fit_load_ramdisk(fit, ramdisk, (void*)dts_addr);
}
#else
(void)ramdisk;

View File

@ -389,39 +389,7 @@ backup_on_failure:
}
#ifdef WOLFBOOT_FIT_RAMDISK
if (ramdisk != NULL) {
int rd_size = 0;
uint8_t *rd_ptr = (uint8_t*)fit_load_image(fit, ramdisk, &rd_size);
if (rd_ptr != NULL && rd_size > 0) {
uint8_t *rd_dst;
/* If WOLFBOOT_LOAD_RAMDISK_ADDRESS is set (nonzero), use it
* as the canonical destination (overrides the FIT's `load`
* property). Otherwise honor whatever fit_load_image
* returned (FIT-specified load addr or in-FIT pointer). */
if (WOLFBOOT_LOAD_RAMDISK_ADDRESS != 0) {
rd_dst = (uint8_t*)WOLFBOOT_LOAD_RAMDISK_ADDRESS;
if (rd_ptr != rd_dst) {
wolfBoot_printf("Loading ramdisk: %p -> %p (%d bytes)\n",
rd_ptr, rd_dst, rd_size);
memcpy(rd_dst, rd_ptr, rd_size);
}
else {
wolfBoot_printf("Loaded ramdisk: %p (%d bytes)\n",
rd_dst, rd_size);
}
}
else {
rd_dst = rd_ptr;
wolfBoot_printf("Loaded ramdisk: %p (%d bytes)\n",
rd_dst, rd_size);
}
if (dts_addr != NULL) {
(void)fdt_fixup_initrd((void*)dts_addr,
(uint64_t)(uintptr_t)rd_dst, (uint64_t)rd_size);
}
}
else {
wolfBoot_printf("FIT: ramdisk node present but load failed\n");
}
(void)fit_load_ramdisk(fit, ramdisk, (void*)dts_addr);
}
#else
(void)ramdisk;