tool: use random names for secure tempfiles

Using random names for tempfiles makes handling them easier. It reduces
the amount of noise in the code because no custom name needs to be
provided for each tempfile. The names were not really useful in any
case.

It also does not burden the developer with ensuring uniqueness of names.
This is relevant when files for multiple generations need to be stored
in the same directory (e.g. because they need to be accessed after
handling one generation).

Out of an abundance of caution, 32 random alphanumeric characters are
chosen for each filename. The tempfile crate, in comparison, only
chooses 8. 32 characters should be enough to avoid collisions, even
if the PRNG is not of cryptographic quality.
This commit is contained in:
nikstur 2023-02-14 01:49:10 +01:00
parent ceed92460f
commit df6b1b07f7
5 changed files with 42 additions and 25 deletions

1
rust/tool/Cargo.lock generated
View File

@ -282,6 +282,7 @@ dependencies = [
"bootspec",
"clap",
"expect-test",
"fastrand",
"filetime",
"goblin",
"nix",

View File

@ -22,6 +22,9 @@ bootspec = { git = "https://github.com/DeterminateSystems/bootspec" }
walkdir = "2.3.2"
time = "0.3.17"
sha2 = "0.10.6"
# Keep the fastrand version aligned with the one from tempfile to avoid two
# different versions.
fastrand = "1.6.0"
[dev-dependencies]
assert_cmd = "2.0.7"

View File

@ -140,19 +140,20 @@ impl Installer {
let os_release = OsRelease::from_generation(generation)
.context("Failed to build OsRelease from generation.")?;
let os_release_path = tempdir
.write_secure_file("os-release", os_release.to_string().as_bytes())
.write_secure_file(os_release.to_string().as_bytes())
.context("Failed to write os-release file.")?;
println!("Appending secrets to initrd...");
let initrd_location = tempdir.path().join("initrd");
fs::copy(
let initrd_content = fs::read(
bootspec
.initrd
.as_ref()
.context("Lanzaboote does not support missing initrd yet")?,
&initrd_location,
)?;
let initrd_location = tempdir
.write_secure_file(initrd_content)
.context("Failed to copy initrd to tempfile.")?;
if let Some(initrd_secrets_script) = &bootspec.initrd_secrets {
append_initrd_secrets(initrd_secrets_script, &initrd_location)?;
}

View File

@ -8,7 +8,7 @@ use anyhow::{Context, Result};
use goblin::pe::PE;
use sha2::{Digest, Sha256};
use crate::utils::SecureTempDirExt;
use crate::utils::{tmpname, SecureTempDirExt};
type Hash = sha2::digest::Output<Sha256>;
@ -30,18 +30,13 @@ pub fn lanzaboote_image(
) -> Result<PathBuf> {
// objcopy can only copy files into the PE binary. That's why we
// have to write the contents of some bootspec properties to disk.
let kernel_cmdline_file =
tempdir.write_secure_file("kernel-cmdline", kernel_cmdline.join(" "))?;
let kernel_cmdline_file = tempdir.write_secure_file(kernel_cmdline.join(" "))?;
let kernel_path_file =
tempdir.write_secure_file("kernel-path", esp_relative_uefi_path(esp, kernel_path)?)?;
let kernel_hash_file =
tempdir.write_secure_file("kernel-hash", file_hash(kernel_path)?.as_slice())?;
let kernel_path_file = tempdir.write_secure_file(esp_relative_uefi_path(esp, kernel_path)?)?;
let kernel_hash_file = tempdir.write_secure_file(file_hash(kernel_path)?.as_slice())?;
let initrd_path_file =
tempdir.write_secure_file("initrd-path", esp_relative_uefi_path(esp, initrd_path)?)?;
let initrd_hash_file =
tempdir.write_secure_file("initrd-hash", file_hash(initrd_path)?.as_slice())?;
let initrd_path_file = tempdir.write_secure_file(esp_relative_uefi_path(esp, initrd_path)?)?;
let initrd_hash_file = tempdir.write_secure_file(file_hash(initrd_path)?.as_slice())?;
let os_release_offs = stub_offset(lanzaboote_stub)?;
let kernel_cmdline_offs = os_release_offs + file_size(os_release)?;
@ -59,7 +54,7 @@ pub fn lanzaboote_image(
s(".kernelh", kernel_hash_file, kernel_hash_offs),
];
let image_path = tempdir.path().join("lanzaboote-stub.efi");
let image_path = tempdir.path().join(tmpname());
wrap_in_pe(lanzaboote_stub, sections, &image_path)?;
Ok(image_path)
}

View File

@ -1,15 +1,20 @@
use std::ffi::OsString;
use std::fs;
use std::io::Write;
use std::iter::repeat_with;
use std::os::unix::fs::OpenOptionsExt;
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use anyhow::{Context, Result};
use tempfile::TempDir;
/// The number of random alphanumeric characters in the tempfiles.
const TEMPFILE_RANDOM_LENGTH: usize = 32;
/// Extension for a temporary directory that enables creating secure temporary files in it.
pub trait SecureTempDirExt {
fn create_secure_file(&self, file_name: &str) -> Result<fs::File>;
fn write_secure_file(&self, file_name: &str, contents: impl AsRef<[u8]>) -> Result<PathBuf>;
fn create_secure_file(&self, path: &Path) -> Result<fs::File>;
fn write_secure_file(&self, contents: impl AsRef<[u8]>) -> Result<PathBuf>;
}
/// This implementation has three useful properties:
@ -25,20 +30,19 @@ pub trait SecureTempDirExt {
/// after they are not needed anymore.
impl SecureTempDirExt for TempDir {
/// Create a temporary file that can only be accessed by the current Linux user.
fn create_secure_file(&self, file_name: &str) -> Result<fs::File> {
let path = self.path().join(file_name);
fn create_secure_file(&self, path: &Path) -> Result<fs::File> {
fs::OpenOptions::new()
.create(true)
.write(true)
.mode(0o600)
.open(&path)
.open(path)
.with_context(|| format!("Failed to create tempfile: {path:?}"))
}
/// Create a temporary file and write a `u8` slice to it.
fn write_secure_file(&self, file_name: &str, contents: impl AsRef<[u8]>) -> Result<PathBuf> {
let path = self.path().join(file_name);
let mut tmpfile = self.create_secure_file(file_name)?;
fn write_secure_file(&self, contents: impl AsRef<[u8]>) -> Result<PathBuf> {
let path = self.path().join(tmpname());
let mut tmpfile = self.create_secure_file(&path)?;
tmpfile
.write_all(contents.as_ref())
@ -47,3 +51,16 @@ impl SecureTempDirExt for TempDir {
Ok(path)
}
}
/// Generate a random (but not cryptographically secure) name for a temporary file.
///
/// This is heavily inspired by the way temporary names are generated in the `tempfile` crate.
/// Since the `tempfile` crate does not expose this functionality, we have to recreate it here.
pub fn tmpname() -> OsString {
let mut buf = OsString::with_capacity(TEMPFILE_RANDOM_LENGTH);
let mut char_buf = [0u8; 4];
for c in repeat_with(fastrand::alphanumeric).take(TEMPFILE_RANDOM_LENGTH) {
buf.push(c.encode_utf8(&mut char_buf));
}
buf
}