Allowing the fs_mgr flag of avb_keys to be a dir
Hard coding multiple keys in the avb_keys flag isn't flexible and
causes some pains when upgrading an Android codebase.
e.g., from Android 10 to Android 11.
This CL supports specifying a directory for the avb_keys
for fs_mgr to list then use the avb keys under the directory.
Bug: 144399552
Test: config a fstab using avb_keys as a dir to boot
Test: atest libfs_avb_test
Test: atest libfs_avb_internal_test
Change-Id: Ie74845d8c8e4aa45e8a9e3b862424cec641f8090
diff --git a/fs_mgr/libfs_avb/Android.bp b/fs_mgr/libfs_avb/Android.bp
index 32702ae..bf51fe7 100644
--- a/fs_mgr/libfs_avb/Android.bp
+++ b/fs_mgr/libfs_avb/Android.bp
@@ -37,11 +37,9 @@
"libfstab",
],
shared_libs: [
+ "libbase",
"libcrypto",
],
- header_libs: [
- "libbase_headers",
- ],
target: {
darwin: {
enabled: false,
diff --git a/fs_mgr/libfs_avb/fs_avb.cpp b/fs_mgr/libfs_avb/fs_avb.cpp
index c4d7511..8770a6b 100644
--- a/fs_mgr/libfs_avb/fs_avb.cpp
+++ b/fs_mgr/libfs_avb/fs_avb.cpp
@@ -22,6 +22,7 @@
#include <sys/ioctl.h>
#include <sys/types.h>
+#include <algorithm>
#include <sstream>
#include <string>
#include <vector>
@@ -308,7 +309,18 @@
return nullptr;
}
- if (!ValidatePublicKeyBlob(public_key_data, Split(fstab_entry.avb_keys, ":"))) {
+ // fstab_entry.avb_keys might be either a directory containing multiple keys,
+ // or a string indicating multiple keys separated by ':'.
+ std::vector<std::string> allowed_avb_keys;
+ auto list_avb_keys_in_dir = ListFiles(fstab_entry.avb_keys);
+ if (list_avb_keys_in_dir) {
+ std::sort(list_avb_keys_in_dir->begin(), list_avb_keys_in_dir->end());
+ allowed_avb_keys = *list_avb_keys_in_dir;
+ } else {
+ allowed_avb_keys = Split(fstab_entry.avb_keys, ":");
+ }
+
+ if (!ValidatePublicKeyBlob(public_key_data, allowed_avb_keys)) {
avb_handle->status_ = AvbHandleStatus::kVerificationError;
LWARNING << "Found unknown public key used to sign " << fstab_entry.mount_point;
if (!allow_verification_error) {
diff --git a/fs_mgr/libfs_avb/tests/util_test.cpp b/fs_mgr/libfs_avb/tests/util_test.cpp
index 12b5acb..e64282b 100644
--- a/fs_mgr/libfs_avb/tests/util_test.cpp
+++ b/fs_mgr/libfs_avb/tests/util_test.cpp
@@ -16,10 +16,12 @@
#include <unistd.h>
+#include <algorithm>
#include <future>
#include <string>
#include <thread>
+#include <android-base/strings.h>
#include <base/files/file_util.h>
#include "fs_avb_test_util.h"
@@ -29,6 +31,7 @@
using android::fs_mgr::BytesToHex;
using android::fs_mgr::FileWaitMode;
using android::fs_mgr::HexToBytes;
+using android::fs_mgr::ListFiles;
using android::fs_mgr::NibbleValue;
using android::fs_mgr::WaitForFile;
@@ -210,4 +213,102 @@
ASSERT_TRUE(base::DeleteFile(wait_path, false /* resursive */));
}
+TEST(BasicUtilTest, ListFiles) {
+ // Gets system tmp dir.
+ base::FilePath tmp_dir;
+ ASSERT_TRUE(GetTempDir(&tmp_dir));
+
+ // Creates a test dir for ListFiles testing.
+ base::FilePath test_dir;
+ ASSERT_TRUE(base::CreateTemporaryDirInDir(tmp_dir, "list-file-tests.", &test_dir));
+
+ // Generates dummy files to list.
+ base::FilePath file_path_1 = test_dir.Append("1.txt");
+ ASSERT_TRUE(base::WriteFile(file_path_1, "1", 1));
+ base::FilePath file_path_2 = test_dir.Append("2.txt");
+ ASSERT_TRUE(base::WriteFile(file_path_2, "22", 2));
+ base::FilePath file_path_3 = test_dir.Append("3.txt");
+ ASSERT_TRUE(base::WriteFile(file_path_3, "333", 3));
+
+ // List files for comparison.
+ auto result = ListFiles(test_dir.value());
+ ASSERT_TRUE(result);
+ ASSERT_TRUE(result.has_value());
+ auto files = result.value();
+ EXPECT_EQ(3UL, files.size());
+ // Sort them offline for comparison.
+ std::sort(files.begin(), files.end());
+ EXPECT_EQ(file_path_1.value(), files[0]);
+ EXPECT_EQ(file_path_2.value(), files[1]);
+ EXPECT_EQ(file_path_3.value(), files[2]);
+
+ ASSERT_TRUE(base::DeleteFile(test_dir, true /* resursive */));
+}
+
+TEST(BasicUtilTest, ListFilesShouldDiscardSymlink) {
+ // Gets system tmp dir.
+ base::FilePath tmp_dir;
+ ASSERT_TRUE(GetTempDir(&tmp_dir));
+
+ // Creates a test dir for ListFiles testing.
+ base::FilePath test_dir;
+ ASSERT_TRUE(base::CreateTemporaryDirInDir(tmp_dir, "list-file-tests.", &test_dir));
+
+ // Generates dummy files to list.
+ base::FilePath file_path_1 = test_dir.Append("1.txt");
+ ASSERT_TRUE(base::WriteFile(file_path_1, "1", 1));
+ base::FilePath file_path_2 = test_dir.Append("2.txt");
+ ASSERT_TRUE(base::WriteFile(file_path_2, "22", 2));
+ // Creates a symlink and checks it won't be returned by ListFiles.
+ base::FilePath file_path_3 = test_dir.Append("3.txt");
+ base::FilePath non_existent_target = test_dir.Append("non_existent_target.txt");
+ ASSERT_TRUE(base::CreateSymbolicLink(non_existent_target, file_path_3));
+
+ // List files for comparison.
+ auto result = ListFiles(test_dir.value());
+ ASSERT_TRUE(result);
+ ASSERT_TRUE(result.has_value());
+ auto files = result.value();
+ EXPECT_EQ(2UL, files.size()); // Should not include the symlink file.
+ // Sort them offline for comparison.
+ std::sort(files.begin(), files.end());
+ EXPECT_EQ(file_path_1.value(), files[0]);
+ EXPECT_EQ(file_path_2.value(), files[1]);
+
+ ASSERT_TRUE(base::DeleteFile(test_dir, true /* resursive */));
+}
+
+TEST(BasicUtilTest, ListFilesOpenDirFailure) {
+ // Gets system tmp dir.
+ base::FilePath tmp_dir;
+ ASSERT_TRUE(GetTempDir(&tmp_dir));
+
+ // Generates dummy files to list.
+ base::FilePath no_such_dir = tmp_dir.Append("not_such_dir");
+
+ auto fail = ListFiles(no_such_dir.value());
+ ASSERT_FALSE(fail);
+ EXPECT_EQ(ENOENT, fail.error().code());
+ EXPECT_TRUE(android::base::StartsWith(fail.error().message(), "Failed to opendir: "));
+}
+
+TEST(BasicUtilTest, ListFilesEmptyDir) {
+ // Gets system tmp dir.
+ base::FilePath tmp_dir;
+ ASSERT_TRUE(GetTempDir(&tmp_dir));
+
+ // Creates a test dir for ListFiles testing.
+ base::FilePath test_dir;
+ ASSERT_TRUE(base::CreateTemporaryDirInDir(tmp_dir, "list-file-tests.", &test_dir));
+
+ // List files without sorting.
+ auto result = ListFiles(test_dir.value());
+ ASSERT_TRUE(result);
+ ASSERT_TRUE(result.has_value());
+ auto files = result.value();
+ EXPECT_EQ(0UL, files.size());
+
+ ASSERT_TRUE(base::DeleteFile(test_dir, true /* resursive */));
+}
+
} // namespace fs_avb_host_test
diff --git a/fs_mgr/libfs_avb/util.cpp b/fs_mgr/libfs_avb/util.cpp
index d214b5b..7783d04 100644
--- a/fs_mgr/libfs_avb/util.cpp
+++ b/fs_mgr/libfs_avb/util.cpp
@@ -16,10 +16,13 @@
#include "util.h"
+#include <dirent.h>
#include <sys/ioctl.h>
+#include <sys/types.h>
#include <thread>
+#include <android-base/stringprintf.h>
#include <android-base/unique_fd.h>
#include <linux/fs.h>
@@ -122,5 +125,23 @@
return ioctl(fd, BLKROSET, &ON) == 0;
}
+Result<std::vector<std::string>> ListFiles(const std::string& dir) {
+ struct dirent* de;
+ std::vector<std::string> files;
+
+ std::unique_ptr<DIR, int (*)(DIR*)> dirp(opendir(dir.c_str()), closedir);
+ if (!dirp) {
+ return ErrnoError() << "Failed to opendir: " << dir;
+ }
+
+ while ((de = readdir(dirp.get()))) {
+ if (de->d_type != DT_REG) continue;
+ std::string full_path = android::base::StringPrintf("%s/%s", dir.c_str(), de->d_name);
+ files.emplace_back(std::move(full_path));
+ }
+
+ return files;
+}
+
} // namespace fs_mgr
} // namespace android
diff --git a/fs_mgr/libfs_avb/util.h b/fs_mgr/libfs_avb/util.h
index 7763da5..427ab7c 100644
--- a/fs_mgr/libfs_avb/util.h
+++ b/fs_mgr/libfs_avb/util.h
@@ -18,6 +18,7 @@
#include <chrono>
#include <string>
+#include <vector>
#ifdef HOST_TEST
#include <base/logging.h>
@@ -25,6 +26,11 @@
#include <android-base/logging.h>
#endif
+#include <android-base/result.h>
+
+using android::base::ErrnoError;
+using android::base::Result;
+
#define FS_AVB_TAG "[libfs_avb]"
// Logs a message to kernel
@@ -60,5 +66,8 @@
bool SetBlockDeviceReadOnly(const std::string& blockdev);
+// Returns a list of file under the dir, no order is guaranteed.
+Result<std::vector<std::string>> ListFiles(const std::string& dir);
+
} // namespace fs_mgr
} // namespace android