Change in ...coreboot[master]: src/security/vboot: Enabled support for dsdt.aml

[Amol N Sukerkar (Code Review)]

Amol N Sukerkar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32157

Change subject: src/security/vboot: Enabled support for dsdt.aml
......................................................................

src/security/vboot: Enabled support for dsdt.aml

This change enables vboot Stage Verification support for dsdt.aml
ACPI table.

BRANCH=none

TEST=Create a coreboot.rom image by enabling CONFIG_VBOOT_STAGE_VERIFICATION
     and CONFIG_VBOOT. Verify that the image boots to authenticated payload
     and graphics is displayed via HDMI and Display Port.

Change-Id: I51a627bc0622da64ce4486f27912753497822ce0
Signed-off-by: Sukerkar, Amol N 
---
M src/security/vboot/Kconfig
M src/security/vboot/Makefile.inc
M src/security/vboot/vboot_logic_ex.c
3 files changed, 36 insertions(+), 20 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/32157/1

diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig
index 036c553..5e60ee7 100644
--- a/src/security/vboot/Kconfig
+++ b/src/security/vboot/Kconfig
@@ -373,10 +373,20 @@
    depends on VBOOT_STAGE_VERIFICATION

config VBOOT_2_1_FSPS_HASH_KEY
-	string "Coreboot fsps.bin Stage Hashing Key(private)"
+	string "Coreboot fsps.bin Hashing Key(private)"
    default "$(VBOOT_SOURCE)/tests/devkeys/fhash.vbprik2"
    depends on VBOOT_STAGE_VERIFICATION

+config VBOOT_2_1_DSDT_AML_HASH_KEY
+	string "Coreboot dsdt.aml Hashing Key(private)"
+	default "$(VBOOT_SOURCE)/tests/devkeys/dhash.vbprik2"
+	depends on VBOOT_STAGE_VERIFICATION
+
+config VBOOT_2_1_TPM2_AML_HASH_KEY
+	string "Coreboot tpm2.aml Hashing Key(private)"
+	default "$(VBOOT_SOURCE)/tests/devkeys/thash.vbprik2"
+	depends on VBOOT_STAGE_VERIFICATION
+
 config VBOOT_2_1_PAYLOAD_HASH_KEY
    string "Coreboot PAYLOAD Stage Hashing Key(private)"
    default "$(VBOOT_SOURCE)/tests/devkeys/phash.vbprik2"
diff --git a/src/security/vboot/Makefile.inc b/src/security/vboot/Makefile.inc
index 1c03c78..b0700a3 100644
--- a/src/security/vboot/Makefile.inc
+++ b/src/security/vboot/Makefile.inc
@@ -354,13 +354,23 @@
 $(obj)/fsps.hash: $(obj)/coreboot.rom
    @printf "    CREATE FSPS.BIN HASH\n"
    $(CBFSTOOL) $< extract -n $(call strip_quotes,$(CONFIG_FSP_S_CBFS)) \
-	-r FW_MAIN_A -f $@.tmp &> /dev/null
+	-r FW_MAIN_A -f $@.tmp
    $(FUTILITY) --vb21 sign \
    	--type rwsig \
    	--prikey "$(CONFIG_VBOOT_2_1_FSPS_HASH_KEY)" \
    	$@.tmp $@
    rm -f $<.tmp $@.tmp

+$(obj)/dsdt.hash: $(obj)/coreboot.rom
+	@printf "    CREATE DSDT.AML HASH\n"
+	$(CBFSTOOL) $< extract -n $(CONFIG_CBFS_PREFIX)/dsdt.aml \
+	-r FW_MAIN_A -f $@.tmp
+	$(FUTILITY) --vb21 sign \
+		--type rwsig \
+		--prikey "$(CONFIG_VBOOT_2_1_DSDT_AML_HASH_KEY)" \
+		$@.tmp $@
+	rm -f $<.tmp $@.tmp
+
 $(obj)/payload.hash: $(obj)/coreboot.rom
    @printf "    CREATE PAYLOAD HASH\n"
    $(CBFSTOOL) $< print -r FW_MAIN_A > $<.tmp
@@ -379,7 +389,8 @@
    	--flags $(CONFIG_VBOOT_KEYBLOCK_PREAMBLE_FLAGS)

$(obj)/VBLOCK_%.bin: $(obj)/FW_MAIN_%.bin $(FUTILITY) \
-	$(obj)/ramstage.hash $(obj)/fsps.hash $(obj)/payload.hash \
+	$(obj)/ramstage.hash $(obj)/fsps.hash $(obj)/dsdt.hash \
+	$(obj)/payload.hash \
    $(obj)/firmware.kb21
    $(FUTILITY) vbutil_firmware \
                 --vblock21 $@ \
