Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherry-pick to earlgrey_1.0.0: [ownership] Fix protect_when_active behavior #25097

Merged
merged 2 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions sw/device/silicon_creator/lib/ownership/ownership.c
Original file line number Diff line number Diff line change
Expand Up @@ -272,14 +272,13 @@ rom_error_t ownership_init(boot_data_t *bootdata, owner_config_t *config,
}

rom_error_t ownership_flash_lockdown(boot_data_t *bootdata,
uint32_t active_slot,
const owner_config_t *config) {
if (bootdata->ownership_state == kOwnershipStateLockedOwner) {
HARDENED_RETURN_IF_ERROR(
owner_block_flash_apply(config->flash, kBootSlotA,
/*lockdown=*/bootdata->primary_bl0_slot));
HARDENED_RETURN_IF_ERROR(
owner_block_flash_apply(config->flash, kBootSlotB,
/*lockdown=*/bootdata->primary_bl0_slot));
HARDENED_RETURN_IF_ERROR(owner_block_flash_apply(config->flash, kBootSlotA,
/*lockdown=*/active_slot));
HARDENED_RETURN_IF_ERROR(owner_block_flash_apply(config->flash, kBootSlotB,
/*lockdown=*/active_slot));
} else {
HARDENED_CHECK_NE(bootdata->ownership_state, kOwnershipStateLockedOwner);
}
Expand Down
2 changes: 2 additions & 0 deletions sw/device/silicon_creator/lib/ownership/ownership.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ rom_error_t ownership_init(boot_data_t *bootdata, owner_config_t *config,
* Lockdown the flash configuration.
*
* @param bootdata The current bootdata.
* @param active_slot The active slot.
* @param config The current owner configuration.
* @return error state.
*/
rom_error_t ownership_flash_lockdown(boot_data_t *bootdata,
uint32_t active_slot,
const owner_config_t *config);

/**
Expand Down
20 changes: 10 additions & 10 deletions sw/device/silicon_creator/lib/ownership/testdata/basic_owner.json5
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
scramble: false,
ecc: false,
high_endurance: false,
protect_when_primary: true
protect_when_active: true
},
{
start: 32,
Expand All @@ -74,7 +74,7 @@
scramble: true,
ecc: true,
high_endurance: false,
protect_when_primary: true
protect_when_active: true
},
{
start: 224,
Expand All @@ -85,7 +85,7 @@
scramble: false,
ecc: false,
high_endurance: true,
protect_when_primary: false
protect_when_active: false
},
{
start: 256,
Expand All @@ -96,7 +96,7 @@
scramble: false,
ecc: false,
high_endurance: false,
protect_when_primary: true
protect_when_active: true
},
{
start: 288,
Expand All @@ -107,7 +107,7 @@
scramble: true,
ecc: true,
high_endurance: false,
protect_when_primary: true
protect_when_active: true
},
{
start: 480,
Expand All @@ -118,7 +118,7 @@
scramble: false,
ecc: false,
high_endurance: true,
protect_when_primary: false
protect_when_active: false
}
]
}
Expand All @@ -136,7 +136,7 @@
scramble: true,
ecc: true,
high_endurance: false,
protect_when_primary: false
protect_when_active: false
},
{
bank: 0,
Expand All @@ -148,7 +148,7 @@
scramble: true,
ecc: true,
high_endurance: false,
protect_when_primary: false
protect_when_active: false
},
{
bank: 0,
Expand All @@ -160,7 +160,7 @@
scramble: true,
ecc: true,
high_endurance: false,
protect_when_primary: false
protect_when_active: false
},
{
bank: 0,
Expand All @@ -172,7 +172,7 @@
scramble: true,
ecc: true,
high_endurance: false,
protect_when_primary: false
protect_when_active: false
}
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -493,10 +493,10 @@
The new flash configuration should have a different scrambling or ECC configuration than the current configuration.
- Boot owner firmware and examine the flash controller's region configuration registers.
- Confirm that in the intermediate "dual-owner" state, the primary half of the flash is configured for the previous owner and the secondary half is configured for the next owner.
Confirm that the `protect_when_primary` condition does not apply during the dual-owner state.
Confirm that the `protect_when_active` condition does not apply during the dual-owner state.
- Activate ownership with the `dummy` activate key.
- Use the rescue protocol to upload owner firmware into the newly-configured flash regions.
- Confirm that in the owned state, the entire flash is configured for the new owner and that the `protect_when_primary` condition removes erase/program permissions appropriately.
- Confirm that in the owned state, the entire flash is configured for the new owner and that the `protect_when_active` condition removes erase/program permissions appropriately.
'''
tags:
[
Expand Down
63 changes: 35 additions & 28 deletions sw/device/silicon_creator/rom_ext/e2e/ownership/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ opentitan_binary(
exec_env = {
"//hw/top_earlgrey:fpga_cw310_rom_ext": None,
},
linker_script = "//sw/device/lib/testing/test_framework:ottf_ld_silicon_owner_slot_virtual",
deps = [
"//hw/top_earlgrey/sw/autogen:top_earlgrey",
"//sw/device/lib/base:status",
Expand Down Expand Up @@ -397,34 +398,40 @@ ownership_transfer_test(
# rom_ext_e2e_testplan.hjson%rom_ext_e2e_flash_permission_test
# Note: rescue-after-activate tests that rescue correctly accesses regions with
# different scrambling/ECC properties than the default flash configuration.
ownership_transfer_test(
name = "flash_permission_test",
srcs = ["flash_regions.c"],
fpga = fpga_params(
binaries = {
":flash_regions": "flash_regions",
},
test_cmd = """
--clear-bitstream
--bootstrap={firmware}
--unlock-mode=Any
--unlock-key=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:unlock_key)
--next-owner-key=$(location //sw/device/silicon_creator/lib/ownership/keys/dummy:owner_key)
--next-unlock-key=$(location //sw/device/silicon_creator/lib/ownership/keys/dummy:unlock_key)
--next-activate-key=$(location //sw/device/silicon_creator/lib/ownership/keys/dummy:activate_key)
--next-application-key=$(location //sw/device/silicon_creator/lib/ownership/keys/dummy:app_prod_ecdsa_pub)
--config-kind=with-flash-locked
--rescue-after-activate={flash_regions}
""",
test_harness = "//sw/host/tests/ownership:flash_permission_test",
),
deps = [
"//hw/top_earlgrey/sw/autogen:top_earlgrey",
"//sw/device/lib/base:status",
"//sw/device/lib/dif:flash_ctrl",
"//sw/device/lib/testing/test_framework:ottf_main",
],
)
[
ownership_transfer_test(
name = "flash_permission_test_slot_{}".format(slot),
srcs = ["flash_regions.c"],
fpga = fpga_params(
binaries = {
":flash_regions": "flash_regions",
},
slot = "Slot{}".format(slot.upper()),
test_cmd = """
--clear-bitstream
--bootstrap={firmware}
--unlock-mode=Any
--unlock-key=$(location //sw/device/silicon_creator/lib/ownership/keys/fake:unlock_key)
--next-owner-key=$(location //sw/device/silicon_creator/lib/ownership/keys/dummy:owner_key)
--next-unlock-key=$(location //sw/device/silicon_creator/lib/ownership/keys/dummy:unlock_key)
--next-activate-key=$(location //sw/device/silicon_creator/lib/ownership/keys/dummy:activate_key)
--next-application-key=$(location //sw/device/silicon_creator/lib/ownership/keys/dummy:app_prod_ecdsa_pub)
--config-kind=with-flash-locked
--rescue-after-activate={flash_regions}
--rescue-slot={slot}
""",
test_harness = "//sw/host/tests/ownership:flash_permission_test",
),
linker_script = "//sw/device/lib/testing/test_framework:ottf_ld_silicon_owner_slot_virtual",
deps = [
"//hw/top_earlgrey/sw/autogen:top_earlgrey",
"//sw/device/lib/base:status",
"//sw/device/lib/dif:flash_ctrl",
"//sw/device/lib/testing/test_framework:ottf_main",
],
)
for slot in ("a", "b")
]

# Tests that a owner config with an improper flash configuration cannot be activated.
ownership_transfer_test(
Expand Down
8 changes: 5 additions & 3 deletions sw/device/silicon_creator/rom_ext/rom_ext.c
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ void rom_ext_sram_exec(owner_sram_exec_mode_t mode) {
OT_WARN_UNUSED_RESULT
static rom_error_t rom_ext_verify(const manifest_t *manifest,
const boot_data_t *boot_data) {
dbg_print_epmp();
RETURN_IF_ERROR(rom_ext_boot_policy_manifest_check(manifest, boot_data));
ownership_key_alg_t key_alg = kOwnershipKeyAlgEcdsaP256;
RETURN_IF_ERROR(owner_keyring_find_key(
Expand Down Expand Up @@ -548,7 +549,7 @@ static rom_error_t rom_ext_attestation_owner(const boot_data_t *boot_data,
}

OT_WARN_UNUSED_RESULT
static rom_error_t rom_ext_boot(boot_data_t *boot_data,
static rom_error_t rom_ext_boot(boot_data_t *boot_data, boot_log_t *boot_log,
const manifest_t *manifest) {
// Generate CDI_1 attestation keys and certificate.
HARDENED_RETURN_IF_ERROR(rom_ext_attestation_owner(boot_data, manifest));
Expand Down Expand Up @@ -692,7 +693,8 @@ static rom_error_t rom_ext_boot(boot_data_t *boot_data,
ibex_addr_remap_lockdown(1);

// Lock the flash according to the ownership configuration.
HARDENED_RETURN_IF_ERROR(ownership_flash_lockdown(boot_data, &owner_config));
HARDENED_RETURN_IF_ERROR(
ownership_flash_lockdown(boot_data, boot_log->bl0_slot, &owner_config));

dbg_print_epmp();

Expand Down Expand Up @@ -876,7 +878,7 @@ static rom_error_t rom_ext_try_next_stage(boot_data_t *boot_data,
boot_log_digest_update(boot_log);

// Boot fails if a verified ROM_EXT cannot be booted.
RETURN_IF_ERROR(rom_ext_boot(boot_data, manifests.ordered[i]));
RETURN_IF_ERROR(rom_ext_boot(boot_data, boot_log, manifests.ordered[i]));
// `rom_ext_boot()` should never return `kErrorOk`, but if it does
// we must shut down the chip instead of trying the next ROM_EXT.
return kErrorRomExtBootFailed;
Expand Down
10 changes: 10 additions & 0 deletions sw/host/opentitanlib/src/chip/boot_svc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,16 @@ with_unknown! {
}
}

impl BootSlot {
pub fn opposite(self) -> Result<Self> {
match self {
BootSlot::SlotA => Ok(BootSlot::SlotB),
BootSlot::SlotB => Ok(BootSlot::SlotA),
_ => Err(ChipDataError::BadSlot(self).into()),
}
}
}

/// The Boot Services header common to all boot services commands and responses.
#[derive(Debug, Default, Serialize, Annotate)]
pub struct Header {
Expand Down
2 changes: 2 additions & 0 deletions sw/host/opentitanlib/src/chip/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ pub enum ChipDataError {
Anyhow(#[from] anyhow::Error),
#[error("bad size: expected {0} bytes, but found {1}")]
BadSize(usize, usize),
#[error("bad slot: {0:x}")]
BadSlot(boot_svc::BootSlot),
#[error("invalid digest")]
InvalidDigest,
}
18 changes: 9 additions & 9 deletions sw/host/opentitanlib/src/ownership/flash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ pub struct FlashFlags {
/// The high endurance feature is enabled in this region.
#[serde(default)]
pub high_endurance: bool,
/// Forbid program and erase operations when in the primary flash side.
/// Forbid program and erase operations when in the active flash side.
#[serde(default)]
pub protect_when_primary: bool,
pub protect_when_active: bool,
/// Lock the configuration of this region.
#[serde(default)]
pub lock: bool,
Expand Down Expand Up @@ -64,7 +64,7 @@ impl FlashFlags {
read: true,
program: true,
erase: true,
protect_when_primary: true,
protect_when_active: true,
..Default::default()
}
}
Expand All @@ -77,7 +77,7 @@ impl FlashFlags {
erase: true,
scramble: true,
ecc: true,
protect_when_primary: true,
protect_when_active: true,
..Default::default()
}
}
Expand Down Expand Up @@ -114,7 +114,7 @@ impl From<u64> for FlashFlags {
read: flags & 0xF == Self::TRUE,
program: (flags >> 4) & 0xF == Self::TRUE,
erase: (flags >> 8) & 0xF == Self::TRUE,
protect_when_primary: (flags >> 24) & 0xF == Self::TRUE,
protect_when_active: (flags >> 24) & 0xF == Self::TRUE,
lock: (flags >> 28) & 0xF == Self::TRUE,

// Second 32-bit word: flash properties.
Expand All @@ -134,7 +134,7 @@ impl From<FlashFlags> for u64 {
if flags.read { FlashFlags::TRUE } else { FlashFlags::FALSE } |
if flags.program { FlashFlags::TRUE } else { FlashFlags::FALSE } << 4 |
if flags.erase { FlashFlags::TRUE } else { FlashFlags::FALSE } << 8 |
if flags.protect_when_primary { FlashFlags::TRUE } else { FlashFlags::FALSE } << 24 |
if flags.protect_when_active { FlashFlags::TRUE } else { FlashFlags::FALSE } << 24 |
if flags.lock { FlashFlags::TRUE } else { FlashFlags::FALSE } << 28 |

// Second 32-bit word: flash properties.
Expand Down Expand Up @@ -263,7 +263,7 @@ r#"00000000: 46 4c 53 48 2c 00 00 00 00 00 00 00 96 09 00 99 FLSH,...........
scramble: false,
ecc: true,
high_endurance: false,
protect_when_primary: false,
protect_when_active: false,
lock: false
},
{
Expand All @@ -275,7 +275,7 @@ r#"00000000: 46 4c 53 48 2c 00 00 00 00 00 00 00 96 09 00 99 FLSH,...........
scramble: false,
ecc: false,
high_endurance: false,
protect_when_primary: false,
protect_when_active: false,
lock: false
},
{
Expand All @@ -287,7 +287,7 @@ r#"00000000: 46 4c 53 48 2c 00 00 00 00 00 00 00 96 09 00 99 FLSH,...........
scramble: true,
ecc: true,
high_endurance: true,
protect_when_primary: false,
protect_when_active: false,
lock: false
}
]
Expand Down
6 changes: 3 additions & 3 deletions sw/host/opentitanlib/src/ownership/flash_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ r#"00000000: 49 4e 46 4f 2c 00 00 00 00 00 00 00 96 09 00 99 INFO,...........
scramble: false,
ecc: true,
high_endurance: false,
protect_when_primary: false,
protect_when_active: false,
lock: false
},
{
Expand All @@ -158,7 +158,7 @@ r#"00000000: 49 4e 46 4f 2c 00 00 00 00 00 00 00 96 09 00 99 INFO,...........
scramble: false,
ecc: false,
high_endurance: false,
protect_when_primary: false,
protect_when_active: false,
lock: false
},
{
Expand All @@ -171,7 +171,7 @@ r#"00000000: 49 4e 46 4f 2c 00 00 00 00 00 00 00 96 09 00 99 INFO,...........
scramble: true,
ecc: true,
high_endurance: true,
protect_when_primary: false,
protect_when_active: false,
lock: false
}
]
Expand Down
Loading
Loading