tequilaOS/platform_system_security
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

[DO NOT MERGE] Fix keychain key upgrade issue

Browse source 
Fix issues in keystore where it didn't handle key upgrade for granted keys
or keys with effective uid (keys under WiFi uid)  correctly.

Test: manual
Bug: 66094261
Change-Id: I6709b7562d47ad6156bee88a9e2d961f8a4a797d
Merged-In: I6709b7562d47ad6156bee88a9e2d961f8a4a797d
This commit is contained in:
Rubin Xu 2017-10-19 16:22:17 +01:00
parent 48dc4ad3b9
commit 2b93ec41b1
3 changed files with 73 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

49
keystore/key_store_service.cpp
View file

@ -191,16 +191,21 @@ KeyStoreServiceReturnCode KeyStoreService::del(const String16& name, int targetU
}
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 @@ KeyStoreServiceReturnCode KeyStoreService::exist(const String16& name, int targe
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,
@ -555,17 +555,16 @@ int64_t KeyStoreService::getmtime(const String16& name, int32_t uid) {
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;
}
@ -573,7 +572,7 @@ int64_t KeyStoreService::getmtime(const String16& name, int32_t uid) {
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;
}
@ -1868,8 +1867,12 @@ KeyStoreServiceReturnCode KeyStoreService::upgradeKeyBlob(const String16& name,
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;
@ -1882,7 +1885,7 @@ KeyStoreServiceReturnCode KeyStoreService::upgradeKeyBlob(const String16& name,
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;

78
keystore/keystore.cpp
View file

@ -159,6 +159,42 @@ android::String8 KeyStore::getKeyNameForUidWithDir(
}
}
NullOr<android::String8> KeyStore::getBlobFileNameIfExists(const android::String8& alias, uid_t uid,
const BlobType type) {
android::String8 filepath8(getKeyNameForUidWithDir(alias, uid, type));
if (!access(filepath8.string(), R_OK | W_OK)) return filepath8;
// If this is one of the legacy UID->UID mappings, use it.
uid_t euid = get_keystore_euid(uid);
if (euid != uid) {
filepath8 = getKeyNameForUidWithDir(alias, euid, type);
if (!access(filepath8.string(), R_OK | W_OK)) return filepath8;
}
// They might be using a granted key (<uid>_<granted_alias>), try parsing the alias.
char* end;
uid_t granter_uid = strtoul(alias, &end, 10);
// Does the alias look like a granted key?
if (end[0] != '_' || end[1] == 0 || (!granter_uid)) {
return {};
}
android::String8 granted_alias(end + 1);
// We might be asked to get a characteristics file, but still need to check grant
// on the key file itself.
android::String8 keyfilepath8 = getKeyNameForUidWithDir(granted_alias, granter_uid, ::TYPE_ANY);
if (!hasGrant(keyfilepath8.string(), uid)) {
return {};
}
// Get the granted blob file path of the desired type
filepath8 = getKeyNameForUidWithDir(granted_alias, granter_uid, type);
if (!access(filepath8.string(), R_OK | W_OK)) return filepath8;
return {};
}
void KeyStore::resetUser(uid_t userId, bool keepUnenryptedEntries) {
android::String8 prefix("");
android::Vector<android::String16> aliases;
@ -517,39 +553,13 @@ bool KeyStore::isHardwareBacked(const android::String16& /*keyType*/) const {
ResponseCode KeyStore::getKeyForName(Blob* keyBlob, const android::String8& keyName,
const uid_t uid, const BlobType type) {
android::String8 filepath8(getKeyNameForUidWithDir(keyName, uid, type));
auto filepath8 = getBlobFileNameIfExists(keyName, uid, type);
uid_t userId = get_user_id(uid);
ResponseCode responseCode = get(filepath8.string(), keyBlob, type, userId);
if (responseCode == ResponseCode::NO_ERROR) {
return responseCode;
}
if (filepath8.isOk())
return get(filepath8.value().string(), keyBlob, type, userId);
// If this is one of the legacy UID->UID mappings, use it.
uid_t euid = get_keystore_euid(uid);
if (euid != uid) {
filepath8 = getKeyNameForUidWithDir(keyName, euid, type);
responseCode = get(filepath8.string(), keyBlob, type, userId);
if (responseCode == ResponseCode::NO_ERROR) {
return responseCode;
}
}
// They might be using a granted key.
android::String8 filename8 = getKeyName(keyName, type);
char* end;
strtoul(filename8.string(), &end, 10);
if (end[0] != '_' || end[1] == 0) {
return ResponseCode::KEY_NOT_FOUND;
}
filepath8 = android::String8::format("%s/%s", getUserState(userId)->getUserDirName(),
filename8.string());
if (!hasGrant(filepath8.string(), uid)) {
return responseCode;
}
// It is a granted key. Try to load it.
return get(filepath8.string(), keyBlob, type, userId);
}
UserState* KeyStore::getUserState(uid_t userId) {
@ -607,7 +617,7 @@ const grant_t* KeyStore::getGrant(const char* filename, uid_t uid) const {
}
bool KeyStore::upgradeBlob(const char* filename, Blob* blob, const uint8_t oldVersion,
const BlobType type, uid_t uid) {
const BlobType type, uid_t userId) {
bool updated = false;
uint8_t version = oldVersion;
@ -617,7 +627,7 @@ bool KeyStore::upgradeBlob(const char* filename, Blob* blob, const uint8_t oldVe
blob->setType(type);
if (type == TYPE_KEY_PAIR) {
importBlobAsKey(blob, filename, uid);
importBlobAsKey(blob, filename, userId);
}
version = 1;
updated = true;
@ -649,7 +659,7 @@ struct BIO_Delete {
};
typedef UniquePtr<BIO, BIO_Delete> Unique_BIO;
ResponseCode KeyStore::importBlobAsKey(Blob* blob, const char* filename, uid_t uid) {
ResponseCode KeyStore::importBlobAsKey(Blob* blob, const char* filename, uid_t userId) {
// We won't even write to the blob directly with this BIO, so const_cast is okay.
Unique_BIO b(BIO_new_mem_buf(const_cast<uint8_t*>(blob->getValue()), blob->getLength()));
if (b.get() == NULL) {
@ -677,13 +687,13 @@ ResponseCode KeyStore::importBlobAsKey(Blob* blob, const char* filename, uid_t u
return ResponseCode::SYSTEM_ERROR;
}
ResponseCode rc = importKey(pkcs8key.get(), len, filename, get_user_id(uid),
ResponseCode rc = importKey(pkcs8key.get(), len, filename, userId,
blob->isEncrypted() ? KEYSTORE_FLAG_ENCRYPTED : KEYSTORE_FLAG_NONE);
if (rc != ResponseCode::NO_ERROR) {
return rc;
}
return get(filename, blob, TYPE_KEY_PAIR, uid);
return get(filename, blob, TYPE_KEY_PAIR, userId);
}
void KeyStore::readMetaData() {

2
keystore/keystore.h
View file

@ -74,6 +74,8 @@ class KeyStore {
const BlobType type);
android::String8 getKeyNameForUidWithDir(const android::String8& keyName, uid_t uid,
const BlobType type);
NullOr<android::String8> getBlobFileNameIfExists(const android::String8& alias, uid_t uid,
const BlobType type);
/*
* Delete entries owned by userId. If keepUnencryptedEntries is true