gscvd: presume GBB flags are zero when hashing the RO space contentsstabilize-15208.Bstabilize-15207.B

It is still being debated who is supposed to make sure that the GBB
flags are set to zero before the root of trust validation is granted
to the AP firmware image, but as of today the approach is that the GBB
flags must be zero at AP RO validation time.

The problem is that when AP RO space signature is created GBB flags
can be set to a non-zero value.

With this patch when AP RO areas contents is hashed, in case GBB flags
are included in one of the ranges, the flags are not read from the
flash, and substituted with zero.

During validation the real flags value is used. A unit test is added
to verify various futility gscvd GBB related situations, the blobs for
the unit test were extracted from a Nivviks firmware image.

BRANCH=none
BUG=b:245799496, b:253540670
TEST='./tests/futility/test_gscvd.sh' and 'make runfutiltests' succeed

Change-Id: I2f047b990cf71ea24d191fc690da08e25ebb10cc
Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3958581
Reviewed-by: Yu-Ping Wu <yupingso@chromium.org>

1 files changed, 78 insertions, 7 deletions

diff --git a/futility/cmd_gscvd.c b/futility/cmd_gscvd.c
index f7dc73b6..5d860452 100644
--- a/futility/cmd_gscvd.c
+++ b/futility/cmd_gscvd.c
@@ -447,6 +447,68 @@ static int add_gbb(struct gscvd_ro_ranges *ranges, const struct file_buf *file)
 }
 
 /**
+ * Extend AP RO hash digest with data from an address range.
+ *
+ * If the flags_offset value is non zero and happens to fall into the passed
+ * in range, do not read values from flash in the flags_offset..+flags_size
+ * range, instead feed zeros to the hashing function.
+ *
+ * NOTE that flags are expected to fully fit into the range, cases of overlap
+ * are not supported.
+ *
+ * @param ap_firmware_file  pointer to the AP firmware file layout descriptor
+ * @param dc  pointer to the hash calculating context
+ * @param offset  offset of the beginning of the range in AP SPI flash
+ * @param size  size of the range
+ * @param flags_offset  if nonzero - offset of the GBB flags field in
+ *			AP SPI flash
+ *
+ * @return VB2_SUCCESS or digest extension error, if any.
+ */
+static vb2_error_t extend_digest(const struct file_buf *ap_firmware_file,
+				 struct vb2_digest_context *dc,
+				 uint32_t offset,
+				 uint32_t size,
+				 uint32_t flags_offset)
+{
+	/* Define it as array to simplify calling vb2_digest_extend() below. */
+	const uint8_t flags[sizeof(vb2_gbb_flags_t)] = {0};
+
+	if (flags_offset &&
+	    (flags_offset >= offset) &&
+	    (flags_offset < (offset + size))) {
+		uint32_t flags_size;
+		vb2_error_t rv;
+
+		/*
+		 * This range includes GBB flags, which need to be zeroized.
+		 *
+		 * First get the hash of up to the flags.
+		 */
+		rv = vb2_digest_extend(dc, ap_firmware_file->data + offset,
+				       flags_offset - offset);
+		if (rv != VB2_SUCCESS)
+			return rv;
+
+		size -= flags_offset - offset;
+		offset = flags_offset;
+
+		/* Now hash the flag space, maybe partially. */
+		flags_size = VB2_MIN(size, sizeof(flags));
+		rv = vb2_digest_extend(dc, flags, flags_size);
+		if (rv != VB2_SUCCESS)
+			return rv;
+
+		/* Update size and offset to cover the rest of the range. */
+		size -= flags_size;
+
+		offset += flags_size;
+	}
+
+	return vb2_digest_extend(dc,ap_firmware_file->data + offset, size);
+}
+
+/**
  * Calculate hash of the RO ranges.
  *
  * @param ap_firmware_file  pointer to the AP firmware file layout descriptor
@@ -456,16 +518,23 @@ static int add_gbb(struct gscvd_ro_ranges *ranges, const struct file_buf *file)
  * @param digest  memory to copy the calculated hash to
  * @param digest_ size requested size of the digest, padded with zeros if the
  *	          SHA digest size is smaller than digest_size
+ * @param override_gbb_flags  if true, replace GBB flags value with zero
  *
  * @return zero on success, -1 on failure.
  */
 static int calculate_ranges_digest(const struct file_buf *ap_firmware_file,
 				   const struct gscvd_ro_ranges *ranges,
 				   enum vb2_hash_algorithm hash_alg,
-				   void *digest, size_t digest_size)
+				   void *digest, size_t digest_size,
+				   bool override_gbb_flags)
 {
 	struct vb2_digest_context dc;
 	size_t i;
+	uint32_t flags_offset = 0;
+
+	if (override_gbb_flags && ap_firmware_file->gbb_area)
+		flags_offset = offsetof(struct vb2_gbb_header, flags) +
+			ap_firmware_file->gbb_area->area_offset;
 
 	/* Calculate the ranges digest. */
 	if (vb2_digest_init(&dc, false, hash_alg, 0) != VB2_SUCCESS) {
@@ -474,10 +543,10 @@ static int calculate_ranges_digest(const struct file_buf *ap_firmware_file,
 	}
 
 	for (i = 0; i < ranges->range_count; i++) {
-		if (vb2_digest_extend(&dc,
-				      ap_firmware_file->data +
-					      ranges->ranges[i].offset,
-				      ranges->ranges[i].size) != VB2_SUCCESS) {
+		if (extend_digest(ap_firmware_file, &dc,
+				  ranges->ranges[i].offset,
+				  ranges->ranges[i].size,
+				  flags_offset) != VB2_SUCCESS) {
 			ERROR("Failed to extend digest!\n");
 			return -1;
 		}
@@ -548,7 +617,8 @@ struct gsc_verification_data *create_gvd(struct file_buf *ap_firmware_file,
 
 	if (calculate_ranges_digest(ap_firmware_file, ranges, gvd->hash_alg,
 				    gvd->ranges_digest,
-				    sizeof(gvd->ranges_digest))) {
+				    sizeof(gvd->ranges_digest),
+				    true)) {
 		free(gvd);
 		return NULL;
 	}
@@ -971,7 +1041,8 @@ static int validate_gscvd(int argc, char *argv[])
 
 		if (calculate_ranges_digest(&ap_firmware_file, &ranges,
 					    gvd->hash_alg, digest,
-					    sizeof(digest)))
+					    sizeof(digest),
+					    false))
 			break;
 
 		if (memcmp(digest, gvd->ranges_digest, sizeof(digest))) {