All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] image-fit-sig: bind a required image key to its own verification
@ 2026-10-13 20:16 anon87111
  2026-10-15  3:27 ` Yao Zi
  0 siblings, 1 reply; 2+ messages in thread
From: anon87111 @ 2026-10-13 20:16 UTC (permalink / raw)
  To: u-boot; +Cc: trini, sjg, 0xiviel

fit_image_verify_required_sigs() walks the keys in the control FDT, picks
out those whose "required" property is "image", and passes each key's node
offset to fit_image_verify_sig(). That function takes the offset as
key_offset and never uses it: it calls fit_image_check_sig() with
required_keynode hardcoded to -1, which both boot/image-fit-sig.c and
include/image.h document as "-1 then any signature will do".

With required_keynode == -1, rsa_verify() skips the "See if we must use a
particular key" branch, tries the key named by the FIT's own key-name-hint
property, and then falls through to "try each of the keys in turn". So
every required-key iteration is satisfied by any signature in the FIT that
verifies under any key in the control FDT, not by the key the iteration is
supposed to be checking.

Where two or more keys are marked required = "image", a single signature
from any one of them therefore satisfies the whole policy. Possession of
one required image key is enough to boot an arbitrary image, and the other
required keys contribute nothing. doc/usage/fit/signature.rst says of
"required": "this indicates that the key must be verified for the image /
configuration to be considered valid."

The configuration path in the same file does this correctly:
fit_config_verify_key() passes key_offset straight through to
fit_config_check_sig(), and fit_config_verify_required_keys() additionally
honours the required-mode "any"/"all" policy, defaulting to "all". The two
functions were written as a pair; only one binds the key.

Reproduced on sandbox at b871f722 with two keys marked required = "image"
and a FIT carrying only a dev signature:

  keys marked required = "image": 2        (key-dev, key-prod)
  FIT signatures present        : sha256,rsa2048:dev only

     Verifying Hash Integrity ... sha256,rsa2048:dev+ sha256,rsa2048:dev+ OK
  ## Transferring control to Linux (at address 000001cc)...
  sandbox: continuing, as we cannot run Linux

The doubled "dev+ dev+" is the defect: one dev signature satisfied both
iterations, including the one for key-prod. The same scenario against the
configuration gate, same keys and same default policy, correctly refuses:

     Verifying Hash Integrity ... sha256,rsa2048:dev+ sha256,rsa2048:dev-  error!
  Verification failed for 'conf-1' config node
  Failed to verify required signature 'key-prod'

With this patch the image gate refuses the same way, and the configuration
gate is unchanged.

The hardcoded -1 dates to 56518e7104 ("image: Support signing of images",
v2013.07), where fit_image_verify_sig() already took sig_offset without
using it. 99f844ba3a ("tools: Pass the key blob around") added key_blob
and left the -1 in place, in the same commit that kept key_offset wired up
on the configuration side. It has survived because nothing exercises it:
there is no required = "image" anywhere in the tree, and
test_required_key() builds only sign-configs-* fixtures, so the image gate
has never been run with more than one required key.

This is separate from, and complementary to, the fail-open gate reported
on the list on 2026-09-07 and patched in "[PATCH 0/2] image-fit-sig: close
the image-signature fail-open gate". That one is about a control FDT with
no /signature node at all; this one is about two keys being marked
required and one signature satisfying both. That fix leaves this behaviour
in place and this fix leaves that one in place, so both are needed. It is
also separate from "[PATCH v10 0/7] Improve FIT signature handling" [1],
which covers the configuration gate.

[1] https://lore.kernel.org/u-boot/20260923114940.349985-1-ludwig.nussel@siemens.com/

Signed-off-by: anon87111 <anon87111@gmail.com>
---
 boot/image-fit-sig.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
index c0fbe81b4..d6c61e104 100644
--- a/boot/image-fit-sig.c
+++ b/boot/image-fit-sig.c
@@ -159,7 +159,8 @@ static int fit_image_verify_sig(const void *fit, int image_noffset,
 		if (!strncmp(name, FIT_SIG_NODENAME,
 			     strlen(FIT_SIG_NODENAME))) {
 			ret = fit_image_check_sig(fit, noffset, data, size,
-						  key_blob, -1, &err_msg);
+						  key_blob, key_offset,
+						  &err_msg);
 			if (ret) {
 				puts("- ");
 			} else {
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] image-fit-sig: bind a required image key to its own verification
  2026-10-13 20:16 [PATCH] image-fit-sig: bind a required image key to its own verification anon87111
@ 2026-10-15  3:27 ` Yao Zi
  0 siblings, 0 replies; 2+ messages in thread
