[automerger skipped] Fix keystore wifi concurrency issue.
am: 1849841b48 -s ours
am skip reason: change_id I8c5602d2c2cb1dd9423df713037e99b247cee71f with SHA1 54fcc9954b is in history
Change-Id: If416ad7de274420185e1cdc497399560f99bcbad
diff --git a/keystore-engine/android_engine.cpp b/keystore-engine/android_engine.cpp
index b7b8199..368590c 100644
--- a/keystore-engine/android_engine.cpp
+++ b/keystore-engine/android_engine.cpp
@@ -207,23 +207,14 @@
NULL /* size */,
NULL /* sign */,
- NULL /* verify */,
NULL /* encrypt */,
NULL /* sign_raw */,
NULL /* decrypt */,
- NULL /* verify_raw */,
rsa_private_transform,
- NULL /* mod_exp */,
- NULL /* bn_mod_exp */,
-
RSA_FLAG_CACHE_PUBLIC | RSA_FLAG_OPAQUE,
-
- NULL /* keygen */,
- NULL /* multi_prime_keygen */,
- NULL /* supports_digest */,
};
const char* ecdsa_get_key_id(const EC_KEY* ec_key) {
@@ -283,7 +274,6 @@
NULL /* finish */,
NULL /* group_order_size */,
ecdsa_sign,
- NULL /* verify */,
ECDSA_FLAG_OPAQUE,
};
diff --git a/keystore/auth_token_table.cpp b/keystore/auth_token_table.cpp
index 3f476cd..46b644d 100644
--- a/keystore/auth_token_table.cpp
+++ b/keystore/auth_token_table.cpp
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#define LOG_TAG "keystore"
+
#include "auth_token_table.h"
#include <assert.h>
@@ -77,6 +79,12 @@
void AuthTokenTable::AddAuthenticationToken(const HardwareAuthToken* auth_token) {
Entry new_entry(auth_token, clock_function_());
+ //STOPSHIP: debug only, to be removed
+ ALOGD("AddAuthenticationToken: timestamp = %llu (%llu), time_received = %lld",
+ static_cast<unsigned long long>(new_entry.timestamp_host_order()),
+ static_cast<unsigned long long>(auth_token->timestamp),
+ static_cast<long long>(new_entry.time_received()));
+
RemoveEntriesSupersededBy(new_entry);
if (entries_.size() >= max_entries_) {
ALOGW("Auth token table filled up; replacing oldest entry");
@@ -207,7 +215,7 @@
: token_(token), time_received_(current_time), last_use_(current_time),
operation_completed_(token_->challenge == 0) {}
-uint32_t AuthTokenTable::Entry::timestamp_host_order() const {
+uint64_t AuthTokenTable::Entry::timestamp_host_order() const {
return ntoh(token_->timestamp);
}
@@ -235,8 +243,8 @@
return (token_->userId == entry.token_->userId &&
token_->authenticatorType == entry.token_->authenticatorType &&
- token_->authenticatorType == entry.token_->authenticatorType &&
- timestamp_host_order() > entry.timestamp_host_order());
+ token_->authenticatorId == entry.token_->authenticatorId &&
+ is_newer_than(&entry));
}
} // namespace keymaster
diff --git a/keystore/auth_token_table.h b/keystore/auth_token_table.h
index 6f7aab1..0056b26 100644
--- a/keystore/auth_token_table.h
+++ b/keystore/auth_token_table.h
@@ -114,9 +114,15 @@
bool Supersedes(const Entry& entry) const;
bool SatisfiesAuth(const std::vector<uint64_t>& sids, HardwareAuthenticatorType auth_type);
- bool is_newer_than(const Entry* entry) {
+ bool is_newer_than(const Entry* entry) const {
if (!entry) return true;
- return timestamp_host_order() > entry->timestamp_host_order();
+ uint64_t ts = timestamp_host_order();
+ uint64_t other_ts = entry->timestamp_host_order();
+ // Normally comparing timestamp_host_order alone is sufficient, but here is an
+ // additional hack to compare time_received value for some devices where their auth
+ // tokens contain fixed timestamp (due to the a stuck secure RTC on them)
+ return (ts > other_ts) ||
+ ((ts == other_ts) && (time_received_ > entry->time_received_));
}
void mark_completed() { operation_completed_ = true; }
@@ -124,7 +130,7 @@
const HardwareAuthToken* token() { return token_.get(); }
time_t time_received() const { return time_received_; }
bool completed() const { return operation_completed_; }
- uint32_t timestamp_host_order() const;
+ uint64_t timestamp_host_order() const;
HardwareAuthenticatorType authenticator_type() const;
private:
diff --git a/keystore/blob.cpp b/keystore/blob.cpp
index a33334e..625d057 100644
--- a/keystore/blob.cpp
+++ b/keystore/blob.cpp
@@ -272,8 +272,9 @@
return ResponseCode::VALUE_CORRUPTED;
}
- if ((isEncrypted() || isSuperEncrypted()) && (state != STATE_NO_ERROR)) {
- return ResponseCode::LOCKED;
+ if ((isEncrypted() || isSuperEncrypted())) {
+ if (state == STATE_LOCKED) return ResponseCode::LOCKED;
+ if (state == STATE_UNINITIALIZED) return ResponseCode::UNINITIALIZED;
}
if (fileLength < offsetof(blobv3, value)) return ResponseCode::VALUE_CORRUPTED;
diff --git a/keystore/grant_store.cpp b/keystore/grant_store.cpp
index 9c2e591..9244b7c 100644
--- a/keystore/grant_store.cpp
+++ b/keystore/grant_store.cpp
@@ -25,8 +25,10 @@
static const char* kKeystoreGrantInfix = "_KEYSTOREGRANT_";
static constexpr size_t kKeystoreGrantInfixLength = 15;
-Grant::Grant(const std::string& alias, const std::string& key_file, const uint64_t grant_no)
- : alias_(alias), key_file_(key_file), grant_no_(grant_no) {}
+Grant::Grant(const std::string& alias, const std::string& owner_dir_name, const uid_t owner_uid,
+ const uint64_t grant_no)
+ : alias_(alias), owner_dir_name_(owner_dir_name), owner_uid_(owner_uid),
+ grant_no_(grant_no) {}
static std::pair<uint64_t, std::string> parseGrantAlias(const std::string& grantAlias) {
auto pos = grantAlias.rfind(kKeystoreGrantInfix);
@@ -39,7 +41,8 @@
return {grant_no, wrapped_alias};
}
-std::string GrantStore::put(const uid_t uid, const std::string& alias, const std::string& key_file) {
+std::string GrantStore::put(const uid_t uid, const std::string& alias,
+ const std::string& owner_dir_name, const uid_t owner_uid) {
std::stringstream s;
s << alias << kKeystoreGrantInfix;
auto& uid_grant_list = grants_[uid];
@@ -47,10 +50,12 @@
bool success = false;
auto iterator = std::find_if(uid_grant_list.begin(), uid_grant_list.end(),
[&](auto& entry) {
- return success = entry.alias_ == alias && entry.key_file_ == key_file;
+ return success = entry.alias_ == alias && entry.owner_dir_name_ == owner_dir_name
+ && entry.owner_uid_ == owner_uid;
});
while (!success) {
- std::tie(iterator, success) = uid_grant_list.emplace(alias, key_file, std::rand());
+ std::tie(iterator, success) = uid_grant_list.emplace(alias, owner_dir_name, owner_uid,
+ std::rand());
}
s << iterator->grant_no_;
return s.str();
@@ -70,10 +75,11 @@
return &(*grant);
}
-bool GrantStore::removeByFileName(const uid_t uid, const std::string& fileName) {
- auto& uid_grant_list = grants_.operator[](uid);
+bool GrantStore::removeByFileAlias(const uid_t granteeUid, const uid_t granterUid,
+ const std::string& alias) {
+ auto& uid_grant_list = grants_[granteeUid];
for (auto i = uid_grant_list.begin(); i != uid_grant_list.end(); ++i) {
- if (i->key_file_ == fileName) {
+ if (i->alias_ == alias && i->owner_uid_ == granterUid) {
uid_grant_list.erase(i);
return true;
}
@@ -81,4 +87,19 @@
return false;
}
+void GrantStore::removeAllGrantsToKey(const uid_t granterUid, const std::string& alias) {
+ for (auto& uid_grant_list : grants_) {
+ for (auto i = uid_grant_list.second.begin(); i != uid_grant_list.second.end(); ++i) {
+ if (i->alias_ == alias && i->owner_uid_ == granterUid) {
+ uid_grant_list.second.erase(i);
+ break;
+ }
+ }
+ }
+}
+
+void GrantStore::removeAllGrantsToUid(const uid_t granteeUid) {
+ grants_.erase(granteeUid);
+}
+
} // namespace keystore
diff --git a/keystore/grant_store.h b/keystore/grant_store.h
index 43e814e..6341c76 100644
--- a/keystore/grant_store.h
+++ b/keystore/grant_store.h
@@ -32,10 +32,14 @@
*/
class Grant {
public:
- Grant(const std::string& alias, const std::string& key_file, const uint64_t grant_no);
- std::string alias_;
- std::string key_file_;
- uint64_t grant_no_;
+ Grant(const std::string& alias, const std::string& owner_dir_name, const uid_t owner_uid,
+ const uint64_t grant_no);
+ // the following three field are used to recover the key filename that the grant refers to
+ std::string alias_; ///< original/wrapped key alias
+ std::string owner_dir_name_; ///< key owner key directory
+ uid_t owner_uid_; ///< key owner uid
+
+ uint64_t grant_no_; ///< numeric grant identifier - randomly assigned
operator const uint64_t&() const { return grant_no_; }
};
@@ -52,9 +56,12 @@
class GrantStore {
public:
GrantStore() : grants_() {}
- std::string put(const uid_t uid, const std::string& alias, const std::string& key_file);
+ std::string put(const uid_t uid, const std::string& alias, const std::string& owner_dir_name,
+ const uid_t owner_uid);
const Grant* get(const uid_t uid, const std::string& alias) const;
- bool removeByFileName(const uid_t uid, const std::string& filename);
+ bool removeByFileAlias(const uid_t granteeUid, const uid_t granterUid, const std::string& alias);
+ void removeAllGrantsToKey(const uid_t granterUid, const std::string& alias);
+ void removeAllGrantsToUid(const uid_t granteeUid);
// GrantStore is neither copyable nor movable.
GrantStore(const GrantStore&) = delete;
diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp
index e038056..39341ef 100644
--- a/keystore/key_store_service.cpp
+++ b/keystore/key_store_service.cpp
@@ -547,7 +547,7 @@
return String16();
}
- return String16(mKeyStore->addGrant(filename.string(), String8(name).string(), granteeUid).c_str());
+ return String16(mKeyStore->addGrant(String8(name).string(), callingUid, granteeUid).c_str());
}
KeyStoreServiceReturnCode KeyStoreService::ungrant(const String16& name, int32_t granteeUid) {
@@ -565,8 +565,8 @@
return (errno != ENOENT) ? ResponseCode::SYSTEM_ERROR : ResponseCode::KEY_NOT_FOUND;
}
- return mKeyStore->removeGrant(filename.string(), granteeUid) ? ResponseCode::NO_ERROR
- : ResponseCode::KEY_NOT_FOUND;
+ return mKeyStore->removeGrant(name8, callingUid, granteeUid) ? ResponseCode::NO_ERROR
+ : ResponseCode::KEY_NOT_FOUND;
}
int64_t KeyStoreService::getmtime(const String16& name, int32_t uid) {
@@ -677,6 +677,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) {
@@ -832,7 +834,26 @@
KeyStoreServiceReturnCode rc =
mKeyStore->getKeyForName(&keyBlob, name8, targetUid, TYPE_KEYMASTER_10);
- if (!rc.isOk()) {
+ if (rc == ResponseCode::UNINITIALIZED) {
+ /*
+ * If we fail reading the blob because the master key is missing we try to retrieve the
+ * key characteristics from the characteristics file. This happens when auth-bound
+ * keys are used after a screen lock has been removed by the user.
+ */
+ rc = mKeyStore->getKeyForName(&keyBlob, name8, targetUid, TYPE_KEY_CHARACTERISTICS);
+ if (!rc.isOk()) {
+ return rc;
+ }
+ AuthorizationSet keyCharacteristics;
+ // TODO write one shot stream buffer to avoid copying (twice here)
+ std::string charBuffer(reinterpret_cast<const char*>(keyBlob.getValue()),
+ keyBlob.getLength());
+ std::stringstream charStream(charBuffer);
+ keyCharacteristics.Deserialize(&charStream);
+
+ outCharacteristics->softwareEnforced = keyCharacteristics.hidl_data();
+ return rc;
+ } else if (!rc.isOk()) {
return rc;
}
@@ -1108,12 +1129,15 @@
persistedCharacteristics.Subtract(teeEnforced);
characteristics.softwareEnforced = persistedCharacteristics.hidl_data();
- result->resultCode = getAuthToken(characteristics, 0, purpose, &authToken,
- /*failOnTokenMissing*/ false);
+ auto authResult = getAuthToken(characteristics, 0, purpose, &authToken,
+ /*failOnTokenMissing*/ false);
// If per-operation auth is needed we need to begin the operation and
// the client will need to authorize that operation before calling
// update. Any other auth issues stop here.
- if (!result->resultCode.isOk() && result->resultCode != ResponseCode::OP_AUTH_NEEDED) return;
+ if (!authResult.isOk() && authResult != ResponseCode::OP_AUTH_NEEDED) {
+ result->resultCode = authResult;
+ return;
+ }
addAuthTokenToParams(&opParams, authToken);
@@ -1188,6 +1212,7 @@
result->handle, keyid, purpose, dev, appToken, std::move(characteristics), pruneable);
assert(characteristics.teeEnforced.size() == 0);
assert(characteristics.softwareEnforced.size() == 0);
+ result->token = operationToken;
if (authToken) {
mOperationMap.setOperationAuthToken(operationToken, authToken);
@@ -1197,8 +1222,11 @@
// application should get an auth token using the handle before the
// first call to update, which will fail if keystore hasn't received the
// auth token.
- // All fields but "token" were set in the begin operation's callback.
- result->token = operationToken;
+ if (result->resultCode == ErrorCode::OK) {
+ result->resultCode = authResult;
+ }
+
+ // Other result fields were set in the begin operation's callback.
}
void KeyStoreService::update(const sp<IBinder>& token, const hidl_vec<KeyParameter>& params,
@@ -1733,6 +1761,7 @@
case AuthTokenTable::AUTH_TOKEN_NOT_FOUND:
case AuthTokenTable::AUTH_TOKEN_EXPIRED:
case AuthTokenTable::AUTH_TOKEN_WRONG_SID:
+ ALOGE("getAuthToken failed: %d", err); //STOPSHIP: debug only, to be removed
return ErrorCode::KEY_USER_NOT_AUTHENTICATED;
case AuthTokenTable::OP_HANDLE_REQUIRED:
return failOnTokenMissing ? KeyStoreServiceReturnCode(ErrorCode::KEY_USER_NOT_AUTHENTICATED)
diff --git a/keystore/keystore.cpp b/keystore/keystore.cpp
index 6d3bac5..a61ef73 100644
--- a/keystore/keystore.cpp
+++ b/keystore/keystore.cpp
@@ -24,6 +24,7 @@
#include <openssl/bio.h>
#include <utils/String16.h>
+#include <utils/String8.h>
#include <keystore/IKeystoreService.h>
@@ -39,6 +40,7 @@
const android::String16 KeyStore::sRSAKeyType("RSA");
using namespace keystore;
+using android::String8;
KeyStore::KeyStore(Entropy* entropy, const km_device_t& device, const km_device_t& fallback,
bool allowNewFallback)
@@ -170,7 +172,8 @@
// They might be using a granted key.
auto grant = mGrants.get(uid, alias.string());
if (grant) {
- filepath8 = grant->key_file_.c_str();
+ filepath8 = String8::format("%s/%s", grant->owner_dir_name_.c_str(),
+ getKeyNameForUid(String8(grant->alias_.c_str()), grant->owner_uid_, type).c_str());
if (!access(filepath8.string(), R_OK | W_OK)) return filepath8;
}
return {};
@@ -331,11 +334,23 @@
mEntropy);
}
+static NullOr<std::tuple<uid_t, std::string>> filename2UidAlias(const std::string& filename);
+
ResponseCode KeyStore::del(const char* filename, const BlobType type, uid_t userId) {
Blob keyBlob;
+ auto uidAlias = filename2UidAlias(filename);
+ uid_t uid;
+ std::string alias;
+ if (uidAlias.isOk()) {
+ std::tie(uid, alias) = std::move(uidAlias).value();
+ }
ResponseCode rc = get(filename, &keyBlob, type, userId);
if (rc == ResponseCode::VALUE_CORRUPTED) {
// The file is corrupt, the best we can do is rm it.
+ if (uidAlias.isOk()) {
+ // remove possible grants
+ mGrants.removeAllGrantsToKey(uid, alias);
+ }
return (unlink(filename) && errno != ENOENT) ?
ResponseCode::SYSTEM_ERROR : ResponseCode::NO_ERROR;
}
@@ -353,8 +368,16 @@
return ResponseCode::SYSTEM_ERROR;
}
- return (unlink(filename) && errno != ENOENT) ?
+ rc = (unlink(filename) && errno != ENOENT) ?
ResponseCode::SYSTEM_ERROR : ResponseCode::NO_ERROR;
+
+ if (rc == ResponseCode::NO_ERROR && keyBlob.getType() != ::TYPE_KEY_CHARACTERISTICS) {
+ // now that we have successfully deleted a key, let's make sure there are no stale grants
+ if (uidAlias.isOk()) {
+ mGrants.removeAllGrantsToKey(uid, alias);
+ }
+ }
+ return rc;
}
/*
@@ -394,6 +417,29 @@
*out = '\0';
}
+static NullOr<std::tuple<uid_t, std::string>> filename2UidAlias(const std::string& filepath) {
+ auto filenamebase = filepath.find_last_of('/');
+ std::string filename = filenamebase == std::string::npos ? filepath :
+ filepath.substr(filenamebase + 1);
+
+ if (filename[0] == '.') return {};
+
+ auto sep = filename.find('_');
+ if (sep == std::string::npos) return {};
+
+ std::stringstream s(filename.substr(0, sep));
+ uid_t uid;
+ s >> uid;
+ if (!s) return {};
+
+ auto alias = filename.substr(sep + 1);
+
+ std::vector<char> alias_buffer(decode_key_length(alias.c_str(), alias.size()) + 1);
+
+ decode_key(alias_buffer.data(), alias.c_str(), alias.size());
+ return std::tuple<uid_t, std::string>(uid, alias_buffer.data());
+}
+
ResponseCode KeyStore::list(const android::String8& prefix,
android::Vector<android::String16>* matches, uid_t userId) {
@@ -437,12 +483,16 @@
return ResponseCode::NO_ERROR;
}
-std::string KeyStore::addGrant(const char* filename, const char* alias, uid_t granteeUid) {
- return mGrants.put(granteeUid, alias, filename);
+std::string KeyStore::addGrant(const char* alias, uid_t granterUid, uid_t granteeUid) {
+ return mGrants.put(granteeUid, alias, getUserStateByUid(granterUid)->getUserDirName(),
+ granterUid);
}
-bool KeyStore::removeGrant(const char* filename, uid_t granteeUid) {
- return mGrants.removeByFileName(granteeUid, filename);
+bool KeyStore::removeGrant(const char* alias, const uid_t granterUid, const uid_t granteeUid) {
+ return mGrants.removeByFileAlias(granteeUid, granterUid, alias);
+}
+void KeyStore::removeAllGrantsToUid(const uid_t granteeUid) {
+ mGrants.removeAllGrantsToUid(granteeUid);
}
ResponseCode KeyStore::importKey(const uint8_t* key, size_t keyLen, const char* filename,
diff --git a/keystore/keystore.h b/keystore/keystore.h
index f208320..a0b747f 100644
--- a/keystore/keystore.h
+++ b/keystore/keystore.h
@@ -89,8 +89,9 @@
ResponseCode list(const android::String8& prefix, android::Vector<android::String16>* matches,
uid_t userId);
- std::string addGrant(const char* filename, const char* alias, uid_t granteeUid);
- bool removeGrant(const char* filename, uid_t granteeUid);
+ std::string addGrant(const char* alias, uid_t granterUid, uid_t granteeUid);
+ bool removeGrant(const char* alias, const uid_t granterUid, const uid_t granteeUid);
+ void removeAllGrantsToUid(const uid_t granteeUid);
ResponseCode importKey(const uint8_t* key, size_t keyLen, const char* filename, uid_t userId,
int32_t flags);
diff --git a/keystore/permissions.h b/keystore/permissions.h
index 80d0e04..1f7b7a6 100644
--- a/keystore/permissions.h
+++ b/keystore/permissions.h
@@ -61,4 +61,52 @@
int configure_selinux();
+/*
+ * Keystore grants.
+ *
+ * What are keystore grants?
+ *
+ * Keystore grants are a mechanism that allows an app to grant the permission to use one of its
+ * keys to an other app.
+ *
+ * Liftime of a grant:
+ *
+ * A keystore grant is ephemeral in that is never persistently stored. When the keystore process
+ * exits, all grants are lost. Also, grants can be explicitly revoked by the granter by invoking
+ * the ungrant operation.
+ *
+ * What happens when a grant is created?
+ *
+ * The grant operation expects a valid key alias and the uid of the grantee, i.e., the app that
+ * shall be allowed to use the key denoted by the alias. It then makes an entry in the grant store
+ * which generates a new alias of the form <alias>_KEYSTOREGRANT_<random_grant_no_>. This grant
+ * alias is returned to the caller which can pass the new alias to the grantee. For every grantee,
+ * the grant store keeps a set of grants, an entry of which holds the following information:
+ * - the owner of the key by uid, aka granter uid,
+ * - the original alias of the granted key, and
+ * - the random grant number.
+ * (See "grant_store.h:class Grant")
+ *
+ * What happens when a grant is used?
+ *
+ * Upon any keystore operation that expects an alias, the alias and the caller's uid are used
+ * to retrieve a key file. If that fails some operations try to retrieve a key file indirectly
+ * through a grant. These operations include:
+ * - attestKey
+ * - begin
+ * - exportKey
+ * - get
+ * - getKeyCharacteristics
+ * - del
+ * - exist
+ * - getmtime
+ * Operations that DO NOT follow the grant indirection are:
+ * - import
+ * - generate
+ * - grant
+ * - ungrant
+ * Especially, the latter two mean that neither can a grantee transitively grant a granted key
+ * to a third, nor can they relinquish access to the key or revoke access to the key by a third.
+ */
+
#endif // KEYSTORE_PERMISSIONS_H_