From 081714cab9ef445f3a576be8f5e1d42be988b5ba Mon Sep 17 00:00:00 2001
From: Alois Wohlschlager <alois1@gmx-topmail.de>
Date: Mon, 23 Jan 2023 18:02:01 +0100
Subject: [PATCH] Pass the built-in cmdline to the kernel

Do not pass our own cmdline on to the kernel. It may have been set by a
malicious boot loader specification entry, and could instruct the
kernel to load an arbitrary unprotected initrd (or perform some other
fun stuff). Instead, always pass the command line built into the UKI,
which is properly authenticated.
---
 rust/stub/src/main.rs         | 24 +++++++++++++-----------
 rust/stub/src/pe_loader.rs    |  6 +++---
 rust/stub/src/uefi_helpers.rs | 12 ------------
 3 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/rust/stub/src/main.rs b/rust/stub/src/main.rs
index da26155..adf4c4a 100644
--- a/rust/stub/src/main.rs
+++ b/rust/stub/src/main.rs
@@ -25,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>;
@@ -67,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.
-fn extract_filename(file_data: &[u8], section: &str) -> Result<CString16> {
-    let filename = pe_section_as_string(file_data, section).ok_or(Status::INVALID_PARAMETER)?;
+/// Extract a string, stored as UTF-8, from a PE section.
+fn extract_string(file_data: &[u8], section: &str) -> Result<CString16> {
+    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.
@@ -92,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")?,
         })
     }
 }
@@ -164,16 +169,13 @@ fn main(handle: Handle, mut system_table: SystemTable<Boot>) -> Status {
         return Status::SECURITY_VIOLATION;
     }
 
-    let kernel_cmdline =
-        booted_image_cmdline(system_table.boot_services()).expect("Failed to fetch command line");
-
     let kernel =
         Image::load(system_table.boot_services(), &kernel_data).expect("Failed to load the kernel");
 
     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 = unsafe { kernel.start(handle, &system_table, &kernel_cmdline) };
+    let status = unsafe { kernel.start(handle, &system_table, &config.cmdline) };
 
     initrd_loader
         .uninstall(system_table.boot_services())
diff --git a/rust/stub/src/pe_loader.rs b/rust/stub/src/pe_loader.rs
index 9ce5305..b60faaa 100644
--- a/rust/stub/src/pe_loader.rs
+++ b/rust/stub/src/pe_loader.rs
@@ -9,7 +9,7 @@ use uefi::{
         boot::{AllocateType, MemoryType},
         Boot, SystemTable,
     },
-    Handle, Status,
+    CStr16, Handle, Status,
 };
 
 /// UEFI mandates 4 KiB pages.
@@ -124,7 +124,7 @@ impl Image {
         self,
         handle: Handle,
         system_table: &SystemTable<Boot>,
-        load_options: &[u8],
+        load_options: &CStr16,
     ) -> Status {
         let mut loaded_image = system_table
             .boot_services()
@@ -146,7 +146,7 @@ impl Image {
             );
             loaded_image.set_load_options(
                 load_options.as_ptr() as *const u8,
-                u32::try_from(load_options.len()).unwrap(),
+                u32::try_from(load_options.num_bytes()).unwrap(),
             );
         }
 
diff --git a/rust/stub/src/uefi_helpers.rs b/rust/stub/src/uefi_helpers.rs
index a2585f7..7bb374b 100644
--- a/rust/stub/src/uefi_helpers.rs
+++ b/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())
-}