From bbbfe171f12824fe77086459af65ff46a7a82c4c Mon Sep 17 00:00:00 2001 From: bigbiff Date: Sun, 30 May 2021 15:52:41 -0400 Subject: [PATCH] fscrypt: updates for wrapped key - During OTA upgrades if security state or ROT changes then Keymaster keys requires upgrade. So for such usescases, if the FBE ephemeral key export fails, check whether KM key requires upgrade and try for exporting ephemeral key again. CRs-Fixed: 2632902 Change-Id: I3ee2fcd97a56b628dc4304867c8f2b8da875f883 Signed-off-by: Neeraj Soni - Commit 77df7f2 / http://aosp/1217657 ("Refactor to use EncryptionPolicy everywhere we used to use raw_ref") unintentionally made fscrypt_initialize_systemwide_keys() start specifying keepOld=true (via default parameter value) when retrieving the system DE key, and likewise for read_or_create_volkey() and volume keys. As a result, if the associated Keymaster key needs to be upgraded, the upgraded key blob gets written to "keymaster_key_blob_upgraded", but it doesn't replace the original "keymaster_key_blob", nor is the original key deleted from Keymaster. This happens at every boot, eventually resulting in the RPMB partition in Keymaster becoming full. Only the metadata encryption key ever needs keepOld=true, since it's the only key that isn't stored in /data, and the purpose of keepOld=true is to allow a key that isn't stored in /data to be committed or rolled back when a userdata checkpoint is committed or rolled back. So, fix this bug by removing the default value of keepOld, and specifying false everywhere except the metadata encryption key. Note that when an affected device gets this fix, it will finally upgrade its system DE key correctly. However, this fix doesn't free up space in Keymaster that was consumed by this bug. Test: On bramble: - Flashed rvc-d1-dev build, with wiping userdata - Flashed a newer build, without wiping userdata - Log expectedly shows key upgrades: $ adb logcat | grep 'Upgrading key' D vold : Upgrading key: /metadata/vold/metadata_encryption/key D vold : Upgrading key: /data/unencrypted/key D vold : Upgrading key: /data/misc/vold/user_keys/de/0 D vold : Upgrading key: /data/misc/vold/user_keys/ce/0/current - Rebooted - Log unexpectedly shows the system DE key being upgraded again: $ adb logcat | grep 'Upgrading key' D vold : Upgrading key: /data/unencrypted/key - "keymaster_key_blob_upgraded" unexpectedly still exists: $ adb shell find /data /metadata -name keymaster_key_blob_upgraded /data/unencrypted/key/keymaster_key_blob_upgraded - Applied this fix and flashed, without wiping userdata - Log shows system DE key being upgraded (expected because due to the bug, the upgraded key didn't replace the original one before) $ adb logcat | grep 'Upgrading key' D vold : Upgrading key: /data/unencrypted/key - "keymaster_key_blob_upgraded" expectedly no longer exists $ adb shell find /data /metadata -name keymaster_key_blob_upgraded - Rebooted - Log expectedly doesn't show any more key upgrades $ adb logcat | grep 'Upgrading key' Bug: 171944521 Bug: 172019387 (cherry picked from commit c493903732d0c17b33091cf722cbcc3262292801) Merged-In: I42d3f5fbe32cb2ec229f4b614cfb271412a3ed29 Change-Id: I42d3f5fbe32cb2ec229f4b614cfb271412a3ed29 Change-Id: I0449b812e91c13020a8b653f2149c33e46027b97 --- crypto/fscrypt/FsCrypt.cpp | 8 +++--- crypto/fscrypt/KeyStorage.cpp | 48 ++++++++++++++++++++------------ crypto/fscrypt/KeyStorage.h | 2 +- crypto/fscrypt/Keymaster.cpp | 8 +++--- crypto/fscrypt/Keymaster.h | 2 +- crypto/fscrypt/MetadataCrypt.cpp | 3 +- partitionmanager.cpp | 1 + 7 files changed, 43 insertions(+), 29 deletions(-) diff --git a/crypto/fscrypt/FsCrypt.cpp b/crypto/fscrypt/FsCrypt.cpp index 418164ed..3f388d39 100755 --- a/crypto/fscrypt/FsCrypt.cpp +++ b/crypto/fscrypt/FsCrypt.cpp @@ -201,7 +201,7 @@ static bool read_and_fixate_user_ce_key(userid_t user_id, auto const paths = get_ce_key_paths(directory_path); for (auto const ce_key_path : paths) { LOG(INFO) << "Trying user CE key " << ce_key_path; - if (retrieveKey(ce_key_path, auth, ce_key)) { + if (retrieveKey(ce_key_path, auth, ce_key, true)) { LOG(INFO) << "Successfully retrieved key"; fixate_user_ce_key(directory_path, ce_key_path, paths); return true; @@ -409,7 +409,7 @@ static bool load_all_de_keys() { userid_t user_id = std::stoi(entry->d_name); auto key_path = de_dir + "/" + entry->d_name; KeyBuffer de_key; - if (!retrieveKey(key_path, kEmptyAuthentication, &de_key)) return false; + if (!retrieveKey(key_path, kEmptyAuthentication, &de_key, true)) return false; EncryptionPolicy de_policy; if (!install_storage_key(DATA_MNT_POINT, options, de_key, &de_policy)) return false; auto ret = s_de_policies.insert({user_id, de_policy}); @@ -707,12 +707,12 @@ static bool fscrypt_rewrap_user_key(userid_t user_id, int serial, auto const directory_path = get_ce_key_directory_path(user_id); KeyBuffer ce_key; std::string ce_key_current_path = get_ce_key_current_path(directory_path); - if (retrieveKey(ce_key_current_path, retrieve_auth, &ce_key)) { + if (retrieveKey(ce_key_current_path, retrieve_auth, &ce_key, true)) { LOG(INFO) << "Successfully retrieved key"; // TODO(147732812): Remove this once Locksettingservice is fixed. // Currently it calls fscrypt_clear_user_key_auth with a secret when lockscreen is // changed from swipe to none or vice-versa - } else if (retrieveKey(ce_key_current_path, kEmptyAuthentication, &ce_key)) { + } else if (retrieveKey(ce_key_current_path, kEmptyAuthentication, &ce_key, true)) { LOG(INFO) << "Successfully retrieved key with empty auth"; } else { LOG(ERROR) << "Failed to retrieve key for user " << user_id; diff --git a/crypto/fscrypt/KeyStorage.cpp b/crypto/fscrypt/KeyStorage.cpp index ad9d4919..83800ce5 100755 --- a/crypto/fscrypt/KeyStorage.cpp +++ b/crypto/fscrypt/KeyStorage.cpp @@ -151,8 +151,20 @@ bool exportWrappedStorageKey(const KeyBuffer& kmKey, KeyBuffer* key) { if (!keymaster) return false; std::string key_temp; - if (!keymaster.exportKey(kmKey, &key_temp)) return false; - *key = KeyBuffer(key_temp.size()); + auto ret = keymaster.exportKey(kmKey, &key_temp); + if (ret != km::ErrorCode::OK) { + if (ret == km::ErrorCode::KEY_REQUIRES_UPGRADE) { + std::string kmKeyStr(reinterpret_cast(kmKey.data()), kmKey.size()); + std::string Keystr; + if (!keymaster.upgradeKey(kmKeyStr, km::AuthorizationSet(), &Keystr)) return false; + KeyBuffer upgradedKey = KeyBuffer(Keystr.size()); + memcpy(reinterpret_cast(upgradedKey.data()), Keystr.c_str(), upgradedKey.size()); + ret = keymaster.exportKey(upgradedKey, &key_temp); + if (ret != km::ErrorCode::OK) return false; + } else { + return false; + } + } *key = KeyBuffer(key_temp.size()); memcpy(reinterpret_cast(key->data()), key_temp.c_str(), key->size()); return true; } @@ -164,7 +176,7 @@ static std::pair beginParams( .Authorization(km::TAG_APPLICATION_ID, km::support::blob2hidlVec(appId)); km::HardwareAuthToken authToken; if (!auth.token.empty()) { - LOG(DEBUG) << "Supplying auth token to Keymaster"; + LOG(INFO) << "Supplying auth token to Keymaster"; authToken = km::support::hidlVec2AuthToken(km::support::blob2hidlVec(auth.token)); } return {paramBuilder, authToken}; @@ -243,21 +255,21 @@ static KeymasterOperation begin(Keymaster& keymaster, const std::string& dir, LOG(DEBUG) << "Upgrading key: " << dir; std::string newKey; if (!keymaster.upgradeKey(kmKey, keyParams, &newKey)) return KeymasterOperation(); - // auto newKeyPath = dir + "/" + kFn_keymaster_key_blob_upgraded; - // if (!writeStringToFile(newKey, newKeyPath)) return KeymasterOperation(); - // if (!keepOld) { - // if (rename(newKeyPath.c_str(), kmKeyPath.c_str()) != 0) { - // PLOG(ERROR) << "Unable to move upgraded key to location: " << kmKeyPath; - // return KeymasterOperation(); - // } - // if (!::FsyncDirectory(dir)) { - // LOG(ERROR) << "Key dir sync failed: " << dir; - // return KeymasterOperation(); - // } - // if (!kmDeleteKey(keymaster, kmKey)) { - // LOG(ERROR) << "Key deletion failed during upgrade, continuing anyway: " << dir; - // } - // } + auto newKeyPath = dir + "/" + kFn_keymaster_key_blob_upgraded; + if (!writeStringToFile(newKey, newKeyPath)) return KeymasterOperation(); + if (!keepOld) { + if (rename(newKeyPath.c_str(), kmKeyPath.c_str()) != 0) { + PLOG(ERROR) << "Unable to move upgraded key to location: " << kmKeyPath; + return KeymasterOperation(); + } + if (!::FsyncDirectory(dir)) { + LOG(ERROR) << "Key dir sync failed: " << dir; + return KeymasterOperation(); + } + if (!kmDeleteKey(keymaster, kmKey)) { + LOG(ERROR) << "Key deletion failed during upgrade, continuing anyway: " << dir; + } + } kmKey = newKey; LOG(INFO) << "Key upgraded: " << dir; } diff --git a/crypto/fscrypt/KeyStorage.h b/crypto/fscrypt/KeyStorage.h index b88cdc6a..2c888202 100755 --- a/crypto/fscrypt/KeyStorage.h +++ b/crypto/fscrypt/KeyStorage.h @@ -60,7 +60,7 @@ bool storeKeyAtomically(const std::string& key_path, const std::string& tmp_path // Retrieve the key from the named directory. bool retrieveKey(const std::string& dir, const KeyAuthentication& auth, KeyBuffer* key, - bool keepOld = false); + bool keepOld); // Securely destroy the key stored in the named directory and delete the directory. bool destroyKey(const std::string& dir); diff --git a/crypto/fscrypt/Keymaster.cpp b/crypto/fscrypt/Keymaster.cpp index 27353f10..c5b7bc3c 100755 --- a/crypto/fscrypt/Keymaster.cpp +++ b/crypto/fscrypt/Keymaster.cpp @@ -136,7 +136,7 @@ bool Keymaster::generateKey(const km::AuthorizationSet& inParams, std::string* k return true; } -bool Keymaster::exportKey(const KeyBuffer& kmKey, std::string* key) { +km::ErrorCode Keymaster::exportKey(const KeyBuffer& kmKey, std::string* key) { auto kmKeyBlob = km::support::blob2hidlVec(std::string(kmKey.data(), kmKey.size())); km::ErrorCode km_error; auto hidlCb = [&](km::ErrorCode ret, const hidl_vec& exportedKeyBlob) { @@ -148,13 +148,13 @@ bool Keymaster::exportKey(const KeyBuffer& kmKey, std::string* key) { auto error = mDevice->exportKey(km::KeyFormat::RAW, kmKeyBlob, {}, {}, hidlCb); if (!error.isOk()) { LOG(ERROR) << "export_key failed: " << error.description(); - return false; + return km::ErrorCode::UNKNOWN_ERROR; } if (km_error != km::ErrorCode::OK) { LOG(ERROR) << "export_key failed, code " << int32_t(km_error); - return false; + return km_error; } - return true; + return km::ErrorCode::OK; } bool Keymaster::deleteKey(const std::string& key) { diff --git a/crypto/fscrypt/Keymaster.h b/crypto/fscrypt/Keymaster.h index c3da4c82..6f74db43 100644 --- a/crypto/fscrypt/Keymaster.h +++ b/crypto/fscrypt/Keymaster.h @@ -112,7 +112,7 @@ class Keymaster { // Generate a key in the keymaster from the given params. bool generateKey(const km::AuthorizationSet& inParams, std::string* key); // Exports a keymaster key with STORAGE_KEY tag wrapped with a per-boot ephemeral key - bool exportKey(const KeyBuffer& kmKey, std::string* key); + km::ErrorCode exportKey(const KeyBuffer& kmKey, std::string* key); // If the keymaster supports it, permanently delete a key. bool deleteKey(const std::string& key); // Replace stored key blob in response to KM_ERROR_KEY_REQUIRES_UPGRADE. diff --git a/crypto/fscrypt/MetadataCrypt.cpp b/crypto/fscrypt/MetadataCrypt.cpp index d875ff04..1504367a 100755 --- a/crypto/fscrypt/MetadataCrypt.cpp +++ b/crypto/fscrypt/MetadataCrypt.cpp @@ -160,6 +160,7 @@ static bool read_key(const std::string& metadata_key_dir, const KeyGeneration& g * or we rebooted before commiting the keys in a freak accident. * Either way, we can re-upgrade the key if we need to. */ + Keymaster keymaster; if (pathExists(newKeyPath)) { if (!android::base::ReadFileToString(newKeyPath, &sKey)) @@ -170,7 +171,7 @@ static bool read_key(const std::string& metadata_key_dir, const KeyGeneration& g unlink(newKeyPath.c_str()); } bool needs_cp = cp_needsCheckpoint(); - if (!retrieveOrGenerateKey(dir, temp, kEmptyAuthentication, gen, key, needs_cp)) return false; + if (!retrieveOrGenerateKey(dir, temp, kEmptyAuthentication, gen, key, true)) return false; if (needs_cp && pathExists(newKeyPath)) std::thread(commit_key, dir).detach(); return true; } diff --git a/partitionmanager.cpp b/partitionmanager.cpp index 158d9167..fe0e3b82 100755 --- a/partitionmanager.cpp +++ b/partitionmanager.cpp @@ -450,6 +450,7 @@ void TWPartitionManager::Decrypt_Data() { } } } else { + LOGINFO("FBE setup failed. Trying FDE..."); Set_FDE_Encrypt_Status(); int password_type = cryptfs_get_password_type(); if (password_type == CRYPT_TYPE_DEFAULT) {