diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index 0c904c46e..272190e33 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -1089,7 +1089,7 @@ int fs_mgr_mount_all(Fstab* fstab, int mount_mode) { // Skips mounting the device. continue; } - } else if (!current_entry.avb_key.empty()) { + } else if (!current_entry.avb_keys.empty()) { if (AvbHandle::SetUpStandaloneAvbHashtree(¤t_entry) == AvbHashtreeResult::kFail) { LERROR << "Failed to set up AVB on standalone partition: " << current_entry.mount_point << ", skipping!"; @@ -1320,7 +1320,7 @@ static int fs_mgr_do_mount_helper(Fstab* fstab, const std::string& n_name, // Skips mounting the device. continue; } - } else if (!fstab_entry.avb_key.empty()) { + } else if (!fstab_entry.avb_keys.empty()) { if (AvbHandle::SetUpStandaloneAvbHashtree(&fstab_entry) == AvbHashtreeResult::kFail) { LERROR << "Failed to set up AVB on standalone partition: " << fstab_entry.mount_point << ", skipping!"; diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp index 82d91444d..5d4a3ccd9 100644 --- a/fs_mgr/fs_mgr_fstab.cpp +++ b/fs_mgr/fs_mgr_fstab.cpp @@ -325,8 +325,8 @@ void ParseFsMgrFlags(const std::string& flags, FstabEntry* entry) { } } else if (StartsWith(flag, "zram_backing_dev_path=")) { entry->zram_backing_dev_path = arg; - } else if (StartsWith(flag, "avb_key=")) { - entry->avb_key = arg; + } else if (StartsWith(flag, "avb_keys=")) { + entry->avb_keys = arg; } else { LWARNING << "Warning: unknown flag: " << flag; } @@ -759,7 +759,8 @@ FstabEntry BuildGsiSystemFstabEntry() { .fs_type = "ext4", .flags = MS_RDONLY, .fs_options = "barrier=1", - .avb_key = "/gsi.avbpubkey", + // could add more keys separated by ':'. + .avb_keys = "/avb/gsi.avbpubkey:", .logical_partition_name = "system"}; system.fs_mgr_flags.wait = true; system.fs_mgr_flags.logical = true; diff --git a/fs_mgr/include_fstab/fstab/fstab.h b/fs_mgr/include_fstab/fstab/fstab.h index a3d9fdd4a..e811447a5 100644 --- a/fs_mgr/include_fstab/fstab/fstab.h +++ b/fs_mgr/include_fstab/fstab/fstab.h @@ -55,7 +55,7 @@ struct FstabEntry { std::string zram_loopback_path; uint64_t zram_loopback_size = 512 * 1024 * 1024; // 512MB by default; std::string zram_backing_dev_path; - std::string avb_key; + std::string avb_keys; struct FsMgrFlags { bool wait : 1; diff --git a/fs_mgr/libfs_avb/avb_util.cpp b/fs_mgr/libfs_avb/avb_util.cpp index 80fa5c402..7d899021a 100644 --- a/fs_mgr/libfs_avb/avb_util.cpp +++ b/fs_mgr/libfs_avb/avb_util.cpp @@ -28,6 +28,7 @@ #include "util.h" using android::base::Basename; +using android::base::ReadFileToString; using android::base::StartsWith; using android::base::unique_fd; @@ -311,7 +312,8 @@ std::unique_ptr GetAvbFooter(int fd) { return footer; } -bool VerifyPublicKeyBlob(const uint8_t* key, size_t length, const std::string& expected_key_blob) { +bool ValidatePublicKeyBlob(const uint8_t* key, size_t length, + const std::string& expected_key_blob) { if (expected_key_blob.empty()) { // no expectation of the key, return true. return true; } @@ -324,6 +326,21 @@ bool VerifyPublicKeyBlob(const uint8_t* key, size_t length, const std::string& e return false; } +bool ValidatePublicKeyBlob(const std::string key_blob_to_validate, + const std::vector& allowed_key_paths) { + std::string allowed_key_blob; + if (key_blob_to_validate.empty()) { + LWARNING << "Failed to validate an empty key"; + return false; + } + for (const auto& path : allowed_key_paths) { + if (ReadFileToString(path, &allowed_key_blob)) { + if (key_blob_to_validate == allowed_key_blob) return true; + } + } + return false; +} + VBMetaVerifyResult VerifyVBMetaSignature(const VBMetaData& vbmeta, const std::string& expected_public_key_blob, std::string* out_public_key_data) { @@ -347,7 +364,7 @@ VBMetaVerifyResult VerifyVBMetaSignature(const VBMetaData& vbmeta, << ": Error verifying vbmeta image: failed to get public key"; return VBMetaVerifyResult::kError; } - if (!VerifyPublicKeyBlob(pk_data, pk_len, expected_public_key_blob)) { + if (!ValidatePublicKeyBlob(pk_data, pk_len, expected_public_key_blob)) { LERROR << vbmeta.partition() << ": Error verifying vbmeta image: public key used to" << " sign data does not match key in chain descriptor"; return VBMetaVerifyResult::kErrorVerification; diff --git a/fs_mgr/libfs_avb/avb_util.h b/fs_mgr/libfs_avb/avb_util.h index 5f413e33d..986a69a92 100644 --- a/fs_mgr/libfs_avb/avb_util.h +++ b/fs_mgr/libfs_avb/avb_util.h @@ -78,7 +78,10 @@ VBMetaVerifyResult VerifyVBMetaSignature(const VBMetaData& vbmeta, const std::string& expected_public_key_blob, std::string* out_public_key_data); -bool VerifyPublicKeyBlob(const uint8_t* key, size_t length, const std::string& expected_key_blob); +bool ValidatePublicKeyBlob(const uint8_t* key, size_t length, const std::string& expected_key_blob); + +bool ValidatePublicKeyBlob(const std::string key_blob_to_validate, + const std::vector& expected_key_paths); // Detects if whether a partition contains a rollback image. bool RollbackDetected(const std::string& partition_name, uint64_t rollback_index); diff --git a/fs_mgr/libfs_avb/fs_avb.cpp b/fs_mgr/libfs_avb/fs_avb.cpp index 02902f008..f0767dc47 100644 --- a/fs_mgr/libfs_avb/fs_avb.cpp +++ b/fs_mgr/libfs_avb/fs_avb.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -40,6 +41,7 @@ using android::base::Basename; using android::base::ParseUint; using android::base::ReadFileToString; +using android::base::Split; using android::base::StringPrintf; namespace android { @@ -264,8 +266,8 @@ AvbUniquePtr AvbHandle::LoadAndVerifyVbmeta( } AvbUniquePtr AvbHandle::LoadAndVerifyVbmeta(const FstabEntry& fstab_entry) { - if (fstab_entry.avb_key.empty()) { - LERROR << "avb_key=/path/to/key is missing for " << fstab_entry.mount_point; + if (fstab_entry.avb_keys.empty()) { + LERROR << "avb_keys=/path/to/key(s) is missing for " << fstab_entry.mount_point; return nullptr; } @@ -273,24 +275,14 @@ AvbUniquePtr AvbHandle::LoadAndVerifyVbmeta(const FstabEntry& fstab_entry) { bool allow_verification_error = IsDeviceUnlocked(); bool rollback_protection = !allow_verification_error; - std::string expected_key_blob; - if (!ReadFileToString(fstab_entry.avb_key, &expected_key_blob)) { - if (!allow_verification_error) { - LERROR << "Failed to load avb_key: " << fstab_entry.avb_key - << " for mount point: " << fstab_entry.mount_point; - return nullptr; - } - LWARNING << "Allowing no expected key blob when verification error is permitted"; - expected_key_blob.clear(); - } - + std::string public_key_data; bool verification_disabled = false; VBMetaVerifyResult verify_result = VBMetaVerifyResult::kError; std::unique_ptr vbmeta = LoadAndVerifyVbmetaByPath( fstab_entry.blk_device, "" /* partition_name, no need for a standalone path */, - expected_key_blob, allow_verification_error, rollback_protection, - false /* not is_chained_vbmeta */, nullptr /* out_public_key_data */, - &verification_disabled, &verify_result); + "" /* expected_public_key_blob, */, allow_verification_error, rollback_protection, + false /* not is_chained_vbmeta */, &public_key_data, &verification_disabled, + &verify_result); if (!vbmeta) { LERROR << "Failed to load vbmeta: " << fstab_entry.blk_device; @@ -316,6 +308,15 @@ AvbUniquePtr AvbHandle::LoadAndVerifyVbmeta(const FstabEntry& fstab_entry) { return nullptr; } + if (!ValidatePublicKeyBlob(public_key_data, Split(fstab_entry.avb_keys, ":"))) { + avb_handle->status_ = AvbHandleStatus::kVerificationError; + LWARNING << "Found unknown public key used to sign " << fstab_entry.mount_point; + if (!allow_verification_error) { + LERROR << "Unknown public key is not allowed"; + return nullptr; + } + } + if (verification_disabled) { LINFO << "AVB verification disabled on: " << fstab_entry.mount_point; avb_handle->status_ = AvbHandleStatus::kVerificationDisabled; diff --git a/fs_mgr/libfs_avb/tests/avb_util_test.cpp b/fs_mgr/libfs_avb/tests/avb_util_test.cpp index e4213b7b6..0d342d3e2 100644 --- a/fs_mgr/libfs_avb/tests/avb_util_test.cpp +++ b/fs_mgr/libfs_avb/tests/avb_util_test.cpp @@ -35,9 +35,9 @@ using android::fs_mgr::GetChainPartitionInfo; using android::fs_mgr::GetTotalSize; using android::fs_mgr::LoadAndVerifyVbmetaByPartition; using android::fs_mgr::LoadAndVerifyVbmetaByPath; +using android::fs_mgr::ValidatePublicKeyBlob; using android::fs_mgr::VBMetaData; using android::fs_mgr::VBMetaVerifyResult; -using android::fs_mgr::VerifyPublicKeyBlob; using android::fs_mgr::VerifyVBMetaData; using android::fs_mgr::VerifyVBMetaSignature; @@ -415,7 +415,7 @@ TEST_F(AvbUtilTest, GetVBMetaHeader) { EXPECT_EQ(content_padding.size() - padding.size(), vbmeta_padding.size()); } -TEST_F(AvbUtilTest, VerifyPublicKeyBlob) { +TEST_F(AvbUtilTest, ValidatePublicKeyBlob) { // Generates a raw key.bin const size_t key_size = 2048; base::FilePath key_path = GenerateImage("key.bin", key_size); @@ -425,12 +425,12 @@ TEST_F(AvbUtilTest, VerifyPublicKeyBlob) { std::string expected_key_blob; EXPECT_TRUE(base::ReadFileToString(key_path, &expected_key_blob)); - EXPECT_TRUE(VerifyPublicKeyBlob(key_data, key_size, expected_key_blob)); + EXPECT_TRUE(ValidatePublicKeyBlob(key_data, key_size, expected_key_blob)); key_data[10] ^= 0x80; // toggles a bit and expects a failure - EXPECT_FALSE(VerifyPublicKeyBlob(key_data, key_size, expected_key_blob)); + EXPECT_FALSE(ValidatePublicKeyBlob(key_data, key_size, expected_key_blob)); key_data[10] ^= 0x80; // toggles the bit again, should pass - EXPECT_TRUE(VerifyPublicKeyBlob(key_data, key_size, expected_key_blob)); + EXPECT_TRUE(ValidatePublicKeyBlob(key_data, key_size, expected_key_blob)); } TEST_F(AvbUtilTest, VerifyEmptyPublicKeyBlob) { @@ -442,7 +442,37 @@ TEST_F(AvbUtilTest, VerifyEmptyPublicKeyBlob) { EXPECT_EQ(key_size, base::ReadFile(key_path, (char*)key_data, key_size)); std::string expected_key_blob = ""; // empty means no expectation, thus return true. - EXPECT_TRUE(VerifyPublicKeyBlob(key_data, key_size, expected_key_blob)); + EXPECT_TRUE(ValidatePublicKeyBlob(key_data, key_size, expected_key_blob)); +} + +TEST_F(AvbUtilTest, ValidatePublicKeyBlob_MultipleAllowedKeys) { + base::FilePath rsa2048_public_key = + ExtractPublicKeyAvb(data_dir_.Append("testkey_rsa2048.pem")); + base::FilePath rsa4096_public_key = + ExtractPublicKeyAvb(data_dir_.Append("testkey_rsa4096.pem")); + base::FilePath rsa8192_public_key = + ExtractPublicKeyAvb(data_dir_.Append("testkey_rsa8192.pem")); + + std::vector allowed_key_paths; + allowed_key_paths.push_back(rsa2048_public_key.value()); + allowed_key_paths.push_back(rsa4096_public_key.value()); + + std::string expected_key_blob_2048; + EXPECT_TRUE(base::ReadFileToString(rsa2048_public_key, &expected_key_blob_2048)); + std::string expected_key_blob_4096; + EXPECT_TRUE(base::ReadFileToString(rsa4096_public_key, &expected_key_blob_4096)); + std::string expected_key_blob_8192; + EXPECT_TRUE(base::ReadFileToString(rsa8192_public_key, &expected_key_blob_8192)); + + EXPECT_TRUE(ValidatePublicKeyBlob(expected_key_blob_2048, allowed_key_paths)); + EXPECT_TRUE(ValidatePublicKeyBlob(expected_key_blob_4096, allowed_key_paths)); + + EXPECT_FALSE(ValidatePublicKeyBlob(expected_key_blob_8192, allowed_key_paths)); + EXPECT_FALSE(ValidatePublicKeyBlob("invalid_content", allowed_key_paths)); + EXPECT_FALSE(ValidatePublicKeyBlob("", allowed_key_paths)); + + allowed_key_paths.push_back(rsa8192_public_key.value()); + EXPECT_TRUE(ValidatePublicKeyBlob(expected_key_blob_8192, allowed_key_paths)); } TEST_F(AvbUtilTest, VerifyVBMetaSignature) { diff --git a/fs_mgr/libfs_avb/tests/fs_avb_device_test.cpp b/fs_mgr/libfs_avb/tests/fs_avb_device_test.cpp index 4631330cf..c8605d748 100644 --- a/fs_mgr/libfs_avb/tests/fs_avb_device_test.cpp +++ b/fs_mgr/libfs_avb/tests/fs_avb_device_test.cpp @@ -114,8 +114,8 @@ TEST(AvbHandleTest, LoadAndVerifyVbmeta_SystemOther) { // Use the 2nd fstab entry, which is for physical system_other partition. FstabEntry* system_other_entry = &fstab[1]; // Assign the default key if it's not specified in the fstab. - if (system_other_entry->avb_key.empty()) { - system_other_entry->avb_key = "/system/etc/security/avb/system_other.avbpubkey"; + if (system_other_entry->avb_keys.empty()) { + system_other_entry->avb_keys = "/system/etc/security/avb/system_other.avbpubkey"; } auto avb_handle = AvbHandle::LoadAndVerifyVbmeta(*system_other_entry); EXPECT_NE(nullptr, avb_handle) << "Failed to load system_other vbmeta. Try 'adb root'?"; diff --git a/init/first_stage_mount.cpp b/init/first_stage_mount.cpp index 6e55c11a2..17cd47077 100644 --- a/init/first_stage_mount.cpp +++ b/init/first_stage_mount.cpp @@ -760,7 +760,7 @@ bool FirstStageMountVBootV2::SetUpDmVerity(FstabEntry* fstab_entry) { if (!InitAvbHandle()) return false; hashtree_result = avb_handle_->SetUpAvbHashtree(fstab_entry, false /* wait_for_verity_dev */); - } else if (!fstab_entry->avb_key.empty()) { + } else if (!fstab_entry->avb_keys.empty()) { if (!InitAvbHandle()) return false; // Checks if hashtree should be disabled from the top-level /vbmeta. if (avb_handle_->status() == AvbHandleStatus::kHashtreeDisabled ||