Fix idmap scan to supply correct policies

Overlayable policies were not being passed correctly to idmap2 create
from scan. This fixes that and adds better error messages for when
policy failures occur.

Bug: 127860892
Test: manual
Change-Id: I8fae20884a75f4c57a0eb4aafdb4e09da3ebaf93

4 files changed, 104 insertions, 40 deletions

diff --git a/cmds/idmap2/idmap2/Scan.cpp b/cmds/idmap2/idmap2/Scan.cpp
index e5f6223ef2b4..e85f132c6072 100644
--- a/cmds/idmap2/idmap2/Scan.cpp
+++ b/cmds/idmap2/idmap2/Scan.cpp
@@ -38,7 +38,10 @@
 
 using android::idmap2::CommandLineOptions;
 using android::idmap2::Idmap;
-using android::idmap2::PoliciesToBitmask;
+using android::idmap2::kPolicyProduct;
+using android::idmap2::kPolicyPublic;
+using android::idmap2::kPolicySystem;
+using android::idmap2::kPolicyVendor;
 using android::idmap2::PolicyBitmask;
 using android::idmap2::PolicyFlags;
 using android::idmap2::Result;
@@ -87,20 +90,22 @@ std::unique_ptr<std::vector<std::string>> FindApkFiles(const std::vector<std::st
   return std::make_unique<std::vector<std::string>>(paths.cbegin(), paths.cend());
 }
 
