Move test utility functions to a common place.

Also removes the dev_mode flag from Firmware Image verification as key signature for the firmware should be checked whether or not dev mode is enabled.

BUG=670
TEST=All tests still pass.

Merge remote branch 'refs/remotes/origin/master' into fixtests

Fix tests, remove dev mode. Move common code.

Review URL: http://codereview.chromium.org/1280002

include/firmware_image.h

@@ -121,8 +121,6 @@ void PrintFirmwareImage(const FirmwareImage* image);
 extern char* kVerifyFirmwareErrors[VERIFY_FIRMWARE_MAX];
 
 /* Checks for the sanity of the firmware header pointed by [header_blob].
- * If [dev_mode] is enabled, also checks the root key signature using the
- * pre-processed public root key [root_key_blob].
  *
  * On success, put signature algorithm in [algorithm], header length
  * in [header_len], and return 0.
@@ -130,7 +128,6 @@ extern char* kVerifyFirmwareErrors[VERIFY_FIRMWARE_MAX];
  */
 int VerifyFirmwareHeader(const uint8_t* root_key_blob,
                          const uint8_t* header_blob,
-                         const int dev_mode,
                          int* algorithm,
                          int* header_len);
 
@@ -157,10 +154,7 @@ int VerifyFirmwareData(RSAPublicKey* sign_key,
                        int firmware_len,
                        int algorithm);
 
-/* Performs a chained verify of the firmware blob [firmware_blob]. If
+/* Performs a chained verify of the firmware blob [firmware_blob].
- * [dev_mode] is 0 [inactive], then the pre-processed public root key
- * [root_key_blob] is used the verify the signature of the signing key,
- * else the check is skipped.
  *
  * Returns 0 on success, error code on failure.
  *
@@ -171,18 +165,14 @@ int VerifyFirmwareData(RSAPublicKey* sign_key,
  * length itself is checked early in the verification process for extra safety.
  */
 int VerifyFirmware(const uint8_t* root_key_blob,
-                   const uint8_t* firmware_blob,
+                   const uint8_t* firmware_blob);
-                   const int dev_mode);
 
-/* Performs a chained verify of the firmware [image]. If [dev_mode] is
+/* Performs a chained verify of the firmware [image].
- * 0 (inactive), then the [root_key] is used to verify the signature of
- * the signing key, else the check is skipped.
  *
  * Returns 0 on success, error code on failure.
  */
 int VerifyFirmwareImage(const RSAPublicKey* root_key,
-                        const FirmwareImage* image,
+                        const FirmwareImage* image);
-                        const int dev_mode);
 
 /* Maps error codes from VerifyFirmware() to error description. */
 const char* VerifyFirmwareErrorString(int error);

include/kernel_image.h

@@ -141,12 +141,12 @@ extern char* kVerifyKernelErrors[VERIFY_KERNEL_MAX];
  * length in [header_len], and return 0.
  * Else, return error code on failure.
  */
-int VerifyFirmwareHeader(const uint8_t* firmware_sign_key_blob,
+int VerifyKernelHeader(const uint8_t* firmware_sign_key_blob,
-                         const uint8_t* kernel_header_blob,
+                       const uint8_t* kernel_header_blob,
-                         const int dev_mode,
+                       const int dev_mode,
-                         int* firmware_algorithm,
+                       int* firmware_algorithm,
-                         int* kernel_algorithm,
+                       int* kernel_algorithm,
-                         int* header_len);
+                       int* header_len);
 
 /* Checks the kernel config (analogous to preamble for firmware) signature on
  * kernel config pointed by [kernel_config_blob] using the signing key

tests/Makefile

@@ -24,21 +24,22 @@ tests: firmware_image_tests \
 	verify_firmware_fuzz_driver \
 	verify_kernel_fuzz_driver
 
-firmware_image_tests: firmware_image_tests.c rollback_index_mock.c
+firmware_image_tests: firmware_image_tests.c rollback_index_mock.c test_common.c
 	$(CC) $(CFLAGS) $(INCLUDES) $^ -o $@ $(LIBS)
 
-firmware_rollback_tests: firmware_rollback_tests.c rollback_index_mock.c test_common.c
+firmware_rollback_tests: firmware_rollback_tests.c rollback_index_mock.c \
+	test_common.c
 	$(CC) $(CFLAGS) $(INCLUDES) $^ -o $@ $(LIBS)
 
 firmware_verify_benchmark: firmware_verify_benchmark.c timer_utils.c \
-	rollback_index_mock.c
+	rollback_index_mock.c test_common.c
 	$(CC) $(CFLAGS) $(INCLUDES) $^ -o $@ -lrt $(LIBS)
 
-kernel_image_tests: kernel_image_tests.c rollback_index_mock.c
+kernel_image_tests: kernel_image_tests.c rollback_index_mock.c test_common.c
 	$(CC) $(CFLAGS) $(INCLUDES) $^ -o $@ $(LIBS)
 
 kernel_verify_benchmark: kernel_verify_benchmark.c timer_utils.c \
-	rollback_index_mock.c
+	rollback_index_mock.c test_common.c
 	$(CC) $(CFLAGS) $(INCLUDES) $^ -o $@ -lrt $(LIBS)
 
 rsa_padding_test: rsa_padding_test.c

tests/firmware_image_tests.c

@@ -11,146 +11,44 @@
 #include "file_keys.h"
 #include "firmware_image.h"
 #include "rsa_utility.h"
+#include "test_common.h"
 #include "utility.h"
-#include "rollback_index.h"
-
-/* ANSI Color coding sequences. */
-#define COL_GREEN "\e[1;32m"
-#define COL_RED "\e[0;31m"
-#define COL_STOP "\e[m"
-
-int TEST_EQ(int result, int expected_result, char* testname) {
-  if (result == expected_result) {
-    fprintf(stderr, "%s Test " COL_GREEN " PASSED\n" COL_STOP, testname);
-    return 1;
-  } else {
-    fprintf(stderr, "%s Test " COL_RED " FAILED\n" COL_STOP, testname);
-    return 0;
-  }
-}
-
-FirmwareImage* GenerateTestFirmwareImage(int algorithm,
-                                         uint8_t* firmware_sign_key,
-                                         int firmware_key_version,
-                                         int firmware_version,
-                                         int firmware_len,
-                                         const char* root_key_file,
-                                         const char* firmware_key_file) {
-  FirmwareImage* image = FirmwareImageNew();
-
-  Memcpy(image->magic, FIRMWARE_MAGIC, FIRMWARE_MAGIC_SIZE);
-  image->firmware_sign_algorithm = algorithm;
-  image->firmware_sign_key = (uint8_t*) Malloc(
-      RSAProcessedKeySize(image->firmware_sign_algorithm));
-  Memcpy(image->firmware_sign_key, firmware_sign_key,
-         RSAProcessedKeySize(image->firmware_sign_algorithm));
-  image->firmware_key_version = firmware_key_version;
-
-  /* Update correct header length. */
-  image->header_len = GetFirmwareHeaderLen(image);
-
-  /* Calculate SHA-512 digest on header and populate header_checksum. */
-  CalculateFirmwareHeaderChecksum(image, image->header_checksum);
-
-  /* Populate firmware and preamble with dummy data. */
-  image->firmware_version = firmware_version;
-  image->firmware_len = firmware_len;
-  image->preamble_signature = image->firmware_signature = NULL;
-  Memset(image->preamble, 'P', FIRMWARE_PREAMBLE_SIZE);
-  image->firmware_data = Malloc(image->firmware_len);
-  Memset(image->firmware_data, 'F', image->firmware_len);
-
-  /* Generate and populate signatures. */
-  if (!AddFirmwareKeySignature(image, root_key_file)) {
-    fprintf(stderr, "Couldn't create key signature.\n");
-    FirmwareImageFree(image);
-    return NULL;
-  }
-
-  if (!AddFirmwareSignature(image, firmware_key_file)) {
-    fprintf(stderr, "Couldn't create firmware and preamble signature.\n");
-    FirmwareImageFree(image);
-    return NULL;
-  }
-  return image;
-}
-
-#define DEV_MODE_ENABLED 1
-#define DEV_MODE_DISABLED 0
 
 /* Normal Firmware Blob Verification Tests. */
