diff --git a/examples/htool_dfu.c b/examples/htool_dfu.c index 6e4c6f0..be11a92 100644 --- a/examples/htool_dfu.c +++ b/examples/htool_dfu.c @@ -152,6 +152,34 @@ int htool_dfu_update(const struct htool_invocation* inv) { goto cleanup2; } + // [`rom_ext_verify` + // call](https://github.com/lowRISC/opentitan/blob/acb2179c7f4b59d4ee2118fb24cfb4f97d920dc9/sw/device/silicon_creator/rom_ext/rom_ext.c#L398-L400) + // will check the application firmware security version against boot policy + // ([ref1](https://github.com/lowRISC/opentitan/blob/acb2179c7f4b59d4ee2118fb24cfb4f97d920dc9/sw/device/silicon_creator/rom_ext/rom_ext_verify.c#L28-L29), + // [ref2](https://github.com/lowRISC/opentitan/blob/7791207ad2a60ca8349fa1a801f3a5c639be21db/sw/device/silicon_creator/rom_ext/rom_ext_boot_policy.c#L73-L75)). + // On failure, it will return error and cause ROM_EXT to [try the other boot + // slot](https://github.com/lowRISC/opentitan/blob/acb2179c7f4b59d4ee2118fb24cfb4f97d920dc9/sw/device/silicon_creator/rom_ext/rom_ext.c#L393-L405) + // for the application firmware. To catch this case earlier, when the desired + // application firmware has security version less than the minimum BL0 + // security version return an error indicating that the firmware update cannot + // be done. + // TODO: Add a flag to convey that the user wants to go ahead and try the + // update regardless of minimum BL0 security version (like maybe for testing + // that it actually works). + if (desired_app.security_version < resp.bl0_min_sec_ver) { + fprintf( + stderr, + "Desired application firmware security version %u is less than " + "currently enforced minimum application firmware security version %u\n", + desired_app.security_version, resp.bl0_min_sec_ver); + retval = -1; + goto cleanup2; + } + // Update the desired ROM_EXT version and app version based on what is + // currently running + const bool expect_desired_rom_ext_after_update = + expect_active_rom_ext_change_with_update(&desired_rom_ext, &resp); + int update_cnt = force ? 2 : dfu_update_count(&desired_rom_ext, &desired_app, &resp); @@ -173,7 +201,8 @@ int htool_dfu_update(const struct htool_invocation* inv) { goto cleanup2; } - if (!libhoth_ot_boot_slot_eq(&resp, &desired_rom_ext, &desired_app)) { + if (!libhoth_ot_boot_slot_eq(&resp, &desired_rom_ext, &desired_app, + expect_desired_rom_ext_after_update)) { fprintf(stderr, "Boot slot is wrong after dfu update %d\n", i); libhoth_print_dfu_error(dev, &resp, LIBHOTH_OK); retval = -1; @@ -181,7 +210,8 @@ int htool_dfu_update(const struct htool_invocation* inv) { } } - if (!libhoth_update_complete(&resp, &desired_rom_ext, &desired_app)) { + if (!libhoth_update_complete(&resp, &desired_rom_ext, &desired_app, + expect_desired_rom_ext_after_update)) { fprintf(stderr, "DFU update failed, running image does not match expected after %d " "dfu updates\n", diff --git a/protocol/dfu_check.c b/protocol/dfu_check.c index d586938..e4c55e5 100644 --- a/protocol/dfu_check.c +++ b/protocol/dfu_check.c @@ -95,7 +95,10 @@ int libhoth_dfu_check(struct libhoth_device* const dev, const uint8_t* image, // parsing purpose libhoth_print_boot_log(resp, &desired_rom_ext, &desired_app); - if (!libhoth_update_complete(resp, &desired_rom_ext, &desired_app)) { + const bool expect_desired_rom_ext = + expect_active_rom_ext_change_with_update(&desired_rom_ext, resp); + if (!libhoth_update_complete(resp, &desired_rom_ext, &desired_app, + expect_desired_rom_ext)) { fprintf( stderr, "Error: Mismatch detected between the current and desired versions.\n"); diff --git a/protocol/opentitan_version.c b/protocol/opentitan_version.c index 45fb94d..f1d0bd8 100644 --- a/protocol/opentitan_version.c +++ b/protocol/opentitan_version.c @@ -214,9 +214,14 @@ const struct opentitan_image_version* libhoth_ot_staged_romext( bool libhoth_ot_boot_slot_eq( const struct opentitan_get_version_resp* resp, const struct opentitan_image_version* desired_romext, - const struct opentitan_image_version* desired_app) { - return libhoth_ot_version_eq(libhoth_ot_boot_romext(resp), desired_romext) && - libhoth_ot_version_eq(libhoth_ot_boot_app(resp), desired_app); + const struct opentitan_image_version* desired_app, + const bool expect_desired_rom_ext) { + bool result = libhoth_ot_version_eq(libhoth_ot_boot_app(resp), desired_app); + if (expect_desired_rom_ext) { + result &= + libhoth_ot_version_eq(libhoth_ot_boot_romext(resp), desired_romext); + } + return result; } bool libhoth_ot_staged_slot_eq( @@ -231,7 +236,69 @@ bool libhoth_ot_staged_slot_eq( bool libhoth_update_complete( const struct opentitan_get_version_resp* resp, const struct opentitan_image_version* desired_romext, - const struct opentitan_image_version* desired_app) { - return libhoth_ot_boot_slot_eq(resp, desired_romext, desired_app) && + const struct opentitan_image_version* desired_app, + const bool expect_desired_rom_ext) { + // In case of ROM_EXT update not occurring due to valid reasons (see + // `expect_active_rom_ext_change_with_update`), the staging side will still be + // running the ROM_EXT from the update package + return libhoth_ot_boot_slot_eq(resp, desired_romext, desired_app, + expect_desired_rom_ext) && libhoth_ot_staged_slot_eq(resp, desired_romext, desired_app); } + +bool expect_active_rom_ext_change_with_update( + const struct opentitan_image_version* desired_rom_ext_version, + const struct opentitan_get_version_resp* current_versions_resp) { + if ((desired_rom_ext_version == NULL) || (current_versions_resp == NULL)) { + return true; + } + const struct opentitan_image_version* current_rom_ext_version = + libhoth_ot_boot_romext(current_versions_resp); + // For booting ROM_EXT, ROM will give priority to slot with greater security + // version first, and then greater major version, and then greater minor + // version + // ([ref](https://github.com/lowRISC/opentitan/blob/acb2179c7f4b59d4ee2118fb24cfb4f97d920dc9/sw/device/silicon_creator/rom/boot_policy.c#L18-L41)) + if (current_rom_ext_version->security_version > + desired_rom_ext_version->security_version) { + fprintf(stderr, + "INFO: Running ROM_EXT security version %u, Desired ROM_EXT " + "security version %u\n", + current_rom_ext_version->security_version, + desired_rom_ext_version->security_version); + return false; + } else if ((current_rom_ext_version->security_version == + desired_rom_ext_version->security_version) && + (current_rom_ext_version->major > + desired_rom_ext_version->major)) { + fprintf(stderr, + "INFO: Running ROM_EXT has major version %u, Desired ROM_EXT major " + "version %u\n", + current_rom_ext_version->major, desired_rom_ext_version->major); + return false; + } else if ((current_rom_ext_version->security_version == + desired_rom_ext_version->security_version) && + (current_rom_ext_version->major == + desired_rom_ext_version->major) && + (current_rom_ext_version->minor > + desired_rom_ext_version->minor)) { + fprintf(stderr, + "INFO: Running ROM_EXT has minor version %u, Desired ROM_EXT minor " + "version %u\n", + current_rom_ext_version->minor, desired_rom_ext_version->minor); + return false; + } + + // There is no need to check minimum ROM_EXT security version. ROM checks the + // security version of currently running ROM_EXT + // [here](https://github.com/lowRISC/opentitan/blob/acb2179c7f4b59d4ee2118fb24cfb4f97d920dc9/sw/device/silicon_creator/rom/boot_policy.c#L55-L60). + // Failure during this check will cause ROM to try the other ROM_EXT with the + // same check applied to it + // [here](https://github.com/lowRISC/opentitan/blob/acb2179c7f4b59d4ee2118fb24cfb4f97d920dc9/sw/device/silicon_creator/rom/rom.c#L761-L785). + // So it is expected that the running ROM_EXT has security version greater + // than or equal to the minimum ROM_EXT security version. In such a case, the + // conditions above will already handle firmware update to image with ROM_EXT + // version less than the minimum ROM_EXT security version (in the ROM_EXT + // security version check between desired and currently running ROM_EXT) + + return true; +} diff --git a/protocol/opentitan_version.h b/protocol/opentitan_version.h index aec870f..8afe8b8 100644 --- a/protocol/opentitan_version.h +++ b/protocol/opentitan_version.h @@ -135,7 +135,8 @@ const struct opentitan_image_version* libhoth_ot_staged_romext( bool libhoth_ot_boot_slot_eq( const struct opentitan_get_version_resp* resp, const struct opentitan_image_version* desired_romext, - const struct opentitan_image_version* desired_app); + const struct opentitan_image_version* desired_app, + const bool expect_desired_rom_ext); bool libhoth_ot_staged_slot_eq( const struct opentitan_get_version_resp* resp, const struct opentitan_image_version* desired_romext, @@ -143,7 +144,12 @@ bool libhoth_ot_staged_slot_eq( bool libhoth_update_complete( const struct opentitan_get_version_resp* resp, const struct opentitan_image_version* desired_romext, - const struct opentitan_image_version* desired_app); + const struct opentitan_image_version* desired_app, + const bool expect_desired_rom_ext); + +bool expect_active_rom_ext_change_with_update( + const struct opentitan_image_version* desired_rom_ext_version, + const struct opentitan_get_version_resp* current_versions_resp); #ifdef __cplusplus } diff --git a/protocol/opentitan_version_test.cc b/protocol/opentitan_version_test.cc index d7e0ead..595e52a 100644 --- a/protocol/opentitan_version_test.cc +++ b/protocol/opentitan_version_test.cc @@ -142,11 +142,23 @@ TEST_F(LibHothTest, opentitan_image_compare_test) { resp.app.booted_slot = kOpentitanBootSlotB; EXPECT_TRUE(libhoth_ot_boot_slot_eq(&resp, libhoth_ot_boot_romext(&resp), - libhoth_ot_boot_app(&resp))); + libhoth_ot_boot_app(&resp), + /*expect_desired_rom_ext*/ true)); + EXPECT_TRUE(libhoth_ot_boot_slot_eq(&resp, libhoth_ot_boot_romext(&resp), + libhoth_ot_boot_app(&resp), + /*expect_desired_rom_ext*/ false)); EXPECT_FALSE(libhoth_ot_boot_slot_eq(&resp, libhoth_ot_staged_romext(&resp), - libhoth_ot_boot_app(&resp))); + libhoth_ot_boot_app(&resp), + /*expect_desired_rom_ext*/ true)); + EXPECT_TRUE(libhoth_ot_boot_slot_eq(&resp, libhoth_ot_staged_romext(&resp), + libhoth_ot_boot_app(&resp), + /*expect_desired_rom_ext*/ false)); + EXPECT_FALSE(libhoth_ot_boot_slot_eq(&resp, libhoth_ot_boot_romext(&resp), + libhoth_ot_staged_app(&resp), + /*expect_desired_rom_ext*/ true)); EXPECT_FALSE(libhoth_ot_boot_slot_eq(&resp, libhoth_ot_boot_romext(&resp), - libhoth_ot_staged_app(&resp))); + libhoth_ot_staged_app(&resp), + /*expect_desired_rom_ext*/ true)); EXPECT_TRUE(libhoth_ot_staged_slot_eq(&resp, libhoth_ot_staged_romext(&resp), libhoth_ot_staged_app(&resp))); @@ -154,7 +166,9 @@ TEST_F(LibHothTest, opentitan_image_compare_test) { libhoth_ot_staged_app(&resp))); EXPECT_FALSE(libhoth_ot_staged_slot_eq(&resp, libhoth_ot_staged_romext(&resp), libhoth_ot_boot_app(&resp))); +} +TEST_F(LibHothTest, TestLibhothUpdateComplete) { struct opentitan_image_version romext = { .major = 6, .minor = 111, @@ -162,23 +176,139 @@ TEST_F(LibHothTest, opentitan_image_compare_test) { .timestamp = 1234, .measurement = {}, }; + struct opentitan_image_version romext_security_version_larger = { + .major = 6, + .minor = 112, + .security_version = 9, + .timestamp = 1234, + .measurement = {}, + }; + struct opentitan_image_version romext_major_version_larger = { + .major = 8, + .minor = 111, + .security_version = 7, + .timestamp = 1234, + .measurement = {}, + }; + struct opentitan_image_version romext_minor_version_larger = { + .major = 6, + .minor = 114, + .security_version = 7, + .timestamp = 1234, + .measurement = {}, + }; + struct opentitan_image_version romext_minor_version_smaller = { + .major = 6, + .minor = 110, + .security_version = 7, + .timestamp = 1234, + .measurement = {}, + }; - struct opentitan_image_version app = { + struct opentitan_image_version app_1 = { .major = 1, .minor = 116, .security_version = 2, .timestamp = 6789, .measurement = {}, }; + struct opentitan_image_version app_2 = { + .major = 1, + .minor = 115, + .security_version = 2, + .timestamp = 6789, + .measurement = {}, + }; + struct opentitan_get_version_resp resp = {}; + + // Case 1: Successful update of both halves + resp.rom_ext.booted_slot = kOpentitanBootSlotA; + resp.app.booted_slot = kOpentitanBootSlotA; resp.rom_ext.slots[0] = romext; resp.rom_ext.slots[1] = romext; - resp.app.slots[0] = app; - resp.app.slots[1] = app; + resp.app.slots[0] = app_1; + resp.app.slots[1] = app_1; + + EXPECT_TRUE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/true)); + EXPECT_TRUE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/false)); - EXPECT_TRUE(libhoth_update_complete(&resp, &romext, &app)); - EXPECT_FALSE(libhoth_update_complete(&resp, &romext, &romext)); - EXPECT_FALSE(libhoth_update_complete(&resp, &app, &app)); + // Case 2: Active ROM_EXT did not change since running ROM_EXT had greater + // security version + resp.rom_ext.booted_slot = kOpentitanBootSlotA; + resp.app.booted_slot = kOpentitanBootSlotA; + resp.rom_ext.slots[0] = romext_security_version_larger; + resp.rom_ext.slots[1] = romext; + resp.app.slots[0] = app_1; + resp.app.slots[1] = app_1; + EXPECT_FALSE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/true)); + EXPECT_TRUE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/false)); + + // Case 3: Active ROM_EXT did not change since running ROM_EXT had greater + // major version + resp.rom_ext.booted_slot = kOpentitanBootSlotA; + resp.app.booted_slot = kOpentitanBootSlotA; + resp.rom_ext.slots[0] = romext_major_version_larger; + resp.rom_ext.slots[1] = romext; + resp.app.slots[0] = app_1; + resp.app.slots[1] = app_1; + EXPECT_FALSE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/true)); + EXPECT_TRUE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/false)); + + // Case 4: Active ROM_EXT did not change since running ROM_EXT had greater + // minor version + resp.rom_ext.booted_slot = kOpentitanBootSlotA; + resp.app.booted_slot = kOpentitanBootSlotA; + resp.rom_ext.slots[0] = romext_minor_version_larger; + resp.rom_ext.slots[1] = romext; + resp.app.slots[0] = app_1; + resp.app.slots[1] = app_1; + EXPECT_FALSE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/true)); + EXPECT_TRUE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/false)); + + // Case 5: ROM_EXT did not change due to some reason + resp.rom_ext.booted_slot = kOpentitanBootSlotA; + resp.app.booted_slot = kOpentitanBootSlotA; + resp.rom_ext.slots[0] = romext_minor_version_smaller; + resp.rom_ext.slots[1] = romext_minor_version_smaller; + resp.app.slots[0] = app_1; + resp.app.slots[1] = app_1; + EXPECT_FALSE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/true)); + EXPECT_FALSE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/false)); + + // Case 6: Application firmware did not update due to some reason + resp.rom_ext.booted_slot = kOpentitanBootSlotA; + resp.app.booted_slot = kOpentitanBootSlotA; + resp.rom_ext.slots[0] = romext; + resp.rom_ext.slots[1] = romext; + resp.app.slots[0] = app_2; + resp.app.slots[1] = app_2; + EXPECT_FALSE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/true)); + EXPECT_FALSE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/false)); + resp.app.slots[0] = app_1; + resp.app.slots[1] = app_2; + EXPECT_FALSE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/true)); + EXPECT_FALSE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/false)); + resp.app.slots[0] = app_2; + resp.app.slots[1] = app_1; + EXPECT_FALSE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/true)); + EXPECT_FALSE(libhoth_update_complete(&resp, &romext, &app_1, + /*expect_desired_rom_ext=*/false)); } TEST_F(LibHothTest, ExtractOtBundleBoundsCheckLargeOffset) {