Adjust CompOS case
Make sure we verify the signatures on CompOS artifacts regardless of
what 'odrefresh --check' returns, and move the check first.
Also make sure we don't use invalid CompOS artifacts if we can't
delete them.
Restore check that all the artifacts that CompOS generated are still
present - this works now we do it before running odrefresh, and while
probably not technically necessary is reassuring.
Fix redundant map copy.
These changes are designed to work both with and without the new
--compilation-os-mode switch, and I've tested both scenarios.
Bug: 211458160
Test: Manual
Change-Id: Ie6a49ab64678329d096b0f0ae70cd966ba8b3bf1
Merged-In: Ie6a49ab64678329d096b0f0ae70cd966ba8b3bf1
diff --git a/ondevice-signing/VerityUtils.cpp b/ondevice-signing/VerityUtils.cpp
index 47cba00..24a46b9 100644
--- a/ondevice-signing/VerityUtils.cpp
+++ b/ondevice-signing/VerityUtils.cpp
@@ -283,9 +283,10 @@
}
Result<void> verifyAllFilesUsingCompOs(const std::string& directory_path,
- std::map<std::string, std::string> digests,
+ const std::map<std::string, std::string>& digests,
const SigningKey& signing_key) {
std::error_code ec;
+ size_t verified_count = 0;
auto it = std::filesystem::recursive_directory_iterator(directory_path, ec);
for (auto end = std::filesystem::recursive_directory_iterator(); it != end; it.increment(ec)) {
auto& path = it->path();
@@ -305,7 +306,9 @@
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) {
+ if (verity_digest.value() == compos_digest) {
+ ++verified_count;
+ } else {
return Error() << "fs-verity digest does not match CompOS digest: " << path;
}
} else {
@@ -333,6 +336,7 @@
if (!enabled.ok()) {
return Error() << enabled.error();
}
+ ++verified_count;
}
} else if (it->is_directory()) {
// These are fine to ignore
@@ -346,6 +350,12 @@
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/include/VerityUtils.h b/ondevice-signing/include/VerityUtils.h
index 7715628..0559c35 100644
--- a/ondevice-signing/include/VerityUtils.h
+++ b/ondevice-signing/include/VerityUtils.h
@@ -34,6 +34,7 @@
android::base::Result<std::map<std::string, std::string>>
addFilesToVerityRecursive(const std::string& path, const SigningKey& key);
-android::base::Result<void> verifyAllFilesUsingCompOs(const std::string& directory_path,
- std::map<std::string, std::string> digests,
- const SigningKey& signing_key);
+android::base::Result<void>
+verifyAllFilesUsingCompOs(const std::string& directory_path,
+ const std::map<std::string, std::string>& digests,
+ const SigningKey& signing_key);
diff --git a/ondevice-signing/odsign_main.cpp b/ondevice-signing/odsign_main.cpp
index 4e9905a..a324857 100644
--- a/ondevice-signing/odsign_main.cpp
+++ b/ondevice-signing/odsign_main.cpp
@@ -525,43 +525,46 @@
return art::odrefresh::ExitCode::kCompilationRequired;
}
- // 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.)
+ // Make sure the artifacts we have are genuinely produced by the current
+ // instance of CompOS.
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());
auto status = verifyAllFilesUsingCompOs(kArtArtifactsDir, compos_digests, signing_key);
if (!status.ok()) {
- LOG(WARNING) << status.error();
+ LOG(WARNING) << "Faild to verify CompOS artifacts: " << status.error();
} else {
- auto persisted = persistDigests(compos_digests, signing_key);
-
- if (!persisted.ok()) {
- LOG(WARNING) << persisted.error();
- } else {
- LOG(INFO) << "Pending artifacts successfully verified.";
+ LOG(INFO) << "CompOS artifacts successfully verified.";
+ odrefresh_status = checkArtifacts();
+ if (odrefresh_status == art::odrefresh::ExitCode::kOkay) {
+ // We have digests of all the files, and they aren't going to change, so
+ // we can just sign them & save them now, and skip checking them later.
+ auto persisted = persistDigests(compos_digests, signing_key);
+ if (!persisted.ok()) {
+ LOG(ERROR) << persisted.error();
+ // Don't try to compile again - if we can't write the digests, things
+ // are pretty bad.
+ return art::odrefresh::ExitCode::kCleanupFailed;
+ }
+ LOG(INFO) << "Persisted CompOS digests.";
*digests_verified = true;
- return art::odrefresh::ExitCode::kOkay;
}
+ return odrefresh_status;
}
}
// We can't use the existing artifacts, so we will need to generate new
// ones.
- removeDirectory(kArtArtifactsDir);
+ if (removeDirectory(kArtArtifactsDir) == 0) {
+ // We have unsigned artifacts that we can't delete, so it's not safe to continue.
+ LOG(ERROR) << "Unable to delete invalid CompOS artifacts";
+ return art::odrefresh::ExitCode::kCleanupFailed;
+ }
+
return art::odrefresh::ExitCode::kCompilationRequired;
}
} // namespace
