tool: write all generation artifacts at once

Previously, generations were installed one after another. Now all
artifacts (kernels, initrd etc.) are first collected and then installed.
This way the writes to the ESP are reduced as duplicate paths are
already removed in the collection phase.

rust/tool/src/install.rs

@@ -1,3 +1,4 @@
+use std::collections::HashMap;
 use std::fs;
 use std::os::unix::prelude::PermissionsExt;
 use std::path::{Path, PathBuf};
@@ -5,6 +6,7 @@ use std::process::Command;
 
 use anyhow::{Context, Result};
 use nix::unistd::sync;
+use tempfile::TempDir;
 
 use crate::esp::{EspGenerationPaths, EspPaths};
 use crate::gc::Roots;
@@ -71,7 +73,7 @@ impl Installer {
                 .take(self.configuration_limit)
                 .collect()
         };
-        self.install_links(links)?;
+        self.install_generations_from_links(&links)?;
 
         self.install_systemd_boot()?;
 
@@ -93,9 +95,66 @@ impl Installer {
         Ok(())
     }
 
-    fn install_links(&mut self, links: Vec<GenerationLink>) -> Result<()> {
+    /// Install all generations from the provided `GenerationLinks`.
+    ///
+    /// Iterates over the links twice:
+    ///     (1) First, building all unsigned artifacts and storing the mapping from source to
+    ///     destination in `GenerationArtifacts`. `GenerationArtifacts` ensures that there are no
+    ///     duplicate destination paths and thus ensures that the hashes embedded in the lanzaboote
+    ///     image do not get invalidated because the files to which they point get overwritten by a
+    ///     later generation.
+    ///     (2) Second, building all signed artifacts using the previously built mapping from source to
+    ///     destination in the `GenerationArtifacts`.
+    ///
+    /// This way, in the second step, all paths and thus all hashes for all generations are already
+    /// known. The signed files can now be constructed with known good hashes **across** all
+    /// generations.
+    fn install_generations_from_links(&mut self, links: &[GenerationLink]) -> Result<()> {
+        // This struct must live for the entire lifetime of this function so that the contained
+        // tempdir does not go out of scope and thus does not get deleted.
+        let mut generation_artifacts =
+            GenerationArtifacts::new().context("Failed to create GenerationArtifacts.")?;
+
+        self.build_generation_artifacts_from_links(
+            &mut generation_artifacts,
+            links,
+            Self::build_unsigned_generation_artifacts,
+        )
+        .context("Failed to build unsigned generation artifacts.")?;
+
+        self.build_generation_artifacts_from_links(
+            &mut generation_artifacts,
+            links,
+            Self::build_signed_generation_artifacts,
+        )
+        .context("Failed to build signed generation artifacts.")?;
+
+        generation_artifacts
+            .install(&self.key_pair)
+            .context("Failed to install files.")?;
+
+        // Sync files to persistent storage. This may improve the
+        // chance of a consistent boot directory in case the system
+        // crashes.
+        sync();
+
+        Ok(())
+    }
+
+    /// Build all generation artifacts from a list of `GenerationLink`s.
+    ///
+    /// This function accepts a closure to build the generation artifacts for a single generation.
+    fn build_generation_artifacts_from_links<F>(
+        &mut self,
+        generation_artifacts: &mut GenerationArtifacts,
+        links: &[GenerationLink],
+        mut build_generation_artifacts: F,
+    ) -> Result<()>
+    where
+        F: FnMut(&mut Self, &Generation, &mut GenerationArtifacts) -> Result<()>,
+    {
         for link in links {
-            let generation_result = Generation::from_link(&link)
+            let generation_result = Generation::from_link(link)
                 .with_context(|| format!("Failed to build generation from link: {link:?}"));
 
             // Ignore failing to read a generation so that old malformed generations do not stop
@@ -108,43 +167,38 @@ impl Installer {
                 }
             };
 
-            println!("Installing generation {generation}");
+            build_generation_artifacts(self, &generation, generation_artifacts)
-
+                .context("Failed to build generation artifacts.")?;
-            self.install_generation(&generation)
-                .context("Failed to install generation")?;
 
             for (name, bootspec) in &generation.spec.bootspec.specialisation {
                 let specialised_generation = generation.specialise(name, bootspec)?;
 
-                println!("Installing specialisation: {name} of generation: {generation}");
+                build_generation_artifacts(self, &specialised_generation, generation_artifacts)
-
+                    .context("Failed to build generation artifacts for specialisation.")?;
-                self.install_generation(&specialised_generation)
-                    .context("Failed to install specialisation")?;
             }
         }
         Ok(())
     }
 
