From f187f0511026ffb65e74e5f0f08baa2d9be3d759 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 13 Oct 2022 03:50:21 +0000 Subject: [PATCH] Stop using the "stretching" file As a small optimization and code simplification, stop reading and writing the "stretching" file alongside each stored key. vold never does key stretching anymore. There was one special case in the code where if the stretching file existed and contained "nopassword", then the secret was ignored. However, this didn't seem to be of any use, especially since it didn't cause Keystore to be used, so it did *not* allow a key stored with no secret to be read if a secret was unexpectedly provided. Bug: 232452368 Bug: 251131631 Bug: 251147505 Change-Id: I5a7cbba7492526e51c451f222b9413d9fae6bce5 --- KeyStorage.cpp | 52 ++++++++------------------------------------------ 1 file changed, 8 insertions(+), 44 deletions(-) diff --git a/KeyStorage.cpp b/KeyStorage.cpp index b8000fa..55c1709 100644 --- a/KeyStorage.cpp +++ b/KeyStorage.cpp @@ -57,16 +57,14 @@ static constexpr size_t SECDISCARDABLE_BYTES = 1 << 14; static const char* kCurrentVersion = "1"; static const char* kRmPath = "/system/bin/rm"; static const char* kSecdiscardPath = "/system/bin/secdiscard"; -static const char* kStretch_none = "none"; -static const char* kStretch_nopassword = "nopassword"; static const char* kHashPrefix_secdiscardable = "Android secdiscardable SHA512"; static const char* kHashPrefix_keygen = "Android key wrapping key generation SHA512"; static const char* kFn_encrypted_key = "encrypted_key"; static const char* kFn_keymaster_key_blob = "keymaster_key_blob"; static const char* kFn_keymaster_key_blob_upgraded = "keymaster_key_blob_upgraded"; static const char* kFn_secdiscardable = "secdiscardable"; -static const char* kFn_stretching = "stretching"; static const char* kFn_version = "version"; +// Note: old key directories may contain a file named "stretching". namespace { @@ -412,36 +410,9 @@ static bool decryptWithKeystoreKey(Keystore& keystore, const std::string& dir, return true; } -static std::string getStretching(const KeyAuthentication& auth) { - if (auth.usesKeystore()) { - return kStretch_nopassword; - } else { - return kStretch_none; - } -} - -static bool stretchSecret(const std::string& stretching, const std::string& secret, - std::string* stretched) { - if (stretching == kStretch_nopassword) { - if (!secret.empty()) { - LOG(WARNING) << "Password present but stretching is nopassword"; - // Continue anyway - } - stretched->clear(); - } else if (stretching == kStretch_none) { - *stretched = secret; - } else { - LOG(ERROR) << "Unknown stretching type: " << stretching; - return false; - } - return true; -} - -static bool generateAppId(const KeyAuthentication& auth, const std::string& stretching, - const std::string& secdiscardable_hash, std::string* appId) { - std::string stretched; - if (!stretchSecret(stretching, auth.secret, &stretched)) return false; - *appId = secdiscardable_hash + stretched; +static std::string generateAppId(const KeyAuthentication& auth, + const std::string& secdiscardable_hash) { + std::string appId = secdiscardable_hash + auth.secret; const std::lock_guard scope_lock(storage_binding_info.guard); switch (storage_binding_info.state) { @@ -449,14 +420,13 @@ static bool generateAppId(const KeyAuthentication& auth, const std::string& stre storage_binding_info.state = StorageBindingInfo::State::NOT_USED; break; case StorageBindingInfo::State::IN_USE: - appId->append(storage_binding_info.seed.begin(), storage_binding_info.seed.end()); + appId.append(storage_binding_info.seed.begin(), storage_binding_info.seed.end()); break; case StorageBindingInfo::State::NOT_USED: // noop break; } - - return true; + return appId; } static void logOpensslError() { @@ -583,10 +553,7 @@ static bool storeKey(const std::string& dir, const KeyAuthentication& auth, cons if (auth.usesKeystore() && !createSecdiscardable(dir + "/" + kFn_secdiscardable, &secdiscardable_hash)) return false; - std::string stretching = getStretching(auth); - if (!writeStringToFile(stretching, dir + "/" + kFn_stretching)) return false; - std::string appId; - if (!generateAppId(auth, stretching, secdiscardable_hash, &appId)) return false; + std::string appId = generateAppId(auth, secdiscardable_hash); std::string encryptedKey; if (auth.usesKeystore()) { Keystore keystore; @@ -638,10 +605,7 @@ bool retrieveKey(const std::string& dir, const KeyAuthentication& auth, KeyBuffe } std::string secdiscardable_hash; if (!readSecdiscardable(dir + "/" + kFn_secdiscardable, &secdiscardable_hash)) return false; - std::string stretching; - if (!readFileToString(dir + "/" + kFn_stretching, &stretching)) return false; - std::string appId; - if (!generateAppId(auth, stretching, secdiscardable_hash, &appId)) return false; + std::string appId = generateAppId(auth, secdiscardable_hash); std::string encryptedMessage; if (!readFileToString(dir + "/" + kFn_encrypted_key, &encryptedMessage)) return false; if (auth.usesKeystore()) {