Repository: chipsalliance/caliptra-sw
Pull request #565 (2023-07-31 19:50:30)

[fix] Always extend LMS vendor public key index into the PCR log.
This change fixes the logic to always extend the LMS vendor public key index into the PCR log. In the case where LMS is disabled, a value of 0xFF will be extended. This change also extends the ROM Verification Config  the PCR.


Patch:
diff --git a/common/src/pcr.rs b/common/src/pcr.rs
index f550954f..cdfdb87d 100644
--- a/common/src/pcr.rs
+++ b/common/src/pcr.rs
@@ -33,6 +33,7 @@ pub enum PcrLogEntryId {
     FmcSvn = 8,                // data size = 1 byte
     FmcFuseSvn = 9,            // data size = 1 byte
     LmsVendorPubKeyIndex = 10, // data size = 1 byte
+    RomVerifyConfig = 11,      // data size = 1 byte
 }
 
 impl From<u16> for PcrLogEntryId {
@@ -49,6 +50,7 @@ impl From<u16> for PcrLogEntryId {
             8 => PcrLogEntryId::FmcSvn,
             9 => PcrLogEntryId::FmcFuseSvn,
             10 => PcrLogEntryId::LmsVendorPubKeyIndex,
+            11 => PcrLogEntryId::RomVerifyConfig,
             _ => PcrLogEntryId::Invalid,
         }
     }
