Defer CE key fixations to checkpoint commit
On the first boot after an upgrade, ensure that any Keystore key
deletions triggered by fscrypt_set_user_key_protection() are deferred
until the userdata filesystem checkpoint is committed, so that the
system doesn't end up in a bad state if the checkpoint is rolled back.
Test: see I77d30f9be57de7b7c4818680732331549ecb73c8
Bug: 232452368
Ignore-AOSP-First: depends on other changes in internal master
Change-Id: I59b758bc13b7a2ae270f1a6c409affe2eb61119c
diff --git a/Checkpoint.cpp b/Checkpoint.cpp
index 948231d..3825af9 100644
--- a/Checkpoint.cpp
+++ b/Checkpoint.cpp
@@ -16,6 +16,8 @@
#define LOG_TAG "Checkpoint"
#include "Checkpoint.h"
+#include "FsCrypt.h"
+#include "KeyStorage.h"
#include "VoldUtil.h"
#include "VolumeManager.h"
@@ -78,6 +80,18 @@
return true;
}
+// Do any work that was deferred until the userdata filesystem checkpoint was
+// committed. This work involves the deletion of resources that aren't covered
+// by the userdata filesystem checkpoint, e.g. Keystore keys.
+void DoCheckpointCommittedWork() {
+ // Take the crypt lock to provide synchronization with the Binder calls that
+ // operate on key directories.
+ std::lock_guard<std::mutex> lock(VolumeManager::Instance()->getCryptLock());
+
+ DeferredCommitKeystoreKeys();
+ fscrypt_deferred_fixate_ce_keys();
+}
+
} // namespace
Status cp_supportsCheckpoint(bool& result) {
@@ -205,6 +219,7 @@
if (!android::base::RemoveFileIfExists(kMetadataCPFile, &err_str))
return error(err_str.c_str());
+ std::thread(DoCheckpointCommittedWork).detach();
return Status::ok();
}
diff --git a/FsCrypt.cpp b/FsCrypt.cpp
index 477db7c..8df438f 100644
--- a/FsCrypt.cpp
+++ b/FsCrypt.cpp
@@ -16,6 +16,7 @@
#include "FsCrypt.h"
+#include "Checkpoint.h"
#include "KeyStorage.h"
#include "KeyUtil.h"
#include "Utils.h"
@@ -112,6 +113,9 @@
std::map<userid_t, EncryptionPolicy> s_de_policies;
std::map<userid_t, EncryptionPolicy> s_ce_policies;
+// CE key fixation operations that have been deferred to checkpoint commit
+std::map<std::string, std::string> s_deferred_fixations;
+
} // namespace
// Returns KeyGeneration suitable for key as described in EncryptionOptions
@@ -214,6 +218,7 @@
LOG(DEBUG) << "Trying user CE key " << ce_key_path;
if (retrieveKey(ce_key_path, auth, ce_key)) {
LOG(DEBUG) << "Successfully retrieved key";
+ s_deferred_fixations.erase(directory_path);
fixate_user_ce_key(directory_path, ce_key_path, paths);
return true;
}
@@ -676,6 +681,7 @@
success &= android::vold::destroyKey(path);
}
}
+ s_deferred_fixations.erase(ce_path);
success &= destroy_dir(ce_path);
auto de_key_path = get_de_key_path(user_id);
@@ -815,13 +821,37 @@
std::string ce_key_path;
if (!get_ce_key_new_path(directory_path, paths, &ce_key_path)) return false;
if (!android::vold::storeKeyAtomically(ce_key_path, user_key_temp, *auth, ce_key)) return false;
- if (!fixate_user_ce_key(directory_path, ce_key_path, paths)) return false;
+
+ // Fixate the key, i.e. delete all other bindings of it. (In practice this
+ // just means the kEmptyAuthentication binding, if there is one.) However,
+ // if a userdata filesystem checkpoint is pending, then we need to delay the
+ // fixation until the checkpoint has been committed, since deleting keys
+ // from Keystore cannot be rolled back.
+ if (android::vold::cp_needsCheckpoint()) {
+ LOG(INFO) << "Deferring fixation of " << directory_path << " until checkpoint is committed";
+ s_deferred_fixations[directory_path] = ce_key_path;
+ } else {
+ s_deferred_fixations.erase(directory_path);
+ if (!fixate_user_ce_key(directory_path, ce_key_path, paths)) return false;
+ }
+
if (s_new_ce_keys.erase(user_id)) {
LOG(INFO) << "Stored CE key for new user " << user_id;
}
return true;
}
+void fscrypt_deferred_fixate_ce_keys() {
+ for (const auto& it : s_deferred_fixations) {
+ const auto& directory_path = it.first;
+ const auto& to_fix = it.second;
+ LOG(INFO) << "Doing deferred fixation of " << directory_path;
+ fixate_user_ce_key(directory_path, to_fix, get_ce_key_paths(directory_path));
+ // Continue on error.
+ }
+ s_deferred_fixations.clear();
+}
+
std::vector<int> fscrypt_get_unlocked_users() {
std::vector<int> user_ids;
for (const auto& it : s_ce_policies) {
diff --git a/FsCrypt.h b/FsCrypt.h
index 2cb47eb..8d84bdc 100644
--- a/FsCrypt.h
+++ b/FsCrypt.h
@@ -26,6 +26,7 @@
bool fscrypt_vold_create_user_key(userid_t user_id, int serial, bool ephemeral);
bool fscrypt_destroy_user_key(userid_t user_id);
bool fscrypt_set_user_key_protection(userid_t user_id, const std::string& secret);
+void fscrypt_deferred_fixate_ce_keys();
std::vector<int> fscrypt_get_unlocked_users();
bool fscrypt_unlock_user_key(userid_t user_id, int serial, const std::string& secret);
diff --git a/KeyStorage.cpp b/KeyStorage.cpp
index eb994e9..33d415e 100644
--- a/KeyStorage.cpp
+++ b/KeyStorage.cpp
@@ -23,7 +23,6 @@
#include <algorithm>
#include <memory>
#include <mutex>
-#include <thread>
#include <vector>
#include <errno.h>
@@ -231,9 +230,8 @@
return true;
}
-static void DeferredCommitKeys() {
- android::base::WaitForProperty("vold.checkpoint_committed", "1");
- LOG(INFO) << "Committing upgraded keys";
+void DeferredCommitKeystoreKeys() {
+ LOG(INFO) << "Committing upgraded Keystore keys";
Keystore keystore;
if (!keystore) {
LOG(ERROR) << "Failed to open Keystore; old keys won't be deleted from Keystore";
@@ -241,10 +239,11 @@
}
std::lock_guard<std::mutex> lock(key_upgrade_lock);
for (auto& dir : key_dirs_to_commit) {
- LOG(INFO) << "Committing upgraded key " << dir;
+ LOG(INFO) << "Committing upgraded Keystore key for " << dir;
CommitUpgradedKey(keystore, dir);
}
key_dirs_to_commit.clear();
+ LOG(INFO) << "Done committing upgraded Keystore keys";
}
// Returns true if the Keystore key in |dir| has already been upgraded and is
@@ -260,7 +259,6 @@
// that key_upgrade_lock is held and that a commit isn't already pending for the
// directory.
static void ScheduleKeyCommit(const std::string& dir) {
- if (key_dirs_to_commit.empty()) std::thread(DeferredCommitKeys).detach();
key_dirs_to_commit.push_back(dir);
}
diff --git a/KeyStorage.h b/KeyStorage.h
index cc2f549..22453ea 100644
--- a/KeyStorage.h
+++ b/KeyStorage.h
@@ -41,6 +41,8 @@
bool createSecdiscardable(const std::string& path, std::string* hash);
bool readSecdiscardable(const std::string& path, std::string* hash);
+void DeferredCommitKeystoreKeys();
+
// 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);