policy: log detail error info in policy crate

Clippy reports the warnings that the Err-variant returned `PolicyError` is very large. One solution is to use `Box` to hold the error information but it is not prefered because we need to minimize the use of heap. Another solution is to reduce the size of structure. By logging the error information inside `policy` crate, the detailed info can be removed to make the size of `PolicyError` much smaller. Signed-off-by: Jiaqi Gao <[email protected]>
intel · Mar 14, 2024 · cc24b23 · cc24b23
1 parent 2d17b22
commit cc24b23
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 87 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/src/migtd/Cargo.toml b/src/migtd/Cargo.toml
@@ -45,7 +45,7 @@ td-benchmark = { path = "../../deps/td-shim/devtools/td-benchmark", default-feat
 default = ["tdx"]
 cet-shstk = ["td-payload/cet-shstk"]
 coverage = ["minicov"]
-main = ["tdx"]
+main = ["tdx", "policy/log"]
 stack-guard = ["td-payload/stack-guard"]
 virtio-vsock = ["vsock/virtio-vsock"]
 virtio-serial = ["virtio_serial"]

diff --git a/src/policy/Cargo.toml b/src/policy/Cargo.toml
@@ -10,6 +10,7 @@ edition = "2018"
 cc-measurement = { path = "../../deps/td-shim/cc-measurement"}
 crypto = { path = "../crypto" }
 lexical-core = { version = "0.8.3", default-features = false, features = ["write-integers", "write-floats", "parse-integers", "parse-floats"] }
+log = { version = "0.4.13", optional = true }
 serde = { version = "1.0", default-features = false, features = ["derive"]}
 serde_json = { version = "1.0", default-features = false, features = ["alloc"] }
 td-shim = { path = "../../deps/td-shim/td-shim", default-features = false }

diff --git a/src/policy/src/lib.rs b/src/policy/src/lib.rs
@@ -8,7 +8,7 @@ extern crate alloc;
 mod config;
 mod verify;
 
-use alloc::{format, string::String};
+use alloc::string::String;
 pub use config::*;
 pub use verify::*;
 
@@ -21,39 +21,9 @@ pub enum PolicyError {
     InvalidEventLog,
     PlatformNotFound(String),
     PlatformNotMatch(String, String),
-    UnqulifiedPlatformInfo(PolicyErrorDetails),
-    UnqulifiedQeInfo(PolicyErrorDetails),
-    UnqulifiedTdxModuleInfo(PolicyErrorDetails),
-    UnqulifiedMigTdInfo(PolicyErrorDetails),
+    UnqulifiedPlatformInfo,
+    UnqulifiedQeInfo,
+    UnqulifiedTdxModuleInfo,
+    UnqulifiedMigTdInfo,
     Crypto,
 }
-
-#[derive(Debug)]
-pub struct PolicyErrorDetails {
-    pub property: String,
-    pub policy: Property,
-    pub local_fmspc: Option<String>,
-    pub remote_fmspc: Option<String>,
-    pub local: String,
-    pub remote: String,
-}
-
-impl PolicyErrorDetails {
-    pub(crate) fn new(
-        property: String,
-        policy: Property,
-        local_fmspc: Option<String>,
-        remote_fmspc: Option<String>,
-        local: &[u8],
-        remote: &[u8],
-    ) -> Self {
-        Self {
-            property,
-            policy,
-            local: format!("{:x?}", local),
-            remote: format!("{:x?}", remote),
-            local_fmspc,
-            remote_fmspc,
-        }
-    }
-}
diff --git a/src/policy/src/verify.rs b/src/policy/src/verify.rs
@@ -15,8 +15,7 @@ use td_shim::event_log::{
 
 use crate::{
     config::{MigPolicy, Property},
-    format_bytes_hex, MigTdInfo, PlatformInfo, PolicyError, PolicyErrorDetails, QeInfo,
-    TdxModuleInfo,
+    format_bytes_hex, MigTdInfo, PlatformInfo, PolicyError, QeInfo, TdxModuleInfo,
 };
 
 const REPORT_DATA_SIZE: usize = 734;
@@ -379,16 +378,15 @@ fn verify_platform_info(
         let verify_result = action.verify(is_src, local, remote);
 
         if !verify_result {
-            return Err(PolicyError::UnqulifiedPlatformInfo(
-                PolicyErrorDetails::new(
-                    name.clone(),
-                    action.clone(),
-                    Some(local_fmspc),
-                    Some(peer_fmspc),
-                    local,
-                    remote,
-                ),
-            ));
+            log_error_status(
+                name.clone(),
+                action.clone(),
+                Some(local_fmspc),
+                Some(peer_fmspc),
+                local,
+                remote,
+            );
+            return Err(PolicyError::UnqulifiedPlatformInfo);
         }
     }
 
@@ -409,14 +407,8 @@ fn verify_qe_info(
         let verify_result = action.verify(is_src, local, remote);
 
         if !verify_result {
-            return Err(PolicyError::UnqulifiedQeInfo(PolicyErrorDetails::new(
-                name.clone(),
-                action.clone(),
-                None,
-                None,
-                local,
-                remote,
-            )));
+            log_error_status(name.clone(), action.clone(), None, None, local, remote);
+            return Err(PolicyError::UnqulifiedQeInfo);
         }
     }
 
@@ -437,9 +429,8 @@ fn verify_tdx_module_info(
         let verify_result = action.verify(is_src, local, remote);
 
         if !verify_result {
-            return Err(PolicyError::UnqulifiedTdxModuleInfo(
-                PolicyErrorDetails::new(name.clone(), action.clone(), None, None, local, remote),
-            ));
+            log_error_status(name.clone(), action.clone(), None, None, local, remote);
+            return Err(PolicyError::UnqulifiedTdxModuleInfo);
         }
     }
 
@@ -462,14 +453,8 @@ fn verify_migtd_info(
         let verify_result = action.verify(is_src, local, remote);
 
         if !verify_result {
-            return Err(PolicyError::UnqulifiedMigTdInfo(PolicyErrorDetails::new(
-                name.clone(),
-                action.clone(),
-                None,
-                None,
-                local,
-                remote,
-            )));
+            log_error_status(name.clone(), action.clone(), None, None, local, remote);
+            return Err(PolicyError::UnqulifiedMigTdInfo);
         }
     }
 
@@ -616,14 +601,8 @@ fn verify_events(
             verify_event(is_src, &event_name, value, local_event_log, peer_event_log);
 
         if !verify_result {
-            return Err(PolicyError::UnqulifiedMigTdInfo(PolicyErrorDetails::new(
-                name.clone(),
-                value.clone(),
-                None,
-                None,
-                &[],
-                &[],
-            )));
+            log_error_status(name.clone(), value.clone(), None, None, &[], &[]);
+            return Err(PolicyError::UnqulifiedMigTdInfo);
         }
     }
 
@@ -659,6 +638,29 @@ fn verify_event(
     }
 }
 
+#[allow(unused_variables)]
+fn log_error_status(
+    property: String,
+    policy: Property,
+    local_fmspc: Option<String>,
+    remote_fmspc: Option<String>,
+    local: &[u8],
+    remote: &[u8],
+) {
+    #[cfg(feature = "log")]
+    {
+        use alloc::format;
+        use log::error;
+
+        error!("Property: {:}\n", property);
+        error!("Policy: {:?}\n", policy);
+        error!("Local FMFPC: {:?}\n", local_fmspc);
+        error!("Remote FMFPC: {:?}\n", remote_fmspc);
+        error!("Local property value: {:?}\n", format!("{:x?}", local));
+        error!("Remote property value: {:?}\n", format!("{:x?}", remote));
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -761,7 +763,7 @@ mod tests {
         );
         assert!(matches!(
             verify_result,
-            Err(PolicyError::UnqulifiedPlatformInfo(_))
+            Err(PolicyError::UnqulifiedPlatformInfo)
         ));
 
         // dst's tdx tcb level is higher than reference
@@ -809,7 +811,7 @@ mod tests {
         );
         assert!(matches!(
             verify_result,
-            Err(PolicyError::UnqulifiedPlatformInfo(_))
+            Err(PolicyError::UnqulifiedPlatformInfo)
         ));
     }
 
@@ -852,7 +854,7 @@ mod tests {
         );
         assert!(matches!(
             verify_result,
-            Err(PolicyError::UnqulifiedMigTdInfo(_))
+            Err(PolicyError::UnqulifiedMigTdInfo)
         ));
         report_peer[Report::R_ATTR_TD].copy_from_slice(&template[Report::R_ATTR_TD]);
 
@@ -869,7 +871,7 @@ mod tests {
         );
         assert!(matches!(
             verify_result,
-            Err(PolicyError::UnqulifiedMigTdInfo(_))
+            Err(PolicyError::UnqulifiedMigTdInfo)
         ));
         report_peer[Report::R_XFAM].copy_from_slice(&template[Report::R_XFAM]);
 
@@ -886,7 +888,7 @@ mod tests {
         );
         assert!(matches!(
             verify_result,
-            Err(PolicyError::UnqulifiedMigTdInfo(_))
+            Err(PolicyError::UnqulifiedMigTdInfo)
         ));
         report_peer[Report::R_MRTD].copy_from_slice(&[0u8; 48]);
 
@@ -903,7 +905,7 @@ mod tests {
         );
         assert!(matches!(
             verify_result,
-            Err(PolicyError::UnqulifiedMigTdInfo(_))
+            Err(PolicyError::UnqulifiedMigTdInfo)
         ));
         report_peer[Report::R_MRCONFIGID].copy_from_slice(&template[Report::R_MRCONFIGID]);
 
@@ -920,7 +922,7 @@ mod tests {
         );
         assert!(matches!(
             verify_result,
-            Err(PolicyError::UnqulifiedMigTdInfo(_))
+            Err(PolicyError::UnqulifiedMigTdInfo)
         ));
         report_peer[Report::R_MROWNER].copy_from_slice(&template[Report::R_MROWNER]);
 
@@ -937,7 +939,7 @@ mod tests {
         );
         assert!(matches!(
             verify_result,
-            Err(PolicyError::UnqulifiedMigTdInfo(_))
+            Err(PolicyError::UnqulifiedMigTdInfo)
         ));
         report_peer[Report::R_MROWNERCONFIG].copy_from_slice(&template[Report::R_MROWNERCONFIG]);
 
@@ -954,7 +956,7 @@ mod tests {
         );
         assert!(matches!(
             verify_result,
-            Err(PolicyError::UnqulifiedMigTdInfo(_))
+            Err(PolicyError::UnqulifiedMigTdInfo)
         ));
         report_peer[Report::R_RTMR0].copy_from_slice(&template[Report::R_RTMR0]);
 
@@ -971,7 +973,7 @@ mod tests {
         );
         assert!(matches!(
             verify_result,
-            Err(PolicyError::UnqulifiedMigTdInfo(_))
+            Err(PolicyError::UnqulifiedMigTdInfo)
         ));
         report_peer[Report::R_RTMR1].copy_from_slice(&template[Report::R_RTMR1]);
 
@@ -988,7 +990,7 @@ mod tests {
         );
         assert!(matches!(
             verify_result,
-            Err(PolicyError::UnqulifiedMigTdInfo(_))
+            Err(PolicyError::UnqulifiedMigTdInfo)
         ));
         report_peer[Report::R_RTMR2].copy_from_slice(&template[Report::R_RTMR2]);
 
@@ -1005,7 +1007,7 @@ mod tests {
         );
         assert!(matches!(
             verify_result,
-            Err(PolicyError::UnqulifiedMigTdInfo(_))
+            Err(PolicyError::UnqulifiedMigTdInfo)
         ));
     }