commit 33ff54fd5272ba4807c704aee8a46ee5d7d29215
author Alyssa Haroldsen <kupiakos@google.com> Tue Apr 12 14:21:46 2022 -0700
committer Chromeos LUCI <chromeos-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Apr 13 10:35:51 2022 +0000
tree 1a2d749c922830acffe60c384d4a45b0bf5cf569
parent e61f21346777c792868c96aad295aa704a41eae3

[futility] Open gscvd in R/O mode when verifying

Otherwise, it will mutate the file under your feet!
This also adds some clarifying messages, including a message if
verification failed due to incorrect signing instead of silent output.

BUG=None
TEST=`futility gscvd <file>` does not change sha256sum of file
BRANCH=None
Signed-off-by: Alyssa Haroldsen <kupiakos@google.com>

Change-Id: Ic793ad47c0160e3cedb7e2b7cc842a3f4380749d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3584690
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
Commit-Queue: Alyssa Haroldsen <kupiakos@google.com>
Tested-by: Alyssa Haroldsen <kupiakos@google.com>

futility/cmd_gscvd.c

diff --git a/futility/cmd_gscvd.c b/futility/cmd_gscvd.c
index 7fff293..e8b2fae 100644
--- a/futility/cmd_gscvd.c
+++ b/futility/cmd_gscvd.c
@@ -40,9 +40,9 @@
  *   AP firmware file is ~/tmp/guybrush-signed:
  *
   ./build/futility/futility gscvd --outfile ~/tmp/guybrush-signed \
-     -R 818100:10000,f00000:100,f80000:2000,f8c000:1000,0x00804000:0x00000800 \
-     -k ~/tmp/packed -p tests/devkeys/arv_platform.vbprivk -b 5a5a4352  \
-     -r tests/devkeys/arv_root.vbpubk ~/tmp/image-guybrush.serial.bin
+    -R 818100:10000,f00000:100,f80000:2000,f8c000:1000,0x00804000:0x00000800 \
+    -k ~/tmp/packed -p tests/devkeys/arv_platform.vbprivk -b 5a5a4352  \
+    -r tests/devkeys/arv_root.vbpubk ~/tmp/image-guybrush.serial.bin
  *------------
  * Command to validate a previously signed AP firmware file. The hash is the
  *  sha256sum of tests/devkeys/kernel_subkey.vbpubk:
@@ -140,12 +140,13 @@
  *
  * @return 0 on success 1 on failure.
  */
-static int load_ap_firmware(const char *file_name, struct file_buf *file)
+static int load_ap_firmware(const char *file_name, struct file_buf *file,
+			int mode)
 {
 	int fd;
 	int rv;
 
-	fd = open(file_name, O_RDWR);
+	fd = open(file_name, mode);
 	if (fd < 0) {
 		ERROR("Can't open %s: %s\n", file_name,
 		      strerror(errno));
@@ -156,7 +157,8 @@
 	do {
 		rv = 1;
 
-		if (futil_map_file(fd, MAP_RW, &file->data, &file->len)) {
+		if (futil_map_file(fd, mode == O_RDWR ? MAP_RW : MAP_RO,
+				   &file->data, &file->len)) {
 			file->data = NULL;
 			break;
 		}
@@ -807,7 +809,7 @@
 
 		rv = -1; /* Speculative, will be cleared on success. */
 
-		if (load_ap_firmware(file_name, &ap_firmware_file))
+		if (load_ap_firmware(file_name, &ap_firmware_file, O_RDONLY))
 			break;
 
 		/* Copy ranges from gscvd to local structure. */
@@ -842,12 +844,15 @@
 			break;
 		}
 
-		if (validate_pubk_signature(&gvd->root_key_header,
-					    kblock))
+		if (validate_pubk_signature(&gvd->root_key_header, kblock)) {
+			ERROR("Keyblock not signed by root key\n");
 			break;
+		}
 
-		if (validate_gvd_signature(gvd, &kblock->data_key))
+		if (validate_gvd_signature(gvd, &kblock->data_key)) {
+			ERROR("GVD not signed by platform key\n");
 			break;
+		}
 
 		rv = 0;
 	} while (false);
@@ -1009,7 +1014,7 @@
 		if (validate_privk(kblock, plat_privk))
 			break;
 
-		if (load_ap_firmware(work_file, &ap_firmware_file))
+		if (load_ap_firmware(work_file, &ap_firmware_file, O_RDWR))
 			break;
 
 		if (verify_ranges(&ranges, &ap_firmware_file))

futility/cmd_show.c

diff --git a/futility/cmd_show.c b/futility/cmd_show.c
index dbe6edd..08974e4 100644
--- a/futility/cmd_show.c
+++ b/futility/cmd_show.c
@@ -635,4 +635,5 @@
 
 DECLARE_FUTIL_COMMAND(verify, do_verify,
 		      VBOOT_VERSION_ALL,
-		      "Verify the signatures of various binary components");
+		      "Verify the signatures of various binary components. "
+		      "This does not verify GSCVD contents.");