Merge "Reuse error mapping logic for key operation metrics" into main am: 6b57def860

Original change: https://android-review.googlesource.com/c/platform/system/security/+/2736534

Change-Id: Ie2d78eca613d4c705ad0e3c75913c2cce14d3e56
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

keystore2/src/error.rs

@@ -13,22 +13,19 @@
 // limitations under the License.
 
 //! Keystore error provides convenience methods and types for Keystore error handling.
-//! Clients of Keystore expect one of two error codes, i.e., a Keystore ResponseCode as
-//! defined by the Keystore AIDL interface, or a Keymint ErrorCode as defined by
-//! the Keymint HAL specification.
-//! This crate provides `Error` which can wrap both. It is to be used
-//! internally by Keystore to diagnose error conditions that need to be reported to
-//! the client. To report the error condition to the client the Keystore AIDL
-//! interface defines a wire type `Result` which is distinctly different from Rust's
-//! `enum Result<T,E>`.
 //!
-//! This crate provides the convenience method `map_or_log_err` to convert `anyhow::Error`
+//! Here are some important types and helper functions:
-//! into this wire type. In addition to handling the conversion of `Error`
-//! to the `Result` wire type it handles any other error by mapping it to
-//! `ResponseCode::SYSTEM_ERROR` and logs any error condition.
 //!
-//! Keystore functions should use `anyhow::Result` to return error conditions, and
+//! `Error` type encapsulate Keystore, Keymint, and Binder errors. It is used internally by
-//! context should be added every time an error is forwarded.
+//! Keystore to diagnose error conditions that need to be reported to the client.
+//!
+//! `SerializedError` is used send error codes on the wire.
+//!
+//! `map_or_log_err` is a convenience method used to convert `anyhow::Error` into `SerializedError`
+//! wire type.
+//!
+//! Keystore functions should use `anyhow::Result` to return error conditions, and context should
+//! be added every time an error is forwarded.
 
 pub use android_hardware_security_keymint::aidl::android::hardware::security::keymint::ErrorCode::ErrorCode;
 pub use android_system_keystore2::aidl::android::system::keystore2::ResponseCode::ResponseCode;
@@ -126,14 +123,6 @@ pub fn map_binder_status_code<T>(r: Result<T, StatusCode>) -> Result<T, Error> {
 ///
 /// All error conditions get logged by this function, except for KEY_NOT_FOUND error.
 ///
-/// All `Error::Rc(x)` and `Error::Km(x)` variants get mapped onto a service specific error
-/// code of x. This is possible because KeyMint `ErrorCode` errors are always negative and
-/// `ResponseCode` codes are always positive.
-/// `selinux::Error::PermissionDenied` is mapped on `ResponseCode::PERMISSION_DENIED`.
-///
-/// All non `Error` error conditions and the Error::Binder variant get mapped onto
-/// ResponseCode::SYSTEM_ERROR`.
-///
 /// `handle_ok` will be called if `result` is `Ok(value)` where `value` will be passed
 /// as argument to `handle_ok`. `handle_ok` must generate a `BinderResult<T>`, but it
 /// typically returns Ok(value).
@@ -200,9 +189,9 @@ where
     result.map_or_else(
         |e| {
             let e = map_err(e);
-            let rc = get_error_code(&e);
+            let rc = anyhow_error_to_serialized_error(&e);
             Err(BinderStatus::new_service_specific_error(
-                rc,
+                rc.0,
                 anyhow_error_to_cstring(&e).as_deref(),
             ))
         },
@@ -210,21 +199,42 @@ where
     )
 }
 
