Move SVN field from ImageTocEntry to ImageHeader.

The SVN field now reflects the image as a whole.
chipsalliance · Jan 27, 2025 · e07ef77 · e07ef77
1 parent b11afd1
commit e07ef77
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 17 deletions.
diff --git a/image/gen/src/generator.rs b/image/gen/src/generator.rs
@@ -352,6 +352,7 @@ impl<Crypto: ImageGeneratorCrypto> ImageGenerator<Crypto> {
             flags: Self::DEFAULT_FLAGS,
             toc_len: MAX_TOC_ENTRY_COUNT,
             toc_digest: digest,
+            svn: config.runtime.svn(),
             ..Default::default()
         };
 
@@ -427,8 +428,7 @@ impl<Crypto: ImageGeneratorCrypto> ImageGenerator<Crypto> {
             r#type: r#type.into(),
             revision: *image.rev(),
             version: image.version(),
-            svn: image.svn(),
-            reserved: 0,
+            reserved: [0; 2],
             load_addr: image.load_addr(),
             entry_point: image.entry_point(),
             offset,

diff --git a/image/types/src/lib.rs b/image/types/src/lib.rs
@@ -275,6 +275,8 @@ impl ImageBundle {
 }
 
 /// Calipatra Image Manifest
+///
+/// NOTE: only the header, fmc, and runtime portions of this struct are covered by signature.
 #[repr(C)]
 #[derive(AsBytes, FromBytes, Clone, Copy, Debug, Zeroize)]
 #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
@@ -521,6 +523,8 @@ pub struct ImageHeader {
     /// TOC Digest
     pub toc_digest: ImageDigest384,
 
+    pub svn: u32,
+
     /// Vendor Data
     pub vendor_data: VendorSignedData,
 
@@ -574,12 +578,8 @@ pub struct ImageTocEntry {
     // Firmware release number
     pub version: u32,
 
-    /// Security Version Number
-    /// Only read for Runtime entries. Not read for FMC.
-    pub svn: u32,
-
     /// Reserved field
-    pub reserved: u32,
+    pub reserved: [u32; 2],
 
     /// Entry Point
     pub load_addr: u32,

diff --git a/image/verify/src/verifier.rs b/image/verify/src/verifier.rs
@@ -128,11 +128,7 @@ impl<Env: ImageVerificationEnv> ImageVerifier<Env> {
         // Verify Runtime
         let runtime_info = self.verify_runtime(image_info.runtime)?;
 
-        // The FMC and RT FW images within the bundle each include an SVN, for
-        // backwards compatibility with 1.x. ROM only inspects the RT FW SVN.
-        let fw_svn = image_info.runtime.svn;
-
-        self.verify_svn(fw_svn)?;
+        self.verify_svn(manifest.header.svn)?;
 
         let effective_fuse_svn = self.effective_fuse_svn();
 
@@ -143,15 +139,15 @@ impl<Env: ImageVerificationEnv> ImageVerifier<Env> {
             owner_pub_keys_digest_in_fuses: header_info.owner_pub_keys_digest_in_fuses,
             fmc: fmc_info,
             runtime: runtime_info,
-            fw_svn,
+            fw_svn: manifest.header.svn,
             effective_fuse_svn,
             log_info: ImageVerificationLogInfo {
                 vendor_ecc_pub_key_idx: header_info.vendor_ecc_pub_key_idx,
                 fuse_vendor_ecc_pub_key_revocation: header_info.vendor_ecc_pub_key_revocation,
                 fuse_vendor_pqc_pub_key_revocation: header_info.vendor_pqc_pub_key_revocation,
                 vendor_pqc_pub_key_idx: header_info.vendor_pqc_pub_key_idx,
                 fw_log_info: FirmwareSvnLogInfo {
-                    manifest_svn: fw_svn,
+                    manifest_svn: manifest.header.svn,
                     reserved: 0,
                     fuse_svn: self.env.runtime_fuse_svn(),
                 },
@@ -2224,7 +2220,6 @@ mod tests {
         let verify_info = ImageTocEntry {
             load_addr: ICCM_ORG,
             entry_point: ICCM_ORG,
-            svn: 1,
             size: 100,
             ..Default::default()
         };
@@ -2287,7 +2282,6 @@ mod tests {
         let verify_info = ImageTocEntry {
             load_addr: ICCM_ORG,
             entry_point: ICCM_ORG,
-            svn: 1,
             size: 100,
             ..Default::default()
         };

diff --git a/rom/dev/tests/rom_integration_tests/test_image_validation.rs b/rom/dev/tests/rom_integration_tests/test_image_validation.rs
@@ -2156,6 +2156,22 @@ fn test_runtime_svn_less_than_fuse_svn() {
     }
 }
 
+#[test]
+fn test_runtime_svn_corruption() {
+    let (mut hw, mut image_bundle) = hw_and_mldsa_image_bundle();
+
+    // Change SVN.
+    image_bundle.manifest.header.svn = image_bundle.manifest.header.svn + 1;
+
+    assert_eq!(
+        hw.upload_firmware(&image_bundle.to_bytes().unwrap())
+            .unwrap_err(),
+        ModelError::MailboxCmdFailed(
+            CaliptraError::IMAGE_VERIFIER_ERR_VENDOR_ECC_SIGNATURE_INVALID.into()
+        )
+    );
+}
+
 #[test]
 fn cert_test_with_custom_dates() {
     for pqc_key_type in helpers::PQC_KEY_TYPE.iter() {

diff --git a/test/tests/caliptra_integration_tests/smoke_test.rs b/test/tests/caliptra_integration_tests/smoke_test.rs
@@ -352,7 +352,7 @@ fn smoke_test() {
             owner_pub_key_hash_from_fuses: true,
             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,
+            fmc_svn: image.manifest.header.svn,
             // This is from the SVN in the fuses (7 bits set)
             fmc_fuse_svn: 7,
             lms_vendor_pub_key_index: image.manifest.header.vendor_pqc_pub_key_idx,