Merge pull request #75 from alois31/unsigned-kernel

Prevent loading of untrusted initrds

rust/stub/src/main.rs

@@ -2,20 +2,22 @@
 #![no_std]
 #![feature(abi_efiapi)]
 #![feature(negative_impls)]
+#![deny(unsafe_op_in_unsafe_fn)]
 
 extern crate alloc;
 
 mod linux_loader;
+mod pe_loader;
 mod pe_section;
 mod uefi_helpers;
 
+use pe_loader::Image;
 use pe_section::{pe_section, pe_section_as_string};
 use sha2::{Digest, Sha256};
 use uefi::{
     prelude::*,
     proto::{
         console::text::Output,
-        loaded_image::LoadedImage,
         media::file::{File, FileAttribute, FileMode, RegularFile},
     },
     CString16, Result,
@@ -23,7 +25,7 @@ use uefi::{
 
 use crate::{
     linux_loader::InitrdLoader,
-    uefi_helpers::{booted_image_cmdline, booted_image_file, read_all},
+    uefi_helpers::{booted_image_file, read_all},
 };
 
 type Hash = sha2::digest::Output<Sha256>;
@@ -65,13 +67,16 @@ struct EmbeddedConfiguration {
     /// The cryptographic hash of the initrd. This hash is computed
     /// over the whole PE binary, not only the embedded initrd.
     initrd_hash: Hash,
+
+    /// The kernel command-line.
+    cmdline: CString16,
 }
 
-/// Extract a filename from a PE section. The filename is stored as UTF-8.
+/// Extract a string, stored as UTF-8, from a PE section.
-fn extract_filename(file_data: &[u8], section: &str) -> Result<CString16> {
+fn extract_string(file_data: &[u8], section: &str) -> Result<CString16> {
-    let filename = pe_section_as_string(file_data, section).ok_or(Status::INVALID_PARAMETER)?;
+    let string = pe_section_as_string(file_data, section).ok_or(Status::INVALID_PARAMETER)?;
 
-    Ok(CString16::try_from(filename.as_str()).map_err(|_| Status::INVALID_PARAMETER)?)
+    Ok(CString16::try_from(string.as_str()).map_err(|_| Status::INVALID_PARAMETER)?)
 }
 
 /// Extract a Blake3 hash from a PE section.
@@ -90,11 +95,13 @@ impl EmbeddedConfiguration {
         let file_data = read_all(file)?;
 
         Ok(Self {
-            kernel_filename: extract_filename(&file_data, ".kernelp")?,
+            kernel_filename: extract_string(&file_data, ".kernelp")?,
             kernel_hash: extract_hash(&file_data, ".kernelh")?,
 
-            initrd_filename: extract_filename(&file_data, ".initrdp")?,
+            initrd_filename: extract_string(&file_data, ".initrdp")?,
             initrd_hash: extract_hash(&file_data, ".initrdh")?,
+
+            cmdline: extract_string(&file_data, ".cmdline")?,
         })
     }
 }
@@ -162,40 +169,13 @@ fn main(handle: Handle, mut system_table: SystemTable<Boot>) -> Status {
         return Status::SECURITY_VIOLATION;
     }
 
-    let kernel_cmdline =
+    let kernel =
-        booted_image_cmdline(system_table.boot_services()).expect("Failed to fetch command line");
+        Image::load(system_table.boot_services(), &kernel_data).expect("Failed to load the kernel");
-
-    let kernel_handle = {
-        system_table
-            .boot_services()
-            .load_image(
-                handle,
-                uefi::table::boot::LoadImageSource::FromBuffer {
-                    buffer: &kernel_data,
-                    file_path: None,
-                },
-            )
-            .expect("UEFI refused to load the kernel image. It may not be signed or it may not have an EFI stub.")
-    };
-
-    let mut kernel_image = system_table
-        .boot_services()
-        .open_protocol_exclusive::<LoadedImage>(kernel_handle)
-        .expect("Failed to open the LoadedImage protocol");
-
-    unsafe {
-        kernel_image.set_load_options(
-            kernel_cmdline.as_ptr() as *const u8,
-            u32::try_from(kernel_cmdline.len()).unwrap(),
-        );
-    }
 
     let mut initrd_loader = InitrdLoader::new(system_table.boot_services(), handle, initrd_data)
         .expect("Failed to load the initrd. It may not be there or it is not signed");
-    let status = system_table
+
-        .boot_services()
+    let status = unsafe { kernel.start(handle, &system_table, &config.cmdline) };
-        .start_image(kernel_handle)
-        .status();
 
     initrd_loader
         .uninstall(system_table.boot_services())

rust/stub/src/pe_loader.rs

@@ -0,0 +1,176 @@
+use core::ffi::c_void;
+
+use alloc::vec::Vec;
+use goblin::pe::PE;
+use uefi::{
+    prelude::BootServices,
+    proto::loaded_image::LoadedImage,
+    table::{
+        boot::{AllocateType, MemoryType},
+        Boot, SystemTable,
+    },
+    CStr16, Handle, Status,
+};
+
+/// UEFI mandates 4 KiB pages.
+const UEFI_PAGE_BITS: usize = 12;
+const UEFI_PAGE_MASK: usize = (1 << UEFI_PAGE_BITS) - 1;
+
+#[cfg(target_arch = "x86_64")]
+fn flush_instruction_cache(_start: *const u8, _length: usize) {
+    // x86_64 mandates coherent instruction cache
+}
+
+pub struct Image {
+    image: &'static mut [u8],
+    entry: extern "efiapi" fn(Handle, SystemTable<Boot>) -> Status,
+}
+
+/// Converts a length in bytes to the number of required pages.
+fn bytes_to_pages(bytes: usize) -> usize {
+    bytes
+        .checked_add(UEFI_PAGE_MASK)
+        .map(|rounded_up| rounded_up >> UEFI_PAGE_BITS)
+        .unwrap_or(1 << (usize::try_from(usize::BITS).unwrap() - UEFI_PAGE_BITS))
+}
+
+impl Image {
+    /// Loads and relocates a PE file.
+    ///
+    /// The image must be handed to [`start`] later. If this does not
+    /// happen, the memory allocated for the unpacked PE binary will
+    /// leak.
+    pub fn load(boot_services: &BootServices, file_data: &[u8]) -> uefi::Result<Image> {
+        let pe = PE::parse(file_data).map_err(|_| Status::LOAD_ERROR)?;
+
+        // Allocate all memory the image will need in virtual memory.
+        // We follow shim here and allocate as EfiLoaderCode.
+        let image = {
+            let section_lengths = pe
+                .sections
+                .iter()
+                .map(|section| {
+                    section
+                        .virtual_address
+                        .checked_add(section.virtual_size)
+                        .ok_or(Status::LOAD_ERROR)
+                })
+                .collect::<Result<Vec<u32>, uefi::Status>>()?;
+
+            let length = usize::try_from(section_lengths.into_iter().max().unwrap_or(0)).unwrap();
+
+            let base = boot_services.allocate_pages(
+                AllocateType::AnyPages,
+                MemoryType::LOADER_CODE,
+                bytes_to_pages(length),
+            )? as *mut u8;
+
+            unsafe {
+                core::ptr::write_bytes(base, 0, length);
+                core::slice::from_raw_parts_mut(base, length)
+            }
+        };
+
+        // Populate all sections in virtual memory.
+        for section in &pe.sections {
+            let copy_size =
+                usize::try_from(u32::min(section.virtual_size, section.size_of_raw_data)).unwrap();
+            let raw_start = usize::try_from(section.pointer_to_raw_data).unwrap();
+            let raw_end = raw_start.checked_add(copy_size).ok_or(Status::LOAD_ERROR)?;
+            let virt_start = usize::try_from(section.virtual_address).unwrap();
+            let virt_end = virt_start
+                .checked_add(copy_size)
+                .ok_or(Status::LOAD_ERROR)?;
+
+            if virt_end > image.len() || raw_end > file_data.len() {
+                return Err(Status::LOAD_ERROR.into());
+            }
+            image[virt_start..virt_end].copy_from_slice(&file_data[raw_start..raw_end]);
+        }
+
+        // Image base relocations are not supported.
+        if pe
+            .header
+            .optional_header
+            .and_then(|h| *h.data_directories.get_base_relocation_table())
+            .is_some()
+        {
+            return Err(Status::INCOMPATIBLE_VERSION.into());
+        }
+
+        flush_instruction_cache(image.as_ptr(), image.len());
+
+        if pe.entry >= image.len() {
+            return Err(Status::LOAD_ERROR.into());
+        }
+        let entry = unsafe { core::mem::transmute(&image[pe.entry]) };
+
+        Ok(Image { image, entry })
+    }
+
+    /// Starts a trusted loaded PE file.
+    /// The caller is responsible for verifying that it trusts the PE file to uphold the invariants detailed below.
+    /// If the entry point returns, the image memory is subsequently deallocated.
+    ///
+    /// # Safety
+    /// The image is assumed to be trusted. This means:
+    /// * The PE file it was loaded from must have been a completely valid EFI application of the correct architecture.
+    /// * If the entry point returns, it must leave the system in a state that allows our stub to continue.
+    ///   In particular:
+    ///   * Only memory it either has allocated, or that belongs to the image, should have been altered.
+    ///   * Memory it has not allocated should not have been freed.
+    ///   * Boot services must not have been exited.
+    pub unsafe fn start(
+        self,
+        handle: Handle,
+        system_table: &SystemTable<Boot>,
+        load_options: &CStr16,
+    ) -> Status {
+        let mut loaded_image = system_table
+            .boot_services()
+            .open_protocol_exclusive::<LoadedImage>(handle)
+            .expect("Failed to open the LoadedImage protocol");
+
+        let (our_data, our_size) = loaded_image.info();
+        let our_load_options = loaded_image
+            .load_options_as_bytes()
+            .map(|options| options.as_ptr_range());
+
+        // It seems to be impossible to allocate custom image handles.
+        // Hence, we reuse our own for the kernel.
+        // The shim does the same thing.
+        unsafe {
+            loaded_image.set_image(
+                self.image.as_ptr() as *const c_void,
+                self.image.len().try_into().unwrap(),
+            );
+            loaded_image.set_load_options(
+                load_options.as_ptr() as *const u8,
+                u32::try_from(load_options.num_bytes()).unwrap(),
+            );
+        }
+
+        let status = (self.entry)(handle, unsafe { system_table.unsafe_clone() });
+
+        // If the kernel has exited boot services, it must not return any more, and has full control over the entire machine.
+        // If the kernel entry point returned, deallocate its image, and restore our loaded image handle.
+        // If it calls Exit(), that call returns directly to systemd-boot. This unfortunately causes a resource leak.
+        system_table
+            .boot_services()
+            .free_pages(self.image.as_ptr() as u64, bytes_to_pages(self.image.len()))
+            .expect("Double free attempted");
+
+        unsafe {
+            loaded_image.set_image(our_data, our_size);
+            match our_load_options {
+                Some(options) => loaded_image.set_load_options(
+                    options.start,
+                    options.end.offset_from(options.start).try_into().unwrap(),
+                ),
+                None => loaded_image.set_load_options(core::ptr::null(), 0),
+            }
+        }
+
+        status
+    }
+}

rust/stub/src/uefi_helpers.rs

@@ -56,15 +56,3 @@ pub fn booted_image_file(boot_services: &BootServices) -> Result<RegularFile> {
         .into_regular_file()
         .ok_or(Status::INVALID_PARAMETER)?)
 }
-
-/// Return the command line of the currently executing image.
-pub fn booted_image_cmdline(boot_services: &BootServices) -> Result<Vec<u8>> {
-    let loaded_image =
-        boot_services.open_protocol_exclusive::<LoadedImage>(boot_services.image_handle())?;
-
-    Ok(loaded_image
-        // If this fails, we have no load options and we return an empty string.
-        .load_options_as_bytes()
-        .map(|b| b.to_vec())
-        .unwrap_or_default())
-}

rust/tool/src/install.rs

@@ -157,8 +157,8 @@ impl Installer {
             append_initrd_secrets(initrd_secrets_script, &initrd_location)?;
         }
 
-        // The initrd doesn't need to be signed. The stub has its hash embedded and will refuse
+        // The initrd and kernel don't need to be signed.
-        // loading it when the hash mismatches.
+        // The stub has their hashes embedded and will refuse loading on 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
@@ -166,7 +166,9 @@ impl Installer {
         // 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)
+        // Do not sign the kernel.
+        // Boot loader specification could be used to make a signed kernel load an unprotected initrd.
+        install(&bootspec.kernel, &esp_gen_paths.kernel)
             .context("Failed to install kernel to ESP.")?;
 
         let lanzaboote_image = pe::lanzaboote_image(