-int VerifyFirmwareTest(uint8_t* firmware_blob, uint8_t* root_key_blob) {
+void VerifyFirmwareTest(uint8_t* firmware_blob, uint8_t* root_key_blob) {
-  int success = 1;
+  TEST_EQ(VerifyFirmware(root_key_blob, firmware_blob),
-  if (!TEST_EQ(VerifyFirmware(root_key_blob, firmware_blob, DEV_MODE_ENABLED),
+          VERIFY_FIRMWARE_SUCCESS,
-               VERIFY_FIRMWARE_SUCCESS,
+          "Normal Firmware Blob Verification");
-               "Normal Firmware Blob Verification (Dev Mode)"))
-    success = 0;
-
-  if (!TEST_EQ(VerifyFirmware(root_key_blob, firmware_blob, DEV_MODE_DISABLED),
-               VERIFY_FIRMWARE_SUCCESS,
-               "Normal Firmware Blob Verification (Trusted)"))
-    success = 0;
-  return success;
 }
 
 /* Normal FirmwareImage Verification Tests. */
-int VerifyFirmwareImageTest(FirmwareImage* image,
+void VerifyFirmwareImageTest(FirmwareImage* image,
-                            RSAPublicKey* root_key) {
+                             RSAPublicKey* root_key) {
-  int success = 1;
+  TEST_EQ(VerifyFirmwareImage(root_key, image),
-  if (!TEST_EQ(VerifyFirmwareImage(root_key, image, DEV_MODE_ENABLED),
+          VERIFY_FIRMWARE_SUCCESS,
-               VERIFY_FIRMWARE_SUCCESS,
+          "Normal FirmwareImage Verification");
-               "Normal FirmwareImage Verification (Dev Mode)"))
-    success = 0;
-
-  if (!TEST_EQ(VerifyFirmwareImage(root_key, image, DEV_MODE_DISABLED),
-               VERIFY_FIRMWARE_SUCCESS,
-               "Normal FirmwareImage Verification (Trusted)"))
-    success = 0;
-  return success;
 }
 
 /* Tampered FirmwareImage Verification Tests. */
-int VerifyFirmwareImageTamperTest(FirmwareImage* image,
+void VerifyFirmwareImageTamperTest(FirmwareImage* image,
-                                  RSAPublicKey* root_key) {
+                                   RSAPublicKey* root_key) {
-  int success = 1;
-  fprintf(stderr, "[[Tampering with firmware preamble....]]\n");
   image->firmware_version = 0;
-  if (!TEST_EQ(VerifyFirmwareImage(root_key, image, DEV_MODE_ENABLED),
+  TEST_EQ(VerifyFirmwareImage(root_key, image),
-               VERIFY_FIRMWARE_PREAMBLE_SIGNATURE_FAILED,
+          VERIFY_FIRMWARE_PREAMBLE_SIGNATURE_FAILED,
-               "FirmwareImage Preamble Tamper Verification (Dev Mode)"))
+          "FirmwareImage Preamble Tamper Verification");
-    success = 0;
-
-  if (!TEST_EQ(VerifyFirmwareImage(root_key, image, DEV_MODE_DISABLED),
-               VERIFY_FIRMWARE_PREAMBLE_SIGNATURE_FAILED,
-               "FirmwareImage Preamble Tamper Verification (Trusted)"))
-    success = 0;
   image->firmware_version = 1;
 
   image->firmware_data[0] = 'T';
-  if (!TEST_EQ(VerifyFirmwareImage(root_key, image, DEV_MODE_ENABLED),
+  TEST_EQ(VerifyFirmwareImage(root_key, image),
-               VERIFY_FIRMWARE_SIGNATURE_FAILED,
+          VERIFY_FIRMWARE_SIGNATURE_FAILED,
-               "FirmwareImage Tamper Verification (Dev Mode)"))
+          "FirmwareImage Data Tamper Verification");
-    success = 0;
-  if (!TEST_EQ(VerifyFirmwareImage(root_key, image, DEV_MODE_DISABLED),
-               VERIFY_FIRMWARE_SIGNATURE_FAILED,
-               "FirmwareImage Tamper Verification (Trusted)"))
-    success = 0;
   image->firmware_data[0] = 'F';
 
-
-  fprintf(stderr, "[[Tampering with root key signature...]]\n");
   image->firmware_key_signature[0] = 0xFF;
   image->firmware_key_signature[1] = 0x00;
-  if (!TEST_EQ(VerifyFirmwareImage(root_key, image, DEV_MODE_ENABLED),
+  TEST_EQ(VerifyFirmwareImage(root_key, image),
-               VERIFY_FIRMWARE_SUCCESS,
+          VERIFY_FIRMWARE_ROOT_SIGNATURE_FAILED,
-               "FirmwareImage Root Signature Tamper Verification (Dev Mode)"))
+          "FirmwareImage Root Signature Tamper Verification");
-    success = 0;
-  if (!TEST_EQ(VerifyFirmwareImage(root_key, image, DEV_MODE_DISABLED),
-               VERIFY_FIRMWARE_ROOT_SIGNATURE_FAILED,
-               "FirmwareImage Root Signature Tamper Verification (Trusted)"))
-    success = 0;
-
-  return success;
 }
 
 int main(int argc, char* argv[]) {
@@ -165,7 +63,6 @@ int main(int argc, char* argv[]) {
   RSAPublicKey* root_key_pub = NULL;
   int error_code = 0;
   int algorithm;
-  SetupTPM();
   if(argc != 6) {
     fprintf(stderr, "Usage: %s <algorithm> <root key> <processed root pubkey>"
             " <signing key> <processed signing key>\n", argv[0]);
@@ -194,13 +91,13 @@ int main(int argc, char* argv[]) {
   firmware_blob = GetFirmwareBlob(image, &firmware_blob_len);
 
   /* Test Firmware blob verify operations. */
-  if (!VerifyFirmwareTest(firmware_blob, root_key_blob))
+  VerifyFirmwareTest(firmware_blob, root_key_blob);
-    error_code = 255;
 
   /* Test FirmwareImage verify operations. */
-  if (!VerifyFirmwareImageTest(image, root_key_pub))
+  VerifyFirmwareImageTest(image, root_key_pub);
-    error_code = 255;
+  VerifyFirmwareImageTamperTest(image, root_key_pub);
-  if (!VerifyFirmwareImageTamperTest(image, root_key_pub))
+
+  if (!gTestSuccess)
     error_code = 255;
 
 failure:

tests/firmware_rollback_tests.c

@@ -15,73 +15,6 @@
 #include "rollback_index.h"
 #include "test_common.h"
 
-/* Generates a test firmware image for rollback tests with a given
- * [firmware_key_version] and [firmware_version]. If [is_corrupt] is 1,
- * then the image has invalid signatures and will fail verification. */
-uint8_t* GenerateRollbackTestImage(int firmware_key_version,
-                                   int firmware_version,
-                                   int is_corrupt) {
-  FirmwareImage* image = NULL;
-  uint8_t* firmware_blob = NULL;
-  const char* firmare_sign_key_pub_file = "testkeys/key_rsa1024.keyb";
-  uint8_t* firmware_sign_key = NULL;
-  const char* root_key_file = "testkeys/key_rsa8192.pem";
-  const char* firmware_key_file = "testkeys/key_rsa1024.pem";
-  uint64_t len;
-  firmware_sign_key = BufferFromFile(firmare_sign_key_pub_file,
-                                     &len);
-  if (!firmware_sign_key)
-    return NULL;
-
-  image = FirmwareImageNew();
-  if (!image)
-    return NULL;
-
-  Memcpy(image->magic, FIRMWARE_MAGIC, FIRMWARE_MAGIC_SIZE);
-  image->firmware_sign_algorithm = 0;  /* RSA1024/SHA1 */
-  image->firmware_sign_key = (uint8_t*) Malloc(
-      RSAProcessedKeySize(image->firmware_sign_algorithm));
-  Memcpy(image->firmware_sign_key, firmware_sign_key,
-         RSAProcessedKeySize(image->firmware_sign_algorithm));
-  image->firmware_key_version = firmware_key_version;
-  Free(firmware_sign_key);
-
-  /* Update correct header length. */
-  image->header_len = GetFirmwareHeaderLen(image);
-
-  /* Calculate SHA-512 digest on header and populate header_checksum. */
-  CalculateFirmwareHeaderChecksum(image, image->header_checksum);
-
-  /* Populate firmware and preamble with dummy data. */
-  image->firmware_version = firmware_version;
-  image->firmware_len = 1;
-  image->preamble_signature = image->firmware_signature = NULL;
-  Memset(image->preamble, 'P', FIRMWARE_PREAMBLE_SIZE);
-  image->firmware_data = Malloc(image->firmware_len);
-  Memset(image->firmware_data, 'F', image->firmware_len);
-
-  /* Generate and populate signatures. */
-  if (!AddFirmwareKeySignature(image, root_key_file)) {
-    fprintf(stderr, "Couldn't create key signature.\n");
-    FirmwareImageFree(image);
-    return NULL;
-  }
-
-  if (!AddFirmwareSignature(image, firmware_key_file)) {
-    fprintf(stderr, "Couldn't create firmware and preamble signature.\n");
-    FirmwareImageFree(image);
-    return NULL;
-  }
-  if (is_corrupt) {
-    /* Invalidate image. */
-    Memset(image->firmware_data, 'X', image->firmware_len);
-  }
-
-  firmware_blob = GetFirmwareBlob(image, &len);
-  FirmwareImageFree(image);
-  return firmware_blob;
-}
-
 /* Tests that check for correctness of the VerifyFirmwareDriver_f() logic
  * and rollback prevention. */
 void VerifyFirmwareDriverTest(void) {

tests/firmware_verify_benchmark.c

@@ -12,6 +12,7 @@
 #include "firmware_image.h"
 #include "padding.h"
 #include "rsa_utility.h"
+#include "test_common.h"
 #include "timer_utils.h"
 #include "utility.h"
 
@@ -34,53 +35,6 @@ const char* g_firmware_size_labels[] = {
 #define NUM_SIZES_TO_TEST (sizeof(g_firmware_sizes_to_test) / \
                            sizeof(g_firmware_sizes_to_test[0]))
 
-uint8_t* GenerateTestFirmwareBlob(int algorithm,
-                                  int firmware_len,
-                                  const uint8_t* firmware_sign_key,
-                                  const char* root_key_file,
-                                  const char* firmware_sign_key_file) {
-  FirmwareImage* image = FirmwareImageNew();
-  uint8_t* firmware_blob = NULL;
-  uint64_t firmware_blob_len = 0;
-
-  Memcpy(image->magic, FIRMWARE_MAGIC, FIRMWARE_MAGIC_SIZE);
-  image->firmware_sign_algorithm = algorithm;
-  image->firmware_sign_key = (uint8_t*) Malloc(
-      RSAProcessedKeySize(image->firmware_sign_algorithm));
-  Memcpy(image->firmware_sign_key, firmware_sign_key,
-         RSAProcessedKeySize(image->firmware_sign_algorithm));
-  image->firmware_key_version = 1;
-
-  /* Update correct header length. */
-  image->header_len = GetFirmwareHeaderLen(image);
-
-  /* Calculate SHA-512 digest on header and populate header_checksum. */
-  CalculateFirmwareHeaderChecksum(image, image->header_checksum);
-
-  /* Populate firmware and preamble with dummy data. */
-  image->firmware_version = 1;
-  image->firmware_len = firmware_len;
-  image->preamble_signature = image->firmware_signature = NULL;
-  Memset(image->preamble, 'P', FIRMWARE_PREAMBLE_SIZE);
-  image->firmware_data = Malloc(image->firmware_len);
-  Memset(image->firmware_data, 'F', image->firmware_len);
-
-  if (!AddFirmwareKeySignature(image, root_key_file)) {
-    fprintf(stderr, "Couldn't create key signature.\n");
-    FirmwareImageFree(image);
-    return NULL;
-  }
-
-  if (!AddFirmwareSignature(image, firmware_sign_key_file)) {
-    fprintf(stderr, "Couldn't create firmware and preamble signature.\n");
-    FirmwareImageFree(image);
-    return NULL;
-  }
-  firmware_blob = GetFirmwareBlob(image, &firmware_blob_len);
-  FirmwareImageFree(image);
-  return firmware_blob;
-}
-
 int SpeedTestAlgorithm(int algorithm) {
   int i, j, key_size, error_code = 0;
   ClockTimerState ct;
@@ -115,8 +69,10 @@ int SpeedTestAlgorithm(int algorithm) {
   /* Generate test images. */
   for (i = 0; i < NUM_SIZES_TO_TEST; ++i) {
     firmware_blobs[i] = GenerateTestFirmwareBlob(algorithm,
-                                                 g_firmware_sizes_to_test[i],
                                                  firmware_sign_key,
+                                                 1,  /* firmware key version. */
+                                                 1,  /* firmware version. */
+                                                 g_firmware_sizes_to_test[i],
                                                  "testkeys/key_rsa8192.pem",
                                                  firmware_sign_key_file);
     if (!firmware_blobs[i]) {
@@ -139,7 +95,7 @@ int SpeedTestAlgorithm(int algorithm) {
     StartTimer(&ct);
     for (j = 0; j < NUM_OPERATIONS; ++j) {
       if (VERIFY_FIRMWARE_SUCCESS !=
-          VerifyFirmware(root_key_blob, firmware_blobs[i], 0))
+          VerifyFirmware(root_key_blob, firmware_blobs[i]))
         fprintf(stderr, "Warning: Firmware Verification Failed.\n");
     }
     StopTimer(&ct);

tests/kernel_image_tests.c

@@ -11,143 +11,70 @@
 #include "file_keys.h"
 #include "kernel_image.h"
 #include "rsa_utility.h"
+#include "test_common.h"
 #include "utility.h"
 
-/* ANSI Color coding sequences. */
-#define COL_GREEN "\e[1;32m"
-#define COL_RED "\e[0;31m"
-#define COL_STOP "\e[m"
-
-int TEST_EQ(int result, int expected_result, char* testname) {
-  if (result == expected_result) {
-    fprintf(stderr, "%s Test " COL_GREEN " PASSED\n" COL_STOP, testname);
-    return 1;
-  }
-  else {
-    fprintf(stderr, "%s Test " COL_RED " FAILED\n" COL_STOP, testname);
-    return 0;
-  }
-}
-
-KernelImage* GenerateTestKernelImage(int firmware_sign_algorithm,
-                                     int kernel_sign_algorithm,
-                                     uint8_t* kernel_sign_key,
-                                     int kernel_key_version,
-                                     int kernel_version,
-                                     int kernel_len) {
-  KernelImage* image = KernelImageNew();
-
-  Memcpy(image->magic, KERNEL_MAGIC, KERNEL_MAGIC_SIZE);
-  image->header_version = 1;
-  image->firmware_sign_algorithm = firmware_sign_algorithm;
-  image->kernel_sign_algorithm = kernel_sign_algorithm;
-  image->kernel_key_version = kernel_key_version;
-  image->kernel_sign_key = (uint8_t*) Malloc(
-      RSAProcessedKeySize(image->kernel_sign_algorithm));
-  Memcpy(image->kernel_sign_key, kernel_sign_key,
-         RSAProcessedKeySize(image->kernel_sign_algorithm));
-
-  /* Update correct header length. */
-  image->header_len = GetKernelHeaderLen(image);
-
-  /* Calculate SHA-512 digest on header and populate header_checksum. */
-  CalculateKernelHeaderChecksum(image, image->header_checksum);
-
-  /* Populate kernel options and data with dummy data. */
-  image->kernel_version = kernel_version;
-  image->options.version[0] = 1;
-  image->options.version[1] = 0;
-  Memset(image->options.cmd_line, 0, sizeof(image->options.cmd_line));
-  image->options.kernel_len = kernel_len;
-  image->options.kernel_load_addr = 0;
-  image->options.kernel_entry_addr = 0;
-  image->kernel_key_signature = image->kernel_signature = NULL;
-  image->kernel_data = Malloc(kernel_len);
-  Memset(image->kernel_data, 'F', kernel_len);
-
-  return image;
-}
-
 #define DEV_MODE_ENABLED 1
 #define DEV_MODE_DISABLED 0
 
 /* Normal Kernel Blob Verification Tests. */
-int VerifyKernelTest(uint8_t* kernel_blob, uint8_t* firmware_key_blob) {
+void VerifyKernelTest(uint8_t* kernel_blob, uint8_t* firmware_key_blob) {
-  int success = 1;
+  TEST_EQ(VerifyKernel(firmware_key_blob, kernel_blob, DEV_MODE_ENABLED),
-  if (!TEST_EQ(VerifyKernel(firmware_key_blob, kernel_blob, DEV_MODE_ENABLED),
+          VERIFY_KERNEL_SUCCESS,
-               VERIFY_KERNEL_SUCCESS,
+          "Normal Kernel Blob Verification (Dev Mode)");
-               "Normal Kernel Blob Verification (Dev Mode)"))
-    success = 0;
 
-  if (!TEST_EQ(VerifyKernel(firmware_key_blob, kernel_blob, DEV_MODE_DISABLED),
+  TEST_EQ(VerifyKernel(firmware_key_blob, kernel_blob, DEV_MODE_DISABLED),
-               VERIFY_KERNEL_SUCCESS,
+          VERIFY_KERNEL_SUCCESS,
-               "Normal Kernel Blob Verification (Trusted)"))
+          "Normal Kernel Blob Verification (Trusted)");
-    success = 0;
-  return success;
 }
 
 
 /* Normal KernelImage Verification Tests. */
-int VerifyKernelImageTest(KernelImage* image,
+void VerifyKernelImageTest(KernelImage* image,
                           RSAPublicKey* firmware_key) {
-  int success = 1;
+  TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_ENABLED),
-  if (!TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_ENABLED),
+          VERIFY_KERNEL_SUCCESS,
-               VERIFY_KERNEL_SUCCESS,
+          "Normal KernelImage Verification (Dev Mode)");
-               "Normal KernelImage Verification (Dev Mode)"))
+  TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_DISABLED),
-    success = 0;
+          VERIFY_KERNEL_SUCCESS,
-
+          "Normal KernelImage Verification (Trusted)");
-  if (!TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_DISABLED),
-               VERIFY_KERNEL_SUCCESS,
-               "Normal KernelImage Verification (Trusted)"))
-    success = 0;
-  return success;
 }
 
 /* Tampered KernelImage Verification Tests. */
-int VerifyKernelImageTamperTest(KernelImage* image,
+void VerifyKernelImageTamperTest(KernelImage* image,
-                                RSAPublicKey* firmware_key) {
+                                 RSAPublicKey* firmware_key) {
-  int success = 1;
-  fprintf(stderr, "[[Tampering with kernel config....]]\n");
   image->options.kernel_load_addr = 0xFFFF;
-  if (!TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_ENABLED),
+  TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_ENABLED),
-               VERIFY_KERNEL_CONFIG_SIGNATURE_FAILED,
+          VERIFY_KERNEL_CONFIG_SIGNATURE_FAILED,
-               "KernelImage Config Tamper Verification (Dev Mode)"))
+          "KernelImage Config Tamper Verification (Dev Mode)");
-    success = 0;
+  TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_DISABLED),
-  if (!TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_DISABLED),
+          VERIFY_KERNEL_CONFIG_SIGNATURE_FAILED,
-               VERIFY_KERNEL_CONFIG_SIGNATURE_FAILED,
+          "KernelImage Config Tamper Verification (Trusted)");
-               "KernelImage Config Tamper Verification (Trusted)"))
-    success = 0;
   image->options.kernel_load_addr = 0;
 
   image->kernel_data[0] = 'T';
