diff --git a/Cargo.lock b/Cargo.lock index 9b2fd00e1..f4f15593a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6140,7 +6140,7 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.48.0", ] [[package]] diff --git a/attestation-service/src/policy_engine/mod.rs b/attestation-service/src/policy_engine/mod.rs index 6a9e78875..1ecad5ec7 100644 --- a/attestation-service/src/policy_engine/mod.rs +++ b/attestation-service/src/policy_engine/mod.rs @@ -35,6 +35,8 @@ pub enum PolicyError { Base64DecodeFailed(#[from] base64::DecodeError), #[error("Illegal policy id. Only support alphabet, numeric, `-` or `_`")] InvalidPolicyId, + #[error("Illegal policy: {0}")] + InvalidPolicy(#[source] anyhow::Error), #[error("Failed to load reference data: {0}")] LoadReferenceDataFailed(#[source] anyhow::Error), #[error("Failed to set input data: {0}")] diff --git a/attestation-service/src/policy_engine/opa/mod.rs b/attestation-service/src/policy_engine/opa/mod.rs index 398293bdd..04bd28cd7 100644 --- a/attestation-service/src/policy_engine/opa/mod.rs +++ b/attestation-service/src/policy_engine/opa/mod.rs @@ -122,6 +122,16 @@ impl PolicyEngine for OPA { return Err(PolicyError::InvalidPolicyId); } + // Check if the policy is valid + { + let policy_content = String::from_utf8(policy_bytes.clone()) + .map_err(|e| PolicyError::InvalidPolicy(e.into()))?; + let mut engine = regorus::Engine::new(); + engine + .add_policy(policy_id.clone(), policy_content) + .map_err(PolicyError::InvalidPolicy)?; + } + let mut policy_file_path = PathBuf::from( &self .policy_dir_path @@ -198,10 +208,9 @@ mod tests { "sourced_data", ]; - fn dummy_reference(product_id: u64, svn: u64, launch_digest: String) -> String { + fn dummy_reference(svn: u64, launch_digest: String) -> String { json!({ "reference": { - "productId": [product_id.to_string()], "svn": [svn.to_string()], "launch_digest": [launch_digest] } @@ -209,10 +218,9 @@ mod tests { .to_string() } - fn dummy_input(product_id: u64, svn: u64, launch_digest: String) -> String { + fn dummy_input(svn: u64, launch_digest: String) -> String { json!({ "sample": { - "productId": product_id.to_string(), "svn": svn.to_string(), "launch_digest": launch_digest } @@ -221,14 +229,12 @@ mod tests { } #[rstest] - #[case(5,5,1,1,"aac43bb3".to_string(),"aac43bb3".to_string(),3,2)] - #[case(5,4,1,1,"aac43bb3".to_string(),"aac43bb3".to_string(),3,97)] - #[case(5,5,1,1,"aac43bb4".to_string(),"aac43bb3".to_string(),33,2)] - #[case(5,5,2,1,"aac43bb4".to_string(),"aac43bb3".to_string(),33,97)] + #[case(1,1,"aac43bb3".to_string(),"aac43bb3".to_string(),3,2)] + #[case(2,1,"aac43bb3".to_string(),"aac43bb3".to_string(),3,97)] + #[case(1,1,"aac43bb4".to_string(),"aac43bb3".to_string(),33,2)] + #[case(2,1,"aac43bb4".to_string(),"aac43bb3".to_string(),33,97)] #[tokio::test] async fn test_evaluate( - #[case] pid_a: u64, - #[case] pid_b: u64, #[case] svn_a: u64, #[case] svn_b: u64, #[case] digest_a: String, @@ -243,8 +249,8 @@ mod tests { let output = opa .evaluate( - &dummy_reference(pid_a, svn_a, digest_a), - &dummy_input(pid_b, svn_b, digest_b), + &dummy_reference(svn_a, digest_a), + &dummy_input(svn_b, digest_b), &default_policy_id, &EAR_RULES, ) diff --git a/attestation-service/src/token/ear_default_policy.rego b/attestation-service/src/token/ear_default_policy.rego index 3353cebd5..c035ef0a5 100644 --- a/attestation-service/src/token/ear_default_policy.rego +++ b/attestation-service/src/token/ear_default_policy.rego @@ -78,10 +78,6 @@ sample_executables := 3 if { # verifications needed to demonstrate that these are genuine/ # supported. sample_hardware := 2 if { - # The sample attester does not report any productId. - # This is an exmple of how a real platform might identify the hardware - # that is running. - input.sample.productId in data.reference.productId input.sample.svn in data.reference.svn } diff --git a/kbs/src/policy_engine/error.rs b/kbs/src/policy_engine/error.rs index 0970b4b5e..d948e1b46 100644 --- a/kbs/src/policy_engine/error.rs +++ b/kbs/src/policy_engine/error.rs @@ -33,4 +33,7 @@ pub enum KbsPolicyEngineError { #[error("Set Policy request is illegal for {0}")] IllegalSetPolicyRequest(&'static str), + + #[error("Failed to set policy, illegal policy: {0}")] + InvalidPolicy(#[source] anyhow::Error), } diff --git a/kbs/src/policy_engine/opa/mod.rs b/kbs/src/policy_engine/opa/mod.rs index 361785bf4..3961f639e 100644 --- a/kbs/src/policy_engine/opa/mod.rs +++ b/kbs/src/policy_engine/opa/mod.rs @@ -61,6 +61,16 @@ impl PolicyEngineInterface for Opa { async fn set_policy(&mut self, policy: &str) -> Result<(), KbsPolicyEngineError> { let policy_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD.decode(policy)?; + // Check if the policy is valid + { + let policy_content = String::from_utf8(policy_bytes.clone()) + .map_err(|e| KbsPolicyEngineError::InvalidPolicy(e.into()))?; + let mut engine = regorus::Engine::new(); + engine + .add_policy(String::from("default"), policy_content) + .map_err(KbsPolicyEngineError::InvalidPolicy)?; + } + tokio::fs::write(&self.policy_path, policy_bytes).await?; Ok(()) @@ -153,6 +163,13 @@ mod tests { res.err().unwrap(), KbsPolicyEngineError::IOError(_) )); + + // Illegal policy + let res = set_policy_from_file(&mut opa, "test/data/policy_invalid_1.rego").await; + assert!(matches!( + res.err().unwrap(), + KbsPolicyEngineError::InvalidPolicy(_) + )); } #[rstest] @@ -167,13 +184,6 @@ mod tests { 1, Err(KbsPolicyEngineError::ResourcePathError) )] - #[case( - "test/data/policy_invalid_1.rego", - "my_repo/Alice/key", - "Alice", - 1, - Err(KbsPolicyEngineError::PolicyLoadError) - )] #[case( "test/data/policy_invalid_2.rego", "my_repo/Alice/key",