AA: fix launch configuration

Before this commit, if no configuration and proper aa_kbc_param given to AA, the launch of AA will panic. This commit will use a default value for aa_kbc_param for AA/CDH. If no env or kernel cmdline, default value offline_fs_kbc and null will be used. If a aa_kbc_param is specified, it will be parsed as kbc:url. If the aa_kbc_param is malwared, the launch process will still fail. Signed-off-by: Xynnn007 <[email protected]>
confidential-containers · Jun 24, 2024 · df60725 · df60725
1 parent 7afa473
commit df60725
Show file tree

Hide file tree

Showing 6 changed files with 94 additions and 68 deletions.
diff --git a/attestation-agent/attestation-agent/src/config/aa_kbc_params.rs b/attestation-agent/attestation-agent/src/config/aa_kbc_params.rs
@@ -17,6 +17,15 @@ pub struct AaKbcParams {
     pub uri: String,
 }
 
+impl Default for AaKbcParams {
+    fn default() -> Self {
+        Self {
+            kbc: "offline_fs_kbc".into(),
+            uri: "".into(),
+        }
+    }
+}
+
 impl TryFrom<String> for AaKbcParams {
     type Error = ParamError;
 
@@ -36,30 +45,36 @@ impl TryFrom<String> for AaKbcParams {
     }
 }
 
