diff --git a/nix/modules/lanzaboote.nix b/nix/modules/lanzaboote.nix index 747c948..ad2942e 100644 --- a/nix/modules/lanzaboote.nix +++ b/nix/modules/lanzaboote.nix @@ -59,6 +59,7 @@ in ''} ${cfg.package}/bin/lzbt install \ + --systemd ${pkgs.systemd} \ --public-key ${cfg.publicKeyFile} \ --private-key ${cfg.privateKeyFile} \ --configuration-limit ${toString configurationLimit} \ diff --git a/rust/tool/src/cli.rs b/rust/tool/src/cli.rs index 8f324cb..e203805 100644 --- a/rust/tool/src/cli.rs +++ b/rust/tool/src/cli.rs @@ -19,6 +19,10 @@ enum Commands { #[derive(Parser)] struct InstallCommand { + /// Systemd path + #[arg(long)] + systemd: PathBuf, + /// sbsign Public Key #[arg(long)] public_key: PathBuf, @@ -60,6 +64,7 @@ fn install(args: InstallCommand) -> Result<()> { install::Installer::new( PathBuf::from(lanzaboote_stub), + args.systemd, key_pair, args.configuration_limit, args.esp, diff --git a/rust/tool/src/install.rs b/rust/tool/src/install.rs index db227c7..2903b9d 100644 --- a/rust/tool/src/install.rs +++ b/rust/tool/src/install.rs @@ -1,7 +1,9 @@ +use std::ffi::CStr; use std::fs; use std::os::unix::prelude::PermissionsExt; use std::path::{Path, PathBuf}; use std::process::Command; +use std::str::FromStr; use anyhow::{Context, Result}; use nix::unistd::sync; @@ -17,6 +19,7 @@ use crate::utils::SecureTempDirExt; pub struct Installer { gc_roots: Roots, lanzaboote_stub: PathBuf, + systemd: PathBuf, key_pair: KeyPair, configuration_limit: usize, esp_paths: EspPaths, @@ -26,6 +29,7 @@ pub struct Installer { impl Installer { pub fn new( lanzaboote_stub: PathBuf, + systemd: PathBuf, key_pair: KeyPair, configuration_limit: usize, esp: PathBuf, @@ -38,6 +42,7 @@ impl Installer { Self { gc_roots, lanzaboote_stub, + systemd, key_pair, configuration_limit, esp_paths, @@ -66,6 +71,8 @@ impl Installer { }; self.install_links(links)?; + self.install_systemd_boot()?; + // Only collect garbage in these two directories. This way, no files that do not belong to // the NixOS installation are deleted. Lanzatool takes full control over the esp/EFI/nixos // directory and deletes ALL files that it doesn't know about. Dual- or multiboot setups @@ -148,24 +155,17 @@ impl Installer { append_initrd_secrets(initrd_secrets_script, &initrd_location)?; } - let systemd_boot = bootspec - .toplevel - .0 - .join("systemd/lib/systemd/boot/efi/systemd-bootx64.efi"); - - [ - (&systemd_boot, &self.esp_paths.efi_fallback), - (&systemd_boot, &self.esp_paths.systemd_boot), - (&bootspec.kernel, &esp_gen_paths.kernel), - ] - .into_iter() - .try_for_each(|(from, to)| install_signed(&self.key_pair, from, to))?; - - // The initrd doesn't need to be signed. Lanzaboote has its - // hash embedded and will refuse loading it when the hash - // mismatches. + // The initrd doesn't need to be signed. The stub has its hash embedded and will refuse + // loading it when the hash mismatches. + // + // The initrd and kernel are not forcibly installed because they are not built + // reproducibly. Forcibly installing (i.e. overwriting) them is likely to break older + // generations that point to the same initrd/kernel because the hash embedded in the stub + // will not match anymore. install(&initrd_location, &esp_gen_paths.initrd) .context("Failed to install initrd to ESP")?; + install_signed(&self.key_pair, &bootspec.kernel, &esp_gen_paths.kernel) + .context("Failed to install kernel to ESP.")?; let lanzaboote_image = pe::lanzaboote_image( &tempdir, @@ -197,6 +197,33 @@ impl Installer { Ok(()) } + + /// Install systemd-boot to ESP. + /// + /// systemd-boot is only updated when a newer version is available OR when the currently + /// installed version is not signed. This enables switching to Lanzaboote without having to + /// manually delete previous unsigned systemd-boot binaries and minimizes the number of writes + /// to the ESP. + /// + /// Checking for the version also allows us to skip buggy systemd versions in the future. + fn install_systemd_boot(&self) -> Result<()> { + let systemd_boot = self + .systemd + .join("lib/systemd/boot/efi/systemd-bootx64.efi"); + + let paths = [ + (&systemd_boot, &self.esp_paths.efi_fallback), + (&systemd_boot, &self.esp_paths.systemd_boot), + ]; + + for (from, to) in paths { + if newer_systemd_boot(from, to)? || !&self.key_pair.verify(to) { + force_install_signed(&self.key_pair, from, to) + .with_context(|| format!("Failed to install systemd-boot binary to: {to:?}"))?; + } + } + Ok(()) + } } /// Install a PE file. The PE gets signed in the process. @@ -206,16 +233,24 @@ fn install_signed(key_pair: &KeyPair, from: &Path, to: &Path) -> Result<()> { if to.exists() { println!("{} already exists, skipping...", to.display()); } else { - println!("Signing and installing {}...", to.display()); - ensure_parent_dir(to); - key_pair - .sign_and_copy(from, to) - .with_context(|| format!("Failed to copy and sign file from {:?} to {:?}", from, to))?; + force_install_signed(key_pair, from, to)?; } Ok(()) } +/// Sign and forcibly install a PE file. +/// +/// If the file already exists at the destination, it is overwritten. +fn force_install_signed(key_pair: &KeyPair, from: &Path, to: &Path) -> Result<()> { + println!("Signing and installing {}...", to.display()); + ensure_parent_dir(to); + key_pair + .sign_and_copy(from, to) + .with_context(|| format!("Failed to copy and sign file from {from:?} to {to:?}"))?; + Ok(()) +} + /// Install an arbitrary file /// /// The file is only copied if it doesn't exist at the destination @@ -281,3 +316,57 @@ fn ensure_parent_dir(path: &Path) { fs::create_dir_all(parent).ok(); } } + +/// Determine if a newer systemd-boot version is available. +/// +/// "Newer" can mean +/// (1) no file exists at the destination, +/// (2) the file at the destination is malformed, +/// (3) a binary with a higher version is available. +fn newer_systemd_boot(from: &Path, to: &Path) -> Result { + // If the file doesn't exists at the destination, it should be installed. + if !to.exists() { + return Ok(true); + } + + // If the version from the source binary cannot be read, something is irrecoverably wrong. + let from_version = systemd_boot_version(from) + .with_context(|| format!("Failed to read systemd-boot version from {from:?}."))?; + + // If the version cannot be read from the destination binary, it is malformed. It should be + // forcibly reinstalled. + let to_version = match systemd_boot_version(to) { + Ok(version) => version, + _ => return Ok(true), + }; + + Ok(from_version > to_version) +} + +/// Read the version of a systemd-boot binary from its `.osrel` section. +/// +/// The version is parsed into a f32 because systemd does not follow strict semver conventions. A +/// f32, however, should parse all systemd versions and enables usefully comparing them. +/// This is a hack and we should replace it with a better solution once we find one. +fn systemd_boot_version(path: &Path) -> Result { + let file_data = fs::read(path).with_context(|| format!("Failed to read file {path:?}"))?; + let section_data = pe::read_section_data(&file_data, ".osrel") + .with_context(|| format!("PE section '.osrel' is empty: {path:?}"))?; + + // The `.osrel` section in the systemd-boot binary is a NUL terminated string and thus needs + // special handling. + let section_data_cstr = + CStr::from_bytes_with_nul(section_data).context("Failed to parse C string.")?; + let section_data_string = section_data_cstr + .to_str() + .context("Failed to convert C string to Rust string.")?; + + let os_release = OsRelease::from_str(section_data_string) + .with_context(|| format!("Failed to parse os-release from {section_data_string}"))?; + + let version_string = os_release + .0 + .get("VERSION") + .context("Failed to extract VERSION key from: {os_release:#?}")?; + Ok(f32::from_str(version_string)?) +} diff --git a/rust/tool/src/os_release.rs b/rust/tool/src/os_release.rs index c030b7d..bb134a0 100644 --- a/rust/tool/src/os_release.rs +++ b/rust/tool/src/os_release.rs @@ -13,7 +13,7 @@ use crate::generation::Generation; /// The BTreeMap is used over a HashMap, so that the keys are ordered. This is irrelevant for /// systemd-boot (which does not care about order when reading the os-release file) but is useful /// for testing. Ordered keys allow using snapshot tests. -pub struct OsRelease(BTreeMap<&'static str, String>); +pub struct OsRelease(pub BTreeMap); impl OsRelease { pub fn from_generation(generation: &Generation) -> Result { @@ -25,10 +25,10 @@ impl OsRelease { // // Because the ID field here does not have the same meaning as in a real os-release file, // it is fine to use a dummy value. - map.insert("ID", String::from("lanza")); - map.insert("PRETTY_NAME", generation.spec.bootspec.label.clone()); + map.insert("ID".into(), String::from("lanza")); + map.insert("PRETTY_NAME".into(), generation.spec.bootspec.label.clone()); map.insert( - "VERSION_ID", + "VERSION_ID".into(), generation .describe() .context("Failed to describe generation.")?, @@ -36,6 +36,31 @@ impl OsRelease { Ok(Self(map)) } + + /// Parse the string representation of a os-release file. + /// + /// **Beware before reusing this function!** + /// + /// This parser might not parse all valid os-release files correctly. It is only designed to + /// read the `VERSION` key from the os-release of a systemd-boot binary. + pub fn from_str(value: &str) -> Result { + let mut map = BTreeMap::new(); + + let key_value_lines = value.lines().map(|x| x.split('=').collect::>()); + + for kv in key_value_lines { + let k = kv + .first() + .with_context(|| format!("Failed to get first element from {kv:?}"))?; + let v = kv + .get(1) + .map(|s| s.trim_matches('"')) + .with_context(|| format!("Failed to get second element from {kv:?}"))?; + map.insert(String::from(*k), v.into()); + } + + Ok(Self(map)) + } } /// Display OsRelease in the format of an os-release file. @@ -47,3 +72,21 @@ impl fmt::Display for OsRelease { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use std::ffi::CStr; + + #[test] + fn parses_correctly_from_str() -> Result<()> { + let os_release_cstr = CStr::from_bytes_with_nul(b"ID=systemd-boot\nVERSION=\"252.1\"\n\0")?; + let os_release_str = os_release_cstr.to_str()?; + let os_release = OsRelease::from_str(os_release_str)?; + + assert!(os_release.0["ID"] == "systemd-boot"); + assert!(os_release.0["VERSION"] == "252.1"); + + Ok(()) + } +} diff --git a/rust/tool/src/pe.rs b/rust/tool/src/pe.rs index 7f32d01..99f184f 100644 --- a/rust/tool/src/pe.rs +++ b/rust/tool/src/pe.rs @@ -180,6 +180,24 @@ fn file_size(path: impl AsRef) -> Result { .size()) } +/// Read the data from a section of a PE binary. +/// +/// The binary is supplied as a `u8` slice. +pub fn read_section_data<'a>(file_data: &'a [u8], section_name: &str) -> Option<&'a [u8]> { + let pe_binary = goblin::pe::PE::parse(file_data).ok()?; + + pe_binary + .sections + .iter() + .find(|s| s.name().unwrap() == section_name) + .and_then(|s| { + let section_start: usize = s.pointer_to_raw_data.try_into().ok()?; + assert!(s.virtual_size <= s.size_of_raw_data); + let section_end: usize = section_start + usize::try_from(s.virtual_size).ok()?; + Some(&file_data[section_start..section_end]) + }) +} + #[cfg(test)] mod tests { use super::*; diff --git a/rust/tool/src/signature.rs b/rust/tool/src/signature.rs index b3f953a..e6fc41d 100644 --- a/rust/tool/src/signature.rs +++ b/rust/tool/src/signature.rs @@ -43,4 +43,30 @@ impl KeyPair { Ok(()) } + + /// Verify the signature of a PE binary. Return true if the signature was verified. + pub fn verify(&self, path: &Path) -> bool { + let args: Vec = vec![ + OsString::from("--cert"), + self.public_key.clone().into(), + path.as_os_str().to_owned(), + ]; + + let output = match Command::new("sbverify").args(&args).output() { + Ok(output) => output, + Err(_) => return false, + }; + + if !output.status.success() { + if std::io::stderr().write_all(&output.stderr).is_err() { + return false; + }; + println!( + "Failed to verify signature using sbverify with args `{:?}`", + &args + ); + return false; + } + true + } } diff --git a/rust/tool/tests/common/mod.rs b/rust/tool/tests/common/mod.rs index 42ad44a..1872789 100644 --- a/rust/tool/tests/common/mod.rs +++ b/rust/tool/tests/common/mod.rs @@ -1,7 +1,6 @@ use std::ffi::OsStr; use std::fs; use std::io::Write; -use std::os::unix::fs::symlink; use std::path::{Path, PathBuf}; use std::process::Output; @@ -74,7 +73,6 @@ fn setup_toplevel(tmpdir: &Path) -> Result { let initrd_path = toplevel.join("initrd"); let kernel_path = toplevel.join("kernel"); - let systemd_path = toplevel.join("systemd"); let nixos_version_path = toplevel.join("nixos-version"); let kernel_modules_path = toplevel.join("kernel-modules/lib/modules/6.1.1"); @@ -84,7 +82,6 @@ fn setup_toplevel(tmpdir: &Path) -> Result { // in isolation this should suffice. fs::copy(&test_systemd_stub, initrd_path)?; fs::copy(&test_systemd_stub, kernel_path)?; - symlink(&test_systemd, systemd_path)?; fs::write(nixos_version_path, b"23.05")?; fs::create_dir_all(kernel_modules_path)?; @@ -114,6 +111,8 @@ pub fn lanzaboote_install( let output = cmd .env("LANZABOOTE_STUB", test_systemd_stub) .arg("install") + .arg("--systemd") + .arg(test_systemd) .arg("--public-key") .arg("tests/fixtures/uefi-keys/db.pem") .arg("--private-key") diff --git a/rust/tool/tests/systemd_boot.rs b/rust/tool/tests/systemd_boot.rs new file mode 100644 index 0000000..89f1cfb --- /dev/null +++ b/rust/tool/tests/systemd_boot.rs @@ -0,0 +1,155 @@ +use sha2::{Digest, Sha256}; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::{fs, os::unix::prelude::MetadataExt}; + +use anyhow::Result; +use tempfile::tempdir; + +mod common; + +#[test] +fn keep_systemd_boot_binaries() -> Result<()> { + let esp = tempdir()?; + let tmpdir = tempdir()?; + let profiles = tempdir()?; + let generation_link = common::setup_generation_link(tmpdir.path(), profiles.path(), 1) + .expect("Failed to setup generation link"); + + let systemd_boot_path = systemd_boot_path(&esp); + let systemd_boot_fallback_path = systemd_boot_fallback_path(&esp); + + let output0 = common::lanzaboote_install(0, esp.path(), vec![&generation_link])?; + assert!(output0.status.success()); + + // Use the modification time instead of a hash because the hash would be the same even if the + // file was overwritten. + let systemd_boot_mtime0 = mtime(&systemd_boot_path); + let systemd_boot_fallback_mtime0 = mtime(&systemd_boot_fallback_path); + + let output1 = common::lanzaboote_install(0, esp.path(), vec![generation_link])?; + assert!(output1.status.success()); + + let systemd_boot_mtime1 = mtime(&systemd_boot_path); + let systemd_boot_fallback_mtime1 = mtime(&systemd_boot_fallback_path); + + assert_eq!( + systemd_boot_mtime0, systemd_boot_mtime1, + "systemd-boot binary was modified on second install." + ); + assert_eq!( + systemd_boot_fallback_mtime0, systemd_boot_fallback_mtime1, + "systemd-boot fallback binary was moidified on second install." + ); + + Ok(()) +} + +#[test] +fn overwrite_malformed_systemd_boot_binaries() -> Result<()> { + let esp = tempdir()?; + let tmpdir = tempdir()?; + let profiles = tempdir()?; + let generation_link = common::setup_generation_link(tmpdir.path(), profiles.path(), 1) + .expect("Failed to setup generation link"); + + let systemd_boot_path = systemd_boot_path(&esp); + let systemd_boot_fallback_path = systemd_boot_fallback_path(&esp); + + let output0 = common::lanzaboote_install(0, esp.path(), vec![&generation_link])?; + assert!(output0.status.success()); + + // Make systemd-boot binaries malformed by truncating them. + fs::File::create(&systemd_boot_path)?; + fs::File::create(&systemd_boot_fallback_path)?; + + let malformed_systemd_boot_hash = hash_file(&systemd_boot_path); + let malformed_systemd_boot_fallback_hash = hash_file(&systemd_boot_fallback_path); + + let output1 = common::lanzaboote_install(0, esp.path(), vec![generation_link])?; + assert!(output1.status.success()); + + let systemd_boot_hash = hash_file(&systemd_boot_path); + let systemd_boot_fallback_hash = hash_file(&systemd_boot_fallback_path); + + assert_ne!( + malformed_systemd_boot_hash, systemd_boot_hash, + "Malformed systemd-boot binaries were not replaced." + ); + assert_ne!( + malformed_systemd_boot_fallback_hash, systemd_boot_fallback_hash, + "Maformed systemd-boot fallback binaries were not replaced." + ); + + Ok(()) +} + +#[test] +fn overwrite_unsigned_systemd_boot_binaries() -> Result<()> { + let esp = tempdir()?; + let tmpdir = tempdir()?; + let profiles = tempdir()?; + let generation_link = common::setup_generation_link(tmpdir.path(), profiles.path(), 1) + .expect("Failed to setup generation link"); + + let systemd_boot_path = systemd_boot_path(&esp); + let systemd_boot_fallback_path = systemd_boot_fallback_path(&esp); + + let output0 = common::lanzaboote_install(0, esp.path(), vec![&generation_link])?; + assert!(output0.status.success()); + + remove_signature(&systemd_boot_path)?; + remove_signature(&systemd_boot_fallback_path)?; + assert!(!verify_signature(&systemd_boot_path)?); + assert!(!verify_signature(&systemd_boot_fallback_path)?); + + let output1 = common::lanzaboote_install(0, esp.path(), vec![generation_link])?; + assert!(output1.status.success()); + + assert!(verify_signature(&systemd_boot_path)?); + assert!(verify_signature(&systemd_boot_fallback_path)?); + + Ok(()) +} + +fn systemd_boot_path(esp: &tempfile::TempDir) -> PathBuf { + esp.path().join("EFI/systemd/systemd-bootx64.efi") +} + +fn systemd_boot_fallback_path(esp: &tempfile::TempDir) -> PathBuf { + esp.path().join("EFI/BOOT/BOOTX64.EFI") +} + +/// Look up the modification time (mtime) of a file. +fn mtime(path: &Path) -> i64 { + fs::metadata(path) + .expect("Failed to read modification time.") + .mtime() +} + +fn hash_file(path: &Path) -> sha2::digest::Output { + Sha256::digest(fs::read(path).expect("Failed to read file to hash.")) +} + +/// Remove signature from a signed PE file. +pub fn remove_signature(path: &Path) -> Result<()> { + let output = Command::new("sbattach") + .arg("--remove") + .arg(path.as_os_str()) + .output()?; + print!("{}", String::from_utf8(output.stdout)?); + print!("{}", String::from_utf8(output.stderr)?); + Ok(()) +} + +/// Verify signature of PE file. +pub fn verify_signature(path: &Path) -> Result { + let output = Command::new("sbverify") + .arg(path.as_os_str()) + .arg("--cert") + .arg("tests/fixtures/uefi-keys/db.pem") + .output()?; + print!("{}", String::from_utf8(output.stdout)?); + print!("{}", String::from_utf8(output.stderr)?); + Ok(output.status.success()) +}