tool: smarter systemd-boot install

The process of installing systemd-boot is "smarter" because it now
considers a a few conditions instead of doing nothing if there is a file
at the deistination path. systemd-boot is now forcibly installed (i.e.
overwriting any file at the destination) if (1) there is no file at the
destination, OR (2) a newer version of systemd-boot is available, OR (3)
the signature of the file at the destination could not be verified.
This commit is contained in:
nikstur 2023-01-18 01:58:45 +01:00
parent a9bce14645
commit cc169689f3
8 changed files with 364 additions and 28 deletions

View File

@ -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} \

View File

@ -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,

View File

@ -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<bool> {
// 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<f32> {
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)?)
}

View File

@ -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<String, String>);
impl OsRelease {
pub fn from_generation(generation: &Generation) -> Result<Self> {
@ -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<Self> {
let mut map = BTreeMap::new();
let key_value_lines = value.lines().map(|x| x.split('=').collect::<Vec<&str>>());
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(())
}
}

View File

@ -180,6 +180,24 @@ fn file_size(path: impl AsRef<Path>) -> Result<u64> {
.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::*;

View File

@ -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<OsString> = 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
}
}

View File

@ -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<PathBuf> {
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<PathBuf> {
// 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")

View File

@ -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> {
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<bool> {
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())
}