| Message ID | 0f920e6ee369718f3b7a0b9e07920383229715fd.1593045943.git.thiruan@linux.microsoft.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for multiple required keys | expand |
Hi Thirupathaiah, On Thu, 25 Jun 2020 at 09:51, Thirupathaiah Annapureddy <thiruan at 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 at 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
Hi Thirupathaiah, On Mon, 29 Jun 2020 at 11:26, Simon Glass <sjg at chromium.org> wrote: > > Hi Thirupathaiah, > > On Thu, 25 Jun 2020 at 09:51, Thirupathaiah Annapureddy > <thiruan at 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 at linux.microsoft.com> > > --- > > common/image-fit-sig.c | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) One more thing...this patch is changing the policy. I think we need a new string property in the DTB alongside the 'required' properly, that indicates whether the image must be signed with all required keys, or just one. Regards, Simon
On 25/06/2020 17.51, Thirupathaiah Annapureddy 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 at linux.microsoft.com> Hi Thirupathaiah This is something I'm quite interested in - see https://lists.denx.de/pipermail/u-boot/2020-January/396629.html . I just never got around to follow up on it due to other tasks. As Simon points out, the policy as to whether one or all (or some other choice) required keys must have signed the image needs to live in the .dtb. I'd appreciate it if you could cc me on subsequent revisions. Thanks, Rasmus
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; + 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)); - return ret; + } else { + verified = 1; + break; } } + if (reqd_sigs && !verified) + return -EPERM; + return 0; }
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 at linux.microsoft.com> --- common/image-fit-sig.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)