* [PATCH u-boot v2019.04-aspeed-openbmc 0/6] Runtime control of vboot via GPIO
@ 2022-01-31 1:25 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
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Andrew Jeffery @ 2022-01-31 1:25 UTC (permalink / raw)
To: openbmc; +Cc: eajames
Hello,
This u-boot series implements support for controlling whether verified
boot is enabled at runtime by measuring the state of the
"bmc-secure-boot" GPIO for AST2600-based BMCs.
Previously, whether or not verified boot was required was configured at
build time.
These patches build on top of the series Eddie recently sent to the list
to enable use of GPIOs in the SPL:
https://lore.kernel.org/openbmc/20220124191503.88452-1-eajames@linux.ibm.com/
I've tested the changes under qemu, and they behave as expected for the
Rainier platform configuration.
I'll figure out how we go about upstreaming the series once Eddie's
series has been picked up.
Please review!
Andrew
Andrew Jeffery (6):
gpio: Add gpio_request_by_line_name()
image: Control FIT uImage signature verification at runtime
ARM: ast2600: Control FIT uImage signature verification at runtime
configs: ast2600: Runtime control of FIT signature verification
ARM: dts: rainier: Add gpio-line-names property with bmc-secure-boot
image: Fix indentation of macros
Kconfig | 9 ++++
arch/arm/dts/ast2600-rainier.dts | 32 +++++++++++++
arch/arm/mach-aspeed/ast2600/Makefile | 1 +
arch/arm/mach-aspeed/ast2600/secure-boot.c | 53 ++++++++++++++++++++++
common/image-fit.c | 17 ++++++-
configs/ast2600_openbmc_spl_emmc_defconfig | 1 +
drivers/gpio/gpio-uclass.c | 26 +++++++++++
include/asm-generic/gpio.h | 19 ++++++++
include/image.h | 13 +++++-
9 files changed, 167 insertions(+), 4 deletions(-)
create mode 100644 arch/arm/mach-aspeed/ast2600/secure-boot.c
--
2.32.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH u-boot v2019.04-aspeed-openbmc 1/6] gpio: Add gpio_request_by_line_name()
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 ` 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
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Andrew Jeffery @ 2022-01-31 1:25 UTC (permalink / raw)
To: openbmc; +Cc: eajames
Add support for the upstream gpio-line-names property already described
in the common GPIO binding document[1]. The ability to search for a line
name allows boards to lift the implementation of common GPIO behaviours
away from specific line indexes on a GPIO controller.
[1] https://github.com/devicetree-org/dt-schema/blob/3c35bfee83c2e38e2ae7af5f83eb89ca94a521e8/dtschema/schemas/gpio/gpio.yaml#L17
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/gpio/gpio-uclass.c | 26 ++++++++++++++++++++++++++
include/asm-generic/gpio.h | 19 +++++++++++++++++++
2 files changed, 45 insertions(+)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 219caa651bb2..425bbc5cb880 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -878,6 +878,32 @@ int gpio_request_by_name(struct udevice *dev, const char *list_name, int index,
index, desc, flags, index > 0, NULL);
}
+int gpio_request_by_line_name(struct udevice *dev, const char *line_name,
+ struct gpio_desc *desc, int flags)
+{
+ int ret;
+
+ ret = dev_read_stringlist_search(dev, "gpio-line-names", line_name);
+ if (ret < 0)
+ return ret;
+
+ desc->dev = dev;
+ desc->offset = ret;
+ desc->flags = 0;
+
+ ret = dm_gpio_request(desc, line_name);
+ if (ret) {
+ debug("%s: dm_gpio_requestf failed\n", __func__);
+ return ret;
+ }
+
+ ret = dm_gpio_set_dir_flags(desc, flags | desc->flags);
+ if (ret)
+ debug("%s: dm_gpio_set_dir failed\n", __func__);
+
+ return ret;
+}
+
int gpio_request_list_by_name_nodev(ofnode node, const char *list_name,
struct gpio_desc *desc, int max_count,
int flags)
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index d6cf18744fda..6ed0ba11b6c1 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -451,6 +451,25 @@ int gpio_claim_vector(const int *gpio_num_array, const char *fmt);
int gpio_request_by_name(struct udevice *dev, const char *list_name,
int index, struct gpio_desc *desc, int flags);
+/* gpio_request_by_line_name - Locate and request a GPIO by line name
+ *
+ * Request a GPIO using the offset of the provided line name in the
+ * gpio-line-names property found in the OF node of the GPIO udevice.
+ *
+ * This allows boards to implement common behaviours using GPIOs while not
+ * requiring specific GPIO offsets be used.
+ *
+ * @dev: An instance of a GPIO controller udevice
+ * @line_name: The name of the GPIO (e.g. "bmc-secure-boot")
+ * @desc: A GPIO descriptor that is populated with the requested GPIO
+ * upon return
+ * @flags: The GPIO settings apply to the request
+ * @return 0 if the named line was found and requested successfully, or a
+ * negative error code if the GPIO cannot be found or the request failed.
+ */
+int gpio_request_by_line_name(struct udevice *dev, const char *line_name,
+ struct gpio_desc *desc, int flags);
+
/**
* gpio_request_list_by_name() - Request a list of GPIOs
*
--
2.32.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH u-boot v2019.04-aspeed-openbmc 2/6] image: Control FIT uImage signature verification at runtime
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-01-31 1:25 ` Andrew Jeffery
2022-02-03 17:22 ` Eddie James
2022-02-08 6:03 ` Joel Stanley
2022-01-31 1:25 ` [PATCH u-boot v2019.04-aspeed-openbmc 3/6] ARM: ast2600: " Andrew Jeffery
` (3 subsequent siblings)
5 siblings, 2 replies; 15+ messages in thread
From: Andrew Jeffery @ 2022-01-31 1:25 UTC (permalink / raw)
To: openbmc; +Cc: eajames
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"
+ 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
+__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 &&
!strncmp(name, FIT_SIG_NODENAME,
strlen(FIT_SIG_NODENAME))) {
ret = fit_image_check_sig(fit, noffset, data,
@@ -1849,7 +1860,9 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
if (image_type == IH_TYPE_KERNEL)
images->fit_uname_cfg = fit_base_uname_config;
- if (IMAGE_ENABLE_VERIFY && images->verify) {
+ if (IMAGE_ENABLE_VERIFY &&
+ fit_image_require_verified() &&
+ images->verify) {
puts(" Verifying Hash Integrity ... ");
if (fit_config_verify(fit, cfg_noffset)) {
puts("Bad Data Hash\n");
diff --git a/include/image.h b/include/image.h
index 937c7eee8ffb..19ea743af08f 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1103,6 +1103,15 @@ int calculate_hash(const void *data, int data_len, const char *algo,
# define IMAGE_ENABLE_VERIFY 0
#endif
+/*
+ * Further, allow run-time control of verification, e.g. via a jumper
+ */
+#if defined(CONFIG_FIT_RUNTIME_SIGNATURE)
+# define fit_image_require_verified() board_fit_image_require_verified()
+#else
+# define fit_image_require_verified() IMAGE_ENABLE_VERIFY
+#endif
+
#ifdef USE_HOSTCC
void *image_get_host_blob(void);
void image_set_host_blob(void *host_blob);
--
2.32.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH u-boot v2019.04-aspeed-openbmc 3/6] ARM: ast2600: Control FIT uImage signature verification at runtime
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-01-31 1:25 ` [PATCH u-boot v2019.04-aspeed-openbmc 2/6] image: Control FIT uImage signature verification at runtime Andrew Jeffery
@ 2022-01-31 1:25 ` 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
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Andrew Jeffery @ 2022-01-31 1:25 UTC (permalink / raw)
To: openbmc; +Cc: eajames
Implement support for disabling signature verification of FIT images at
runtime by sampling the "bmc-secure-boot" GPIO. If the line name is not
provided in the devicetree then secure-boot continues to be required as
if the feature were not present.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
arch/arm/mach-aspeed/ast2600/Makefile | 1 +
arch/arm/mach-aspeed/ast2600/secure-boot.c | 53 ++++++++++++++++++++++
2 files changed, 54 insertions(+)
create mode 100644 arch/arm/mach-aspeed/ast2600/secure-boot.c
diff --git a/arch/arm/mach-aspeed/ast2600/Makefile b/arch/arm/mach-aspeed/ast2600/Makefile
index d07e8c737cfe..70b7ae11df56 100644
--- a/arch/arm/mach-aspeed/ast2600/Makefile
+++ b/arch/arm/mach-aspeed/ast2600/Makefile
@@ -1,4 +1,5 @@
obj-y += platform.o board_common.o scu_info.o utils.o cache.o
+obj-$(CONFIG_FIT_RUNTIME_SIGNATURE) += secure-boot.o
obj-$(CONFIG_ASPEED_SECURE_BOOT) += crypto.o aspeed_verify.o
obj-$(CONFIG_ASPEED_LOADERS) += spl_boot.o
obj-$(CONFIG_SPL_BUILD) += spl.o
diff --git a/arch/arm/mach-aspeed/ast2600/secure-boot.c b/arch/arm/mach-aspeed/ast2600/secure-boot.c
new file mode 100644
index 000000000000..ced353686387
--- /dev/null
+++ b/arch/arm/mach-aspeed/ast2600/secure-boot.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// (C) Copyright IBM Corp. 2022
+
+#include <common.h>
+#include <asm-generic/gpio.h>
+#include <dm.h>
+
+static int aspeed_get_chained_secboot_state(void)
+{
+ struct gpio_desc desc;
+ struct udevice *dev;
+ int secboot;
+ int rc;
+
+ rc = uclass_get_device_by_driver(UCLASS_GPIO,
+ DM_GET_DRIVER(gpio_aspeed),
+ &dev);
+ if (rc < 0) {
+ debug("Warning: GPIO initialization failure: %d\n", rc);
+ return rc;
+ }
+
+ rc = gpio_request_by_line_name(dev, "bmc-secure-boot", &desc,
+ GPIOD_IS_IN);
+ if (rc < 0) {
+ debug("Failed to acquire secure-boot GPIO: %d\n", rc);
+ return rc;
+ }
+
+ secboot = dm_gpio_get_value(&desc);
+ if (secboot < 0)
+ debug("Failed to read secure-boot GPIO value: %d\n", rc);
+
+ rc = dm_gpio_free(dev, &desc);
+ if (rc < 0)
+ debug("Failed to free secure-boot GPIO: %d\n", rc);
+
+ return secboot;
+}
+
+int board_fit_image_require_verified(void)
+{
+ int secboot;
+
+ secboot = aspeed_get_chained_secboot_state();
+
+ /*
+ * If secure-boot is enabled then require signature verification.
+ * Otherwise, if we fail to read the GPIO, enforce FIT signature
+ * verification
+ */
+ return secboot >= 0 ? secboot : 1;
+}
--
2.32.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH u-boot v2019.04-aspeed-openbmc 4/6] configs: ast2600: Runtime control of FIT signature verification
2022-01-31 1:25 [PATCH u-boot v2019.04-aspeed-openbmc 0/6] Runtime control of vboot via GPIO Andrew Jeffery
` (2 preceding siblings ...)
2022-01-31 1:25 ` [PATCH u-boot v2019.04-aspeed-openbmc 3/6] ARM: ast2600: " Andrew Jeffery
@ 2022-01-31 1:25 ` 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-01-31 1:25 ` [PATCH u-boot v2019.04-aspeed-openbmc 6/6] image: Fix indentation of macros Andrew Jeffery
5 siblings, 1 reply; 15+ messages in thread
From: Andrew Jeffery @ 2022-01-31 1:25 UTC (permalink / raw)
To: openbmc; +Cc: eajames
Turn on runtime control of FIT signature verification for systems using
the ast2600_openbmc_spl_emmc_defconfig, such as IBM's Rainier platform.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
configs/ast2600_openbmc_spl_emmc_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
index 5f50298a589c..a3d229d786b8 100644
--- a/configs/ast2600_openbmc_spl_emmc_defconfig
+++ b/configs/ast2600_openbmc_spl_emmc_defconfig
@@ -29,6 +29,7 @@ CONFIG_NR_DRAM_BANKS=1
CONFIG_FIT=y
CONFIG_FIT_ENABLE_SHA512_SUPPORT=y
CONFIG_FIT_SIGNATURE=y
+CONFIG_FIT_RUNTIME_SIGNATURE=y
CONFIG_SPL_FIT_SIGNATURE=y
CONFIG_SPL_LOAD_FIT=y
CONFIG_USE_BOOTARGS=y
--
2.32.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH u-boot v2019.04-aspeed-openbmc 5/6] ARM: dts: rainier: Add gpio-line-names property with bmc-secure-boot
2022-01-31 1:25 [PATCH u-boot v2019.04-aspeed-openbmc 0/6] Runtime control of vboot via GPIO Andrew Jeffery
` (3 preceding siblings ...)
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-01-31 1:25 ` 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
5 siblings, 1 reply; 15+ messages in thread
From: Andrew Jeffery @ 2022-01-31 1:25 UTC (permalink / raw)
To: openbmc; +Cc: eajames
The "bmc-secure-boot" GPIO controls at runtime whether FIT signature
verification is performed by u-boot and the SPL.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
arch/arm/dts/ast2600-rainier.dts | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/arch/arm/dts/ast2600-rainier.dts b/arch/arm/dts/ast2600-rainier.dts
index d0e82d151239..af35afb911fb 100755
--- a/arch/arm/dts/ast2600-rainier.dts
+++ b/arch/arm/dts/ast2600-rainier.dts
@@ -104,6 +104,38 @@
sdhci-drive-type = <1>;
};
+&gpio0 {
+ status = "okay";
+
+ gpio-line-names =
+ /*A0-A7*/ "","","","","","","","",
+ /*B0-B7*/ "","","","","","","","",
+ /*C0-C7*/ "","","","","","","","",
+ /*D0-D7*/ "","","","","","","","",
+ /*E0-E7*/ "","","","","","","","",
+ /*F0-F7*/ "","","","","","","","",
+ /*G0-G7*/ "","","","","","","","",
+ /*H0-H7*/ "","","","","","","","",
+ /*I0-I7*/ "","","","","","","bmc-secure-boot","",
+ /*J0-J7*/ "","","","","","","","",
+ /*K0-K7*/ "","","","","","","","",
+ /*L0-L7*/ "","","","","","","","",
+ /*M0-M7*/ "","","","","","","","",
+ /*N0-N7*/ "","","","","","","","",
+ /*O0-O7*/ "","","","","","","","",
+ /*P0-P7*/ "","","","","","","","",
+ /*Q0-Q7*/ "","","","","","","","",
+ /*R0-R7*/ "","","","","","","","",
+ /*S0-S7*/ "","","","","","","","",
+ /*T0-T7*/ "","","","","","","","",
+ /*U0-U7*/ "","","","","","","","",
+ /*V0-V7*/ "","","","","","","","",
+ /*W0-W7*/ "","","","","","","","",
+ /*X0-X7*/ "","","","","","","","",
+ /*Y0-Y7*/ "","","","","","","","",
+ /*Z0-Z7*/ "","","","","","","","";
+};
+
&hace {
u-boot,dm-pre-reloc;
status = "okay";
--
2.32.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH u-boot v2019.04-aspeed-openbmc 6/6] image: Fix indentation of macros
2022-01-31 1:25 [PATCH u-boot v2019.04-aspeed-openbmc 0/6] Runtime control of vboot via GPIO Andrew Jeffery
` (4 preceding siblings ...)
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-01-31 1:25 ` Andrew Jeffery
2022-02-03 17:29 ` Eddie James
5 siblings, 1 reply; 15+ messages in thread
From: Andrew Jeffery @ 2022-01-31 1:25 UTC (permalink / raw)
To: openbmc; +Cc: eajames
Make it clear which level of nesting they belong to.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
include/image.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/image.h b/include/image.h
index 19ea743af08f..9bccf00bb5cb 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1093,8 +1093,8 @@ int calculate_hash(const void *data, int data_len, const char *algo,
# ifdef USE_HOSTCC
# define IMAGE_ENABLE_SIGN 1
# define IMAGE_ENABLE_VERIFY 1
-# include <openssl/evp.h>
-#else
+# include <openssl/evp.h>
+# else
# define IMAGE_ENABLE_SIGN 0
# define IMAGE_ENABLE_VERIFY 1
# endif
--
2.32.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH u-boot v2019.04-aspeed-openbmc 2/6] image: Control FIT uImage signature verification at runtime
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
1 sibling, 0 replies; 15+ messages in thread
From: Eddie James @ 2022-02-03 17:22 UTC (permalink / raw)
To: Andrew Jeffery, openbmc
On 1/30/22 19:25, Andrew Jeffery 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.
Reviewed-by: Eddie James <eajames@linux.ibm.com
>
> 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"
> + 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
> +__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 &&
> !strncmp(name, FIT_SIG_NODENAME,
> strlen(FIT_SIG_NODENAME))) {
> ret = fit_image_check_sig(fit, noffset, data,
> @@ -1849,7 +1860,9 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
> if (image_type == IH_TYPE_KERNEL)
> images->fit_uname_cfg = fit_base_uname_config;
>
> - if (IMAGE_ENABLE_VERIFY && images->verify) {
> + if (IMAGE_ENABLE_VERIFY &&
> + fit_image_require_verified() &&
> + images->verify) {
> puts(" Verifying Hash Integrity ... ");
> if (fit_config_verify(fit, cfg_noffset)) {
> puts("Bad Data Hash\n");
> diff --git a/include/image.h b/include/image.h
> index 937c7eee8ffb..19ea743af08f 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1103,6 +1103,15 @@ int calculate_hash(const void *data, int data_len, const char *algo,
> # define IMAGE_ENABLE_VERIFY 0
> #endif
>
> +/*
> + * Further, allow run-time control of verification, e.g. via a jumper
> + */
> +#if defined(CONFIG_FIT_RUNTIME_SIGNATURE)
> +# define fit_image_require_verified() board_fit_image_require_verified()
> +#else
> +# define fit_image_require_verified() IMAGE_ENABLE_VERIFY
> +#endif
> +
> #ifdef USE_HOSTCC
> void *image_get_host_blob(void);
> void image_set_host_blob(void *host_blob);
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH u-boot v2019.04-aspeed-openbmc 1/6] gpio: Add gpio_request_by_line_name()
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
0 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2022-02-03 17:23 UTC (permalink / raw)
To: Andrew Jeffery, openbmc
On 1/30/22 19:25, Andrew Jeffery wrote:
> Add support for the upstream gpio-line-names property already described
> in the common GPIO binding document[1]. The ability to search for a line
> name allows boards to lift the implementation of common GPIO behaviours
> away from specific line indexes on a GPIO controller.
>
> [1] https://github.com/devicetree-org/dt-schema/blob/3c35bfee83c2e38e2ae7af5f83eb89ca94a521e8/dtschema/schemas/gpio/gpio.yaml#L17
Reviewed-by: Eddie James <eajames@linux.ibm.com>
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> drivers/gpio/gpio-uclass.c | 26 ++++++++++++++++++++++++++
> include/asm-generic/gpio.h | 19 +++++++++++++++++++
> 2 files changed, 45 insertions(+)
>
> diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> index 219caa651bb2..425bbc5cb880 100644
> --- a/drivers/gpio/gpio-uclass.c
> +++ b/drivers/gpio/gpio-uclass.c
> @@ -878,6 +878,32 @@ int gpio_request_by_name(struct udevice *dev, const char *list_name, int index,
> index, desc, flags, index > 0, NULL);
> }
>
> +int gpio_request_by_line_name(struct udevice *dev, const char *line_name,
> + struct gpio_desc *desc, int flags)
> +{
> + int ret;
> +
> + ret = dev_read_stringlist_search(dev, "gpio-line-names", line_name);
> + if (ret < 0)
> + return ret;
> +
> + desc->dev = dev;
> + desc->offset = ret;
> + desc->flags = 0;
> +
> + ret = dm_gpio_request(desc, line_name);
> + if (ret) {
> + debug("%s: dm_gpio_requestf failed\n", __func__);
> + return ret;
> + }
> +
> + ret = dm_gpio_set_dir_flags(desc, flags | desc->flags);
> + if (ret)
> + debug("%s: dm_gpio_set_dir failed\n", __func__);
> +
> + return ret;
> +}
> +
> int gpio_request_list_by_name_nodev(ofnode node, const char *list_name,
> struct gpio_desc *desc, int max_count,
> int flags)
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index d6cf18744fda..6ed0ba11b6c1 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -451,6 +451,25 @@ int gpio_claim_vector(const int *gpio_num_array, const char *fmt);
> int gpio_request_by_name(struct udevice *dev, const char *list_name,
> int index, struct gpio_desc *desc, int flags);
>
> +/* gpio_request_by_line_name - Locate and request a GPIO by line name
> + *
> + * Request a GPIO using the offset of the provided line name in the
> + * gpio-line-names property found in the OF node of the GPIO udevice.
> + *
> + * This allows boards to implement common behaviours using GPIOs while not
> + * requiring specific GPIO offsets be used.
> + *
> + * @dev: An instance of a GPIO controller udevice
> + * @line_name: The name of the GPIO (e.g. "bmc-secure-boot")
> + * @desc: A GPIO descriptor that is populated with the requested GPIO
> + * upon return
> + * @flags: The GPIO settings apply to the request
> + * @return 0 if the named line was found and requested successfully, or a
> + * negative error code if the GPIO cannot be found or the request failed.
> + */
> +int gpio_request_by_line_name(struct udevice *dev, const char *line_name,
> + struct gpio_desc *desc, int flags);
> +
> /**
> * gpio_request_list_by_name() - Request a list of GPIOs
> *
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH u-boot v2019.04-aspeed-openbmc 3/6] ARM: ast2600: Control FIT uImage signature verification at runtime
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
0 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2022-02-03 17:25 UTC (permalink / raw)
To: Andrew Jeffery, openbmc
On 1/30/22 19:25, Andrew Jeffery wrote:
> Implement support for disabling signature verification of FIT images at
> runtime by sampling the "bmc-secure-boot" GPIO. If the line name is not
> provided in the devicetree then secure-boot continues to be required as
> if the feature were not present.
Reviewed-by: Eddie James <eajames@linux.ibm.com>
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> arch/arm/mach-aspeed/ast2600/Makefile | 1 +
> arch/arm/mach-aspeed/ast2600/secure-boot.c | 53 ++++++++++++++++++++++
> 2 files changed, 54 insertions(+)
> create mode 100644 arch/arm/mach-aspeed/ast2600/secure-boot.c
>
> diff --git a/arch/arm/mach-aspeed/ast2600/Makefile b/arch/arm/mach-aspeed/ast2600/Makefile
> index d07e8c737cfe..70b7ae11df56 100644
> --- a/arch/arm/mach-aspeed/ast2600/Makefile
> +++ b/arch/arm/mach-aspeed/ast2600/Makefile
> @@ -1,4 +1,5 @@
> obj-y += platform.o board_common.o scu_info.o utils.o cache.o
> +obj-$(CONFIG_FIT_RUNTIME_SIGNATURE) += secure-boot.o
> obj-$(CONFIG_ASPEED_SECURE_BOOT) += crypto.o aspeed_verify.o
> obj-$(CONFIG_ASPEED_LOADERS) += spl_boot.o
> obj-$(CONFIG_SPL_BUILD) += spl.o
> diff --git a/arch/arm/mach-aspeed/ast2600/secure-boot.c b/arch/arm/mach-aspeed/ast2600/secure-boot.c
> new file mode 100644
> index 000000000000..ced353686387
> --- /dev/null
> +++ b/arch/arm/mach-aspeed/ast2600/secure-boot.c
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// (C) Copyright IBM Corp. 2022
> +
> +#include <common.h>
> +#include <asm-generic/gpio.h>
> +#include <dm.h>
> +
> +static int aspeed_get_chained_secboot_state(void)
> +{
> + struct gpio_desc desc;
> + struct udevice *dev;
> + int secboot;
> + int rc;
> +
> + rc = uclass_get_device_by_driver(UCLASS_GPIO,
> + DM_GET_DRIVER(gpio_aspeed),
> + &dev);
> + if (rc < 0) {
> + debug("Warning: GPIO initialization failure: %d\n", rc);
> + return rc;
> + }
> +
> + rc = gpio_request_by_line_name(dev, "bmc-secure-boot", &desc,
> + GPIOD_IS_IN);
> + if (rc < 0) {
> + debug("Failed to acquire secure-boot GPIO: %d\n", rc);
> + return rc;
> + }
> +
> + secboot = dm_gpio_get_value(&desc);
> + if (secboot < 0)
> + debug("Failed to read secure-boot GPIO value: %d\n", rc);
> +
> + rc = dm_gpio_free(dev, &desc);
> + if (rc < 0)
> + debug("Failed to free secure-boot GPIO: %d\n", rc);
> +
> + return secboot;
> +}
> +
> +int board_fit_image_require_verified(void)
> +{
> + int secboot;
> +
> + secboot = aspeed_get_chained_secboot_state();
> +
> + /*
> + * If secure-boot is enabled then require signature verification.
> + * Otherwise, if we fail to read the GPIO, enforce FIT signature
> + * verification
> + */
> + return secboot >= 0 ? secboot : 1;
> +}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH u-boot v2019.04-aspeed-openbmc 4/6] configs: ast2600: Runtime control of FIT signature verification
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
0 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2022-02-03 17:27 UTC (permalink / raw)
To: Andrew Jeffery, openbmc
On 1/30/22 19:25, Andrew Jeffery wrote:
> Turn on runtime control of FIT signature verification for systems using
> the ast2600_openbmc_spl_emmc_defconfig, such as IBM's Rainier platform.
Reviewed-by: Eddie James <eajames@linux.ibm.com>
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> configs/ast2600_openbmc_spl_emmc_defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
> index 5f50298a589c..a3d229d786b8 100644
> --- a/configs/ast2600_openbmc_spl_emmc_defconfig
> +++ b/configs/ast2600_openbmc_spl_emmc_defconfig
> @@ -29,6 +29,7 @@ CONFIG_NR_DRAM_BANKS=1
> CONFIG_FIT=y
> CONFIG_FIT_ENABLE_SHA512_SUPPORT=y
> CONFIG_FIT_SIGNATURE=y
> +CONFIG_FIT_RUNTIME_SIGNATURE=y
> CONFIG_SPL_FIT_SIGNATURE=y
> CONFIG_SPL_LOAD_FIT=y
> CONFIG_USE_BOOTARGS=y
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH u-boot v2019.04-aspeed-openbmc 5/6] ARM: dts: rainier: Add gpio-line-names property with bmc-secure-boot
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
0 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2022-02-03 17:28 UTC (permalink / raw)
To: Andrew Jeffery, openbmc
On 1/30/22 19:25, Andrew Jeffery wrote:
> The "bmc-secure-boot" GPIO controls at runtime whether FIT signature
> verification is performed by u-boot and the SPL.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> arch/arm/dts/ast2600-rainier.dts | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/arch/arm/dts/ast2600-rainier.dts b/arch/arm/dts/ast2600-rainier.dts
> index d0e82d151239..af35afb911fb 100755
> --- a/arch/arm/dts/ast2600-rainier.dts
> +++ b/arch/arm/dts/ast2600-rainier.dts
> @@ -104,6 +104,38 @@
> sdhci-drive-type = <1>;
> };
>
> +&gpio0 {
> + status = "okay";
I'm surprised you don't need the u-boot,dm-pre-reloc flag here?
> +
> + gpio-line-names =
> + /*A0-A7*/ "","","","","","","","",
> + /*B0-B7*/ "","","","","","","","",
> + /*C0-C7*/ "","","","","","","","",
> + /*D0-D7*/ "","","","","","","","",
> + /*E0-E7*/ "","","","","","","","",
> + /*F0-F7*/ "","","","","","","","",
> + /*G0-G7*/ "","","","","","","","",
> + /*H0-H7*/ "","","","","","","","",
> + /*I0-I7*/ "","","","","","","bmc-secure-boot","",
> + /*J0-J7*/ "","","","","","","","",
> + /*K0-K7*/ "","","","","","","","",
> + /*L0-L7*/ "","","","","","","","",
> + /*M0-M7*/ "","","","","","","","",
> + /*N0-N7*/ "","","","","","","","",
> + /*O0-O7*/ "","","","","","","","",
> + /*P0-P7*/ "","","","","","","","",
> + /*Q0-Q7*/ "","","","","","","","",
> + /*R0-R7*/ "","","","","","","","",
> + /*S0-S7*/ "","","","","","","","",
> + /*T0-T7*/ "","","","","","","","",
> + /*U0-U7*/ "","","","","","","","",
> + /*V0-V7*/ "","","","","","","","",
> + /*W0-W7*/ "","","","","","","","",
> + /*X0-X7*/ "","","","","","","","",
> + /*Y0-Y7*/ "","","","","","","","",
> + /*Z0-Z7*/ "","","","","","","","";
> +};
> +
> &hace {
> u-boot,dm-pre-reloc;
> status = "okay";
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH u-boot v2019.04-aspeed-openbmc 6/6] image: Fix indentation of macros
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
0 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2022-02-03 17:29 UTC (permalink / raw)
To: Andrew Jeffery, openbmc
On 1/30/22 19:25, Andrew Jeffery wrote:
> Make it clear which level of nesting they belong to.
Reviewed-by: Eddie James <eajames@linux.ibm.com>
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> include/image.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/image.h b/include/image.h
> index 19ea743af08f..9bccf00bb5cb 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1093,8 +1093,8 @@ int calculate_hash(const void *data, int data_len, const char *algo,
> # ifdef USE_HOSTCC
> # define IMAGE_ENABLE_SIGN 1
> # define IMAGE_ENABLE_VERIFY 1
> -# include <openssl/evp.h>
> -#else
> +# include <openssl/evp.h>
> +# else
> # define IMAGE_ENABLE_SIGN 0
> # define IMAGE_ENABLE_VERIFY 1
> # endif
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH u-boot v2019.04-aspeed-openbmc 2/6] image: Control FIT uImage signature verification at runtime
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
1 sibling, 1 reply; 15+ messages in thread
From: Joel Stanley @ 2022-02-08 6:03 UTC (permalink / raw)
To: Andrew Jeffery; +Cc: OpenBMC Maillist, Eddie James
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?
> + 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?
> +__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
> !strncmp(name, FIT_SIG_NODENAME,
> strlen(FIT_SIG_NODENAME))) {
> ret = fit_image_check_sig(fit, noffset, data,
> @@ -1849,7 +1860,9 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
> if (image_type == IH_TYPE_KERNEL)
> images->fit_uname_cfg = fit_base_uname_config;
>
> - if (IMAGE_ENABLE_VERIFY && images->verify) {
> + if (IMAGE_ENABLE_VERIFY &&
> + fit_image_require_verified() &&
> + images->verify) {
> puts(" Verifying Hash Integrity ... ");
> if (fit_config_verify(fit, cfg_noffset)) {
> puts("Bad Data Hash\n");
> diff --git a/include/image.h b/include/image.h
> index 937c7eee8ffb..19ea743af08f 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1103,6 +1103,15 @@ int calculate_hash(const void *data, int data_len, const char *algo,
> # define IMAGE_ENABLE_VERIFY 0
> #endif
>
> +/*
> + * Further, allow run-time control of verification, e.g. via a jumper
> + */
> +#if defined(CONFIG_FIT_RUNTIME_SIGNATURE)
> +# define fit_image_require_verified() board_fit_image_require_verified()
> +#else
> +# define fit_image_require_verified() IMAGE_ENABLE_VERIFY
> +#endif
> +
> #ifdef USE_HOSTCC
> void *image_get_host_blob(void);
> void image_set_host_blob(void *host_blob);
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH u-boot v2019.04-aspeed-openbmc 2/6] image: Control FIT uImage signature verification at runtime
2022-02-08 6:03 ` Joel Stanley
@ 2022-02-08 21:58 ` Andrew Jeffery
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Jeffery @ 2022-02-08 21:58 UTC (permalink / raw)
To: Joel Stanley; +Cc: OpenBMC Maillist, Eddie James
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
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-02-08 22:00 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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
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).