Revert^2 "[rkpd_client] Add Error type to rkpd_client"
This reverts commit f84c46c3b3cad3ea4908ab44c361f637c7fcdb09.
Reason for revert: Reland the original cl aosp/2821995
with an adjustment about the Timeout error type in order
to maintain the original ResponseCode.
Test: atest RkpdAppIntegrationTests
Bug: 310139666
Change-Id: Id4ee05eb616c125f9d28b25f4668ca3071ccb26c
diff --git a/keystore2/src/error.rs b/keystore2/src/error.rs
index 1a048b6..ac5ba4c 100644
--- a/keystore2/src/error.rs
+++ b/keystore2/src/error.rs
@@ -27,7 +27,9 @@
//! 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,
@@ -66,6 +68,49 @@
}
}
+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::RetryableTimeout => Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR),
+ 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.,
@@ -409,4 +454,35 @@
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::RetryableTimeout, ResponseCode::OUT_OF_KEYS_TRANSIENT_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 3f7833e..14c61fb 100644
--- a/keystore2/src/remote_provisioning.rs
+++ b/keystore2/src/remote_provisioning.rs
@@ -31,6 +31,7 @@
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;
@@ -102,7 +103,7 @@
Err(e) => {
if self.is_rkp_only() {
log::error!("Error occurred: {:?}", e);
- return Err(e);
+ return Err(wrapped_rkpd_error_to_ks_error(&e)).context(format!("{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 fe64150..d8a5276 100644
--- a/keystore2/src/rkpd_client.rs
+++ b/keystore2/src/rkpd_client.rs
@@ -14,7 +14,6 @@
//! 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,
@@ -24,8 +23,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;
@@ -41,6 +40,44 @@
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,
+
+ /// Retryable timeout when waiting for a callback.
+ #[error("Retryable timeout when waiting for a callback")]
+ RetryableTimeout,
+
+ /// 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> {
@@ -87,17 +124,17 @@
fn onCancel(&self) -> binder::Result<()> {
log::warn!("IGetRegistrationCallback cancelled");
self.registration_tx.send(
- Err(Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR))
+ Err(Error::RequestCancelled)
.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::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)).context(
- source_location_msg!("GetRegistrationCallback failed: {:?}", description),
- ));
+ self.registration_tx.send(
+ Err(Error::GetRegistrationFailed)
+ .context(source_location_msg!("GetRegistrationCallback failed: {:?}", description)),
+ );
Ok(())
}
}
@@ -105,7 +142,8 @@
/// 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> =
- map_binder_status_code(binder::get_interface("remote_provisioning"))
+ binder::get_interface("remote_provisioning")
+ .map_err(Error::from)
.context(source_location_msg!("Trying to connect to IRemoteProvisioning service."))?;
let (tx, rx) = oneshot::channel();
@@ -116,8 +154,7 @@
.context(source_location_msg!("Trying to get registration."))?;
match timeout(RKPD_TIMEOUT, rx).await {
- Err(e) => Err(Error::Rc(ResponseCode::SYSTEM_ERROR))
- .context(source_location_msg!("Waiting for RKPD: {:?}", e)),
+ Err(e) => Err(Error::Timeout).context(source_location_msg!("Waiting for RKPD: {:?}", e)),
Ok(v) => v.unwrap(),
}
}
@@ -148,28 +185,13 @@
fn onCancel(&self) -> binder::Result<()> {
log::warn!("IGetKeyCallback cancelled");
self.key_tx.send(
- Err(Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR))
- .context(source_location_msg!("GetKeyCallback cancelled.")),
+ Err(Error::RequestCancelled).context(source_location_msg!("GetKeyCallback cancelled.")),
);
Ok(())
}
fn onError(&self, error: GetKeyErrorCode, description: &str) -> binder::Result<()> {
log::error!("IGetKeyCallback failed: {description}");
- 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!(
+ self.key_tx.send(Err(Error::GetKeyFailed(error)).context(source_location_msg!(
"GetKeyCallback failed: {:?} {:?}",
error,
description
@@ -195,7 +217,7 @@
if let Err(e) = registration.cancelGetKey(&cb) {
log::error!("IRegistration::cancelGetKey failed: {:?}", e);
}
- Err(Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR))
+ Err(Error::RetryableTimeout)
.context(source_location_msg!("Waiting for RKPD key timed out: {:?}", e))
}
Ok(v) => v.unwrap(),
@@ -234,9 +256,9 @@
}
fn onError(&self, error: &str) -> binder::Result<()> {
- log::error!("IGetRegistrationCallback failed: {error}");
+ log::error!("IStoreUpgradedKeyCallback failed: {error}");
self.completer.send(
- Err(Error::Rc(ResponseCode::SYSTEM_ERROR))
+ Err(Error::StoreUpgradedKeyFailed)
.context(source_location_msg!("Failed to store upgraded key: {:?}", error)),
);
Ok(())
@@ -256,7 +278,7 @@
.context(source_location_msg!("Failed to store upgraded blob with RKPD."))?;
match timeout(RKPD_TIMEOUT, rx).await {
- Err(e) => Err(Error::Rc(ResponseCode::SYSTEM_ERROR))
+ Err(e) => Err(Error::Timeout)
.context(source_location_msg!("Waiting for RKPD to complete storing key: {:?}", e)),
Ok(v) => v.unwrap(),
}
@@ -291,7 +313,6 @@
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};
@@ -415,10 +436,7 @@
assert!(cb.onCancel().is_ok());
let result = tokio_rt().block_on(rx).unwrap();
- assert_eq!(
- result.unwrap_err().downcast::<Error>().unwrap(),
- Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
- );
+ assert_eq!(result.unwrap_err().downcast::<Error>().unwrap(), Error::RequestCancelled);
}
#[test]
@@ -428,10 +446,7 @@
assert!(cb.onError("error").is_ok());
let result = tokio_rt().block_on(rx).unwrap();
- assert_eq!(
- result.unwrap_err().downcast::<Error>().unwrap(),
- Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
- );
+ assert_eq!(result.unwrap_err().downcast::<Error>().unwrap(), Error::GetRegistrationFailed);
}
#[test]
@@ -453,29 +468,11 @@
assert!(cb.onCancel().is_ok());
let result = tokio_rt().block_on(rx).unwrap();
- assert_eq!(
- result.unwrap_err().downcast::<Error>().unwrap(),
- Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
- );
+ assert_eq!(result.unwrap_err().downcast::<Error>().unwrap(), Error::RequestCancelled);
}
#[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);
@@ -484,7 +481,7 @@
let result = tokio_rt().block_on(rx).unwrap();
assert_eq!(
result.unwrap_err().downcast::<Error>().unwrap(),
- Error::Rc(error_mapping[&get_key_error]),
+ Error::GetKeyFailed(get_key_error),
);
}
}
@@ -505,10 +502,7 @@
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::Rc(ResponseCode::SYSTEM_ERROR)
- );
+ assert_eq!(result.unwrap_err().downcast::<Error>().unwrap(), Error::StoreUpgradedKeyFailed);
}
#[test]
@@ -532,10 +526,7 @@
let result =
tokio_rt().block_on(get_rkpd_attestation_key_from_registration_async(®istration, 0));
- assert_eq!(
- result.unwrap_err().downcast::<Error>().unwrap(),
- Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
- );
+ assert_eq!(result.unwrap_err().downcast::<Error>().unwrap(), Error::RetryableTimeout);
}
#[test]
@@ -560,10 +551,7 @@
&[],
&[],
));
- assert_eq!(
- result.unwrap_err().downcast::<Error>().unwrap(),
- Error::Rc(ResponseCode::SYSTEM_ERROR)
- );
+ assert_eq!(result.unwrap_err().downcast::<Error>().unwrap(), Error::Timeout);
}
#[test]
diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index 830fbe1..f2b332b 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -19,7 +19,9 @@
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, Error, ErrorCode};
+use crate::error::{
+ self, map_km_error, map_or_log_err, wrapped_rkpd_error_to_ks_error, Error, ErrorCode,
+};
use crate::globals::{
get_remotely_provisioned_component_name, DB, ENFORCEMENTS, LEGACY_IMPORTER, SUPER_KEY,
};
@@ -900,8 +902,11 @@
f,
|upgraded_blob| {
let _wp = wd::watch_millis("Calling store_rkpd_attestation_key()", 500);
- store_rkpd_attestation_key(&rpc_name, key_blob, upgraded_blob)
- .context(ks_err!("Failed store_rkpd_attestation_key()."))
+ 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(())
+ }
},
)
.context(ks_err!())