Wim Vervoorn has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
vendorcode/eltan/security: Switch to vb2 vboot library
The eltan verified_boot is using the vboot 2.1 data structures and code, as well as the fwlib21 build target, they are all depreciated. Refer to CB:37654 for more information.
The verified_boot code is updated to use the vb2 structures and code and make sure only public functions are used.
BUG=N/A TEST=build
Change-Id: I1e1a7bce6110fe35221a4d7a47c1eb7c7074c318 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/vendorcode/eltan/security/include/cb_sha.h M src/vendorcode/eltan/security/lib/Makefile.inc M src/vendorcode/eltan/security/lib/cb_sha.c M src/vendorcode/eltan/security/verified_boot/Kconfig M src/vendorcode/eltan/security/verified_boot/vboot_check.c 5 files changed, 69 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38590/1
diff --git a/src/vendorcode/eltan/security/include/cb_sha.h b/src/vendorcode/eltan/security/include/cb_sha.h index 9a231d8..8b4e647 100644 --- a/src/vendorcode/eltan/security/include/cb_sha.h +++ b/src/vendorcode/eltan/security/include/cb_sha.h @@ -16,9 +16,8 @@ #ifndef __SECURITY_CB_SHA_H__ #define __SECURITY_CB_SHA_H__
-#include <2rsa.h> -#include <vb21_common.h> #include <vb2_api.h> +#include <vb2_sha.h>
vb2_error_t cb_sha_little_endian(enum vb2_hash_algorithm hash_alg, const uint8_t *data, uint32_t len, uint8_t *digest); diff --git a/src/vendorcode/eltan/security/lib/Makefile.inc b/src/vendorcode/eltan/security/lib/Makefile.inc index 2e11fb5..45a185a 100644 --- a/src/vendorcode/eltan/security/lib/Makefile.inc +++ b/src/vendorcode/eltan/security/lib/Makefile.inc @@ -16,7 +16,7 @@ # call with $1 = stage name to create rules for building the library # for the stage and adding it to the stage's set of object files. define vendor-security-lib -VEN_SEC_LIB_$(1) = $(obj)/external/ven_sec_lib-$(1)/vboot_fw21.a +VEN_SEC_LIB_$(1) = $(obj)/external/ven_sec_lib-$(1)/vboot_fw.a VEN_SEC_CFLAGS_$(1) += $$(patsubst -I%,-I$(top)/%,\ $$(patsubst $(src)/%.h,$(top)/$(src)/%.h,\ $$(filter-out -I$(obj), $$(CPPFLAGS_$(1))))) @@ -32,29 +32,28 @@ $(MAKE) -C $(VBOOT_SOURCE) \ BUILD=$$(abspath $$(dir $$(VEN_SEC_LIB_$(1)))) \ V=$(V) \ - fwlib21 + fwlib endef # vendor-security-for-stage
CFLAGS_common += -I3rdparty/vboot/firmware/2lib/include -CFLAGS_common += -I3rdparty/vboot/firmware/lib21/include
ifneq ($(filter y,$(CONFIG_VENDORCODE_ELTAN_VBOOT) $(CONFIG_VENDORCODE_ELTAN_MBOOT)),)
bootblock-y += cb_sha.c bootblock-y += ../../../../security/vboot/vboot_logic.c $(eval $(call vendor-security-lib,bootblock)) -bootblock-srcs += $(obj)/external/ven_sec_lib-bootblock/vboot_fw21.a +bootblock-srcs += $(obj)/external/ven_sec_lib-bootblock/vboot_fw.a
postcar-y += cb_sha.c $(eval $(call vendor-security-lib,postcar)) -postcar-srcs += $(obj)/external/ven_sec_lib-postcar/vboot_fw21.a +postcar-srcs += $(obj)/external/ven_sec_lib-postcar/vboot_fw.a
ramstage-y += cb_sha.c $(eval $(call vendor-security-lib,ramstage)) -ramstage-srcs += $(obj)/external/ven_sec_lib-ramstage/vboot_fw21.a +ramstage-srcs += $(obj)/external/ven_sec_lib-ramstage/vboot_fw.a
romstage-y += cb_sha.c $(eval $(call vendor-security-lib,romstage)) -romstage-srcs += $(obj)/external/ven_sec_lib-romstage/vboot_fw21.a +romstage-srcs += $(obj)/external/ven_sec_lib-romstage/vboot_fw.a
-endif \ No newline at end of file +endif diff --git a/src/vendorcode/eltan/security/lib/cb_sha.c b/src/vendorcode/eltan/security/lib/cb_sha.c index 20a84af..b9777b7 100644 --- a/src/vendorcode/eltan/security/lib/cb_sha.c +++ b/src/vendorcode/eltan/security/lib/cb_sha.c @@ -20,11 +20,19 @@ { int i; int rv; - uint32_t digest_size = vb2_digest_size(hash_alg); + uint32_t digest_size; uint8_t result[VB2_MAX_DIGEST_SIZE];
- if (!digest_size) + switch (hash_alg) { + case VB2_HASH_SHA256: + digest_size = VB2_SHA256_DIGEST_SIZE; + break; + case VB2_HASH_SHA512: + digest_size = VB2_SHA512_DIGEST_SIZE; + break; + default: return VB2_ERROR_SHA_INIT_ALGORITHM; + }
rv = vb2_digest_buffer(data, len, hash_alg, (uint8_t *)&result, digest_size); if (rv) diff --git a/src/vendorcode/eltan/security/verified_boot/Kconfig b/src/vendorcode/eltan/security/verified_boot/Kconfig index ab254c4..3f95bef 100644 --- a/src/vendorcode/eltan/security/verified_boot/Kconfig +++ b/src/vendorcode/eltan/security/verified_boot/Kconfig @@ -61,7 +61,6 @@
config VENDORCODE_ELTAN_VBOOT_KEY_SIZE int - default 610 if VENDORCODE_ELTAN_VBOOT_USE_SHA512 - default 576 + default 552
endmenu # Verified Boot (verified_boot) diff --git a/src/vendorcode/eltan/security/verified_boot/vboot_check.c b/src/vendorcode/eltan/security/verified_boot/vboot_check.c index 461a847..ce7e99c 100644 --- a/src/vendorcode/eltan/security/verified_boot/vboot_check.c +++ b/src/vendorcode/eltan/security/verified_boot/vboot_check.c @@ -13,6 +13,9 @@ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. */ + +#define NEED_VB20_INTERNALS /* Peeking into vb2_shared_data */ + #include <boot_device.h> #include <bootmem.h> #include <cbfs.h> @@ -32,12 +35,17 @@ int verified_boot_check_manifest(void) { uint8_t *buffer; - uint8_t sig_buffer[1024]; /* used to build vb21_signature */ - size_t size = 0; - struct vb2_public_key key; - struct vb2_workbuf wb; - struct vb21_signature *vb2_sig_hdr = (struct vb21_signature *)sig_buffer; - uint8_t wb_buffer[1024]; + struct vb2_context *ctx; + struct vb2_kernel_preamble *pre; + static struct vb2_shared_data *sd; + size_t size; + uint8_t wb_buffer[2800]; + + if (vb2api_init(&wb_buffer, sizeof(wb_buffer), &ctx)) { + goto fail; + } + + sd = vb2_get_sd(ctx);
buffer = cbfs_boot_map_with_leak(RSA_PUBLICKEY_FILE_NAME, CBFS_TYPE_RAW, &size); if (!buffer || !size) { @@ -46,48 +54,61 @@ }
if ((size != CONFIG_VENDORCODE_ELTAN_VBOOT_KEY_SIZE) || - (buffer != (void *)CONFIG_VENDORCODE_ELTAN_VBOOT_KEY_LOCATION)) { + (buffer != (void *)CONFIG_VENDORCODE_ELTAN_VBOOT_KEY_LOCATION)) { printk(BIOS_ERR, "ERROR: Illegal public key!\n"); goto fail; }
- if (vb21_unpack_key(&key, buffer, size)) { - printk(BIOS_ERR, "ERROR: Invalid public key!\n"); + /* + * Check if all items will fit into workbuffer: + * vb2_shared data, Public Key, Preamble data + */ + if ((sd->workbuf_used + size + sizeof(struct vb2_kernel_preamble) + + ((CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_ITEMS * DIGEST_SIZE) + (2048/8))) > + sizeof(wb_buffer)) { + printk(BIOS_ERR, "ERROR: Work buffer too small\n"); goto fail; }
+ /* Add public key */ + sd->data_key_offset = sd->workbuf_used; + sd->data_key_size = size; + sd->workbuf_used += sd->data_key_size; + memcpy((void *)((void *)sd + (long)sd->data_key_offset), (uint8_t *)buffer, size); + + /* Fill preamble area */ + sd->preamble_size = sizeof(struct vb2_kernel_preamble); + sd->preamble_offset = sd->data_key_offset + sd->data_key_size; + sd->workbuf_used += sd->preamble_size; + pre = (struct vb2_kernel_preamble *)((void *)sd + (long)sd->preamble_offset); + + pre->flags = VB2_FIRMWARE_PREAMBLE_DISALLOW_HWCRYPTO; + + /* Fill body_signature (vb2_structure). RSA2048 key is used */ cbfs_boot_map_with_leak("oemmanifest.bin", CBFS_TYPE_RAW, &size); - if (size != (CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_ITEMS * DIGEST_SIZE) + - vb2_rsa_sig_size(VB2_SIG_RSA2048)) { + if (size != ((CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_ITEMS * DIGEST_SIZE) + (2048/8))) { printk(BIOS_ERR, "ERROR: Incorrect manifest size!\n"); goto fail; } - - /* prepare work buffer structure */ - wb.buf = (uint8_t *)&wb_buffer; - wb.size = sizeof(wb_buffer); - - /* Build vb2_sig_hdr buffer */ - vb2_sig_hdr->sig_offset = sizeof(struct vb21_signature) + - (CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_ITEMS * DIGEST_SIZE); - vb2_sig_hdr->sig_alg = VB2_SIG_RSA2048; - vb2_sig_hdr->sig_size = vb2_rsa_sig_size(VB2_SIG_RSA2048); - vb2_sig_hdr->hash_alg = HASH_ALG; - vb2_sig_hdr->data_size = CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_ITEMS * DIGEST_SIZE; - memcpy(&sig_buffer[sizeof(struct vb21_signature)], + pre->body_signature.data_size = CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_ITEMS * + DIGEST_SIZE; + pre->body_signature.sig_offset = sizeof(struct vb2_signature) + + pre->body_signature.data_size; + pre->body_signature.sig_size = size - pre->body_signature.data_size; + sd->workbuf_used += size; + memcpy((void *)((void *)&pre->body_signature + (long)sizeof(struct vb2_signature)), (uint8_t *)CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_LOC, size);
- if (vb21_verify_data(&sig_buffer[sizeof(struct vb21_signature)], vb2_sig_hdr->data_size, - (struct vb21_signature *)&sig_buffer, &key, &wb)) { - printk(BIOS_ERR, "ERROR: Signature verification failed for hash table\n"); + + if (vb2api_verify_kernel_data(ctx, (void *)CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_LOC, + pre->body_signature.data_size)) goto fail; - }
printk(BIOS_INFO, "%s: Successfully verified hash_table signature.\n", __func__); return 0;
fail: - die("HASH table verification failed!\n"); + die("ERROR: HASH table verification failed!\n"); return -1; }
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38590/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38590/1//COMMIT_MSG@10 PS1, Line 10: depreciated deprecated
https://review.coreboot.org/c/coreboot/+/38590/1/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/Kconfig:
https://review.coreboot.org/c/coreboot/+/38590/1/src/vendorcode/eltan/securi... PS1, Line 64: default 552 Why is this changed?
Hello Frans Hendriks, build bot (Jenkins), Joel Kitching, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38590
to look at the new patch set (#2).
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
vendorcode/eltan/security: Switch to vb2 vboot library
The eltan verified_boot is using the vboot 2.1 data structures and code, as well as the fwlib21 build target, they are all deprecated. Refer to CB:37654 for more information.
The verified_boot code is updated to use the vb2 structures and code and make sure only public functions are used.
BUG=N/A TEST=build
Change-Id: I1e1a7bce6110fe35221a4d7a47c1eb7c7074c318 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/vendorcode/eltan/security/include/cb_sha.h M src/vendorcode/eltan/security/lib/Makefile.inc M src/vendorcode/eltan/security/lib/cb_sha.c M src/vendorcode/eltan/security/verified_boot/Kconfig M src/vendorcode/eltan/security/verified_boot/vboot_check.c 5 files changed, 69 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38590/2
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38590/1/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/Kconfig:
https://review.coreboot.org/c/coreboot/+/38590/1/src/vendorcode/eltan/securi... PS1, Line 64: default 552
Why is this changed?
The size of the public key that is included in coreboot is identical in both cases. So there is no need any longer to make this difference.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 2: Code-Review+1
(2 comments)
Thanks, this is a step in the right direction.
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/lib/Makefile.inc:
PS2: Can't you just remove this whole file and select CONFIG_VBOOT_LIB now?
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... PS2, Line 17: #define NEED_VB20_INTERNALS /* Peeking into vb2_shared_data */ What is the long-term plan here? We do not want future development on vboot to be restricted by random vendorcode users. This macro is supposed to be deprecated anyway and Joel is working hard to remove it from all of coreboot.
Are you ever going to drop all of this and completely move to the standard implementation in src/security/vboot? If not, this *will* break sooner or later, and at that point if it's a choice between not being able to uprev vboot anymore or deleting your vendorcode, we'll probably eventually have to do the latter.
Hello Julius Werner, Frans Hendriks, build bot (Jenkins), Joel Kitching, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38590
to look at the new patch set (#3).
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
vendorcode/eltan/security: Switch to vb2 vboot library
The eltan verified_boot is using the vboot 2.1 data structures and code, as well as the fwlib21 build target, they are all depreciated. Refer to CB:37654 for more information.
The verified_boot code is updated to use the vb2 structures and code and make sure only public functions are used.
BUG=N/A TEST=tested on facebook fbg1701
Change-Id: I1e1a7bce6110fe35221a4d7a47c1eb7c7074c318 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/security/vboot/Kconfig M src/vendorcode/eltan/security/Makefile.inc D src/vendorcode/eltan/security/include/cb_sha.h D src/vendorcode/eltan/security/lib/Makefile.inc D src/vendorcode/eltan/security/lib/cb_sha.c M src/vendorcode/eltan/security/mboot/Kconfig M src/vendorcode/eltan/security/mboot/mboot.c M src/vendorcode/eltan/security/mboot/mboot.h M src/vendorcode/eltan/security/verified_boot/Kconfig M src/vendorcode/eltan/security/verified_boot/Makefile.inc M src/vendorcode/eltan/security/verified_boot/vboot_check.c M src/vendorcode/eltan/security/verified_boot/vboot_check.h 12 files changed, 58 insertions(+), 168 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38590/3
Hello Aaron Durbin, Julius Werner, Frans Hendriks, build bot (Jenkins), Joel Kitching, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38590
to look at the new patch set (#4).
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
vendorcode/eltan/security: Switch to vb2 vboot library
The eltan verified_boot is using the vboot 2.1 data structures and code, as well as the fwlib21 build target, they are all depreciated. Refer to CB:37654 for more information.
The verified_boot code is updated to use the vb2 structures and code and make sure only public functions are used.
BUG=N/A TEST=tested on facebook fbg1701
Change-Id: I1e1a7bce6110fe35221a4d7a47c1eb7c7074c318 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/security/vboot/Kconfig M src/vendorcode/eltan/security/Makefile.inc D src/vendorcode/eltan/security/include/cb_sha.h D src/vendorcode/eltan/security/lib/Makefile.inc D src/vendorcode/eltan/security/lib/cb_sha.c M src/vendorcode/eltan/security/mboot/Kconfig M src/vendorcode/eltan/security/mboot/mboot.c M src/vendorcode/eltan/security/mboot/mboot.h M src/vendorcode/eltan/security/verified_boot/Kconfig M src/vendorcode/eltan/security/verified_boot/Makefile.inc M src/vendorcode/eltan/security/verified_boot/vboot_check.c M src/vendorcode/eltan/security/verified_boot/vboot_check.h 12 files changed, 59 insertions(+), 169 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38590/4
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/lib/Makefile.inc:
PS2:
Can't you just remove this whole file and select CONFIG_VBOOT_LIB now?
Thanks for the suggestion. We have removed what's left of this library so now we're only using the vboot library.
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... PS2, Line 17: #define NEED_VB20_INTERNALS /* Peeking into vb2_shared_data */
What is the long-term plan here? We do not want future development on vboot to be restricted by rand […]
Understood. For the next project we moved to a vboot implementation. For this one we don't have plans to switch at the moment. Is there a timeline from your side?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/38590/1/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/Kconfig:
https://review.coreboot.org/c/coreboot/+/38590/1/src/vendorcode/eltan/securi... PS1, Line 64: default 552
The size of the public key that is included in coreboot is identical in both cases. […]
Thank you. I’d like to see such comments in the commit message.
Hello Aaron Durbin, Julius Werner, Frans Hendriks, Paul Menzel, build bot (Jenkins), Joel Kitching, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38590
to look at the new patch set (#5).
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
vendorcode/eltan/security: Switch to vb2 vboot library
The eltan verified_boot is using the vboot 2.1 data structures and code, as well as the fwlib21 build target, they are all deprecated. Refer to CB:37654 for more information.
The verified_boot code is updated to use the vb2 structures and code and make sure only public functions are used.
BUG=N/A TEST=tested on facebook fbg1701
Change-Id: I1e1a7bce6110fe35221a4d7a47c1eb7c7074c318 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/security/vboot/Kconfig M src/vendorcode/eltan/security/Makefile.inc D src/vendorcode/eltan/security/include/cb_sha.h D src/vendorcode/eltan/security/lib/Makefile.inc D src/vendorcode/eltan/security/lib/cb_sha.c M src/vendorcode/eltan/security/mboot/Kconfig M src/vendorcode/eltan/security/mboot/mboot.c M src/vendorcode/eltan/security/mboot/mboot.h M src/vendorcode/eltan/security/verified_boot/Kconfig M src/vendorcode/eltan/security/verified_boot/Makefile.inc M src/vendorcode/eltan/security/verified_boot/vboot_check.c M src/vendorcode/eltan/security/verified_boot/vboot_check.h 12 files changed, 59 insertions(+), 169 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38590/5
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38590/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38590/1//COMMIT_MSG@10 PS1, Line 10: depreciated
deprecated
Done
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... PS2, Line 17: #define NEED_VB20_INTERNALS /* Peeking into vb2_shared_data */
Is there a timeline from your side?
It's more of a question of when the next situation comes up where we want to change some interface you're using here and it's not trivial to adapt your code to the changes. I can't predict when that happens, could be next week or not for another year. But when it happens we wouldn't want to get stuck and be unable to continue our work because of this.
If you're using this for a current product but are planning to abandon it afterwards, it would probably be best if you could cut off a branch for work on that product (here or on some local mirror of yours) and then delete this code from master.
https://review.coreboot.org/c/coreboot/+/38590/5/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/5/src/vendorcode/eltan/securi... PS5, Line 162: vb2_digest_buffer Uhh... you sure this preserves behavior? Because your previous code was byte-swapping this hash (which is odd, btw, I've never seen any other code treat a whole 32-byte hash as "little-endian"), and you're not doing that anymore now.
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... PS2, Line 17: #define NEED_VB20_INTERNALS /* Peeking into vb2_shared_data */
Is there a timeline from your side? […]
Understood, the issue is that we need to have the code for this project upstreamed.
Basically what we need to have is a way to add a public key to the workbuffer so this can be used for the signature verification. I haven't really found a public API to do this. Do you have suggestion? Or alternatively is it possible to make the api to do this public?
https://review.coreboot.org/c/coreboot/+/38590/5/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/5/src/vendorcode/eltan/securi... PS5, Line 162: vb2_digest_buffer
Uhh... […]
We verified the behavior. It turned out that the need for the "little-endian" hashes was caused by legacy in the scripts we used to create them. After correcting the scripts there is no need any longer to do this. The impact of the change is now handled in the script to generate the manifest containing the hashes.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38590/5/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/5/src/vendorcode/eltan/securi... PS5, Line 162: vb2_digest_buffer
We verified the behavior. […]
Please note this in the commit message.
Hello Aaron Durbin, Julius Werner, Frans Hendriks, Paul Menzel, build bot (Jenkins), Joel Kitching, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38590
to look at the new patch set (#6).
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
vendorcode/eltan/security: Switch to vb2 vboot library
The eltan verified_boot is using the vboot 2.1 data structures and code, as well as the fwlib21 build target, they are all deprecated. Refer to CB:37654 for more information.
The verified_boot code is updated to use the vb2 structures and code and make sure only public functions are used.
To align with standard handing the endianness of the hash entries in the manifest is changed. The openssl dgst output can now be used as is.
BUG=N/A TEST=tested on facebook fbg1701
Change-Id: I1e1a7bce6110fe35221a4d7a47c1eb7c7074c318 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/security/vboot/Kconfig M src/vendorcode/eltan/security/Makefile.inc D src/vendorcode/eltan/security/include/cb_sha.h D src/vendorcode/eltan/security/lib/Makefile.inc D src/vendorcode/eltan/security/lib/cb_sha.c M src/vendorcode/eltan/security/mboot/Kconfig M src/vendorcode/eltan/security/mboot/mboot.c M src/vendorcode/eltan/security/mboot/mboot.h M src/vendorcode/eltan/security/verified_boot/Kconfig M src/vendorcode/eltan/security/verified_boot/Makefile.inc M src/vendorcode/eltan/security/verified_boot/vboot_check.c M src/vendorcode/eltan/security/verified_boot/vboot_check.h 12 files changed, 59 insertions(+), 169 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38590/6
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38590/5/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/5/src/vendorcode/eltan/securi... PS5, Line 162: vb2_digest_buffer
Please note this in the commit message.
Done
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... PS2, Line 17: #define NEED_VB20_INTERNALS /* Peeking into vb2_shared_data */
Understood, the issue is that we need to have the code for this project upstreamed.
Can you explain why? I assume you are not planning to ship firmware updates from ToT coreboot forever? I think most companies cut a stabilization branch for their products at some point anyway to decrease maintenance effort and risk for future updates.
Basically what we need to have is a way to add a public key to the workbuffer so this can be used for the signature verification. I haven't really found a public API to do this. Do you have suggestion? Or alternatively is it possible to make the api to do this public?
Can you describe in more high level what goals you're trying to achieve (e.g. what you need integrity checked or measured and why)? In general our goal with the vboot/coreboot integration is to eventually support enough options to cover everyone's high-level needs, and if your situation really has a useful use case that the existing solution cannot cover we can talk about how to add it. But we basically already support full measurement and integrity checking of the firmware, and I don't really understand what your solution is achieving that is different from that. The inner workings of vboot and the work buffer, how exactly keys are handled and what is signed by what are supposed to be implementation details. We want to make sure that the solution overall covers the use cases people have, but we don't want everyone to design their own key ladder and support all possible different ways to plug crypto primitives together (because that wouldn't be maintainable).
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 6: Code-Review+2
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 6:
(2 comments)
Thanks for your work on this, Wim. This is a move in the right direction, allowing us to remove the fwlib21 target. On the other hand, it just shifts our pain point to another area -- namely, now you are using vboot2 internals instead of vboot2.1 internals. That means we *still* need to keep NEED_VB20_INTERNALS around, a macro we plan to deprecate within the next month or so.
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... PS2, Line 17: #define NEED_VB20_INTERNALS /* Peeking into vb2_shared_data */
Understood, the issue is that we need to have the code for this project upstreamed. […]
Julius has made some good points here. Could we know more about where vboot falls short of covering Eltan verified boot requirements?
https://review.coreboot.org/c/coreboot/+/38590/6/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/6/src/vendorcode/eltan/securi... PS6, Line 17: /* Peeking into vb2_shared_data */ May as well delete this comment since that's not the only thing you're doing here.
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... PS2, Line 17: #define NEED_VB20_INTERNALS /* Peeking into vb2_shared_data */
Julius has made some good points here. […]
Let me explain what we are trying to do at a high level. What this does is provide a simple verified boot solution for systems that don't have a TPM (I know that for this specific system a TPM is available but that is not relevant for the verified boot solution).
This verified boot solution depends on the bootblock being read-only while the remainder of the can be reflashed. The verification relies on a manifest containing SHA-256 hashes of the components that need to be verified. This manifest is in the RW portion of the flash.
The manifest is signed so its integrity can be verified. The public key used for this verification is contained in the RO area as well.
At this point in time we can do everything we need using the public API except preparing an environment that can be used to perform the verification of the manifest. As you can see in the code we can use the vb2api_verify_kernel_data for the verification.
The issue we still have is to find a proper way to setup the environment to actually perform this verification. The api that is available to do this totally relies on the vboot implementation and implements the steps required for this. That is not what we need in this case.
So looking at the functionality the ideal solution for us would be to have a way to import a key into the environment and to have the possibility to verify the signature of a custom datablock (the manifest in our case). The call to verify would be very similar to what's in the vb2api_verify_kernel_data at this moment except that it is not tied to the kernel.
I have marked the issue as resolved so it's possible for this patch to proceed but we can still continue this discussion and find a way to come to a workable solution for all of us.
Personally I don't think that adding the possibility to perform a signature verification on a custom data block would compromise the integrity of your solution in any way or make it less maintaineable.
Hello Aaron Durbin, Julius Werner, Frans Hendriks, Paul Menzel, build bot (Jenkins), Joel Kitching, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38590
to look at the new patch set (#7).
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
vendorcode/eltan/security: Switch to vb2 vboot library
The eltan verified_boot is using the vboot 2.1 data structures and code, as well as the fwlib21 build target, they are all deprecated. Refer to CB:37654 for more information.
The verified_boot code is updated to use the vb2 structures and code and make sure only public functions are used.
BUG=N/A TEST=build
Change-Id: I1e1a7bce6110fe35221a4d7a47c1eb7c7074c318 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/security/vboot/Kconfig M src/vendorcode/eltan/security/Makefile.inc D src/vendorcode/eltan/security/include/cb_sha.h D src/vendorcode/eltan/security/lib/Makefile.inc D src/vendorcode/eltan/security/lib/cb_sha.c M src/vendorcode/eltan/security/mboot/Kconfig M src/vendorcode/eltan/security/mboot/mboot.c M src/vendorcode/eltan/security/mboot/mboot.h M src/vendorcode/eltan/security/verified_boot/Kconfig M src/vendorcode/eltan/security/verified_boot/Makefile.inc M src/vendorcode/eltan/security/verified_boot/vboot_check.c M src/vendorcode/eltan/security/verified_boot/vboot_check.h 12 files changed, 59 insertions(+), 169 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38590/7
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38590/6/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/6/src/vendorcode/eltan/securi... PS6, Line 17: /* Peeking into vb2_shared_data */
May as well delete this comment since that's not the only thing you're doing here.
Done
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 7: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... PS2, Line 17: #define NEED_VB20_INTERNALS /* Peeking into vb2_shared_data */
Let me explain what we are trying to do at a high level. […]
Yeah, I don't see anything here that you couldn't achieve with standard vboot. You should not design a custom solution for this, what we have essentially does what you want already.
You do not need a TPM to run vboot. The TPM is only used for rollback protection and developer mode. If you don't want either of those features, just select CONFIG_VBOOT_MOCK_SECDATA and the TPM stuff will be mocked out.
The existing vboot already starts from a write-protected read-only part and then chains into an updateable read-write part, where the read-write part is signed by keys in the read-only part. Sounds like this is exactly the system you're looking for, so why don't you just use it?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 7:
The question about reusing vboot or not is orthogonal to this specific change. While it may be interesting to pursue it further, there's no need to block this commit to wait on a decision.
This is another step towards removing fwlib21 from vboot, thanks Wim for helping here!
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
vendorcode/eltan/security: Switch to vb2 vboot library
The eltan verified_boot is using the vboot 2.1 data structures and code, as well as the fwlib21 build target, they are all deprecated. Refer to CB:37654 for more information.
The verified_boot code is updated to use the vb2 structures and code and make sure only public functions are used.
BUG=N/A TEST=build
Change-Id: I1e1a7bce6110fe35221a4d7a47c1eb7c7074c318 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/38590 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Frans Hendriks fhendriks@eltan.com --- M src/security/vboot/Kconfig M src/vendorcode/eltan/security/Makefile.inc D src/vendorcode/eltan/security/include/cb_sha.h D src/vendorcode/eltan/security/lib/Makefile.inc D src/vendorcode/eltan/security/lib/cb_sha.c M src/vendorcode/eltan/security/mboot/Kconfig M src/vendorcode/eltan/security/mboot/mboot.c M src/vendorcode/eltan/security/mboot/mboot.h M src/vendorcode/eltan/security/verified_boot/Kconfig M src/vendorcode/eltan/security/verified_boot/Makefile.inc M src/vendorcode/eltan/security/verified_boot/vboot_check.c M src/vendorcode/eltan/security/verified_boot/vboot_check.h 12 files changed, 59 insertions(+), 169 deletions(-)
Approvals: build bot (Jenkins): Verified Frans Hendriks: Looks good to me, approved
diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index 30b99af..ea70e65 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -17,7 +17,6 @@
config VBOOT_LIB bool - depends on !VENDORCODE_ELTAN_VBOOT && !VENDORCODE_ELTAN_MBOOT help Build and link the vboot library. Makes the vboot API accessible across all coreboot stages, without enabling vboot verification. For verification, diff --git a/src/vendorcode/eltan/security/Makefile.inc b/src/vendorcode/eltan/security/Makefile.inc index 16f17fd..c0d9057 100644 --- a/src/vendorcode/eltan/security/Makefile.inc +++ b/src/vendorcode/eltan/security/Makefile.inc @@ -12,7 +12,6 @@ ## GNU General Public License for more details. ##
-subdirs-y += lib subdirs-y += verified_boot subdirs-y += mboot
diff --git a/src/vendorcode/eltan/security/include/cb_sha.h b/src/vendorcode/eltan/security/include/cb_sha.h deleted file mode 100644 index 9a231d8..0000000 --- a/src/vendorcode/eltan/security/include/cb_sha.h +++ /dev/null @@ -1,26 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2018-2019, Eltan B.V. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#ifndef __SECURITY_CB_SHA_H__ -#define __SECURITY_CB_SHA_H__ - -#include <2rsa.h> -#include <vb21_common.h> -#include <vb2_api.h> - -vb2_error_t cb_sha_little_endian(enum vb2_hash_algorithm hash_alg, const uint8_t *data, - uint32_t len, uint8_t *digest); - -#endif diff --git a/src/vendorcode/eltan/security/lib/Makefile.inc b/src/vendorcode/eltan/security/lib/Makefile.inc deleted file mode 100644 index 2e11fb5..0000000 --- a/src/vendorcode/eltan/security/lib/Makefile.inc +++ /dev/null @@ -1,60 +0,0 @@ -# -# This file is part of the coreboot project. -# -# Copyright (C) 2018-2019 Eltan B.V. -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; version 2 of the License. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# - -# call with $1 = stage name to create rules for building the library -# for the stage and adding it to the stage's set of object files. -define vendor-security-lib -VEN_SEC_LIB_$(1) = $(obj)/external/ven_sec_lib-$(1)/vboot_fw21.a -VEN_SEC_CFLAGS_$(1) += $$(patsubst -I%,-I$(top)/%,\ - $$(patsubst $(src)/%.h,$(top)/$(src)/%.h,\ - $$(filter-out -I$(obj), $$(CPPFLAGS_$(1))))) -VEN_SEC_CFLAGS_$(1) += $$(CFLAGS_$(1)) -VEN_SEC_CFLAGS_$(1) += $$($(1)-c-ccopts) -VEN_SEC_CFLAGS_$(1) += -I$(abspath $(obj)) -Wno-missing-prototypes - -$$(VEN_SEC_LIB_$(1)): $(obj)/config.h - printf " MAKE $(subst $(obj)/,,$(@))\n" - +FIRMWARE_ARCH=$$(ARCHDIR-$$(ARCH-$(1)-y)) \ - CC="$$(CC_$(1))" \ - CFLAGS="$$(VEN_SEC_CFLAGS_$(1))" VBOOT2="y" \ - $(MAKE) -C $(VBOOT_SOURCE) \ - BUILD=$$(abspath $$(dir $$(VEN_SEC_LIB_$(1)))) \ - V=$(V) \ - fwlib21 -endef # vendor-security-for-stage - -CFLAGS_common += -I3rdparty/vboot/firmware/2lib/include -CFLAGS_common += -I3rdparty/vboot/firmware/lib21/include - -ifneq ($(filter y,$(CONFIG_VENDORCODE_ELTAN_VBOOT) $(CONFIG_VENDORCODE_ELTAN_MBOOT)),) - -bootblock-y += cb_sha.c -bootblock-y += ../../../../security/vboot/vboot_logic.c -$(eval $(call vendor-security-lib,bootblock)) -bootblock-srcs += $(obj)/external/ven_sec_lib-bootblock/vboot_fw21.a - -postcar-y += cb_sha.c -$(eval $(call vendor-security-lib,postcar)) -postcar-srcs += $(obj)/external/ven_sec_lib-postcar/vboot_fw21.a - -ramstage-y += cb_sha.c -$(eval $(call vendor-security-lib,ramstage)) -ramstage-srcs += $(obj)/external/ven_sec_lib-ramstage/vboot_fw21.a - -romstage-y += cb_sha.c -$(eval $(call vendor-security-lib,romstage)) -romstage-srcs += $(obj)/external/ven_sec_lib-romstage/vboot_fw21.a - -endif \ No newline at end of file diff --git a/src/vendorcode/eltan/security/lib/cb_sha.c b/src/vendorcode/eltan/security/lib/cb_sha.c deleted file mode 100644 index 20a84af..0000000 --- a/src/vendorcode/eltan/security/lib/cb_sha.c +++ /dev/null @@ -1,38 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2019 Eltan B.V. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#include <cb_sha.h> - -vb2_error_t cb_sha_little_endian(enum vb2_hash_algorithm hash_alg, const uint8_t *data, - uint32_t len, uint8_t *digest) -{ - int i; - int rv; - uint32_t digest_size = vb2_digest_size(hash_alg); - uint8_t result[VB2_MAX_DIGEST_SIZE]; - - if (!digest_size) - return VB2_ERROR_SHA_INIT_ALGORITHM; - - rv = vb2_digest_buffer(data, len, hash_alg, (uint8_t *)&result, digest_size); - if (rv) - return rv; - - for (i = 0; i < digest_size; ++i) { - /* use little endian */ - digest[digest_size - i - 1] = result[i]; - } - return rv; -} diff --git a/src/vendorcode/eltan/security/mboot/Kconfig b/src/vendorcode/eltan/security/mboot/Kconfig index c4e8dba..b95c125 100644 --- a/src/vendorcode/eltan/security/mboot/Kconfig +++ b/src/vendorcode/eltan/security/mboot/Kconfig @@ -17,6 +17,7 @@ config VENDORCODE_ELTAN_MBOOT bool "Measure firmware with mboot." default n + select VBOOT_LIB help Enabling MBOOT will use mboot to measure the components of the firmware (stages, payload, etc). diff --git a/src/vendorcode/eltan/security/mboot/mboot.c b/src/vendorcode/eltan/security/mboot/mboot.c index c5523a5..4429c1f 100644 --- a/src/vendorcode/eltan/security/mboot/mboot.c +++ b/src/vendorcode/eltan/security/mboot/mboot.c @@ -142,8 +142,8 @@ /* The hash is provided as data */ memcpy(digest->digest.sha256, (void *)hashData, hashDataLen); } else { - if (cb_sha_little_endian(VB2_HASH_SHA256, hashData, hashDataLen, - digest->digest.sha256)) + if (vb2_digest_buffer(hashData, hashDataLen, VB2_HASH_SHA256, digest->digest.sha256, + VB2_SHA256_DIGEST_SIZE)) return TPM_E_IOERROR; }
diff --git a/src/vendorcode/eltan/security/mboot/mboot.h b/src/vendorcode/eltan/security/mboot/mboot.h index 79f2308..9cb94b1 100644 --- a/src/vendorcode/eltan/security/mboot/mboot.h +++ b/src/vendorcode/eltan/security/mboot/mboot.h @@ -20,7 +20,6 @@ #include <arch/io.h> #include <arch/acpi.h> #include <string.h> -#include <cb_sha.h> #include <console/console.h> #include <cbfs.h> #include <lib.h> diff --git a/src/vendorcode/eltan/security/verified_boot/Kconfig b/src/vendorcode/eltan/security/verified_boot/Kconfig index ab254c4..d6ff541 100644 --- a/src/vendorcode/eltan/security/verified_boot/Kconfig +++ b/src/vendorcode/eltan/security/verified_boot/Kconfig @@ -18,6 +18,7 @@ bool "Enable Verified Boot" depends on !VBOOT default n + select VBOOT_LIB
config VENDORCODE_ELTAN_VBOOT_SIGNED_MANIFEST bool "Enable Signed Manifest" @@ -57,11 +58,10 @@ config VENDORCODE_ELTAN_VBOOT_KEY_FILE string "Verified boot Key File" depends on VENDORCODE_ELTAN_VBOOT_SIGNED_MANIFEST - default "3rdparty/eltan/verified_boot/Keys/key.vbpubk2" + default "3rdparty/eltan/verified_boot/Keys/key.vbpubk"
config VENDORCODE_ELTAN_VBOOT_KEY_SIZE int - default 610 if VENDORCODE_ELTAN_VBOOT_USE_SHA512 - default 576 + default 552
endmenu # Verified Boot (verified_boot) diff --git a/src/vendorcode/eltan/security/verified_boot/Makefile.inc b/src/vendorcode/eltan/security/verified_boot/Makefile.inc index 97d8f81..827535b 100644 --- a/src/vendorcode/eltan/security/verified_boot/Makefile.inc +++ b/src/vendorcode/eltan/security/verified_boot/Makefile.inc @@ -17,6 +17,7 @@
CPPFLAGS_common += -I$(src)/security/vboot
+bootblock-y += ../../../../security/vboot/vboot_logic.c bootblock-y += vboot_check.c postcar-y += vboot_check.c romstage-y += vboot_check.c diff --git a/src/vendorcode/eltan/security/verified_boot/vboot_check.c b/src/vendorcode/eltan/security/verified_boot/vboot_check.c index 461a847..2edd8f9 100644 --- a/src/vendorcode/eltan/security/verified_boot/vboot_check.c +++ b/src/vendorcode/eltan/security/verified_boot/vboot_check.c @@ -13,6 +13,9 @@ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. */ + +#define NEED_VB20_INTERNALS + #include <boot_device.h> #include <bootmem.h> #include <cbfs.h> @@ -32,12 +35,17 @@ int verified_boot_check_manifest(void) { uint8_t *buffer; - uint8_t sig_buffer[1024]; /* used to build vb21_signature */ - size_t size = 0; - struct vb2_public_key key; - struct vb2_workbuf wb; - struct vb21_signature *vb2_sig_hdr = (struct vb21_signature *)sig_buffer; - uint8_t wb_buffer[1024]; + struct vb2_context *ctx; + struct vb2_kernel_preamble *pre; + static struct vb2_shared_data *sd; + size_t size; + uint8_t wb_buffer[2800]; + + if (vb2api_init(&wb_buffer, sizeof(wb_buffer), &ctx)) { + goto fail; + } + + sd = vb2_get_sd(ctx);
buffer = cbfs_boot_map_with_leak(RSA_PUBLICKEY_FILE_NAME, CBFS_TYPE_RAW, &size); if (!buffer || !size) { @@ -46,48 +54,61 @@ }
if ((size != CONFIG_VENDORCODE_ELTAN_VBOOT_KEY_SIZE) || - (buffer != (void *)CONFIG_VENDORCODE_ELTAN_VBOOT_KEY_LOCATION)) { + (buffer != (void *)CONFIG_VENDORCODE_ELTAN_VBOOT_KEY_LOCATION)) { printk(BIOS_ERR, "ERROR: Illegal public key!\n"); goto fail; }
- if (vb21_unpack_key(&key, buffer, size)) { - printk(BIOS_ERR, "ERROR: Invalid public key!\n"); + /* + * Check if all items will fit into workbuffer: + * vb2_shared data, Public Key, Preamble data + */ + if ((sd->workbuf_used + size + sizeof(struct vb2_kernel_preamble) + + ((CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_ITEMS * DIGEST_SIZE) + (2048/8))) > + sizeof(wb_buffer)) { + printk(BIOS_ERR, "ERROR: Work buffer too small\n"); goto fail; }
+ /* Add public key */ + sd->data_key_offset = sd->workbuf_used; + sd->data_key_size = size; + sd->workbuf_used += sd->data_key_size; + memcpy((void *)((void *)sd + (long)sd->data_key_offset), (uint8_t *)buffer, size); + + /* Fill preamble area */ + sd->preamble_size = sizeof(struct vb2_kernel_preamble); + sd->preamble_offset = sd->data_key_offset + sd->data_key_size; + sd->workbuf_used += sd->preamble_size; + pre = (struct vb2_kernel_preamble *)((void *)sd + (long)sd->preamble_offset); + + pre->flags = VB2_FIRMWARE_PREAMBLE_DISALLOW_HWCRYPTO; + + /* Fill body_signature (vb2_structure). RSA2048 key is used */ cbfs_boot_map_with_leak("oemmanifest.bin", CBFS_TYPE_RAW, &size); - if (size != (CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_ITEMS * DIGEST_SIZE) + - vb2_rsa_sig_size(VB2_SIG_RSA2048)) { + if (size != ((CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_ITEMS * DIGEST_SIZE) + (2048/8))) { printk(BIOS_ERR, "ERROR: Incorrect manifest size!\n"); goto fail; } - - /* prepare work buffer structure */ - wb.buf = (uint8_t *)&wb_buffer; - wb.size = sizeof(wb_buffer); - - /* Build vb2_sig_hdr buffer */ - vb2_sig_hdr->sig_offset = sizeof(struct vb21_signature) + - (CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_ITEMS * DIGEST_SIZE); - vb2_sig_hdr->sig_alg = VB2_SIG_RSA2048; - vb2_sig_hdr->sig_size = vb2_rsa_sig_size(VB2_SIG_RSA2048); - vb2_sig_hdr->hash_alg = HASH_ALG; - vb2_sig_hdr->data_size = CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_ITEMS * DIGEST_SIZE; - memcpy(&sig_buffer[sizeof(struct vb21_signature)], + pre->body_signature.data_size = CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_ITEMS * + DIGEST_SIZE; + pre->body_signature.sig_offset = sizeof(struct vb2_signature) + + pre->body_signature.data_size; + pre->body_signature.sig_size = size - pre->body_signature.data_size; + sd->workbuf_used += size; + memcpy((void *)((void *)&pre->body_signature + (long)sizeof(struct vb2_signature)), (uint8_t *)CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_LOC, size);
- if (vb21_verify_data(&sig_buffer[sizeof(struct vb21_signature)], vb2_sig_hdr->data_size, - (struct vb21_signature *)&sig_buffer, &key, &wb)) { - printk(BIOS_ERR, "ERROR: Signature verification failed for hash table\n"); + + if (vb2api_verify_kernel_data(ctx, (void *)CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_LOC, + pre->body_signature.data_size)) goto fail; - }
printk(BIOS_INFO, "%s: Successfully verified hash_table signature.\n", __func__); return 0;
fail: - die("HASH table verification failed!\n"); + die("ERROR: HASH table verification failed!\n"); return -1; }
@@ -131,20 +152,14 @@ uint32_t hash_index, int32_t pcr) { uint8_t digest[DIGEST_SIZE]; - int hash_algorithm; vb2_error_t status;
printk(BIOS_DEBUG, "%s: %s HASH verification buffer %p size %d\n", __func__, name, start, (int)size);
if (start && size) { - if (CONFIG(VENDORCODE_ELTAN_VBOOT_USE_SHA512)) - hash_algorithm = VB2_HASH_SHA512; - else - hash_algorithm = VB2_HASH_SHA256;
- status = cb_sha_little_endian(hash_algorithm, (const uint8_t *)start, size, - digest); + status = vb2_digest_buffer((const uint8_t *)start, size, HASH_ALG, digest, DIGEST_SIZE); if ((CONFIG(VENDORCODE_ELTAN_VBOOT) && memcmp((void *)( (uint8_t *)CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_LOC + sizeof(digest) * hash_index), digest, sizeof(digest))) || status) { diff --git a/src/vendorcode/eltan/security/verified_boot/vboot_check.h b/src/vendorcode/eltan/security/verified_boot/vboot_check.h index bd28492..d4f3b5e 100644 --- a/src/vendorcode/eltan/security/verified_boot/vboot_check.h +++ b/src/vendorcode/eltan/security/verified_boot/vboot_check.h @@ -23,7 +23,7 @@ #include <lib.h> #include CONFIG_VENDORCODE_ELTAN_VBOOT_MANIFEST #include <console/console.h> -#include <cb_sha.h> +#include <vb2_sha.h> #include <string.h> #include <program_loading.h> #include <mboot.h>
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 8:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/437 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/436 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/435
Please note: This test is under development and might not be accurate at all!
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... PS2, Line 17: #define NEED_VB20_INTERNALS /* Peeking into vb2_shared_data */
Yeah, I don't see anything here that you couldn't achieve with standard vboot. […]
I think we look at the vboot library (not the complete solution) in a different way. As far as we are concerned it is a crypto library for use in coreboot. For a large part it indeed serves the purpose. We can create hashes etc.
However there is one are that is not covered and that is signature verification, and the related key import. We have no problems to provide a key with a vboot wrapper but we would like to be able to verify the signature of a random datablock. Basically this has nothing to do with vboot itself. The datablock may not even be included in coreboot image. This can be e.g. an FPGA image that we need to program.
We can't do this as a fixed hash as in this case the image would need to be fixed on coreboot build and that's what we want to avoid.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38590 )
Change subject: vendorcode/eltan/security: Switch to vb2 vboot library ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... File src/vendorcode/eltan/security/verified_boot/vboot_check.c:
https://review.coreboot.org/c/coreboot/+/38590/2/src/vendorcode/eltan/securi... PS2, Line 17: #define NEED_VB20_INTERNALS /* Peeking into vb2_shared_data */
As far as we are concerned it is a crypto library for use in coreboot.
For crypto primitives, that is correct. So I am okay with you using functions like vb2_digest_buffer() and assume those (or equivalents that we could easily patch into your code for you) will always stay available, because the format of a raw SHA256 hash will never change. (Whether RSA primitives like vb2_rsa_verify_digest() also belong to that is an open question... nobody has needed those outside the normal vboot paths yet, but I guess we could allow that if necessary.) But anything above that, our keyblocks, preambles and the workbuffer, are custom to our specific verification solution and therefore subject to change as we develop it forward. We do not want you to call any interfaces involving those (e.g. vb2api_verify_kernel_data()) because that makes it hard for us to change those interfaces later. They were not meant as a toolbox with APIs set in stone.
However there is one are that is not covered and that is signature verification, and the related key import. We have no problems to provide a key with a vboot wrapper but we would like to be able to verify the signature of a random datablock. Basically this has nothing to do with vboot itself. The datablock may not even be included in coreboot image. This can be e.g. an FPGA image that we need to program.
Well... yeah, okay, we don't have a solution we can offer you there. The kernel verification parts of vboot are not really in a good state right now due to a decade of built up cruft and we can't just freeze everything the way it looks right now to support your use case. We have somewhat vague ideas of eventually bringing kernel verification into coreboot but that's still going to be years out.
I don't really see anything you're doing here doing anything like that though? The code you have here just verifies firmware parts (coreboot stages, oproms, etc.), so why do you need an intermediate signature? You can just use the normal vboot verification which automatically covers the CBFS stuff already, and for anything else (e.g. oproms) you want to chain you can just put their hash in the CBFS. You'd do a firmware update when you update those anyway.
If you actually want to do kernel verification you'd do that from a payload (not directly from coreboot) anyway, and there you could link whatever frozen version of vboot you want.
-
9elements QA (Code Review) -
Frans Hendriks (Code Review)
-
Joel Kitching (Code Review)
-
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Wim Vervoorn (Code Review)