Merge "Fix keystore wifi concurrency issue."
diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp
index a7fcd38..8efc7c7 100644
--- a/keystore/key_store_service.cpp
+++ b/keystore/key_store_service.cpp
@@ -84,6 +84,7 @@
}
#define AIDL_RETURN(rc) (*_aidl_return = KeyStoreServiceReturnCode(rc).getErrorCode(), Status::ok())
+#define KEYSTORE_SERVICE_LOCK std::lock_guard<std::mutex> keystore_lock(keystoreServiceMutex_)
std::pair<KeyStoreServiceReturnCode, bool> hadFactoryResetSinceIdRotation() {
struct stat sbuf;
@@ -147,6 +148,7 @@
} // anonymous namespace
Status KeyStoreService::getState(int32_t userId, int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
if (!checkBinderPermission(P_GET_STATE)) {
*aidl_return = static_cast<int32_t>(ResponseCode::PERMISSION_DENIED);
return Status::ok();
@@ -156,6 +158,7 @@
}
Status KeyStoreService::get(const String16& name, int32_t uid, ::std::vector<uint8_t>* item) {
+ KEYSTORE_SERVICE_LOCK;
uid_t targetUid = getEffectiveUid(uid);
if (!checkBinderPermission(P_GET, targetUid)) {
// see keystore/keystore.h
@@ -185,6 +188,7 @@
Status KeyStoreService::insert(const String16& name, const ::std::vector<uint8_t>& item,
int targetUid, int32_t flags, int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
targetUid = getEffectiveUid(targetUid);
KeyStoreServiceReturnCode result =
checkBinderPermissionAndKeystoreState(P_INSERT, targetUid, flags & KEYSTORE_FLAG_ENCRYPTED);
@@ -210,6 +214,7 @@
}
Status KeyStoreService::del(const String16& name, int targetUid, int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
targetUid = getEffectiveUid(targetUid);
if (!checkBinderPermission(P_DELETE, targetUid)) {
*aidl_return = static_cast<int32_t>(ResponseCode::PERMISSION_DENIED);
@@ -230,6 +235,7 @@
}
Status KeyStoreService::exist(const String16& name, int targetUid, int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
targetUid = getEffectiveUid(targetUid);
if (!checkBinderPermission(P_EXIST, targetUid)) {
*aidl_return = static_cast<int32_t>(ResponseCode::PERMISSION_DENIED);
@@ -245,6 +251,7 @@
Status KeyStoreService::list(const String16& prefix, int32_t targetUid,
::std::vector<::android::String16>* matches) {
+ KEYSTORE_SERVICE_LOCK;
targetUid = getEffectiveUid(targetUid);
if (!checkBinderPermission(P_LIST, targetUid)) {
return Status::fromServiceSpecificError(
@@ -283,6 +290,7 @@
*/
Status KeyStoreService::listUidsOfAuthBoundKeys(std::vector<std::string>* uidsOut,
int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
const int32_t callingUid = IPCThreadState::self()->getCallingUid();
const int32_t userId = get_user_id(callingUid);
const int32_t appId = get_app_id(callingUid);
@@ -346,6 +354,7 @@
}
Status KeyStoreService::reset(int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
if (!checkBinderPermission(P_RESET)) {
*aidl_return = static_cast<int32_t>(ResponseCode::PERMISSION_DENIED);
return Status::ok();
@@ -359,6 +368,7 @@
Status KeyStoreService::onUserPasswordChanged(int32_t userId, const String16& password,
int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
if (!checkBinderPermission(P_PASSWORD)) {
*aidl_return = static_cast<int32_t>(ResponseCode::PERMISSION_DENIED);
return Status::ok();
@@ -400,6 +410,7 @@
}
Status KeyStoreService::onUserAdded(int32_t userId, int32_t parentId, int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
if (!checkBinderPermission(P_USER_CHANGED)) {
*aidl_return = static_cast<int32_t>(ResponseCode::PERMISSION_DENIED);
return Status::ok();
@@ -425,6 +436,7 @@
}
Status KeyStoreService::onUserRemoved(int32_t userId, int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
if (!checkBinderPermission(P_USER_CHANGED)) {
*aidl_return = static_cast<int32_t>(ResponseCode::PERMISSION_DENIED);
return Status::ok();
@@ -436,6 +448,7 @@
}
Status KeyStoreService::lock(int32_t userId, int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
if (!checkBinderPermission(P_LOCK)) {
*aidl_return = static_cast<int32_t>(ResponseCode::PERMISSION_DENIED);
return Status::ok();
@@ -455,6 +468,7 @@
}
Status KeyStoreService::unlock(int32_t userId, const String16& pw, int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
if (!checkBinderPermission(P_UNLOCK)) {
*aidl_return = static_cast<int32_t>(ResponseCode::PERMISSION_DENIED);
return Status::ok();
@@ -485,6 +499,7 @@
}
Status KeyStoreService::isEmpty(int32_t userId, int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
if (!checkBinderPermission(P_IS_EMPTY)) {
*aidl_return = static_cast<int32_t>(false);
return Status::ok();
@@ -496,6 +511,7 @@
Status KeyStoreService::grant(const String16& name, int32_t granteeUid,
::android::String16* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
uid_t callingUid = IPCThreadState::self()->getCallingUid();
auto result =
checkBinderPermissionAndKeystoreState(P_GRANT, /*targetUid=*/-1, /*checkUnlocked=*/false);
@@ -516,6 +532,7 @@
}
Status KeyStoreService::ungrant(const String16& name, int32_t granteeUid, int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
uid_t callingUid = IPCThreadState::self()->getCallingUid();
KeyStoreServiceReturnCode result =
checkBinderPermissionAndKeystoreState(P_GRANT, /*targetUid=*/-1, /*checkUnlocked=*/false);
@@ -536,6 +553,7 @@
}
Status KeyStoreService::getmtime(const String16& name, int32_t uid, int64_t* time) {
+ KEYSTORE_SERVICE_LOCK;
uid_t targetUid = getEffectiveUid(uid);
if (!checkBinderPermission(P_GET, targetUid)) {
ALOGW("permission denied for %d: getmtime", targetUid);
@@ -574,11 +592,13 @@
}
Status KeyStoreService::is_hardware_backed(const String16& keyType, int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
*aidl_return = static_cast<int32_t>(mKeyStore->isHardwareBacked(keyType) ? 1 : 0);
return Status::ok();
}
Status KeyStoreService::clear_uid(int64_t targetUid64, int32_t* _aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
uid_t targetUid = getEffectiveUid(targetUid64);
if (!checkBinderPermissionSelfOrSystem(P_CLEAR_UID, targetUid)) {
return AIDL_RETURN(ResponseCode::PERMISSION_DENIED);
@@ -618,6 +638,7 @@
Status KeyStoreService::addRngEntropy(
const ::android::sp<::android::security::keystore::IKeystoreResponseCallback>& cb,
const ::std::vector<uint8_t>& entropy, int32_t flags, int32_t* _aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
auto device = mKeyStore->getDevice(flagsToSecurityLevel(flags));
if (!device) {
return AIDL_RETURN(ErrorCode::HARDWARE_TYPE_UNAVAILABLE);
@@ -634,6 +655,7 @@
const ::android::sp<::android::security::keystore::IKeystoreKeyCharacteristicsCallback>& cb,
const String16& name, const KeymasterArguments& params, const ::std::vector<uint8_t>& entropy,
int uid, int flags, int32_t* _aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
// TODO(jbires): remove this getCallingUid call upon implementation of b/25646100
uid_t originalUid = IPCThreadState::self()->getCallingUid();
uid = getEffectiveUid(uid);
@@ -695,6 +717,7 @@
const String16& name, const ::android::security::keymaster::KeymasterBlob& clientId,
const ::android::security::keymaster::KeymasterBlob& appData, int32_t uid,
int32_t* _aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
uid_t targetUid = getEffectiveUid(uid);
uid_t callingUid = IPCThreadState::self()->getCallingUid();
@@ -741,6 +764,7 @@
const ::android::sp<::android::security::keystore::IKeystoreKeyCharacteristicsCallback>& cb,
const String16& name, const KeymasterArguments& params, int32_t format,
const ::std::vector<uint8_t>& keyData, int uid, int flags, int32_t* _aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
uid = getEffectiveUid(uid);
auto logOnScopeExit = android::base::make_scope_guard([&] {
if (__android_log_security()) {
@@ -797,6 +821,7 @@
const ::android::security::keymaster::KeymasterBlob& clientId,
const ::android::security::keymaster::KeymasterBlob& appData, int32_t uid,
int32_t* _aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
uid_t targetUid = getEffectiveUid(uid);
uid_t callingUid = IPCThreadState::self()->getCallingUid();
@@ -832,6 +857,7 @@
bool pruneable, const KeymasterArguments& params,
const ::std::vector<uint8_t>& entropy, int32_t uid,
int32_t* _aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
uid_t callingUid = IPCThreadState::self()->getCallingUid();
uid_t targetUid = getEffectiveUid(uid);
if (!is_granted_to(callingUid, targetUid)) {
@@ -880,6 +906,7 @@
const ::android::sp<::android::IBinder>& token,
const ::android::security::keymaster::KeymasterArguments& params,
const ::std::vector<uint8_t>& input, int32_t* _aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
if (!checkAllowedOperationParams(params.getParameters())) {
return AIDL_RETURN(ErrorCode::INVALID_ARGUMENT);
}
@@ -904,6 +931,7 @@
const ::android::security::keymaster::KeymasterArguments& params,
const ::std::vector<uint8_t>& signature,
const ::std::vector<uint8_t>& entropy, int32_t* _aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
if (!checkAllowedOperationParams(params.getParameters())) {
return AIDL_RETURN(ErrorCode::INVALID_ARGUMENT);
}
@@ -927,6 +955,7 @@
Status KeyStoreService::abort(const ::android::sp<IKeystoreResponseCallback>& cb,
const ::android::sp<::android::IBinder>& token,
int32_t* _aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
auto dev = getOperationDevice(token);
if (!dev) {
return AIDL_RETURN(ErrorCode::INVALID_OPERATION_HANDLE);
@@ -939,6 +968,7 @@
Status KeyStoreService::addAuthToken(const ::std::vector<uint8_t>& authTokenAsVector,
int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
// TODO(swillden): When gatekeeper and fingerprint are ready, this should be updated to
// receive a HardwareAuthToken, rather than an opaque byte array.
@@ -993,6 +1023,7 @@
Status KeyStoreService::attestKey(
const ::android::sp<::android::security::keystore::IKeystoreCertificateChainCallback>& cb,
const String16& name, const KeymasterArguments& params, int32_t* _aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
// check null output if method signature is updated and return ErrorCode::OUTPUT_PARAMETER_NULL
if (!checkAllowedOperationParams(params.getParameters())) {
return AIDL_RETURN(ErrorCode::INVALID_ARGUMENT);
@@ -1054,6 +1085,7 @@
Status KeyStoreService::attestDeviceIds(
const ::android::sp<::android::security::keystore::IKeystoreCertificateChainCallback>& cb,
const KeymasterArguments& params, int32_t* _aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
// check null output if method signature is updated and return ErrorCode::OUTPUT_PARAMETER_NULL
if (!checkAllowedOperationParams(params.getParameters())) {
@@ -1144,6 +1176,7 @@
}
Status KeyStoreService::onDeviceOffBody(int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
// TODO(tuckeris): add permission check. This should be callable from ClockworkHome only.
mKeyStore->getAuthTokenTable().onDeviceOffBody();
*aidl_return = static_cast<int32_t>(ResponseCode::NO_ERROR);
@@ -1156,6 +1189,7 @@
const ::android::String16& wrappingKeyAlias, const ::std::vector<uint8_t>& maskingKey,
const KeymasterArguments& params, int64_t rootSid, int64_t fingerprintSid,
int32_t* _aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
uid_t callingUid = IPCThreadState::self()->getCallingUid();
@@ -1205,16 +1239,19 @@
const ::std::vector<uint8_t>& extraData,
const String16& locale, int32_t uiOptionsAsFlags,
int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
return mKeyStore->getConfirmationManager().presentConfirmationPrompt(
listener, promptText, extraData, locale, uiOptionsAsFlags, aidl_return);
}
Status KeyStoreService::cancelConfirmationPrompt(const sp<IBinder>& listener,
int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
return mKeyStore->getConfirmationManager().cancelConfirmationPrompt(listener, aidl_return);
}
Status KeyStoreService::isConfirmationPromptSupported(bool* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
return mKeyStore->getConfirmationManager().isConfirmationPromptSupported(aidl_return);
}
@@ -1330,6 +1367,7 @@
Status KeyStoreService::onKeyguardVisibilityChanged(bool isShowing, int32_t userId,
int32_t* aidl_return) {
+ KEYSTORE_SERVICE_LOCK;
mKeyStore->getEnforcementPolicy().set_device_locked(isShowing, userId);
*aidl_return = static_cast<int32_t>(ResponseCode::NO_ERROR);
diff --git a/keystore/key_store_service.h b/keystore/key_store_service.h
index 2171213..2fdc3dd 100644
--- a/keystore/key_store_service.h
+++ b/keystore/key_store_service.h
@@ -35,6 +35,8 @@
#include <keystore/OperationResult.h>
#include <keystore/keystore_return_types.h>
+#include <mutex>
+
namespace keystore {
// Class provides implementation for generated BnKeystoreService.h based on
@@ -230,6 +232,17 @@
sp<KeyStore> mKeyStore;
+ /**
+ * This mutex locks keystore operations from concurrent execution.
+ * The keystore service has always been conceptually single threaded. Even with the introduction
+ * of keymaster workers, it was assumed that the dispatcher thread executes exclusively on
+ * certain code paths. With the introduction of wifi Keystore service in the keystore process
+ * this assumption no longer holds as the hwbinder thread servicing this interface makes
+ * functions (rather than IPC) calls into keystore. This mutex protects the keystore logic
+ * from concurrent execution.
+ */
+ std::mutex keystoreServiceMutex_;
+
std::mutex operationDeviceMapMutex_;
std::map<sp<IBinder>, std::shared_ptr<KeymasterWorker>> operationDeviceMap_;
