From 254a4e4a542e4decf221a645a918aeb587708d89 Mon Sep 17 00:00:00 2001 From: Brian Fjeldstad Date: Thu, 8 Oct 2026 16:08:27 +0000 Subject: [PATCH 1/5] engine: deploy and open verity root hash signature partitions Implements install/update support for the optional verity root hash signature partition introduced in the previous two PRs of this stack. - `deploy_images` streams the signature partition's content from the OS image when both the image and the Host Configuration declare a signature device; errors if only one side declares it. - `setup_verity_devices` prefers the new declarative signature partition when `VerityDevice.signature_device_id` is set: reads the PKCS#7/DER signature directly from the raw block device (no mount needed, since the DER encoding is self-delimiting), writes it to a temp file, and opens the verity device with `veritysetup open --root-hash-signature=...`. Falls back to the existing `internal_params.veritySignaturePaths` file-based mechanism, then to a plain open, preserving prior behavior when no signature partition is configured. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/trident/src/engine/storage/image.rs | 40 ++++++++ crates/trident/src/engine/storage/verity.rs | 100 ++++++++++++++++++++ 2 files changed, 140 insertions(+) diff --git a/crates/trident/src/engine/storage/image.rs b/crates/trident/src/engine/storage/image.rs index 235f7f4507..1d66935563 100644 --- a/crates/trident/src/engine/storage/image.rs +++ b/crates/trident/src/engine/storage/image.rs @@ -120,6 +120,46 @@ pub(super) fn deploy_images(ctx: &EngineContext) -> Result<(), TridentError> { FileSystemResize::NoResize, ), ); + + // If the OS image carries a detached root hash signature, deploy + // it to the partition declared for it in the Host Configuration. + match ( + image_file_verity.signature_image_file.as_ref(), + verity_device.signature_device_id.as_ref(), + ) { + (Some(signature_image_file), Some(signature_device_id)) => { + combined_images.insert( + signature_image_file.path.clone(), + ( + signature_device_id.clone(), + metric_label, + signature_image_file, + FileSystemResize::NoResize, + ), + ); + } + (Some(_), None) => { + return Err(TridentError::new(InternalError::Internal( + "OS image has a verity root hash signature, but no signature device is \ + declared in the Host Configuration", + ))) + .message(format!( + "Verity device '{id}' has a signature in the OS image but no \ + signature_device_id in the Host Configuration" + )); + } + (None, Some(_)) => { + return Err(TridentError::new(InternalError::Internal( + "Host Configuration declares a verity root hash signature device, but \ + the OS image has no corresponding signature", + ))) + .message(format!( + "Verity device '{id}' has a signature_device_id in the Host \ + Configuration but no signature in the OS image" + )); + } + (None, None) => {} + } } else { // For non-verity devices, we can deploy the image directly. diff --git a/crates/trident/src/engine/storage/verity.rs b/crates/trident/src/engine/storage/verity.rs index 9b4c613ffb..ee06371ab5 100644 --- a/crates/trident/src/engine/storage/verity.rs +++ b/crates/trident/src/engine/storage/verity.rs @@ -113,6 +113,23 @@ pub(super) fn setup_verity_devices(ctx: &EngineContext) -> Result<(), Error> { // Create the internal representation of the verity device. let verity_dev = VerityDeviceUtils::new(update_name, data_dev, hash_dev, root_hash); + // Prefer the declarative signature partition approach, if a signature + // device is configured on the verity device. + if let Some(signature_device_id) = verity_device.signature_device_id.as_ref() { + return open_verity_device_with_signature_partition( + ctx, + &verity_device.id, + verity_dev, + signature_device_id, + ) + .with_context(|| { + format!( + "Failed to open verity device '{}' with signature partition '{}'", + verity_device.id, signature_device_id + ) + }); + } + // Check internal parameters for verity signatures. if let Some(signature_file_map) = ctx .spec @@ -301,6 +318,89 @@ fn open_verity_device_with_signature( )) } +/// Open a verity device using a root hash signature read directly from a +/// dedicated signature partition. +/// +/// This is the declarative counterpart to [`open_verity_device_with_signature`]: +/// instead of locating a signature file inside a mounted filesystem, the +/// signature bytes are read directly from the raw block device declared via +/// `VerityDevice.signature_device_id`. No mount is required. The PKCS#7/DER +/// encoding is self-delimiting, so trailing zero-padding in a partition +/// larger than the signature blob itself is harmless. +/// +/// The certificate matching the signature MUST exist in the kernel keyring, +/// otherwise the operation WILL fail. +fn open_verity_device_with_signature_partition( + ctx: &EngineContext, + verity_device_id: &BlockDeviceId, + verity_device: VerityDeviceUtils, + signature_device_id: &BlockDeviceId, +) -> Result<(), Error> { + let signature_block_device_path = ctx + .get_block_device_path(signature_device_id) + .with_context(|| { + format!("Failed to find path for block device '{signature_device_id}'") + })?; + + debug!( + "Reading root hash signature for verity device '{}' from partition '{}' [{}]", + verity_device_id, + signature_device_id, + signature_block_device_path.display() + ); + + let signature_bytes = fs::read(&signature_block_device_path).with_context(|| { + format!( + "Failed to read signature partition '{}' [{}]", + signature_device_id, + signature_block_device_path.display() + ) + })?; + + // Create a temporary file to hold a copy of the signature, since + // veritysetup expects a path to a file containing the signature. + let temp_signature_file_path = NamedTempFile::new() + .context("Failed to create temporary file for verity signature")? + .into_temp_path(); + + fs::write(&temp_signature_file_path, &signature_bytes).with_context(|| { + format!( + "Failed to write signature to temporary file '{}'", + temp_signature_file_path.display() + ) + })?; + + // Try to print signature info + match veritysetup::get_verity_signature_info(&temp_signature_file_path) { + Ok(signature_info) => { + debug!( + "Signature partition '{}' for verity device '{}' info:\n{}", + signature_device_id, verity_device_id, signature_info + ); + } + Err(e) => { + warn!( + "Failed to get signature info from partition '{}': {e:?}", + signature_device_id, + ); + } + } + + debug!( + "Opening verity device '{}' with signature from partition '{}' [{}]", + verity_device_id, + signature_device_id, + temp_signature_file_path.display(), + ); + + verity_device + .open_with_signature(&temp_signature_file_path) + .context(format!( + "Failed to open verity device '{}' with signature from partition '{}'", + verity_device_id, signature_device_id + )) +} + /// Get the verity data and hash paths. /// /// Verity data and hash devices are fetched from the engine context. From b1e1993b57766369fc8e70d0feff12fdc98c0560 Mon Sep 17 00:00:00 2001 From: Brian Fjeldstad Date: Thu, 8 Oct 2026 16:38:49 +0000 Subject: [PATCH 2/5] engine: wire VerityHashSignatureDevice graph reference Construct the special storage-graph reference connecting a verity device to its optional root hash signature device. This activates the validation rules added in the HostConfig API PR (restricting the signature device to a LinuxGeneric partition, enforcing homogeneous partition types, etc.), and is paired with the engine-side install/update support in this PR. --- crates/trident/src/engine/storage/image.rs | 10 +++---- crates/trident/src/engine/storage/verity.rs | 26 +++++++++---------- .../config/host/storage/storage_graph/node.rs | 12 +++++++-- .../host/storage/storage_graph/references.rs | 1 - 4 files changed, 28 insertions(+), 21 deletions(-) diff --git a/crates/trident/src/engine/storage/image.rs b/crates/trident/src/engine/storage/image.rs index 1d66935563..7d0fe18cb0 100644 --- a/crates/trident/src/engine/storage/image.rs +++ b/crates/trident/src/engine/storage/image.rs @@ -125,13 +125,13 @@ pub(super) fn deploy_images(ctx: &EngineContext) -> Result<(), TridentError> { // it to the partition declared for it in the Host Configuration. match ( image_file_verity.signature_image_file.as_ref(), - verity_device.signature_device_id.as_ref(), + verity_device.hash_signature_device_id.as_ref(), ) { - (Some(signature_image_file), Some(signature_device_id)) => { + (Some(signature_image_file), Some(hash_signature_device_id)) => { combined_images.insert( signature_image_file.path.clone(), ( - signature_device_id.clone(), + hash_signature_device_id.clone(), metric_label, signature_image_file, FileSystemResize::NoResize, @@ -145,7 +145,7 @@ pub(super) fn deploy_images(ctx: &EngineContext) -> Result<(), TridentError> { ))) .message(format!( "Verity device '{id}' has a signature in the OS image but no \ - signature_device_id in the Host Configuration" + hash_signature_device_id in the Host Configuration" )); } (None, Some(_)) => { @@ -154,7 +154,7 @@ pub(super) fn deploy_images(ctx: &EngineContext) -> Result<(), TridentError> { the OS image has no corresponding signature", ))) .message(format!( - "Verity device '{id}' has a signature_device_id in the Host \ + "Verity device '{id}' has a hash_signature_device_id in the Host \ Configuration but no signature in the OS image" )); } diff --git a/crates/trident/src/engine/storage/verity.rs b/crates/trident/src/engine/storage/verity.rs index ee06371ab5..a408073d4c 100644 --- a/crates/trident/src/engine/storage/verity.rs +++ b/crates/trident/src/engine/storage/verity.rs @@ -115,17 +115,17 @@ pub(super) fn setup_verity_devices(ctx: &EngineContext) -> Result<(), Error> { // Prefer the declarative signature partition approach, if a signature // device is configured on the verity device. - if let Some(signature_device_id) = verity_device.signature_device_id.as_ref() { + if let Some(hash_signature_device_id) = verity_device.hash_signature_device_id.as_ref() { return open_verity_device_with_signature_partition( ctx, &verity_device.id, verity_dev, - signature_device_id, + hash_signature_device_id, ) .with_context(|| { format!( "Failed to open verity device '{}' with signature partition '{}'", - verity_device.id, signature_device_id + verity_device.id, hash_signature_device_id ) }); } @@ -324,7 +324,7 @@ fn open_verity_device_with_signature( /// This is the declarative counterpart to [`open_verity_device_with_signature`]: /// instead of locating a signature file inside a mounted filesystem, the /// signature bytes are read directly from the raw block device declared via -/// `VerityDevice.signature_device_id`. No mount is required. The PKCS#7/DER +/// `VerityDevice.hash_signature_device_id`. No mount is required. The PKCS#7/DER /// encoding is self-delimiting, so trailing zero-padding in a partition /// larger than the signature blob itself is harmless. /// @@ -334,25 +334,25 @@ fn open_verity_device_with_signature_partition( ctx: &EngineContext, verity_device_id: &BlockDeviceId, verity_device: VerityDeviceUtils, - signature_device_id: &BlockDeviceId, + hash_signature_device_id: &BlockDeviceId, ) -> Result<(), Error> { let signature_block_device_path = ctx - .get_block_device_path(signature_device_id) + .get_block_device_path(hash_signature_device_id) .with_context(|| { - format!("Failed to find path for block device '{signature_device_id}'") + format!("Failed to find path for block device '{hash_signature_device_id}'") })?; debug!( "Reading root hash signature for verity device '{}' from partition '{}' [{}]", verity_device_id, - signature_device_id, + hash_signature_device_id, signature_block_device_path.display() ); let signature_bytes = fs::read(&signature_block_device_path).with_context(|| { format!( "Failed to read signature partition '{}' [{}]", - signature_device_id, + hash_signature_device_id, signature_block_device_path.display() ) })?; @@ -375,13 +375,13 @@ fn open_verity_device_with_signature_partition( Ok(signature_info) => { debug!( "Signature partition '{}' for verity device '{}' info:\n{}", - signature_device_id, verity_device_id, signature_info + hash_signature_device_id, verity_device_id, signature_info ); } Err(e) => { warn!( "Failed to get signature info from partition '{}': {e:?}", - signature_device_id, + hash_signature_device_id, ); } } @@ -389,7 +389,7 @@ fn open_verity_device_with_signature_partition( debug!( "Opening verity device '{}' with signature from partition '{}' [{}]", verity_device_id, - signature_device_id, + hash_signature_device_id, temp_signature_file_path.display(), ); @@ -397,7 +397,7 @@ fn open_verity_device_with_signature_partition( .open_with_signature(&temp_signature_file_path) .context(format!( "Failed to open verity device '{}' with signature from partition '{}'", - verity_device_id, signature_device_id + verity_device_id, hash_signature_device_id )) } diff --git a/crates/trident_api/src/config/host/storage/storage_graph/node.rs b/crates/trident_api/src/config/host/storage/storage_graph/node.rs index 2c87cca640..5e8a5d0e9c 100644 --- a/crates/trident_api/src/config/host/storage/storage_graph/node.rs +++ b/crates/trident_api/src/config/host/storage/storage_graph/node.rs @@ -144,7 +144,7 @@ impl StorageGraphNode { vec![StorageReference::new_regular(&encrypted_volume.device_id)] } HostConfigBlockDevice::VerityDevice(verity_device) => { - vec![ + let mut refs = vec![ StorageReference::new_special( SpecialReferenceKind::VerityDataDevice, &verity_device.data_device_id, @@ -153,7 +153,15 @@ impl StorageGraphNode { SpecialReferenceKind::VerityHashDevice, &verity_device.hash_device_id, ), - ] + ]; + if let Some(hash_signature_device_id) = &verity_device.hash_signature_device_id + { + refs.push(StorageReference::new_special( + SpecialReferenceKind::VerityHashSignatureDevice, + hash_signature_device_id, + )); + } + refs } }, Self::FileSystem(fs) => fs diff --git a/crates/trident_api/src/config/host/storage/storage_graph/references.rs b/crates/trident_api/src/config/host/storage/storage_graph/references.rs index 83260e41e2..630f6b20bd 100644 --- a/crates/trident_api/src/config/host/storage/storage_graph/references.rs +++ b/crates/trident_api/src/config/host/storage/storage_graph/references.rs @@ -31,7 +31,6 @@ pub enum SpecialReferenceKind { VerityHashDevice, /// A reference to a Verity device's root hash signature device. - #[allow(dead_code)] VerityHashSignatureDevice, } From 7f06881f657f82713417aaf4426f049231941b53 Mon Sep 17 00:00:00 2001 From: Brian Fjeldstad Date: Thu, 8 Oct 2026 20:36:57 +0000 Subject: [PATCH 3/5] engine: bound signature partition read, add tests, fix cardinality Addresses review feedback on PR #836: - Bound the number of bytes read from a dedicated verity root hash signature partition (extracted into read_signature_partition_bytes) instead of fs::read()-ing the whole block device. A Grow-sized signature partition can no longer be read entirely into memory; oversized partitions are now rejected with a clear error. - Add unit tests: read_signature_partition_bytes bounds enforcement, and a setup_verity_devices test proving the declarative hash_signature_device_id path takes precedence over the legacy veritySignaturePaths internal parameter. While adding that precedence test, found and fixed a latent bug it would otherwise have hit: BlkDevReferrerKind::VerityDevice::valid_target_count() was still hardcoded to exactly 2 targets, so any verity device that actually sets hash_signature_device_id (the whole point of this feature) fails storage graph validation with InvalidTargetCount before ever reaching the engine. Widened to accept 2-3 targets. All existing trident_api/trident unit tests (195 + 407) still pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/trident/src/engine/storage/verity.rs | 185 +++++++++++++++++- .../host/storage/storage_graph/rules/mod.rs | 2 +- 2 files changed, 177 insertions(+), 10 deletions(-) diff --git a/crates/trident/src/engine/storage/verity.rs b/crates/trident/src/engine/storage/verity.rs index a408073d4c..70df41b7f0 100644 --- a/crates/trident/src/engine/storage/verity.rs +++ b/crates/trident/src/engine/storage/verity.rs @@ -1,6 +1,7 @@ use std::{ collections::{HashMap, HashSet}, fs, + io::Read, path::{Path, PathBuf}, }; @@ -318,6 +319,44 @@ fn open_verity_device_with_signature( )) } +/// Upper bound, in bytes, on data read from a dedicated verity root hash +/// signature partition. A detached PKCS#7 signature (including an embedded +/// certificate chain) is at most a few tens of KiB; this generous cap keeps +/// memory and temporary-file usage bounded regardless of the signature +/// partition's declared size (which, like the hash partition, MAY use +/// `PartitionSize::Grow` and consume the remainder of the disk). +const MAX_VERITY_SIGNATURE_PARTITION_SIZE: u64 = 1024 * 1024; // 1 MiB + +/// Reads up to [`MAX_VERITY_SIGNATURE_PARTITION_SIZE`] bytes from `path`, +/// returning an error if the file/device holds more than that. This bounds +/// memory usage when reading a signature partition whose declared size may be +/// much larger than the actual PKCS#7 signature blob it holds (e.g. a +/// `PartitionSize::Grow` signature partition). +fn read_signature_partition_bytes(path: &Path) -> Result, Error> { + let mut file = fs::File::open(path) + .with_context(|| format!("Failed to open signature partition '{}'", path.display()))?; + + // Read at most MAX_VERITY_SIGNATURE_PARTITION_SIZE + 1 bytes: if we get + // back more than the cap, the partition holds more than a plausible + // signature and we reject it below, rather than silently truncating it. + let mut bytes = Vec::new(); + let bytes_read = file + .by_ref() + .take(MAX_VERITY_SIGNATURE_PARTITION_SIZE + 1) + .read_to_end(&mut bytes) + .with_context(|| format!("Failed to read signature partition '{}'", path.display()))?; + + ensure!( + bytes_read as u64 <= MAX_VERITY_SIGNATURE_PARTITION_SIZE, + "Signature partition '{}' exceeds the maximum expected size of {} bytes for a verity \ + root hash signature", + path.display(), + MAX_VERITY_SIGNATURE_PARTITION_SIZE + ); + + Ok(bytes) +} + /// Open a verity device using a root hash signature read directly from a /// dedicated signature partition. /// @@ -326,7 +365,10 @@ fn open_verity_device_with_signature( /// signature bytes are read directly from the raw block device declared via /// `VerityDevice.hash_signature_device_id`. No mount is required. The PKCS#7/DER /// encoding is self-delimiting, so trailing zero-padding in a partition -/// larger than the signature blob itself is harmless. +/// larger than the signature blob itself is harmless. The read itself is +/// bounded (see [`read_signature_partition_bytes`]) so that an oversized +/// (e.g. `Grow`-sized) signature partition cannot be read entirely into +/// memory. /// /// The certificate matching the signature MUST exist in the kernel keyring, /// otherwise the operation WILL fail. @@ -349,13 +391,13 @@ fn open_verity_device_with_signature_partition( signature_block_device_path.display() ); - let signature_bytes = fs::read(&signature_block_device_path).with_context(|| { - format!( - "Failed to read signature partition '{}' [{}]", - hash_signature_device_id, - signature_block_device_path.display() - ) - })?; + let signature_bytes = read_signature_partition_bytes(&signature_block_device_path) + .with_context(|| { + format!( + "Failed to read signature partition '{hash_signature_device_id}' [{}]", + signature_block_device_path.display() + ) + })?; // Create a temporary file to hold a copy of the signature, since // veritysetup expects a path to a file containing the signature. @@ -543,7 +585,13 @@ mod tests { use super::*; use sysdefs::partition_types::DiscoverablePartitionType; - use trident_api::constants::ROOT_MOUNT_POINT_PATH; + use trident_api::{ + config::{ + Disk, FileSystem, FileSystemSource, HostConfiguration, Partition, PartitionType, + Storage, VerityDevice, + }, + constants::ROOT_MOUNT_POINT_PATH, + }; use crate::osimage::{ mock::{MockImage, MockOsImage}, @@ -639,6 +687,125 @@ mod tests { "Got unexpected error" ); } + + /// Tests [`read_signature_partition_bytes`] bounds the number of bytes + /// read from a signature partition. + #[test] + fn test_read_signature_partition_bytes() { + use std::io::Write; + + // A small file within the cap is read back in full. + let mut small_file = NamedTempFile::new().unwrap(); + small_file.write_all(b"a small pkcs7 signature").unwrap(); + let bytes = read_signature_partition_bytes(small_file.path()).unwrap(); + assert_eq!(bytes, b"a small pkcs7 signature"); + + // A file larger than MAX_VERITY_SIGNATURE_PARTITION_SIZE is rejected, + // instead of being silently read into memory in full (this is what a + // `PartitionSize::Grow` signature partition would look like). + let mut big_file = NamedTempFile::new().unwrap(); + let oversized = vec![0u8; (MAX_VERITY_SIGNATURE_PARTITION_SIZE + 1) as usize]; + big_file.write_all(&oversized).unwrap(); + let err = read_signature_partition_bytes(big_file.path()).unwrap_err(); + assert!( + err.to_string() + .contains("exceeds the maximum expected size"), + "Got unexpected error: {err}" + ); + } + + /// Tests that [`setup_verity_devices`] prefers the declarative + /// `hash_signature_device_id` signature partition over the legacy + /// `veritySignaturePaths` internal parameter when both are present on the + /// same verity device. + #[test] + fn test_setup_verity_devices_signature_partition_takes_precedence() { + let mut hc = HostConfiguration { + storage: Storage { + disks: vec![Disk { + id: "sdb".to_string(), + device: PathBuf::from("/dev/sdb"), + partitions: vec![ + Partition { + id: "root-hash".to_string(), + partition_type: PartitionType::RootVerity, + size: 4096.into(), + uuid: None, + label: None, + }, + Partition { + id: "root-data".to_string(), + partition_type: PartitionType::Root, + size: 4096.into(), + uuid: None, + label: None, + }, + Partition { + id: "root-hash-sig".to_string(), + partition_type: PartitionType::LinuxGeneric, + size: 4096.into(), + uuid: None, + label: None, + }, + ], + ..Default::default() + }], + verity: vec![VerityDevice { + id: "root".into(), + name: "root".into(), + data_device_id: "root-data".into(), + hash_device_id: "root-hash".into(), + hash_signature_device_id: Some("root-hash-sig".into()), + ..Default::default() + }], + filesystems: vec![FileSystem { + device_id: Some("root".to_string()), + mount_point: Some(ROOT_MOUNT_POINT_PATH.into()), + source: FileSystemSource::Image, + is_esp: false, + }], + ..Default::default() + }, + ..Default::default() + }; + + // Also configure the legacy signature-file mechanism for the same + // verity device, to prove it is NOT consulted while + // hash_signature_device_id is set. + hc.internal_params = serde_yaml::from_str(&format!( + "{}:\n root: /mnt/some/signature/file", + VERITY_SIGNATURE_PATHS + )) + .unwrap(); + + let ctx = EngineContext::default() + .with_spec(hc) + .with_image(MockOsImage::new().with_image(MockImage::new( + ROOT_MOUNT_POINT_PATH, + OsImageFileSystemType::Ext4, + DiscoverablePartitionType::Root, + Some("deadbeef".to_string()), + ))) + .with_partition_paths( + [ + ("sdb", PathBuf::from("/dev/sdb")), + ("root-hash", PathBuf::from("/dev/sdb1")), + ("root-data", PathBuf::from("/dev/sdb2")), + // Deliberately do NOT register "root-hash-sig": if the + // declarative signature-partition path is taken (as it + // should be), setup fails here with a specific error + // naming that partition, before any device is opened. + ] + .into_iter(), + ); + + let err = setup_verity_devices(&ctx).unwrap_err(); + let message = err.to_string(); + assert!( + message.contains("signature partition 'root-hash-sig'"), + "Expected the declarative signature partition to be tried first, got: {message}" + ); + } } #[cfg(feature = "functional-test")] diff --git a/crates/trident_api/src/config/host/storage/storage_graph/rules/mod.rs b/crates/trident_api/src/config/host/storage/storage_graph/rules/mod.rs index f9fec356fd..7a5df551dc 100644 --- a/crates/trident_api/src/config/host/storage/storage_graph/rules/mod.rs +++ b/crates/trident_api/src/config/host/storage/storage_graph/rules/mod.rs @@ -161,7 +161,7 @@ impl BlkDevReferrerKind { Self::RaidArray => ValidCardinality::new_at_least(2), Self::ABVolume => ValidCardinality::new_exact(2), Self::EncryptedVolume => ValidCardinality::new_exact(1), - Self::VerityDevice => ValidCardinality::new_exact(2), + Self::VerityDevice => ValidCardinality::new_range(2, 3), Self::Swap => ValidCardinality::new_exact(1), Self::FileSystemNew => ValidCardinality::new_at_most(1), From 0a065c4a6b4bff4031f329829ad924c0846f0768 Mon Sep 17 00:00:00 2001 From: Brian Fjeldstad Date: Thu, 8 Oct 2026 22:46:39 +0000 Subject: [PATCH 4/5] engine: pass the signature partition's device path straight to veritysetup Addresses review feedback on PR #836 (and supersedes my own earlier fix to the same code, which was still wrong): open_verity_device_with_signature_partition used to read the signature partition's bytes into memory (bounded by an arbitrary 1 MiB cap) and copy them into a temporary file for veritysetup. That cap was itself broken: block-device reads don't EOF until the partition ends, and deployment doesn't truncate/zero the tail beyond the real image bytes, so any real signature partition sized (e.g. via PartitionSize::Grow) past 1 MiB would be rejected even though the actual DER payload inside it is tiny. The deeper problem was reading the bytes at all. `veritysetup --root-hash-signature=FILE` and the existing open_with_signature() wrapper just take a path - there's no distinction between a regular file and a block device special file, and both `open()`/`read()` identically either way. The PKCS#7/DER blob is self-delimiting, so whatever parses it stops at the structurally-defined length regardless of trailing zero-padding. Trident's job for this partition should stop exactly where it stops for the hash partition: deploy the COSI image bytes onto the right partition (already handled uniformly for every image via deploy_os_image_file's uncompressed_size-bounded stream), and nothing else - no read-back, no cap, no temp file. Deleted read_signature_partition_bytes and the temp-file copy entirely; the signature partition's resolved device path is now passed directly to open_with_signature, exactly like data_device_path/hash_device_path already are. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/trident/src/engine/storage/verity.rs | 118 +++----------------- 1 file changed, 15 insertions(+), 103 deletions(-) diff --git a/crates/trident/src/engine/storage/verity.rs b/crates/trident/src/engine/storage/verity.rs index 70df41b7f0..9bac2a5a2f 100644 --- a/crates/trident/src/engine/storage/verity.rs +++ b/crates/trident/src/engine/storage/verity.rs @@ -1,7 +1,6 @@ use std::{ collections::{HashMap, HashSet}, fs, - io::Read, path::{Path, PathBuf}, }; @@ -319,56 +318,23 @@ fn open_verity_device_with_signature( )) } -/// Upper bound, in bytes, on data read from a dedicated verity root hash -/// signature partition. A detached PKCS#7 signature (including an embedded -/// certificate chain) is at most a few tens of KiB; this generous cap keeps -/// memory and temporary-file usage bounded regardless of the signature -/// partition's declared size (which, like the hash partition, MAY use -/// `PartitionSize::Grow` and consume the remainder of the disk). -const MAX_VERITY_SIGNATURE_PARTITION_SIZE: u64 = 1024 * 1024; // 1 MiB - -/// Reads up to [`MAX_VERITY_SIGNATURE_PARTITION_SIZE`] bytes from `path`, -/// returning an error if the file/device holds more than that. This bounds -/// memory usage when reading a signature partition whose declared size may be -/// much larger than the actual PKCS#7 signature blob it holds (e.g. a -/// `PartitionSize::Grow` signature partition). -fn read_signature_partition_bytes(path: &Path) -> Result, Error> { - let mut file = fs::File::open(path) - .with_context(|| format!("Failed to open signature partition '{}'", path.display()))?; - - // Read at most MAX_VERITY_SIGNATURE_PARTITION_SIZE + 1 bytes: if we get - // back more than the cap, the partition holds more than a plausible - // signature and we reject it below, rather than silently truncating it. - let mut bytes = Vec::new(); - let bytes_read = file - .by_ref() - .take(MAX_VERITY_SIGNATURE_PARTITION_SIZE + 1) - .read_to_end(&mut bytes) - .with_context(|| format!("Failed to read signature partition '{}'", path.display()))?; - - ensure!( - bytes_read as u64 <= MAX_VERITY_SIGNATURE_PARTITION_SIZE, - "Signature partition '{}' exceeds the maximum expected size of {} bytes for a verity \ - root hash signature", - path.display(), - MAX_VERITY_SIGNATURE_PARTITION_SIZE - ); - - Ok(bytes) -} - /// Open a verity device using a root hash signature read directly from a /// dedicated signature partition. /// /// This is the declarative counterpart to [`open_verity_device_with_signature`]: /// instead of locating a signature file inside a mounted filesystem, the -/// signature bytes are read directly from the raw block device declared via -/// `VerityDevice.hash_signature_device_id`. No mount is required. The PKCS#7/DER -/// encoding is self-delimiting, so trailing zero-padding in a partition -/// larger than the signature blob itself is harmless. The read itself is -/// bounded (see [`read_signature_partition_bytes`]) so that an oversized -/// (e.g. `Grow`-sized) signature partition cannot be read entirely into -/// memory. +/// signature partition's own block device path is passed straight to +/// `veritysetup --root-hash-signature=`, exactly like the data and hash +/// devices are already passed by path - no mount, and no copy into a +/// temporary file, is required. `veritysetup`/the kernel treat `FILE` as any +/// path it can `open()`/`read()`, which a block device special file +/// satisfies identically to a regular file; the PKCS#7/DER encoding is +/// self-delimiting, so trailing zero-padding in a partition larger than the +/// signature blob itself (e.g. a `PartitionSize::Grow` signature partition) +/// is harmless to whatever parses it. Trident's own responsibility for this +/// partition ends at deployment time, same as for the hash partition: get +/// the right bytes onto the right partition (see `image.rs::deploy_images`), +/// and nothing else. /// /// The certificate matching the signature MUST exist in the kernel keyring, /// otherwise the operation WILL fail. @@ -384,36 +350,8 @@ fn open_verity_device_with_signature_partition( format!("Failed to find path for block device '{hash_signature_device_id}'") })?; - debug!( - "Reading root hash signature for verity device '{}' from partition '{}' [{}]", - verity_device_id, - hash_signature_device_id, - signature_block_device_path.display() - ); - - let signature_bytes = read_signature_partition_bytes(&signature_block_device_path) - .with_context(|| { - format!( - "Failed to read signature partition '{hash_signature_device_id}' [{}]", - signature_block_device_path.display() - ) - })?; - - // Create a temporary file to hold a copy of the signature, since - // veritysetup expects a path to a file containing the signature. - let temp_signature_file_path = NamedTempFile::new() - .context("Failed to create temporary file for verity signature")? - .into_temp_path(); - - fs::write(&temp_signature_file_path, &signature_bytes).with_context(|| { - format!( - "Failed to write signature to temporary file '{}'", - temp_signature_file_path.display() - ) - })?; - // Try to print signature info - match veritysetup::get_verity_signature_info(&temp_signature_file_path) { + match veritysetup::get_verity_signature_info(&signature_block_device_path) { Ok(signature_info) => { debug!( "Signature partition '{}' for verity device '{}' info:\n{}", @@ -432,11 +370,11 @@ fn open_verity_device_with_signature_partition( "Opening verity device '{}' with signature from partition '{}' [{}]", verity_device_id, hash_signature_device_id, - temp_signature_file_path.display(), + signature_block_device_path.display(), ); verity_device - .open_with_signature(&temp_signature_file_path) + .open_with_signature(&signature_block_device_path) .context(format!( "Failed to open verity device '{}' with signature from partition '{}'", verity_device_id, hash_signature_device_id @@ -688,32 +626,6 @@ mod tests { ); } - /// Tests [`read_signature_partition_bytes`] bounds the number of bytes - /// read from a signature partition. - #[test] - fn test_read_signature_partition_bytes() { - use std::io::Write; - - // A small file within the cap is read back in full. - let mut small_file = NamedTempFile::new().unwrap(); - small_file.write_all(b"a small pkcs7 signature").unwrap(); - let bytes = read_signature_partition_bytes(small_file.path()).unwrap(); - assert_eq!(bytes, b"a small pkcs7 signature"); - - // A file larger than MAX_VERITY_SIGNATURE_PARTITION_SIZE is rejected, - // instead of being silently read into memory in full (this is what a - // `PartitionSize::Grow` signature partition would look like). - let mut big_file = NamedTempFile::new().unwrap(); - let oversized = vec![0u8; (MAX_VERITY_SIGNATURE_PARTITION_SIZE + 1) as usize]; - big_file.write_all(&oversized).unwrap(); - let err = read_signature_partition_bytes(big_file.path()).unwrap_err(); - assert!( - err.to_string() - .contains("exceeds the maximum expected size"), - "Got unexpected error: {err}" - ); - } - /// Tests that [`setup_verity_devices`] prefers the declarative /// `hash_signature_device_id` signature partition over the legacy /// `veritySignaturePaths` internal parameter when both are present on the From ed7e8b700d81d7c57be4b3dfb9e05eaadc048b33 Mon Sep 17 00:00:00 2001 From: Brian Fjeldstad Date: Sat, 10 Oct 2026 17:01:52 +0000 Subject: [PATCH 5/5] fix: extract bounded DER signatures before verity activation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d61c42e7-3c43-4fe5-8c89-89d38ff9528b --- crates/trident/src/engine/storage/verity.rs | 135 +++++++++++++++++--- 1 file changed, 114 insertions(+), 21 deletions(-) diff --git a/crates/trident/src/engine/storage/verity.rs b/crates/trident/src/engine/storage/verity.rs index 9bac2a5a2f..ec4b2a1f21 100644 --- a/crates/trident/src/engine/storage/verity.rs +++ b/crates/trident/src/engine/storage/verity.rs @@ -1,6 +1,7 @@ use std::{ collections::{HashMap, HashSet}, fs, + io::{self, Read, Write}, path::{Path, PathBuf}, }; @@ -318,26 +319,11 @@ fn open_verity_device_with_signature( )) } -/// Open a verity device using a root hash signature read directly from a -/// dedicated signature partition. +/// Open a verity device using a signature from a dedicated partition. /// -/// This is the declarative counterpart to [`open_verity_device_with_signature`]: -/// instead of locating a signature file inside a mounted filesystem, the -/// signature partition's own block device path is passed straight to -/// `veritysetup --root-hash-signature=`, exactly like the data and hash -/// devices are already passed by path - no mount, and no copy into a -/// temporary file, is required. `veritysetup`/the kernel treat `FILE` as any -/// path it can `open()`/`read()`, which a block device special file -/// satisfies identically to a regular file; the PKCS#7/DER encoding is -/// self-delimiting, so trailing zero-padding in a partition larger than the -/// signature blob itself (e.g. a `PartitionSize::Grow` signature partition) -/// is harmless to whatever parses it. Trident's own responsibility for this -/// partition ends at deployment time, same as for the hash partition: get -/// the right bytes onto the right partition (see `image.rs::deploy_images`), -/// and nothing else. -/// -/// The certificate matching the signature MUST exist in the kernel keyring, -/// otherwise the operation WILL fail. +/// cryptsetup requires a regular signature file. Extract only the bounded +/// DER object, excluding partition padding, for diagnostics and activation. +/// The matching certificate must exist in the kernel keyring. fn open_verity_device_with_signature_partition( ctx: &EngineContext, verity_device_id: &BlockDeviceId, @@ -350,8 +336,18 @@ fn open_verity_device_with_signature_partition( format!("Failed to find path for block device '{hash_signature_device_id}'") })?; + let mut source = fs::File::open(&signature_block_device_path).with_context(|| { + format!( + "Failed to read signature partition '{}'", + signature_block_device_path.display() + ) + })?; + let signature = extract_verity_signature(&mut source).with_context(|| { + format!("Failed to extract signature from partition '{hash_signature_device_id}'") + })?; + // Try to print signature info - match veritysetup::get_verity_signature_info(&signature_block_device_path) { + match veritysetup::get_verity_signature_info(signature.path()) { Ok(signature_info) => { debug!( "Signature partition '{}' for verity device '{}' info:\n{}", @@ -374,13 +370,69 @@ fn open_verity_device_with_signature_partition( ); verity_device - .open_with_signature(&signature_block_device_path) + .open_with_signature(signature.path()) .context(format!( "Failed to open verity device '{}' with signature from partition '{}'", verity_device_id, hash_signature_device_id )) } +const MAX_VERITY_SIGNATURE_SIZE: usize = 1024 * 1024; + +fn extract_verity_signature(reader: &mut impl Read) -> Result { + let mut header = [0u8; 2]; + reader + .read_exact(&mut header) + .context("Failed to read DER signature header")?; + ensure!(header[0] == 0x30, "Signature must be a DER sequence"); + + let mut length_bytes = [0u8; 4]; + let length_count = if header[1] & 0x80 != 0 { + usize::from(header[1] & 0x7f) + } else { + 0 + }; + let content_length = if header[1] & 0x80 == 0 { + usize::from(header[1]) + } else { + ensure!( + (1..=length_bytes.len()).contains(&length_count), + "Invalid DER length encoding" + ); + reader + .read_exact(&mut length_bytes[..length_count]) + .context("Failed to read DER signature length")?; + ensure!(length_bytes[0] != 0, "Non-minimal DER length encoding"); + let length = length_bytes[..length_count] + .iter() + .fold(0usize, |length, byte| (length << 8) | usize::from(*byte)); + ensure!(length >= 128, "Non-minimal DER length encoding"); + length + }; + ensure!(content_length > 0, "Signature DER sequence is empty"); + let header_length = header.len() + length_count; + ensure!( + content_length <= MAX_VERITY_SIGNATURE_SIZE - header_length, + "Signature exceeds the {MAX_VERITY_SIGNATURE_SIZE}-byte limit" + ); + + let mut signature = + NamedTempFile::new().context("Failed to create signature temporary file")?; + signature + .write_all(&header) + .context("Failed to write DER signature header")?; + signature + .write_all(&length_bytes[..length_count]) + .context("Failed to write DER signature length")?; + let copied = io::copy(&mut reader.take(content_length as u64), &mut signature) + .context("Failed to copy DER signature")?; + ensure!(copied == content_length as u64, "Truncated DER signature"); + signature + .flush() + .context("Failed to flush signature temporary file")?; + Ok(signature) +} + /// Get the verity data and hash paths. /// /// Verity data and hash devices are fetched from the engine context. @@ -536,6 +588,47 @@ mod tests { OsImage, OsImageFileSystemType, }; + #[test] + fn test_extract_verity_signature_excludes_partition_padding() { + for encoded in [ + vec![0x30, 3, 2, 1, 0], + [vec![0x30, 0x81, 0x80], vec![0; 128]].concat(), + [ + vec![0x30, 0x83, 0x0f, 0xff, 0xfb], + vec![0; MAX_VERITY_SIGNATURE_SIZE - 5], + ] + .concat(), + ] { + let partition = [encoded.clone(), vec![0; 4096]].concat(); + let mut reader = io::Cursor::new(partition); + let signature = extract_verity_signature(&mut reader).unwrap(); + assert_eq!(fs::read(signature.path()).unwrap(), encoded); + assert_eq!(reader.position(), encoded.len() as u64); + assert!(signature.as_file().metadata().unwrap().is_file()); + } + } + + #[test] + fn test_extract_verity_signature_rejects_invalid_der() { + for (encoded, expected) in [ + (vec![], "header"), + (vec![0x30], "header"), + (vec![0x31, 1, 0], "DER sequence"), + (vec![0x30, 0], "empty"), + (vec![0x30, 0x80], "length encoding"), + (vec![0x30, 0x85], "length encoding"), + (vec![0x30, 0x82, 1], "signature length"), + (vec![0x30, 0x82, 0, 0x80], "Non-minimal"), + (vec![0x30, 0x81, 0x7f], "Non-minimal"), + (vec![0x30, 0x83, 0x10, 0, 0], "limit"), + (vec![0x30, 3, 0], "Truncated"), + ] { + let mut reader = io::Cursor::new(encoded); + let error = extract_verity_signature(&mut reader).unwrap_err(); + assert!(error.to_string().contains(expected), "{error:#}"); + } + } + #[test] fn test_get_updated_device_name() { assert_eq!(get_updated_device_name("root"), "root_new");