KeyStorage: rework key upgrade handling
Remove the error-prone 'keepOld' parameter, and instead make begin()
(renamed to BeginKeymasterOp()) do all the key upgrade handling.
Don't handle /data and /metadata differently anymore. Previously, when
a checkpoint is active, key blob files were replaced on /data
immediately; only the actual Keymaster key deletion was delayed until
checkpoint commit. But it's easier to just delay the key blob file
replacement too, as we have to implement that for /metadata anyway.
Also be more vigilant about deleting any leftover upgraded keys.
Test: Tested on bramble using an OTA rvc-d1-release => master. In OTA
success case, verified via logcat that the keys were upgraded and
then were committed after the boot succeeded. In OTA failure
case, verified that the device still boots -- i.e., the old keys
weren't lost. Verified that in either case, no
keymaster_key_blob_upgraded files were left over. Finally, also
tried 'pm create-user' and 'pm remove-user' and verified via
logcat that the Keymaster keys still get deleted.
Change-Id: Ic9c3e63e0bcae0c608fc79050ca4a1676b3852ee
diff --git a/MetadataCrypt.cpp b/MetadataCrypt.cpp
index f794ee3..fdee21f 100644
--- a/MetadataCrypt.cpp
+++ b/MetadataCrypt.cpp
@@ -17,17 +17,13 @@
#include "MetadataCrypt.h"
#include "KeyBuffer.h"
-#include <algorithm>
#include <string>
-#include <thread>
-#include <vector>
#include <fcntl.h>
#include <sys/param.h>
#include <sys/stat.h>
#include <sys/types.h>
-#include <android-base/file.h>
#include <android-base/logging.h>
#include <android-base/properties.h>
#include <android-base/strings.h>
@@ -64,9 +60,6 @@
static const std::string kDmNameUserdata = "userdata";
-static const char* kFn_keymaster_key_blob = "keymaster_key_blob";
-static const char* kFn_keymaster_key_blob_upgraded = "keymaster_key_blob_upgraded";
-
// The first entry in this table is the default crypto type.
constexpr CryptoType supported_crypto_types[] = {aes_256_xts, adiantum};
@@ -111,31 +104,6 @@
return true;
}
-// Note: It is possible to orphan a key if it is removed before deleting
-// Update this once keymaster APIs change, and we have a proper commit.
-static void commit_key(const std::string& dir) {
- while (!android::base::WaitForProperty("vold.checkpoint_committed", "1")) {
- LOG(ERROR) << "Wait for boot timed out";
- }
- Keymaster keymaster;
- auto keyPath = dir + "/" + kFn_keymaster_key_blob;
- auto newKeyPath = dir + "/" + kFn_keymaster_key_blob_upgraded;
- std::string key;
-
- if (!android::base::ReadFileToString(keyPath, &key)) {
- LOG(ERROR) << "Failed to read old key: " << dir;
- return;
- }
- if (rename(newKeyPath.c_str(), keyPath.c_str()) != 0) {
- PLOG(ERROR) << "Unable to move upgraded key to location: " << keyPath;
- return;
- }
- if (!keymaster.deleteKey(key)) {
- LOG(ERROR) << "Key deletion failed during upgrade, continuing anyway: " << dir;
- }
- LOG(INFO) << "Old Key deleted: " << dir;
-}
-
static bool read_key(const std::string& metadata_key_dir, const KeyGeneration& gen,
KeyBuffer* key) {
if (metadata_key_dir.empty()) {
@@ -150,25 +118,7 @@
return false;
}
auto temp = metadata_key_dir + "/tmp";
- auto newKeyPath = dir + "/" + kFn_keymaster_key_blob_upgraded;
- /* If we have a leftover upgraded key, delete it.
- * We either failed an update and must return to the old key,
- * 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))
- LOG(ERROR) << "Failed to read incomplete key: " << dir;
- else if (!keymaster.deleteKey(sKey))
- LOG(ERROR) << "Incomplete key deletion failed, continuing anyway: " << dir;
- else
- unlink(newKeyPath.c_str());
- }
- bool needs_cp = cp_needsCheckpoint();
- if (!retrieveOrGenerateKey(dir, temp, kEmptyAuthentication, gen, key, needs_cp)) return false;
- if (needs_cp && pathExists(newKeyPath)) std::thread(commit_key, dir).detach();
- return true;
+ return retrieveOrGenerateKey(dir, temp, kEmptyAuthentication, gen, key);
}
static bool get_number_of_sectors(const std::string& real_blkdev, uint64_t* nr_sec) {