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>

6 files changed, 186 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))) {
diff --git a/tests/futility/data/nivviks.FMAP b/tests/futility/data/nivviks.FMAP
new file mode 100644
index 00000000..06f408eb
--- /dev/null
+++ b/tests/futility/data/nivviks.FMAP
diff --git a/tests/futility/data/nivviks.GBB b/tests/futility/data/nivviks.GBB
new file mode 100644
index 00000000..39f5c57a
--- /dev/null
+++ b/tests/futility/data/nivviks.GBB
diff --git a/tests/futility/data/nivviks.RO_GSCVD b/tests/futility/data/nivviks.RO_GSCVD
new file mode 100644
index 00000000..165c689b
--- /dev/null
+++ b/tests/futility/data/nivviks.RO_GSCVD
diff --git a/tests/futility/run_test_scripts.sh b/tests/futility/run_test_scripts.sh
index e0c8292d..dc87f76b 100755
--- a/tests/futility/run_test_scripts.sh
+++ b/tests/futility/run_test_scripts.sh
@@ -36,6 +36,7 @@ ${SCRIPT_DIR}/futility/test_sign_keyblocks.sh
 ${SCRIPT_DIR}/futility/test_sign_usbpd1.sh
 ${SCRIPT_DIR}/futility/test_update.sh
 ${SCRIPT_DIR}/futility/test_file_types.sh
+${SCRIPT_DIR}/futility/test_gscvd.sh
 "
 
 # Get ready...
diff --git a/tests/futility/test_gscvd.sh b/tests/futility/test_gscvd.sh
new file mode 100755
index 00000000..54d48479
--- /dev/null
+++ b/tests/futility/test_gscvd.sh
@@ -0,0 +1,107 @@
+#!/bin/bash -eux
+# Copyright 2022 The ChromiumOS Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+if [[ -z ${SCRIPT_DIR+x} ]]; then
+  # must be running standalone
+  SCRIPT_DIR="$(readlink -f "$(dirname "$0")"/..)"
+  FUTILITY="${SCRIPT_DIR}/../build/futility/futility"
+fi
+
+if [[ ! -e ${FUTILITY} ]]; then
+  echo "The futility app not available, run 'make futil' in the top directory" \
+       >&2
+  exit 1
+fi
+
+KEYS_DIR="$(readlink -f "${SCRIPT_DIR}/devkeys")"
+TMPD="$(mktemp -d /tmp/"$(basename "$0")".XXXXX)"
+trap '/bin/rm -rf "${TMPD}"' EXIT
+
+# Test FMAP sections were taken from Nivviks image, which is 32M in size.
+FW_IMAGE_SIZE_K=$(( 32 * 1024 ))
+
+# FMAP offset in the original image
+FMAP_OFFSET_K=28696
+
+main() {
+  local bios_blob
+  local command_args
+  local fmap_blob
+  local hwid
+  local pubkhash
+  local section
+
+  cd "${SCRIPT_DIR}/futility"
+
+  # Create a blob of the firmware image size.
+  bios_blob="${TMPD}/image.bin"
+  cat /dev/zero | tr '\000' '\377' | \
+    dd of="${bios_blob}" bs=1K count="${FW_IMAGE_SIZE_K}" status=none
+
+  # Paste the FMAP blob at the known location
+  fmap_blob="data/nivviks.FMAP"
+  dd if="${fmap_blob}" of="${bios_blob}" bs=1K seek="${FMAP_OFFSET_K}" \
+     conv=notrunc status=none
+
+  # Paste other available FMAP areas into the image.
+  command_args=()
+  for section in data/nivviks.[A-Z]*; do
+    local name
+
+    if [[ ${section} =~ .*FMAP ]]; then
+      continue
+    fi
+
+    name="${section##*.}"
+    command_args+=( "${name}:${section}" )
+  done
+  "${FUTILITY}" load_fmap "${bios_blob}" "${command_args[@]}"
+
+  # Make sure gbb flags are nonzero
+  "${FUTILITY}" gbb --set --flags=1 "${bios_blob}"
+
+  # Sign the blob using ranges already present in the RO_GSCVD section.
+  "${FUTILITY}" gscvd --keyblock "${KEYS_DIR}"/arv_platform.keyblock \
+                --platform_priv "${KEYS_DIR}"/arv_platform.vbprivk \
+                --board_id XYZ1 \
+                --root_pub_key "${KEYS_DIR}"/arv_root.vbpubk "${bios_blob}"
+
+  # Calculate root pub key hash
+  pubkhash="$( "${FUTILITY}" gscvd --root_pub_key \
+      "${KEYS_DIR}"/arv_root.vbpubk | tail -1)"
+
+  # Run verification, this one is expected to fail because GBB flags are not
+  # zero.
+  if "${FUTILITY}" gscvd "${bios_blob}" "${pubkhash}" 2>/dev/null ; then
+    echo "Unexpected signature match!" >&2
+    exit 1
+  fi
+
+  # Clear the flags and try verifying again, should succeed this time.
+  "${FUTILITY}" gbb --set --flags=0 "${bios_blob}"
+  if ! "${FUTILITY}" gscvd "${bios_blob}" "${pubkhash}" 2>/dev/null ; then
+    echo "Unexpected signature MISmatch!" >&2
+    exit 1
+  fi
+
+  # Change HWID and see that signature still matches.
+  hwid="$("${FUTILITY}" gbb --hwid "${bios_blob}" | sed 's/.*: //')"
+  "${FUTILITY}" gbb  --set --hwid="${hwid}xx" "${bios_blob}"
+  if ! "${FUTILITY}" gscvd "${bios_blob}" "${pubkhash}" 2>/dev/null ; then
+    echo "Unexpected signature MISmatch after modifying HWID!" >&2
+    exit 1
+  fi
+
+  # Modify the recovery key and see that signature verification fails.
+  "${FUTILITY}" gbb --set \
+                --recoverykey="${KEYS_DIR}"/recovery_kernel_data_key.vbpubk \
+                 "${bios_blob}"
+  if "${FUTILITY}" gscvd "${bios_blob}" "${pubkhash}" 2>/dev/null ; then
+    echo "Unexpected signature match after updating recovery key!" >&2
+    exit 1
+  fi
+}
+
+main "$@"