From 2d30b890d24767a50b15a28ee6aa933f4f67d765 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 28 Jul 2022 18:06:42 +0000 Subject: [PATCH] KeyStorage: don't request rollback resistance for wrapped storage keys Hardware-wrapped inline encryption keys (a.k.a. "wrapped storage keys" or "TAG_STORAGE_KEY keys") are being generated with rollback resistance enabled, but are never deleted. This leaks the space that KeyMint implementations reserve for rollback-resistant keys, e.g. space in the RPMB. This is a problem especially for the per-boot key, as that gets regenerated every time the device is rebooted. After enough reboots, KeyMint runs out of space for rollback-resistant keys. This stops any new or upgraded keys from being rollback-resistant, reducing security. This bug affects all devices that use HW-wrapped inline encryption keys for FBE (have "wrappedkey_v0" in the options for fileencryption in their fstab), and whose KeyMint implementations support TAG_STORAGE_KEY in combination with TAG_ROLLBACK_RESISTANCE. But it's more of a problem on devices that are rebooted frequently, as per the above. Fix this bug by not requesting rollback resistance for HW-wrapped inline encryption keys. It was a mistake for these keys to ever be rollback- resistant, as they are simply a stand-in for raw keys. Secure deletion instead has to happen higher up the stack, via the Keystore key that encrypts these keys being deleted, or via the Keystore key and/or Weaver slot needed to decrypt the user's synthetic password being deleted. (It was also a mistake for HW-wrapped inline encryption keys to use Keystore at all. The revised design for them that I'm working on for upstream Linux doesn't use Keystore. But for now, Android uses Keystore for them, and the fix is to not request rollback resistance.) Bug: 240533602 Fixes: 3dfb094cb26c ("vold: Support Storage keys for FBE") Change-Id: I648a1af9e16787dfcfeefa2b2f2e4a72cac2c6a6 --- KeyStorage.cpp | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/KeyStorage.cpp b/KeyStorage.cpp index 3ede67e..b4abc27 100644 --- a/KeyStorage.cpp +++ b/KeyStorage.cpp @@ -117,9 +117,13 @@ static void hashWithPrefix(char const* prefix, const std::string& tohash, std::s SHA512_Final(reinterpret_cast(&(*res)[0]), &c); } -// Generates a keystore key, using rollback resistance if supported. -static bool generateKeystoreKey(Keystore& keystore, const km::AuthorizationSetBuilder& paramBuilder, - std::string* key) { +static bool generateKeyStorageKey(Keystore& keystore, const std::string& appId, std::string* key) { + auto paramBuilder = km::AuthorizationSetBuilder() + .AesEncryptionKey(AES_KEY_BYTES * 8) + .GcmModeMinMacLen(GCM_MAC_BYTES * 8) + .Authorization(km::TAG_APPLICATION_ID, appId) + .Authorization(km::TAG_NO_AUTH_REQUIRED); + LOG(DEBUG) << "Generating \"key storage\" key"; auto paramsWithRollback = paramBuilder; paramsWithRollback.Authorization(km::TAG_ROLLBACK_RESISTANCE); @@ -132,23 +136,13 @@ static bool generateKeystoreKey(Keystore& keystore, const km::AuthorizationSetBu return true; } -static bool generateKeyStorageKey(Keystore& keystore, const std::string& appId, std::string* key) { - auto paramBuilder = km::AuthorizationSetBuilder() - .AesEncryptionKey(AES_KEY_BYTES * 8) - .GcmModeMinMacLen(GCM_MAC_BYTES * 8) - .Authorization(km::TAG_APPLICATION_ID, appId) - .Authorization(km::TAG_NO_AUTH_REQUIRED); - LOG(DEBUG) << "Generating \"key storage\" key"; - return generateKeystoreKey(keystore, paramBuilder, key); -} - bool generateWrappedStorageKey(KeyBuffer* key) { Keystore keystore; if (!keystore) return false; std::string key_temp; auto paramBuilder = km::AuthorizationSetBuilder().AesEncryptionKey(AES_KEY_BYTES * 8); paramBuilder.Authorization(km::TAG_STORAGE_KEY); - if (!generateKeystoreKey(keystore, paramBuilder, &key_temp)) return false; + if (!keystore.generateKey(paramBuilder, &key_temp)) return false; *key = KeyBuffer(key_temp.size()); memcpy(reinterpret_cast(key->data()), key_temp.c_str(), key->size()); return true;