diff --git a/CHANGELOG.md b/CHANGELOG.md index 937f676..8b91749 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,26 @@ All notable changes to bootintel-cli are documented here. Format follows [Keep a ## [Unreleased] +### Added +- **`bootintel verdict` now reports board info and the flash partition table** + when a capture contains a `bdinfo` or `mtdparts` dump. The partition rows carry + the `mask_flags` read-only marker, which is the operationally interesting + column: it says which partitions an operator at that prompt can rewrite. + + Both were dead code on the engine side and the port started by measuring that, + not by assuming it. Across all 31 public corpus logs they produced nothing, + while two of those logs contain a full bdinfo dump. The cause was a command + gate: the blocks were only parsed once the prompt regex had matched the line + where the command was typed, and `Boot-> bdinfo` is not U-Boot's default + prompt. They are now recognised by their own shape. + + The discriminator between board info and an environment variable is the + whitespace around the `=`: `printenv` emits `baudrate=115200`, `bdinfo` pads to + a column and emits `baudrate = 115200 bps`. + + `verdict --json` gained `bdinfo` and `mtd_device` under `uboot_shell`, and a + top-level `mtd_partitions` array, matching the engine's placement. + ## [0.10.0] — 2026-09-28 — the kernel's own hardening report `bootintel verdict` now answers for captures that never reach a U-Boot prompt. diff --git a/crates/cli/src/cmd/verdict.rs b/crates/cli/src/cmd/verdict.rs index 44aad1d..9fa64a2 100644 --- a/crates/cli/src/cmd/verdict.rs +++ b/crates/cli/src/cmd/verdict.rs @@ -180,6 +180,22 @@ fn json( shell.insert("env_used_bytes".into(), used.into()); shell.insert("env_total_bytes".into(), total.into()); } + // Same placement as the engine: board info and the flash device hang off the + // shell, partitions are top level and tagged with the parser that found them. + if !session.bdinfo.is_empty() { + shell.insert( + "bdinfo".into(), + session + .bdinfo + .iter() + .map(|(k, v)| (k.clone(), serde_json::Value::from(v.clone()))) + .collect::>() + .into(), + ); + } + if let Some(dev) = &session.mtd_device { + shell.insert("mtd_device".into(), dev.clone().into()); + } // Mirrors the engine's `boot_integrity` key names, and omits what was not // observed rather than emitting nulls: absence of a field means the capture // said nothing, which is different from a field saying "no". @@ -259,6 +275,13 @@ fn json( "uboot_env": session.env.iter() .map(|(k, v)| (k.clone(), serde_json::Value::from(v.clone()))) .collect::>(), + "mtd_partitions": session.mtd_partitions.iter().map(|p| serde_json::json!({ + "name": p.name, + "size": p.size, + "offset": p.offset, + "read_only": p.read_only, + "source": "uboot_mtdparts", + })).collect::>(), "boot_chain_verdict": verdicts.iter().map(|v| serde_json::json!({ "title": v.title, "state": v.state, @@ -320,6 +343,35 @@ pub(crate) fn write_text( session.env.len(), if session.env.len() == 1 { "" } else { "s" } )?; + if !session.bdinfo.is_empty() { + writeln!( + out, + " board info {} field{}", + session.bdinfo.len(), + if session.bdinfo.len() == 1 { "" } else { "s" } + )?; + } + if !session.mtd_partitions.is_empty() { + let dev = session.mtd_device.as_deref().unwrap_or("flash"); + writeln!( + out, + " {} partitions on {}", + session.mtd_partitions.len(), + sanitize_for_term(dev) + )?; + for p in &session.mtd_partitions { + // The read-only flag is the operationally interesting column: it is + // what says which partitions an operator at this prompt can rewrite. + writeln!( + out, + " {:<16} 0x{:08x} @ 0x{:08x}{}", + sanitize_for_term(&p.name), + p.size, + p.offset, + if p.read_only { " read-only" } else { "" } + )?; + } + } if let Some(check) = &integrity.image_check { let mechanism = if check == "fit_hash" { let algos = if integrity.image_hash_algorithms.is_empty() { diff --git a/crates/detectors/src/boot_chain.rs b/crates/detectors/src/boot_chain.rs index 47f42cb..a45209b 100644 --- a/crates/detectors/src/boot_chain.rs +++ b/crates/detectors/src/boot_chain.rs @@ -46,6 +46,12 @@ pub struct UbootSession { pub env: BTreeMap, pub env_used_bytes: Option, pub env_total_bytes: Option, + /// `bdinfo` output: what the board reports about itself. + pub bdinfo: BTreeMap, + /// The flash device named by an `mtdparts` dump. + pub mtd_device: Option, + /// Partitions from an `mtdparts` dump, in the order printed. + pub mtd_partitions: Vec, } use std::sync::LazyLock; @@ -239,6 +245,72 @@ pub fn parse_integrity(log: &str) -> BootIntegrity { bi } +// bdinfo prints aligned `name = value`. WHITESPACE BOTH SIDES OF THE `=` IS +// REQUIRED, and it is what separates a bdinfo line from an environment line: +// `printenv` emits `ethaddr=00:1F:...` with no spaces, bdinfo pads to a column +// and emits `ethaddr = 00:1F:...`. Without that, every environment dump +// containing baudrate or ethaddr would also be read as board info. +static RE_BDINFO: LazyLock = + LazyLock::new(|| Regex::new(r"^\s*([A-Za-z][\w /()\-]{0,31}?)\s+=\s+(\S.*?)\s*$").unwrap()); + +/// The allowlist is what makes bdinfo self-evidencing, which matters because the +/// command that produced it cannot be relied on: bootintel-20 prints a full dump +/// after `Boot-> bdinfo`, and `Boot->` is not U-Boot's default prompt, so a +/// command-gated parser read that dump as nothing at all. +/// +/// `start` and `size` are deliberately absent: too generic to stand alone. +/// bootintel-5 prints an MTD table as `mtd_part[0]:` / `name = KERNEL` / +/// `size = 0x180000`, and `size` in this list recorded that as board info. +const BDINFO_KEYS: &[&str] = &[ + "arch_number", + "boot_params", + "dram bank", + "flashstart", + "flashsize", + "flashoffset", + "baudrate", + "relocaddr", + "reloc off", + "ethaddr", + "ip_addr", + "fdt_blob", + "irq_sp", + "sp start", + "eth0name", + "memstart", + "memsize", + "eth1name", + "ethaddr1", + "current eth", + "fdt_addr", + "sp_start", + "reloc_off", + "dram_bank", +]; + +// `device nor0 , # parts = 4`. The bracketed chip identifier is optional +// and the space in `# parts` is real. The engine's first version of this pattern +// required `#parts` with no space and no brackets, so it never matched U-Boot's +// actual output: the partition lines parsed and the device name did not. No +// public corpus log contains an mtdparts dump, so nothing caught it until a +// fixture was written for the documented format. +static RE_MTD_DEV: LazyLock = LazyLock::new(|| { + Regex::new(r"(?i)^\s*device\s+(\S+)(?:\s+<[^>]*>)?\s*,\s*#\s*parts\s*=\s*(\d+)").unwrap() +}); +static RE_MTD_PART: LazyLock = LazyLock::new(|| { + Regex::new(r"(?i)^\s*\d+:\s*(\S+)\s+0x([0-9a-f]+)\s+0x([0-9a-f]+)\s+(\d)").unwrap() +}); + +/// One partition as `mtdparts` printed it at the prompt. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub struct MtdPartition { + pub name: String, + pub size: u64, + pub offset: u64, + /// The `mask_flags` column: 1 means the partition is marked read-only. + pub read_only: bool, +} + /// Truncate to `max` CHARACTERS, mirroring Python's `s[:max]`. /// /// `String::truncate` counts bytes and panics mid-codepoint, and a capture is @@ -326,6 +398,49 @@ pub fn parse_session(log: &str) -> UbootSession { continue; } + // bdinfo and mtdparts, recognised by their own shape rather than by + // having seen the command that produced them, for the reason in + // BDINFO_KEYS. Checked before the continuation logic so a bdinfo line + // is never appended to a buffered environment value. + if let Some(caps) = RE_BDINFO.captures(line) { + let key = caps[1].trim(); + if BDINFO_KEYS.contains(&key.to_ascii_lowercase().as_str()) { + s.bdinfo + .entry(key.to_string()) + .or_insert_with(|| caps[2].trim().to_string()); + note(&mut s, line); + continue; + } + } + if let Some(caps) = RE_MTD_DEV.captures(line) { + if s.mtd_device.is_none() { + s.mtd_device = Some(caps[1].to_string()); + } + note(&mut s, line); + continue; + } + if let Some(caps) = RE_MTD_PART.captures(line) { + // Hex without a `0x`, as U-Boot prints it. A width beyond u64 is not + // a partition table, so a failed parse drops the row rather than + // inventing a zero. + if let (Ok(size), Ok(offset)) = ( + u64::from_str_radix(&caps[2], 16), + u64::from_str_radix(&caps[3], 16), + ) { + let part = MtdPartition { + name: caps[1].to_string(), + size, + offset, + read_only: &caps[4] == "1", + }; + if !s.mtd_partitions.contains(&part) { + s.mtd_partitions.push(part); + } + note(&mut s, line); + continue; + } + } + // A long U-Boot value wraps in a terminal capture, so the continuation // has no `KEY=`. Treating it as a boundary discarded whole // environments. Append instead, bounded: a couple of wrapped lines is diff --git a/crates/detectors/tests/boot_chain.rs b/crates/detectors/tests/boot_chain.rs index 4b0b58d..e4fcb3d 100644 --- a/crates/detectors/tests/boot_chain.rs +++ b/crates/detectors/tests/boot_chain.rs @@ -149,6 +149,23 @@ fn render(name: &str, log: &str) -> Vec { "ignored_kernel_parameters", h.ignored_kernel_parameters.as_deref(), ); + for (key, value) in &session.bdinfo { + field(&mut out, " ", "bdinfo", &format!("{key}={value}")); + } + if let Some(dev) = &session.mtd_device { + field(&mut out, " ", "mtd_device", dev); + } + for part in &session.mtd_partitions { + field( + &mut out, + " ", + "mtd_part", + &format!( + "{} size=0x{:x} offset=0x{:x} ro={}", + part.name, part.size, part.offset, part.read_only + ), + ); + } for (key, value) in &session.env { field(&mut out, " ", "env", &format!("{key}={value}")); } @@ -461,3 +478,82 @@ fn a_check_with_no_captured_result_is_unknown_rather_than_passing() { .expect("a verdict"); assert_eq!(v.state, "unknown"); } + +/// bdinfo is recognised by its own shape, because the command that produced it +/// cannot be relied on: bootintel-20 prints a full dump after `Boot-> bdinfo`, +/// and `Boot->` is not U-Boot's default prompt. A command-gated parser read that +/// entire dump as nothing. +#[test] +fn a_bdinfo_dump_behind_a_vendor_prompt_is_still_read() { + let log = "Boot-> bdinfo\n\ + boot_params = 0x87D2EFB0\n\ + memstart = 0x80000000\n\ + flashsize = 0x01000000\n\ + ethaddr = 00:1F:45:F2:B7:3B\n"; + let s = boot_chain::parse_session(log); + assert_eq!( + s.bdinfo.get("boot_params").map(String::as_str), + Some("0x87D2EFB0") + ); + assert_eq!( + s.bdinfo.get("memstart").map(String::as_str), + Some("0x80000000") + ); + assert_eq!(s.bdinfo.len(), 4); + assert!(s.reached, "a bdinfo dump proves someone was at the prompt"); +} + +/// The discriminator between board info and an environment variable is the +/// whitespace around the `=`. Without it, every environment dump containing +/// baudrate or ethaddr would also be recorded as board info. +#[test] +fn an_environment_line_is_not_board_info() { + let log = "=> printenv\nbaudrate=115200\nethaddr=00:11:22:33:44:55\n\ + Environment size: 40/65532 bytes\n"; + let s = boot_chain::parse_session(log); + assert!( + s.bdinfo.is_empty(), + "read the environment as board info: {:?}", + s.bdinfo + ); + assert_eq!(s.env.len(), 2); +} + +/// `size` and `start` are too generic to be board-info keys on their own. +/// bootintel-5 prints an MTD table in a vendor format whose rows are exactly +/// `size = 0x180000`, and treating that as bdinfo was a live false positive. +#[test] +fn a_vendor_mtd_table_is_not_board_info() { + let log = "mtd_part[0]:\nname = KERNEL\nsize = 0x180000\noffset = 0x37000\n"; + let s = boot_chain::parse_session(log); + assert!(s.bdinfo.is_empty(), "{:?}", s.bdinfo); +} + +/// U-Boot prints `device nor0 , # parts = 4`: the bracketed chip id is +/// optional and the space in `# parts` is real. The engine's first pattern +/// required neither and so never matched, recording the partitions but not the +/// device they belong to. +#[test] +fn an_mtdparts_dump_is_parsed_including_its_device() { + let log = "=> mtdparts\n\n\ + device nor0 , # parts = 4\n \ + #: name size offset mask_flags\n \ + 0: u-boot 0x00020000 0x00000000 1\n \ + 1: kernel 0x00100000 0x00020000 0\n"; + let s = boot_chain::parse_session(log); + assert_eq!(s.mtd_device.as_deref(), Some("nor0")); + assert_eq!(s.mtd_partitions.len(), 2); + assert_eq!(s.mtd_partitions[0].name, "u-boot"); + assert_eq!(s.mtd_partitions[0].size, 0x20000); + assert!( + s.mtd_partitions[0].read_only, + "mask_flags 1 means read-only" + ); + assert!(!s.mtd_partitions[1].read_only); +} + +#[test] +fn a_device_line_without_the_bracketed_chip_id_also_parses() { + let s = boot_chain::parse_session("device nand0, #parts = 2\n"); + assert_eq!(s.mtd_device.as_deref(), Some("nand0")); +} diff --git a/crates/detectors/tests/fixtures/boot_chain/expect.txt b/crates/detectors/tests/fixtures/boot_chain/expect.txt index a48e7a2..38d370f 100644 --- a/crates/detectors/tests/fixtures/boot_chain/expect.txt +++ b/crates/detectors/tests/fixtures/boot_chain/expect.txt @@ -66,6 +66,51 @@ evidence Environment size: 412/65532 bytes detail The environment occupies 412 of 65532 bytes of writable storage, so `saveenv` can persist a change across reboots. remediation Build with a read-only or signed environment for production. +## fixture mtdparts.log fnv1a64=0d8c70196d235993 + reached true + evidence => mtdparts + env_bytes 30/65532 + mtd_device nor0 + mtd_part u-boot size=0x20000 offset=0x0 ro=true + mtd_part kernel size=0x100000 offset=0x20000 ro=false + mtd_part rootfs size=0x6c0000 offset=0x120000 ro=false + mtd_part art size=0x10000 offset=0x7f0000 ro=true + env bootcmd=bootm 0x9f020000 + verdict + title U-Boot shell reached + state confirmed + severity high + evidence => mtdparts + detail An operator interrupted autoboot and got a command prompt. Everything below was read from the device, not inferred from its boot output. + remediation Set bootdelay=-1 and build with CONFIG_AUTOBOOT_KEYED so the prompt needs a password. + verdict + title Autoboot delay + state unknown + severity info + evidence bootdelay absent + detail bootdelay is not set in the environment, so the built-in default applies and cannot be read from here. + remediation + verdict + title Boot command + state exposed + severity high + evidence bootcmd=bootm 0x9f020000 + detail bootcmd is readable and, with the prompt reachable, settable. Whoever holds the console decides what the device boots. + remediation Lock the environment (CONFIG_ENV_IS_NOWHERE or a signed env) and require a password at the prompt. + verdict + title Image verification + state unknown + severity info + evidence bootcmd=bootm 0x9f020000 + detail bootcmd boots an image without a visible verification step. That is not proof verification is absent: a FIT signature check can be implicit in the image. Confirm with the boot output of an actual `bootm`. + remediation + verdict + title Environment storage + state confirmed + severity medium + evidence Environment size: 30/65532 bytes + detail The environment occupies 30 of 65532 bytes of writable storage, so `saveenv` can persist a change across reboots. + remediation Build with a read-only or signed environment for production. ## fixture wrapped-env.log fnv1a64=ef1ee16cc0231e53 reached true evidence Environment size: 843/65532 bytes @@ -249,6 +294,15 @@ integrity image_check_result=passed integrity image_check_evidence=Verifying Checksum ... OK integrity image_check_failed=Bad Magic Number + bdinfo baudrate=115200 bps + bdinfo boot_params=0x87D2EFB0 + bdinfo ethaddr=00:1F:45:F2:B7:3B + bdinfo flashoffset=0x00000000 + bdinfo flashsize=0x01000000 + bdinfo flashstart=0xBF000000 + bdinfo ip_addr=192.168.1.20 + bdinfo memsize=0x08000000 + bdinfo memstart=0x80000000 env AP_FLAG=0 env LOAD_SENSOR_IMG=0 env MOSTRECENTKERNEL=1 diff --git a/crates/detectors/tests/fixtures/boot_chain/mtdparts.log b/crates/detectors/tests/fixtures/boot_chain/mtdparts.log new file mode 100644 index 0000000..39894de --- /dev/null +++ b/crates/detectors/tests/fixtures/boot_chain/mtdparts.log @@ -0,0 +1,13 @@ +=> mtdparts + +device nor0 , # parts = 4 + #: name size offset mask_flags + 0: u-boot 0x00020000 0x00000000 1 + 1: kernel 0x00100000 0x00020000 0 + 2: rootfs 0x006c0000 0x00120000 0 + 3: art 0x00010000 0x007f0000 1 + +=> printenv +bootcmd=bootm 0x9f020000 +Environment size: 30/65532 bytes +=>