Change in ...coreboot[master]: Documentation/security/vboot: Add logic to verify stage/blob using VB...
Amol N Sukerkar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32159
Change subject: Documentation/security/vboot: Add logic to verify stage/blob using VBOOT 2.1 library ......................................................................
Documentation/security/vboot: Add logic to verify stage/blob using VBOOT 2.1 library
Added documentation to explain the logic that makes use of VBOOT 2.1 library to verify Coreboot stages/blobs.
Signed-off-by: Sukerkar, Amol N amol.n.sukerkar@intel.com Change-Id: I1eb174bb4f4d84eb8f6befdce18421b6b85ccc02 --- M Documentation/security/index.md A Documentation/security/vboot/flash_partition.png A Documentation/security/vboot/vboot_21_logic.png A Documentation/security/vboot/vboot_flow_20.png A Documentation/security/vboot/vboot_flow_21.png A Documentation/security/vboot/verified_boot_21.md 6 files changed, 115 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/32159/1
diff --git a/Documentation/security/index.md b/Documentation/security/index.md index 89db42e..aebfc82 100644 --- a/Documentation/security/index.md +++ b/Documentation/security/index.md @@ -5,3 +5,4 @@ ## Vendor
- [Measured Boot](vboot/measured_boot.md) +- [Verified Boot with VBOOT 2.1](vboot/verified_boot_21.md) diff --git a/Documentation/security/vboot/flash_partition.png b/Documentation/security/vboot/flash_partition.png new file mode 100644 index 0000000..91e459b --- /dev/null +++ b/Documentation/security/vboot/flash_partition.png Binary files differ diff --git a/Documentation/security/vboot/vboot_21_logic.png b/Documentation/security/vboot/vboot_21_logic.png new file mode 100644 index 0000000..d32e99e --- /dev/null +++ b/Documentation/security/vboot/vboot_21_logic.png Binary files differ diff --git a/Documentation/security/vboot/vboot_flow_20.png b/Documentation/security/vboot/vboot_flow_20.png new file mode 100644 index 0000000..fce4c5d --- /dev/null +++ b/Documentation/security/vboot/vboot_flow_20.png Binary files differ diff --git a/Documentation/security/vboot/vboot_flow_21.png b/Documentation/security/vboot/vboot_flow_21.png new file mode 100644 index 0000000..24859fe --- /dev/null +++ b/Documentation/security/vboot/vboot_flow_21.png Binary files differ diff --git a/Documentation/security/vboot/verified_boot_21.md b/Documentation/security/vboot/verified_boot_21.md new file mode 100644 index 0000000..15253b3 --- /dev/null +++ b/Documentation/security/vboot/verified_boot_21.md @@ -0,0 +1,114 @@ +# Enabling Intel BootGuard Support in Coreboot + +## Introduction + +One of the primary and key requirement for Intel customers is to enable Secure +Boot in their platform where root of trust resides in the hardware and is +immutable. While the obvious choice when it comes to hardware root of trust is +Intel Bootguard for IA platforms, the mechanism that extends the chain +of trust from hardware to bootloader, to payload/OS loader and eventually to the +OS is currently not implemented in Coreboot mainly due to licensing constraints. +This document describes the mechanism implemented in Coreboot using Google VBOOT +libraries version 2.1 that makes use of Intel BootGuard technology as Root of +Trust and extends the chain of trust to Coreboot which in turn extends it to the +payload. Note: UEFI payload will use secure boot mechanism to verify and launch +OS but that is beyond the scope of this document and will not be covered here. +More details about VBOOT support in Coreboot are available at +https://www.coreboot.org/git-docs/Intel/vboot.html. + + +## Intel BootGuard Technology + +Intel BootGuard is a platform boot integrity +protection technology. It allows initial stage of bootloader to be verified (and +measured in TPM) by a piece of firmware (ACM) which itself is verified by Intel +CPU microcode. A high level summary of the steps required to enable Intel +BootGuard are: 1. Use Intel FSP-T with Coreboot bootloader. It contains the +logic of correctly handling BtGuard enabled state. 2. Integrate ACM +(Authenticated Code module) binary in bootloader image. 3. Generate BtGuard Key +Manifest(BtG KM) and BtGuard Boot PolicyManifest(BtG BPM) and embed them in +bootloader image. a. BtG KM contains the hash of the key used for signing BtG +BPM. BtG KM is signed by the key whose hash is embedded in field-programmable +fuses. b. BtG BPM contains the hash of initial stage of boot loader. It also +stores other policies related to Intel TXT, BtG DMA protection etc. 4. Add +entries for CPU microcode patch, ACM, BtG KM and BtG BPM in FIT table. 5. +Update BootGuard related field-programmable fuses on the test platform. + + +## VBOOT in Coreboot + +The current implementation of VBOOT tool support and logic +in Coreboot is at version 2.0. The architecture, design and usage of this +feature has been described here, +https://www.coreboot.org/git-docs/Intel/vboot.html. + +### VBOOT 2.0 in Coreboot (Currently available) + +As described in the location mentioned above, the VBOOT 2.0 verified boot logic +flow works as follows. Upon boot, verstage attempts to verify the read-write +section A. It gets the public root key from GBB area and verifies the VBLOCK +area in read-write section A. If the verification is successful, then verstage +instructs Coreboot to boot rest of the firmware in read-write section A +(romstage, postcar, ramstage and payload). If the verification fails, then, +VBOOT falls back on read-only area to boot. +The flow chart below shows this flow: + +**VBOOT 2.0 Verification Flow in Coreboot** +![VBOOT_20_Flow_in_Coreboot][VBOOT_Flow_20] + +[VBOOT_Flow_20]: vboot_flow_20.png + +While this design implements verified boot to a certain extent, it does not take +into account a few use-cases and concerns some Intel customers run into. A +couple of them are as listed below, +- Some hardware designs cannot support ‘read-only’ flash region as Root of Trust + and therefore prefer Intel BootGuard technology as RoT for verified boot +mechanism. +- In some use-cases, some of the firmware components may come from different + media, for instance, customer could boot payload from USB thumb drive instead +of SPI flash. In that case, the entire read-write section will not have all the +firmware components. There needs to be a mechanism to verify payload along with +other components. +- In exisiting implementation, all the firmware components in the boot chain are + verified in verstage. This may result in a TOCTOU attack where right after the +verification phase, some firmware stages/components (such as payload, FSP, etc.) +can be swapped with malware. To extend the vboot security model, the mechanism +described below is proposed where the root of trust Bootguard begins by +verifying the IBB and the chain of trust is extended only to the next +stage/component at every verification pass. For instance, once Bootguard +verifies IBB which contains verstage, romstage and postcar), postcar then uses +the VBOOT mechanism to verify ramstage. Once ramstage is loaded into DRAM, +ramstage in turn will verify FSP and payload. + +### Proposed Changes using VBOOT2.1 Libraries + +The proposed changes are described as follows. Upon power on of the device, +Intel Bootguard attempts to verify IBB. IBB in this case, replaces the read-only +portion of the flash map and contains bootblock, verstage, romstage and postcar +stage. If the verification is successful, Intel Bootguard launches IBB and the +system boots until postcar stage. In postcar stage, GBB is extracted and GBB +verified the VBLOCK. Once VBLOCK is verified, postcar stage extracts the +ramstage hash from VBLOCK and verifies the ramstage after it has been loaded +into DRAM. This is done to ensure maximum security. Once ramstage is verified, +ramstage is launched. At this point, ramstage extracts the hash of FSPS, DSDT +ACPI table and payload in that order, and verifies and launches them +sequentially. At any point, if the verification fails, the system boot will +halt. + +**Stage/Blob Verification using VBOOT2.1 Library** +![VBOOT_Stage_Blob_Verification][VBOOT_Flow_21] + +[VBOOT_Flow_21]: vboot_flow_21.png + +### Flash Partition and Code Flow in Coreboot + +**Flash Partition for Verification using VBOOT 2.1 Library** +![VBOOT_Verification_2_1_Flash][Flash_Partition] + +[Flash_Partition]: flash_partition.png + +**Verification Logic using VBOOT 2.1 Library** +![VBOOT_Verification_2_1_Logic][vboot_21_logic] + +[vboot_21_logic]: vboot_21_logic.png +
Julius Werner 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:
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.)
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
Julius Werner 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:
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 :-)).
Oh, okay, I didn't realize that. That's all the more reason we should talk. We are generally happy about taking external submissions to the vboot project, but if you just turn up with a huge code dump without prior discussion, that would likely be a problem.
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.
Right, that's one of my main concerns here... you're introducing this huge new thing on the side that duplicates/forks a lot of code and doesn't really interact with the old version. We don't want to maintain many different versions of vboot logic that all do somewhat the same things in a slightly different way, we'd rather organically evolve a single version forward to eventually support all the different use cases people have, where only the code that actually needs to be different for that is different.
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.
Okay, great. I would suggest a Google Hangouts VC at a PDT and Asia friendly time (so Joel can join us if he wants to) if that works for you. How about some time next week at 5pm? Maybe Monday? (Also asking others here in case you are interested in this.)
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:
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 :-)).
Oh, okay, I didn't realize that. That's all the more reason we should talk. We are generally happy about taking external submissions to the vboot project, but if you just turn up with a huge code dump without prior discussion, that would likely be a problem.
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.
Right, that's one of my main concerns here... you're introducing this huge new thing on the side that duplicates/forks a lot of code and doesn't really interact with the old version. We don't want to maintain many different versions of vboot logic that all do somewhat the same things in a slightly different way, we'd rather organically evolve a single version forward to eventually support all the different use cases people have, where only the code that actually needs to be different for that is different.
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.
Okay, great. I would suggest a Google Hangouts VC at a PDT and Asia friendly time (so Joel can join us if he wants to) if that works for you. How about some time next week at 5pm? Maybe Monday? (Also asking others here in case you are interested in this.)
Next week sounds good. I have to also inquire if some internal folks are interested. I will let you know as soon as I get confirmation (by Monday or so).
Philipp Deppenwiese 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:
Please add me to the VC as well, thanks
Paul Menzel 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:
(51 comments)
https://review.coreboot.org/#/c/32159/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32159/1//COMMIT_MSG@7 PS1, Line 7: Documentation/security/vboot: Add logic to verify stage/blob using Commit messages summaries have to fit on one line.
https://review.coreboot.org/#/c/32159/1//COMMIT_MSG@7 PS1, Line 7: Documentation/security/vboot: Add logic to verify stage/blob using : VBOOT 2.1 library Maybe:
security: Document logic to verify stag/blob using vboot 2.1
https://review.coreboot.org/#/c/32159/1//COMMIT_MSG@8 PS1, Line 8: VBOOT vboot
https://review.coreboot.org/#/c/32159/1//COMMIT_MSG@10 PS1, Line 10: Added Add
https://review.coreboot.org/#/c/32159/1//COMMIT_MSG@11 PS1, Line 11: Coreboot coreboot
https://review.coreboot.org/#/c/32159/1/Documentation/security/index.md File Documentation/security/index.md:
https://review.coreboot.org/#/c/32159/1/Documentation/security/index.md@8 PS1, Line 8: VBOOT vboot
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... File Documentation/security/vboot/verified_boot_21.md:
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 1: BootGuard Boot Guard
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 1: Coreboot lowercase: coreboot
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 8: Bootguard Boot Guard
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 10: Coreboot coreboot
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 11: Coreboot coreboot
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 11: This document describes the mechanism implemented in Coreboot using Google VBOOT Please add a blank line above to separate the paragraphs.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 11: VBOOT vboot
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 12: BootGuard Boot Guard
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 12: Root root
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 13: Trust trust
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 13: Coreboot coreboot
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 16: VBOOT vboot
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 16: Coreboot coreboot
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 20: BootGuard Boot Guard
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 22: BootGuard Boot Guard
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 24: ACM As this is documentation, please also give the full name or link to the corresponding documentation.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 24: measured in TPM) by a piece of firmware (ACM) which itself is verified by Intel : CPU microcode Verified by microcode?
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 26: BootGuard Boot Guard
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 28: Authenticated Code module Please move that above.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 29: PolicyManifest(BtG BPM) Ditto.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 29: Manifest(BtG KM) Add a space before (.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 29: in in the
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 35: BootGuard Boot Guard
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 26: BootGuard are: 1. Use Intel FSP-T with Coreboot bootloader. It contains the : logic of correctly handling BtGuard enabled state. 2. Integrate ACM : (Authenticated Code module) binary in bootloader image. 3. Generate BtGuard Key : Manifest(BtG KM) and BtGuard Boot PolicyManifest(BtG BPM) and embed them in : bootloader image. a. BtG KM contains the hash of the key used for signing BtG : BPM. BtG KM is signed by the key whose hash is embedded in field-programmable : fuses. b. BtG BPM contains the hash of initial stage of boot loader. It also : stores other policies related to Intel TXT, BtG DMA protection etc. 4. Add : entries for CPU microcode patch, ACM, BtG KM and BtG BPM in FIT table. 5. : Update BootGuard related field-programmable fuses on the test platform. Please format this as a list.
- a
- b
- c
- …
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 41: Coreboot coreboot
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 41: 2.0 Below you use 2.1
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 42: has been is
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 42: feature has been described here, : https://www.coreboot.org/git-docs/Intel/vboot.html Make it a link.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 45: Coreboot coreboot
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 51: Coreboot coreboot
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 63: , Please use a dot.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 64: Root of Trust root of trust
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 64: - Some hardware designs cannot support ‘read-only’ flash region as Root of Trust Please add a blank line above.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 65: BootGuard Boot Guard
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 68: Remove or align all lines.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 75: One space for consistency.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 76: Bootguard Boot Guard
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 78: Bootguard Boot Guard
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 79: ) Remove.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 80: VBOOT vboot
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 86: Bootguard Boot Guard
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 88: Bootguard Boot Guard
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 89: GBB Elaborate what GBB is?
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 90: verified verifies?
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 92: This is done to ensure maximum security. That sounds like marketing speech?
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:
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 :-)).
Oh, okay, I didn't realize that. That's all the more reason we should talk. We are generally happy about taking external submissions to the vboot project, but if you just turn up with a huge code dump without prior discussion, that would likely be a problem.
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.
Right, that's one of my main concerns here... you're introducing this huge new thing on the side that duplicates/forks a lot of code and doesn't really interact with the old version. We don't want to maintain many different versions of vboot logic that all do somewhat the same things in a slightly different way, we'd rather organically evolve a single version forward to eventually support all the different use cases people have, where only the code that actually needs to be different for that is different.
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.
Okay, great. I would suggest a Google Hangouts VC at a PDT and Asia friendly time (so Joel can join us if he wants to) if that works for you. How about some time next week at 5pm? Maybe Monday? (Also asking others here in case you are interested in this.)
Hi Julius,
Is it possible to schedule Google Hangouts VC for this Thursday,April 11th? Also note that I have to add a couple internal folks to the meeting. So, please send me the details once you finalize. Thanks!
Julius Werner 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:
Is it possible to schedule Google Hangouts VC for this Thursday,April 11th? Also note that I have to add a couple internal folks to the meeting. So, please send me the details once you finalize. Thanks!
Okay, sounds good... I added your email to a meeting for Thursday at 5pm (PDT). Please add whoever else wants to join from your side. If anyone else here (Philipp? David?) is interested in this let me know and I can add you as well.
David Hendricks 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:
Is it possible to schedule Google Hangouts VC for this Thursday,April 11th? Also note that I have to add a couple internal folks to the meeting. So, please send me the details once you finalize. Thanks!
Okay, sounds good... I added your email to a meeting for Thursday at 5pm (PDT). Please add whoever else wants to join from your side. If anyone else here (Philipp? David?) is interested in this let me know and I can add you as well.
Thanks for setting this up, Julius. I believe Philipp will arrive at SFO on Thursday, however I am not sure about the time. Please add me, too; even though I'm not as deeply involved in vboot design as you guys this will impact work we're doing at FB.
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:
(50 comments)
Patch Set 1:
Is it possible to schedule Google Hangouts VC for this Thursday,April 11th? Also note that I have to add a couple internal folks to the meeting. So, please send me the details once you finalize. Thanks!
Okay, sounds good... I added your email to a meeting for Thursday at 5pm (PDT). Please add whoever else wants to join from your side. If anyone else here (Philipp? David?) is interested in this let me know and I can add you as well.
Thanks, Julius.
https://review.coreboot.org/#/c/32159/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32159/1//COMMIT_MSG@7 PS1, Line 7: Documentation/security/vboot: Add logic to verify stage/blob using
Commit messages summaries have to fit on one line.
Ack
https://review.coreboot.org/#/c/32159/1//COMMIT_MSG@8 PS1, Line 8: VBOOT
vboot
Ack
https://review.coreboot.org/#/c/32159/1//COMMIT_MSG@7 PS1, Line 7: Documentation/security/vboot: Add logic to verify stage/blob using : VBOOT 2.1 library
Maybe: […]
This looks better although it appears to break from coreboot commit message convention where entire path of the file to be committed from root directory is listed at the beginning of the summary line. I'll make the change nevertheless so it can be reviewed.
https://review.coreboot.org/#/c/32159/1//COMMIT_MSG@10 PS1, Line 10: Added
Add
Ack
https://review.coreboot.org/#/c/32159/1//COMMIT_MSG@11 PS1, Line 11: Coreboot
coreboot
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/index.md File Documentation/security/index.md:
https://review.coreboot.org/#/c/32159/1/Documentation/security/index.md@8 PS1, Line 8: VBOOT
vboot
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... File Documentation/security/vboot/verified_boot_21.md:
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 1: BootGuard
Boot Guard
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 1: Coreboot
lowercase: coreboot
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 8: Bootguard
Boot Guard
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 10: Coreboot
coreboot
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 11: This document describes the mechanism implemented in Coreboot using Google VBOOT
Please add a blank line above to separate the paragraphs.
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 11: VBOOT
vboot
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 11: Coreboot
coreboot
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 12: Root
root
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 12: BootGuard
Boot Guard
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 13: Trust
trust
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 13: Coreboot
coreboot
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 16: VBOOT
vboot
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 16: Coreboot
coreboot
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 20: BootGuard
Boot Guard
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 22: BootGuard
Boot Guard
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 24: ACM
As this is documentation, please also give the full name or link to the corresponding documentation.
Added information about ACM.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 24: measured in TPM) by a piece of firmware (ACM) which itself is verified by Intel : CPU microcode
Verified by microcode?
Removing microcode. Just CPU will suffice in this case.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 26: BootGuard
Boot Guard
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 28: Authenticated Code module
Please move that above.
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 29: in
in the
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 29: PolicyManifest(BtG BPM)
Ditto.
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 29: Manifest(BtG KM)
Add a space before (.
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 35: BootGuard
Boot Guard
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 26: BootGuard are: 1. Use Intel FSP-T with Coreboot bootloader. It contains the : logic of correctly handling BtGuard enabled state. 2. Integrate ACM : (Authenticated Code module) binary in bootloader image. 3. Generate BtGuard Key : Manifest(BtG KM) and BtGuard Boot PolicyManifest(BtG BPM) and embed them in : bootloader image. a. BtG KM contains the hash of the key used for signing BtG : BPM. BtG KM is signed by the key whose hash is embedded in field-programmable : fuses. b. BtG BPM contains the hash of initial stage of boot loader. It also : stores other policies related to Intel TXT, BtG DMA protection etc. 4. Add : entries for CPU microcode patch, ACM, BtG KM and BtG BPM in FIT table. 5. : Update BootGuard related field-programmable fuses on the test platform.
Please format this as a list. […]
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 41: 2.0
Below you use 2. […]
Yes, 2.0 is the current implementation available in the open source code. 2.1 is our enhancement that makes use of vboot 2.1 libraries that is part of this upstreaming effort.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 41: Coreboot
coreboot
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 42: has been
is
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 42: feature has been described here, : https://www.coreboot.org/git-docs/Intel/vboot.html
Make it a link.
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 45: Coreboot
coreboot
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 51: Coreboot
coreboot
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 63: ,
Please use a dot.
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 64: - Some hardware designs cannot support ‘read-only’ flash region as Root of Trust
Please add a blank line above.
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 64: Root of Trust
root of trust
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 65: BootGuard
Boot Guard
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 68:
Remove or align all lines.
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 75:
One space for consistency.
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 76: Bootguard
Boot Guard
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 78: Bootguard
Boot Guard
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 79: )
Remove.
Thanks for the catch. It was actually missing the open parenthesis. Will add.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 80: VBOOT
vboot
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 86: Bootguard
Boot Guard
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 88: Bootguard
Boot Guard
Ack
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 89: GBB
Elaborate what GBB is?
On line 17, I have pasted the link to vboot git-docs in coreboot.org. That link explains what GBB is. If I also add it here, this document will bloat in size.
https://review.coreboot.org/#/c/32159/1/Documentation/security/vboot/verifie... PS1, Line 90: verified
verifies?
Ack
Hello Aaron Durbin, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32159
to look at the new patch set (#2).
Change subject: security: Document logic to verify stage/blob using vboot 2.1 library ......................................................................
security: Document logic to verify stage/blob using vboot 2.1 library
Add documentation to explain the logic that makes use of vboot 2.1 library to verify coreboot stages/blobs.
Signed-off-by: Sukerkar, Amol N amol.n.sukerkar@intel.com Change-Id: I1eb174bb4f4d84eb8f6befdce18421b6b85ccc02 --- M Documentation/security/index.md A Documentation/security/vboot/flash_partition.png A Documentation/security/vboot/vboot_21_logic.png A Documentation/security/vboot/vboot_flow_20.png A Documentation/security/vboot/vboot_flow_21.png A Documentation/security/vboot/verified_boot_21.md 6 files changed, 123 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/32159/2
Attention is currently required from: Amol N Sukerkar. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32159 )
Change subject: security: Document logic to verify stage/blob using vboot 2.1 library ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2: What’s the status of this effort?
Julius Werner has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/32159 )
Change subject: security: Document logic to verify stage/blob using vboot 2.1 library ......................................................................
Abandoned
This direction of development was abandoned and instead the CONFIG_CBFS_VERIFICATION effort is intended to solve this use case.
-
Amol N Sukerkar (Code Review) -
David Hendricks (Code Review)
-
Julius Werner (Code Review)
-
Paul Menzel (Code Review)
-
Philipp Deppenwiese (Code Review)