From: Yao Zi @ 2026-10-15  3:27 UTC (permalink / raw)
  To: anon87111, u-boot; +Cc: trini, sjg, 0xiviel, Yao Zi

On Tue, Oct 13, 2026 at 10:16:08PM +0200, anon87111 wrote:
> fit_image_verify_required_sigs() walks the keys in the control FDT, picks
> out those whose "required" property is "image", and passes each key's node
> offset to fit_image_verify_sig(). That function takes the offset as
> key_offset and never uses it: it calls fit_image_check_sig() with
> required_keynode hardcoded to -1, which both boot/image-fit-sig.c and
> include/image.h document as "-1 then any signature will do".
> 
> With required_keynode == -1, rsa_verify() skips the "See if we must use a
> particular key" branch, tries the key named by the FIT's own key-name-hint
> property, and then falls through to "try each of the keys in turn". So
> every required-key iteration is satisfied by any signature in the FIT that
> verifies under any key in the control FDT, not by the key the iteration is
> supposed to be checking.
> 
> Where two or more keys are marked required = "image", a single signature
> from any one of them therefore satisfies the whole policy. Possession of
> one required image key is enough to boot an arbitrary image, and the other
> required keys contribute nothing. doc/usage/fit/signature.rst says of
> "required": "this indicates that the key must be verified for the image /
> configuration to be considered valid."
> 
> The configuration path in the same file does this correctly:
> fit_config_verify_key() passes key_offset straight through to
> fit_config_check_sig(), and fit_config_verify_required_keys() additionally
> honours the required-mode "any"/"all" policy, defaulting to "all". The two
> functions were written as a pair; only one binds the key.
> 
> Reproduced on sandbox at b871f722 with two keys marked required = "image"
> and a FIT carrying only a dev signature:
> 
>   keys marked required = "image": 2        (key-dev, key-prod)
>   FIT signatures present        : sha256,rsa2048:dev only
> 
>      Verifying Hash Integrity ... sha256,rsa2048:dev+ sha256,rsa2048:dev+ OK
>   ## Transferring control to Linux (at address 000001cc)...
>   sandbox: continuing, as we cannot run Linux
> 
> The doubled "dev+ dev+" is the defect: one dev signature satisfied both
> iterations, including the one for key-prod. The same scenario against the
> configuration gate, same keys and same default policy, correctly refuses:
> 
>      Verifying Hash Integrity ... sha256,rsa2048:dev+ sha256,rsa2048:dev-  error!
>   Verification failed for 'conf-1' config node
>   Failed to verify required signature 'key-prod'
> 
> With this patch the image gate refuses the same way, and the configuration
> gate is unchanged.
> 
> The hardcoded -1 dates to 56518e7104 ("image: Support signing of images",
> v2013.07), where fit_image_verify_sig() already took sig_offset without
> using it. 99f844ba3a ("tools: Pass the key blob around") added key_blob
> and left the -1 in place, in the same commit that kept key_offset wired up
> on the configuration side. It has survived because nothing exercises it:
> there is no required = "image" anywhere in the tree, and
> test_required_key() builds only sign-configs-* fixtures, so the image gate
> has never been run with more than one required key.

Should this commit carry a fixes tag?

> This is separate from, and complementary to, the fail-open gate reported
> on the list on 2026-09-07 and patched in "[PATCH 0/2] image-fit-sig: close
> the image-signature fail-open gate". That one is about a control FDT with
> no /signature node at all; this one is about two keys being marked
> required and one signature satisfying both. That fix leaves this behaviour
> in place and this fix leaves that one in place, so both are needed. It is
> also separate from "[PATCH v10 0/7] Improve FIT signature handling" [1],
> which covers the configuration gate.

I only give it a brief glance, (at least) this paragraph sounds like
background, too detailed and less relevant to put in the commit message.
You might want to move it under the "---" line.

Regards,
Yao Zi

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-15  3:27 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-13 20:16 [PATCH] image-fit-sig: bind a required image key to its own verification anon87111
2026-10-15  3:27 ` Yao Zi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.