[DO NOT MERGE] Fix keychain key upgrade issue
am: 58291e0ff3
Change-Id: Ib1f3f0cea8c13b83b8918e38f7dbdd4d4afd9d7b
diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp
index 020bfe4..8fef42f 100644
--- a/keystore/key_store_service.cpp
+++ b/keystore/key_store_service.cpp
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#define LOG_TAG "keystore"
+
#include "key_store_service.h"
#include <fcntl.h>
@@ -111,16 +113,21 @@
return ::PERMISSION_DENIED;
}
String8 name8(name);
- String8 filename(mKeyStore->getKeyNameForUidWithDir(name8, targetUid, ::TYPE_ANY));
- int32_t result = mKeyStore->del(filename.string(), ::TYPE_ANY, get_user_id(targetUid));
- if (result != ::NO_ERROR) {
+ String8 filename = mKeyStore->getBlobFileNameIfExists(name8, targetUid, ::TYPE_ANY);
+ if (filename.isEmpty()) return ResponseCode::KEY_NOT_FOUND;
+
+ ResponseCode result = mKeyStore->del(filename.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.isEmpty()) {
+ return mKeyStore->del(filename.string(), ::TYPE_KEY_CHARACTERISTICS,
+ get_user_id(targetUid));
+ }
+ return ResponseCode::NO_ERROR;
}
int32_t KeyStoreService::exist(const String16& name, int targetUid) {
@@ -129,13 +136,8 @@
return ::PERMISSION_DENIED;
}
- String8 name8(name);
- String8 filename(mKeyStore->getKeyNameForUidWithDir(name8, targetUid, ::TYPE_ANY));
-
- if (access(filename.string(), R_OK) == -1) {
- return (errno != ENOENT) ? ::SYSTEM_ERROR : ::KEY_NOT_FOUND;
- }
- return ::NO_ERROR;
+ String8 filename = mKeyStore->getBlobFileNameIfExists(String8(name), targetUid, ::TYPE_ANY);
+ return (!filename.isEmpty()) ? ResponseCode::NO_ERROR : ResponseCode::KEY_NOT_FOUND;
}
int32_t KeyStoreService::list(const String16& prefix, int targetUid, Vector<String16>* matches) {
@@ -467,10 +469,9 @@
return -1L;
}
- String8 name8(name);
- String8 filename(mKeyStore->getKeyNameForUidWithDir(name8, targetUid, ::TYPE_ANY));
+ String8 filename = mKeyStore->getBlobFileNameIfExists(String8(name), targetUid, ::TYPE_ANY);
- if (access(filename.string(), R_OK) == -1) {
+ if (filename.isEmpty()) {
ALOGW("could not access %s for getmtime", filename.string());
return -1L;
}
@@ -1647,12 +1648,17 @@
UniquePtr<uint8_t, Malloc_Delete> upgraded_key_deleter(
const_cast<uint8_t*>(upgraded_key.key_material));
- rc = del(name, uid);
+ String8 filename = mKeyStore->getBlobFileNameIfExists(name8, uid, ::TYPE_KEYMASTER_10);
+ if (filename.isEmpty()) {
+ ALOGI("trying to upgrade a non existing blob");
+ return KEY_NOT_FOUND;
+ }
+ rc = mKeyStore->del(filename.string(), ::TYPE_ANY, get_user_id(uid));
if (rc != ::NO_ERROR) {
+ ALOGI("upgradeKeyBlob keystore->del failed %d", rc);
return rc;
}
- String8 filename(mKeyStore->getKeyNameForUidWithDir(name8, uid, ::TYPE_KEYMASTER_10));
Blob newBlob(upgraded_key.key_material, upgraded_key.key_material_size, nullptr /* info */,
0 /* infoLength */, ::TYPE_KEYMASTER_10);
newBlob.setFallback(blob->isFallback());
diff --git a/keystore/keystore.cpp b/keystore/keystore.cpp
index d0a61bf..a7a81e8 100644
--- a/keystore/keystore.cpp
+++ b/keystore/keystore.cpp
@@ -150,6 +150,41 @@
}
}
+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();
+ }
+ 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 android::String8();
+ }
+ // 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 android::String8();
+}
+
void KeyStore::resetUser(uid_t userId, bool keepUnenryptedEntries) {
android::String8 prefix("");
android::Vector<android::String16> aliases;
@@ -508,39 +543,13 @@
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 == NO_ERROR) {
- return responseCode;
- }
+ if (!filepath8.isEmpty())
+ return get(filepath8.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 == 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 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);
+ return ResponseCode::KEY_NOT_FOUND;
}
UserState* KeyStore::getUserState(uid_t userId) {
@@ -598,7 +607,7 @@
}
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;
@@ -608,7 +617,7 @@
blob->setType(type);
if (type == TYPE_KEY_PAIR) {
- importBlobAsKey(blob, filename, uid);
+ importBlobAsKey(blob, filename, userId);
}
version = 1;
updated = true;
@@ -640,7 +649,7 @@
};
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) {
@@ -668,13 +677,13 @@
return 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 != NO_ERROR) {
return rc;
}
- return get(filename, blob, TYPE_KEY_PAIR, uid);
+ return get(filename, blob, TYPE_KEY_PAIR, userId);
}
void KeyStore::readMetaData() {
diff --git a/keystore/keystore.h b/keystore/keystore.h
index 278a0a0..fee4527 100644
--- a/keystore/keystore.h
+++ b/keystore/keystore.h
@@ -58,6 +58,8 @@
const BlobType type);
android::String8 getKeyNameForUidWithDir(const android::String8& keyName, uid_t uid,
const BlobType type);
+ android::String8 getBlobFileNameIfExists(const android::String8& alias, uid_t uid,
+ const BlobType type);
/*
* Delete entries owned by userId. If keepUnencryptedEntries is true