-    fn install_generation(&mut self, generation: &Generation) -> Result<()> {
+    /// Build the unsigned generation artifacts for a single generation.
+    ///
+    /// Stores the mapping from source to destination for the artifacts in the provided
+    /// `GenerationArtifacts`. Does not install any files to the ESP.
+    ///
+    /// Because this function already has an complete view of all required paths in the ESP for
+    /// this generation, it stores all paths as GC roots.
+    fn build_unsigned_generation_artifacts(
+        &mut self,
+        generation: &Generation,
+        generation_artifacts: &mut GenerationArtifacts,
+    ) -> Result<()> {
+        let tempdir = &generation_artifacts.tempdir;
+
         let bootspec = &generation.spec.bootspec;
 
         let esp_gen_paths = EspGenerationPaths::new(&self.esp_paths, generation)?;
         self.gc_roots.extend(esp_gen_paths.to_iter());
 
-        let kernel_cmdline =
-            assemble_kernel_cmdline(&bootspec.init, bootspec.kernel_params.clone());
-
-        // This tempdir must live for the entire lifetime of the current function.
-        let tempdir = tempfile::tempdir()?;
-
-        let os_release = OsRelease::from_generation(generation)
-            .context("Failed to build OsRelease from generation.")?;
-        let os_release_path = tempdir
-            .write_secure_file(os_release.to_string().as_bytes())
-            .context("Failed to write os-release file.")?;
-
-        println!("Appending secrets to initrd...");
-
         let initrd_content = fs::read(
             bootspec
                 .initrd
@@ -158,47 +212,70 @@ impl Installer {
             append_initrd_secrets(initrd_secrets_script, &initrd_location)?;
         }
 
-        // The initrd and kernel don't need to be signed.
+        // The initrd and kernel don't need to be signed. The stub has their hashes embedded and
-        // The stub has their hashes embedded and will refuse loading on hash mismatches.
+        // will refuse loading on hash mismatches.
         //
-        // The initrd and kernel are not forcibly installed because they are not built
+        // The kernel is not signed because systemd-boot could be tricked into loading the signed
-        // reproducibly. Forcibly installing (i.e. overwriting) them is likely to break older
+        // kernel in combination with an malicious unsigned initrd. This could be achieved because
-        // generations that point to the same initrd/kernel because the hash embedded in the stub
+        // systemd-boot also honors the type #1 boot loader specification.
-        // will not match anymore.
+        generation_artifacts.add_unsigned(&bootspec.kernel, &esp_gen_paths.kernel);
-        install(&initrd_location, &esp_gen_paths.initrd)
+        generation_artifacts.add_unsigned(&initrd_location, &esp_gen_paths.initrd);
-            .context("Failed to install initrd to ESP")?;
+
-        // Do not sign the kernel.
+        Ok(())
-        // Boot loader specification could be used to make a signed kernel load an unprotected initrd.
+    }
-        install(&bootspec.kernel, &esp_gen_paths.kernel)
+
-            .context("Failed to install kernel to ESP.")?;
+    /// Build the signed generation artifacts for a single generation.
+    ///
+    /// Stores the mapping from source to destination for the artifacts in the provided
+    /// `GenerationArtifacts`. Does not install any files to the ESP.
+    ///
+    /// This function expects an already pre-populated `GenerationArtifacts`. It can only be called
+    /// if ALL unsigned artifacts are already built and stored in `GenerationArtifacts`. More
+    /// specifically, this function can only be called after `build_unsigned_generation_artifacts`
+    /// has been executed.
+    fn build_signed_generation_artifacts(
+        &mut self,
+        generation: &Generation,
+        generation_artifacts: &mut GenerationArtifacts,
+    ) -> Result<()> {
+        let tempdir = &generation_artifacts.tempdir;
+
+        let bootspec = &generation.spec.bootspec;
+
+        let esp_gen_paths = EspGenerationPaths::new(&self.esp_paths, generation)?;
+
+        let kernel_cmdline =
+            assemble_kernel_cmdline(&bootspec.init, bootspec.kernel_params.clone());
+
+        let os_release = OsRelease::from_generation(generation)
+            .context("Failed to build OsRelease from generation.")?;
+        let os_release_path = tempdir
+            .write_secure_file(os_release.to_string().as_bytes())
+            .context("Failed to write os-release file.")?;
+
+        let kernel_path = generation_artifacts
+            .unsigned_files
+            .get(&esp_gen_paths.kernel)
+            .context("Failed to retrieve kernel path from GenerationArtifacts.")?;
+
+        let initrd_path = generation_artifacts
+            .unsigned_files
+            .get(&esp_gen_paths.initrd)
+            .context("Failed to retrieve initrd path from GenerationArtifacts.")?;
 
         let lanzaboote_image = pe::lanzaboote_image(
-            &tempdir,
+            tempdir,
             &self.lanzaboote_stub,
             &os_release_path,
             &kernel_cmdline,
-            &esp_gen_paths.kernel,
+            kernel_path,
-            &esp_gen_paths.initrd,
+            initrd_path,
+            &esp_gen_paths,
             &self.esp_paths.esp,
         )
-        .context("Failed to assemble stub")?;
+        .context("Failed to assemble lanzaboote image.")?;
 
-        install_signed(
+        generation_artifacts.add_signed(&lanzaboote_image, &esp_gen_paths.lanzaboote_image);
-            &self.key_pair,
-            &lanzaboote_image,
-            &esp_gen_paths.lanzaboote_image,
-        )
-        .context("Failed to install lanzaboote")?;
-
-        // Sync files to persistent storage. This may improve the
-        // chance of a consistent boot directory in case the system
-        // crashes.
-        sync();
-
-        println!(
-            "Successfully installed lanzaboote to '{}'",
-            self.esp_paths.esp.display()
-        );
 
         Ok(())
     }
@@ -243,6 +320,65 @@ impl Installer {
     }
 }
 
+/// Stores the source and destination of all artifacts needed to install all generations.
+///
+/// The key feature of this data structure is that the mappings are automatically deduplicated
+/// because they are stored in a HashMap using the destination as the key. Thus, there is only
+/// unique destination paths.
+///
+/// This enables a two step installaton process where all artifacts across all generations are
+/// first collected and then installed. This deduplication in the collection phase reduces the
+/// number of accesesses and writes to the ESP. More importantly, however, in the second step, all
+/// paths on the ESP are uniqely determined and the images can be generated while being sure that
+/// the hashes embedded in them will point to a valid file on the ESP because the file will not be
+/// overwritten by a later generation.
+struct GenerationArtifacts {
+    /// Temporary directory that stores all temporary files that are created when building the
+    /// GenerationArtifacts.
+    tempdir: TempDir,
+    /// Mapping of signed files from their destinations to their source.
+    signed_files: HashMap<PathBuf, PathBuf>,
+    /// Mapping of unsigned files from their destinations to their source.
+    unsigned_files: HashMap<PathBuf, PathBuf>,
+}
+
+impl GenerationArtifacts {
+    fn new() -> Result<Self> {
+        Ok(Self {
+            tempdir: TempDir::new().context("Failed to create temporary directory.")?,
+            signed_files: HashMap::new(),
+            unsigned_files: HashMap::new(),
+        })
+    }
+
+    /// Add source and destination of a PE file to be signed.
+    ///
+    /// Files are stored in the HashMap using their destination path as the key to ensure that the
+    /// destination paths are unique.
+    fn add_signed(&mut self, from: &Path, to: &Path) {
+        self.signed_files.insert(to.into(), from.into());
+    }
+
+    /// Add source and destination of an arbitrary file.
+    fn add_unsigned(&mut self, from: &Path, to: &Path) {
+        self.unsigned_files.insert(to.into(), from.into());
+    }
+
+    /// Install all files to the ESP.
+    fn install(&self, key_pair: &KeyPair) -> Result<()> {
+        for (to, from) in &self.signed_files {
+            install_signed(key_pair, from, to)
+                .with_context(|| format!("Failed to sign and install from {from:?} to {to:?}"))?;
+        }
+
+        for (to, from) in &self.unsigned_files {
+            install(from, to)
+                .with_context(|| format!("Failed to install from {from:?} to {to:?}"))?;
+        }
+        Ok(())
+    }
+}
+
 /// Install a PE file. The PE gets signed in the process.
 ///
 /// The file is only signed and copied if it doesn't exist at the destination

rust/tool/src/pe.rs

@@ -7,35 +7,37 @@ use std::process::Command;
 use anyhow::{Context, Result};
 use goblin::pe::PE;
 use sha2::{Digest, Sha256};
+use tempfile::TempDir;
 
+use crate::esp::EspGenerationPaths;
 use crate::utils::{tmpname, SecureTempDirExt};
 
 type Hash = sha2::digest::Output<Sha256>;
 
-/// Attach all information that lanzaboote needs into the PE binary.
+/// Assemble a lanzaboote image.
-///
+#[allow(clippy::too_many_arguments)]
-/// When this function is called the referenced files already need to
-/// be present in the ESP. This is required, because we need to read
-/// them to compute hashes.
 pub fn lanzaboote_image(
     // Because the returned path of this function is inside the tempdir as well, the tempdir must
     // live longer than the function. This is why it cannot be created inside the function.
-    tempdir: &tempfile::TempDir,
+    tempdir: &TempDir,
     lanzaboote_stub: &Path,
     os_release: &Path,
     kernel_cmdline: &[String],
     kernel_path: &Path,
     initrd_path: &Path,
+    esp_gen_paths: &EspGenerationPaths,
     esp: &Path,
 ) -> Result<PathBuf> {
     // objcopy can only copy files into the PE binary. That's why we
     // have to write the contents of some bootspec properties to disk.
     let kernel_cmdline_file = tempdir.write_secure_file(kernel_cmdline.join(" "))?;
 
-    let kernel_path_file = tempdir.write_secure_file(esp_relative_uefi_path(esp, kernel_path)?)?;
+    let kernel_path_file =
+        tempdir.write_secure_file(esp_relative_uefi_path(esp, &esp_gen_paths.kernel)?)?;
     let kernel_hash_file = tempdir.write_secure_file(file_hash(kernel_path)?.as_slice())?;
 
-    let initrd_path_file = tempdir.write_secure_file(esp_relative_uefi_path(esp, initrd_path)?)?;
+    let initrd_path_file =
+        tempdir.write_secure_file(esp_relative_uefi_path(esp, &esp_gen_paths.initrd)?)?;
     let initrd_hash_file = tempdir.write_secure_file(file_hash(initrd_path)?.as_slice())?;
 
     let os_release_offs = stub_offset(lanzaboote_stub)?;