Repository: chipsalliance/caliptra-sw
Pull request #477 (2023-07-15 03:05:28)

[fix] FMC: Address PCR handling issues.
This commit addresses the  problem of  FMC "Journey PCR", on a warm reset,  being extended with an additional measurement of the same RT module that was there for the previous boot. Additionally it is  fixes the bug of FMC not locking journey and current PCRs and not erasing the current PCR before extending it. 

rusty1968: Follow up needed : Sppend to the PCR log in DCCM, and  test all the extensions performed by FMC

Patch:
diff --git a/Cargo.lock b/Cargo.lock
index b0779bdf..96e931ca 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -306,6 +306,7 @@ dependencies = [
  "caliptra-builder",
  "caliptra-cpu",
  "caliptra-drivers",
+ "caliptra-registers",
  "caliptra_common",
  "cfg-if 1.0.0",
  "ufmt",
diff --git a/common/src/lib.rs b/common/src/lib.rs
index 3e6e0a65..f46fd6cc 100644
--- a/common/src/lib.rs
+++ b/common/src/lib.rs
@@ -23,7 +23,7 @@ pub use hand_off::{
 
 pub use boot_status::RomBootStatus;
 pub use fuse::{FuseLogEntry, FuseLogEntryId};
-pub use pcr::{PcrLogEntry, PcrLogEntryId};
+pub use pcr::{PcrLogEntry, PcrLogEntryId, RT_FW_CURRENT_PCR, RT_FW_JOURNEY_PCR};
 pub use printer::HexBytes;
 pub use printer::MutablePrinter;
 
diff --git a/common/src/pcr.rs b/common/src/pcr.rs
index 9d2d830f..4b07f6b4 100644
--- a/common/src/pcr.rs
+++ b/common/src/pcr.rs
@@ -11,6 +11,7 @@ Abstract:
 
 --*/
 
+use caliptra_drivers::PcrId;
 use zerocopy::{AsBytes, FromBytes};
 
 // PcrLogEntryId is used to identify the PCR entry and
@@ -65,3 +66,6 @@ pub struct PcrLogEntry {
 
     pub reserved: [u8; 4],
 }
+
+pub const RT_FW_CURRENT_PCR: PcrId = PcrId::PcrId3;
+pub const RT_FW_JOURNEY_PCR: PcrId = PcrId::PcrId2;
diff --git a/fmc/src/flow/pcr.rs b/fmc/src/flow/pcr.rs
index 35efab0d..f22d4cb7 100644
--- a/fmc/src/flow/pcr.rs
+++ b/fmc/src/flow/pcr.rs
@@ -25,8 +25,7 @@ use crate::fmc_env::FmcEnv;
 use crate::HandOff;
 use caliptra_drivers::{okref, CaliptraResult, PcrId};
 
-const CURRENT_PCR: PcrId = PcrId::PcrId3;
-const JOURNEY_PCR: PcrId = PcrId::PcrId2;
+use caliptra_common::{RT_FW_CURRENT_PCR, RT_FW_JOURNEY_PCR};
 
 /// Extend current PCR
 ///
@@ -34,7 +33,11 @@ const JOURNEY_PCR: PcrId = PcrId::PcrId2;
 ///
 /// * `env` - FMC Environment
 pub fn extend_current_pcr(env: &mut FmcEnv, hand_off: &HandOff) -> CaliptraResult<()> {
-    extend_pcr_common(env, hand_off, CURRENT_PCR)
+    // Clear current PCR before extending it.
+    if env.soc_ifc.reset_reason() == caliptra_drivers::ResetReason::UpdateReset {
+        env.pcr_bank.erase_pcr(RT_FW_CURRENT_PCR)?;
+    }
+    extend_pcr_common(env, hand_off, RT_FW_CURRENT_PCR)
 }
 
 /// Extend journey PCR
@@ -43,7 +46,7 @@ pub fn extend_current_pcr(env: &mut FmcEnv, hand_off: &HandOff) -> CaliptraResul
 ///
 /// * `env` - FMC Environment
 pub fn extend_journey_pcr(env: &mut FmcEnv, hand_off: &HandOff) -> CaliptraResult<()> {
-    extend_pcr_common(env, hand_off, JOURNEY_PCR)
+    extend_pcr_common(env, hand_off, RT_FW_JOURNEY_PCR)
 }
 
 /// Extend common data into PCR
diff --git a/fmc/src/flow/rt_alias.rs b/fmc/src/flow/rt_alias.rs
index ee70f3ad..4301e426 100644
--- a/fmc/src/flow/rt_alias.rs
+++ b/fmc/src/flow/rt_alias.rs
@@ -23,7 +23,9 @@ use caliptra_common::cprintln;
 use caliptra_common::crypto::Ecc384KeyPair;
 use caliptra_common::keyids::{KEY_ID_RT_CDI, KEY_ID_RT_PRIV_KEY, KEY_ID_TMP};
 use caliptra_common::HexBytes;
-use caliptra_drivers::{okref, report_boot_status, CaliptraError, CaliptraResult, KeyId};
+use caliptra_drivers::{
+    okref, report_boot_status, CaliptraError, CaliptraResult, KeyId, ResetReason,
+};
 use caliptra_x509::{NotAfter, NotBefore, RtAliasCertTbs, RtAliasCertTbsParams};
 
 const SHA384_HASH_SIZE: usize = 48;
@@ -104,6 +106,13 @@ impl RtAliasLayer {
         Self::extend_pcrs(env, hand_off)?;
         cprintln!("[alias rt] Extend RT PCRs Done");
 
+        cprintln!("[alias rt] Lock RT PCRs");
+        env.pcr_bank
+            .set_pcr_lock(caliptra_common::RT_FW_CURRENT_PCR);
+        env.pcr_bank
+            .set_pcr_lock(caliptra_common::RT_FW_JOURNEY_PCR);
+        cprintln!("[alias rt] Lock RT PCRs Done");
+
         // Retrieve Dice Input Layer from Hand Off and Derive Key
         match Self::dice_input_from_hand_off(hand_off, env) {
             Ok(input) => {
@@ -147,7 +156,10 @@ impl RtAliasLayer {
     /// * `hand_off` - HandOff
     pub fn extend_pcrs(env: &mut FmcEnv, hand_off: &HandOff) -> CaliptraResult<()> {
         extend_current_pcr(env, hand_off)?;
-        extend_journey_pcr(env, hand_off)?;
+        match env.soc_ifc.reset_reason() {
+            ResetReason::ColdReset | ResetReason::UpdateReset => extend_journey_pcr(env, hand_off)?,
+            _ => cprintln!("[alias rt : skip journey pcr extension"),
+        }
         Ok(())
     }
 


Test patch:
diff --git a/fmc/test-fw/test-rt/Cargo.toml b/fmc/test-fw/test-rt/Cargo.toml
index 394147c3..d5a410bb 100644
--- a/fmc/test-fw/test-rt/Cargo.toml
+++ b/fmc/test-fw/test-rt/Cargo.toml
@@ -8,6 +8,7 @@ edition = "2021"
 [dependencies]
 caliptra-cpu = { version = "0.1.0", path = "../../../cpu" }
 caliptra-drivers = { path = "../../../drivers" }
+caliptra-registers = { version = "0.1.0", path = "../../../registers" }
 caliptra_common = { path = "../../../common", default-features = false }
 ufmt = "0.2.0"
 zerocopy = "0.6.1"
@@ -21,8 +22,5 @@ caliptra-builder = { path = "../../../builder" }
 [features]
 riscv = ["caliptra-cpu/riscv"]
 default = ["std"]
-emu = [
-    "caliptra_common/emu",
-    "caliptra-drivers/emu"
-]
+emu = ["caliptra_common/emu", "caliptra-drivers/emu"]
 std = ["ufmt/std", "caliptra_common/std"]
diff --git a/fmc/test-fw/test-rt/src/main.rs b/fmc/test-fw/test-rt/src/main.rs
index 9946a1a0..48bccd56 100644
--- a/fmc/test-fw/test-rt/src/main.rs
+++ b/fmc/test-fw/test-rt/src/main.rs
@@ -16,7 +16,8 @@ Abstract:
 
 use caliptra_common::cprintln;
 use caliptra_cpu::TrapRecord;
-use caliptra_drivers::{report_fw_error_non_fatal, Mailbox};
+use caliptra_drivers::{report_fw_error_non_fatal, Mailbox, PcrBank};
+use caliptra_registers::pv::PvReg;
 use core::hint::black_box;
 
 #[cfg(feature = "std")]
@@ -36,6 +37,15 @@ pub extern "C" fn entry_point() -> ! {
     cprintln!("{}", BANNER);
 
     if let Some(_fht) = caliptra_common::FirmwareHandoffTable::try_load() {
+        // Test PCR is locked.
+        let mut pcr_bank = unsafe { PcrBank::new(PvReg::new()) };
+        // Test erasing pcr. This should fail.
+        assert!(pcr_bank
+            .erase_pcr(caliptra_common::RT_FW_CURRENT_PCR)
+            .is_err());
+        assert!(pcr_bank
+            .erase_pcr(caliptra_common::RT_FW_JOURNEY_PCR)
+            .is_err());
         caliptra_drivers::ExitCtrl::exit(0)
     } else {
         cprintln!("FHT not loaded");