diff --git a/firmware/lib/include/vboot_common.h b/firmware/lib/include/vboot_common.h index 74d85800b8..5951286ce7 100644 --- a/firmware/lib/include/vboot_common.h +++ b/firmware/lib/include/vboot_common.h @@ -84,11 +84,11 @@ int VerifyDigest(const uint8_t* digest, const VbSignature *sig, /* Checks the sanity of a key block of size [size] bytes, using public - * key [key]. If [key]==NULL, uses only the block checksum to verify - * the key block. Header fields are also checked for sanity. Does not - * verify key index or key block flags. */ + * key [key]. If hash_only is non-zero, uses only the block checksum + * to verify the key block. Header fields are also checked for + * sanity. Does not verify key index or key block flags. */ int KeyBlockVerify(const VbKeyBlockHeader* block, uint64_t size, - const VbPublicKey *key); + const VbPublicKey *key, int hash_only); /* Checks the sanity of a firmware preamble of size [size] bytes, diff --git a/firmware/lib/vboot_common.c b/firmware/lib/vboot_common.c index 2635fe64a1..61510d1e31 100644 --- a/firmware/lib/vboot_common.c +++ b/firmware/lib/vboot_common.c @@ -56,12 +56,19 @@ int VerifyMemberInside(const void* parent, uint64_t parent_size, if (end > parent_size) return 1; + if (UINT64_MAX - end < member_size) + return 1; /* Detect wraparound in integer math */ if (end + member_size > parent_size) return 1; + if (UINT64_MAX - end < member_data_offset) + return 1; end += member_data_offset; if (end > parent_size) return 1; + + if (UINT64_MAX - end < member_data_size) + return 1; if (end + member_data_size > parent_size) return 1; @@ -163,7 +170,7 @@ int VerifyDigest(const uint8_t* digest, const VbSignature *sig, int KeyBlockVerify(const VbKeyBlockHeader* block, uint64_t size, - const VbPublicKey *key) { + const VbPublicKey *key, int hash_only) { const VbSignature* sig; @@ -180,13 +187,43 @@ int KeyBlockVerify(const VbKeyBlockHeader* block, uint64_t size, VBDEBUG(("Not enough data for key block.\n")); return VBOOT_KEY_BLOCK_INVALID; } + if (!hash_only && !key) { + VBDEBUG(("Missing required public key.\n")); + return VBOOT_PUBLIC_KEY_INVALID; + } - /* Check signature or hash, depending on whether we provide a key. Note that + /* Check signature or hash, depending on the hash_only parameter. Note that * we don't require a key even if the keyblock has a signature, because the * caller may not care if the keyblock itself is signed (for example, booting * a Google-signed kernel in developer mode). */ - if (key) { + if (hash_only) { + /* Check hash */ + uint8_t* header_checksum = NULL; + int rv; + + sig = &block->key_block_checksum; + + if (VerifySignatureInside(block, block->key_block_size, sig)) { + VBDEBUG(("Key block hash off end of block\n")); + return VBOOT_KEY_BLOCK_INVALID; + } + if (sig->sig_size != SHA512_DIGEST_SIZE) { + VBDEBUG(("Wrong hash size for key block.\n")); + return VBOOT_KEY_BLOCK_INVALID; + } + + VBDEBUG(("Checking key block hash only...\n")); + header_checksum = DigestBuf((const uint8_t*)block, sig->data_size, + SHA512_DIGEST_ALGORITHM); + rv = SafeMemcmp(header_checksum, GetSignatureDataC(sig), + SHA512_DIGEST_SIZE); + Free(header_checksum); + if (rv) { + VBDEBUG(("Invalid key block hash.\n")); + return VBOOT_KEY_BLOCK_HASH; + } + } else { /* Check signature */ RSAPublicKey* rsa; int rv; @@ -216,32 +253,6 @@ int KeyBlockVerify(const VbKeyBlockHeader* block, uint64_t size, VBDEBUG(("Invalid key block signature.\n")); return VBOOT_KEY_BLOCK_SIGNATURE; } - } else { - /* Check hash */ - uint8_t* header_checksum = NULL; - int rv; - - sig = &block->key_block_checksum; - - if (VerifySignatureInside(block, block->key_block_size, sig)) { - VBDEBUG(("Key block hash off end of block\n")); - return VBOOT_KEY_BLOCK_INVALID; - } - if (sig->sig_size != SHA512_DIGEST_SIZE) { - VBDEBUG(("Wrong hash size for key block.\n")); - return VBOOT_KEY_BLOCK_INVALID; - } - - VBDEBUG(("Checking key block hash only...\n")); - header_checksum = DigestBuf((const uint8_t*)block, sig->data_size, - SHA512_DIGEST_ALGORITHM); - rv = SafeMemcmp(header_checksum, GetSignatureDataC(sig), - SHA512_DIGEST_SIZE); - Free(header_checksum); - if (rv) { - VBDEBUG(("Invalid key block hash.\n")); - return VBOOT_KEY_BLOCK_HASH; - } } /* Verify we signed enough data */ diff --git a/firmware/lib/vboot_firmware.c b/firmware/lib/vboot_firmware.c index f76e3452cf..ac162467ce 100644 --- a/firmware/lib/vboot_firmware.c +++ b/firmware/lib/vboot_firmware.c @@ -91,7 +91,7 @@ int LoadFirmware(LoadFirmwareParams* params) { key_block = (VbKeyBlockHeader*)params->verification_block_1; vblock_size = params->verification_size_1; } - if ((0 != KeyBlockVerify(key_block, vblock_size, root_key))) { + if ((0 != KeyBlockVerify(key_block, vblock_size, root_key, 0))) { VBDEBUG(("Key block verification failed.\n")); continue; } diff --git a/firmware/lib/vboot_kernel.c b/firmware/lib/vboot_kernel.c index 4b2a030be3..2402a090ea 100644 --- a/firmware/lib/vboot_kernel.c +++ b/firmware/lib/vboot_kernel.c @@ -168,10 +168,6 @@ int LoadKernel(LoadKernelParams* params) { return (status == TPM_E_MUST_REBOOT ? LOAD_KERNEL_REBOOT : LOAD_KERNEL_RECOVERY); } - } else if (is_dev && !is_rec) { - /* In developer mode, we ignore the kernel subkey, and just use - * the SHA-512 hash to verify the key block. */ - kernel_subkey = NULL; } do { @@ -215,9 +211,11 @@ int LoadKernel(LoadKernelParams* params) { if (0 != BootDeviceReadLBA(part_start, kbuf_sectors, kbuf)) continue; - /* Verify the key block */ + /* Verify the key block. In developer mode, we ignore the key + * and use only the SHA-512 hash to verify the key block. */ key_block = (VbKeyBlockHeader*)kbuf; - if ((0 != KeyBlockVerify(key_block, KBUF_SIZE, kernel_subkey))) { + if ((0 != KeyBlockVerify(key_block, KBUF_SIZE, kernel_subkey, + is_dev && !is_rec))) { VBDEBUG(("Verifying key block failed.\n")); continue; } diff --git a/firmware/linktest/main.c b/firmware/linktest/main.c index 02576edf4b..17811f6f14 100644 --- a/firmware/linktest/main.c +++ b/firmware/linktest/main.c @@ -70,7 +70,7 @@ int main(void) PublicKeyToRSA(0); VerifyData(0, 0, 0, 0); VerifyDigest(0, 0, 0); - KeyBlockVerify(0, 0, 0); + KeyBlockVerify(0, 0, 0, 0); VerifyFirmwarePreamble(0, 0, 0); VerifyKernelPreamble(0, 0, 0); diff --git a/firmware/version.c b/firmware/version.c index 40e4e8ed3f..c50bc7d1cb 100644 --- a/firmware/version.c +++ b/firmware/version.c @@ -1 +1 @@ -char* VbootVersion = "VBOOv=da8eb0da"; +char* VbootVersion = "VBOOv=4fa4f8d2"; diff --git a/host/lib/host_keyblock.c b/host/lib/host_keyblock.c index 1c1fa1275b..2ad62b07d1 100644 --- a/host/lib/host_keyblock.c +++ b/host/lib/host_keyblock.c @@ -5,8 +5,6 @@ * Host functions for verified boot. */ -/* TODO: change all 'return 0', 'return 1' into meaningful return codes */ - #include "host_keyblock.h" #include "cryptolib.h" @@ -22,7 +20,8 @@ VbKeyBlockHeader* KeyBlockCreate(const VbPublicKey* data_key, VbKeyBlockHeader* h; uint64_t signed_size = sizeof(VbKeyBlockHeader) + data_key->key_size; uint64_t block_size = (signed_size + SHA512_DIGEST_SIZE + - (signing_key ? siglen_map[signing_key->algorithm] : 0)); + (signing_key ? + siglen_map[signing_key->algorithm] : 0)); uint8_t* data_key_dest; uint8_t* block_sig_dest; uint8_t* block_chk_dest; @@ -89,7 +88,7 @@ VbKeyBlockHeader* KeyBlockRead(const char* filename) { /* Verify the hash of the key block, since we can do that without * the public signing key. */ - if (0 != KeyBlockVerify(block, file_size, NULL)) { + if (0 != KeyBlockVerify(block, file_size, NULL, 1)) { VBDEBUG(("Invalid key block file: filename\n", filename)); Free(block); return NULL; diff --git a/tests/vboot_common3_tests.c b/tests/vboot_common3_tests.c index ba0e058c93..714d255e26 100644 --- a/tests/vboot_common3_tests.c +++ b/tests/vboot_common3_tests.c @@ -40,73 +40,77 @@ static void KeyBlockVerifyTest(const VbPublicKey* public_key, hsize = (unsigned) hdr->key_block_size; h = (VbKeyBlockHeader*)Malloc(hsize + 1024); - TEST_EQ(KeyBlockVerify(hdr, hsize, NULL), 0, + TEST_EQ(KeyBlockVerify(hdr, hsize, NULL, 1), 0, "KeyBlockVerify() ok using checksum"); - TEST_EQ(KeyBlockVerify(hdr, hsize, public_key), 0, + TEST_EQ(KeyBlockVerify(hdr, hsize, public_key, 0), 0, "KeyBlockVerify() ok using key"); + TEST_NEQ(KeyBlockVerify(hdr, hsize, NULL, 0), 0, + "KeyBlockVerify() missing key"); - TEST_NEQ(KeyBlockVerify(hdr, hsize - 1, NULL), 0, "KeyBlockVerify() size--"); - TEST_EQ(KeyBlockVerify(hdr, hsize + 1, NULL), 0, "KeyBlockVerify() size++"); + TEST_NEQ(KeyBlockVerify(hdr, hsize - 1, NULL, 1), 0, + "KeyBlockVerify() size--"); + TEST_EQ(KeyBlockVerify(hdr, hsize + 1, NULL, 1), 0, + "KeyBlockVerify() size++"); Memcpy(h, hdr, hsize); h->magic[0] &= 0x12; - TEST_NEQ(KeyBlockVerify(h, hsize, NULL), 0, "KeyBlockVerify() magic"); + TEST_NEQ(KeyBlockVerify(h, hsize, NULL, 1), 0, "KeyBlockVerify() magic"); /* Care about major version but not minor */ Memcpy(h, hdr, hsize); h->header_version_major++; ReChecksumKeyBlock(h); - TEST_NEQ(KeyBlockVerify(h, hsize, NULL), 0, "KeyBlockVerify() major++"); + TEST_NEQ(KeyBlockVerify(h, hsize, NULL, 1), 0, "KeyBlockVerify() major++"); Memcpy(h, hdr, hsize); h->header_version_major--; ReChecksumKeyBlock(h); - TEST_NEQ(KeyBlockVerify(h, hsize, NULL), 0, "KeyBlockVerify() major--"); + TEST_NEQ(KeyBlockVerify(h, hsize, NULL, 1), 0, "KeyBlockVerify() major--"); Memcpy(h, hdr, hsize); h->header_version_minor++; ReChecksumKeyBlock(h); - TEST_EQ(KeyBlockVerify(h, hsize, NULL), 0, "KeyBlockVerify() minor++"); + TEST_EQ(KeyBlockVerify(h, hsize, NULL, 1), 0, "KeyBlockVerify() minor++"); Memcpy(h, hdr, hsize); h->header_version_minor--; ReChecksumKeyBlock(h); - TEST_EQ(KeyBlockVerify(h, hsize, NULL), 0, "KeyBlockVerify() minor--"); + TEST_EQ(KeyBlockVerify(h, hsize, NULL, 1), 0, "KeyBlockVerify() minor--"); /* Check hash */ Memcpy(h, hdr, hsize); h->key_block_checksum.sig_offset = hsize; ReChecksumKeyBlock(h); - TEST_NEQ(KeyBlockVerify(h, hsize, NULL), 0, + TEST_NEQ(KeyBlockVerify(h, hsize, NULL, 1), 0, "KeyBlockVerify() checksum off end"); Memcpy(h, hdr, hsize); h->key_block_checksum.sig_size /= 2; ReChecksumKeyBlock(h); - TEST_NEQ(KeyBlockVerify(h, hsize, NULL), 0, + TEST_NEQ(KeyBlockVerify(h, hsize, NULL, 1), 0, "KeyBlockVerify() checksum too small"); Memcpy(h, hdr, hsize); GetPublicKeyData(&h->data_key)[0] ^= 0x34; - TEST_NEQ(KeyBlockVerify(h, hsize, NULL), 0, + TEST_NEQ(KeyBlockVerify(h, hsize, NULL, 1), 0, "KeyBlockVerify() checksum mismatch"); /* Check signature */ Memcpy(h, hdr, hsize); h->key_block_signature.sig_offset = hsize; ReChecksumKeyBlock(h); - TEST_NEQ(KeyBlockVerify(h, hsize, public_key), 0, + TEST_NEQ(KeyBlockVerify(h, hsize, public_key, 0), 0, "KeyBlockVerify() sig off end"); Memcpy(h, hdr, hsize); h->key_block_signature.sig_size--; ReChecksumKeyBlock(h); - TEST_NEQ(KeyBlockVerify(h, hsize, public_key), 0, + TEST_NEQ(KeyBlockVerify(h, hsize, public_key, 0), 0, "KeyBlockVerify() sig too small"); Memcpy(h, hdr, hsize); GetPublicKeyData(&h->data_key)[0] ^= 0x34; - TEST_NEQ(KeyBlockVerify(h, hsize, public_key), 0, + TEST_NEQ(KeyBlockVerify(h, hsize, public_key, 0), 0, "KeyBlockVerify() sig mismatch"); /* Check that we signed header and data key */ @@ -115,13 +119,13 @@ static void KeyBlockVerifyTest(const VbPublicKey* public_key, h->data_key.key_offset = 0; h->data_key.key_size = 0; ReChecksumKeyBlock(h); - TEST_NEQ(KeyBlockVerify(h, hsize, NULL), 0, + TEST_NEQ(KeyBlockVerify(h, hsize, NULL, 1), 0, "KeyBlockVerify() didn't sign header"); Memcpy(h, hdr, hsize); h->data_key.key_offset = hsize; ReChecksumKeyBlock(h); - TEST_NEQ(KeyBlockVerify(h, hsize, NULL), 0, + TEST_NEQ(KeyBlockVerify(h, hsize, NULL, 1), 0, "KeyBlockVerify() data key off end"); /* TODO: verify parser can support a bigger header (i.e., one where diff --git a/utility/dev_sign_file.c b/utility/dev_sign_file.c index d59ec4adb1..3892d02faa 100644 --- a/utility/dev_sign_file.c +++ b/utility/dev_sign_file.c @@ -210,8 +210,8 @@ static int Verify(const char* filename, const char* vblock_file) { Debug("Current buf offset is at 0x%" PRIx64 " bytes\n", current_buf_offset); - /* Check the keyblock */ - if (0 != KeyBlockVerify(key_block, file_size, NULL)) { + /* Check the key block (hash only) */ + if (0 != KeyBlockVerify(key_block, file_size, NULL, 1)) { error("Error verifying key block.\n"); return 1; } diff --git a/utility/vbutil_firmware.c b/utility/vbutil_firmware.c index 3992a3a8b6..d58e2a3a96 100644 --- a/utility/vbutil_firmware.c +++ b/utility/vbutil_firmware.c @@ -200,7 +200,7 @@ static int Verify(const char* infile, const char* signpubkey, /* Verify key block */ key_block = (VbKeyBlockHeader*)blob; - if (0 != KeyBlockVerify(key_block, blob_size, sign_key)) { + if (0 != KeyBlockVerify(key_block, blob_size, sign_key, 0)) { error("Error verifying key block.\n"); return 1; } diff --git a/utility/vbutil_kernel.c b/utility/vbutil_kernel.c index eeae96e13d..763480e35f 100644 --- a/utility/vbutil_kernel.c +++ b/utility/vbutil_kernel.c @@ -641,7 +641,8 @@ static int Verify(const char* infile, const char* signpubkey, int verbose) { /* Verify key block */ key_block = bp->key_block; - if (0 != KeyBlockVerify(key_block, bp->blob_size, sign_key)) { + if (0 != KeyBlockVerify(key_block, bp->blob_size, sign_key, + (sign_key ? 0 : 1))) { error("Error verifying key block.\n"); goto verify_exit; } diff --git a/utility/vbutil_keyblock.c b/utility/vbutil_keyblock.c index a540ab59d2..7d5b7c137a 100644 --- a/utility/vbutil_keyblock.c +++ b/utility/vbutil_keyblock.c @@ -138,7 +138,7 @@ static int Unpack(const char* infile, const char* datapubkey, fprintf(stderr, "vbutil_keyblock: Error reading signpubkey.\n"); return 1; } - if (0 != KeyBlockVerify(block, block->key_block_size, sign_key)) { + if (0 != KeyBlockVerify(block, block->key_block_size, sign_key, 0)) { fprintf(stderr, "vbutil_keyblock: Error verifying key block.\n"); return 1; }