Re: [PATCH 1/2] vboot: add support for multiple required keys

Hi Thirupathaiah,

On Thu, 25 Jun 2020 at 09:51, Thirupathaiah Annapureddy thiruan@linux.microsoft.com wrote:

Currently Verified Boot fails if there is a signature verification failure using required key in U-boot DTB. This patch adds support for multiple required keys. This means if verified boot passes with one of the required keys, u-boot will continue the OS hand off.

There was a prior attempt to resolve this with the following patch: https://lists.denx.de/pipermail/u-boot/2019-April/366047.html The above patch was failing "make tests".

Signed-off-by: Thirupathaiah Annapureddy thiruan@linux.microsoft.com

common/image-fit-sig.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c index cc1967109e..4d25d4c541 100644 --- a/common/image-fit-sig.c +++ b/common/image-fit-sig.c @@ -416,6 +416,8 @@ int fit_config_verify_required_sigs(const void *fit, int conf_noffset, { int noffset; int sig_node;

•   int verified = 0;
  

Can this be a bool?

•   int reqd_sigs = 0;
  
    /* Work out what we need to verify */
    sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME);
  

@@ -433,15 +435,23 @@ 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));
  

This message is confusing if we then decide that things are OK. I think it would be better to change the message to a positive "Verified required signatured xxx" if !ret

•                   return ret;
  

•           } else {
  

•                   verified = 1;
  

•                   break;
            }
    }
  
  

•   if (reqd_sigs && !verified)
  

•           return -EPERM;
  

This needs a message, something like "No required signatures were verified"

and then list them in a for() loop.

•   return 0;
  

}

-- 2.17.1

Regards, Simon