diff --git a/src/security/vboot/vboot_logic_ex.c b/src/security/vboot/vboot_logic_ex.c
index 0a071f0..000d99e 100644
--- a/src/security/vboot/vboot_logic_ex.c
+++ b/src/security/vboot/vboot_logic_ex.c
@@ -13,7 +13,6 @@
  * GNU General Public License for more details.
  */

-#include <security/tpm/antirollback.h>
 #include <arch/exception.h>
 #include <assert.h>
 #include <bootmode.h>
@@ -30,6 +29,8 @@
 #include <cbmem.h>
 #include <rmodule.h>

+#include "antirollback.h"
+
 /* For individual stage verification design, only
  * RW Region A is relevant. So, force CBFS to bot
  * from RW Region A */
@@ -60,16 +61,6 @@
    return;
 }

-int vb2ex_tpm_clear_owner(struct vb2_context *ctx)
-{
-	uint32_t rv;
-	printk(BIOS_INFO, "Clearing TPM owner\n");
-	rv = tpm_clear_and_reenable();
-	if (rv)
-		return VB2_ERROR_EX_TPM_CLEAR_OWNER;
-	return VB2_SUCCESS;
-}
-
 int vb2ex_read_resource(struct vb2_context *ctx,
    		enum vb2_resource_index index,
    		uint32_t offset,
@@ -163,18 +154,22 @@
                 const struct vb2_id tmp_id = VB2_ID_RAMSTAGE;
                 memcpy(id, &tmp_id, sizeof(*id));
         } else if (ENV_RAMSTAGE) {
-		/* In RAMSTAGE, we verify FSPS and PAYLOAD,
-		 * conditionally, so, get the appropriate ID */
+		/* In RAMSTAGE, we verify FSPS, dsdt.aml, tpm2.aml
+		 * and PAYLOAD, conditionally, so, get the appropriate ID
+		 */
                 if (!memcmp(name, CONFIG_CBFS_PREFIX"/payload",
-						sizeof(name))) {
+				sizeof(CONFIG_CBFS_PREFIX"/payload"))) {
                         const struct vb2_id tmp_id = VB2_ID_PAYLOAD;
                         memcpy(id, &tmp_id, sizeof(*id));
                 } else if (!memcmp(name, CONFIG_FSP_S_CBFS,
-						sizeof(name))) {
+					sizeof(CONFIG_FSP_S_CBFS))) {
                         const struct vb2_id tmp_id = VB2_ID_FSPS;
                         memcpy(id, &tmp_id, sizeof(*id));
-                }
-                else
+                } else if (!memcmp(name, CONFIG_CBFS_PREFIX"/dsdt.aml",
+				sizeof(CONFIG_CBFS_PREFIX"/dsdt.aml"))) {
+			const struct vb2_id tmp_id = VB2_ID_DSDT_AML;
+			memcpy(id, &tmp_id, sizeof(*id));
+		} else
                         die("Invalid component");
         } else
                 die("Invalid stage");

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32157 )

Change subject: src/security/vboot: Enabled support for dsdt.aml
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/#/c/32157/1/src/security/vboot/vboot_logic_ex.c 
File src/security/vboot/vboot_logic_ex.c:

https://review.coreboot.org/#/c/32157/1/src/security/vboot/vboot_logic_ex.c@... 
PS1, Line 168:                 } else if (!memcmp(name, CONFIG_CBFS_PREFIX"/dsdt.aml",
code indent should use tabs where possible

https://review.coreboot.org/#/c/32157/1/src/security/vboot/vboot_logic_ex.c@... 
PS1, Line 168:                 } else if (!memcmp(name, CONFIG_CBFS_PREFIX"/dsdt.aml",
please, no spaces at the start of a line

[Amol N Sukerkar (Code Review)]

Hello Aaron Durbin, Subrata Banik, 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/+/32157

to look at the new patch set (#2).

Change subject: src/security/vboot: Enabled support for dsdt.aml
......................................................................

src/security/vboot: Enabled support for dsdt.aml

This change enables vboot Stage Verification support for dsdt.aml
ACPI table.

BRANCH=none

TEST=Create a coreboot.rom image by enabling CONFIG_VBOOT_STAGE_VERIFICATION
     and CONFIG_VBOOT. Verify that the image boots to authenticated payload
     and graphics is displayed via HDMI and Display Port.

Change-Id: I51a627bc0622da64ce4486f27912753497822ce0
Signed-off-by: Sukerkar, Amol N 
---
M src/security/vboot/Kconfig
M src/security/vboot/Makefile.inc
M src/security/vboot/vboot_logic_ex.c
3 files changed, 36 insertions(+), 20 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/32157/2

Change in coreboot[master]: src/security/vboot: Enabled support for dsdt.aml

[Julius Werner (Code Review)]

Julius Werner has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/32157 )

Change subject: src/security/vboot: Enabled support for dsdt.aml
......................................................................

Abandoned

This direction of development was abandoned and instead the CONFIG_CBFS_VERIFICATION effort is intended to solve this use case. See CB:32159 for original discussion.