Skip to content

Commit

Permalink
CDH: fix aa_kbc_params and config reading
Browse files Browse the repository at this point in the history 
CDH should firstly read config specified with commandline, and then env.

aa_kbc_param should be firstly read directly from CDH's config file. If
there is no config file, try to read from env and then kernel cmdline.
If still no values found, use a default one with offline_fs_kbc.

Close confidential-containers#593

Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>
  • Loading branch information
@Xynnn007
Xynnn007 committed Jun 24, 2024
1 parent d7feaf8 commit 6f7a16f
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 134 deletions.
49 changes: 2 additions & 47 deletions attestation-agent/attestation-agent/src/config/aa_kbc_params.rs
View file
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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()
}
Expand All @@ -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")?;
Expand Down
106 changes: 81 additions & 25 deletions confidential-data-hub/hub/src/bin/config/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")] {
Expand All @@ -19,7 +18,7 @@ cfg_if::cfg_if! {
}
}

#[derive(Deserialize, Debug)]
#[derive(Deserialize, Debug, PartialEq)]
pub struct KbsConfig {
pub name: String,

Expand All @@ -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,

Expand All @@ -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 one.");
Self::default()
}
};

config.update_from_kernel_cmdline()?;
Ok(config)
}

Expand Down Expand Up @@ -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 update_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="))
Expand All @@ -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);
}
Expand All @@ -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(
Expand Down Expand Up @@ -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}"));
}
}
15 changes: 1 addition & 14 deletions confidential-data-hub/hub/src/bin/grpc-cdh.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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>()?;
Expand Down
44 changes: 1 addition & 43 deletions confidential-data-hub/hub/src/bin/ttrpc-cdh.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand All @@ -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
Expand Down Expand Up @@ -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");
}
}
12 changes: 7 additions & 5 deletions confidential-data-hub/kms/src/plugins/kbs/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ mod sev;

mod offline_fs;

use std::sync::Arc;
use std::{env, sync::Arc};

use async_trait::async_trait;
use attestation_agent::config::aa_kbc_params;
use attestation_agent::config::aa_kbc_params::AaKbcParams;
use lazy_static::lazy_static;
pub use resource_uri::ResourceUri;
use tokio::sync::Mutex;
Expand All @@ -33,11 +33,13 @@ enum RealClient {

impl RealClient {
async fn new() -> Result<Self> {
let params = aa_kbc_params::get_params()?;
let params = env::var("AA_KBC_PARAMS").expect("must be initialized");
let params = AaKbcParams::try_from(params)
.map_err(|e| Error::KbsClientError(format!("Failed to parse aa_kbc_params: {e}")))?;

let c = match params.kbc() {
let c = match &params.kbc[..] {
#[cfg(feature = "kbs")]
"cc_kbc" => RealClient::Cc(cc_kbc::CcKbc::new(params.uri()).await?),
"cc_kbc" => RealClient::Cc(cc_kbc::CcKbc::new(&params.uri).await?),
#[cfg(feature = "sev")]
"online_sev_kbc" => RealClient::Sev(sev::OnlineSevKbc::new(params.uri()).await?),
"offline_fs_kbc" => RealClient::OfflineFs(offline_fs::OfflineFsKbc::new().await?),
Expand Down

0 comments on commit 6f7a16f

Please sign in to comment.