Make kernel signature a part of the kernel preamble.

With this change, the kernel signature is a part of the preamble block (and therefore, used during preamble signature verification).

BUG=670
TEST=image verification tests still pass. corrected splicing test expectations (and it passes).

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

tests/kernel_splicing_tests.c

@@ -65,12 +65,12 @@ void VerifyKernelSplicingTest()
   Memcpy(image2->kernel_data, image1->kernel_data,
          image2->kernel_len);
 
-  TEST_EQ(VerifyKernelImage(firmware_key, image2, 0),
+  TEST_NEQ(VerifyKernelImage(firmware_key, image2, 0),
-          VERIFY_KERNEL_SIGNATURE_FAILED,
+           VERIFY_KERNEL_SUCCESS,
-          "KernelImage kernel_data Splicing");
+           "KernelImage kernel_data Splicing");
   kernel_blob = GetKernelBlob(image2, &len);
-  TEST_EQ(VerifyKernel(firmware_key_blob, kernel_blob, 0),
+  TEST_NEQ(VerifyKernel(firmware_key_blob, kernel_blob, 0),
-          VERIFY_KERNEL_SIGNATURE_FAILED,
+          VERIFY_KERNEL_SUCCESS,
           "Kernel Blob kernel_data Splicing");
 }
 

tests/test_common.c

@@ -33,6 +33,18 @@ int TEST_EQ(int result, int expected_result, char* testname) {
   }
 }
 
