Change in ...coreboot[master]: src/lib: Implemented function calls to verify a stage after it has be...

[Amol N Sukerkar (Code Review)]

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

Change subject: src/lib: Implemented function calls to verify a stage after it has been loaded into DRAM
......................................................................

src/lib: Implemented function calls to verify a stage after it has been loaded into DRAM

This support enables a user to implement a stage verification mechanism
AFTER the stage has been loaded into DRAM. This feature is currently
used by VBOOT_STAGE_VERIFICATION

TEST=Create a coreboot.rom image which has keyblock and VBLOCK
     with VBOOT version 2.1 structures. This is done by enabling
     CONFIG_VBOOT_STAGE_VERIFICATION. Verify that the image boots
     to authenticated payload.

Change-Id: I649f511bc5375448dd7625b57a680135395d1062
Signed-off-by: Sukerkar, Amol N 
---
M src/lib/Kconfig
M src/lib/cbfs.c
M src/lib/selfboot.c
3 files changed, 59 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/32151/1

diff --git a/src/lib/Kconfig b/src/lib/Kconfig
index 2f10c1c..8111f8d 100644
--- a/src/lib/Kconfig
+++ b/src/lib/Kconfig
@@ -30,6 +30,13 @@
      Selected by features that require to parse and manipulate a flattened
      devicetree in ramstage.

+config VERIFY_ONLY_PAYLOAD_IN_RAMSTAGE
+	bool "Verify only payload in ramstage"
+	default n
+	help
+	  Selected by features that require verified boot but when only payload
+	  is verified in ramstage.
+
 if RAMSTAGE_LIBHWBASE

config HWBASE_DYNAMIC_MMIO
diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c
index 728674f..9da0fae 100644
--- a/src/lib/cbfs.c
+++ b/src/lib/cbfs.c
@@ -3,6 +3,7 @@
  *
  * Copyright (C) 2011 secunet Security Networks AG
  * Copyright 2015 Google Inc.
+ * Copyright 2019 Intel Corp.
  *
  * 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
@@ -36,6 +37,28 @@
 #define DEBUG(x...)
 #endif

+/* This is marked as weak so some verification mechanism can
+ * use it to verify after loading into DRAM. Primarily
+ * overriden by VBOOT mechanism.
+ */
+void __weak verify_stage_if_required(const struct region_device *rdev)
+{
+	/* no op */
+}
+
+/* This function checks if a certain stage/binary meets the criteria
+ * to be verified AFTER it is loaded into DRAM
+ */
+static int cbfs_verification_meets_criteria(void)
+{
+	/* if this is true, it means we are verifying nothing here
+	 * that is loaded in RAMSTAGE */
+	if (IS_ENABLED(CONFIG_VERIFY_ONLY_PAYLOAD_IN_RAMSTAGE))
+		return !ENV_RAMSTAGE;
+	else
+		return 0;
+}
+
 int cbfs_boot_locate(struct cbfsf *fh, const char *name, uint32_t *type)
 {
    struct region_device rdev;
@@ -109,6 +132,11 @@
    		return 0;
    	if (rdev_readat(rdev, buffer, offset, in_size) != in_size)
    		return 0;
+
+		/* If the stage/binary loaded in DRAM requires verification
+		 * proceed if it meets the required criteria */
+		if (cbfs_verification_meets_criteria())
+			verify_stage_if_required(rdev);
    	return in_size;

case CBFS_COMPRESS_LZ4:
@@ -127,6 +155,12 @@
    	timestamp_add_now(TS_START_ULZ4F);
    	out_size = ulz4fn(compr_start, in_size, buffer, buffer_size);
    	timestamp_add_now(TS_END_ULZ4F);
+
+		/* If the stage/binary loaded in DRAM requires verification
+		 * proceed if it meets the required criteria */
+		if (cbfs_verification_meets_criteria())
+			verify_stage_if_required(rdev);
+
    	return out_size;

case CBFS_COMPRESS_LZMA:
@@ -149,6 +183,11 @@

rdev_munmap(rdev, map);

+		/* If the stage/binary loaded in DRAM requires verification
+		 * proceed if it meets the required criteria */
+		if (cbfs_verification_meets_criteria())
+			verify_stage_if_required(rdev);
+
    	return out_size;

default:
diff --git a/src/lib/selfboot.c b/src/lib/selfboot.c
index 9aa4741..3d438e8 100644
--- a/src/lib/selfboot.c
+++ b/src/lib/selfboot.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2003 Eric W. Biederman 
  * Copyright (C) 2009 Ron Minnich 
  * Copyright (C) 2016 George Trudeau 
+ * Copyright (C) 2019 Intel Corp.
  *
  * 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
@@ -32,6 +33,15 @@
 /* The type syntax for C is essentially unparsable. -- Rob Pike */
 typedef int (*checker_t)(struct cbfs_payload_segment *cbfssegs, void *args);

+/* This is marked as weak so some verification mechanism can
+ * use it to verify after loading into DRAM. Primarily
+ * overriden by VBOOT mechanism.
+ */
+void __weak verify_stage_if_required(const struct region_device *rdev)
+{
+        /* no op */
+}
+
 /* Decode a serialized cbfs payload segment
  * from memory into native endianness.
  */
@@ -269,6 +279,9 @@

rdev_munmap(prog_rdev(payload), data);

+	/* verify payload using a secure boot mechanism if required */
+	verify_stage_if_required(prog_rdev(payload));
+
    /* Pass cbtables to payload if architecture desires it. */
    prog_set_entry(payload, (void *)entry, cbmem_find(CBMEM_ID_CBTABLE));

[build bot (Jenkins) (Code Review)]

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

Change subject: src/lib: Implemented function calls to verify a stage after it has been loaded into DRAM
......................................................................

Patch Set 1:

(3 comments)

https://review.coreboot.org/#/c/32151/1/src/lib/cbfs.c 
File src/lib/cbfs.c:

https://review.coreboot.org/#/c/32151/1/src/lib/cbfs.c@42 
PS1, Line 42:  * overriden by VBOOT mechanism.
'overriden' may be misspelled - perhaps 'overridden'?

https://review.coreboot.org/#/c/32151/1/src/lib/selfboot.c 
File src/lib/selfboot.c:

https://review.coreboot.org/#/c/32151/1/src/lib/selfboot.c@38 
PS1, Line 38:  * overriden by VBOOT mechanism.
'overriden' may be misspelled - perhaps 'overridden'?

https://review.coreboot.org/#/c/32151/1/src/lib/selfboot.c@42 
PS1, Line 42:         /* no op */
code indent should use tabs where possible

Change in coreboot[master]: src/lib: Implemented function calls to verify a stage after it has be...

[Julius Werner (Code Review)]

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

Change subject: src/lib: Implemented function calls to verify a stage after it has been loaded into DRAM
......................................................................

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.