From: Janis Danisevskis Date: Fri, 2 Nov 2018 21:51:02 +0000 (-0700) Subject: Multi-threaded Keystore X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=def8d71e399ef68399610158b64a1d578f25b453;p=android-x86%2Fsystem-hardware-interfaces.git Multi-threaded Keystore This patch changes the calling code in the wifi interface to use the new asynchronous keystore api model. Test: would be nice Bug: 111443219 Change-Id: Iee6a6fede4670dfe503dc7b87a34b88c1c8f01f4 --- diff --git a/wifi/keystore/1.0/default/Android.bp b/wifi/keystore/1.0/default/Android.bp index b569113..e7d1d35 100644 --- a/wifi/keystore/1.0/default/Android.bp +++ b/wifi/keystore/1.0/default/Android.bp @@ -20,4 +20,5 @@ cc_library_shared { "libutils", ], export_include_dirs: ["include"], + cpp_std: "c++17", } diff --git a/wifi/keystore/1.0/default/include/wifikeystorehal/keystore.h b/wifi/keystore/1.0/default/include/wifikeystorehal/keystore.h index 48b2c39..2e2bed6 100644 --- a/wifi/keystore/1.0/default/include/wifikeystorehal/keystore.h +++ b/wifi/keystore/1.0/default/include/wifikeystorehal/keystore.h @@ -5,7 +5,7 @@ #include #include -#include +#include #include namespace android { diff --git a/wifi/keystore/1.0/default/keystore.cpp b/wifi/keystore/1.0/default/keystore.cpp index 23aed30..ac22528 100644 --- a/wifi/keystore/1.0/default/keystore.cpp +++ b/wifi/keystore/1.0/default/keystore.cpp @@ -1,17 +1,22 @@ #include -#include +#include +#include +#include #include #include #include #include #include +#include #include #include #include #include +#include #include +#include #include #include "include/wifikeystorehal/keystore.h" @@ -37,6 +42,12 @@ using KSReturn = keystore::KeyStoreNativeReturnCode; namespace { constexpr const char kKeystoreServiceName[] = "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__ << " " @@ -48,7 +59,7 @@ namespace keystore { namespace V1_0 { namespace implementation { -using security::IKeystoreService; +using security::keystore::IKeystoreService; // Methods from ::android::hardware::wifi::keystore::V1_0::IKeystore follow. Return Keystore::getBlob(const hidl_string& key, getBlob_cb _hidl_cb) { sp service = interface_cast( @@ -79,22 +90,33 @@ Return Keystore::getPublicKey(const hidl_string& keyId, getPublicKey_cb _h return Void(); } - ExportResult result; + int32_t error_code; + android::sp promise(new KeystoreExportPromise); + auto future = promise->get_future(); auto binder_result = service->exportKey( - String16(keyId.c_str()), static_cast(KeyFormat::X509), - KeymasterBlob() /* clientId */, KeymasterBlob() /* appData */, UID_SELF, &result); + promise, String16(keyId.c_str()), static_cast(KeyFormat::X509), + KeymasterBlob() /* clientId */, KeymasterBlob() /* appData */, UID_SELF, &error_code); if (!binder_result.isOk()) { LOG(ERROR) << AT << "communication error while calling keystore"; _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); return Void(); } - 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; _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); return Void(); } - _hidl_cb(KeystoreStatusCode::SUCCESS, result.exportData); + auto export_result = future.get(); + if (!export_result.resultCode.isOk()) { + LOG(ERROR) << AT << "exportKey failed: " << int32_t(export_result.resultCode); + _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); + return Void(); + } + + _hidl_cb(KeystoreStatusCode::SUCCESS, export_result.exportData); return Void(); } @@ -123,21 +145,33 @@ Return Keystore::sign(const hidl_string& keyId, const hidl_vec& d return Void(); } - KeyCharacteristics keyCharacteristics; String16 key_name16(keyId.c_str()); - int32_t aidl_result; - auto binder_result = service->getKeyCharacteristics( - key_name16, KeymasterBlob(), KeymasterBlob(), UID_SELF, &keyCharacteristics, &aidl_result); + int32_t error_code; + android::sp 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"; _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); return Void(); } - if (KSReturn(aidl_result).isOk()) { - LOG(ERROR) << AT << "getKeyCharacteristics failed: " << aidl_result; + KSReturn rc(error_code); + if (!rc.isOk()) { + LOG(ERROR) << AT << "getKeyCharacteristics failed: " << error_code; + _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); + return Void(); + } + + auto [km_response, characteristics] = kc_future.get(); + + if (KSReturn(km_response.response_code()).isOk()) { + LOG(ERROR) << AT << "getKeyCharacteristics failed: " << km_response.response_code(); + _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); + return Void(); } - auto algorithm = getKeyAlgoritmFromKeyCharacteristics(keyCharacteristics); + auto algorithm = getKeyAlgoritmFromKeyCharacteristics(characteristics); if (!algorithm.isOk()) { LOG(ERROR) << AT << "could not get algorithm from key characteristics"; _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); @@ -150,15 +184,25 @@ Return Keystore::sign(const hidl_string& keyId, const hidl_vec& d params[2] = Authorization(TAG_ALGORITHM, algorithm.value()); android::sp token(new android::BBinder); - OperationResult result; - binder_result = service->begin(token, key_name16, (int)KeyPurpose::SIGN, true /*pruneable*/, - KeymasterArguments(params), std::vector() /* entropy */, - UID_SELF, &result); + sp 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() /* entropy */, UID_SELF, &error_code); if (!binder_result.isOk()) { LOG(ERROR) << AT << "communication error while calling keystore"; _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); return Void(); } + + rc = KSReturn(error_code); + if (!rc.isOk()) { + LOG(ERROR) << AT << "Keystore begin returned: " << int32_t(rc); + _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); + return Void(); + } + + OperationResult result = future.get(); if (!result.resultCode.isOk()) { LOG(ERROR) << AT << "begin failed: " << int32_t(result.resultCode); _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); @@ -169,13 +213,24 @@ Return Keystore::sign(const hidl_string& keyId, const hidl_vec& d const uint8_t* in = dataToSign.data(); size_t len = dataToSign.size(); do { - binder_result = service->update(handle, KeymasterArguments(params), - std::vector(in, in + len), &result); + future = {}; + binder_result = service->update(promise, handle, KeymasterArguments(params), + std::vector(in, in + len), &error_code); if (!binder_result.isOk()) { LOG(ERROR) << AT << "communication error while calling keystore"; _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); return Void(); } + + rc = KSReturn(error_code); + if (!rc.isOk()) { + LOG(ERROR) << AT << "Keystore update returned: " << int32_t(rc); + _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); + return Void(); + } + + result = future.get(); + if (!result.resultCode.isOk()) { LOG(ERROR) << AT << "update failed: " << int32_t(result.resultCode); _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); @@ -183,7 +238,22 @@ Return Keystore::sign(const hidl_string& keyId, const hidl_vec& d } if ((size_t)result.inputConsumed > len) { LOG(ERROR) << AT << "update consumed more data than provided"; - service->abort(handle, &aidl_result); + sp 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"; + _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); + return Void(); + } + // 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); + } _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); return Void(); } @@ -191,14 +261,28 @@ Return Keystore::sign(const hidl_string& keyId, const hidl_vec& d in += result.inputConsumed; } while (len > 0); - binder_result = - service->finish(handle, KeymasterArguments(params), std::vector() /* signature */, - std::vector() /* entropy */, &result); + future = {}; + promise = new OperationResultPromise(); + future = promise->get_future(); + + binder_result = service->finish(promise, handle, KeymasterArguments(params), + std::vector() /* signature */, + std::vector() /* entropy */, &error_code); if (!binder_result.isOk()) { LOG(ERROR) << AT << "communication error while calling keystore"; _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); return Void(); } + + rc = KSReturn(error_code); + if (!rc.isOk()) { + LOG(ERROR) << AT << "Keystore finish returned: " << int32_t(rc); + _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {}); + return Void(); + } + + result = future.get(); + if (!result.resultCode.isOk()) { LOG(ERROR) << AT << "finish failed: " << int32_t(result.resultCode); _hidl_cb(KeystoreStatusCode::ERROR_UNKNOWN, {});