Merge pull request #107 from nix-community/random-names-for-tmpfiles

tool: use random names for secure tempfiles

rust/tool/Cargo.lock

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

rust/tool/Cargo.toml

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

rust/tool/src/install.rs

@@ -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");
+        let initrd_content = fs::read(
-        fs::copy(
             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)?;
         }

rust/tool/src/pe.rs

@@ -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 =
+    let kernel_cmdline_file = tempdir.write_secure_file(kernel_cmdline.join(" "))?;
-        tempdir.write_secure_file("kernel-cmdline", kernel_cmdline.join(" "))?;
 
-    let kernel_path_file =
+    let kernel_path_file = tempdir.write_secure_file(esp_relative_uefi_path(esp, kernel_path)?)?;
-        tempdir.write_secure_file("kernel-path", esp_relative_uefi_path(esp, kernel_path)?)?;
+    let kernel_hash_file = tempdir.write_secure_file(file_hash(kernel_path)?.as_slice())?;
-    let kernel_hash_file =
-        tempdir.write_secure_file("kernel-hash", file_hash(kernel_path)?.as_slice())?;
 
-    let initrd_path_file =
+    let initrd_path_file = tempdir.write_secure_file(esp_relative_uefi_path(esp, initrd_path)?)?;
-        tempdir.write_secure_file("initrd-path", esp_relative_uefi_path(esp, initrd_path)?)?;
+    let initrd_hash_file = tempdir.write_secure_file(file_hash(initrd_path)?.as_slice())?;
-    let initrd_hash_file =
-        tempdir.write_secure_file("initrd-hash", 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)
 }

rust/tool/src/utils.rs

@@ -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 create_secure_file(&self, path: &Path) -> Result<fs::File>;
-    fn write_secure_file(&self, file_name: &str, contents: impl AsRef<[u8]>) -> Result<PathBuf>;
+    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> {
+    fn create_secure_file(&self, path: &Path) -> Result<fs::File> {
-        let path = self.path().join(file_name);
         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> {
+    fn write_secure_file(&self, contents: impl AsRef<[u8]>) -> Result<PathBuf> {
-        let path = self.path().join(file_name);
+        let path = self.path().join(tmpname());
-        let mut tmpfile = self.create_secure_file(file_name)?;
+        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
+}