-/// Returns the error code given a reference to the error
+/// This type is used to send error codes on the wire.
-pub fn get_error_code(e: &anyhow::Error) -> i32 {
+///
+/// Errors are squashed into one number space using following rules:
+/// - All Keystore and Keymint errors codes are identity mapped. It's possible because by
+///   convention Keystore `ResponseCode` errors are positive, and Keymint `ErrorCode` errors are
+///   negative.
+/// - `selinux::Error::PermissionDenied` is mapped to `ResponseCode::PERMISSION_DENIED`.
+/// - All other error conditions, e.g. Binder errors, are mapped to `ResponseCode::SYSTEM_ERROR`.
+///
+/// The type should be used to forward all error codes to clients of Keystore AIDL interface and to
+/// metrics events.
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
+pub struct SerializedError(pub i32);
+
+/// Returns a SerializedError given a reference to Error.
+pub fn error_to_serialized_error(e: &Error) -> SerializedError {
+    match e {
+        Error::Rc(rcode) => SerializedError(rcode.0),
+        Error::Km(ec) => SerializedError(ec.0),
+        // Binder errors are reported as system error.
+        Error::Binder(_, _) | Error::BinderTransaction(_) => {
+            SerializedError(ResponseCode::SYSTEM_ERROR.0)
+        }
+    }
+}
+
+/// Returns a SerializedError given a reference to anyhow::Error.
+pub fn anyhow_error_to_serialized_error(e: &anyhow::Error) -> SerializedError {
     let root_cause = e.root_cause();
     match root_cause.downcast_ref::<Error>() {
-        Some(Error::Rc(rcode)) => rcode.0,
+        Some(e) => error_to_serialized_error(e),
-        Some(Error::Km(ec)) => ec.0,
-        // If an Error::Binder reaches this stage we report a system error.
-        // The exception code and possible service specific error will be
-        // printed in the error log above.
-        Some(Error::Binder(_, _)) | Some(Error::BinderTransaction(_)) => {
-            ResponseCode::SYSTEM_ERROR.0
-        }
         None => match root_cause.downcast_ref::<selinux::Error>() {
-            Some(selinux::Error::PermissionDenied) => ResponseCode::PERMISSION_DENIED.0,
+            Some(selinux::Error::PermissionDenied) => {
-            _ => ResponseCode::SYSTEM_ERROR.0,
+                SerializedError(ResponseCode::PERMISSION_DENIED.0)
+            }
+            _ => SerializedError(ResponseCode::SYSTEM_ERROR.0),
         },
     }
 }

keystore2/src/metrics_store.rs

@@ -17,7 +17,7 @@
 //!    stores them in an in-memory store.
 //! 2. Returns the collected metrics when requested by the statsd proxy.
 
-use crate::error::get_error_code;
+use crate::error::anyhow_error_to_serialized_error;
 use crate::globals::DB;
 use crate::key_parameter::KeyParameterValue as KsKeyParamValue;
 use crate::ks_err;
@@ -202,7 +202,7 @@ fn process_key_creation_event_stats<U>(
     };
 
     if let Err(ref e) = result {
-        key_creation_with_general_info.error_code = get_error_code(e);
+        key_creation_with_general_info.error_code = anyhow_error_to_serialized_error(e).0;
     }
 
     key_creation_with_auth_info.security_level = process_security_level(sec_level);

keystore2/src/operation.rs

@@ -126,7 +126,10 @@
 //! Either way, we have to revaluate the pruning scores.
 
 use crate::enforcements::AuthInfo;
-use crate::error::{map_err_with, map_km_error, map_or_log_err, Error, ErrorCode, ResponseCode};
+use crate::error::{
+    error_to_serialized_error, map_err_with, map_km_error, map_or_log_err, Error, ErrorCode,
+    ResponseCode, SerializedError,
+};
 use crate::ks_err;
 use crate::metrics_store::log_key_operation_event_stats;
 use crate::utils::watchdog as wd;
@@ -162,7 +165,7 @@ pub enum Outcome {
     /// Operation is pruned.
     Pruned,
     /// Operation is failed with the error code.
-    ErrorCode(ErrorCode),
+    ErrorCode(SerializedError),
 }
 
 /// Operation bundles all of the operation related resources and tracks the operation's
@@ -305,8 +308,7 @@ impl Operation {
         err: Result<T, Error>,
     ) -> Result<T, Error> {
         match &err {
-            Err(Error::Km(e)) => *locked_outcome = Outcome::ErrorCode(*e),
+            Err(e) => *locked_outcome = Outcome::ErrorCode(error_to_serialized_error(e)),
-            Err(_) => *locked_outcome = Outcome::ErrorCode(ErrorCode::UNKNOWN_ERROR),
             Ok(_) => (),
         }
         err