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
diff --git a/tests/kernel_splicing_tests.c b/tests/kernel_splicing_tests.c
index 9335d93..241f109 100644
--- a/tests/kernel_splicing_tests.c
+++ b/tests/kernel_splicing_tests.c
@@ -65,12 +65,12 @@
Memcpy(image2->kernel_data, image1->kernel_data,
image2->kernel_len);
- TEST_EQ(VerifyKernelImage(firmware_key, image2, 0),
- VERIFY_KERNEL_SIGNATURE_FAILED,
- "KernelImage kernel_data Splicing");
+ TEST_NEQ(VerifyKernelImage(firmware_key, image2, 0),
+ VERIFY_KERNEL_SUCCESS,
+ "KernelImage kernel_data Splicing");
kernel_blob = GetKernelBlob(image2, &len);
- TEST_EQ(VerifyKernel(firmware_key_blob, kernel_blob, 0),
- VERIFY_KERNEL_SIGNATURE_FAILED,
+ TEST_NEQ(VerifyKernel(firmware_key_blob, kernel_blob, 0),
+ VERIFY_KERNEL_SUCCESS,
"Kernel Blob kernel_data Splicing");
}
diff --git a/tests/test_common.c b/tests/test_common.c
index 53c6fdf..5c39413 100644
--- a/tests/test_common.c
+++ b/tests/test_common.c
@@ -33,6 +33,18 @@
}
}
+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,
diff --git a/tests/test_common.h b/tests/test_common.h
index d73dc0e..c840415 100644
--- a/tests/test_common.h
+++ b/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,
diff --git a/vboot_firmware/include/kernel_image_fw.h b/vboot_firmware/include/kernel_image_fw.h
index 17458e6..03bcf85 100644
--- a/vboot_firmware/include/kernel_image_fw.h
+++ b/vboot_firmware/include/kernel_image_fw.h
@@ -49,15 +49,12 @@
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 the kernel preamble. */
-
- /* 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* preamble_signature; /* signature on preamble, (includes
+ [kernel_signature]) */
uint8_t* kernel_data; /* Actual kernel data. */
} KernelImage;
@@ -74,8 +71,9 @@
extern char* kVerifyKernelErrors[VERIFY_KERNEL_MAX];
-/* Returns the length of the verified boot kernel preamble. */
-uint64_t GetKernelPreambleLen(void);
+/* Returns the length of the verified boot kernel preamble based on
+ * 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 algorithm,
uint64_t* kernel_len);
-/* Checks the signature on the kernel data at location [kernel_data_start].
- * The length of the actual kernel data is kernel_len and it is assumed to
- * be prepended with the signature whose size depends on the signature_algorithm
- * [algorithm].
+/* Checks [kernel_signature] on the kernel data at location [kernel_data]. The
+ * signature is assumed to be generated using algorithm [algorithm].
+ * The length of the kernel data is [kernel_len].
*
* Return 0 on success, error code on failure.
*/
int VerifyKernelData(RSAPublicKey* kernel_sign_key,
- const uint8_t* kernel_config_start,
- const uint8_t* kernel_data_start,
+ const uint8_t* kernel_signature,
+ const uint8_t* kernel_data,
uint64_t kernel_len,
int algorithm);
@@ -128,8 +125,7 @@
* 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
- * [preamble_blob], pointer to expected kernel data signature
+ * Fills in a 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 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,
diff --git a/vboot_firmware/lib/kernel_image_fw.c b/vboot_firmware/lib/kernel_image_fw.c
index 5a1e978..5a6afcf 100644
--- a/vboot_firmware/lib/kernel_image_fw.c
+++ b/vboot_firmware/lib/kernel_image_fw.c
@@ -26,12 +26,13 @@
"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 @@
RSAProcessedKeySize(kernel_sign_algorithm) + /* kernel_sign_key */
FIELD_LEN(header_checksum) +
siglen_map[firmware_sign_algorithm] + /* kernel_key_signature */
- GetKernelPreambleLen() +
- siglen_map[kernel_sign_algorithm] + /* preamble_signature */
- siglen_map[kernel_sign_algorithm]); /* kernel_signature */
+ GetKernelPreambleLen(kernel_sign_algorithm) +
+ siglen_map[kernel_sign_algorithm]); /* preamble_signature */
return len;
}
@@ -157,7 +157,7 @@
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 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() +
- signature_len);
-
- /* Since the kernel signature is computed over the kernel preamble
- * and kernel image data, which does not form a contiguous
- * 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);
+ if (!RSAVerifyBinary_f(NULL, kernel_sign_key, /* Key to use */
+ kernel_data, /* Data to verify */
+ kernel_len, /* Length of data */
+ kernel_signature, /* Expected Signature */
+ algorithm))
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 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* kernel_sign_key_ptr; /* Pointer to signing key. */
+ const uint8_t* header_ptr = NULL; /* Pointer to key header. */
+ 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 @@
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);
- if ((error_code = VerifyKernelPreamble(*kernel_sign_key, *preamble_blob,
+ preamble_ptr = (header_ptr + header_len + kernel_key_signature_len);
+ 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 +
- GetKernelPreambleLen() +
- kernel_signature_len); /* Skip preamble. */
+ *expected_kernel_signature = (preamble_ptr +
+ GetKernelPreambleLen(*kernel_sign_algorithm) -
+ kernel_signature_len); /* Skip beginning of
+ * preamble. */
return 0;
}
@@ -277,13 +259,16 @@
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 @@
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() +
- 2 * kernel_signature_len); /* preamble and kernel signature. */
+ GetKernelPreambleLen(kernel_sign_algorithm) +
+ 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_ptr, /* Start of kernel image */
- kernel_len, /* Length of kernel image. */
+ kernel_signature, /* kernel signature */
+ kernel_ptr, /* Start of kernel data */
+ kernel_len, /* Length of kernel data. */
kernel_sign_algorithm))) {
RSAPublicKeyFree(kernel_sign_key);
return error_code; /* AKA jump to recovery. */
diff --git a/vboot_firmware/linktest/main.c b/vboot_firmware/linktest/main.c
index cf08e26..0897c1e 100644
--- a/vboot_firmware/linktest/main.c
+++ b/vboot_firmware/linktest/main.c
@@ -21,7 +21,7 @@
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);
diff --git a/vkernel/kernel_image.c b/vkernel/kernel_image.c
index fc71f35..5eeb784 100644
--- a/vkernel/kernel_image.c
+++ b/vkernel/kernel_image.c
@@ -222,8 +222,9 @@
uint8_t* preamble_blob = NULL;
MemcpyState st;
- preamble_blob = (uint8_t*) Malloc(GetKernelPreambleLen());
- st.remaining_len = GetKernelPreambleLen();
+ preamble_blob = (uint8_t*) Malloc(
+ 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 @@
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 @@
*blob_len = (FIELD_LEN(magic) +
GetKernelHeaderLen(image) +
kernel_key_signature_len +
- GetKernelPreambleLen() +
- 2 * kernel_signature_len +
+ GetKernelPreambleLen(image->kernel_sign_algorithm) +
+ kernel_signature_len +
image->kernel_len);
kernel_blob = (uint8_t*) Malloc(*blob_len);
st.remaining_len = *blob_len;
@@ -276,13 +279,14 @@
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 kernel_signature_size;
int error_code = 0;
DigestContext ctx;
- DigestContext kernel_ctx;
if (!image)
return VERIFY_KERNEL_INVALID_IMAGE;
@@ -433,6 +436,8 @@
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 @@
}
/* Verify kernel signature - kernel signature is computed on the contents
- of kernel version + kernel options + kernel_data. */
- DigestInit(&kernel_ctx, image->kernel_sign_algorithm);
- DigestUpdate(&kernel_ctx, (uint8_t*) &image->kernel_version,
- FIELD_LEN(kernel_version));
- DigestUpdate(&kernel_ctx, (uint8_t*) &image->kernel_len,
- FIELD_LEN(kernel_len));
- DigestUpdate(&kernel_ctx, (uint8_t*) &image->bootloader_offset,
- FIELD_LEN(bootloader_offset));
- 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);
+ * of kernel_data.
+ * Association between the kernel_data and preamble is maintained by making
+ * the kernel signature a part of the preamble and verifying it as part
+ * of preamble signature checking. */
+
+ kernel_digest = DigestBuf(image->kernel_data,
+ image->kernel_len,
+ image->kernel_sign_algorithm);
if (!RSAVerify(kernel_sign_key, image->kernel_signature,
kernel_signature_size, image->kernel_sign_algorithm,
kernel_digest)) {
@@ -505,33 +503,17 @@
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);
- if (!(preamble_signature = SignatureBuf(preamble_blob,
- GetKernelPreambleLen(),
- kernel_signing_key_file,
- 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);
+ /* Kernel signature must be calculated first as its used for computing the
+ * preamble signature. */
+ kernel_buf = (uint8_t*) Malloc(image->kernel_len);
+ Memcpy(kernel_buf, 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 @@
}
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;
}