Hi Daniel, On 2026-07-12T00:33:05, Daniel Golle <[email protected]> wrote: > test: fit: verify dm-verity roothash is covered by the config signature > > A dm-verity protected filesystem image is not hashed by U-Boot; its > integrity is delegated to the kernel, which trusts the roothash taken > from the FIT dm-verity subnode. For that chain of trust to hold, the > roothash (and salt) must be part of the region covered by the > configuration signature, otherwise an attacker can replace both the > filesystem and the roothash while keeping the signature valid. > > Add two independent checks of this property: > > - test/py/tests/test_fit_verity_sign.py signs a configuration that > references a filesystem image carrying a dm-verity subnode, then > confirms that tampering the roothash or the salt is rejected by > fit_check_sign. A control that tampers a byte known to be signed > proves the check can fail. > > - test/boot/fit_verity.c gains a runtime unit test that builds the > exact node list the configuration signature is computed over, > turns it into hashed regions and checks both that the roothash > [...] > > boot/image-fit-sig.c | 25 +++-- > include/image.h | 25 +++++ > test/boot/fit_verity.c | 194 > ++++++++++++++++++++++++++++++++++ > test/py/tests/test_fit_verity_sign.py | 162 ++++++++++++++++++++++++++++ > 4 files changed, 399 insertions(+), 7 deletions(-)
Somehow I didn't get this email, but I'll try to reply here. > diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c > @@ -340,8 +340,19 @@ static int fit_config_add_hash(const void *fit, int > image_noffset, > +#if CONFIG_IS_ENABLED(UNIT_TEST) > +#define VISIBLE_IF_UT > +#else > +#define VISIBLE_IF_UT static > +#endif Tom commented on this, but I'll just add this. Linux has VISIBLE_IF_KUNIT but in U-Boot so far we just stop the static when the function is needed elsewhere. LTO can be used to avoid the code-size increase, for platforms that want it. > diff --git a/test/boot/fit_verity.c b/test/boot/fit_verity.c > @@ -304,3 +309,192 @@ static int fit_verity_test_bad_blocksize(struct > unit_test_state *uts) > + count = fdt_find_regions(buf, node_inc, count, NULL, 0, fdt_regions, > + ARRAY_SIZE(fdt_regions) - 1, region_path, > + sizeof(region_path), 0); > + ut_assert(count > 0); fdt_find_regions() can return a count larger than the max passed in, meaning the region array was exhausted (see the 'count >= max_regions - 1' check in fit_config_check_sig()). If that ever happens here, fit_region_make_list() reads past the filled entries. Please can you assert that count is within the array size, matching the real verification path? > diff --git a/test/boot/fit_verity.c b/test/boot/fit_verity.c > @@ -304,3 +309,192 @@ static int fit_verity_test_bad_blocksize(struct > unit_test_state *uts) > + /* Tampering the roothash must change the signed hash. */ > + ((u8 *)digest)[0] ^= 0xff; You already have digest_off, so you could write to buf[digest_off] instead of casting away the const from fdt_getprop(). Both work since buf is writable, but avoiding the cast is cleaner. > diff --git a/test/boot/fit_verity.c b/test/boot/fit_verity.c > @@ -304,3 +309,192 @@ static int fit_verity_test_bad_blocksize(struct > unit_test_state *uts) > + digest = fdt_getprop(buf, verity_node, FIT_VERITY_DIGEST_PROP, > + &digest_len); The python test also tampers the salt, but the unit test only checks the digest. Since both properties sit in the same dm-verity node the coverage result is the same, so I don't feel strongly, but a one-line comment noting that the digest check stands in for the whole node would make the asymmetry clearer. What do you think? Regards, Simon
