Hi Thirupathaiah,
On Fri, 17 Jul 2020 at 21:20, Thirupathaiah Annapureddy
wrote:
>
> Currently FIT image must be signed by all required conf keys. This means
> Verified Boot fails if there is a signature verification failure
> using any required key in U-boot DTB.
>
> This patch introduces a new policy in DTB that can be set to any required
> conf key. This means if verified boot passes with one of the required
> keys, u-boot will continue the OS hand off.
U-Boot
>
> There were prior attempts to address this:
> https://lists.denx.de/pipermail/u-boot/2019-April/366047.html
> The above patch was failing "make tests".
> https://lists.denx.de/pipermail/u-boot/2020-January/396629.html
>
> Signed-off-by: Thirupathaiah Annapureddy
> ---
>
> Changes in v2:
> - Modify fit_config_verify_required_sigs() to process required-mode
> policy variable in U-boot DTB.
>
> common/image-fit-sig.c | 30 +++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass
Please see below
>
> diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
> index cc1967109e..d9b1c93a9b 100644
> --- a/common/image-fit-sig.c
> +++ b/common/image-fit-sig.c
> @@ -416,6 +416,10 @@ int fit_config_verify_required_sigs(const void *fit, int
> conf_noffset,
> {
> int noffset;
> int sig_node;
> + int verified = 0;
> + int reqd_sigs = 0;
> + bool reqd_policy_all = true;
> + const char *reqd_mode;
>
> /* Work out what we need to verify */
> sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME);
> @@ -425,6 +429,14 @@ int fit_config_verify_required_sigs(const void *fit, int
> conf_noffset,
> return 0;
> }
>
> + /* Get required-mode policy property from DTB */
> + reqd_mode = fdt_getprop(sig_blob, sig_node, "required-mode", NULL);
> + if (reqd_mode && !strcmp(reqd_mode, "any"))
> + reqd_policy_all = false;
> +
> + debug("%s: required-mode policy set to '%s'\n", __func__,
> + reqd_policy_all ? "all" : "any");
> +
> fdt_for_each_subnode(noffset, sig_blob, sig_node) {
> const char *required;
> int ret;
> @@ -433,15 +445,27 @@ int fit_config_verify_required_sigs(const void *fit,
> int conf_noffset,
>NULL);
> if (!required || strcmp(required, "conf"))
> continue;
> +
> + reqd_sigs++;
> +
> ret = fit_config_verify_sig(fit, conf_noffset, sig_blob,
> noffset);
> if (ret) {
> - printf("Failed to verify required signature '%s'\n",
> - fit_get_name(sig_blob, noffset, NULL));
> - return ret;
> + if (reqd_policy_all) {
> + printf("Failed to verify required signature
> '%s'\n",
> + fit_get_name(sig_blob, noffset, NULL));
I think we should keep this message outside the if(). Otherwise there
is no indication what is going on.
> + return ret;
> + }
> + } else {
> + verified++;
> + if (!reqd_policy_all)
> + break;
> }
> }
>
> + if (reqd_sigs && !verified)
I think we need a message here, indicating that no signature was verified.
> + return -EPERM;
> +
> return 0;
> }
>
> --
> 2.25.2
>
