From ca070a9eec85125d49521e6ebff36e008026f262 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Sun, 13 Aug 2023 12:02:35 +0200 Subject: [PATCH] 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();