Merge pull request #17 from blitz/appease-clippy

Lanzatool: appease clippy

flake.nix

@@ -159,13 +159,11 @@
         };
       in
         {
-          # TODO Will be enabled by nikstur.
+          lanzatool-unwrapped-clippy = craneLib.cargoClippy {
-          #
+            src = lanzatool-unwrapped-src;
-          # lanzatool-unwrapped-clippy = craneLib.cargoClippy {
+            cargoArtifacts = lanzatool-unwrapped-deps;
-          #   src = lanzatool-unwrapped-src;
+            cargoClippyExtraArgs = "--all-targets -- --deny warnings";
-          #   cargoArtifacts = lanzatool-unwrapped-deps;
+          };
-          #   cargoClippyExtraArgs = "--all-targets -- --deny warnings";
-          # };
 
           # TODO: user mode: OK
           # TODO: how to get in: {deployed, audited} mode ?

rust/lanzatool/src/cli.rs

@@ -4,6 +4,7 @@ use anyhow::{Context, Result};
 use clap::{Parser, Subcommand};
 
 use crate::install;
+use crate::signature::KeyPair;
 
 #[derive(Parser)]
 pub struct Cli {
@@ -60,11 +61,12 @@ fn install(args: InstallCommand) -> Result<()> {
     let initrd_stub = std::env::var("LANZABOOTE_INITRD_STUB")
         .context("Failed to read LANZABOOTE_INITRD_STUB env variable")?;
 
+    let key_pair = KeyPair::new(&args.public_key, &args.private_key);
+
     install::Installer::new(
         PathBuf::from(lanzaboote_stub),
         PathBuf::from(initrd_stub),
-        args.public_key,
+        key_pair,
-        args.private_key,
         args.pki_bundle,
         args.auto_enroll,
         args.esp,

rust/lanzatool/src/esp.rs

@@ -32,7 +32,7 @@ impl EspPaths {
             kernel: esp_nixos.join(nixos_path(&bootspec.kernel, "bzImage")?),
             initrd: esp_nixos.join(nixos_path(&bootspec.initrd, "initrd")?),
             linux: esp_linux.clone(),
-            lanzaboote_image: esp_linux.join(generation_path(&generation)),
+            lanzaboote_image: esp_linux.join(generation_path(generation)),
             efi_fallback_dir: esp_efi_fallback_dir.clone(),
             efi_fallback: esp_efi_fallback_dir.join("BOOTX64.EFI"),
             systemd: esp_systemd.clone(),
@@ -42,12 +42,17 @@ impl EspPaths {
 }
 
 fn nixos_path(path: impl AsRef<Path>, name: &str) -> Result<PathBuf> {
-    let resolved = path.as_ref().read_link().unwrap_or(path.as_ref().into());
+    let resolved = path
+        .as_ref()
+        .read_link()
+        .unwrap_or_else(|_| path.as_ref().into());
 
-    let parent = resolved.parent().ok_or(anyhow::anyhow!(format!(
+    let parent = resolved.parent().ok_or_else(|| {
+        anyhow::anyhow!(format!(
             "Path: {} does not have a parent",
             resolved.display()
-    )))?;
+        ))
+    })?;
 
     let without_store = parent.strip_prefix("/nix/store").with_context(|| {
         format!(

rust/lanzatool/src/generation.rs

@@ -16,7 +16,7 @@ impl Generation {
     pub fn from_toplevel(toplevel: impl AsRef<Path>) -> Result<Self> {
         let bootspec_path = toplevel.as_ref().join("bootspec/boot.v1.json");
         let bootspec: Bootspec = serde_json::from_slice(
-            &fs::read(&bootspec_path).context("Failed to read bootspec file")?,
+            &fs::read(bootspec_path).context("Failed to read bootspec file")?,
         )
         .context("Failed to parse bootspec json")?;
 
@@ -34,18 +34,19 @@ impl fmt::Display for Generation {
 }
 
 fn parse_version(toplevel: impl AsRef<Path>) -> Result<u64> {
-    let file_name = toplevel.as_ref().file_name().ok_or(anyhow::anyhow!(
+    let file_name = toplevel
-        "Failed to extract file name from generation"
+        .as_ref()
-    ))?;
+        .file_name()
+        .ok_or_else(|| anyhow::anyhow!("Failed to extract file name from generation"))?;
 
     let file_name_str = file_name
         .to_str()
         .with_context(|| "Failed to convert file name of generation to string")?;
 
     let generation_version = file_name_str
-        .split("-")
+        .split('-')
         .nth(1)
-        .ok_or(anyhow::anyhow!("Failed to extract version from generation"))?;
+        .ok_or_else(|| anyhow::anyhow!("Failed to extract version from generation"))?;
 
     let parsed_generation_version = generation_version
         .parse()

rust/lanzatool/src/install.rs

@@ -10,13 +10,12 @@ use tempfile::tempdir;
 use crate::esp::EspPaths;
 use crate::generation::Generation;
 use crate::pe;
-use crate::signer::Signer;
+use crate::signature::KeyPair;
 
 pub struct Installer {
     lanzaboote_stub: PathBuf,
     initrd_stub: PathBuf,
-    public_key: PathBuf,
+    key_pair: KeyPair,
-    private_key: PathBuf,
     _pki_bundle: Option<PathBuf>,
     _auto_enroll: bool,
     esp: PathBuf,
@@ -27,8 +26,7 @@ impl Installer {
     pub fn new(
         lanzaboote_stub: PathBuf,
         initrd_stub: PathBuf,
-        public_key: PathBuf,
+        key_pair: KeyPair,
-        private_key: PathBuf,
         _pki_bundle: Option<PathBuf>,
         _auto_enroll: bool,
         esp: PathBuf,
@@ -37,8 +35,7 @@ impl Installer {
         Self {
             lanzaboote_stub,
             initrd_stub,
-            public_key,
+            key_pair,
-            private_key,
             _pki_bundle,
             _auto_enroll,
             esp,
@@ -96,15 +93,13 @@ impl Installer {
         let initrd_location = secure_temp_dir.path().join("initrd");
         copy(&bootspec.initrd, &initrd_location)?;
         if let Some(initrd_secrets_script) = &bootspec.initrd_secrets {
-            append_initrd_secrets(&initrd_secrets_script, &initrd_location)?;
+            append_initrd_secrets(initrd_secrets_script, &initrd_location)?;
         }
         let wrapped_initrd = pe::wrap_initrd(&secure_temp_dir, &self.initrd_stub, &initrd_location)
             .context("Failed to assemble stub")?;
 
         println!("Sign and copy files to EFI system partition...");
 
-        let signer = Signer::new(&self.public_key, &self.private_key);
-
         let systemd_boot = bootspec
             .toplevel
             .join("systemd/lib/systemd/boot/efi/systemd-bootx64.efi");
@@ -121,7 +116,7 @@ impl Installer {
             println!("Signing {}...", to.display());
 
             ensure_parent_dir(to);
-            signer.sign_and_copy(&from, &to).with_context(|| {
+            self.key_pair.sign_and_copy(from, to).with_context(|| {
                 format!("Failed to copy and sign file from {:?} to {:?}", from, to)
             })?;
             // Call sync to improve the likelihood that file is actually written to disk
@@ -149,8 +144,7 @@ pub fn append_initrd_secrets(
         return Err(anyhow::anyhow!(
             "Failed to append initrd secrets with args `{:?}`",
             vec![append_initrd_secrets_path, initrd_path]
-        )
+        ));
-        .into());
     }
 
     Ok(())

rust/lanzatool/src/main.rs

@@ -4,7 +4,7 @@ mod esp;
 mod generation;
 mod install;
 mod pe;
-mod signer;
+mod signature;
 mod utils;
 
 use anyhow::Result;

rust/lanzatool/src/pe.rs

@@ -1,7 +1,7 @@
 use std::fs;
 use std::io::Write;
-use std::os::unix::prelude::OpenOptionsExt;
 use std::os::unix::fs::MetadataExt;
+use std::os::unix::prelude::OpenOptionsExt;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 
@@ -23,18 +23,21 @@ pub fn lanzaboote_image(
 ) -> Result<PathBuf> {
     // 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(&target_dir,
+    let (kernel_cmdline_file, _) =
-        "kernel-cmdline",
+        write_to_tmp(target_dir, "kernel-cmdline", kernel_cmdline.join(" "))?;
-        kernel_cmdline.join(" "))?;
+    let (kernel_path_file, _) = write_to_tmp(
-    let (kernel_path_file, _) = write_to_tmp(&target_dir, 
+        target_dir,
         "kernel-esp-path",
-        esp_relative_path_string(esp, kernel_path))?;
+        esp_relative_path_string(esp, kernel_path),
-    let (initrd_path_file, _) = write_to_tmp(&target_dir, 
+    )?;
+    let (initrd_path_file, _) = write_to_tmp(
+        target_dir,
         "initrd-esp-path",
-        esp_relative_path_string(esp, initrd_path))?;
+        esp_relative_path_string(esp, initrd_path),
+    )?;
 
     let os_release_offs = stub_offset(lanzaboote_stub)?;
-    let kernel_cmdline_offs = os_release_offs + file_size(&os_release)?;
+    let kernel_cmdline_offs = os_release_offs + file_size(os_release)?;
     let initrd_path_offs = kernel_cmdline_offs + file_size(&kernel_cmdline_file)?;
     let kernel_path_offs = initrd_path_offs + file_size(&initrd_path_file)?;
 
@@ -45,7 +48,7 @@ pub fn lanzaboote_image(
         s(".kernelp", kernel_path_file, kernel_path_offs),
     ];
 
-    wrap_in_pe(&target_dir, "lanzaboote-stub.efi", &lanzaboote_stub, sections)
+    wrap_in_pe(target_dir, "lanzaboote-stub.efi", lanzaboote_stub, sections)
 }
 
 pub fn wrap_initrd(target_dir: &TempDir, initrd_stub: &Path, initrd: &Path) -> Result<PathBuf> {
@@ -54,7 +57,12 @@ pub fn wrap_initrd(target_dir: &TempDir, initrd_stub: &Path, initrd: &Path) -> R
     wrap_in_pe(target_dir, "wrapped-initrd.exe", initrd_stub, sections)
 }
 
-fn wrap_in_pe(target_dir: &TempDir, filename: &str, stub: &Path, sections: Vec<Section>) -> Result<PathBuf> {
+fn wrap_in_pe(
+    target_dir: &TempDir,
+    filename: &str,
+    stub: &Path,
+    sections: Vec<Section>,
+) -> Result<PathBuf> {
     let image_path = target_dir.path().join(filename);
     let _ = fs::OpenOptions::new()
         .create(true)
@@ -64,7 +72,10 @@ fn wrap_in_pe(target_dir: &TempDir, filename: &str, stub: &Path, sections: Vec<S
         .context("Failed to generate named temp file")?;
 
     let mut args: Vec<String> = sections.iter().flat_map(Section::to_objcopy).collect();
-    let extra_args = vec![utils::path_to_string(stub), utils::path_to_string(&image_path)];
+    let extra_args = vec![
+        utils::path_to_string(stub),
+        utils::path_to_string(&image_path),
+    ];
     args.extend(extra_args);
 
     let status = Command::new("objcopy")
@@ -72,7 +83,10 @@ fn wrap_in_pe(target_dir: &TempDir, filename: &str, stub: &Path, sections: Vec<S
         .status()
         .context("Failed to run objcopy command")?;
     if !status.success() {
-        return Err(anyhow::anyhow!("Failed to wrap in pe with args `{:?}`", &args).into());
+        return Err(anyhow::anyhow!(
+            "Failed to wrap in pe with args `{:?}`",
+            &args
+        ));
     }
 
     Ok(image_path)
@@ -103,7 +117,11 @@ fn s(name: &'static str, file_path: impl AsRef<Path>, offset: u64) -> Section {
     }
 }
 
-fn write_to_tmp(secure_temp: &TempDir, filename: &str, contents: impl AsRef<[u8]>) -> Result<(PathBuf, fs::File)> {
+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)
@@ -125,7 +143,7 @@ fn esp_relative_path_string(esp: &Path, path: &Path) -> String {
         .into_os_string()
         .into_string()
         .expect("Failed to convert path '{}' to a relative string path")
-        .replace("/", "\\");
+        .replace('/', "\\");
     format!("\\{}", &relative_path_string)
 }
 
@@ -140,7 +158,7 @@ fn stub_offset(binary: &Path) -> Result<u64> {
     Ok(u64::from(
         pe.sections
             .last()
-            .and_then(|s| Some(s.virtual_size + s.virtual_address))
+            .map(|s| s.virtual_size + s.virtual_address)
             .expect("Failed to calculate offset"),
     ) + image_base)
 }

rust/lanzatool/src/signature.rs

@@ -5,12 +5,12 @@ use anyhow::Result;
 
 use crate::utils;
 
-pub struct Signer {
+pub struct KeyPair {
     pub private_key: PathBuf,
     pub public_key: PathBuf,
 }
 
-impl Signer {
+impl KeyPair {
     pub fn new(public_key: &Path, private_key: &Path) -> Self {
         Self {
             public_key: public_key.into(),
@@ -36,8 +36,7 @@ impl Signer {
             return Err(anyhow::anyhow!(
                 "Failed to sign file using sbsign with args `{:?}`",
                 &args
-            )
+            ));
-            .into());
         }
 
         Ok(())

rust/lanzatool/src/utils.rs

@@ -2,8 +2,10 @@ use std::path::Path;
 
 // All Linux file paths should be convertable to strings
 pub fn path_to_string(path: impl AsRef<Path>) -> String {
-    String::from(path.as_ref().to_str().expect(&format!(
+    String::from(path.as_ref().to_str().unwrap_or_else(|| {
+        panic!(
             "Failed to convert path '{}' to a string",
             path.as_ref().display()
-    )))
+        )
+    }))
 }