-pub fn get_value() -> Result<String, ParamError> {
-    // first check env
-    if let Ok(params) = env::var("AA_KBC_PARAMS") {
-        debug!("get aa_kbc_params from env.");
-        return Ok(params);
+impl AaKbcParams {
+    fn get_value() -> Result<String, ParamError> {
+        // first check env
+        if let Ok(params) = env::var("AA_KBC_PARAMS") {
+            debug!("get aa_kbc_params from env.");
+            return Ok(params);
+        }
+
+        // finally use the kernel cmdline
+        Self::from_cmdline()
     }
 
-    // finally use the kernel cmdline
-    from_cmdline()
-}
+    pub fn new() -> Result<Self, ParamError> {
+        let Ok(value) = Self::get_value() else {
+            debug!("failed to get aa_kbc_params in either both env or kernel cmdline, use `offline_fs_kbc::null` as default.");
+            return Ok(Self::default());
+        };
 
-pub fn get_params() -> Result<AaKbcParams, ParamError> {
-    let value = get_value()?;
-    value.try_into()
-}
+        value.try_into()
+    }
 
-fn from_cmdline() -> Result<String, ParamError> {
-    debug!("get aa_kbc_params from kernel cmdline");
-    let cmdline = std::fs::read_to_string("/proc/cmdline")?;
-    let value = cmdline
-        .split_ascii_whitespace()
-        .find(|para| para.starts_with("agent.aa_kbc_params="))
-        .ok_or(ParamError::MissingInCmdline)?
-        .strip_prefix("agent.aa_kbc_params=")
-        .expect("must have a prefix");
-    Ok(value.into())
+    fn from_cmdline() -> Result<String, ParamError> {
+        debug!("get aa_kbc_params from kernel cmdline");
+        let cmdline = std::fs::read_to_string("/proc/cmdline")?;
+        let value = cmdline
+            .split_ascii_whitespace()
+            .find(|para| para.starts_with("agent.aa_kbc_params="))
+            .ok_or(ParamError::MissingInCmdline)?
+            .strip_prefix("agent.aa_kbc_params=")
+            .expect("must have a prefix");
+        Ok(value.into())
+    }
 }
diff --git a/attestation-agent/attestation-agent/src/config/coco_as.rs b/attestation-agent/attestation-agent/src/config/coco_as.rs
@@ -3,21 +3,22 @@
 // SPDX-License-Identifier: Apache-2.0
 //
 
+use anyhow::Result;
 use serde::Deserialize;
 
-use super::aa_kbc_params;
+use super::aa_kbc_params::AaKbcParams;
 
 #[derive(Clone, Debug, Deserialize)]
 pub struct CoCoASConfig {
     /// URL Address of Attestation Service.
     pub url: String,
 }
 
-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.clone(),
-        }
+impl CoCoASConfig {
+    pub fn new() -> Result<Self> {
+        let aa_kbc_params = AaKbcParams::new()?;
+        Ok(Self {
+            url: aa_kbc_params.uri,
+        })
     }
 }
diff --git a/attestation-agent/attestation-agent/src/config/kbs.rs b/attestation-agent/attestation-agent/src/config/kbs.rs
@@ -3,9 +3,10 @@
 // SPDX-License-Identifier: Apache-2.0
 //
 
+use anyhow::Result;
 use serde::Deserialize;
 
-use super::aa_kbc_params;
+use super::aa_kbc_params::AaKbcParams;
 
 #[derive(Clone, Debug, Deserialize)]
 pub struct KbsConfig {
@@ -16,12 +17,12 @@ pub struct KbsConfig {
     pub cert: Option<String>,
 }
 
-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.clone(),
+impl KbsConfig {
+    pub fn new() -> Result<Self> {
+        let aa_kbc_params = AaKbcParams::new()?;
+        Ok(Self {
+            url: aa_kbc_params.uri,
             cert: None,
-        }
+        })
     }
 }
diff --git a/attestation-agent/attestation-agent/src/config/mod.rs b/attestation-agent/attestation-agent/src/config/mod.rs
@@ -16,14 +16,22 @@ pub mod kbs;
 
 pub const DEFAULT_AA_CONFIG_PATH: &str = "/etc/attestation-agent.conf";
 
-#[derive(Clone, Debug, Deserialize, Default)]
+#[derive(Clone, Debug, Deserialize)]
 pub struct Config {
     /// configs about token
     pub token_configs: TokenConfigs,
     // TODO: Add more fields that accessing AS needs.
 }
 
-#[derive(Clone, Debug, Deserialize, Default)]
+impl Config {
+    pub fn new() -> Result<Self> {
+        Ok(Self {
+            token_configs: TokenConfigs::new()?,
+        })
+    }
+}
+
+#[derive(Clone, Debug, Deserialize)]
 pub struct TokenConfigs {
     /// This config item is used when `coco_as` feature is enabled.
     #[cfg(feature = "coco_as")]
@@ -34,6 +42,18 @@ pub struct TokenConfigs {
     pub kbs: kbs::KbsConfig,
 }
 
+impl TokenConfigs {
+    pub fn new() -> Result<Self> {
+        Ok(Self {
+            #[cfg(feature = "coco_as")]
+            coco_as: coco_as::CoCoASConfig::new()?,
+
+            #[cfg(feature = "kbs")]
+            kbs: kbs::KbsConfig::new()?,
+        })
+    }
+}
+
 impl TryFrom<&str> for Config {
     type Error = config::ConfigError;
     fn try_from(config_path: &str) -> Result<Self, Self::Error> {

diff --git a/attestation-agent/attestation-agent/src/lib.rs b/attestation-agent/attestation-agent/src/lib.rs
@@ -73,13 +73,15 @@ pub struct AttestationAgent {
 }
 
 impl Default for AttestationAgent {
+    /// This function would panic if a malformed `aa_kbc_param` is given
+    /// either env or kernel cmdline
     fn default() -> Self {
         if let Ok(_config) = Config::try_from(config::DEFAULT_AA_CONFIG_PATH) {
             return AttestationAgent { _config };
         }
 
         AttestationAgent {
-            _config: Config::default(),
+            _config: Config::new().expect("AA initialize"),
         }
     }
 }
@@ -94,7 +96,7 @@ impl AttestationAgent {
             }
             None => {
                 warn!("No AA config file specified. Using a default configuration.");
-                Config::default()
+                Config::new()?
             }
         };
 

diff --git a/confidential-data-hub/hub/src/bin/config/mod.rs b/confidential-data-hub/hub/src/bin/config/mod.rs
@@ -6,6 +6,7 @@
 use std::{env, fs, path::Path};
 
 use anyhow::*;
+use attestation_agent::config::aa_kbc_params::AaKbcParams;
 use config::{Config, File};
 use log::{debug, info};
 use serde::Deserialize;
@@ -27,24 +28,16 @@ pub struct KbsConfig {
     pub kbs_cert: Option<String>,
 }
 
-impl Default for KbsConfig {
-    fn default() -> Self {
+impl KbsConfig {
+    fn new() -> Result<Self> {
         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,
-                }
-            }
-        }
+        let aa_kbc_params =
+            AaKbcParams::new().context("failed to read aa_kbc_params to initialize KbsConfig")?;
+        Ok(KbsConfig {
+            name: aa_kbc_params.kbc,
+            url: aa_kbc_params.uri,
+            kbs_cert: None,
+        })
     }
 }
 
@@ -64,16 +57,6 @@ pub struct CdhConfig {
     pub socket: String,
 }
 
-impl Default for CdhConfig {
-    fn default() -> Self {
-        Self {
-            socket: DEFAULT_CDH_SOCKET_ADDR.into(),
-            kbc: KbsConfig::default(),
-            credentials: Vec::default(),
-        }
-    }
-}
-
 impl CdhConfig {
     pub fn new(config_path: Option<String>) -> Result<Self> {
         let config_path = config_path.or_else(|| {
@@ -95,7 +78,11 @@ impl CdhConfig {
             }
             None => {
                 info!("No config path specified, use a default config.");
-                Self::default()
+                Self {
+                    kbc: KbsConfig::new()?,
+                    credentials: Vec::new(),
+                    socket: DEFAULT_CDH_SOCKET_ADDR.into(),
+                }
             }
         };