From a3150dca118477573ca0399cc7c0f3df0a769683 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Sat, 26 Nov 2022 01:24:33 +0100 Subject: [PATCH 1/2] lanzatool: perform secure assembling for lanzaboote_image and PE wrapping --- rust/lanzatool/src/install.rs | 17 +++++++- rust/lanzatool/src/pe.rs | 73 +++++++++++++++++++++++++---------- 2 files changed, 69 insertions(+), 21 deletions(-) diff --git a/rust/lanzatool/src/install.rs b/rust/lanzatool/src/install.rs index ceb1b01..50809f9 100644 --- a/rust/lanzatool/src/install.rs +++ b/rust/lanzatool/src/install.rs @@ -10,6 +10,8 @@ use crate::esp::EspPaths; use crate::pe; use crate::signer::Signer; +use tempfile::tempdir; + pub fn install( public_key: &Path, private_key: &Path, @@ -31,12 +33,22 @@ pub fn install( let kernel_cmdline = assemble_kernel_cmdline(bootspec_doc.init, bootspec_doc.kernel_params); + // prepare a secure temporary directory + // permission bits are not set, because when files below + // are opened, they are opened with 600 mode bits. + // hence, they cannot be read except by the current user + // which is assumed to be root in most cases. + // TODO(Raito): prove to niksnur this is actually acceptable. + let secure_temp_dir = tempdir()?; + let lanzaboote_image = pe::lanzaboote_image( + &secure_temp_dir, lanzaboote_stub, &bootspec_doc.extension.os_release, &kernel_cmdline, &esp_paths.kernel, &esp_paths.initrd, + &bootspec_doc.initrd_secrets, &esp_paths.esp, ) .context("Failed to assemble stub")?; @@ -44,7 +56,7 @@ pub fn install( println!("Wrapping initrd into a PE binary..."); let wrapped_initrd = - pe::wrap_initrd(initrd_stub, &bootspec_doc.initrd).context("Failed to assemble stub")?; + pe::wrap_initrd(&secure_temp_dir, initrd_stub, &bootspec_doc.initrd).context("Failed to assemble stub")?; println!("Copy files to EFI system partition..."); @@ -65,6 +77,9 @@ pub fn install( copy(&source, &target)?; } + // TODO: we should implement sign_and_copy which would be secure + // by construction for TOCTOU. + println!("Signing files..."); let signer = Signer::new(&public_key, &private_key); diff --git a/rust/lanzatool/src/pe.rs b/rust/lanzatool/src/pe.rs index f4d0bfb..56147a7 100644 --- a/rust/lanzatool/src/pe.rs +++ b/rust/lanzatool/src/pe.rs @@ -1,28 +1,42 @@ use std::fs; use std::io::Write; +use std::os::unix::prelude::OpenOptionsExt; use std::os::unix::fs::MetadataExt; use std::path::{Path, PathBuf}; use std::process::Command; use anyhow::{Context, Result}; use goblin::pe::PE; -use tempfile::NamedTempFile; use crate::utils; +use tempfile::TempDir; + pub fn lanzaboote_image( + target_dir: &TempDir, lanzaboote_stub: &Path, os_release: &Path, kernel_cmdline: &[String], kernel_path: &Path, initrd_path: &Path, + append_initrd_secrets_path: &Option, esp: &Path, ) -> Result { // objcopy copies files into the PE binary. That's why we have to write the contents // of some bootspec properties to disk - let kernel_cmdline_file = write_to_tmp(kernel_cmdline.join(" "))?; - let kernel_path_file = write_to_tmp(esp_relative_path_string(esp, kernel_path))?; - let initrd_path_file = write_to_tmp(esp_relative_path_string(esp, initrd_path))?; + let (kernel_cmdline_file, _) = write_to_tmp(&target_dir, + "kernel-cmdline", + kernel_cmdline.join(" "))?; + let (kernel_path_file, _) = write_to_tmp(&target_dir, + "kernel", + esp_relative_path_string(esp, kernel_path))?; + let (initrd_path_file, _) = write_to_tmp(&target_dir, + "initrd", + esp_relative_path_string(esp, initrd_path))?; + + if let Some(initrd_secret_script) = append_initrd_secrets_path { + append_initrd_secrets(&initrd_secret_script, &initrd_path_file)?; + } let os_release_offs = stub_offset(lanzaboote_stub)?; let kernel_cmdline_offs = os_release_offs + file_size(&os_release)?; @@ -36,20 +50,40 @@ pub fn lanzaboote_image( s(".kernelp", kernel_path_file, kernel_path_offs), ]; - wrap_in_pe(&lanzaboote_stub, sections) + wrap_in_pe(&target_dir, "lanzaboote-stub.efi", &lanzaboote_stub, sections) } -pub fn wrap_initrd(initrd_stub: &Path, initrd: &Path) -> Result { +pub fn append_initrd_secrets(append_initrd_secrets_path: &Path, initrd_path: &PathBuf) -> Result<()> { + let status = Command::new(append_initrd_secrets_path) + .args(vec![ + initrd_path + ]) + .status() + .context("Failed to append initrd secrets")?; + if !status.success() { + return Err(anyhow::anyhow!("Failed to append initrd secrets with args `{:?}`", vec![append_initrd_secrets_path, initrd_path]).into()); + } + + Ok(()) +} + +pub fn wrap_initrd(target_dir: &TempDir, initrd_stub: &Path, initrd: &Path) -> Result { let initrd_offs = stub_offset(initrd_stub)?; let sections = vec![s(".initrd", initrd, initrd_offs)]; - wrap_in_pe(initrd_stub, sections) + wrap_in_pe(target_dir, "wrapped-initrd.exe", initrd_stub, sections) } -fn wrap_in_pe(stub: &Path, sections: Vec
) -> Result { - let image = NamedTempFile::new().context("Failed to generate named temp file")?; +fn wrap_in_pe(target_dir: &TempDir, filename: &str, stub: &Path, sections: Vec
) -> Result { + let image_path = target_dir.path().join(filename); + let _ = fs::OpenOptions::new() + .create(true) + .write(true) + .mode(0o600) + .open(&image_path) + .context("Failed to generate named temp file")?; let mut args: Vec = sections.iter().flat_map(Section::to_objcopy).collect(); - let extra_args = vec![utils::path_to_string(stub), utils::path_to_string(&image)]; + let extra_args = vec![utils::path_to_string(stub), utils::path_to_string(&image_path)]; args.extend(extra_args); let status = Command::new("objcopy") @@ -60,13 +94,7 @@ fn wrap_in_pe(stub: &Path, sections: Vec
) -> Result { return Err(anyhow::anyhow!("Failed to wrap in pe with args `{:?}`", &args).into()); } - let (_, persistent_image) = image.keep().with_context(|| { - format!( - "Failed to persist image with stub: {} from temporary file", - stub.display() - ) - })?; - Ok(persistent_image) + Ok(image_path) } struct Section { @@ -94,12 +122,17 @@ fn s(name: &'static str, file_path: impl AsRef, offset: u64) -> Section { } } -fn write_to_tmp(contents: impl AsRef<[u8]>) -> Result { - let mut tmpfile = NamedTempFile::new().context("Failed to create tempfile")?; +fn write_to_tmp(secure_temp: &TempDir, filename: &str, contents: impl AsRef<[u8]>) -> Result<(PathBuf, fs::File)> { + let mut tmpfile = fs::OpenOptions::new() + .create(true) + .write(true) + .mode(0o600) + .open(secure_temp.path().join(filename)) + .context("Failed to create tempfile")?; tmpfile .write_all(contents.as_ref()) .context("Failed to write to tempfile")?; - Ok(tmpfile.keep()?.1) + Ok((secure_temp.path().join(filename), tmpfile)) } fn esp_relative_path_string(esp: &Path, path: &Path) -> String { From 9f65f752892c0ef0600ac589bf115e6e40410062 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Sat, 26 Nov 2022 01:50:51 +0100 Subject: [PATCH 2/2] feature: support initrd secrets --- flake.nix | 23 +++++++++++++++++++++++ nix/lanzaboote.nix | 3 +++ rust/lanzatool/src/install.rs | 25 +++++++++++++++++++++++-- rust/lanzatool/src/pe.rs | 23 ++--------------------- 4 files changed, 51 insertions(+), 23 deletions(-) diff --git a/flake.nix b/flake.nix index 01203b6..7386e97 100644 --- a/flake.nix +++ b/flake.nix @@ -226,6 +226,29 @@ assert "Secure Boot: enabled (user)" in machine.succeed("bootctl status") ''; }; + # So, this is the responsibility of the lanzatool install + # to run the append-initrd-secret script + # This test assert that lanzatool still do the right thing + # preDeviceCommands should not have any root filesystem mounted + # so it should not be able to find /etc/iamasecret, other than the + # initrd's one. + # which should exist IF lanzatool do the right thing. + lanzaboote-with-initrd-secrets = mkSecureBootTest { + name = "signed-files-boot-with-secrets-under-secureboot"; + machine = { ... }: { + boot.initrd.secrets = { + "/etc/iamasecret" = (pkgs.writeText "iamsecret" "this is a very secure secret"); + }; + + boot.initrd.preDeviceCommands = '' + grep "this is a very secure secret" /etc/iamasecret + ''; + }; + testScript = '' + machine.start() + assert "Secure Boot: enabled (user)" in machine.succeed("bootctl status") + ''; + }; is-initrd-secured = mkUnsignedTest { name = "unsigned-initrd-do-not-boot-under-secureboot"; path = { diff --git a/nix/lanzaboote.nix b/nix/lanzaboote.nix index 2149bd9..0bf9b75 100644 --- a/nix/lanzaboote.nix +++ b/nix/lanzaboote.nix @@ -33,6 +33,9 @@ in }; config = mkIf cfg.enable { + # bootspec is putting at false + # until we fix this upstream, we will mkForce it. + boot.loader.supportsInitrdSecrets = mkForce true; boot.loader.external = { enable = true; passBootspec = true; diff --git a/rust/lanzatool/src/install.rs b/rust/lanzatool/src/install.rs index 50809f9..cfbbe4b 100644 --- a/rust/lanzatool/src/install.rs +++ b/rust/lanzatool/src/install.rs @@ -12,6 +12,8 @@ use crate::signer::Signer; use tempfile::tempdir; +use std::process::Command; + pub fn install( public_key: &Path, private_key: &Path, @@ -48,15 +50,20 @@ pub fn install( &kernel_cmdline, &esp_paths.kernel, &esp_paths.initrd, - &bootspec_doc.initrd_secrets, &esp_paths.esp, ) .context("Failed to assemble stub")?; println!("Wrapping initrd into a PE binary..."); + let initrd_location = secure_temp_dir.path().join("initrd"); + copy(&bootspec_doc.initrd, &initrd_location)?; + if let Some(initrd_secrets_script) = bootspec_doc.initrd_secrets { + append_initrd_secrets(&initrd_secrets_script, + &initrd_location)?; + } let wrapped_initrd = - pe::wrap_initrd(&secure_temp_dir, initrd_stub, &bootspec_doc.initrd).context("Failed to assemble stub")?; + pe::wrap_initrd(&secure_temp_dir, initrd_stub, &initrd_location).context("Failed to assemble stub")?; println!("Copy files to EFI system partition..."); @@ -108,6 +115,20 @@ pub fn install( Ok(()) } +pub fn append_initrd_secrets(append_initrd_secrets_path: &Path, initrd_path: &PathBuf) -> Result<()> { + let status = Command::new(append_initrd_secrets_path) + .args(vec![ + initrd_path + ]) + .status() + .context("Failed to append initrd secrets")?; + if !status.success() { + return Err(anyhow::anyhow!("Failed to append initrd secrets with args `{:?}`", vec![append_initrd_secrets_path, initrd_path]).into()); + } + + Ok(()) +} + fn assemble_kernel_cmdline(init: PathBuf, kernel_params: Vec) -> Vec { let init_string = init .into_os_string() diff --git a/rust/lanzatool/src/pe.rs b/rust/lanzatool/src/pe.rs index 56147a7..75e4e15 100644 --- a/rust/lanzatool/src/pe.rs +++ b/rust/lanzatool/src/pe.rs @@ -19,7 +19,6 @@ pub fn lanzaboote_image( kernel_cmdline: &[String], kernel_path: &Path, initrd_path: &Path, - append_initrd_secrets_path: &Option, esp: &Path, ) -> Result { // objcopy copies files into the PE binary. That's why we have to write the contents @@ -28,16 +27,12 @@ pub fn lanzaboote_image( "kernel-cmdline", kernel_cmdline.join(" "))?; let (kernel_path_file, _) = write_to_tmp(&target_dir, - "kernel", + "kernel-esp-path", esp_relative_path_string(esp, kernel_path))?; let (initrd_path_file, _) = write_to_tmp(&target_dir, - "initrd", + "initrd-esp-path", esp_relative_path_string(esp, initrd_path))?; - if let Some(initrd_secret_script) = append_initrd_secrets_path { - append_initrd_secrets(&initrd_secret_script, &initrd_path_file)?; - } - let os_release_offs = stub_offset(lanzaboote_stub)?; let kernel_cmdline_offs = os_release_offs + file_size(&os_release)?; let initrd_path_offs = kernel_cmdline_offs + file_size(&kernel_cmdline_file)?; @@ -53,20 +48,6 @@ pub fn lanzaboote_image( wrap_in_pe(&target_dir, "lanzaboote-stub.efi", &lanzaboote_stub, sections) } -pub fn append_initrd_secrets(append_initrd_secrets_path: &Path, initrd_path: &PathBuf) -> Result<()> { - let status = Command::new(append_initrd_secrets_path) - .args(vec![ - initrd_path - ]) - .status() - .context("Failed to append initrd secrets")?; - if !status.success() { - return Err(anyhow::anyhow!("Failed to append initrd secrets with args `{:?}`", vec![append_initrd_secrets_path, initrd_path]).into()); - } - - Ok(()) -} - pub fn wrap_initrd(target_dir: &TempDir, initrd_stub: &Path, initrd: &Path) -> Result { let initrd_offs = stub_offset(initrd_stub)?; let sections = vec![s(".initrd", initrd, initrd_offs)];