ROM: Hitless update doesn't fail when FMC SVN is changed

[korran]

I noticed something interesting last week. This test passes when it probably shouldn't:

#[test]
fn test_update_fmc_svn_mismatch() {
    const ORIGINAL_FMC_SVN: u32 = 3;
    const NEW_FMC_SVN: u32 = 6;

    let rom = caliptra_builder::build_firmware_rom(firmware::rom_from_env()).unwrap();
    let image_bundle = caliptra_builder::build_and_sign_image(
        &TEST_FMC_WITH_UART,
        &APP_WITH_UART,
        ImageOptions { 
            fmc_version: 0, 
            fmc_min_svn: 0, 
            fmc_svn: ORIGINAL_FMC_SVN,
            ..Default::default()
        }
    )
    .unwrap();
    let mut hw = caliptra_hw_model::new(BootParams {
        init_params: InitParams {
            rom: &rom,
            ..Default::default()
        },
        fw_image: Some(&image_bundle.to_bytes().unwrap()),
        ..Default::default()
    })
    .unwrap();

    let image_bundle2 = caliptra_builder::build_and_sign_image(
        &TEST_FMC_WITH_UART,
        &APP_WITH_UART,
        ImageOptions { 
            fmc_version: 1, 
            fmc_min_svn: 1, 
            fmc_svn: NEW_FMC_SVN,
            ..Default::default()
        }
    )
    .unwrap();

    // This should probably fail; fmc_svn fields are different
    hw.upload_firmware(&image_bundle2.to_bytes().unwrap()).unwrap();

    let pcr_entry_arr = hw.mailbox_execute(0x1000_0000, &[]).unwrap().unwrap();
    let mut pcr_entries = vec![PcrLogEntry::default(); pcr_entry_arr.len() / mem::size_of::<PcrLogEntry>()];
    pcr_entries.as_bytes_mut().copy_from_slice(&pcr_entry_arr);
    let device_status_entry = pcr_entries.iter().find(|e| e.id == PcrLogEntryId::DeviceStatus as u16).unwrap();
    let fmc_svn = device_status_entry.measured_data()[4];

    // But the PCR log still has the original SVN...
    assert_eq!(fmc_svn as u32, ORIGINAL_FMC_SVN);
}

[mhatrevi]

Only the FMC image digest is compared for differentiation during Update Reset and not the svn, so the test succeeding is expected. Also, the FMC svn from the updated firmware is not stored in the data vault (since FMC is expected to remain the same). For logging to the PCR log, the FMC svn is picked from the data vault, hence the old value is seen in the PCR log.

[korran]

I think this is harmless with how the ROM is now, but could be a trap for future changes.

https://github.com/chipsalliance/caliptra-sw/blob/45cef07f6f7421e744c6b101ff07d142fa7c3d4f/rom/dev/src/flow/update_reset.rs#L78-L86

info.fmc.svn is NEW_FMC_SVN from the test. I confirmed that the ROM doesn't do anything with this value today (preferring the value from the datavault). However, having this incorrect value floating around for someone to use by mistake in the future is a bit error-prone. I think a decent argument can be made for failing the update if any of the FMC fields in the image header change. Maybe not for 1.0 though...

[mhatrevi]

"I think a decent argument can be made for failing the update if any of the FMC fields in the image header change. Maybe not for 1.0 though..."

Fair point. We'll revisit this for 2.0. Added the 2.0 tag to this.