[coreboot-gerrit] [M] Change in coreboot[master]: [WIP] security/tpm: make use of PCRs configurable via Kconfig

[Sergii Dmytruk (Code Review)]

Attention is currently required from: Michał Żygowski, Maciej Pijanowski, Christian Walter, Julius Werner, Krystian Hebel, Yu-Ping Wu.

Hello Michał Żygowski, Maciej Pijanowski, Krystian Hebel,

I'd like you to do a code review.
Please visit

https://review.coreboot.org/c/coreboot/+/68750

to review the following change.

Change subject: [WIP] security/tpm: make use of PCRs configurable via Kconfig
......................................................................

[WIP] security/tpm: make use of PCRs configurable via Kconfig

At this moment, only GBB flags are moved from PCR-0 to PCR-1 when
vboot-compatibility is not enabled.

Change-Id: Ib3a192d902072f6f8d415c2952a36522b5bf09f9
Ticket: https://ticket.coreboot.org/issues/424
Signed-off-by: Sergii Dmytruk 
---
M src/security/tpm/Kconfig
M src/security/tpm/tspi/crtm.c
M src/security/tpm/tspi/crtm.h
M src/security/vboot/vboot_logic.c
4 files changed, 39 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/68750/1

diff --git a/src/security/tpm/Kconfig b/src/security/tpm/Kconfig
index 81506b4..d3bf1ff 100644
--- a/src/security/tpm/Kconfig
+++ b/src/security/tpm/Kconfig
@@ -166,4 +166,20 @@
      Runtime data whitelist of cbfs filenames. Needs to be a
      space delimited list

+config PCR_BOOT_MODE
+	int
+	default 0 if NEED_VBOOT_COMPATIBILITY
+	default 1
+config PCR_HWID
+	int
+	default 1
+config PCR_CRTM
+	int
+	default 2
+# PCR for measuring data which changes during runtime
+# e.g. CMOS, NVRAM...
+config PCR_RUNTIME_DATA
+	int
+	default 3
+
 endmenu # Trusted Platform Module (tpm)
diff --git a/src/security/tpm/tspi/crtm.c b/src/security/tpm/tspi/crtm.c
index 8eefc11..aea0069 100644
--- a/src/security/tpm/tspi/crtm.c
+++ b/src/security/tpm/tspi/crtm.c
@@ -44,7 +44,7 @@

struct region_device fmap;
    if (fmap_locate_area_as_rdev("FMAP", &fmap) == 0) {
-		if (tpm_measure_region(&fmap, TPM_CRTM_PCR, "FMAP: FMAP")) {
+		if (tpm_measure_region(&fmap, CONFIG_PCR_CRTM, "FMAP: FMAP")) {
    		printk(BIOS_ERR,
    		       "TSPI: Couldn't measure FMAP into CRTM!\n");
    		return VB2_ERROR_UNKNOWN;
@@ -58,7 +58,7 @@
    	struct region_device bootblock_fmap;
    	if (fmap_locate_area_as_rdev("BOOTBLOCK", &bootblock_fmap) == 0) {
    		if (tpm_measure_region(&bootblock_fmap,
-					TPM_CRTM_PCR,
+					CONFIG_PCR_CRTM,
    				"FMAP: BOOTBLOCK"))
    			return VB2_ERROR_UNKNOWN;
    	}
@@ -77,7 +77,7 @@
    	/* Since none of the above conditions are met let the SOC code measure the
    	 * bootblock. This accomplishes for cases where the bootblock is treated
    	 * in a special way (e.g. part of IFWI or located in a different CBFS). */
-		if (tspi_soc_measure_bootblock(TPM_CRTM_PCR)) {
+		if (tspi_soc_measure_bootblock(CONFIG_PCR_CRTM)) {
    		printk(BIOS_INFO,
    		       "TSPI: Couldn't measure bootblock into CRTM on SoC level!\n");
    		return VB2_ERROR_UNKNOWN;
@@ -122,7 +122,7 @@

switch (type) {
    case CBFS_TYPE_MRC_CACHE:
-		pcr_index = TPM_RUNTIME_DATA_PCR;
+		pcr_index = CONFIG_PCR_RUNTIME_DATA;
    	break;
    /*
     * mrc.bin is code executed on CPU, so it
@@ -132,13 +132,13 @@
    case CBFS_TYPE_STAGE:
    case CBFS_TYPE_SELF:
    case CBFS_TYPE_FIT_PAYLOAD:
-		pcr_index = TPM_CRTM_PCR;
+		pcr_index = CONFIG_PCR_CRTM;
    	break;
    default:
    	if (is_runtime_data(name))
-			pcr_index = TPM_RUNTIME_DATA_PCR;
+			pcr_index = CONFIG_PCR_RUNTIME_DATA;
    	else
-			pcr_index = TPM_CRTM_PCR;
+			pcr_index = CONFIG_PCR_CRTM;
    	break;
    }

diff --git a/src/security/tpm/tspi/crtm.h b/src/security/tpm/tspi/crtm.h
index 5cdc0e3..7084fc8 100644
--- a/src/security/tpm/tspi/crtm.h
+++ b/src/security/tpm/tspi/crtm.h
@@ -8,14 +8,6 @@
 #include <types.h>
 #include <vb2_sha.h>

-/* CRTM */
-#define TPM_CRTM_PCR 2
-
-/* PCR for measuring data which changes during runtime
- * e.g. CMOS, NVRAM...
- */
-#define TPM_RUNTIME_DATA_PCR 3
-
 #if CONFIG(TPM_LOG_CB) && CONFIG(TPM1)
 #  define TPM_MEASURE_ALGO VB2_HASH_SHA1
 #elif CONFIG(TPM_LOG_CB) && CONFIG(TPM2)
diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c
index 2230b5ab..1951447 100644
--- a/src/security/vboot/vboot_logic.c
+++ b/src/security/vboot/vboot_logic.c
@@ -187,8 +187,8 @@

static uint32_t extend_pcrs(struct vb2_context *ctx)
 {
-	return vboot_extend_pcr(ctx, 0, BOOT_MODE_PCR) ||
-		   vboot_extend_pcr(ctx, 1, HWID_DIGEST_PCR);
+	return vboot_extend_pcr(ctx, CONFIG_PCR_BOOT_MODE, BOOT_MODE_PCR) ||
+		   vboot_extend_pcr(ctx, CONFIG_PCR_HWID, HWID_DIGEST_PCR);
 }

#define EC_EFS_BOOT_MODE_VERIFIED_RW	0x00