[opentitantool] do not create duplicate extension entries

It was observed that repetitive invoking 'opentitiantool image manifest update ...' passing an --spx_key and same manifest input file which included SPX key and signature extensions repetitively is causing addition of the SPX key and SPX signature space every time opentitantool is invoked. This patch fixes the problem. All opentitantool and opentitanlib tests pass, and running the same invocation does not cause additions of the SPX key and signature spaces any more. Signed-off-by: Vadim Bendebury <[email protected]>
lowRISC · Sep 17, 2024 · 7e34e67 · 7e34e67
1 parent 4b26cb4
commit 7e34e67
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 10 deletions.
diff --git a/sw/host/opentitanlib/src/image/manifest_def.rs b/sw/host/opentitanlib/src/image/manifest_def.rs
@@ -177,8 +177,23 @@ impl ManifestPacked<[ManifestExtTableEntry; CHIP_MANIFEST_EXT_TABLE_COUNT]>
 
     fn overwrite(&mut self, o: Self) {
         for i in 0..self.len() {
-            if !matches!(o[i].0, ManifestExtEntryVar::None) {
-                self[i].0 = o[i].0.clone();
+            match o[i].0 {
+                ManifestExtEntryVar::Name(other_id) => match self[i].0 {
+                    ManifestExtEntryVar::IdOffset {
+                        identifier: self_id,
+                        offset: _,
+                    } => {
+                        if self_id == other_id {
+                            // Do not overwrite existing entries with matching IDs.
+                            continue;
+                        } else {
+                            self[i].0 = o[i].0.clone()
+                        }
+                    }
+                    _ => self[i].0 = o[i].0.clone(),
+                },
+                ManifestExtEntryVar::None => (),
+                _ => self[i].0 = o[i].0.clone(),
             }
         }
     }

diff --git a/sw/host/opentitantool/src/command/image.rs b/sw/host/opentitantool/src/command/image.rs
@@ -185,7 +185,6 @@ impl CommandDispatch for ManifestUpdateCommand {
         image
             .manifest_sanity_check()
             .context("Image doesn't appear to contain a manifest, or the manifest is corrupted")?;
-
         // Load the manifest HJSON definition and update the image.
         if let Some(manifest) = &self.manifest {
             let def = ManifestSpec::read_from_file(manifest)?;
@@ -242,13 +241,21 @@ impl CommandDispatch for ManifestUpdateCommand {
             image.add_manifest_extension(key_ext)?;
             spx_private_key = sk;
 
-            // Allocate space for `spx_signature` (this impacts the manifest
-            // `length` field which is in the signed region of the image).
-            // Adding this facilitates offline signing.
-            image.allocate_manifest_extension(
-                ManifestExtId::spx_signature.into(),
-                std::mem::size_of::<ManifestExtSpxSignature>(),
-            )?;
+            if !image
+                .borrow_manifest()?
+                .extensions
+                .entries
+                .iter()
+                .any(|e| e.identifier == u32::from(ManifestExtId::spx_signature) && e.offset != 0)
+            {
+                // Allocate space for `spx_signature` (this impacts the manifest
+                // `length` field which is in the signed region of the image).
+                // Adding this facilitates offline signing.
+                image.allocate_manifest_extension(
+                    ManifestExtId::spx_signature.into(),
+                    std::mem::size_of::<ManifestExtSpxSignature>(),
+                )?;
+            }
         }
 
         // Update the manifest fields that are in the unsigned region.