From 112c25f8e5005f2f9987b53dfbb33d9abb88b859 Mon Sep 17 00:00:00 2001 From: Chris Frantz Date: Wed, 22 Jan 2025 20:09:20 -0800 Subject: [PATCH 1/2] [rom_e2e] Add tests for a bad immutable section Add tests that check if a valid signed ROM_EXT with an invalid immutable section can boot. The correct behavior is that the ROM should determine that the immutable section is invalid and try the other slot. Signed-off-by: Chris Frantz (cherry picked from commit f17d30fef8bb739b24dd3c4284ff20124488f1fd) --- .../rom/e2e/immutable_rom_ext_section/BUILD | 103 +++++++++++++++++- .../immutable_rom_ext_section_test.c | 8 +- 2 files changed, 108 insertions(+), 3 deletions(-) diff --git a/sw/device/silicon_creator/rom/e2e/immutable_rom_ext_section/BUILD b/sw/device/silicon_creator/rom/e2e/immutable_rom_ext_section/BUILD index feffefc89b261..17bc851ebcca6 100644 --- a/sw/device/silicon_creator/rom/e2e/immutable_rom_ext_section/BUILD +++ b/sw/device/silicon_creator/rom/e2e/immutable_rom_ext_section/BUILD @@ -36,21 +36,25 @@ ROM_EXT_SLOTS = [ "name": "a", "slot": "a", "offset": SLOTS["a"], + "linker_script": "//sw/device/lib/testing/test_framework:ottf_ld_silicon_creator_slot_a", }, { "name": "virtual_a", "slot": "a", "offset": SLOTS["a"], + "linker_script": "//sw/device/lib/testing/test_framework:ottf_ld_silicon_creator_slot_virtual", }, { "name": "b", "slot": "b", "offset": SLOTS["b"], + "linker_script": "//sw/device/lib/testing/test_framework:ottf_ld_silicon_creator_slot_b", }, { "name": "virtual_b", "slot": "b", "offset": SLOTS["b"], + "linker_script": "//sw/device/lib/testing/test_framework:ottf_ld_silicon_creator_slot_virtual", }, ] @@ -195,7 +199,7 @@ IMMUTABLE_PARTITION_TEST_CASES = [ exec_env = [ "//hw/top_earlgrey:fpga_cw310_sival", ], - linker_script = "//sw/device/lib/testing/test_framework:ottf_ld_silicon_creator_slot_{}".format(s["slot"]), + linker_script = s["linker_script"], deps = [ "//hw/top:otp_ctrl_c_regs", "//hw/top_earlgrey/sw/autogen:top_earlgrey", @@ -249,3 +253,100 @@ test_suite( for t in IMMUTABLE_PARTITION_TEST_CASES ], ) + +BAD_SECTION_BINS = { + "invalid": { + # l b a t u m m i + "message": "0x6c626174756d6d69", + }, + "valid": { + # l b a t u m m I + "message": "0x6c626174756d6d49", + }, +} + +# Both `imm_section_{valid,invalid}` are valid signed ROM_EXTs. There is a +# single bit difference in the immutable section ("Immutable" vs "immutable"). +# The former produces an immutable section with a valid hash, whereas the +# later produces an invalid hash. +[ + opentitan_binary( + name = "imm_section_{}".format(name), + testonly = True, + srcs = ["immutable_rom_ext_section_test.c"], + defines = [ + "IMMUTABLE_MESSAGE={}".format(param["message"]), + ], + exec_env = [ + "//hw/top_earlgrey:fpga_cw310_sival", + ], + linker_script = "//sw/device/lib/testing/test_framework:ottf_ld_silicon_creator_slot_virtual", + deps = [ + "//hw/top:otp_ctrl_c_regs", + "//hw/top_earlgrey/sw/autogen:top_earlgrey", + "//sw/device/lib/base:hardened", + "//sw/device/lib/base:status", + "//sw/device/lib/testing/test_framework:ottf_main", + "//sw/device/silicon_creator/lib/drivers:otp", + "//sw/device/silicon_creator/lib/drivers:retention_sram", + "//sw/device/silicon_creator/lib/drivers:uart", + ], + ) + for name, param in BAD_SECTION_BINS.items() +] + +# Tests that "imm_section_invalid" is functional when OTP disabled the immutable +# section. This test proves that `imm_section_invalid` is a valid signed +# ROM_EXT stage. +opentitan_test( + name = "immutable_section_invalid_ok_when_disabled", + exec_env = { + "//hw/top_earlgrey:fpga_cw310_sival": None, + }, + fpga = fpga_params( + assemble = "{invalid}@0", + binaries = { + ":imm_section_invalid": "invalid", + }, + otp = ":otp_img_immutable_rom_ext_exec_disabled_hash_valid_virtual_a", + ), +) + +# Tests that we can boot "imm_section_valid" when there is a valid signed +# ROM_EXT with an invalid immutable section in the primary slot. +# The ROM should skip the primary slot and boot the secondary slot. +opentitan_test( + name = "immutable_section_a_bad_b_good", + exec_env = { + "//hw/top_earlgrey:fpga_cw310_sival": None, + }, + fpga = fpga_params( + assemble = "{invalid}@0 {valid}@0x80000", + binaries = { + ":imm_section_invalid": "invalid", + ":imm_section_valid": "valid", + }, + exit_failure = DEFAULT_TEST_FAILURE_MSG, + exit_success = "(?msR)Immutable$.*PASS!$", + otp = ":otp_img_immutable_rom_ext_exec_enabled_hash_valid_virtual_a", + ), +) + +# Test that the ROM fails to boot when both slots have valid signatures +# and invalid immutable sections. +opentitan_test( + name = "immutable_section_a_bad_b_bad", + exec_env = { + "//hw/top_earlgrey:fpga_cw310_sival": None, + }, + fpga = fpga_params( + assemble = "{invalid}@0 {invalid}@0x80000", + binaries = { + ":imm_section_invalid": "invalid", + }, + exit_failure = DEFAULT_TEST_SUCCESS_MSG, + # This fault code is kErrorRomImmSection. + exit_success = "BFV:034d5203$", + otp = ":otp_img_immutable_rom_ext_exec_enabled_hash_valid_virtual_a", + ), +) diff --git a/sw/device/silicon_creator/rom/e2e/immutable_rom_ext_section/immutable_rom_ext_section_test.c b/sw/device/silicon_creator/rom/e2e/immutable_rom_ext_section/immutable_rom_ext_section_test.c index 908f1075f1145..17933901776be 100644 --- a/sw/device/silicon_creator/rom/e2e/immutable_rom_ext_section/immutable_rom_ext_section_test.c +++ b/sw/device/silicon_creator/rom/e2e/immutable_rom_ext_section/immutable_rom_ext_section_test.c @@ -17,6 +17,11 @@ OTTF_DEFINE_TEST_CONFIG(); +#ifndef IMMUTABLE_MESSAGE +// l b a t u m m I +#define IMMUTABLE_MESSAGE 0x6c626174756d6d49; +#endif + enum { kImmutableRomExtSectionHashSizeIn32BitWords = OTP_CTRL_PARAM_CREATOR_SW_CFG_IMMUTABLE_ROM_EXT_SHA256_HASH_SIZE / @@ -26,8 +31,7 @@ enum { OT_USED OT_SECTION(".rom_ext_immutable") void rom_ext_non_mutable(void) { // Print "Immutable" to the UART console. - // l b a t u m m I - const uint64_t kStr1 = 0x6c626174756d6d49; + const uint64_t kStr1 = IMMUTABLE_MESSAGE; // e const uint32_t kStr2 = 0x65; const uint32_t kNewline = 0x0a0d; From 23e236d89a656ce92936568a689284989fd5c6f0 Mon Sep 17 00:00:00 2001 From: Chris Frantz Date: Wed, 22 Jan 2025 20:11:21 -0800 Subject: [PATCH 2/2] [rom] Examine the immutable section and skip if invalid 1. Refactor immutable section verification into a separate function. 2. When booting, examine the immutable section and fail the boot with an error if the immutable section is invalid. If the failure occurs in the primary slot, the ROM should attempt to boot the secondary slot. Signed-off-by: Chris Frantz (cherry picked from commit 1561505b64e36c5434917d78c1b9ca160bdf637c) --- rules/const.bzl | 3 + sw/device/silicon_creator/lib/error.h | 1 + .../rom/e2e/immutable_rom_ext_section/BUILD | 6 +- sw/device/silicon_creator/rom/rom.c | 62 +++++++++++++------ 4 files changed, 49 insertions(+), 23 deletions(-) diff --git a/rules/const.bzl b/rules/const.bzl index 9967962c58295..b7e43127bd3bd 100644 --- a/rules/const.bzl +++ b/rules/const.bzl @@ -108,6 +108,9 @@ CONST = struct( WRITE_CHECK = 0x0242440d, DATA_INVALID = 0x0342440d, ), + ROM = struct( + IMM_SECTION = 0x034d5203, + ), UNKNOWN = 0xffffffff, OK = 0x739, ), diff --git a/sw/device/silicon_creator/lib/error.h b/sw/device/silicon_creator/lib/error.h index 6bdaf8c0fe6e5..787a628ea84aa 100644 --- a/sw/device/silicon_creator/lib/error.h +++ b/sw/device/silicon_creator/lib/error.h @@ -115,6 +115,7 @@ enum module_ { \ X(kErrorRomBootFailed, ERROR_(1, kModuleRom, kFailedPrecondition)), \ X(kErrorRomResetReasonFault, ERROR_(2, kModuleRom, kUnknown)), \ + X(kErrorRomImmSection, ERROR_(3, kModuleRom, kInvalidArgument)), \ \ /* The high-byte of kErrorInterrupt is modified with the interrupt cause */ \ X(kErrorInterrupt, ERROR_(0, kModuleInterrupt, kUnknown)), \ diff --git a/sw/device/silicon_creator/rom/e2e/immutable_rom_ext_section/BUILD b/sw/device/silicon_creator/rom/e2e/immutable_rom_ext_section/BUILD index 17bc851ebcca6..96f673cbe44c1 100644 --- a/sw/device/silicon_creator/rom/e2e/immutable_rom_ext_section/BUILD +++ b/sw/device/silicon_creator/rom/e2e/immutable_rom_ext_section/BUILD @@ -73,7 +73,7 @@ IMMUTABLE_PARTITION_TEST_CASES = [ "CREATOR_SW_CFG_IMMUTABLE_ROM_EXT_EN": otp_hex(CONST.HARDENED_TRUE), "CREATOR_SW_CFG_IMMUTABLE_ROM_EXT_SHA256_HASH": otp_hex(0x1234), }, - "exit_success": MSG_TEMPLATE_BFV.format(hex_digits(CONST.BFV.INTERRUPT.ILLEGAL_INSTRUCTION)), + "exit_success": MSG_TEMPLATE_BFV.format(hex_digits(CONST.BFV.ROM.IMM_SECTION)), "exit_failure": DEFAULT_TEST_SUCCESS_MSG, }, { @@ -116,7 +116,7 @@ IMMUTABLE_PARTITION_TEST_CASES = [ # The hash check should fail, since the offset is included in the hash, # triggering a hardened check fail (which executes an unimp; triggering # an exception and shutdown). - "exit_success": MSG_TEMPLATE_BFV.format(hex_digits(CONST.BFV.INTERRUPT.ILLEGAL_INSTRUCTION)), + "exit_success": MSG_TEMPLATE_BFV.format(hex_digits(CONST.BFV.ROM.IMM_SECTION)), "exit_failure": DEFAULT_TEST_SUCCESS_MSG, }, { @@ -137,7 +137,7 @@ IMMUTABLE_PARTITION_TEST_CASES = [ # The hash check should fail, since the length is included in the hash, # triggering a hardened check fail (which executes an unimp; triggering # an exception and shutdown). - "exit_success": MSG_TEMPLATE_BFV.format(hex_digits(CONST.BFV.INTERRUPT.ILLEGAL_INSTRUCTION)), + "exit_success": MSG_TEMPLATE_BFV.format(hex_digits(CONST.BFV.ROM.IMM_SECTION)), "exit_failure": DEFAULT_TEST_SUCCESS_MSG, }, { diff --git a/sw/device/silicon_creator/rom/rom.c b/sw/device/silicon_creator/rom/rom.c index 0f646a47656fb..33e6eee51ed3e 100644 --- a/sw/device/silicon_creator/rom/rom.c +++ b/sw/device/silicon_creator/rom/rom.c @@ -543,7 +543,9 @@ static rom_error_t rom_measure_otp_partitions( * @return rom_error_t Result of the operation. */ OT_WARN_UNUSED_RESULT -static rom_error_t rom_boot(const manifest_t *manifest, uint32_t flash_exec) { +static rom_error_t rom_boot(const manifest_t *manifest, + uintptr_t imm_section_entry_point, + uint32_t flash_exec) { CFI_FUNC_COUNTER_INCREMENT(rom_counters, kCfiRomBoot, 1); HARDENED_RETURN_IF_ERROR(sc_keymgr_state_check(kScKeymgrStateReset)); @@ -668,7 +670,19 @@ static rom_error_t rom_boot(const manifest_t *manifest, uint32_t flash_exec) { // In a normal build, this function inlines to nothing. stack_utilization_print(); - // (Potentially) Execute the immutable ROM_EXT section. + if (imm_section_entry_point != kHardenedBoolFalse) { + ((rom_ext_entry_point *)imm_section_entry_point)(); + } + // Jump to ROM_EXT. + ((rom_ext_entry_point *)entry_point)(); + return kErrorRomBootFailed; +} + +static rom_error_t rom_verify_immutable_section( + rom_error_t verify_result, const manifest_t *manifest, + uintptr_t *imm_section_entry_point) { + *imm_section_entry_point = kHardenedBoolFalse; + // Verify the immutable ROM_EXT section. uint32_t rom_ext_immutable_section_enabled = otp_read32(OTP_CTRL_PARAM_CREATOR_SW_CFG_IMMUTABLE_ROM_EXT_EN_OFFSET); if (launder32(rom_ext_immutable_section_enabled) == kHardenedBoolTrue) { @@ -680,14 +694,6 @@ static rom_error_t rom_boot(const manifest_t *manifest, uint32_t flash_exec) { OTP_CTRL_PARAM_CREATOR_SW_CFG_IMMUTABLE_ROM_EXT_LENGTH_OFFSET); uintptr_t immutable_rom_ext_entry_point = (uintptr_t)manifest + immutable_rom_ext_start_offset; - // If address translation is enabled, adjust the entry_point. - if (launder32(manifest->address_translation) == kHardenedBoolTrue) { - HARDENED_CHECK_EQ(manifest->address_translation, kHardenedBoolTrue); - immutable_rom_ext_entry_point = - rom_ext_vma_get(manifest, immutable_rom_ext_entry_point); - } else { - HARDENED_CHECK_NE(manifest->address_translation, kHardenedBoolTrue); - } // Compute a hash of the code section. // Include the start offset and the length of the section in the hash. @@ -707,17 +713,26 @@ static rom_error_t rom_boot(const manifest_t *manifest, uint32_t flash_exec) { otp_read(OTP_CTRL_PARAM_CREATOR_SW_CFG_IMMUTABLE_ROM_EXT_SHA256_HASH_OFFSET, immutable_rom_ext_hash.digest, kHmacDigestNumWords); for (size_t i = 0; i < kHmacDigestNumWords; ++i) { - HARDENED_CHECK_EQ(immutable_rom_ext_hash.digest[i], - actual_immutable_section_digest.digest[i]); + if (immutable_rom_ext_hash.digest[i] != + actual_immutable_section_digest.digest[i]) { + verify_result = kErrorRomImmSection; + } + } + // If address translation is enabled, adjust the entry_point. + if (launder32(manifest->address_translation) == kHardenedBoolTrue) { + HARDENED_CHECK_EQ(manifest->address_translation, kHardenedBoolTrue); + immutable_rom_ext_entry_point = + rom_ext_vma_get(manifest, immutable_rom_ext_entry_point); + } else { + HARDENED_CHECK_NE(manifest->address_translation, kHardenedBoolTrue); + } + if (verify_result == kErrorOk) { + *imm_section_entry_point = immutable_rom_ext_entry_point; } - ((rom_ext_entry_point *)immutable_rom_ext_entry_point)(); } else { HARDENED_CHECK_NE(rom_ext_immutable_section_enabled, kHardenedBoolTrue); } - - // Jump to ROM_EXT. - ((rom_ext_entry_point *)entry_point)(); - return kErrorRomBootFailed; + return verify_result; } /** @@ -734,9 +749,12 @@ static rom_error_t rom_try_boot(void) { boot_policy_manifests_t manifests = boot_policy_manifests_get(); uint32_t flash_exec = 0; + uintptr_t imm_section_entry_point = kHardenedBoolFalse; CFI_FUNC_COUNTER_PREPCALL(rom_counters, kCfiRomTryBoot, 2, kCfiRomVerify); rom_error_t error = rom_verify(manifests.ordered[0], &flash_exec); + error = rom_verify_immutable_section(error, manifests.ordered[0], + &imm_section_entry_point); CFI_FUNC_COUNTER_INCREMENT(rom_counters, kCfiRomTryBoot, 4); if (launder32(error) == kErrorOk) { @@ -744,17 +762,21 @@ static rom_error_t rom_try_boot(void) { CFI_FUNC_COUNTER_CHECK(rom_counters, kCfiRomVerify, 3); CFI_FUNC_COUNTER_INIT(rom_counters, kCfiRomTryBoot); CFI_FUNC_COUNTER_PREPCALL(rom_counters, kCfiRomTryBoot, 1, kCfiRomBoot); - HARDENED_RETURN_IF_ERROR(rom_boot(manifests.ordered[0], flash_exec)); + HARDENED_RETURN_IF_ERROR( + rom_boot(manifests.ordered[0], imm_section_entry_point, flash_exec)); return kErrorRomBootFailed; } CFI_FUNC_COUNTER_PREPCALL(rom_counters, kCfiRomTryBoot, 5, kCfiRomVerify); - HARDENED_RETURN_IF_ERROR(rom_verify(manifests.ordered[1], &flash_exec)); + error = rom_verify(manifests.ordered[1], &flash_exec); + HARDENED_RETURN_IF_ERROR(rom_verify_immutable_section( + error, manifests.ordered[1], &imm_section_entry_point)); CFI_FUNC_COUNTER_INCREMENT(rom_counters, kCfiRomTryBoot, 7); CFI_FUNC_COUNTER_CHECK(rom_counters, kCfiRomVerify, 3); CFI_FUNC_COUNTER_PREPCALL(rom_counters, kCfiRomTryBoot, 8, kCfiRomBoot); - HARDENED_RETURN_IF_ERROR(rom_boot(manifests.ordered[1], flash_exec)); + HARDENED_RETURN_IF_ERROR( + rom_boot(manifests.ordered[1], imm_section_entry_point, flash_exec)); return kErrorRomBootFailed; }