Reuse error mapping logic for key operation metrics
Define SerializedError wire type for convenience and type safety. It
does not change the rules of how errors are downcasted to an i32.
Change operation outcome errors from Keymint ErrorCode to
SerializedError. This has an intended effect of binder errors being
reported to metrics as ResponseCode::SYSTEM_ERROR instead of
ErrorCode::UNKNOWN_ERROR.
Also update comments.
Bug: 298194325
Test: m
Change-Id: Ieff70245b776c38845c4f5142ab13d438ff79104
diff --git a/keystore2/src/error.rs b/keystore2/src/error.rs
index d6ae0ce..1a048b6 100644
--- a/keystore2/src/error.rs
+++ b/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`
-//! 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.
+//! Here are some important types and helper functions:
//!
-//! Keystore functions should use `anyhow::Result` to return error conditions, and
-//! context should be added every time an error is forwarded.
+//! `Error` type encapsulate Keystore, Keymint, and Binder errors. It is used internally by
+//! 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 @@
///
/// 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 @@
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 @@
)
}
-/// Returns the error code given a reference to the error
-pub fn get_error_code(e: &anyhow::Error) -> i32 {
+/// This type is used to send error codes on the wire.
+///
+/// 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(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
- }
+ Some(e) => error_to_serialized_error(e),
None => match root_cause.downcast_ref::<selinux::Error>() {
- Some(selinux::Error::PermissionDenied) => ResponseCode::PERMISSION_DENIED.0,
- _ => ResponseCode::SYSTEM_ERROR.0,
+ Some(selinux::Error::PermissionDenied) => {
+ SerializedError(ResponseCode::PERMISSION_DENIED.0)
+ }
+ _ => SerializedError(ResponseCode::SYSTEM_ERROR.0),
},
}
}
diff --git a/keystore2/src/metrics_store.rs b/keystore2/src/metrics_store.rs
index 5d311f0..6dca74a 100644
--- a/keystore2/src/metrics_store.rs
+++ b/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 @@
};
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);
diff --git a/keystore2/src/operation.rs b/keystore2/src/operation.rs
index 2034a8a..eabc1ab 100644
--- a/keystore2/src/operation.rs
+++ b/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 @@
/// 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 @@
err: Result<T, Error>,
) -> Result<T, Error> {
match &err {
- Err(Error::Km(e)) => *locked_outcome = Outcome::ErrorCode(*e),
- Err(_) => *locked_outcome = Outcome::ErrorCode(ErrorCode::UNKNOWN_ERROR),
+ Err(e) => *locked_outcome = Outcome::ErrorCode(error_to_serialized_error(e)),
Ok(_) => (),
}
err