Allow overrides for update_client-based extension updater's URL / CUP.

Background: there are two extension updaters in Chrome: the
update_client updater (new) and the extension_downloader client (old).

Prior to this change, changing the extension updater URL would
effectively disable the update_client-based updater and the
extension_downloader client would take over. This makes it impossible
to use the update_client updater with non-prod or local copies of CWS.

A user/attacker with control over Chrome's command line can redirect the
extension update check to a server of their choice.  Furthermore, CUP is
disabled for this transaction (because the target server presumably does
not have the pinned CUP private key).

This is not expected to have any tangible security impact, because:
1 - Prior to this change, the extension_downloader updater would still
redirect the request, and extension_downloader doesn't use CUP anyways.
2 - Activating this feature requires attacker presence on-disk or a
confused user.
3 - A MITM attacker must also compromise TLS to subvert the update
check.
4 - The updater will still only accept properly-signed CRX files as
update payloads, even if an attack establishes control of the update
check.

Bug: 1077122
Fixed: 1077122
Change-Id: I4a9169f2741900906bfa63da40196aa0f887e70a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2180862
Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776102}

chrome/browser/chromeos/app_mode/fake_cws.cc

@@ -11,6 +11,7 @@
 #include "base/files/file_util.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_tokenizer.h"
 #include "base/strings/string_util.h"
 #include "base/threading/thread_restrictions.h"
 #include "chrome/common/chrome_paths.h"
@@ -21,6 +22,7 @@
 #include "extensions/common/extensions_client.h"
 #include "net/base/url_util.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_request.h"
 
 using net::test_server::BasicHttpResponse;
 using net::test_server::HttpRequest;
@@ -59,6 +61,63 @@ const char kUpdateContentTemplate[] =
       "$APPS"
     "</gupdate>";
 
+const char kAppNoUpdateTemplateJSON[] =
+    "{\"appid\": \"$AppId\","
+    " \"status\": \"ok\","
+    " \"updatecheck\": { \"status\": \"noupdate\" }"
+    "}";
+
+const char kAppHasUpdateTemplateJSON[] =
+    "{"
+    "  \"appid\": \"$AppId\","
+    "  \"status\": \"ok\","
+    "  \"updatecheck\": {"
+    "    \"status\": \"ok\","
+    "    \"manifest\": {"
+    "      \"version\": \"$Version\","
+    "      \"packages\": {"
+    "        \"package\": ["
+    "          {"
+    "            \"fp\": \"1.$FP\","
+    "            \"size\": \"$Size\","
+    "            \"hash_sha256\": \"$FP\","
+    "            \"name\": \"\""
+    "          }"
+    "        ]"
+    "      }"
+    "    },"
+    "    \"urls\": { \"url\": [ { \"codebase\": \"$CrxDownloadUrl\"} ] }"
+    "  }"
+    "}";
+
+const char kUpdateContentTemplateJSON[] =
+    ")]}'\n"
+    "{"
+    "  \"response\": {"
+    "    \"protocol\": \"3.1\","
+    "    \"daystart\": {"
+    "      \"elapsed_days\": 2569,"
+    "      \"elapsed_seconds\": 36478"
+    "    },"
+    "    \"app\": ["
+    "      $APPS"
+    "    ]"
+    "  }"
+    "}";
+
+const char kAppIdHeader[] = "X-Goog-Update-AppId";
+
+bool GetAppIdsFromHeader(const HttpRequest::HeaderMap& headers,
+                         std::vector<std::string>* ids) {
+  if (headers.count(kAppIdHeader) == 0)
+    return false;
+  base::StringTokenizer t(headers.at(kAppIdHeader), ",");
+  while (t.GetNext()) {
+    ids->push_back(t.token());
+  }
+  return !ids->empty();
+}
+
 bool GetAppIdsFromUpdateUrl(const GURL& update_url,
                             std::vector<std::string>* ids) {
   for (net::QueryIterator it(update_url); !it.IsAtEnd(); it.Advance()) {
@@ -80,6 +139,40 @@ bool GetAppIdsFromUpdateUrl(const GURL& update_url,
 // FakeCWS will hold the ScopedIgnoreContentVerifierForTest instance.
 bool g_is_fakecws_active = false;
 
+std::string ApplyHasNoUpdateTemplate(std::string app_id,
+                                     bool use_json,
+                                     bool use_private_store) {
+  std::string update_check_content(use_json ? kAppNoUpdateTemplateJSON
+                                            : kAppNoUpdateTemplate);
+  base::ReplaceSubstringsAfterOffset(&update_check_content, 0, "$AppId",
+                                     app_id);
+  return update_check_content;
+}
+
+std::string ApplyHasUpdateTemplate(std::string app_id,
+                                   GURL download_url,
+                                   std::string sha256_hex,
+                                   int size,
+                                   std::string version,
+                                   bool use_json,
+                                   bool use_private_store) {
+  std::string update_check_content(
+      use_json ? kAppHasUpdateTemplateJSON
+               : use_private_store ? kPrivateStoreAppHasUpdateTemplate
+                                   : kAppHasUpdateTemplate);
+  base::ReplaceSubstringsAfterOffset(&update_check_content, 0, "$AppId",
+                                     app_id);
+  base::ReplaceSubstringsAfterOffset(&update_check_content, 0,
+                                     "$CrxDownloadUrl", download_url.spec());
+  base::ReplaceSubstringsAfterOffset(&update_check_content, 0, "$FP",
+                                     sha256_hex);
+  base::ReplaceSubstringsAfterOffset(&update_check_content, 0, "$Size",
+                                     base::NumberToString(size));
+  base::ReplaceSubstringsAfterOffset(&update_check_content, 0, "$Version",
+                                     version);
+  return update_check_content;
+}
+
 }  // namespace
 
 FakeCWS::FakeCWS() : update_check_count_(0) {
@@ -101,8 +194,7 @@ FakeCWS::~FakeCWS() {
 }
 
 void FakeCWS::Init(net::EmbeddedTestServer* embedded_test_server) {
-  has_update_template_ = kAppHasUpdateTemplate;
-  no_update_template_ = kAppNoUpdateTemplate;
+  use_private_store_templates_ = false;
   update_check_end_point_ = "/update_check.xml";
 
   SetupWebStoreURL(embedded_test_server->base_url());
@@ -113,8 +205,7 @@ void FakeCWS::Init(net::EmbeddedTestServer* embedded_test_server) {
 
 void FakeCWS::InitAsPrivateStore(net::EmbeddedTestServer* embedded_test_server,
                                  const std::string& update_check_end_point) {
-  has_update_template_ = kPrivateStoreAppHasUpdateTemplate;
-  no_update_template_ = kAppNoUpdateTemplate;
+  use_private_store_templates_ = true;
   update_check_end_point_ = update_check_end_point;
 
   SetupWebStoreURL(embedded_test_server->base_url());
@@ -143,25 +234,14 @@ void FakeCWS::SetUpdateCrx(const std::string& app_id,
   const std::string sha256 = crypto::SHA256HashString(crx_content);
   const std::string sha256_hex = base::HexEncode(sha256.c_str(), sha256.size());
 
-  std::string update_check_content(has_update_template_);
-  base::ReplaceSubstringsAfterOffset(&update_check_content, 0, "$AppId",
-                                     app_id);
-  base::ReplaceSubstringsAfterOffset(
-      &update_check_content, 0, "$CrxDownloadUrl", crx_download_url.spec());
-  base::ReplaceSubstringsAfterOffset(&update_check_content, 0, "$FP",
-                                     sha256_hex);
-  base::ReplaceSubstringsAfterOffset(&update_check_content, 0, "$Size",
-                                     base::NumberToString(crx_content.size()));
-  base::ReplaceSubstringsAfterOffset(&update_check_content, 0, "$Version",
-                                     version);
-  id_to_update_check_content_map_[app_id] = update_check_content;
+  id_to_update_check_content_map_[app_id] =
+      base::BindRepeating(&ApplyHasUpdateTemplate, app_id, crx_download_url,
+                          sha256_hex, crx_content.size(), version);
 }
 
 void FakeCWS::SetNoUpdate(const std::string& app_id) {
-  std::string app_update_check_content(no_update_template_);
-  base::ReplaceSubstringsAfterOffset(&app_update_check_content, 0, "$AppId",
-                                     app_id);
-  id_to_update_check_content_map_[app_id] = app_update_check_content;
+  id_to_update_check_content_map_[app_id] =
+      base::BindRepeating(&ApplyHasNoUpdateTemplate, app_id);
 }
 
 int FakeCWS::GetUpdateCheckCountAndReset() {
@@ -199,19 +279,25 @@ void FakeCWS::OverrideGalleryCommandlineSwitches() {
 }
 
 bool FakeCWS::GetUpdateCheckContent(const std::vector<std::string>& ids,
-                                    std::string* update_check_content) {
+                                    std::string* update_check_content,
+                                    bool use_json) {
   std::string apps_content;
+  bool need_comma = false;
   for (const std::string& id : ids) {
     std::string app_update_content;
     auto it = id_to_update_check_content_map_.find(id);
     if (it == id_to_update_check_content_map_.end())
       return false;
-    apps_content.append(it->second);
+    if (need_comma)
+      apps_content.append(",");
+    apps_content.append(it->second.Run(use_json, use_private_store_templates_));
+    need_comma = use_json;
   }
   if (apps_content.empty())
     return false;
 
-  *update_check_content = kUpdateContentTemplate;
+  *update_check_content =
+      use_json ? kUpdateContentTemplateJSON : kUpdateContentTemplate;
   base::ReplaceSubstringsAfterOffset(update_check_content, 0, "$APPS",
                                      apps_content);
   return true;
@@ -224,14 +310,18 @@ std::unique_ptr<HttpResponse> FakeCWS::HandleRequest(
   if (request_path.find(update_check_end_point_) != std::string::npos &&
       !id_to_update_check_content_map_.empty()) {
     std::vector<std::string> ids;
-    if (GetAppIdsFromUpdateUrl(request_url, &ids)) {
+    if (GetAppIdsFromHeader(request.headers, &ids) ||
+        GetAppIdsFromUpdateUrl(request_url, &ids)) {
+      bool use_json =
+          request.content.size() > 0 && request.content.at(0) == '{';
       std::string update_check_content;
-      if (GetUpdateCheckContent(ids, &update_check_content)) {
+      if (GetUpdateCheckContent(ids, &update_check_content, use_json)) {
         ++update_check_count_;
         std::unique_ptr<BasicHttpResponse> http_response(
             new BasicHttpResponse());
         http_response->set_code(net::HTTP_OK);
-        http_response->set_content_type("text/xml");
+        if (!use_json)
+          http_response->set_content_type("text/xml");
         http_response->set_content(update_check_content);
         return std::move(http_response);
       }

chrome/browser/chromeos/app_mode/fake_cws.h

@@ -10,6 +10,7 @@
 #include <string>
 #include <vector>
 
+#include "base/callback_forward.h"
 #include "base/macros.h"
 #include "extensions/browser/scoped_ignore_content_verifier_for_test.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ -58,7 +59,8 @@ class FakeCWS {
   void OverrideGalleryCommandlineSwitches();
 
   bool GetUpdateCheckContent(const std::vector<std::string>& ids,
-                             std::string* update_check_content);
+                             std::string* update_check_content,
+                             bool use_json);
 
   // Request handler for kiosk app update server.
   std::unique_ptr<net::test_server::HttpResponse> HandleRequest(
@@ -66,12 +68,13 @@ class FakeCWS {
 
   GURL web_store_url_;
 
-  std::string has_update_template_;
-  std::string no_update_template_;
+  bool use_private_store_templates_;
   std::string update_check_end_point_;
 
-  // Map keyed by app_id to app_update_content.
-  std::map<std::string, std::string> id_to_update_check_content_map_;
+  // Map keyed by app_id to partially-bound functions that can generate the
+  // app's update content.
+  std::map<std::string, base::RepeatingCallback<std::string(bool, bool)>>
+      id_to_update_check_content_map_;
   int update_check_count_;
 
   // FakeCWS overrides Chrome Web Store URLs, so extensions it provides in tests

chrome/browser/extensions/chrome_extensions_browser_client.cc

@@ -8,6 +8,8 @@
 
 #include "base/command_line.h"
 #include "base/logging.h"
+#include "base/optional.h"
+#include "base/strings/string_util.h"
 #include "base/version.h"
 #include "build/build_config.h"
 #include "chrome/browser/app_mode/app_mode_utils.h"
@@ -60,6 +62,7 @@
 #include "extensions/browser/extensions_browser_interface_binders.h"
 #include "extensions/browser/pref_names.h"
 #include "extensions/browser/url_request_util.h"
+#include "extensions/common/extension_urls.h"
 #include "extensions/common/features/feature_channel.h"
 
 #if defined(OS_CHROMEOS)
@@ -77,6 +80,9 @@ namespace extensions {
 
 namespace {
 
+const char kCrxUrlPath[] = "/service/update2/crx";
+const char kJsonUrlPath[] = "/service/update2/json";
+
 // If true, the extensions client will behave as though there is always a
 // new chrome update.
 bool g_did_chrome_update_for_testing = false;
@@ -443,8 +449,17 @@ void ChromeExtensionsBrowserClient::AttachExtensionTaskManagerTag(
 scoped_refptr<update_client::UpdateClient>
 ChromeExtensionsBrowserClient::CreateUpdateClient(
     content::BrowserContext* context) {
+  base::Optional<GURL> override_url;
+  GURL update_url = extension_urls::GetWebstoreUpdateUrl();
+  if (update_url != extension_urls::GetDefaultWebstoreUpdateUrl()) {
+    if (update_url.path() == kCrxUrlPath) {
+      override_url = update_url.GetWithEmptyPath().Resolve(kJsonUrlPath);
+    } else {
+      override_url = update_url;
+    }
+  }
   return update_client::UpdateClientFactory(
-      ChromeUpdateClientConfig::Create(context));
+      ChromeUpdateClientConfig::Create(context, override_url));
 }
 
 std::unique_ptr<content::BluetoothChooser>

chrome/browser/extensions/extension_disabled_ui_browsertest.cc

@@ -65,10 +65,7 @@ class ExtensionDisabledGlobalErrorTest
         test_dir.AppendASCII("v1"),
         scoped_temp_dir_.GetPath().AppendASCII("permissions1.crx"), pem_path,
         base::FilePath());
-    path_v2_ = PackExtensionWithOptions(
-        test_dir.AppendASCII("v2"),
-        scoped_temp_dir_.GetPath().AppendASCII("permissions2.crx"), pem_path,
-        base::FilePath());
+    path_v2_ = test_dir.AppendASCII("v2.crx");
     path_v3_ = PackExtensionWithOptions(
         test_dir.AppendASCII("v3"),
         scoped_temp_dir_.GetPath().AppendASCII("permissions3.crx"), pem_path,
@@ -233,13 +230,12 @@ IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
         if (path == "/autoupdate/updates.xml") {
           content::URLLoaderInterceptor::WriteResponse(
               test_data_dir_.AppendASCII("permissions_increase")
-                  .AppendASCII("updates.xml"),
+                  .AppendASCII("updates.json"),
               params->client.get());
           return true;
         } else if (path == "/autoupdate/v2.crx") {
-          content::URLLoaderInterceptor::WriteResponse(
-              scoped_temp_dir_.GetPath().AppendASCII("permissions2.crx"),
-              params->client.get());
+          content::URLLoaderInterceptor::WriteResponse(path_v2_,
+                                                       params->client.get());
           return true;
         }
         return false;
@@ -283,9 +279,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest, RemoteInstall) {
               params->client.get());
           return true;
         } else if (path == "/autoupdate/v2.crx") {
-          content::URLLoaderInterceptor::WriteResponse(
-              scoped_temp_dir_.GetPath().AppendASCII("permissions2.crx"),
-              params->client.get());
+          content::URLLoaderInterceptor::WriteResponse(path_v2_,
+                                                       params->client.get());
           return true;
         }
         return false;

chrome/browser/extensions/updater/chrome_update_client_config.cc

@@ -99,14 +99,16 @@ void ExtensionActivityDataService::ClearActiveBit(const std::string& id) {
 // For privacy reasons, requires encryption of the component updater
 // communication with the update backend.
 ChromeUpdateClientConfig::ChromeUpdateClientConfig(
-    content::BrowserContext* context)
+    content::BrowserContext* context,
+    base::Optional<GURL> url_override)
     : context_(context),
       impl_(ExtensionUpdateClientCommandLineConfigPolicy(
                 base::CommandLine::ForCurrentProcess()),
             /*require_encryption=*/true),
       pref_service_(ExtensionPrefs::Get(context)->pref_service()),
       activity_data_service_(std::make_unique<ExtensionActivityDataService>(
-          ExtensionPrefs::Get(context))) {
+          ExtensionPrefs::Get(context))),
+      url_override_(url_override) {
   DCHECK(pref_service_);
 }
 
@@ -127,10 +129,14 @@ int ChromeUpdateClientConfig::UpdateDelay() const {
 }
 
 std::vector<GURL> ChromeUpdateClientConfig::UpdateUrl() const {
+  if (url_override_.has_value())
+    return {*url_override_};
   return impl_.UpdateUrl();
 }
 
 std::vector<GURL> ChromeUpdateClientConfig::PingUrl() const {
+  if (url_override_.has_value())
+    return {*url_override_};
   return impl_.PingUrl();
 }
 
@@ -220,6 +226,8 @@ bool ChromeUpdateClientConfig::EnabledBackgroundDownloader() const {
 }
 
 bool ChromeUpdateClientConfig::EnabledCupSigning() const {
+  if (url_override_.has_value())
+    return false;
   return impl_.EnabledCupSigning();
 }
 
@@ -245,10 +253,11 @@ ChromeUpdateClientConfig::~ChromeUpdateClientConfig() = default;
 
 // static
 scoped_refptr<ChromeUpdateClientConfig> ChromeUpdateClientConfig::Create(
-    content::BrowserContext* context) {
+    content::BrowserContext* context,
+    base::Optional<GURL> update_url_override) {
   FactoryCallback& factory = GetFactoryCallback();
-  return factory.is_null() ? scoped_refptr<ChromeUpdateClientConfig>(
-                                 new ChromeUpdateClientConfig(context))
+  return factory.is_null() ? base::MakeRefCounted<ChromeUpdateClientConfig>(
+                                 context, update_url_override)
                            : factory.Run(context);
 }
 

chrome/browser/extensions/updater/chrome_update_client_config.h

@@ -13,9 +13,12 @@
 #include "base/containers/flat_map.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/optional.h"
 #include "components/component_updater/configurator_impl.h"
 #include "components/update_client/configurator.h"
 
+class GURL;
+
 namespace content {
 class BrowserContext;
 }
@@ -37,7 +40,11 @@ class ChromeUpdateClientConfig : public update_client::Configurator {
           content::BrowserContext* context)>;
 
   static scoped_refptr<ChromeUpdateClientConfig> Create(
-      content::BrowserContext* context);
+      content::BrowserContext* context,
+      base::Optional<GURL> url_override);
+
+  ChromeUpdateClientConfig(content::BrowserContext* context,
+                           base::Optional<GURL> url_override);
 
   int InitialDelay() const override;
   int NextCheckDelay() const override;
@@ -71,7 +78,6 @@ class ChromeUpdateClientConfig : public update_client::Configurator {
   friend class base::RefCountedThreadSafe<ChromeUpdateClientConfig>;
   friend class ExtensionUpdateClientBaseTest;
 
-  explicit ChromeUpdateClientConfig(content::BrowserContext* context);
   ~ChromeUpdateClientConfig() override;
 
   // Injects a new client config by changing the creation factory.
@@ -87,6 +93,7 @@ class ChromeUpdateClientConfig : public update_client::Configurator {
   scoped_refptr<update_client::NetworkFetcherFactory> network_fetcher_factory_;
   scoped_refptr<update_client::UnzipperFactory> unzip_factory_;
   scoped_refptr<update_client::PatcherFactory> patch_factory_;
+  base::Optional<GURL> url_override_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeUpdateClientConfig);
 };

chrome/browser/extensions/updater/extension_update_client_base_browsertest.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/extensions/updater/extension_update_client_base_browsertest.h"
 
 #include "base/bind.h"
+#include "base/optional.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/task/post_task.h"
@@ -32,7 +33,7 @@ class TestChromeUpdateClientConfig
   TestChromeUpdateClientConfig(content::BrowserContext* context,
                                const std::vector<GURL>& update_url,
                                const std::vector<GURL>& ping_url)
-      : extensions::ChromeUpdateClientConfig(context),
+      : extensions::ChromeUpdateClientConfig(context, base::nullopt),
         update_url_(update_url),
         ping_url_(ping_url) {}
 

chrome/test/data/extensions/permissions_increase/updates.json

+)]}'
+{
+  "response": {
+    "protocol": "3.1",
+    "app": [
+      {
+        "appid": "pgdpcfcocojkjfbgpiianjngphoopgmo",
+        "updatecheck": {
+          "status": "ok",
+          "manifest": {
+            "version": "2",
+            "packages": {
+              "package": [
+                {
+                  "size": 1103,
+                  "hash_sha256": "dc0b73d9daa1236d0ecd7a0370729e99fb9dbfccf97cea2d2bbf02fb584b2853",
+                  "name": "v2.crx",
+                  "fp": "1.dc0b73d9daa1236d0ecd7a0370729e99fb9dbfccf97cea2d2bbf02fb584b2853"
+                }
+              ]
+            }
+          },
+          "urls": {
+            "url": [
+              {"codebase": "http://localhost/autoupdate/"}
+            ]
+          }
+        }
+      }
+    ]
+  }
+}

extensions/browser/updater/update_data_provider.cc

@@ -26,6 +26,7 @@
 #include "extensions/browser/install/crx_install_error.h"
 #include "extensions/browser/updater/manifest_fetch_data.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/extension_urls.h"
 #include "extensions/common/verifier_formats.h"
 
 namespace extensions {
@@ -108,9 +109,11 @@ UpdateDataProvider::GetData(bool install_immediately,
       crx_component->fingerprint = extension->DifferentialFingerprint();
     }
     crx_component->allows_background_download = false;
-    crx_component->requires_network_encryption = true;
+    bool allow_dev = extension_urls::GetWebstoreUpdateUrl() !=
+                     extension_urls::GetDefaultWebstoreUpdateUrl();
+    crx_component->requires_network_encryption = !allow_dev;
     crx_component->crx_format_requirement =
-        extension->from_webstore() ? GetWebstoreVerifierFormat(false)
+        extension->from_webstore() ? GetWebstoreVerifierFormat(allow_dev)
                                    : GetPolicyVerifierFormat();
     crx_component->installer = base::MakeRefCounted<ExtensionInstaller>(
         id, extension->path(), install_immediately,

extensions/browser/updater/update_service.cc

@@ -87,26 +87,19 @@ void UpdateService::SendUninstallPing(const std::string& id,
 }
 
 bool UpdateService::CanUpdate(const std::string& extension_id) const {
-  // It's possible to change Webstore update URL from command line (through
-  // apps-gallery-update-url command line switch). When Webstore update URL is
-  // different the default Webstore update URL, we won't support updating
-  // extensions through UpdateService.
-  if (extension_urls::GetDefaultWebstoreUpdateUrl() !=
-      extension_urls::GetWebstoreUpdateUrl())
-    return false;
-
   // Won't update extensions with empty IDs.
   if (extension_id.empty())
     return false;
 
   // We can only update extensions that have been installed on the system.
-  // Furthermore, we can only update extensions that were installed from the
-  // webstore or extensions with empty update URLs not converted from user
-  // scripts.
   const ExtensionRegistry* registry = ExtensionRegistry::Get(browser_context_);
   const Extension* extension = registry->GetInstalledExtension(extension_id);
   if (extension == nullptr)
     return false;
+
+  // Furthermore, we can only update extensions that were installed from the
+  // default webstore or extensions with empty update URLs not converted from
+  // user scripts.
   const GURL& update_url = ManifestURL::GetUpdateURL(extension);
   if (update_url.is_empty())
     return !extension->converted_from_user_script();

extensions/browser/updater/update_service_unittest.cc

@@ -745,7 +745,7 @@ class UpdateServiceCanUpdateFeatureEnabledNonDefaultUpdateUrl
     auto* command_line = base::CommandLine::ForCurrentProcess();
     // Note: |offstore_extension_|'s update url is the same.
     command_line->AppendSwitchASCII("apps-gallery-update-url",
-                                    "http://localhost/test/updates.xml");
+                                    "http://localhost/test2/updates.xml");
     ExtensionsClient::Get()->InitializeWebStoreUrls(
         base::CommandLine::ForCurrentProcess());
   }
@@ -756,7 +756,7 @@ TEST_F(UpdateServiceCanUpdateTest, UpdateService_CanUpdate) {
   EXPECT_TRUE(update_service()->CanUpdate(store_extension_->id()));
   // ... and extensions with empty update URL.
   EXPECT_TRUE(update_service()->CanUpdate(emptyurl_extension_->id()));
-  // It can't update off-store extrensions.
+  // It can't update off-store extensions.
   EXPECT_FALSE(update_service()->CanUpdate(offstore_extension_->id()));
   // ... or extensions with empty update URL converted from user script.
   EXPECT_FALSE(update_service()->CanUpdate(userscript_extension_->id()));
@@ -768,10 +768,10 @@ TEST_F(UpdateServiceCanUpdateTest, UpdateService_CanUpdate) {
 
 TEST_F(UpdateServiceCanUpdateFeatureEnabledNonDefaultUpdateUrl,
        UpdateService_CanUpdate) {
-  // Update service cannot update extensions when the default webstore update
-  // url is changed.
+  // Update service can update extensions when the default webstore update url
+  // is changed.
   EXPECT_FALSE(update_service()->CanUpdate(store_extension_->id()));
-  EXPECT_FALSE(update_service()->CanUpdate(emptyurl_extension_->id()));
+  EXPECT_TRUE(update_service()->CanUpdate(emptyurl_extension_->id()));
   EXPECT_FALSE(update_service()->CanUpdate(offstore_extension_->id()));
   EXPECT_FALSE(update_service()->CanUpdate(userscript_extension_->id()));
   EXPECT_FALSE(update_service()->CanUpdate(std::string(32, 'a')));