From: "Andrew Jeffery" <andrew@aj.id.au>
To: "Joel Stanley" <joel@jms.id.au>
Cc: OpenBMC Maillist <openbmc@lists.ozlabs.org>,
Eddie James <eajames@linux.ibm.com>
Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc 2/6] image: Control FIT uImage signature verification at runtime
Date: Wed, 09 Feb 2022 08:28:53 +1030 [thread overview]
Message-ID: <49472ed5-6989-4650-9c61-d609b2dae8ee@www.fastmail.com> (raw)
In-Reply-To: <CACPK8XcY=afrQ9bE2A3q1tC8Hhxmx3oVM7k_C_fVoYGbLJ4OUg@mail.gmail.com>
On Tue, 8 Feb 2022, at 16:33, Joel Stanley wrote:
> On Mon, 31 Jan 2022 at 01:26, Andrew Jeffery <andrew@aj.id.au> wrote:
>>
>> Some platform designs include support for disabling secure-boot via a
>> jumper on the board. Sometimes this control can be separate from the
>> mechanism enabling the root-of-trust for the platform. Add support for
>> this latter scenario by allowing boards to implement
>> board_fit_image_require_verfied(), which is then invoked in the usual
>> FIT verification paths.
>>
>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>> ---
>> Kconfig | 9 +++++++++
>> common/image-fit.c | 17 +++++++++++++++--
>> include/image.h | 9 +++++++++
>> 3 files changed, 33 insertions(+), 2 deletions(-)
>>
>> diff --git a/Kconfig b/Kconfig
>> index c3dfa8de47c8..11f796035ae4 100644
>> --- a/Kconfig
>> +++ b/Kconfig
>> @@ -322,6 +322,15 @@ config FIT_SIGNATURE
>> format support in this case, enable it using
>> CONFIG_IMAGE_FORMAT_LEGACY.
>>
>> +if FIT_SIGNATURE
>> +config FIT_RUNTIME_SIGNATURE
>> + bool "Control verification of FIT uImages at runtime"
>
> Can you follow the pattern of the other FIT_ options by making this
> depends on FIT_SIGNATURE?
Yeah, I didn't think about this enough :)
>
>> + help
>> + This option allows board support to disable verification of
>> + signatures at runtime, for example through the state of a GPIO.
>> +endif # FIT_SIGNATURE
>> +
>> +
>> config FIT_SIGNATURE_MAX_SIZE
>> hex "Max size of signed FIT structures"
>> depends on FIT_SIGNATURE
>> diff --git a/common/image-fit.c b/common/image-fit.c
>> index 3c8667f93de2..eb1e66b02b68 100644
>> --- a/common/image-fit.c
>> +++ b/common/image-fit.c
>> @@ -1199,6 +1199,14 @@ static int fit_image_check_hash(const void *fit, int noffset, const void *data,
>> return 0;
>> }
>>
>> +#ifndef __weak
>> +#define __weak
>> +#endif
>
> Shouldn't we always get this from linux/compiler.h?
I'll think about this some more as this file gets linked into the tools as well as the firmware.
But probably.
>
>> +__weak int board_fit_image_require_verified(void)
>> +{
>> + return 1;
>> +}
>> +
>> int fit_image_verify_with_data(const void *fit, int image_noffset,
>> const void *data, size_t size)
>> {
>> @@ -1209,6 +1217,7 @@ int fit_image_verify_with_data(const void *fit, int image_noffset,
>>
>> /* Verify all required signatures */
>> if (IMAGE_ENABLE_VERIFY &&
>> + fit_image_require_verified() &&
>> fit_image_verify_required_sigs(fit, image_noffset, data, size,
>> gd_fdt_blob(), &verify_all)) {
>> err_msg = "Unable to verify required signature";
>> @@ -1230,7 +1239,9 @@ int fit_image_verify_with_data(const void *fit, int image_noffset,
>> &err_msg))
>> goto error;
>> puts("+ ");
>> - } else if (IMAGE_ENABLE_VERIFY && verify_all &&
>> + } else if (IMAGE_ENABLE_VERIFY &&
>> + fit_image_require_verified() &&
>> + verify_all &&
>
> reading through this it's quite confusing.
>
> We have IMAGE_ENABLE_VERIFY, a compile time constant that will be true
> if CONFIG_FIT_SIGNATURE is enabled.
>
> We're adding a function that will override this.
>
> So we could have a function:
>
> __weak bool fit_enable_verification(void)
> {
> return IMAGE_ENABLE_VERIFY;
> }
>
> The downside of this would be if a board were to implement this but
> not have FIT_SIGNATURE enabled then they could return true when they
> shouldn't. You could go back to this:
>
> static bool fit_enable_verification(void)
> {
> return IMAGE_ENABLE_VERIFY && board_fit_image_require_verified();
> }
>
> And drop the ifdefs from image.h
This sounds attractive, let me poke at it.
Thanks for thinking about it.
Andrew
next prev parent reply other threads:[~2022-02-08 22:00 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-31 1:25 [PATCH u-boot v2019.04-aspeed-openbmc 0/6] Runtime control of vboot via GPIO Andrew Jeffery
2022-01-31 1:25 ` [PATCH u-boot v2019.04-aspeed-openbmc 1/6] gpio: Add gpio_request_by_line_name() Andrew Jeffery
2022-02-03 17:23 ` Eddie James
2022-01-31 1:25 ` [PATCH u-boot v2019.04-aspeed-openbmc 2/6] image: Control FIT uImage signature verification at runtime Andrew Jeffery
2022-02-03 17:22 ` Eddie James
2022-02-08 6:03 ` Joel Stanley
2022-02-08 21:58 ` Andrew Jeffery [this message]
2022-01-31 1:25 ` [PATCH u-boot v2019.04-aspeed-openbmc 3/6] ARM: ast2600: " Andrew Jeffery
2022-02-03 17:25 ` Eddie James
2022-01-31 1:25 ` [PATCH u-boot v2019.04-aspeed-openbmc 4/6] configs: ast2600: Runtime control of FIT signature verification Andrew Jeffery
2022-02-03 17:27 ` Eddie James
2022-01-31 1:25 ` [PATCH u-boot v2019.04-aspeed-openbmc 5/6] ARM: dts: rainier: Add gpio-line-names property with bmc-secure-boot Andrew Jeffery
2022-02-03 17:28 ` Eddie James
2022-01-31 1:25 ` [PATCH u-boot v2019.04-aspeed-openbmc 6/6] image: Fix indentation of macros Andrew Jeffery
2022-02-03 17:29 ` Eddie James
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=49472ed5-6989-4650-9c61-d609b2dae8ee@www.fastmail.com \
--to=andrew@aj.id.au \
--cc=eajames@linux.ibm.com \
--cc=joel@jms.id.au \
--cc=openbmc@lists.ozlabs.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).