CDH: fix aa_kbc_params and config reading

attestation-agent/attestation-agent/src/config/aa_kbc_params.rs

@@ -1,38 +1,20 @@
 use log::debug;
-use serde::Deserialize;
 use std::env;
-use std::path::Path;
-use std::sync::OnceLock;
 use thiserror::Error;
 
-const PEER_POD_CONFIG_PATH: &str = "/run/peerpod/daemon.json";
-static KATA_AGENT_CONFIG_PATH: OnceLock<String> = OnceLock::new();
-
 #[derive(Error, Debug)]
 pub enum ParamError {
     #[error("illegal aa_kbc_params format: {0}")]
     IllegalFormat(String),
-    #[error("unable to read `aa_kbc_params` entry from kata-agent config file")]
-    AgentConfigParsing(#[from] toml::de::Error),
     #[error("io error")]
     Io(#[from] std::io::Error),
     #[error("no `agent.aa_kbc_params` provided in kernel commandline")]
     MissingInCmdline,
 }
 
 pub struct AaKbcParams {
-    kbc: String,
-    uri: String,
-}
-
-impl AaKbcParams {
-    pub fn kbc(&self) -> &str {
-        &self.kbc
-    }
-
-    pub fn uri(&self) -> &str {
-        &self.uri
-    }
+    pub kbc: String,
+    pub uri: String,
 }
 
 impl TryFrom<String> for AaKbcParams {
@@ -61,11 +43,6 @@ pub fn get_value() -> Result<String, ParamError> {
         return Ok(params);
     }
 
-    // second check whether we are in a peer pod
-    if Path::new(PEER_POD_CONFIG_PATH).exists() {
-        return from_config_file();
-    }
-
     // finally use the kernel cmdline
     from_cmdline()
 }
@@ -75,28 +52,6 @@ pub fn get_params() -> Result<AaKbcParams, ParamError> {
     value.try_into()
 }
 
-// We only care about the aa_kbc_params value at the moment
-#[derive(Debug, Deserialize)]
-struct AgentConfig {
-    aa_kbc_params: String,
-}
-
-fn from_config_file() -> Result<String, ParamError> {
-    debug!("get aa_kbc_params from file");
-
-    // check env for KATA_AGENT_CONFIG_PATH, fall back to default path
-    let path: &String = KATA_AGENT_CONFIG_PATH.get_or_init(|| {
-        env::var("KATA_AGENT_CONFIG_PATH").unwrap_or_else(|_| "/etc/agent-config.toml".into())
-    });
-
-    debug!("reading agent config from {}", path);
-    let agent_config_str = std::fs::read_to_string(path)?;
-
-    let agent_config: AgentConfig = toml::from_str(&agent_config_str)?;
-
-    Ok(agent_config.aa_kbc_params)
-}
-
 fn from_cmdline() -> Result<String, ParamError> {
     debug!("get aa_kbc_params from kernel cmdline");
     let cmdline = std::fs::read_to_string("/proc/cmdline")?;

attestation-agent/attestation-agent/src/config/coco_as.rs

@@ -17,7 +17,7 @@ impl Default for CoCoASConfig {
     fn default() -> Self {
         let aa_kbc_params = aa_kbc_params::get_params().expect("failed to get aa_kbc_params");
         Self {
-            url: aa_kbc_params.uri().into(),
+            url: aa_kbc_params.uri.clone(),
         }
     }
 }

attestation-agent/attestation-agent/src/config/kbs.rs

@@ -20,7 +20,7 @@ impl Default for KbsConfig {
     fn default() -> Self {
         let aa_kbc_params = aa_kbc_params::get_params().expect("failed to get aa_kbc_params");
         Self {
-            url: aa_kbc_params.uri().into(),
+            url: aa_kbc_params.uri.clone(),
             cert: None,
         }
     }

confidential-data-hub/hub/src/bin/config/mod.rs

@@ -3,13 +3,12 @@
 // SPDX-License-Identifier: Apache-2.0
 //
 
-use std::env;
+use std::{env, fs, path::Path};
 
 use anyhow::*;
 use config::{Config, File};
-use log::{debug, warn};
+use log::{debug, info};
 use serde::Deserialize;
-use tokio::fs;
 
 cfg_if::cfg_if! {
     if #[cfg(feature = "ttrpc")] {
@@ -19,7 +18,7 @@ cfg_if::cfg_if! {
     }
 }
 
-#[derive(Deserialize, Debug)]
+#[derive(Deserialize, Debug, PartialEq)]
 pub struct KbsConfig {
     pub name: String,
 
@@ -30,21 +29,32 @@ pub struct KbsConfig {
 
 impl Default for KbsConfig {
     fn default() -> Self {
-        Self {
-            name: "offline_fs_kbc".into(),
-            url: "null".into(),
-            kbs_cert: None,
+        debug!("Try to get kbc and url from env and kernel commandline.");
+        match attestation_agent::config::aa_kbc_params::get_params() {
+            std::result::Result::Ok(aa_kbc_params) => KbsConfig {
+                name: aa_kbc_params.kbc,
+                url: aa_kbc_params.uri,
+                kbs_cert: None,
+            },
+            Err(_) => {
+                debug!("Failed to get aa_kbc_params from env or kernel cmdline. Use offline_fs_kbc by default.");
+                KbsConfig {
+                    name: "offline_fs_kbc".into(),
+                    url: "".into(),
+                    kbs_cert: None,
+                }
+            }
         }
     }
 }
 
-#[derive(Deserialize, Debug)]
+#[derive(Deserialize, Debug, PartialEq)]
 pub struct Credential {
     pub resource_uri: String,
     pub path: String,
 }
 
-#[derive(Deserialize, Debug)]
+#[derive(Deserialize, Debug, PartialEq)]
 pub struct CdhConfig {
     pub kbc: KbsConfig,
 
@@ -65,13 +75,31 @@ impl Default for CdhConfig {
 }
 
 impl CdhConfig {
-    pub async fn init(config_path: &str) -> Result<Self> {
-        let mut config = Self::from_file(config_path).unwrap_or_else(|e| {
-            warn!("read config file {config_path} failed {e:?}, use a default config where `aa_kbc_params` = offline_fs_kbc::null.");
-            Self::default()
+    pub fn new(config_path: Option<String>) -> Result<Self> {
+        let config_path = config_path.or_else(|| {
+            if let std::result::Result::Ok(env_path) = env::var("CDH_CONFIG_PATH") {
+                debug!("Read CDH's config path from env: {env_path}");
+                return Some(env_path);
+            }
+            None
         });
 
-        config.update_from_kernel_cmdline().await?;
+        let mut config = match config_path {
+            Some(path) => {
+                info!("Use configuration file {path}");
+                if !Path::new(&path).exists() {
+                    bail!("Config file {path} not found.")
+                }
+
+                Self::from_file(&path)?
+            }
+            None => {
+                info!("No config path specified, use a default config.");
+                Self::default()
+            }
+        };
+
+        config.extend_credentials_from_kernel_cmdline()?;
         Ok(config)
     }
 
@@ -102,10 +130,8 @@ impl CdhConfig {
     /// path.
     ///
     /// TODO: delete this way after initdata mechanism could cover CDH's launch config.
-    async fn update_from_kernel_cmdline(&mut self) -> Result<()> {
-        let cmdline = fs::read_to_string("/proc/cmdline")
-            .await
-            .context("read kernel cmdline failed")?;
+    fn extend_credentials_from_kernel_cmdline(&mut self) -> Result<()> {
+        let cmdline = fs::read_to_string("/proc/cmdline").context("read kernel cmdline failed")?;
         let kbs_resources = cmdline
             .split_ascii_whitespace()
             .find(|para| para.starts_with("cdh.kbs_resources="))
@@ -127,15 +153,13 @@ impl CdhConfig {
 
 impl CdhConfig {
     pub fn set_configuration_envs(&self) {
-        if attestation_agent::config::aa_kbc_params::get_value().is_err() {
-            debug!("No aa_kbc_params provided in kernel cmdline, env and peerpod config.");
-            // KBS configurations
+        if env::var("AA_KBC_PARAMS").is_err() {
             env::set_var(
                 "AA_KBC_PARAMS",
                 format!("{}::{}", self.kbc.name, self.kbc.url),
             );
         }
-
+        // KBS configurations
         if let Some(kbs_cert) = &self.kbc.kbs_cert {
             env::set_var("KBS_CERT", kbs_cert);
         }
@@ -144,11 +168,12 @@ impl CdhConfig {
 
 #[cfg(test)]
 mod tests {
-    use std::io::Write;
+    use std::{env, io::Write};
 
+    use anyhow::anyhow;
     use rstest::rstest;
 
-    use crate::CdhConfig;
+    use crate::{config::DEFAULT_CDH_SOCKET_ADDR, CdhConfig, KbsConfig};
 
     #[rstest]
     #[case(
@@ -200,4 +225,35 @@ path = "/run/confidential-containers/cdh/kms-credential/aliyun/config.toml"
         let res = CdhConfig::from_file(file.path().to_str().unwrap());
         assert_eq!(res.is_ok(), successful, "{res:?}");
     }
+
+    #[test]
+    fn test_config_path() {
+        // --config takes precedence,
+        // then env.CDH_CONFIG_PATH
+
+        let config = CdhConfig::new(None).expect("Must be successful");
+        let expected = CdhConfig {
+            kbc: KbsConfig {
+                name: "offline_fs_kbc".into(),
+                url: "".into(),
+                kbs_cert: None,
+            },
+            credentials: Vec::new(),
+            socket: DEFAULT_CDH_SOCKET_ADDR.into(),
+        };
+        assert_eq!(config, expected);
+
+        let config = CdhConfig::new(Some("/thing".into())).unwrap_err();
+        let expected = anyhow!("Config file /thing not found.");
+        assert_eq!(format!("{config}"), format!("{expected}"));
+
+        env::set_var("CDH_CONFIG_PATH", "/byenv");
+        let config = CdhConfig::new(None).unwrap_err();
+        let expected = anyhow!("Config file /byenv not found.");
+        assert_eq!(format!("{config}"), format!("{expected}"));
+
+        let config = CdhConfig::new(Some("/thing".into())).unwrap_err();
+        let expected = anyhow!("Config file /thing not found.");
+        assert_eq!(format!("{config}"), format!("{expected}"));
+    }
 }

confidential-data-hub/hub/src/bin/grpc-cdh.rs

@@ -17,8 +17,6 @@ mod message;
 
 use config::*;
 
-const DEFAULT_CONFIG_PATH: &str = "/etc/confidential-data-hub.conf";
-
 const VERSION: &str = include_str!(concat!(env!("OUT_DIR"), "/version"));
 
 #[derive(Debug, Parser)]
@@ -31,23 +29,12 @@ struct Cli {
     config: Option<String>,
 }
 
-fn get_config_path(cli: Cli) -> String {
-    cli.config.unwrap_or_else(|| {
-        if let Ok(env_path) = env::var("CDH_CONFIG_PATH") {
-            return env_path;
-        }
-        DEFAULT_CONFIG_PATH.into()
-    })
-}
-
 #[tokio::main]
 async fn main() -> Result<()> {
     env_logger::init_from_env(env_logger::Env::new().default_filter_or("info"));
     let cli = Cli::parse();
-    let config_path = get_config_path(cli);
-    info!("Use configuration file {}", config_path);
 
-    let config = CdhConfig::init(&config_path).await?;
+    let config = CdhConfig::new(cli.config)?;
     config.set_configuration_envs();
 
     let cdh_socket = config.socket.parse::<SocketAddr>()?;

confidential-data-hub/hub/src/bin/ttrpc-cdh.rs

@@ -28,8 +28,6 @@ mod ttrpc_server;
 
 use config::*;
 
-const DEFAULT_CONFIG_PATH: &str = "/etc/confidential-data-hub.conf";
-
 const UNIX_SOCKET_PREFIX: &str = "unix://";
 
 const VERSION: &str = include_str!(concat!(env!("OUT_DIR"), "/version"));
@@ -52,23 +50,12 @@ macro_rules! ttrpc_service {
     }};
 }
 
-fn get_config_path(cli: Cli) -> String {
-    cli.config.unwrap_or_else(|| {
-        if let Ok(env_path) = env::var("CDH_CONFIG_PATH") {
-            return env_path;
-        }
-        DEFAULT_CONFIG_PATH.into()
-    })
-}
-
 #[tokio::main]
 async fn main() -> Result<()> {
     env_logger::init_from_env(env_logger::Env::new().default_filter_or("info"));
     let cli = Cli::parse();
-    let config_path = get_config_path(cli);
-    info!("Use configuration file {}", config_path);
 
-    let config = CdhConfig::init(&config_path).await?;
+    let config = CdhConfig::new(cli.config)?;
     config.set_configuration_envs();
 
     let unix_socket_path = config
@@ -136,32 +123,3 @@ async fn create_socket_parent_directory(unix_socket_file: &str) -> Result<()> {
     fs::create_dir_all(parent_directory).await?;
     Ok(())
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn test_get_config_path() {
-        // --config takes precedence,
-        // then env.CDH_CONFIG_PATH,
-        // then DEFAULT_CONFIG_PATH.
-
-        let cli = Cli { config: None };
-        assert_eq!(get_config_path(cli), DEFAULT_CONFIG_PATH);
-
-        let cli = Cli {
-            config: Some("/thing".into()),
-        };
-        assert_eq!(get_config_path(cli), "/thing");
-
-        env::set_var("CDH_CONFIG_PATH", "/byenv");
-        let cli = Cli { config: None };
-        assert_eq!(get_config_path(cli), "/byenv");
-
-        let cli = Cli {
-            config: Some("/thing".into()),
-        };
-        assert_eq!(get_config_path(cli), "/thing");
-    }
-}