Fixing security vuln by tightening race condition window.
A proper fix for this feature requires reworking binder permission
checking to take the selinux context and not the pid. This is feature
work that should be done for P to properly fix these race conditions
that occur elsewhere in the code.
Bug: 68217699
Test: KeyStore keygen permissions cannot be bypassed through PID cycling
Change-Id: I1ba5210010d6c413c9b1dbde3df0cc566400bfac
diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp
index c2879c8..ac10921 100644
--- a/keystore/key_store_service.cpp
+++ b/keystore/key_store_service.cpp
@@ -797,6 +797,8 @@
const ::std::vector<uint8_t>& entropy, int uid, int flags,
android::security::keymaster::KeyCharacteristics* outCharacteristics,
int32_t* aidl_return) {
+ // TODO(jbires): remove this getCallingUid call upon implementation of b/25646100
+ uid_t originalUid = IPCThreadState::self()->getCallingUid();
uid = getEffectiveUid(uid);
KeyStoreServiceReturnCode rc =
checkBinderPermissionAndKeystoreState(P_INSERT, uid, flags & KEYSTORE_FLAG_ENCRYPTED);
@@ -811,7 +813,9 @@
}
if (containsTag(params.getParameters(), Tag::INCLUDE_UNIQUE_ID)) {
- if (!checkBinderPermission(P_GEN_UNIQUE_ID)) {
+ //TODO(jbires): remove uid checking upon implementation of b/25646100
+ if (!checkBinderPermission(P_GEN_UNIQUE_ID) &&
+ originalUid != IPCThreadState::self()->getCallingUid()) {
*aidl_return = static_cast<int32_t>(ResponseCode::PERMISSION_DENIED);
return Status::ok();
}