Merge pull request kata-containers#9749 from mkulke/mkulke/configure-…

…guest-components-spawning

CoCo: introduce config for guest-components procs

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` |

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,19 +60,32 @@ 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
 #[strum(serialize_all = "kebab-case")]
 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<String>,
     pub no_proxy: Option<String>,
     pub guest_components_rest_api: Option<GuestComponentsFeatures>,
+    pub guest_components_procs: Option<GuestComponentsProcs>,
 }
 
 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<GuestComponentsFea
         .map_err(|_| anyhow!(ERR_INVALID_GUEST_COMPONENTS_REST_API_VALUE))
 }
 
+#[instrument]
+fn get_guest_components_procs_value(param: &str) -> Result<GuestComponentsProcs> {
+    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<GuestComponentsProcs>,
+        }
+
+        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(

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(())
 }