-  if (!TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_ENABLED),
+  TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_ENABLED),
-               VERIFY_KERNEL_SIGNATURE_FAILED,
+          VERIFY_KERNEL_SIGNATURE_FAILED,
-               "KernelImage Tamper Verification (Dev Mode)"))
+          "KernelImage Tamper Verification (Dev Mode)");
-    success = 0;
+  TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_DISABLED),
-  if (!TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_DISABLED),
+          VERIFY_KERNEL_SIGNATURE_FAILED,
-               VERIFY_KERNEL_SIGNATURE_FAILED,
+          "KernelImage Tamper Verification (Trusted)");
-               "KernelImage Tamper Verification (Trusted)"))
-    success = 0;
   image->kernel_data[0] = 'F';
 
-
-  fprintf(stderr, "[[Tampering with kernel key signature...]]\n");
   image->kernel_key_signature[0] = 0xFF;
   image->kernel_key_signature[1] = 0x00;
-  if (!TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_ENABLED),
+  TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_ENABLED),
-               VERIFY_KERNEL_SUCCESS,
+          VERIFY_KERNEL_SUCCESS,
-               "KernelImage Key Signature Tamper Verification (Dev Mode)"))
+          "KernelImage Key Signature Tamper Verification (Dev Mode)");
-    success = 0;
+  TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_DISABLED),
-  if (!TEST_EQ(VerifyKernelImage(firmware_key, image, DEV_MODE_DISABLED),
+          VERIFY_KERNEL_KEY_SIGNATURE_FAILED,
-               VERIFY_KERNEL_KEY_SIGNATURE_FAILED,
+          "KernelImage Key Signature Tamper Verification (Trusted)");
-               "KernelImage Key Signature Tamper Verification (Trusted)"))
-    success = 0;
-
-  return success;
 }
 
 int main(int argc, char* argv[]) {
   uint64_t len;
+  const char* firmware_key_file = NULL;
+  const char* kernel_key_file = NULL;
   uint8_t* kernel_sign_key_buf = NULL;
   uint8_t* firmware_key_blob = NULL;
   uint8_t* kernel_blob = NULL;
@@ -171,6 +98,9 @@ int main(int argc, char* argv[]) {
   firmware_key = RSAPublicKeyFromFile(argv[4]);
   firmware_key_blob = BufferFromFile(argv[4], &len);
   kernel_sign_key_buf = BufferFromFile(argv[6], &len);
+  firmware_key_file = argv[3];
+  kernel_key_file = argv[5];
+
   if (!firmware_key || !kernel_sign_key_buf || !kernel_sign_key_buf) {
     error_code = 1;
     goto failure;
@@ -181,35 +111,24 @@ int main(int argc, char* argv[]) {
                                   kernel_sign_key_buf,
                                   1,  /* Kernel Key Version */
                                   1,  /* Kernel Version */
-                                  1000);  /* Kernel Size */
+                                  1000,  /* Kernel Size */
+                                  firmware_key_file,
+                                  kernel_key_file);
   if (!image) {
     error_code = 1;
     goto failure;
   }
 
-  /* Generate and populate signatures. */
-  if (!AddKernelKeySignature(image, argv[3])) {
-    fprintf(stderr, "Couldn't create key signature.\n");
-    error_code = 1;
-    goto failure;
-  }
-
-  if (!AddKernelSignature(image, argv[5])) {
-    fprintf(stderr, "Couldn't create kernel option and kernel signature.\n");
-    error_code = 1;
-    goto failure;
-  }
-
   kernel_blob = GetKernelBlob(image, &kernel_blob_len);
 
   /* Test Kernel blob verify operations. */
-  if (!VerifyKernelTest(kernel_blob, firmware_key_blob))
+  VerifyKernelTest(kernel_blob, firmware_key_blob);
-    error_code = 255;
 
   /* Test KernelImage verify operations. */
-  if (!VerifyKernelImageTest(image, firmware_key))
+  VerifyKernelImageTest(image, firmware_key);
-    error_code = 255;
+  VerifyKernelImageTamperTest(image, firmware_key);
-  if (!VerifyKernelImageTamperTest(image, firmware_key))
+
+  if (!gTestSuccess)
     error_code = 255;
 
 failure:

tests/kernel_verify_benchmark.c

@@ -12,6 +12,7 @@
 #include "kernel_image.h"
 #include "padding.h"
 #include "rsa_utility.h"
+#include "test_common.h"
 #include "timer_utils.h"
 #include "utility.h"
 
@@ -40,63 +41,6 @@ const char* g_kernel_size_labels[] = {
 #define NUM_SIZES_TO_TEST (sizeof(g_kernel_sizes_to_test) / \
                            sizeof(g_kernel_sizes_to_test[0]))
 
-uint8_t* GenerateTestKernelBlob(int firmware_sign_algorithm,
-                                int kernel_sign_algorithm,
-                                int kernel_len,
-                                const uint8_t* kernel_sign_key,
-                                const char* firmware_key_file,
-                                const char* kernel_key_file) {
-  KernelImage* image = KernelImageNew();
-  uint8_t* kernel_blob = NULL;
-  uint64_t kernel_blob_len = 0;
-
-  Memcpy(image->magic, KERNEL_MAGIC, KERNEL_MAGIC_SIZE);
-  image->header_version = 1;
-  image->firmware_sign_algorithm = firmware_sign_algorithm;
-  image->kernel_sign_algorithm = kernel_sign_algorithm;
-  image->kernel_key_version = 1;
-  image->kernel_sign_key = (uint8_t*) Malloc(
-      RSAProcessedKeySize(image->kernel_sign_algorithm));
-  Memcpy(image->kernel_sign_key, kernel_sign_key,
-         RSAProcessedKeySize(image->kernel_sign_algorithm));
-
-  /* Update correct header length. */
-  image->header_len = GetKernelHeaderLen(image);
-
-  /* Calculate SHA-512 digest on header and populate header_checksum. */
-  CalculateKernelHeaderChecksum(image, image->header_checksum);
-
-  /* Populate kernel options and data with dummy data. */
-  image->kernel_version = 1;
-  image->options.version[0] = 1;
-  image->options.version[1] = 0;
-  Memset(image->options.cmd_line, 0, sizeof(image->options.cmd_line));
-  image->options.kernel_len = kernel_len;
-  image->options.kernel_load_addr = 0;
-  image->options.kernel_entry_addr = 0;
-  image->kernel_key_signature = image->kernel_signature = NULL;
-  image->kernel_data = Malloc(kernel_len);
-  /* TODO(gauravsh): Populate this with random data, to remove data-dependent
-   * timing artificats. */
-  Memset(image->kernel_data, 'K', kernel_len);
-
-  if (!AddKernelKeySignature(image, firmware_key_file)) {
-    fprintf(stderr, "Couldn't create key signature.\n");
-    KernelImageFree(image);
-    return NULL;
-  }
-
-  if (!AddKernelSignature(image, kernel_key_file)) {
-    fprintf(stderr, "Couldn't create kernel option and kernel  signature.\n");
-    KernelImageFree(image);
-    return NULL;
-  }
-
-  kernel_blob = GetKernelBlob(image, &kernel_blob_len);
-  KernelImageFree(image);
-  return kernel_blob;
-}
-
 int SpeedTestAlgorithm(int firmware_sign_algorithm,
                        int kernel_sign_algorithm) {
   int i, j, error_code = 0;
@@ -139,8 +83,10 @@ int SpeedTestAlgorithm(int firmware_sign_algorithm,
   for (i = 0; i < NUM_SIZES_TO_TEST; ++i) {
     kernel_blobs[i] = GenerateTestKernelBlob(firmware_sign_algorithm,
                                              kernel_sign_algorithm,
-                                             g_kernel_sizes_to_test[i],
                                              kernel_sign_key,
+                                             1,  /* kernel key version. */
+                                             1,  /* kernel version. */
+                                             g_kernel_sizes_to_test[i],
                                              firmware_sign_key_file,
                                              kernel_sign_key_file);
     if (!kernel_blobs[i]) {

tests/test_common.c

@@ -9,9 +9,13 @@
 
 #include <stdio.h>
 
+#include "file_keys.h"
+#include "rsa_utility.h"
+#include "utility.h"
+
 /* ANSI Color coding sequences. */
 #define COL_GREEN "\e[1;32m"
-#define COL_RED "\e[0;31m]"
+#define COL_RED "\e[0;31m"
 #define COL_STOP "\e[m"
 
 /* Global test success flag. */
@@ -28,3 +32,184 @@ int TEST_EQ(int result, int expected_result, char* testname) {
     return 0;
   }
 }
+
+FirmwareImage* GenerateTestFirmwareImage(int algorithm,
+                                         const uint8_t* firmware_sign_key,
+                                         int firmware_key_version,
+                                         int firmware_version,
+                                         int firmware_len,
+                                         const char* root_key_file,
+                                         const char* firmware_key_file) {
+  FirmwareImage* image = FirmwareImageNew();
+
+  Memcpy(image->magic, FIRMWARE_MAGIC, FIRMWARE_MAGIC_SIZE);
+  image->firmware_sign_algorithm = algorithm;
+  image->firmware_sign_key = (uint8_t*) Malloc(
+      RSAProcessedKeySize(image->firmware_sign_algorithm));
+  Memcpy(image->firmware_sign_key, firmware_sign_key,
+         RSAProcessedKeySize(image->firmware_sign_algorithm));
+  image->firmware_key_version = firmware_key_version;
+
+  /* Update correct header length. */
+  image->header_len = GetFirmwareHeaderLen(image);
+
+  /* Calculate SHA-512 digest on header and populate header_checksum. */
+  CalculateFirmwareHeaderChecksum(image, image->header_checksum);
+
+  /* Populate firmware and preamble with dummy data. */
+  image->firmware_version = firmware_version;
+  image->firmware_len = firmware_len;
+  image->preamble_signature = image->firmware_signature = NULL;
+  Memset(image->preamble, 'P', FIRMWARE_PREAMBLE_SIZE);
+  image->firmware_data = Malloc(image->firmware_len);
+  Memset(image->firmware_data, 'F', image->firmware_len);
+
+  /* Generate and populate signatures. */
+  if (!AddFirmwareKeySignature(image, root_key_file)) {
+    fprintf(stderr, "Couldn't create key signature.\n");
+    FirmwareImageFree(image);
+    return NULL;
+  }
+
+  if (!AddFirmwareSignature(image, firmware_key_file)) {
+    fprintf(stderr, "Couldn't create firmware and preamble signature.\n");
+    FirmwareImageFree(image);
+    return NULL;
+  }
+  return image;
+}
+
+uint8_t* GenerateTestFirmwareBlob(int algorithm,
+                                  const uint8_t* firmware_sign_key,
+                                  int firmware_key_version,
+                                  int firmware_version,
+                                  int firmware_len,
+                                  const char* root_key_file,
+                                  const char* firmware_key_file) {
+  FirmwareImage* image = NULL;
+  uint8_t* firmware_blob = NULL;
+  uint64_t firmware_blob_len = 0;
+
+  image = GenerateTestFirmwareImage(algorithm,
+                                    firmware_sign_key,
+                                    firmware_key_version,
+                                    firmware_version,
+                                    firmware_len,
+                                    root_key_file,
+                                    firmware_key_file);
+  firmware_blob = GetFirmwareBlob(image, &firmware_blob_len);
+  FirmwareImageFree(image);
+  return firmware_blob;
+}
+
+uint8_t* GenerateRollbackTestImage(int firmware_key_version,
+                                   int firmware_version,
+                                   int is_corrupt) {
+  FirmwareImage* image = NULL;
+  uint64_t len;
+  uint8_t* firmware_blob = NULL;
+  uint8_t* firmware_sign_key = NULL;
+
+  firmware_sign_key = BufferFromFile("testkeys/key_rsa1024.keyb",
+                                     &len);
+  if (!firmware_sign_key)
+    return NULL;
+  image = GenerateTestFirmwareImage(0,  /* RSA1024/SHA1 */
+                                    firmware_sign_key,
+                                    firmware_key_version,
+                                    firmware_version,
+                                    1,  /* Firmware length. */
+                                    "testkeys/key_rsa8192.pem",
+                                    "testkeys/key_rsa1024.pem");
+  if (!image)
+    return NULL;
+  if (is_corrupt) {
+    /* Invalidate image. */
+    Memset(image->firmware_data, 'X', image->firmware_len);
+  }
+
+  firmware_blob = GetFirmwareBlob(image, &len);
+  FirmwareImageFree(image);
+  return firmware_blob;
+}
+
+
+KernelImage* GenerateTestKernelImage(int firmware_sign_algorithm,
+                                     int kernel_sign_algorithm,
+                                     const uint8_t* kernel_sign_key,
+                                     int kernel_key_version,
+                                     int kernel_version,
+                                     int kernel_len,
+                                     const char* firmware_key_file,
+                                     const char* kernel_key_file) {
+  KernelImage* image = KernelImageNew();
+
+  Memcpy(image->magic, KERNEL_MAGIC, KERNEL_MAGIC_SIZE);
+  image->header_version = 1;
+  image->firmware_sign_algorithm = firmware_sign_algorithm;
+  image->kernel_sign_algorithm = kernel_sign_algorithm;
+  image->kernel_key_version = kernel_key_version;
+  image->kernel_sign_key = (uint8_t*) Malloc(
+      RSAProcessedKeySize(image->kernel_sign_algorithm));
+  Memcpy(image->kernel_sign_key, kernel_sign_key,
+         RSAProcessedKeySize(image->kernel_sign_algorithm));
+
+  /* Update correct header length. */
+  image->header_len = GetKernelHeaderLen(image);
+
+  /* Calculate SHA-512 digest on header and populate header_checksum. */
+  CalculateKernelHeaderChecksum(image, image->header_checksum);
+
+  /* Populate kernel options and data with dummy data. */
+  image->kernel_version = kernel_version;
+  image->options.version[0] = 1;
+  image->options.version[1] = 0;
+  Memset(image->options.cmd_line, 0, sizeof(image->options.cmd_line));
+  image->options.kernel_len = kernel_len;
+  image->options.kernel_load_addr = 0;
+  image->options.kernel_entry_addr = 0;
+  image->kernel_key_signature = image->kernel_signature = NULL;
+  image->kernel_data = Malloc(kernel_len);
+  Memset(image->kernel_data, 'F', kernel_len);
+
+  /* Generate and populate signatures. */
+  if (!AddKernelKeySignature(image, firmware_key_file)) {
+    fprintf(stderr, "Couldn't create key signature.\n");
+    KernelImageFree(image);
+    return NULL;
+  }
+
+  if (!AddKernelSignature(image, kernel_key_file)) {
+    fprintf(stderr, "Couldn't create kernel option and kernel signature.\n");
+    KernelImageFree(image);
+    return NULL;
+  }
+
+  return image;
+}
+
+uint8_t* GenerateTestKernelBlob(int firmware_sign_algorithm,
+                                int kernel_sign_algorithm,
+                                const uint8_t* kernel_sign_key,
+                                int kernel_key_version,
+                                int kernel_version,
+                                int kernel_len,
+                                const char* firmware_key_file,
+                                const char* kernel_key_file) {
+  KernelImage* image = NULL;
+  uint8_t* kernel_blob = NULL;
+  uint64_t kernel_blob_len = 0;
+
+  image = GenerateTestKernelImage(firmware_sign_algorithm,
+                                  kernel_sign_algorithm,
+                                  kernel_sign_key,
+                                  kernel_key_version,
+                                  kernel_version,
+                                  kernel_len,
+                                  firmware_key_file,
+                                  kernel_key_file);
+
+  kernel_blob = GetKernelBlob(image, &kernel_blob_len);
+  KernelImageFree(image);
+  return kernel_blob;
+}

tests/test_common.h

@@ -7,7 +7,54 @@
 #ifndef VBOOT_REFERENCE_TEST_COMMON_H_
 #define VBOOT_REFERENCE_TEST_COMMON_H_
 
-int TEST_EQ(int result, int expected_result, char* testname);
+#include <stdint.h>
+
+#include "firmware_image.h"
+#include "kernel_image.h"
+
 extern int gTestSuccess;
+/* Return 1 if result is equal to expected_result, else return 0.
+ * Also update the global gTestSuccess flag if test fails. */
+int TEST_EQ(int result, int expected_result, char* testname);
+
+/* Test firmware image generation functions. */
+FirmwareImage* GenerateTestFirmwareImage(int algorithm,
+                                         const uint8_t* firmware_sign_key,
+                                         int firmware_key_version,
+                                         int firmware_version,
+                                         int firmware_len,
+                                         const char* root_key_file,
+                                         const char* firmware_key_file);
+uint8_t* GenerateTestFirmwareBlob(int algorithm,
+                                  const uint8_t* firmware_sign_key,
+                                  int firmware_key_version,
+                                  int firmware_version,
+                                  int firmware_len,
+                                  const char* root_key_file,
+                                  const char* firmware_key_file);
+
+/* Test kernel image generation functions. */
+KernelImage* GenerateTestKernelImage(int firmware_sign_algorithm,
+                                     int kernel_sign_algorithm,
+                                     const uint8_t* kernel_sign_key,
+                                     int kernel_key_version,
+                                     int kernel_version,
+                                     int kernel_len,
+                                     const char* firmware_key_file,
+                                     const char* kernel_key_file);
+uint8_t* GenerateTestKernelBlob(int firmware_sign_algorithm,
+                                int kernel_sign_algorithm,
+                                const uint8_t* kernel_sign_key,
+                                int kernel_key_version,
+                                int kernel_version,
+                                int kernel_len,
+                                const char* firmware_key_file,
+                                const char* kernel_key_file);
+/* Generates a test firmware image for rollback tests with a given
+ * [firmware_key_version] and [firmware_version]. If [is_corrupt] is 1,
+ * then the image has invalid signatures and will fail verification. */
+uint8_t* GenerateRollbackTestImage(int firmware_key_version,
+                                   int firmware_version,
+                                   int is_corrupt);
 
 #endif  /* VBOOT_REFERENCE_TEST_COMMON_H_ */

tests/verify_firmware_fuzz_driver.c

@@ -28,8 +28,7 @@ int VerifySignedFirmware(const char* image_file,
     error_code = 1;
   }
 
-  if (!error_code && (error = VerifyFirmware(root_key_blob, firmware_blob,
+  if (!error_code && (error = VerifyFirmware(root_key_blob, firmware_blob))) {
-                                             0))) { /* Trusted Mode. */
     fprintf(stderr, "%s\n", VerifyFirmwareErrorString(error));
     error_code = 1;
   }

utils/firmware_image.c

@@ -322,7 +322,6 @@ char* kVerifyFirmwareErrors[VERIFY_FIRMWARE_MAX] = {
 
 int VerifyFirmwareHeader(const uint8_t* root_key_blob,
                          const uint8_t* header_blob,
-                         const int dev_mode,
                          int* algorithm,
                          int* header_len) {
   int firmware_sign_key_len;
@@ -421,8 +420,7 @@ int VerifyFirmwareData(RSAPublicKey* firmware_sign_key,
 }
 
 int VerifyFirmware(const uint8_t* root_key_blob,
-                   const uint8_t* firmware_blob,
+                   const uint8_t* firmware_blob) {
-                   const int dev_mode) {
   int error_code;
   int algorithm;  /* Signing key algorithm. */
   RSAPublicKey* firmware_sign_key = NULL;
@@ -441,7 +439,7 @@ int VerifyFirmware(const uint8_t* root_key_blob,
   header_ptr = firmware_blob + FIRMWARE_MAGIC_SIZE;
 
   /* Only continue if header verification succeeds. */
-  if ((error_code = VerifyFirmwareHeader(root_key_blob, header_ptr, dev_mode,
+  if ((error_code = VerifyFirmwareHeader(root_key_blob, header_ptr,
                                          &algorithm, &header_len)))
     return error_code;  /* AKA jump to revovery. */
 
@@ -481,8 +479,7 @@ int VerifyFirmware(const uint8_t* root_key_blob,
 }
 
 int VerifyFirmwareImage(const RSAPublicKey* root_key,
-                        const FirmwareImage* image,
+                        const FirmwareImage* image) {
-                        const int dev_mode) {
   RSAPublicKey* firmware_sign_key = NULL;
   uint8_t* header_digest = NULL;
   uint8_t* preamble_digest = NULL;
@@ -502,31 +499,26 @@ int VerifyFirmwareImage(const RSAPublicKey* root_key,
    *  1) verifying the header length is correct.
    *  2) header_checksum is correct.
    */
-  /* TODO(gauravsh): The [dev_mode] switch is actually irrelevant
+
-   * for the firmware verification.
+  /* Check key signature. */
-   * Change this to always verify the root key signature and change
+  DigestInit(&ctx, ROOT_SIGNATURE_ALGORITHM);
-   * test expectations appropriately.
+  DigestUpdate(&ctx, (uint8_t*) &image->header_len,
-   */
+               FIELD_LEN(header_len));
-  if (!dev_mode) {
+  DigestUpdate(&ctx, (uint8_t*) &image->firmware_sign_algorithm,
-    DigestInit(&ctx, ROOT_SIGNATURE_ALGORITHM);
+               FIELD_LEN(firmware_sign_algorithm));
-    DigestUpdate(&ctx, (uint8_t*) &image->header_len,
+  DigestUpdate(&ctx, (uint8_t*) &image->firmware_key_version,
-                 FIELD_LEN(header_len));
+               FIELD_LEN(firmware_key_version));
-    DigestUpdate(&ctx, (uint8_t*) &image->firmware_sign_algorithm,
+  DigestUpdate(&ctx, image->firmware_sign_key,
-                 FIELD_LEN(firmware_sign_algorithm));
+               RSAProcessedKeySize(image->firmware_sign_algorithm));
-    DigestUpdate(&ctx, (uint8_t*) &image->firmware_key_version,
+  DigestUpdate(&ctx, image->header_checksum,
-                 FIELD_LEN(firmware_key_version));
+               FIELD_LEN(header_checksum));
-    DigestUpdate(&ctx, image->firmware_sign_key,
+  header_digest = DigestFinal(&ctx);
-                 RSAProcessedKeySize(image->firmware_sign_algorithm));
+  if (!RSAVerify(root_key, image->firmware_key_signature,
-    DigestUpdate(&ctx, image->header_checksum,
+                 FIELD_LEN(firmware_key_signature),
-                 FIELD_LEN(header_checksum));
+                 ROOT_SIGNATURE_ALGORITHM,
-    header_digest = DigestFinal(&ctx);
+                 header_digest)) {
-    if (!RSAVerify(root_key, image->firmware_key_signature,
+    error_code =  VERIFY_FIRMWARE_ROOT_SIGNATURE_FAILED;
-                    FIELD_LEN(firmware_key_signature),
+    goto verify_failure;
-                    ROOT_SIGNATURE_ALGORITHM,
-                    header_digest)) {
-      error_code =  VERIFY_FIRMWARE_ROOT_SIGNATURE_FAILED;
-      goto verify_failure;
-    }
   }
 
   /* Get sign key to verify the rest of the firmware. */
@@ -682,15 +674,13 @@ int VerifyFirmwareDriver_f(uint8_t* root_key_blob,
   stored_lversion = CombineUint16Pair(GetStoredVersion(FIRMWARE_KEY_VERSION),
                                       GetStoredVersion(FIRMWARE_VERSION));
   /* Always try FirmwareA first. */
-  if (VERIFY_FIRMWARE_SUCCESS == VerifyFirmware(root_key_blob, firmwareA,
+  if (VERIFY_FIRMWARE_SUCCESS == VerifyFirmware(root_key_blob, firmwareA))
-                                                0))
     firmwareA_is_verified = 1;
   if (firmwareA_is_verified && (stored_lversion < firmwareA_lversion)) {
     /* Stored version may need to be updated but only if FirmwareB
      * is successfully verified and has a logical version greater than
      * the stored logical version. */
-    if (VERIFY_FIRMWARE_SUCCESS == VerifyFirmware(root_key_blob, firmwareB,
+    if (VERIFY_FIRMWARE_SUCCESS == VerifyFirmware(root_key_blob, firmwareB)) {
-                                                    0)) {
         if (stored_lversion < firmwareB_lversion) {
           WriteStoredVersion(FIRMWARE_KEY_VERSION,
                              (uint16_t) (min_lversion >> 16));
@@ -733,8 +723,7 @@ int VerifyFirmwareDriver_f(uint8_t* root_key_blob,
      * If FirmwareB is not a rollback, then we attempt to do the verification.
      */
     if (stored_lversion <= firmwareB_lversion &&
-        (VERIFY_FIRMWARE_SUCCESS == VerifyFirmware(root_key_blob, firmwareB,
+        (VERIFY_FIRMWARE_SUCCESS == VerifyFirmware(root_key_blob, firmwareB)))
-                                                   0)))
       return BOOT_FIRMWARE_B_CONTINUE;
   }
   /* D'oh: No bootable firmware. */

utils/firmware_utility.cc

@@ -226,8 +226,8 @@ bool FirmwareUtility::VerifySignedImage(void) {
     cerr << "Couldn't read firmware image or malformed image.\n";
     return false;
   }
-  if (!(error = VerifyFirmwareImage(root_key_pub_, image_,
+  if (VERIFY_FIRMWARE_SUCCESS ==
-                                    0)))  // Trusted Mode.
+      (error = VerifyFirmwareImage(root_key_pub_, image_)))
     return true;
   cerr << VerifyFirmwareErrorString(error) << "\n";
   return false;;