From b2d7f5343d341c86887d20c61e10fdf8975150de Mon Sep 17 00:00:00 2001 From: Winson Date: Mon, 4 Feb 2019 16:32:43 -0800 Subject: [PATCH] Signature policy for overlayable items Add encoding/decoding of new policy for overlays. Signature enforces that an overlay package is signed with the same key as the actor of the target resource, so that an overlay can be installed by the user as a normal app but restricted to those built by the author of the actor (which can be the same as the target). This also enforces that a valid policy is specified. This doesn't implement the actors nor the signature check. Bug: 119402606 Test: ResourceParserTest ParseOverlayablePolicy Test: ProtoSerializerTest SerializeAndDeserializeOverlayable Test: aapt2_tests Change-Id: I8495ad790c2ebd51759bc6eba81149680c209475 --- .../include/androidfw/ResourceTypes.h | 4 ++ tools/aapt2/ResourceParser.cpp | 14 ++++++ tools/aapt2/ResourceParser_test.cpp | 48 +++++++++++++------ tools/aapt2/ResourceTable.h | 3 ++ tools/aapt2/Resources.proto | 4 +- .../format/binary/BinaryResourceParser.cpp | 4 ++ tools/aapt2/format/binary/TableFlattener.cpp | 40 +++++++++------- .../format/binary/TableFlattener_test.cpp | 34 ++++++------- tools/aapt2/format/proto/ProtoDeserialize.cpp | 3 ++ tools/aapt2/format/proto/ProtoSerialize.cpp | 3 ++ .../format/proto/ProtoSerialize_test.cpp | 13 +++++ 11 files changed, 122 insertions(+), 48 deletions(-) diff --git a/libs/androidfw/include/androidfw/ResourceTypes.h b/libs/androidfw/include/androidfw/ResourceTypes.h index 742813e58129c..875b90b7ac653 100644 --- a/libs/androidfw/include/androidfw/ResourceTypes.h +++ b/libs/androidfw/include/androidfw/ResourceTypes.h @@ -1647,6 +1647,10 @@ struct ResTable_overlayable_policy_header // The overlay must reside of the product partition or must have existed on the product // partition before an upgrade to overlay these resources. POLICY_PRODUCT_PARTITION = 0x00000008, + + // The overlay must be signed with the same signature as the actor of the target resource, + // which can be separate or the same as the target package with the resource. + POLICY_SIGNATURE = 0x00000010, }; uint32_t policy_flags; diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 2f8ca2d62061e..b6691702b3eea 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -1113,6 +1113,13 @@ bool ResourceParser::ParseOverlayable(xml::XmlPullParser* parser, ParsedResource const std::string& element_name = parser->element_name(); const std::string& element_namespace = parser->element_namespace(); if (element_namespace.empty() && element_name == "item") { + if (current_policies == OverlayableItem::Policy::kNone) { + diag_->Error(DiagMessage(element_source) + << " within an must be inside a block"); + error = true; + continue; + } + // Items specify the name and type of resource that should be overlayable Maybe item_name = xml::FindNonEmptyAttribute(parser, "name"); if (!item_name) { @@ -1169,6 +1176,8 @@ bool ResourceParser::ParseOverlayable(xml::XmlPullParser* parser, ParsedResource current_policies |= OverlayableItem::Policy::kSystem; } else if (trimmed_part == "vendor") { current_policies |= OverlayableItem::Policy::kVendor; + } else if (trimmed_part == "signature") { + current_policies |= OverlayableItem::Policy::kSignature; } else { diag_->Error(DiagMessage(element_source) << " has unsupported type '" << trimmed_part << "'"); @@ -1176,6 +1185,11 @@ bool ResourceParser::ParseOverlayable(xml::XmlPullParser* parser, ParsedResource continue; } } + } else { + diag_->Error(DiagMessage(element_source) + << " must have a 'type' attribute"); + error = true; + continue; } } else if (!ShouldIgnoreElement(element_namespace, element_name)) { diag_->Error(DiagMessage(element_source) << "invalid element <" << element_name << "> " diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp index 827c7deaf4528..25b76b0c1fa0f 100644 --- a/tools/aapt2/ResourceParser_test.cpp +++ b/tools/aapt2/ResourceParser_test.cpp @@ -894,8 +894,10 @@ TEST_F(ResourceParserTest, ParsePlatformIndependentNewline) { TEST_F(ResourceParserTest, ParseOverlayable) { std::string input = R"( - - + + + + )"; ASSERT_TRUE(TestParse(input)); @@ -906,7 +908,7 @@ TEST_F(ResourceParserTest, ParseOverlayable) { OverlayableItem& result_overlayable_item = search_result.value().entry->overlayable_item.value(); EXPECT_THAT(result_overlayable_item.overlayable->name, Eq("Name")); EXPECT_THAT(result_overlayable_item.overlayable->actor, Eq("overlay://theme")); - EXPECT_THAT(result_overlayable_item.policies, Eq(OverlayableItem::Policy::kNone)); + EXPECT_THAT(result_overlayable_item.policies, Eq(OverlayableItem::Policy::kSignature)); search_result = table_.FindResource(test::ParseNameOrDie("drawable/bar")); ASSERT_TRUE(search_result); @@ -915,7 +917,7 @@ TEST_F(ResourceParserTest, ParseOverlayable) { result_overlayable_item = search_result.value().entry->overlayable_item.value(); EXPECT_THAT(result_overlayable_item.overlayable->name, Eq("Name")); EXPECT_THAT(result_overlayable_item.overlayable->actor, Eq("overlay://theme")); - EXPECT_THAT(result_overlayable_item.policies, Eq(OverlayableItem::Policy::kNone)); + EXPECT_THAT(result_overlayable_item.policies, Eq(OverlayableItem::Policy::kSignature)); } TEST_F(ResourceParserTest, ParseOverlayableRequiresName) { @@ -931,7 +933,6 @@ TEST_F(ResourceParserTest, ParseOverlayableBadActorFail) { TEST_F(ResourceParserTest, ParseOverlayablePolicy) { std::string input = R"( - @@ -944,23 +945,18 @@ TEST_F(ResourceParserTest, ParseOverlayablePolicy) { + + + )"; ASSERT_TRUE(TestParse(input)); - auto search_result = table_.FindResource(test::ParseNameOrDie("string/foo")); + auto search_result = table_.FindResource(test::ParseNameOrDie("string/bar")); ASSERT_TRUE(search_result); ASSERT_THAT(search_result.value().entry, NotNull()); ASSERT_TRUE(search_result.value().entry->overlayable_item); OverlayableItem 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(OverlayableItem::Policy::kNone)); - - search_result = table_.FindResource(test::ParseNameOrDie("string/bar")); - 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(OverlayableItem::Policy::kProduct)); search_result = table_.FindResource(test::ParseNameOrDie("string/fiz")); @@ -986,6 +982,30 @@ TEST_F(ResourceParserTest, ParseOverlayablePolicy) { 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(OverlayableItem::Policy::kPublic)); + + search_result = table_.FindResource(test::ParseNameOrDie("string/foz")); + 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(OverlayableItem::Policy::kSignature)); +} + +TEST_F(ResourceParserTest, ParseOverlayableNoPolicyError) { + std::string input = R"( + + + )"; + EXPECT_FALSE(TestParse(input)); + + input = R"( + + + + + )"; + EXPECT_FALSE(TestParse(input)); } TEST_F(ResourceParserTest, ParseOverlayableBadPolicyError) { diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h index 7ca99ea42b50e..32dfd260e53c3 100644 --- a/tools/aapt2/ResourceTable.h +++ b/tools/aapt2/ResourceTable.h @@ -92,6 +92,9 @@ struct OverlayableItem { // The resource can be overlaid by any overlay on the product partition. kProduct = 0x08, + + // The resource can be overlaid by any overlay signed with the same signature as its actor. + kSignature = 0x010, }; std::shared_ptr overlayable; diff --git a/tools/aapt2/Resources.proto b/tools/aapt2/Resources.proto index 9a1d94288363c..a2fd7c664b04d 100644 --- a/tools/aapt2/Resources.proto +++ b/tools/aapt2/Resources.proto @@ -138,10 +138,10 @@ message AllowNew { // Represents a set of overlayable resources. message Overlayable { - // The name of the . + // The name of the . string name = 1; - // The location of the declaration in the source. + // The location of the declaration in the source. Source source = 2; // The component responsible for enabling and disabling overlays targeting this . diff --git a/tools/aapt2/format/binary/BinaryResourceParser.cpp b/tools/aapt2/format/binary/BinaryResourceParser.cpp index 40aaa05c2b30a..14906adec7824 100644 --- a/tools/aapt2/format/binary/BinaryResourceParser.cpp +++ b/tools/aapt2/format/binary/BinaryResourceParser.cpp @@ -473,6 +473,10 @@ bool BinaryResourceParser::ParseOverlayable(const ResChunk_header* chunk) { & ResTable_overlayable_policy_header::POLICY_PRODUCT_PARTITION) { policies |= OverlayableItem::Policy::kProduct; } + if (policy_header->policy_flags + & ResTable_overlayable_policy_header::POLICY_SIGNATURE) { + policies |= OverlayableItem::Policy::kSignature; + } const ResTable_ref* const ref_begin = reinterpret_cast( ((uint8_t *)policy_header) + util::DeviceToHost32(policy_header->header.headerSize)); diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp index 9d341cc1ca4a2..5d7e291447d94 100644 --- a/tools/aapt2/format/binary/TableFlattener.cpp +++ b/tools/aapt2/format/binary/TableFlattener.cpp @@ -274,7 +274,9 @@ class PackageFlattener { FlattenLibrarySpec(buffer); } - FlattenOverlayable(buffer); + if (!FlattenOverlayable(buffer)) { + return false; + } pkg_writer.Finish(); return true; @@ -468,23 +470,29 @@ class PackageFlattener { overlayable_chunk = &chunk; } + if (item.policies == 0) { + context_->GetDiagnostics()->Error(DiagMessage(item.overlayable->source) + << "overlayable " + << entry->name + << " does not specify policy"); + return false; + } + uint32_t policy_flags = 0; - if (item.policies == OverlayableItem::Policy::kNone) { - // Encode overlayable entries defined without a policy as publicly overlayable + if (item.policies & OverlayableItem::Policy::kPublic) { policy_flags |= ResTable_overlayable_policy_header::POLICY_PUBLIC; - } else { - if (item.policies & OverlayableItem::Policy::kPublic) { - policy_flags |= ResTable_overlayable_policy_header::POLICY_PUBLIC; - } - if (item.policies & OverlayableItem::Policy::kSystem) { - policy_flags |= ResTable_overlayable_policy_header::POLICY_SYSTEM_PARTITION; - } - if (item.policies & OverlayableItem::Policy::kVendor) { - policy_flags |= ResTable_overlayable_policy_header::POLICY_VENDOR_PARTITION; - } - if (item.policies & OverlayableItem::Policy::kProduct) { - policy_flags |= ResTable_overlayable_policy_header::POLICY_PRODUCT_PARTITION; - } + } + if (item.policies & OverlayableItem::Policy::kSystem) { + policy_flags |= ResTable_overlayable_policy_header::POLICY_SYSTEM_PARTITION; + } + if (item.policies & OverlayableItem::Policy::kVendor) { + policy_flags |= ResTable_overlayable_policy_header::POLICY_VENDOR_PARTITION; + } + if (item.policies & OverlayableItem::Policy::kProduct) { + policy_flags |= ResTable_overlayable_policy_header::POLICY_PRODUCT_PARTITION; + } + if (item.policies & OverlayableItem::Policy::kSignature) { + policy_flags |= ResTable_overlayable_policy_header::POLICY_SIGNATURE; } auto policy = overlayable_chunk->policy_ids.find(policy_flags); diff --git a/tools/aapt2/format/binary/TableFlattener_test.cpp b/tools/aapt2/format/binary/TableFlattener_test.cpp index ddc1173993900..4c5dbec8ade8b 100644 --- a/tools/aapt2/format/binary/TableFlattener_test.cpp +++ b/tools/aapt2/format/binary/TableFlattener_test.cpp @@ -671,9 +671,6 @@ TEST_F(TableFlattenerTest, FlattenMultipleOverlayablePolicies) { overlayable_item_two.policies |= OverlayableItem::Policy::kSystem; overlayable_item_two.policies |= OverlayableItem::Policy::kVendor; - std::string name_three = "com.app.test:integer/overlayable_three_item"; - OverlayableItem overlayable_item_three(overlayable); - std::unique_ptr table = test::ResourceTableBuilder() .SetPackageId("com.app.test", 0x7f) @@ -683,8 +680,6 @@ TEST_F(TableFlattenerTest, FlattenMultipleOverlayablePolicies) { .SetOverlayable(name_one, overlayable_item_one) .AddSimple(name_two, ResourceId(0x7f020002)) .SetOverlayable(name_two, overlayable_item_two) - .AddSimple(name_three, ResourceId(0x7f020003)) - .SetOverlayable(name_three, overlayable_item_three) .Build(); ResourceTable output_table; @@ -713,16 +708,6 @@ TEST_F(TableFlattenerTest, FlattenMultipleOverlayablePolicies) { EXPECT_EQ(overlayable_item.policies, OverlayableItem::Policy::kSystem | OverlayableItem::Policy::kProduct | OverlayableItem::Policy::kVendor); - - search_result = output_table.FindResource(test::ParseNameOrDie(name_three)); - ASSERT_TRUE(search_result); - ASSERT_THAT(search_result.value().entry, NotNull()); - ASSERT_TRUE(search_result.value().entry->overlayable_item); - overlayable_item = search_result.value().entry->overlayable_item.value(); - EXPECT_EQ(overlayable_item.policies, OverlayableItem::Policy::kPublic); - EXPECT_EQ(overlayable_item.overlayable->name, "TestName"); - EXPECT_EQ(overlayable_item.overlayable->actor, "overlay://theme"); - EXPECT_EQ(overlayable_item.policies, OverlayableItem::Policy::kPublic); } TEST_F(TableFlattenerTest, FlattenMultipleOverlayable) { @@ -745,6 +730,8 @@ TEST_F(TableFlattenerTest, FlattenMultipleOverlayable) { std::string name_three = "com.app.test:integer/overlayable_three"; OverlayableItem overlayable_item_three(group_one); + overlayable_item_three.policies |= OverlayableItem::Policy::kSignature; + std::unique_ptr table = test::ResourceTableBuilder() .SetPackageId("com.app.test", 0x7f) @@ -793,7 +780,22 @@ TEST_F(TableFlattenerTest, FlattenMultipleOverlayable) { 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, OverlayableItem::Policy::kPublic); + EXPECT_EQ(result_overlayable.policies, OverlayableItem::Policy::kSignature); +} + +TEST_F(TableFlattenerTest, FlattenOverlayableNoPolicyFails) { + auto group = std::make_shared("TestName", "overlay://theme"); + std::string name_zero = "com.app.test:integer/overlayable_zero"; + OverlayableItem overlayable_item_zero(group); + + std::unique_ptr table = + test::ResourceTableBuilder() + .SetPackageId("com.app.test", 0x7f) + .AddSimple(name_zero, ResourceId(0x7f020000)) + .SetOverlayable(name_zero, overlayable_item_zero) + .Build(); + ResourceTable output_table; + ASSERT_FALSE(Flatten(context_.get(), {}, table.get(), &output_table)); } } // namespace aapt diff --git a/tools/aapt2/format/proto/ProtoDeserialize.cpp b/tools/aapt2/format/proto/ProtoDeserialize.cpp index aff1b391f861f..06f1bf74d7e7f 100644 --- a/tools/aapt2/format/proto/ProtoDeserialize.cpp +++ b/tools/aapt2/format/proto/ProtoDeserialize.cpp @@ -390,6 +390,9 @@ bool DeserializeOverlayableItemFromPb(const pb::OverlayableItem& pb_overlayable, case pb::OverlayableItem::PRODUCT: out_overlayable->policies |= OverlayableItem::Policy::kProduct; break; + case pb::OverlayableItem::SIGNATURE: + out_overlayable->policies |= OverlayableItem::Policy::kSignature; + 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 b549e2369f980..eb2b1a2f35d37 100644 --- a/tools/aapt2/format/proto/ProtoSerialize.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize.cpp @@ -309,6 +309,9 @@ static void SerializeOverlayableItemToPb(const OverlayableItem& overlayable_item if (overlayable_item.policies & OverlayableItem::Policy::kVendor) { pb_overlayable_item->add_policy(pb::OverlayableItem::VENDOR); } + if (overlayable_item.policies & OverlayableItem::Policy::kSignature) { + pb_overlayable_item->add_policy(pb::OverlayableItem::SIGNATURE); + } SerializeSourceToPb(overlayable_item.source, source_pool, pb_overlayable_item->mutable_source()); diff --git a/tools/aapt2/format/proto/ProtoSerialize_test.cpp b/tools/aapt2/format/proto/ProtoSerialize_test.cpp index cce3939704cf4..d369ac4c88160 100644 --- a/tools/aapt2/format/proto/ProtoSerialize_test.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize_test.cpp @@ -526,6 +526,10 @@ TEST(ProtoSerializeTest, SerializeAndDeserializeOverlayable) { "FontPack", "overlay://theme")); overlayable_item_baz.policies |= OverlayableItem::Policy::kPublic; + OverlayableItem overlayable_item_boz(std::make_shared( + "IconPack", "overlay://theme")); + overlayable_item_boz.policies |= OverlayableItem::Policy::kSignature; + OverlayableItem overlayable_item_biz(std::make_shared( "Other", "overlay://customization")); overlayable_item_biz.comment ="comment"; @@ -536,6 +540,7 @@ TEST(ProtoSerializeTest, SerializeAndDeserializeOverlayable) { .SetOverlayable("com.app.a:bool/foo", overlayable_item_foo) .SetOverlayable("com.app.a:bool/bar", overlayable_item_bar) .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) .AddValue("com.app.a:bool/fiz", ResourceUtils::TryParseBool("true")) .Build(); @@ -576,6 +581,14 @@ TEST(ProtoSerializeTest, SerializeAndDeserializeOverlayable) { EXPECT_THAT(overlayable_item.overlayable->actor, Eq("overlay://theme")); EXPECT_THAT(overlayable_item.policies, Eq(OverlayableItem::Policy::kPublic)); + search_result = new_table.FindResource(test::ParseNameOrDie("com.app.a:bool/boz")); + 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("IconPack")); + EXPECT_THAT(overlayable_item.overlayable->actor, Eq("overlay://theme")); + EXPECT_THAT(overlayable_item.policies, Eq(OverlayableItem::Policy::kSignature)); + search_result = new_table.FindResource(test::ParseNameOrDie("com.app.a:bool/biz")); ASSERT_TRUE(search_result); ASSERT_TRUE(search_result.value().entry->overlayable_item);