Revert "[rkpd_client] Add Error type to rkpd_client"
This reverts commit 2dbabf3b7232781d61faba97865a061b3bae5fd9.
Reason for revert: DroidMonitor revert for b/310139666
Bug: 310139666
Change-Id: I1213940cc4e3112038c1cc66f5a218a9378d6b0f
diff --git a/keystore2/src/error.rs b/keystore2/src/error.rs
index 8797e23..1a048b6 100644
--- a/keystore2/src/error.rs
+++ b/keystore2/src/error.rs
@@ -27,9 +27,7 @@
//! Keystore functions should use `anyhow::Result` to return error conditions, and context should
//! be added every time an error is forwarded.
-use crate::rkpd_client::Error as RkpdError;
pub use android_hardware_security_keymint::aidl::android::hardware::security::keymint::ErrorCode::ErrorCode;
-use android_security_rkp_aidl::aidl::android::security::rkp::IGetKeyCallback::ErrorCode::ErrorCode as GetKeyErrorCode;
pub use android_system_keystore2::aidl::android::system::keystore2::ResponseCode::ResponseCode;
use android_system_keystore2::binder::{
ExceptionCode, Result as BinderResult, Status as BinderStatus, StatusCode,
@@ -68,48 +66,6 @@
}
}
-impl From<RkpdError> for Error {
- fn from(e: RkpdError) -> Self {
- match e {
- RkpdError::RequestCancelled | RkpdError::GetRegistrationFailed => {
- Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
- }
- RkpdError::GetKeyFailed(e) => {
- let response_code = match e {
- GetKeyErrorCode::ERROR_UNKNOWN => ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR,
- GetKeyErrorCode::ERROR_PERMANENT => ResponseCode::OUT_OF_KEYS_PERMANENT_ERROR,
- GetKeyErrorCode::ERROR_PENDING_INTERNET_CONNECTIVITY => {
- ResponseCode::OUT_OF_KEYS_PENDING_INTERNET_CONNECTIVITY
- }
- GetKeyErrorCode::ERROR_REQUIRES_SECURITY_PATCH => {
- ResponseCode::OUT_OF_KEYS_REQUIRES_SYSTEM_UPGRADE
- }
- _ => {
- log::error!("Unexpected get key error from rkpd: {e:?}");
- ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR
- }
- };
- Error::Rc(response_code)
- }
- RkpdError::StoreUpgradedKeyFailed | RkpdError::Timeout => {
- Error::Rc(ResponseCode::SYSTEM_ERROR)
- }
- RkpdError::BinderTransaction(s) => Error::BinderTransaction(s),
- }
- }
-}
-
-/// Maps an `rkpd_client::Error` that is wrapped with an `anyhow::Error` to a keystore2 `Error`.
-pub fn wrapped_rkpd_error_to_ks_error(e: &anyhow::Error) -> Error {
- match e.downcast_ref::<RkpdError>() {
- Some(e) => Error::from(*e),
- None => {
- log::error!("Failed to downcast the anyhow::Error to rkpd_client::Error: {e:?}");
- Error::Rc(ResponseCode::SYSTEM_ERROR)
- }
- }
-}
-
/// Helper function to map the binder status we get from calls into KeyMint
/// to a Keystore Error. We don't create an anyhow error here to make
/// it easier to evaluate KeyMint errors, which we must do in some cases, e.g.,
@@ -453,34 +409,4 @@
expected_error_string
);
}
-
- #[test]
- fn rkpd_error_is_in_sync_with_response_code() {
- let error_mapping = [
- (RkpdError::RequestCancelled, ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR),
- (RkpdError::GetRegistrationFailed, ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR),
- (
- RkpdError::GetKeyFailed(GetKeyErrorCode::ERROR_UNKNOWN),
- ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR,
- ),
- (
- RkpdError::GetKeyFailed(GetKeyErrorCode::ERROR_PERMANENT),
- ResponseCode::OUT_OF_KEYS_PERMANENT_ERROR,
- ),
- (
- RkpdError::GetKeyFailed(GetKeyErrorCode::ERROR_PENDING_INTERNET_CONNECTIVITY),
- ResponseCode::OUT_OF_KEYS_PENDING_INTERNET_CONNECTIVITY,
- ),
- (
- RkpdError::GetKeyFailed(GetKeyErrorCode::ERROR_REQUIRES_SECURITY_PATCH),
- ResponseCode::OUT_OF_KEYS_REQUIRES_SYSTEM_UPGRADE,
- ),
- (RkpdError::StoreUpgradedKeyFailed, ResponseCode::SYSTEM_ERROR),
- (RkpdError::Timeout, ResponseCode::SYSTEM_ERROR),
- ];
- for (rkpd_error, expected_response_code) in error_mapping {
- let e: Error = rkpd_error.into();
- assert_eq!(e, Error::Rc(expected_response_code));
- }
- }
} // mod tests
diff --git a/keystore2/src/remote_provisioning.rs b/keystore2/src/remote_provisioning.rs
index 14c61fb..3f7833e 100644
--- a/keystore2/src/remote_provisioning.rs
+++ b/keystore2/src/remote_provisioning.rs
@@ -31,7 +31,6 @@
use keystore2_crypto::parse_subject_from_certificate;
use crate::database::Uuid;
-use crate::error::wrapped_rkpd_error_to_ks_error;
use crate::globals::get_remotely_provisioned_component_name;
use crate::ks_err;
use crate::metrics_store::log_rkp_error_stats;
@@ -103,7 +102,7 @@
Err(e) => {
if self.is_rkp_only() {
log::error!("Error occurred: {:?}", e);
- return Err(wrapped_rkpd_error_to_ks_error(&e)).context(format!("{e:?}"));
+ return Err(e);
}
log::warn!("Error occurred: {:?}", e);
log_rkp_error_stats(
diff --git a/keystore2/src/rkpd_client.rs b/keystore2/src/rkpd_client.rs
index 9868838..fe64150 100644
--- a/keystore2/src/rkpd_client.rs
+++ b/keystore2/src/rkpd_client.rs
@@ -14,6 +14,7 @@
//! Helper wrapper around RKPD interface.
+use crate::error::{map_binder_status_code, Error, ResponseCode};
use android_security_rkp_aidl::aidl::android::security::rkp::{
IGetKeyCallback::BnGetKeyCallback, IGetKeyCallback::ErrorCode::ErrorCode as GetKeyErrorCode,
IGetKeyCallback::IGetKeyCallback, IGetRegistrationCallback::BnGetRegistrationCallback,
@@ -23,8 +24,8 @@
IStoreUpgradedKeyCallback::IStoreUpgradedKeyCallback,
RemotelyProvisionedKey::RemotelyProvisionedKey,
};
+use android_security_rkp_aidl::binder::{BinderFeatures, Interface, Strong};
use anyhow::{Context, Result};
-use binder::{BinderFeatures, Interface, StatusCode, Strong};
use message_macro::source_location_msg;
use std::sync::Mutex;
use std::time::Duration;
@@ -40,40 +41,6 @@
tokio::runtime::Builder::new_current_thread().enable_all().build().unwrap()
}
-/// Errors occurred during the interaction with RKPD.
-#[derive(Debug, Clone, Copy, thiserror::Error, PartialEq, Eq)]
-pub enum Error {
- /// An RKPD request gets cancelled.
- #[error("An RKPD request gets cancelled")]
- RequestCancelled,
-
- /// Failed to get registration.
- #[error("Failed to get registration")]
- GetRegistrationFailed,
-
- /// Failed to get key.
- #[error("Failed to get key: {0:?}")]
- GetKeyFailed(GetKeyErrorCode),
-
- /// Failed to store upgraded key.
- #[error("Failed to store upgraded key")]
- StoreUpgradedKeyFailed,
-
- /// Timeout when waiting for a callback.
- #[error("Timeout when waiting for a callback")]
- Timeout,
-
- /// Wraps a Binder status code.
- #[error("Binder transaction error {0:?}")]
- BinderTransaction(StatusCode),
-}
-
-impl From<StatusCode> for Error {
- fn from(s: StatusCode) -> Self {
- Self::BinderTransaction(s)
- }
-}
-
/// Thread-safe channel for sending a value once and only once. If a value has
/// already been send, subsequent calls to send will noop.
struct SafeSender<T> {
@@ -120,17 +87,17 @@
fn onCancel(&self) -> binder::Result<()> {
log::warn!("IGetRegistrationCallback cancelled");
self.registration_tx.send(
- Err(Error::RequestCancelled)
+ Err(Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR))
.context(source_location_msg!("GetRegistrationCallback cancelled.")),
);
Ok(())
}
fn onError(&self, description: &str) -> binder::Result<()> {
log::error!("IGetRegistrationCallback failed: '{description}'");
- self.registration_tx.send(
- Err(Error::GetRegistrationFailed)
- .context(source_location_msg!("GetRegistrationCallback failed: {:?}", description)),
- );
+ self.registration_tx
+ .send(Err(Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)).context(
+ source_location_msg!("GetRegistrationCallback failed: {:?}", description),
+ ));
Ok(())
}
}
@@ -138,8 +105,7 @@
/// Make a new connection to a IRegistration service.
async fn get_rkpd_registration(rpc_name: &str) -> Result<binder::Strong<dyn IRegistration>> {
let remote_provisioning: Strong<dyn IRemoteProvisioning> =
- binder::get_interface("remote_provisioning")
- .map_err(Error::from)
+ map_binder_status_code(binder::get_interface("remote_provisioning"))
.context(source_location_msg!("Trying to connect to IRemoteProvisioning service."))?;
let (tx, rx) = oneshot::channel();
@@ -150,7 +116,8 @@
.context(source_location_msg!("Trying to get registration."))?;
match timeout(RKPD_TIMEOUT, rx).await {
- Err(e) => Err(Error::Timeout).context(source_location_msg!("Waiting for RKPD: {:?}", e)),
+ Err(e) => Err(Error::Rc(ResponseCode::SYSTEM_ERROR))
+ .context(source_location_msg!("Waiting for RKPD: {:?}", e)),
Ok(v) => v.unwrap(),
}
}
@@ -181,13 +148,28 @@
fn onCancel(&self) -> binder::Result<()> {
log::warn!("IGetKeyCallback cancelled");
self.key_tx.send(
- Err(Error::RequestCancelled).context(source_location_msg!("GetKeyCallback cancelled.")),
+ Err(Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR))
+ .context(source_location_msg!("GetKeyCallback cancelled.")),
);
Ok(())
}
fn onError(&self, error: GetKeyErrorCode, description: &str) -> binder::Result<()> {
log::error!("IGetKeyCallback failed: {description}");
- self.key_tx.send(Err(Error::GetKeyFailed(error)).context(source_location_msg!(
+ let rc = match error {
+ GetKeyErrorCode::ERROR_UNKNOWN => ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR,
+ GetKeyErrorCode::ERROR_PERMANENT => ResponseCode::OUT_OF_KEYS_PERMANENT_ERROR,
+ GetKeyErrorCode::ERROR_PENDING_INTERNET_CONNECTIVITY => {
+ ResponseCode::OUT_OF_KEYS_PENDING_INTERNET_CONNECTIVITY
+ }
+ GetKeyErrorCode::ERROR_REQUIRES_SECURITY_PATCH => {
+ ResponseCode::OUT_OF_KEYS_REQUIRES_SYSTEM_UPGRADE
+ }
+ _ => {
+ log::error!("Unexpected error from rkpd: {error:?}");
+ ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR
+ }
+ };
+ self.key_tx.send(Err(Error::Rc(rc)).context(source_location_msg!(
"GetKeyCallback failed: {:?} {:?}",
error,
description
@@ -213,7 +195,7 @@
if let Err(e) = registration.cancelGetKey(&cb) {
log::error!("IRegistration::cancelGetKey failed: {:?}", e);
}
- Err(Error::Timeout)
+ Err(Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR))
.context(source_location_msg!("Waiting for RKPD key timed out: {:?}", e))
}
Ok(v) => v.unwrap(),
@@ -252,9 +234,9 @@
}
fn onError(&self, error: &str) -> binder::Result<()> {
- log::error!("IStoreUpgradedKeyCallback failed: {error}");
+ log::error!("IGetRegistrationCallback failed: {error}");
self.completer.send(
- Err(Error::StoreUpgradedKeyFailed)
+ Err(Error::Rc(ResponseCode::SYSTEM_ERROR))
.context(source_location_msg!("Failed to store upgraded key: {:?}", error)),
);
Ok(())
@@ -274,7 +256,7 @@
.context(source_location_msg!("Failed to store upgraded blob with RKPD."))?;
match timeout(RKPD_TIMEOUT, rx).await {
- Err(e) => Err(Error::Timeout)
+ Err(e) => Err(Error::Rc(ResponseCode::SYSTEM_ERROR))
.context(source_location_msg!("Waiting for RKPD to complete storing key: {:?}", e)),
Ok(v) => v.unwrap(),
}
@@ -309,6 +291,7 @@
mod tests {
use super::*;
use android_security_rkp_aidl::aidl::android::security::rkp::IRegistration::BnRegistration;
+ use std::collections::HashMap;
use std::sync::atomic::{AtomicU32, Ordering};
use std::sync::{Arc, Mutex};
@@ -432,7 +415,10 @@
assert!(cb.onCancel().is_ok());
let result = tokio_rt().block_on(rx).unwrap();
- assert_eq!(result.unwrap_err().downcast::<Error>().unwrap(), Error::RequestCancelled);
+ assert_eq!(
+ result.unwrap_err().downcast::<Error>().unwrap(),
+ Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
+ );
}
#[test]
@@ -442,7 +428,10 @@
assert!(cb.onError("error").is_ok());
let result = tokio_rt().block_on(rx).unwrap();
- assert_eq!(result.unwrap_err().downcast::<Error>().unwrap(), Error::GetRegistrationFailed);
+ assert_eq!(
+ result.unwrap_err().downcast::<Error>().unwrap(),
+ Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
+ );
}
#[test]
@@ -464,11 +453,29 @@
assert!(cb.onCancel().is_ok());
let result = tokio_rt().block_on(rx).unwrap();
- assert_eq!(result.unwrap_err().downcast::<Error>().unwrap(), Error::RequestCancelled);
+ assert_eq!(
+ result.unwrap_err().downcast::<Error>().unwrap(),
+ Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
+ );
}
#[test]
fn test_get_key_cb_error() {
+ let error_mapping = HashMap::from([
+ (GetKeyErrorCode::ERROR_UNKNOWN, ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR),
+ (GetKeyErrorCode::ERROR_PERMANENT, ResponseCode::OUT_OF_KEYS_PERMANENT_ERROR),
+ (
+ GetKeyErrorCode::ERROR_PENDING_INTERNET_CONNECTIVITY,
+ ResponseCode::OUT_OF_KEYS_PENDING_INTERNET_CONNECTIVITY,
+ ),
+ (
+ GetKeyErrorCode::ERROR_REQUIRES_SECURITY_PATCH,
+ ResponseCode::OUT_OF_KEYS_REQUIRES_SYSTEM_UPGRADE,
+ ),
+ ]);
+
+ // Loop over the generated list of enum values to better ensure this test stays in
+ // sync with the AIDL.
for get_key_error in GetKeyErrorCode::enum_values() {
let (tx, rx) = oneshot::channel();
let cb = GetKeyCallback::new_native_binder(tx);
@@ -477,7 +484,7 @@
let result = tokio_rt().block_on(rx).unwrap();
assert_eq!(
result.unwrap_err().downcast::<Error>().unwrap(),
- Error::GetKeyFailed(get_key_error),
+ Error::Rc(error_mapping[&get_key_error]),
);
}
}
@@ -498,7 +505,10 @@
assert!(cb.onError("oh no! it failed").is_ok());
let result = tokio_rt().block_on(rx).unwrap();
- assert_eq!(result.unwrap_err().downcast::<Error>().unwrap(), Error::StoreUpgradedKeyFailed);
+ assert_eq!(
+ result.unwrap_err().downcast::<Error>().unwrap(),
+ Error::Rc(ResponseCode::SYSTEM_ERROR)
+ );
}
#[test]
@@ -522,7 +532,10 @@
let result =
tokio_rt().block_on(get_rkpd_attestation_key_from_registration_async(®istration, 0));
- assert_eq!(result.unwrap_err().downcast::<Error>().unwrap(), Error::Timeout);
+ assert_eq!(
+ result.unwrap_err().downcast::<Error>().unwrap(),
+ Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
+ );
}
#[test]
@@ -547,7 +560,10 @@
&[],
&[],
));
- assert_eq!(result.unwrap_err().downcast::<Error>().unwrap(), Error::Timeout);
+ assert_eq!(
+ result.unwrap_err().downcast::<Error>().unwrap(),
+ Error::Rc(ResponseCode::SYSTEM_ERROR)
+ );
}
#[test]
diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index f2b332b..830fbe1 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -19,9 +19,7 @@
log_key_deleted, log_key_generated, log_key_imported, log_key_integrity_violation,
};
use crate::database::{BlobInfo, CertificateInfo, KeyIdGuard};
-use crate::error::{
- self, map_km_error, map_or_log_err, wrapped_rkpd_error_to_ks_error, Error, ErrorCode,
-};
+use crate::error::{self, map_km_error, map_or_log_err, Error, ErrorCode};
use crate::globals::{
get_remotely_provisioned_component_name, DB, ENFORCEMENTS, LEGACY_IMPORTER, SUPER_KEY,
};
@@ -902,11 +900,8 @@
f,
|upgraded_blob| {
let _wp = wd::watch_millis("Calling store_rkpd_attestation_key()", 500);
- if let Err(e) = store_rkpd_attestation_key(&rpc_name, key_blob, upgraded_blob) {
- Err(wrapped_rkpd_error_to_ks_error(&e)).context(format!("{e:?}"))
- } else {
- Ok(())
- }
+ store_rkpd_attestation_key(&rpc_name, key_blob, upgraded_blob)
+ .context(ks_err!("Failed store_rkpd_attestation_key()."))
},
)
.context(ks_err!())