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

View Change

To view, visit change 32159. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1eb174bb4f4d84eb8f6befdce18421b6b85ccc02
Gerrit-Change-Number: 32159
Gerrit-PatchSet: 1
Gerrit-Owner: Amol N Sukerkar <amol.n.sukerkar@intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Amol N Sukerkar <amol.n.sukerkar@intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan@google.com>
Gerrit-CC: Joel Kitching <kitching@google.com>
Gerrit-CC: Julius Werner <jwerner@chromium.org>
Gerrit-CC: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
Gerrit-Comment-Date: Tue, 02 Apr 2019 23:06:33 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment