Replace string secret with a byte[] for CE storage in vold binder

Replace the current `string secret` argument to the lock/unlock of
CE storage with a `byte[]`. This is part of an effort to remove
instances of the LSKF and LSKF-derived secrets that are available
in a RAMdump -- since the strings are passed from Java, they cannot
be cleared, but `byte[]` can be.

This CL is the described argument change, and the propagation of this
change to the various functions that are called by the vold binder
functions.

Bug: 320392352
Test: Manual upgrade test:
	1. Flash the device with a build not including these changes
	2. Rebuild with these changes
	3. Flash the device (but do not wipe) with the build including
	   these changes
	4. See if the device boots and works normally -- if the CE
	   storage cannot be unlocked it will not start up and be usable
	   when the user logs in.
Change-Id: Icd4c925f2fd79e7533fdf9027e16f6736dbe1ab3

5 files changed, 22 insertions, 36 deletions

diff --git a/FsCrypt.cpp b/FsCrypt.cpp
index 51b35c51..3eb45994 100644
--- a/FsCrypt.cpp
+++ b/FsCrypt.cpp
@@ -675,26 +675,13 @@ bool fscrypt_destroy_user_keys(userid_t user_id) {
     return success;
 }
 
-static bool parse_hex(const std::string& hex, std::string* result) {
-    if (hex == "!") {
-        *result = "";
-        return true;
-    }
-    if (android::vold::HexToStr(hex, *result) != 0) {
-        LOG(ERROR) << "Invalid FBE hex string";  // Don't log the string for security reasons
-        return false;
-    }
-    return true;
-}
-
-static std::optional<android::vold::KeyAuthentication> authentication_from_hex(
-        const std::string& secret_hex) {
-    std::string secret;
-    if (!parse_hex(secret_hex, &secret)) return std::optional<android::vold::KeyAuthentication>();
-    if (secret.empty()) {
+static android::vold::KeyAuthentication authentication_from_secret(
+        const std::vector<uint8_t>& secret) {
+    std::string secret_str(secret.begin(), secret.end());
+    if (secret_str.empty()) {
         return kEmptyAuthentication;
     } else {
-        return android::vold::KeyAuthentication(secret);
+        return android::vold::KeyAuthentication(secret_str);
     }
 }
 
@@ -743,12 +730,11 @@ static bool destroy_volkey(const std::string& misc_path, const std::string& volu
 // re-encrypting the CE key upon upgrade from an Android version where the CE
 // key was stored with kEmptyAuthentication when the user didn't have an LSKF.
 // See the comments below for the different cases handled.
-bool fscrypt_set_ce_key_protection(userid_t user_id, const std::string& secret_hex) {
+bool fscrypt_set_ce_key_protection(userid_t user_id, const std::vector<uint8_t>& secret) {
     LOG(DEBUG) << "fscrypt_set_ce_key_protection " << user_id;
     if (!IsFbeEnabled()) return true;
-    auto auth = authentication_from_hex(secret_hex);
-    if (!auth) return false;
-    if (auth->secret.empty()) {
+    auto auth = authentication_from_secret(secret);
+    if (auth.secret.empty()) {
         LOG(ERROR) << "fscrypt_set_ce_key_protection: secret must be nonempty";
         return false;
     }
@@ -776,7 +762,7 @@ bool fscrypt_set_ce_key_protection(userid_t user_id, const std::string& secret_h
             // with the given secret.  This isn't expected, but in theory it
             // could happen if an upgrade is requested for a user more than once
             // due to a power-off or other interruption.
-            if (read_and_fixate_user_ce_key(user_id, *auth, &ce_key)) {
+            if (read_and_fixate_user_ce_key(user_id, auth, &ce_key)) {
                 LOG(WARNING) << "CE key is already protected by given secret";
                 return true;
             }
@@ -802,7 +788,7 @@ bool fscrypt_set_ce_key_protection(userid_t user_id, const std::string& secret_h
     auto const paths = get_ce_key_paths(directory_path);
     std::string ce_key_path;
     if (!get_ce_key_new_path(directory_path, paths, &ce_key_path)) return false;
-    if (!android::vold::storeKeyAtomically(ce_key_path, user_key_temp, *auth, ce_key)) return false;
+    if (!android::vold::storeKeyAtomically(ce_key_path, user_key_temp, auth, ce_key)) return false;
 
     // Fixate the key, i.e. delete all other bindings of it.  (In practice this
     // just means the kEmptyAuthentication binding, if there is one.)  However,
@@ -845,17 +831,16 @@ std::vector<int> fscrypt_get_unlocked_users() {
 // Unlocks internal CE storage for the given user.  This only unlocks internal storage, since
 // fscrypt_prepare_user_storage() has to be called for each adoptable storage volume anyway (since
 // the volume might have been absent when the user was created), and that handles the unlocking.
-bool fscrypt_unlock_ce_storage(userid_t user_id, const std::string& secret_hex) {
+bool fscrypt_unlock_ce_storage(userid_t user_id, const std::vector<uint8_t>& secret) {
     LOG(DEBUG) << "fscrypt_unlock_ce_storage " << user_id;
     if (!IsFbeEnabled()) return true;
     if (s_ce_policies.count(user_id) != 0) {
         LOG(WARNING) << "CE storage for user " << user_id << " is already unlocked";
         return true;
     }
-    auto auth = authentication_from_hex(secret_hex);
-    if (!auth) return false;
+    auto auth = authentication_from_secret(secret);
     KeyBuffer ce_key;
-    if (!read_and_fixate_user_ce_key(user_id, *auth, &ce_key)) return false;
+    if (!read_and_fixate_user_ce_key(user_id, auth, &ce_key)) return false;
     EncryptionPolicy ce_policy;
     if (!install_storage_key(DATA_MNT_POINT, s_data_options, ce_key, &ce_policy)) return false;
     s_ce_policies[user_id].internal = ce_policy;
diff --git a/FsCrypt.h b/FsCrypt.h
index afcedfbe..be21fba2 100644
--- a/FsCrypt.h
+++ b/FsCrypt.h
@@ -25,11 +25,11 @@ bool fscrypt_init_user0();
 extern bool fscrypt_init_user0_done;
 bool fscrypt_create_user_keys(userid_t user_id, bool ephemeral);
 bool fscrypt_destroy_user_keys(userid_t user_id);
-bool fscrypt_set_ce_key_protection(userid_t user_id, const std::string& secret);
+bool fscrypt_set_ce_key_protection(userid_t user_id, const std::vector<uint8_t>& secret);
 void fscrypt_deferred_fixate_ce_keys();
 
 std::vector<int> fscrypt_get_unlocked_users();
-bool fscrypt_unlock_ce_storage(userid_t user_id, const std::string& secret);
+bool fscrypt_unlock_ce_storage(userid_t user_id, const std::vector<uint8_t>& secret);
 bool fscrypt_lock_ce_storage(userid_t user_id);
 
 bool fscrypt_prepare_user_storage(const std::string& volume_uuid, userid_t user_id, int flags);
diff --git a/VoldNativeService.cpp b/VoldNativeService.cpp
index 96f4eaff..b36856a8 100644
--- a/VoldNativeService.cpp
+++ b/VoldNativeService.cpp
@@ -630,7 +630,7 @@ binder::Status VoldNativeService::destroyUserStorageKeys(int32_t userId) {
 }
 
 binder::Status VoldNativeService::setCeStorageProtection(int32_t userId,
-                                                         const std::string& secret) {
+                                                         const std::vector<uint8_t>& secret) {
     ENFORCE_SYSTEM_OR_ROOT;
     ACQUIRE_CRYPT_LOCK;
 
@@ -645,7 +645,8 @@ binder::Status VoldNativeService::getUnlockedUsers(std::vector<int>* _aidl_retur
     return Ok();
 }
 
-binder::Status VoldNativeService::unlockCeStorage(int32_t userId, const std::string& secret) {
+binder::Status VoldNativeService::unlockCeStorage(int32_t userId,
+                                                  const std::vector<uint8_t>& secret) {
     ENFORCE_SYSTEM_OR_ROOT;
     ACQUIRE_CRYPT_LOCK;
 
diff --git a/VoldNativeService.h b/VoldNativeService.h
index bb00d35f..a4fbf009 100644
--- a/VoldNativeService.h
+++ b/VoldNativeService.h
@@ -116,10 +116,10 @@ class VoldNativeService : public BinderService<VoldNativeService>, public os::Bn
     binder::Status createUserStorageKeys(int32_t userId, bool ephemeral);
     binder::Status destroyUserStorageKeys(int32_t userId);
 
-    binder::Status setCeStorageProtection(int32_t userId, const std::string& secret);
+    binder::Status setCeStorageProtection(int32_t userId, const std::vector<uint8_t>& secret);
 
     binder::Status getUnlockedUsers(std::vector<int>* _aidl_return);
-    binder::Status unlockCeStorage(int32_t userId, const std::string& secret);
+    binder::Status unlockCeStorage(int32_t userId, const std::vector<uint8_t>& secret);
     binder::Status lockCeStorage(int32_t userId);
 
     binder::Status prepareUserStorage(const std::optional<std::string>& uuid, int32_t userId,
diff --git a/binder/android/os/IVold.aidl b/binder/android/os/IVold.aidl
index dfccc004..d37697be 100644
--- a/binder/android/os/IVold.aidl
+++ b/binder/android/os/IVold.aidl
@@ -91,10 +91,10 @@ interface IVold {
     void createUserStorageKeys(int userId, boolean ephemeral);
     void destroyUserStorageKeys(int userId);
 
-    void setCeStorageProtection(int userId, @utf8InCpp String secret);
+    void setCeStorageProtection(int userId, in byte[] secret);
 
     int[] getUnlockedUsers();
-    void unlockCeStorage(int userId, @utf8InCpp String secret);
+    void unlockCeStorage(int userId, in byte[] secret);
     void lockCeStorage(int userId);
 
     void prepareUserStorage(@nullable @utf8InCpp String uuid, int userId, int storageFlags);