commit 08f4bdfe9836d8a5ae0033f2abcce31c9ccee25a
author Eric Biggers <ebiggers@google.com> Fri Oct 07 05:19:50 2022 +0000
committer Eric Biggers <ebiggers@google.com> Fri Oct 07 16:26:29 2022 +0000
tree 978e158823e5af32e395730e0a5131e87aa59022
parent 03ad91c3c2a359aaf57411ab5a8816f17265fb86

Don't use a secdiscardable file for keys encrypted by SP

Storage keys that are encrypted by the user's synthetic password don't
need to be securely deletable by vold, since secure deletion is already
implemented at a higher level: the synthetic password protectors managed
by LockSettingsService.  Therefore, remove the use of the secdiscardable
file by vold in this case to improve performance.

Bug: 232452368
Bug: 251131631
Bug: 251147505
Change-Id: I847d6cd3b289dbeb1ca2760d6e261a78c179cad0

KeyStorage.cpp

diff --git a/KeyStorage.cpp b/KeyStorage.cpp
index b4abc27..b8000fa 100644
--- a/KeyStorage.cpp
+++ b/KeyStorage.cpp
@@ -191,9 +191,13 @@
 }
 
 bool readSecdiscardable(const std::string& filename, std::string* hash) {
-    std::string secdiscardable;
-    if (!readFileToString(filename, &secdiscardable)) return false;
-    hashWithPrefix(kHashPrefix_secdiscardable, secdiscardable, hash);
+    if (pathExists(filename)) {
+        std::string secdiscardable;
+        if (!readFileToString(filename, &secdiscardable)) return false;
+        hashWithPrefix(kHashPrefix_secdiscardable, secdiscardable, hash);
+    } else {
+        *hash = "";
+    }
     return true;
 }
 
@@ -563,9 +567,12 @@
 
 // Creates a directory at the given path |dir| and stores |key| in it, in such a
 // way that it can only be retrieved via Keystore (if no secret is given in
-// |auth|) or with the given secret (if a secret is given in |auth|), and can be
-// securely deleted.  If a storage binding seed has been set, then the storage
-// binding seed will be required to retrieve the key as well.
+// |auth|) or with the given secret (if a secret is given in |auth|).  In the
+// former case, an attempt is made to make the key securely deletable.  In the
+// latter case, secure deletion is expected to be handled at a higher level.
+//
+// If a storage binding seed has been set, then the storage binding seed will be
+// required to retrieve the key as well.
 static bool storeKey(const std::string& dir, const KeyAuthentication& auth, const KeyBuffer& key) {
     if (TEMP_FAILURE_RETRY(mkdir(dir.c_str(), 0700)) == -1) {
         PLOG(ERROR) << "key mkdir " << dir;
@@ -573,7 +580,9 @@
     }
     if (!writeStringToFile(kCurrentVersion, dir + "/" + kFn_version)) return false;
     std::string secdiscardable_hash;
-    if (!createSecdiscardable(dir + "/" + kFn_secdiscardable, &secdiscardable_hash)) return false;
+    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;