From 7a6af09319b2109769e446f78208dc70cb4745f4 Mon Sep 17 00:00:00 2001 From: Mike Ebersol Date: Thu, 30 Apr 2026 16:54:43 -0700 Subject: [PATCH 1/3] [Copilot] Use zeroize for secure secret cleanup Add the zeroize crate to securely clear cryptographic keys and secrets from memory when they are dropped, preventing sensitive data from persisting after Rust objects are deallocated. Changes: - Add zeroize 1.8 with derive feature as a workspace dependency - Derive Zeroize/ZeroizeOnDrop on HardwareDerivedKeys (AES + HMAC keys) - Derive Zeroize/ZeroizeOnDrop on Keys (VMGS ingress/egress keys) - Derive Zeroize/ZeroizeOnDrop on DekKp, GspKp, KeyProtector, HardwareKeyProtector, and GuestSecretKey structs - Replace .fill(0) with .zeroize() in vmgs_impl.rs and lib.rs to prevent dead-store elimination by the compiler --- Cargo.lock | 17 +++++++++++++++++ Cargo.toml | 1 + openhcl/openhcl_attestation_protocol/Cargo.toml | 1 + .../openhcl_attestation_protocol/src/vmgs.rs | 13 ++++++++----- openhcl/underhill_attestation/Cargo.toml | 1 + .../src/hardware_key_sealing.rs | 3 +++ openhcl/underhill_attestation/src/lib.rs | 5 +++-- vm/vmgs/vmgs/Cargo.toml | 1 + vm/vmgs/vmgs/src/vmgs_impl.rs | 4 +++- 9 files changed, 38 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cc95e2e8e3..8d0a109f16 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5060,6 +5060,7 @@ dependencies = [ "serde_json", "x86defs", "zerocopy", + "zeroize", ] [[package]] @@ -8285,6 +8286,7 @@ dependencies = [ "vmgs", "vmgs_format", "zerocopy", + "zeroize", ] [[package]] @@ -9850,6 +9852,7 @@ dependencies = [ "tracing", "vmgs_format", "zerocopy", + "zeroize", ] [[package]] @@ -11048,3 +11051,17 @@ name = "zeroize" version = "1.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ced3678a2879b30306d323f4542626697a464a97c0a07c9aebf7ebca65cd4dde" +dependencies = [ + "zeroize_derive", +] + +[[package]] +name = "zeroize_derive" +version = "1.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85a5b4158499876c763cb03bc4e49185d3cccbabb15b33c627f7884f43db852e" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.106", +] diff --git a/Cargo.toml b/Cargo.toml index 2fc5b515c9..47531f8b76 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -589,6 +589,7 @@ xshell = "=0.2.2" # pin to 0.2.2 to work around https://github.com/matklad/xshel xshell-macros = "0.2" # We add the derive feature here since the vast majority of our crates use it. zerocopy = { version = "0.8.25", features = ["derive"]} +zeroize = { version = "1.8", features = ["derive"]} [profile.release] panic = 'abort' diff --git a/openhcl/openhcl_attestation_protocol/Cargo.toml b/openhcl/openhcl_attestation_protocol/Cargo.toml index e8e7dc8ef2..53436898db 100644 --- a/openhcl/openhcl_attestation_protocol/Cargo.toml +++ b/openhcl/openhcl_attestation_protocol/Cargo.toml @@ -20,6 +20,7 @@ hex.workspace = true serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["std"] } zerocopy.workspace = true +zeroize.workspace = true [lints] workspace = true diff --git a/openhcl/openhcl_attestation_protocol/src/vmgs.rs b/openhcl/openhcl_attestation_protocol/src/vmgs.rs index 0a72629705..2bac8476fd 100644 --- a/openhcl/openhcl_attestation_protocol/src/vmgs.rs +++ b/openhcl/openhcl_attestation_protocol/src/vmgs.rs @@ -7,6 +7,8 @@ use zerocopy::FromBytes; use zerocopy::Immutable; use zerocopy::IntoBytes; use zerocopy::KnownLayout; +use zeroize::Zeroize; +use zeroize::ZeroizeOnDrop; /// Number of the key protector entries. /// One for ingress, and one for egress @@ -23,7 +25,7 @@ pub const KEY_PROTECTOR_SIZE: usize = size_of::(); /// DEK key protector entry. #[repr(C)] -#[derive(Debug, IntoBytes, Immutable, KnownLayout, FromBytes)] +#[derive(Debug, IntoBytes, Immutable, KnownLayout, FromBytes, Zeroize, ZeroizeOnDrop)] pub struct DekKp { /// DEK buffer pub dek_buffer: [u8; DEK_BUFFER_SIZE], @@ -31,7 +33,7 @@ pub struct DekKp { /// GSP key protector entry. #[repr(C)] -#[derive(Debug, IntoBytes, Immutable, KnownLayout, FromBytes)] +#[derive(Debug, IntoBytes, Immutable, KnownLayout, FromBytes, Zeroize, ZeroizeOnDrop)] pub struct GspKp { /// GSP data size pub gsp_length: u32, @@ -41,7 +43,7 @@ pub struct GspKp { /// The data format of the `FileId::KEY_PROTECTOR` entry in the VMGS file. #[repr(C)] -#[derive(Debug, IntoBytes, Immutable, KnownLayout, FromBytes)] +#[derive(Debug, IntoBytes, Immutable, KnownLayout, FromBytes, Zeroize, ZeroizeOnDrop)] pub struct KeyProtector { /// Array of DEK entries pub dek: [DekKp; NUMBER_KP], @@ -123,9 +125,10 @@ impl HardwareKeyProtectorHeader { /// The data format of the `FileId::HW_KEY_PROTECTOR` entry in the VMGS file. #[repr(C)] -#[derive(Debug, IntoBytes, Immutable, KnownLayout, FromBytes)] +#[derive(Debug, IntoBytes, Immutable, KnownLayout, FromBytes, Zeroize, ZeroizeOnDrop)] pub struct HardwareKeyProtector { /// Header + #[zeroize(skip)] pub header: HardwareKeyProtectorHeader, /// Random IV for AES-CBC pub iv: [u8; AES_CBC_IV_LENGTH], @@ -140,7 +143,7 @@ pub const GUEST_SECRET_KEY_MAX_SIZE: usize = 2048; /// The data format of the `FileId::GUEST_SECRET_KEY` entry in the VMGS file. #[repr(C)] -#[derive(Debug, IntoBytes, Immutable, KnownLayout, FromBytes)] +#[derive(Debug, IntoBytes, Immutable, KnownLayout, FromBytes, Zeroize, ZeroizeOnDrop)] pub struct GuestSecretKey { /// the guest secret key to be provisioned to vTPM pub guest_secret_key: [u8; GUEST_SECRET_KEY_MAX_SIZE], diff --git a/openhcl/underhill_attestation/Cargo.toml b/openhcl/underhill_attestation/Cargo.toml index 6174cb9efc..2444882864 100644 --- a/openhcl/underhill_attestation/Cargo.toml +++ b/openhcl/underhill_attestation/Cargo.toml @@ -28,6 +28,7 @@ serde_json = { workspace = true, features = ["std"] } static_assertions.workspace = true thiserror.workspace = true zerocopy.workspace = true +zeroize.workspace = true [target.'cfg(target_os = "linux")'.dev-dependencies] disklayer_ram.workspace = true diff --git a/openhcl/underhill_attestation/src/hardware_key_sealing.rs b/openhcl/underhill_attestation/src/hardware_key_sealing.rs index f75bf7de50..e8c73656df 100644 --- a/openhcl/underhill_attestation/src/hardware_key_sealing.rs +++ b/openhcl/underhill_attestation/src/hardware_key_sealing.rs @@ -11,6 +11,8 @@ use openhcl_attestation_protocol::vmgs; use openhcl_attestation_protocol::vmgs::HardwareKeyProtector; use thiserror::Error; use zerocopy::IntoBytes; +use zeroize::Zeroize; +use zeroize::ZeroizeOnDrop; #[derive(Debug, Error)] pub(crate) enum HardwareDerivedKeysError { @@ -39,6 +41,7 @@ pub(crate) enum HardwareKeySealingError { } /// Hold the hardware-derived keys. +#[derive(Zeroize, ZeroizeOnDrop)] pub struct HardwareDerivedKeys { tcb_version: u64, aes_key: [u8; vmgs::AES_CBC_KEY_LENGTH], diff --git a/openhcl/underhill_attestation/src/lib.rs b/openhcl/underhill_attestation/src/lib.rs index 22772728b0..4e37bcdf67 100644 --- a/openhcl/underhill_attestation/src/lib.rs +++ b/openhcl/underhill_attestation/src/lib.rs @@ -53,6 +53,7 @@ use tee_call::TeeCall; use thiserror::Error; use zerocopy::FromZeros; use zerocopy::IntoBytes; +use zeroize::Zeroize; /// An attestation error. #[derive(Debug, Error)] @@ -180,7 +181,7 @@ fn derive_key( Ok(output.try_into().unwrap()) } -#[derive(Debug)] +#[derive(Debug, zeroize::Zeroize, zeroize::ZeroizeOnDrop)] struct Keys { ingress: [u8; AES_GCM_KEY_LENGTH], decrypt_egress: Option<[u8; AES_GCM_KEY_LENGTH]>, @@ -1337,7 +1338,7 @@ async fn persist_all_key_protectors( // Remove ingress KP & DEK, no longer applies to data store key_protector.dek[key_protector.active_kp as usize % NUMBER_KP] .dek_buffer - .fill(0); + .zeroize(); key_protector.gsp[key_protector.active_kp as usize % NUMBER_KP].gsp_length = 0; key_protector.active_kp += 1; diff --git a/vm/vmgs/vmgs/Cargo.toml b/vm/vmgs/vmgs/Cargo.toml index 77606fc5e5..c4a38303e7 100644 --- a/vm/vmgs/vmgs/Cargo.toml +++ b/vm/vmgs/vmgs/Cargo.toml @@ -36,6 +36,7 @@ thiserror.workspace = true tracing.workspace = true cvm_tracing.workspace = true zerocopy.workspace = true +zeroize.workspace = true [dev-dependencies] pal_async.workspace = true diff --git a/vm/vmgs/vmgs/src/vmgs_impl.rs b/vm/vmgs/vmgs/src/vmgs_impl.rs index d0c1c79fd6..c97c12450a 100644 --- a/vm/vmgs/vmgs/src/vmgs_impl.rs +++ b/vm/vmgs/vmgs/src/vmgs_impl.rs @@ -39,6 +39,8 @@ use vmgs_format::VmgsProvisioningReason; use zerocopy::FromBytes; use zerocopy::FromZeros; use zerocopy::IntoBytes; +#[cfg(feature = "encryption")] +use zeroize::Zeroize; /// Operation types for provisioning telemetry. #[derive(Debug)] @@ -1234,7 +1236,7 @@ impl Vmgs { let mut temp_state = self.temp_state(); // Remove the corresponding datastore_key - temp_state.datastore_keys[key_index].fill(0); + temp_state.datastore_keys[key_index].zeroize(); // Remove the corresponding metadata_key temp_state.encrypted_metadata_keys[key_index] = VmgsEncryptionKey::new_zeroed(); From c994b17f491857eb490d046e1b1f86e846331495 Mon Sep 17 00:00:00 2001 From: Mike Ebersol Date: Thu, 30 Apr 2026 17:34:33 -0700 Subject: [PATCH 2/3] vmgs: make zeroize dep conditional on encryption feature Address review feedback: zeroize is only used behind cfg(feature = encryption) in vmgs_impl.rs, so make it an optional dependency gated on the encryption feature rather than an unconditional dependency. --- vm/vmgs/vmgs/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vm/vmgs/vmgs/Cargo.toml b/vm/vmgs/vmgs/Cargo.toml index c4a38303e7..bf9759c3fc 100644 --- a/vm/vmgs/vmgs/Cargo.toml +++ b/vm/vmgs/vmgs/Cargo.toml @@ -12,7 +12,7 @@ default = [] inspect = ["vmgs_format/inspect", "dep:inspect", "dep:inspect_counters"] save_restore = ["dep:mesh_protobuf"] mesh = ["dep:mesh_protobuf"] -encryption = ["dep:crypto"] +encryption = ["dep:crypto", "dep:zeroize"] test_helpers = [] # run encryption unit tests in CI @@ -36,7 +36,7 @@ thiserror.workspace = true tracing.workspace = true cvm_tracing.workspace = true zerocopy.workspace = true -zeroize.workspace = true +zeroize = { workspace = true, optional = true } [dev-dependencies] pal_async.workspace = true From 79ad325fe551232a049c73e03c15013630828834 Mon Sep 17 00:00:00 2001 From: Mike Ebersol Date: Fri, 1 May 2026 16:07:35 -0700 Subject: [PATCH 3/3] underhill_attestation: also zeroize gsp_buffer before persisting Address review feedback: setting gsp_length = 0 only clears the length field, leaving the gsp_buffer contents intact in the struct written back to VMGS. Add gsp_buffer.zeroize() for consistency with dek_buffer. --- openhcl/underhill_attestation/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openhcl/underhill_attestation/src/lib.rs b/openhcl/underhill_attestation/src/lib.rs index 4e37bcdf67..0183f47b59 100644 --- a/openhcl/underhill_attestation/src/lib.rs +++ b/openhcl/underhill_attestation/src/lib.rs @@ -1340,6 +1340,9 @@ async fn persist_all_key_protectors( .dek_buffer .zeroize(); key_protector.gsp[key_protector.active_kp as usize % NUMBER_KP].gsp_length = 0; + key_protector.gsp[key_protector.active_kp as usize % NUMBER_KP] + .gsp_buffer + .zeroize(); key_protector.active_kp += 1; vmgs::write_key_protector(key_protector, vmgs)