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

[Simon Glass]

Hi Thirupathaiah,

On Wed, 8 Jul 2020 at 16:47, Thirupathaiah Annapureddy
<thiruan at linux.microsoft.com> wrote:
>
> Hi Simon,
>
> Thanks a lot for reviewing the patch.
>
> I would appreciate if you could clarify the following in-line questions:
>
> On 6/29/2020 10:31 AM, Simon Glass wrote:
> > 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 assume you are referring to the policy of conf signing with all required
> keys instead of just one. I just wanted to double check.

The signing is a separate thing.

My comment was about the verification step in U-Boot. We need a policy
to say whether the config should be signed with all required keys or
just one.

>
> However I did not see any test in test_vboot.py for verifying this policy.
> So I thought signing with all required keys is not by design and it is
> an unintended bug. Could you please clarify on this?

As it is written, a required key is required, and the presence of a
different required key doesn't change that. But I am happy to provide
a way to change this policy. I just don't want to surprise people.

Of course the policy change needs to be in the signature DTB, not the
signed FIT.

Yes you should add a test for the new behaviour. I am a bit worried
about how long the vboot tests take so perhaps we can reduce this.


>
> >
> > 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