chore: Remove attached LUKS header from Store partition (#10604)

The Store partition is now formatted with a detached LUKS header only. A
legacy attached header still present on a device (which was the previous
approach) is wiped on the next open, so only the detached header remains
going forward.

In addition, `guest_disk` now uses a shared metrics `Registry` and
writes all metrics once. The PR adds the
`guest_disk_store_attached_luks2_header_status` gauge to track the
header migration state.

---------

Co-authored-by: IDX GitHub Automation <infra+github-automation@dfinity.org>

Author: frankdavid

rs/ic_os/guest_upgrade/client/src/lib.rs

@@ -198,46 +198,32 @@ impl DiskEncryptionKeyExchangeClientAgent {
             .key
             .context("GetKeyResponse does not contain a key")?;
 
-        self.persist_disk_encryption_artifacts(
-            disk_encryption_key,
-            // This can be None when upgrading from older GuestOS-s that do not populate this field.
-            // TODO: Error on None once all GuestOS-s support detached headers
-            disk_encryption_data.luks_header,
-        )
-        .await?;
+        let luks_header = disk_encryption_data
+            .luks_header
+            .context("Server did not send a Store LUKS header")?;
+
+        self.persist_disk_encryption_artifacts(disk_encryption_key, luks_header)
+            .await?;
 
         Ok(())
     }
 
     async fn persist_disk_encryption_artifacts(
         &self,
         disk_encryption_key: Vec<u8>,
-        luks_header: Option<Vec<u8>>,
+        luks_header: Vec<u8>,
     ) -> Result<()> {
         let disk_encryption_key =
             String::from_utf8(disk_encryption_key).context("Key is not valid UTF-8")?;
 
-        match luks_header {
-            Some(luks_header) => {
-                tokio::fs::write(&self.store_luks_header_path, &luks_header)
-                    .await
-                    .with_context(|| {
-                        format!(
-                            "Failed to write store LUKS header to {}",
-                            self.store_luks_header_path.display()
-                        )
-                    })?;
-            }
-            None => {
-                println!(
-                    "GetKeyResponse does not contain a store LUKS header; recovering it locally from {}",
-                    self.store_device_path.display()
-                );
-                self.crypto_ops
-                    .backup_luks_header(&self.store_device_path, &self.store_luks_header_path)
-                    .context("Local LUKS header backup failed")?;
-            }
-        }
+        tokio::fs::write(&self.store_luks_header_path, &luks_header)
+            .await
+            .with_context(|| {
+                format!(
+                    "Failed to write Store LUKS header to {}",
+                    self.store_luks_header_path.display()
+                )
+            })?;
 
         tokio::fs::write(&self.previous_key_path, disk_encryption_key)
             .await

rs/ic_os/guest_upgrade/server/src/lib.rs

@@ -42,10 +42,6 @@ pub struct DiskEncryptionKeyExchangeServerAgent {
     registry_client: Arc<dyn RegistryClient>,
     store_device_path: PathBuf,
     store_luks_header_path: PathBuf,
-    /// If true, the server will send the Store LUKS header to the client.
-    /// Only false in unit tests testing backwards compatibility.
-    // TODO: Remove when all deployed GuestOS versions support sending the Store LUKS header.
-    send_luks_header: bool,
     port: u16,
     success_timeout: Duration,
 }
@@ -60,7 +56,6 @@ impl DiskEncryptionKeyExchangeServerAgent {
         registry_client: Arc<dyn RegistryClient>,
         store_device_path: PathBuf,
         store_luks_header_path: PathBuf,
-        send_luks_header: bool,
         port: u16,
         success_timeout: Duration,
     ) -> Self {
@@ -73,7 +68,6 @@ impl DiskEncryptionKeyExchangeServerAgent {
             registry_client,
             store_device_path,
             store_luks_header_path,
-            send_luks_header,
             port,
             success_timeout,
         }
@@ -105,7 +99,6 @@ impl DiskEncryptionKeyExchangeServerAgent {
             self.trusted_execution_environment_config.clone(),
             self.store_device_path.clone(),
             self.store_luks_header_path.clone(),
-            self.send_luks_header,
             status_sender,
             expected_measurements,
         ));

rs/ic_os/guest_upgrade/server/src/orchestrator.rs

@@ -75,7 +75,6 @@ pub fn new_disk_encryption_key_exchange_server_agent_for_orchestrator(
         registry_client,
         STORE_DEVICE.into(),
         DEFAULT_STORE_LUKS_HEADER_PATH.into(),
-        /*send_luks_header=*/ true,
         DEFAULT_SERVER_PORT,
         DEFAULT_SUCCESS_TIMEOUT,
     ))

rs/ic_os/guest_upgrade/server/src/service.rs

@@ -30,10 +30,6 @@ pub struct DiskEncryptionKeyExchangeServiceImpl {
     sev_root_certificate_verification: SevRootCertificateVerification,
     store_device_path: PathBuf,
     store_luks_header_path: PathBuf,
-    /// If true, the server will send the Store LUKS header to the client.
-    /// Only false in unit tests testing backwards compatibility.
-    // TODO: Remove when all deployed GuestOS versions support sending the Store LUKS header.
-    send_luks_header: bool,
 }
 
 impl DiskEncryptionKeyExchangeServiceImpl {
@@ -44,7 +40,6 @@ impl DiskEncryptionKeyExchangeServiceImpl {
         trusted_execution_environment_config: TrustedExecutionEnvironmentConfig,
         store_device_path: PathBuf,
         store_luks_header_path: PathBuf,
-        send_luks_header: bool,
         status_sender: Sender<Result<(), String>>,
         expected_measurements: Vec<Vec<u8>>,
     ) -> Self {
@@ -57,7 +52,6 @@ impl DiskEncryptionKeyExchangeServiceImpl {
             sev_root_certificate_verification,
             store_device_path,
             store_luks_header_path,
-            send_luks_header,
         }
     }
 
@@ -155,17 +149,9 @@ impl DiskEncryptionKeyExchangeServiceImpl {
         let mut sev_firmware = self.sev_firmware_factory.deref()()
             .map_err(|e| Status::internal(format!("Failed to create SEV firmware: {e:?}")))?;
 
-        let luks_header = if self.send_luks_header {
-            Some(
-                tokio::fs::read(&self.store_luks_header_path)
-                    .await
-                    .map_err(|e| {
-                        Status::internal(format!("Failed to read Store LUKS header: {e:#}"))
-                    })?,
-            )
-        } else {
-            None
-        };
+        let luks_header = tokio::fs::read(&self.store_luks_header_path)
+            .await
+            .map_err(|e| Status::internal(format!("Failed to read Store LUKS header: {e:#}")))?;
 
         Ok(Response::new(GetDiskEncryptionKeyResponse {
             key: Some(
@@ -179,7 +165,7 @@ impl DiskEncryptionKeyExchangeServiceImpl {
                 .into_bytes(),
             ),
             sev_attestation_package: Some(my_attestation_package.into()),
-            luks_header,
+            luks_header: Some(luks_header),
         }))
     }
 

rs/ic_os/guest_upgrade/tests/src/lib.rs

@@ -68,8 +68,6 @@ struct TestConfig {
     /// Allows testing invalid client attestation data.
     client_custom_data_override: Option<[u8; 64]>,
     can_open_disk: bool,
-    server_returns_luks_header: bool,
-    client_recovered_luks_header: Option<Vec<u8>>,
 }
 
 impl Default for TestConfig {
@@ -84,8 +82,6 @@ impl Default for TestConfig {
             client_chip_id: DEFAULT_CHIP_ID,
             server_chip_id: DEFAULT_CHIP_ID,
             can_open_disk: false,
-            server_returns_luks_header: true,
-            client_recovered_luks_header: None,
         }
     }
 }
@@ -114,8 +110,6 @@ struct DiskEncryptionKeyExchangeTestFixture {
     server_port: u16,
     /// True to assume that the disk can already be opened without key exchange
     can_open_disk: bool,
-    server_returns_luks_header: bool,
-    client_recovered_luks_header: Option<Vec<u8>>,
 }
 
 impl DiskEncryptionKeyExchangeTestFixture {
@@ -206,8 +200,6 @@ impl DiskEncryptionKeyExchangeTestFixture {
             client_store_luks_header,
             server_port,
             can_open_disk: config.can_open_disk,
-            server_returns_luks_header: config.server_returns_luks_header,
-            client_recovered_luks_header: config.client_recovered_luks_header,
         }
     }
 
@@ -264,7 +256,6 @@ impl DiskEncryptionKeyExchangeTestFixture {
             self.registry_client.clone(),
             self.server_store.path().to_path_buf(),
             self.server_store_luks_header.path().to_path_buf(),
-            self.server_returns_luks_header,
             self.server_port,
             Duration::from_secs(2),
         )
@@ -281,22 +272,6 @@ impl DiskEncryptionKeyExchangeTestFixture {
             .expect_can_open_store()
             .returning(move |_, _, _, _| Ok(can_open_disk));
 
-        if let Some(recovered_luks_header) = self.client_recovered_luks_header.clone() {
-            let expected_store_device_path = store_device_path.clone();
-            crypto_ops
-                .expect_backup_luks_header()
-                .times(1)
-                .withf(move |store_device_path, _| {
-                    store_device_path == expected_store_device_path.as_path()
-                })
-                .returning(move |_, store_luks_header_path| {
-                    std::fs::write(store_luks_header_path, &recovered_luks_header)?;
-                    Ok(())
-                });
-        } else {
-            crypto_ops.expect_backup_luks_header().times(0);
-        }
-
         DiskEncryptionKeyExchangeClientAgent::new(
             self.client_guestos_config.clone(),
             SevRootCertificateVerification::TestOnlySkipVerification,
@@ -339,15 +314,6 @@ impl DiskEncryptionKeyExchangeTestFixture {
         );
     }
 
-    fn verify_client_luks_header_matches(&self, expected: &[u8]) {
-        let luks_header = std::fs::read(self.client_store_luks_header.path())
-            .expect("Failed to read client Store LUKS header");
-        assert_eq!(
-            luks_header, expected,
-            "Store LUKS header file content does not match expected content"
-        );
-    }
-
     fn verify_no_artifacts_persisted(&self) {
         assert!(
             std::fs::read(self.previous_key.path()).unwrap().is_empty(),
@@ -417,23 +383,6 @@ async fn test_exchange_keys_successfully() {
     fixture.verify_luks_header_populated();
 }
 
-#[tokio::test]
-async fn test_exchange_keys_recovers_luks_header_when_server_omits_it() {
-    let recovered_luks_header = sample_luks_header_bytes();
-    let fixture = DiskEncryptionKeyExchangeTestFixture::new(TestConfig {
-        server_returns_luks_header: false,
-        client_recovered_luks_header: Some(recovered_luks_header.clone()),
-        ..Default::default()
-    });
-    let (server_result, client_result) = fixture.run_key_exchange_test().await;
-
-    server_result.expect("Key exchange should succeed");
-    client_result.expect("Key exchange should succeed");
-
-    fixture.verify_previous_key_populated();
-    fixture.verify_client_luks_header_matches(&recovered_luks_header);
-}
-
 #[tokio::test]
 async fn test_client_measurement_not_in_registry() {
     let config = TestConfig {

rs/ic_os/os_tools/guest_disk/src/crypt.rs

@@ -6,9 +6,11 @@ use libcryptsetup_rs::consts::vals::{CryptKdf, EncryptionFormat, KeyslotInfo};
 use libcryptsetup_rs::{
     CryptDevice, CryptInit, CryptParamsLuks2Ref, CryptSettingsHandle, CryptTokenInfo, TokenInput,
 };
+use prometheus::Registry;
 use serde::{Deserialize, Serialize};
 use std::fs;
-use std::fs::File;
+use std::fs::{File, OpenOptions};
+use std::io::{Read, Write};
 use std::os::unix::fs::PermissionsExt;
 use std::path::Path;
 use tracing::{info, warn};
@@ -141,7 +143,7 @@ pub fn activate_crypt_device(
     passphrase: &[u8],
     flags: CryptActivate,
     verify_luks_params: bool,
-    metrics_file: Option<&Path>,
+    metrics_registry: Option<&Registry>,
 ) -> Result<(CryptDevice, u32)> {
     let mut crypt_device = open_luks2_device(device_path, header_location)?;
 
@@ -153,9 +155,9 @@ pub fn activate_crypt_device(
         .activate_by_passphrase(Some(name), None, passphrase, flags)
         .context("Failed to activate cryptographic device")?;
 
-    if let Some(metrics_file) = metrics_file {
+    if let Some(registry) = metrics_registry {
         let log_result = luks_parameters.and_then(|luks_parameters| {
-            export_luks_parameters(metrics_file, &luks_parameters, device_path, active_keyslot)
+            export_luks_parameters(registry, &luks_parameters, device_path, active_keyslot)
         });
         if let Err(e) = log_result {
             warn!("Failed to export LUKS parameters: {e:#}");
@@ -210,6 +212,10 @@ pub fn format_crypt_device(
             .context("Failed to create detached LUKS header file")?
             .set_len(16 * 1024 * 1024)
             .context("Failed to set size of detached LUKS header file")?;
+        // The replica (running as user ic-replica) needs read access to the detached header so
+        // that it can share it during an upgrade.
+        fs::set_permissions(header_path, fs::Permissions::from_mode(0o644))
+            .context("Failed to set permissions on detached LUKS header file")?;
     }
 
     let mut crypt_device = obtain_crypt_device_handle(device_path, header_location)?;
@@ -311,6 +317,60 @@ pub fn backup_luks_header_to_file(device_path: &Path, header_path: &Path) -> Res
     Ok(())
 }
 
+/// Checks whether the device at `device_path` contains a valid LUKS2 header in the attached
+/// (on-device) position.
+pub fn has_attached_luks_header(device_path: &Path) -> Result<bool> {
+    const LUKS_MAGIC: &[u8; 4] = b"LUKS";
+
+    let mut start = vec![];
+    std::fs::File::open(device_path)
+        .context("Failed to open device for header check")?
+        .take(LUKS_MAGIC.len() as u64)
+        .read_to_end(&mut start)
+        .context("Failed to read first few bytes for header check")?;
+
+    Ok(start == LUKS_MAGIC)
+}
+
+/// Wipes (zeroizes) the header area of the device at `device_path`, removing any attached LUKS
+/// header.
+///
+/// This is used when the Store partition is opened with a detached header: if the data device still
+/// carries a legacy attached header (from an older GuestOS that wrote both), we wipe it so that
+/// only the detached header remains going forward.
+pub fn wipe_attached_luks_header(device_path: &Path) -> Result<()> {
+    let mut crypt_device = open_luks2_device(device_path, LuksHeaderLocation::Attached)
+        .context("Failed to open LUKS device to determine header size")?;
+    // `get_data_offset` returns the offset in 512-byte sectors; convert to bytes.
+    let data_offset_sectors = crypt_device.status_handle().get_data_offset();
+    let header_size_bytes = data_offset_sectors * 512;
+
+    let mut device = OpenOptions::new()
+        .write(true)
+        .open(device_path)
+        .with_context(|| {
+            format!(
+                "Failed to open device {} for writing",
+                device_path.display()
+            )
+        })?;
+    device
+        .write_all(&vec![0_u8; header_size_bytes as usize])
+        .with_context(|| {
+            format!(
+                "Failed to wipe LUKS header on device {}",
+                device_path.display()
+            )
+        })?;
+    device.flush().with_context(|| {
+        format!(
+            "Failed to flush LUKS header wipe on device {}",
+            device_path.display()
+        )
+    })?;
+    Ok(())
+}
+
 /// Checks if the LUKS parameters match the expected values set in format_crypt_device.
 /// If verify_luks_params is false, it will only log a warning if the verification fails.
 fn maybe_verify_luks_parameters(