libvirt: Don't reuse a stale secure boot VARS template

crates/integration-tests/src/tests/libvirt_verb.rs

@@ -1201,3 +1201,101 @@ fn test_libvirt_run_journal_output() -> TestResult {
     Ok(())
 }
 integration_test!(test_libvirt_run_journal_output);
+
+/// Generate a secure boot key set (PK, KEK, db, GUID.txt) whose certificates
+/// have `CN=<name>-<key>`.
+fn generate_secure_boot_keys(sh: &xshell::Shell, dir: &std::path::Path, name: &str) -> TestResult {
+    std::fs::create_dir_all(dir)?;
+    for key in ["PK", "KEK", "db"] {
+        let keyfile = dir.join(format!("{key}.key"));
+        let crtfile = dir.join(format!("{key}.crt"));
+        let subject = format!("/CN={name}-{key}/");
+        cmd!(
+            sh,
+            "openssl req -x509 -newkey rsa:2048 -nodes -days 1 -subj {subject} -keyout {keyfile} -out {crtfile}"
+        )
+        .quiet()
+        .ignore_stderr()
+        .run()?;
+    }
+    std::fs::write(dir.join("GUID.txt"), uuid::Uuid::new_v4().to_string())?;
+    Ok(())
+}
+
+/// Recreating a VM under the same name must enroll the new secure boot keys
+/// rather than reuse the old VM's OVMF_VARS, and `libvirt rm` must remove
+/// that file (libvirt does, as the domain's NVRAM).
+fn test_libvirt_secure_boot_vars_not_reused() -> TestResult {
+    let sh = shell()?;
+    if cmd!(sh, "virt-fw-vars --help")
+        .quiet()
+        .ignore_stdout()
+        .ignore_stderr()
+        .run()
+        .is_err()
+    {
+        println!("Skipping test: virt-fw-vars not found");
+        return Ok(());
+    }
+    let bck = get_bck_command()?;
+    let test_image = &get_test_image();
+    let label = LIBVIRT_INTEGRATION_TEST_LABEL;
+    let domain_name = format!("test-sb-vars-{}", random_suffix());
+    let tmp = tempfile::TempDir::new()?;
+    let (keys_a, keys_b) = (tmp.path().join("a"), tmp.path().join("b"));
+    generate_secure_boot_keys(&sh, &keys_a, "bcvk-test-a")?;
+    generate_secure_boot_keys(&sh, &keys_b, "bcvk-test-b")?;
+
+    defer! {
+        cleanup_domain(&domain_name);
+    }
+
+    let run_with_keys = |keys: &std::path::Path| {
+        cmd!(
+            sh,
+            "{bck} libvirt run --filesystem ext4 --name {domain_name} --label {label} --secure-boot-keys {keys} {test_image}"
+        )
+        .run()
+    };
+    // Returns the NVRAM path and the enrolled PK's subject
+    let enrolled_pk = || -> anyhow::Result<(String, String)> {
+        let xml = cmd!(sh, "virsh dumpxml {domain_name}").read()?;
+        let dom = parse_xml_dom(&xml).map_err(|e| anyhow::anyhow!("parsing domain XML: {e}"))?;
+        let nvram = dom
+            .find("nvram")
+            .map(|n| n.text_content().trim().to_owned())
+            .filter(|p| !p.is_empty())
+            .ok_or_else(|| anyhow::anyhow!("no nvram path in domain XML"))?;
+        let vars = cmd!(sh, "virt-fw-vars -i {nvram} --print --verbose")
+            .ignore_stderr()
+            .read()?;
+        let subject = vars
+            .lines()
+            .skip_while(|l| !l.starts_with("name=PK "))
+            .find_map(|l| l.trim().strip_prefix("subject "))
+            .ok_or_else(|| anyhow::anyhow!("no PK enrolled in {nvram}"))?
+            .to_owned();
+        Ok((nvram, subject))
+    };
+
+    run_with_keys(&keys_a)?;
+    let (nvram, subject) = enrolled_pk()?;
+    assert_eq!(subject, "CN=bcvk-test-a-PK");
+    let stale = tmp.path().join("stale_VARS.fd");
+    std::fs::copy(&nvram, &stale)?;
+
+    cmd!(sh, "{bck} libvirt rm --force --stop {domain_name}").run()?;
+    assert!(
+        !std::path::Path::new(&nvram).exists(),
+        "libvirt rm left {nvram} behind"
+    );
+
+    // Put the old file back, as an older bcvk or a transient VM leaves it
+    std::fs::copy(&stale, &nvram)?;
+    run_with_keys(&keys_b)?;
+    let (_, subject) = enrolled_pk()?;
+    assert_eq!(subject, "CN=bcvk-test-b-PK");
+
+    Ok(())
+}
+integration_test!(test_libvirt_secure_boot_vars_not_reused);

