Fix multiple issues with the keystore grant mechanism
1. Ungrant did not check the callers uid which allowed any caller
to remove grants to any key.
2. Grants were not removed when a key was deleted.
3. clean_uid did not clear the grant cache of the target uid.
This would leave state grants that could have been used
by a new app that happend to get the same uid as the one
that was previously uninstalled.
4. Various paths did not respect grants: del, exist, getmtime
The del path was particularly awkward because it is required
by upgradeKeyBlob. This means it must work when a key that needs
upgrading is accessed through a grant alias.
Bug: 65851049
Merged-In: I6709b7562d47ad6156bee88a9e2d961f8a4a797d
Change-Id: I6709b7562d47ad6156bee88a9e2d961f8a4a797d
diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp
index c8842c5..85de181 100644
--- a/keystore/key_store_service.cpp
+++ b/keystore/key_store_service.cpp
@@ -191,16 +191,21 @@
}
String8 name8(name);
ALOGI("del %s %d", name8.string(), targetUid);
- String8 filename(mKeyStore->getKeyNameForUidWithDir(name8, targetUid, ::TYPE_ANY));
- ResponseCode result = mKeyStore->del(filename.string(), ::TYPE_ANY, get_user_id(targetUid));
+ auto filename = mKeyStore->getBlobFileNameIfExists(name8, targetUid, ::TYPE_ANY);
+ if (!filename.isOk()) return ResponseCode::KEY_NOT_FOUND;
+
+ ResponseCode result = mKeyStore->del(filename.value().string(), ::TYPE_ANY,
+ get_user_id(targetUid));
if (result != ResponseCode::NO_ERROR) {
return result;
}
- // Also delete any characteristics files
- String8 chrFilename(
- mKeyStore->getKeyNameForUidWithDir(name8, targetUid, ::TYPE_KEY_CHARACTERISTICS));
- return mKeyStore->del(chrFilename.string(), ::TYPE_KEY_CHARACTERISTICS, get_user_id(targetUid));
+ filename = mKeyStore->getBlobFileNameIfExists(name8, targetUid, ::TYPE_KEY_CHARACTERISTICS);
+ if (filename.isOk()) {
+ return mKeyStore->del(filename.value().string(), ::TYPE_KEY_CHARACTERISTICS,
+ get_user_id(targetUid));
+ }
+ return ResponseCode::NO_ERROR;
}
KeyStoreServiceReturnCode KeyStoreService::exist(const String16& name, int targetUid) {
@@ -209,13 +214,8 @@
return ResponseCode::PERMISSION_DENIED;
}
- String8 name8(name);
- String8 filename(mKeyStore->getKeyNameForUidWithDir(name8, targetUid, ::TYPE_ANY));
-
- if (access(filename.string(), R_OK) == -1) {
- return (errno != ENOENT) ? ResponseCode::SYSTEM_ERROR : ResponseCode::KEY_NOT_FOUND;
- }
- return ResponseCode::NO_ERROR;
+ auto filename = mKeyStore->getBlobFileNameIfExists(String8(name), targetUid, ::TYPE_ANY);
+ return filename.isOk() ? ResponseCode::NO_ERROR : ResponseCode::KEY_NOT_FOUND;
}
KeyStoreServiceReturnCode KeyStoreService::list(const String16& prefix, int targetUid,
@@ -543,7 +543,7 @@
return (errno != ENOENT) ? ResponseCode::SYSTEM_ERROR : ResponseCode::KEY_NOT_FOUND;
}
- return mKeyStore->removeGrant(name8, granteeUid) ? ResponseCode::NO_ERROR
+ return mKeyStore->removeGrant(name8, callingUid, granteeUid) ? ResponseCode::NO_ERROR
: ResponseCode::KEY_NOT_FOUND;
}
@@ -554,17 +554,16 @@
return -1L;
}
- String8 name8(name);
- String8 filename(mKeyStore->getKeyNameForUidWithDir(name8, targetUid, ::TYPE_ANY));
+ auto filename = mKeyStore->getBlobFileNameIfExists(String8(name), targetUid, ::TYPE_ANY);
- if (access(filename.string(), R_OK) == -1) {
- ALOGW("could not access %s for getmtime", filename.string());
+ if (!filename.isOk()) {
+ ALOGW("could not access %s for getmtime", filename.value().string());
return -1L;
}
- int fd = TEMP_FAILURE_RETRY(open(filename.string(), O_NOFOLLOW, O_RDONLY));
+ int fd = TEMP_FAILURE_RETRY(open(filename.value().string(), O_NOFOLLOW, O_RDONLY));
if (fd < 0) {
- ALOGW("could not open %s for getmtime", filename.string());
+ ALOGW("could not open %s for getmtime", filename.value().string());
return -1L;
}
@@ -572,7 +571,7 @@
int ret = fstat(fd, &s);
close(fd);
if (ret == -1) {
- ALOGW("could not stat %s for getmtime", filename.string());
+ ALOGW("could not stat %s for getmtime", filename.value().string());
return -1L;
}
@@ -652,6 +651,8 @@
}
ALOGI("clear_uid %" PRId64, targetUid64);
+ mKeyStore->removeAllGrantsToUid(targetUid);
+
String8 prefix = String8::format("%u_", targetUid);
Vector<String16> aliases;
if (mKeyStore->list(prefix, &aliases, get_user_id(targetUid)) != ResponseCode::NO_ERROR) {
@@ -1893,8 +1894,12 @@
return;
}
- String8 filename(mKeyStore->getKeyNameForUidWithDir(name8, uid, ::TYPE_KEYMASTER_10));
- error = mKeyStore->del(filename.string(), ::TYPE_ANY, get_user_id(uid));
+ auto filename = mKeyStore->getBlobFileNameIfExists(name8, uid, ::TYPE_KEYMASTER_10);
+ if (!filename.isOk()) {
+ ALOGI("trying to upgrade a non existing blob");
+ return;
+ }
+ error = mKeyStore->del(filename.value().string(), ::TYPE_ANY, get_user_id(uid));
if (!error.isOk()) {
ALOGI("upgradeKeyBlob keystore->del failed %d", (int)error);
return;
@@ -1907,7 +1912,7 @@
newBlob.setSuperEncrypted(blob->isSuperEncrypted());
newBlob.setCriticalToDeviceEncryption(blob->isCriticalToDeviceEncryption());
- error = mKeyStore->put(filename.string(), &newBlob, get_user_id(uid));
+ error = mKeyStore->put(filename.value().string(), &newBlob, get_user_id(uid));
if (!error.isOk()) {
ALOGI("upgradeKeyBlob keystore->put failed %d", (int)error);
return;