-PolicyBitmask PolicyForPath(const std::string& apk_path) {
-  static const std::vector<std::pair<std::string, PolicyBitmask>> values = {
-      {"/product/", PolicyFlags::POLICY_PRODUCT_PARTITION},
-      {"/system/", PolicyFlags::POLICY_SYSTEM_PARTITION},
-      {"/vendor/", PolicyFlags::POLICY_VENDOR_PARTITION},
+std::vector<std::string> PoliciesForPath(const std::string& apk_path) {
+  static const std::vector<std::pair<std::string, std::string>> values = {
+      {"/product/", kPolicyProduct},
+      {"/system/", kPolicySystem},
+      {"/vendor/", kPolicyVendor},
   };
 
+  std::vector<std::string> fulfilled_policies = {kPolicyPublic};
   for (auto const& pair : values) {
     if (apk_path.compare(0, pair.first.size(), pair.first) == 0) {
-      return pair.second | PolicyFlags::POLICY_PUBLIC;
+      fulfilled_policies.emplace_back(pair.second);
+      break;
     }
   }
 
-  return PolicyFlags::POLICY_PUBLIC;
+  return fulfilled_policies;
 }
 
 }  // namespace
@@ -161,21 +166,17 @@ bool Scan(const std::vector<std::string>& args, std::ostream& out_error) {
       continue;
     }
 
-    PolicyBitmask fulfilled_policies;
+    std::vector<std::string> fulfilled_policies;
     if (!override_policies.empty()) {
-      auto conv_result = PoliciesToBitmask(override_policies);
-      if (conv_result) {
-        fulfilled_policies = *conv_result;
-      } else {
-        out_error << "error: " << conv_result.GetErrorMessage() << std::endl;
-        return false;
-      }
+      fulfilled_policies = override_policies;
     } else {
-      fulfilled_policies = PolicyForPath(path);
+      fulfilled_policies = PoliciesForPath(path);
     }
 
     bool ignore_overlayable = false;
-    if ((fulfilled_policies & PolicyFlags::POLICY_VENDOR_PARTITION) != 0 && !VendorIsQOrLater()) {
+    if (std::find(fulfilled_policies.begin(), fulfilled_policies.end(), kPolicyVendor) !=
+            fulfilled_policies.end() &&
+        !VendorIsQOrLater()) {
       // If the overlay is on a pre-Q vendor partition, do not enforce overlayable
       // restrictions on this overlay because the pre-Q platform has no understanding of
       // overlayable.
@@ -185,7 +186,7 @@ bool Scan(const std::vector<std::string>& args, std::ostream& out_error) {
     std::string idmap_path = Idmap::CanonicalIdmapPathFor(output_directory, path);
 
     // Sort the static overlays in ascending priority order
-    InputOverlay input{path, idmap_path, overlay_info->priority, override_policies,
+    InputOverlay input{path, idmap_path, overlay_info->priority, fulfilled_policies,
                        ignore_overlayable};
     interesting_apks.insert(
         std::lower_bound(interesting_apks.begin(), interesting_apks.end(), input), input);
@@ -211,8 +212,8 @@ bool Scan(const std::vector<std::string>& args, std::ostream& out_error) {
       }
 
       for (const std::string& policy : overlay.policies) {
-        verify_args.emplace_back("--policy");
-        verify_args.emplace_back(policy);
+        create_args.emplace_back("--policy");
+        create_args.emplace_back(policy);
       }
 
       if (!Create(create_args, out_error)) {
diff --git a/cmds/idmap2/include/idmap2/Policies.h b/cmds/idmap2/include/idmap2/Policies.h
index 911d3f2cd109..cd76b84ccc0a 100644
--- a/cmds/idmap2/include/idmap2/Policies.h
+++ b/cmds/idmap2/include/idmap2/Policies.h
@@ -27,13 +27,21 @@
 
 namespace android::idmap2 {
 
+constexpr const char* kPolicyPublic = "public";
+constexpr const char* kPolicyProduct = "product";
+constexpr const char* kPolicySystem = "system";
+constexpr const char* kPolicyVendor = "vendor";
+constexpr const char* kPolicySignature = "signature";
+
 using PolicyFlags = ResTable_overlayable_policy_header::PolicyFlags;
 using PolicyBitmask = uint32_t;
 
-// Parses a the string representation of a set of policies into a bitmask. The format of the string
-// is the same as for the <policy> element.
+// Parses the string representations of policies into a bitmask.
 Result<PolicyBitmask> PoliciesToBitmask(const std::vector<std::string>& policies);
 
+// Retrieves the string representations of policies in the bitmask.
+std::vector<std::string> BitmaskToPolicies(const PolicyBitmask& bitmask);
+
 }  // namespace android::idmap2
 
 #endif  // IDMAP2_INCLUDE_IDMAP2_POLICIES_H_
diff --git a/cmds/idmap2/libidmap2/Idmap.cpp b/cmds/idmap2/libidmap2/Idmap.cpp
index f4c3e9ca180f..a927a69b2585 100644
--- a/cmds/idmap2/libidmap2/Idmap.cpp
+++ b/cmds/idmap2/libidmap2/Idmap.cpp
@@ -284,9 +284,23 @@ std::unique_ptr<const Idmap> Idmap::FromBinaryStream(std::istream& stream,
   return std::move(idmap);
 }
 
-bool CheckOverlayable(const LoadedPackage& target_package,
-                      const utils::OverlayManifestInfo& overlay_info,
-                      const PolicyBitmask& fulfilled_policies, const ResourceId& resid) {
+std::string ConcatPolicies(const std::vector<std::string>& policies) {
+  std::string message;
+  for (const std::string& policy : policies) {
+    if (message.empty()) {
+      message.append(policy);
+    } else {
+      message.append(policy);
+      message.append("|");
+    }
+  }
+
+  return message;
+}
+
+Result<Unit> CheckOverlayable(const LoadedPackage& target_package,
+                              const utils::OverlayManifestInfo& overlay_info,
+                              const PolicyBitmask& fulfilled_policies, const ResourceId& resid) {
   static constexpr const PolicyBitmask sDefaultPolicies =
       PolicyFlags::POLICY_SYSTEM_PARTITION | PolicyFlags::POLICY_VENDOR_PARTITION |
       PolicyFlags::POLICY_PRODUCT_PARTITION | PolicyFlags::POLICY_SIGNATURE;
@@ -294,23 +308,34 @@ bool CheckOverlayable(const LoadedPackage& target_package,
   // If the resource does not have an overlayable definition, allow the resource to be overlaid if
   // the overlay is preinstalled or signed with the same signature as the target.
   if (!target_package.DefinesOverlayable()) {
-    return (sDefaultPolicies & fulfilled_policies) != 0;
+    return (sDefaultPolicies & fulfilled_policies) != 0
+               ? Result<Unit>({})
+               : Error(
+                     "overlay must be preinstalled or signed with the same signature as the "
+                     "target");
   }
 
   const OverlayableInfo* overlayable_info = target_package.GetOverlayableInfo(resid);
   if (overlayable_info == nullptr) {
     // Do not allow non-overlayable resources to be overlaid.
-    return false;
+    return Error("resource has no overlayable declaration");
   }
 
   if (overlay_info.target_name != overlayable_info->name) {
     // If the overlay supplies a target overlayable name, the resource must belong to the
     // overlayable defined with the specified name to be overlaid.
-    return false;
+    return Error("<overlay> android:targetName '%s' does not match overlayable name '%s'",
+                 overlay_info.target_name.c_str(), overlayable_info->name.c_str());
   }
 
   // Enforce policy restrictions if the resource is declared as overlayable.
-  return (overlayable_info->policy_flags & fulfilled_policies) != 0;
+  if ((overlayable_info->policy_flags & fulfilled_policies) == 0) {
+    return Error("overlay with policies '%s' does not fulfill any overlayable policies '%s'",
+                 ConcatPolicies(BitmaskToPolicies(fulfilled_policies)).c_str(),
+                 ConcatPolicies(BitmaskToPolicies(overlayable_info->policy_flags)).c_str());
+  }
+
+  return Result<Unit>({});
 }
 
 std::unique_ptr<const Idmap> Idmap::FromApkAssets(
@@ -426,11 +451,15 @@ std::unique_ptr<const Idmap> Idmap::FromApkAssets(
       continue;
     }
 
-    if (enforce_overlayable &&
-        !CheckOverlayable(*target_pkg, *overlay_info, fulfilled_policies, target_resid)) {
-      LOG(WARNING) << "overlay \"" << overlay_apk_path << "\" is not allowed to overlay resource \""
-                   << full_name << "\"" << std::endl;
-      continue;
+    if (!enforce_overlayable) {
+      Result<Unit> success =
+          CheckOverlayable(*target_pkg, *overlay_info, fulfilled_policies, target_resid);
+      if (!success) {
+        LOG(WARNING) << "overlay \"" << overlay_apk_path
+                     << "\" is not allowed to overlay resource \"" << full_name
+                     << "\": " << success.GetErrorMessage();
+        continue;
+      }
     }
 
     matching_resources.Add(target_resid, overlay_resid);
diff --git a/cmds/idmap2/libidmap2/Policies.cpp b/cmds/idmap2/libidmap2/Policies.cpp
index c6ba87dfc2fb..7c4555633fd2 100644
--- a/cmds/idmap2/libidmap2/Policies.cpp
+++ b/cmds/idmap2/libidmap2/Policies.cpp
@@ -30,12 +30,13 @@ namespace android::idmap2 {
 namespace {
 
 const std::map<android::StringPiece, PolicyFlags> kStringToFlag = {
-    {"public", PolicyFlags::POLICY_PUBLIC},
-    {"product", PolicyFlags::POLICY_PRODUCT_PARTITION},
-    {"system", PolicyFlags::POLICY_SYSTEM_PARTITION},
-    {"vendor", PolicyFlags::POLICY_VENDOR_PARTITION},
-    {"signature", PolicyFlags::POLICY_SIGNATURE},
+    {kPolicyPublic, PolicyFlags::POLICY_PUBLIC},
+    {kPolicyProduct, PolicyFlags::POLICY_PRODUCT_PARTITION},
+    {kPolicySystem, PolicyFlags::POLICY_SYSTEM_PARTITION},
+    {kPolicyVendor, PolicyFlags::POLICY_VENDOR_PARTITION},
+    {kPolicySignature, PolicyFlags::POLICY_SIGNATURE},
 };
+
 }  // namespace
 
 Result<PolicyBitmask> PoliciesToBitmask(const std::vector<std::string>& policies) {
@@ -52,4 +53,29 @@ Result<PolicyBitmask> PoliciesToBitmask(const std::vector<std::string>& policies
   return Result<PolicyBitmask>(bitmask);
 }
 
+std::vector<std::string> BitmaskToPolicies(const PolicyBitmask& bitmask) {
+  std::vector<std::string> policies;
+  if ((bitmask & PolicyFlags::POLICY_PUBLIC) != 0) {
+    policies.emplace_back(kPolicyPublic);
+  }
+
+  if ((bitmask & PolicyFlags::POLICY_PRODUCT_PARTITION) != 0) {
+    policies.emplace_back(kPolicyProduct);
+  }
+
+  if ((bitmask & PolicyFlags::POLICY_SYSTEM_PARTITION) != 0) {
+    policies.emplace_back(kPolicySystem);
+  }
+
+  if ((bitmask & PolicyFlags::POLICY_VENDOR_PARTITION) != 0) {
+    policies.emplace_back(kPolicyVendor);
+  }
+
+  if ((bitmask & PolicyFlags::POLICY_SIGNATURE) != 0) {
+    policies.emplace_back(kPolicySignature);
+  }
+
+  return policies;
+}
+
 }  // namespace android::idmap2