[coreboot-gerrit] Change in ...coreboot[master]: Documentation/security/vboot: Add logic to verify stage/blob using VB...

[Amol N Sukerkar (Code Review)]

Amol N Sukerkar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32159 )

Change subject: Documentation/security/vboot: Add logic to verify stage/blob using VBOOT 2.1 library
......................................................................

Patch Set 1:
> Patch Set 1:
>
> Hi Amol,
>
> I'm a little surprised by this change. Have you talked to any of the current vboot owners about this before? Everyone I've talked to seems to have had no idea that you were planning to do anything like this.
>
> It would have probably been better to reach out to other vboot developers or start a discussion on the mailing list to align on overall design goals before writing all this code. I don't think we can have everyone implementing their own verified boot solution side by side... we should aim on having a single, coherent design that can cover all use cases. We're certainly open to expanding the existing vboot to be more flexible (including interworking with SoC-based verification mechanisms like BootGuard) but we should try to find a way to do it that doesn't fork so much code between different modes of vboot.
>
> Looks to me like your main goals here are a) moving the root of trust into BootGuard and b) preventing TOCTOU issues? We've had our own ideas about those things as well... in fact I was planning to start some work soon(ish) to allow verifying post-verstage stages when vboot is booting in read-only ("recovery") mode. If you combine that with linking verstage and bootblock together (CONFIG_VBOOT_SEPARATE_VERSTAGE=n) and cover the bootblock+verstage binary, the FMAP and the GBB with BootGuard, you can essentially have your whole verification chain anchored in BootGuard. TOCTOU is a harder issue to solve, and your approach of verifying stages individually on load is essentially the right one, but it seems pretty inflexible in the way that you need BootGuard to cover the hashes of everything you may want to load explicitly. Different platforms and different architectures often require all kinds of different files to be loaded, so rather than having a hardcoded list of hashes I'd favor an approach that keeps the hashes in CBFS metadata (generated/packaged right with the file itself), and then covers the metadata for the whole CBFS with a single "master hash" that's chained to the root of trust.
>
> Maybe it would be best if we would set up a meeting with interested parties to discuss the future vboot roadmap for those features, or have that discussion on the mailing list? I don't think we can merge this patch train as is because it breaks away too far from the existing vboot code (rather than expanding it in a way that can cover multiple use cases) and seems too focused on only supporting Intel BootGuard on x86 boards (we'd like to keep vboot as SoC-agnostic as possible).
>
> (PS: The term "vboot 2.1" is already used by the vboot project itself to refer to a newer key/signature structure format that coreboot isn't using yet. I don't think your patches have anything to do with that, so naming it like that is confusing.)

Hi Julius,

First, my code "does" use the VBOOT 2.1 library from vboot_reference. I am little held up with getting permissions to vboot_reference repo. Once that is done, I am planning to raise a review there with the new utilities we implemented (with all the original utilities untouched :-)). Also, the logic using VBOOT 2.1 libraries in vboot_logic_ex.c is the new logic we are introducing without interfering with the existing logic in vboot_logic.c. We are also thinking of a similar implementation as you mentioned above to make the the verification more flexible and generic.

This implementation does make use of BootGuard as root of trust but this implementation will still be good if BootGuard implementation is replaced with Chromebook style root-of-trust which is read-only flash partition.

I very much welcome the idea of a meeting between the concerned parties so we can discuss this implementation in detail and discuss any potential changes to it. Please let me know how we can co-ordinate towards this meeting.

Thanks,
Amol