github/kata-containers
1
0
Fork 1
mirror of https://github.com/kata-containers/kata-containers.git synced 2025-08-22 17:59:31 +00:00
Code Issues Projects Releases Wiki Activity

agent: Add PoC container signature validation

Browse Source 
- Add new agent configuration policy path parameter
- Update agent pull image to use the policy path if specified and
otherwise fall back to the accept all policy
- Remove the double copy of the image during pulling
- Ensure that temporary directories are always removed

Fixes: #2682
Signed-off-by: stevenhorsman <steven@uk.ibm.com>
This commit is contained in:
stevenhorsman 2021-11-11 16:10:07 +00:00 committed by Samuel Ortiz
parent 6f1bdd7079
commit d17aaba475
2 changed files with 161 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

114
src/agent/src/config.rs
View File

@ -23,6 +23,7 @@ const LOG_VPORT_OPTION: &str = "agent.log_vport";
const CONTAINER_PIPE_SIZE_OPTION: &str = "agent.container_pipe_size";
const UNIFIED_CGROUP_HIERARCHY_OPTION: &str = "agent.unified_cgroup_hierarchy";
const CONFIG_FILE: &str = "agent.config_file";
const CONTAINER_POLICY_FILE: &str = "agent.container_policy_file";
const DEFAULT_LOG_LEVEL: slog::Level = slog::Level::Info;
const DEFAULT_HOTPLUG_TIMEOUT: time::Duration = time::Duration::from_secs(3);
@ -51,6 +52,11 @@ 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_CONTAINER_POLICY_PATH_VALUE: &str = "invalid container_policy_file value";
const ERR_INVALID_CONTAINER_POLICY_PATH_KEY: &str = "invalid container_policy_file key";
const ERR_INVALID_CONTAINER_POLICY_ABSOLUTE: &str =
"container_policy_file path must be an absolute file path";
#[derive(Debug, Default, Deserialize)]
pub struct EndpointsConfig {
pub allowed: Vec<String>,
@ -76,6 +82,7 @@ pub struct AgentConfig {
pub tracing: bool,
pub endpoints: AgentEndpoints,
pub supports_seccomp: bool,
pub container_policy_path: String,
}
#[derive(Debug, Deserialize)]
@ -91,6 +98,7 @@ pub struct AgentConfigBuilder {
pub unified_cgroup_hierarchy: Option<bool>,
pub tracing: Option<bool>,
pub endpoints: Option<EndpointsConfig>,
pub container_policy_path: Option<String>,
}
macro_rules! config_override {
@ -152,6 +160,7 @@ impl Default for AgentConfig {
tracing: false,
endpoints: Default::default(),
supports_seccomp: rpc::have_seccomp(),
container_policy_path: String::from(""),
}
}
}
@ -180,6 +189,7 @@ impl FromStr for AgentConfig {
config_override!(agent_config_builder, agent_config, server_addr);
config_override!(agent_config_builder, agent_config, unified_cgroup_hierarchy);
config_override!(agent_config_builder, agent_config, tracing);
config_override!(agent_config_builder, agent_config, container_policy_path);
// Populate the allowed endpoints hash set, if we got any from the config file.
if let Some(endpoints) = agent_config_builder.endpoints {
@ -267,6 +277,13 @@ impl AgentConfig {
config.unified_cgroup_hierarchy,
get_bool_value
);
parse_cmdline_param!(
param,
CONTAINER_POLICY_FILE,
config.container_policy_path,
get_container_policy_path_value
);
}
if let Ok(addr) = env::var(SERVER_ADDR_ENV_VAR) {
@ -420,6 +437,29 @@ fn get_container_pipe_size(param: &str) -> Result<i32> {
Ok(value)
}
#[instrument]
fn get_container_policy_path_value(param: &str) -> Result<String> {
let fields: Vec<&str> = param.split('=').collect();
ensure!(!fields[0].is_empty(), ERR_INVALID_CONTAINER_POLICY_PATH_KEY);
ensure!(fields.len() == 2, ERR_INVALID_CONTAINER_POLICY_PATH_VALUE);
let key = fields[0];
ensure!(
key == CONTAINER_POLICY_FILE,
ERR_INVALID_CONTAINER_POLICY_PATH_KEY
);
let value = String::from(fields[1]);
ensure!(!value.is_empty(), ERR_INVALID_CONTAINER_POLICY_PATH_VALUE);
ensure!(
value.starts_with('/'),
ERR_INVALID_CONTAINER_POLICY_ABSOLUTE
);
ensure!(!value.contains(".."), ERR_INVALID_CONTAINER_POLICY_ABSOLUTE);
Ok(value)
}
#[cfg(test)]
mod tests {
use super::*;
@ -462,6 +502,7 @@ mod tests {
assert!(!config.dev_mode);
assert_eq!(config.log_level, DEFAULT_LOG_LEVEL);
assert_eq!(config.hotplug_timeout, DEFAULT_HOTPLUG_TIMEOUT);
assert_eq!(config.container_policy_path, "");
}
#[test]
@ -480,6 +521,7 @@ mod tests {
server_addr: &'a str,
unified_cgroup_hierarchy: bool,
tracing: bool,
container_policy_path: &'a str,
}
impl Default for TestData<'_> {
@ -495,6 +537,7 @@ mod tests {
server_addr: TEST_SERVER_ADDR,
unified_cgroup_hierarchy: false,
tracing: false,
container_policy_path: "",
}
}
}
@ -864,6 +907,11 @@ mod tests {
tracing: true,
..Default::default()
},
TestData {
contents: "agent.container_policy_file=/etc/containers/policy.json",
container_policy_path: "/etc/containers/policy.json",
..Default::default()
},
];
let dir = tempdir().expect("failed to create tmpdir");
@ -1347,6 +1395,72 @@ Caused by:
}
}
#[test]
fn test_get_container_policy_path_value() {
#[derive(Debug)]
struct TestData<'a> {
param: &'a str,
result: Result<String>,
}
let tests = &[
TestData {
param: "",
result: Err(anyhow!(ERR_INVALID_CONTAINER_POLICY_PATH_KEY)),
},
TestData {
param: "agent.container_policy_file",
result: Err(anyhow!(ERR_INVALID_CONTAINER_POLICY_PATH_VALUE)),
},
TestData {
param: "agent.container_policy_file=",
result: Err(anyhow!(ERR_INVALID_CONTAINER_POLICY_PATH_VALUE)),
},
TestData {
param: "foo=bar",
result: Err(anyhow!(ERR_INVALID_CONTAINER_POLICY_PATH_KEY)),
},
TestData {
param: "agent.policy_path=/another/absolute/path.json",
result: Err(anyhow!(ERR_INVALID_CONTAINER_POLICY_PATH_KEY)),
},
TestData {
param: "agent.container_policy_file=/etc/container/policy.json",
result: Ok("/etc/container/policy.json".into()),
},
TestData {
param: "agent.container_policy_file=/another/absolute/path.json",
result: Ok("/another/absolute/path.json".into()),
},
TestData {
param: "agent.container_policy_file=./relative/path.json",
result: Err(anyhow!(ERR_INVALID_CONTAINER_POLICY_ABSOLUTE)),
},
TestData {
param: "agent.container_policy_file=./relative/path.json",
result: Err(anyhow!(ERR_INVALID_CONTAINER_POLICY_ABSOLUTE)),
},
TestData {
param: "agent.container_policy_file=../../relative/path.json",
result: Err(anyhow!(ERR_INVALID_CONTAINER_POLICY_ABSOLUTE)),
},
TestData {
param: "agent.container_policy_file=junk_string",
result: Err(anyhow!(ERR_INVALID_CONTAINER_POLICY_ABSOLUTE)),
},
];
for (i, d) in tests.iter().enumerate() {
let msg = format!("test[{}]: {:?}", i, d);
let result = get_container_policy_path_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(

92
src/agent/src/image_rpc.rs
View File

@ -4,7 +4,7 @@
//
use std::fs;
use std::path::PathBuf;
use std::path::Path;
use std::process::{Command, ExitStatus};
use std::sync::Arc;
@ -16,6 +16,7 @@ use ttrpc::{self, error::get_rpc_status as ttrpc_error};
use crate::rpc::{verify_cid, CONTAINER_BASE};
use crate::sandbox::Sandbox;
use crate::AGENT_CONFIG;
const SKOPEO_PATH: &str = "/usr/bin/skopeo";
const UMOCI_PATH: &str = "/usr/local/bin/umoci";
@ -37,73 +38,69 @@ impl ImageService {
Self { sandbox }
}
fn build_oci_path(cid: &str) -> PathBuf {
let mut oci_path = PathBuf::from("/tmp");
oci_path.push(cid);
oci_path.push(IMAGE_OCI);
oci_path
}
fn pull_image_from_registry(image: &str, cid: &str, source_creds: &Option<&str>) -> Result<()> {
fn pull_image_from_registry(
image: &str,
cid: &str,
source_creds: &Option<&str>,
policy_path: &Option<&String>,
) -> Result<()> {
let source_image = format!("{}{}", "docker://", image);
let mut manifest_path = PathBuf::from("/tmp");
manifest_path.push(cid);
manifest_path.push("image_manifest");
let target_path_manifest = format!("dir://{}", manifest_path.to_string_lossy());
// Define the target transport and path for the OCI image, without signature
let oci_path = Self::build_oci_path(cid);
let tmp_cid_path = Path::new("/tmp/").join(cid);
let oci_path = tmp_cid_path.join(IMAGE_OCI);
let target_path_oci = format!("oci://{}", oci_path.to_string_lossy());
fs::create_dir_all(&manifest_path)?;
fs::create_dir_all(&oci_path)?;
info!(sl!(), "Attempting to pull image {}...", &source_image);
let mut pull_command = Command::new(SKOPEO_PATH);
pull_command
// TODO: need to create a proper policy
.arg("--insecure-policy")
.arg("copy")
.arg(source_image)
.arg(&target_path_manifest);
.arg(&target_path_oci)
.arg("--remove-signatures"); //umoci requires signatures to be removed
// If source credentials were passed (so not using an anonymous registry), pass them through
if let Some(source_creds) = source_creds {
pull_command.arg("--src-creds").arg(source_creds);
}
// If a policy_path provided, use it, otherwise fall back to allow all image registries
if let Some(policy_path) = policy_path {
pull_command.arg("--policy").arg(policy_path);
} else {
info!(
sl!(),
"No policy path was supplied, so revert to allow all images to be pulled."
);
pull_command.arg("--insecure-policy");
}
debug!(sl!(), "skopeo command: {:?}", &pull_command);
let status: ExitStatus = pull_command.status()?;
ensure!(
status.success(),
"failed to copy image manifest: {:?}",
status,
);
// Copy image from one local file-system to another
// Resulting image is still stored in manifest format, but no longer includes the signature
// The image with a signature can then be unpacked into a bundle
let status: ExitStatus = Command::new(SKOPEO_PATH)
.arg("--insecure-policy")
.arg("copy")
.arg(&target_path_manifest)
.arg(&target_path_oci)
.arg("--remove-signatures")
.status()?;
if !status.success() {
let mut error_message = format!("failed to pull image: {:?}", status);
ensure!(status.success(), "failed to copy image oci: {:?}", status);
// To save space delete the manifest.
// TODO LATER - when verify image is added, this will need moving the end of that, if required
fs::remove_dir_all(&manifest_path)?;
if let Err(e) = fs::remove_dir_all(&tmp_cid_path) {
error_message.push_str(&format!(
" and clean up of temporary container directory {:?} failed with error {:?}",
tmp_cid_path, e
));
};
return Err(anyhow!(error_message));
}
Ok(())
}
fn unpack_image(cid: &str) -> Result<()> {
let source_path_oci = Self::build_oci_path(cid);
let target_path_bundle = format!("{}{}{}", CONTAINER_BASE, "/", cid);
let tmp_cid_path = Path::new("/tmp/").join(cid);
let source_path_oci = tmp_cid_path.join(IMAGE_OCI);
info!(sl!(), "unpack image"; "cid" => cid, "target_bundle_path" => &target_path_bundle);
let target_path_bundle = Path::new(CONTAINER_BASE).join(cid);
info!(sl!(), "unpack image {:?} to {:?}", cid, target_path_bundle);
// Unpack image
let status: ExitStatus = Command::new(UMOCI_PATH)
@ -116,7 +113,7 @@ impl ImageService {
ensure!(status.success(), "failed to unpack image: {:?}", status);
// To save space delete the oci image after unpack
fs::remove_dir_all(&source_path_oci)?;
fs::remove_dir_all(&tmp_cid_path)?;
Ok(())
}
@ -138,7 +135,12 @@ impl ImageService {
let source_creds = (!req.get_source_creds().is_empty()).then(|| req.get_source_creds());
Self::pull_image_from_registry(image, cid, &source_creds)?;
// Read the policy path from the agent config
let config_policy_path = &AGENT_CONFIG.read().await.container_policy_path;
let policy_path = (!config_policy_path.is_empty()).then(|| config_policy_path);
info!(sl!(), "Using container policy_path: {:?}...", &policy_path);
Self::pull_image_from_registry(image, cid, &source_creds, &policy_path)?;
Self::unpack_image(cid)?;
let mut sandbox = self.sandbox.lock().await;