From 240914d763c64fb2e4b932804e7273a95d085862 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Sat, 12 Aug 2023 15:23:21 +0200 Subject: [PATCH 1/5] tool: make kernels and initrds content-addressed Kernels and initrds on the ESP are now content-addressed. By definition, it is impossible for two different kernels or initrds to ever end up at the same place, even in the presence of changing initrd secrets or other unreproducibility. The basic advantage of this is that installing the kernel or initrd for a generation can never break another generation. In turn, this enables the following two improvements: * All generations can be installed independently. In particular, the installation can be performed in one pass, one generation at a time. As a result, the code is significantly simplified, and memory usage (due to the temporary files) does not grow with the number of generations any more. * Generations that already have their files in place on the ESP do not need to be reinstalled. This will be taken advantage of in a subsequent commit. --- nix/tests/lanzaboote.nix | 59 ++--- rust/tool/Cargo.lock | 7 + rust/tool/shared/src/esp.rs | 100 +------- rust/tool/shared/src/pe.rs | 16 +- rust/tool/systemd/Cargo.toml | 1 + rust/tool/systemd/src/install.rs | 321 ++++++++------------------ rust/tool/systemd/tests/common/mod.rs | 13 +- rust/tool/systemd/tests/gc.rs | 45 +++- rust/tool/systemd/tests/install.rs | 48 ++-- 9 files changed, 191 insertions(+), 419 deletions(-) diff --git a/nix/tests/lanzaboote.nix b/nix/tests/lanzaboote.nix index 8791882..55e6ae7 100644 --- a/nix/tests/lanzaboote.nix +++ b/nix/tests/lanzaboote.nix @@ -133,30 +133,13 @@ let # `src` is copied to `dst` inside th VM. Optionally append some random data # ("crap") to the end of the file at `dst`. This is useful to easily change # the hash of a file and produce a hash mismatch when booting the stub. - mkHashMismatchTest = { name, path, appendCrap ? false, useSecureBoot ? true }: mkSecureBootTest { + mkHashMismatchTest = { name, appendCrapGlob, useSecureBoot ? true }: mkSecureBootTest { inherit name; inherit useSecureBoot; testScript = '' - import json - import os.path - bootspec = None - - def convert_to_esp(store_file_path): - store_dir = os.path.basename(os.path.dirname(store_file_path)) - filename = os.path.basename(store_file_path) - return f'/boot/EFI/nixos/{store_dir}-{filename}.efi' - machine.start() - bootspec = json.loads(machine.succeed("cat /run/current-system/boot.json")).get('org.nixos.bootspec.v1') - assert bootspec is not None, "Unsupported bootspec version!" - src_path = ${path.src} - dst_path = ${path.dst} - machine.succeed(f"cp -rf {src_path} {dst_path}") - '' + lib.optionalString appendCrap '' - machine.succeed(f"echo Foo >> {dst_path}") - '' + - '' + machine.succeed("echo some_garbage_to_change_the_hash | tee -a ${appendCrapGlob} > /dev/null") machine.succeed("sync") machine.crash() machine.start() @@ -174,24 +157,12 @@ let # that would make the kernel still accept it. mkModifiedInitrdTest = { name, useSecureBoot }: mkHashMismatchTest { inherit name useSecureBoot; - - path = { - src = "bootspec.get('initrd')"; - dst = "convert_to_esp(bootspec.get('initrd'))"; - }; - - appendCrap = true; + appendCrapGlob = "/boot/EFI/nixos/initrd-*.efi"; }; mkModifiedKernelTest = { name, useSecureBoot }: mkHashMismatchTest { inherit name useSecureBoot; - - path = { - src = "bootspec.get('kernel')"; - dst = "convert_to_esp(bootspec.get('kernel'))"; - }; - - appendCrap = true; + appendCrapGlob = "/boot/EFI/nixos/kernel-*.efi"; }; in @@ -248,8 +219,9 @@ in # path) does not change. # # An unfortunate result of this NixOS feature is that updating the secrets - # without creating a new initrd might break previous generations. Lanzaboote - # has no control over that. + # without creating a new initrd might break previous generations. Verify that + # a new initrd (which is supposed to only differ by the secrets) is created + # in this case. # # This tests uses a specialisation to imitate a newer generation. This works # because `lzbt` installs the specialisation of a generation AFTER installing @@ -279,12 +251,19 @@ in machine.start() machine.wait_for_unit("multi-user.target") - # Assert that only two boot files exists (a single kernel and a single - # initrd). If there are two initrds, the test would not be able to test - # updating the secret of an already existing initrd. - assert int(machine.succeed("ls -1 /boot/EFI/nixos | wc -l")) == 2 + # Assert that only three boot files exists (a single kernel and a two + # initrds). + assert int(machine.succeed("ls -1 /boot/EFI/nixos | wc -l")) == 3 - # It is expected that the initrd contains the new secret. + # It is expected that the initrd contains the original secret. + machine.succeed("cmp ${originalSecret} /secret-from-initramfs") + + machine.succeed("bootctl set-default nixos-generation-1-specialisation-variant.efi") + machine.succeed("sync") + machine.crash() + machine.start() + machine.wait_for_unit("multi-user.target") + # It is expected that the initrd of the specialisation contains the new secret. machine.succeed("cmp ${newSecret} /secret-from-initramfs") ''; }; diff --git a/rust/tool/Cargo.lock b/rust/tool/Cargo.lock index 6af64c2..47314fb 100644 --- a/rust/tool/Cargo.lock +++ b/rust/tool/Cargo.lock @@ -103,6 +103,12 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" +[[package]] +name = "base32ct" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "396664016f30ad5ab761000391a5c0b436f7bfac738858263eb25897658b98c9" + [[package]] name = "bitflags" version = "1.3.2" @@ -501,6 +507,7 @@ version = "0.3.0" dependencies = [ "anyhow", "assert_cmd", + "base32ct", "clap", "expect-test", "filetime", diff --git a/rust/tool/shared/src/esp.rs b/rust/tool/shared/src/esp.rs index 73a1fa6..dbab2cc 100644 --- a/rust/tool/shared/src/esp.rs +++ b/rust/tool/shared/src/esp.rs @@ -1,13 +1,6 @@ -use std::{ - array::IntoIter, - path::{Path, PathBuf}, -}; - -use anyhow::{bail, Context, Result}; -use indoc::indoc; +use std::path::{Path, PathBuf}; use crate::architecture::Architecture; -use crate::generation::Generation; /// Generic ESP paths which can be specific to a bootloader pub trait EspPaths { @@ -23,94 +16,3 @@ pub trait EspPaths { /// Returns the path containing Linux EFI binaries fn linux_path(&self) -> &Path; } - -/// Paths to the boot files of a specific generation. -pub struct EspGenerationPaths { - pub kernel: PathBuf, - pub initrd: PathBuf, - pub lanzaboote_image: PathBuf, -} - -impl EspGenerationPaths { - pub fn new>( - esp_paths: &P, - generation: &Generation, - system: Architecture, - ) -> Result { - let bootspec = &generation.spec.bootspec.bootspec; - let bootspec_system: Architecture = Architecture::from_nixos_system(&bootspec.system)?; - - if system != bootspec_system { - bail!(indoc! {r#" - The CPU architecture declared in your module differs from the one declared in the - bootspec of the current generation. - "#}) - } - - Ok(Self { - kernel: esp_paths - .nixos_path() - .join(nixos_path(&bootspec.kernel, "bzImage")?), - initrd: esp_paths.nixos_path().join(nixos_path( - bootspec - .initrd - .as_ref() - .context("Lanzaboote does not support missing initrd yet")?, - "initrd", - )?), - lanzaboote_image: esp_paths.linux_path().join(generation_path(generation)), - }) - } - - /// Return the used file paths to store as garbage collection roots. - pub fn to_iter(&self) -> IntoIter<&PathBuf, 3> { - [&self.kernel, &self.initrd, &self.lanzaboote_image].into_iter() - } -} - -fn nixos_path(path: impl AsRef, name: &str) -> Result { - let resolved = path - .as_ref() - .read_link() - .unwrap_or_else(|_| path.as_ref().into()); - - let parent_final_component = resolved - .parent() - .and_then(|x| x.file_name()) - .and_then(|x| x.to_str()) - .with_context(|| format!("Failed to extract final component from: {:?}", resolved))?; - - let nixos_filename = format!("{}-{}.efi", parent_final_component, name); - - Ok(PathBuf::from(nixos_filename)) -} - -fn generation_path(generation: &Generation) -> PathBuf { - if let Some(specialisation_name) = generation.is_specialised() { - PathBuf::from(format!( - "nixos-generation-{}-specialisation-{}.efi", - generation, specialisation_name - )) - } else { - PathBuf::from(format!("nixos-generation-{}.efi", generation)) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn nixos_path_creates_correct_filename_from_nix_store_path() -> Result<()> { - let path = - Path::new("/nix/store/xqplddjjjy1lhzyzbcv4dza11ccpcfds-initrd-linux-6.1.1/initrd"); - - let generated_filename = nixos_path(path, "initrd")?; - - let expected_filename = - PathBuf::from("xqplddjjjy1lhzyzbcv4dza11ccpcfds-initrd-linux-6.1.1-initrd.efi"); - - assert_eq!(generated_filename, expected_filename); - Ok(()) - } -} diff --git a/rust/tool/shared/src/pe.rs b/rust/tool/shared/src/pe.rs index 2a8ebb2..3333c61 100644 --- a/rust/tool/shared/src/pe.rs +++ b/rust/tool/shared/src/pe.rs @@ -8,7 +8,6 @@ use anyhow::{Context, Result}; use goblin::pe::PE; use tempfile::TempDir; -use crate::esp::EspGenerationPaths; use crate::utils::{file_hash, tmpname, SecureTempDirExt}; /// Assemble a lanzaboote image. @@ -20,9 +19,10 @@ pub fn lanzaboote_image( lanzaboote_stub: &Path, os_release: &Path, kernel_cmdline: &[String], - kernel_path: &Path, - initrd_path: &Path, - esp_gen_paths: &EspGenerationPaths, + kernel_source: &Path, + kernel_target: &Path, + initrd_source: &Path, + initrd_target: &Path, esp: &Path, ) -> Result { // objcopy can only copy files into the PE binary. That's why we @@ -30,12 +30,12 @@ pub fn lanzaboote_image( let kernel_cmdline_file = tempdir.write_secure_file(kernel_cmdline.join(" "))?; 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())?; + tempdir.write_secure_file(esp_relative_uefi_path(esp, kernel_target)?)?; + let kernel_hash_file = tempdir.write_secure_file(file_hash(kernel_source)?.as_slice())?; 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())?; + tempdir.write_secure_file(esp_relative_uefi_path(esp, initrd_target)?)?; + let initrd_hash_file = tempdir.write_secure_file(file_hash(initrd_source)?.as_slice())?; let os_release_offs = stub_offset(lanzaboote_stub)?; let kernel_cmdline_offs = os_release_offs + file_size(os_release)?; diff --git a/rust/tool/systemd/Cargo.toml b/rust/tool/systemd/Cargo.toml index 33784be..0a0f240 100644 --- a/rust/tool/systemd/Cargo.toml +++ b/rust/tool/systemd/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" [dependencies] anyhow = "1.0.71" +base32ct = { version = "0.2.0", features = ["alloc"] } stderrlog = "0.5.4" log = { version = "0.4.18", features = ["std"] } clap = { version = "4.3.1", features = ["derive"] } diff --git a/rust/tool/systemd/src/install.rs b/rust/tool/systemd/src/install.rs index bdcb676..926ca9e 100644 --- a/rust/tool/systemd/src/install.rs +++ b/rust/tool/systemd/src/install.rs @@ -1,4 +1,5 @@ -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeSet; +use std::ffi::OsStr; use std::fs::{self, File}; use std::os::fd::AsRawFd; use std::os::unix::prelude::PermissionsExt; @@ -7,6 +8,7 @@ use std::process::Command; use std::string::ToString; use anyhow::{anyhow, Context, Result}; +use base32ct::{Base32Unpadded, Encoding}; use nix::unistd::syncfs; use tempfile::TempDir; @@ -14,7 +16,7 @@ use crate::architecture::SystemdArchitectureExt; use crate::esp::SystemdEspPaths; use crate::version::SystemdVersion; use lanzaboote_tool::architecture::Architecture; -use lanzaboote_tool::esp::{EspGenerationPaths, EspPaths}; +use lanzaboote_tool::esp::EspPaths; use lanzaboote_tool::gc::Roots; use lanzaboote_tool::generation::{Generation, GenerationLink}; use lanzaboote_tool::os_release::OsRelease; @@ -74,9 +76,7 @@ impl Installer { .map(GenerationLink::from_path) .collect::>>()?; - // Sort the links by version. The links need to always be sorted to ensure the secrets of - // the latest generation are appended to the initrd when multiple generations point to the - // same initrd. + // Sort the links by version, so that the limit actually skips the oldest generations. links.sort_by_key(|l| l.version); // A configuration limit of 0 means there is no limit. @@ -129,64 +129,7 @@ impl Installer { } /// 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. - let boot = File::open(&self.esp_paths.esp).context("Failed to open ESP root directory.")?; - syncfs(boot.as_raw_fd()).context("Failed to sync ESP filesystem.")?; - - 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( - &mut self, - generation_artifacts: &mut GenerationArtifacts, - links: &[GenerationLink], - mut build_generation_artifacts: F, - ) -> Result<()> - where - F: FnMut(&mut Self, &Generation, &mut GenerationArtifacts) -> Result<()>, - { let generations = links .iter() .filter_map(|link| { @@ -214,122 +157,124 @@ impl Installer { } for generation in generations { - build_generation_artifacts(self, &generation, generation_artifacts) - .context("Failed to build generation artifacts.")?; - + // The kernels and initrds are content-addressed. + // Thus, this cannot overwrite files of old generation with different content. + self.install_generation(&generation) + .context("Failed to install generation.")?; for (name, bootspec) in &generation.spec.bootspec.specialisations { let specialised_generation = generation.specialise(name, bootspec)?; - - 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.")?; } } + // Sync files to persistent storage. This may improve the + // chance of a consistent boot directory in case the system + // crashes. + let boot = File::open(&self.esp_paths.esp).context("Failed to open ESP root directory.")?; + syncfs(boot.as_raw_fd()).context("Failed to sync ESP filesystem.")?; + Ok(()) } - /// Build the unsigned generation artifacts for a single generation. + /// Install the given `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; - + /// The kernel and initrd are content-addressed, and the stub name identifies the generation. + /// Hence, this function cannot overwrite files of other generations with different contents. + /// All installed files are added as garbage collector roots. + fn install_generation(&mut self, generation: &Generation) -> Result<()> { + let tempdir = TempDir::new().context("Failed to create temporary directory.")?; let bootspec = &generation.spec.bootspec.bootspec; - let esp_gen_paths = EspGenerationPaths::new(&self.esp_paths, generation, self.arch)?; - self.gc_roots.extend(esp_gen_paths.to_iter()); + // The kernel is a file in /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-linux-/. + // (On x86, that file is called bzImage, but other architectures may differ.) + let kernel_dirname = bootspec + .kernel + .parent() + .and_then(Path::file_name) + .and_then(OsStr::to_str) + .context("Failed to extract the kernel directory name.")?; + let kernel_version = kernel_dirname + .rsplit('-') + .next() + .context("Failed to extract the kernel version.")?; - let initrd_content = fs::read( - bootspec - .initrd - .as_ref() - .context("Lanzaboote does not support missing initrd yet")?, - )?; + // Install the kernel and record its path on the ESP. + let kernel_target = self + .install_nixos_ca(&bootspec.kernel, &format!("kernel-{}", kernel_version)) + .context("Failed to install the kernel.")?; + + // Assemble and install the initrd, and record its path on the ESP. let initrd_location = tempdir - .write_secure_file(initrd_content) - .context("Failed to copy initrd to tempfile.")?; + .write_secure_file( + fs::read( + bootspec + .initrd + .as_ref() + .context("Lanzaboote does not support missing initrd yet.")?, + ) + .context("Failed to read the initrd.")?, + ) + .context("Failed to copy the initrd to the temporary directory.")?; if let Some(initrd_secrets_script) = &bootspec.initrd_secrets { append_initrd_secrets(initrd_secrets_script, &initrd_location)?; } + let initrd_target = self + .install_nixos_ca(&initrd_location, &format!("initrd-{}", kernel_version)) + .context("Failed to install the initrd.")?; - // The initrd and kernel don't need to be signed. The stub has their hashes embedded and - // will refuse loading on hash mismatches. - // - // The kernel is not signed because systemd-boot could be tricked into loading the signed - // kernel in combination with an malicious unsigned initrd. This could be achieved because - // systemd-boot also honors the type #1 boot loader specification. - generation_artifacts.add_unsigned(&bootspec.kernel, &esp_gen_paths.kernel); - generation_artifacts.add_unsigned(&initrd_location, &esp_gen_paths.initrd); - - Ok(()) - } - - /// 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.bootspec; - - let esp_gen_paths = EspGenerationPaths::new(&self.esp_paths, generation, self.arch)?; - - let kernel_cmdline = - assemble_kernel_cmdline(&bootspec.init, bootspec.kernel_params.clone()); - + // Assemble, sign and install the Lanzaboote stub. 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: &Path = generation_artifacts - .files - .get(&esp_gen_paths.kernel) - .context("Failed to retrieve kernel path from GenerationArtifacts.")? - .into(); - - let initrd_path = generation_artifacts - .files - .get(&esp_gen_paths.initrd) - .context("Failed to retrieve initrd path from GenerationArtifacts.")? - .into(); - + let kernel_cmdline = + assemble_kernel_cmdline(&bootspec.init, bootspec.kernel_params.clone()); let lanzaboote_image = pe::lanzaboote_image( - tempdir, + &tempdir, &self.lanzaboote_stub, &os_release_path, &kernel_cmdline, - kernel_path, - initrd_path, - &esp_gen_paths, + &bootspec.kernel, + &kernel_target, + &initrd_location, + &initrd_target, &self.esp_paths.esp, ) .context("Failed to assemble lanzaboote image.")?; - - generation_artifacts.add_signed(&lanzaboote_image, &esp_gen_paths.lanzaboote_image); + let stub_name = if let Some(specialisation_name) = generation.is_specialised() { + PathBuf::from(format!( + "nixos-generation-{}-specialisation-{}.efi", + generation, specialisation_name + )) + } else { + PathBuf::from(format!("nixos-generation-{}.efi", generation)) + }; + let stub_target = self.esp_paths.linux.join(stub_name); + self.gc_roots.extend([&stub_target]); + install_signed(&self.key_pair, &lanzaboote_image, &stub_target) + .context("Failed to install the Lanzaboote stub.")?; Ok(()) } + /// Install a content-addressed file to the `EFI/nixos` directory on the ESP. + /// + /// It is automatically added to the garbage collector roots. + /// The full path to the target file is returned. + fn install_nixos_ca(&mut self, from: &Path, label: &str) -> Result { + let hash = file_hash(from).context("Failed to read the source file.")?; + let to = self.esp_paths.nixos.join(format!( + "{}-{}.efi", + label, + Base32Unpadded::encode_string(&hash) + )); + self.gc_roots.extend([&to]); + install(from, &to)?; + Ok(to) + } + /// Install systemd-boot to ESP. /// /// systemd-boot is only updated when a newer version is available OR when the currently @@ -380,92 +325,6 @@ impl Installer { } } -/// A location in the ESP together with information whether the file -/// needs to be signed. -#[derive(Debug, Clone, PartialEq, Eq)] -enum FileSource { - SignedFile(PathBuf), - UnsignedFile(PathBuf), -} - -impl<'a> From<&'a FileSource> for &'a Path { - fn from(value: &'a FileSource) -> Self { - match value { - FileSource::SignedFile(p) | FileSource::UnsignedFile(p) => p, - } - } -} - -/// 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 installation 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 uniquely 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, - - /// A mapping from target location to source. - files: BTreeMap, -} - -impl GenerationArtifacts { - fn new() -> Result { - Ok(Self { - tempdir: TempDir::new().context("Failed to create temporary directory.")?, - files: Default::default(), - }) - } - - /// Add a file to be installed. - /// - /// Adding the same file multiple times with the same source is ok - /// and will drop the old source. - fn add_file(&mut self, from: FileSource, to: &Path) { - if let Some(_prev_from) = self.files.insert(to.to_path_buf(), from) { - // Should we log something here? - } - } - - /// 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.add_file(FileSource::SignedFile(from.to_path_buf()), to); - } - - /// Add source and destination of an arbitrary file. - fn add_unsigned(&mut self, from: &Path, to: &Path) { - self.add_file(FileSource::UnsignedFile(from.to_path_buf()), to); - } - - /// Install all files to the ESP. - fn install(&self, key_pair: &KeyPair) -> Result<()> { - for (to, from) in &self.files { - match from { - FileSource::SignedFile(from) => { - install_signed(key_pair, from, to).with_context(|| { - format!("Failed to sign and install from {from:?} to {to:?}") - })? - } - FileSource::UnsignedFile(from) => 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. /// /// If the file already exists at the destination, it is overwritten. diff --git a/rust/tool/systemd/tests/common/mod.rs b/rust/tool/systemd/tests/common/mod.rs index ac17003..65beb2e 100644 --- a/rust/tool/systemd/tests/common/mod.rs +++ b/rust/tool/systemd/tests/common/mod.rs @@ -58,8 +58,9 @@ pub fn setup_generation_link_from_toplevel( let bootspec = json!({ "org.nixos.bootspec.v1": { "init": format!("init-v{}", version), - "initrd": toplevel.join("initrd"), - "kernel": toplevel.join("kernel"), + // Normally, these are in the Nix store. + "initrd": toplevel.join("eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-6.1.1/initrd"), + "kernel": toplevel.join("eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-6.1.1/kernel"), "kernelParams": [ "amd_iommu=on", "amd_iommu=pt", @@ -96,10 +97,12 @@ pub fn setup_generation_link_from_toplevel( /// it (and when it goes out of scope). pub fn setup_toplevel(tmpdir: &Path) -> Result { let system = Architecture::from_nixos_system(SYSTEM)?; + // Generate a random toplevel name so that multiple toplevel paths can live alongside each // other in the same directory. let toplevel = tmpdir.join(format!("toplevel-{}", random_string(8))); - fs::create_dir(&toplevel)?; + let fake_store_path = toplevel.join("eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-6.1.1"); + fs::create_dir_all(&fake_store_path)?; let test_systemd = systemd_location_from_env()?; let systemd_stub_filename = system.systemd_stub_filename(); @@ -108,8 +111,8 @@ pub fn setup_toplevel(tmpdir: &Path) -> Result { systemd_stub_filename = systemd_stub_filename.display() ); - let initrd_path = toplevel.join("initrd"); - let kernel_path = toplevel.join("kernel"); + let initrd_path = fake_store_path.join("initrd"); + let kernel_path = fake_store_path.join("kernel"); let nixos_version_path = toplevel.join("nixos-version"); let kernel_modules_path = toplevel.join("kernel-modules/lib/modules/6.1.1"); diff --git a/rust/tool/systemd/tests/gc.rs b/rust/tool/systemd/tests/gc.rs index e969cd6..d988ade 100644 --- a/rust/tool/systemd/tests/gc.rs +++ b/rust/tool/systemd/tests/gc.rs @@ -29,17 +29,58 @@ fn keep_only_configured_number_of_generations() -> Result<()> { assert_eq!(stub_count(), 3, "Wrong number of stubs after installation"); assert_eq!( kernel_and_initrd_count(), - 6, + 2, "Wrong number of kernels & initrds after installation" ); // Call `lanzatool install` again with a config limit of 2 and assert that one is deleted. + // In addition, the garbage kernel should be deleted as well. let output1 = common::lanzaboote_install(2, esp_mountpoint.path(), generation_links)?; assert!(output1.status.success()); assert_eq!(stub_count(), 2, "Wrong number of stubs after gc."); assert_eq!( kernel_and_initrd_count(), - 4, + 2, + "Wrong number of kernels & initrds after gc." + ); + + Ok(()) +} + +#[test] +fn delete_garbage_kernel() -> Result<()> { + let esp_mountpoint = tempdir()?; + let tmpdir = tempdir()?; + let profiles = tempdir()?; + let generation_links: Vec = [1, 2, 3] + .into_iter() + .map(|v| { + common::setup_generation_link(tmpdir.path(), profiles.path(), v) + .expect("Failed to setup generation link") + }) + .collect(); + let stub_count = || count_files(&esp_mountpoint.path().join("EFI/Linux")).unwrap(); + let kernel_and_initrd_count = || count_files(&esp_mountpoint.path().join("EFI/nixos")).unwrap(); + + // Install all 3 generations. + let output0 = common::lanzaboote_install(0, esp_mountpoint.path(), generation_links.clone())?; + assert!(output0.status.success()); + + // Create a garbage kernel, which should be deleted. + fs::write( + esp_mountpoint.path().join("EFI/nixos/kernel-garbage.efi"), + "garbage", + )?; + + // Call `lanzatool install` again with a config limit of 2. + // In addition, the garbage kernel should be deleted as well. + let output1 = common::lanzaboote_install(2, esp_mountpoint.path(), generation_links)?; + assert!(output1.status.success()); + + assert_eq!(stub_count(), 2, "Wrong number of stubs after gc."); + assert_eq!( + kernel_and_initrd_count(), + 2, "Wrong number of kernels & initrds after gc." ); diff --git a/rust/tool/systemd/tests/install.rs b/rust/tool/systemd/tests/install.rs index ccb4c08..f9f6c03 100644 --- a/rust/tool/systemd/tests/install.rs +++ b/rust/tool/systemd/tests/install.rs @@ -1,7 +1,7 @@ -use std::fs; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; -use anyhow::{Context, Result}; +use anyhow::Result; +use base32ct::{Base32Unpadded, Encoding}; use tempfile::{tempdir, TempDir}; mod common; @@ -67,7 +67,7 @@ fn overwrite_unsigned_images() -> Result<()> { } #[test] -fn overwrite_unsigned_files() -> Result<()> { +fn content_addressing_works() -> Result<()> { let esp = tempdir()?; let tmpdir = tempdir()?; let profiles = tempdir()?; @@ -76,24 +76,21 @@ fn overwrite_unsigned_files() -> Result<()> { let generation_link = setup_generation_link_from_toplevel(&toplevel, profiles.path(), 1)?; let generation_links = vec![generation_link]; - let kernel_hash_source = hash_file(&toplevel.join("kernel")); - - let nixos_dir = esp.path().join("EFI/nixos"); - let kernel_path = nixos_dir.join(nixos_path(toplevel.join("kernel"), "bzImage")?); - - fs::create_dir_all(&nixos_dir)?; - fs::write(&kernel_path, b"Existing kernel")?; - let kernel_hash_existing = hash_file(&kernel_path); + let kernel_hash_source = + hash_file(&toplevel.join("eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-6.1.1/kernel")); let output0 = common::lanzaboote_install(1, esp.path(), generation_links)?; assert!(output0.status.success()); - let kernel_hash_overwritten = hash_file(&kernel_path); + let kernel_path = esp.path().join(format!( + "EFI/nixos/kernel-6.1.1-{}.efi", + Base32Unpadded::encode_string(&kernel_hash_source) + )); - // Assert existing kernel was overwritten. - assert_ne!(kernel_hash_existing, kernel_hash_overwritten); - // Assert overwritten kernel is the source kernel. - assert_eq!(kernel_hash_source, kernel_hash_overwritten); + // Implicitly assert that the content-addressed file actually exists. + let kernel_hash = hash_file(&kernel_path); + // Assert the written kernel is the source kernel. + assert_eq!(kernel_hash_source, kernel_hash); Ok(()) } @@ -102,20 +99,3 @@ fn image_path(esp: &TempDir, version: u64) -> PathBuf { esp.path() .join(format!("EFI/Linux/nixos-generation-{version}.efi")) } - -fn nixos_path(path: impl AsRef, name: &str) -> Result { - let resolved = path - .as_ref() - .read_link() - .unwrap_or_else(|_| path.as_ref().into()); - - let parent_final_component = resolved - .parent() - .and_then(|x| x.file_name()) - .and_then(|x| x.to_str()) - .with_context(|| format!("Failed to extract final component from: {:?}", resolved))?; - - let nixos_filename = format!("{}-{}.efi", parent_final_component, name); - - Ok(PathBuf::from(nixos_filename)) -} From ca070a9eec85125d49521e6ebff36e008026f262 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Sun, 13 Aug 2023 12:02:35 +0200 Subject: [PATCH 2/5] tool: make stubs input-addressed The stubs on the ESP are now input-addressed, where the inputs are the system toplevel and the public key used for signature. This way, it is guaranteed that any stub at a given path will boot the desired system, even in the presence of one of the two edge-cases where it was not previously guaranteed: * The latest generation was deleted at one point, and its generation number was reused by a different system configuration. This is detected because the toplevel will change. * The secure boot signing key was rotated, so old stubs would not boot at all any more. This is detected because the public key will change. Avoiding these two cases will allow to skip reinstallation of stubs that are already in place at the correct path. --- nix/tests/lanzaboote.nix | 8 ++--- rust/tool/shared/src/generation.rs | 9 +++++- rust/tool/systemd/Cargo.toml | 4 +-- rust/tool/systemd/src/install.rs | 23 +++++++++++--- rust/tool/systemd/tests/common/mod.rs | 24 +++++++++++++- rust/tool/systemd/tests/install.rs | 46 ++++++++++++++++++++------- rust/tool/systemd/tests/os_release.rs | 12 +++---- 7 files changed, 95 insertions(+), 31 deletions(-) diff --git a/nix/tests/lanzaboote.nix b/nix/tests/lanzaboote.nix index 55e6ae7..d9e6458 100644 --- a/nix/tests/lanzaboote.nix +++ b/nix/tests/lanzaboote.nix @@ -258,7 +258,7 @@ in # It is expected that the initrd contains the original secret. machine.succeed("cmp ${originalSecret} /secret-from-initramfs") - machine.succeed("bootctl set-default nixos-generation-1-specialisation-variant.efi") + machine.succeed("bootctl set-default nixos-generation-1-specialisation-variant-\*.efi") machine.succeed("sync") machine.crash() machine.start() @@ -301,7 +301,7 @@ in machine.start() print(machine.succeed("ls -lah /boot/EFI/Linux")) # TODO: make it more reliable to find this filename, i.e. read it from somewhere? - machine.succeed("bootctl set-default nixos-generation-1-specialisation-variant.efi") + machine.succeed("bootctl set-default nixos-generation-1-specialisation-variant-\*.efi") machine.succeed("sync") machine.fail("efibootmgr") machine.crash() @@ -359,8 +359,8 @@ in # TODO: this should work -- machine.succeed("efibootmgr -d /dev/vda -c -l \\EFI\\Linux\\nixos-generation-1.efi") -- efivars are not persisted # across reboots atm? # cheat code no 1 - machine.succeed("cp /boot/EFI/Linux/nixos-generation-1.efi /boot/EFI/BOOT/BOOTX64.EFI") - machine.succeed("cp /boot/EFI/Linux/nixos-generation-1.efi /boot/EFI/systemd/systemd-bootx64.efi") + machine.succeed("cp /boot/EFI/Linux/nixos-generation-1-*.efi /boot/EFI/BOOT/BOOTX64.EFI") + machine.succeed("cp /boot/EFI/Linux/nixos-generation-1-*.efi /boot/EFI/systemd/systemd-bootx64.efi") # Let's reboot. machine.succeed("sync") diff --git a/rust/tool/shared/src/generation.rs b/rust/tool/shared/src/generation.rs index 2686dbb..3edf9cc 100644 --- a/rust/tool/shared/src/generation.rs +++ b/rust/tool/shared/src/generation.rs @@ -84,7 +84,14 @@ impl Generation { .build_time .map(|x| x.to_string()) .unwrap_or_else(|| String::from("Unknown")); - format!("Generation {}, Built on {}", self.version, build_time) + if self.is_specialised().is_some() { + format!( + "Generation {}-specialised, Built on {}", + self.version, build_time + ) + } else { + format!("Generation {}, Built on {}", self.version, build_time) + } } } diff --git a/rust/tool/systemd/Cargo.toml b/rust/tool/systemd/Cargo.toml index 0a0f240..47f2898 100644 --- a/rust/tool/systemd/Cargo.toml +++ b/rust/tool/systemd/Cargo.toml @@ -13,6 +13,8 @@ log = { version = "0.4.18", features = ["std"] } clap = { version = "4.3.1", features = ["derive"] } lanzaboote_tool = { path = "../shared" } indoc = "2.0.1" +serde_json = "1.0.96" +sha2 = "0.10.6" tempfile = "3.5.0" nix = { version = "0.26.2", default-features = false, features = [ "fs" ] } @@ -21,8 +23,6 @@ assert_cmd = "2.0.11" expect-test = "1.4.1" filetime = "0.2.21" rand = "0.8.5" -serde_json = "1.0.96" goblin = "0.6.1" serde = { version = "1.0.163", features = ["derive"] } walkdir = "2.3.3" -sha2 = "0.10.6" diff --git a/rust/tool/systemd/src/install.rs b/rust/tool/systemd/src/install.rs index 926ca9e..dc12cf8 100644 --- a/rust/tool/systemd/src/install.rs +++ b/rust/tool/systemd/src/install.rs @@ -2,7 +2,7 @@ use std::collections::BTreeSet; use std::ffi::OsStr; use std::fs::{self, File}; use std::os::fd::AsRawFd; -use std::os::unix::prelude::PermissionsExt; +use std::os::unix::prelude::{OsStrExt, PermissionsExt}; use std::path::{Path, PathBuf}; use std::process::Command; use std::string::ToString; @@ -10,6 +10,7 @@ use std::string::ToString; use anyhow::{anyhow, Context, Result}; use base32ct::{Base32Unpadded, Encoding}; use nix::unistd::syncfs; +use sha2::{Digest, Sha256}; use tempfile::TempDir; use crate::architecture::SystemdArchitectureExt; @@ -243,13 +244,27 @@ impl Installer { &self.esp_paths.esp, ) .context("Failed to assemble lanzaboote image.")?; + let stub_inputs = [ + // Generation numbers can be reused if the latest generation was deleted. + // To detect this, the stub path depends on the actual toplevel used. + ("toplevel", bootspec.toplevel.0.as_os_str().as_bytes()), + // If the key is rotated, the signed stubs must be re-generated. + // So we make their path depend on the public key used for signature. + ("public_key", &fs::read(&self.key_pair.public_key)?), + ]; + let stub_input_hash = Base32Unpadded::encode_string(&Sha256::digest( + serde_json::to_string(&stub_inputs).unwrap(), + )); let stub_name = if let Some(specialisation_name) = generation.is_specialised() { PathBuf::from(format!( - "nixos-generation-{}-specialisation-{}.efi", - generation, specialisation_name + "nixos-generation-{}-specialisation-{}-{}.efi", + generation, specialisation_name, stub_input_hash )) } else { - PathBuf::from(format!("nixos-generation-{}.efi", generation)) + PathBuf::from(format!( + "nixos-generation-{}-{}.efi", + generation, stub_input_hash + )) }; let stub_target = self.esp_paths.linux.join(stub_name); self.gc_roots.extend([&stub_target]); diff --git a/rust/tool/systemd/tests/common/mod.rs b/rust/tool/systemd/tests/common/mod.rs index 65beb2e..5fb09a0 100644 --- a/rust/tool/systemd/tests/common/mod.rs +++ b/rust/tool/systemd/tests/common/mod.rs @@ -6,16 +6,18 @@ use std::ffi::OsStr; use std::fs; use std::io::Write; -use std::os::unix::prelude::MetadataExt; +use std::os::unix::prelude::{MetadataExt, OsStrExt}; use std::path::{Path, PathBuf}; use std::process::Output; use anyhow::{Context, Result}; use assert_cmd::Command; +use base32ct::{Base32Unpadded, Encoding}; use rand::distributions::Alphanumeric; use rand::{thread_rng, Rng}; use serde_json::json; use sha2::{Digest, Sha256}; +use tempfile::TempDir; use lanzaboote_tool::architecture::Architecture; use lzbt_systemd::architecture::SystemdArchitectureExt; @@ -237,3 +239,23 @@ pub fn verify_signature(path: &Path) -> Result { pub fn count_files(path: &Path) -> Result { Ok(fs::read_dir(path)?.count()) } + +pub fn image_path(esp: &TempDir, version: u64, toplevel: &Path) -> Result { + let stub_inputs = [ + // Generation numbers can be reused if the latest generation was deleted. + // To detect this, the stub path depends on the actual toplevel used. + ("toplevel", toplevel.as_os_str().as_bytes()), + // If the key is rotated, the signed stubs must be re-generated. + // So we make their path depend on the public key used for signature. + ( + "public_key", + &std::fs::read("tests/fixtures/uefi-keys/db.pem")?, + ), + ]; + let stub_input_hash = Base32Unpadded::encode_string(&Sha256::digest( + serde_json::to_string(&stub_inputs).unwrap(), + )); + Ok(esp.path().join(format!( + "EFI/Linux/nixos-generation-{version}-{stub_input_hash}.efi" + ))) +} diff --git a/rust/tool/systemd/tests/install.rs b/rust/tool/systemd/tests/install.rs index f9f6c03..5348530 100644 --- a/rust/tool/systemd/tests/install.rs +++ b/rust/tool/systemd/tests/install.rs @@ -1,8 +1,6 @@ -use std::path::PathBuf; - use anyhow::Result; use base32ct::{Base32Unpadded, Encoding}; -use tempfile::{tempdir, TempDir}; +use tempfile::tempdir; mod common; @@ -42,12 +40,13 @@ fn overwrite_unsigned_images() -> Result<()> { let esp = tempdir()?; let tmpdir = tempdir()?; let profiles = tempdir()?; + let toplevel = common::setup_toplevel(tmpdir.path())?; - let image1 = image_path(&esp, 1); - let image2 = image_path(&esp, 2); + let image1 = common::image_path(&esp, 1, &toplevel)?; + let image2 = common::image_path(&esp, 2, &toplevel)?; - let generation_link1 = common::setup_generation_link(tmpdir.path(), profiles.path(), 1)?; - let generation_link2 = common::setup_generation_link(tmpdir.path(), profiles.path(), 2)?; + let generation_link1 = setup_generation_link_from_toplevel(&toplevel, profiles.path(), 1)?; + let generation_link2 = setup_generation_link_from_toplevel(&toplevel, profiles.path(), 2)?; let generation_links = vec![generation_link1, generation_link2]; let output1 = common::lanzaboote_install(0, esp.path(), generation_links.clone())?; @@ -66,6 +65,34 @@ fn overwrite_unsigned_images() -> Result<()> { Ok(()) } +#[test] +fn detect_generation_number_reuse() -> Result<()> { + let esp = tempdir()?; + let tmpdir = tempdir()?; + let profiles = tempdir()?; + let toplevel1 = common::setup_toplevel(tmpdir.path())?; + let toplevel2 = common::setup_toplevel(tmpdir.path())?; + + let image1 = common::image_path(&esp, 1, &toplevel1)?; + // this deliberately gets the same number! + let image2 = common::image_path(&esp, 1, &toplevel2)?; + + let generation_link1 = setup_generation_link_from_toplevel(&toplevel1, profiles.path(), 1)?; + let output1 = common::lanzaboote_install(0, esp.path(), vec![generation_link1])?; + assert!(output1.status.success()); + assert!(image1.exists()); + assert!(!image2.exists()); + + std::fs::remove_dir_all(profiles.path().join("system-1-link"))?; + let generation_link2 = setup_generation_link_from_toplevel(&toplevel2, profiles.path(), 1)?; + let output2 = common::lanzaboote_install(0, esp.path(), vec![generation_link2])?; + assert!(output2.status.success()); + assert!(!image1.exists()); + assert!(image2.exists()); + + Ok(()) +} + #[test] fn content_addressing_works() -> Result<()> { let esp = tempdir()?; @@ -94,8 +121,3 @@ fn content_addressing_works() -> Result<()> { Ok(()) } - -fn image_path(esp: &TempDir, version: u64) -> PathBuf { - esp.path() - .join(format!("EFI/Linux/nixos-generation-{version}.efi")) -} diff --git a/rust/tool/systemd/tests/os_release.rs b/rust/tool/systemd/tests/os_release.rs index 5203723..eb1f627 100644 --- a/rust/tool/systemd/tests/os_release.rs +++ b/rust/tool/systemd/tests/os_release.rs @@ -11,18 +11,16 @@ fn generate_expected_os_release() -> Result<()> { let esp_mountpoint = tempdir()?; let tmpdir = tempdir()?; let profiles = tempdir()?; + let toplevel = common::setup_toplevel(tmpdir.path())?; - let generation_link = common::setup_generation_link(tmpdir.path(), profiles.path(), 1) - .expect("Failed to setup generation link"); + let generation_link = + common::setup_generation_link_from_toplevel(&toplevel, profiles.path(), 1) + .expect("Failed to setup generation link"); let output0 = common::lanzaboote_install(0, esp_mountpoint.path(), vec![generation_link])?; assert!(output0.status.success()); - let stub_data = fs::read( - esp_mountpoint - .path() - .join("EFI/Linux/nixos-generation-1.efi"), - )?; + let stub_data = fs::read(common::image_path(&esp_mountpoint, 1, &toplevel)?)?; let os_release_section = pe_section(&stub_data, ".osrel") .context("Failed to read .osrelease PE section.")? .to_owned(); From 4fd37670e28c990bc56abea2bfe45114fbdf7fc6 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Sun, 13 Aug 2023 18:29:40 +0200 Subject: [PATCH 3/5] tool: stop most overwriting in the ESP Since most files (stubs, kernels and initrds) on the ESP are properly input-addressed or content-addressed now, there is no point in overwriting them any more. Hence we detect what generations are already properly installed, and don't reinstall them any more. This approach leads to two distinct improvements: * Rollbacks are more reliable, because initrd secrets and stubs do not change any more for existing generations (with the necessary exception of stubs in case of signature key rotation). In particular, the risk of a newer stub breaking (for example, because of bad interactions with certain firmware) old and previously working generations is avoided. * Kernels and initrds that are not going to be (re)installed anyway are not read and hashed any more. This significantly reduces the I/O and CPU time required for the installation process, particularly when there is a large number of generations. The following drawbacks are noted: * The first time installation is performed after these changes, most of the ESP is re-written at a different path; as a result, the disk usage increases to roughly the double until the GC is performed. * If multiple generations share a bare initrd, but have different secrets scripts, the final initrds will now be separated, leading to increased disk usage. However, this situation should be rare, and the previous behavior was arguably incorrect anyway. * If the files on the ESP are corrupted, running the installation again will not overwrite them with the correct versions. Since the files are written atomically, this situation should not happen except in case of file system corruption, and it is questionable whether overwriting really fixes the problem in this case. --- rust/tool/systemd/src/install.rs | 93 ++++++++++++++++++++++-------- rust/tool/systemd/tests/install.rs | 4 +- 2 files changed, 72 insertions(+), 25 deletions(-) diff --git a/rust/tool/systemd/src/install.rs b/rust/tool/systemd/src/install.rs index dc12cf8..b04ad1b 100644 --- a/rust/tool/systemd/src/install.rs +++ b/rust/tool/systemd/src/install.rs @@ -184,6 +184,11 @@ impl Installer { /// Hence, this function cannot overwrite files of other generations with different contents. /// All installed files are added as garbage collector roots. fn install_generation(&mut self, generation: &Generation) -> Result<()> { + // If the generation is already properly installed, don't overwrite it. + if self.register_installed_generation(generation).is_ok() { + return Ok(()); + } + let tempdir = TempDir::new().context("Failed to create temporary directory.")?; let bootspec = &generation.spec.bootspec.bootspec; @@ -244,29 +249,10 @@ impl Installer { &self.esp_paths.esp, ) .context("Failed to assemble lanzaboote image.")?; - let stub_inputs = [ - // Generation numbers can be reused if the latest generation was deleted. - // To detect this, the stub path depends on the actual toplevel used. - ("toplevel", bootspec.toplevel.0.as_os_str().as_bytes()), - // If the key is rotated, the signed stubs must be re-generated. - // So we make their path depend on the public key used for signature. - ("public_key", &fs::read(&self.key_pair.public_key)?), - ]; - let stub_input_hash = Base32Unpadded::encode_string(&Sha256::digest( - serde_json::to_string(&stub_inputs).unwrap(), - )); - let stub_name = if let Some(specialisation_name) = generation.is_specialised() { - PathBuf::from(format!( - "nixos-generation-{}-specialisation-{}-{}.efi", - generation, specialisation_name, stub_input_hash - )) - } else { - PathBuf::from(format!( - "nixos-generation-{}-{}.efi", - generation, stub_input_hash - )) - }; - let stub_target = self.esp_paths.linux.join(stub_name); + let stub_target = self + .esp_paths + .linux + .join(stub_name(generation, &self.key_pair.public_key)?); self.gc_roots.extend([&stub_target]); install_signed(&self.key_pair, &lanzaboote_image, &stub_target) .context("Failed to install the Lanzaboote stub.")?; @@ -274,6 +260,33 @@ impl Installer { Ok(()) } + /// Register the files of an already installed generation as garbage collection roots. + /// + /// An error should not be considered fatal; the generation should be (re-)installed instead. + fn register_installed_generation(&mut self, generation: &Generation) -> Result<()> { + let stub_target = self + .esp_paths + .linux + .join(stub_name(generation, &self.key_pair.public_key)?); + let stub = fs::read(&stub_target)?; + let kernel_path = resolve_efi_path( + &self.esp_paths.esp, + pe::read_section_data(&stub, ".kernelp").context("Missing kernel path.")?, + )?; + let initrd_path = resolve_efi_path( + &self.esp_paths.esp, + pe::read_section_data(&stub, ".initrdp").context("Missing initrd path.")?, + )?; + + if !kernel_path.exists() && !initrd_path.exists() { + anyhow::bail!("Missing kernel or initrd."); + } + self.gc_roots + .extend([&stub_target, &kernel_path, &initrd_path]); + + Ok(()) + } + /// Install a content-addressed file to the `EFI/nixos` directory on the ESP. /// /// It is automatically added to the garbage collector roots. @@ -340,6 +353,40 @@ impl Installer { } } +/// Translate an EFI path to an absolute path on the mounted ESP. +fn resolve_efi_path(esp: &Path, efi_path: &[u8]) -> Result { + Ok(esp.join(std::str::from_utf8(&efi_path[1..])?.replace('\\', "/"))) +} + +/// Compute the file name to be used for the stub of a certain generation, signed with the given key. +/// +/// The generated name is input-addressed by the toplevel corresponding to the generation and the public part of the signing key. +fn stub_name(generation: &Generation, public_key: &Path) -> Result { + let bootspec = &generation.spec.bootspec.bootspec; + let stub_inputs = [ + // Generation numbers can be reused if the latest generation was deleted. + // To detect this, the stub path depends on the actual toplevel used. + ("toplevel", bootspec.toplevel.0.as_os_str().as_bytes()), + // If the key is rotated, the signed stubs must be re-generated. + // So we make their path depend on the public key used for signature. + ("public_key", &fs::read(public_key)?), + ]; + let stub_input_hash = Base32Unpadded::encode_string(&Sha256::digest( + serde_json::to_string(&stub_inputs).unwrap(), + )); + if let Some(specialisation_name) = generation.is_specialised() { + Ok(PathBuf::from(format!( + "nixos-generation-{}-specialisation-{}-{}.efi", + generation, specialisation_name, stub_input_hash + ))) + } else { + Ok(PathBuf::from(format!( + "nixos-generation-{}-{}.efi", + generation, stub_input_hash + ))) + } +} + /// Install a PE file. The PE gets signed in the process. /// /// If the file already exists at the destination, it is overwritten. diff --git a/rust/tool/systemd/tests/install.rs b/rust/tool/systemd/tests/install.rs index 5348530..9093114 100644 --- a/rust/tool/systemd/tests/install.rs +++ b/rust/tool/systemd/tests/install.rs @@ -36,7 +36,7 @@ fn do_not_install_duplicates() -> Result<()> { } #[test] -fn overwrite_unsigned_images() -> Result<()> { +fn do_not_overwrite_images() -> Result<()> { let esp = tempdir()?; let tmpdir = tempdir()?; let profiles = tempdir()?; @@ -59,7 +59,7 @@ fn overwrite_unsigned_images() -> Result<()> { let output2 = common::lanzaboote_install(0, esp.path(), generation_links)?; assert!(output2.status.success()); - assert!(verify_signature(&image1)?); + assert!(!verify_signature(&image1)?); assert!(verify_signature(&image2)?); Ok(()) From 90a1adac54c083026d81eddc9e74b1d64754e5af Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Sun, 24 Sep 2023 11:27:59 +0200 Subject: [PATCH 4/5] tool: fix atomic write Atomic write works by first writing a temporary file, then syncing that temporary file to ensure it is fully on disk before the program can continue, and in the last step renaming the temporary file to the target. The middle step was missing, which is likely to lead to a truncated target file being present after power loss. Add this step. Furthermore, even with this fix, atomicity is not fully guaranteed, because FAT32 can become corrupted after power loss due to its design shortcomings. Even though we cannot really do anything about this case, adjust the comment to at least acknowledge the situation. --- rust/tool/systemd/src/install.rs | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/rust/tool/systemd/src/install.rs b/rust/tool/systemd/src/install.rs index b04ad1b..2db01a7 100644 --- a/rust/tool/systemd/src/install.rs +++ b/rust/tool/systemd/src/install.rs @@ -465,17 +465,29 @@ fn assemble_kernel_cmdline(init: &Path, kernel_params: Vec) -> Vec Result<()> { - let to_tmp = to.with_extension(".tmp"); - - fs::copy(from, &to_tmp) - .with_context(|| format!("Failed to copy from {from:?} to {to_tmp:?}",))?; - - fs::rename(&to_tmp, to).with_context(|| { - format!("Failed to move temporary file {to_tmp:?} to final location {to:?}") - }) + let tmp = to.with_extension(".tmp"); + { + let mut from_file = + File::open(from).with_context(|| format!("Failed to read the source file {from:?}"))?; + let mut tmp_file = File::create(&tmp) + .with_context(|| format!("Failed to create the temporary file {tmp:?}"))?; + std::io::copy(&mut from_file, &mut tmp_file).with_context(|| { + format!("Failed to copy from {from:?} to the temporary file {tmp:?}") + })?; + tmp_file + .sync_all() + .with_context(|| format!("Failed to sync the temporary file {tmp:?}"))?; + } + fs::rename(&tmp, to) + .with_context(|| format!("Failed to move temporary file {tmp:?} to target {to:?}")) } /// Set the octal permission bits of the specified file. From a78b9682b77cf0fb2d301f82d55ca5d887cbcafb Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Sun, 24 Sep 2023 13:30:36 +0200 Subject: [PATCH 5/5] docs: Improve troubleshooting documentation Due to the temporarily doubled ESP space usage, it is now easier to run into the out of space issue (once). Document how to proceed in this case without having to delete any generations. Furthermore, recovery in case of ESP corruption is now slightly more involved, because not all files are rewritten all the time. Adjust the documentation accordingly. --- docs/TROUBLESHOOTING.md | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 0e136fc..d9ab13c 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -14,6 +14,10 @@ It is recommended run a garbage collection regularly, and monitor the ESP usage **Warning:** It is recommended to not delete the currently booted kernel and initrd, and to not reboot the system before running `nixos-rebuild boot` again, to minimize the risk of accidentally rendering the system unbootable. +**Note:** When upgrading Lanzaboote from version 0.3.0, or from git master prior to the merge of PR #204, ESP space usage is temporarily doubled. +Hence it is possible for this error to occur even if there was plenty (but less than half) free space available prior to the installation. +In this case, it is not necessary to delete any generations, and you can proceed directly to deleting some kernels and initrds before running the installation again. + ## Power failed during bootloader installation, and now the system does not boot any more Due to the shortcomings of the FAT32 filesystem, in rare cases, it is possible for the ESP to become corrupted after power loss. @@ -26,10 +30,15 @@ In this case, the steps below will not help, and standard rollback procedures sh ### The system can still boot an older generation In case an older generation still works, the recovery can be carried out from within the booted system. +Run `nix-shell -p openssl sbctl` to ensure the tools required for recovery are available. -1. Run `nixos-rebuild boot`. - This should reinstall all generations and thus overwrite the corrupted files. -2. Reboot the system, it should now work again. +1. Run `sudo sbctl verify /boot/EFI/Linux/nixos-generation-*.efi` to check the Lanzaboote stubs. + Files that have a crossmark on their left are corrupted and must be deleted. +2. Run `for file in /boot/EFI/nixos/*.efi; do hash=$(openssl dgst -sha256 -binary "$file" | base32 | tr -d = | LC_ALL=C tr [:upper:] [:lower:]); if [[ $file != *$hash.efi ]]; then echo $file; fi; done` to check the kernels and initrds. + Any files that are printed are corrupted and must be deleted. +3. Run `nixos-rebuild boot`. + This should reinstate all files that are required for the newer generations to boot. +4. Reboot the system, it should now work again. ### The system cannot boot any generation anymore @@ -45,11 +54,12 @@ A more recent medium must be used for the recovery procedure to work reliably. 3. Mount all partitions belonging to the system to be recovered under `/mnt`, just like you would for installation. 1. In case the ESP does not mount, or only mounts in read-only mode, due to corruption, try `fsck.fat` first. If that fails as well or the ESP still does not mount, it needs to be reformatted using `mkfs.fat`. -4. Enter the recovery shell by running `nixos-enter`. +4. Delete the corrupted files on the ESP, using `rm -fr /mnt/boot/EFI/Linux/nixos-generation-*.efi /mnt/boot/efi/nixos`. +5. Enter the recovery shell by running `nixos-enter`. Then, run `nixos-rebuild boot` to install the bootloader again. -5. Exit the recovery shell and unmount all filesystems. -6. Reboot the system to verify that everything works again. -7. Enable Secure Boot again in the firmware settings. +6. Exit the recovery shell and unmount all filesystems. +7. Reboot the system to verify that everything works again. +8. Enable Secure Boot again in the firmware settings. ## The system doesn't boot with Secure Boot enabled