diff --git a/src/core/wxc/Cargo.toml b/src/core/wxc/Cargo.toml index 8064062d2..629558df4 100644 --- a/src/core/wxc/Cargo.toml +++ b/src/core/wxc/Cargo.toml @@ -26,8 +26,12 @@ nanvix_runner = { workspace = true, optional = true } nanvix_binaries = { path = "../../backends/nanvix/binaries", optional = true } wslc_common = { workspace = true, optional = true } isolation_session_bindings = { workspace = true, optional = true } -# Shared PLM dep for the singleton env-var name and `wait_until_cleared` -# helper — single source of truth so the two crates can't drift. +# Shared PLM dep: `wpr_command` (safe wpr.exe resolver used for +# ctrl-c cleanup), `CTRL_HANDLER_DRAIN_TIMEOUT`, `wait_until_cleared`, +# and the `singleton::*` primitives — single source of truth so the +# two crates can't drift. The `--wxc-singleton-held-by-parent` handoff +# is a CLI arg on `plm.exe`, not an env var, because ShellExecuteExW +# with `runas` drops the caller's env block on the way through UAC. plm = { path = "../../host/plm" } tempfile.workspace = true diff --git a/src/core/wxc/src/audit.rs b/src/core/wxc/src/audit.rs index a08286620..2e93e85cf 100644 --- a/src/core/wxc/src/audit.rs +++ b/src/core/wxc/src/audit.rs @@ -50,8 +50,11 @@ pub fn plm_exe_path() -> Option { .and_then(|p| p.parent().map(|d| d.join("plm.exe"))) } -/// Run `plm.exe ` synchronously and route stdio -/// through to wxc-exec's console. Audit tracing is a best-effort +/// Run `plm.exe ` synchronously via +/// `run_plm_elevated`, which captures the child's stdout/stderr into +/// temp files (the UAC broker can't inherit our stdio) and replays +/// them only on non-zero exit or when `verbose` is set — the happy +/// path is deliberately silent. Audit tracing is a best-effort /// diagnostic: missing-binary / spawn / non-zero-exit conditions are /// logged and returned as `false` — this function never calls /// `process::exit` on its own. The caller (currently the `--audit` @@ -185,8 +188,7 @@ pub fn mark_audit_active() { /// applies the same hardening in its own resolver. pub fn cancel_active_audit_trace() { if AUDIT_ACTIVE.swap(false, Ordering::SeqCst) { - let wpr = resolve_system32_wpr(); - let _ = std::process::Command::new(&wpr) + let _ = plm::wpr_path::wpr_command() .arg("-cancel") .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) @@ -194,27 +196,6 @@ pub fn cancel_active_audit_trace() { } } -/// Resolve `\wpr.exe` via `GetSystemDirectoryW`. Reading -/// `%SystemRoot%` from the process env is unsafe because UAC inherits -/// env from the unelevated parent — a standard user could `setx -/// SystemRoot=C:\\Users\\Public\\evil` and plant `wpr.exe` for a later -/// admin run. `GetSystemDirectoryW` is kernel-published and not -/// env-spoofable. -fn resolve_system32_wpr() -> std::path::PathBuf { - use windows::Win32::System::SystemInformation::GetSystemDirectoryW; - let mut buf = vec![0u16; 260]; - // SAFETY: buf is initialized; we pass valid length and own the - // memory for the duration of the call. - let n = unsafe { GetSystemDirectoryW(Some(&mut buf)) }; - if n == 0 || (n as usize) > buf.len() { - return std::path::PathBuf::from("C:\\Windows\\System32\\wpr.exe"); - } - let dir = wxc_common::string_util::from_wide(&buf[..n as usize]); - let mut p = std::path::PathBuf::from(dir); - p.push("wpr.exe"); - p -} - /// Stack-owned guard: ensures the audit trace is cancelled on panic /// unwind and on normal function return. pub struct AuditTraceGuard; diff --git a/src/core/wxc/src/main.rs b/src/core/wxc/src/main.rs index 39829dfeb..23ac377af 100644 --- a/src/core/wxc/src/main.rs +++ b/src/core/wxc/src/main.rs @@ -343,9 +343,6 @@ fn config_file_path(cli: &Cli) -> Option { .map(std::path::PathBuf::from) } -/// Path to `plm.exe`, expected to sit next to `wxc-exec.exe` in the -/// same install directory. Returns `None` when the current exe path -/// can't be resolved. #[cfg(target_os = "windows")] use audit::{ cancel_active_audit_trace, mark_audit_active, release_audit_singleton, run_plm_command, diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index 9696e5aac..eedeab115 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -709,8 +709,8 @@ fn convert_wire_config( .capabilities .push("permissiveLearningMode".to_string()); logger.log("WARNING: 'learningMode' enabled - AppContainer restrictions will NOT be enforced (DEBUG BUILD ONLY)\n"); - eprintln!( - "[mxc] permissiveLearningMode injected via 'learningMode: true' - AppContainer restrictions are NOT enforced" + logger.log( + "[mxc] permissiveLearningMode injected via 'learningMode: true' - AppContainer restrictions are NOT enforced\n", ); } #[cfg(not(debug_assertions))] @@ -722,8 +722,8 @@ fn convert_wire_config( if let Some(caps) = ac.capabilities { #[cfg(debug_assertions)] if caps.iter().any(|c| c == "permissiveLearningMode") { - eprintln!( - "[mxc] permissiveLearningMode present in policy capabilities - AppContainer restrictions are NOT enforced" + logger.log( + "[mxc] permissiveLearningMode present in policy capabilities - AppContainer restrictions are NOT enforced\n", ); } policy.capabilities.extend(caps); diff --git a/src/host/plm/readme.md b/src/host/plm/readme.md index 1fcbf4e7e..d046a1874 100644 --- a/src/host/plm/readme.md +++ b/src/host/plm/readme.md @@ -2,28 +2,33 @@ `plm.exe` is the Windows-only trace driver for permissive learning mode. Long-form, it captures the access-denied events emitted by Windows' permissive sandbox layer, decodes them into structured findings, and merges those findings back into an MXC container config so the next enforcing run succeeds. -This PR introduces the **trace-lifecycle skeleton only**: WPR start/stop, the host-wide singleton mutex, the embedded `plm.wprp` materializer, and the `wxc-exec --audit` plumbing. Event parsing, capability extraction, filesystem/UI merging, and the adjusted-config writer arrive in subsequent PRs. +This PR introduces **filesystem extraction**: `EventID=14` records are walked from the captured `.etl`, file paths are extracted and merged into `filesystem.{readwritePaths,readonlyPaths}` on a copy of the input config. The host-wide singleton mutex, embedded `plm.wprp` materializer, and `wxc-exec --audit` plumbing landed in the previous PR. Capability extraction, UI relaxation, and the `Adjusted_*.json` writer arrive in subsequent PRs. -PLM is invoked automatically by [`wxc-exec --audit`](../../../README.md#audit-mode-permissive-learning-mode); the standalone CLI documented here is for capturing traces, interactive iteration, and (later) debugging the parser itself. +PLM is invoked automatically by [`wxc-exec --audit`](../../../README.md#audit-mode-permissive-learning-mode); the standalone CLI documented here is for capturing traces, interactive iteration, and debugging the parser itself. -## How it works (skeleton) +## How it works 1. **Capture** — `plm start` calls `wpr -start !AccessFailureProfile -filemode`, enabling the `Microsoft-Windows-Privacy-Auditing-PermissiveLearningMode` and `Microsoft-Windows-Kernel-General` ETW providers in a secure realtime collector. 2. **Run** — the operator runs the workload. The OS-side permissive sandbox logs `EventID=14` / `EventID=27` for every access that *would* have been denied. -3. **Stop** — `plm stop` calls `wpr -stop ` and records the captured trace location. -4. **Parse / Merge** — *(arrives in later PRs)* the `.etl` is walked with `EvtQuery` / `EvtRender` and findings are merged into a copy of the input config as `Adjusted_.json`. +3. **Stop** — `plm stop` calls `wpr -stop ` and walks the `.etl` with `EvtQuery` / `EvtRender`. +4. **Parse** — for each `EventID=14`, the parser pulls the file path / access mask. Capability ACE-blob decoding lands in a later PR; `EventID=27` UI relaxation lands in a later PR. +5. **Merge** — file paths are added to `filesystem.readwritePaths` / `filesystem.readonlyPaths` on a copy of the input config. The `Adjusted_*.json` writer arrives in the next PR; this PR only prints the per-event summary. ## Layout (this PR) -| File | Role | -|-----------------------|-------------------------------------------------------------------------------------| -| `src/main.rs` | `clap` dispatch for `plm start` / `plm stop` / `plm log` (`extract-caps` lands later) | -| `src/start.rs` | `wpr -cancel` (best-effort) + `wpr -start …!AccessFailureProfile -filemode` | -| `src/stop.rs` | `wpr -stop` (or skip with `--trace-file`); parse + merge arrive in later PRs | -| `src/log.rs` | Interactive mode: Enter to start, Enter to stop; preview arrives in later PRs | -| `src/coordination.rs` | Cross-process singleton named-mutex + bypass-env-var coordination for `plm log` | -| `src/wpr_path.rs` | Resolves `wpr.exe` to its absolute `%SystemRoot%\System32` path (PATH-spoof-safe) | -| `src/profile_gen.rs` | Inline WPR profile (`EMBEDDED_WPRP`) + run-time writer that drops `plm.wprp` next to `plm.exe` when missing | +| File | Role | +|-------------------------|-----------------------------------------------------------------------------------| +| `src/main.rs` | `clap` dispatch for `plm start` / `plm stop` / `plm log` (`extract-caps` lands later) | +| `src/start.rs` | `wpr -cancel` (best-effort) + `wpr -start …!AccessFailureProfile -filemode` | +| `src/stop.rs` | `wpr -stop` (or skip with `--trace-file`) + parse + FS merge | +| `src/log.rs` | Interactive mode: Enter to start, Enter to stop, then diff vs a blank config | +| `src/event_parser.rs` | `EvtQuery` / `EvtRender` walk; shared `ParseAccumulator` + per-event dispatcher | +| `src/access_failure.rs` | `EventID=14` decoder: file-path normalization, post-XPath filters | +| `src/access_event.rs` | `LearningModeAccessEvent` plain struct | +| `src/config.rs` | JSON load/mutate; path merge into `filesystem.{readwritePaths,readonlyPaths}` | +| `src/coordination.rs` | Cross-process singleton named-mutex + bypass-env-var coordination for `plm log` | +| `src/wpr_path.rs` | Resolves `wpr.exe` to its absolute `%SystemRoot%\System32` path (PATH-spoof-safe) | +| `src/profile_gen.rs` | Inline WPR profile (`EMBEDDED_WPRP`) + run-time writer that drops `plm.wprp` next to `plm.exe` when missing | ## CLI @@ -49,7 +54,7 @@ plm.exe stop [--config-path ] [--log-dir ] [--bin-path ] [--verbose-logging] ``` -`--config-path` / `--adjusted-config-path` are accepted today so `wxc-exec --audit` can pass them through; the merge that consumes them arrives in subsequent PRs. +`--config-path` drives an in-memory filesystem merge against the input config; the `Adjusted_*.json` writer that persists it arrives in the config-generation PR. `--adjusted-config-path` is accepted today so `wxc-exec --audit` can pass it through. ### `plm log` @@ -75,9 +80,9 @@ The WPR profile is embedded into `plm.exe` itself (see `src/profile_gen.rs`); on ## Limitations - **Windows-only.** Uses `wpr.exe` and Job-Object UI-limit semantics that have no portable equivalent. -- **No parse-and-merge yet.** `plm stop` writes the captured `.etl` to the log directory but does not yet produce an `Adjusted_*.json`. Later PRs add file-path extraction, capability extraction, UI-policy extraction, and the adjusted-config writer. +- **No adjusted-config writer yet.** `plm stop` merges discovered paths into an in-memory copy of the input config and prints the per-event summary, but does not yet write an `Adjusted_*.json`. Later PRs add the adjusted-config writer, capability extraction, and UI-policy extraction. ## See also -- [`docs/base-process-container/guide.md`](../../../docs/base-process-container/guide.md) — process-container backend overview +- [`docs/process-container/guide.md`](../../../docs/process-container/guide.md) — process-container backend overview - [README → Debugging → Audit Mode](../../../README.md#audit-mode-permissive-learning-mode) — `wxc-exec --audit` integration diff --git a/src/host/plm/src/access_event.rs b/src/host/plm/src/access_event.rs new file mode 100644 index 000000000..681ac8f96 --- /dev/null +++ b/src/host/plm/src/access_event.rs @@ -0,0 +1,19 @@ +//! Port of the `LearningModeAccessEvent` PowerShell class from +//! `stop_plm_logging.ps1`. One file-access record from a PLM WPR trace. + +use chrono::{DateTime, Utc}; + +#[derive(Debug, Clone)] +// Fields kept for parity with the PowerShell version even though the +// orchestrator currently only uses file_path and access_mask. +#[allow(dead_code)] +pub struct LearningModeAccessEvent { + pub time_created: DateTime, + pub process_id: u32, + pub thread_id: u32, + pub learning_mode: String, // Permissive/Enforcing + pub resource_type: String, // File/Directory + pub file_path: String, + pub app_path: String, + pub access_mask: u32, +} diff --git a/src/host/plm/src/access_failure.rs b/src/host/plm/src/access_failure.rs new file mode 100644 index 000000000..ffcdbddd8 --- /dev/null +++ b/src/host/plm/src/access_failure.rs @@ -0,0 +1,378 @@ +//! EventID=14 (access-failure) decode + consume. +//! +//! The Permissive-Learning-Mode provider emits one `EventID=14` per +//! file/capability access that *would* have been denied. This module +//! owns: +//! * the EventData property indices for that schema, +//! * file-path normalization (NT-object / verbatim / DOS-device +//! prefixes -> DOS form), +//! * the post-XPath filters (current-directory, drive-letter, +//! self-access, invalid filename chars), +//! * the per-event accumulator helper that pushes the resulting +//! access event into `ParseAccumulator`. +//! +//! ACE-blob → capability-name extraction lands in a later PR; this PR +//! only collects file paths and access masks. +//! +//! `ParseAccumulator` (in `event_parser`) owns the mutable state; +//! `consume_access_failure` is the only public entry point. + +use crate::event_parser::{ParseAccumulator, ParsedEvent}; + +// File path we treat as "no useful info" and skip. +const MOUNT_POINT_MANAGER: &str = "\\Device\\MountPointManager"; + +// EventData property indexes for EventID=14 (matches the PowerShell +// parser's index map). +const LEARNING_MODE_INDEX: usize = 0; +const RESOURCE_TYPE_INDEX: usize = 1; +pub(crate) const FILE_PATH_INDEX: usize = 2; +const APP_PATH_INDEX: usize = 3; +const ACCESS_MASK_INDEX: usize = 5; + +/// Per-event consume helper for `EventID=14`. Applies the post-XPath +/// filters and pushes a `LearningModeAccessEvent` into +/// `acc.valid_access_events` on success. +pub(crate) fn consume_access_failure(acc: &mut ParseAccumulator<'_>, mut ev: ParsedEvent) { + // Pull the file path. Absent paths typically mean capability-only + // resource accesses; the capability-extraction PR will use the + // DACL ACE blob for those, but this PR drops them. + let mut file_path = match ev.event_data.get_mut(FILE_PATH_INDEX) { + Some(s) if !s.is_empty() => std::mem::take(s), + _ => return, + }; + + if file_path.eq_ignore_ascii_case(MOUNT_POINT_MANAGER) { + return; + } + + normalize_file_path_in_place(&mut file_path); + if acc.is_skippable(&file_path) { + return; + } + + // Skip self-events: the app accessing its own binary. The app path + // is stored without a drive letter (HardDiskVolume form), so we + // compare against the file path minus its drive letter. `get(3..)` + // (rather than slicing) avoids a panic when the path contains non- + // ASCII bytes spanning indices 1-2 (e.g. `C:éx`). + let app_path = ev + .event_data + .get_mut(APP_PATH_INDEX) + .map(std::mem::take) + .unwrap_or_default(); + if !app_path.is_empty() { + if let Some(tail) = file_path.get(3..) { + if !tail.is_empty() && app_path.ends_with(tail) { + return; + } + } + } + + if !looks_like_valid_path(&file_path) { + return; + } + + let learning_mode = ev + .event_data + .get_mut(LEARNING_MODE_INDEX) + .map(std::mem::take) + .unwrap_or_default(); + let resource_type = ev + .event_data + .get_mut(RESOURCE_TYPE_INDEX) + .map(std::mem::take) + .unwrap_or_default(); + let access_mask = ev + .event_data + .get(ACCESS_MASK_INDEX) + .and_then(|s| parse_int_loose(s)) + .unwrap_or(0); + + if acc.verbose { + println!("{app_path}"); + println!("{file_path}"); + } + + trim_backslashes_in_place(&mut file_path); + + acc.valid_access_events + .push(crate::access_event::LearningModeAccessEvent { + time_created: ev.time_created, + process_id: ev.process_id, + thread_id: ev.thread_id, + learning_mode, + resource_type, + file_path, + app_path, + access_mask, + }); +} + +/// Strip Windows path-namespace prefixes (`\??\`, `\\?\`, `\\.\`) so +/// downstream filters that expect a DOS form (`C:\...`) see one. +/// +/// All three prefixes are exactly 4 bytes; their leading and trailing +/// bytes are both `\\`, and the middle pair is `??`, `\?`, or `\.`. +/// Encoded as a 2-byte tuple match for clarity. +pub(crate) fn normalize_file_path_in_place(s: &mut String) { + let lead = s.len() - s.trim_start().len(); + if lead > 0 { + s.drain(..lead); + } + let end_len = s.trim_end().len(); + s.truncate(end_len); + + if s.len() >= 4 { + let h = s.as_bytes(); + let prefix_match = h[0] == b'\\' + && h[3] == b'\\' + && matches!((h[1], h[2]), (b'?', b'?') | (b'\\', b'?') | (b'\\', b'.')); + if prefix_match { + s.drain(..4); + } + } +} + +/// Strip leading + trailing `\` from a `String` in place. Mirrors +/// `str::trim_matches('\\')` without the `.to_string()` round-trip the +/// hot path used to do. +pub(crate) fn trim_backslashes_in_place(s: &mut String) { + let lead = s.len() - s.trim_start_matches('\\').len(); + if lead > 0 { + s.drain(..lead); + } + let end_len = s.trim_end_matches('\\').len(); + s.truncate(end_len); +} + +/// `Test-Path -IsValid` equivalent: reject control bytes and Windows +/// wildcards which the OS itself refuses. +pub(crate) fn looks_like_valid_path(path: &str) -> bool { + const BAD: &[char] = &['<', '>', '"', '|', '?', '*']; + !path.chars().any(|c| (c as u32) < 32 || BAD.contains(&c)) +} + +/// Accept decimal or `0x`-prefixed hex. +pub(crate) fn parse_int_loose(s: &str) -> Option { + let t = s.trim(); + if let Some(rest) = t.strip_prefix("0x").or_else(|| t.strip_prefix("0X")) { + u32::from_str_radix(rest, 16).ok() + } else { + t.parse::().ok() + } +} + +// ---- Test-only helpers --------------------------------------------------- + +/// Allocating sibling of `normalize_file_path_in_place`, kept for tests +/// that want a `&str` -> `String` API. The hot path uses the in-place +/// variant. +#[cfg(test)] +pub(crate) fn normalize_file_path(p: &str) -> String { + let mut s = p.to_string(); + normalize_file_path_in_place(&mut s); + s +} + +/// Free-function sibling of `ParseAccumulator::is_skippable` exposed +/// for unit tests that don't want to build a whole accumulator. Logic +/// must stay in lock-step with the cached form on `ParseAccumulator`. +#[cfg(test)] +pub(crate) fn is_skippable( + file_path: &str, + current_directory: Option<&str>, + verbose: bool, +) -> bool { + if let Some(cwd) = current_directory { + // Defensive: refuse to treat a bare drive root ("C:" / "C:\\") as a + // CWD prefix — otherwise the `format!("{cwd}\\")` match below would + // swallow every event under that drive. Equality match still applies. + let cwd_trimmed = cwd.trim_end_matches('\\'); + let is_drive_root = cwd_trimmed.len() == 2 + && cwd_trimmed.chars().nth(1) == Some(':') + && cwd_trimmed + .chars() + .next() + .map(|c| c.is_ascii_alphabetic()) + .unwrap_or(false); + let normalized = file_path.trim_end_matches('\\'); + if normalized.eq_ignore_ascii_case(cwd_trimmed) + || (!is_drive_root + && normalized + .to_ascii_lowercase() + .starts_with(&format!("{}\\", cwd_trimmed.to_ascii_lowercase()))) + { + if verbose { + println!("Skipping current-directory event: {file_path}"); + } + return true; + } + } + if file_path.len() < 4 { + if verbose { + println!("Skipping too-short path event: {file_path}"); + } + return true; + } + let second = file_path.chars().nth(1); + if second != Some(':') { + if verbose { + println!("Skipping non-drive-letter path event: {file_path}"); + } + return true; + } + false +} + +/// Shared `EventID=14` XML fixture used by tests in this module and +/// by the mixed-stream integration test in `event_parser`. +#[cfg(test)] +pub(crate) fn make_event_xml(file_path: &str, mask_hex: &str) -> String { + format!( + r#" + + 14 + + + + + Permissive + File + {file_path} + App.exe + 0 + {mask_hex} + + "# + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::event_parser::parse_events_from_xml; + + #[test] + fn normalize_file_path_strips_nt_object_prefix() { + assert_eq!(normalize_file_path("\\??\\C:\\foo"), "C:\\foo"); + assert_eq!(normalize_file_path("\\??\\c:\\foo"), "c:\\foo"); + assert_eq!(normalize_file_path("C:\\foo"), "C:\\foo"); + } + + /// Verbatim (`\\?\C:\...`) and DOS-device (`\\.\C:\...`) prefixes + /// must be stripped before `is_skippable`'s drive-letter gate; + /// otherwise the kernel provider's natural rendering of those + /// forms drops every event. + #[test] + fn normalize_file_path_strips_verbatim_and_dos_device_prefixes() { + // Each `\` doubles in a Rust string literal; on-disk path is + // `\\?\C:\foo`. + assert_eq!(normalize_file_path("\\\\?\\C:\\foo"), "C:\\foo"); + assert_eq!(normalize_file_path("\\\\.\\C:\\foo"), "C:\\foo"); + assert_eq!(normalize_file_path("\\\\?\\c:\\foo"), "c:\\foo"); + } + + /// After the prefix strip, a normalized path with a drive letter + /// must survive `is_skippable`. Integration between + /// `normalize_file_path` and the drive-letter gate. + #[test] + fn verbatim_prefix_path_survives_is_skippable() { + let normalized = normalize_file_path("\\\\?\\C:\\Users\\test\\foo.txt"); + assert!(!is_skippable(&normalized, None, false)); + } + + #[test] + fn is_skippable_rejects_short_and_non_drive_letter() { + assert!(is_skippable("abc", None, false)); + assert!(is_skippable("\\\\server\\share", None, false)); + assert!(!is_skippable("C:\\foo", None, false)); + } + + #[test] + fn is_skippable_filters_current_directory() { + assert!(is_skippable( + "C:\\repo\\src\\main.rs", + Some("C:\\repo"), + false + )); + assert!(!is_skippable( + "C:\\not-repo\\src\\main.rs", + Some("C:\\repo"), + false + )); + } + + /// A CWD of bare `C:\` (drive root) must NOT swallow every event + /// on that drive. Only an explicit equality match against the + /// drive root is honored. + #[test] + fn is_skippable_does_not_treat_drive_root_cwd_as_prefix() { + assert!(!is_skippable( + "C:\\Windows\\System32\\foo.dll", + Some("C:\\"), + false + )); + assert!(!is_skippable( + "C:\\Windows\\System32\\foo.dll", + Some("C:"), + false + )); + assert!(is_skippable("C:\\", Some("C:\\"), false)); + } + + #[test] + fn looks_like_valid_path_rejects_control_and_wildcards() { + assert!(!looks_like_valid_path("C:\\f\x00oo")); + assert!(!looks_like_valid_path("C:\\foo*")); + assert!(!looks_like_valid_path("C:\\foo?")); + assert!(looks_like_valid_path("C:\\foo\\bar.txt")); + } + + #[test] + fn parse_events_from_xml_accumulates_access_events() { + let xmls = [ + make_event_xml("C:\\Users\\test\\foo.txt", "0x1"), + make_event_xml("C:\\Users\\test\\bar.txt", "0x2"), + ]; + let result = parse_events_from_xml(xmls.iter(), None, false); + assert_eq!(result.valid_access_events.len(), 2); + assert_eq!( + result.valid_access_events[0].file_path, + "C:\\Users\\test\\foo.txt" + ); + assert_eq!(result.valid_access_events[0].access_mask, 0x1); + assert_eq!(result.valid_access_events[1].access_mask, 0x2); + } + + /// When a single rendered event is malformed we must not abort + /// the whole trace — every subsequent valid event would silently + /// disappear, leaving PLM under-granting on the next adjust pass. + /// The accumulator's `consume` swallows per-event parse failures; + /// this test pins that. + #[test] + fn parse_events_from_xml_skips_malformed_and_continues() { + let valid_a = make_event_xml("C:\\Users\\test\\a.txt", "0x1"); + let valid_b = make_event_xml("C:\\Users\\test\\b.txt", "0x2"); + let xmls: Vec = vec![ + valid_a, + "not xml".to_string(), + "".to_string(), + valid_b, + ]; + let result = parse_events_from_xml(xmls.iter(), None, false); + assert_eq!( + result.valid_access_events.len(), + 2, + "malformed events should be skipped, valid ones still collected" + ); + assert_eq!( + result.valid_access_events[0].file_path, + "C:\\Users\\test\\a.txt" + ); + assert_eq!( + result.valid_access_events[1].file_path, + "C:\\Users\\test\\b.txt" + ); + } +} diff --git a/src/host/plm/src/config.rs b/src/host/plm/src/config.rs new file mode 100644 index 000000000..ff986fe4f --- /dev/null +++ b/src/host/plm/src/config.rs @@ -0,0 +1,463 @@ +//! Port of the config-update logic from `stop_plm_logging.ps1`. +//! +//! Reads an MXC container config (JSON) and merges discovered +//! file-access paths into it. Capability-merge and the Adjusted_*.json +//! writer arrive in the config-generation PR. + +use anyhow::{Context, Result}; +use serde_json::{json, Value}; +use std::collections::HashSet; +use std::path::Path; + +use crate::access_event::LearningModeAccessEvent; + +// Write Masks (mirror of Get-PlmAccessMasks in stop_plm_logging.ps1) +const FILE_WRITE_MASK: u32 = 0x2; +const FILE_APPEND_MASK: u32 = 0x4; +const WRITE_EXTENDED_ATTRIBUTE_WRITE_MASK: u32 = 0x10; +const DIRECTORY_DELETE_MASK: u32 = 0x40; +const WRITE_ATTRIBUTE_WRITE_MASK: u32 = 0x100; +const FILE_DELETE_MASK: u32 = 0x10000; +const FILE_WRITE_DAC: u32 = 0x40000; +const FILE_WRITE_OWNER: u32 = 0x80000; + +pub const WRITE_MASK: u32 = FILE_WRITE_MASK + | FILE_APPEND_MASK + | WRITE_EXTENDED_ATTRIBUTE_WRITE_MASK + | DIRECTORY_DELETE_MASK + | WRITE_ATTRIBUTE_WRITE_MASK + | FILE_DELETE_MASK + | FILE_WRITE_DAC + | FILE_WRITE_OWNER; + +// Read Masks +const READ_DATA_MASK: u32 = 0x1; +const READ_EXTENDED_ATTRIBUTE_MASK: u32 = 0x8; +const DIRECTORY_TRAVERSE_MASK: u32 = 0x20; +const READ_ATTRIBUTE_MASK: u32 = 0x80; +const READ_CONTROL_MASK: u32 = 0x20000; +const SYNCHRONIZE_MASK: u32 = 0x100000; + +pub const READ_MASK: u32 = READ_DATA_MASK + | READ_EXTENDED_ATTRIBUTE_MASK + | DIRECTORY_TRAVERSE_MASK + | READ_ATTRIBUTE_MASK + | READ_CONTROL_MASK + | SYNCHRONIZE_MASK; + +#[derive(Debug)] +pub struct AddedPaths { + pub readwrite: Vec, + pub readonly: Vec, +} + +pub fn load_config(path: &Path) -> Result { + let raw = std::fs::read_to_string(path) + .with_context(|| format!("failed to read {}", path.display()))?; + let v: Value = serde_json::from_str(&raw) + .with_context(|| format!("failed to parse JSON {}", path.display()))?; + Ok(v) +} + +/// Ensure `config.filesystem.{readwritePaths,readonlyPaths}` exist. +/// +/// Returns an error (rather than panicking) when the operator-supplied +/// config has the wrong JSON shape (root is not an object, or `filesystem` +/// exists but is not an object). +pub fn initialize_filesystem(config: &mut Value) -> Result<()> { + let obj = config + .as_object_mut() + .ok_or_else(|| anyhow::anyhow!("config root must be a JSON object"))?; + if !obj.contains_key("filesystem") { + obj.insert("filesystem".into(), json!({})); + } + let fs = obj + .get_mut("filesystem") + .and_then(|v| v.as_object_mut()) + .ok_or_else(|| anyhow::anyhow!("`filesystem` must be a JSON object"))?; + if !fs.contains_key("readwritePaths") { + fs.insert("readwritePaths".into(), json!([])); + } else if !fs["readwritePaths"].is_array() { + anyhow::bail!("`filesystem.readwritePaths` must be a JSON array"); + } + if !fs.contains_key("readonlyPaths") { + fs.insert("readonlyPaths".into(), json!([])); + } else if !fs["readonlyPaths"].is_array() { + anyhow::bail!("`filesystem.readonlyPaths` must be a JSON array"); + } + Ok(()) +} + +pub fn deny_file_set(config: &Value) -> HashSet { + let mut out = HashSet::new(); + if let Some(arr) = config + .get("filesystem") + .and_then(|fs| fs.get("deniedPaths")) + .and_then(|v| v.as_array()) + { + for v in arr { + if let Some(s) = v.as_str() { + out.insert(s.to_string()); + } + } + } + out +} + +/// Strip a Windows verbatim (`\\?\`) or device (`\\.\`) prefix from `path`. +/// Returns `None` for UNC verbatim (`\\?\UNC\…`) — those paths are +/// network shares and never represent local drive roots. +/// +/// Used by `is_drive_root` and `normalize_path` so the rest of the +/// comparison machinery only deals with plain drive-letter forms even +/// when ETW hands us the verbatim variant that an audited process +/// passed straight to NtCreateFile. +fn strip_verbatim_or_device_prefix(s: &str) -> Option<&str> { + // UNC verbatim: never a drive root, never legal for policy widening. + if let Some(head) = s.get(..8) { + if head.eq_ignore_ascii_case("\\\\?\\UNC\\") { + return None; + } + } + if let Some(head) = s.get(..4) { + if head == "\\\\?\\" || head == "\\\\.\\" { + return Some(&s[4..]); + } + // `\??\` is the NT-object-manager equivalent of `\\?\`. + // Strip it here so any call site that bypasses + // `normalize_file_path` (e.g. the self-event filter on a raw + // `file_path`) still produces a comparable form. + if head == "\\??\\" { + return Some(&s[4..]); + } + } + Some(s) +} + +/// Normalize a Windows path for comparison-only use (returned form is +/// lowercase ASCII, `\`-separated, trailing separators / dots / +/// spaces stripped from every component — mirrors +/// `RtlDosPathNameToNtPathName`). +/// +/// Returns `None` for UNC verbatim (`\\?\UNC\…`) or paths containing +/// `:` outside the drive-letter separator (alternate data streams, +/// which the kernel resolves to the parent object — must not bypass +/// deny matching). +/// +/// Not applied to strings stored in policy arrays — those keep their +/// original case for operator readability. +fn normalize_path(p: &str) -> Option { + // 1. Strip verbatim / device prefix (`\\?\`, `\\.\`, and the + // NT-object `\??\` prefix). UNC verbatim is rejected because + // we don't grant policy to network shares. The `\??\` prefix + // must be stripped here too (not only in `event_parser`), so + // events that bypass that layer don't leak the literal prefix + // into config storage. + let stripped = strip_verbatim_or_device_prefix(p)?; + + // 2. Collapse `/` → `\`, lowercase ASCII in a single pass. + let mut s = String::with_capacity(stripped.len()); + for c in stripped.chars() { + let c = if c == '/' { '\\' } else { c }; + s.push(c.to_ascii_lowercase()); + } + + // 3. Reject `:` outside the drive-letter separator (s[1]). + // ADS-on-directory (`C:\Secrets:hidden`) escapes deny matching + // otherwise because the byte after the matched prefix is `:`, + // not a separator. + for (i, b) in s.bytes().enumerate() { + if b == b':' && i != 1 { + return None; + } + } + + // 4. Trim trailing separators from the whole string, then per-component + // strip trailing dots and spaces. Leave the drive-letter component + // ("c:") untouched. + // + // `..` and `.` segments break + // the deny-prefix invariant — `C:\Windows\.\System32\evil` and + // `C:\dir\..\Secrets\token.dat` would normalize to forms that + // miss a deny entry on the canonical parent. Rather than + // canonicalize (which requires filesystem access we don't + // have here), reject any input containing a `.` or `..` + // pure-segment. Callers fall back to "no match" semantics, + // which for a deny rule is the safe failure mode (the policy + // won't widen on an event we can't prove safe). + let trimmed = s.trim_end_matches('\\'); + let mut parts: Vec = Vec::new(); + let mut in_leading_unc = true; + for (i, part) in trimmed.split('\\').enumerate() { + if i == 0 && part.len() == 2 && part.as_bytes()[1] == b':' { + parts.push(part.to_string()); + in_leading_unc = false; + continue; + } + // Leading empty segments (UNC `\\server\share\x` splits to + // `["", "", "server", ...]`) are preserved verbatim; deny + // matching treats UNC as pass-through. Once we see a + // non-empty segment, leave the leading-UNC mode and any + // future empty segment is an error (doubled backslash etc). + if part.is_empty() && in_leading_unc && i < 2 { + parts.push(String::new()); + continue; + } + in_leading_unc = false; + let stripped = part.trim_end_matches(['.', ' ']); + // Reject pure traversal segments and other empty segments + // (`..` or `.`, or empty after trim — these arise from + // malformed inputs or doubled backslashes in non-UNC paths). + if part == "." || part == ".." || stripped.is_empty() { + return None; + } + parts.push(stripped.to_string()); + } + Some(parts.join("\\")) +} + +/// Returns true iff `file_path_norm` is equal to, or strictly nested +/// under, any of the entries in `paths_norm`. +/// +/// **Both inputs must already be normalized via `normalize_path`.** The +/// hot path in `update_from_access_events` normalizes once per event and +/// once per shadow-vector seed; doing it again here would re-allocate +/// `2·(|rw|+|ro|+|deny|)` strings per event and dominate wall time on +/// long traces. Comparisons are pure byte-slice equality on already- +/// lowercased data. +/// +/// Complexity is O(N) over `paths_norm`. The hot loop short-circuits +/// the exact-match case via a parallel `HashSet` before calling here, +/// so the practical cost is bounded by the number of *prefix* matches. +/// For traces that grow `readwritePaths` to >1k entries this is the +/// dominant CPU; a follow-up could switch to a path-component trie, +/// but a naive sorted-vec binary search is wrong here (multiple +/// distinct prefixes can match a single event path, and the +/// lexicographically-max element ≤ ev is not guaranteed to be one of +/// them — see `C:\foo bar` vs `C:\foo\baz`). The early-bail on first +/// byte below removes most cross-drive-letter false candidates. +fn path_starts_with_any_norm>( + file_path_norm: &str, + paths_norm: impl IntoIterator, +) -> bool { + let fp_bytes = file_path_norm.as_bytes(); + let fp_first = fp_bytes.first().copied(); + for p in paths_norm { + let pn = p.as_ref(); + if pn.is_empty() { + continue; + } + let pn_bytes = pn.as_bytes(); + if pn_bytes.first().copied() != fp_first { + continue; + } + if file_path_norm == pn { + return true; + } + if fp_bytes.len() > pn_bytes.len() + && fp_bytes[pn_bytes.len()] == b'\\' + && file_path_norm.starts_with(pn) + { + return true; + } + } + false +} + +fn trim_trailing_separators(s: &str) -> &str { + s.trim_end_matches(['\\', '/']) +} + +/// True iff `path` denotes a drive root like `C:\` (or `C:` / `C:/` or +/// the verbatim/device variants `\\?\C:\` / `\\.\C:\`). +/// +/// We refuse to emit a bare drive root into `filesystem.readwritePaths` +/// because that would grant the entire volume. Accepting only the +/// bare `[A-Za-z]:` form would let `\\?\C:\` (the form ETW emits when +/// an audited process called `NtCreateFile` with a verbatim path) +/// bypass the guard. +fn is_drive_root(path: &str) -> bool { + let stripped = match strip_verbatim_or_device_prefix(path) { + Some(s) => s, + // UNC verbatim is not a drive root. + None => return false, + }; + let trimmed = trim_trailing_separators(stripped); + let bytes = trimmed.as_bytes(); + bytes.len() == 2 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':' +} + +fn json_array_strings(v: &Value) -> Vec { + v.as_array() + .map(|arr| { + arr.iter() + .filter_map(|e| e.as_str().map(|s| s.to_string())) + .collect() + }) + .unwrap_or_default() +} + +pub fn update_from_access_events( + config: &mut Value, + bin_path: &str, + events: &[LearningModeAccessEvent], + deny_set: &HashSet, + verbose: bool, +) -> Result { + let mut added_rw: Vec = Vec::new(); + let mut added_ro: Vec = Vec::new(); + let mut seen_rw: HashSet = HashSet::new(); + let mut seen_ro: HashSet = HashSet::new(); + + // Pre-normalize every comparison input exactly once (here, outside the + // per-event loop). The hot path then does pure byte-slice compares + // with no allocation. The JSON array clone is hoisted out of the + // loop and the per-call `to_ascii_lowercase()` is eliminated. For + // exact-match idempotence (the common + // case after a few hundred events) we also maintain a `HashSet` of + // already-covered normalized forms for O(1) short-circuit. + let bin_path_norm = normalize_path(bin_path); + let deny_norm: Vec = deny_set.iter().filter_map(|s| normalize_path(s)).collect(); + let deny_set_norm: HashSet<&str> = deny_norm.iter().map(|s| s.as_str()).collect(); + + let mut rw_existing_norm: Vec = + json_array_strings(&config["filesystem"]["readwritePaths"]) + .into_iter() + .filter_map(|s| normalize_path(&s)) + .collect(); + let mut rw_existing_set: HashSet = rw_existing_norm.iter().cloned().collect(); + let mut ro_existing_norm: Vec = + json_array_strings(&config["filesystem"]["readonlyPaths"]) + .into_iter() + .filter_map(|s| normalize_path(&s)) + .collect(); + let mut ro_existing_set: HashSet = ro_existing_norm.iter().cloned().collect(); + + for ev in events { + // Normalize this event's path once. Reject ADS-on-directory and + // UNC-verbatim forms (`normalize_path` returns None) so they + // can't bypass deny matching. + let ev_norm = match normalize_path(&ev.file_path) { + Some(n) => n, + None => { + if verbose { + println!( + "Skipping un-normalizable path (UNC verbatim or stream syntax): {}", + ev.file_path + ); + } + continue; + } + }; + + // Self-access filter: events whose path equals the audited + // application's binary (in any spelling — raw, verbatim, + // lower/upper case) are noise and skipped. The verbatim-prefixed + // variant matters because `stop.rs` canonicalises bin_path via + // `canonicalize()`, which on Windows returns the `\\?\C:\…` form + // while ETW reports the plain `C:\…` form; comparing only the + // raw strings let the binary path leak into the output + // config as a readonly entry. + let is_self_event = ev.file_path.eq_ignore_ascii_case(bin_path) + || bin_path_norm + .as_ref() + .is_some_and(|bp_norm| bp_norm == &ev_norm); + if is_self_event { + if verbose { + println!("File {} is the binary path, skipping event.", ev.file_path); + } + continue; + } + + // Deny check: exact-set first (O(1)), then prefix scan over the + // (typically very small) deny vector. + if deny_set_norm.contains(ev_norm.as_str()) + || path_starts_with_any_norm(&ev_norm, &deny_norm) + { + continue; + } + + // Already-covered short-circuit for readwrite policy. + if rw_existing_set.contains(&ev_norm) + || path_starts_with_any_norm(&ev_norm, &rw_existing_norm) + { + continue; + } + + // Process Write Requests + if (ev.access_mask & WRITE_MASK) != 0 { + // Emit a file-scope grant for the exact path the audited + // process wrote to. The policy schema accepts individual + // file entries in `filesystem.readwritePaths`, so there is + // no need to widen the grant to the containing directory + // (which would over-grant to unrelated siblings). + // + // Refuse to emit a bare drive root — that would grant the + // entire volume. Legitimate write events target specific + // files under a drive root, so a raw `C:\` event is either + // a metadata operation we don't need to authorize or a + // path we can't safely widen; either way, skip it. + if is_drive_root(&ev.file_path) { + if verbose { + println!("Skipping write event at bare drive root: {}", ev.file_path); + } + continue; + } + let arr = config["filesystem"]["readwritePaths"] + .as_array_mut() + .ok_or_else(|| { + anyhow::anyhow!("`filesystem.readwritePaths` must be a JSON array") + })?; + arr.push(Value::String(ev.file_path.clone())); + if rw_existing_set.insert(ev_norm.clone()) { + rw_existing_norm.push(ev_norm.clone()); + } + if seen_rw.insert(ev_norm) { + added_rw.push(ev.file_path.clone()); + } + continue; + } + + // Process Read Requests + if (ev.access_mask & READ_MASK) != 0 { + if ro_existing_set.contains(&ev_norm) + || path_starts_with_any_norm(&ev_norm, &ro_existing_norm) + { + continue; + } + let arr = config["filesystem"]["readonlyPaths"] + .as_array_mut() + .ok_or_else(|| { + anyhow::anyhow!("`filesystem.readonlyPaths` must be a JSON array") + })?; + arr.push(Value::String(ev.file_path.clone())); + if ro_existing_set.insert(ev_norm.clone()) { + ro_existing_norm.push(ev_norm.clone()); + } + if seen_ro.insert(ev_norm) { + added_ro.push(ev.file_path.clone()); + } + } + } + + Ok(AddedPaths { + readwrite: added_rw, + readonly: added_ro, + }) +} + +pub fn write_added_paths_summary(added: &AddedPaths) { + println!(); + if !added.readwrite.is_empty() { + println!("Added to readwritePaths ({}):", added.readwrite.len()); + for p in &added.readwrite { + println!(" + {p}"); + } + } + if !added.readonly.is_empty() { + println!("Added to readonlyPaths ({}):", added.readonly.len()); + for p in &added.readonly { + println!(" + {p}"); + } + } +} diff --git a/src/host/plm/src/event_parser.rs b/src/host/plm/src/event_parser.rs new file mode 100644 index 000000000..04f446e16 --- /dev/null +++ b/src/host/plm/src/event_parser.rs @@ -0,0 +1,499 @@ +//! Walks a sequence of WinEvent records produced by the permissive +//! learning-mode trace and returns the file-access events that survived +//! filtering. +//! +//! This PR introduces filesystem extraction only. Capability extraction +//! (`EventID=14` DACL ACE blobs) lands in a later PR; UI relaxation +//! (`EventID=27`) lands in a later PR. `requested_capabilities` is +//! exposed today as an always-empty placeholder so call-sites in +//! `stop`/`log` can stay stable. + +use anyhow::Result; +use chrono::{DateTime, TimeZone, Utc}; +use std::collections::HashSet; +#[cfg(target_os = "windows")] +use std::path::Path; + +#[cfg(target_os = "windows")] +use windows::core::PCWSTR; +#[cfg(target_os = "windows")] +use windows::Win32::Foundation::ERROR_NO_MORE_ITEMS; +#[cfg(target_os = "windows")] +use windows::Win32::System::EventLog::{ + EvtClose, EvtNext, EvtQuery, EvtQueryFilePath, EvtQueryForwardDirection, EvtRender, + EvtRenderEventXml, EVT_HANDLE, +}; + +use crate::access_event::LearningModeAccessEvent; + +/// RAII wrapper that calls `EvtClose` on drop. A panic or `?`-early +/// return inside the rendering loop no longer leaks kernel ETW handles. +#[cfg(target_os = "windows")] +struct EvtHandleOwned(EVT_HANDLE); + +#[cfg(target_os = "windows")] +impl Drop for EvtHandleOwned { + fn drop(&mut self) { + unsafe { + let _ = EvtClose(self.0); + } + } +} + +pub struct ParseResult { + pub valid_access_events: Vec, + /// Placeholder for the capability-extraction PR. Always empty in + /// this PR; the field exists today so `stop`/`log` call-sites stay + /// stable across the split. + pub requested_capabilities: HashSet, +} + +impl ParseResult { + /// True when the trace produced nothing mergeable into a config. + pub fn is_empty(&self) -> bool { + self.valid_access_events.is_empty() && self.requested_capabilities.is_empty() + } +} + +#[cfg(target_os = "windows")] +fn to_wide_z(s: &str) -> Vec { + s.encode_utf16().chain(std::iter::once(0)).collect() +} + +/// Stream every event matching the access-failure XPath query out of an +/// .etl file, invoking `on_xml` once per rendered event XML string. The +/// caller-supplied closure accumulates state; this keeps peak memory +/// bounded (a `Vec` buffer could run into multi-GB on hour-long +/// traces). +/// +/// `EvtNext` batch size is intentionally large (256) to reduce +/// user→kernel transitions on traces with tens of thousands of events. +/// End-of-stream is distinguished from real errors by matching +/// `ERROR_NO_MORE_ITEMS`; any other `EvtNext` / `EvtRender` failure is +/// propagated rather than silently dropped — silent failure would look +/// like a successful but short trace and cause PLM to under-grant on +/// the next run. +#[cfg(target_os = "windows")] +fn for_each_event_xml(trace_file: &Path, mut on_xml: F) -> Result<()> +where + F: FnMut(&str) -> Result<()>, +{ + let path_w = to_wide_z(&trace_file.to_string_lossy()); + let query_w = to_wide_z("*[System[EventID=14 or EventID=27]]"); + + let h_query = EvtHandleOwned(unsafe { + EvtQuery( + None, + PCWSTR(path_w.as_ptr()), + PCWSTR(query_w.as_ptr()), + EvtQueryFilePath.0 | EvtQueryForwardDirection.0, + ) + }?); + + // Reusable scratch buffer for `render_event_xml` so we don't + // allocate a fresh Vec per event. + let mut render_buf: Vec = Vec::new(); + let mut rendered_count: usize = 0; + const BATCH: usize = 256; + loop { + let mut events: [isize; BATCH] = [0isize; BATCH]; + let mut returned: u32 = 0; + let next_ok = unsafe { + EvtNext( + h_query.0, + &mut events, + u32::MAX, // INFINITE + 0, + &mut returned as *mut _, + ) + }; + if let Err(e) = &next_ok { + if e.code() == ERROR_NO_MORE_ITEMS.to_hresult() { + break; + } + return Err(anyhow::anyhow!( + "EvtNext failed mid-stream (rendered {} events so far): {e}", + rendered_count + )); + } + if returned == 0 { + break; + } + + // Wrap ALL returned slots into `EvtHandleOwned` up front so a + // mid-batch `?` propagation drops (and closes) the still-owned + // slots after the failing index. + let owned: Vec = events + .iter() + .take(returned as usize) + .map(|&slot| EvtHandleOwned(EVT_HANDLE(slot))) + .collect(); + for handle in &owned { + let xml = render_event_xml(handle.0, &mut render_buf).map_err(|e| { + anyhow::anyhow!( + "EvtRender failed at event {} of batch (rendered {} so far): {e}", + rendered_count, + rendered_count + ) + })?; + on_xml(&xml)?; + rendered_count += 1; + } + } + Ok(()) +} + +#[cfg(target_os = "windows")] +fn render_event_xml(event: EVT_HANDLE, buf: &mut Vec) -> Result { + use windows::Win32::Foundation::{GetLastError, ERROR_INSUFFICIENT_BUFFER}; + + // Keep `buf` at `len == 0` while `EvtRender` writes through the raw + // pointer using the explicit byte-size argument; only extend `len` + // to the returned u16 count on the SUCCESS path so callers reusing + // `render_buf` across events never observe uninitialized u16s. + // + // `clear()` runs BEFORE the reserve so that `Vec::reserve` — + // which guarantees `capacity ≥ len + additional`, not + // `capacity ≥ additional` — actually reaches the + // `INITIAL_GUESS_U16` target on the first call where `len` had + // been left non-zero by the previous event. + // + // `EvtRender` writes UTF-16, so the backing buffer is `Vec` + // to guarantee 2-byte alignment (`Vec` is only 1-byte-aligned + // and casting `.as_ptr()` to `*const u16` would be UB even on x86). + // Note: `EvtRender`'s `BufferSize` / `BufferUsed` parameters are + // BYTE counts, so multiply/divide by `size_of::()` at the + // Win32 boundary. + const INITIAL_GUESS_U16: usize = 4 * 1024; + buf.clear(); + if buf.capacity() < INITIAL_GUESS_U16 { + buf.reserve(INITIAL_GUESS_U16); + } + let cap_u16 = buf.capacity(); + let cap_bytes = cap_u16 * std::mem::size_of::(); + + let mut needed: u32 = 0; + let mut count: u32 = 0; + let first = unsafe { + EvtRender( + None, + event, + EvtRenderEventXml.0, + cap_bytes as u32, + Some(buf.as_mut_ptr() as *mut _), + &mut needed as *mut _, + &mut count as *mut _, + ) + }; + + if first.is_err() { + // ERROR_INSUFFICIENT_BUFFER means `needed` is now valid (in + // bytes); grow and retry once. Any other error is fatal. + let win_err = unsafe { GetLastError() }; + if win_err != ERROR_INSUFFICIENT_BUFFER { + return Err(anyhow::anyhow!( + "EvtRender failed (Win32 error {:?})", + win_err + )); + } + if needed == 0 { + return Err(anyhow::anyhow!("EvtRender returned zero size")); + } + let needed_u16 = (needed as usize).div_ceil(std::mem::size_of::()); + if buf.capacity() < needed_u16 { + // `Vec::reserve(additional)` measures from `len`, not + // `capacity` — since `buf` is empty (cleared above), + // `additional == needed_u16` gets us `capacity ≥ needed_u16`. + buf.reserve(needed_u16); + } + let new_cap_u16 = buf.capacity(); + let new_cap_bytes = new_cap_u16 * std::mem::size_of::(); + let second = unsafe { + EvtRender( + None, + event, + EvtRenderEventXml.0, + new_cap_bytes as u32, + Some(buf.as_mut_ptr() as *mut _), + &mut needed as *mut _, + &mut count as *mut _, + ) + }; + // Propagate any error AFTER ensuring `buf` is still at len=0 + // (no uninit u16s exposed to the reused-buffer caller path). + second?; + } + + // `needed` is bytes written including the terminating NUL. + let init_u16 = (needed as usize / std::mem::size_of::()).min(buf.capacity()); + unsafe { + buf.set_len(init_u16); + } + let trimmed = match buf.iter().position(|&c| c == 0) { + Some(n) => &buf[..n], + None => &buf[..], + }; + Ok(String::from_utf16_lossy(trimmed)) +} + +/// Decoded XML view of a single event's interesting fields. +pub(crate) struct ParsedEvent { + #[allow(dead_code)] + pub(crate) event_id: u32, + pub(crate) time_created: DateTime, + pub(crate) process_id: u32, + pub(crate) thread_id: u32, + /// EventData/Data values in document order. + pub(crate) event_data: Vec, +} + +pub(crate) fn parse_event_xml(xml: &str) -> Option { + let doc = roxmltree::Document::parse(xml).ok()?; + let root = doc.root_element(); + + let system = root.children().find(|n| n.has_tag_name("System"))?; + let event_id = system + .children() + .find(|n| n.has_tag_name("EventID")) + .and_then(|n| n.text()) + .and_then(|s| s.parse::().ok()) + .unwrap_or(0); + + let time_created = system + .children() + .find(|n| n.has_tag_name("TimeCreated")) + .and_then(|n| n.attribute("SystemTime")) + .and_then(|s| DateTime::parse_from_rfc3339(s).ok()) + .map(|dt| dt.with_timezone(&Utc)) + .unwrap_or_else(|| Utc.timestamp_opt(0, 0).unwrap()); + + let execution = system.children().find(|n| n.has_tag_name("Execution")); + let process_id = execution + .and_then(|n| n.attribute("ProcessID")) + .and_then(|s| s.parse::().ok()) + .unwrap_or(0); + let thread_id = execution + .and_then(|n| n.attribute("ThreadID")) + .and_then(|s| s.parse::().ok()) + .unwrap_or(0); + + let mut event_data = Vec::new(); + if let Some(ed) = root.children().find(|n| n.has_tag_name("EventData")) { + for child in ed.children().filter(|n| n.is_element()) { + let tag = child.tag_name().name(); + if tag == "Data" || tag == "ComplexData" { + event_data.push(child.text().unwrap_or("").to_string()); + } + } + } + + Some(ParsedEvent { + event_id, + time_created, + process_id, + thread_id, + event_data, + }) +} + +/// Mutable per-trace accumulator. Fields are `pub(crate)` so the +/// sibling event-type decoders can write into them directly without an +/// inflated method surface. +pub(crate) struct ParseAccumulator<'a> { + #[allow(dead_code)] + pub(crate) current_directory: Option<&'a str>, + /// Cached lowercase form of `current_directory` with trailing `\\` + /// trimmed (computed once at construction so the hot + /// `is_skippable` path doesn't allocate two `String`s per event). + /// `None` when `current_directory` is `None` or is a bare drive + /// root. + pub(crate) cwd_lc_trimmed: Option, + pub(crate) cwd_lc_prefix: Option, + pub(crate) verbose: bool, + pub(crate) valid_access_events: Vec, + pub(crate) requested_capabilities: HashSet, +} + +impl<'a> ParseAccumulator<'a> { + fn new(current_directory: Option<&'a str>, verbose: bool) -> Self { + let (cwd_lc_trimmed, cwd_lc_prefix) = match current_directory { + Some(cwd) => { + let trimmed = cwd.trim_end_matches('\\'); + let is_drive_root = trimmed.len() == 2 + && trimmed.chars().nth(1) == Some(':') + && trimmed + .chars() + .next() + .map(|c| c.is_ascii_alphabetic()) + .unwrap_or(false); + let lc = trimmed.to_ascii_lowercase(); + let prefix = if is_drive_root { + None + } else { + Some(format!("{lc}\\")) + }; + (Some(lc), prefix) + } + None => (None, None), + }; + Self { + current_directory, + cwd_lc_trimmed, + cwd_lc_prefix, + verbose, + valid_access_events: Vec::new(), + requested_capabilities: HashSet::new(), + } + } + + /// Hot-path CWD / drive-letter filter for access events. Uses + /// precomputed lowercase forms of `current_directory` to avoid two + /// `String` allocs per event. + pub(crate) fn is_skippable(&self, file_path: &str) -> bool { + if let (Some(cwd_lc), prefix_opt) = (&self.cwd_lc_trimmed, &self.cwd_lc_prefix) { + let normalized = file_path.trim_end_matches('\\'); + let nb = normalized.as_bytes(); + let cwd_b = cwd_lc.as_bytes(); + let cwd_eq = nb.len() == cwd_b.len() + && nb.iter().zip(cwd_b).all(|(a, b)| a.eq_ignore_ascii_case(b)); + let prefix_match = prefix_opt + .as_deref() + .map(|p| { + let pb = p.as_bytes(); + nb.len() >= pb.len() + && nb[..pb.len()] + .iter() + .zip(pb) + .all(|(a, b)| a.eq_ignore_ascii_case(b)) + }) + .unwrap_or(false); + if cwd_eq || prefix_match { + if self.verbose { + println!("Skipping current-directory event: {file_path}"); + } + return true; + } + } + if file_path.len() < 4 { + if self.verbose { + println!("Skipping too-short path event: {file_path}"); + } + return true; + } + let second = file_path.chars().nth(1); + if second != Some(':') { + if self.verbose { + println!("Skipping non-drive-letter path event: {file_path}"); + } + return true; + } + false + } + + /// Per-event entry point. Decodes the XML, dispatches by event id, + /// and silently swallows malformed records (so a bad event mid-trace + /// doesn't abort the rest). EventID=27 (UI violation) is recognized + /// by the XPath filter today but contributes no relaxation until the + /// UI-policy PR. + fn consume(&mut self, xml: &str) { + let Some(ev) = parse_event_xml(xml) else { + return; + }; + match ev.event_id { + 27 => { + // UI-violation dispatch arrives in a later PR. + } + _ => crate::access_failure::consume_access_failure(self, ev), + } + } + + fn into_result(self) -> ParseResult { + ParseResult { + valid_access_events: self.valid_access_events, + requested_capabilities: self.requested_capabilities, + } + } +} + +#[cfg(target_os = "windows")] +pub fn parse_events( + trace_file: &Path, + current_directory: Option<&str>, + verbose: bool, +) -> Result { + let mut acc = ParseAccumulator::new(current_directory, verbose); + for_each_event_xml(trace_file, |xml| { + acc.consume(xml); + Ok(()) + })?; + Ok(acc.into_result()) +} + +/// Fixture-test seam: drive the same per-event accumulator +/// `parse_events` uses, but pull XML strings from an iterator rather +/// than a live ETW session. +pub fn parse_events_from_xml( + xmls: I, + current_directory: Option<&str>, + verbose: bool, +) -> ParseResult +where + I: IntoIterator, + S: AsRef, +{ + let mut acc = ParseAccumulator::new(current_directory, verbose); + for xml in xmls { + acc.consume(xml.as_ref()); + } + acc.into_result() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::access_failure::{make_event_xml, FILE_PATH_INDEX}; + + const ACCESS_MASK_INDEX: usize = 5; + + #[test] + fn parse_event_xml_extracts_event_id_and_data() { + let xml = r#" + + 14 + + + + + Permissive + File + C:\Users\test\foo.txt + App.exe + 0 + 0x1 + + "#; + let ev = parse_event_xml(xml).expect("xml should parse"); + assert_eq!(ev.event_id, 14); + assert_eq!(ev.process_id, 111); + assert_eq!(ev.thread_id, 222); + assert_eq!(ev.event_data.len(), 6); + assert_eq!(ev.event_data[FILE_PATH_INDEX], "C:\\Users\\test\\foo.txt"); + assert_eq!(ev.event_data[ACCESS_MASK_INDEX], "0x1"); + } + + #[test] + fn parse_event_xml_returns_none_for_malformed() { + assert!(parse_event_xml("not xml").is_none()); + assert!(parse_event_xml("").is_none()); + } + + #[test] + fn parse_events_from_xml_drives_access_failure_dispatch() { + // Single fs-only event; ensure the dispatcher runs and the + // event is collected. + let xml = make_event_xml("C:\\app\\foo.txt", "0x1"); + let result = parse_events_from_xml(vec![xml], None, false); + assert_eq!(result.valid_access_events.len(), 1); + assert_eq!(result.valid_access_events[0].file_path, "C:\\app\\foo.txt"); + } +} diff --git a/src/host/plm/src/lib.rs b/src/host/plm/src/lib.rs index 232085b2f..ae0371747 100644 --- a/src/host/plm/src/lib.rs +++ b/src/host/plm/src/lib.rs @@ -5,7 +5,11 @@ //! Pure-data modules compile cross-platform; Windows-only items are //! gated per-module. The `plm` binary in `main.rs` is Windows-only. +pub mod access_event; +pub mod access_failure; +pub mod config; pub mod coordination; +pub mod event_parser; pub mod profile_gen; #[cfg(target_os = "windows")] diff --git a/src/host/plm/src/log.rs b/src/host/plm/src/log.rs index d8ab38d18..a4fe10374 100644 --- a/src/host/plm/src/log.rs +++ b/src/host/plm/src/log.rs @@ -10,12 +10,17 @@ use anyhow::{Context, Result}; use chrono::Local; +use serde_json::{json, Value}; use std::io::{self, BufRead, Write}; use std::path::{Path, PathBuf}; +use crate::config::{ + deny_file_set, initialize_filesystem, update_from_access_events, write_added_paths_summary, +}; use crate::coordination::PLM_LOG_START_IN_FLIGHT; +use crate::event_parser::parse_events; use crate::start; -use crate::wpr_path::wpr_command; +use crate::stop::{stop_plm_trace_with, WprExeStopper}; use std::sync::atomic::Ordering; fn prompt_enter(message: &str) -> Result<()> { @@ -30,23 +35,6 @@ fn prompt_enter(message: &str) -> Result<()> { Ok(()) } -fn stop_wpr_trace(trace_file: &Path) -> Result<()> { - // Capture stdio rather than inheriting so `wpr -stop`'s progress - // bar (`100% [>>>>>>>>>]`) and other chatter don't leak into any - // wrapping tool's stdout. On non-zero exit we replay the captured - // streams via the shared `replay_wpr_output` helper so operators - // can still see wpr's own diagnostic. - let output = wpr_command() - .args(["-stop", &trace_file.to_string_lossy()]) - .output() - .context("failed to spawn wpr -stop")?; - if !output.status.success() { - crate::start::replay_wpr_output("stop", &output); - anyhow::bail!("wpr -stop exited with {}", output.status); - } - Ok(()) -} - pub fn run( wprp_path: &Path, verbose: bool, @@ -75,14 +63,49 @@ pub fn run( // parallel `plm log` invocations from colliding on the same .etl. let stamp = Local::now().format("%Y-%m-%d_%H%M%S%.3f").to_string(); let trace_file: PathBuf = std::env::temp_dir().join(format!("plm_log_{stamp}.etl")); - stop_wpr_trace(&trace_file)?; + stop_plm_trace_with(&mut WprExeStopper, &trace_file)?; // Kernel session is torn down; safe to clear the active flag so // any subsequent Ctrl+C doesn't issue a stale `wpr -cancel`. on_trace_stopped(); - println!("Trace captured at {}.", trace_file.display()); if verbose { - println!("verbose logging requested."); + println!("Beginning event parsing, this may take several minutes"); } + + let cwd = std::env::current_dir() + .ok() + .map(|p| p.to_string_lossy().trim_end_matches('\\').to_string()); + let parse = parse_events(&trace_file, cwd.as_deref(), verbose); + + // Clean up the temp .etl regardless of parse outcome. + let _ = std::fs::remove_file(&trace_file); + + let parse = parse?; + + // Synthesize a blank config and run the FS merge to preview what a + // policy authored from scratch would receive. Capability and UI + // merging arrive in later PRs. + let mut blank: Value = json!({}); + initialize_filesystem(&mut blank)?; + let deny = deny_file_set(&blank); + + // For a blank config there is no app binary to skip -- pass a path + // that will never match a real event's file path. + let bin_path = String::from("\\\\plm-blank-config-bin-sentinel"); + + let added = update_from_access_events( + &mut blank, + &bin_path, + &parse.valid_access_events, + &deny, + verbose, + )?; + + write_added_paths_summary(&added); + + println!(); + println!("Blank config after merge:"); + println!("{}", serde_json::to_string_pretty(&blank)?); + Ok(()) } diff --git a/src/host/plm/src/main.rs b/src/host/plm/src/main.rs index 3b950bea1..31b67d9cf 100644 --- a/src/host/plm/src/main.rs +++ b/src/host/plm/src/main.rs @@ -245,7 +245,8 @@ enum Cmd { #[arg(long)] log_dir: Option, /// Path treated as the application binary's location. Defaults - /// to the directory containing the plm executable. + /// to the directory containing the plm executable. Used as the + /// self-access filter root in the adjusted config. #[arg(long)] bin_path: Option, /// Path to the MXC container config (JSON) to update. @@ -384,7 +385,8 @@ fn main() -> Result<()> { // env-spoofable) plus the OS TrustedInstaller ACL on that // directory as the trust boundary; see `wpr_path` module docs for // why we do not run WinVerifyTrust on the resolved binary. - plm::wpr_path::verify_wpr_signed().map_err(|e| anyhow::anyhow!("wpr.exe check failed: {e}"))?; + plm::wpr_path::verify_wpr_present() + .map_err(|e| anyhow::anyhow!("wpr.exe check failed: {e}"))?; // Install the Ctrl+C handler unconditionally so signals during any // subcommand (in particular interactive `log`) tear down the WPR diff --git a/src/host/plm/src/stop.rs b/src/host/plm/src/stop.rs index 0f01d02c1..3a698e4eb 100644 --- a/src/host/plm/src/stop.rs +++ b/src/host/plm/src/stop.rs @@ -9,6 +9,11 @@ use chrono::Local; use std::path::{Path, PathBuf}; use std::process::ExitStatus; +use crate::config::{ + deny_file_set, initialize_filesystem, load_config, update_from_access_events, + write_added_paths_summary, +}; +use crate::event_parser::parse_events; use crate::wpr_path::wpr_command; pub struct StopOptions { @@ -34,20 +39,12 @@ pub struct WprExeStopper; impl WprStopper for WprExeStopper { fn stop(&mut self, trace_file: &Path) -> Result { - // Capture stdio rather than inheriting so a successful `wpr - // -stop` doesn't leak wpr chatter into any wrapping tool (e.g. - // `wxc-exec --audit`). On non-zero exit we replay the captured - // streams so operators can still see wpr's own diagnostic. let mut cmd = wpr_command(); let resolved = cmd.get_program().to_string_lossy().into_owned(); - let output = cmd - .args(["-stop", &trace_file.to_string_lossy()]) - .output() - .map_err(|e| anyhow::anyhow!("failed to spawn wpr -stop ({resolved}): {e}"))?; - if !output.status.success() { - crate::start::replay_wpr_output("stop", &output); - } - Ok(output.status) + cmd.arg("-stop") + .arg(trace_file) + .status() + .map_err(|e| anyhow::anyhow!("failed to spawn wpr -stop ({resolved}): {e}")) } } @@ -65,9 +62,10 @@ fn stop_plm_trace(trace_file: &Path) -> Result<()> { } /// Resolve `--bin-path` (or fall back to the calling exe directory) -/// to its canonical form. Exposed even though the self-access filter -/// consumer isn't wired here, so the canonicalize fallback chain is -/// pinned by tests. +/// to its canonical form. Consumed by `update_from_access_events` as +/// the self-access filter: events referencing this path are dropped +/// from the adjusted config so the container never grants itself +/// broad access to its own binary directory. /// /// Fallback chain: /// 1. `canonicalize(opt.bin_path)` if `Some` @@ -96,20 +94,25 @@ pub fn resolve_bin_path(opt: Option<&Path>, exe_dir: &Path) -> (PathBuf, Option< } pub fn run(opts: StopOptions, exe_dir: &Path) -> Result<()> { - // $LogDir defaults to "\logs\". The sub-second - // component makes parallel PLM runs finishing in the same second - // land in distinct directories. + // $LogDir defaults to "\logs\_pid". + // Including PID + sub-second component avoids collisions when + // parallel PLM tasks finish in the same second. let log_dir = opts.log_dir.unwrap_or_else(|| { - let stamp = Local::now().format("%Y-%m-%d_%H%M%S%.3f").to_string(); + let stamp = format!( + "{}_pid{}", + Local::now().format("%Y-%m-%d_%H%M%S%.3f"), + std::process::id() + ); exe_dir.join("logs").join(stamp) }); std::fs::create_dir_all(&log_dir) .with_context(|| format!("failed to create log dir {}", log_dir.display()))?; - // Resolve bin_path so the operator-facing warning path is - // exercised and the canonical form is on disk for downstream - // consumers, even though the self-access filter isn't wired here. - let (_bin_path, warning) = resolve_bin_path(opts.bin_path.as_deref(), exe_dir); + // Resolve bin_path to its canonical form so the self-access filter + // in `config::update_from_access_events` can compare it against the + // verbatim-prefixed paths ETW emits. The fallback chain is in + // `resolve_bin_path`. + let (bin_path, warning) = resolve_bin_path(opts.bin_path.as_deref(), exe_dir); if let Some(w) = warning { eprintln!("[plm] warning: {w}"); } @@ -127,20 +130,77 @@ pub fn run(opts: StopOptions, exe_dir: &Path) -> Result<()> { p }; - println!("Trace captured at {}.", trace_file.display()); + if opts.verbose { + println!("Beginning event parsing, this may take several minutes"); + } - // `config_path` / `adjusted_config_path` are accepted today so the - // wxc-exec --audit harness can pass them through without breaking - // for downstream consumers. - if let Some(p) = opts.config_path.as_ref() { - let _ = p; + // Current directory at parse time -- events under this path are + // treated as test scaffolding noise and skipped. + let cwd = std::env::current_dir() + .ok() + .map(|p| p.to_string_lossy().trim_end_matches('\\').to_string()); + + let parse = parse_events(&trace_file, cwd.as_deref(), opts.verbose)?; + + let config_path = match opts.config_path.as_ref() { + Some(p) => p, + None => return Ok(()), + }; + + // Load the source config into memory FIRST, before any disk + // side effect touches the log directory. If the source is + // unreadable or malformed we want to bail before we've + // produced a half-populated log_dir (bare trace.etl + no + // config, no adjusted). + let base_config = load_config(config_path)?; + + // Copy the original config alongside the trace unconditionally + // so operators always have a snapshot of the exact input that + // produced this run's `trace.etl`, even when the parse yielded + // nothing mergeable. The copy MUST land on disk before we + // attempt any edit-and-save cycle below: it's the operator's + // only record of the pre-edit state, and losing it turns an + // Adjusted_*.json into an un-auditable delta. + let leaf = config_path + .file_name() + .map(|s| s.to_string_lossy().into_owned()) + .unwrap_or_else(|| "config.json".into()); + let dest_config = log_dir.join(&leaf); + std::fs::copy(config_path, &dest_config) + .with_context(|| format!("failed to copy {}", config_path.display()))?; + + if parse.is_empty() { + // Nothing mergeable -- skip producing an Adjusted_*.json (which + // would be byte-identical to the input and confuse the harness's + // diff-based pass/fail signal). + return Ok(()); } + + // Edit the pre-loaded copy of the config in memory rather than + // re-reading `dest_config` — this avoids a read-after-write on + // Windows where an AV filter can occasionally serve a stale or + // empty buffer for a file that `std::fs::copy` just wrote. + let mut config = base_config; + initialize_filesystem(&mut config)?; + let deny = deny_file_set(&config); + + let bin_path_s = bin_path.to_string_lossy().into_owned(); + let added = update_from_access_events( + &mut config, + &bin_path_s, + &parse.valid_access_events, + &deny, + opts.verbose, + )?; + + write_added_paths_summary(&added); + + // `adjusted_config_path` is accepted today so the wxc-exec --audit + // harness can pass it through; the Adjusted_*.json writer arrives + // in the next PR (config-generation). if let Some(p) = opts.adjusted_config_path.as_ref() { let _ = p; } - if opts.verbose { - println!("verbose logging is a no-op in this build."); - } Ok(()) } diff --git a/src/host/plm/src/wpr_path.rs b/src/host/plm/src/wpr_path.rs index 11e652a00..9d24ce4b6 100644 --- a/src/host/plm/src/wpr_path.rs +++ b/src/host/plm/src/wpr_path.rs @@ -37,7 +37,7 @@ //! and any write to `%SystemDirectory%` requires `TrustedInstaller` //! (or SYSTEM) — a strictly higher privilege than the admin //! elevation PLM already runs at — the path resolution itself is our -//! security boundary. We keep `verify_wpr_signed` as a thin sanity +//! security boundary. We keep `verify_wpr_present` as a thin sanity //! check that the binary actually exists at the resolved path. use std::path::PathBuf; @@ -45,31 +45,44 @@ use std::process::Command; use std::sync::OnceLock; /// Cached absolute path to `wpr.exe`, resolved on first use. -static WPR_PATH: OnceLock = OnceLock::new(); +static WPR_PATH: OnceLock> = OnceLock::new(); /// Resolve `\wpr.exe` via `GetSystemDirectoryW`. The kernel /// publishes this value at process creation and the env block cannot /// override it, so this is safe even when the parent (unelevated) /// process set `SystemRoot` to an attacker-controlled directory. /// -/// Falls back to `C:\\Windows\\System32\\wpr.exe` only if the API call -/// itself fails (which on a real Windows install does not happen). +/// If `GetSystemDirectoryW` reports the initial 260-wide buffer is +/// insufficient (return value `>= buf.len()`, per Win32 semantics), +/// we retry once with the required size. Only if the API returns 0 +/// (a Win32 failure, which does not happen on a real Windows install) +/// do we surface `None` to the caller. #[cfg(target_os = "windows")] -fn resolve_wpr_path() -> PathBuf { +fn resolve_wpr_path() -> Option { use windows::Win32::System::SystemInformation::GetSystemDirectoryW; + let mut buf = vec![0u16; 260]; // SAFETY: buf is initialized; we pass a valid length and own the // memory for the duration of the call. - let n = unsafe { GetSystemDirectoryW(Some(&mut buf)) }; - if n == 0 || (n as usize) > buf.len() { - // API failed or buffer somehow too small: use a hardcoded - // fallback rather than reading the env block. - return PathBuf::from("C:\\Windows\\System32\\wpr.exe"); + let mut n = unsafe { GetSystemDirectoryW(Some(&mut buf)) }; + if n == 0 { + return None; + } + // Per docs: on success `n` is the length WITHOUT the terminating + // NUL and is strictly less than the buffer size. If `n` is >= + // buffer size, the buffer was too small and `n` is the required + // size INCLUDING the NUL — grow and retry once. + if (n as usize) >= buf.len() { + buf = vec![0u16; n as usize]; + n = unsafe { GetSystemDirectoryW(Some(&mut buf)) }; + if n == 0 || (n as usize) >= buf.len() { + return None; + } } let dir = wxc_common::string_util::from_wide(&buf[..n as usize]); let mut p = PathBuf::from(dir); p.push("wpr.exe"); - p + Some(p) } /// Sanity-check that the resolved `wpr.exe` actually exists on disk. @@ -82,13 +95,17 @@ fn resolve_wpr_path() -> PathBuf { /// would be defence against a strictly higher privilege than the one /// we hold. See the module doc for the full rationale. /// -/// Returns `Err` if the resolved path doesn't exist on disk, which -/// indicates a broken/stripped Windows install (WPT not present) — -/// something the caller must surface with a clear message rather than -/// let `CreateProcess` fail cryptically later. +/// Returns `Err` if `GetSystemDirectoryW` failed outright, or if the +/// resolved path doesn't exist on disk — both indicate a +/// broken/stripped Windows install (WPT not present or `System32` +/// unreadable), which the caller must surface with a clear message +/// rather than let `CreateProcess` fail cryptically later. #[cfg(target_os = "windows")] -pub fn verify_wpr_signed() -> Result<(), String> { - let path = WPR_PATH.get_or_init(resolve_wpr_path); +pub fn verify_wpr_present() -> Result<(), String> { + let slot = WPR_PATH.get_or_init(resolve_wpr_path); + let path = slot + .as_ref() + .ok_or_else(|| "GetSystemDirectoryW failed; cannot locate wpr.exe".to_string())?; if !path.is_file() { return Err(format!( "wpr.exe not found at {} — install the Windows Performance Toolkit \ @@ -102,25 +119,25 @@ pub fn verify_wpr_signed() -> Result<(), String> { /// Non-Windows stub — PLM is Windows-only, but the crate builds /// cross-platform for CI parity, so this always succeeds. #[cfg(not(target_os = "windows"))] -pub fn verify_wpr_signed() -> Result<(), String> { +pub fn verify_wpr_present() -> Result<(), String> { Ok(()) } /// Return a `Command` rooted at the absolute `wpr.exe` path. Callers /// should still build their own `.args(...)` chain on top. /// -/// On Windows we tack on `CREATE_NO_WINDOW` (0x08000000) so the child -/// wpr.exe process has no attached console. wpr renders its -/// `100% [>>>>>>]` progress bar via `WriteConsoleW`, which writes -/// **directly to the console handle** — that bypasses any stdio pipe -/// redirection (`.stdout(Stdio::piped())` / `.output()`), so without -/// this flag the progress bar leaks onto the wrapping tool's terminal -/// even though we capture stdout/stderr. Regular `printf`-style -/// stdout/stderr traffic still gets captured through the pipes and is -/// replayed on failure via `replay_wpr_output`. +/// Assumes `verify_wpr_present` has already been called and succeeded +/// (which cached the resolved path). If `GetSystemDirectoryW` failed +/// at first-call time, we fall back to a bare `wpr.exe` argument so +/// `CreateProcess` produces a clear "not found" error — the caller +/// should have surfaced the `verify_wpr_present` failure before +/// getting here. pub fn wpr_command() -> Command { - let p = WPR_PATH.get_or_init(resolve_wpr_path); - let mut cmd = Command::new(p); + let slot = WPR_PATH.get_or_init(resolve_wpr_path); + let mut cmd = match slot { + Some(p) => Command::new(p), + None => Command::new("wpr.exe"), + }; #[cfg(target_os = "windows")] { use std::os::windows::process::CommandExt; @@ -136,7 +153,7 @@ mod tests { #[test] fn resolves_absolute_system_directory_wpr() { - let p = resolve_wpr_path(); + let p = resolve_wpr_path().expect("GetSystemDirectoryW should succeed on Windows CI"); assert!( p.is_absolute(), "wpr path must be absolute: {}", @@ -164,7 +181,7 @@ mod tests { fn ignores_system_root_env_var() { let original = std::env::var_os("SystemRoot"); std::env::set_var("SystemRoot", "C:\\Users\\Public\\evil"); - let p = resolve_wpr_path(); + let p = resolve_wpr_path().expect("GetSystemDirectoryW should succeed on Windows CI"); let s = p.to_string_lossy().to_ascii_lowercase(); assert!( !s.contains("public") && !s.contains("evil"),