mirror of
https://github.com/kata-containers/kata-containers.git
synced 2026-07-25 22:49:59 +00:00
agent: honor agent.debug_console_shell for the debug console
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 <ffidencio@nvidia.com> Assisted-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -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<time::Duration>,
|
||||
pub visible_cdi_devices: Option<bool>,
|
||||
pub debug_console_vport: Option<i32>,
|
||||
pub debug_console_shell: Option<String>,
|
||||
pub log_vport: Option<i32>,
|
||||
pub container_pipe_size: Option<i32>,
|
||||
pub server_addr: Option<String>,
|
||||
@@ -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!(
|
||||
|
||||
@@ -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<SyncMutex<Vec<String>>> = {
|
||||
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<String> {
|
||||
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
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user