Add whitelist for extensions to put <all_urls> 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": ["<all_urls>"] } 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}
hylschwy · Sep 25, 2014 · 860c654 · 860c654
1 parent 4a06278
commit 860c654
Show file tree

Hide file tree

Showing 9 changed files with 70 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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<InstallWarning> install_warnings;
   scoped_ptr<ExternallyConnectableInfo> 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> ExternallyConnectableInfo::FromValue(
     const base::Value& value,
+    bool allow_all_urls,
     std::vector<InstallWarning>* install_warnings,
     base::string16* error) {
   scoped_ptr<ExternallyConnectable> externally_connectable =
@@ -116,6 +119,11 @@ scoped_ptr<ExternallyConnectableInfo> ExternallyConnectableInfo::FromValue(
         return scoped_ptr<ExternallyConnectableInfo>();
       }
 
+      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
@@ -57,6 +57,7 @@ struct ExternallyConnectableInfo : public Extension::ManifestData {
   // the manifest. Sets |error| and returns an empty scoped_ptr on failure.
   static scoped_ptr<ExternallyConnectableInfo> FromValue(
       const base::Value& value,
+      bool allow_all_urls,
       std::vector<InstallWarning>* 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
@@ -253,12 +253,31 @@ TEST_F(ExternallyConnectableTest, WarningNoAllURLs) {
       ErrorUtils::FormatErrorMessage(errors::kErrorWildcardHostsNotAllowed,
                                      "<all_urls>"));
   ExternallyConnectableInfo* info = GetExternallyConnectableInfo(extension);
+  EXPECT_FALSE(info->matches.MatchesAllURLs());
   EXPECT_FALSE(info->matches.ContainsPattern(
       URLPattern(URLPattern::SCHEME_ALL, "<all_urls>")));
   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> 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> extension =
+      LoadAndExpectSuccess("externally_connectable_all_urls_whitelisted.json");
+  ExternallyConnectableInfo* info = GetExternallyConnectableInfo(extension);
+  EXPECT_TRUE(info->matches.MatchesAllURLs());
+  URLPattern pattern(URLPattern::SCHEME_ALL, "<all_urls>");
+  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> extension = LoadAndExpectWarning(
       "externally_connectable_error_wildcard_host.json",

diff --git 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
@@ -36,6 +36,8 @@ std::vector<APIPermissionInfo*> 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
@@ -0,0 +1,11 @@
+{
+  "name": "A non-whitelisted extension that requests <all_urls> for externally_connectable",
+  "version": "1",
+  "manifest_version": 2,
+  "permissions": ["externally_connectable.all_urls"],
+  "externally_connectable": {
+    "matches": [
+      "<all_urls>"
+    ]
+  }
+}
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
@@ -0,0 +1,12 @@
+{
+  "name": "A whitelisted extension that requests <all_urls> 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": [
+      "<all_urls>"
+    ]
+  }
+}