Merge branch 'main' into fix-jwe

confidential-containers · Dec 12, 2024 · 30b71a3 · 30b71a3
2 parents 7f00472 + 681ee98
commit 30b71a3
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 24 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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,21 +208,19 @@ 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]
             }
         })
         .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
@@ -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
@@ -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
@@ -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",