From b509c1beee75883a9f4a224ddd7b67efef4dcc00 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Mon, 22 Apr 2024 19:46:33 +0000 Subject: [PATCH 1/7] agent: lock anyhow version to 1.0.58 Lock anyhow version to 1.0.58 because: - Versions between 1.0.59 - 1.0.76 have not been tested yet using Kata CI. However, those versions pass "make test" for the Kata Agent. - Versions 1.0.77 or newer fail during "make test" - see https://github.com/kata-containers/kata-containers/issues/9538. Signed-off-by: Dan Mihai --- src/agent/Cargo.toml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/agent/Cargo.toml b/src/agent/Cargo.toml index 470d4050da..12a7a09ea6 100644 --- a/src/agent/Cargo.toml +++ b/src/agent/Cargo.toml @@ -56,7 +56,14 @@ log = "0.4.11" cfg-if = "1.0.0" prometheus = { version = "0.13.0", features = ["process"] } procfs = "0.12.0" -anyhow = "1.0.32" + +# anyhow is currently locked at 1.0.58 because: +# - Versions between 1.0.59 - 1.0.76 have not been tested yet using Kata CI. +# However, those versions are passing "make test" for the Kata Agent. +# - Versions 1.0.77 or newer fail during "make test" - see +# https://github.com/kata-containers/kata-containers/issues/9538 +anyhow = "=1.0.58" + cgroups = { package = "cgroups-rs", version = "0.3.3" } # Tracing From 58e608d61a8992dd594338f7725cb770edff9b69 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Thu, 18 Apr 2024 02:15:13 +0000 Subject: [PATCH 2/7] tests: remove k8s-policy-set-keys.bats Remove k8s-policy-set-keys.bats in preparation for using the regorus crate instead of the OPA daemon for evaluating the Agent Policy. This test depended on sending HTTP requests to OPA. Fixes: #9388 Signed-off-by: Dan Mihai --- .../kubernetes/k8s-policy-set-keys.bats | 46 ------------------- .../kubernetes/run_kubernetes_tests.sh | 1 - 2 files changed, 47 deletions(-) delete mode 100644 tests/integration/kubernetes/k8s-policy-set-keys.bats diff --git a/tests/integration/kubernetes/k8s-policy-set-keys.bats b/tests/integration/kubernetes/k8s-policy-set-keys.bats deleted file mode 100644 index d1de0018f7..0000000000 --- a/tests/integration/kubernetes/k8s-policy-set-keys.bats +++ /dev/null @@ -1,46 +0,0 @@ -#!/usr/bin/env bats -# -# Copyright (c) 2023 Microsoft. -# -# SPDX-License-Identifier: Apache-2.0 -# - -load "${BATS_TEST_DIRNAME}/../../common.bash" -load "${BATS_TEST_DIRNAME}/tests_common.sh" - -setup() { - policy_tests_enabled || skip "Policy tests are disabled." - - get_pod_config_dir - pod_name="set-keys-test" - pod_yaml="${pod_config_dir}/k8s-policy-set-keys.yaml" - set_keys_policy=$(base64 -w 0 "${pod_config_dir}/k8s-policy-set-keys.rego") -} - -@test "Set guest keys using policy" { - yq write -i "${pod_yaml}" \ - 'metadata.annotations."io.katacontainers.config.agent.policy"' \ - "${set_keys_policy}" - - # Create the pod - kubectl create -f "${pod_yaml}" - - # Wait for pod to start - kubectl wait --for=condition=Ready --timeout=$timeout pod "$pod_name" - - # Obtain the keys from the policy by querying the OPA service - my_test_data="http://localhost:8181/v1/data/agent_policy/my_test_data" - kubectl exec "$pod_name" -- wget -O - "$my_test_data/default/key/ssh-demo" | grep "{\"result\":\"HUlOu8NWz8si11OZUzUJMnjiq/iZyHBJZMSD3BaqgMc=\"}" - kubectl exec "$pod_name" -- wget -O - "$my_test_data/default/key/enabled" | grep "{\"result\":false}" - kubectl exec "$pod_name" -- wget -O - "$my_test_data/key1" | grep "{\"result\":\[\"abc\",\"9876\",\"xyz\"\]}" - kubectl exec "$pod_name" -- wget -O - "$my_test_data/key2" | grep "{\"result\":45}" -} - -teardown() { - policy_tests_enabled || skip "Policy tests are disabled." - - # Debugging information - kubectl describe "pod/$pod_name" - - kubectl delete pod "$pod_name" -} diff --git a/tests/integration/kubernetes/run_kubernetes_tests.sh b/tests/integration/kubernetes/run_kubernetes_tests.sh index ebbb0dfb7b..7aae576a82 100755 --- a/tests/integration/kubernetes/run_kubernetes_tests.sh +++ b/tests/integration/kubernetes/run_kubernetes_tests.sh @@ -54,7 +54,6 @@ else "k8s-policy-job.bats" \ "k8s-policy-pod.bats" \ "k8s-policy-rc.bats" \ - "k8s-policy-set-keys.bats" \ "k8s-port-forward.bats" \ "k8s-projected-volume.bats" \ "k8s-qos-pods.bats" \ From df23eb09a6b7a229f5c099d8a04b799a97ced141 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Mon, 22 Apr 2024 19:58:30 +0000 Subject: [PATCH 3/7] agent: use regorus instead of opa Implement Agent Policy using the regorus crate instead of the OPA daemon. The OPA daemon will be removed from the Guest rootfs in a future PR. Fixes: #9388 Signed-off-by: Dan Mihai --- src/agent/Cargo.lock | 149 ++++++++++++++++++----- src/agent/Cargo.toml | 14 +-- src/agent/src/main.rs | 16 +-- src/agent/src/policy.rs | 260 ++++++++++------------------------------ 4 files changed, 191 insertions(+), 248 deletions(-) diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index 4a1d4757e3..f7e94eaa6f 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -1997,6 +1997,15 @@ dependencies = [ "either", ] +[[package]] +name = "itertools" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba291022dbbd398a455acf126c1e341954079855bc60dfdda641363bd6922569" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.10" @@ -2103,7 +2112,6 @@ dependencies = [ "clap", "const_format", "futures", - "http", "image-rs", "ipnetwork", "kata-sys-util", @@ -2124,7 +2132,7 @@ dependencies = [ "protobuf 3.2.0", "protocols", "regex", - "reqwest", + "regorus", "rstest", "rtnetlink", "rustjail", @@ -2443,9 +2451,9 @@ checksum = "0e7465ac9959cc2b1404e8e2367b43684a6d13790fe23056cc8c6c5a6b7bcb94" [[package]] name = "memchr" -version = "2.5.0" +version = "2.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2dffe52ecf27772e601905b7522cb4ef790d2cc203488bbd0e2fe85fcb74566d" +checksum = "6c8640c5d730cb13ebd907d8d04b52f55ac9a2eec55b440c8892f40d56c76c1d" [[package]] name = "memoffset" @@ -2675,6 +2683,31 @@ dependencies = [ "winapi", ] +[[package]] +name = "num" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3135b08af27d103b0a51f2ae0f8632117b7b185ccf931445affa8df530576a41" +dependencies = [ + "num-bigint", + "num-complex", + "num-integer", + "num-iter", + "num-rational", + "num-traits", +] + +[[package]] +name = "num-bigint" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "608e7659b5c3d7cba262d894801b9ec9d00de989e8a82bd4bef91d08da45cdc0" +dependencies = [ + "autocfg", + "num-integer", + "num-traits", +] + [[package]] name = "num-bigint-dig" version = "0.8.4" @@ -2693,12 +2726,20 @@ dependencies = [ ] [[package]] -name = "num-integer" -version = "0.1.45" +name = "num-complex" +version = "0.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "225d3389fb3509a24c93f5c29eb6bde2586b98d9f016636dff58d7c6f7569cd9" +checksum = "23c6602fda94a57c990fe0df199a035d83576b496aa29f4e634a8ac6004e68a6" +dependencies = [ + "num-traits", +] + +[[package]] +name = "num-integer" +version = "0.1.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7969661fd2958a5cb096e56c8e1ad0444ac2bbcd0061bd28660485a44879858f" dependencies = [ - "autocfg", "num-traits", ] @@ -2714,10 +2755,22 @@ dependencies = [ ] [[package]] -name = "num-traits" -version = "0.2.15" +name = "num-rational" +version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "578ede34cf02f8924ab9447f50c28075b4d3e5b269972345e7e0372b38c6cdcd" +checksum = "0638a1c9d0a3c0914158145bc76cff373a75a627e6ecbfb71cbe6f453a5a19b0" +dependencies = [ + "autocfg", + "num-bigint", + "num-integer", + "num-traits", +] + +[[package]] +name = "num-traits" +version = "0.2.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da0df0e5185db44f69b44f26786fe401b6c293d1907744beaa7fa62b2e5a517a" dependencies = [ "autocfg", "libm", @@ -3348,7 +3401,7 @@ checksum = "355f634b43cdd80724ee7848f95770e7e70eefa6dcf14fea676216573b8fd603" dependencies = [ "bytes 1.5.0", "heck 0.3.3", - "itertools", + "itertools 0.10.3", "log", "multimap", "petgraph 0.5.1", @@ -3366,7 +3419,7 @@ checksum = "119533552c9a7ffacc21e099c24a0ac8bb19c2a2a3f363de84cd9b844feab270" dependencies = [ "bytes 1.5.0", "heck 0.4.1", - "itertools", + "itertools 0.10.3", "lazy_static", "log", "multimap", @@ -3387,7 +3440,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "600d2f334aa05acb02a755e217ef1ab6dea4d51b58b7846588b747edec04efba" dependencies = [ "anyhow", - "itertools", + "itertools 0.10.3", "proc-macro2", "quote", "syn 1.0.109", @@ -3400,7 +3453,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5d2d8d10f3c6ded6da8b05b5fb3b8a5082514344d56c9f871412d29b4e075b4" dependencies = [ "anyhow", - "itertools", + "itertools 0.10.3", "proc-macro2", "quote", "syn 1.0.109", @@ -3626,14 +3679,14 @@ dependencies = [ [[package]] name = "regex" -version = "1.9.4" +version = "1.10.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "12de2eff854e5fa4b1295edd650e227e9d8fb0c9e90b12e7f36d6a6811791a29" +checksum = "c117dbdfde9c8308975b6a18d71f3f385c89461f7b3fb054288ecf2a2058ba4c" dependencies = [ "aho-corasick", "memchr", - "regex-automata 0.3.7", - "regex-syntax 0.7.5", + "regex-automata 0.4.6", + "regex-syntax 0.8.3", ] [[package]] @@ -3647,13 +3700,13 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.3.7" +version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "49530408a136e16e5b486e883fbb6ba058e8e4e8ae6621a77b048b314336e629" +checksum = "86b83b8b9847f9bf95ef68afb0b8e6cdb80f498442f5179a29fad448fcc1eaea" dependencies = [ "aho-corasick", "memchr", - "regex-syntax 0.7.5", + "regex-syntax 0.8.3", ] [[package]] @@ -3664,9 +3717,25 @@ checksum = "49b3de9ec5dc0a3417da371aab17d729997c15010e7fd24ff707773a33bddb64" [[package]] name = "regex-syntax" -version = "0.7.5" +version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dbb5fb1acd8a1a18b3dd5be62d25485eb770e05afb408a9627d14d451bae12da" +checksum = "adad44e29e4c806119491a7f06f03de4d1af22c3a680dd47f1e6e179439d1f56" + +[[package]] +name = "regorus" +version = "0.1.3" +source = "git+https://github.com/microsoft/regorus?rev=316f3a76#316f3a7692fe7dbb9e71890174d549de3da939f8" +dependencies = [ + "anyhow", + "itertools 0.12.1", + "lazy_static", + "num", + "rand", + "regex", + "scientific", + "serde", + "serde_json", +] [[package]] name = "relative-path" @@ -4113,6 +4182,26 @@ dependencies = [ "uriparse", ] +[[package]] +name = "scientific" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc53198b8e237c451c68dba8411a1f8bd92787657689f24d67ae3d6b98c39f59" +dependencies = [ + "scientific-macro", +] + +[[package]] +name = "scientific-macro" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2ee4885492bb655bfa05d039cd9163eb8fe9f79ddebf00ca23a1637510c2fd2" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.52", +] + [[package]] name = "scopeguard" version = "1.1.0" @@ -4192,9 +4281,9 @@ checksum = "92d43fe69e652f3df9bdc2b85b2854a0825b86e4fb76bc44d945137d053639ca" [[package]] name = "serde" -version = "1.0.164" +version = "1.0.198" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e8c8cf938e98f769bc164923b06dce91cea1751522f46f8466461af04c9027d" +checksum = "9846a40c979031340571da2545a4e5b7c4163bdae79b301d5f86d03979451fcc" dependencies = [ "serde_derive", ] @@ -4231,9 +4320,9 @@ checksum = "794e44574226fc701e3be5c651feb7939038fc67fb73f6f4dd5c4ba90fd3be70" [[package]] name = "serde_derive" -version = "1.0.164" +version = "1.0.198" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d9735b638ccc51c28bf6914d90a2e9725b377144fc612c49a611fddd1b631d68" +checksum = "e88edab869b01783ba905e7d0153f9fc1a6505a96e4ad3018011eedb838566d9" dependencies = [ "proc-macro2", "quote", @@ -4242,9 +4331,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.81" +version = "1.0.116" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b7ce2b32a1aed03c558dc61a5cd328f15aff2dbc17daad8fb8af04d2100e15c" +checksum = "3e17db7126d17feb94eb3fad46bf1a96b034e8aacbc2e775fe81505f8b0b2813" dependencies = [ "itoa", "ryu", diff --git a/src/agent/Cargo.toml b/src/agent/Cargo.toml index 12a7a09ea6..0445f30e61 100644 --- a/src/agent/Cargo.toml +++ b/src/agent/Cargo.toml @@ -19,7 +19,7 @@ serde_json = "1.0.39" scan_fmt = "0.2.3" scopeguard = "1.0.0" thiserror = "1.0.26" -regex = "1.5.6" +regex = "1.10.4" serial_test = "0.5.1" oci-distribution = "0.10.0" url = "2.5.0" @@ -80,14 +80,12 @@ clap = { version = "3.0.1", features = ["derive"] } strum = "0.26.2" strum_macros = "0.26.2" -# Communication with the OPA service -http = { version = "0.2.8", optional = true } -reqwest = { version = "0.11.14", optional = true } -# The "vendored" feature for openssl is required for musl build -openssl = { version = "0.10.54", features = ["vendored"], optional = true } - # Image pull/decrypt image-rs = { git = "https://github.com/confidential-containers/guest-components", rev = "ca6b438", default-features = true, optional = true } +openssl = { version = "0.10.54", features = ["vendored"], optional = true } + +# Agent Policy +regorus = { git = "https://github.com/microsoft/regorus", rev = "316f3a76", default-features = false, features = ["arc", "regex"], optional = true } [dev-dependencies] tempfile = "3.1.0" @@ -107,7 +105,7 @@ lto = true default-pull = ["guest-pull"] seccomp = ["rustjail/seccomp"] standard-oci-runtime = ["rustjail/standard-oci-runtime"] -agent-policy = ["http", "openssl", "reqwest"] +agent-policy = ["regorus"] guest-pull = ["image-rs", "openssl"] [[bin]] diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index 38d51e552f..be91b2bf23 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -379,14 +379,9 @@ async fn start_sandbox( #[cfg(feature = "guest-pull")] image::set_proxy_env_vars().await; - // - When init_mode is true, enabling the localhost link during the - // handle_localhost call above is required before starting OPA with the - // initialize_policy call below. - // - When init_mode is false, the Policy could be initialized earlier, - // because initialize_policy doesn't start OPA. OPA is started by - // systemd after localhost has been enabled. + // TODO: initialize earlier. #[cfg(feature = "agent-policy")] - if let Err(e) = initialize_policy(init_mode).await { + if let Err(e) = initialize_policy().await { error!(logger, "Failed to initialize agent policy: {:?}", e); // Continuing execution without a security policy could be dangerous. std::process::abort(); @@ -541,14 +536,11 @@ fn init_agent_as_init(logger: &Logger, unified_cgroup_hierarchy: bool) -> Result } #[cfg(feature = "agent-policy")] -async fn initialize_policy(init_mode: bool) -> Result<()> { - let opa_addr = "localhost:8181"; - let agent_policy_path = "/agent_policy"; - let default_agent_policy = "/etc/kata-opa/default-policy.rego"; +async fn initialize_policy() -> Result<()> { AGENT_POLICY .lock() .await - .initialize(init_mode, opa_addr, agent_policy_path, default_agent_policy) + .initialize("/etc/kata-opa/default-policy.rego") .await } diff --git a/src/agent/src/policy.rs b/src/agent/src/policy.rs index 6809ae51a4..d709515ffd 100644 --- a/src/agent/src/policy.rs +++ b/src/agent/src/policy.rs @@ -3,21 +3,14 @@ // SPDX-License-Identifier: Apache-2.0 // -use anyhow::{bail, Result}; +use anyhow::Result; use protobuf::MessageDyn; -use serde::{Deserialize, Serialize}; use slog::Drain; use tokio::io::AsyncWriteExt; -use tokio::time::{sleep, Duration}; use crate::rpc::ttrpc_error; use crate::AGENT_POLICY; -static EMPTY_JSON_INPUT: &str = "{\"input\":{}}"; - -static OPA_DATA_PATH: &str = "/data"; -static OPA_POLICIES_PATH: &str = "/policies"; - static POLICY_LOG_FILE: &str = "/tmp/policy.txt"; /// Convenience macro to obtain the scope logger @@ -28,14 +21,21 @@ macro_rules! sl { } async fn allow_request(policy: &mut AgentPolicy, ep: &str, request: &str) -> ttrpc::Result<()> { - if !policy.allow_request(ep, request).await { - warn!(sl!(), "{ep} is blocked by policy"); - Err(ttrpc_error( - ttrpc::Code::PERMISSION_DENIED, - format!("{ep} is blocked by policy"), - )) - } else { - Ok(()) + match policy.allow_request(ep, request).await { + Ok((allowed, prints)) => { + if allowed { + Ok(()) + } else { + Err(ttrpc_error( + ttrpc::Code::PERMISSION_DENIED, + format!("{ep} is blocked by policy: {prints}"), + )) + } + } + Err(e) => Err(ttrpc_error( + ttrpc::Code::INTERNAL, + format!("{ep}: internal error {e}"), + )), } } @@ -55,32 +55,17 @@ pub async fn do_set_policy(req: &protocols::agent::SetPolicyRequest) -> ttrpc::R .map_err(|e| ttrpc_error(ttrpc::Code::INVALID_ARGUMENT, e)) } -/// Example of HTTP response from OPA: {"result":true} -#[derive(Debug, Serialize, Deserialize)] -struct AllowResponse { - result: bool, -} - /// Singleton policy object. #[derive(Debug, Default)] pub struct AgentPolicy { /// When true policy errors are ignored, for debug purposes. allow_failures: bool, - /// OPA path used to query if an Agent gRPC request should be allowed. - /// The request name (e.g., CreateContainerRequest) must be added to - /// this path. - query_path: String, - - /// OPA path used to add or delete a rego format Policy. - policy_path: String, - - /// Client used to connect a single time to the OPA service and reused - /// for all the future communication with OPA. - opa_client: Option, - /// "/tmp/policy.txt" log file for policy activity. log_file: Option, + + /// Regorus engine + engine: regorus::Engine, } impl AgentPolicy { @@ -88,18 +73,20 @@ impl AgentPolicy { pub fn new() -> Self { Self { allow_failures: false, + engine: Self::new_engine(), ..Default::default() } } - /// Wait for OPA to start and connect to it. - pub async fn initialize( - &mut self, - launch_opa: bool, - opa_addr: &str, - policy_name: &str, - default_policy: &str, - ) -> Result<()> { + fn new_engine() -> regorus::Engine { + let mut engine = regorus::Engine::new(); + engine.set_strict_builtin_errors(false); + engine.set_gather_prints(true); + engine + } + + /// Initialize regorus. + pub async fn initialize(&mut self, default_policy_file: &str) -> Result<()> { if sl!().is_enabled(slog::Level::Debug) { self.log_file = Some( tokio::fs::OpenOptions::new() @@ -112,147 +99,45 @@ impl AgentPolicy { debug!(sl!(), "policy: log file: {}", POLICY_LOG_FILE); } - if launch_opa { - start_opa(opa_addr)?; - } - - let opa_uri = format!("http://{opa_addr}/v1"); - self.query_path = format!("{opa_uri}{OPA_DATA_PATH}{policy_name}/"); - self.policy_path = format!("{opa_uri}{OPA_POLICIES_PATH}{policy_name}"); - let opa_client = reqwest::Client::builder().http1_only().build()?; - let policy = tokio::fs::read_to_string(default_policy).await?; - - // This loop is necessary to get the opa_client connected to the - // OPA service while that service is starting. Future requests to - // OPA are expected to work without retrying, after connecting - // successfully for the first time. - for i in 0..50 { - if i > 0 { - sleep(Duration::from_millis(100)).await; - debug!(sl!(), "policy initialize: PUT failed, retrying"); - } - - // Set-up the default policy. - if opa_client - .put(&self.policy_path) - .body(policy.clone()) - .send() - .await - .is_ok() - { - self.opa_client = Some(opa_client); - - // Check if requests causing policy errors should actually - // be allowed. That is an insecure configuration but is - // useful for allowing insecure pods to start, then connect to - // them and inspect Guest logs for the root cause of a failure. - // - // Note that post_query returns Ok(false) in case - // AllowRequestsFailingPolicy was not defined in the policy. - self.allow_failures = self - .post_query("AllowRequestsFailingPolicy", EMPTY_JSON_INPUT) - .await?; - return Ok(()); - } - } - bail!("Failed to connect to OPA") + self.engine.add_policy_from_file(default_policy_file)?; + self.engine.set_input_json("{}")?; + self.allow_failures = match self.allow_request("AllowRequestsFailingPolicy", "{}").await { + Ok((allowed, _prints)) => allowed, + Err(_) => false, + }; + Ok(()) } - /// Ask OPA to check if an API call should be allowed or not. - pub async fn allow_request(&mut self, ep: &str, request: &str) -> bool { - let post_input = format!("{{\"input\":{request}}}"); - self.log_opa_input(ep, &post_input).await; - match self.post_query(ep, &post_input).await { - Err(e) => { - debug!( - sl!(), - "policy: failed to query endpoint {}: {:?}. Returning false.", ep, e - ); - false - } - Ok(allowed) => allowed, - } - } - - /// Replace the Policy in OPA. - pub async fn set_policy(&mut self, policy: &str) -> Result<()> { - if let Some(opa_client) = &mut self.opa_client { - // Delete the old rules. - opa_client.delete(&self.policy_path).send().await?; - - // Put the new rules. - opa_client - .put(&self.policy_path) - .body(policy.to_string()) - .send() - .await?; - - // Check if requests causing policy errors should actually be allowed. - // That is an insecure configuration but is useful for allowing insecure - // pods to start, then connect to them and inspect Guest logs for the - // root cause of a failure. - // - // Note that post_query returns Ok(false) in case - // AllowRequestsFailingPolicy was not defined in the policy. - self.allow_failures = self - .post_query("AllowRequestsFailingPolicy", EMPTY_JSON_INPUT) - .await?; - - Ok(()) - } else { - bail!("Agent Policy is not initialized") - } - } - - // Post query to OPA. - async fn post_query(&mut self, ep: &str, post_input: &str) -> Result { + /// Ask regorus if an API call should be allowed or not. + async fn allow_request(&mut self, ep: &str, ep_input: &str) -> Result<(bool, String)> { debug!(sl!(), "policy check: {ep}"); + self.log_eval_input(ep, ep_input).await; - if let Some(opa_client) = &mut self.opa_client { - let uri = format!("{}{ep}", &self.query_path); - let response = opa_client - .post(uri) - .body(post_input.to_string()) - .send() - .await?; + let query = format!("data.agent_policy.{ep}"); + self.engine.set_input_json(ep_input)?; - if response.status() != http::StatusCode::OK { - bail!("policy: POST {} response status {}", ep, response.status()); - } - - let http_response = response.text().await?; - let opa_response: serde_json::Result = - serde_json::from_str(&http_response); - - match opa_response { - Ok(resp) => { - if !resp.result { - if self.allow_failures { - warn!( - sl!(), - "policy: POST {} response <{}>. Ignoring error!", ep, http_response - ); - return Ok(true); - } else { - error!(sl!(), "policy: POST {} response <{}>", ep, http_response); - } - } - Ok(resp.result) - } - Err(_) => { - warn!( - sl!(), - "policy: endpoint {} not found in policy. Returning false.", ep, - ); - Ok(false) - } - } - } else { - bail!("Agent Policy is not initialized") + let mut allow = self.engine.eval_bool_query(query, false)?; + if !allow && self.allow_failures { + allow = true; } + + let prints = match self.engine.take_prints() { + Ok(p) => p.join(" "), + Err(e) => format!("Failed to get policy log: {e}"), + }; + + Ok((allow, prints)) } - async fn log_opa_input(&mut self, ep: &str, input: &str) { + /// Replace the Policy in regorus. + pub async fn set_policy(&mut self, policy: &str) -> Result<()> { + self.engine = Self::new_engine(); + self.engine + .add_policy("agent_policy".to_string(), policy.to_string())?; + Ok(()) + } + + async fn log_eval_input(&mut self, ep: &str, input: &str) { if let Some(log_file) = &mut self.log_file { match ep { "StatsContainerRequest" | "ReadStreamRequest" | "SetPolicyRequest" => { @@ -267,33 +152,12 @@ impl AgentPolicy { let log_entry = format!("[\"ep\":\"{ep}\",{input}],\n\n"); if let Err(e) = log_file.write_all(log_entry.as_bytes()).await { - warn!(sl!(), "policy: log_opa_input: write_all failed: {}", e); + warn!(sl!(), "policy: log_eval_input: write_all failed: {}", e); } else if let Err(e) = log_file.flush().await { - warn!(sl!(), "policy: log_opa_input: flush failed: {}", e); + warn!(sl!(), "policy: log_eval_input: flush failed: {}", e); } } } } } } - -fn start_opa(opa_addr: &str) -> Result<()> { - let bin_dirs = vec!["/bin", "/usr/bin", "/usr/local/bin"]; - for bin_dir in &bin_dirs { - let opa_path = bin_dir.to_string() + "/opa"; - if std::fs::metadata(&opa_path).is_ok() { - // args copied from kata-opa.service.in. - std::process::Command::new(&opa_path) - .arg("run") - .arg("--server") - .arg("--disable-telemetry") - .arg("--addr") - .arg(opa_addr) - .arg("--log-level") - .arg("info") - .spawn()?; - return Ok(()); - } - } - bail!("OPA binary not found in {:?}", &bin_dirs); -} From ed6412b63c5005bd22ceebec53306c166f222353 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Thu, 18 Apr 2024 00:03:20 +0000 Subject: [PATCH 4/7] tests: k8s: reduce the policy tests output noise Hide some of the kubectl output, to reduce the size and redundancy of this output. Fixes: #9388 Signed-off-by: Dan Mihai --- tests/integration/kubernetes/k8s-policy-job.bats | 4 +++- tests/integration/kubernetes/k8s-policy-pod.bats | 8 +++++--- tests/integration/kubernetes/k8s-policy-rc.bats | 4 +++- tests/integration/kubernetes/tests_common.sh | 2 +- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/integration/kubernetes/k8s-policy-job.bats b/tests/integration/kubernetes/k8s-policy-job.bats index 3be495da09..caf36ed71d 100644 --- a/tests/integration/kubernetes/k8s-policy-job.bats +++ b/tests/integration/kubernetes/k8s-policy-job.bats @@ -176,7 +176,9 @@ teardown() { # Debugging information for pod_name in ${pod_names[@]}; do info "Pod ${pod_name}:" - kubectl describe pod "${pod_name}" + + # Don't print the "Message:" line because it contains a truncated policy log. + kubectl describe pod "${pod_name}" | grep -v "Message:" done info "Job ${job_name}:" diff --git a/tests/integration/kubernetes/k8s-policy-pod.bats b/tests/integration/kubernetes/k8s-policy-pod.bats index d0c9290b83..7699768aff 100644 --- a/tests/integration/kubernetes/k8s-policy-pod.bats +++ b/tests/integration/kubernetes/k8s-policy-pod.bats @@ -138,14 +138,16 @@ test_pod_policy_error() { command="kubectl describe pod ${pod_name} | grep FailedPostStartHook" info "Waiting ${wait_time} seconds for: ${command}" - waitForProcess "${wait_time}" "$sleep_time" "${command}" + + # Don't print the "Message:" line because it contains a truncated policy log. + waitForProcess "${wait_time}" "$sleep_time" "${command}" | grep -v "Message:" } teardown() { policy_tests_enabled || skip "Policy tests are disabled." - # Debugging information - kubectl describe pod "${pod_name}" + # Debugging information. Don't print the "Message:" line because it contains a truncated policy log. + kubectl describe pod "${pod_name}" | grep -v "Message:" # Clean-up kubectl delete pod "${pod_name}" diff --git a/tests/integration/kubernetes/k8s-policy-rc.bats b/tests/integration/kubernetes/k8s-policy-rc.bats index e60428a387..f64b136006 100644 --- a/tests/integration/kubernetes/k8s-policy-rc.bats +++ b/tests/integration/kubernetes/k8s-policy-rc.bats @@ -163,7 +163,9 @@ teardown() { for pod_name in ${launched_pods[@]}; do info "Pod ${pod_name}:" - kubectl describe pod "${pod_name}" + + # Don't print the "Message:" line because it contains a truncated policy log. + kubectl describe pod "${pod_name}" | grep -v "Message:" done # Clean-up diff --git a/tests/integration/kubernetes/tests_common.sh b/tests/integration/kubernetes/tests_common.sh index 67802dd441..5e631f5c3b 100644 --- a/tests/integration/kubernetes/tests_common.sh +++ b/tests/integration/kubernetes/tests_common.sh @@ -314,5 +314,5 @@ wait_for_blocked_request() { command="kubectl describe pod ${pod} | grep \"${endpoint} is blocked by policy\"" info "Waiting ${wait_time} seconds for: ${command}" - waitForProcess "${wait_time}" "$sleep_time" "${command}" || return 1 + waitForProcess "${wait_time}" "$sleep_time" "${command}" >/dev/null 2>/dev/null } From 5d31eb484797071cf2eed0e95c9c0ec4bfc3f016 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Mon, 22 Apr 2024 23:21:17 +0000 Subject: [PATCH 5/7] agent: use regorus 0.1.4 Use regorus 0.1.4 from crates.io, instead of its source code repository. Signed-off-by: Dan Mihai --- src/agent/Cargo.lock | 5 +++-- src/agent/Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index f7e94eaa6f..8a6b8d14ef 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -3723,8 +3723,9 @@ checksum = "adad44e29e4c806119491a7f06f03de4d1af22c3a680dd47f1e6e179439d1f56" [[package]] name = "regorus" -version = "0.1.3" -source = "git+https://github.com/microsoft/regorus?rev=316f3a76#316f3a7692fe7dbb9e71890174d549de3da939f8" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac45cc36ed8e6188529f44deff00c078d9cd0e6cdc831c4dde96c9d6eeb3a46c" dependencies = [ "anyhow", "itertools 0.12.1", diff --git a/src/agent/Cargo.toml b/src/agent/Cargo.toml index 0445f30e61..27e7fd5a19 100644 --- a/src/agent/Cargo.toml +++ b/src/agent/Cargo.toml @@ -85,7 +85,7 @@ image-rs = { git = "https://github.com/confidential-containers/guest-components" openssl = { version = "0.10.54", features = ["vendored"], optional = true } # Agent Policy -regorus = { git = "https://github.com/microsoft/regorus", rev = "316f3a76", default-features = false, features = ["arc", "regex"], optional = true } +regorus = { version = "0.1.4", default-features = false, features = ["arc", "regex"], optional = true } [dev-dependencies] tempfile = "3.1.0" From e5c3f5fa9b4017b4892c3855701347ebbb8da9f5 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Tue, 23 Apr 2024 16:07:03 +0000 Subject: [PATCH 6/7] tests: no generated policy for untested platforms Avoid auto-generating Policy on platforms that haven't been tested yet with auto-generated Policy. Support for auto-generated Policy on these additional platforms is coming up in future PRs, so the tests being fixed here were prematurely enabled. Signed-off-by: Dan Mihai --- tests/integration/kubernetes/k8s-policy-job.bats | 4 ++-- tests/integration/kubernetes/k8s-policy-pod.bats | 4 ++-- tests/integration/kubernetes/k8s-policy-rc.bats | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/integration/kubernetes/k8s-policy-job.bats b/tests/integration/kubernetes/k8s-policy-job.bats index caf36ed71d..81fd105ed5 100644 --- a/tests/integration/kubernetes/k8s-policy-job.bats +++ b/tests/integration/kubernetes/k8s-policy-job.bats @@ -9,7 +9,7 @@ load "${BATS_TEST_DIRNAME}/../../common.bash" load "${BATS_TEST_DIRNAME}/tests_common.sh" setup() { - policy_tests_enabled || skip "Policy tests are disabled." + auto_generate_policy_enabled || skip "Auto-generated policy tests are disabled." get_pod_config_dir @@ -171,7 +171,7 @@ test_job_policy_error() { } teardown() { - policy_tests_enabled || skip "Policy tests are disabled." + auto_generate_policy_enabled || skip "Auto-generated policy tests are disabled." # Debugging information for pod_name in ${pod_names[@]}; do diff --git a/tests/integration/kubernetes/k8s-policy-pod.bats b/tests/integration/kubernetes/k8s-policy-pod.bats index 7699768aff..5a6be1c1eb 100644 --- a/tests/integration/kubernetes/k8s-policy-pod.bats +++ b/tests/integration/kubernetes/k8s-policy-pod.bats @@ -9,7 +9,7 @@ load "${BATS_TEST_DIRNAME}/../../common.bash" load "${BATS_TEST_DIRNAME}/tests_common.sh" setup() { - policy_tests_enabled || skip "Policy tests are disabled." + auto_generate_policy_enabled || skip "Auto-generated policy tests are disabled." configmap_name="policy-configmap" pod_name="policy-pod" @@ -144,7 +144,7 @@ test_pod_policy_error() { } teardown() { - policy_tests_enabled || skip "Policy tests are disabled." + auto_generate_policy_enabled || skip "Auto-generated policy tests are disabled." # Debugging information. Don't print the "Message:" line because it contains a truncated policy log. kubectl describe pod "${pod_name}" | grep -v "Message:" diff --git a/tests/integration/kubernetes/k8s-policy-rc.bats b/tests/integration/kubernetes/k8s-policy-rc.bats index f64b136006..363b6cf14c 100644 --- a/tests/integration/kubernetes/k8s-policy-rc.bats +++ b/tests/integration/kubernetes/k8s-policy-rc.bats @@ -9,7 +9,7 @@ load "${BATS_TEST_DIRNAME}/../../common.bash" load "${BATS_TEST_DIRNAME}/tests_common.sh" setup() { - policy_tests_enabled || skip "Policy tests are disabled." + auto_generate_policy_enabled || skip "Auto-generated policy tests are disabled." replication_name="policy-rc-test" app_name="policy-nginx-rc" @@ -156,7 +156,7 @@ test_rc_policy() { } teardown() { - policy_tests_enabled || skip "Policy tests are disabled." + auto_generate_policy_enabled || skip "Auto-generated policy tests are disabled." # Debugging information kubectl describe rc "${replication_name}" From ff385eac41d6d47c001b7708c981022b35ce0567 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Wed, 24 Apr 2024 14:53:51 +0000 Subject: [PATCH 7/7] agent: remove unnecessary comment Remove reminder to initialize Policy earlier, because currently there are no plans to initialize earlier. Signed-off-by: Dan Mihai --- src/agent/src/main.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index be91b2bf23..681ff06ecb 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -379,7 +379,6 @@ async fn start_sandbox( #[cfg(feature = "guest-pull")] image::set_proxy_env_vars().await; - // TODO: initialize earlier. #[cfg(feature = "agent-policy")] if let Err(e) = initialize_policy().await { error!(logger, "Failed to initialize agent policy: {:?}", e);