Repository: chipsalliance/caliptra-sw
Pull request #319 (2023-05-25 21:45:20)

[fix] Vendor signature should not include owner signed data
This change updates the vendor header signature computation to not include the owner signed data.


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 69b6c207..103e8122 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -384,6 +384,7 @@ dependencies = [
  "anyhow",
  "bitflags 2.0.2",
  "caliptra-image-types",
+ "memoffset",
  "zerocopy",
 ]
 
@@ -423,6 +424,7 @@ dependencies = [
  "bitflags 2.0.2",
  "caliptra-drivers",
  "caliptra-image-types",
+ "memoffset",
  "zerocopy",
 ]
 
@@ -460,6 +462,7 @@ dependencies = [
  "caliptra_common",
  "cfg-if",
  "hex",
+ "memoffset",
  "openssl",
  "ufmt",
  "zerocopy",
diff --git a/image/gen/Cargo.toml b/image/gen/Cargo.toml
index 346ff8b5..eb9c8083 100644
--- a/image/gen/Cargo.toml
+++ b/image/gen/Cargo.toml
@@ -13,3 +13,4 @@ caliptra-image-types = { path = "../types", features = ["std"] }
 bitflags = "2.0.1"
 zerocopy = "0.6.1"
 anyhow = "1.0.70"
+memoffset = "0.8.0"
diff --git a/image/gen/src/generator.rs b/image/gen/src/generator.rs
index d1483150..fef69e00 100644
--- a/image/gen/src/generator.rs
+++ b/image/gen/src/generator.rs
@@ -13,6 +13,7 @@ Abstract:
 --*/
 use anyhow::bail;
 use caliptra_image_types::*;
+use memoffset::offset_of;
 use zerocopy::AsBytes;
 
 use crate::*;
@@ -65,9 +66,15 @@ impl<Crypto: ImageGeneratorCrypto> ImageGenerator<Crypto> {
         let toc_digest = self.toc_digest(&fmc_toc, &runtime_toc)?;
         let header = self.gen_header(config, ecc_key_idx, Self::DEFAULT_FLAGS, toc_digest)?;
 
-        // Create Preamable
-        let header_digest = self.header_digest(&header)?;
-        let preamble = self.gen_preamble(config, ecc_key_idx, &header_digest)?;
+        // Create Preamble
+        let header_digest_vendor = self.header_digest_vendor(&header)?;
+        let header_digest_owner = self.header_digest_owner(&header)?;
+        let preamble = self.gen_preamble(
+            config,
+            ecc_key_idx,
+            &header_digest_vendor,
+            &header_digest_owner,
+        )?;
 
         // Create Manifest
         let manifest = ImageManifest {
@@ -94,7 +101,8 @@ impl<Crypto: ImageGeneratorCrypto> ImageGenerator<Crypto> {
         &self,
         config: &ImageGeneratorConfig<E>,
         ecc_key_idx: u32,
-        digest: &ImageDigest,
+        digest_vendor: &ImageDigest,
+        digest_owner: &ImageDigest,
     ) -> anyhow::Result<ImagePreamble>
     where
         E: ImageGenratorExecutable,
@@ -104,7 +112,7 @@ impl<Crypto: ImageGeneratorCrypto> ImageGenerator<Crypto> {
 
         if let Some(priv_keys) = config.vendor_config.priv_keys {
             let sig = self.crypto.ecdsa384_sign(
-                digest,
+                digest_vendor,
                 &priv_keys.ecc_priv_keys[ecc_key_idx as usize],
                 &config.vendor_config.pub_keys.ecc_pub_keys[ecc_key_idx as usize],
             )?;
@@ -114,7 +122,7 @@ impl<Crypto: ImageGeneratorCrypto> ImageGenerator<Crypto> {
         if let Some(owner_config) = &config.owner_config {
             if let Some(priv_keys) = &owner_config.priv_keys {
                 let sig = self.crypto.ecdsa384_sign(
-                    digest,
+                    digest_owner,
                     &priv_keys.ecc_priv_key,
                     &owner_config.pub_keys.ecc_pub_key,
                 )?;
@@ -156,8 +164,8 @@ impl<Crypto: ImageGeneratorCrypto> ImageGenerator<Crypto> {
             ..Default::default()
         };
 
-        header.vendor_not_before = config.vendor_config.not_before;
-        header.vendor_not_after = config.vendor_config.not_after;
+        header.vendor_data.vendor_not_before = config.vendor_config.not_before;
+        header.vendor_data.vendor_not_after = config.vendor_config.not_after;
 
         if let Some(owner_config) = &config.owner_config {
             header.owner_data.owner_not_before = owner_config.not_before;
@@ -167,8 +175,16 @@ impl<Crypto: ImageGeneratorCrypto> ImageGenerator<Crypto> {
         Ok(header)
     }
 
-    /// Calculate header digest
-    pub fn header_digest(&self, header: &ImageHeader) -> anyhow::Result<ImageDigest> {
+    /// Calculate header digest for vendor.
+    /// Vendor digest is calculated upto the `owner_data` field.
+    pub fn header_digest_vendor(&self, header: &ImageHeader) -> anyhow::Result<ImageDigest> {
+        let offset = offset_of!(ImageHeader, owner_data);
+        self.crypto
+            .sha384_digest(header.as_bytes().get(..offset).unwrap())
+    }
+
+    /// Calculate header digest for owner.
+    pub fn header_digest_owner(&self, header: &ImageHeader) -> anyhow::Result<ImageDigest> {
         self.crypto.sha384_digest(header.as_bytes())
     }
 
diff --git a/image/types/src/lib.rs b/image/types/src/lib.rs
index 238b96b5..b3533f8f 100644
--- a/image/types/src/lib.rs
+++ b/image/types/src/lib.rs
@@ -305,6 +305,18 @@ pub struct ImagePreamble {
     pub _rsvd: [u32; 2],
 }
 
+#[repr(C)]
+#[derive(AsBytes, FromBytes, Default, Debug)]
+pub struct VendorSignedData {
+    /// Vendor Start Date [ASN1 Time Format] For LDEV-Id certificate.
+    pub vendor_not_before: [u8; 15],
+
+    /// Vendor End Date [ASN1 Time Format] For LDEV-Id certificate.
+    pub vendor_not_after: [u8; 15],
+
+    reserved: [u8; 2],
+}
+
 #[repr(C)]
 #[derive(AsBytes, FromBytes, Default, Debug)]
 pub struct OwnerSignedData {
@@ -313,6 +325,8 @@ pub struct OwnerSignedData {
 
     /// Owner End Date [ASN1 Time Format] For LDEV-Id certificate: Takes Preference over vendor end date
     pub owner_not_after: [u8; 15],
+
+    reserved: [u8; 2],
 }
 
 /// Caliptra Image header
@@ -334,11 +348,8 @@ pub struct ImageHeader {
     /// TOC Digest
     pub toc_digest: ImageDigest,
 
-    /// Vendor Start Date [ASN1 Time Format] For LDEV-Id certificate
-    pub vendor_not_before: [u8; 15],
-
-    /// Vendor End Date [ASN1 Time Format] For LDEV-Id certificate
-    pub vendor_not_after: [u8; 15],
+    /// Vendor Data
+    pub vendor_data: VendorSignedData,
 
     /// The Signed owner data
     pub owner_data: OwnerSignedData,
diff --git a/image/verify/Cargo.toml b/image/verify/Cargo.toml
index aa893415..b1ac0906 100644
--- a/image/verify/Cargo.toml
+++ b/image/verify/Cargo.toml
@@ -13,6 +13,7 @@ caliptra-image-types = { path = "../types", default-features = false }
 caliptra-drivers = { path = "../../drivers" }
 bitflags = "2.0.1"
 zerocopy = "0.6.1"
+memoffset = "0.8.0"
 
 [features]
 default = ["std"]
diff --git a/image/verify/src/verifier.rs b/image/verify/src/verifier.rs
index 5e36623f..36aad1ad 100644
--- a/image/verify/src/verifier.rs
+++ b/image/verify/src/verifier.rs
@@ -17,6 +17,7 @@ use core::num::NonZeroU32;
 use crate::*;
 use caliptra_drivers::*;
 use caliptra_image_types::*;
+use memoffset::offset_of;
 
 const ZERO_DIGEST: ImageDigest = [0u32; SHA384_DIGEST_WORD_SIZE];
 
@@ -315,14 +316,22 @@ impl<Env: ImageVerificationEnv> ImageVerifier<Env> {
     ) -> CaliptraResult<TocInfo<'a>> {
         // Calculate the digest for the header
         let range = ImageManifest::header_range();
-        let digest = self
+        let vendor_header_len = offset_of!(ImageHeader, owner_data);
+
+        // Vendor header digest is calculated up to the owner_data field.
+        let digest_vendor = self
+            .env
+            .sha384_digest(range.start, vendor_header_len as u32)
+            .map_err(|_| err_u32!(HeaderDigestFailure))?;
+
+        let digest_owner = self
             .env
             .sha384_digest(range.start, range.len() as u32)
             .map_err(|_| err_u32!(HeaderDigestFailure))?;
 
         // Verify vendor signature
         let (pub_key, sig) = info.vendor_info;
-        self.verify_vendor_sig(&digest, pub_key, sig)?;
+        self.verify_vendor_sig(&digest_vendor, pub_key, sig)?;
 
         // Verify the ECC public key index used verify header signature is encoded
         // in the header
@@ -332,7 +341,7 @@ impl<Env: ImageVerificationEnv> ImageVerifier<Env> {
 
         // Verify owner signature
         if let Some((pub_key, sig)) = info.owner_info {
-            self.verify_owner_sig(&digest, pub_key, sig)?;
+            self.verify_owner_sig(&digest_owner, pub_key, sig)?;
         }
 
         let verif_info = TocInfo {
diff --git a/rom/dev/Cargo.toml b/rom/dev/Cargo.toml
index 2905b87a..58e3ecd2 100644
--- a/rom/dev/Cargo.toml
+++ b/rom/dev/Cargo.toml
@@ -33,6 +33,7 @@ caliptra-image-fake-keys = { path = "../../image/fake-keys" }
 caliptra-image-types = { path = "../../image/types" }
 hex = "0.4.3"
 openssl = "0.10"
+memoffset = "0.8.0"
 
 [features]
 riscv = []
diff --git a/rom/dev/src/flow/cold_reset/fmc_alias.rs b/rom/dev/src/flow/cold_reset/fmc_alias.rs
index a994f1cd..ee8475cb 100644
--- a/rom/dev/src/flow/cold_reset/fmc_alias.rs
+++ b/rom/dev/src/flow/cold_reset/fmc_alias.rs
@@ -134,11 +134,11 @@ impl DiceLayer for FmcAliasLayer {
         let mut nf = NotAfter::default();
         let null_time = [0u8; 15];
 
-        if manifest.header.vendor_not_after != null_time
-            && manifest.header.vendor_not_before != null_time
+        if manifest.header.vendor_data.vendor_not_after != null_time
+            && manifest.header.vendor_data.vendor_not_before != null_time
         {
-            nf.not_after = manifest.header.vendor_not_after;
-            nb.not_before = manifest.header.vendor_not_before;
+            nf.not_after = manifest.header.vendor_data.vendor_not_after;
+            nb.not_before = manifest.header.vendor_data.vendor_not_before;
         }
 
         //The owner values takes preference


Test patch:
diff --git a/rom/dev/tests/test_image_validation.rs b/rom/dev/tests/test_image_validation.rs
index 055f62f5..179ec0fd 100644
--- a/rom/dev/tests/test_image_validation.rs
+++ b/rom/dev/tests/test_image_validation.rs
@@ -1258,12 +1258,19 @@ fn update_header(image_bundle: &mut ImageBundle) {
     };
 
     let gen = ImageGenerator::new(OsslCrypto::default());
-    let digest = gen.header_digest(&image_bundle.manifest.header).unwrap();
+    let header_digest_vendor = gen
+        .header_digest_vendor(&image_bundle.manifest.header)
+        .unwrap();
+    let header_digest_owner = gen
+        .header_digest_owner(&image_bundle.manifest.header)
+        .unwrap();
+
     image_bundle.manifest.preamble = gen
         .gen_preamble(
             &config,
             image_bundle.manifest.preamble.vendor_ecc_pub_key_idx,
-            &digest,
+            &header_digest_vendor,
+            &header_digest_owner,
         )
         .unwrap();
 }