Allow fallback from https to http for component update checks.
Component updater relies on its custom message signing protocol to
guarantee the integrity of update checks.
As a failover mechanism and to improve the update success rates,
fallback to HTTP is allowed. This is similar to
what Google Update (Omaha) has been doing.
BUG=590070
Review URL: https://codereview.chromium.org/1740333002
Cr-Commit-Position: refs/heads/master@{#384993}
diff --git a/chrome/browser/component_updater/caps_installer_win.cc b/chrome/browser/component_updater/caps_installer_win.cc
index 78942ba..cd4cd9b 100644
--- a/chrome/browser/component_updater/caps_installer_win.cc
+++ b/chrome/browser/component_updater/caps_installer_win.cc
@@ -7,6 +7,7 @@
#include <stdint.h>
#include <string>
#include <utility>
+#include <vector>
#include "base/bind.h"
#include "base/command_line.h"
@@ -78,6 +79,8 @@
bool CanAutoUpdate() const override { return true; }
+ bool RequiresNetworkEncryption() const override { return false; }
+
bool OnCustomInstall(const base::DictionaryValue& manifest,
const base::FilePath& install_dir) override {
return true;
diff --git a/chrome/browser/component_updater/chrome_component_updater_configurator.cc b/chrome/browser/component_updater/chrome_component_updater_configurator.cc
index f915a8c..97e99e7 100644
--- a/chrome/browser/component_updater/chrome_component_updater_configurator.cc
+++ b/chrome/browser/component_updater/chrome_component_updater_configurator.cc
@@ -64,10 +64,13 @@
~ChromeConfigurator() override {}
};
+// Allows the component updater to use non-encrypted communication with the
+// update backend. The security of the update checks is enforced using
+// a custom message signing protocol and it does not depend on using HTTPS.
ChromeConfigurator::ChromeConfigurator(
const base::CommandLine* cmdline,
net::URLRequestContextGetter* url_request_getter)
- : configurator_impl_(cmdline, url_request_getter) {}
+ : configurator_impl_(cmdline, url_request_getter, false) {}
int ChromeConfigurator::InitialDelay() const {
return configurator_impl_.InitialDelay();
diff --git a/chrome/browser/component_updater/chrome_component_updater_configurator_unittest.cc b/chrome/browser/component_updater/chrome_component_updater_configurator_unittest.cc
index 996fd0da..dcd57143 100644
--- a/chrome/browser/component_updater/chrome_component_updater_configurator_unittest.cc
+++ b/chrome/browser/component_updater/chrome_component_updater_configurator_unittest.cc
@@ -9,6 +9,8 @@
#include "base/memory/ref_counted.h"
#include "chrome/browser/component_updater/chrome_component_updater_configurator.h"
#include "components/component_updater/component_updater_switches.h"
+#include "components/component_updater/component_updater_url_constants.h"
+#include "components/component_updater/configurator_impl.h"
#include "components/update_client/configurator.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
@@ -67,7 +69,7 @@
const auto urls = config->UpdateUrl();
// Expect the default url to be cryptographically secure.
- EXPECT_GE(urls.size(), 1ul);
+ EXPECT_GE(urls.size(), 1u);
EXPECT_TRUE(urls.front().SchemeIsCryptographic());
}
@@ -78,4 +80,35 @@
EXPECT_TRUE(config->UseCupSigning());
}
+TEST(ChromeComponentUpdaterConfiguratorTest, TestUseEncryption) {
+ base::CommandLine* cmdline = base::CommandLine::ForCurrentProcess();
+ const auto config(MakeChromeComponentUpdaterConfigurator(cmdline, nullptr));
+
+ const auto urls = config->UpdateUrl();
+ ASSERT_EQ(2u, urls.size());
+ ASSERT_STREQ(kUpdaterDefaultUrl, urls[0].spec().c_str());
+ ASSERT_STREQ(kUpdaterFallbackUrl, urls[1].spec().c_str());
+
+ ASSERT_EQ(config->UpdateUrl(), config->PingUrl());
+
+ // Use the configurator implementation to test the filtering of
+ // unencrypted URLs.
+ {
+ const ConfiguratorImpl config(cmdline, nullptr, true);
+ const auto urls = config.UpdateUrl();
+ ASSERT_EQ(1u, urls.size());
+ ASSERT_STREQ(kUpdaterDefaultUrl, urls[0].spec().c_str());
+ ASSERT_EQ(config.UpdateUrl(), config.PingUrl());
+ }
+
+ {
+ const ConfiguratorImpl config(cmdline, nullptr, false);
+ const auto urls = config.UpdateUrl();
+ ASSERT_EQ(2u, urls.size());
+ ASSERT_STREQ(kUpdaterDefaultUrl, urls[0].spec().c_str());
+ ASSERT_STREQ(kUpdaterFallbackUrl, urls[1].spec().c_str());
+ ASSERT_EQ(config.UpdateUrl(), config.PingUrl());
+ }
+}
+
} // namespace component_updater
diff --git a/chrome/browser/component_updater/cld_component_installer.cc b/chrome/browser/component_updater/cld_component_installer.cc
index d287f43..e96e001 100644
--- a/chrome/browser/component_updater/cld_component_installer.cc
+++ b/chrome/browser/component_updater/cld_component_installer.cc
@@ -56,6 +56,10 @@
return true;
}
+bool CldComponentInstallerTraits::RequiresNetworkEncryption() const {
+ return false;
+}
+
bool CldComponentInstallerTraits::OnCustomInstall(
const base::DictionaryValue& manifest,
const base::FilePath& install_dir) {
diff --git a/chrome/browser/component_updater/cld_component_installer.h b/chrome/browser/component_updater/cld_component_installer.h
index 74600bb..7fc25d1 100644
--- a/chrome/browser/component_updater/cld_component_installer.h
+++ b/chrome/browser/component_updater/cld_component_installer.h
@@ -42,6 +42,7 @@
// The following methods override ComponentInstallerTraits.
bool CanAutoUpdate() const override;
+ bool RequiresNetworkEncryption() const override;
bool OnCustomInstall(const base::DictionaryValue& manifest,
const base::FilePath& install_dir) override;
bool VerifyInstallation(const base::DictionaryValue& manifest,
diff --git a/chrome/browser/component_updater/ev_whitelist_component_installer.cc b/chrome/browser/component_updater/ev_whitelist_component_installer.cc
index 77e6e9d..db98433 100644
--- a/chrome/browser/component_updater/ev_whitelist_component_installer.cc
+++ b/chrome/browser/component_updater/ev_whitelist_component_installer.cc
@@ -75,6 +75,10 @@
return true;
}
+bool EVWhitelistComponentInstallerTraits::RequiresNetworkEncryption() const {
+ return false;
+}
+
bool EVWhitelistComponentInstallerTraits::OnCustomInstall(
const base::DictionaryValue& manifest,
const base::FilePath& install_dir) {
diff --git a/chrome/browser/component_updater/ev_whitelist_component_installer.h b/chrome/browser/component_updater/ev_whitelist_component_installer.h
index 9e765d8..933c345 100644
--- a/chrome/browser/component_updater/ev_whitelist_component_installer.h
+++ b/chrome/browser/component_updater/ev_whitelist_component_installer.h
@@ -30,6 +30,7 @@
private:
// The following methods override ComponentInstallerTraits.
bool CanAutoUpdate() const override;
+ bool RequiresNetworkEncryption() const override;
bool OnCustomInstall(const base::DictionaryValue& manifest,
const base::FilePath& install_dir) override;
bool VerifyInstallation(const base::DictionaryValue& manifest,
diff --git a/chrome/browser/component_updater/supervised_user_whitelist_installer.cc b/chrome/browser/component_updater/supervised_user_whitelist_installer.cc
index 2fe544d6..42f3ed6 100644
--- a/chrome/browser/component_updater/supervised_user_whitelist_installer.cc
+++ b/chrome/browser/component_updater/supervised_user_whitelist_installer.cc
@@ -5,6 +5,7 @@
#include "chrome/browser/component_updater/supervised_user_whitelist_installer.h"
#include <stddef.h>
+#include <map>
#include <utility>
#include "base/bind.h"
@@ -248,6 +249,7 @@
bool VerifyInstallation(const base::DictionaryValue& manifest,
const base::FilePath& install_dir) const override;
bool CanAutoUpdate() const override;
+ bool RequiresNetworkEncryption() const override;
bool OnCustomInstall(const base::DictionaryValue& manifest,
const base::FilePath& install_dir) override;
void ComponentReady(const base::Version& version,
@@ -276,6 +278,11 @@
return true;
}
+bool SupervisedUserWhitelistComponentInstallerTraits::
+ RequiresNetworkEncryption() const {
+ return true;
+}
+
bool SupervisedUserWhitelistComponentInstallerTraits::OnCustomInstall(
const base::DictionaryValue& manifest,
const base::FilePath& install_dir) {
diff --git a/chrome/browser/component_updater/sw_reporter_installer_win.cc b/chrome/browser/component_updater/sw_reporter_installer_win.cc
index 025eacf..f69c808 100644
--- a/chrome/browser/component_updater/sw_reporter_installer_win.cc
+++ b/chrome/browser/component_updater/sw_reporter_installer_win.cc
@@ -115,6 +115,8 @@
bool CanAutoUpdate() const override { return true; }
+ bool RequiresNetworkEncryption() const override { return false; }
+
bool OnCustomInstall(const base::DictionaryValue& manifest,
const base::FilePath& install_dir) override {
return true;
diff --git a/chrome/browser/component_updater/widevine_cdm_component_installer.cc b/chrome/browser/component_updater/widevine_cdm_component_installer.cc
index 8f74a949b..f7b135e9 100644
--- a/chrome/browser/component_updater/widevine_cdm_component_installer.cc
+++ b/chrome/browser/component_updater/widevine_cdm_component_installer.cc
@@ -236,6 +236,7 @@
private:
// The following methods override ComponentInstallerTraits.
bool CanAutoUpdate() const override;
+ bool RequiresNetworkEncryption() const override;
bool OnCustomInstall(const base::DictionaryValue& manifest,
const base::FilePath& install_dir) override;
bool VerifyInstallation(
@@ -268,6 +269,10 @@
return true;
}
+bool WidevineCdmComponentInstallerTraits::RequiresNetworkEncryption() const {
+ return false;
+}
+
bool WidevineCdmComponentInstallerTraits::OnCustomInstall(
const base::DictionaryValue& manifest,
const base::FilePath& install_dir) {
diff --git a/chrome/browser/extensions/updater/chrome_update_client_config.cc b/chrome/browser/extensions/updater/chrome_update_client_config.cc
index 9dcb208..4dfb696 100644
--- a/chrome/browser/extensions/updater/chrome_update_client_config.cc
+++ b/chrome/browser/extensions/updater/chrome_update_client_config.cc
@@ -13,11 +13,13 @@
namespace extensions {
+// For privacy reasons, requires encryption of the component updater
+// communication with the update backend.
ChromeUpdateClientConfig::ChromeUpdateClientConfig(
content::BrowserContext* context)
: impl_(base::CommandLine::ForCurrentProcess(),
- context->GetRequestContext()) {
-}
+ context->GetRequestContext(),
+ true) {}
int ChromeUpdateClientConfig::InitialDelay() const {
return impl_.InitialDelay();
diff --git a/chrome/browser/net/crl_set_fetcher.cc b/chrome/browser/net/crl_set_fetcher.cc
index f2d9caf..0245433 100644
--- a/chrome/browser/net/crl_set_fetcher.cc
+++ b/chrome/browser/net/crl_set_fetcher.cc
@@ -144,7 +144,8 @@
component.installer = this;
component.name = "CRLSet";
component.version = Version(base::UintToString(sequence_of_loaded_crl));
- component.allow_background_download = false;
+ component.allows_background_download = false;
+ component.requires_network_encryption = false;
if (!component.version.IsValid()) {
NOTREACHED();
component.version = Version("0");
diff --git a/components/component_updater/component_updater_service.cc b/components/component_updater/component_updater_service.cc
index 8f0515bd..775e780 100644
--- a/components/component_updater/component_updater_service.cc
+++ b/components/component_updater/component_updater_service.cc
@@ -264,16 +264,33 @@
UMA_HISTOGRAM_ENUMERATION("ComponentUpdater.Calls", UPDATE_TYPE_AUTOMATIC,
UPDATE_TYPE_COUNT);
- std::vector<std::string> ids;
+ std::vector<std::string> secure_ids; // Requires HTTPS for update checks.
+ std::vector<std::string> unsecure_ids; // Can fallback to HTTP.
for (const auto id : components_order_) {
DCHECK(components_.find(id) != components_.end());
- ids.push_back(id);
+
+ auto component(GetComponent(id));
+ if (!component || component->requires_network_encryption)
+ secure_ids.push_back(id);
+ else
+ unsecure_ids.push_back(id);
}
- update_client_->Update(
- ids, base::Bind(&CrxUpdateService::OnUpdate, base::Unretained(this)),
- base::Bind(&CrxUpdateService::OnUpdateComplete, base::Unretained(this),
- base::TimeTicks::Now()));
+ if (!unsecure_ids.empty()) {
+ update_client_->Update(
+ unsecure_ids,
+ base::Bind(&CrxUpdateService::OnUpdate, base::Unretained(this)),
+ base::Bind(&CrxUpdateService::OnUpdateComplete, base::Unretained(this),
+ base::TimeTicks::Now()));
+ }
+
+ if (!secure_ids.empty()) {
+ update_client_->Update(
+ secure_ids,
+ base::Bind(&CrxUpdateService::OnUpdate, base::Unretained(this)),
+ base::Bind(&CrxUpdateService::OnUpdateComplete, base::Unretained(this),
+ base::TimeTicks::Now()));
+ }
return true;
}
diff --git a/components/component_updater/component_updater_url_constants.cc b/components/component_updater/component_updater_url_constants.cc
index 3cb8ef1..c762cc91 100644
--- a/components/component_updater/component_updater_url_constants.cc
+++ b/components/component_updater/component_updater_url_constants.cc
@@ -7,10 +7,15 @@
namespace component_updater {
// The default URL for the v3 protocol service endpoint. In some cases, the
-// component updater is allowed to fall back to and alternate URL source, if
+// component updater is allowed to fall back to other URL endpoints, if
// the request to the default URL source fails.
+//
+// The responses to the requests made to these endpoints are always signed.
+//
// The value of |kDefaultUrlSource| can be overridden with
// --component-updater=url-source=someurl.
const char kUpdaterDefaultUrl[] = "https://clients2.google.com/service/update2";
+const char kUpdaterFallbackUrl[] = "http://clients2.google.com/service/update2";
+
} // namespace component_updater
diff --git a/components/component_updater/component_updater_url_constants.h b/components/component_updater/component_updater_url_constants.h
index 3195dd8..10f3287 100644
--- a/components/component_updater/component_updater_url_constants.h
+++ b/components/component_updater/component_updater_url_constants.h
@@ -8,6 +8,7 @@
namespace component_updater {
extern const char kUpdaterDefaultUrl[];
+extern const char kUpdaterFallbackUrl[];
} // namespace component_updater
diff --git a/components/component_updater/configurator_impl.cc b/components/component_updater/configurator_impl.cc
index 6372118..8da26fd6 100644
--- a/components/component_updater/configurator_impl.cc
+++ b/components/component_updater/configurator_impl.cc
@@ -15,7 +15,7 @@
#include "build/build_config.h"
#include "components/component_updater/component_updater_switches.h"
#include "components/component_updater/component_updater_url_constants.h"
-#include "components/update_client/configurator.h"
+#include "components/update_client/utils.h"
#include "components/version_info/version_info.h"
#if defined(OS_WIN)
@@ -82,12 +82,14 @@
ConfiguratorImpl::ConfiguratorImpl(
const base::CommandLine* cmdline,
- net::URLRequestContextGetter* url_request_getter)
+ net::URLRequestContextGetter* url_request_getter,
+ bool require_encryption)
: url_request_getter_(url_request_getter),
fast_update_(false),
pings_enabled_(false),
deltas_enabled_(false),
- background_downloads_enabled_(false) {
+ background_downloads_enabled_(false),
+ require_encryption_(require_encryption) {
// Parse comma-delimited debug flags.
std::vector<std::string> switch_values = base::SplitString(
cmdline->GetSwitchValueASCII(switches::kComponentUpdater), ",",
@@ -140,9 +142,14 @@
std::vector<GURL> urls;
if (url_source_override_.is_valid()) {
urls.push_back(GURL(url_source_override_));
- } else {
- urls.push_back(GURL(kUpdaterDefaultUrl));
+ return urls;
}
+
+ urls.push_back(GURL(kUpdaterDefaultUrl));
+ urls.push_back(GURL(kUpdaterFallbackUrl));
+ if (require_encryption_)
+ update_client::RemoveUnsecureUrls(&urls);
+
return urls;
}
diff --git a/components/component_updater/configurator_impl.h b/components/component_updater/configurator_impl.h
index d5f99cf..1fa95f1 100644
--- a/components/component_updater/configurator_impl.h
+++ b/components/component_updater/configurator_impl.h
@@ -27,7 +27,8 @@
class ConfiguratorImpl {
public:
ConfiguratorImpl(const base::CommandLine* cmdline,
- net::URLRequestContextGetter* url_request_getter);
+ net::URLRequestContextGetter* url_request_getter,
+ bool require_encryption);
~ConfiguratorImpl();
@@ -92,6 +93,7 @@
bool pings_enabled_;
bool deltas_enabled_;
bool background_downloads_enabled_;
+ bool require_encryption_;
DISALLOW_COPY_AND_ASSIGN(ConfiguratorImpl);
};
diff --git a/components/component_updater/default_component_installer.cc b/components/component_updater/default_component_installer.cc
index b286e29..8f5bba6 100644
--- a/components/component_updater/default_component_installer.cc
+++ b/components/component_updater/default_component_installer.cc
@@ -260,6 +260,8 @@
if (installer_traits_->CanAutoUpdate()) {
CrxComponent crx;
crx.name = installer_traits_->GetName();
+ crx.requires_network_encryption =
+ installer_traits_->RequiresNetworkEncryption();
crx.installer = this;
crx.version = current_version_;
crx.fingerprint = current_fingerprint_;
diff --git a/components/component_updater/default_component_installer.h b/components/component_updater/default_component_installer.h
index 5b99491..9de80d6 100644
--- a/components/component_updater/default_component_installer.h
+++ b/components/component_updater/default_component_installer.h
@@ -49,6 +49,10 @@
// during component registration from the UI thread.
virtual bool CanAutoUpdate() const = 0;
+ // Returns true if the network communication related to this component
+ // must be encrypted.
+ virtual bool RequiresNetworkEncryption() const = 0;
+
// OnCustomInstall is called during the installation process. Components that
// require custom installation operations should implement them here.
// Returns false if a custom operation failed, and true otherwise.
diff --git a/components/update_client/action_update.cc b/components/update_client/action_update.cc
index ea0ad6e..c8045a2 100644
--- a/components/update_client/action_update.cc
+++ b/components/update_client/action_update.cc
@@ -294,7 +294,7 @@
DCHECK(thread_checker_.CalledOnValidThread());
// On demand component updates are always downloaded in foreground.
- return !item->on_demand && item->component.allow_background_download &&
+ return !item->on_demand && item->component.allows_background_download &&
update_context_->config->UseBackgroundDownloader();
}
diff --git a/components/update_client/action_update_check.cc b/components/update_client/action_update_check.cc
index 9e31eb6..13455f5 100644
--- a/components/update_client/action_update_check.cc
+++ b/components/update_client/action_update_check.cc
@@ -97,9 +97,12 @@
void ActionUpdateCheck::UpdateCheckComplete(
int error,
- const UpdateResponse::Results& results) {
+ const UpdateResponse::Results& results,
+ int retry_after_sec) {
DCHECK(thread_checker_.CalledOnValidThread());
+ update_context_->retry_after_sec_ = retry_after_sec;
+
if (!error)
OnUpdateCheckSucceeded(results);
else
diff --git a/components/update_client/action_update_check.h b/components/update_client/action_update_check.h
index 7576d64..67fc4d1 100644
--- a/components/update_client/action_update_check.h
+++ b/components/update_client/action_update_check.h
@@ -36,7 +36,9 @@
void Run(UpdateContext* update_context, Callback callback) override;
private:
- void UpdateCheckComplete(int error, const UpdateResponse::Results& results);
+ void UpdateCheckComplete(int error,
+ const UpdateResponse::Results& results,
+ int retry_after_sec);
void OnUpdateCheckSucceeded(const UpdateResponse::Results& results);
void OnUpdateCheckFailed(int error);
diff --git a/components/update_client/ping_manager.cc b/components/update_client/ping_manager.cc
index 19188145..2d665933 100644
--- a/components/update_client/ping_manager.cc
+++ b/components/update_client/ping_manager.cc
@@ -179,7 +179,9 @@
bool SendPing(const CrxUpdateItem* item);
private:
- void OnRequestSenderComplete(int error, const std::string& response);
+ void OnRequestSenderComplete(int error,
+ const std::string& response,
+ int retry_after_sec);
const scoped_refptr<Configurator> config_;
scoped_ptr<RequestSender> request_sender_;
@@ -196,7 +198,8 @@
}
void PingSender::OnRequestSenderComplete(int error,
- const std::string& response) {
+ const std::string& response,
+ int retry_after_sec) {
DCHECK(thread_checker_.CalledOnValidThread());
delete this;
}
@@ -205,7 +208,9 @@
DCHECK(item);
DCHECK(thread_checker_.CalledOnValidThread());
- std::vector<GURL> urls(config_->PingUrl());
+ auto urls(config_->PingUrl());
+ if (item->component.requires_network_encryption)
+ RemoveUnsecureUrls(&urls);
if (urls.empty())
return false;
@@ -225,12 +230,14 @@
PingManager::~PingManager() {
}
-// Sends a fire and forget ping when the updates are complete. The ping
-// sender object self-deletes after sending the ping has completed asynchrously.
-void PingManager::SendPing(const CrxUpdateItem* item) {
- PingSender* ping_sender(new PingSender(config_));
+bool PingManager::SendPing(const CrxUpdateItem* item) {
+ scoped_ptr<PingSender> ping_sender(new PingSender(config_));
if (!ping_sender->SendPing(item))
- delete ping_sender;
+ return false;
+
+ // The ping sender object self-deletes after sending the ping asynchrously.
+ ping_sender.release();
+ return true;
}
} // namespace update_client
diff --git a/components/update_client/ping_manager.h b/components/update_client/ping_manager.h
index d5a6311d..c907e7d 100644
--- a/components/update_client/ping_manager.h
+++ b/components/update_client/ping_manager.h
@@ -19,7 +19,11 @@
explicit PingManager(const scoped_refptr<Configurator>& config);
virtual ~PingManager();
- virtual void SendPing(const CrxUpdateItem* item);
+ // Sends a fire and forget ping for the |item|. Returns true if the
+ // ping is queued up and may be sent in the future, or false, if an error
+ // occurs right away. The ping itself is not persisted and it will be
+ // discarded if it can't be sent for any reason.
+ virtual bool SendPing(const CrxUpdateItem* item);
private:
const scoped_refptr<Configurator> config_;
diff --git a/components/update_client/ping_manager_unittest.cc b/components/update_client/ping_manager_unittest.cc
index cf1d147..e25dc94 100644
--- a/components/update_client/ping_manager_unittest.cc
+++ b/components/update_client/ping_manager_unittest.cc
@@ -177,4 +177,23 @@
interceptor->Reset();
}
+// Tests that sending the ping fails when the component requires encryption but
+// the ping URL is unsecure.
+TEST_F(ComponentUpdaterPingManagerTest, PingManagerRequiresEncryptionTest) {
+ config_->SetPingUrl(GURL("http:\\foo\bar"));
+
+ {
+ CrxUpdateItem item;
+ item.component.requires_network_encryption = true;
+
+ EXPECT_FALSE(ping_manager_->SendPing(&item));
+ }
+
+ {
+ // Tests that the default for |requires_network_encryption| is true.
+ CrxUpdateItem item;
+ EXPECT_FALSE(ping_manager_->SendPing(&item));
+ }
+}
+
} // namespace update_client
diff --git a/components/update_client/request_sender.cc b/components/update_client/request_sender.cc
index e8fd4660..f3e5814 100644
--- a/components/update_client/request_sender.cc
+++ b/components/update_client/request_sender.cc
@@ -4,6 +4,8 @@
#include "components/update_client/request_sender.h"
+#include <algorithm>
+
#include "base/base64.h"
#include "base/bind.h"
#include "base/bind_helpers.h"
@@ -17,6 +19,7 @@
#include "components/update_client/utils.h"
#include "net/http/http_response_headers.h"
#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_request_status.h"
namespace update_client {
@@ -28,6 +31,25 @@
"MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEB+Yi+3SdJKCyFJmm+suW3CyXygvVsbDbPnJgoC"
"X4GeTtoL8Q/WjPx7CGtXOL1Xjbx0VPPN3DrvqZSL/oXy9hVw==";
+// The ETag header carries the ECSDA signature of the protocol response, if
+// signing has been used.
+const char kHeaderEtag[] = "ETag";
+
+// The server uses the optional X-Retry-After header to indicate that the
+// current request should not be attempted again. Any response received along
+// with the X-Retry-After header should be interpreted as it would have been
+// without the X-Retry-After header.
+//
+// In addition to the presence of the header, the value of the header is
+// used as a signal for when to do future update checks, but only when the
+// response is over https. Values over http are not trusted and are ignored.
+//
+// The value of the header is the number of seconds to wait before trying to do
+// a subsequent update check. The upper bound for the number of seconds to wait
+// before trying to do a subsequent update check is capped at 24 hours.
+const char kHeaderXRetryAfter[] = "X-Retry-After";
+const int64_t kMaxRetryAfterSec = 24 * 60 * 60;
+
} // namespace
// This value is chosen not to conflict with network errors defined by
@@ -55,7 +77,7 @@
request_sender_callback_ = request_sender_callback;
if (urls_.empty()) {
- return HandleSendError(-1);
+ return HandleSendError(-1, 0);
}
cur_url_ = urls_.begin();
@@ -63,7 +85,7 @@
if (use_signing_) {
public_key_ = GetKey(kKeyPubBytesBase64);
if (public_key_.empty())
- return HandleSendError(-1);
+ return HandleSendError(-1, 0);
}
SendInternal();
@@ -91,16 +113,18 @@
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::Bind(&RequestSender::SendInternalComplete, base::Unretained(this),
- -1, std::string(), std::string()));
+ -1, std::string(), std::string(), 0));
}
void RequestSender::SendInternalComplete(int error,
const std::string& response_body,
- const std::string& response_etag) {
+ const std::string& response_etag,
+ int retry_after_sec) {
if (!error) {
if (!use_signing_) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(request_sender_callback_, 0, response_body));
+ FROM_HERE, base::Bind(request_sender_callback_, 0, response_body,
+ retry_after_sec));
return;
}
@@ -108,7 +132,8 @@
DCHECK(signer_.get());
if (signer_->ValidateResponse(response_body, response_etag)) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(request_sender_callback_, 0, response_body));
+ FROM_HERE, base::Bind(request_sender_callback_, 0, response_body,
+ retry_after_sec));
return;
}
@@ -116,35 +141,49 @@
}
DCHECK(error);
- if (++cur_url_ != urls_.end() &&
+
+ // A positive |retry_after_sec| is a hint from the server that the client
+ // should not send further request until the cooldown has expired.
+ if (retry_after_sec <= 0 && ++cur_url_ != urls_.end() &&
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::Bind(&RequestSender::SendInternal, base::Unretained(this)))) {
return;
}
- HandleSendError(error);
+ HandleSendError(error, retry_after_sec);
}
void RequestSender::OnURLFetchComplete(const net::URLFetcher* source) {
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(source);
- VLOG(1) << "request completed from url: " << source->GetOriginalURL().spec();
+ const GURL original_url(source->GetOriginalURL());
+ VLOG(1) << "request completed from url: " << original_url.spec();
const int fetch_error(GetFetchError(*source));
std::string response_body;
CHECK(source->GetResponseAsString(&response_body));
+ int64_t retry_after_sec(-1);
+ const auto status(source->GetStatus().status());
+ if (original_url.SchemeIsCryptographic() &&
+ status == net::URLRequestStatus::SUCCESS) {
+ retry_after_sec = GetInt64HeaderValue(source, kHeaderXRetryAfter);
+ retry_after_sec = std::min(retry_after_sec, kMaxRetryAfterSec);
+ }
+
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE,
- base::Bind(&RequestSender::SendInternalComplete, base::Unretained(this),
- fetch_error, response_body, GetServerETag(source)));
+ FROM_HERE, base::Bind(&RequestSender::SendInternalComplete,
+ base::Unretained(this), fetch_error, response_body,
+ GetStringHeaderValue(source, kHeaderEtag),
+ static_cast<int>(retry_after_sec)));
}
-void RequestSender::HandleSendError(int error) {
+void RequestSender::HandleSendError(int error, int retry_after_sec) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(request_sender_callback_, error, std::string()));
+ FROM_HERE, base::Bind(request_sender_callback_, error, std::string(),
+ retry_after_sec));
}
std::string RequestSender::GetKey(const char* key_bytes_base64) {
@@ -166,15 +205,23 @@
return url.ReplaceComponents(replacements);
}
-std::string RequestSender::GetServerETag(const net::URLFetcher* source) {
+std::string RequestSender::GetStringHeaderValue(const net::URLFetcher* source,
+ const char* header_name) {
const auto response_headers(source->GetResponseHeaders());
if (!response_headers)
return std::string();
std::string etag;
- return response_headers->EnumerateHeader(nullptr, "ETag", &etag)
+ return response_headers->EnumerateHeader(nullptr, header_name, &etag)
? etag
: std::string();
}
+int64_t RequestSender::GetInt64HeaderValue(const net::URLFetcher* source,
+ const char* header_name) {
+ const auto response_headers(source->GetResponseHeaders());
+ return response_headers ? response_headers->GetInt64HeaderValue(header_name)
+ : -1;
+}
+
} // namespace update_client
diff --git a/components/update_client/request_sender.h b/components/update_client/request_sender.h
index ddf5cca..b5daaa2e 100644
--- a/components/update_client/request_sender.h
+++ b/components/update_client/request_sender.h
@@ -5,6 +5,8 @@
#ifndef COMPONENTS_UPDATE_CLIENT_REQUEST_SENDER_H_
#define COMPONENTS_UPDATE_CLIENT_REQUEST_SENDER_H_
+#include <stdint.h>
+
#include <string>
#include <vector>
@@ -35,8 +37,13 @@
class RequestSender : public net::URLFetcherDelegate {
public:
// If |error| is 0, then the response is provided in the |response| parameter.
- using RequestSenderCallback =
- base::Callback<void(int error, const std::string& response)>;
+ // |retry_after_sec| contains the value of the X-Retry-After response header,
+ // when the response was received from a cryptographically secure URL. The
+ // range for this value is [-1, 86400]. If |retry_after_sec| is -1 it means
+ // that the header could not be found, or trusted, or had an invalid value.
+ // The upper bound represents a delay of one day.
+ using RequestSenderCallback = base::Callback<
+ void(int error, const std::string& response, int retry_after_sec)>;
static int kErrorResponseNotTrusted;
@@ -57,9 +64,15 @@
// Decodes and returns the public key used by CUP.
static std::string GetKey(const char* key_bytes_base64);
- // Returns the Etag of the server response or an empty string if the
- // Etag is not available.
- static std::string GetServerETag(const net::URLFetcher* source);
+ // Returns the string value of a header of the server response or an empty
+ // string if the header is not available.
+ static std::string GetStringHeaderValue(const net::URLFetcher* source,
+ const char* header_name);
+
+ // Returns the integral value of a header of the server response or -1 if
+ // if the header is not available or a conversion error has occured.
+ static int64_t GetInt64HeaderValue(const net::URLFetcher* source,
+ const char* header_name);
// Overrides for URLFetcherDelegate.
void OnURLFetchComplete(const net::URLFetcher* source) override;
@@ -67,19 +80,20 @@
// Implements the error handling and url fallback mechanism.
void SendInternal();
- // Called when SendInternal complets. |response_body|and |response_etag|
+ // Called when SendInternal completes. |response_body| and |response_etag|
// contain the body and the etag associated with the HTTP response.
void SendInternalComplete(int error,
const std::string& response_body,
- const std::string& response_etag);
+ const std::string& response_etag,
+ int retry_after_sec);
// Helper function to handle a non-continuable error in Send.
- void HandleSendError(int error);
+ void HandleSendError(int error, int retry_after_sec);
base::ThreadChecker thread_checker_;
const scoped_refptr<Configurator> config_;
- bool use_signing_;
+ bool use_signing_; // True if CUP signing is used.
std::vector<GURL> urls_;
std::string request_body_;
RequestSenderCallback request_sender_callback_;
diff --git a/components/update_client/request_sender_unittest.cc b/components/update_client/request_sender_unittest.cc
index 5132fd9..5582132 100644
--- a/components/update_client/request_sender_unittest.cc
+++ b/components/update_client/request_sender_unittest.cc
@@ -45,7 +45,9 @@
void SetUp() override;
void TearDown() override;
- void RequestSenderComplete(int error, const std::string& response);
+ void RequestSenderComplete(int error,
+ const std::string& response,
+ int retry_after_sec);
protected:
void Quit();
@@ -124,7 +126,8 @@
}
void RequestSenderTest::RequestSenderComplete(int error,
- const std::string& response) {
+ const std::string& response,
+ int retry_after_sec) {
error_ = error;
response_ = response;
diff --git a/components/update_client/test_configurator.cc b/components/update_client/test_configurator.cc
index 93279c4..6ba5392 100644
--- a/components/update_client/test_configurator.cc
+++ b/components/update_client/test_configurator.cc
@@ -55,10 +55,16 @@
}
std::vector<GURL> TestConfigurator::UpdateUrl() const {
+ if (!update_check_url_.is_empty())
+ return std::vector<GURL>(1, update_check_url_);
+
return MakeDefaultUrls();
}
std::vector<GURL> TestConfigurator::PingUrl() const {
+ if (!ping_url_.is_empty())
+ return std::vector<GURL>(1, ping_url_);
+
return UpdateUrl();
}
@@ -133,6 +139,14 @@
download_preference_ = download_preference;
}
+void TestConfigurator::SetUpdateCheckUrl(const GURL& url) {
+ update_check_url_ = url;
+}
+
+void TestConfigurator::SetPingUrl(const GURL& url) {
+ ping_url_ = url;
+}
+
scoped_refptr<base::SequencedTaskRunner>
TestConfigurator::GetSequencedTaskRunner() const {
DCHECK(worker_task_runner_.get());
diff --git a/components/update_client/test_configurator.h b/components/update_client/test_configurator.h
index 391ebc94..ed5bf32bc 100644
--- a/components/update_client/test_configurator.h
+++ b/components/update_client/test_configurator.h
@@ -83,6 +83,8 @@
void SetInitialDelay(int seconds);
void SetDownloadPreference(const std::string& download_preference);
void SetUseCupSigning(bool use_cup_signing);
+ void SetUpdateCheckUrl(const GURL& url);
+ void SetPingUrl(const GURL& url);
private:
friend class base::RefCountedThreadSafe<TestConfigurator>;
@@ -96,6 +98,8 @@
int ondemand_time_;
std::string download_preference_;
bool use_cup_signing_;
+ GURL update_check_url_;
+ GURL ping_url_;
scoped_refptr<net::TestURLRequestContextGetter> context_;
diff --git a/components/update_client/update_checker.cc b/components/update_client/update_checker.cc
index dc98251..234ef22 100644
--- a/components/update_client/update_checker.cc
+++ b/components/update_client/update_checker.cc
@@ -33,6 +33,15 @@
return IsValidBrand(brand) ? brand : std::string("");
}
+// Returns true if at least one item requires network encryption.
+bool IsEncryptionRequired(const std::vector<CrxUpdateItem*>& items) {
+ for (const auto& item : items) {
+ if (item->component.requires_network_encryption)
+ return true;
+ }
+ return false;
+}
+
// Builds an update check request for |components|. |additional_attributes| is
// serialized as part of the <request> element of the request to customize it
// with data that is not platform or component specific. For each |item|, a
@@ -93,7 +102,9 @@
const UpdateCheckCallback& update_check_callback) override;
private:
- void OnRequestSenderComplete(int error, const std::string& response);
+ void OnRequestSenderComplete(int error,
+ const std::string& response,
+ int retry_after_sec);
base::ThreadChecker thread_checker_;
const scoped_refptr<Configurator> config_;
@@ -123,26 +134,30 @@
update_check_callback_ = update_check_callback;
+ auto urls(config_->UpdateUrl());
+ if (IsEncryptionRequired(items_to_check))
+ RemoveUnsecureUrls(&urls);
+
request_sender_.reset(new RequestSender(config_));
request_sender_->Send(
config_->UseCupSigning(),
BuildUpdateCheckRequest(*config_, items_to_check, additional_attributes),
- config_->UpdateUrl(),
- base::Bind(&UpdateCheckerImpl::OnRequestSenderComplete,
- base::Unretained(this)));
+ urls, base::Bind(&UpdateCheckerImpl::OnRequestSenderComplete,
+ base::Unretained(this)));
return true;
}
void UpdateCheckerImpl::OnRequestSenderComplete(int error,
- const std::string& response) {
+ const std::string& response,
+ int retry_after_sec) {
DCHECK(thread_checker_.CalledOnValidThread());
if (!error) {
UpdateResponse update_response;
if (update_response.Parse(response)) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE,
- base::Bind(update_check_callback_, error, update_response.results()));
+ FROM_HERE, base::Bind(update_check_callback_, error,
+ update_response.results(), retry_after_sec));
return;
}
@@ -151,8 +166,8 @@
}
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE,
- base::Bind(update_check_callback_, error, UpdateResponse::Results()));
+ FROM_HERE, base::Bind(update_check_callback_, error,
+ UpdateResponse::Results(), retry_after_sec));
}
} // namespace
diff --git a/components/update_client/update_checker.h b/components/update_client/update_checker.h
index 1de7f0c..1f0eb993 100644
--- a/components/update_client/update_checker.h
+++ b/components/update_client/update_checker.h
@@ -25,7 +25,9 @@
class UpdateChecker {
public:
using UpdateCheckCallback =
- base::Callback<void(int error, const UpdateResponse::Results& results)>;
+ base::Callback<void(int error,
+ const UpdateResponse::Results& results,
+ int retry_after_sec)>;
using Factory =
scoped_ptr<UpdateChecker> (*)(const scoped_refptr<Configurator>& config);
diff --git a/components/update_client/update_checker_unittest.cc b/components/update_client/update_checker_unittest.cc
index 26873c7..3348996 100644
--- a/components/update_client/update_checker_unittest.cc
+++ b/components/update_client/update_checker_unittest.cc
@@ -47,7 +47,9 @@
void SetUp() override;
void TearDown() override;
- void UpdateCheckComplete(int error, const UpdateResponse::Results& results);
+ void UpdateCheckComplete(int error,
+ const UpdateResponse::Results& results,
+ int retry_after_sec);
protected:
void Quit();
@@ -129,7 +131,8 @@
void UpdateCheckerTest::UpdateCheckComplete(
int error,
- const UpdateResponse::Results& results) {
+ const UpdateResponse::Results& results,
+ int retry_after_sec) {
error_ = error;
results_ = results;
Quit();
@@ -308,4 +311,25 @@
EXPECT_EQ(0ul, results_.list.size());
}
+// Tests that the UpdateCheckers will not make an update check for a
+// component that requires encryption when the update check URL is unsecure.
+TEST_F(UpdateCheckerTest, UpdateCheckRequiresEncryptionError) {
+ config_->SetUpdateCheckUrl(GURL("http:\\foo\bar"));
+
+ update_checker_ = UpdateChecker::Create(config_);
+
+ CrxUpdateItem item(BuildCrxUpdateItem());
+ item.component.requires_network_encryption = true;
+ std::vector<CrxUpdateItem*> items_to_check;
+ items_to_check.push_back(&item);
+
+ update_checker_->CheckForUpdates(
+ items_to_check, "", base::Bind(&UpdateCheckerTest::UpdateCheckComplete,
+ base::Unretained(this)));
+ RunThreads();
+
+ EXPECT_EQ(-1, error_);
+ EXPECT_EQ(0u, results_.list.size());
+}
+
} // namespace update_client
diff --git a/components/update_client/update_client.cc b/components/update_client/update_client.cc
index 86b3dfc..8fa551d 100644
--- a/components/update_client/update_client.cc
+++ b/components/update_client/update_client.cc
@@ -52,8 +52,8 @@
CrxUpdateItem::~CrxUpdateItem() {
}
-CrxComponent::CrxComponent() : allow_background_download(true) {
-}
+CrxComponent::CrxComponent()
+ : allows_background_download(true), requires_network_encryption(true) {}
CrxComponent::CrxComponent(const CrxComponent& other) = default;
diff --git a/components/update_client/update_client.h b/components/update_client/update_client.h
index 12d19c9..e354fb6 100644
--- a/components/update_client/update_client.h
+++ b/components/update_client/update_client.h
@@ -143,6 +143,7 @@
ERROR_UPDATE_INVALID_ARGUMENT = -1,
ERROR_UPDATE_IN_PROGRESS = 1,
ERROR_UPDATE_CANCELED = 2,
+ ERROR_UPDATE_RETRY_LATER = 3,
};
// Defines an interface for a generic CRX installer.
@@ -197,10 +198,14 @@
std::string name; // Optional.
// Specifies that the CRX can be background-downloaded in some cases.
- // The default for this value is |true| and the value can be overriden at
- // the registration time. This is a temporary change until the issue
- // crbug/340448 is resolved.
- bool allow_background_download;
+ // The default for this value is |true|.
+ bool allows_background_download;
+
+ // Specifies that the update checks and pings associated with this component
+ // require confidentiality. The default for this value is |true|. As a side
+ // note, the confidentiality of the downloads is enforced by the server,
+ // which only returns secure download URLs in this case.
+ bool requires_network_encryption;
};
// All methods are safe to call only from the browser's main thread. Once an
diff --git a/components/update_client/update_client_unittest.cc b/components/update_client/update_client_unittest.cc
index a0bee0f..957f6973 100644
--- a/components/update_client/update_client_unittest.cc
+++ b/components/update_client/update_client_unittest.cc
@@ -97,7 +97,7 @@
explicit FakePingManagerImpl(const scoped_refptr<Configurator>& config);
~FakePingManagerImpl() override;
- void SendPing(const CrxUpdateItem* item) override;
+ bool SendPing(const CrxUpdateItem* item) override;
const std::vector<CrxUpdateItem>& items() const;
@@ -113,8 +113,9 @@
FakePingManagerImpl::~FakePingManagerImpl() {
}
-void FakePingManagerImpl::SendPing(const CrxUpdateItem* item) {
+bool FakePingManagerImpl::SendPing(const CrxUpdateItem* item) {
items_.push_back(*item);
+ return true;
}
const std::vector<CrxUpdateItem>& FakePingManagerImpl::items() const {
@@ -227,7 +228,7 @@
const UpdateCheckCallback& update_check_callback) override {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- base::Bind(update_check_callback, 0, UpdateResponse::Results()));
+ base::Bind(update_check_callback, 0, UpdateResponse::Results(), 0));
return true;
}
};
@@ -368,7 +369,7 @@
results.list.push_back(result);
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(update_check_callback, 0, results));
+ FROM_HERE, base::Bind(update_check_callback, 0, results, 0));
return true;
}
};
@@ -580,7 +581,7 @@
results.list.push_back(result2);
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(update_check_callback, 0, results));
+ FROM_HERE, base::Bind(update_check_callback, 0, results, 0));
return true;
}
};
@@ -827,7 +828,7 @@
results.list.push_back(result2);
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(update_check_callback, 0, results));
+ FROM_HERE, base::Bind(update_check_callback, 0, results, 0));
return true;
}
};
@@ -1099,7 +1100,7 @@
}
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(update_check_callback, 0, results));
+ FROM_HERE, base::Bind(update_check_callback, 0, results, 0));
return true;
}
};
@@ -1340,7 +1341,7 @@
results.list.push_back(result);
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(update_check_callback, 0, results));
+ FROM_HERE, base::Bind(update_check_callback, 0, results, 0));
return true;
}
};
@@ -1573,7 +1574,7 @@
}
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(update_check_callback, 0, results));
+ FROM_HERE, base::Bind(update_check_callback, 0, results, 0));
return true;
}
};
@@ -1778,7 +1779,7 @@
const UpdateCheckCallback& update_check_callback) override {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- base::Bind(update_check_callback, 0, UpdateResponse::Results()));
+ base::Bind(update_check_callback, 0, UpdateResponse::Results(), 0));
return true;
}
};
@@ -1911,7 +1912,7 @@
results.list.push_back(result);
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(update_check_callback, 0, results));
+ FROM_HERE, base::Bind(update_check_callback, 0, results, 0));
return true;
}
};
@@ -2063,7 +2064,7 @@
const UpdateCheckCallback& update_check_callback) override {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- base::Bind(update_check_callback, 0, UpdateResponse::Results()));
+ base::Bind(update_check_callback, 0, UpdateResponse::Results(), 0));
return true;
}
};
@@ -2243,4 +2244,174 @@
base::Version("1.0"), 10);
}
+TEST_F(UpdateClientTest, RetryAfter) {
+ class DataCallbackFake {
+ public:
+ static void Callback(const std::vector<std::string>& ids,
+ std::vector<CrxComponent>* components) {
+ CrxComponent crx;
+ crx.name = "test_jebg";
+ crx.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash));
+ crx.version = Version("0.9");
+ crx.installer = new TestInstaller;
+ components->push_back(crx);
+ }
+ };
+
+ class CompletionCallbackFake {
+ public:
+ static void Callback(const base::Closure& quit_closure, int error) {
+ static int num_call = 0;
+ ++num_call;
+
+ EXPECT_LE(num_call, 4);
+
+ if (num_call == 1) {
+ EXPECT_EQ(0, error);
+ } else if (num_call == 2) {
+ // This request is throttled since the update engine received a
+ // positive |retry_after_sec| value in the update check response.
+ EXPECT_EQ(Error::ERROR_UPDATE_RETRY_LATER, error);
+ } else if (num_call == 3) {
+ // This request is a foreground Install, which is never throttled.
+ // The update engine received a |retry_after_sec| value of 0, which
+ // resets the throttling.
+ EXPECT_EQ(0, error);
+ } else if (num_call == 4) {
+ // This request succeeds since there is no throttling in effect.
+ EXPECT_EQ(0, error);
+ }
+
+ quit_closure.Run();
+ }
+ };
+
+ class FakeUpdateChecker : public UpdateChecker {
+ public:
+ static scoped_ptr<UpdateChecker> Create(
+ const scoped_refptr<Configurator>& config) {
+ return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
+ }
+
+ bool CheckForUpdates(
+ const std::vector<CrxUpdateItem*>& items_to_check,
+ const std::string& additional_attributes,
+ const UpdateCheckCallback& update_check_callback) override {
+ static int num_call = 0;
+ ++num_call;
+
+ EXPECT_LE(num_call, 3);
+
+ int retry_after_sec(0);
+ if (num_call == 1) {
+ // Throttle the next call.
+ retry_after_sec = 60 * 60; // 1 hour.
+ }
+
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE, base::Bind(update_check_callback, 0,
+ UpdateResponse::Results(), retry_after_sec));
+ return true;
+ }
+ };
+
+ class FakeCrxDownloader : public CrxDownloader {
+ public:
+ static scoped_ptr<CrxDownloader> Create(
+ bool is_background_download,
+ net::URLRequestContextGetter* context_getter,
+ const scoped_refptr<base::SequencedTaskRunner>& task_runner) {
+ return scoped_ptr<CrxDownloader>(new FakeCrxDownloader());
+ }
+
+ private:
+ FakeCrxDownloader()
+ : CrxDownloader(base::ThreadTaskRunnerHandle::Get(), nullptr) {}
+ ~FakeCrxDownloader() override {}
+
+ void DoStartDownload(const GURL& url) override { EXPECT_TRUE(false); }
+ };
+
+ class FakePingManager : public FakePingManagerImpl {
+ public:
+ explicit FakePingManager(const scoped_refptr<Configurator>& config)
+ : FakePingManagerImpl(config) {}
+ ~FakePingManager() override { EXPECT_TRUE(items().empty()); }
+ };
+
+ scoped_ptr<PingManager> ping_manager(new FakePingManager(config()));
+ scoped_refptr<UpdateClient> update_client(new UpdateClientImpl(
+ config(), std::move(ping_manager), &FakeUpdateChecker::Create,
+ &FakeCrxDownloader::Create));
+
+ MockObserver observer;
+
+ InSequence seq;
+ EXPECT_CALL(observer, OnEvent(Events::COMPONENT_CHECKING_FOR_UPDATES,
+ "jebgalgnebhfojomionfpkfelancnnkf"))
+ .Times(1);
+ EXPECT_CALL(observer, OnEvent(Events::COMPONENT_NOT_UPDATED,
+ "jebgalgnebhfojomionfpkfelancnnkf"))
+ .Times(1);
+ EXPECT_CALL(observer, OnEvent(Events::COMPONENT_CHECKING_FOR_UPDATES,
+ "jebgalgnebhfojomionfpkfelancnnkf"))
+ .Times(1);
+ EXPECT_CALL(observer, OnEvent(Events::COMPONENT_NOT_UPDATED,
+ "jebgalgnebhfojomionfpkfelancnnkf"))
+ .Times(1);
+ EXPECT_CALL(observer, OnEvent(Events::COMPONENT_CHECKING_FOR_UPDATES,
+ "jebgalgnebhfojomionfpkfelancnnkf"))
+ .Times(1);
+ EXPECT_CALL(observer, OnEvent(Events::COMPONENT_NOT_UPDATED,
+ "jebgalgnebhfojomionfpkfelancnnkf"))
+ .Times(1);
+
+ update_client->AddObserver(&observer);
+
+ std::vector<std::string> ids;
+ ids.push_back(std::string("jebgalgnebhfojomionfpkfelancnnkf"));
+
+ {
+ // The engine handles this Update call but responds with a valid
+ // |retry_after_sec|, which causes subsequent calls to fail.
+ base::RunLoop runloop;
+ update_client->Update(
+ ids, base::Bind(&DataCallbackFake::Callback),
+ base::Bind(&CompletionCallbackFake::Callback, runloop.QuitClosure()));
+ runloop.Run();
+ }
+
+ {
+ // This call will result in a completion callback invoked with
+ // Error::ERROR_UPDATE_RETRY_LATER.
+ base::RunLoop runloop;
+ update_client->Update(
+ ids, base::Bind(&DataCallbackFake::Callback),
+ base::Bind(&CompletionCallbackFake::Callback, runloop.QuitClosure()));
+ runloop.Run();
+ }
+
+ {
+ // The Install call is handled, and the throttling is reset due to
+ // the value of |retry_after_sec| in the completion callback.
+ base::RunLoop runloop;
+ update_client->Install(
+ std::string("jebgalgnebhfojomionfpkfelancnnkf"),
+ base::Bind(&DataCallbackFake::Callback),
+ base::Bind(&CompletionCallbackFake::Callback, runloop.QuitClosure()));
+ runloop.Run();
+ }
+
+ {
+ // This call succeeds.
+ base::RunLoop runloop;
+ update_client->Update(
+ ids, base::Bind(&DataCallbackFake::Callback),
+ base::Bind(&CompletionCallbackFake::Callback, runloop.QuitClosure()));
+ runloop.Run();
+ }
+
+ update_client->RemoveObserver(&observer);
+}
+
} // namespace update_client
diff --git a/components/update_client/update_engine.cc b/components/update_client/update_engine.cc
index 06cf6eb..20f9a6a 100644
--- a/components/update_client/update_engine.cc
+++ b/components/update_client/update_engine.cc
@@ -9,6 +9,7 @@
#include "base/location.h"
#include "base/stl_util.h"
#include "base/thread_task_runner_handle.h"
+#include "base/time/time.h"
#include "components/update_client/action_update_check.h"
#include "components/update_client/configurator.h"
#include "components/update_client/crx_update_item.h"
@@ -36,8 +37,8 @@
blocking_task_runner(config->GetSequencedTaskRunner()),
update_checker_factory(update_checker_factory),
crx_downloader_factory(crx_downloader_factory),
- ping_manager(ping_manager) {
-}
+ ping_manager(ping_manager),
+ retry_after_sec_(0) {}
UpdateContext::~UpdateContext() {
STLDeleteElements(&update_items);
@@ -84,6 +85,12 @@
const CompletionCallback& callback) {
DCHECK(thread_checker_.CalledOnValidThread());
+ if (IsThrottled(is_foreground)) {
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE, base::Bind(callback, Error::ERROR_UPDATE_RETRY_LATER));
+ return;
+ }
+
scoped_ptr<UpdateContext> update_context(new UpdateContext(
config_, is_foreground, ids, crx_data_callback,
notify_observers_callback_, callback, update_checker_factory_,
@@ -109,6 +116,18 @@
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(update_contexts_.find(update_context) != update_contexts_.end());
+ const int throttle_sec(update_context->retry_after_sec_);
+ DCHECK_LE(throttle_sec, 24 * 60 * 60);
+
+ // Only positive values for throttle_sec are effective. 0 means that no
+ // throttling occurs and has the effect of resetting the member.
+ // Negative values are not trusted and are ignored.
+ if (throttle_sec >= 0)
+ throttle_updates_until_ =
+ throttle_sec
+ ? base::Time::Now() + base::TimeDelta::FromSeconds(throttle_sec)
+ : base::Time();
+
auto callback = update_context->callback;
update_contexts_.erase(update_context);
@@ -117,4 +136,16 @@
callback.Run(error);
}
+bool UpdateEngine::IsThrottled(bool is_foreground) const {
+ if (is_foreground || throttle_updates_until_.is_null())
+ return false;
+
+ const auto now(base::Time::Now());
+
+ // Throttle the calls in the interval (t - 1 day, t) to limit the effect of
+ // unset clocks or clock drift.
+ return throttle_updates_until_ - base::TimeDelta::FromDays(1) < now &&
+ now < throttle_updates_until_;
+}
+
} // namespace update_client
diff --git a/components/update_client/update_engine.h b/components/update_client/update_engine.h
index f2a577a3..1d69ece 100644
--- a/components/update_client/update_engine.h
+++ b/components/update_client/update_engine.h
@@ -63,6 +63,10 @@
private:
void UpdateComplete(UpdateContext* update_context, int error);
+ // Returns true if the update engine rejects this update call because it
+ // occurs too soon.
+ bool IsThrottled(bool is_foreground) const;
+
base::ThreadChecker thread_checker_;
scoped_refptr<Configurator> config_;
@@ -79,6 +83,12 @@
// Contains the contexts associated with each update in progress.
std::set<UpdateContext*> update_contexts_;
+ // Implements a rate limiting mechanism for background update checks. Has the
+ // effect of rejecting the update call if the update call occurs before
+ // a certain time, which is negotiated with the server as part of the
+ // update protocol. See the comments for X-Retry-After header.
+ base::Time throttle_updates_until_;
+
DISALLOW_COPY_AND_ASSIGN(UpdateEngine);
};
@@ -135,6 +145,9 @@
// Contains the ids of the items to update.
std::queue<std::string> queue;
+
+ // The time in seconds to wait until doing further update checks.
+ int retry_after_sec_;
};
} // namespace update_client
diff --git a/components/update_client/utils.cc b/components/update_client/utils.cc
index a76e073..caa00cea 100644
--- a/components/update_client/utils.cc
+++ b/components/update_client/utils.cc
@@ -34,6 +34,7 @@
#include "net/url_request/url_fetcher.h"
#include "net/url_request/url_request_context_getter.h"
#include "net/url_request/url_request_status.h"
+#include "url/gurl.h"
namespace update_client {
@@ -238,4 +239,12 @@
}) == brand.end();
}
+void RemoveUnsecureUrls(std::vector<GURL>* urls) {
+ DCHECK(urls);
+ urls->erase(std::remove_if(
+ urls->begin(), urls->end(),
+ [](const GURL& url) { return !url.SchemeIsCryptographic(); }),
+ urls->end());
+}
+
} // namespace update_client
diff --git a/components/update_client/utils.h b/components/update_client/utils.h
index bd428ca..af353f7 100644
--- a/components/update_client/utils.h
+++ b/components/update_client/utils.h
@@ -6,6 +6,7 @@
#define COMPONENTS_UPDATE_CLIENT_UTILS_H_
#include <string>
+#include <vector>
#include "base/memory/scoped_ptr.h"
@@ -98,6 +99,9 @@
// Returns true if the |brand| parameter matches ^([a-zA-Z]{4})?$ .
bool IsValidBrand(const std::string& brand);
+// Removes the unsecure urls in the |urls| parameter.
+void RemoveUnsecureUrls(std::vector<GURL>* urls);
+
} // namespace update_client
#endif // COMPONENTS_UPDATE_CLIENT_UTILS_H_
diff --git a/components/update_client/utils_unittest.cc b/components/update_client/utils_unittest.cc
index 81530705..b15fcac4 100644
--- a/components/update_client/utils_unittest.cc
+++ b/components/update_client/utils_unittest.cc
@@ -81,4 +81,35 @@
EXPECT_FALSE(IsValidBrand(std::string("\xaa"))); // Has non-ASCII char.
}
+TEST(UpdateClientUtils, RemoveUnsecureUrls) {
+ const GURL test1[] = {GURL("http://foo"), GURL("https://foo")};
+ std::vector<GURL> urls(std::begin(test1), std::end(test1));
+ RemoveUnsecureUrls(&urls);
+ EXPECT_EQ(1u, urls.size());
+ EXPECT_EQ(urls[0], GURL("https://foo"));
+
+ const GURL test2[] = {GURL("https://foo"), GURL("http://foo")};
+ urls.assign(std::begin(test2), std::end(test2));
+ RemoveUnsecureUrls(&urls);
+ EXPECT_EQ(1u, urls.size());
+ EXPECT_EQ(urls[0], GURL("https://foo"));
+
+ const GURL test3[] = {GURL("https://foo"), GURL("https://bar")};
+ urls.assign(std::begin(test3), std::end(test3));
+ RemoveUnsecureUrls(&urls);
+ EXPECT_EQ(2u, urls.size());
+ EXPECT_EQ(urls[0], GURL("https://foo"));
+ EXPECT_EQ(urls[1], GURL("https://bar"));
+
+ const GURL test4[] = {GURL("http://foo")};
+ urls.assign(std::begin(test4), std::end(test4));
+ RemoveUnsecureUrls(&urls);
+ EXPECT_EQ(0u, urls.size());
+
+ const GURL test5[] = {GURL("http://foo"), GURL("http://bar")};
+ urls.assign(std::begin(test5), std::end(test5));
+ RemoveUnsecureUrls(&urls);
+ EXPECT_EQ(0u, urls.size());
+}
+
} // namespace update_client
diff --git a/extensions/browser/updater/update_data_provider.cc b/extensions/browser/updater/update_data_provider.cc
index 1adb8f9..b86eaa8 100644
--- a/extensions/browser/updater/update_data_provider.cc
+++ b/extensions/browser/updater/update_data_provider.cc
@@ -48,8 +48,8 @@
crypto::SHA256HashString(pubkey_bytes, info->pk_hash.data(),
info->pk_hash.size());
info->version = *extension->version();
- info->allow_background_download = false;
-
+ info->allows_background_download = false;
+ info->requires_network_encryption = true;
info->installer = new UpdateInstallShim(
id, extension->path(),
base::Bind(&UpdateDataProvider::RunInstallCallback, this));
diff --git a/ios/chrome/browser/component_updater/ios_component_updater_configurator.cc b/ios/chrome/browser/component_updater/ios_component_updater_configurator.cc
index 0ad785b5..c92046e 100644
--- a/ios/chrome/browser/component_updater/ios_component_updater_configurator.cc
+++ b/ios/chrome/browser/component_updater/ios_component_updater_configurator.cc
@@ -57,10 +57,13 @@
~IOSConfigurator() override {}
};
+// Allows the component updater to use non-encrypted communication with the
+// update backend. The security of the update checks is enforced using
+// a custom message signing protocol and it does not depend on using HTTPS.
IOSConfigurator::IOSConfigurator(
const base::CommandLine* cmdline,
net::URLRequestContextGetter* url_request_getter)
- : configurator_impl_(cmdline, url_request_getter) {}
+ : configurator_impl_(cmdline, url_request_getter, false) {}
int IOSConfigurator::InitialDelay() const {
return configurator_impl_.InitialDelay();
diff --git a/ios/chrome/browser/net/crl_set_fetcher.cc b/ios/chrome/browser/net/crl_set_fetcher.cc
index 71d16a23..a7e78a23 100644
--- a/ios/chrome/browser/net/crl_set_fetcher.cc
+++ b/ios/chrome/browser/net/crl_set_fetcher.cc
@@ -136,7 +136,8 @@
component.installer = this;
component.name = "CRLSet";
component.version = Version(base::UintToString(sequence_of_loaded_crl));
- component.allow_background_download = false;
+ component.allows_background_download = false;
+ component.requires_network_encryption = false;
if (!component.version.IsValid()) {
NOTREACHED();
component.version = Version("0");
