vboot_api_kernel: Remove assumptions about EC-RW hash type and size
With newer PD chips and different update mechanisms, we can no longer
guarantee that the "hash" (really just a sort of version identifier) of
an EC-RW image will always be a SHA256. This patch removes any hardcoded
assumptions about that from vboot, and instead accepts any hash size
returned by VbExEcHashImage() and VbExEcGetExpectedImageHash().
It also removes the assumption that the hash can be regenerated by
running SHA256 over the full image returned by VbExEcGetExpectedImage().
We can thus no longer support VBERROR_EC_GET_EXPECTED_HASH_FROM_IMAGE,
which is fine since that functionality hasn't been needed for years and
there would be no reason why we might need it in the future. This also
allows simplifying the code flow of EcUpdateImage() a bit (since you can
really just return very early if you already figured out that you don't
need to update).
BRANCH=None
BUG=chrome-os-partner:53780
TEST=Tested software sync on Oak both after cold and warm boot.
Change-Id: I498f3d39085a38740734fff9f2d1a186a0801489
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/348001
Reviewed-by: Randall Spangler <rspangler@chromium.org>
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index 5adeac4..90703a9 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -714,11 +714,11 @@
(VbSharedDataHeader *)cparams->shared_data_blob;
int rv;
int hash_size;
+ int ec_hash_size;
const uint8_t *hash = NULL;
const uint8_t *expected = NULL;
const uint8_t *ec_hash = NULL;
int expected_size;
- uint8_t expected_hash[SHA256_DIGEST_SIZE];
int i;
int rw_request = select != VB_SELECT_FIRMWARE_READONLY;
@@ -727,202 +727,141 @@
"Check for %s update\n", rw_request ? "RW" : "RO"));
/* Get current EC hash. */
- rv = VbExEcHashImage(devidx, select, &ec_hash, &hash_size);
-
+ rv = VbExEcHashImage(devidx, select, &ec_hash, &ec_hash_size);
if (rv) {
VBDEBUG(("EcUpdateImage() - "
"VbExEcHashImage() returned %d\n", rv));
VbSetRecoveryRequest(VBNV_RECOVERY_EC_HASH_FAILED);
return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
}
- if (hash_size != SHA256_DIGEST_SIZE) {
- VBDEBUG(("EcUpdateImage() - "
- "VbExEcHashImage() says size %d, not %d\n",
- hash_size, SHA256_DIGEST_SIZE));
- VbSetRecoveryRequest(VBNV_RECOVERY_EC_HASH_SIZE);
- return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
- }
- VBDEBUG(("EC-%s hash:", rw_request ? "RW" : "RO"));
- for (i = 0; i < SHA256_DIGEST_SIZE; i++)
+ VBDEBUG(("EC-%s hash: ", rw_request ? "RW" : "RO"));
+ for (i = 0; i < ec_hash_size; i++)
VBDEBUG(("%02x",ec_hash[i]));
VBDEBUG(("\n"));
/* Get expected EC hash. */
rv = VbExEcGetExpectedImageHash(devidx, select, &hash, &hash_size);
-
- if (rv == VBERROR_EC_GET_EXPECTED_HASH_FROM_IMAGE) {
- /*
- * BIOS has verified EC image but doesn't have a precomputed
- * hash for it, so we must compute the hash ourselves.
- */
- hash = NULL;
- } else if (rv) {
+ if (rv) {
VBDEBUG(("EcUpdateImage() - "
"VbExEcGetExpectedImageHash() returned %d\n", rv));
VbSetRecoveryRequest(VBNV_RECOVERY_EC_EXPECTED_HASH);
return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
- } else if (hash_size != SHA256_DIGEST_SIZE) {
+ }
+ if (ec_hash_size != hash_size) {
VBDEBUG(("EcUpdateImage() - "
- "VbExEcGetExpectedImageHash() says size %d, not %d\n",
- hash_size, SHA256_DIGEST_SIZE));
- VbSetRecoveryRequest(VBNV_RECOVERY_EC_EXPECTED_HASH);
+ "EC uses %d-byte hash, but AP-RW contains %d bytes\n",
+ ec_hash_size, hash_size));
+ VbSetRecoveryRequest(VBNV_RECOVERY_EC_HASH_SIZE);
return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
- } else {
- VBDEBUG(("Expected hash:"));
- for (i = 0; i < SHA256_DIGEST_SIZE; i++)
- VBDEBUG(("%02x", hash[i]));
- VBDEBUG(("\n"));
- *need_update = SafeMemcmp(ec_hash, hash, SHA256_DIGEST_SIZE);
}
- /*
- * Get expected EC image if we're sure we need to update (because the
- * expected hash didn't match the EC) or we still don't know (because
- * there was no expected hash and we need the image to compute one
- * ourselves).
- */
- if (*need_update || !hash) {
- /* Get expected EC image */
- rv = VbExEcGetExpectedImage(devidx, select, &expected,
- &expected_size);
- if (rv) {
- VBDEBUG(("EcUpdateImage() - "
- "VbExEcGetExpectedImage() returned %d\n", rv));
- VbSetRecoveryRequest(VBNV_RECOVERY_EC_EXPECTED_IMAGE);
- return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
- }
- VBDEBUG(("EcUpdateImage() - image len = %d\n", expected_size));
+ VBDEBUG(("Expected hash: "));
+ for (i = 0; i < hash_size; i++)
+ VBDEBUG(("%02x", hash[i]));
+ VBDEBUG(("\n"));
+ *need_update = SafeMemcmp(ec_hash, hash, hash_size);
- /* Hash expected image */
- internal_SHA256(expected, expected_size, expected_hash);
- VBDEBUG(("Computed hash of expected image:"));
- for (i = 0; i < SHA256_DIGEST_SIZE; i++)
- VBDEBUG(("%02x", expected_hash[i]));
- VBDEBUG(("\n"));
- }
+ if (!*need_update)
+ return VBERROR_SUCCESS;
- if (!hash) {
- /*
- * BIOS didn't have expected EC hash, so check if we need
- * update by comparing EC hash to the one we just computed.
- */
- *need_update = SafeMemcmp(ec_hash, expected_hash,
- SHA256_DIGEST_SIZE);
- } else if (*need_update && SafeMemcmp(hash, expected_hash,
- SHA256_DIGEST_SIZE)) {
- /*
- * We need to update, but the expected EC image doesn't match
- * the expected EC hash we were given.
- */
+ /* Get expected EC image */
+ rv = VbExEcGetExpectedImage(devidx, select, &expected, &expected_size);
+ if (rv) {
VBDEBUG(("EcUpdateImage() - "
"VbExEcGetExpectedImage() returned %d\n", rv));
- VbSetRecoveryRequest(VBNV_RECOVERY_EC_HASH_MISMATCH);
+ VbSetRecoveryRequest(VBNV_RECOVERY_EC_EXPECTED_IMAGE);
+ return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+ }
+ VBDEBUG(("EcUpdateImage() - image len = %d\n", expected_size));
+
+ if (in_rw && rw_request) {
+ /*
+ * Check if BIOS should also load VGA Option ROM when
+ * rebooting to save another reboot if possible.
+ */
+ if ((shared->flags & VBSD_EC_SLOW_UPDATE) &&
+ (shared->flags & VBSD_OPROM_MATTERS) &&
+ !(shared->flags & VBSD_OPROM_LOADED)) {
+ VBDEBUG(("EcUpdateImage() - Reboot to "
+ "load VGA Option ROM\n"));
+ VbNvSet(&vnc, VBNV_OPROM_NEEDED, 1);
+ }
+
+ /*
+ * EC is running the wrong RW image. Reboot the EC to
+ * RO so we can update it on the next boot.
+ */
+ VBDEBUG(("EcUpdateImage() - "
+ "in RW, need to update RW, so reboot\n"));
return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
}
- if (in_rw && rw_request) {
- if (*need_update) {
- /*
- * Check if BIOS should also load VGA Option ROM when
- * rebooting to save another reboot if possible.
- */
- if ((shared->flags & VBSD_EC_SLOW_UPDATE) &&
- (shared->flags & VBSD_OPROM_MATTERS) &&
- !(shared->flags & VBSD_OPROM_LOADED)) {
- VBDEBUG(("EcUpdateImage() - Reboot to "
- "load VGA Option ROM\n"));
- VbNvSet(&vnc, VBNV_OPROM_NEEDED, 1);
- }
+ VBDEBUG(("EcUpdateImage() updating EC-%s...\n",
+ rw_request ? "RW" : "RO"));
- /*
- * EC is running the wrong RW image. Reboot the EC to
- * RO so we can update it on the next boot.
- */
- VBDEBUG(("EcUpdateImage() - "
- "in RW, need to update RW, so reboot\n"));
- return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+ if (shared->flags & VBSD_EC_SLOW_UPDATE) {
+ VBDEBUG(("EcUpdateImage() - EC is slow. Show WAIT screen.\n"));
+
+ /* Ensure the VGA Option ROM is loaded */
+ if ((shared->flags & VBSD_OPROM_MATTERS) &&
+ !(shared->flags & VBSD_OPROM_LOADED)) {
+ VBDEBUG(("EcUpdateImage() - Reboot to "
+ "load VGA Option ROM\n"));
+ VbNvSet(&vnc, VBNV_OPROM_NEEDED, 1);
+ return VBERROR_VGA_OPROM_MISMATCH;
}
- VBDEBUG(("EcUpdateImage() in EC-RW and it matches\n"));
- return VBERROR_SUCCESS;
+ VbDisplayScreen(cparams, VB_SCREEN_WAIT, 0, &vnc);
}
- /* Update EC if necessary */
+ rv = VbExEcUpdateImage(devidx, select, expected, expected_size);
+ if (rv != VBERROR_SUCCESS) {
+ VBDEBUG(("EcUpdateImage() - "
+ "VbExEcUpdateImage() returned %d\n", rv));
- if (*need_update) {
- VBDEBUG(("EcUpdateImage() updating EC-%s...\n",
- rw_request ? "RW" : "RO"));
+ /*
+ * The EC may know it needs a reboot. It may need to
+ * unprotect the region before updating, or may need to
+ * reboot after updating. Either way, it's not an error
+ * requiring recovery mode.
+ *
+ * If we fail for any other reason, trigger recovery
+ * mode.
+ */
+ if (rv != VBERROR_EC_REBOOT_TO_RO_REQUIRED)
+ VbSetRecoveryRequest(VBNV_RECOVERY_EC_UPDATE);
- if (shared->flags & VBSD_EC_SLOW_UPDATE) {
- VBDEBUG(("EcUpdateImage() - "
- "EC is slow. Show WAIT screen.\n"));
-
- /* Ensure the VGA Option ROM is loaded */
- if ((shared->flags & VBSD_OPROM_MATTERS) &&
- !(shared->flags & VBSD_OPROM_LOADED)) {
- VBDEBUG(("EcUpdateImage() - Reboot to "
- "load VGA Option ROM\n"));
- VbNvSet(&vnc, VBNV_OPROM_NEEDED, 1);
- return VBERROR_VGA_OPROM_MISMATCH;
- }
-
- VbDisplayScreen(cparams, VB_SCREEN_WAIT, 0, &vnc);
- }
-
- rv = VbExEcUpdateImage(devidx, select, expected, expected_size);
-
- if (rv != VBERROR_SUCCESS) {
- VBDEBUG(("EcUpdateImage() - "
- "VbExEcUpdateImage() returned %d\n", rv));
-
- /*
- * The EC may know it needs a reboot. It may need to
- * unprotect the region before updating, or may need to
- * reboot after updating. Either way, it's not an error
- * requiring recovery mode.
- *
- * If we fail for any other reason, trigger recovery
- * mode.
- */
- if (rv != VBERROR_EC_REBOOT_TO_RO_REQUIRED)
- VbSetRecoveryRequest(VBNV_RECOVERY_EC_UPDATE);
-
- return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
- }
-
+ return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
}
/* Verify the EC was updated properly */
- if (*need_update) {
- /* Get current EC hash. */
- rv = VbExEcHashImage(devidx, select, &ec_hash, &hash_size);
-
- if (rv) {
- VBDEBUG(("EcUpdateImage() - "
- "VbExEcHashImage() returned %d\n", rv));
- VbSetRecoveryRequest(VBNV_RECOVERY_EC_HASH_FAILED);
- return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
- }
- if (hash_size != SHA256_DIGEST_SIZE) {
- VBDEBUG(("EcUpdateImage() - "
- "VbExEcHashImage() says size %d, not %d\n",
- hash_size, SHA256_DIGEST_SIZE));
- VbSetRecoveryRequest(VBNV_RECOVERY_EC_HASH_SIZE);
- return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
- }
- VBDEBUG(("Updated EC-%s hash:", rw_request ? "RW" : "RO"));
- for (i = 0; i < SHA256_DIGEST_SIZE; i++)
- VBDEBUG(("%02x",ec_hash[i]));
- VBDEBUG(("\n"));
-
- if (SafeMemcmp(ec_hash, hash, SHA256_DIGEST_SIZE)){
- VBDEBUG(("EcUpdateImage() - "
- "Failed to update EC-%s\n", rw_request ?
- "RW" : "RO"));
- VbSetRecoveryRequest(VBNV_RECOVERY_EC_UPDATE);
- return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
- }
+ rv = VbExEcHashImage(devidx, select, &ec_hash, &ec_hash_size);
+ if (rv) {
+ VBDEBUG(("EcUpdateImage() - "
+ "VbExEcHashImage() returned %d\n", rv));
+ VbSetRecoveryRequest(VBNV_RECOVERY_EC_HASH_FAILED);
+ return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
}
+ if (hash_size != ec_hash_size) {
+ VBDEBUG(("EcUpdateImage() - "
+ "VbExEcHashImage() says size %d, not %d\n",
+ ec_hash_size, hash_size));
+ VbSetRecoveryRequest(VBNV_RECOVERY_EC_HASH_SIZE);
+ return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+ }
+ VBDEBUG(("Updated EC-%s hash: ", rw_request ? "RW" : "RO"));
+ for (i = 0; i < ec_hash_size; i++)
+ VBDEBUG(("%02x",ec_hash[i]));
+ VBDEBUG(("\n"));
+
+ if (SafeMemcmp(ec_hash, hash, hash_size)){
+ VBDEBUG(("EcUpdateImage() - "
+ "Failed to update EC-%s\n", rw_request ?
+ "RW" : "RO"));
+ VbSetRecoveryRequest(VBNV_RECOVERY_EC_UPDATE);
+ return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+ }
+
return VBERROR_SUCCESS;
}
diff --git a/tests/vboot_api_kernel3_tests.c b/tests/vboot_api_kernel3_tests.c
index d6f740b..6ca58f2 100644
--- a/tests/vboot_api_kernel3_tests.c
+++ b/tests/vboot_api_kernel3_tests.c
@@ -47,7 +47,6 @@
static uint8_t want_ec_hash[32];
static uint8_t update_hash;
static int want_ec_hash_size;
-static uint8_t mock_sha[32];
static uint32_t screens_displayed[8];
static uint32_t screens_count = 0;
@@ -104,9 +103,6 @@
update_hash = 42;
- Memset(mock_sha, 0, sizeof(want_ec_hash));
- mock_sha[0] = 42;
-
// TODO: ensure these are actually needed
Memset(screens_displayed, 0, sizeof(screens_displayed));
@@ -182,16 +178,7 @@
*hash = want_ec_hash;
*hash_size = want_ec_hash_size;
- if (want_ec_hash_size == -1)
- return VBERROR_EC_GET_EXPECTED_HASH_FROM_IMAGE;
- else
- return want_ec_hash_size ? VBERROR_SUCCESS : VBERROR_SIMULATED;
-}
-
-uint8_t *internal_SHA256(const uint8_t *data, uint64_t len, uint8_t *digest)
-{
- Memcpy(digest, mock_sha, sizeof(mock_sha));
- return digest;
+ return want_ec_hash_size ? VBERROR_SUCCESS : VBERROR_SIMULATED;
}
VbError_t VbExEcUpdateImage(int devidx, enum VbSelectFirmware_t select,
@@ -296,30 +283,17 @@
ResetMocks();
want_ec_hash_size = 16;
test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
- VBNV_RECOVERY_EC_EXPECTED_HASH,
- "Bad precalculated hash size");
+ VBNV_RECOVERY_EC_HASH_SIZE,
+ "Hash size mismatch");
ResetMocks();
- mock_in_rw = 1;
- want_ec_hash_size = -1;
- test_ssync(0, 0, "No precomputed hash");
-
- ResetMocks();
- want_ec_hash_size = -1;
- get_expected_retval = VBERROR_SIMULATED;
- test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
- VBNV_RECOVERY_EC_EXPECTED_IMAGE, "Can't fetch image");
+ want_ec_hash_size = 4;
+ mock_ec_rw_hash_size = 4;
+ test_ssync(0, 0, "Custom hash size");
/* Updates required */
ResetMocks();
mock_in_rw = 1;
- want_ec_hash[0]++;
- test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
- VBNV_RECOVERY_EC_HASH_MISMATCH,
- "Precalculated hash mismatch");
-
- ResetMocks();
- mock_in_rw = 1;
mock_ec_rw_hash[0]++;
test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
0, "Pending update needs reboot");
