diff --git a/crates/composefs-boot/Cargo.toml b/crates/composefs-boot/Cargo.toml index 455a6e8..f70829b 100644 --- a/crates/composefs-boot/Cargo.toml +++ b/crates/composefs-boot/Cargo.toml @@ -12,6 +12,7 @@ version.workspace = true [dependencies] anyhow = { version = "1.0.87", default-features = false } +bootc-kernel-cmdline = { version = "0.0.0", git = "https://github.com/bootc-dev/bootc", rev = "86f11577fdf69bfe7f416beba3e4ea117c26aca4", package = "bootc-kernel-cmdline" } composefs = { workspace = true } hex = { version = "0.4.0", default-features = false, features = ["std"] } regex-automata = { version = "0.4.4", default-features = false, features=["hybrid", "std", "syntax"] } diff --git a/crates/composefs-boot/src/bootloader.rs b/crates/composefs-boot/src/bootloader.rs index 3e8eaa4..be80a4e 100644 --- a/crates/composefs-boot/src/bootloader.rs +++ b/crates/composefs-boot/src/bootloader.rs @@ -9,7 +9,7 @@ use composefs::{ tree::{Directory, FileSystem, ImageError, Inode, LeafContent, RegularFile}, }; -use crate::cmdline::{make_cmdline_composefs, split_cmdline}; +use crate::cmdline::{Cmdline, Parameter}; /// Strips the key (if it matches) plus the following whitespace from a single line in a "Type #1 /// Boot Loader Specification Entry" file. @@ -65,30 +65,17 @@ impl BootLoaderEntryFile { /// /// arg can be something like "composefs=xyz" but it can also be something like "rw". In /// either case, if the argument already existed, it will be replaced. - pub fn add_cmdline(&mut self, arg: &str) { - let key = match arg.find('=') { - Some(pos) => &arg[..=pos], // include the '=' - None => arg, - }; - + pub fn add_cmdline(&mut self, arg: &Parameter) { // There are three possible paths in this function: // 1. options line with key= already in it (replace it) // 2. options line with no key= in it (append key=value) // 3. no options line (append the entire thing) for line in &mut self.lines { if let Some(cmdline) = strip_ble_key(line, "options") { - let segment = split_cmdline(cmdline).find(|s| s.starts_with(key)); - - if let Some(old) = segment { - // 1. Replace existing key - let range = substr_range(line, old).unwrap(); - line.replace_range(range, arg); - } else { - // 2. Append new argument - line.push(' '); - line.push_str(arg); - } + let mut cmdline = Cmdline::from(cmdline); + cmdline.add_or_modify(arg); + *line = format!("options {cmdline}"); return; } } @@ -99,9 +86,21 @@ impl BootLoaderEntryFile { /// Adjusts the kernel command-line arguments by adding a composefs= parameter (if appropriate) /// and adding additional arguments, as requested. - pub fn adjust_cmdline(&mut self, composefs: Option<&str>, insecure: bool, extra: &[&str]) { + pub fn adjust_cmdline( + &mut self, + composefs: Option<&T>, + insecure: bool, + extra: &[Parameter], + ) { if let Some(id) = composefs { - self.add_cmdline(&make_cmdline_composefs(id, insecure)); + let id = id.to_hex(); + let cfs_str = match insecure { + true => format!("composefs=?{id}"), + false => format!("composefs={id}"), + }; + + let param = Parameter::parse(&cfs_str).unwrap(); + self.add_cmdline(¶m); } for item in extra { @@ -400,8 +399,27 @@ pub fn get_boot_resources( #[cfg(test)] mod tests { + use composefs::fsverity::Sha256HashValue; + use zerocopy::FromZeros; + use super::*; + fn sha256() -> Sha256HashValue { + Sha256HashValue::new_zeroed() + } + + fn sha256str() -> String { + sha256().to_hex() + } + + fn param(input: &str) -> Parameter<'_> { + Parameter::parse(input).unwrap() + } + + fn params<'a>(input: &'a [&'a str]) -> Vec> { + input.iter().map(|p| param(*p)).collect() + } + #[test] fn test_bootloader_entry_file_new() { let content = "title Test Entry\nversion 1.0\nlinux /vmlinuz\ninitrd /initramfs.img\noptions quiet splash\n"; @@ -490,7 +508,7 @@ mod tests { #[test] fn test_add_cmdline_new_options_line() { let mut entry = BootLoaderEntryFile::new("title Test Entry\nlinux /vmlinuz\n"); - entry.add_cmdline("quiet"); + entry.add_cmdline(¶m("quiet")); assert_eq!(entry.lines.len(), 3); assert_eq!(entry.lines[2], "options quiet"); @@ -499,7 +517,7 @@ mod tests { #[test] fn test_add_cmdline_append_to_existing_options() { let mut entry = BootLoaderEntryFile::new("title Test Entry\noptions splash\n"); - entry.add_cmdline("quiet"); + entry.add_cmdline(¶m("quiet")); assert_eq!(entry.lines.len(), 2); assert_eq!(entry.lines[1], "options splash quiet"); @@ -509,7 +527,7 @@ mod tests { fn test_add_cmdline_replace_existing_key_value() { let mut entry = BootLoaderEntryFile::new("title Test Entry\noptions quiet splash root=/dev/sda1\n"); - entry.add_cmdline("root=/dev/sda2"); + entry.add_cmdline(¶m("root=/dev/sda2")); assert_eq!(entry.lines.len(), 2); assert_eq!(entry.lines[1], "options quiet splash root=/dev/sda2"); @@ -518,20 +536,20 @@ mod tests { #[test] fn test_add_cmdline_replace_existing_key_only() { let mut entry = BootLoaderEntryFile::new("title Test Entry\noptions quiet rw splash\n"); - entry.add_cmdline("rw"); // Same key, should replace itself (no-op in this case) + entry.add_cmdline(¶m("rw")); // Same key, should replace itself (no-op in this case) assert_eq!(entry.lines.len(), 2); assert_eq!(entry.lines[1], "options quiet rw splash"); // Test replacing with different key - entry.add_cmdline("ro"); + entry.add_cmdline(¶m("ro")); assert_eq!(entry.lines[1], "options quiet rw splash ro"); } #[test] fn test_add_cmdline_key_with_equals() { let mut entry = BootLoaderEntryFile::new("title Test Entry\noptions quiet\n"); - entry.add_cmdline("composefs=abc123"); + entry.add_cmdline(¶m("composefs=abc123")); assert_eq!(entry.lines.len(), 2); assert_eq!(entry.lines[1], "options quiet composefs=abc123"); @@ -541,7 +559,7 @@ mod tests { fn test_add_cmdline_replace_key_with_equals() { let mut entry = BootLoaderEntryFile::new("title Test Entry\noptions quiet composefs=old123\n"); - entry.add_cmdline("composefs=new456"); + entry.add_cmdline(¶m("composefs=new456")); assert_eq!(entry.lines.len(), 2); assert_eq!(entry.lines[1], "options quiet composefs=new456"); @@ -550,26 +568,33 @@ mod tests { #[test] fn test_adjust_cmdline_with_composefs() { let mut entry = BootLoaderEntryFile::new("title Test Entry\nlinux /vmlinuz\n"); - entry.adjust_cmdline(Some("abc123"), false, &["quiet", "splash"]); + entry.adjust_cmdline(Some(&sha256()), false, ¶ms(&["quiet", "splash"])); assert_eq!(entry.lines.len(), 3); - assert_eq!(entry.lines[2], "options composefs=abc123 quiet splash"); + assert_eq!( + entry.lines[2], + format!("options composefs={} quiet splash", sha256str()) + ); } #[test] fn test_adjust_cmdline_with_composefs_insecure() { let mut entry = BootLoaderEntryFile::new("title Test Entry\nlinux /vmlinuz\n"); - entry.adjust_cmdline(Some("abc123"), true, &[]); + entry.adjust_cmdline(Some(&sha256()), true, &[]); assert_eq!(entry.lines.len(), 3); // Assuming make_cmdline_composefs adds digest=off for insecure mode - assert!(entry.lines[2].contains("abc123")); + assert!(entry.lines[2].contains(&sha256str())); } #[test] fn test_adjust_cmdline_no_composefs() { let mut entry = BootLoaderEntryFile::new("title Test Entry\nlinux /vmlinuz\n"); - entry.adjust_cmdline(None, false, &["quiet", "splash"]); + entry.adjust_cmdline( + None::<&Sha256HashValue>, + false, + ¶ms(&["quiet", "splash"]), + ); assert_eq!(entry.lines.len(), 3); assert_eq!(entry.lines[2], "options quiet splash"); @@ -578,11 +603,11 @@ mod tests { #[test] fn test_adjust_cmdline_existing_options() { let mut entry = BootLoaderEntryFile::new("title Test Entry\noptions root=/dev/sda1\n"); - entry.adjust_cmdline(Some("abc123"), false, &["quiet"]); + entry.adjust_cmdline(Some(&sha256()), false, ¶ms(&["quiet"])); assert_eq!(entry.lines.len(), 2); assert!(entry.lines[1].contains("root=/dev/sda1")); - assert!(entry.lines[1].contains("abc123")); + assert!(entry.lines[1].contains(&sha256str())); assert!(entry.lines[1].contains("quiet")); } diff --git a/crates/composefs-boot/src/cmdline.rs b/crates/composefs-boot/src/cmdline.rs index 55be01b..0d8583d 100644 --- a/crates/composefs-boot/src/cmdline.rs +++ b/crates/composefs-boot/src/cmdline.rs @@ -1,47 +1,15 @@ -use anyhow::{Context, Result}; +use anyhow::Result; +pub(crate) use bootc_kernel_cmdline::utf8::{Cmdline, Parameter}; use composefs::fsverity::FsVerityHashValue; -/// Perform kernel command line splitting. -/// -/// The way this works in the kernel is to split on whitespace with an extremely simple quoting -/// mechanism: whitespace inside of double quotes is literal, but there is no escaping mechanism. -/// That means that having a literal double quote in the cmdline is effectively impossible. -pub(crate) fn split_cmdline(cmdline: &str) -> impl Iterator { - let mut in_quotes = false; - - cmdline.split(move |c: char| { - if c == '"' { - in_quotes = !in_quotes; - } - !in_quotes && c.is_ascii_whitespace() - }) -} - -/// Gets the value of an entry from the kernel cmdline. -/// -/// The prefix should be something like "composefs=". -/// -/// This iterates the entries in the provided cmdline string searching for an entry that starts -/// with the provided prefix. This will successfully handle quoting of other items in the cmdline, -/// but the value of the searched entry is returned verbatim (ie: not dequoted). -pub fn get_cmdline_value<'a>(cmdline: &'a str, prefix: &str) -> Option<&'a str> { - split_cmdline(cmdline).find_map(|item| item.strip_prefix(prefix)) -} - pub fn get_cmdline_composefs( cmdline: &str, ) -> Result<(ObjectID, bool)> { - let id = get_cmdline_value(cmdline, "composefs=").context("composefs= value not found")?; + let cmdline = Cmdline::from(cmdline); + let id = cmdline.require_value_of("composefs")?; if let Some(stripped) = id.strip_prefix('?') { Ok((ObjectID::from_hex(stripped)?, true)) } else { Ok((ObjectID::from_hex(id)?, false)) } } - -pub fn make_cmdline_composefs(id: &str, insecure: bool) -> String { - match insecure { - true => format!("composefs=?{id}"), - false => format!("composefs={id}"), - } -} diff --git a/crates/composefs-boot/src/write_boot.rs b/crates/composefs-boot/src/write_boot.rs index 394d231..c2e2ec6 100644 --- a/crates/composefs-boot/src/write_boot.rs +++ b/crates/composefs-boot/src/write_boot.rs @@ -9,7 +9,7 @@ use composefs::{fsverity::FsVerityHashValue, repository::Repository}; use crate::{ bootloader::{BootEntry, Type1Entry, Type2Entry}, - cmdline::get_cmdline_composefs, + cmdline::{get_cmdline_composefs, Parameter}, uki, }; @@ -29,8 +29,15 @@ pub fn write_t1_simple( bootdir.to_path_buf() }; + let cmdline_extra = cmdline_extra + .iter() + .map(|p| { + Parameter::parse(*p).ok_or(anyhow::anyhow!("could not parse command line parameter")) + }) + .collect::>>()?; + t1.entry - .adjust_cmdline(Some(&root_id.to_hex()), insecure, cmdline_extra); + .adjust_cmdline(Some(root_id), insecure, &cmdline_extra); // Write the content before we write the loader entry for (filename, file) in &t1.files {