Handle errors codes from rkpd getKey operations
Now we'll get error codes bubbled up from rkpd (via the remote
provisioning system service). Convert those errors into meaningful
keystore errors so clients can act on them.
Test: keystore2_tests
Test: RkpdAppIntegrationTests
Test: CtsKeystoreTestCases:android.keystore.cts.KeyStoreExceptionTest
Bug: 264888027
Change-Id: Ib574fe4da0443f32f95f8579c4a308d36fe4b46f
diff --git a/identity/RemotelyProvisionedKey.cpp b/identity/RemotelyProvisionedKey.cpp
index 7e90d63..784a680 100644
--- a/identity/RemotelyProvisionedKey.cpp
+++ b/identity/RemotelyProvisionedKey.cpp
@@ -21,6 +21,7 @@
#include <android-base/logging.h>
#include <android/security/rkp/BnGetKeyCallback.h>
#include <android/security/rkp/BnGetRegistrationCallback.h>
+#include <android/security/rkp/IGetKeyCallback.h>
#include <android/security/rkp/IRemoteProvisioning.h>
#include <binder/IServiceManager.h>
#include <binder/Status.h>
@@ -38,6 +39,7 @@
using ::android::hardware::security::keymint::RpcHardwareInfo;
using ::android::security::rkp::BnGetKeyCallback;
using ::android::security::rkp::BnGetRegistrationCallback;
+using ::android::security::rkp::IGetKeyCallback;
using ::android::security::rkp::IRegistration;
using ::android::security::rkp::IRemoteProvisioning;
using ::android::security::rkp::RemotelyProvisionedKey;
@@ -96,11 +98,11 @@
keyPromise_.set_value(std::nullopt);
return Status::ok();
}
- Status onError(const String16& error) override {
+ Status onError(IGetKeyCallback::ErrorCode error, const String16& description) override {
if (called_.test_and_set()) {
return Status::ok();
}
- LOG(ERROR) << "GetKeyCallback failed: " << error;
+ LOG(ERROR) << "GetKeyCallback failed: " << static_cast<int>(error) << ", " << description;
keyPromise_.set_value(std::nullopt);
return Status::ok();
}
@@ -124,7 +126,8 @@
auto cb = sp<GetKeyCallback>::make(std::move(keyPromise_));
auto status = registration->getKey(keyId_, cb);
if (!status.isOk()) {
- cb->onError(String16("Failed to register GetKeyCallback"));
+ cb->onError(IGetKeyCallback::ErrorCode::ERROR_UNKNOWN,
+ String16("Failed to register GetKeyCallback"));
}
return Status::ok();
}
diff --git a/keystore2/src/error.rs b/keystore2/src/error.rs
index d1d58a4..3ca3942 100644
--- a/keystore2/src/error.rs
+++ b/keystore2/src/error.rs
@@ -71,11 +71,6 @@
pub fn perm() -> Self {
Error::Rc(ResponseCode::PERMISSION_DENIED)
}
-
- /// Short hand for `Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)`
- pub fn out_of_keys() -> Self {
- Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
- }
}
/// Helper function to map the binder status we get from calls into KeyMint
diff --git a/keystore2/src/rkpd_client.rs b/keystore2/src/rkpd_client.rs
index d0d036c..f1e8e11 100644
--- a/keystore2/src/rkpd_client.rs
+++ b/keystore2/src/rkpd_client.rs
@@ -13,16 +13,15 @@
// limitations under the License.
//! Helper wrapper around RKPD interface.
-// TODO(b/264891956): Return RKP specific errors.
-use crate::error::{map_binder_status_code, Error};
+use crate::error::{map_binder_status_code, Error, ResponseCode};
use crate::globals::get_remotely_provisioned_component_name;
use crate::ks_err;
use crate::utils::watchdog as wd;
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::SecurityLevel::SecurityLevel;
use android_security_rkp_aidl::aidl::android::security::rkp::{
- IGetKeyCallback::BnGetKeyCallback, IGetKeyCallback::IGetKeyCallback,
- IGetRegistrationCallback::BnGetRegistrationCallback,
+ IGetKeyCallback::BnGetKeyCallback, IGetKeyCallback::ErrorCode::ErrorCode as GetKeyErrorCode,
+ IGetKeyCallback::IGetKeyCallback, IGetRegistrationCallback::BnGetRegistrationCallback,
IGetRegistrationCallback::IGetRegistrationCallback, IRegistration::IRegistration,
IRemoteProvisioning::IRemoteProvisioning,
IStoreUpgradedKeyCallback::BnStoreUpgradedKeyCallback,
@@ -30,7 +29,6 @@
RemotelyProvisionedKey::RemotelyProvisionedKey,
};
use android_security_rkp_aidl::binder::{BinderFeatures, Interface, Strong};
-use android_system_keystore2::aidl::android::system::keystore2::ResponseCode::ResponseCode;
use anyhow::{Context, Result};
use std::sync::Mutex;
use std::time::Duration;
@@ -91,17 +89,17 @@
let _wp = wd::watch_millis("IGetRegistrationCallback::onCancel", 500);
log::warn!("IGetRegistrationCallback cancelled");
self.registration_tx.send(
- Err(Error::Rc(ResponseCode::OUT_OF_KEYS))
+ Err(Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR))
.context(ks_err!("GetRegistrationCallback cancelled.")),
);
Ok(())
}
- fn onError(&self, error: &str) -> binder::Result<()> {
+ fn onError(&self, description: &str) -> binder::Result<()> {
let _wp = wd::watch_millis("IGetRegistrationCallback::onError", 500);
- log::error!("IGetRegistrationCallback failed: '{error}'");
+ log::error!("IGetRegistrationCallback failed: '{description}'");
self.registration_tx.send(
- Err(Error::Rc(ResponseCode::OUT_OF_KEYS))
- .context(ks_err!("GetRegistrationCallback failed: {:?}", error)),
+ Err(Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR))
+ .context(ks_err!("GetRegistrationCallback failed: {:?}", description)),
);
Ok(())
}
@@ -161,17 +159,33 @@
let _wp = wd::watch_millis("IGetKeyCallback::onCancel", 500);
log::warn!("IGetKeyCallback cancelled");
self.key_tx.send(
- Err(Error::Rc(ResponseCode::OUT_OF_KEYS)).context(ks_err!("GetKeyCallback cancelled.")),
+ Err(Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR))
+ .context(ks_err!("GetKeyCallback cancelled.")),
);
Ok(())
}
- fn onError(&self, error: &str) -> binder::Result<()> {
+ fn onError(&self, error: GetKeyErrorCode, description: &str) -> binder::Result<()> {
let _wp = wd::watch_millis("IGetKeyCallback::onError", 500);
- log::error!("IGetKeyCallback failed: {error}");
- self.key_tx.send(
- Err(Error::Rc(ResponseCode::OUT_OF_KEYS))
- .context(ks_err!("GetKeyCallback failed: {:?}", error)),
- );
+ 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(ks_err!(
+ "GetKeyCallback failed: {:?} {:?}",
+ error,
+ description
+ )));
Ok(())
}
}
@@ -188,7 +202,7 @@
.context(ks_err!("Trying to get key."))?;
match timeout(RKPD_TIMEOUT, rx).await {
- Err(e) => Err(Error::Rc(ResponseCode::OUT_OF_KEYS))
+ Err(e) => Err(Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR))
.context(ks_err!("Waiting for RKPD key timed out: {:?}", e)),
Ok(v) => v.unwrap(),
}
@@ -298,6 +312,7 @@
};
use android_security_rkp_aidl::aidl::android::security::rkp::IRegistration::BnRegistration;
use keystore2_crypto::parse_subject_from_certificate;
+ use std::collections::HashMap;
use std::sync::atomic::{AtomicU32, Ordering};
#[derive(Default)]
@@ -403,7 +418,7 @@
let result = tokio_rt().block_on(rx).unwrap();
assert_eq!(
result.unwrap_err().downcast::<Error>().unwrap(),
- Error::Rc(ResponseCode::OUT_OF_KEYS)
+ Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
);
}
@@ -416,7 +431,7 @@
let result = tokio_rt().block_on(rx).unwrap();
assert_eq!(
result.unwrap_err().downcast::<Error>().unwrap(),
- Error::Rc(ResponseCode::OUT_OF_KEYS)
+ Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
);
}
@@ -441,21 +456,38 @@
let result = tokio_rt().block_on(rx).unwrap();
assert_eq!(
result.unwrap_err().downcast::<Error>().unwrap(),
- Error::Rc(ResponseCode::OUT_OF_KEYS)
+ Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
);
}
#[test]
fn test_get_key_cb_error() {
- let (tx, rx) = oneshot::channel();
- let cb = GetKeyCallback::new_native_binder(tx);
- assert!(cb.onError("error").is_ok());
+ 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,
+ ),
+ ]);
- let result = tokio_rt().block_on(rx).unwrap();
- assert_eq!(
- result.unwrap_err().downcast::<Error>().unwrap(),
- Error::Rc(ResponseCode::OUT_OF_KEYS)
- );
+ // 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);
+ assert!(cb.onError(get_key_error, "error").is_ok());
+
+ let result = tokio_rt().block_on(rx).unwrap();
+ assert_eq!(
+ result.unwrap_err().downcast::<Error>().unwrap(),
+ Error::Rc(error_mapping[&get_key_error]),
+ );
+ }
}
#[test]
@@ -503,7 +535,7 @@
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)
+ Error::Rc(ResponseCode::OUT_OF_KEYS_TRANSIENT_ERROR)
);
}
