Change in ...coreboot[master]: vendorcode/eltan Add hashing library used for measured and verified b...
Frans Hendriks has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30831
Change subject: vendorcode/eltan Add hashing library used for measured and verified boot. ......................................................................
vendorcode/eltan Add hashing library used for measured and verified boot.
To avoid having the whole 3rdparty/vboot/firmware included a small hashing library has been created. Create library which is a 'wrapper' using only sha1, sha256 and sha512 of 3rdparty/vboot/firmware.
Fucntions cb_sha1(), cb_sha256() and cb_sha512 can be used for hashing.
BUG=N/A TEST=Created binary and verify logging on Facebok FBG-1701
Change-Id: If828bde54c79e836a5b05ff0447645d7e06e819a Signed-off-by: Frans Hendriks fhendriks@eltan.com --- A src/vendorcode/eltan/security/include/cb_sha1.h A src/vendorcode/eltan/security/include/cb_sha256.h A src/vendorcode/eltan/security/include/cb_sha512.h A src/vendorcode/eltan/security/include/cryptolib.h A src/vendorcode/eltan/security/lib/Makefile.inc A src/vendorcode/eltan/security/lib/cb_sha1.c A src/vendorcode/eltan/security/lib/cb_sha256.c A src/vendorcode/eltan/security/lib/cb_sha512.c 8 files changed, 297 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/30831/1
diff --git a/src/vendorcode/eltan/security/include/cb_sha1.h b/src/vendorcode/eltan/security/include/cb_sha1.h new file mode 100644 index 0000000..3b72355 --- /dev/null +++ b/src/vendorcode/eltan/security/include/cb_sha1.h @@ -0,0 +1,21 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018. 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_SHA1_H__ +#define __SECURITY_SHA1_H__ + +uint8_t *cb_sha1(const uint8_t *data, uint64_t len, uint8_t *digest); + +#endif diff --git a/src/vendorcode/eltan/security/include/cb_sha256.h b/src/vendorcode/eltan/security/include/cb_sha256.h new file mode 100644 index 0000000..3b45f73 --- /dev/null +++ b/src/vendorcode/eltan/security/include/cb_sha256.h @@ -0,0 +1,23 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018. 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_SHA256_H__ +#define __SECURITY_SHA256_H__ + +uint8_t *cb_sha256(const uint8_t *data, uint64_t len, uint8_t *digest); +uint8_t *cb_sha256_ex(const uint8_t *data, uint64_t len, uint8_t *digest, + bool endian); + +#endif diff --git a/src/vendorcode/eltan/security/include/cb_sha512.h b/src/vendorcode/eltan/security/include/cb_sha512.h new file mode 100644 index 0000000..a383cf0 --- /dev/null +++ b/src/vendorcode/eltan/security/include/cb_sha512.h @@ -0,0 +1,23 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018. 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_SHA512_H__ +#define __SECURITY_SHA512_H__ + +uint8_t *cb_sha512(const uint8_t *data, uint64_t len, uint8_t *digest); +uint8_t *cb_sha512_ex(const uint8_t *data, uint64_t len, uint8_t *digest, + bool endian); + +#endif diff --git a/src/vendorcode/eltan/security/include/cryptolib.h b/src/vendorcode/eltan/security/include/cryptolib.h new file mode 100644 index 0000000..ac1668a --- /dev/null +++ b/src/vendorcode/eltan/security/include/cryptolib.h @@ -0,0 +1,29 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018. 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_CRYPTOLIB_H__ +#define __SECURITY_CRYPTOLIB_H__ + +#define NEED_VB2_SHA_LIBRARY + +#include <2rsa.h> +#include <vb21_common.h> +#include <vb2_api.h> + +#include "cb_sha1.h" +#include "cb_sha512.h" +#include "cb_sha256.h" + +#endif \ No newline at end of file diff --git a/src/vendorcode/eltan/security/lib/Makefile.inc b/src/vendorcode/eltan/security/lib/Makefile.inc new file mode 100644 index 0000000..9e2fc39 --- /dev/null +++ b/src/vendorcode/eltan/security/lib/Makefile.inc @@ -0,0 +1,52 @@ +# +# This file is part of the coreboot project. +# +# Copyright (C) 2018 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. +# + +SECURITYLIB_INCLUDES = -I3rdparty/vboot/firmware/2lib/include -I3rdparty/vboot/firmware/lib21/include + +CPPFLAGS_common+=$(SECURITYLIB_INCLUDES) + +ifeq ($(CONFIG_VERIFIED_BOOT),y) +bootblock-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += $(top)/3rdparty/vboot/firmware/2lib/2common.c +bootblock-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += $(top)/3rdparty/vboot/firmware/2lib/2rsa.c +bootblock-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += $(top)/3rdparty/vboot/firmware/2lib/2sha_utility.c +bootblock-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += $(top)/3rdparty/vboot/firmware/lib21/packed_key.c +ifeq ($(CONFIG_VERIFIED_BOOT_USE_SHA512),y) +bootblock-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += cb_sha512.c +bootblock-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += $(top)/3rdparty/vboot/firmware/2lib/2sha512.c +else +bootblock-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += cb_sha256.c +bootblock-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += $(top)/3rdparty/vboot/firmware/2lib/2sha256.c +endif +endif + +ifeq ($(CONFIG_MBOOT),y) +ramstage-y += $(top)/3rdparty/vboot/firmware/2lib/2sha1.c +ramstage-y += $(top)/3rdparty/vboot/firmware/2lib/2sha512.c +ramstage-y += $(top)/3rdparty/vboot/firmware/2lib/2sha256.c +ramstage-y += cb_sha1.c +ramstage-y += cb_sha512.c +ramstage-y += cb_sha256.c + +romstage-y += $(top)/3rdparty/vboot/firmware/2lib/2common.c +romstage-y += $(top)/3rdparty/vboot/firmware/2lib/2rsa.c +romstage-y += $(top)/3rdparty/vboot/firmware/2lib/2sha1.c +romstage-y += $(top)/3rdparty/vboot/firmware/2lib/2sha256.c +romstage-y += $(top)/3rdparty/vboot/firmware/2lib/2sha512.c +romstage-y += $(top)/3rdparty/vboot/firmware/2lib/2sha_utility.c +romstage-y += $(top)/3rdparty/vboot/firmware/lib21/packed_key.c +romstage-y += cb_sha1.c +romstage-y += cb_sha512.c +romstage-y += cb_sha256.c +endif \ No newline at end of file diff --git a/src/vendorcode/eltan/security/lib/cb_sha1.c b/src/vendorcode/eltan/security/lib/cb_sha1.c new file mode 100644 index 0000000..fd96943 --- /dev/null +++ b/src/vendorcode/eltan/security/lib/cb_sha1.c @@ -0,0 +1,27 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018 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 <cryptolib.h> + +uint8_t *cb_sha1(const uint8_t *data, uint64_t len, uint8_t *digest) +{ + struct vb2_sha1_context ctx; + + vb2_sha1_init(&ctx); + vb2_sha1_update(&ctx, data, len); + vb2_sha1_finalize(&ctx, digest); + + return digest; +} diff --git a/src/vendorcode/eltan/security/lib/cb_sha256.c b/src/vendorcode/eltan/security/lib/cb_sha256.c new file mode 100644 index 0000000..b02ebb2 --- /dev/null +++ b/src/vendorcode/eltan/security/lib/cb_sha256.c @@ -0,0 +1,61 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018 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 <cryptolib.h> + +uint8_t *cb_sha256_ex(const uint8_t *data, uint64_t len, uint8_t *digest, + bool endian) +{ + int i; + const uint8_t *input_ptr; + uint8_t result[VB2_SHA256_DIGEST_SIZE]; + uint8_t *result_ptr; + uint64_t remaining_len; + struct vb2_sha256_context ctx; + + vb2_sha256_init(&ctx); + + input_ptr = data; + remaining_len = len; + + /* Process data in at most UINT32_MAX byte chunks at a time. */ + while (remaining_len) { + uint32_t block_size; + block_size = (uint32_t) ((remaining_len >= UINT32_MAX) ? + UINT32_MAX : remaining_len); + vb2_sha256_update(&ctx, input_ptr, block_size); + remaining_len -= block_size; + input_ptr += block_size; + } + + result_ptr = result; + vb2_sha256_finalize(&ctx, result_ptr); + for (i = 0; i < VB2_SHA256_DIGEST_SIZE; ++i) { + if (endian) { + /* use big endian here */ + digest[i] = *result_ptr++; + } else { + /* use little endian here */ + digest[VB2_SHA256_DIGEST_SIZE - i - 1] = *result_ptr++; + } + } + return digest; +} + +uint8_t *cb_sha256(const uint8_t *data, uint64_t len, uint8_t *digest) +{ + /* Returned the little endian SHA256 digest */ + return cb_sha256_ex(data, len, digest, 0); +} \ No newline at end of file diff --git a/src/vendorcode/eltan/security/lib/cb_sha512.c b/src/vendorcode/eltan/security/lib/cb_sha512.c new file mode 100644 index 0000000..9d713e7 --- /dev/null +++ b/src/vendorcode/eltan/security/lib/cb_sha512.c @@ -0,0 +1,61 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018 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 <cryptolib.h> + +uint8_t *cb_sha512_ex(const uint8_t *data, uint64_t len, uint8_t *digest, + bool endian) +{ + int i; + const uint8_t *input_ptr; + uint8_t result[VB2_SHA512_DIGEST_SIZE]; + uint8_t *result_ptr; + uint64_t remaining_len; + struct vb2_sha512_context ctx; + + vb2_sha512_init(&ctx); + + input_ptr = data; + remaining_len = len; + + /* Process data in at most UINT32_MAX byte chunks at a time. */ + while (remaining_len) { + uint32_t block_size; + block_size = (uint32_t) ((remaining_len >= UINT32_MAX) ? + UINT32_MAX : remaining_len); + vb2_sha512_update(&ctx, input_ptr, block_size); + remaining_len -= block_size; + input_ptr += block_size; + } + + result_ptr = result; + vb2_sha512_finalize(&ctx, result_ptr); + for (i = 0; i < VB2_SHA512_DIGEST_SIZE; ++i) { + if (endian) { + /* use big endian here */ + digest[i] = *result_ptr++; + } else { + /* use little endian here */ + digest[VB2_SHA512_DIGEST_SIZE - i - 1] = *result_ptr++; + } + } + return digest; +} + +uint8_t *cb_sha512(const uint8_t *data, uint64_t len, uint8_t *digest) +{ + /* Returned the little endian SHA512 digest */ + return cb_sha512_ex(data, len, digest, 0); +} \ No newline at end of file
Hello Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30831
to look at the new patch set (#2).
Change subject: vendorcode/eltan Add hashing library used for measured and verified boot. ......................................................................
vendorcode/eltan Add hashing library used for measured and verified boot.
To avoid having the whole 3rdparty/vboot/firmware included a small hashing library has been created. Create library which is a 'wrapper' using only sha1, sha256 and sha512 of 3rdparty/vboot/firmware.
Functions cb_sha1(), cb_sha256() and cb_sha512 can be used for hashing.
BUG=N/A TEST=Created binary and verify logging on Facebok FBG-1701
Change-Id: If828bde54c79e836a5b05ff0447645d7e06e819a Signed-off-by: Frans Hendriks fhendriks@eltan.com --- A src/vendorcode/eltan/security/include/cb_sha1.h A src/vendorcode/eltan/security/include/cb_sha256.h A src/vendorcode/eltan/security/include/cb_sha512.h A src/vendorcode/eltan/security/include/cryptolib.h A src/vendorcode/eltan/security/lib/Makefile.inc A src/vendorcode/eltan/security/lib/cb_sha1.c A src/vendorcode/eltan/security/lib/cb_sha256.c A src/vendorcode/eltan/security/lib/cb_sha512.c 8 files changed, 297 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/30831/2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan Add hashing library used for measured and verified boot. ......................................................................
Patch Set 2:
As I mentioned on CL:1386444, you shouldn't be doing this. vboot is meant to be a library, not a collection of files to be pulled in individually. You can't just pull in files from a different code base and expect your CFLAGS and other environment factors to "just work" for them.
The correct solution for this is to build vboot_fw20.a separately and link to it like we're already doing in src/security/vboot/Makefile.inc. If you don't want to use CONFIG_VBOOT (i.e. don't actually want to use vboot for verification), you should refactor that Makefile so that the vboot library is always built and linked unconditionally, and CONFIG_VBOOT just determines whether coreboot's verification support code and code paths get built. This way code that doesn't want to use vboot for verification can still pull in crypto code from it.
I already discussed and agreed on this approach with Philipp once in CB:22872. I don't know if he had time to work on this since, but that's the general plan of action for how to get general crypto code into coreboot.
Hello Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30831
to look at the new patch set (#3).
Change subject: vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot. ......................................................................
vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot.
coreboot does not contains separate hashing support, beside in 3rdparty/vboot. Create library which is a 'wrapper' using only SHA-1, SHA-256 and SHA-512 support from 3rdparty/vboot/firmware.
Functions cb_sha1(), cb_sha256() and cb_sha512 can be used for hashing.
BUG=N/A TEST=Created binary and verify logging on Facebok FBG-1701
Change-Id: If828bde54c79e836a5b05ff0447645d7e06e819a Signed-off-by: Frans Hendriks fhendriks@eltan.com --- A src/vendorcode/eltan/security/include/cb_sha1.h A src/vendorcode/eltan/security/include/cb_sha256.h A src/vendorcode/eltan/security/include/cb_sha512.h A src/vendorcode/eltan/security/include/cryptolib.h A src/vendorcode/eltan/security/lib/Makefile.inc A src/vendorcode/eltan/security/lib/cb_sha1.c A src/vendorcode/eltan/security/lib/cb_sha256.c A src/vendorcode/eltan/security/lib/cb_sha512.c 8 files changed, 308 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/30831/3
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot. ......................................................................
Patch Set 4:
generally speaking this is generic code. Any particular reason to namespace it away into vendorcode/eltan?
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot. ......................................................................
Patch Set 4:
Patch Set 4:
generally speaking this is generic code. Any particular reason to namespace it away into vendorcode/eltan?
This library is part of the measure_vboot_vendorcode topic.
I started with uploading a generic patch containing measured boot and verified boot. This verified boot includes checking bootblock and not using ChromeOS. I got comment to split into parts (as I did) and also this solution should be placed into a vendor specific directory. For this reason I placed all code into vendorcode/eltan
For us it's no problem to place the code in vendorcode, but I'm not sure about the main reason for this request.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot. ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
generally speaking this is generic code. Any particular reason to namespace it away into vendorcode/eltan?
This library is part of the measure_vboot_vendorcode topic.
I started with uploading a generic patch containing measured boot and verified boot. This verified boot includes checking bootblock and not using ChromeOS. I got comment to split into parts (as I did) and also this solution should be placed into a vendor specific directory. For this reason I placed all code into vendorcode/eltan
For us it's no problem to place the code in vendorcode, but I'm not sure about the main reason for this request.
I took a look at the other mboot commits now, and those are fine in vendorcode since it's neatly isolated.
I was referring to the vboot convenience wrappers in this commit: they're more generally useful than just for mboot and hiding them here means that it's rather likely we'll see similar functions being reimplemented in short order.
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot. ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
generally speaking this is generic code. Any particular reason to namespace it away into vendorcode/eltan?
This library is part of the measure_vboot_vendorcode topic.
I started with uploading a generic patch containing measured boot and verified boot. This verified boot includes checking bootblock and not using ChromeOS. I got comment to split into parts (as I did) and also this solution should be placed into a vendor specific directory. For this reason I placed all code into vendorcode/eltan
For us it's no problem to place the code in vendorcode, but I'm not sure about the main reason for this request.
I took a look at the other mboot commits now, and those are fine in vendorcode since it's neatly isolated.
I was referring to the vboot convenience wrappers in this commit: they're more generally useful than just for mboot and hiding them here means that it's rather likely we'll see similar functions being reimplemented in short order.
Would it make sence to adapt this patch for now? In later stage this lib can be merged/move from vendorcode?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot. ......................................................................
Patch Set 4:
Patch Set 4: In later stage this lib can be merged/move from vendorcode?
can you work on that?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot. ......................................................................
Patch Set 4: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot. ......................................................................
Patch Set 4: Code-Review-1
Would it make sence to adapt this patch for now? In later stage this lib can be merged/move from vendorcode?
This should not be moved out of vendorcode the way it is done here. We should not be duplicating this code. Our goal with vboot is to integrate it as a generic security library that can offer both high-level verification services as well as individual low-level crypto primitives. We should *not* be duplicating the same functionality between vboot and coreboot.
FWIW I don't think this should be committed even here, because it's going down the wrong path and the alternative is honestly not that hard to achieve (you just need a handful of changes to src/security/vboot/Makefile.inc to make it build and link the library even when CONFIG_VBOOT is false, and then you can literally just call vb2api_digest_buffer() wherever you want). I don't care that much about what you're doing in vendorcode, but if you ever plan to move this out it would save you effort later to just do it correctly right away.
I also don't understand any of the mboot stuff. We have measured boot support in the tree now, you should be using that or suggesting ways to adapt it to what you need rather than rolling your own.
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot. ......................................................................
Patch Set 4:
Patch Set 4: Code-Review-1
Would it make sence to adapt this patch for now? In later stage this lib can be merged/move from vendorcode?
This should not be moved out of vendorcode the way it is done here. We should not be duplicating this code. Our goal with vboot is to integrate it as a generic security library that can offer both high-level verification services as well as individual low-level crypto primitives. We should *not* be duplicating the same functionality between vboot and coreboot.
FWIW I don't think this should be committed even here, because it's going down the wrong path and the alternative is honestly not that hard to achieve (you just need a handful of changes to src/security/vboot/Makefile.inc to make it build and link the library even when CONFIG_VBOOT is false, and then you can literally just call vb2api_digest_buffer() wherever you want). I don't care that much about what you're doing in vendorcode, but if you ever plan to move this out it would save you effort later to just do it correctly right away.
I also don't understand any of the mboot stuff. We have measured boot support in the tree now, you should be using that or suggesting ways to adapt it to what you need rather than rolling your own.
Some background about this vendorcode: This 'vendorcode' measured (and) library were created before this all of this was available in the coreboot. From reviewers was the request to place this code in 'vendorcode'. At that stage someone pointed to gerrit patches with mboot support. This patches results into gerrit build issues at that moment.
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot. ......................................................................
Patch Set 4:
Patch Set 4: Code-Review-1
Would it make sence to adapt this patch for now? In later stage this lib can be merged/move from vendorcode?
This should not be moved out of vendorcode the way it is done here. We should not be duplicating this code. Our goal with vboot is to integrate it as a generic security library that can offer both high-level verification services as well as individual low-level crypto primitives. We should *not* be duplicating the same functionality between vboot and coreboot.
FWIW I don't think this should be committed even here, because it's going down the wrong path and the alternative is honestly not that hard to achieve (you just need a handful of changes to src/security/vboot/Makefile.inc to make it build and link the library even when CONFIG_VBOOT is false, and then you can literally just call vb2api_digest_buffer() wherever you want). I don't care that much about what you're doing in vendorcode, but if you ever plan to move this out it would save you effort later to just do it correctly right away.
I also don't understand any of the mboot stuff. We have measured boot support in the tree now, you should be using that or suggesting ways to adapt it to what you need rather than rolling your own.
Where can I find vb2ap_digest_buffer()?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot. ......................................................................
Patch Set 4:
Where can I find vb2ap_digest_buffer()?
It was added recently in https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_referen..., looks like nobody has uprevved the coreboot submodule after that commit yet. Feel free to do that. It's just a wrapper around vb2_digest_buffer() so you could also just call that for now.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot. ......................................................................
Patch Set 4:
In fact, it looks like we're removing the wrapper again in CL:1583822, so just use vb2_digest_buffer().
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Add hashing library used for measured and verified boot. ......................................................................
Patch Set 4:
Patch Set 4:
In fact, it looks like we're removing the wrapper again in CL:1583822, so just use vb2_digest_buffer().
Thanks for the update.
Hello Julius Werner, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30831
to look at the new patch set (#5).
Change subject: vendorcode/eltan/security/lib: Implement SHA endian function ......................................................................
vendorcode/eltan/security/lib: Implement SHA endian function
digest from vb2_digest_bufer() does not contains the correct endian. Create cb_sha_endian() which can convert the calculated digest into big endian or little endian when required.
BUG=N/A TEST=Created binary and verify logging on Facebok FBG-1701
Change-Id: If828bde54c79e836a5b05ff0447645d7e06e819a Signed-off-by: Frans Hendriks fhendriks@eltan.com --- A src/vendorcode/eltan/security/include/cb_sha.h A src/vendorcode/eltan/security/lib/Makefile.inc A src/vendorcode/eltan/security/lib/cb_sha.c 3 files changed, 148 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/30831/5
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Implement SHA endian function ......................................................................
Patch Set 5: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30831 )
Change subject: vendorcode/eltan/security/lib: Implement SHA endian function ......................................................................
vendorcode/eltan/security/lib: Implement SHA endian function
digest from vb2_digest_bufer() does not contains the correct endian. Create cb_sha_endian() which can convert the calculated digest into big endian or little endian when required.
BUG=N/A TEST=Created binary and verify logging on Facebok FBG-1701
Change-Id: If828bde54c79e836a5b05ff0447645d7e06e819a Signed-off-by: Frans Hendriks fhendriks@eltan.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/30831 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Philipp Deppenwiese zaolin.daisuki@gmail.com --- A src/vendorcode/eltan/security/include/cb_sha.h A src/vendorcode/eltan/security/lib/Makefile.inc A src/vendorcode/eltan/security/lib/cb_sha.c 3 files changed, 148 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Philipp Deppenwiese: Looks good to me, approved
diff --git a/src/vendorcode/eltan/security/include/cb_sha.h b/src/vendorcode/eltan/security/include/cb_sha.h new file mode 100644 index 0000000..4d087f4 --- /dev/null +++ b/src/vendorcode/eltan/security/include/cb_sha.h @@ -0,0 +1,33 @@ +/* + * 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> + +/* Supported Algorithm types for hash */ +enum endian_algorithm { + NO_ENDIAN_ALGORITHM = 0, + BIG_ENDIAN_ALGORITHM = 1, + LITTLE_ENDIAN_ALGORITHM = 2, +}; + +int cb_sha_endian(enum vb2_hash_algorithm hash_alg, const uint8_t *data, uint32_t len, + uint8_t *digest, enum endian_algorithm endian); + +#endif \ No newline at end of file diff --git a/src/vendorcode/eltan/security/lib/Makefile.inc b/src/vendorcode/eltan/security/lib/Makefile.inc new file mode 100644 index 0000000..5ef1bca --- /dev/null +++ b/src/vendorcode/eltan/security/lib/Makefile.inc @@ -0,0 +1,59 @@ +# +# 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-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += cb_sha.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 new file mode 100644 index 0000000..47cd10a --- /dev/null +++ b/src/vendorcode/eltan/security/lib/cb_sha.c @@ -0,0 +1,56 @@ +/* + * 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> + +int cb_sha_endian(enum vb2_hash_algorithm hash_alg, const uint8_t *data, uint32_t len, + uint8_t *digest, enum endian_algorithm endian) +{ + int i; + int rv; + uint32_t digest_size; + uint8_t *result_ptr; + uint8_t result[VB2_MAX_DIGEST_SIZE]; + + switch (hash_alg) { + case VB2_HASH_SHA1: + digest_size = VB2_SHA1_DIGEST_SIZE; + break; + 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; + } + + result_ptr = result; + rv = vb2_digest_buffer(data, len, hash_alg, result_ptr, digest_size); + if (rv || (endian == NO_ENDIAN_ALGORITHM)) + return rv; + + for (i = 0; i < digest_size; ++i) { + if (endian == BIG_ENDIAN_ALGORITHM) { + /* use big endian */ + digest[i] = *result_ptr++; + } else { + /* use little endian */ + digest[digest_size - i - 1] = *result_ptr++; + } + } + return rv; +}
-
Frans Hendriks (Code Review) -
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)
-
Philipp Deppenwiese (Code Review)