All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yao Zi <me@ziyao.cc>
To: anon87111 <anon87111@gmail.com>, u-boot@lists.u-boot-project.org
Cc: trini@konsulko.com, sjg@chromium.org, 0xiviel@gmail.com,
	Yao Zi <me@ziyao.cc>
Subject: Re: [PATCH] image-fit-sig: bind a required image key to its own verification
Date: Thu, 15 Oct 2026 03:27:04 +0000	[thread overview]
Message-ID: <atBICLs6No2HdGP5@pie> (raw)
In-Reply-To: <e4f97d42-fb3a-4c5f-8f11-90a87b2172f0@gmail.com>

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

      reply	other threads:[~2026-10-15  3:27 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=atBICLs6No2HdGP5@pie \
    --to=me@ziyao.cc \
    --cc=0xiviel@gmail.com \
    --cc=anon87111@gmail.com \
    --cc=sjg@chromium.org \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.u-boot-project.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.