fixed VerifyKernelHeader
removed extra debugging
fixed printf() format specifiers

Review URL: http://codereview.chromium.org/2561001
diff --git a/utility/Makefile b/utility/Makefile
index b7e9ded..23a6e17 100644
--- a/utility/Makefile
+++ b/utility/Makefile
@@ -26,10 +26,9 @@
 		firmware_utility \
 		gbb_utility \
 		kernel_utility \
+		load_kernel_test \
 		signature_digest_utility \
 		verify_data
-# Note: load_kernel_test is not part of TARGET_BINS, since it's a
-# temporary test.
 
 all: $(TARGET_BINS) subdirs
 
diff --git a/utility/load_kernel_test.c b/utility/load_kernel_test.c
index 9de9e59..da3532c 100644
--- a/utility/load_kernel_test.c
+++ b/utility/load_kernel_test.c
@@ -7,6 +7,7 @@
  * RSA verification implementation.
  */
 
+#include <inttypes.h>  /* For PRIu64 macro */
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -33,11 +34,11 @@
 
 /* Boot device stub implementations to read from the image file */
 int BootDeviceReadLBA(uint64_t lba_start, uint64_t lba_count, void *buffer) {
-  printf("Read(%llu, %llu)\n", lba_start, lba_count);
+  printf("Read(%" PRIu64 ", %" PRIu64 ")\n", lba_start, lba_count);
 
   if (lba_start > lkp.ending_lba ||
       lba_start + lba_count - 1 > lkp.ending_lba) {
-    fprintf(stderr, "Read overrun: %llu + %llu > %llu\n",
+    fprintf(stderr, "Read overrun: %" PRIu64 " + %" PRIu64 " > %" PRIu64 "\n",
             lba_start, lba_count, lkp.ending_lba);
     return 1;
   }
@@ -53,11 +54,11 @@
 
 int BootDeviceWriteLBA(uint64_t lba_start, uint64_t lba_count,
                        const void *buffer) {
-  printf("Write(%llu, %llu)\n", lba_start, lba_count);
+  printf("Write(%" PRIu64 ", %" PRIu64 ")\n", lba_start, lba_count);
 
   if (lba_start > lkp.ending_lba ||
       lba_start + lba_count - 1 > lkp.ending_lba) {
-    fprintf(stderr, "Read overrun: %llu + %llu > %llu\n",
+    fprintf(stderr, "Read overrun: %" PRIu64 " + %" PRIu64 " > %" PRIu64 "\n",
             lba_start, lba_count, lkp.ending_lba);
     return 1;
   }
@@ -77,24 +78,45 @@
 /* Main routine */
 int main(int argc, char* argv[]) {
 
-  const char *image_name;
+  const char* image_name;
+  const char* keyfile_name;
   int rv;
 
   Memset(&lkp, 0, sizeof(LoadKernelParams));
   lkp.bytes_per_lba = LBA_BYTES;
 
   /* Read command line parameters */
-  if (2 > argc) {
-    fprintf(stderr, "usage: %s <drive_image>\n", argv[0]);
+  if (3 > argc) {
+    fprintf(stderr, "usage: %s <drive_image> <sign_key>\n", argv[0]);
     return 1;
   }
   image_name = argv[1];
-  printf("Reading from image: %s\n", image_name);
+  keyfile_name = argv[2];
 
-  /* TODO: Read header signing key blob */
-  lkp.header_sign_key_blob = NULL;
+  /* Read header signing key blob */
+  {
+    FILE* f;
+    int key_size;
+    printf("Reading key from: %s\n", keyfile_name);
+    f = fopen(keyfile_name, "rb");
+    if (!f) {
+      fprintf(stderr, "Unable to open key file %s\n", keyfile_name);
+      return 1;
+    }
+    fseek(f, 0, SEEK_END);
+    key_size = ftell(f);
+    rewind(f);
+    lkp.header_sign_key_blob = Malloc(key_size);
+    printf("Reading %d bytes of key\n", key_size);
+    if (fread(lkp.header_sign_key_blob, key_size, 1, f) != 1) {
+      fprintf(stderr, "Unable to read key data\n");
+      return 1;
+    }
+    fclose(f);
+  }
 
   /* Get image size */
+  printf("Reading from image: %s\n", image_name);
   image_file = fopen(image_name, "rb");
   if (!image_file) {
     fprintf(stderr, "Unable to open image file %s\n", image_name);
@@ -103,7 +125,7 @@
   fseek(image_file, 0, SEEK_END);
   lkp.ending_lba = (ftell(image_file) / LBA_BYTES) - 1;
   rewind(image_file);
-  printf("Ending LBA: %llu\n", lkp.ending_lba);
+  printf("Ending LBA: %" PRIu64 "\n", lkp.ending_lba);
 
   /* Allocate a buffer for the kernel */
   lkp.kernel_buffer = Malloc(KERNEL_BUFFER_SIZE);
@@ -119,6 +141,12 @@
   rv = LoadKernel(&lkp);
   printf("LoadKernel() returned %d\n", rv);
 
+  if (LOAD_KERNEL_SUCCESS == rv) {
+    printf("Partition number:   %" PRIu64 "\n", lkp.partition_number);
+    printf("Bootloader address: %" PRIu64 "\n", lkp.bootloader_address);
+    printf("Bootloader size:    %" PRIu64 "\n", lkp.bootloader_size);
+  }
+
   fclose(image_file);
   Free(lkp.kernel_buffer);
   return 0;
diff --git a/vboot_firmware/include/kernel_image_fw.h b/vboot_firmware/include/kernel_image_fw.h
index 4cd53d2..60b9f99 100644
--- a/vboot_firmware/include/kernel_image_fw.h
+++ b/vboot_firmware/include/kernel_image_fw.h
@@ -132,8 +132,9 @@
  * kernel_header_blob.  image->kernel_data is set to NULL, since it's not
  * part of the header and preamble data itself.
  *
- * The signing key to use for kernel data verification is returned in
- * [kernel_sign_key], This must be free-d explicitly by the caller after use.
+ * On success, the signing key to use for kernel data verification is
+ * returned in [kernel_sign_key], This must be free-d explicitly by
+ * the caller after use.  On failure, the signing key is set to NULL.
  *
  * Returns 0 on success, error code on failure.
  */
diff --git a/vboot_firmware/include/load_kernel_fw.h b/vboot_firmware/include/load_kernel_fw.h
index f34d531..f4c512a 100644
--- a/vboot_firmware/include/load_kernel_fw.h
+++ b/vboot_firmware/include/load_kernel_fw.h
@@ -38,7 +38,7 @@
    * LOAD_KERNEL_SUCCESS */
   uint64_t partition_number;    /* Partition number to boot on current device
                                  * (1...M) */
-  void *bootloader_start;       /* Start of bootloader image */
+  uint64_t bootloader_address;  /* Address of bootloader image in RAM */
   uint64_t bootloader_size;     /* Size of bootloader image in bytes */
 } LoadKernelParams;
 
diff --git a/vboot_firmware/lib/kernel_image_fw.c b/vboot_firmware/lib/kernel_image_fw.c
index 6d41e5e..afa01dd 100644
--- a/vboot_firmware/lib/kernel_image_fw.c
+++ b/vboot_firmware/lib/kernel_image_fw.c
@@ -250,15 +250,16 @@
   kernel_signature_len = siglen_map[kernel_sign_algorithm];
   kernel_key_signature_len = siglen_map[firmware_sign_algorithm];
   image->kernel_key_signature = (uint8_t*)st.remaining_buf;
-  StatefulSkip(&st, kernel_signature_len);
+  StatefulSkip(&st, kernel_key_signature_len);
 
   /* Only continue if preamble verification succeeds. */
   /* TODO: should pass the remaining len into VerifyKernelPreamble() */
   preamble_ptr = (const uint8_t*)st.remaining_buf;
   if ((error_code = VerifyKernelPreamble(*kernel_sign_key, preamble_ptr,
-                                         kernel_sign_algorithm,
+                                    kernel_sign_algorithm,
                                          &kernel_len))) {
     RSAPublicKeyFree(*kernel_sign_key);
+    *kernel_sign_key = NULL;
     return error_code;  /* AKA jump to recovery. */
   }
 
diff --git a/vboot_firmware/lib/load_kernel_fw.c b/vboot_firmware/lib/load_kernel_fw.c
index dec92d1..ba515a2 100644
--- a/vboot_firmware/lib/load_kernel_fw.c
+++ b/vboot_firmware/lib/load_kernel_fw.c
@@ -18,6 +18,7 @@
 
 // TODO: for testing
 #include <stdio.h>
+#include <inttypes.h>  /* For PRIu64 macro */
 #include "cgptlib_internal.h"
 
 /* TODO: Remove this terrible hack which fakes partition attributes
@@ -28,11 +29,10 @@
   GptEntry* e;
   int i;
   printf("Hacking partition attributes...\n");
-  printf("Note that GUIDs below have first 3 fields endian-swapped\n");
 
   for (i = 0, e = entries; i < 12; i++, e++) {
 
-    printf("%2d %08x %04x %04x %02x %02x %02x %02x %02x %02x %02x %02x\n",
+    printf("%2d %08x %04x %04x %02x %02x %02x %02x %02x %02x %02x %02x",
            i,
            e->type.u.Uuid.time_low,
            e->type.u.Uuid.time_mid,
@@ -46,6 +46,8 @@
            e->type.u.Uuid.node[4],
            e->type.u.Uuid.node[5]
            );
+    printf(" %8" PRIu64 " %8" PRIu64"\n", e->starting_lba,
+           e->ending_lba - e->starting_lba + 1);
     if (!IsKernelEntry(e))
       continue;
     printf("Hacking attributes for kernel partition %d\n", i);
@@ -144,6 +146,11 @@
   uint16_t lowest_kernel_version = 0xFFFF;
   KernelImage *kim = NULL;
 
+  /* Clear output params in case we fail */
+  params->partition_number = 0;
+  params->bootloader_address = 0;
+  params->bootloader_size = 0;
+
   /* Read current kernel key index from TPM.  Assumes TPM is already
    * initialized. */
   /* TODO: Is that a safe assumption?  Normally, SetupTPM() would be called
@@ -161,8 +168,6 @@
     if (0 != AllocAndReadGptData(&gpt))
       break;
 
-    fprintf(stderr, "RRS1\n");
-
     /* Initialize GPT library */
     if (GPT_SUCCESS != GptInit(&gpt))
       break;
@@ -180,15 +185,11 @@
     if (!kim)
       break;
 
-    fprintf(stderr, "RRS2\n");
-
     /* Loop over candidate kernel partitions */
     while (GPT_SUCCESS == GptNextKernelEntry(&gpt, &part_start, &part_size)) {
       RSAPublicKey *kernel_sign_key = NULL;
       int kernel_start, kernel_sectors;
 
-      fprintf(stderr, "RRS3\n");
-
       /* Found at least one kernel partition. */
       found_partition = 1;
 
@@ -198,8 +199,6 @@
       if (0 != BootDeviceReadLBA(part_start, kbuf_sectors, kbuf))
         continue;
 
-      fprintf(stderr, "RRS4\n");
-
       /* Verify the kernel header and preamble */
       if (VERIFY_KERNEL_SUCCESS != VerifyKernelHeader(
               params->header_sign_key_blob,
@@ -211,7 +210,17 @@
         continue;
       }
 
-      fprintf(stderr, "RRS5\n");
+      printf("Kernel header:\n");
+      printf("header version:     %d\n", kim->header_version);
+      printf("header len:         %d\n", kim->header_len);
+      printf("firmware sign alg:  %d\n", kim->firmware_sign_algorithm);
+      printf("kernel sign alg:    %d\n", kim->kernel_sign_algorithm);
+      printf("kernel key version: %d\n", kim->kernel_key_version);
+      printf("kernel version:     %d\n", kim->kernel_version);
+      printf("kernel len:         %" PRIu64 "\n", kim->kernel_len);
+      printf("bootloader addr:    %" PRIu64 "\n", kim->bootloader_offset);
+      printf("bootloader size:    %" PRIu64 "\n", kim->bootloader_size);
+      printf("padded header size: %" PRIu64 "\n", kim->padded_header_size);
 
       /* Check for rollback of key version */
       if (kim->kernel_key_version < tpm_kernel_key_version) {
@@ -270,8 +279,7 @@
       if (-1 == good_partition) {
         good_partition = gpt.current_kernel;
         params->partition_number = gpt.current_kernel;
-        params->bootloader_start = (uint8_t*)params->kernel_buffer +
-            kim->bootloader_offset;
+        params->bootloader_address = kim->bootloader_offset;
         params->bootloader_size = kim->bootloader_size;
 
         /* If the good partition's key version is the same as the tpm, then