From 5ad8a4c8e3dc4e16701bc12e225933d2fa258168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Wed, 22 Jul 2026 19:10:51 +0200 Subject: [PATCH] agent: honor agent.debug_console_shell for the debug console MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an agent.debug_console_shell option, parsed from the kernel cmdline (mirroring agent.debug_console_vport) and overridable from the agent config file. When set, the debug console prefers it over the built-in /bin/bash and /bin/sh candidates, falling back to those if it is missing at runtime. This lets an operator point the debug console at a richer shell shipped in a guest extension (e.g. the devkit extension) without rebuilding the base rootfs. The configured shell is only honored if it resolves - after canonicalization, so symlink and ".." escapes are caught - to a path under /run/kata-extensions/. Extension images are dm-verity measured and read-only, so this confines the debug console to trusted, integrity-verified binaries: a host that can influence the agent kernel cmdline (untrusted in the CoCo threat model) cannot repoint the debug console at a container's rootfs to run tenant binaries or read confidential data in the guest-root namespace. Anything outside the prefix is ignored and the built-in candidates are used instead. No runtime emits this option yet; that follows in later commits. Signed-off-by: Fabiano Fidêncio Assisted-by: Cursor --- src/agent/src/config.rs | 21 ++++++++ src/agent/src/console.rs | 101 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 121 insertions(+), 1 deletion(-) diff --git a/src/agent/src/config.rs b/src/agent/src/config.rs index adfdf397c3..b9def67917 100644 --- a/src/agent/src/config.rs +++ b/src/agent/src/config.rs @@ -28,6 +28,7 @@ const CDI_TIMEOUT_OPTION: &str = "agent.cdi_timeout"; const LAUNCH_PROCESS_TIMEOUT_OPTION: &str = "agent.launch_process_timeout"; const VISIBLE_CDI_DEVICES_OPTION: &str = "agent.visible_cdi_devices"; const DEBUG_CONSOLE_VPORT_OPTION: &str = "agent.debug_console_vport"; +const DEBUG_CONSOLE_SHELL_OPTION: &str = "agent.debug_console_shell"; const LOG_VPORT_OPTION: &str = "agent.log_vport"; const CONTAINER_PIPE_SIZE_OPTION: &str = "agent.container_pipe_size"; const CGROUP_NO_V1: &str = "cgroup_no_v1"; @@ -136,6 +137,9 @@ pub struct AgentConfig { pub launch_process_timeout: time::Duration, pub visible_cdi_devices: bool, pub debug_console_vport: i32, + // Guest path of the shell the debug console execs; empty uses the built-in + // candidates (/bin/bash, /bin/sh). + pub debug_console_shell: String, pub log_vport: i32, pub container_pipe_size: i32, pub server_addr: String, @@ -171,6 +175,7 @@ pub struct AgentConfigBuilder { pub launch_process_timeout: Option, pub visible_cdi_devices: Option, pub debug_console_vport: Option, + pub debug_console_shell: Option, pub log_vport: Option, pub container_pipe_size: Option, pub server_addr: Option, @@ -267,6 +272,7 @@ impl Default for AgentConfig { launch_process_timeout: DEFAULT_LAUNCH_PROCESS_TIMEOUT, visible_cdi_devices: false, debug_console_vport: 0, + debug_console_shell: String::from(""), log_vport: 0, container_pipe_size: DEFAULT_CONTAINER_PIPE_SIZE, server_addr: format!("{VSOCK_ADDR}:{DEFAULT_AGENT_VSOCK_PORT}"), @@ -310,6 +316,7 @@ impl FromStr for AgentConfig { config_override!(agent_config_builder, agent_config, launch_process_timeout); config_override!(agent_config_builder, agent_config, visible_cdi_devices); config_override!(agent_config_builder, agent_config, debug_console_vport); + config_override!(agent_config_builder, agent_config, debug_console_shell); config_override!(agent_config_builder, agent_config, log_vport); config_override!(agent_config_builder, agent_config, container_pipe_size); config_override!(agent_config_builder, agent_config, server_addr); @@ -515,6 +522,12 @@ impl AgentConfig { get_number_value, |port: &i32| *port > 0 ); + parse_cmdline_param!( + param, + DEBUG_CONSOLE_SHELL_OPTION, + config.debug_console_shell, + get_string_value + ); parse_cmdline_param!( param, LOG_VPORT_OPTION, @@ -902,6 +915,7 @@ mod tests { contents: &'a str, env_vars: Vec<&'a str>, debug_console: bool, + debug_console_shell: &'a str, dev_mode: bool, log_level: slog::Level, hotplug_timeout: time::Duration, @@ -926,6 +940,7 @@ mod tests { contents: "", env_vars: Vec::new(), debug_console: false, + debug_console_shell: "", dev_mode: false, log_level: DEFAULT_LOG_LEVEL, hotplug_timeout: DEFAULT_HOTPLUG_TIMEOUT, @@ -1001,6 +1016,11 @@ mod tests { debug_console: true, ..Default::default() }, + TestData { + contents: "agent.debug_console_shell=/run/kata-extensions/devkit/bin/devkit-sh", + debug_console_shell: "/run/kata-extensions/devkit/bin/devkit-sh", + ..Default::default() + }, TestData { contents: " agent.debug_console ", debug_console: true, @@ -1498,6 +1518,7 @@ mod tests { AgentConfig::from_cmdline(filename, vec![]).expect("Failed to parse command line"); assert_eq!(d.debug_console, config.debug_console, "{msg}"); + assert_eq!(d.debug_console_shell, config.debug_console_shell, "{msg}"); assert_eq!(d.dev_mode, config.dev_mode, "{msg}"); assert_eq!(d.cgroup_no_v1, config.cgroup_no_v1, "{msg}"); assert_eq!( diff --git a/src/agent/src/console.rs b/src/agent/src/console.rs index cb0b4121c0..d32dff4621 100644 --- a/src/agent/src/console.rs +++ b/src/agent/src/console.rs @@ -29,6 +29,16 @@ use tokio::sync::watch::Receiver; const CONSOLE_PATH: &str = "/dev/console"; +// A configured debug_console_shell is honored only if it resolves under this +// prefix (a guest extension mount). Extension images are dm-verity measured and +// read-only, so this confines the debug console to trusted binaries: a host that +// can influence the agent cmdline (untrusted under CoCo) cannot repoint it at a +// container rootfs to run tenant binaries or read guest-root data. +// +// Only referenced under `#[cfg(not(test))]`, so it reads as dead code in tests. +#[cfg_attr(test, allow(dead_code))] +const DEBUG_CONSOLE_SHELL_ALLOWED_PREFIX: &str = "/run/kata-extensions/"; + lazy_static! { static ref SHELLS: Arc>> = { let mut v = Vec::new(); @@ -46,6 +56,20 @@ pub fn initialize() { lazy_static::initialize(&SHELLS); } +// Honor `shell` only if it canonicalizes (resolving symlinks and `..`) to a path +// under `prefix`. Canonicalization is what makes this safe: a raw prefix check +// could be bypassed by a symlink or `..` escape. The original path is returned so +// an entrypoint symlink under the prefix (devkit-sh -> devkit-enter) stays the +// exec target, since it is equally trusted. +fn shell_under_prefix(shell: &str, prefix: &str) -> Option { + if shell.is_empty() { + return None; + } + + let canonical = std::fs::canonicalize(shell).ok()?; + canonical.starts_with(prefix).then(|| shell.to_string()) +} + pub async fn debug_console_handler( logger: Logger, port: u32, @@ -53,7 +77,26 @@ pub async fn debug_console_handler( ) -> Result<()> { let logger = logger.new(o!("subsystem" => "debug-console")); - let shells = SHELLS.lock().unwrap().to_vec(); + // Only mutated under `#[cfg(not(test))]`, so it need not be mut in tests. + #[cfg_attr(test, allow(unused_mut))] + let mut shells = SHELLS.lock().unwrap().to_vec(); + + // Prefer a configured debug_console_shell over the built-ins, but only if it + // passes the extension-prefix guard. + #[cfg(not(test))] + { + let configured = crate::AGENT_CONFIG.debug_console_shell.clone(); + match shell_under_prefix(&configured, DEBUG_CONSOLE_SHELL_ALLOWED_PREFIX) { + Some(sh) => shells.insert(0, sh), + None if !configured.is_empty() => warn!( + logger, + "ignoring debug_console_shell outside {}: {}", + DEBUG_CONSOLE_SHELL_ALLOWED_PREFIX, + configured + ), + None => {} + } + } let shell = shells .into_iter() @@ -306,4 +349,60 @@ mod tests { "no shell found to launch debug console" ); } + + #[test] + fn test_shell_under_prefix() { + use std::os::unix::fs::symlink; + + let dir = tempdir().expect("failed to create tmpdir"); + let root = dir.path(); + + // Simulate an extension mount ("allowed") and a container rootfs + // ("other") side by side under the same tmpdir. + let allowed = root.join("allowed"); + let other = root.join("other"); + std::fs::create_dir_all(allowed.join("bin")).unwrap(); + std::fs::create_dir_all(&other).unwrap(); + + let prefix = allowed.to_str().unwrap(); + + // Inside the prefix: honored. + let inside = allowed.join("bin").join("devkit-enter"); + std::fs::write(&inside, b"#!/bin/sh\n").unwrap(); + let inside = inside.to_str().unwrap(); + assert_eq!(shell_under_prefix(inside, prefix), Some(inside.to_string())); + + // A symlink that lives inside the prefix but targets another file inside + // the prefix is honored, and the (symlink) path is what gets returned. + let link = allowed.join("bin").join("devkit-sh"); + symlink("devkit-enter", &link).unwrap(); + let link = link.to_str().unwrap(); + assert_eq!(shell_under_prefix(link, prefix), Some(link.to_string())); + + // Outside the prefix (e.g. a container binary): rejected. + let outside = other.join("sh"); + std::fs::write(&outside, b"#!/bin/sh\n").unwrap(); + assert_eq!(shell_under_prefix(outside.to_str().unwrap(), prefix), None); + + // A symlink under the prefix that escapes it via its target: rejected, + // because canonicalization resolves it outside the prefix. + let escape = allowed.join("escape"); + symlink(&outside, &escape).unwrap(); + assert_eq!(shell_under_prefix(escape.to_str().unwrap(), prefix), None); + + // A `..` traversal that escapes the prefix: rejected. + let traversal = allowed.join("bin").join("..").join("..").join("other"); + let traversal = traversal.join("sh"); + assert_eq!( + shell_under_prefix(traversal.to_str().unwrap(), prefix), + None + ); + + // Empty and non-existent paths: rejected (no shell configured / bad path). + assert_eq!(shell_under_prefix("", prefix), None); + assert_eq!( + shell_under_prefix(allowed.join("bin").join("enoent").to_str().unwrap(), prefix), + None + ); + } }