idmap2: Set aidl_return to true on success

idmap2d did not set the aidl return value to true when
IdmapHeader::IsUpToDate succeeded. This regressed OMS initialization
time.

This change sets aidl_return to true when the header is up-to-date and
also replaces usages of `uint32_t` for fulfilled_policies with
`PolicyBitmask`.

Bug: 158491243
Bug: 158535227
Bug: 158468195
Test: enable/disable overlays and onserve that verify succeeds
Test: change the values of enforce overlayable and fulfilled policies
      and confirm that idmap verify fails
Change-Id: If8febbfd0283d53211263690f85e2970706a33b7

5 files changed, 18 insertions, 19 deletions

diff --git a/cmds/idmap2/idmap2/CommandUtils.cpp b/cmds/idmap2/idmap2/CommandUtils.cpp
index e058cd6e7e70..8f5845bf2e53 100644
--- a/cmds/idmap2/idmap2/CommandUtils.cpp
+++ b/cmds/idmap2/idmap2/CommandUtils.cpp
@@ -29,7 +29,7 @@ using android::idmap2::Result;
 using android::idmap2::Unit;
 
 Result<Unit> Verify(const std::string& idmap_path, const std::string& target_path,
-                    const std::string& overlay_path, uint32_t fulfilled_policies,
+                    const std::string& overlay_path, PolicyBitmask fulfilled_policies,
                     bool enforce_overlayable) {
   SYSTRACE << "Verify " << idmap_path;
   std::ifstream fin(idmap_path);
diff --git a/cmds/idmap2/idmap2/CommandUtils.h b/cmds/idmap2/idmap2/CommandUtils.h
index 99605de30988..e717e046d15d 100644
--- a/cmds/idmap2/idmap2/CommandUtils.h
+++ b/cmds/idmap2/idmap2/CommandUtils.h
@@ -17,12 +17,13 @@
 #ifndef IDMAP2_IDMAP2_COMMAND_UTILS_H_
 #define IDMAP2_IDMAP2_COMMAND_UTILS_H_
 
+#include "idmap2/PolicyUtils.h"
 #include "idmap2/Result.h"
 
 android::idmap2::Result<android::idmap2::Unit> Verify(const std::string& idmap_path,
                                                       const std::string& target_path,
                                                       const std::string& overlay_path,
-                                                      uint32_t fulfilled_policies,
+                                                      PolicyBitmask fulfilled_policies,
                                                       bool enforce_overlayable);
 
 #endif  // IDMAP2_IDMAP2_COMMAND_UTILS_H_
diff --git a/cmds/idmap2/idmap2d/Idmap2Service.cpp b/cmds/idmap2/idmap2d/Idmap2Service.cpp
index 908d96612269..f95b73f17222 100644
--- a/cmds/idmap2/idmap2d/Idmap2Service.cpp
+++ b/cmds/idmap2/idmap2d/Idmap2Service.cpp
@@ -70,12 +70,12 @@ PolicyBitmask ConvertAidlArgToPolicyBitmask(int32_t arg) {
 }
 
 Status GetCrc(const std::string& apk_path, uint32_t* out_crc) {
-  const auto overlay_zip = ZipFile::Open(apk_path);
-  if (!overlay_zip) {
+  const auto zip = ZipFile::Open(apk_path);
+  if (!zip) {
     return error(StringPrintf("failed to open apk %s", apk_path.c_str()));
   }
 
-  const auto crc = GetPackageCrc(*overlay_zip);
+  const auto crc = GetPackageCrc(*zip);
   if (!crc) {
     return error(crc.GetErrorMessage());
   }
@@ -121,6 +121,7 @@ Status Idmap2Service::verifyIdmap(const std::string& target_apk_path,
                                   bool* _aidl_return) {
   SYSTRACE << "Idmap2Service::verifyIdmap " << overlay_apk_path;
   assert(_aidl_return);
+
   const std::string idmap_path = Idmap::CanonicalIdmapPathFor(kIdmapCacheDir, overlay_apk_path);
   std::ifstream fin(idmap_path);
   const std::unique_ptr<const IdmapHeader> header = IdmapHeader::FromBinaryStream(fin);
@@ -156,13 +157,10 @@ Status Idmap2Service::verifyIdmap(const std::string& target_apk_path,
 
   auto up_to_date =
       header->IsUpToDate(target_apk_path.c_str(), overlay_apk_path.c_str(), target_crc, overlay_crc,
-                         fulfilled_policies, enforce_overlayable);
-  if (!up_to_date) {
-    *_aidl_return = false;
-    return error(up_to_date.GetErrorMessage());
-  }
+                         ConvertAidlArgToPolicyBitmask(fulfilled_policies), enforce_overlayable);
 
-  return ok();
+  *_aidl_return = static_cast<bool>(up_to_date);
+  return *_aidl_return ? ok() : error(up_to_date.GetErrorMessage());
 }
 
 Status Idmap2Service::createIdmap(const std::string& target_apk_path,
diff --git a/cmds/idmap2/include/idmap2/Idmap.h b/cmds/idmap2/include/idmap2/Idmap.h
index 8f25b8d6a734..0f05592b70f3 100644
--- a/cmds/idmap2/include/idmap2/Idmap.h
+++ b/cmds/idmap2/include/idmap2/Idmap.h
@@ -141,9 +141,9 @@ class IdmapHeader {
   // field *must* be incremented. Because of this, we know that if the idmap
   // header is up-to-date the entire file is up-to-date.
   Result<Unit> IsUpToDate(const char* target_path, const char* overlay_path,
-                          uint32_t fulfilled_policies, bool enforce_overlayable) const;
+                          PolicyBitmask fulfilled_policies, bool enforce_overlayable) const;
   Result<Unit> IsUpToDate(const char* target_path, const char* overlay_path, uint32_t target_crc,
-                          uint32_t overlay_crc, uint32_t fulfilled_policies,
+                          uint32_t overlay_crc, PolicyBitmask fulfilled_policies,
                           bool enforce_overlayable) const;
 
   void accept(Visitor* v) const;
diff --git a/cmds/idmap2/libidmap2/Idmap.cpp b/cmds/idmap2/libidmap2/Idmap.cpp
index 0bea21735b24..23c25a7089de 100644
--- a/cmds/idmap2/libidmap2/Idmap.cpp
+++ b/cmds/idmap2/libidmap2/Idmap.cpp
@@ -115,8 +115,7 @@ std::unique_ptr<const IdmapHeader> IdmapHeader::FromBinaryStream(std::istream& s
   uint8_t enforce_overlayable;
   if (!Read32(stream, &idmap_header->magic_) || !Read32(stream, &idmap_header->version_) ||
       !Read32(stream, &idmap_header->target_crc_) || !Read32(stream, &idmap_header->overlay_crc_) ||
-      !Read32(stream, &idmap_header->fulfilled_policies_) ||
-      !Read8(stream, &enforce_overlayable) ||
+      !Read32(stream, &idmap_header->fulfilled_policies_) || !Read8(stream, &enforce_overlayable) ||
       !ReadString256(stream, idmap_header->target_path_) ||
       !ReadString256(stream, idmap_header->overlay_path_)) {
     return nullptr;
@@ -134,7 +133,8 @@ std::unique_ptr<const IdmapHeader> IdmapHeader::FromBinaryStream(std::istream& s
 }
 
 Result<Unit> IdmapHeader::IsUpToDate(const char* target_path, const char* overlay_path,
-                                     uint32_t fulfilled_policies, bool enforce_overlayable) const {
+                                     PolicyBitmask fulfilled_policies,
+                                     bool enforce_overlayable) const {
   const std::unique_ptr<const ZipFile> target_zip = ZipFile::Open(target_path);
   if (!target_zip) {
     return Error("failed to open target %s", target_path);
@@ -161,7 +161,8 @@ Result<Unit> IdmapHeader::IsUpToDate(const char* target_path, const char* overla
 
 Result<Unit> IdmapHeader::IsUpToDate(const char* target_path, const char* overlay_path,
                                      uint32_t target_crc, uint32_t overlay_crc,
-                                     uint32_t fulfilled_policies, bool enforce_overlayable) const {
+                                     PolicyBitmask fulfilled_policies,
+                                     bool enforce_overlayable) const {
   if (magic_ != kIdmapMagic) {
     return Error("bad magic: actual 0x%08x, expected 0x%08x", magic_, kIdmapMagic);
   }
@@ -187,8 +188,7 @@ Result<Unit> IdmapHeader::IsUpToDate(const char* target_path, const char* overla
 
   if (enforce_overlayable != enforce_overlayable_) {
     return Error("bad enforce overlayable: idmap version %s, file system version %s",
-                 enforce_overlayable ? "true" : "false",
-                 enforce_overlayable_ ? "true" : "false");
+                 enforce_overlayable ? "true" : "false", enforce_overlayable_ ? "true" : "false");
   }
 
   if (strcmp(target_path, target_path_) != 0) {