tequilaOS/platform_system_security
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge "Revert^2 "[rkpd_client] Add Error type to rkpd_client"" into main am: 5c96cefe87

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

Change-Id: I774724e99e87aa1f1eaf3cb71693525dee90e23b
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
Alice Wang 2023-11-13 22:13:11 +00:00 committed by Automerger Merge Worker
commit 9e519c85b2
4 changed files with 146 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

76
keystore2/src/error.rs
View file

@ -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 Error {
}
}
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 @@ pub mod tests {
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

3
keystore2/src/remote_provisioning.rs
View file

@ -31,6 +31,7 @@ use anyhow::{Context, Result};
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 @@ impl RemProvState {
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(

132
keystore2/src/rkpd_client.rs
View file

@ -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 @@ use android_security_rkp_aidl::aidl::android::security::rkp::{
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 @@ fn tokio_rt() -> tokio::runtime::Runtime {
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 @@ impl IGetRegistrationCallback for GetRegistrationCallback {
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 @@ impl IGetRegistrationCallback for GetRegistrationCallback {
/// 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 @@ async fn get_rkpd_registration(rpc_name: &str) -> Result<binder::Strong<dyn IReg
.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 @@ impl IGetKeyCallback for GetKeyCallback {
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 @@ async fn get_rkpd_attestation_key_from_registration_async(
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 @@ impl IStoreUpgradedKeyCallback for StoreUpgradedKeyCallback {
}
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 @@ async fn store_rkpd_attestation_key_with_registration_async(
.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 @@ pub fn store_rkpd_attestation_key(
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 @@ mod tests {
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 @@ mod tests {
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 @@ mod tests {
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 @@ mod tests {
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 @@ mod tests {
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 @@ mod tests {
let result =
tokio_rt().block_on(get_rkpd_attestation_key_from_registration_async(&registration, 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 @@ mod tests {
&[],
&[],
));
assert_eq!(
result.unwrap_err().downcast::<Error>().unwrap(),
Error::Rc(ResponseCode::SYSTEM_ERROR)
);
assert_eq!(result.unwrap_err().downcast::<Error>().unwrap(), Error::Timeout);
}
#[test]

11
keystore2/src/security_level.rs
View file

@ -19,7 +19,9 @@ use crate::audit_log::{
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 @@ impl KeystoreSecurityLevel {
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!())