From 9f04dc4c8b6d8ec4e9dacffbc570cfecafa92854 Mon Sep 17 00:00:00 2001 From: Magnus Kulke Date: Thu, 30 May 2024 17:47:47 +0200 Subject: [PATCH] agent: introduce config for coco attestion procs fixes #9748 A configuration option `guest_component_procs` has been introduced that indicates which guest component processes are supposed to be spawned by the agent. The default behaviour remains that all of those processes are actively spawned by the agent. At the moment this is based on presence of binaries in the rootfs and the guest_component_api_rest option. The new option is incremental: none -> attestation-agent -> confidential-data-hub -> api-server-rest e.g. api-server-rest implies attestation-agent and confidential-data-hub the `none` option has been removed from guest_component_api_rest, since this is addresses by the introduced option. To not change expected behaviour for non-coco guests we still will still only attempt to spawn the processes if the requested attestation binaries are present on the rootfs, and issue in warning in those cases. Signed-off-by: Magnus Kulke --- src/agent/README.md | 3 +- src/agent/src/config.rs | 132 +++++++++++++++++++++++++++++++++++++--- src/agent/src/main.rs | 93 ++++++++++++++++++++-------- 3 files changed, 194 insertions(+), 34 deletions(-) diff --git a/src/agent/README.md b/src/agent/README.md index bf94c90ece..f6f9047449 100644 --- a/src/agent/README.md +++ b/src/agent/README.md @@ -126,7 +126,8 @@ The kata agent has the ability to configure agent options in guest kernel comman | `agent.debug_console_vport` | Debug console port | Allow to specify the `vsock` port to connect the debugging console | integer | `0` | | `agent.devmode` | Developer mode | Allow the agent process to coredump | boolean | `false` | | `agent.hotplug_timeout` | Hotplug timeout | Allow to configure hotplug timeout(seconds) of block devices | integer | `3` | -| `agent.guest_components_rest_api` | `api-server-rest` configuration | Select the features that the API Server Rest attestation component will run with. Valid values are `all`, `attestation`, `resource`, or `none` to not launch the `api-server-rest` component | string | `resource` | +| `agent.guest_components_rest_api` | `api-server-rest` configuration | Select the features that the API Server Rest attestation component will run with. Valid values are `all`, `attestation`, `resource` | string | `resource` | +| `agent.guest_components_procs` | guest-components processes | Attestation-related processes that should be spawned as children of the guest. Valid values are `none`, `attestation-agent`, `confidential-data-hub` (implies `attestation-agent`), `api-server-rest` (implies `attestation-agent` and `confidential-data-hub`) | string | `api-server-rest` | | `agent.https_proxy` | HTTPS proxy | Allow to configure `https_proxy` in the guest | string | `""` | | `agent.log` | Log level | Allow the agent log level to be changed (produces more or less output) | string | `"info"` | | `agent.log_vport` | Log port | Allow to specify the `vsock` port to read logs | integer | `0` | diff --git a/src/agent/src/config.rs b/src/agent/src/config.rs index 041506c18c..ee0eb7b0c5 100644 --- a/src/agent/src/config.rs +++ b/src/agent/src/config.rs @@ -28,6 +28,7 @@ const CONTAINER_PIPE_SIZE_OPTION: &str = "agent.container_pipe_size"; const UNIFIED_CGROUP_HIERARCHY_OPTION: &str = "systemd.unified_cgroup_hierarchy"; const CONFIG_FILE: &str = "agent.config_file"; const GUEST_COMPONENTS_REST_API_OPTION: &str = "agent.guest_components_rest_api"; +const GUEST_COMPONENTS_PROCS_OPTION: &str = "agent.guest_components_procs"; // Configure the proxy settings for HTTPS requests in the guest, // to solve the problem of not being able to access the specified image in some cases. @@ -59,7 +60,8 @@ const ERR_INVALID_CONTAINER_PIPE_SIZE_PARAM: &str = "unable to parse container p const ERR_INVALID_CONTAINER_PIPE_SIZE_KEY: &str = "invalid container pipe size key name"; const ERR_INVALID_CONTAINER_PIPE_NEGATIVE: &str = "container pipe size should not be negative"; -const ERR_INVALID_GUEST_COMPONENTS_REST_API_VALUE: &str = "invalid guest components rest api feature given. Valid values are `all`, `attestation`, `resource`, or `none`"; +const ERR_INVALID_GUEST_COMPONENTS_REST_API_VALUE: &str = "invalid guest components rest api feature given. Valid values are `all`, `attestation`, `resource`"; +const ERR_INVALID_GUEST_COMPONENTS_PROCS_VALUE: &str = "invalid guest components process param given. Valid values are `attestation-agent`, `confidential-data-hub`, `api-server-rest`, or `none`"; #[derive(Clone, Copy, Debug, Default, Display, Deserialize, EnumString, PartialEq)] // Features seem to typically be in kebab-case format, but we only have single words at the moment @@ -67,11 +69,23 @@ const ERR_INVALID_GUEST_COMPONENTS_REST_API_VALUE: &str = "invalid guest compone pub enum GuestComponentsFeatures { All, Attestation, - None, #[default] Resource, } +#[derive(Clone, Copy, Debug, Default, Display, Deserialize, EnumString, PartialEq)] +/// Attestation-related processes that we want to spawn as children of the agent +#[strum(serialize_all = "kebab-case")] +pub enum GuestComponentsProcs { + None, + /// ApiServerRest implies ConfidentialDataHub and AttestationAgent + #[default] + ApiServerRest, + AttestationAgent, + /// ConfidentialDataHub implies AttestationAgent + ConfidentialDataHub, +} + #[derive(Debug)] pub struct AgentConfig { pub debug_console: bool, @@ -89,6 +103,7 @@ pub struct AgentConfig { pub https_proxy: String, pub no_proxy: String, pub guest_components_rest_api: GuestComponentsFeatures, + pub guest_components_procs: GuestComponentsProcs, } #[derive(Debug, Deserialize)] @@ -107,6 +122,7 @@ pub struct AgentConfigBuilder { pub https_proxy: Option, pub no_proxy: Option, pub guest_components_rest_api: Option, + pub guest_components_procs: Option, } macro_rules! config_override { @@ -171,6 +187,7 @@ impl Default for AgentConfig { https_proxy: String::from(""), no_proxy: String::from(""), guest_components_rest_api: GuestComponentsFeatures::default(), + guest_components_procs: GuestComponentsProcs::default(), } } } @@ -314,6 +331,12 @@ impl AgentConfig { config.guest_components_rest_api, get_guest_components_features_value ); + parse_cmdline_param!( + param, + GUEST_COMPONENTS_PROCS_OPTION, + config.guest_components_procs, + get_guest_components_procs_value + ); } if let Ok(addr) = env::var(SERVER_ADDR_ENV_VAR) { @@ -480,6 +503,19 @@ fn get_guest_components_features_value(param: &str) -> Result Result { + let fields: Vec<&str> = param.split('=').collect(); + ensure!(fields.len() >= 2, ERR_INVALID_GET_VALUE_PARAM); + + // We need name (but the value can be blank) + ensure!(!fields[0].is_empty(), ERR_INVALID_GET_VALUE_NO_NAME); + + let value = fields[1..].join("="); + GuestComponentsProcs::from_str(&value) + .map_err(|_| anyhow!(ERR_INVALID_GUEST_COMPONENTS_PROCS_VALUE)) +} + #[cfg(test)] mod tests { use test_utils::assert_result; @@ -519,6 +555,7 @@ mod tests { https_proxy: &'a str, no_proxy: &'a str, guest_components_rest_api: GuestComponentsFeatures, + guest_components_procs: GuestComponentsProcs, } impl Default for TestData<'_> { @@ -537,6 +574,7 @@ mod tests { https_proxy: "", no_proxy: "", guest_components_rest_api: GuestComponentsFeatures::default(), + guest_components_procs: GuestComponentsProcs::default(), } } } @@ -942,8 +980,23 @@ mod tests { ..Default::default() }, TestData { - contents: "agent.guest_components_rest_api=none", - guest_components_rest_api: GuestComponentsFeatures::None, + contents: "agent.guest_components_procs=api-server-rest", + guest_components_procs: GuestComponentsProcs::ApiServerRest, + ..Default::default() + }, + TestData { + contents: "agent.guest_components_procs=confidential-data-hub", + guest_components_procs: GuestComponentsProcs::ConfidentialDataHub, + ..Default::default() + }, + TestData { + contents: "agent.guest_components_procs=attestation-agent", + guest_components_procs: GuestComponentsProcs::AttestationAgent, + ..Default::default() + }, + TestData { + contents: "agent.guest_components_procs=none", + guest_components_procs: GuestComponentsProcs::None, ..Default::default() }, ]; @@ -1000,6 +1053,11 @@ mod tests { "{}", msg ); + assert_eq!( + d.guest_components_procs, config.guest_components_procs, + "{}", + msg + ); for v in vars_to_unset { env::remove_var(v); @@ -1500,10 +1558,6 @@ Caused by: param: "x=attestation", result: Ok(GuestComponentsFeatures::Attestation), }, - TestData { - param: "x=none", - result: Ok(GuestComponentsFeatures::None), - }, TestData { param: "x=resource", result: Ok(GuestComponentsFeatures::Resource), @@ -1533,6 +1587,68 @@ Caused by: } } + #[test] + fn test_get_guest_components_procs_value() { + #[derive(Debug)] + struct TestData<'a> { + param: &'a str, + result: Result, + } + + let tests = &[ + TestData { + param: "", + result: Err(anyhow!(ERR_INVALID_GET_VALUE_PARAM)), + }, + TestData { + param: "=", + result: Err(anyhow!(ERR_INVALID_GET_VALUE_NO_NAME)), + }, + TestData { + param: "==", + result: Err(anyhow!(ERR_INVALID_GET_VALUE_NO_NAME)), + }, + TestData { + param: "x=attestation-agent", + result: Ok(GuestComponentsProcs::AttestationAgent), + }, + TestData { + param: "x=confidential-data-hub", + result: Ok(GuestComponentsProcs::ConfidentialDataHub), + }, + TestData { + param: "x=none", + result: Ok(GuestComponentsProcs::None), + }, + TestData { + param: "x=api-server-rest", + result: Ok(GuestComponentsProcs::ApiServerRest), + }, + TestData { + param: "x===", + result: Err(anyhow!(ERR_INVALID_GUEST_COMPONENTS_PROCS_VALUE)), + }, + TestData { + param: "x==x", + result: Err(anyhow!(ERR_INVALID_GUEST_COMPONENTS_PROCS_VALUE)), + }, + TestData { + param: "x=x", + result: Err(anyhow!(ERR_INVALID_GUEST_COMPONENTS_PROCS_VALUE)), + }, + ]; + + for (i, d) in tests.iter().enumerate() { + let msg = format!("test[{}]: {:?}", i, d); + + let result = get_guest_components_procs_value(d.param); + + let msg = format!("{}: result: {:?}", msg, result); + + assert_result!(d.result, result, msg); + } + } + #[test] fn test_config_builder_from_string() { let config = AgentConfig::from_str( diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index 681ff06ecb..0450b1bcc3 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -59,11 +59,11 @@ mod util; mod version; mod watcher; -use config::GuestComponentsFeatures; +use config::GuestComponentsProcs; use mount::{cgroups_mount, general_mount}; use sandbox::Sandbox; use signal::setup_signal_handler; -use slog::{error, info, o, warn, Logger}; +use slog::{debug, error, info, o, warn, Logger}; use uevent::watch_uevents; use futures::future::join_all; @@ -403,8 +403,16 @@ async fn start_sandbox( let (tx, rx) = tokio::sync::oneshot::channel(); sandbox.lock().await.sender = Some(tx); - if Path::new(CDH_PATH).exists() && Path::new(AA_PATH).exists() { - init_attestation_components(logger, config)?; + let gc_procs = config.guest_components_procs; + if gc_procs != GuestComponentsProcs::None { + if !attestation_binaries_available(logger, &gc_procs) { + warn!( + logger, + "attestation binaries requested for launch not available" + ); + } else { + init_attestation_components(logger, config)?; + } } // vsock:///dev/vsock, port @@ -417,9 +425,33 @@ async fn start_sandbox( Ok(()) } +// Check if required attestation binaries are available on the rootfs. +fn attestation_binaries_available(logger: &Logger, procs: &GuestComponentsProcs) -> bool { + let binaries = match procs { + GuestComponentsProcs::AttestationAgent => vec![AA_PATH], + GuestComponentsProcs::ConfidentialDataHub => vec![AA_PATH, CDH_PATH], + GuestComponentsProcs::ApiServerRest => vec![AA_PATH, CDH_PATH, API_SERVER_PATH], + _ => vec![], + }; + for binary in binaries.iter() { + if !Path::new(binary).exists() { + warn!(logger, "{} not found", binary); + return false; + } + } + true +} + // Start-up attestation-agent, CDH and api-server-rest if they are packaged in the rootfs -fn init_attestation_components(logger: &Logger, _config: &AgentConfig) -> Result<()> { - // The Attestation Agent will run for the duration of the guest. +// and the corresponding procs are enabled in the agent configuration. the process will be +// launched in the background and the function will return immediately. +fn init_attestation_components(logger: &Logger, config: &AgentConfig) -> Result<()> { + // skip launch of any guest-component + if config.guest_components_procs == GuestComponentsProcs::None { + return Ok(()); + } + + debug!(logger, "spawning attestation-agent process {}", AA_PATH); launch_process( logger, AA_PATH, @@ -429,32 +461,43 @@ fn init_attestation_components(logger: &Logger, _config: &AgentConfig) -> Result ) .map_err(|e| anyhow!("launch_process {} failed: {:?}", AA_PATH, e))?; - if let Err(e) = launch_process( + // skip launch of confidential-data-hub and api-server-rest + if config.guest_components_procs == GuestComponentsProcs::AttestationAgent { + return Ok(()); + } + + debug!( + logger, + "spawning confidential-data-hub process {}", CDH_PATH + ); + launch_process( logger, CDH_PATH, &vec![], CDH_SOCKET, DEFAULT_LAUNCH_PROCESS_TIMEOUT, - ) { - error!(logger, "launch_process {} failed: {:?}", CDH_PATH, e); - } else { - let features = _config.guest_components_rest_api; - match features { - GuestComponentsFeatures::None => {} - _ => { - if let Err(e) = launch_process( - logger, - API_SERVER_PATH, - &vec!["--features", &features.to_string()], - "", - 0, - ) { - error!(logger, "launch_process {} failed: {:?}", API_SERVER_PATH, e); - } - } - } + ) + .map_err(|e| anyhow!("launch_process {} failed: {:?}", CDH_PATH, e))?; + + // skip launch of api-server-rest + if config.guest_components_procs == GuestComponentsProcs::ConfidentialDataHub { + return Ok(()); } + let features = config.guest_components_rest_api; + debug!( + logger, + "spawning api-server-rest process {} --features {}", API_SERVER_PATH, features + ); + launch_process( + logger, + API_SERVER_PATH, + &vec!["--features", &features.to_string()], + "", + 0, + ) + .map_err(|e| anyhow!("launch_process {} failed: {:?}", API_SERVER_PATH, e))?; + Ok(()) }