From 42e3810e13b22a087914c3d2b32e4d900a8ecbf3 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Wed, 7 Jun 2017 10:46:12 -0700 Subject: [PATCH 01/19] Remove timout logic in waiting vold.post_fs_data_done This code should not be timing out, since it has no graceful way to recover. Bug: 62308812 Test: marlin boot Change-Id: I1284f9a34e83e6451622a702d2bee40b08877db2 --- cryptfs.cpp | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/cryptfs.cpp b/cryptfs.cpp index 6319362..484a107 100644 --- a/cryptfs.cpp +++ b/cryptfs.cpp @@ -1302,29 +1302,24 @@ int wait_and_unmount(const char *mountpoint, bool kill) return rc; } -static int prep_data_fs(void) +static void prep_data_fs(void) { - int i; - // NOTE: post_fs_data results in init calling back around to vold, so all // callers to this method must be async /* Do the prep of the /data filesystem */ property_set("vold.post_fs_data_done", "0"); property_set("vold.decrypt", "trigger_post_fs_data"); - SLOGD("Just triggered post_fs_data\n"); + SLOGD("Just triggered post_fs_data"); /* Wait a max of 50 seconds, hopefully it takes much less */ - if (!android::base::WaitForProperty("vold.post_fs_data_done", + while (!android::base::WaitForProperty("vold.post_fs_data_done", "1", - std::chrono::seconds(50))) { - /* Ugh, we failed to prep /data in time. Bail. */ - SLOGE("post_fs_data timed out!\n"); - return -1; - } else { - SLOGD("post_fs_data done\n"); - return 0; + std::chrono::seconds(15))) { + /* We timed out to prep /data in time. Continue wait. */ + SLOGE("waited 15s for vold.post_fs_data_done, still waiting..."); } + SLOGD("post_fs_data done"); } static void cryptfs_set_corrupt() @@ -1475,9 +1470,7 @@ static int cryptfs_restart_internal(int restart_main) } /* Create necessary paths on /data */ - if (prep_data_fs()) { - return -1; - } + prep_data_fs(); property_set("vold.decrypt", "trigger_load_persist_props"); /* startup service classes main and late_start */ @@ -2216,9 +2209,7 @@ int cryptfs_enable_internal(char *howarg, int crypt_type, const char *passwd, /* restart the framework. */ /* Create necessary paths on /data */ - if (prep_data_fs()) { - goto error_shutting_down; - } + prep_data_fs(); /* Ugh, shutting down the framework is not synchronous, so until it * can be fixed, this horrible hack will wait a moment for it all to From e4c93da49297f70c8f0fc11fbc5c21efeedc5e98 Mon Sep 17 00:00:00 2001 From: Paul Crowley Date: Fri, 16 Jun 2017 09:21:18 -0700 Subject: [PATCH 02/19] Abolish AutoCloseFD.h in favour of unique_fd Android has a standard way to do what AutoCloseFD.h does, so use that instead. Refactor before work on the bug. Bug: 36029169 Test: Deleted a user and checked that secdiscard logs looked good. Change-Id: I5d8bedfb3fa1f032fd2bced88b1b561e4a8c2ff4 --- AutoCloseFD.h | 50 ----------------------------------------------- MetadataCrypt.cpp | 12 +++++++----- secdiscard.cpp | 13 ++++++------ 3 files changed, 14 insertions(+), 61 deletions(-) delete mode 100644 AutoCloseFD.h diff --git a/AutoCloseFD.h b/AutoCloseFD.h deleted file mode 100644 index 9b68469..0000000 --- a/AutoCloseFD.h +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include - -#include -#include -#include -#include - -#include - -// File descriptor which is automatically closed when this object is destroyed. -// Cannot be copied, since that would cause double-closes. -class AutoCloseFD { -public: - AutoCloseFD(const char *path, int flags = O_RDONLY, int mode = 0): - fd{TEMP_FAILURE_RETRY(open(path, flags | O_CLOEXEC, mode))} {} - AutoCloseFD(const std::string &path, int flags = O_RDONLY, int mode = 0): - AutoCloseFD(path.c_str(), flags, mode) {} - ~AutoCloseFD() { - if (fd != -1) { - int preserve_errno = errno; - if (close(fd) == -1) { - PLOG(ERROR) << "close(2) failed"; - }; - errno = preserve_errno; - } - } - AutoCloseFD(const AutoCloseFD&) = delete; - AutoCloseFD& operator=(const AutoCloseFD&) = delete; - explicit operator bool() {return fd != -1;} - int get() const {return fd;} -private: - const int fd; -}; - diff --git a/MetadataCrypt.cpp b/MetadataCrypt.cpp index b707549..743e08c 100644 --- a/MetadataCrypt.cpp +++ b/MetadataCrypt.cpp @@ -29,10 +29,10 @@ #include #include +#include #include #include -#include "AutoCloseFD.h" #include "EncryptInplace.h" #include "KeyStorage.h" #include "KeyUtil.h" @@ -105,8 +105,9 @@ static std::string default_key_params(const std::string& real_blkdev, const std: } static bool get_number_of_sectors(const std::string& real_blkdev, uint64_t *nr_sec) { - AutoCloseFD dev_fd(real_blkdev, O_RDONLY); - if (!dev_fd) { + android::base::unique_fd dev_fd(TEMP_FAILURE_RETRY(open( + real_blkdev.c_str(), O_RDONLY | O_CLOEXEC, 0))); + if (dev_fd == -1) { PLOG(ERROR) << "Unable to open " << real_blkdev << " to measure size"; return false; } @@ -143,8 +144,9 @@ static struct dm_ioctl* dm_ioctl_init(char *buffer, size_t buffer_size, static bool create_crypto_blk_dev(const std::string& dm_name, uint64_t nr_sec, const std::string& target_type, const std::string& crypt_params, std::string* crypto_blkdev) { - AutoCloseFD dm_fd("/dev/device-mapper", O_RDWR); - if (!dm_fd) { + android::base::unique_fd dm_fd(TEMP_FAILURE_RETRY(open( + "/dev/device-mapper", O_RDWR | O_CLOEXEC, 0))); + if (dm_fd == -1) { PLOG(ERROR) << "Cannot open device-mapper"; return false; } diff --git a/secdiscard.cpp b/secdiscard.cpp index fe51990..a335ab6 100644 --- a/secdiscard.cpp +++ b/secdiscard.cpp @@ -29,8 +29,7 @@ #include #include - -#include +#include namespace { @@ -107,8 +106,9 @@ bool secdiscard_path(const std::string &path) { if (block_device.empty()) { return false; } - AutoCloseFD fs_fd(block_device, O_RDWR | O_LARGEFILE); - if (!fs_fd) { + android::base::unique_fd fs_fd(TEMP_FAILURE_RETRY(open( + block_device.c_str(), O_RDWR | O_LARGEFILE | O_CLOEXEC, 0))); + if (fs_fd == -1) { PLOG(ERROR) << "Failed to open device " << block_device; return false; } @@ -128,8 +128,9 @@ bool secdiscard_path(const std::string &path) { // Read the file's FIEMAP std::unique_ptr path_fiemap(const std::string &path, uint32_t extent_count) { - AutoCloseFD fd(path); - if (!fd) { + android::base::unique_fd fd(TEMP_FAILURE_RETRY(open( + path.c_str(), O_RDONLY | O_CLOEXEC, 0))); + if (fd == -1) { if (errno == ENOENT) { PLOG(DEBUG) << "Unable to open " << path; } else { From 03f89d3d9540c7f5fbc609d5bbcd99e444a06503 Mon Sep 17 00:00:00 2001 From: Paul Crowley Date: Fri, 16 Jun 2017 09:37:31 -0700 Subject: [PATCH 03/19] Move functions useful for crypto test into their own file More refactoring in advance of work on bug. Bug: 36029169 Test: compiles. Change-Id: Ic4cdd4761e4c2b11a3ddca5c3bbc4d5e42fac9d4 --- Android.mk | 5 +- FileDeviceUtils.cpp | 115 ++++++++++++++++++++++++++++++++++++++++++++ FileDeviceUtils.h | 35 ++++++++++++++ secdiscard.cpp | 78 ++---------------------------- 4 files changed, 158 insertions(+), 75 deletions(-) create mode 100644 FileDeviceUtils.cpp create mode 100644 FileDeviceUtils.h diff --git a/Android.mk b/Android.mk index e92955f..4971ec7 100644 --- a/Android.mk +++ b/Android.mk @@ -158,7 +158,10 @@ LOCAL_CLANG := true LOCAL_TIDY := true LOCAL_TIDY_FLAGS := $(common_local_tidy_flags) LOCAL_TIDY_CHECKS := $(common_local_tidy_checks) -LOCAL_SRC_FILES:= secdiscard.cpp +LOCAL_SRC_FILES:= \ + FileDeviceUtils.cpp \ + secdiscard.cpp \ + LOCAL_MODULE:= secdiscard LOCAL_SHARED_LIBRARIES := libbase LOCAL_CFLAGS := $(vold_cflags) diff --git a/FileDeviceUtils.cpp b/FileDeviceUtils.cpp new file mode 100644 index 0000000..bc9f4bd --- /dev/null +++ b/FileDeviceUtils.cpp @@ -0,0 +1,115 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "FileDeviceUtils.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +namespace { + +std::unique_ptr alloc_fiemap(uint32_t extent_count); + +} + +namespace android { +namespace vold { + +// Given a file path, look for the corresponding block device in /proc/mount +std::string BlockDeviceForPath(const std::string &path) +{ + std::unique_ptr mnts(setmntent("/proc/mounts", "re"), endmntent); + if (!mnts) { + PLOG(ERROR) << "Unable to open /proc/mounts"; + return ""; + } + std::string result; + size_t best_length = 0; + struct mntent *mnt; // getmntent returns a thread local, so it's safe. + while ((mnt = getmntent(mnts.get())) != nullptr) { + auto l = strlen(mnt->mnt_dir); + if (l > best_length && + path.size() > l && + path[l] == '/' && + path.compare(0, l, mnt->mnt_dir) == 0) { + result = mnt->mnt_fsname; + best_length = l; + } + } + if (result.empty()) { + LOG(ERROR) <<"Didn't find a mountpoint to match path " << path; + return ""; + } + LOG(DEBUG) << "For path " << path << " block device is " << result; + return result; +} + +std::unique_ptr PathFiemap(const std::string &path, uint32_t extent_count) +{ + android::base::unique_fd fd(TEMP_FAILURE_RETRY(open( + path.c_str(), O_RDONLY | O_CLOEXEC, 0))); + if (fd == -1) { + if (errno == ENOENT) { + PLOG(DEBUG) << "Unable to open " << path; + } else { + PLOG(ERROR) << "Unable to open " << path; + } + return nullptr; + } + auto fiemap = alloc_fiemap(extent_count); + if (ioctl(fd.get(), FS_IOC_FIEMAP, fiemap.get()) != 0) { + PLOG(ERROR) << "Unable to FIEMAP " << path; + return nullptr; + } + auto mapped = fiemap->fm_mapped_extents; + if (mapped < 1 || mapped > extent_count) { + LOG(ERROR) << "Extent count not in bounds 1 <= " << mapped << " <= " << extent_count + << " in " << path; + return nullptr; + } + return fiemap; +} + +} // namespace vold +} // namespace android + +namespace { + +std::unique_ptr alloc_fiemap(uint32_t extent_count) +{ + size_t allocsize = offsetof(struct fiemap, fm_extents[extent_count]); + std::unique_ptr res(new (::operator new (allocsize)) struct fiemap); + memset(res.get(), 0, allocsize); + res->fm_start = 0; + res->fm_length = UINT64_MAX; + res->fm_flags = 0; + res->fm_extent_count = extent_count; + res->fm_mapped_extents = 0; + return res; +} + +} diff --git a/FileDeviceUtils.h b/FileDeviceUtils.h new file mode 100644 index 0000000..4c1d49a --- /dev/null +++ b/FileDeviceUtils.h @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_VOLD_FILEDEVICEUTILS_H +#define ANDROID_VOLD_FILEDEVICEUTILS_H + +#include +#include + +namespace android { +namespace vold { + +// Given a file path, look for the corresponding block device in /proc/mount +std::string BlockDeviceForPath(const std::string &path); + +// Read the file's FIEMAP +std::unique_ptr PathFiemap(const std::string &path, uint32_t extent_count); + +} // namespace vold +} // namespace android + +#endif diff --git a/secdiscard.cpp b/secdiscard.cpp index a335ab6..f9532ea 100644 --- a/secdiscard.cpp +++ b/secdiscard.cpp @@ -31,6 +31,8 @@ #include #include +#include "FileDeviceUtils.h" + namespace { struct Options { @@ -43,10 +45,7 @@ constexpr uint32_t max_extents = 32; bool read_command_line(int argc, const char * const argv[], Options &options); void usage(const char *progname); bool secdiscard_path(const std::string &path); -std::unique_ptr path_fiemap(const std::string &path, uint32_t extent_count); bool check_fiemap(const struct fiemap &fiemap, const std::string &path); -std::unique_ptr alloc_fiemap(uint32_t extent_count); -std::string block_device_for_path(const std::string &path); bool overwrite_with_zeros(int fd, off64_t start, off64_t length); } @@ -98,11 +97,11 @@ void usage(const char *progname) { // BLKSECDISCARD all content in "path", if it's small enough. bool secdiscard_path(const std::string &path) { - auto fiemap = path_fiemap(path, max_extents); + auto fiemap = android::vold::PathFiemap(path, max_extents); if (!fiemap || !check_fiemap(*fiemap, path)) { return false; } - auto block_device = block_device_for_path(path); + auto block_device = android::vold::BlockDeviceForPath(path); if (block_device.empty()) { return false; } @@ -125,33 +124,6 @@ bool secdiscard_path(const std::string &path) { return true; } -// Read the file's FIEMAP -std::unique_ptr path_fiemap(const std::string &path, uint32_t extent_count) -{ - android::base::unique_fd fd(TEMP_FAILURE_RETRY(open( - path.c_str(), O_RDONLY | O_CLOEXEC, 0))); - if (fd == -1) { - if (errno == ENOENT) { - PLOG(DEBUG) << "Unable to open " << path; - } else { - PLOG(ERROR) << "Unable to open " << path; - } - return nullptr; - } - auto fiemap = alloc_fiemap(extent_count); - if (ioctl(fd.get(), FS_IOC_FIEMAP, fiemap.get()) != 0) { - PLOG(ERROR) << "Unable to FIEMAP " << path; - return nullptr; - } - auto mapped = fiemap->fm_mapped_extents; - if (mapped < 1 || mapped > extent_count) { - LOG(ERROR) << "Extent count not in bounds 1 <= " << mapped << " <= " << extent_count - << " in " << path; - return nullptr; - } - return fiemap; -} - // Ensure that the FIEMAP covers the file and is OK to discard bool check_fiemap(const struct fiemap &fiemap, const std::string &path) { auto mapped = fiemap.fm_mapped_extents; @@ -169,48 +141,6 @@ bool check_fiemap(const struct fiemap &fiemap, const std::string &path) { return true; } -std::unique_ptr alloc_fiemap(uint32_t extent_count) -{ - size_t allocsize = offsetof(struct fiemap, fm_extents[extent_count]); - std::unique_ptr res(new (::operator new (allocsize)) struct fiemap); - memset(res.get(), 0, allocsize); - res->fm_start = 0; - res->fm_length = UINT64_MAX; - res->fm_flags = 0; - res->fm_extent_count = extent_count; - res->fm_mapped_extents = 0; - return res; -} - -// Given a file path, look for the corresponding block device in /proc/mount -std::string block_device_for_path(const std::string &path) -{ - std::unique_ptr mnts(setmntent("/proc/mounts", "re"), endmntent); - if (!mnts) { - PLOG(ERROR) << "Unable to open /proc/mounts"; - return ""; - } - std::string result; - size_t best_length = 0; - struct mntent *mnt; // getmntent returns a thread local, so it's safe. - while ((mnt = getmntent(mnts.get())) != nullptr) { - auto l = strlen(mnt->mnt_dir); - if (l > best_length && - path.size() > l && - path[l] == '/' && - path.compare(0, l, mnt->mnt_dir) == 0) { - result = mnt->mnt_fsname; - best_length = l; - } - } - if (result.empty()) { - LOG(ERROR) <<"Didn't find a mountpoint to match path " << path; - return ""; - } - LOG(DEBUG) << "For path " << path << " block device is " << result; - return result; -} - bool overwrite_with_zeros(int fd, off64_t start, off64_t length) { if (lseek64(fd, start, SEEK_SET) != start) { PLOG(ERROR) << "Seek failed for zero overwrite"; From ab48bc9dbdeed89f270c0482023a5bc848556c99 Mon Sep 17 00:00:00 2001 From: Jaegeuk Kim Date: Mon, 5 Jun 2017 10:22:04 -0700 Subject: [PATCH 04/19] cryptfs: call format_f2fs correctly with proper flags Change-Id: Ia493e6f758ff5dd5dd41479193ab237d4306d464 Signed-off-by: Jaegeuk Kim --- cryptfs.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/cryptfs.cpp b/cryptfs.cpp index 484a107..43c8177 100644 --- a/cryptfs.cpp +++ b/cryptfs.cpp @@ -1938,15 +1938,17 @@ static int cryptfs_enable_wipe(char *crypto_blkdev, off64_t size, int type) SLOGI("Making empty filesystem with command %s %s %s %s %s %s\n", args[0], args[1], args[2], args[3], args[4], args[5]); } else if (type == F2FS_FS) { - args[0] = "/system/bin/mkfs.f2fs"; + args[0] = "/system/bin/make_f2fs"; args[1] = "-t"; args[2] = "-d1"; - args[3] = crypto_blkdev; + args[3] = "-f"; + args[4] = "-O encrypt"; + args[5] = crypto_blkdev; snprintf(size_str, sizeof(size_str), "%" PRId64, size); - args[4] = size_str; - num_args = 5; - SLOGI("Making empty filesystem with command %s %s %s %s %s\n", - args[0], args[1], args[2], args[3], args[4]); + args[6] = size_str; + num_args = 7; + SLOGI("Making empty filesystem with command %s %s %s %s %s %s %s\n", + args[0], args[1], args[2], args[3], args[4], args[5], args[6]); } else { SLOGE("cryptfs_enable_wipe(): unknown filesystem type %d\n", type); return -1; From cd8bfe3d7f0186192d27a49593fd7b97c8a3213d Mon Sep 17 00:00:00 2001 From: Paul Crowley Date: Mon, 19 Jun 2017 16:05:55 -0700 Subject: [PATCH 05/19] Label keys with all the possible FBE prefixes that might apply We don't know which FS and kernel version is going to want these keys, so put them in the kernel three times with all three possible prefixes. Test: Marlin set up before this change successfully boots after it. Change-Id: I6ccfe0894551ba068de9bf5e23fe4fd1e10e36b1 --- KeyUtil.cpp | 58 ++++++++++++++++++++++++++++++++--------------------- KeyUtil.h | 1 - 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/KeyUtil.cpp b/KeyUtil.cpp index 865cc11..a75dfbb 100644 --- a/KeyUtil.cpp +++ b/KeyUtil.cpp @@ -73,9 +73,16 @@ static bool fillKey(const std::string& key, ext4_encryption_key* ext4_key) { return true; } -std::string keyname(const std::string& raw_ref) { +static char const* const NAME_PREFIXES[] = { + "ext4", + "f2fs", + "fscrypt", + nullptr +}; + +static std::string keyname(const std::string& prefix, const std::string& raw_ref) { std::ostringstream o; - o << "ext4:"; + o << prefix << ":"; for (auto i : raw_ref) { o << std::hex << std::setw(2) << std::setfill('0') << (int)i; } @@ -98,37 +105,42 @@ bool installKey(const std::string& key, std::string* raw_ref) { ext4_encryption_key ext4_key; if (!fillKey(key, &ext4_key)) return false; *raw_ref = generateKeyRef(ext4_key.raw, ext4_key.size); - auto ref = keyname(*raw_ref); key_serial_t device_keyring; if (!e4cryptKeyring(&device_keyring)) return false; - key_serial_t key_id = - add_key("logon", ref.c_str(), (void*)&ext4_key, sizeof(ext4_key), device_keyring); - if (key_id == -1) { - PLOG(ERROR) << "Failed to insert key into keyring " << device_keyring; - return false; + for (char const* const* name_prefix = NAME_PREFIXES; *name_prefix != nullptr; name_prefix++) { + auto ref = keyname(*name_prefix, *raw_ref); + key_serial_t key_id = + add_key("logon", ref.c_str(), (void*)&ext4_key, sizeof(ext4_key), device_keyring); + if (key_id == -1) { + PLOG(ERROR) << "Failed to insert key into keyring " << device_keyring; + return false; + } + LOG(DEBUG) << "Added key " << key_id << " (" << ref << ") to keyring " << device_keyring + << " in process " << getpid(); } - LOG(DEBUG) << "Added key " << key_id << " (" << ref << ") to keyring " << device_keyring - << " in process " << getpid(); - return true; } bool evictKey(const std::string& raw_ref) { - auto ref = keyname(raw_ref); key_serial_t device_keyring; if (!e4cryptKeyring(&device_keyring)) return false; - auto key_serial = keyctl_search(device_keyring, "logon", ref.c_str(), 0); - - // Unlink the key from the keyring. Prefer unlinking to revoking or - // invalidating, since unlinking is actually no less secure currently, and - // it avoids bugs in certain kernel versions where the keyring key is - // referenced from places it shouldn't be. - if (keyctl_unlink(key_serial, device_keyring) != 0) { - PLOG(ERROR) << "Failed to unlink key with serial " << key_serial << " ref " << ref; - return false; + bool success = true; + for (char const* const* name_prefix = NAME_PREFIXES; *name_prefix != nullptr; name_prefix++) { + auto ref = keyname(*name_prefix, raw_ref); + auto key_serial = keyctl_search(device_keyring, "logon", ref.c_str(), 0); + + // Unlink the key from the keyring. Prefer unlinking to revoking or + // invalidating, since unlinking is actually no less secure currently, and + // it avoids bugs in certain kernel versions where the keyring key is + // referenced from places it shouldn't be. + if (keyctl_unlink(key_serial, device_keyring) != 0) { + PLOG(ERROR) << "Failed to unlink key with serial " << key_serial << " ref " << ref; + success = false; + } else { + LOG(DEBUG) << "Unlinked key with serial " << key_serial << " ref " << ref; + } } - LOG(DEBUG) << "Unlinked key with serial " << key_serial << " ref " << ref; - return true; + return success; } bool retrieveAndInstallKey(bool create_if_absent, const std::string& key_path, diff --git a/KeyUtil.h b/KeyUtil.h index f8fb634..d4c97b9 100644 --- a/KeyUtil.h +++ b/KeyUtil.h @@ -36,7 +36,6 @@ struct ext4_encryption_key { uint32_t size; }; -std::string keyname(const std::string& raw_ref); bool randomKey(std::string* key); bool installKey(const std::string& key, std::string* raw_ref); bool evictKey(const std::string& raw_ref); From 46bb69f49a8630cc97e84976d0327495b7dd5708 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 21 Jun 2017 13:52:23 -0600 Subject: [PATCH 06/19] Progress towards FBE and adoptable storage. Offer to adopt storage devices on FBE devices, but keep it guarded behind a system property for now, since we still need to work out key storage details. When migrating shared storage, leave user-specific /data/media directories in place, since they already have the needed crypto policies defined. Enable journaling, quotas, and encrypt options when formatting newly adopted devices. installd already gracefully handles older partitions without quota enabled. Test: cts-tradefed run commandAndExit cts-dev --abi armeabi-v7a -m CtsAppSecurityHostTestCases -t android.appsecurity.cts.AdoptableHostTest Bug: 62290006, 36757864, 29117062, 37395736 Bug: 29923055, 25861755, 30230655, 37436961 Change-Id: Ibbeb6ec9db2394a279bbac221a2b20711d65494e --- Disk.cpp | 13 ++++++++++--- MoveTask.cpp | 13 +++++++++---- Utils.cpp | 11 +++++++++++ Utils.h | 1 + fs/Ext4.cpp | 8 +++++++- 5 files changed, 38 insertions(+), 8 deletions(-) diff --git a/Disk.cpp b/Disk.cpp index b424aba..9c22400 100644 --- a/Disk.cpp +++ b/Disk.cpp @@ -24,6 +24,7 @@ #include "Ext4Crypt.h" #include +#include #include #include #include @@ -446,7 +447,8 @@ status_t Disk::partitionPrivate() { status_t Disk::partitionMixed(int8_t ratio) { int res; - if (e4crypt_is_native()) { + if (e4crypt_is_native() + && !android::base::GetBoolProperty("persist.sys.adoptable_fbe", false)) { LOG(ERROR) << "Private volumes not yet supported on FBE devices"; return -EINVAL; } @@ -469,9 +471,14 @@ status_t Disk::partitionMixed(int8_t ratio) { // We've had some success above, so generate both the private partition // GUID and encryption key and persist them. std::string partGuidRaw; + if (GenerateRandomUuid(partGuidRaw) != OK) { + LOG(ERROR) << "Failed to generate GUID"; + return -EIO; + } + std::string keyRaw; - if (ReadRandomBytes(16, partGuidRaw) || ReadRandomBytes(16, keyRaw)) { - LOG(ERROR) << "Failed to generate GUID or key"; + if (ReadRandomBytes(16, keyRaw) != OK) { + LOG(ERROR) << "Failed to generate key"; return -EIO; } diff --git a/MoveTask.cpp b/MoveTask.cpp index ea64a1c..c565752 100644 --- a/MoveTask.cpp +++ b/MoveTask.cpp @@ -62,7 +62,8 @@ static void notifyProgress(int progress) { StringPrintf("%d", progress).c_str(), false); } -static status_t pushBackContents(const std::string& path, std::vector& cmd) { +static status_t pushBackContents(const std::string& path, std::vector& cmd, + bool addWildcard) { DIR* dir = opendir(path.c_str()); if (dir == NULL) { return -1; @@ -73,7 +74,11 @@ static status_t pushBackContents(const std::string& path, std::vectord_name, ".")) || (!strcmp(ent->d_name, ".."))) { continue; } - cmd.push_back(StringPrintf("%s/%s", path.c_str(), ent->d_name)); + if (addWildcard) { + cmd.push_back(StringPrintf("%s/%s/*", path.c_str(), ent->d_name)); + } else { + cmd.push_back(StringPrintf("%s/%s", path.c_str(), ent->d_name)); + } found = true; } closedir(dir); @@ -90,7 +95,7 @@ static status_t execRm(const std::string& path, int startProgress, int stepProgr cmd.push_back(kRmPath); cmd.push_back("-f"); /* force: remove without confirmation, no error if it doesn't exist */ cmd.push_back("-R"); /* recursive: remove directory contents */ - if (pushBackContents(path, cmd) != OK) { + if (pushBackContents(path, cmd, true) != OK) { LOG(WARNING) << "No contents in " << path; return OK; } @@ -140,7 +145,7 @@ static status_t execCp(const std::string& fromPath, const std::string& toPath, cmd.push_back("-R"); /* recurse into subdirectories (DEST must be a directory) */ cmd.push_back("-P"); /* Do not follow symlinks [default] */ cmd.push_back("-d"); /* don't dereference symlinks */ - if (pushBackContents(fromPath, cmd) != OK) { + if (pushBackContents(fromPath, cmd, false) != OK) { LOG(WARNING) << "No contents in " << fromPath; return OK; } diff --git a/Utils.cpp b/Utils.cpp index 529cfb2..9699777 100644 --- a/Utils.cpp +++ b/Utils.cpp @@ -372,6 +372,17 @@ status_t ReadRandomBytes(size_t bytes, std::string& out) { } } +status_t GenerateRandomUuid(std::string& out) { + status_t res = ReadRandomBytes(16, out); + if (res == OK) { + out[6] &= 0x0f; /* clear version */ + out[6] |= 0x40; /* set to version 4 */ + out[8] &= 0x3f; /* clear variant */ + out[8] |= 0x80; /* set to IETF variant */ + } + return res; +} + status_t HexToStr(const std::string& hex, std::string& str) { str.clear(); bool even = true; diff --git a/Utils.h b/Utils.h index 813ffac..7272fe1 100644 --- a/Utils.h +++ b/Utils.h @@ -78,6 +78,7 @@ status_t ForkExecvp(const std::vector& args, pid_t ForkExecvpAsync(const std::vector& args); status_t ReadRandomBytes(size_t bytes, std::string& out); +status_t GenerateRandomUuid(std::string& out); /* Converts hex string to raw bytes, ignoring [ :-] */ status_t HexToStr(const std::string& hex, std::string& str); diff --git a/fs/Ext4.cpp b/fs/Ext4.cpp index 0670bb5..041ce90 100644 --- a/fs/Ext4.cpp +++ b/fs/Ext4.cpp @@ -45,6 +45,7 @@ #include #include "Ext4.h" +#include "Ext4Crypt.h" #include "Utils.h" #include "VoldUtil.h" @@ -180,8 +181,13 @@ status_t Format(const std::string& source, unsigned long numSectors, cmd.push_back("-M"); cmd.push_back(target); + std::string options("has_journal,quota"); + if (e4crypt_is_native()) { + options += ",encrypt"; + } + cmd.push_back("-O"); - cmd.push_back("^has_journal"); + cmd.push_back(options); cmd.push_back(source); From d794526962c385af307597f27d26aeb43703e6a1 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Mon, 26 Jun 2017 16:09:11 -0600 Subject: [PATCH 07/19] Fully switch to mke2fs; set policies everywhere. Older make_ext4fs doesn't support enabling quotas, so switch everyone over to using mke2fs for adoptable storage. Remove UUID check so that we start setting ext4-crypto policies on adoptable storage devices; a future change will handle the actual key management. Bug: 30230655, 36757864 Test: cts-tradefed run commandAndExit cts-dev --abi armeabi-v7a -m CtsAppSecurityHostTestCases -t android.appsecurity.cts.AdoptableHostTest Change-Id: I021f85b1be8431044c239521c37be96534682746 --- Android.mk | 2 ++ Ext4Crypt.cpp | 6 ++---- fs/Ext4.cpp | 21 +-------------------- 3 files changed, 5 insertions(+), 24 deletions(-) diff --git a/Android.mk b/Android.mk index 4971ec7..9dba651 100644 --- a/Android.mk +++ b/Android.mk @@ -88,6 +88,8 @@ ifeq ($(TARGET_USERIMAGES_USE_EXT4), true) vold_cflags += -DTARGET_USES_MKE2FS required_modules += mke2fs else + # Adoptable storage has fully moved to mke2fs, so we need both tools + required_modules += mke2fs required_modules += make_ext4fs endif endif diff --git a/Ext4Crypt.cpp b/Ext4Crypt.cpp index c3e0cc3..13cff0d 100644 --- a/Ext4Crypt.cpp +++ b/Ext4Crypt.cpp @@ -599,8 +599,7 @@ bool e4crypt_prepare_user_storage(const char* volume_uuid, userid_t user_id, int if (!prepare_dir(misc_de_path, 01771, AID_SYSTEM, AID_MISC)) return false; if (!prepare_dir(user_de_path, 0771, AID_SYSTEM, AID_SYSTEM)) return false; - // For now, FBE is only supported on internal storage - if (e4crypt_is_native() && volume_uuid == nullptr) { + if (e4crypt_is_native()) { std::string de_raw_ref; if (!lookup_key_ref(s_de_key_raw_refs, user_id, &de_raw_ref)) return false; if (!ensure_policy(de_raw_ref, system_de_path)) return false; @@ -621,8 +620,7 @@ bool e4crypt_prepare_user_storage(const char* volume_uuid, userid_t user_id, int if (!prepare_dir(media_ce_path, 0770, AID_MEDIA_RW, AID_MEDIA_RW)) return false; if (!prepare_dir(user_ce_path, 0771, AID_SYSTEM, AID_SYSTEM)) return false; - // For now, FBE is only supported on internal storage - if (e4crypt_is_native() && volume_uuid == nullptr) { + if (e4crypt_is_native()) { std::string ce_raw_ref; if (!lookup_key_ref(s_ce_key_raw_refs, user_id, &ce_raw_ref)) return false; if (!ensure_policy(ce_raw_ref, system_ce_path)) return false; diff --git a/fs/Ext4.cpp b/fs/Ext4.cpp index 041ce90..adb8f2e 100644 --- a/fs/Ext4.cpp +++ b/fs/Ext4.cpp @@ -56,11 +56,7 @@ namespace vold { namespace ext4 { static const char* kResizefsPath = "/system/bin/resize2fs"; -#ifdef TARGET_USES_MKE2FS static const char* kMkfsPath = "/system/bin/mke2fs"; -#else -static const char* kMkfsPath = "/system/bin/make_ext4fs"; -#endif static const char* kFsckPath = "/system/bin/e2fsck"; bool IsSupported() { @@ -171,7 +167,6 @@ status_t Format(const std::string& source, unsigned long numSectors, std::vector cmd; cmd.push_back(kMkfsPath); -#ifdef TARGET_USES_MKE2FS cmd.push_back("-b"); cmd.push_back("4096"); @@ -191,24 +186,10 @@ status_t Format(const std::string& source, unsigned long numSectors, cmd.push_back(source); - if (numSectors) - cmd.push_back(StringPrintf("%lu", numSectors * (4096 / 512))); -#else - cmd.push_back("-J"); - - cmd.push_back("-a"); - cmd.push_back(target); - if (numSectors) { - cmd.push_back("-l"); - cmd.push_back(StringPrintf("%lu", numSectors * 512)); + cmd.push_back(StringPrintf("%lu", numSectors * (4096 / 512))); } - // Always generate a real UUID - cmd.push_back("-u"); - cmd.push_back(source); -#endif - return ForkExecvp(cmd); } From 1d79d1014e481c92c3f802bbaf174409d191571f Mon Sep 17 00:00:00 2001 From: Daniel Rosenberg Date: Tue, 11 Jul 2017 17:59:55 -0700 Subject: [PATCH 08/19] Check if sdcard daemon exited. If the system is using sdcardfs, the sdcard daemon exits after mounting. If it's using FUSE, the sdcard daemon runs until we go to unmount. Bug: 37638548 Test: Run "adb shell ps | grep -w Z" with sdcardfs enabled. The sdcard daemon should not be listed. Run again with sdcardfs disabled. The daemon should be running, and vold should not be stuck waiting on it. Change-Id: I930d22b35194ec99e7a6a4a022a04d36f4f39a34 --- EmulatedVolume.cpp | 2 ++ PublicVolume.cpp | 2 ++ 2 files changed, 4 insertions(+) diff --git a/EmulatedVolume.cpp b/EmulatedVolume.cpp index 44ad22a..df91904 100644 --- a/EmulatedVolume.cpp +++ b/EmulatedVolume.cpp @@ -103,6 +103,8 @@ status_t EmulatedVolume::doMount() { LOG(VERBOSE) << "Waiting for FUSE to spin up..."; usleep(50000); // 50ms } + /* sdcardfs will have exited already. FUSE will still be running */ + TEMP_FAILURE_RETRY(waitpid(mFusePid, nullptr, WNOHANG)); return OK; } diff --git a/PublicVolume.cpp b/PublicVolume.cpp index 119d92c..f976c4a 100644 --- a/PublicVolume.cpp +++ b/PublicVolume.cpp @@ -190,6 +190,8 @@ status_t PublicVolume::doMount() { LOG(VERBOSE) << "Waiting for FUSE to spin up..."; usleep(50000); // 50ms } + /* sdcardfs will have exited already. FUSE will still be running */ + TEMP_FAILURE_RETRY(waitpid(mFusePid, nullptr, WNOHANG)); return OK; } From 95a92f9203ca68ab446a7cffd23450f04930fe38 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Mon, 17 Jul 2017 13:57:18 -0600 Subject: [PATCH 09/19] Only enable quotas when supported by device. Otherwise we might end up creating ext4 partitions that the device can't mount. Bug: 63763609 Test: builds, boots Exempt-From-Owner-Approval: Bug 63673347 Change-Id: I5f6cf73f23a55bc0dea9480523f19049313c3dd1 --- fs/Ext4.cpp | 6 +++++- main.cpp | 13 ++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/fs/Ext4.cpp b/fs/Ext4.cpp index adb8f2e..0cf4f9e 100644 --- a/fs/Ext4.cpp +++ b/fs/Ext4.cpp @@ -38,6 +38,7 @@ #define LOG_TAG "Vold" #include +#include #include #include #include @@ -176,7 +177,10 @@ status_t Format(const std::string& source, unsigned long numSectors, cmd.push_back("-M"); cmd.push_back(target); - std::string options("has_journal,quota"); + std::string options("has_journal"); + if (android::base::GetBoolProperty("vold.has_quota", false)) { + options += ",quota"; + } if (e4crypt_is_native()) { options += ",encrypt"; } diff --git a/main.cpp b/main.cpp index 4657377..30c839e 100644 --- a/main.cpp +++ b/main.cpp @@ -39,7 +39,7 @@ #include #include -static int process_config(VolumeManager *vm, bool* has_adoptable); +static int process_config(VolumeManager *vm, bool* has_adoptable, bool* has_quota); static void coldboot(const char *path); static void parse_args(int argc, char** argv); @@ -107,8 +107,9 @@ int main(int argc, char** argv) { } bool has_adoptable; + bool has_quota; - if (process_config(vm, &has_adoptable)) { + if (process_config(vm, &has_adoptable, &has_quota)) { PLOG(ERROR) << "Error reading configuration... continuing anyways"; } @@ -133,6 +134,7 @@ int main(int argc, char** argv) { // This call should go after listeners are started to avoid // a deadlock between vold and init (see b/34278978 for details) property_set("vold.has_adoptable", has_adoptable ? "1" : "0"); + property_set("vold.has_quota", has_quota ? "1" : "0"); // Do coldboot here so it won't block booting, // also the cold boot is needed in case we have flash drive @@ -214,7 +216,7 @@ static void coldboot(const char *path) { } } -static int process_config(VolumeManager *vm, bool* has_adoptable) { +static int process_config(VolumeManager *vm, bool* has_adoptable, bool* has_quota) { fstab = fs_mgr_read_fstab_default(); if (!fstab) { PLOG(ERROR) << "Failed to open default fstab"; @@ -223,7 +225,12 @@ static int process_config(VolumeManager *vm, bool* has_adoptable) { /* Loop through entries looking for ones that vold manages */ *has_adoptable = false; + *has_quota = false; for (int i = 0; i < fstab->num_entries; i++) { + if (fs_mgr_is_quota(&fstab->recs[i])) { + *has_quota = true; + } + if (fs_mgr_is_voldmanaged(&fstab->recs[i])) { if (fs_mgr_is_nonremovable(&fstab->recs[i])) { LOG(WARNING) << "nonremovable no longer supported; ignoring volume"; From b350ed02d5fc1f5d7e517dc7e7472543601fe18c Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Thu, 27 Jul 2017 17:34:57 +0100 Subject: [PATCH 10/19] Drop inode and page caches after evicting CE key. Bug: 63257991 Test: Turning work profile off and attempting to read profile files. Change-Id: I36f8ae9a8894f88950f50aed4a06645fab7e998b --- Ext4Crypt.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Ext4Crypt.cpp b/Ext4Crypt.cpp index 13cff0d..f9d4cf8 100644 --- a/Ext4Crypt.cpp +++ b/Ext4Crypt.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -54,6 +55,7 @@ #include using android::base::StringPrintf; +using android::base::WriteStringToFile; using android::vold::kEmptyAuthentication; // NOTE: keep in sync with StorageManager @@ -399,6 +401,15 @@ bool e4crypt_vold_create_user_key(userid_t user_id, int serial, bool ephemeral) return true; } +static void drop_caches() { + // Clean any dirty pages (otherwise they won't be dropped). + sync(); + // Drop inode and page caches. + if (!WriteStringToFile("3", "/proc/sys/vm/drop_caches")) { + PLOG(ERROR) << "Failed to drop caches during key eviction"; + } +} + static bool evict_ce_key(userid_t user_id) { s_ce_keys.erase(user_id); bool success = true; @@ -406,6 +417,7 @@ static bool evict_ce_key(userid_t user_id) { // If we haven't loaded the CE key, no need to evict it. if (lookup_key_ref(s_ce_key_raw_refs, user_id, &raw_ref)) { success &= android::vold::evictKey(raw_ref); + drop_caches(); } s_ce_key_raw_refs.erase(user_id); return success; From 4cc6baf616bee2c8ce6d8935c16641a66112fa95 Mon Sep 17 00:00:00 2001 From: Ravisankar Reddy Date: Mon, 3 Jul 2017 11:25:33 +0900 Subject: [PATCH 11/19] Add noatime to vfat and exfat testNoAtime is new cts testcase, which verifies all writable block filesystems are mounted "noatime" toavoid unnecessary flash churn. So add noatime for vfat. Bug: 64137815 Test: run cts -m m CtsOsTestCases -t android.os.cts.EnvironmentTest#testNoAtime Change-Id: I4f42b54ed0d66e09964351da26d0d3bf38d573d6 --- fs/Vfat.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/Vfat.cpp b/fs/Vfat.cpp index 1803c4b..dc1fe33 100644 --- a/fs/Vfat.cpp +++ b/fs/Vfat.cpp @@ -133,7 +133,7 @@ status_t Mount(const std::string& source, const std::string& target, bool ro, const char* c_source = source.c_str(); const char* c_target = target.c_str(); - flags = MS_NODEV | MS_NOSUID | MS_DIRSYNC; + flags = MS_NODEV | MS_NOSUID | MS_DIRSYNC | MS_NOATIME; flags |= (executable ? 0 : MS_NOEXEC); flags |= (ro ? MS_RDONLY : 0); From e2e2d308df2da26838de32852318bc2cb690d052 Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Tue, 1 Aug 2017 17:15:53 +0100 Subject: [PATCH 12/19] Zero memory used for encryuption keys. std::vector with custom zeroing allocator is used instead of std::string for data that can contain encryption keys. Bug: 64201177 Test: manually created a managed profile, changed it's credentials Test: manually upgraded a phone with profile from O to MR1. Change-Id: Ic31877049f69eba9f8ea64fd99acaaca5a01d3dd --- Android.mk | 1 + Ext4Crypt.cpp | 17 +++++++------ KeyBuffer.cpp | 37 ++++++++++++++++++++++++++++ KeyBuffer.h | 63 +++++++++++++++++++++++++++++++++++++++++++++++ KeyStorage.cpp | 16 ++++++------ KeyStorage.h | 8 +++--- KeyUtil.cpp | 32 ++++++++++++++++++------ KeyUtil.h | 23 +++++------------ Keymaster.cpp | 25 +++++++++---------- Keymaster.h | 15 ++++++++++- MetadataCrypt.cpp | 31 +++++++++++++---------- Utils.cpp | 19 +++++++++++--- Utils.h | 5 ++++ 13 files changed, 217 insertions(+), 75 deletions(-) create mode 100644 KeyBuffer.cpp create mode 100644 KeyBuffer.h diff --git a/Android.mk b/Android.mk index 9dba651..d0b199d 100644 --- a/Android.mk +++ b/Android.mk @@ -27,6 +27,7 @@ common_src_files := \ MoveTask.cpp \ Benchmark.cpp \ TrimTask.cpp \ + KeyBuffer.cpp \ Keymaster.cpp \ KeyStorage.cpp \ KeyUtil.cpp \ diff --git a/Ext4Crypt.cpp b/Ext4Crypt.cpp index f9d4cf8..dc2e42a 100644 --- a/Ext4Crypt.cpp +++ b/Ext4Crypt.cpp @@ -57,6 +57,7 @@ using android::base::StringPrintf; using android::base::WriteStringToFile; using android::vold::kEmptyAuthentication; +using android::vold::KeyBuffer; // NOTE: keep in sync with StorageManager static constexpr int FLAG_STORAGE_DE = 1 << 0; @@ -80,7 +81,7 @@ std::set s_ephemeral_users; std::map s_de_key_raw_refs; std::map s_ce_key_raw_refs; // TODO abolish this map, per b/26948053 -std::map s_ce_keys; +std::map s_ce_keys; } @@ -170,7 +171,7 @@ static void fixate_user_ce_key(const std::string& directory_path, const std::str static bool read_and_fixate_user_ce_key(userid_t user_id, const android::vold::KeyAuthentication& auth, - std::string *ce_key) { + KeyBuffer *ce_key) { auto const directory_path = get_ce_key_directory_path(user_id); auto const paths = get_ce_key_paths(directory_path); for (auto const ce_key_path: paths) { @@ -188,11 +189,11 @@ static bool read_and_fixate_user_ce_key(userid_t user_id, static bool read_and_install_user_ce_key(userid_t user_id, const android::vold::KeyAuthentication& auth) { if (s_ce_key_raw_refs.count(user_id) != 0) return true; - std::string ce_key; + KeyBuffer ce_key; if (!read_and_fixate_user_ce_key(user_id, auth, &ce_key)) return false; std::string ce_raw_ref; if (!android::vold::installKey(ce_key, &ce_raw_ref)) return false; - s_ce_keys[user_id] = ce_key; + s_ce_keys[user_id] = std::move(ce_key); s_ce_key_raw_refs[user_id] = ce_raw_ref; LOG(DEBUG) << "Installed ce key for user " << user_id; return true; @@ -219,7 +220,7 @@ static bool destroy_dir(const std::string& dir) { // NB this assumes that there is only one thread listening for crypt commands, because // it creates keys in a fixed location. static bool create_and_install_user_keys(userid_t user_id, bool create_ephemeral) { - std::string de_key, ce_key; + KeyBuffer de_key, ce_key; if (!android::vold::randomKey(&de_key)) return false; if (!android::vold::randomKey(&ce_key)) return false; if (create_ephemeral) { @@ -306,7 +307,7 @@ static bool load_all_de_keys() { userid_t user_id = atoi(entry->d_name); if (s_de_key_raw_refs.count(user_id) == 0) { auto key_path = de_dir + "/" + entry->d_name; - std::string key; + KeyBuffer key; if (!android::vold::retrieveKey(key_path, kEmptyAuthentication, &key)) return false; std::string raw_ref; if (!android::vold::installKey(key, &raw_ref)) return false; @@ -411,7 +412,7 @@ static void drop_caches() { } static bool evict_ce_key(userid_t user_id) { - s_ce_keys.erase(user_id); + s_ce_keys.erase(user_id); bool success = true; std::string raw_ref; // If we haven't loaded the CE key, no need to evict it. @@ -509,7 +510,7 @@ bool e4crypt_add_user_key_auth(userid_t user_id, int serial, const char* token_h LOG(ERROR) << "Key not loaded into memory, can't change for user " << user_id; return false; } - auto ce_key = it->second; + const auto &ce_key = it->second; auto const directory_path = get_ce_key_directory_path(user_id); auto const paths = get_ce_key_paths(directory_path); std::string ce_key_path; diff --git a/KeyBuffer.cpp b/KeyBuffer.cpp new file mode 100644 index 0000000..e7aede5 --- /dev/null +++ b/KeyBuffer.cpp @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "KeyBuffer.h" + +#include +#include + +namespace android { +namespace vold { + +KeyBuffer operator+(KeyBuffer&& lhs, const KeyBuffer& rhs) { + std::copy(rhs.begin(), rhs.end(), std::back_inserter(lhs)); + return std::move(lhs); +} + +KeyBuffer operator+(KeyBuffer&& lhs, const char* rhs) { + std::copy(rhs, rhs + strlen(rhs), std::back_inserter(lhs)); + return std::move(lhs); +} + +} // namespace vold +} // namespace android + diff --git a/KeyBuffer.h b/KeyBuffer.h new file mode 100644 index 0000000..2087187 --- /dev/null +++ b/KeyBuffer.h @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_VOLD_KEYBUFFER_H +#define ANDROID_VOLD_KEYBUFFER_H + +#include +#include +#include + +namespace android { +namespace vold { + +/** + * Variant of memset() that should never be optimized away. Borrowed from keymaster code. + */ +#ifdef __clang__ +#define OPTNONE __attribute__((optnone)) +#else // not __clang__ +#define OPTNONE __attribute__((optimize("O0"))) +#endif // not __clang__ +inline OPTNONE void* memset_s(void* s, int c, size_t n) { + if (!s) + return s; + return memset(s, c, n); +} +#undef OPTNONE + +// Allocator that delegates useful work to standard one but zeroes data before deallocating. +class ZeroingAllocator : public std::allocator { + public: + void deallocate(pointer p, size_type n) + { + memset_s(p, 0, n); + std::allocator::deallocate(p, n); + } +}; + +// Char vector that zeroes memory when deallocating. +using KeyBuffer = std::vector; + +// Convenience methods to concatenate key buffers. +KeyBuffer operator+(KeyBuffer&& lhs, const KeyBuffer& rhs); +KeyBuffer operator+(KeyBuffer&& lhs, const char* rhs); + +} // namespace vold +} // namespace android + +#endif + diff --git a/KeyStorage.cpp b/KeyStorage.cpp index b4f85f4..9d61555 100644 --- a/KeyStorage.cpp +++ b/KeyStorage.cpp @@ -195,7 +195,7 @@ static KeymasterOperation begin(Keymaster& keymaster, const std::string& dir, static bool encryptWithKeymasterKey(Keymaster& keymaster, const std::string& dir, const AuthorizationSet &keyParams, - const std::string& message, std::string* ciphertext) { + const KeyBuffer& message, std::string* ciphertext) { AuthorizationSet opParams; AuthorizationSet outParams; auto opHandle = begin(keymaster, dir, KeyPurpose::ENCRYPT, keyParams, opParams, &outParams); @@ -220,7 +220,7 @@ static bool encryptWithKeymasterKey(Keymaster& keymaster, const std::string& dir static bool decryptWithKeymasterKey(Keymaster& keymaster, const std::string& dir, const AuthorizationSet &keyParams, - const std::string& ciphertext, std::string* message) { + const std::string& ciphertext, KeyBuffer* message) { auto nonce = ciphertext.substr(0, GCM_NONCE_BYTES); auto bodyAndMac = ciphertext.substr(GCM_NONCE_BYTES); auto opParams = AuthorizationSetBuilder() @@ -305,7 +305,7 @@ static void logOpensslError() { } static bool encryptWithoutKeymaster(const std::string& preKey, - const std::string& plaintext, std::string* ciphertext) { + const KeyBuffer& plaintext, std::string* ciphertext) { auto key = hashWithPrefix(kHashPrefix_keygen, preKey); key.resize(AES_KEY_BYTES); if (!readRandomBytesOrLog(GCM_NONCE_BYTES, ciphertext)) return false; @@ -351,7 +351,7 @@ static bool encryptWithoutKeymaster(const std::string& preKey, } static bool decryptWithoutKeymaster(const std::string& preKey, - const std::string& ciphertext, std::string* plaintext) { + const std::string& ciphertext, KeyBuffer* plaintext) { if (ciphertext.size() < GCM_NONCE_BYTES + GCM_MAC_BYTES) { LOG(ERROR) << "GCM ciphertext too small: " << ciphertext.size(); return false; @@ -370,7 +370,7 @@ static bool decryptWithoutKeymaster(const std::string& preKey, logOpensslError(); return false; } - plaintext->resize(ciphertext.size() - GCM_NONCE_BYTES - GCM_MAC_BYTES); + *plaintext = KeyBuffer(ciphertext.size() - GCM_NONCE_BYTES - GCM_MAC_BYTES); int outlen; if (1 != EVP_DecryptUpdate(ctx.get(), reinterpret_cast(&(*plaintext)[0]), &outlen, @@ -404,7 +404,7 @@ bool pathExists(const std::string& path) { return access(path.c_str(), F_OK) == 0; } -bool storeKey(const std::string& dir, const KeyAuthentication& auth, const std::string& key) { +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; return false; @@ -442,7 +442,7 @@ bool storeKey(const std::string& dir, const KeyAuthentication& auth, const std:: } bool storeKeyAtomically(const std::string& key_path, const std::string& tmp_path, - const KeyAuthentication& auth, const std::string& key) { + const KeyAuthentication& auth, const KeyBuffer& key) { if (pathExists(key_path)) { LOG(ERROR) << "Already exists, cannot create key at: " << key_path; return false; @@ -460,7 +460,7 @@ bool storeKeyAtomically(const std::string& key_path, const std::string& tmp_path return true; } -bool retrieveKey(const std::string& dir, const KeyAuthentication& auth, std::string* key) { +bool retrieveKey(const std::string& dir, const KeyAuthentication& auth, KeyBuffer* key) { std::string version; if (!readFileToString(dir + "/" + kFn_version, &version)) return false; if (version != kCurrentVersion) { diff --git a/KeyStorage.h b/KeyStorage.h index 63345f4..655cd17 100644 --- a/KeyStorage.h +++ b/KeyStorage.h @@ -17,6 +17,8 @@ #ifndef ANDROID_VOLD_KEYSTORAGE_H #define ANDROID_VOLD_KEYSTORAGE_H +#include "KeyBuffer.h" + #include namespace android { @@ -46,17 +48,17 @@ bool pathExists(const std::string& path); // in such a way that it can only be retrieved via Keymaster and // can be securely deleted. // It's safe to move/rename the directory after creation. -bool storeKey(const std::string& dir, const KeyAuthentication& auth, const std::string& key); +bool storeKey(const std::string& dir, const KeyAuthentication& auth, const KeyBuffer& key); // Create a directory at the named path, and store "key" in it as storeKey // This version creates the key in "tmp_path" then atomically renames "tmp_path" // to "key_path" thereby ensuring that the key is either stored entirely or // not at all. bool storeKeyAtomically(const std::string& key_path, const std::string& tmp_path, - const KeyAuthentication& auth, const std::string& key); + const KeyAuthentication& auth, const KeyBuffer& key); // Retrieve the key from the named directory. -bool retrieveKey(const std::string& dir, const KeyAuthentication& auth, std::string* key); +bool retrieveKey(const std::string& dir, const KeyAuthentication& auth, KeyBuffer* key); // Securely destroy the key stored in the named directory and delete the directory. bool destroyKey(const std::string& dir); diff --git a/KeyUtil.cpp b/KeyUtil.cpp index a75dfbb..7bbbf01 100644 --- a/KeyUtil.cpp +++ b/KeyUtil.cpp @@ -32,8 +32,23 @@ namespace android { namespace vold { -bool randomKey(std::string* key) { - if (ReadRandomBytes(EXT4_AES_256_XTS_KEY_SIZE, *key) != 0) { +// ext4enc:TODO get this const from somewhere good +const int EXT4_KEY_DESCRIPTOR_SIZE = 8; + +// ext4enc:TODO Include structure from somewhere sensible +// MUST be in sync with ext4_crypto.c in kernel +constexpr int EXT4_ENCRYPTION_MODE_AES_256_XTS = 1; +constexpr int EXT4_AES_256_XTS_KEY_SIZE = 64; +constexpr int EXT4_MAX_KEY_SIZE = 64; +struct ext4_encryption_key { + uint32_t mode; + char raw[EXT4_MAX_KEY_SIZE]; + uint32_t size; +}; + +bool randomKey(KeyBuffer* key) { + *key = KeyBuffer(EXT4_AES_256_XTS_KEY_SIZE); + if (ReadRandomBytes(key->size(), key->data()) != 0) { // TODO status_t plays badly with PLOG, fix it. LOG(ERROR) << "Random read failed"; return false; @@ -60,7 +75,7 @@ static std::string generateKeyRef(const char* key, int length) { return std::string((char*)key_ref2, EXT4_KEY_DESCRIPTOR_SIZE); } -static bool fillKey(const std::string& key, ext4_encryption_key* ext4_key) { +static bool fillKey(const KeyBuffer& key, ext4_encryption_key* ext4_key) { if (key.size() != EXT4_AES_256_XTS_KEY_SIZE) { LOG(ERROR) << "Wrong size key " << key.size(); return false; @@ -101,8 +116,11 @@ static bool e4cryptKeyring(key_serial_t* device_keyring) { // Install password into global keyring // Return raw key reference for use in policy -bool installKey(const std::string& key, std::string* raw_ref) { - ext4_encryption_key ext4_key; +bool installKey(const KeyBuffer& key, std::string* raw_ref) { + // Place ext4_encryption_key into automatically zeroing buffer. + KeyBuffer ext4KeyBuffer(sizeof(ext4_encryption_key)); + ext4_encryption_key &ext4_key = *reinterpret_cast(ext4KeyBuffer.data()); + if (!fillKey(key, &ext4_key)) return false; *raw_ref = generateKeyRef(ext4_key.raw, ext4_key.size); key_serial_t device_keyring; @@ -145,7 +163,7 @@ bool evictKey(const std::string& raw_ref) { bool retrieveAndInstallKey(bool create_if_absent, const std::string& key_path, const std::string& tmp_path, std::string* key_ref) { - std::string key; + KeyBuffer key; if (pathExists(key_path)) { LOG(DEBUG) << "Key exists, using: " << key_path; if (!retrieveKey(key_path, kEmptyAuthentication, &key)) return false; @@ -168,7 +186,7 @@ bool retrieveAndInstallKey(bool create_if_absent, const std::string& key_path, } bool retrieveKey(bool create_if_absent, const std::string& key_path, - const std::string& tmp_path, std::string* key) { + const std::string& tmp_path, KeyBuffer* key) { if (pathExists(key_path)) { LOG(DEBUG) << "Key exists, using: " << key_path; if (!retrieveKey(key_path, kEmptyAuthentication, key)) return false; diff --git a/KeyUtil.h b/KeyUtil.h index d4c97b9..412b0ae 100644 --- a/KeyUtil.h +++ b/KeyUtil.h @@ -17,32 +17,21 @@ #ifndef ANDROID_VOLD_KEYUTIL_H #define ANDROID_VOLD_KEYUTIL_H +#include "KeyBuffer.h" + #include +#include namespace android { namespace vold { -// ext4enc:TODO get this const from somewhere good -const int EXT4_KEY_DESCRIPTOR_SIZE = 8; - -// ext4enc:TODO Include structure from somewhere sensible -// MUST be in sync with ext4_crypto.c in kernel -constexpr int EXT4_ENCRYPTION_MODE_AES_256_XTS = 1; -constexpr int EXT4_AES_256_XTS_KEY_SIZE = 64; -constexpr int EXT4_MAX_KEY_SIZE = 64; -struct ext4_encryption_key { - uint32_t mode; - char raw[EXT4_MAX_KEY_SIZE]; - uint32_t size; -}; - -bool randomKey(std::string* key); -bool installKey(const std::string& key, std::string* raw_ref); +bool randomKey(KeyBuffer* key); +bool installKey(const KeyBuffer& key, std::string* raw_ref); bool evictKey(const std::string& raw_ref); bool retrieveAndInstallKey(bool create_if_absent, const std::string& key_path, const std::string& tmp_path, std::string* key_ref); bool retrieveKey(bool create_if_absent, const std::string& key_path, - const std::string& tmp_path, std::string* key); + const std::string& tmp_path, KeyBuffer* key); } // namespace vold } // namespace android diff --git a/Keymaster.cpp b/Keymaster.cpp index ffa3a7a..1bbeb61 100644 --- a/Keymaster.cpp +++ b/Keymaster.cpp @@ -31,25 +31,23 @@ KeymasterOperation::~KeymasterOperation() { if (mDevice.get()) mDevice->abort(mOpHandle); } -bool KeymasterOperation::updateCompletely(const std::string& input, std::string* output) { - if (output) - output->clear(); - auto it = input.begin(); - uint32_t inputConsumed; +bool KeymasterOperation::updateCompletely(const char* input, size_t inputLen, + const std::function consumer) { + uint32_t inputConsumed = 0; ErrorCode km_error; - auto hidlCB = [&] (ErrorCode ret, uint32_t _inputConsumed, + auto hidlCB = [&] (ErrorCode ret, uint32_t inputConsumedDelta, const hidl_vec& /*ignored*/, const hidl_vec& _output) { km_error = ret; if (km_error != ErrorCode::OK) return; - inputConsumed = _inputConsumed; - if (output) - output->append(reinterpret_cast(&_output[0]), _output.size()); + inputConsumed += inputConsumedDelta; + consumer(reinterpret_cast(&_output[0]), _output.size()); }; - while (it != input.end()) { - size_t toRead = static_cast(input.end() - it); - auto inputBlob = blob2hidlVec(reinterpret_cast(&*it), toRead); + while (inputConsumed != inputLen) { + size_t toRead = static_cast(inputLen - inputConsumed); + auto inputBlob = + blob2hidlVec(reinterpret_cast(&input[inputConsumed]), toRead); auto error = mDevice->update(mOpHandle, hidl_vec(), inputBlob, hidlCB); if (!error.isOk()) { LOG(ERROR) << "update failed: " << error.description(); @@ -61,12 +59,11 @@ bool KeymasterOperation::updateCompletely(const std::string& input, std::string* mDevice = nullptr; return false; } - if (inputConsumed > toRead) { + if (inputConsumed > inputLen) { LOG(ERROR) << "update reported too much input consumed"; mDevice = nullptr; return false; } - it += inputConsumed; } return true; } diff --git a/Keymaster.h b/Keymaster.h index 4bc0df7..dc6f1bc 100644 --- a/Keymaster.h +++ b/Keymaster.h @@ -19,6 +19,8 @@ #ifdef __cplusplus +#include "KeyBuffer.h" + #include #include #include @@ -51,7 +53,14 @@ class KeymasterOperation { ErrorCode errorCode() { return mError; } // Call "update" repeatedly until all of the input is consumed, and // concatenate the output. Return true on success. - bool updateCompletely(const std::string& input, std::string* output); + template + bool updateCompletely(TI& input, TO* output) { + if (output) output->clear(); + return updateCompletely(input.data(), input.size(), [&](const char* b, size_t n) { + if (output) std::copy(b, b+n, std::back_inserter(*output)); + }); + } + // Finish and write the output to this string, unless pointer is null. bool finish(std::string* output); // Move constructor @@ -80,6 +89,10 @@ class KeymasterOperation { KeymasterOperation(ErrorCode error) : mDevice{nullptr}, mOpHandle{0}, mError {error} {} + + bool updateCompletely(const char* input, size_t inputLen, + const std::function consumer); + sp mDevice; uint64_t mOpHandle; ErrorCode mError; diff --git a/MetadataCrypt.cpp b/MetadataCrypt.cpp index 743e08c..8311813 100644 --- a/MetadataCrypt.cpp +++ b/MetadataCrypt.cpp @@ -14,11 +14,13 @@ * limitations under the License. */ +#include "KeyBuffer.h" #include "MetadataCrypt.h" #include #include #include +#include #include #include @@ -45,6 +47,8 @@ extern struct fstab *fstab; #define TABLE_LOAD_RETRIES 10 #define DEFAULT_KEY_TARGET_TYPE "default-key" +using android::vold::KeyBuffer; + static const std::string kDmNameUserdata = "userdata"; static bool mount_via_fs_mgr(const char* mount_point, const char* blk_device) { @@ -68,7 +72,7 @@ static bool mount_via_fs_mgr(const char* mount_point, const char* blk_device) { return true; } -static bool read_key(bool create_if_absent, std::string* key) { +static bool read_key(bool create_if_absent, KeyBuffer* key) { auto data_rec = fs_mgr_get_crypt_entry(fstab); if (!data_rec) { LOG(ERROR) << "Failed to get data_rec"; @@ -93,14 +97,14 @@ static bool read_key(bool create_if_absent, std::string* key) { return true; } -static std::string default_key_params(const std::string& real_blkdev, const std::string& key) { - std::string hex_key; +static KeyBuffer default_key_params(const std::string& real_blkdev, const KeyBuffer& key) { + KeyBuffer hex_key; if (android::vold::StrToHex(key, hex_key) != android::OK) { LOG(ERROR) << "Failed to turn key to hex"; - return ""; + return KeyBuffer(); } - auto res = std::string() + "AES-256-XTS " + hex_key + " " + real_blkdev + " 0"; - LOG(DEBUG) << "crypt_params: " << res; + auto res = KeyBuffer() + "AES-256-XTS " + hex_key + " " + real_blkdev.c_str() + " 0"; + LOG(DEBUG) << "crypt_params: " << std::string(res.data(), res.size()); return res; } @@ -142,7 +146,7 @@ static struct dm_ioctl* dm_ioctl_init(char *buffer, size_t buffer_size, } static bool create_crypto_blk_dev(const std::string& dm_name, uint64_t nr_sec, - const std::string& target_type, const std::string& crypt_params, + const std::string& target_type, const KeyBuffer& crypt_params, std::string* crypto_blkdev) { android::base::unique_fd dm_fd(TEMP_FAILURE_RETRY(open( "/dev/device-mapper", O_RDWR | O_CLOEXEC, 0))); @@ -167,9 +171,9 @@ static bool create_crypto_blk_dev(const std::string& dm_name, uint64_t nr_sec, (io->dev & 0xff) | ((io->dev >> 12) & 0xfff00)); io = dm_ioctl_init(buffer, sizeof(buffer), dm_name); - unsigned long paramix = io->data_start + sizeof(struct dm_target_spec); - unsigned long nullix = paramix + crypt_params.size(); - unsigned long endix = (nullix + 1 + 7) & 8; // Add room for \0 and align to 8 byte boundary + size_t paramix = io->data_start + sizeof(struct dm_target_spec); + size_t nullix = paramix + crypt_params.size(); + size_t endix = (nullix + 1 + 7) & 8; // Add room for \0 and align to 8 byte boundary if (endix > sizeof(buffer)) { LOG(ERROR) << "crypt_params too big for DM_CRYPT_BUF_SIZE"; @@ -182,7 +186,8 @@ static bool create_crypto_blk_dev(const std::string& dm_name, uint64_t nr_sec, tgt->sector_start = 0; tgt->length = nr_sec; target_type.copy(tgt->target_type, sizeof(tgt->target_type)); - crypt_params.copy(buffer + paramix, sizeof(buffer) - paramix); + memcpy(buffer + paramix, crypt_params.data(), + std::min(crypt_params.size(), sizeof(buffer) - paramix)); buffer[nullix] = '\0'; tgt->next = endix; @@ -246,7 +251,7 @@ static void async_kick_off() { bool e4crypt_mount_metadata_encrypted() { LOG(DEBUG) << "e4crypt_mount_default_encrypted"; - std::string key; + KeyBuffer key; if (!read_key(false, &key)) return false; auto data_rec = fs_mgr_get_crypt_entry(fstab); if (!data_rec) { @@ -275,7 +280,7 @@ bool e4crypt_enable_crypto() { return false; } - std::string key_ref; + KeyBuffer key_ref; if (!read_key(true, &key_ref)) return false; auto data_rec = fs_mgr_get_crypt_entry(fstab); diff --git a/Utils.cpp b/Utils.cpp index 395a890..b6c7bf8 100644 --- a/Utils.cpp +++ b/Utils.cpp @@ -351,18 +351,20 @@ pid_t ForkExecvpAsync(const std::vector& args) { } status_t ReadRandomBytes(size_t bytes, std::string& out) { - out.clear(); + out.resize(bytes); + return ReadRandomBytes(bytes, &out[0]); +} +status_t ReadRandomBytes(size_t bytes, char* buf) { int fd = TEMP_FAILURE_RETRY(open("/dev/urandom", O_RDONLY | O_CLOEXEC | O_NOFOLLOW)); if (fd == -1) { return -errno; } - char buf[BUFSIZ]; size_t n; - while ((n = TEMP_FAILURE_RETRY(read(fd, &buf[0], std::min(sizeof(buf), bytes)))) > 0) { - out.append(buf, n); + while ((n = TEMP_FAILURE_RETRY(read(fd, &buf[0], bytes))) > 0) { bytes -= n; + buf += n; } close(fd); @@ -434,6 +436,15 @@ status_t StrToHex(const std::string& str, std::string& hex) { return OK; } +status_t StrToHex(const KeyBuffer& str, KeyBuffer& hex) { + hex.clear(); + for (size_t i = 0; i < str.size(); i++) { + hex.push_back(kLookup[(str.data()[i] & 0xF0) >> 4]); + hex.push_back(kLookup[str.data()[i] & 0x0F]); + } + return OK; +} + status_t NormalizeHex(const std::string& in, std::string& out) { std::string tmp; if (HexToStr(in, tmp)) { diff --git a/Utils.h b/Utils.h index 7272fe1..82ac440 100644 --- a/Utils.h +++ b/Utils.h @@ -17,6 +17,8 @@ #ifndef ANDROID_VOLD_UTILS_H #define ANDROID_VOLD_UTILS_H +#include "KeyBuffer.h" + #include #include #include @@ -78,12 +80,15 @@ status_t ForkExecvp(const std::vector& args, pid_t ForkExecvpAsync(const std::vector& args); status_t ReadRandomBytes(size_t bytes, std::string& out); +status_t ReadRandomBytes(size_t bytes, char* buffer); status_t GenerateRandomUuid(std::string& out); /* Converts hex string to raw bytes, ignoring [ :-] */ status_t HexToStr(const std::string& hex, std::string& str); /* Converts raw bytes to hex string */ status_t StrToHex(const std::string& str, std::string& hex); +/* Converts raw key bytes to hex string */ +status_t StrToHex(const KeyBuffer& str, KeyBuffer& hex); /* Normalize given hex string into consistent format */ status_t NormalizeHex(const std::string& in, std::string& out); From 7056de1b423e10b16149df3db0102aff714f2648 Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Mon, 14 Aug 2017 11:32:13 +0800 Subject: [PATCH 13/19] mInternalEmulated could be used after shutdown() called It fixes the findvolume() / reset() use-after-free issue after shutdown called to avoid vold crash. bug: 64833901 Test: test reboot Fixes: a5bbb5e3c13d ("make shutdown safe for double calls.") Signed-off-by: Gao Xiang (cherry picked from commit d263da88076f5299e6202f8b388eab79f6fdd495) Change-Id: I636b28f30fb82e4672d88144cd04072d24ef3b85 --- VolumeManager.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/VolumeManager.cpp b/VolumeManager.cpp index 8398498..13a943f 100644 --- a/VolumeManager.cpp +++ b/VolumeManager.cpp @@ -425,7 +425,10 @@ std::shared_ptr VolumeManager::findDisk(const std::string& } std::shared_ptr VolumeManager::findVolume(const std::string& id) { - if (mInternalEmulated->getId() == id) { + // Vold could receive "mount" after "shutdown" command in the extreme case. + // If this happens, mInternalEmulated will equal nullptr and + // we need to deal with it in order to avoid null pointer crash. + if (mInternalEmulated != nullptr && mInternalEmulated->getId() == id) { return mInternalEmulated; } for (const auto& disk : mDisks) { @@ -689,8 +692,10 @@ next: int VolumeManager::reset() { // Tear down all existing disks/volumes and start from a blank slate so // newly connected framework hears all events. - mInternalEmulated->destroy(); - mInternalEmulated->create(); + if (mInternalEmulated != nullptr) { + mInternalEmulated->destroy(); + mInternalEmulated->create(); + } for (const auto& disk : mDisks) { disk->destroy(); disk->create(); From 1b38e330035833971663eeb5bccfa16e4ffc1e72 Mon Sep 17 00:00:00 2001 From: Daniel Rosenberg Date: Wed, 6 Sep 2017 15:25:40 -0700 Subject: [PATCH 14/19] Add support for gid derivation on private volumes This sdcardfs feature was moved under a mount option and is only needed on private volumes Test: Private emulated volume should attempt to mount with derive_gid option. Bug: 63245673 Change-Id: I40a8b15c298c815a4643007b9eca8269379fd2ac --- EmulatedVolume.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/EmulatedVolume.cpp b/EmulatedVolume.cpp index df91904..21b290a 100644 --- a/EmulatedVolume.cpp +++ b/EmulatedVolume.cpp @@ -84,6 +84,7 @@ status_t EmulatedVolume::doMount() { "-g", "1023", // AID_MEDIA_RW "-m", "-w", + "-G", mRawPath.c_str(), label.c_str(), NULL)) { From 5744dfe3cc9bf7caa8a297f87528419979b138ff Mon Sep 17 00:00:00 2001 From: "Chen, Luhai" Date: Fri, 18 Aug 2017 14:49:45 +0800 Subject: [PATCH 15/19] Fix keyname generation issue The keyname binded to keyring return a wrong string when there are binary char larger than 127, the sign extension will introduce unexpect FFFFFF string to the keyname. Bug: 65423023 Test: local build with boot test and device encryption status check. Change-Id: I26482c98ac1858a63b9f5c3f84a8699fd6a21cd7 Signed-off-by: Ai, Ting A Signed-off-by: Chen, Luhai --- KeyUtil.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/KeyUtil.cpp b/KeyUtil.cpp index 7bbbf01..dbc73c1 100644 --- a/KeyUtil.cpp +++ b/KeyUtil.cpp @@ -98,7 +98,7 @@ static char const* const NAME_PREFIXES[] = { static std::string keyname(const std::string& prefix, const std::string& raw_ref) { std::ostringstream o; o << prefix << ":"; - for (auto i : raw_ref) { + for (unsigned char i : raw_ref) { o << std::hex << std::setw(2) << std::setfill('0') << (int)i; } return o.str(); From a3a60b372efb48a672848c7797efd57a4ec92303 Mon Sep 17 00:00:00 2001 From: Richard Uhler Date: Thu, 14 Sep 2017 14:51:04 +0000 Subject: [PATCH 16/19] Revert "Add support for gid derivation on private volumes" This reverts commit 1b38e330035833971663eeb5bccfa16e4ffc1e72. Causes a boot loop on bullhead. Bug: 63245673 Bug: 65660058 Change-Id: I9c8afd3ba22547aff5aff06b71cb8ff3b8a07350 --- EmulatedVolume.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/EmulatedVolume.cpp b/EmulatedVolume.cpp index 21b290a..df91904 100644 --- a/EmulatedVolume.cpp +++ b/EmulatedVolume.cpp @@ -84,7 +84,6 @@ status_t EmulatedVolume::doMount() { "-g", "1023", // AID_MEDIA_RW "-m", "-w", - "-G", mRawPath.c_str(), label.c_str(), NULL)) { From 75ae529bf86c33106e6a716ad054c6eea6deabc5 Mon Sep 17 00:00:00 2001 From: Rom Lemarchand Date: Fri, 15 Sep 2017 18:48:01 +0000 Subject: [PATCH 17/19] Revert "Revert "Add support for gid derivation on private volumes"" This reverts commit a3a60b372efb48a672848c7797efd57a4ec92303. Reason for revert: All kernel prebuilts merged Bug: 65600849 Bug: 65573871 Change-Id: I75b0cb1d82213b875cbef8d39f4f1a8fb34b9795 --- EmulatedVolume.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/EmulatedVolume.cpp b/EmulatedVolume.cpp index df91904..21b290a 100644 --- a/EmulatedVolume.cpp +++ b/EmulatedVolume.cpp @@ -84,6 +84,7 @@ status_t EmulatedVolume::doMount() { "-g", "1023", // AID_MEDIA_RW "-m", "-w", + "-G", mRawPath.c_str(), label.c_str(), NULL)) { From 958c216d870d72303255720f3307c997041e0901 Mon Sep 17 00:00:00 2001 From: Rom Lemarchand Date: Fri, 15 Sep 2017 18:48:01 +0000 Subject: [PATCH 18/19] Revert "Revert "Add support for gid derivation on private volumes"" This reverts commit a3a60b372efb48a672848c7797efd57a4ec92303. Reason for revert: All kernel prebuilts merged (cherry picked from commit 75ae529bf86c33106e6a716ad054c6eea6deabc5) Bug: 65600849 Bug: 65573871 Test: m Change-Id: I75b0cb1d82213b875cbef8d39f4f1a8fb34b9795 --- EmulatedVolume.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/EmulatedVolume.cpp b/EmulatedVolume.cpp index df91904..21b290a 100644 --- a/EmulatedVolume.cpp +++ b/EmulatedVolume.cpp @@ -84,6 +84,7 @@ status_t EmulatedVolume::doMount() { "-g", "1023", // AID_MEDIA_RW "-m", "-w", + "-G", mRawPath.c_str(), label.c_str(), NULL)) { From a04014bf26a3b1f83e15d9ea99fb05963598049e Mon Sep 17 00:00:00 2001 From: Paul Crowley Date: Mon, 2 Oct 2017 16:18:56 -0700 Subject: [PATCH 19/19] Remove CheckBattery altogether Test: changed Angler fstab to encryptable and encrypted. Bug: 16868177 Change-Id: I17d36ea838d6d96f0752b2d6d03b1f9a781ed018 --- Android.mk | 1 - CheckBattery.cpp | 33 --------------------------------- CheckBattery.h | 31 ------------------------------- EncryptInplace.cpp | 15 --------------- cryptfs.cpp | 6 ------ 5 files changed, 86 deletions(-) delete mode 100644 CheckBattery.cpp delete mode 100644 CheckBattery.h diff --git a/Android.mk b/Android.mk index 770f0b8..8004fc3 100644 --- a/Android.mk +++ b/Android.mk @@ -14,7 +14,6 @@ common_src_files := \ Loop.cpp \ Devmapper.cpp \ ResponseCode.cpp \ - CheckBattery.cpp \ Ext4Crypt.cpp \ VoldUtil.c \ cryptfs.cpp \ diff --git a/CheckBattery.cpp b/CheckBattery.cpp deleted file mode 100644 index 27b3b0b..0000000 --- a/CheckBattery.cpp +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright (C) 2014 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#define LOG_TAG "VoldCheckBattery" -#include - -extern "C" -{ - int is_battery_ok_to_start() - { - // Bug 16868177 exists to purge this code completely - return true; //is_battery_ok(START_THRESHOLD); - } - - int is_battery_ok_to_continue() - { - // Bug 16868177 exists to purge this code completely - return true; //is_battery_ok(CONTINUE_THRESHOLD); - } -} diff --git a/CheckBattery.h b/CheckBattery.h deleted file mode 100644 index dd11ba9..0000000 --- a/CheckBattery.h +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright (C) 2014 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef _CHECKBATTERY_H__ -#define _CHECKBATTERY_H__ - -#ifdef __cplusplus -extern "C" { -#endif - -int is_battery_ok_to_start(); -int is_battery_ok_to_continue(); - -#ifdef __cplusplus -} -#endif - -#endif diff --git a/EncryptInplace.cpp b/EncryptInplace.cpp index 9aa2a21..6ef1cb0 100644 --- a/EncryptInplace.cpp +++ b/EncryptInplace.cpp @@ -33,7 +33,6 @@ #include "cutils/properties.h" #define LOG_TAG "EncryptInplace" #include "cutils/log.h" -#include "CheckBattery.h" // HORRIBLE HACK, FIXME #include "cryptfs.h" @@ -245,13 +244,6 @@ static int encrypt_groups(struct encryptGroupsData* data) goto errout; } } - - if (!is_battery_ok_to_continue()) { - SLOGE("Stopping encryption due to low battery"); - rc = 0; - goto errout; - } - } if (flush_outstanding_data(data)) { goto errout; @@ -574,13 +566,6 @@ static int cryptfs_enable_inplace_full(char *crypto_blkdev, char *real_blkdev, CRYPT_SECTORS_PER_BUFSIZE, i * CRYPT_SECTORS_PER_BUFSIZE); } - - if (!is_battery_ok_to_continue()) { - SLOGE("Stopping encryption due to low battery"); - *size_already_done += (i + 1) * CRYPT_SECTORS_PER_BUFSIZE - 1; - rc = 0; - goto errout; - } } /* Do any remaining sectors */ diff --git a/cryptfs.cpp b/cryptfs.cpp index 8d70fb2..fe4e648 100644 --- a/cryptfs.cpp +++ b/cryptfs.cpp @@ -58,7 +58,6 @@ #include "VoldUtil.h" #include "Ext4Crypt.h" #include "f2fs_sparseblock.h" -#include "CheckBattery.h" #include "EncryptInplace.h" #include "Process.h" #include "Keymaster.h" @@ -2029,11 +2028,6 @@ static int cryptfs_enable_all_volumes(struct crypt_mnt_ftr *crypt_ftr, int how, off64_t cur_encryption_done=0, tot_encryption_size=0; int rc = -1; - if (!is_battery_ok_to_start()) { - SLOGW("Not starting encryption due to low battery"); - return 0; - } - /* The size of the userdata partition, and add in the vold volumes below */ tot_encryption_size = crypt_ftr->fs_size;