Fix checking of compos.info
We now load the file and its signature before odrefresh deletes them
rather than after.
And remove the check that all files listed are still
present. odrefresh can and does delete some if it decides they are not
needed, and that's fine. odrefresh will also complain if files that
should be present are not, so we will still detect if an attacker has
deleted artifacts.
Bug: 161471326
Bug: 209572241
Test: composd_cmd staged-apex-compile, reboot, odsign accepts artifacts
Change-Id: I449bd19a691c6070ce5b7295a5e4c8a7a61edfbd
diff --git a/ondevice-signing/VerityUtils.cpp b/ondevice-signing/VerityUtils.cpp
index b3b7520..47cba00 100644
--- a/ondevice-signing/VerityUtils.cpp
+++ b/ondevice-signing/VerityUtils.cpp
@@ -287,7 +287,6 @@
const SigningKey& signing_key) {
std::error_code ec;
auto it = std::filesystem::recursive_directory_iterator(directory_path, ec);
- size_t verified_count = 0;
for (auto end = std::filesystem::recursive_directory_iterator(); it != end; it.increment(ec)) {
auto& path = it->path();
if (it->is_regular_file()) {
@@ -306,9 +305,7 @@
if (verity_digest.ok()) {
// The file is already in fs-verity. We need to make sure it was signed
// by CompOS, so we just check that it has the digest we expect.
- if (verity_digest.value() == compos_digest) {
- ++verified_count;
- } else {
+ if (verity_digest.value() != compos_digest) {
return Error() << "fs-verity digest does not match CompOS digest: " << path;
}
} else {
@@ -336,7 +333,6 @@
if (!enabled.ok()) {
return Error() << enabled.error();
}
- ++verified_count;
}
} else if (it->is_directory()) {
// These are fine to ignore
@@ -349,10 +345,6 @@
if (ec) {
return Error() << "Failed to iterate " << directory_path << ": " << ec.message();
}
- // Make sure all the files we expected have been seen
- if (verified_count != digests.size()) {
- return Error() << "Verified " << verified_count << "files, but expected " << digests.size();
- }
return {};
}
diff --git a/ondevice-signing/odsign_main.cpp b/ondevice-signing/odsign_main.cpp
index 2885176..0433a3e 100644
--- a/ondevice-signing/odsign_main.cpp
+++ b/ondevice-signing/odsign_main.cpp
@@ -531,19 +531,21 @@
return art::odrefresh::ExitCode::kCompilationRequired;
}
- odrefresh_status = checkArtifacts();
- if (odrefresh_status != art::odrefresh::ExitCode::kOkay) {
- LOG(WARNING) << "Pending artifacts are not OK";
- return odrefresh_status;
- }
-
- // The artifacts appear to be up to date - but we haven't
- // verified that they are genuine yet.
-
+ // Read the CompOS signature before checking, otherwise odrefresh will delete it
+ // (And there's no point checking the artifacts if we don't have a valid signature.)
auto compos_info = getComposInfo(compos_key);
if (!compos_info.ok()) {
LOG(WARNING) << compos_info.error();
} else {
+ odrefresh_status = checkArtifacts();
+ if (odrefresh_status != art::odrefresh::ExitCode::kOkay) {
+ LOG(WARNING) << "Pending artifacts are not OK";
+ return odrefresh_status;
+ }
+
+ // The artifacts appear to be up to date - but we haven't
+ // verified that they are genuine yet.
+
std::map<std::string, std::string> compos_digests(compos_info->file_hashes().begin(),
compos_info->file_hashes().end());