crates/kit/src/libvirt/domain.rs

@@ -52,8 +52,8 @@ pub struct DomainBuilder {
     tpm: bool,
     ovmf_code_path: Option<String>, // Custom OVMF_CODE path for secure boot
     ovmf_code_format: Option<String>, // Format of OVMF_CODE (raw, qcow2)
-    nvram_template: Option<String>, // Custom NVRAM template with enrolled keys
-    nvram_format: Option<String>,   // Format of NVRAM template (raw, qcow2)
+    nvram_path: Option<String>,     // NVRAM file with enrolled keys
+    nvram_format: Option<String>,   // Format of the NVRAM file (raw, qcow2)
     firmware_log: Option<FirmwareLogOutput>, // OVMF debug log output via isa-debugcon
     virtio_console_log: Option<String>, // Virtio console log file path (hvc0 — OS/journald)
     serial_console_log: Option<String>, // Serial console log file path (ttyS0 — UEFI/bootloader)
@@ -89,7 +89,7 @@ impl DomainBuilder {
             tpm: true,      // Default to enabled
             ovmf_code_path: None,
             ovmf_code_format: None,
-            nvram_template: None,
+            nvram_path: None,
             nvram_format: None,
             firmware_log: None,
             virtio_console_log: None,
@@ -190,12 +190,13 @@ impl DomainBuilder {
         self
     }
 
-    /// Set custom NVRAM template path and format with enrolled secure boot keys
+    /// Set the NVRAM file path and format, e.g. with enrolled secure boot keys
     ///
+    /// The file is used as is, and removed by `virsh undefine --nvram`.
     /// Format must be specified (either "raw" or "qcow2") and should come from
     /// the QEMU firmware interop JSON descriptors.
-    pub fn with_nvram_template(mut self, path: &str, format: &str) -> Self {
-        self.nvram_template = Some(path.to_string());
+    pub fn with_nvram(mut self, path: &str, format: &str) -> Self {
+        self.nvram_path = Some(path.to_string());
         self.nvram_format = Some(format.to_string());
         self
     }
@@ -340,21 +341,17 @@ impl DomainBuilder {
                 }
                 writer.write_text_element_with_attrs("loader", ovmf_code, &loader_attrs)?;
 
-                // Add NVRAM element if template is specified
-                if let Some(ref nvram_template) = self.nvram_template {
+                // Add NVRAM element if a path is specified
+                if let Some(ref nvram_path) = self.nvram_path {
                     // Format is required and comes from QEMU firmware interop JSON descriptors
                     let nvram_fmt = self
                         .nvram_format
                         .as_deref()
-                        .expect("nvram_format must be set when nvram_template is set");
+                        .expect("nvram_format must be set when nvram_path is set");
                     writer.write_text_element_with_attrs(
                         "nvram",
-                        "", // Empty content, template attr provides the source
-                        &[
-                            ("template", nvram_template),
-                            ("templateFormat", nvram_fmt),
-                            ("format", nvram_fmt),
-                        ],
+                        nvram_path,
+                        &[("format", nvram_fmt)],
                     )?;
                 }
             } else if secure_boot {
@@ -847,16 +844,18 @@ mod tests {
             .with_name("test-custom-secboot")
             .with_firmware(FirmwareType::UefiSecure)
             .with_ovmf_code_path("/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd", "raw")
-            .with_nvram_template("/var/lib/libvirt/qemu/nvram/custom_VARS.fd", "raw")
+            .with_nvram("/var/lib/libvirt/images/custom_VARS.fd", "raw")
             .build_xml()
             .unwrap();
 
         // Should have custom loader path
         assert!(xml.contains("/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd"));
 
-        // Should have nvram template
-        assert!(xml.contains("nvram"));
-        assert!(xml.contains("template=\"/var/lib/libvirt/qemu/nvram/custom_VARS.fd\""));
+        // The NVRAM is the custom file itself, not a template libvirt copies
+        assert!(
+            xml.contains(r#"<nvram format="raw">/var/lib/libvirt/images/custom_VARS.fd</nvram>"#)
+        );
+        assert!(!xml.contains("template="));
 
         // Should have secure loader attributes
         assert!(xml.contains("readonly=\"yes\""));

crates/kit/src/libvirt/run.rs

@@ -1211,10 +1211,10 @@ fn create_libvirt_domain_from_disk(
 
         eyre::ensure!(opts.firmware == FirmwareType::UefiSecure);
 
-        // Place the OVMF vars file in the libvirt storage pool so it's lifecycled with the VM
+        // The VARS file is the domain's NVRAM, so `virsh undefine --nvram` removes it
         let pool_path = get_libvirt_storage_pool_path(global_opts.connect.as_deref())
             .context("Failed to get libvirt storage pool path for secure boot vars")?;
-        let vars_output_path = pool_path.join(format!("{}_OVMF_VARS.fd", domain_name));
+        let vars_output_path = pool_path.join(secureboot::vars_filename(domain_name));
 
         info!("Setting up secure boot configuration from {}", keys);
         let config = secureboot::setup_secure_boot(&keys, &vars_output_path)
@@ -1281,14 +1281,14 @@ fn create_libvirt_domain_from_disk(
         let firmware_info = crate::libvirt::secureboot::find_secure_boot_firmware()
             .context("Failed to find secure boot firmware")?;
         let sb_vars_path = sb_config
-            .vars_template
+            .vars_path
             .canonicalize_utf8()
             .context("Canonicalizing secureboot vars path")?;
 
         // Use the formats from the firmware descriptors
         domain_builder = domain_builder
             .with_ovmf_code_path(firmware_info.code_path.as_str(), &firmware_info.code_format)
-            .with_nvram_template(sb_vars_path.as_str(), &sb_config.vars_format);
+            .with_nvram(sb_vars_path.as_str(), &sb_config.vars_format);
 
         // Add secure boot keys path to metadata for reference
         domain_builder =

crates/kit/src/libvirt/secureboot.rs

@@ -128,9 +128,9 @@ pub struct FirmwareInfo {
 pub struct SecureBootConfig {
     /// Directory containing the secure boot keys
     pub key_dir: Utf8PathBuf,
-    /// Path to custom OVMF_VARS template with enrolled keys
-    pub vars_template: Utf8PathBuf,
-    /// Format of the NVRAM template file (raw, qcow2)
+    /// Path to the domain's OVMF_VARS with enrolled keys
+    pub vars_path: Utf8PathBuf,
+    /// Format of the OVMF_VARS file (raw, qcow2)
     pub vars_format: String,
     /// GUID for the key owner
     #[allow(dead_code)]
@@ -260,10 +260,17 @@ pub fn customize_ovmf_vars(
     Ok(())
 }
 
+/// File name of the per-domain OVMF_VARS with enrolled keys, which is kept in
+/// the libvirt storage pool.
+pub(crate) fn vars_filename(domain_name: &str) -> String {
+    format!("{domain_name}_OVMF_VARS.fd")
+}
+
 /// Load and setup secure boot configuration from existing keys
 ///
-/// The `vars_output_path` should be in the libvirt storage pool so that
-/// the OVMF vars file is lifecycled with the VM (e.g., deleted with `--nvram`).
+/// The `vars_output_path` becomes the domain's NVRAM, so libvirt removes it
+/// when the domain is undefined with `--nvram`. It is always regenerated: a
+/// file left behind (e.g. by a transient VM) may have different keys enrolled.
 pub fn setup_secure_boot(
     key_dir: &Utf8Path,
     vars_output_path: &Utf8Path,
@@ -274,19 +281,16 @@ pub fn setup_secure_boot(
     // Find the system firmware (includes format info)
     let firmware_info = find_firmware_from_descriptors(true)?;
 
-    // Check if custom vars template already exists at the output path
-    if !vars_output_path.exists() {
-        tracing::info!(
-            "Creating custom OVMF_VARS template with enrolled keys at {}",
-            vars_output_path
-        );
-        customize_ovmf_vars(&keys, &firmware_info.vars_path, vars_output_path)?;
-    }
+    tracing::info!(
+        "Creating OVMF_VARS with enrolled keys at {}",
+        vars_output_path
+    );
+    customize_ovmf_vars(&keys, &firmware_info.vars_path, vars_output_path)?;
 
     // virt-fw-vars preserves the input format, so the output has the same format as the input
     Ok(SecureBootConfig {
         key_dir: key_dir.to_owned(),
-        vars_template: vars_output_path.to_owned(),
+        vars_path: vars_output_path.to_owned(),
         vars_format: firmware_info.vars_format,
         guid: keys.guid,
     })