From 860c6547ac322954027fc226e75881f15585f14b Mon Sep 17 00:00:00 2001 From: asargent Date: Thu, 25 Sep 2014 14:47:50 -0700 Subject: [PATCH] Add whitelist for extensions to put in externally_connectable Normally extensions must specify a concrete list of url patterns. But with this patch, extensions on the whitelist can do: "permissions": ["externally_connectable.all_urls"], "externally_connectable": { "matches": [""] } and they will be able to be connected to from any url. Also add an entry to the whitelist for the CryptoToken extension. BUG=417062,417494 Review URL: https://codereview.chromium.org/599163003 Cr-Commit-Position: refs/heads/master@{#296799} --- .../permissions/permission_set_unittest.cc | 3 +++ .../common/api/_permission_features.json | 10 ++++++++++ .../externally_connectable.cc | 14 +++++++++++--- .../externally_connectable.h | 1 + .../externally_connectable_unittest.cc | 19 +++++++++++++++++++ .../common/permissions/api_permission.h | 1 + .../permissions/extensions_api_permissions.cc | 2 ++ ..._connectable_all_urls_not_whitelisted.json | 11 +++++++++++ ...ally_connectable_all_urls_whitelisted.json | 12 ++++++++++++ 9 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 extensions/test/data/manifest_tests/externally_connectable_all_urls_not_whitelisted.json create mode 100644 extensions/test/data/manifest_tests/externally_connectable_all_urls_whitelisted.json diff --git a/chrome/common/extensions/permissions/permission_set_unittest.cc b/chrome/common/extensions/permissions/permission_set_unittest.cc index 70c5f685ddea1b..9370e446a0077b 100644 --- a/chrome/common/extensions/permissions/permission_set_unittest.cc +++ b/chrome/common/extensions/permissions/permission_set_unittest.cc @@ -769,6 +769,9 @@ TEST(PermissionsTest, PermissionMessages) { skip.insert(APIPermission::kSocket); skip.insert(APIPermission::kUsbDevice); + // We already have a generic message for declaring externally_connectable. + skip.insert(APIPermission::kExternallyConnectableAllUrls); + PermissionsInfo* info = PermissionsInfo::GetInstance(); APIPermissionSet permissions = info->GetAll(); for (APIPermissionSet::const_iterator i = permissions.begin(); diff --git a/extensions/common/api/_permission_features.json b/extensions/common/api/_permission_features.json index ca3ac5759e5395..0e1b58f9d32993 100644 --- a/extensions/common/api/_permission_features.json +++ b/extensions/common/api/_permission_features.json @@ -107,6 +107,16 @@ ] } ], + "externally_connectable.all_urls": { + "channel": "stable", + "extension_types": [ + "extension", "hosted_app", "legacy_packaged_app", "platform_app" + ], + "whitelist": [ + "54ECAB4579BDE8FDAF9B29ED335F9946EE504A52", // Used in unit tests + "E24F1786D842E91E74C27929B0B3715A4689A473" // http://crbug.com/417494 + ] + }, "hid": [ { "channel": "stable", diff --git a/extensions/common/manifest_handlers/externally_connectable.cc b/extensions/common/manifest_handlers/externally_connectable.cc index 0a06c019ae2b37..1314c6d1cb0fa4 100644 --- a/extensions/common/manifest_handlers/externally_connectable.cc +++ b/extensions/common/manifest_handlers/externally_connectable.cc @@ -63,11 +63,13 @@ bool ExternallyConnectableHandler::Parse(Extension* extension, const base::Value* externally_connectable = NULL; CHECK(extension->manifest()->Get(keys::kExternallyConnectable, &externally_connectable)); + bool allow_all_urls = PermissionsParser::HasAPIPermission( + extension, APIPermission::kExternallyConnectableAllUrls); + std::vector install_warnings; scoped_ptr info = - ExternallyConnectableInfo::FromValue(*externally_connectable, - &install_warnings, - error); + ExternallyConnectableInfo::FromValue( + *externally_connectable, allow_all_urls, &install_warnings, error); if (!info) return false; if (!info->matches.is_empty()) { @@ -93,6 +95,7 @@ ExternallyConnectableInfo* ExternallyConnectableInfo::Get( // static scoped_ptr ExternallyConnectableInfo::FromValue( const base::Value& value, + bool allow_all_urls, std::vector* install_warnings, base::string16* error) { scoped_ptr externally_connectable = @@ -116,6 +119,11 @@ scoped_ptr ExternallyConnectableInfo::FromValue( return scoped_ptr(); } + if (allow_all_urls && pattern.match_all_urls()) { + matches.AddPattern(pattern); + continue; + } + // Wildcard hosts are not allowed. if (pattern.host().empty()) { // Warning not error for forwards compatibility. diff --git a/extensions/common/manifest_handlers/externally_connectable.h b/extensions/common/manifest_handlers/externally_connectable.h index 960904e01a1366..f72ff456465777 100644 --- a/extensions/common/manifest_handlers/externally_connectable.h +++ b/extensions/common/manifest_handlers/externally_connectable.h @@ -57,6 +57,7 @@ struct ExternallyConnectableInfo : public Extension::ManifestData { // the manifest. Sets |error| and returns an empty scoped_ptr on failure. static scoped_ptr FromValue( const base::Value& value, + bool allow_all_urls, std::vector* install_warnings, base::string16* error); diff --git a/extensions/common/manifest_handlers/externally_connectable_unittest.cc b/extensions/common/manifest_handlers/externally_connectable_unittest.cc index 7064869c3ab364..9dedff3a4f09a9 100644 --- a/extensions/common/manifest_handlers/externally_connectable_unittest.cc +++ b/extensions/common/manifest_handlers/externally_connectable_unittest.cc @@ -253,12 +253,31 @@ TEST_F(ExternallyConnectableTest, WarningNoAllURLs) { ErrorUtils::FormatErrorMessage(errors::kErrorWildcardHostsNotAllowed, "")); ExternallyConnectableInfo* info = GetExternallyConnectableInfo(extension); + EXPECT_FALSE(info->matches.MatchesAllURLs()); EXPECT_FALSE(info->matches.ContainsPattern( URLPattern(URLPattern::SCHEME_ALL, ""))); EXPECT_TRUE(info->matches.MatchesURL(GURL("https://example.com"))); EXPECT_TRUE(info->matches.MatchesURL(GURL("http://build.chromium.org"))); } +TEST_F(ExternallyConnectableTest, AllURLsNotWhitelisted) { + scoped_refptr extension = LoadAndExpectSuccess( + "externally_connectable_all_urls_not_whitelisted.json"); + ExternallyConnectableInfo* info = GetExternallyConnectableInfo(extension); + EXPECT_FALSE(info->matches.MatchesAllURLs()); +} + +TEST_F(ExternallyConnectableTest, AllURLsWhitelisted) { + scoped_refptr extension = + LoadAndExpectSuccess("externally_connectable_all_urls_whitelisted.json"); + ExternallyConnectableInfo* info = GetExternallyConnectableInfo(extension); + EXPECT_TRUE(info->matches.MatchesAllURLs()); + URLPattern pattern(URLPattern::SCHEME_ALL, ""); + EXPECT_TRUE(info->matches.ContainsPattern(pattern)); + EXPECT_TRUE(info->matches.MatchesURL(GURL("https://example.com"))); + EXPECT_TRUE(info->matches.MatchesURL(GURL("http://build.chromium.org"))); +} + TEST_F(ExternallyConnectableTest, WarningWildcardHost) { scoped_refptr extension = LoadAndExpectWarning( "externally_connectable_error_wildcard_host.json", diff --git a/extensions/common/permissions/api_permission.h b/extensions/common/permissions/api_permission.h index d15dee322f0fb0..e7b76da53346b1 100644 --- a/extensions/common/permissions/api_permission.h +++ b/extensions/common/permissions/api_permission.h @@ -88,6 +88,7 @@ class APIPermission { kEnterprisePlatformKeysPrivate, kExperienceSamplingPrivate, kExperimental, + kExternallyConnectableAllUrls, kFeedbackPrivate, kFileBrowserHandler, kFileBrowserHandlerInternal, diff --git a/extensions/common/permissions/extensions_api_permissions.cc b/extensions/common/permissions/extensions_api_permissions.cc index efad143d916ec8..498990ce3949ee 100644 --- a/extensions/common/permissions/extensions_api_permissions.cc +++ b/extensions/common/permissions/extensions_api_permissions.cc @@ -36,6 +36,8 @@ std::vector ExtensionsAPIPermissions::GetAllPermissions() APIPermissionInfo::kFlagNone, IDS_EXTENSION_PROMPT_WARNING_AUDIO_CAPTURE, PermissionMessage::kAudioCapture}, {APIPermission::kDns, "dns"}, + {APIPermission::kExternallyConnectableAllUrls, + "externally_connectable.all_urls"}, {APIPermission::kFullscreen, "app.window.fullscreen"}, {APIPermission::kHid, "hid", APIPermissionInfo::kFlagNone, IDS_EXTENSION_PROMPT_WARNING_HID, PermissionMessage::kHid}, diff --git a/extensions/test/data/manifest_tests/externally_connectable_all_urls_not_whitelisted.json b/extensions/test/data/manifest_tests/externally_connectable_all_urls_not_whitelisted.json new file mode 100644 index 00000000000000..b545e139f64c9e --- /dev/null +++ b/extensions/test/data/manifest_tests/externally_connectable_all_urls_not_whitelisted.json @@ -0,0 +1,11 @@ +{ + "name": "A non-whitelisted extension that requests for externally_connectable", + "version": "1", + "manifest_version": 2, + "permissions": ["externally_connectable.all_urls"], + "externally_connectable": { + "matches": [ + "" + ] + } +} diff --git a/extensions/test/data/manifest_tests/externally_connectable_all_urls_whitelisted.json b/extensions/test/data/manifest_tests/externally_connectable_all_urls_whitelisted.json new file mode 100644 index 00000000000000..8fbc59e2de57ae --- /dev/null +++ b/extensions/test/data/manifest_tests/externally_connectable_all_urls_whitelisted.json @@ -0,0 +1,12 @@ +{ + "name": "A whitelisted extension that requests for externally_connectable", + "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwBulalpRjkun/sRoaxIRpg6+qM6lJlI1whKuTJP9TzYxdTs955Wmfj6CmX07+VIbsFnAAcI67F2LhbmXiueYe39xGvvVQ3w5l+WPdJ8a9AV/t6Afpi3H0LRjB1oIB2k1rp/6j5tDY2zly4Q/eqlxEB+Y81RBw6gbo9LpFEOAxsZwRGMs2z1x7I7MybY38HFKM6d58U6ovm5QJuhgGWfmGI+4TdRf61OMxBPXArBDmNQGElsJJmclebD5fS2ynXcmN+/1e2oAdu6saQbb82gec7x8fOi9TEeYIFiQSPGY3/vlK5dSKGYU59O6nGIZMh2JA605IGuat//NdXDH77Yw+QIDAQAB", + "version": "1", + "manifest_version": 2, + "permissions": ["externally_connectable.all_urls"], + "externally_connectable": { + "matches": [ + "" + ] + } +}