Multi-threaded keystore
This patches changes the keystore to use the asychronous api model for
begin, update, finish, and abort.
Also removes unused class KeystoreArguments (aidl and implementation).
Test: Keystore CTS tests
Bug: 111443219
Change-Id: Icc6def9ff6dbe32193272d7d015079a006ebc430
diff --git a/keystore-engine/keystore_backend_binder.cpp b/keystore-engine/keystore_backend_binder.cpp
index cd407c8..0e38b50 100644
--- a/keystore-engine/keystore_backend_binder.cpp
+++ b/keystore-engine/keystore_backend_binder.cpp
@@ -23,18 +23,22 @@
#include "keystore_backend_binder.h"
#include <android-base/logging.h>
-#include <android/security/IKeystoreService.h>
+#include <android/security/keystore/IKeystoreService.h>
#include <binder/IServiceManager.h>
#include <keystore/KeyCharacteristics.h>
#include <keystore/KeymasterArguments.h>
#include <keystore/KeymasterBlob.h>
+#include <keystore/KeystoreResponse.h>
#include <keystore/OperationResult.h>
#include <keystore/keymaster_types.h>
#include <keystore/keystore.h>
#include <keystore/keystore_hidl_support.h>
+#include <keystore/keystore_promises.h>
#include <keystore/keystore_return_types.h>
-using android::security::IKeystoreService;
+#include <future>
+
+using android::security::keystore::IKeystoreService;
using namespace android;
using keystore::hidl_vec;
@@ -60,7 +64,13 @@
namespace {
const char keystore_service_name[] = "android.security.keystore";
constexpr int32_t UID_SELF = -1;
-};
+
+using keystore::KeyCharacteristicsPromise;
+using keystore::KeystoreExportPromise;
+using keystore::KeystoreResponsePromise;
+using keystore::OperationResultPromise;
+
+} // namespace
#define AT __func__ << ":" << __LINE__ << " "
@@ -88,20 +98,29 @@
return -1;
}
- KeyCharacteristics keyCharacteristics;
String16 key_name16(key_id);
- int32_t aidl_result;
- auto binder_result = service->getKeyCharacteristics(
- key_name16, KeymasterBlob(), KeymasterBlob(), UID_SELF, &keyCharacteristics, &aidl_result);
+ int32_t error_code;
+ android::sp<KeyCharacteristicsPromise> kc_promise(new KeyCharacteristicsPromise);
+ auto kc_future = kc_promise->get_future();
+ auto binder_result = service->getKeyCharacteristics(kc_promise, key_name16, KeymasterBlob(),
+ KeymasterBlob(), UID_SELF, &error_code);
if (!binder_result.isOk()) {
LOG(ERROR) << AT << "communication error while calling keystore";
return -1;
}
- if (KSReturn(aidl_result).isOk()) {
- LOG(ERROR) << AT << "getKeyCharacteristics failed: " << aidl_result;
+ if (KSReturn(error_code).isOk()) {
+ LOG(ERROR) << AT << "getKeyCharacteristics failed: " << error_code;
+ return -1;
}
- auto algorithm = getKeyAlgoritmFromKeyCharacteristics(keyCharacteristics);
+ auto [km_response, characteristics] = kc_future.get();
+
+ if (KSReturn(km_response.response_code()).isOk()) {
+ LOG(ERROR) << AT << "getKeyCharacteristics failed: " << km_response.response_code();
+ return -1;
+ }
+
+ auto algorithm = getKeyAlgoritmFromKeyCharacteristics(characteristics);
if (!algorithm.isOk()) {
LOG(ERROR) << AT << "could not get algorithm from key characteristics";
return -1;
@@ -113,14 +132,23 @@
params[2] = Authorization(TAG_ALGORITHM, algorithm.value());
android::sp<android::IBinder> token(new android::BBinder);
- OperationResult result;
- binder_result = service->begin(token, key_name16, (int)KeyPurpose::SIGN, true /*pruneable*/,
- KeymasterArguments(params), std::vector<uint8_t>() /* entropy */,
- UID_SELF, &result);
+ sp<OperationResultPromise> promise(new OperationResultPromise());
+ auto future = promise->get_future();
+ binder_result = service->begin(promise, token, key_name16, (int)KeyPurpose::SIGN,
+ true /*pruneable*/, KeymasterArguments(params),
+ std::vector<uint8_t>() /* entropy */, UID_SELF, &error_code);
if (!binder_result.isOk()) {
LOG(ERROR) << AT << "communication error while calling keystore";
return -1;
}
+
+ keystore::KeyStoreNativeReturnCode rc(error_code);
+ if (!rc.isOk()) {
+ LOG(ERROR) << AT << "Keystore begin returned: " << error_code;
+ return -1;
+ }
+ OperationResult result = future.get();
+
if (!result.resultCode.isOk()) {
LOG(ERROR) << AT << "begin failed: " << int32_t(result.resultCode);
return -1;
@@ -128,32 +156,71 @@
auto handle = std::move(result.token);
do {
- binder_result = service->update(handle, KeymasterArguments(params),
- std::vector<uint8_t>(in, in + len), &result);
+ future = {};
+ promise = new OperationResultPromise();
+ future = promise->get_future();
+ binder_result = service->update(promise, handle, KeymasterArguments(params),
+ std::vector<uint8_t>(in, in + len), &error_code);
if (!binder_result.isOk()) {
LOG(ERROR) << AT << "communication error while calling keystore";
return -1;
}
+
+ rc = keystore::KeyStoreNativeReturnCode(error_code);
+ if (!rc.isOk()) {
+ LOG(ERROR) << AT << "Keystore update returned: " << error_code;
+ return -1;
+ }
+ result = future.get();
+
if (!result.resultCode.isOk()) {
LOG(ERROR) << AT << "update failed: " << int32_t(result.resultCode);
return -1;
}
+
if (result.inputConsumed > len) {
LOG(ERROR) << AT << "update consumed more data than provided";
- service->abort(handle, &aidl_result);
+ sp<KeystoreResponsePromise> abortPromise(new KeystoreResponsePromise);
+ auto abortFuture = abortPromise->get_future();
+ binder_result = service->abort(abortPromise, handle, &error_code);
+ if (!binder_result.isOk()) {
+ LOG(ERROR) << AT << "communication error while calling keystore";
+ return -1;
+ }
+ // This is mainly for logging since we already failed.
+ // But if abort returned OK we have to wait untill abort calls the callback
+ // hence the call to abortFuture.get().
+ if (!KSReturn(error_code).isOk()) {
+ LOG(ERROR) << AT << "abort failed: " << error_code;
+ } else if (!(rc = KSReturn(abortFuture.get().response_code())).isOk()) {
+ LOG(ERROR) << AT << "abort failed: " << int32_t(rc);
+ }
return -1;
}
len -= result.inputConsumed;
in += result.inputConsumed;
} while (len > 0);
- binder_result =
- service->finish(handle, KeymasterArguments(params), std::vector<uint8_t>() /* signature */,
- std::vector<uint8_t>() /* entropy */, &result);
+ future = {};
+ promise = new OperationResultPromise();
+ future = promise->get_future();
+
+ binder_result = service->finish(promise, handle, KeymasterArguments(params),
+ std::vector<uint8_t>() /* signature */,
+ std::vector<uint8_t>() /* entropy */, &error_code);
+
if (!binder_result.isOk()) {
LOG(ERROR) << AT << "communication error while calling keystore";
return -1;
}
+
+ rc = keystore::KeyStoreNativeReturnCode(error_code);
+ if (!rc.isOk()) {
+ LOG(ERROR) << AT << "Keystore finish returned: " << error_code;
+ return -1;
+ }
+ result = future.get();
+
if (!result.resultCode.isOk()) {
LOG(ERROR) << AT << "finish failed: " << int32_t(result.resultCode);
return -1;
@@ -180,25 +247,34 @@
return -1;
}
- ExportResult result;
- auto binder_result = service->exportKey(String16(key_id), static_cast<int32_t>(KeyFormat::X509),
- KeymasterBlob() /* clientId */,
- KeymasterBlob() /* appData */, UID_SELF, &result);
+ int32_t error_code;
+ android::sp<KeystoreExportPromise> promise(new KeystoreExportPromise);
+ auto future = promise->get_future();
+ auto binder_result = service->exportKey(
+ promise, String16(key_id), static_cast<int32_t>(KeyFormat::X509),
+ KeymasterBlob() /* clientId */, KeymasterBlob() /* appData */, UID_SELF, &error_code);
if (!binder_result.isOk()) {
LOG(ERROR) << AT << "communication error while calling keystore";
return -1;
}
- if (!result.resultCode.isOk()) {
- LOG(ERROR) << AT << "exportKey failed: " << int32_t(result.resultCode);
+
+ KSReturn rc(error_code);
+ if (!rc.isOk()) {
+ LOG(ERROR) << AT << "exportKey failed: " << error_code;
return -1;
}
- hidl_vec<uint8_t> reply_hidl(result.exportData);
+ auto export_result = future.get();
+ if (!export_result.resultCode.isOk()) {
+ LOG(ERROR) << AT << "exportKey failed: " << int32_t(export_result.resultCode);
+ return -1;
+ }
+
if (pubkey_len) {
- *pubkey_len = reply_hidl.size();
+ *pubkey_len = export_result.exportData.size();
}
if (pubkey) {
- *pubkey = reply_hidl.releaseData();
+ *pubkey = export_result.exportData.releaseData();
}
return 0;
}