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/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp
index 77e7fc5..9b70079 100644
--- a/tools/aapt2/ResourceParser_test.cpp
+++ b/tools/aapt2/ResourceParser_test.cpp
@@ -1007,6 +1007,9 @@
<policy type="oem">
<item type="string" name="buz" />
</policy>
+ <policy type="actor">
+ <item type="string" name="actor" />
+ </policy>
</overlayable>)";
ASSERT_TRUE(TestParse(input));
@@ -1065,6 +1068,14 @@
result_overlayable_item = search_result.value().entry->overlayable_item.value();
EXPECT_THAT(result_overlayable_item.overlayable->name, Eq("Name"));
EXPECT_THAT(result_overlayable_item.policies, Eq(PolicyFlags::OEM_PARTITION));
+
+ search_result = table_.FindResource(test::ParseNameOrDie("string/actor"));
+ ASSERT_TRUE(search_result);
+ ASSERT_THAT(search_result.value().entry, NotNull());
+ ASSERT_TRUE(search_result.value().entry->overlayable_item);
+ result_overlayable_item = search_result.value().entry->overlayable_item.value();
+ EXPECT_THAT(result_overlayable_item.overlayable->name, Eq("Name"));
+ EXPECT_THAT(result_overlayable_item.policies, Eq(PolicyFlags::ACTOR_SIGNATURE));
}
TEST_F(ResourceParserTest, ParseOverlayableNoPolicyError) {
diff --git a/tools/aapt2/Resources.proto b/tools/aapt2/Resources.proto
index 8a2f5af..ab9ce66 100644
--- a/tools/aapt2/Resources.proto
+++ b/tools/aapt2/Resources.proto
@@ -167,6 +167,7 @@
SIGNATURE = 5;
ODM = 6;
OEM = 7;
+ ACTOR = 8;
}
// The location of the <item> declaration in source.
diff --git a/tools/aapt2/format/binary/TableFlattener_test.cpp b/tools/aapt2/format/binary/TableFlattener_test.cpp
index e1e0f17..59627ce 100644
--- a/tools/aapt2/format/binary/TableFlattener_test.cpp
+++ b/tools/aapt2/format/binary/TableFlattener_test.cpp
@@ -775,6 +775,7 @@
std::string name_three = "com.app.test:integer/overlayable_three";
OverlayableItem overlayable_item_three(group_one);
overlayable_item_three.policies |= PolicyFlags::SIGNATURE;
+ overlayable_item_three.policies |= PolicyFlags::ACTOR_SIGNATURE;
std::unique_ptr<ResourceTable> table =
test::ResourceTableBuilder()
@@ -828,7 +829,8 @@
result_overlayable = search_result.value().entry->overlayable_item.value();
EXPECT_EQ(result_overlayable.overlayable->name, "OtherName");
EXPECT_EQ(result_overlayable.overlayable->actor, "overlay://customization");
- EXPECT_EQ(result_overlayable.policies, PolicyFlags::SIGNATURE);
+ EXPECT_EQ(result_overlayable.policies, PolicyFlags::SIGNATURE
+ | PolicyFlags::ACTOR_SIGNATURE);
}
TEST_F(TableFlattenerTest, FlattenOverlayableNoPolicyFails) {
diff --git a/tools/aapt2/format/proto/ProtoDeserialize.cpp b/tools/aapt2/format/proto/ProtoDeserialize.cpp
index db8e093..2fd01d7 100644
--- a/tools/aapt2/format/proto/ProtoDeserialize.cpp
+++ b/tools/aapt2/format/proto/ProtoDeserialize.cpp
@@ -401,6 +401,9 @@
case pb::OverlayableItem::OEM:
out_overlayable->policies |= PolicyFlags::OEM_PARTITION;
break;
+ case pb::OverlayableItem::ACTOR:
+ out_overlayable->policies |= PolicyFlags::ACTOR_SIGNATURE;
+ break;
default:
*out_error = "unknown overlayable policy";
return false;
diff --git a/tools/aapt2/format/proto/ProtoSerialize.cpp b/tools/aapt2/format/proto/ProtoSerialize.cpp
index 6bc4524d..ba6df22 100644
--- a/tools/aapt2/format/proto/ProtoSerialize.cpp
+++ b/tools/aapt2/format/proto/ProtoSerialize.cpp
@@ -322,6 +322,9 @@
if (overlayable_item.policies & PolicyFlags::OEM_PARTITION) {
pb_overlayable_item->add_policy(pb::OverlayableItem::OEM);
}
+ if (overlayable_item.policies & PolicyFlags::ACTOR_SIGNATURE) {
+ pb_overlayable_item->add_policy(pb::OverlayableItem::ACTOR);
+ }
if (source_pool != nullptr) {
SerializeSourceToPb(overlayable_item.source, source_pool,
diff --git a/tools/aapt2/format/proto/ProtoSerialize_test.cpp b/tools/aapt2/format/proto/ProtoSerialize_test.cpp
index cdc8659..1a7de6d 100644
--- a/tools/aapt2/format/proto/ProtoSerialize_test.cpp
+++ b/tools/aapt2/format/proto/ProtoSerialize_test.cpp
@@ -536,6 +536,11 @@
overlayable_item_boz.policies |= PolicyFlags::ODM_PARTITION;
overlayable_item_boz.policies |= PolicyFlags::OEM_PARTITION;
+ OverlayableItem overlayable_item_actor_config(std::make_shared<Overlayable>(
+ "ActorConfig", "overlay://theme"));
+ overlayable_item_actor_config.policies |= PolicyFlags::SIGNATURE;
+ overlayable_item_actor_config.policies |= PolicyFlags::ACTOR_SIGNATURE;
+
OverlayableItem overlayable_item_biz(std::make_shared<Overlayable>(
"Other", "overlay://customization"));
overlayable_item_biz.comment ="comment";
@@ -548,6 +553,7 @@
.SetOverlayable("com.app.a:bool/baz", overlayable_item_baz)
.SetOverlayable("com.app.a:bool/boz", overlayable_item_boz)
.SetOverlayable("com.app.a:bool/biz", overlayable_item_biz)
+ .SetOverlayable("com.app.a:bool/actor_config", overlayable_item_actor_config)
.AddValue("com.app.a:bool/fiz", ResourceUtils::TryParseBool("true"))
.Build();
@@ -597,6 +603,15 @@
| PolicyFlags::ODM_PARTITION
| PolicyFlags::OEM_PARTITION));
+ search_result = new_table.FindResource(test::ParseNameOrDie("com.app.a:bool/actor_config"));
+ ASSERT_TRUE(search_result);
+ ASSERT_TRUE(search_result.value().entry->overlayable_item);
+ overlayable_item = search_result.value().entry->overlayable_item.value();
+ EXPECT_THAT(overlayable_item.overlayable->name, Eq("ActorConfig"));
+ EXPECT_THAT(overlayable_item.overlayable->actor, Eq("overlay://theme"));
+ EXPECT_THAT(overlayable_item.policies, Eq(PolicyFlags::SIGNATURE
+ | PolicyFlags::ACTOR_SIGNATURE));
+
search_result = new_table.FindResource(test::ParseNameOrDie("com.app.a:bool/biz"));
ASSERT_TRUE(search_result);
ASSERT_TRUE(search_result.value().entry->overlayable_item);