Merge "Adjust CompOS case"
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