Hi James,
On 2026-07-08T23:17:07, James Hilliard <[email protected]> wrote:
> tools: fit: sign all config image references
>
> Target-side configuration verification builds the signed-region list from
> every image-reference property in the selected configuration. Host-side
> signing still uses either the signature node sign-images property or the
> legacy kernel/fdt/script default list.
>
> This lets mkimage generate configuration signatures which U-Boot cannot
> verify when the configuration references other image types, such as
> firmware, loadables or ramdisk entries. It also lets the host and target
> disagree when sign-images names only a subset of the configuration images.
>
> Build the host-side signing list from the configuration properties in the
> same way as target-side verification. This makes signed configurations
> cover the root node, the configuration node, every referenced image node,
> and its hash/cipher subnodes, regardless of image type.
>
> Update the signed-configuration documentation to describe the same rule and
> to stop recommending sign-images as a selection mechanism.
>
> [...]
>
> doc/board/ti/k3.rst | 10 ++---
> doc/usage/fit/beaglebone_vboot.rst | 5 +--
> doc/usage/fit/sign-configs.rst | 7 +++-
> doc/usage/fit/signature.rst | 25 ++++++------
> doc/usage/fit/uefi.rst | 2 -
> tools/fit_image.c | 12 +++---
> tools/image-host.c | 82
> +++++++++++++++-----------------------
> 7 files changed, 63 insertions(+), 80 deletions(-)
> Build the host-side signing list from the configuration properties in the
> same way as target-side verification. This makes signed configurations
> cover the root node, the configuration node, every referenced image node,
> and its hash/cipher subnodes, regardless of image type.
Since you are changing mkimage behaviour you may need a test. The existing
vboot .its files only reference kernel and fdt, so nothing checks that a
ramdisk, firmware or loadables entry is covered by the signature. Please
can you extend one of the sign-configs .its files (or add a new one) with
such an entry and check the resulting hashed-nodes, or that tampering with
the extra image is detected? It may be worth keeping one file with
sign-images present, to confirm it is ignored, and dropping it from the
others.
I did check that signing and fit_check_sign work with a ramdisk entry, and
that the existing check in fit_image.c still catches dangling image
references, so the silent continue on a missing image node is fine.
> diff --git a/tools/image-host.c b/tools/image-host.c
> @@ -1162,27 +1162,6 @@ static int strlist_add(struct strlist *list, const
> char *str)
> - /* If there is an 'sign-image' property, use that */
> - prop = fdt_getprop(fit, noffset, 'sign-images', lenp);
> - if (prop) {
> - *allow_missingp = 0;
> - return *lenp ? prop : NULL;
> - }
With this gone, sign-images is silently ignored, so anyone using it to
sign a subset gets a different signature with no indication of why. Please
can you add a one-line warning from mkimage when the property is present,
to help people migrate?
> diff --git a/tools/image-host.c b/tools/image-host.c
> @@ -1306,34 +1286,38 @@ static int fit_config_get_hash_list(const void *fit,
> int conf_noffset,
> + /* Skip properties that are not image references */
> + if (!strcmp(prop_name, FIT_DESC_PROP) ||
> + !strcmp(prop_name, FIT_COMPAT_PROP) ||
> + !strcmp(prop_name, FIT_DEFAULT_PROP))
> + continue;
This skip list duplicates the one in fit_config_get_hash_list() in
boot/image-fit-sig.c and any drift between the two breaks verification,
which is exactly the bug this patch fixes. Please can you put the list in
one place, e.g. a shared array or helper declared in image.h? BTW the
comment on the target-side function mentions 'load-only' being skipped,
but neither implementation does that, so that needs fixing too.
> diff --git a/doc/usage/fit/sign-configs.rst b/doc/usage/fit/sign-configs.rst
> @@ -45,8 +45,13 @@ Signed configurations
> +For signed configurations, mkimage signs every image referenced by the
> +configuration node, such as `kernel, fdt, ramdisk, firmware and
> +loadables entries. No sign-images property is required. Older FIT
> +source files may still include sign-images`, but current mkimage and U-Boot
> +verification do not use it to limit the signed image list.
Please can you also mention that every referenced image must have at least
one hash subnode? Images outside the old default list (e.g. loadables
without hashes) previously signed fine, but fit_config_add_hash() now
fails with -ENOMSG for them, so this is a new requirement on existing
source files.
Regards,
Simon
