Christian Walter has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: src/security/vboot: Add Support for Intel PTT ......................................................................
src/security/vboot: Add Support for Intel PTT
Add support for Intel PTT. For supporting Intel PTT we need to disable read and write access to the TPM NVRAM during the bootblock. TPM NVRAM will only be available once the DRAM is initialized. To circumvent this, we mock secdata if HAVE_INTEL_PTT is set. The underlying problem is, that the iTPM only supports a stripped down instruction set while the Intel ME is not fully booted up. Details can be found in Intel document number 571993 - Paragraph 2.10.
Change-Id: I08c9a839f53f96506be5fb68f7c1ed5bf6692505 Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/security/vboot/Kconfig M src/security/vboot/secdata_tpm.c 2 files changed, 23 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/34510/1
diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index fa98935..ac0c09d 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -26,6 +26,9 @@
if VBOOT
+comment "Anti-Rollback Protection disabled due to Intel PTT" + depends on HAVE_INTEL_PTT + config VBOOT_MEASURED_BOOT bool "Enable Measured Boot" default n diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index 39cd614..6e3cdf7 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -274,11 +274,17 @@
uint32_t antirollback_lock_space_firmware(void) { + if (CONFIG(HAVE_INTEL_PTT)) + return VB2_SUCCESS; + return tlcl_lock_nv_write(FIRMWARE_NV_INDEX); }
uint32_t antirollback_lock_space_rec_hash(void) { + if (CONFIG(HAVE_INTEL_PTT)) + return VB2_SUCCESS; + return tlcl_lock_nv_write(REC_HASH_NV_INDEX); }
@@ -462,6 +468,12 @@ if (rv) return rv;
+ /* If we are using Intel PTT, we do not have antirollback protection. */ + if (CONFIG(HAVE_INTEL_PTT)) { + vb2api_secdata_create(ctx); + return VB2_SUCCESS; + } + /* Read the firmware space. */ rv = read_space_firmware(ctx); if (rv == TPM_E_BADINDEX) { @@ -481,6 +493,8 @@
uint32_t antirollback_write_space_firmware(struct vb2_context *ctx) { + if (CONFIG(HAVE_INTEL_PTT)) + return VB2_SUCCESS; if (CONFIG(CR50_IMMEDIATELY_COMMIT_FW_SECDATA)) tlcl_cr50_enable_nvcommits(); return write_secdata(FIRMWARE_NV_INDEX, ctx->secdata, VB2_SECDATA_SIZE); @@ -488,6 +502,9 @@
uint32_t antirollback_read_space_rec_hash(uint8_t *data, uint32_t size) { + if (CONFIG(HAVE_INTEL_PTT)) + return VB2_SUCCESS; + if (size != REC_HASH_NV_SIZE) { VBDEBUG("TPM: Incorrect buffer size for rec hash. " "(Expected=0x%x Actual=0x%x).\n", REC_HASH_NV_SIZE, @@ -499,6 +516,9 @@
uint32_t antirollback_write_space_rec_hash(const uint8_t *data, uint32_t size) { + if (CONFIG(HAVE_INTEL_PTT)) + return VB2_SUCCESS; + uint8_t spc_data[REC_HASH_NV_SIZE]; uint32_t rv;
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: src/security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 3:
Why not use the existing VBOOT_MOCK_SECDATA for this? You could add a 'default y if INTEL_PTT' there or 'select' it from CONFIG_INTEL_PTT.
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: src/security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 4:
Patch Set 3:
Why not use the existing VBOOT_MOCK_SECDATA for this? You could add a 'default y if INTEL_PTT' there or 'select' it from CONFIG_INTEL_PTT.
VBOOT_MOCK_SECDATA is actually used if you want to do VBOOT without a TPM. We just stub every function (except the NVRAM read) and go with it. When Intel PTT is used, we do not have access to NVRAM, but we can e.g. extend PCRs. So we can do a verified and measured boot with PTT, if we do not use the NVRAM for antirollback. So in my opinion it's a different functionality and should not be merged with VBOOT_MOCK_SECDATA.
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 6: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 6: Code-Review-1
VBOOT_MOCK_SECDATA is actually used if you want to do VBOOT without a TPM. We just stub every function (except the NVRAM read) and go with it. When Intel PTT is used, we do not have access to NVRAM, but we can e.g. extend PCRs. So we can do a verified and measured boot with PTT, if we do not use the NVRAM for antirollback. So in my opinion it's a different functionality and should not be merged with VBOOT_MOCK_SECDATA.
Well, in that case we'll need to add clean new options to model this difference, though. I don't want if (TPM_VENDOR_DETAILS) splattered all over vboot code. "secdata" specifically refers to NVRAM spaces, so keeping that name for things that ignore NVRAM spaces makes sense. We only used it to guard some other things because we never had a case where those things were available but NVRAM is not. Maybe it's as simple as switching those to check (CONFIG(TPM1) || CONFIG(TPM2)) instead? (It would probably be a good idea to split the non-secdata TPM stuff into a separate file, then.)
Philipp Deppenwiese has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Removed Code-Review+2 by Philipp Deppenwiese zaolin.daisuki@gmail.com
Hello Patrick Rudolph, Aaron Durbin, Felix Held, Julius Werner, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34510
to look at the new patch set (#7).
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
security/vboot: Add Support for Intel PTT
Add support for Intel PTT. For supporting Intel PTT we need to disable read and write access to the TPM NVRAM during the bootblock. TPM NVRAM will only be available once the DRAM is initialized. To circumvent this, we mock secdata if HAVE_INTEL_PTT is set. The underlying problem is, that the iTPM only supports a stripped down instruction set while the Intel ME is not fully booted up. Details can be found in Intel document number 571993 - Paragraph 2.10.
Change-Id: I08c9a839f53f96506be5fb68f7c1ed5bf6692505 Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/security/vboot/Kconfig M src/security/vboot/secdata_mock.c 2 files changed, 75 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/34510/7
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34510/12/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/c/coreboot/+/34510/12/src/security/vboot/Kconfig... PS12, Line 22: select VBOOT_MOCK_SECDATA if HAVE_INTEL_PTT Can we move this select (and the comment below) in the INTEL_PTT Kconfig instead? (Or if you want to leave the comment here, just tie it to MOCK_SECDATA instead.)
https://review.coreboot.org/c/coreboot/+/34510/12/src/security/vboot/secdata... File src/security/vboot/secdata_mock.c:
https://review.coreboot.org/c/coreboot/+/34510/12/src/security/vboot/secdata... PS12, Line 72: #if CONFIG(TPM2) Please don't just hack this in here, make it clean. There should be a separate file for the things that don't depend on MOCK_SECDATA -- don't copy&paste whole functions. The vboot_setup_tpm() call should be moved directly into verstage_main() so you don't need to create another fake version antirollback_read_space_firmware().
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 12:
(2 comments)
Patch Set 6: Code-Review-1
VBOOT_MOCK_SECDATA is actually used if you want to do VBOOT without a TPM. We just stub every function (except the NVRAM read) and go with it. When Intel PTT is used, we do not have access to NVRAM, but we can e.g. extend PCRs. So we can do a verified and measured boot with PTT, if we do not use the NVRAM for antirollback. So in my opinion it's a different functionality and should not be merged with VBOOT_MOCK_SECDATA.
Well, in that case we'll need to add clean new options to model this difference, though. I don't want if (TPM_VENDOR_DETAILS) splattered all over vboot code. "secdata" specifically refers to NVRAM spaces, so keeping that name for things that ignore NVRAM spaces makes sense. We only used it to guard some other things because we never had a case where those things were available but NVRAM is not. Maybe it's as simple as switching those to check (CONFIG(TPM1) || CONFIG(TPM2)) instead? (It would probably be a good idea to split the non-secdata TPM stuff into a separate file, then.)
So i merged it into secdata_mock
https://review.coreboot.org/c/coreboot/+/34510/12/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/c/coreboot/+/34510/12/src/security/vboot/Kconfig... PS12, Line 22: select VBOOT_MOCK_SECDATA if HAVE_INTEL_PTT
Can we move this select (and the comment below) in the INTEL_PTT Kconfig instead? (Or if you want to […]
Ack
https://review.coreboot.org/c/coreboot/+/34510/12/src/security/vboot/secdata... File src/security/vboot/secdata_mock.c:
https://review.coreboot.org/c/coreboot/+/34510/12/src/security/vboot/secdata... PS12, Line 72: #if CONFIG(TPM2)
Please don't just hack this in here, make it clean. […]
Ack
Hello Patrick Rudolph, Aaron Durbin, Felix Held, Julius Werner, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34510
to look at the new patch set (#13).
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
security/vboot: Add Support for Intel PTT
Add support for Intel PTT. For supporting Intel PTT we need to disable read and write access to the TPM NVRAM during the bootblock. TPM NVRAM will only be available once the DRAM is initialized. To circumvent this, we mock secdata if HAVE_INTEL_PTT is set. The underlying problem is, that the iTPM only supports a stripped down instruction set while the Intel ME is not fully booted up. Details can be found in Intel document number 571993 - Paragraph 2.10.
Change-Id: I08c9a839f53f96506be5fb68f7c1ed5bf6692505 Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/drivers/intel/ptt/Kconfig M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc A src/security/vboot/secdata_common.c M src/security/vboot/secdata_mock.c M src/security/vboot/secdata_tpm.c M src/security/vboot/vboot_logic.c 7 files changed, 93 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/34510/13
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 17:
(8 comments)
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/Kconfig... PS17, Line 29: comment "Anti-Rollback Protection disabled due to Intel PTT" Can we just turn this into a generic warning about MOCK_SECDATA?
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/Makefil... File src/security/vboot/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/Makefil... PS17, Line 92: romstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += secdata_common.c I don't think you need this for romstage, that's only for the rec_hash stuff.
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... File src/security/vboot/secdata_common.c:
PS17: This file should not be called secdata_common.c, it should be called tpm.c or tpm_common.c or something. It contains all the TPM-related stuff that's specifically not related to secdata.
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... PS17, Line 19: #if CONFIG(TPM1) || CONFIG(TPM2) Let's do this in the header that defines the prototypes for this, that pattern is more common in coreboot (e.g. see how timestamp_get() is handled depending on Kconfig in <timestamp.h>).
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... File src/security/vboot/secdata_mock.c:
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... PS17, Line 38: #include <console/console.h> Doesn't need to be added here?
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... PS17, Line 72: uint32_t antirollback_read_space_firmware(struct vb2_context *ctx) nit: doesn't really need to move?
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... PS17, Line 425: rv = vboot_setup_tpm(ctx); This should no longer be here now.
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/vboot_l... File src/security/vboot/vboot_logic.c:
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/vboot_l... PS17, Line 337: rv = vboot_setup_tpm(&ctx); Let's continue not checking the return value here to fit with the previous behavior and the comment above... e.g. you can write this
if (!vboot_setup_tpm(&ctx)) antirollback_read_space_firmware(&ctx);
If you want an error message, you can print it inside vboot_setup_tpm() (but I think tpm_setup() should already do that anyway?).
Hello Patrick Rudolph, Aaron Durbin, Julius Werner, Philipp Deppenwiese, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34510
to look at the new patch set (#18).
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
security/vboot: Add Support for Intel PTT
Add support for Intel PTT. For supporting Intel PTT we need to disable read and write access to the TPM NVRAM during the bootblock. TPM NVRAM will only be available once the DRAM is initialized. To circumvent this, we mock secdata if HAVE_INTEL_PTT is set. The underlying problem is, that the iTPM only supports a stripped down instruction set while the Intel ME is not fully booted up. Details can be found in Intel document number 571993 - Paragraph 2.10.
Change-Id: I08c9a839f53f96506be5fb68f7c1ed5bf6692505 Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/drivers/intel/ptt/Kconfig M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc M src/security/vboot/antirollback.h M src/security/vboot/secdata_mock.c M src/security/vboot/secdata_tpm.c A src/security/vboot/tpm_common.c A src/security/vboot/tpm_common.h M src/security/vboot/vboot_logic.c 9 files changed, 99 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/34510/18
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 18:
(8 comments)
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/Kconfig... PS17, Line 29: comment "Anti-Rollback Protection disabled due to Intel PTT"
Can we just turn this into a generic warning about MOCK_SECDATA?
Ack
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/Makefil... File src/security/vboot/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/Makefil... PS17, Line 92: romstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += secdata_common.c
I don't think you need this for romstage, that's only for the rec_hash stuff.
Ack
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... File src/security/vboot/secdata_common.c:
PS17:
This file should not be called secdata_common.c, it should be called tpm.c or tpm_common. […]
I mean, these function where in secdata_* before - but i really dont care. I'll rename it tpm_common
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... PS17, Line 19: #if CONFIG(TPM1) || CONFIG(TPM2)
Let's do this in the header that defines the prototypes for this, that pattern is more common in cor […]
Ack
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... File src/security/vboot/secdata_mock.c:
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... PS17, Line 38: #include <console/console.h>
Doesn't need to be added here?
Ack
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... PS17, Line 72: uint32_t antirollback_read_space_firmware(struct vb2_context *ctx)
nit: doesn't really need to move?
Ack
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... PS17, Line 425: rv = vboot_setup_tpm(ctx);
This should no longer be here now.
Ack
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/vboot_l... File src/security/vboot/vboot_logic.c:
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/vboot_l... PS17, Line 337: rv = vboot_setup_tpm(&ctx);
Let's continue not checking the return value here to fit with the previous behavior and the comment […]
Ack
Hello Patrick Rudolph, Aaron Durbin, Julius Werner, Philipp Deppenwiese, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34510
to look at the new patch set (#19).
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
security/vboot: Add Support for Intel PTT
Add support for Intel PTT. For supporting Intel PTT we need to disable read and write access to the TPM NVRAM during the bootblock. TPM NVRAM will only be available once the DRAM is initialized. To circumvent this, we mock secdata if HAVE_INTEL_PTT is set. The underlying problem is, that the iTPM only supports a stripped down instruction set while the Intel ME is not fully booted up. Details can be found in Intel document number 571993 - Paragraph 2.10.
Change-Id: I08c9a839f53f96506be5fb68f7c1ed5bf6692505 Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/drivers/intel/ptt/Kconfig M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc M src/security/vboot/antirollback.h M src/security/vboot/secdata_mock.c M src/security/vboot/secdata_tpm.c A src/security/vboot/tpm_common.c A src/security/vboot/tpm_common.h M src/security/vboot/vboot_logic.c 9 files changed, 99 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/34510/19
Hello Patrick Rudolph, Aaron Durbin, Julius Werner, Philipp Deppenwiese, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34510
to look at the new patch set (#20).
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
security/vboot: Add Support for Intel PTT
Add support for Intel PTT. For supporting Intel PTT we need to disable read and write access to the TPM NVRAM during the bootblock. TPM NVRAM will only be available once the DRAM is initialized. To circumvent this, we mock secdata if HAVE_INTEL_PTT is set. The underlying problem is, that the iTPM only supports a stripped down instruction set while the Intel ME is not fully booted up. Details can be found in Intel document number 571993 - Paragraph 2.10.
Change-Id: I08c9a839f53f96506be5fb68f7c1ed5bf6692505 Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/drivers/intel/ptt/Kconfig M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc M src/security/vboot/antirollback.h M src/security/vboot/secdata_mock.c M src/security/vboot/secdata_tpm.c A src/security/vboot/tpm_common.c A src/security/vboot/tpm_common.h M src/security/vboot/vboot_logic.c 9 files changed, 102 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/34510/20
Hello Patrick Rudolph, Aaron Durbin, Julius Werner, Philipp Deppenwiese, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34510
to look at the new patch set (#21).
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
security/vboot: Add Support for Intel PTT
Add support for Intel PTT. For supporting Intel PTT we need to disable read and write access to the TPM NVRAM during the bootblock. TPM NVRAM will only be available once the DRAM is initialized. To circumvent this, we mock secdata if HAVE_INTEL_PTT is set. The underlying problem is, that the iTPM only supports a stripped down instruction set while the Intel ME is not fully booted up. Details can be found in Intel document number 571993 - Paragraph 2.10.
Change-Id: I08c9a839f53f96506be5fb68f7c1ed5bf6692505 Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/drivers/intel/ptt/Kconfig M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc M src/security/vboot/antirollback.h M src/security/vboot/secdata_mock.c M src/security/vboot/secdata_tpm.c A src/security/vboot/tpm_common.c A src/security/vboot/tpm_common.h M src/security/vboot/vboot_logic.c 9 files changed, 101 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/34510/21
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 22: Code-Review+2
(5 comments)
Thanks, looks good now.
https://review.coreboot.org/c/coreboot/+/34510/17/src/security/vboot/secdata... File src/security/vboot/secdata_common.c:
PS17:
I mean, these function where in secdata_* before - but i really dont care. […]
Yes, again, we didn't used to have a distinction between TPM NVRAM stuff and other TPM stuff before, that's why we threw it all together. But if you want to add that distinction now, you'll have to separate things out correctly.
https://review.coreboot.org/c/coreboot/+/34510/22/src/security/vboot/tpm_com... File src/security/vboot/tpm_common.h:
https://review.coreboot.org/c/coreboot/+/34510/22/src/security/vboot/tpm_com... PS22, Line 23: /* Start of the root of trust */ nit: these comments really belong to the real prototypes above, not the stubs down here
https://review.coreboot.org/c/coreboot/+/34510/22/src/security/vboot/tpm_com... File src/security/vboot/tpm_common.c:
https://review.coreboot.org/c/coreboot/+/34510/22/src/security/vboot/tpm_com... PS22, Line 19: #define TPM_PCR_GBB_FLAGS_NAME "GBB flags" nit: not your patch's fault, but this should've never been called "GBB flags", that's not what it is. It should just be called "boot mode".
https://review.coreboot.org/c/coreboot/+/34510/22/src/security/vboot/tpm_com... PS22, Line 47: case BOOT_MODE_PCR: nit: Because I just had to spend another 10 minutes trying to figure out why this is written as it is... could you add a little comment here explaining
/* SHA1 of (devmode|recmode|keyblock) bits */
https://review.coreboot.org/c/coreboot/+/34510/22/src/security/vboot/tpm_com... PS22, Line 50: case HWID_DIGEST_PCR: ...and here
/* SHA256 of HWID */
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 22:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34510/22/src/security/vboot/tpm_com... File src/security/vboot/tpm_common.h:
https://review.coreboot.org/c/coreboot/+/34510/22/src/security/vboot/tpm_com... PS22, Line 23: /* Start of the root of trust */
nit: these comments really belong to the real prototypes above, not the stubs down here
Ack
https://review.coreboot.org/c/coreboot/+/34510/22/src/security/vboot/tpm_com... File src/security/vboot/tpm_common.c:
https://review.coreboot.org/c/coreboot/+/34510/22/src/security/vboot/tpm_com... PS22, Line 19: #define TPM_PCR_GBB_FLAGS_NAME "GBB flags"
nit: not your patch's fault, but this should've never been called "GBB flags", that's not what it is […]
Ack
https://review.coreboot.org/c/coreboot/+/34510/22/src/security/vboot/tpm_com... PS22, Line 47: case BOOT_MODE_PCR:
nit: Because I just had to spend another 10 minutes trying to figure out why this is written as it i […]
Ack
https://review.coreboot.org/c/coreboot/+/34510/22/src/security/vboot/tpm_com... PS22, Line 50: case HWID_DIGEST_PCR:
...and here […]
Ack
Hello Patrick Rudolph, Aaron Durbin, Julius Werner, Philipp Deppenwiese, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34510
to look at the new patch set (#23).
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
security/vboot: Add Support for Intel PTT
Add support for Intel PTT. For supporting Intel PTT we need to disable read and write access to the TPM NVRAM during the bootblock. TPM NVRAM will only be available once the DRAM is initialized. To circumvent this, we mock secdata if HAVE_INTEL_PTT is set. The underlying problem is, that the iTPM only supports a stripped down instruction set while the Intel ME is not fully booted up. Details can be found in Intel document number 571993 - Paragraph 2.10.
Change-Id: I08c9a839f53f96506be5fb68f7c1ed5bf6692505 Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/drivers/intel/ptt/Kconfig M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc M src/security/vboot/antirollback.h M src/security/vboot/secdata_mock.c M src/security/vboot/secdata_tpm.c A src/security/vboot/tpm_common.c A src/security/vboot/tpm_common.h M src/security/vboot/vboot_logic.c 9 files changed, 103 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/34510/23
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 23: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 23: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/34510/23/src/security/vboot/tpm_com... File src/security/vboot/tpm_common.c:
https://review.coreboot.org/c/coreboot/+/34510/23/src/security/vboot/tpm_com... PS23, Line 19: TPM_PCR_GBB_FLAGS_NAME nit: please change the constant name as well (also, maybe "VBOOT: boot mode" and "VBOOT: GBB HWID" would go better with the way the other TCPA entries are usually named?)
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 23:
Waiting to see if Julius' comment is addressed before merging.
Hello Patrick Rudolph, Aaron Durbin, Julius Werner, Philipp Deppenwiese, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34510
to look at the new patch set (#24).
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
security/vboot: Add Support for Intel PTT
Add support for Intel PTT. For supporting Intel PTT we need to disable read and write access to the TPM NVRAM during the bootblock. TPM NVRAM will only be available once the DRAM is initialized. To circumvent this, we mock secdata if HAVE_INTEL_PTT is set. The underlying problem is, that the iTPM only supports a stripped down instruction set while the Intel ME is not fully booted up. Details can be found in Intel document number 571993 - Paragraph 2.10.
Change-Id: I08c9a839f53f96506be5fb68f7c1ed5bf6692505 Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/drivers/intel/ptt/Kconfig M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc M src/security/vboot/antirollback.h M src/security/vboot/secdata_mock.c M src/security/vboot/secdata_tpm.c A src/security/vboot/tpm_common.c A src/security/vboot/tpm_common.h M src/security/vboot/vboot_logic.c 9 files changed, 103 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/34510/24
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 24:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34510/23/src/security/vboot/tpm_com... File src/security/vboot/tpm_common.c:
https://review.coreboot.org/c/coreboot/+/34510/23/src/security/vboot/tpm_com... PS23, Line 19: TPM_PCR_GBB_FLAGS_NAME
nit: please change the constant name as well (also, maybe "VBOOT: boot mode" and "VBOOT: GBB HWID" w […]
Ack
Hello Patrick Rudolph, Aaron Durbin, Julius Werner, Philipp Deppenwiese, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34510
to look at the new patch set (#25).
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
security/vboot: Add Support for Intel PTT
Add support for Intel PTT. For supporting Intel PTT we need to disable read and write access to the TPM NVRAM during the bootblock. TPM NVRAM will only be available once the DRAM is initialized. To circumvent this, we mock secdata if HAVE_INTEL_PTT is set. The underlying problem is, that the iTPM only supports a stripped down instruction set while the Intel ME is not fully booted up. Details can be found in Intel document number 571993 - Paragraph 2.10.
Change-Id: I08c9a839f53f96506be5fb68f7c1ed5bf6692505 Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/drivers/intel/ptt/Kconfig M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc M src/security/vboot/antirollback.h M src/security/vboot/secdata_mock.c M src/security/vboot/secdata_tpm.c A src/security/vboot/tpm_common.c A src/security/vboot/tpm_common.h M src/security/vboot/vboot_logic.c 9 files changed, 103 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/34510/25
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 26: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 26: Code-Review+2
Philipp Deppenwiese has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
security/vboot: Add Support for Intel PTT
Add support for Intel PTT. For supporting Intel PTT we need to disable read and write access to the TPM NVRAM during the bootblock. TPM NVRAM will only be available once the DRAM is initialized. To circumvent this, we mock secdata if HAVE_INTEL_PTT is set. The underlying problem is, that the iTPM only supports a stripped down instruction set while the Intel ME is not fully booted up. Details can be found in Intel document number 571993 - Paragraph 2.10.
Change-Id: I08c9a839f53f96506be5fb68f7c1ed5bf6692505 Signed-off-by: Christian Walter christian.walter@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34510 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Philipp Deppenwiese zaolin.daisuki@gmail.com Reviewed-by: Julius Werner jwerner@chromium.org --- M src/drivers/intel/ptt/Kconfig M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc M src/security/vboot/antirollback.h M src/security/vboot/secdata_mock.c M src/security/vboot/secdata_tpm.c A src/security/vboot/tpm_common.c A src/security/vboot/tpm_common.h M src/security/vboot/vboot_logic.c 9 files changed, 103 insertions(+), 56 deletions(-)
Approvals: build bot (Jenkins): Verified Philipp Deppenwiese: Looks good to me, approved Julius Werner: Looks good to me, approved
diff --git a/src/drivers/intel/ptt/Kconfig b/src/drivers/intel/ptt/Kconfig index c013f42..fb70f9a 100644 --- a/src/drivers/intel/ptt/Kconfig +++ b/src/drivers/intel/ptt/Kconfig @@ -1,5 +1,6 @@ config HAVE_INTEL_PTT bool default n + select VBOOT_MOCK_SECDATA if VBOOT help Activate if your platform has Intel Platform Trust Technology like Intel iTPM and you want to use it. diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index ea1f738..c5146c6 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -26,10 +26,13 @@
if VBOOT
+comment "Anti-Rollback Protection disabled because mocking secdata is enabled." + depends on VBOOT_MOCK_SECDATA + config VBOOT_MEASURED_BOOT bool "Enable Measured Boot" default n - depends on !VBOOT_MOCK_SECDATA + depends on TPM1 || TPM2 depends on !VBOOT_RETURN_FROM_VERSTAGE help Enables measured boot mode in vboot (experimental) diff --git a/src/security/vboot/Makefile.inc b/src/security/vboot/Makefile.inc index 6d19529..d554f10 100644 --- a/src/security/vboot/Makefile.inc +++ b/src/security/vboot/Makefile.inc @@ -88,6 +88,11 @@ verstage-y += secdata_tpm.c romstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += secdata_tpm.c endif + +ifneq ($(CONFIG_TPM1)$(CONFIG_TPM2),) +verstage-y += tpm_common.c +endif + romstage-y += vboot_logic.c romstage-y += common.c
diff --git a/src/security/vboot/antirollback.h b/src/security/vboot/antirollback.h index 62d2e20..5af9236 100644 --- a/src/security/vboot/antirollback.h +++ b/src/security/vboot/antirollback.h @@ -83,11 +83,4 @@ /* Lock down recovery hash space in TPM. */ uint32_t antirollback_lock_space_rec_hash(void);
-/* Start of the root of trust */ -uint32_t vboot_setup_tpm(struct vb2_context *ctx); - -/* vboot_extend_pcr function for vb2 context */ -uint32_t vboot_extend_pcr(struct vb2_context *ctx, int pcr, - enum vb2_pcr_digest which_digest); - #endif /* ANTIROLLBACK_H_ */ diff --git a/src/security/vboot/secdata_mock.c b/src/security/vboot/secdata_mock.c index 3075d33..43206df 100644 --- a/src/security/vboot/secdata_mock.c +++ b/src/security/vboot/secdata_mock.c @@ -43,12 +43,6 @@ return VB2_SUCCESS; }
-uint32_t vboot_extend_pcr(struct vb2_context *ctx, int pcr, - enum vb2_pcr_digest which_digest) -{ - return VB2_SUCCESS; -} - uint32_t antirollback_read_space_firmware(struct vb2_context *ctx) { vb2api_secdata_create(ctx); @@ -60,7 +54,7 @@ return VB2_SUCCESS; }
-uint32_t antirollback_lock_space_firmware() +uint32_t antirollback_lock_space_firmware(void) { return VB2_SUCCESS; } diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index 39cd614..09c7e72 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -33,6 +33,7 @@ */
#include <security/vboot/antirollback.h> +#include <security/vboot/tpm_common.h> #include <stdlib.h> #include <string.h> #include <security/tpm/tspi.h> @@ -65,31 +66,6 @@
static uint32_t safe_write(uint32_t index, const void *data, uint32_t length);
-uint32_t vboot_extend_pcr(struct vb2_context *ctx, int pcr, - enum vb2_pcr_digest which_digest) -{ - uint8_t buffer[VB2_PCR_DIGEST_RECOMMENDED_SIZE]; - uint32_t size = sizeof(buffer); - int rv; - - rv = vb2api_get_pcr_digest(ctx, which_digest, buffer, &size); - if (rv != VB2_SUCCESS) - return rv; - if (size < TPM_PCR_MINIMUM_DIGEST_SIZE) - return VB2_ERROR_UNKNOWN; - - switch (which_digest) { - case BOOT_MODE_PCR: - return tpm_extend_pcr(pcr, VB2_HASH_SHA1, buffer, size, - TPM_PCR_GBB_FLAGS_NAME); - case HWID_DIGEST_PCR: - return tpm_extend_pcr(pcr, VB2_HASH_SHA256, buffer, - size, TPM_PCR_GBB_HWID_NAME); - default: - return VB2_ERROR_UNKNOWN; - } -} - static uint32_t read_space_firmware(struct vb2_context *ctx) { int attempts = 3; @@ -443,25 +419,10 @@ return TPM_SUCCESS; }
-uint32_t vboot_setup_tpm(struct vb2_context *ctx) -{ - uint32_t result; - - result = tpm_setup(ctx->flags & VB2_CONTEXT_S3_RESUME); - if (result == TPM_E_MUST_REBOOT) - ctx->flags |= VB2_CONTEXT_SECDATA_WANTS_REBOOT; - - return result; -} - uint32_t antirollback_read_space_firmware(struct vb2_context *ctx) { uint32_t rv;
- rv = vboot_setup_tpm(ctx); - if (rv) - return rv; - /* Read the firmware space. */ rv = read_space_firmware(ctx); if (rv == TPM_E_BADINDEX) { diff --git a/src/security/vboot/tpm_common.c b/src/security/vboot/tpm_common.c new file mode 100644 index 0000000..1a07ef6 --- /dev/null +++ b/src/security/vboot/tpm_common.c @@ -0,0 +1,58 @@ +/* + * This file is part of the coreboot project. + * + * 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 <security/tpm/tspi.h> +#include <vb2_api.h> +#include <security/vboot/tpm_common.h> + +#define TPM_PCR_BOOT_MODE "VBOOT: boot mode" +#define TPM_PCR_GBB_HWID_NAME "VBOOT: GBB HWID" + +uint32_t vboot_setup_tpm(struct vb2_context *ctx) +{ + uint32_t result; + + result = tpm_setup(ctx->flags & VB2_CONTEXT_S3_RESUME); + if (result == TPM_E_MUST_REBOOT) + ctx->flags |= VB2_CONTEXT_SECDATA_WANTS_REBOOT; + + return result; +} + +uint32_t vboot_extend_pcr(struct vb2_context *ctx, int pcr, + enum vb2_pcr_digest which_digest) +{ + uint8_t buffer[VB2_PCR_DIGEST_RECOMMENDED_SIZE]; + uint32_t size = sizeof(buffer); + int rv; + + rv = vb2api_get_pcr_digest(ctx, which_digest, buffer, &size); + if (rv != VB2_SUCCESS) + return rv; + if (size < TPM_PCR_MINIMUM_DIGEST_SIZE) + return VB2_ERROR_UNKNOWN; + + switch (which_digest) { + /* SHA1 of (devmode|recmode|keyblock) bits */ + case BOOT_MODE_PCR: + return tpm_extend_pcr(pcr, VB2_HASH_SHA1, buffer, size, + TPM_PCR_BOOT_MODE); + /* SHA256 of HWID */ + case HWID_DIGEST_PCR: + return tpm_extend_pcr(pcr, VB2_HASH_SHA256, buffer, + size, TPM_PCR_GBB_HWID_NAME); + default: + return VB2_ERROR_UNKNOWN; + } +} diff --git a/src/security/vboot/tpm_common.h b/src/security/vboot/tpm_common.h new file mode 100644 index 0000000..6bb32bb --- /dev/null +++ b/src/security/vboot/tpm_common.h @@ -0,0 +1,29 @@ +/* + * This file is part of the coreboot project. + * + * 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. + */ + +#if CONFIG(TPM1) || CONFIG(TPM2) + +/* Start of the root of trust */ +uint32_t vboot_setup_tpm(struct vb2_context *ctx); + +/* vboot_extend_pcr function for vb2 context */ +uint32_t vboot_extend_pcr(struct vb2_context *ctx, int pcr, + enum vb2_pcr_digest which_digest); + +#else + +#define vboot_setup_tpm(ctx) 0 + +#define vboot_extend_pcr(ctx, pcr, which_digest) 0 + +#endif diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index 2468f5f..c61d6be 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -25,6 +25,7 @@ #include <security/vboot/misc.h> #include <security/vboot/vbnv.h> #include <security/vboot/vboot_crtm.h> +#include <security/vboot/tpm_common.h>
#include "antirollback.h"
@@ -334,7 +335,9 @@ * check the return value here because vb2api_fw_phase1 will catch * invalid secdata and tell us what to do (=reboot). */ timestamp_add_now(TS_START_TPMINIT); - antirollback_read_space_firmware(&ctx); + rv = vboot_setup_tpm(&ctx); + if (rv) + antirollback_read_space_firmware(&ctx); timestamp_add_now(TS_END_TPMINIT);
/* Enable measured boot mode */
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 27:
+Furquan
hatch is default going into recovery always due to this CL
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34510 )
Change subject: security/vboot: Add Support for Intel PTT ......................................................................
Patch Set 27:
Patch Set 27:
+Furquan
hatch is default going into recovery always due to this CL
Fix uploaded here: https://review.coreboot.org/c/coreboot/+/34790
-
Christian Walter (Code Review) -
Furquan Shaikh (Code Review)
-
Julius Werner (Code Review)
-
Martin Roth (Code Review)
-
Philipp Deppenwiese (Code Review)
-
Subrata Banik (Code Review)