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

Always use RenameKeyDir() when moving/renaming key directories

Browse source 
Make fixate_user_ce_key() use RenameKeyDir() to rename key directories
so that any deferred commits for these directories are also updated
appropriately.

This fixes a potential lost Keymaster key upgrade if a key were to be
re-wrapped while a user data checkpoint is pending.  This isn't a huge
issue as the key will just get upgraded again, but this should be fixed.

[ebiggers@ - cleaned up slightly from satyat@'s original change]

Bug: 190398249
Change-Id: Ic6c5b4468d07ab335368e3d373916145d096af01
This commit is contained in:
Satya Tangirala 2021-06-08 12:55:24 -07:00 committed by Eric Biggers
parent 107d21d484
commit 0f890a93e1
3 changed files with 13 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
FsCrypt.cpp
View file

@ -186,10 +186,7 @@ static void fixate_user_ce_key(const std::string& directory_path, const std::str
auto const current_path = get_ce_key_current_path(directory_path); auto const current_path = get_ce_key_current_path(directory_path);
if (to_fix != current_path) { if (to_fix != current_path) {
LOG(DEBUG) << "Renaming " << to_fix << " to " << current_path; LOG(DEBUG) << "Renaming " << to_fix << " to " << current_path;
if (rename(to_fix.c_str(), current_path.c_str()) != 0) { if (!android::vold::RenameKeyDir(to_fix, current_path)) return;
PLOG(WARNING) << "Unable to rename " << to_fix << " to " << current_path;
return;
}
} }
android::vold::FsyncDirectory(directory_path); android::vold::FsyncDirectory(directory_path);
} }

16
KeyStorage.cpp
View file

@ -288,9 +288,7 @@ static void CancelPendingKeyCommit(const std::string& dir) {
} }
} }
// Renames a key directory. Also updates the deferred commit vector bool RenameKeyDir(const std::string& old_name, const std::string& new_name) {
// (key_dirs_to_commit) appropriately.
static bool RenameKeyDir(const std::string& old_name, const std::string& new_name) {
std::lock_guard<std::mutex> lock(key_upgrade_lock); std::lock_guard<std::mutex> lock(key_upgrade_lock);
// Find the entry in key_dirs_to_commit (if any) for this directory so that // Find the entry in key_dirs_to_commit (if any) for this directory so that
@ -301,7 +299,11 @@ static bool RenameKeyDir(const std::string& old_name, const std::string& new_nam
if (IsSameFile(old_name, *it)) break; if (IsSameFile(old_name, *it)) break;
} }
if (rename(old_name.c_str(), new_name.c_str()) != 0) return false; if (rename(old_name.c_str(), new_name.c_str()) != 0) {
PLOG(ERROR) << "Failed to rename key directory \"" << old_name << "\" to \"" << new_name
<< "\"";
return false;
}
if (it != key_dirs_to_commit.end()) *it = new_name; if (it != key_dirs_to_commit.end()) *it = new_name;
@ -614,10 +616,8 @@ bool storeKeyAtomically(const std::string& key_path, const std::string& tmp_path
} }
if (!storeKey(tmp_path, auth, key)) return false; if (!storeKey(tmp_path, auth, key)) return false;
if (!RenameKeyDir(tmp_path, key_path)) { if (!RenameKeyDir(tmp_path, key_path)) return false;
PLOG(ERROR) << "Unable to move new key to location: " << key_path;
return false;
}
if (!FsyncParentDirectory(key_path)) return false; if (!FsyncParentDirectory(key_path)) return false;
LOG(DEBUG) << "Created key: " << key_path; LOG(DEBUG) << "Created key: " << key_path;
return true; return true;

4
KeyStorage.h
View file

@ -41,6 +41,10 @@ extern const KeyAuthentication kEmptyAuthentication;
bool createSecdiscardable(const std::string& path, std::string* hash); bool createSecdiscardable(const std::string& path, std::string* hash);
bool readSecdiscardable(const std::string& path, std::string* hash); bool readSecdiscardable(const std::string& path, std::string* hash);
// Renames a key directory while also managing deferred commits appropriately.
// This method should be used whenever a key directory needs to be moved/renamed.
bool RenameKeyDir(const std::string& old_name, const std::string& new_name);
// Create a directory at the named path, and store "key" in it, // Create a directory at the named path, and store "key" in it,
// in such a way that it can only be retrieved via Keymaster and // in such a way that it can only be retrieved via Keymaster and
// can be securely deleted. // can be securely deleted.