From df60725afe0ba452a25a740cf460c2855442c49a Mon Sep 17 00:00:00 2001 From: Xynnn007 Date: Mon, 24 Jun 2024 18:05:30 +0800 Subject: [PATCH] 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 --- .../src/config/aa_kbc_params.rs | 59 ++++++++++++------- .../attestation-agent/src/config/coco_as.rs | 15 ++--- .../attestation-agent/src/config/kbs.rs | 15 ++--- .../attestation-agent/src/config/mod.rs | 24 +++++++- .../attestation-agent/src/lib.rs | 6 +- .../hub/src/bin/config/mod.rs | 43 +++++--------- 6 files changed, 94 insertions(+), 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 index d8e10b483..16f7b58a4 100644 --- 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 for AaKbcParams { type Error = ParamError; @@ -36,30 +45,36 @@ impl TryFrom for AaKbcParams { } } -pub fn get_value() -> Result { - // 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 { + // 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 { + 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 { - let value = get_value()?; - value.try_into() -} + value.try_into() + } -fn from_cmdline() -> Result { - 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 { + 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 index a076e6620..81380d69c 100644 --- a/attestation-agent/attestation-agent/src/config/coco_as.rs +++ b/attestation-agent/attestation-agent/src/config/coco_as.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 CoCoASConfig { @@ -13,11 +14,11 @@ pub struct CoCoASConfig { 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 { + 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 index 1ccf7f07d..41ba5ce26 100644 --- 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, } -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 { + 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 index 15a3a604f..851cf4a46 100644 --- 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 { + 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 { + 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 { diff --git a/attestation-agent/attestation-agent/src/lib.rs b/attestation-agent/attestation-agent/src/lib.rs index e57551352..d40311d99 100644 --- 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 index b31acfdae..5d06627ac 100644 --- 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, } -impl Default for KbsConfig { - fn default() -> Self { +impl KbsConfig { + fn new() -> Result { 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) -> Result { 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(), + } } };