diff options
author | Hung-Te Lin <hungte@chromium.org> | 2019-02-12 16:43:28 +0800 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2019-02-13 10:43:06 -0800 |
commit | 5b2159a6bbe5b1ecb7fba2654cab82aa8643a6c5 (patch) | |
tree | 1a1aabc1ac9cbfda1fa6b3ff6aa93118c6c3d29e | |
parent | 99a9a808fc38f790fa3ead22c209012c0e96938b (diff) | |
download | vboot-5b2159a6bbe5b1ecb7fba2654cab82aa8643a6c5.tar.gz |
futility: updater: Improve error message when key conflicts
Many firmware developers will try to flash a local built firmware (i.e,
DEV key signed) on a MP device (with write protection enabled).
The updater used to provide feedback like:
ERROR: verify_keyblock: Failed verifying key block.
INFO: Current (RO) firmware image has root key: ade780ffd0...732867181bae
WARNING: Target (RW) image is signed by rootkey: b11d74edd2...e1135b49e7f0.
ERROR: RW not signed by same RO root key
>> FAILED: Firmware updater aborted.
This is correctly identifying the root cause, but not helpful for
developers to figure out what to do, and may be confused with the DEV
re-key safety check (which needs --force).
Also, when developers try to do "--mode=factory --force", the message
was:
updater_setup_config: Factory mode needs WP disabled.
Where the 'WP' is again not clear enough.
With this change, we're improving the error messages so that:
- Being consistent on 'root key' instead of 'rootkey'.
- Being consistent for having period for error messages, except those
ended with root key hash (for easier copy-paste).
- Say 'Write Protection' instead of 'WP'.
- When re-keying with WP enabled, print a better hint:
"To change keys in RO area, you have to first remove write protection
(https://goo.gl/ces83U)."
BUG=None
TEST=make futil; tests/futility/run_test_scripts.sh $(pwd)/build/futility
BRANCH=none
Change-Id: Ia74d7b113766d09428a4d0897918b4f17b4afae7
Reviewed-on: https://chromium-review.googlesource.com/1465709
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Matthew Blecker <matthewb@chromium.org>
-rw-r--r-- | futility/updater.c | 41 | ||||
-rwxr-xr-x | tests/futility/test_update.sh | 8 |
2 files changed, 30 insertions, 19 deletions
diff --git a/futility/updater.c b/futility/updater.c index d1352ef1..0880208d 100644 --- a/futility/updater.c +++ b/futility/updater.c @@ -26,6 +26,7 @@ #define COMMAND_BUFFER_SIZE 256 #define RETURN_ON_FAILURE(x) do {int r = (x); if (r) return r;} while (0); #define FLASHROM_OUTPUT_WP_PATTERN "write protect is " +#define REMOVE_WP_URL "https://goo.gl/ces83U" /* System environment values. */ static const char * const FWACT_A = "A", @@ -1291,9 +1292,9 @@ static enum rootkey_compat_result check_compatible_root_key( "Maybe RW corrupted?"); return ROOTKEY_COMPAT_ERROR; } - WARN("Target (RW) image is signed by rootkey: %s.", + WARN("Target (RW) image is signed by root key: %s%s", rootkey_rw ? packed_key_sha1_string(rootkey_rw) : - "<invalid>"); + "<invalid>", to_dev ? " (DEV/unsigned)" : ""); return to_dev ? ROOTKEY_COMPAT_REKEY_TO_DEV : ROOTKEY_COMPAT_REKEY; } @@ -1421,7 +1422,8 @@ const char * const updater_error_messages[] = { [UPDATE_ERR_WRITE_FIRMWARE] = "Failed writing firmware.", [UPDATE_ERR_PLATFORM] = "Your system platform is not compatible.", [UPDATE_ERR_TARGET] = "No valid RW target to update. Abort.", - [UPDATE_ERR_ROOT_KEY] = "RW not signed by same RO root key", + [UPDATE_ERR_ROOT_KEY] = "RW signed by incompatible root key " + "(different from RO).", [UPDATE_ERR_TPM_ROLLBACK] = "RW not usable due to TPM anti-rollback.", [UPDATE_ERR_UNKNOWN] = "Unknown error.", }; @@ -1501,7 +1503,7 @@ static enum updater_error_codes update_try_rw_firmware( * This was also known as --mode=recovery, --wp=1 in legacy updater. * Returns UPDATE_ERR_DONE if success, otherwise error. */ -static enum updater_error_codes update_rw_firmrware( +static enum updater_error_codes update_rw_firmware( struct updater_config *cfg, struct firmware_image *image_from, struct firmware_image *image_to) @@ -1560,7 +1562,6 @@ static enum updater_error_codes update_whole_firmware( if (preserve_images(cfg)) DEBUG("Failed to preserve some sections - ignore."); - INFO("Checking compatibility..."); if (check_compatible_tpm_keys(cfg, image_to)) return UPDATE_ERR_TPM_ROLLBACK; @@ -1598,7 +1599,9 @@ static enum updater_error_codes update_whole_firmware( */ enum updater_error_codes update_firmware(struct updater_config *cfg) { - int wp_enabled; + int wp_enabled, done = 0; + enum updater_error_codes r = UPDATE_ERR_UNKNOWN; + struct firmware_image *image_from = &cfg->image_current, *image_to = &cfg->image; if (!image_to->data) @@ -1649,18 +1652,25 @@ enum updater_error_codes update_firmware(struct updater_config *cfg) return update_legacy_firmware(cfg, image_to); if (cfg->try_update) { - enum updater_error_codes r; r = update_try_rw_firmware(cfg, image_from, image_to, wp_enabled); - if (r != UPDATE_ERR_NEED_RO_UPDATE) - return r; - WARN("%s", updater_error_messages[r]); + if (r == UPDATE_ERR_NEED_RO_UPDATE) + WARN("%s", updater_error_messages[r]); + else + done = 1; } - if (wp_enabled) - return update_rw_firmrware(cfg, image_from, image_to); - else - return update_whole_firmware(cfg, image_to); + if (!done) { + r = wp_enabled ? update_rw_firmware(cfg, image_from, image_to) : + update_whole_firmware(cfg, image_to); + } + + /* Providing more hints for what to do on failure. */ + if (r == UPDATE_ERR_ROOT_KEY && wp_enabled) + ERROR("To change keys in RO area, you must first remove " + "write protection ( " REMOVE_WP_URL " )."); + + return r; } /* @@ -2060,7 +2070,8 @@ int updater_setup_config(struct updater_config *cfg, } if (check_wp_disabled && is_write_protection_enabled(cfg)) { errorcnt++; - ERROR("Factory mode needs WP disabled."); + ERROR("Please remove write protection for factory mode " + "( " REMOVE_WP_URL " )."); } if (!errorcnt && do_output) { const char *r = arg->output_dir; diff --git a/tests/futility/test_update.sh b/tests/futility/test_update.sh index 302bf26a..eac9933a 100755 --- a/tests/futility/test_update.sh +++ b/tests/futility/test_update.sh @@ -215,7 +215,7 @@ test_update "RW update (incompatible platform)" \ -i "${LINK_BIOS}" --wp=1 --sys_props 0,0x10001,1 test_update "RW update (incompatible rootkey)" \ - "${FROM_DIFFERENT_ROOTKEY_IMAGE}" "!RW not signed by same RO root key" \ + "${FROM_DIFFERENT_ROOTKEY_IMAGE}" "!RW signed by incompatible root key" \ -i "${TO_IMAGE}" --wp=1 --sys_props 0,0x10001,1 test_update "RW update (TPM Anti-rollback: data key)" \ @@ -243,7 +243,7 @@ test_update "RW update (incompatible platform)" \ -i "${LINK_BIOS}" -t --wp=1 --sys_props 0x10001,1 test_update "RW update (incompatible rootkey)" \ - "${FROM_DIFFERENT_ROOTKEY_IMAGE}" "!RW not signed by same RO root key" \ + "${FROM_DIFFERENT_ROOTKEY_IMAGE}" "!RW signed by incompatible root key" \ -i "${TO_IMAGE}" -t --wp=1 --sys_props 0,0x10001,1 test_update "RW update (TPM Anti-rollback: data key)" \ @@ -277,11 +277,11 @@ test_update "Factory mode update (WP=0)" \ --factory -i "${TO_IMAGE}" --wp=0 --sys_props 0,0x10001,1 test_update "Factory mode update (WP=1)" \ - "${FROM_IMAGE}" "!needs WP disabled" \ + "${FROM_IMAGE}" "!remove write protection for factory mode" \ -i "${TO_IMAGE}" --wp=1 --sys_props 0,0x10001,1 --mode=factory test_update "Factory mode update (WP=1)" \ - "${FROM_IMAGE}" "!needs WP disabled" \ + "${FROM_IMAGE}" "!remove write protection for factory mode" \ --factory -i "${TO_IMAGE}" --wp=1 --sys_props 0,0x10001,1 test_update "Factory mode update (GBB=0 -> 39)" \ |