+int TEST_NEQ(int result, int not_expected_result, char* testname) {
+  if (result != not_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);
+    gTestSuccess = 0;
+    return 0;
+  }
+}
+
 FirmwareImage* GenerateTestFirmwareImage(int algorithm,
                                          const uint8_t* firmware_sign_key,
                                          int firmware_key_version,

tests/test_common.h

@@ -13,9 +13,13 @@
 #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);
+/* Return 0 if result is equal to not_expected_result, else return 1.
+ * Also update the global gTestSuccess flag if test fails. */
+int TEST_NEQ(int result, int not_expected_result, char* testname);
 
 /* Test firmware image generation functions. */
 FirmwareImage* GenerateTestFirmwareImage(int algorithm,

vboot_firmware/include/kernel_image_fw.h

@@ -49,15 +49,12 @@ typedef struct KernelImage {
   uint64_t bootloader_offset;  /* Offset of bootloader in kernel_data. */
   uint64_t bootloader_size;  /* Size of bootloader in bytes. */
   uint64_t padded_header_size;  /* start of kernel_data in disk partition */
+  uint8_t* kernel_signature;  /* Signature on [kernel_data] below.
+                               * NOTE: This is only considered valid
+                               * if preamble_signature successfully verifies. */
   /* end of preamble */
-
+  uint8_t* preamble_signature;  /* signature on preamble, (includes
-  uint8_t* preamble_signature;  /* Signature on the kernel preamble. */
+                                   [kernel_signature]) */
-
-  /* The kernel signature comes first as it may allow us to parallelize
-   * the kernel data fetch and RSA public key operation.
-   */
-  uint8_t* kernel_signature;  /* Signature on the concatenation of
-                               * the kernel preamble and [kernel_data]. */
   uint8_t* kernel_data;  /* Actual kernel data. */
 
 } KernelImage;
@@ -74,8 +71,9 @@ typedef struct KernelImage {
 
 extern char* kVerifyKernelErrors[VERIFY_KERNEL_MAX];
 
-/* Returns the length of the verified boot kernel preamble. */
+/* Returns the length of the verified boot kernel preamble based on
-uint64_t GetKernelPreambleLen(void);
+ * kernel signing algorithm [algorithm]. */
+uint64_t GetKernelPreambleLen(int algorithm);
 
 /* Returns the length of the Kernel Verified Boot header excluding
  * [kernel_data].
@@ -111,16 +109,15 @@ int VerifyKernelPreamble(RSAPublicKey* kernel_sign_key,
                          int algorithm,
                          uint64_t* kernel_len);
 
-/* Checks the signature on the kernel data at location [kernel_data_start].
+/* Checks [kernel_signature] on the kernel data at location [kernel_data]. The
- * The length of the actual kernel data is kernel_len and it is assumed to
+ * signature is assumed to be generated using algorithm [algorithm].
- * be prepended with the signature whose size depends on the signature_algorithm
+ * The length of the kernel data is [kernel_len].
- * [algorithm].
  *
  * Return 0 on success, error code on failure.
  */
 int VerifyKernelData(RSAPublicKey* kernel_sign_key,
-                     const uint8_t* kernel_config_start,
+                     const uint8_t* kernel_signature,
-                     const uint8_t* kernel_data_start,
+                     const uint8_t* kernel_data,
                      uint64_t kernel_len,
                      int algorithm);
 
@@ -128,8 +125,7 @@ int VerifyKernelData(RSAPublicKey* kernel_sign_key,
  * using the firmware public key [firmware_key_blob]. If [dev_mode] is 1
  * (active), then key header verification is skipped.
  *
- * Fills in a pointer to preamble blob within [kernel_header_blob] in
+ * Fills in a pointer to expected kernel data signature
- * [preamble_blob], pointer to expected kernel data signature
  * within [kernel_header_blob] in [expected_kernel_signature].
  *
  * The signing key to use for kernel data verification is returned in
@@ -142,7 +138,6 @@ int VerifyKernelData(RSAPublicKey* kernel_sign_key,
 int VerifyKernelHeader(const uint8_t* firmware_key_blob,
                        const uint8_t* kernel_header_blob,
                        const int dev_mode,
-                       const uint8_t** preamble_blob,
                        const uint8_t** expected_kernel_signature,
                        RSAPublicKey** kernel_sign_key,
                        int* kernel_sign_algorithm,

vboot_firmware/lib/kernel_image_fw.c

@@ -26,12 +26,13 @@ char* kVerifyKernelErrors[VERIFY_KERNEL_MAX] = {
   "Wrong Kernel Magic.",
 };
 
-inline uint64_t GetKernelPreambleLen(void) {
+inline uint64_t GetKernelPreambleLen(int algorithm) {
   return (FIELD_LEN(kernel_version) +
           FIELD_LEN(kernel_len) +
           FIELD_LEN(bootloader_offset) +
           FIELD_LEN(bootloader_size) +
-          FIELD_LEN(padded_header_size));
+          FIELD_LEN(padded_header_size) +
+          siglen_map[algorithm]);
 }
 
 uint64_t GetVblockHeaderSize(const uint8_t* vkernel_blob) {
@@ -66,9 +67,8 @@ uint64_t GetVblockHeaderSize(const uint8_t* vkernel_blob) {
           RSAProcessedKeySize(kernel_sign_algorithm) +  /* kernel_sign_key */
           FIELD_LEN(header_checksum) +
           siglen_map[firmware_sign_algorithm] +  /* kernel_key_signature */
-          GetKernelPreambleLen() +
+          GetKernelPreambleLen(kernel_sign_algorithm) +
-          siglen_map[kernel_sign_algorithm] +  /* preamble_signature */
+          siglen_map[kernel_sign_algorithm]);  /* preamble_signature */
-          siglen_map[kernel_sign_algorithm]);  /* kernel_signature */
   return len;
 }
 
@@ -157,7 +157,7 @@ int VerifyKernelPreamble(RSAPublicKey* kernel_sign_key,
                          const uint8_t* preamble_blob,
                          int algorithm,
                          uint64_t* kernel_len) {
-  int preamble_len = GetKernelPreambleLen();
+  int preamble_len = GetKernelPreambleLen(algorithm);
   if (!RSAVerifyBinary_f(NULL, kernel_sign_key, /* Key to use */
                          preamble_blob,  /* Data to verify */
                          preamble_len, /* Length of data */
@@ -171,43 +171,23 @@ int VerifyKernelPreamble(RSAPublicKey* kernel_sign_key,
 }
 
 int VerifyKernelData(RSAPublicKey* kernel_sign_key,
-                     const uint8_t* preamble_blob,
+                     const uint8_t* kernel_signature,
                      const uint8_t* kernel_data,
                      uint64_t kernel_len,
                      int algorithm) {
-  int signature_len = siglen_map[algorithm];
-  const uint8_t* kernel_signature = NULL;
-  uint8_t* digest = NULL;
-  DigestContext ctx;
 
-  kernel_signature = preamble_blob + (GetKernelPreambleLen() +
+  if (!RSAVerifyBinary_f(NULL, kernel_sign_key, /* Key to use */
-                                      signature_len);
+                         kernel_data,  /* Data to verify */
-
+                         kernel_len, /* Length of data */
-  /* Since the kernel signature is computed over the kernel preamble
+                         kernel_signature,  /* Expected Signature */
-   * and kernel image data, which does not form a contiguous
+                         algorithm))
-   * region of memory, we calculate the message digest ourselves. */
-  DigestInit(&ctx, algorithm);
-  DigestUpdate(&ctx,
-               preamble_blob,
-               GetKernelPreambleLen());
-  DigestUpdate(&ctx, kernel_data, kernel_len);
-  digest = DigestFinal(&ctx);
-  if (!RSAVerifyBinaryWithDigest_f(
-          NULL, kernel_sign_key,  /* Key to use. */
-          digest, /* Digest of the data to verify. */
-          kernel_signature,  /* Expected Signature */
-          algorithm)) {
-    Free(digest);
     return VERIFY_KERNEL_SIGNATURE_FAILED;
-  }
-  Free(digest);
   return 0;
 }
 
 int VerifyKernelHeader(const uint8_t* firmware_key_blob,
                        const uint8_t* kernel_header_blob,
                        const int dev_mode,
-                       const uint8_t** preamble_blob,
                        const uint8_t** expected_kernel_signature,
                        RSAPublicKey** kernel_sign_key,
                        int* kernel_sign_algorithm,
@@ -216,8 +196,9 @@ int VerifyKernelHeader(const uint8_t* firmware_key_blob,
   int firmware_sign_algorithm;  /* Firmware signing key algorithm. */
   int kernel_sign_key_len, kernel_key_signature_len, kernel_signature_len,
       header_len;
-  const uint8_t* header_ptr;  /* Pointer to header. */
+  const uint8_t* header_ptr = NULL;  /* Pointer to key header. */
-  const uint8_t* kernel_sign_key_ptr;  /* Pointer to signing key. */
+  const uint8_t* preamble_ptr = NULL;  /* Pointer to start of preamble. */
+  const uint8_t* kernel_sign_key_ptr = NULL;  /* Pointer to signing key. */
 
   /* Note: All the offset calculations are based on struct FirmwareImage which
    * is defined in include/firmware_image.h. */
@@ -250,16 +231,17 @@ int VerifyKernelHeader(const uint8_t* firmware_key_blob,
   kernel_key_signature_len = siglen_map[firmware_sign_algorithm];
 
   /* Only continue if preamble verification succeeds. */
-  *preamble_blob = (header_ptr + header_len + kernel_key_signature_len);
+  preamble_ptr = (header_ptr + header_len + kernel_key_signature_len);
-  if ((error_code = VerifyKernelPreamble(*kernel_sign_key, *preamble_blob,
+  if ((error_code = VerifyKernelPreamble(*kernel_sign_key, preamble_ptr,
                                          *kernel_sign_algorithm,
                                          kernel_len))) {
     RSAPublicKeyFree(*kernel_sign_key);
     return error_code;  /* AKA jump to recovery. */
   }
-  *expected_kernel_signature = (*preamble_blob +
+  *expected_kernel_signature = (preamble_ptr +
-                                GetKernelPreambleLen() +
+                                GetKernelPreambleLen(*kernel_sign_algorithm) -
-                                kernel_signature_len);  /* Skip preamble. */
+                                kernel_signature_len);  /* Skip beginning of
+                                                         * preamble. */
   return 0;
 }
 
@@ -277,13 +259,16 @@ int VerifyKernel(const uint8_t* firmware_key_blob,
   const uint8_t* kernel_sign_key_ptr;  /* Pointer to signing key. */
   const uint8_t* preamble_ptr;  /* Pointer to kernel preamble block. */
   const uint8_t* kernel_ptr;  /* Pointer to kernel signature/data. */
+  const uint8_t* kernel_signature;
 
   /* Note: All the offset calculations are based on struct FirmwareImage which
    * is defined in include/firmware_image.h. */
 
   /* Compare magic bytes. */
-  if (SafeMemcmp(kernel_blob, KERNEL_MAGIC, KERNEL_MAGIC_SIZE))
+  if (SafeMemcmp(kernel_blob, KERNEL_MAGIC, KERNEL_MAGIC_SIZE)) {
+    debug("VerifyKernel: Kernel magic bytes not found.\n");
     return VERIFY_KERNEL_WRONG_MAGIC;
+  }
   header_ptr = kernel_blob + KERNEL_MAGIC_SIZE;
 
   /* Only continue if header verification succeeds. */
@@ -311,18 +296,21 @@ int VerifyKernel(const uint8_t* firmware_key_blob,
   if ((error_code = VerifyKernelPreamble(kernel_sign_key, preamble_ptr,
                                          kernel_sign_algorithm,
                                          &kernel_len))) {
+    debug("VerifyKernel: Kernel preamble verification failed.\n");
     RSAPublicKeyFree(kernel_sign_key);
     return error_code;  /* AKA jump to recovery. */
   }
   /* Only continue if kernel data verification succeeds. */
   kernel_ptr = (preamble_ptr +
-                GetKernelPreambleLen() +
+                GetKernelPreambleLen(kernel_sign_algorithm) +
-                2 * kernel_signature_len);  /* preamble and kernel signature. */
+                kernel_signature_len);  /* preamble signature. */
+  kernel_signature = kernel_ptr - 2 * kernel_signature_len; /* end of kernel
+                                                             * preamble. */
 
   if ((error_code = VerifyKernelData(kernel_sign_key,  /* Verification key */
-                                     preamble_ptr,  /* Start of preamble */
+                                     kernel_signature,  /* kernel signature */
-                                     kernel_ptr,  /* Start of kernel image */
+                                     kernel_ptr,  /* Start of kernel data */
-                                     kernel_len,  /* Length of kernel image. */
+                                     kernel_len,  /* Length of kernel data. */
                                      kernel_sign_algorithm))) {
     RSAPublicKeyFree(kernel_sign_key);
     return error_code;  /* AKA jump to recovery. */

vboot_firmware/linktest/main.c

@@ -21,7 +21,7 @@ int main(void)
   VerifyKernelKeyHeader(0, 0, 0, 0, 0, 0);
   VerifyKernelPreamble(0, 0, 0, 0);
   VerifyKernelData(0, 0, 0, 0, 0);
-  VerifyKernelHeader(0, 0, 0, 0, 0, 0, 0, 0);
+  VerifyKernelHeader(0, 0, 0, 0, 0, 0, 0);
   VerifyKernel(0, 0, 0);
   GetLogicalKernelVersion(0);
   VerifyKernelDriver_f(0, 0, 0, 0);

vkernel/kernel_image.c

@@ -222,8 +222,9 @@ uint8_t* GetKernelPreambleBlob(const KernelImage* image) {
   uint8_t* preamble_blob = NULL;
   MemcpyState st;
 
-  preamble_blob = (uint8_t*) Malloc(GetKernelPreambleLen());
+  preamble_blob = (uint8_t*) Malloc(
-  st.remaining_len = GetKernelPreambleLen();
+      GetKernelPreambleLen(image->kernel_sign_algorithm));
+  st.remaining_len = GetKernelPreambleLen(image->kernel_sign_algorithm);
   st.remaining_buf = preamble_blob;
   st.overrun = 0;
 
@@ -233,6 +234,8 @@ uint8_t* GetKernelPreambleBlob(const KernelImage* image) {
   StatefulMemcpy_r(&st, &image->bootloader_size, FIELD_LEN(bootloader_size));
   StatefulMemcpy_r(&st, &image->padded_header_size,
                    FIELD_LEN(padded_header_size));
+  StatefulMemcpy_r(&st, image->kernel_signature,
+                   siglen_map[image->kernel_sign_algorithm]);
 
   if (st.overrun || st.remaining_len != 0) {  /* Overrun or Underrun. */
     Free(preamble_blob);
@@ -255,8 +258,8 @@ uint8_t* GetKernelBlob(const KernelImage* image, uint64_t* blob_len) {
   *blob_len = (FIELD_LEN(magic) +
                GetKernelHeaderLen(image) +
                kernel_key_signature_len +
-               GetKernelPreambleLen() +
+               GetKernelPreambleLen(image->kernel_sign_algorithm) +
-               2 * kernel_signature_len +
+               kernel_signature_len +
                image->kernel_len);
   kernel_blob = (uint8_t*) Malloc(*blob_len);
   st.remaining_len = *blob_len;
@@ -276,13 +279,14 @@ uint8_t* GetKernelBlob(const KernelImage* image, uint64_t* blob_len) {
   StatefulMemcpy_r(&st, &image->bootloader_size, FIELD_LEN(bootloader_size));
   StatefulMemcpy_r(&st, &image->padded_header_size,
                    FIELD_LEN(padded_header_size));
-  StatefulMemcpy_r(&st, image->preamble_signature, kernel_signature_len);
   StatefulMemcpy_r(&st, image->kernel_signature, kernel_signature_len);
+  StatefulMemcpy_r(&st, image->preamble_signature, kernel_signature_len);
   StatefulMemcpy_r(&st, image->kernel_data, image->kernel_len);
 
   Free(header_blob);
 
   if (st.overrun || st.remaining_len != 0) {  /* Underrun or Overrun. */
+    debug("GetKernelBlob() failed.\n");
     Free(kernel_blob);
     return NULL;
   }
@@ -371,7 +375,6 @@ int VerifyKernelImage(const RSAPublicKey* firmware_key,
   int kernel_signature_size;
   int error_code = 0;
   DigestContext ctx;
-  DigestContext kernel_ctx;
   if (!image)
     return VERIFY_KERNEL_INVALID_IMAGE;
 
@@ -433,6 +436,8 @@ int VerifyKernelImage(const RSAPublicKey* firmware_key,
                FIELD_LEN(bootloader_size));
   DigestUpdate(&ctx, (uint8_t*) &image->padded_header_size,
                FIELD_LEN(padded_header_size));
+  DigestUpdate(&ctx, (uint8_t*) image->kernel_signature,
+               kernel_signature_size);
   preamble_digest = DigestFinal(&ctx);
   if (!RSAVerify(kernel_sign_key, image->preamble_signature,
                  kernel_signature_size, image->kernel_sign_algorithm,
@@ -442,21 +447,14 @@ int VerifyKernelImage(const RSAPublicKey* firmware_key,
   }
 
   /* Verify kernel signature - kernel signature is computed on the contents
-     of kernel version + kernel options + kernel_data. */
+   * of kernel_data.
-  DigestInit(&kernel_ctx, image->kernel_sign_algorithm);
+   * Association between the kernel_data and preamble is maintained by making
-  DigestUpdate(&kernel_ctx, (uint8_t*) &image->kernel_version,
+   * the kernel signature a part of the preamble and verifying it as part
-               FIELD_LEN(kernel_version));
+   * of preamble signature checking. */
-  DigestUpdate(&kernel_ctx, (uint8_t*) &image->kernel_len,
+
-               FIELD_LEN(kernel_len));
+  kernel_digest = DigestBuf(image->kernel_data,
-  DigestUpdate(&kernel_ctx, (uint8_t*) &image->bootloader_offset,
+                            image->kernel_len,
-               FIELD_LEN(bootloader_offset));
+                            image->kernel_sign_algorithm);
-  DigestUpdate(&kernel_ctx, (uint8_t*) &image->bootloader_size,
-               FIELD_LEN(bootloader_size));
-  DigestUpdate(&kernel_ctx, (uint8_t*) &image->padded_header_size,
-               FIELD_LEN(padded_header_size));
-  DigestUpdate(&kernel_ctx, (uint8_t*) image->kernel_data,
-               image->kernel_len);
-  kernel_digest = DigestFinal(&kernel_ctx);
   if (!RSAVerify(kernel_sign_key, image->kernel_signature,
                  kernel_signature_size, image->kernel_sign_algorithm,
                  kernel_digest)) {
@@ -505,33 +503,17 @@ int AddKernelSignature(KernelImage* image,
   uint8_t* preamble_signature = NULL;
   uint8_t* kernel_signature = NULL;
   uint8_t* kernel_buf;
-  int signature_len = siglen_map[image->kernel_sign_algorithm];
+  int algorithm = image->kernel_sign_algorithm;
+  int signature_len = siglen_map[algorithm];
 
-  preamble_blob = GetKernelPreambleBlob(image);
+  /* Kernel signature must be calculated first as its used for computing the
-  if (!(preamble_signature = SignatureBuf(preamble_blob,
+   * preamble signature. */
-                                          GetKernelPreambleLen(),
+  kernel_buf = (uint8_t*) Malloc(image->kernel_len);
-                                          kernel_signing_key_file,
+  Memcpy(kernel_buf, image->kernel_data, image->kernel_len);
-                                          image->kernel_sign_algorithm))) {
-    debug("Could not compute signature on the kernel preamble.\n");
-    Free(preamble_blob);
-    return 0;
-  }
-
-  image->preamble_signature = (uint8_t*) Malloc(signature_len);
-  Memcpy(image->preamble_signature, preamble_signature, signature_len);
-  Free(preamble_signature);
-  /* Kernel signature muse be calculated on the kernel version, options and
-   * kernel data to avoid splicing attacks. */
-  kernel_buf = (uint8_t*) Malloc(GetKernelPreambleLen() +
-                                 image->kernel_len);
-  Memcpy(kernel_buf, preamble_blob, GetKernelPreambleLen());
-  Memcpy(kernel_buf + GetKernelPreambleLen(), image->kernel_data,
-         image->kernel_len);
   if (!(kernel_signature = SignatureBuf(kernel_buf,
-                                        GetKernelPreambleLen() +
                                         image->kernel_len,
                                         kernel_signing_key_file,
-                                        image->kernel_sign_algorithm))) {
+                                        algorithm))) {
     Free(preamble_blob);
     Free(kernel_buf);
     debug("Could not compute signature on the kernel.\n");
@@ -539,9 +521,24 @@ int AddKernelSignature(KernelImage* image,
   }
   image->kernel_signature = (uint8_t*) Malloc(signature_len);
   Memcpy(image->kernel_signature, kernel_signature, signature_len);
+
+
+  preamble_blob = GetKernelPreambleBlob(image);
+  if (!(preamble_signature = SignatureBuf(preamble_blob,
+                                          GetKernelPreambleLen(algorithm),
+                                          kernel_signing_key_file,
+                                          algorithm))) {
+    debug("Could not compute signature on the kernel preamble.\n");
+    Free(preamble_blob);
+    return 0;
+  }
+  image->preamble_signature = (uint8_t*) Malloc(signature_len);
+  Memcpy(image->preamble_signature, preamble_signature, signature_len);
+
+  Free(preamble_signature);
+  Free(preamble_blob);
   Free(kernel_signature);
   Free(kernel_buf);
-  Free(preamble_blob);
   return 1;
 }