diff --git a/drivers/src/fuse_bank.rs b/drivers/src/fuse_bank.rs
index 5ab26ac7..50b5e9b1 100644
--- a/drivers/src/fuse_bank.rs
+++ b/drivers/src/fuse_bank.rs
@@ -61,7 +61,7 @@ impl From<IdevidCertAttr> for usize {
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub enum LmsVerifyConfig {
+pub enum RomVerifyConfig {
     EcdsaOnly = 0,
     EcdsaAndLms = 1,
 }
@@ -276,16 +276,16 @@ impl FuseBank<'_> {
     /// * None
     ///
     /// # Returns
-    ///     LmsVerifyConfig
+    ///     RomVerifyConfig
     ///         EcdsaOnly: Verify Caliptra firmware images with ECDSA-only
     ///         EcdsaAndLms: Verify Caliptra firmware images with ECDSA and LMS
     ///
-    pub fn lms_verify(&self) -> LmsVerifyConfig {
+    pub fn lms_verify(&self) -> RomVerifyConfig {
         let soc_ifc_regs = self.soc_ifc.regs();
         if !soc_ifc_regs.fuse_lms_verify().read().lms_verify() {
-            LmsVerifyConfig::EcdsaOnly
+            RomVerifyConfig::EcdsaOnly
         } else {
-            LmsVerifyConfig::EcdsaAndLms
+            RomVerifyConfig::EcdsaAndLms
         }
     }
 }
diff --git a/drivers/src/lib.rs b/drivers/src/lib.rs
index 12a89f7a..4cb4de92 100644
--- a/drivers/src/lib.rs
+++ b/drivers/src/lib.rs
@@ -58,7 +58,7 @@ pub use ecc384::{
 pub use error_reporter::{report_fw_error_fatal, report_fw_error_non_fatal};
 pub use exit_ctrl::ExitCtrl;
 pub use fuse_bank::{
-    FuseBank, IdevidCertAttr, LmsVerifyConfig, VendorPubKeyRevocation, X509KeyIdAlgo,
+    FuseBank, IdevidCertAttr, RomVerifyConfig, VendorPubKeyRevocation, X509KeyIdAlgo,
 };
 pub use hmac384::{Hmac384, Hmac384Data, Hmac384Key, Hmac384Op, Hmac384Tag};
 pub use hmac384_kdf::hmac384_kdf;
diff --git a/rom/dev/src/pcr.rs b/rom/dev/src/pcr.rs
index 94fcf9be..b9fb4524 100644
--- a/rom/dev/src/pcr.rs
+++ b/rom/dev/src/pcr.rs
@@ -91,12 +91,14 @@ pub(crate) fn extend_pcr0(
     pcr.extend(env.data_vault.fmc_tci(), PcrLogEntryId::FmcTci)?;
     pcr.extend_u8(env.data_vault.fmc_svn() as u8, PcrLogEntryId::FmcSvn)?;
     pcr.extend_u8(info.fmc.effective_fuse_svn as u8, PcrLogEntryId::FmcFuseSvn)?;
-    if let Some(vendor_lms_pub_key_idx) = info.vendor_lms_pub_key_idx {
-        pcr.extend_u8(
-            vendor_lms_pub_key_idx as u8,
-            PcrLogEntryId::LmsVendorPubKeyIndex,
-        )?;
-    }
+    pcr.extend_u8(
+        env.data_vault.lms_vendor_pk_index() as u8,
+        PcrLogEntryId::LmsVendorPubKeyIndex,
+    )?;
+    pcr.extend_u8(
+        env.soc_ifc.fuse_bank().lms_verify() as u8,
+        PcrLogEntryId::RomVerifyConfig,
+    )?;
 
     Ok(())
 }
diff --git a/rom/dev/src/verifier.rs b/rom/dev/src/verifier.rs
index 4ad5f3c9..1be81e83 100644
--- a/rom/dev/src/verifier.rs
+++ b/rom/dev/src/verifier.rs
@@ -143,6 +143,6 @@ impl<'a> ImageVerificationEnv for &mut RomImageVerificationEnv<'a> {
     }
 
     fn lms_verify_enabled(&self) -> bool {
-        self.soc_ifc.fuse_bank().lms_verify() == LmsVerifyConfig::EcdsaAndLms
+        self.soc_ifc.fuse_bank().lms_verify() == RomVerifyConfig::EcdsaAndLms
     }
 }


Test patch:
diff --git a/rom/dev/tests/test_fmcalias_derivation.rs b/rom/dev/tests/test_fmcalias_derivation.rs
index 8d70c515..1b05e8f2 100644
--- a/rom/dev/tests/test_fmcalias_derivation.rs
+++ b/rom/dev/tests/test_fmcalias_derivation.rs
@@ -4,7 +4,7 @@ use caliptra_builder::{FwId, ImageOptions, APP_WITH_UART, ROM_WITH_UART};
 use caliptra_common::RomBootStatus::ColdResetComplete;
 use caliptra_common::{FirmwareHandoffTable, FuseLogEntry, FuseLogEntryId};
 use caliptra_common::{PcrLogEntry, PcrLogEntryId};
-use caliptra_drivers::ColdResetEntry4;
+use caliptra_drivers::{ColdResetEntry4, RomVerifyConfig};
 use caliptra_error::CaliptraError;
 use caliptra_hw_model::{BootParams, Fuses, HwModel, InitParams, ModelError, SecurityState};
 use caliptra_image_fake_keys::{OWNER_CONFIG, VENDOR_CONFIG_KEY_1};
@@ -215,6 +215,17 @@ fn test_pcr_log() {
         pcr_log_entry.pcr_data[0] as u8,
         VENDOR_CONFIG_KEY_1.lms_key_idx as u8
     );
+
+    // Check Rom verify config
+    pcr_log_entry_offset += core::mem::size_of::<PcrLogEntry>();
+    let pcr_log_entry =
+        PcrLogEntry::read_from_prefix(pcr_entry_arr[pcr_log_entry_offset..].as_bytes()).unwrap();
+    assert_eq!(pcr_log_entry.id, PcrLogEntryId::RomVerifyConfig as u16);
+    assert_eq!(pcr_log_entry.pcr_id, 0);
+    assert_eq!(
+        pcr_log_entry.pcr_data[0] as u8,
+        RomVerifyConfig::EcdsaAndLms as u8
+    );
 }
 
 #[test]
diff --git a/test/src/derive.rs b/test/src/derive.rs
index 645d2f0c..efc5ea51 100644
--- a/test/src/derive.rs
+++ b/test/src/derive.rs
@@ -306,10 +306,12 @@ pub struct Pcr0Input {
     pub fuse_anti_rollback_disable: bool,
     pub vendor_pub_key_hash: [u32; 12],
     pub owner_pub_key_hash: [u32; 12],
-    pub vendor_pub_key_index: u32,
+    pub ecc_vendor_pub_key_index: u32,
     pub fmc_digest: [u32; 12],
     pub fmc_svn: u32,
     pub fmc_fuse_svn: u32,
+    pub lms_vendor_pub_key_index: u32,
+    pub rom_verify_config: u32,
 }
 impl Pcr0Input {}
 
@@ -333,10 +335,12 @@ impl Pcr0 {
             &mut value,
             swap_word_bytes(&input.owner_pub_key_hash).as_bytes(),
         );
-        extend(&mut value, &[input.vendor_pub_key_index as u8]);
+        extend(&mut value, &[input.ecc_vendor_pub_key_index as u8]);
         extend(&mut value, swap_word_bytes(&input.fmc_digest).as_bytes());
         extend(&mut value, &[input.fmc_svn as u8]);
         extend(&mut value, &[input.fmc_fuse_svn as u8]);
+        extend(&mut value, &[input.lms_vendor_pub_key_index as u8]);
+        extend(&mut value, &[input.rom_verify_config as u8]);
 
         let mut result: [u32; 12] = zerocopy::transmute!(value);
         swap_word_bytes_inplace(&mut result);
@@ -359,19 +363,21 @@ fn test_derive_pcr0() {
             0xdc1a27ef, 0x0c08201a, 0x8b066094, 0x118c29fe, 0x0bc2270e, 0xbd965c43, 0xf7b9a68d,
             0x8eaf37fa, 0x968ca8d8, 0x13b2920b, 0x3b88b026, 0xf2f0ebb0,
         ],
-        vendor_pub_key_index: 0,
+        ecc_vendor_pub_key_index: 0,
         fmc_digest: [
             0xe44ea855, 0x9fcf4063, 0xd3110a9a, 0xd60579db, 0xe03e6dd7, 0x4556cd98, 0xb2b941f5,
             0x1bb5034b, 0x587eea1f, 0xfcdd0e0f, 0x8e88d406, 0x3327a3fe,
         ],
         fmc_svn: 5,
         fmc_fuse_svn: 2,
+        lms_vendor_pub_key_index: u32::MAX,
+        rom_verify_config: 1, // RomVerifyConfig::EcdsaAndLms
     });
     assert_eq!(
         pcr0,
         Pcr0([
-            0xd8f3fd4, 0x4698aaae, 0x7bacaf67, 0x714a8035, 0x9a8d3a51, 0x3fcde890, 0x8039f4c1,
-            0x77f9d5a9, 0x77b8ecd5, 0xf29b3fa9, 0x30e25097, 0xe1d82b14,
+            0xB38A7B77, 0xC68EB393, 0x868CF1B6, 0x86D1A737, 0x865E57CE, 0xC776EB1D, 0x48D45DCF,
+            0xD6C90BA0, 0x7B5F5D82, 0x8B2CCBFF, 0xC204B621, 0xAD1193D5
         ],)
     )
 }
diff --git a/test/tests/smoke_test.rs b/test/tests/smoke_test.rs
index ca400ad8..79d015e4 100644
--- a/test/tests/smoke_test.rs
+++ b/test/tests/smoke_test.rs
@@ -271,11 +271,13 @@ fn smoke_test() {
             fuse_anti_rollback_disable: false,
             vendor_pub_key_hash: vendor_pk_hash,
             owner_pub_key_hash: owner_pk_hash,
-            vendor_pub_key_index: image.manifest.preamble.vendor_ecc_pub_key_idx,
+            ecc_vendor_pub_key_index: image.manifest.preamble.vendor_ecc_pub_key_idx,
             fmc_digest: image.manifest.fmc.digest,
             fmc_svn: image.manifest.fmc.svn,
             // This is from the SVN in the fuses (7 bits set)
             fmc_fuse_svn: 7,
+            lms_vendor_pub_key_index: u32::MAX,
+            rom_verify_config: 0, // RomVerifyConfig::EcdsaOnly
         }),
         &expected_ldevid_key,
     );