From 1e6a5f51065173224700d551693867bd33c7e5b9 Mon Sep 17 00:00:00 2001 From: Paul Crowley Date: Fri, 6 Aug 2021 15:16:10 -0700 Subject: [PATCH] Detect factory reset and deleteAllKeys Where metadata encryption is enabled, if there is no metadata encryption key present and we are generating one anew, then there has been a factory reset, and this is the first key to be generated. We then call deleteAllKeys to ensure data from before the factory reset is securely deleted. This shouldn't really be necessary; the factory reset call itself should be doing this. However there are currently three factory reset paths (settings, recovery, fastboot -w) and it is not clear that all three are doing this correctly on all devices. Obviously an attacker can prevent this code from being run by running a version of the OS that does not include this change; however, if the bootloader is locked, then keys will be version bound such that they will only work on locked devices with a sufficiently recent version of the OS. If every sufficiently recent signed version of the OS includes this change the attack is defeated. Bug: 187105270 Test: booted Cuttlefish twice, checked logs Change-Id: I9c5c547140e8b1bbffb9c1d215f75251f0f1354e --- Keystore.cpp | 13 +++++++++++++ Keystore.h | 3 +++ MetadataCrypt.cpp | 11 +++++++++++ 3 files changed, 27 insertions(+) diff --git a/Keystore.cpp b/Keystore.cpp index 0995d05..a017d68 100644 --- a/Keystore.cpp +++ b/Keystore.cpp @@ -230,5 +230,18 @@ void Keystore::earlyBootEnded() { logKeystore2ExceptionIfPresent(rc, "earlyBootEnded"); } +void Keystore::deleteAllKeys() { + ::ndk::SpAIBinder binder(AServiceManager_getService(maintenance_service_name)); + auto maint_service = ks2_maint::IKeystoreMaintenance::fromBinder(binder); + + if (!maint_service) { + LOG(ERROR) << "Unable to connect to keystore2 maintenance service for deleteAllKeys"; + return; + } + + auto rc = maint_service->deleteAllKeys(); + logKeystore2ExceptionIfPresent(rc, "deleteAllKeys"); +} + } // namespace vold } // namespace android diff --git a/Keystore.h b/Keystore.h index 05a8370..d8c488e 100644 --- a/Keystore.h +++ b/Keystore.h @@ -126,6 +126,9 @@ class Keystore { // be created or used. static void earlyBootEnded(); + // Tell all Keymint devices to delete all rollback-protected keys. + static void deleteAllKeys(); + private: std::shared_ptr securityLevel; DISALLOW_COPY_AND_ASSIGN(Keystore); diff --git a/MetadataCrypt.cpp b/MetadataCrypt.cpp index 5fe7918..277a908 100644 --- a/MetadataCrypt.cpp +++ b/MetadataCrypt.cpp @@ -113,6 +113,17 @@ static bool read_key(const std::string& metadata_key_dir, const KeyGeneration& g auto dir = metadata_key_dir + "/key"; LOG(DEBUG) << "metadata_key_dir/key: " << dir; if (!MkdirsSync(dir, 0700)) return false; + if (!pathExists(dir)) { + auto delete_all = android::base::GetBoolProperty( + "ro.crypto.metadata_init_delete_all_keys.enabled", false); + if (delete_all) { + LOG(INFO) << "Metadata key does not exist, calling deleteAllKeys"; + Keystore::deleteAllKeys(); + } else { + LOG(DEBUG) << "Metadata key does not exist but " + "ro.crypto.metadata_init_delete_all_keys.enabled is false"; + } + } auto temp = metadata_key_dir + "/tmp"; return retrieveOrGenerateKey(dir, temp, kEmptyAuthentication, gen, key); }