From 5177ed2e5030ed46ec34a581793890eaa167e4b1 Mon Sep 17 00:00:00 2001 From: Ellen Arteca Date: Wed, 3 Apr 2024 21:18:01 +0000 Subject: [PATCH] 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 --- FsCrypt.cpp | 41 ++++++++++++------------------------ FsCrypt.h | 4 ++-- VoldNativeService.cpp | 5 +++-- VoldNativeService.h | 4 ++-- binder/android/os/IVold.aidl | 4 ++-- 5 files changed, 22 insertions(+), 36 deletions(-) diff --git a/FsCrypt.cpp b/FsCrypt.cpp index 51b35c5..3eb4599 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 authentication_from_hex( - const std::string& secret_hex) { - std::string secret; - if (!parse_hex(secret_hex, &secret)) return std::optional(); - if (secret.empty()) { +static android::vold::KeyAuthentication authentication_from_secret( + const std::vector& 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& 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 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& 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 afcedfb..be21fba 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& secret); void fscrypt_deferred_fixate_ce_keys(); std::vector 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& 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 96f4eaf..b36856a 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& secret) { ENFORCE_SYSTEM_OR_ROOT; ACQUIRE_CRYPT_LOCK; @@ -645,7 +645,8 @@ binder::Status VoldNativeService::getUnlockedUsers(std::vector* _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& secret) { ENFORCE_SYSTEM_OR_ROOT; ACQUIRE_CRYPT_LOCK; diff --git a/VoldNativeService.h b/VoldNativeService.h index bb00d35..a4fbf00 100644 --- a/VoldNativeService.h +++ b/VoldNativeService.h @@ -116,10 +116,10 @@ class VoldNativeService : public BinderService, 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& secret); binder::Status getUnlockedUsers(std::vector* _aidl_return); - binder::Status unlockCeStorage(int32_t userId, const std::string& secret); + binder::Status unlockCeStorage(int32_t userId, const std::vector& secret); binder::Status lockCeStorage(int32_t userId); binder::Status prepareUserStorage(const std::optional& uuid, int32_t userId, diff --git a/binder/android/os/IVold.aidl b/binder/android/os/IVold.aidl index dfccc00..d37697b 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);