Actor signature overlayable policy
There are cases where an app can ship overlays for itself,
but the "signature" policy as described would open up
a vulnerability by allowing the system actor to create
and sign any arbitrary overlay that will apply to the target.
To prevent this, redefine "signature" as target package only,
and introduce "actor" for checking against the actor signature.
Any app that wishes to use both can include both policies.
Bug: 130563563
Test: m aapt2_tests idmapt2_tests and run from host test output
Test: atest libandroidfw_tests
Change-Id: I1c583a5b37f4abbeb18fc6a35c502377d8977a41
diff --git a/cmds/idmap2/tests/IdmapTests.cpp b/cmds/idmap2/tests/IdmapTests.cpp
index 76c6eaf..87da36c 100644
--- a/cmds/idmap2/tests/IdmapTests.cpp
+++ b/cmds/idmap2/tests/IdmapTests.cpp
@@ -279,17 +279,25 @@
const auto& target_entries = data->GetTargetEntries();
ASSERT_EQ(target_entries.size(), 4U);
- ASSERT_TARGET_ENTRY(target_entries[0], 0x7f010000, Res_value::TYPE_DYNAMIC_REFERENCE, 0x00010000);
- ASSERT_TARGET_ENTRY(target_entries[1], 0x7f02000c, Res_value::TYPE_DYNAMIC_REFERENCE, 0x00020000);
- ASSERT_TARGET_ENTRY(target_entries[2], 0x7f02000e, Res_value::TYPE_DYNAMIC_REFERENCE, 0x00020001);
- ASSERT_TARGET_ENTRY(target_entries[3], 0x7f02000f, Res_value::TYPE_DYNAMIC_REFERENCE, 0x00020002);
+ ASSERT_TARGET_ENTRY(target_entries[0], R::target::integer::int1,
+ Res_value::TYPE_DYNAMIC_REFERENCE, R::overlay_shared::integer::int1);
+ ASSERT_TARGET_ENTRY(target_entries[1], R::target::string::str1, Res_value::TYPE_DYNAMIC_REFERENCE,
+ R::overlay_shared::string::str1);
+ ASSERT_TARGET_ENTRY(target_entries[2], R::target::string::str3, Res_value::TYPE_DYNAMIC_REFERENCE,
+ R::overlay_shared::string::str3);
+ ASSERT_TARGET_ENTRY(target_entries[3], R::target::string::str4, Res_value::TYPE_DYNAMIC_REFERENCE,
+ R::overlay_shared::string::str4);
const auto& overlay_entries = data->GetOverlayEntries();
ASSERT_EQ(target_entries.size(), 4U);
- ASSERT_OVERLAY_ENTRY(overlay_entries[0], 0x00010000, 0x7f010000);
- ASSERT_OVERLAY_ENTRY(overlay_entries[1], 0x00020000, 0x7f02000c);
- ASSERT_OVERLAY_ENTRY(overlay_entries[2], 0x00020001, 0x7f02000e);
- ASSERT_OVERLAY_ENTRY(overlay_entries[3], 0x00020002, 0x7f02000f);
+ ASSERT_OVERLAY_ENTRY(overlay_entries[0], R::overlay_shared::integer::int1,
+ R::target::integer::int1);
+ ASSERT_OVERLAY_ENTRY(overlay_entries[1], R::overlay_shared::string::str1,
+ R::target::string::str1);
+ ASSERT_OVERLAY_ENTRY(overlay_entries[2], R::overlay_shared::string::str3,
+ R::target::string::str3);
+ ASSERT_OVERLAY_ENTRY(overlay_entries[3], R::overlay_shared::string::str4,
+ R::target::string::str4);
}
TEST(IdmapTests, CreateIdmapDataDoNotRewriteNonOverlayResourceId) {
diff --git a/cmds/idmap2/tests/PoliciesTests.cpp b/cmds/idmap2/tests/PoliciesTests.cpp
index 4b395c7..1b27759 100644
--- a/cmds/idmap2/tests/PoliciesTests.cpp
+++ b/cmds/idmap2/tests/PoliciesTests.cpp
@@ -67,6 +67,14 @@
const auto bitmask10 = PoliciesToBitmaskResult({"system "});
ASSERT_FALSE(bitmask10);
+
+ const auto bitmask11 = PoliciesToBitmaskResult({"signature"});
+ ASSERT_TRUE(bitmask11);
+ ASSERT_EQ(*bitmask11, PolicyFlags::SIGNATURE);
+
+ const auto bitmask12 = PoliciesToBitmaskResult({"actor"});
+ ASSERT_TRUE(bitmask12);
+ ASSERT_EQ(*bitmask12, PolicyFlags::ACTOR_SIGNATURE);
}
TEST(PoliciesTests, BitmaskToPolicies) {
@@ -91,6 +99,14 @@
ASSERT_EQ(policies3[3], "public");
ASSERT_EQ(policies3[4], "system");
ASSERT_EQ(policies3[5], "vendor");
+
+ const auto policies4 = BitmaskToPolicies(PolicyFlags::SIGNATURE);
+ ASSERT_EQ(1, policies4.size());
+ ASSERT_EQ(policies4[0], "signature");
+
+ const auto policies5 = BitmaskToPolicies(PolicyFlags::ACTOR_SIGNATURE);
+ ASSERT_EQ(1, policies5.size());
+ ASSERT_EQ(policies5[0], "actor");
}
} // namespace android::idmap2
diff --git a/cmds/idmap2/tests/R.h b/cmds/idmap2/tests/R.h
index b2a5891..aed263a 100644
--- a/cmds/idmap2/tests/R.h
+++ b/cmds/idmap2/tests/R.h
@@ -40,16 +40,17 @@
namespace string {
constexpr ResourceId not_overlayable = 0x7f020003;
constexpr ResourceId other = 0x7f020004;
- constexpr ResourceId policy_odm = 0x7f020005;
- constexpr ResourceId policy_oem = 0x7f020006;
- constexpr ResourceId policy_product = 0x7f020007;
- constexpr ResourceId policy_public = 0x7f020008;
- constexpr ResourceId policy_signature = 0x7f020009;
- constexpr ResourceId policy_system = 0x7f02000a;
- constexpr ResourceId policy_system_vendor = 0x7f02000b;
- constexpr ResourceId str1 = 0x7f02000c;
- constexpr ResourceId str3 = 0x7f02000e;
- constexpr ResourceId str4 = 0x7f02000f;
+ constexpr ResourceId policy_actor = 0x7f020005;
+ constexpr ResourceId policy_odm = 0x7f020006;
+ constexpr ResourceId policy_oem = 0x7f020007;
+ constexpr ResourceId policy_product = 0x7f020008;
+ constexpr ResourceId policy_public = 0x7f020009;
+ constexpr ResourceId policy_signature = 0x7f02000a;
+ constexpr ResourceId policy_system = 0x7f02000b;
+ constexpr ResourceId policy_system_vendor = 0x7f02000c;
+ constexpr ResourceId str1 = 0x7f02000d;
+ constexpr ResourceId str3 = 0x7f02000f;
+ constexpr ResourceId str4 = 0x7f020010;
namespace literal {
inline const std::string str1 = hexify(R::target::string::str1);
@@ -70,6 +71,17 @@
}
}
+namespace R::overlay_shared {
+ namespace integer {
+ constexpr ResourceId int1 = 0x00010000;
+ }
+ namespace string {
+ constexpr ResourceId str1 = 0x00020000;
+ constexpr ResourceId str3 = 0x00020001;
+ constexpr ResourceId str4 = 0x00020002;
+ }
+}
+
namespace R::system_overlay::string {
constexpr ResourceId policy_public = 0x7f010000;
constexpr ResourceId policy_system = 0x7f010001;
@@ -79,13 +91,14 @@
namespace R::system_overlay_invalid::string {
constexpr ResourceId not_overlayable = 0x7f010000;
constexpr ResourceId other = 0x7f010001;
- constexpr ResourceId policy_odm = 0x7f010002;
- constexpr ResourceId policy_oem = 0x7f010003;
- constexpr ResourceId policy_product = 0x7f010004;
- constexpr ResourceId policy_public = 0x7f010005;
- constexpr ResourceId policy_signature = 0x7f010006;
- constexpr ResourceId policy_system = 0x7f010007;
- constexpr ResourceId policy_system_vendor = 0x7f010008;
+ constexpr ResourceId policy_actor = 0x7f010002;
+ constexpr ResourceId policy_odm = 0x7f010003;
+ constexpr ResourceId policy_oem = 0x7f010004;
+ constexpr ResourceId policy_product = 0x7f010005;
+ constexpr ResourceId policy_public = 0x7f010006;
+ constexpr ResourceId policy_signature = 0x7f010007;
+ constexpr ResourceId policy_system = 0x7f010008;
+ constexpr ResourceId policy_system_vendor = 0x7f010009;
};
// clang-format on
diff --git a/cmds/idmap2/tests/ResourceMappingTests.cpp b/cmds/idmap2/tests/ResourceMappingTests.cpp
index 8d0e660..de039f4 100644
--- a/cmds/idmap2/tests/ResourceMappingTests.cpp
+++ b/cmds/idmap2/tests/ResourceMappingTests.cpp
@@ -237,12 +237,15 @@
ASSERT_TRUE(resources) << resources.GetErrorMessage();
auto& res = *resources;
- ASSERT_EQ(res.GetTargetToOverlayMap().size(), 9U);
+ ASSERT_EQ(res.GetTargetToOverlayMap().size(), 10U);
ASSERT_RESULT(MappingExists(res, R::target::string::not_overlayable, Res_value::TYPE_REFERENCE,
R::system_overlay_invalid::string::not_overlayable,
false /* rewrite */));
ASSERT_RESULT(MappingExists(res, R::target::string::other, Res_value::TYPE_REFERENCE,
R::system_overlay_invalid::string::other, false /* rewrite */));
+ ASSERT_RESULT(MappingExists(res, R::target::string::policy_actor, Res_value::TYPE_REFERENCE,
+ R::system_overlay_invalid::string::policy_actor,
+ false /* rewrite */));
ASSERT_RESULT(MappingExists(res, R::target::string::policy_odm, Res_value::TYPE_REFERENCE,
R::system_overlay_invalid::string::policy_odm, false /* rewrite */));
ASSERT_RESULT(MappingExists(res, R::target::string::policy_oem, Res_value::TYPE_REFERENCE,
@@ -306,12 +309,15 @@
ASSERT_TRUE(resources) << resources.GetErrorMessage();
auto& res = *resources;
- ASSERT_EQ(resources->GetTargetToOverlayMap().size(), 9U);
+ ASSERT_EQ(resources->GetTargetToOverlayMap().size(), 10U);
ASSERT_RESULT(MappingExists(res, R::target::string::not_overlayable, Res_value::TYPE_REFERENCE,
R::system_overlay_invalid::string::not_overlayable,
false /* rewrite */));
ASSERT_RESULT(MappingExists(res, R::target::string::other, Res_value::TYPE_REFERENCE,
R::system_overlay_invalid::string::other, false /* rewrite */));
+ ASSERT_RESULT(MappingExists(res, R::target::string::policy_actor, Res_value::TYPE_REFERENCE,
+ R::system_overlay_invalid::string::policy_actor,
+ false /* rewrite */));
ASSERT_RESULT(MappingExists(res, R::target::string::policy_odm, Res_value::TYPE_REFERENCE,
R::system_overlay_invalid::string::policy_odm,
false /* rewrite */));
diff --git a/cmds/idmap2/tests/TestConstants.h b/cmds/idmap2/tests/TestConstants.h
index c874dc9..6bc924e 100644
--- a/cmds/idmap2/tests/TestConstants.h
+++ b/cmds/idmap2/tests/TestConstants.h
@@ -19,8 +19,8 @@
namespace android::idmap2::TestConstants {
-constexpr const auto TARGET_CRC = 0x76a20829;
-constexpr const auto TARGET_CRC_STRING = "76a20829";
+constexpr const auto TARGET_CRC = 0x41c60c8c;
+constexpr const auto TARGET_CRC_STRING = "41c60c8c";
constexpr const auto OVERLAY_CRC = 0xc054fb26;
constexpr const auto OVERLAY_CRC_STRING = "c054fb26";
diff --git a/cmds/idmap2/tests/data/system-overlay-invalid/res/values/values.xml b/cmds/idmap2/tests/data/system-overlay-invalid/res/values/values.xml
index 9ebfae4..7119d82 100644
--- a/cmds/idmap2/tests/data/system-overlay-invalid/res/values/values.xml
+++ b/cmds/idmap2/tests/data/system-overlay-invalid/res/values/values.xml
@@ -25,6 +25,7 @@
<string name="policy_signature">policy_signature</string>
<string name="policy_odm">policy_odm</string>
<string name="policy_oem">policy_oem</string>
+ <string name="policy_actor">policy_actor</string>
<!-- Requests to overlay a resource that is not declared as overlayable. -->
<string name="not_overlayable">not_overlayable</string>
diff --git a/cmds/idmap2/tests/data/system-overlay-invalid/system-overlay-invalid.apk b/cmds/idmap2/tests/data/system-overlay-invalid/system-overlay-invalid.apk
index 1456e74..bd99098 100644
--- a/cmds/idmap2/tests/data/system-overlay-invalid/system-overlay-invalid.apk
+++ b/cmds/idmap2/tests/data/system-overlay-invalid/system-overlay-invalid.apk
Binary files differ
diff --git a/cmds/idmap2/tests/data/target/res/values/overlayable.xml b/cmds/idmap2/tests/data/target/res/values/overlayable.xml
index 8389f56..ad4cd48 100644
--- a/cmds/idmap2/tests/data/target/res/values/overlayable.xml
+++ b/cmds/idmap2/tests/data/target/res/values/overlayable.xml
@@ -41,6 +41,10 @@
<item type="string" name="policy_oem" />
</policy>
+ <policy type="actor">
+ <item type="string" name="policy_actor" />
+ </policy>
+
<!-- Resources publicly overlayable -->
<policy type="public">
<item type="string" name="policy_public" />
@@ -63,4 +67,4 @@
<item type="string" name="other" />
</policy>
</overlayable>
-</resources>
\ No newline at end of file
+</resources>
diff --git a/cmds/idmap2/tests/data/target/res/values/values.xml b/cmds/idmap2/tests/data/target/res/values/values.xml
index a892c98..5230e25 100644
--- a/cmds/idmap2/tests/data/target/res/values/values.xml
+++ b/cmds/idmap2/tests/data/target/res/values/values.xml
@@ -36,6 +36,7 @@
<string name="policy_signature">policy_signature</string>
<string name="policy_system">policy_system</string>
<string name="policy_system_vendor">policy_system_vendor</string>
+ <string name="policy_actor">policy_actor</string>
<string name="other">other</string>
</resources>
diff --git a/cmds/idmap2/tests/data/target/target-no-overlayable.apk b/cmds/idmap2/tests/data/target/target-no-overlayable.apk
index 2eb7c47..58504a7 100644
--- a/cmds/idmap2/tests/data/target/target-no-overlayable.apk
+++ b/cmds/idmap2/tests/data/target/target-no-overlayable.apk
Binary files differ
diff --git a/cmds/idmap2/tests/data/target/target.apk b/cmds/idmap2/tests/data/target/target.apk
index 251cf46..c80e5eb 100644
--- a/cmds/idmap2/tests/data/target/target.apk
+++ b/cmds/idmap2/tests/data/target/target.apk
Binary files differ