Change error.rs to use generated types.
This patch also removes AidlResult form the error module.
The new version of the Keystore 2.0 AIDL spec requires that ResponseCode
and ErrorCode will be returned as service specific error, instead of the
Result type.
Test: keystore2_test
Change-Id: I6f730b282c84abc8be2e693e5d2c7053648d7588
diff --git a/keystore2/src/error.rs b/keystore2/src/error.rs
index a285bda..f4da749 100644
--- a/keystore2/src/error.rs
+++ b/keystore2/src/error.rs
@@ -31,35 +31,16 @@
//! context should be added every time an error is forwarded.
use std::cmp::PartialEq;
-use std::convert::From;
-use keystore_aidl_generated as aidl;
-use keystore_aidl_generated::ResponseCode as AidlRc;
+pub use android_hardware_keymint::aidl::android::hardware::keymint::ErrorCode as Ec;
+pub use android_security_keystore2::aidl::android::security::keystore2::ResponseCode as Rc;
+
+use android_hardware_keymint::aidl::android::hardware::keymint::ErrorCode::ErrorCode;
+use android_security_keystore2::aidl::android::security::keystore2::ResponseCode::ResponseCode;
use keystore2_selinux as selinux;
-pub use aidl::ResponseCode;
-
-/// AidlResult wraps the `android.security.keystore2.Result` generated from AIDL
-#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct AidlResult(aidl::Result);
-
-impl AidlResult {
- /// Creates an instance of AidlResult indicating no error has occurred.
- pub fn ok() -> Self {
- Self(aidl::Result { rc: AidlRc::Ok, km_error_code: 0 })
- }
-
- /// Creates an instance of AidlResult indicating the given ResponseCode.
- pub fn rc(rc: AidlRc) -> Self {
- Self(aidl::Result { rc, km_error_code: 0 })
- }
-
- /// Creates an instance of AidlResult indicating the given KM ErrorCode.
- pub fn ec(ec: aidl::ErrorCode) -> Self {
- Self(aidl::Result { rc: AidlRc::KeymintErrorCode, km_error_code: ec })
- }
-}
+use android_security_keystore2::binder::{Result as BinderResult, Status as BinderStatus};
/// This is the main Keystore error type. It wraps the Keystore `ResponseCode` generated
/// from AIDL in the `Rc` variant and Keymint `ErrorCode` in the Km variant.
@@ -67,35 +48,21 @@
pub enum Error {
/// Wraps a Keystore `ResponseCode` as defined by the Keystore AIDL interface specification.
#[error("Error::Rc({0:?})")]
- Rc(AidlRc),
+ Rc(ResponseCode),
/// Wraps a Keymint `ErrorCode` as defined by the Keymint AIDL interface specification.
#[error("Error::Km({0:?})")]
- Km(aidl::ErrorCode), // TODO Keymint ErrorCode is a generated AIDL type.
+ Km(ErrorCode),
}
impl Error {
/// Short hand for `Error::Rc(ResponseCode::SystemError)`
pub fn sys() -> Self {
- Error::Rc(AidlRc::SystemError)
+ Error::Rc(Rc::SystemError)
}
/// Short hand for `Error::Rc(ResponseCode::PermissionDenied`
pub fn perm() -> Self {
- Error::Rc(AidlRc::PermissionDenied)
- }
-}
-
-impl From<anyhow::Error> for AidlResult {
- fn from(error: anyhow::Error) -> Self {
- let root_cause = error.root_cause();
- match root_cause.downcast_ref::<Error>() {
- Some(Error::Rc(rcode)) => AidlResult::rc(*rcode),
- Some(Error::Km(ec)) => AidlResult::ec(*ec),
- None => match root_cause.downcast_ref::<selinux::Error>() {
- Some(selinux::Error::PermissionDenied) => AidlResult::rc(AidlRc::PermissionDenied),
- _ => AidlResult::rc(AidlRc::SystemError),
- },
- }
+ Error::Rc(Rc::PermissionDenied)
}
}
@@ -129,14 +96,23 @@
///
/// aidl_result_ = map_or_log_err(loadKey(), |r| { some_side_effect(); AidlResult::rc(r) });
/// ```
-pub fn map_or_log_err<T>(
- result: anyhow::Result<T>,
- handle_ok: impl FnOnce(T) -> AidlResult,
-) -> AidlResult {
+pub fn map_or_log_err<T, U, F>(result: anyhow::Result<U>, handle_ok: F) -> BinderResult<T>
+where
+ F: FnOnce(U) -> BinderResult<T>,
+{
result.map_or_else(
|e| {
log::error!("{:?}", e);
- e.into()
+ let root_cause = e.root_cause();
+ let rc = match root_cause.downcast_ref::<Error>() {
+ Some(Error::Rc(rcode)) => *rcode,
+ Some(Error::Km(ec)) => *ec,
+ None => match root_cause.downcast_ref::<selinux::Error>() {
+ Some(selinux::Error::PermissionDenied) => Rc::PermissionDenied,
+ _ => Rc::SystemError,
+ },
+ };
+ Err(BinderStatus::new_service_specific_error(rc, None))
},
handle_ok,
)
@@ -145,16 +121,14 @@
#[cfg(test)]
pub mod tests {
- use anyhow::{anyhow, Context, Result};
-
- use super::aidl::ErrorCode;
use super::*;
+ use anyhow::{anyhow, Context};
- fn nested_nested_rc(rc: AidlRc) -> anyhow::Result<()> {
+ fn nested_nested_rc(rc: ResponseCode) -> anyhow::Result<()> {
Err(anyhow!(Error::Rc(rc))).context("nested nested rc")
}
- fn nested_rc(rc: AidlRc) -> anyhow::Result<()> {
+ fn nested_rc(rc: ResponseCode) -> anyhow::Result<()> {
nested_nested_rc(rc).context("nested rc")
}
@@ -166,11 +140,11 @@
nested_nested_ec(ec).context("nested ec")
}
- fn nested_nested_ok(rc: AidlRc) -> anyhow::Result<AidlRc> {
+ fn nested_nested_ok(rc: ResponseCode) -> anyhow::Result<ResponseCode> {
Ok(rc)
}
- fn nested_ok(rc: AidlRc) -> anyhow::Result<AidlRc> {
+ fn nested_ok(rc: ResponseCode) -> anyhow::Result<ResponseCode> {
nested_nested_ok(rc).context("nested ok")
}
@@ -203,90 +177,52 @@
.with_tag("keystore_error_tests")
.with_min_level(log::Level::Debug),
);
- // All Error::Rc(x) get mapped on aidl::Result{x, 0}
+ // All Error::Rc(x) get mapped on a service specific error
+ // code of x.
+ for rc in Rc::Ok..Rc::BackendBusy {
+ assert_eq!(
+ Result::<(), i32>::Err(rc),
+ map_or_log_err(nested_rc(rc), |_| Err(BinderStatus::ok()))
+ .map_err(|s| s.service_specific_error())
+ );
+ }
+
+ // All KeystoreKerror::Km(x) get mapped on a service
+ // specific error of x.
+ for ec in Ec::UNKNOWN_ERROR..Ec::ROOT_OF_TRUST_ALREADY_SET {
+ assert_eq!(
+ Result::<(), i32>::Err(ec),
+ map_or_log_err(nested_ec(ec), |_| Err(BinderStatus::ok()))
+ .map_err(|s| s.service_specific_error())
+ );
+ }
+
+ // selinux::Error::Perm() needs to be mapped to Rc::PermissionDenied
assert_eq!(
- AidlResult::rc(AidlRc::Ok),
- map_or_log_err(nested_rc(AidlRc::Ok), |_| AidlResult::ec(0))
- );
- assert_eq!(
- AidlResult::rc(AidlRc::Locked),
- map_or_log_err(nested_rc(AidlRc::Locked), |_| AidlResult::ec(0))
- );
- assert_eq!(
- AidlResult::rc(AidlRc::Uninitialized),
- map_or_log_err(nested_rc(AidlRc::Uninitialized), |_| AidlResult::ec(0))
- );
- assert_eq!(
- AidlResult::rc(AidlRc::SystemError),
- map_or_log_err(nested_rc(AidlRc::SystemError), |_| AidlResult::ec(0))
- );
- assert_eq!(
- AidlResult::rc(AidlRc::PermissionDenied),
- map_or_log_err(nested_rc(AidlRc::PermissionDenied), |_| AidlResult::ec(0))
- );
- assert_eq!(
- AidlResult::rc(AidlRc::KeyNotFound),
- map_or_log_err(nested_rc(AidlRc::KeyNotFound), |_| AidlResult::ec(0))
- );
- assert_eq!(
- AidlResult::rc(AidlRc::ValueCorrupted),
- map_or_log_err(nested_rc(AidlRc::ValueCorrupted), |_| AidlResult::ec(0))
- );
- assert_eq!(
- AidlResult::rc(AidlRc::WrongPassword),
- map_or_log_err(nested_rc(AidlRc::WrongPassword), |_| AidlResult::ec(0))
- );
- assert_eq!(
- AidlResult::rc(AidlRc::OpAuthNeeded),
- map_or_log_err(nested_rc(AidlRc::OpAuthNeeded), |_| AidlResult::ec(0))
- );
- assert_eq!(
- AidlResult::rc(AidlRc::KeyPermanentlyInvalidated),
- map_or_log_err(nested_rc(AidlRc::KeyPermanentlyInvalidated), |_| AidlResult::ec(0))
- );
- assert_eq!(
- AidlResult::rc(AidlRc::NoSuchSecurityLevel),
- map_or_log_err(nested_rc(AidlRc::NoSuchSecurityLevel), |_| AidlResult::ec(0))
- );
- assert_eq!(
- AidlResult::rc(AidlRc::KeymintErrorCode),
- map_or_log_err(nested_rc(AidlRc::KeymintErrorCode), |_| AidlResult::ec(0))
- );
- assert_eq!(
- AidlResult::rc(AidlRc::BackendBusy),
- map_or_log_err(nested_rc(AidlRc::BackendBusy), |_| AidlResult::ec(0))
+ Result::<(), i32>::Err(Rc::PermissionDenied),
+ map_or_log_err(nested_selinux_perm(), |_| Err(BinderStatus::ok()))
+ .map_err(|s| s.service_specific_error())
);
- // All KeystoreKerror::Km(x) get mapped on
- // aidl::Result{AidlRc::KeymintErrorCode, x}
+ // All other errors get mapped on System Error.
assert_eq!(
- AidlResult::ec(-7),
- map_or_log_err(nested_ec(-7), |_| AidlResult::rc(AidlRc::SystemError))
- );
-
- // All other get mapped on System Error.
- assert_eq!(
- AidlResult::rc(AidlRc::SystemError),
- map_or_log_err(nested_other_error(), |_| AidlResult::ec(0))
+ Result::<(), i32>::Err(Rc::SystemError),
+ map_or_log_err(nested_other_error(), |_| Err(BinderStatus::ok()))
+ .map_err(|s| s.service_specific_error())
);
// Result::Ok variants get passed to the ok handler.
- assert_eq!(
- AidlResult::rc(AidlRc::OpAuthNeeded),
- map_or_log_err(nested_ok(AidlRc::OpAuthNeeded), AidlResult::rc)
- );
- assert_eq!(AidlResult::ok(), map_or_log_err(nested_ok(AidlRc::Ok), AidlResult::rc));
+ assert_eq!(Ok(Rc::OpAuthNeeded), map_or_log_err(nested_ok(Rc::OpAuthNeeded), Ok));
+ assert_eq!(Ok(Rc::Ok), map_or_log_err(nested_ok(Rc::Ok), Ok));
- // selinux::Error::Perm() needs to be mapped to AidlRc::PermissionDenied
- assert_eq!(
- AidlResult::rc(AidlRc::PermissionDenied),
- map_or_log_err(nested_selinux_perm(), |_| AidlResult::ec(0))
- );
Ok(())
}
//Helper function to test whether error cases are handled as expected.
- pub fn check_result_contains_error_string<T>(result: Result<T>, expected_error_string: &str) {
+ pub fn check_result_contains_error_string<T>(
+ result: anyhow::Result<T>,
+ expected_error_string: &str,
+ ) {
let error_str = format!(
"{:#?}",
result.err().unwrap_or_else(|| panic!("Expected the error: {}", expected_error_string))