fs_mgr: stop allowing the FDE fstab options
Since Android 10, new devices have been required to use FBE instead of
FDE. Therefore, the FDE code is no longer needed.
Make fs_mgr reject fstabs where FDE is enabled.
Unfortunately, there is a quirk where the "encryptable" flag (which was
originally meant just for FDE) was overloaded to identify adoptable
storage volumes. It appears that we have to keep supporting this use
case. Therefore, don't reject the "encryptable" flag completely.
Instead, just reject "encryptable" when it appears without
"voldmanaged", or without "userdata" as its argument.
Here are some references for how "encryptable=userdata" is being used to
identify adoptable storage volumes:
* https://source.android.com/devices/storage/config#adoptable_storage
* https://cs.android.com/android/platform/superproject/+/f26c7e9b12e05a6737a96b44bada77232e08ed87:system/vold/main.cpp;l=269
* https://cs.android.com/android/platform/superproject/+/f26c7e9b12e05a6737a96b44bada77232e08ed87:device/google/cuttlefish/shared/config/fstab.f2fs;l=17
* https://cs.android.com/android/platform/superproject/+/f26c7e9b12e05a6737a96b44bada77232e08ed87:device/generic/goldfish/fstab.ranchu;l=7
[ebiggers@: modified from a WIP CL by paulcrowley@]
Bug: 191796797
Change-Id: I3c4bbbe549cc6e24607f230fad27ea0d4d35ce09
diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp
index 609bd11..159be67 100644
--- a/fs_mgr/fs_mgr_fstab.cpp
+++ b/fs_mgr/fs_mgr_fstab.cpp
@@ -146,7 +146,7 @@
entry->fs_options = std::move(fs_options);
}
-void ParseFsMgrFlags(const std::string& flags, FstabEntry* entry) {
+bool ParseFsMgrFlags(const std::string& flags, FstabEntry* entry) {
for (const auto& flag : Split(flags, ",")) {
if (flag.empty() || flag == "defaults") continue;
std::string arg;
@@ -188,10 +188,20 @@
#undef CheckFlag
// Then handle flags that take an argument.
- if (StartsWith(flag, "encryptable=")) {
- // The encryptable flag is followed by an = and the location of the keys.
+ if (flag == "encryptable=userdata") {
+ // The "encryptable" flag identifies adoptable storage volumes. The
+ // argument to this flag must be "userdata".
+ //
+ // Historical note: this flag was originally meant just for /data,
+ // to indicate that FDE (full disk encryption) can be enabled.
+ // Unfortunately, it was also overloaded to identify adoptable
+ // storage volumes. Today, FDE is no longer supported, leaving only
+ // the adoptable storage volume meaning for this flag.
entry->fs_mgr_flags.crypt = true;
- entry->key_loc = arg;
+ } else if (StartsWith(flag, "encryptable=") || StartsWith(flag, "forceencrypt=") ||
+ StartsWith(flag, "forcefdeorfbe=")) {
+ LERROR << "flag no longer supported: " << flag;
+ return false;
} else if (StartsWith(flag, "voldmanaged=")) {
// The voldmanaged flag is followed by an = and the label, a colon and the partition
// number or the word "auto", e.g. voldmanaged=sdcard:3
@@ -235,18 +245,8 @@
LWARNING << "Warning: zramsize= flag malformed: " << arg;
}
}
- } else if (StartsWith(flag, "forceencrypt=")) {
- // The forceencrypt flag is followed by an = and the location of the keys.
- entry->fs_mgr_flags.force_crypt = true;
- entry->key_loc = arg;
} else if (StartsWith(flag, "fileencryption=")) {
ParseFileEncryption(arg, entry);
- } else if (StartsWith(flag, "forcefdeorfbe=")) {
- // The forcefdeorfbe flag is followed by an = and the location of the keys. Get it and
- // return it.
- entry->fs_mgr_flags.force_fde_or_fbe = true;
- entry->key_loc = arg;
- entry->encryption_options = "aes-256-xts:aes-256-cts";
} else if (StartsWith(flag, "max_comp_streams=")) {
if (!ParseInt(arg, &entry->max_comp_streams)) {
LWARNING << "Warning: max_comp_streams= flag malformed: " << arg;
@@ -306,6 +306,13 @@
LWARNING << "Warning: unknown flag: " << flag;
}
}
+
+ if (entry->fs_mgr_flags.crypt && !entry->fs_mgr_flags.vold_managed) {
+ LERROR << "FDE is no longer supported; 'encryptable' can only be used for adoptable "
+ "storage";
+ return false;
+ }
+ return true;
}
std::string InitAndroidDtDir() {
@@ -576,7 +583,10 @@
goto err;
}
- ParseFsMgrFlags(p, &entry);
+ if (!ParseFsMgrFlags(p, &entry)) {
+ LERROR << "Error parsing fs_mgr_flags";
+ goto err;
+ }
if (entry.fs_mgr_flags.logical) {
entry.logical_partition_name = entry.blk_device;
diff --git a/fs_mgr/tests/fs_mgr_test.cpp b/fs_mgr/tests/fs_mgr_test.cpp
index 94e1abb..c77874a 100644
--- a/fs_mgr/tests/fs_mgr_test.cpp
+++ b/fs_mgr/tests/fs_mgr_test.cpp
@@ -488,18 +488,16 @@
TemporaryFile tf;
ASSERT_TRUE(tf.fd != -1);
std::string fstab_contents = R"fs(
-source none0 swap defaults encryptable,forceencrypt,fileencryption,forcefdeorfbe,keydirectory,length,swapprio,zramsize,max_comp_streams,reservedsize,eraseblk,logicalblk,sysfs_path,zram_backingdev_size
+source none0 swap defaults fileencryption,keydirectory,length,swapprio,zramsize,max_comp_streams,reservedsize,eraseblk,logicalblk,sysfs_path,zram_backingdev_size
-source none1 swap defaults encryptable=,forceencrypt=,fileencryption=,keydirectory=,length=,swapprio=,zramsize=,max_comp_streams=,avb=,reservedsize=,eraseblk=,logicalblk=,sysfs_path=,zram_backingdev_size=
-
-source none2 swap defaults forcefdeorfbe=
+source none1 swap defaults fileencryption=,keydirectory=,length=,swapprio=,zramsize=,max_comp_streams=,avb=,reservedsize=,eraseblk=,logicalblk=,sysfs_path=,zram_backingdev_size=
)fs";
ASSERT_TRUE(android::base::WriteStringToFile(fstab_contents, tf.path));
Fstab fstab;
EXPECT_TRUE(ReadFstabFromFile(tf.path, &fstab));
- ASSERT_LE(3U, fstab.size());
+ ASSERT_LE(2U, fstab.size());
auto entry = fstab.begin();
EXPECT_EQ("none0", entry->mount_point);
@@ -526,8 +524,6 @@
EXPECT_EQ("none1", entry->mount_point);
{
FstabEntry::FsMgrFlags flags = {};
- flags.crypt = true;
- flags.force_crypt = true;
flags.file_encryption = true;
flags.avb = true;
EXPECT_TRUE(CompareFlags(flags, entry->fs_mgr_flags));
@@ -546,24 +542,26 @@
EXPECT_EQ(0, entry->logical_blk_size);
EXPECT_EQ("", entry->sysfs_path);
EXPECT_EQ(0U, entry->zram_backingdev_size);
- entry++;
-
- // forcefdeorfbe has its own encryption_options defaults, so test it separately.
- EXPECT_EQ("none2", entry->mount_point);
- {
- FstabEntry::FsMgrFlags flags = {};
- flags.force_fde_or_fbe = true;
- EXPECT_TRUE(CompareFlags(flags, entry->fs_mgr_flags));
- }
- EXPECT_EQ("aes-256-xts:aes-256-cts", entry->encryption_options);
- EXPECT_EQ("", entry->key_loc);
}
-TEST(fs_mgr, ReadFstabFromFile_FsMgrOptions_Encryptable) {
+// FDE is no longer supported, so an fstab with FDE enabled should be rejected.
+TEST(fs_mgr, ReadFstabFromFile_FsMgrOptions_FDE) {
TemporaryFile tf;
ASSERT_TRUE(tf.fd != -1);
std::string fstab_contents = R"fs(
-source none0 swap defaults encryptable=/dir/key
+source /data ext4 noatime forceencrypt=footer
+)fs";
+ ASSERT_TRUE(android::base::WriteStringToFile(fstab_contents, tf.path));
+
+ Fstab fstab;
+ EXPECT_FALSE(ReadFstabFromFile(tf.path, &fstab));
+}
+
+TEST(fs_mgr, ReadFstabFromFile_FsMgrOptions_AdoptableStorage) {
+ TemporaryFile tf;
+ ASSERT_TRUE(tf.fd != -1);
+ std::string fstab_contents = R"fs(
+source none0 swap defaults encryptable=userdata,voldmanaged=sdcard:auto
)fs";
ASSERT_TRUE(android::base::WriteStringToFile(fstab_contents, tf.path));
@@ -573,11 +571,11 @@
FstabEntry::FsMgrFlags flags = {};
flags.crypt = true;
+ flags.vold_managed = true;
auto entry = fstab.begin();
EXPECT_EQ("none0", entry->mount_point);
EXPECT_TRUE(CompareFlags(flags, entry->fs_mgr_flags));
- EXPECT_EQ("/dir/key", entry->key_loc);
}
TEST(fs_mgr, ReadFstabFromFile_FsMgrOptions_VoldManaged) {
@@ -725,53 +723,6 @@
EXPECT_EQ(0, entry->zram_size);
}
-TEST(fs_mgr, ReadFstabFromFile_FsMgrOptions_ForceEncrypt) {
- TemporaryFile tf;
- ASSERT_TRUE(tf.fd != -1);
- std::string fstab_contents = R"fs(
-source none0 swap defaults forceencrypt=/dir/key
-)fs";
-
- ASSERT_TRUE(android::base::WriteStringToFile(fstab_contents, tf.path));
-
- Fstab fstab;
- EXPECT_TRUE(ReadFstabFromFile(tf.path, &fstab));
- ASSERT_LE(1U, fstab.size());
-
- auto entry = fstab.begin();
- EXPECT_EQ("none0", entry->mount_point);
-
- FstabEntry::FsMgrFlags flags = {};
- flags.force_crypt = true;
- EXPECT_TRUE(CompareFlags(flags, entry->fs_mgr_flags));
-
- EXPECT_EQ("/dir/key", entry->key_loc);
-}
-
-TEST(fs_mgr, ReadFstabFromFile_FsMgrOptions_ForceFdeOrFbe) {
- TemporaryFile tf;
- ASSERT_TRUE(tf.fd != -1);
- std::string fstab_contents = R"fs(
-source none0 swap defaults forcefdeorfbe=/dir/key
-)fs";
-
- ASSERT_TRUE(android::base::WriteStringToFile(fstab_contents, tf.path));
-
- Fstab fstab;
- EXPECT_TRUE(ReadFstabFromFile(tf.path, &fstab));
- ASSERT_LE(1U, fstab.size());
-
- auto entry = fstab.begin();
- EXPECT_EQ("none0", entry->mount_point);
-
- FstabEntry::FsMgrFlags flags = {};
- flags.force_fde_or_fbe = true;
- EXPECT_TRUE(CompareFlags(flags, entry->fs_mgr_flags));
-
- EXPECT_EQ("/dir/key", entry->key_loc);
- EXPECT_EQ("aes-256-xts:aes-256-cts", entry->encryption_options);
-}
-
TEST(fs_mgr, ReadFstabFromFile_FsMgrOptions_FileEncryption) {
TemporaryFile tf;
ASSERT_TRUE(tf.fd != -1);