Add additional checks to ROM_EXT signer
This change adds the checks in
`sw/device/silicon_creator/lib/manifest.h` to the signer so that
errors can be caught earlier.
Signed-off-by: Alphan Ulusoy <alphan@google.com>
diff --git a/sw/device/silicon_creator/lib/manifest.h b/sw/device/silicon_creator/lib/manifest.h
index fe72d94..c9a9830 100644
--- a/sw/device/silicon_creator/lib/manifest.h
+++ b/sw/device/silicon_creator/lib/manifest.h
@@ -146,11 +146,9 @@
/**
* Allowed bounds for the `length` field.
*/
-enum {
- // FIXME: Update min value after we have a fairly representative ROM_EXT.
- kManifestLengthMin = sizeof(manifest_t),
- kManifestLengthMax = 64 * 1024,
-};
+// FIXME: Update min value after we have a fairly representative ROM_EXT.
+#define MANIFEST_LENGTH_FIELD_MIN MANIFEST_SIZE
+#define MANIFEST_LENGTH_FIELD_MAX 65536
/**
* Gets the signed region of an image.
@@ -164,8 +162,8 @@
*/
inline rom_error_t manifest_signed_region_get(
const manifest_t *manifest, manifest_signed_region_t *signed_region) {
- if (manifest->length < kManifestLengthMin ||
- manifest->length > kManifestLengthMax) {
+ if (manifest->length < MANIFEST_LENGTH_FIELD_MIN ||
+ manifest->length > MANIFEST_LENGTH_FIELD_MAX) {
return kErrorManifestBadLength;
}
*signed_region = (manifest_signed_region_t){
diff --git a/sw/device/silicon_creator/lib/manifest_unittest.cc b/sw/device/silicon_creator/lib/manifest_unittest.cc
index 9093415..06e6dfe 100644
--- a/sw/device/silicon_creator/lib/manifest_unittest.cc
+++ b/sw/device/silicon_creator/lib/manifest_unittest.cc
@@ -26,11 +26,11 @@
manifest_t manifest{};
manifest_signed_region_t signed_region;
- manifest.length = kManifestLengthMax + 1;
+ manifest.length = MANIFEST_LENGTH_FIELD_MAX + 1;
EXPECT_EQ(manifest_signed_region_get(&manifest, &signed_region),
kErrorManifestBadLength);
- manifest.length = kManifestLengthMin - 1;
+ manifest.length = MANIFEST_LENGTH_FIELD_MIN - 1;
EXPECT_EQ(manifest_signed_region_get(&manifest, &signed_region),
kErrorManifestBadLength);
}
diff --git a/sw/host/rom_ext_image_tools/signer/image/src/manifest.rs b/sw/host/rom_ext_image_tools/signer/image/src/manifest.rs
index a5539a2..a22166d 100644
--- a/sw/host/rom_ext_image_tools/signer/image/src/manifest.rs
+++ b/sw/host/rom_ext_image_tools/signer/image/src/manifest.rs
@@ -27,6 +27,8 @@
// -- -I./ -Isw/device/lib/base/freestanding
pub const MANIFEST_SIZE: u32 = 848;
+pub const MANIFEST_LENGTH_FIELD_MIN: u32 = 848;
+pub const MANIFEST_LENGTH_FIELD_MAX: u32 = 65536;
/// Manifest for boot stage images stored in flash.
#[repr(C)]
diff --git a/sw/host/rom_ext_image_tools/signer/src/main.rs b/sw/host/rom_ext_image_tools/signer/src/main.rs
index d574606..55db46c 100644
--- a/sw/host/rom_ext_image_tools/signer/src/main.rs
+++ b/sw/host/rom_ext_image_tools/signer/src/main.rs
@@ -122,19 +122,6 @@
..Default::default()
};
- // Code region must be 32-bit aligned due to PMP constraints.
- fn is_word_aligned(addr: u32) -> bool {
- return addr % 4 == 0;
- }
- ensure!(
- is_word_aligned(image.manifest.code_start),
- "Unaligned code start."
- );
- ensure!(
- is_word_aligned(image.manifest.code_end),
- "Unaligned code end."
- );
-
let exponent_be = key.public_exponent_be();
let dest = image.manifest.exponent.as_bytes_mut().iter_mut();
let src = exponent_be.iter().rev().copied();
@@ -151,6 +138,44 @@
*d = s;
}
+ /// Checks if an address is word (32-bit) aligned.
+ fn is_word_aligned(addr: u32) -> bool {
+ return addr % 4 == 0;
+ }
+
+ ensure!(
+ is_word_aligned(image.manifest.code_start),
+ "`code_start` is not word aligned."
+ );
+ ensure!(
+ is_word_aligned(image.manifest.code_end),
+ "`code_end` is not word aligned."
+ );
+ ensure!(
+ is_word_aligned(image.manifest.entry_point),
+ "`entry_point` is not word aligned."
+ );
+ ensure!(
+ (manifest::MANIFEST_SIZE..image.manifest.code_end)
+ .contains(&image.manifest.code_start),
+ "`code_start` is outside the allowed range."
+ );
+ ensure!(
+ (manifest::MANIFEST_SIZE..=image.manifest.length)
+ .contains(&image.manifest.code_end),
+ "`code_end` is outside the allowed range."
+ );
+ ensure!(
+ (image.manifest.code_start..image.manifest.code_end)
+ .contains(&image.manifest.entry_point),
+ "`entry_point` is outside the code region."
+ );
+ ensure!(
+ (manifest::MANIFEST_LENGTH_FIELD_MIN..=manifest::MANIFEST_LENGTH_FIELD_MAX)
+ .contains(&image.manifest.length),
+ "`length` is outside the allowed range."
+ );
+
Ok(())
}
