Skip to content

Commit 323ab59

Browse files
committed
Fixes from peer review (Thank you Daniele and Marco)
1 parent 93e9cb6 commit 323ab59

8 files changed

Lines changed: 134 additions & 257 deletions

File tree

.gitignore

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,7 @@ tools/unit-tests/unit-update-ram-nofixed
187187
tools/unit-tests/unit-max-space
188188
tools/unit-tests/unit-sdhci-disk-unaligned
189189
tools/unit-tests/unit-fwtpm-stub
190-
191-
192-
190+
tools/unit-tests/unit-gzip
193191

194192

195193
# Elf preprocessing tools
@@ -380,4 +378,3 @@ image.ub
380378
system-default.dtb
381379
test_output/
382380
sdcard.img
383-

docs/Targets.md

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,8 +1289,11 @@ mkimage -f hal/mpfs250.its fitImage
12891289
```
12901290
12911291
At boot, wolfBoot decompresses the kernel into `0x80200000` directly out of
1292-
the FIT `data` blob and verifies the FIT `hash-1` SHA-256 against the
1293-
decompressed bytes (defense-in-depth on top of the outer wolfBoot signature).
1292+
the FIT `data` blob. Image integrity is provided by the outer wolfBoot
1293+
signature over the entire FIT (which covers the compressed `data` bytes per
1294+
the FIT spec), and post-decompress integrity by gzip's CRC32 + ISIZE
1295+
trailer; per-image `hash-1` subnodes are not re-verified at runtime since
1296+
they would be redundant with the outer signature.
12941297

12951298
##### Option B - Uncompressed FIT (`GZIP=0`)
12961299

@@ -3500,8 +3503,9 @@ Compressed (gzip) ramdisks are supported transparently when `GZIP=1` is set
35003503
(the same gzip path used for the kernel handles `compression = "gzip"` on
35013504
the ramdisk node). The outer wolfBoot signature already authenticates the
35023505
entire FIT, so the ramdisk inherits authentication without per-image
3503-
hashing - though if the FIT does include a `hash-1` subnode under the
3504-
ramdisk image, wolfBoot will verify it after decompression.
3506+
hashing. Per-image `hash-1` subnodes (if present) are not re-verified at
3507+
runtime - per the FIT spec they hash the in-FIT `data` bytes, which the
3508+
outer wolfBoot signature already covers.
35053509
35063510
Example FIT layout:
35073511
@@ -3743,8 +3747,10 @@ sf write ${loadaddr} 0x800000 ${filesize}
37433747
37443748
The compressed FIT is roughly half the size of the uncompressed equivalent
37453749
on a typical PetaLinux ARM64 kernel, which lets a larger kernel fit in the
3746-
existing 44 MB QSPI partition. wolfBoot decompresses to `0x00200000` at boot
3747-
and verifies the FIT `hash-1` SHA-256 against the decompressed bytes.
3750+
existing 44 MB QSPI partition. wolfBoot decompresses to `0x00200000` at boot.
3751+
Integrity is provided by the outer wolfBoot signature over the entire FIT
3752+
plus gzip's CRC32 + ISIZE trailer on the decompressed payload; per-image
3753+
`hash-1` subnodes are not re-verified at runtime.
37483754
37493755
##### Option B - Uncompressed kernel (`GZIP=0`)
37503756

include/fdt.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,14 @@ void* fit_load_image_ex(void* fdt, const char* image, int* lenp, uint32_t out_ma
178178
* or a negative FDT_ERR_*. */
179179
int fdt_fixup_initrd(void* fdt, uint64_t start, uint64_t size);
180180

181+
#ifdef WOLFBOOT_FIT_RAMDISK
182+
/* Load a FIT ramdisk subimage (optionally relocated to
183+
* WOLFBOOT_LOAD_RAMDISK_ADDRESS) and patch /chosen/linux,initrd-*
184+
* in the supplied DTB. Returns 0 on success, -1 on load failure.
185+
* Callers typically ignore the return value (log-and-continue). */
186+
int fit_load_ramdisk(void* fit, const char* ramdisk_node, void* dts_addr);
187+
#endif
188+
181189
#ifdef __cplusplus
182190
}
183191
#endif

include/gzip.h

Lines changed: 3 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,14 @@
4040
#define WOLFBOOT_GZIP_E_ISIZE -7 /* trailer ISIZE mismatch */
4141
#define WOLFBOOT_GZIP_E_PARAM -8 /* invalid parameter */
4242

43-
/* RFC 1952 gzip wrapper constants */
43+
/* RFC 1952 gzip wrapper constants (format-defining; useful to callers
44+
* that pre-validate the gzip magic/method before invoking the inflater).
45+
* All other DEFLATE-internal constants live in src/gzip.c. */
4446
#define GZIP_MAGIC_ID1 0x1FU /* first magic byte */
4547
#define GZIP_MAGIC_ID2 0x8BU /* second magic byte */
4648
#define GZIP_CM_DEFLATE 8 /* CM = DEFLATE */
4749
#define GZIP_HEADER_MIN_SIZE 10 /* magic+CM+FLG+MTIME+XFL+OS */
4850
#define GZIP_TRAILER_SIZE 8 /* CRC32 + ISIZE */
49-
#define GZIP_CRC32_INIT 0xFFFFFFFFU
50-
#define GZIP_CRC32_FINAL_XOR 0xFFFFFFFFU
51-
#define GZIP_CRC32_POLY 0xEDB88320U /* IEEE 802.3 reflected */
5251

5352
/* RFC 1952 Sec. 2.3.1 header flag bits (FLG byte) */
5453
#define GZIP_FLG_FTEXT 0x01
@@ -58,46 +57,6 @@
5857
#define GZIP_FLG_FCOMMENT 0x10
5958
#define GZIP_FLG_RESERVED 0xE0
6059

61-
/* RFC 1951 DEFLATE - alphabet sizes */
62-
#define GZIP_MAX_HUFF_BITS 15 /* max Huffman code length */
63-
#define GZIP_CL_CODES 19 /* code-length alphabet */
64-
#define GZIP_LITLEN_CODES 288 /* literal/length alphabet */
65-
#define GZIP_DIST_CODES 32 /* distance alphabet */
66-
67-
/* RFC 1951 DEFLATE - fixed Huffman boundaries (Sec. 3.2.6) */
68-
#define GZIP_FIXED_LIT_END_8BIT 144 /* 0..143 -> 8 bits */
69-
#define GZIP_FIXED_LIT_END_9BIT 256 /* 144..255 -> 9 bits */
70-
#define GZIP_FIXED_LIT_END_7BIT 280 /* 256..279 -> 7 bits */
71-
#define GZIP_FIXED_LIT_END 288 /* 280..287 -> 8 bits */
72-
#define GZIP_FIXED_DIST_COUNT 30 /* 0..29 -> 5 bits */
73-
74-
/* RFC 1951 DEFLATE - alphabet bounds (Sec. 3.2.4 / 3.2.5) */
75-
#define GZIP_EOB_SYMBOL 256 /* end-of-block marker */
76-
#define GZIP_LENGTH_CODE_BASE 257 /* first length code */
77-
#define GZIP_LENGTH_CODE_COUNT 29 /* 257..285 */
78-
#define GZIP_DIST_CODE_COUNT 30 /* 0..29 */
79-
80-
/* RFC 1951 DEFLATE - dynamic block header (Sec. 3.2.7) */
81-
#define GZIP_HLIT_BITS 5 /* HLIT field width */
82-
#define GZIP_HDIST_BITS 5 /* HDIST field width */
83-
#define GZIP_HCLEN_BITS 4 /* HCLEN field width */
84-
#define GZIP_HLIT_BASE 257 /* HLIT + 257 */
85-
#define GZIP_HDIST_BASE 1 /* HDIST + 1 */
86-
#define GZIP_HCLEN_BASE 4 /* HCLEN + 4 */
87-
#define GZIP_CL_LEN_BITS 3 /* code-length code is 3 bits */
88-
89-
/* RFC 1951 DEFLATE - run-length repeat symbols (Sec. 3.2.7).
90-
* sym 16: 2 extra bits, repeat previous length 3..6 times
91-
* sym 17: 3 extra bits, repeat zero 3..10 times
92-
* sym 18: 7 extra bits, repeat zero 11..138 times
93-
*/
94-
#define GZIP_REPEAT_PREV_EXTRA 2
95-
#define GZIP_REPEAT_PREV_BASE 3
96-
#define GZIP_REPEAT_Z3_EXTRA 3
97-
#define GZIP_REPEAT_Z3_BASE 3
98-
#define GZIP_REPEAT_Z7_EXTRA 7
99-
#define GZIP_REPEAT_Z7_BASE 11
100-
10160
/* Decompress a gzip stream.
10261
*
10362
* in - pointer to gzip stream (RFC 1952 wrapper around RFC 1951 DEFLATE)

src/fdt.c

Lines changed: 59 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,7 @@
3232

3333
#ifdef WOLFBOOT_GZIP
3434
#include "gzip.h"
35-
#ifdef WOLFBOOT_HASH_SHA256
36-
#include <wolfssl/wolfcrypt/sha256.h>
3735
#endif
38-
#ifdef WOLFBOOT_HASH_SHA384
39-
#include <wolfssl/wolfcrypt/sha512.h>
40-
#endif
41-
#endif /* WOLFBOOT_GZIP */
4236

4337
/* Default upper bound on a single FIT subimage's decompressed size.
4438
* 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)
902896
return 0;
903897
}
904898

905-
#ifdef WOLFBOOT_GZIP
906-
/* Verify FIT per-subimage hash-1 subnode against the loaded/decompressed
907-
* image bytes. This is defense-in-depth: the outer wolfBoot signature
908-
* already authenticates the entire FIT blob, but recomputing the per-image
909-
* hash catches inflater bugs and corrupt streams that still parse.
910-
*
911-
* Returns 0 on success or when no usable hash node is present.
912-
* Returns negative on hash mismatch.
913-
*
914-
* If hash-1.algo names an algorithm that is not compiled into this build,
915-
* a warning is printed and 0 is returned (best-effort verification). */
916-
static int fit_verify_hash(const void *fdt, int img_off,
917-
const uint8_t *data, uint32_t data_len)
918-
{
919-
int ret = 0;
920-
int done = 0;
921-
int hash_off, len = 0;
922-
const char *algo = NULL;
923-
const uint8_t *value = NULL;
924-
#if defined(WOLFBOOT_HASH_SHA256) || defined(WOLFBOOT_HASH_SHA384)
925-
int did_init = 0;
926-
#endif
927-
#ifdef WOLFBOOT_HASH_SHA256
928-
wc_Sha256 sha256_ctx;
929-
uint8_t sha256_digest[WC_SHA256_DIGEST_SIZE];
930-
#endif
931-
#ifdef WOLFBOOT_HASH_SHA384
932-
wc_Sha384 sha384_ctx;
933-
uint8_t sha384_digest[WC_SHA384_DIGEST_SIZE];
899+
#ifdef WOLFBOOT_FIT_RAMDISK
900+
/* Defensive fallback: targets without a fixed relocation address
901+
* leave WOLFBOOT_LOAD_RAMDISK_ADDRESS at 0, in which case the
902+
* ramdisk is used in place. */
903+
#ifndef WOLFBOOT_LOAD_RAMDISK_ADDRESS
904+
#define WOLFBOOT_LOAD_RAMDISK_ADDRESS 0
934905
#endif
935906

936-
hash_off = fdt_subnode_offset_namelen(fdt, img_off, "hash-1", 6);
937-
if (hash_off < 0) {
938-
done = 1; /* no hash-1 subnode; nothing to verify */
939-
}
907+
/* Load a FIT ramdisk subimage and patch the DTB's /chosen
908+
* linux,initrd-{start,end} to point at it. If
909+
* WOLFBOOT_LOAD_RAMDISK_ADDRESS is nonzero, the ramdisk is relocated
910+
* to that fixed address (overrides the FIT's `load` property);
911+
* otherwise the address fit_load_image returned (FIT-specified or
912+
* in-FIT pointer) is used as-is. Caller passes the DTB pointer for
913+
* the initrd fixup, or NULL to skip the fixup.
914+
*
915+
* Returns 0 on success, -1 if the ramdisk node was found but the
916+
* load failed. The current callers ignore the return value
917+
* (log-and-continue), so a missing/failed ramdisk does not abort
918+
* the boot. */
919+
int fit_load_ramdisk(void* fit, const char* ramdisk_node, void* dts_addr)
920+
{
921+
int rd_size = 0;
922+
uint8_t *rd_ptr;
923+
uint8_t *rd_dst;
940924

941-
if (!done) {
942-
algo = (const char*)fdt_getprop(fdt, hash_off, "algo", &len);
943-
if (algo == NULL || len <= 0) {
944-
wolfBoot_printf("FIT hash-1: missing algo\n");
945-
done = 1;
946-
}
925+
if (fit == NULL || ramdisk_node == NULL) {
926+
return -1;
947927
}
948928

949-
if (!done) {
950-
value = (const uint8_t*)fdt_getprop(fdt, hash_off, "value", &len);
951-
if (value == NULL) {
952-
/* mkimage emits the hash node but populates 'value' only after
953-
* signing; an empty 'value' on an unsigned tree is benign. */
954-
done = 1;
955-
}
929+
rd_ptr = (uint8_t*)fit_load_image(fit, ramdisk_node, &rd_size);
930+
if (rd_ptr == NULL || rd_size <= 0) {
931+
wolfBoot_printf("FIT: ramdisk node present but load failed\n");
932+
return -1;
956933
}
957934

958-
#ifdef WOLFBOOT_HASH_SHA256
959-
if (!done && strcmp(algo, "sha256") == 0) {
960-
if (len != WC_SHA256_DIGEST_SIZE) {
961-
wolfBoot_printf("FIT hash-1: bad sha256 value len %d\n", len);
962-
ret = -1;
963-
}
964-
if (ret == 0) {
965-
ret = wc_InitSha256(&sha256_ctx);
966-
if (ret == 0) {
967-
did_init = 1;
968-
}
969-
}
970-
if (ret == 0) {
971-
ret = wc_Sha256Update(&sha256_ctx, data, (word32)data_len);
972-
}
973-
if (ret == 0) {
974-
ret = wc_Sha256Final(&sha256_ctx, sha256_digest);
975-
}
976-
if (did_init) {
977-
wc_Sha256Free(&sha256_ctx);
978-
did_init = 0;
979-
}
980-
if (ret != 0) {
981-
wolfBoot_printf("FIT hash-1 (sha256): wc_Sha256 failed rc=%d\n",
982-
ret);
983-
ret = -1;
984-
}
985-
else if (memcmp(sha256_digest, value, WC_SHA256_DIGEST_SIZE) != 0) {
986-
wolfBoot_printf("FIT hash-1 (sha256): MISMATCH\n");
987-
ret = -1;
935+
if (WOLFBOOT_LOAD_RAMDISK_ADDRESS != 0) {
936+
rd_dst = (uint8_t*)WOLFBOOT_LOAD_RAMDISK_ADDRESS;
937+
if (rd_ptr != rd_dst) {
938+
wolfBoot_printf("Loading ramdisk: %p -> %p (%d bytes)\n",
939+
rd_ptr, rd_dst, rd_size);
940+
memcpy(rd_dst, rd_ptr, rd_size);
988941
}
989942
else {
990-
wolfBoot_printf("FIT hash-1 (sha256): OK\n");
943+
wolfBoot_printf("Loaded ramdisk: %p (%d bytes)\n",
944+
rd_dst, rd_size);
991945
}
992-
done = 1;
993946
}
994-
#endif
995-
996-
#ifdef WOLFBOOT_HASH_SHA384
997-
if (!done && strcmp(algo, "sha384") == 0) {
998-
if (len != WC_SHA384_DIGEST_SIZE) {
999-
wolfBoot_printf("FIT hash-1: bad sha384 value len %d\n", len);
1000-
ret = -1;
1001-
}
1002-
if (ret == 0) {
1003-
ret = wc_InitSha384(&sha384_ctx);
1004-
if (ret == 0) {
1005-
did_init = 1;
1006-
}
1007-
}
1008-
if (ret == 0) {
1009-
ret = wc_Sha384Update(&sha384_ctx, data, (word32)data_len);
1010-
}
1011-
if (ret == 0) {
1012-
ret = wc_Sha384Final(&sha384_ctx, sha384_digest);
1013-
}
1014-
if (did_init) {
1015-
wc_Sha384Free(&sha384_ctx);
1016-
did_init = 0;
1017-
}
1018-
if (ret != 0) {
1019-
wolfBoot_printf("FIT hash-1 (sha384): wc_Sha384 failed rc=%d\n",
1020-
ret);
1021-
ret = -1;
1022-
}
1023-
else if (memcmp(sha384_digest, value, WC_SHA384_DIGEST_SIZE) != 0) {
1024-
wolfBoot_printf("FIT hash-1 (sha384): MISMATCH\n");
1025-
ret = -1;
1026-
}
1027-
else {
1028-
wolfBoot_printf("FIT hash-1 (sha384): OK\n");
1029-
}
1030-
done = 1;
947+
else {
948+
rd_dst = rd_ptr;
949+
wolfBoot_printf("Loaded ramdisk: %p (%d bytes)\n",
950+
rd_dst, rd_size);
1031951
}
1032-
#endif
1033952

1034-
if ((ret == 0) && !done) {
1035-
wolfBoot_printf("FIT hash-1: algo '%s' not built in, skipping\n",
1036-
algo);
953+
if (dts_addr != NULL) {
954+
(void)fdt_fixup_initrd(dts_addr,
955+
(uint64_t)(uintptr_t)rd_dst, (uint64_t)rd_size);
1037956
}
1038-
return ret;
957+
958+
return 0;
1039959
}
1040-
#endif /* WOLFBOOT_GZIP */
960+
#endif /* WOLFBOOT_FIT_RAMDISK */
1041961

1042962
void* fit_load_image_ex(void* fdt, const char* image, int* lenp,
1043963
uint32_t out_max)
@@ -1112,16 +1032,18 @@ void* fit_load_image_ex(void* fdt, const char* image, int* lenp,
11121032
memcpy(load, data, len);
11131033
}
11141034

1115-
#ifdef WOLFBOOT_GZIP
1116-
/* Defense-in-depth: verify FIT hash-1 against loaded
1117-
* bytes */
1118-
if (fit_verify_hash(fdt, off, (const uint8_t*)load,
1119-
(uint32_t)len) != 0) {
1120-
wolfBoot_printf("FIT hash verification failed for "
1121-
"%s\n", image);
1122-
return NULL;
1123-
}
1124-
#endif
1035+
/* No per-image hash-1 re-verification here. Per the
1036+
* FIT spec (and U-Boot's reference implementation), a
1037+
* hash-N subnode's value is computed over the image
1038+
* node's `data` property bytes verbatim - which means
1039+
* the compressed bytes when compression="gzip". The
1040+
* outer wolfBoot signature
1041+
* (wolfBoot_verify_authenticity) already authenticates
1042+
* the entire FIT, including those data bytes, so a
1043+
* runtime per-image hash check would be redundant.
1044+
* Inflater bugs on the decompressed payload are
1045+
* caught by gzip's own CRC32 + ISIZE trailer inside
1046+
* wolfBoot_gunzip. */
11251047

11261048
/* load should always have entry, but if not use load
11271049
* address */

0 commit comments

Comments
 (0)