Implement Windows GPO support for "dlpref".

This is a feature of the update protocol, which allows the
server to prioritize returning urls that can be cached
by downstream proxies.

BUG=579762

Review URL: https://codereview.chromium.org/1606943007

Cr-Commit-Position: refs/heads/master@{#371576}

chrome/browser/component_updater/chrome_component_updater_configurator.cc

@@ -7,11 +7,18 @@
 #include <string>
 #include <vector>
 
+#include "base/strings/sys_string_conversions.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/version.h"
+#if defined(OS_WIN)
+#include "base/win/win_util.h"
+#endif
 #include "chrome/browser/component_updater/component_patcher_operation_out_of_process.h"
 #include "chrome/browser/update_client/chrome_update_query_params_delegate.h"
 #include "chrome/common/channel_info.h"
+#if defined(OS_WIN)
+#include "chrome/installer/util/google_update_settings.h"
+#endif
 #include "components/component_updater/configurator_impl.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -37,6 +44,7 @@ class ChromeConfigurator : public update_client::Configurator {
   std::string GetLang() const override;
   std::string GetOSLongName() const override;
   std::string ExtraRequestParams() const override;
+  std::string GetDownloadPreference() const override;
   net::URLRequestContextGetter* RequestContext() const override;
   scoped_refptr<update_client::OutOfProcessPatcher> CreateOutOfProcessPatcher()
       const override;
@@ -106,6 +114,19 @@ std::string ChromeConfigurator::ExtraRequestParams() const {
   return configurator_impl_.ExtraRequestParams();
 }
 
+std::string ChromeConfigurator::GetDownloadPreference() const {
+#if defined(OS_WIN)
+  // This group policy is supported only on Windows and only for computers
+  // which are joined to a Windows domain.
+  return base::win::IsEnrolledToDomain()
+             ? base::SysWideToUTF8(
+                   GoogleUpdateSettings::GetDownloadPreference())
+             : std::string("");
+#else
+  return std::string("");
+#endif
+}
+
 net::URLRequestContextGetter* ChromeConfigurator::RequestContext() const {
   return configurator_impl_.RequestContext();
 }

chrome/browser/extensions/updater/chrome_update_client_config.cc

@@ -66,6 +66,10 @@ std::string ChromeUpdateClientConfig::ExtraRequestParams() const {
   return impl_.ExtraRequestParams();
 }
 
+std::string ChromeUpdateClientConfig::GetDownloadPreference() const {
+  return std::string();
+}
+
 net::URLRequestContextGetter* ChromeUpdateClientConfig::RequestContext() const {
   return impl_.RequestContext();
 }

chrome/browser/extensions/updater/chrome_update_client_config.h

@@ -35,6 +35,7 @@ class ChromeUpdateClientConfig : public UpdateClientConfig {
   std::string GetLang() const override;
   std::string GetOSLongName() const override;
   std::string ExtraRequestParams() const override;
+  std::string GetDownloadPreference() const override;
   net::URLRequestContextGetter* RequestContext() const override;
   scoped_refptr<update_client::OutOfProcessPatcher> CreateOutOfProcessPatcher()
       const override;

chrome/installer/util/google_update_settings.cc

@@ -8,6 +8,7 @@
 
 #include <algorithm>
 #include <limits>
+#include <vector>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ -36,6 +37,8 @@ using installer::InstallationState;
 const wchar_t GoogleUpdateSettings::kPoliciesKey[] =
     L"SOFTWARE\\Policies\\Google\\Update";
 const wchar_t GoogleUpdateSettings::kUpdatePolicyValue[] = L"UpdateDefault";
+const wchar_t GoogleUpdateSettings::kDownloadPreferencePolicyValue[] =
+    L"DownloadPreference";
 const wchar_t GoogleUpdateSettings::kUpdateOverrideValuePrefix[] = L"Update";
 const wchar_t GoogleUpdateSettings::kCheckPeriodOverrideMinutes[] =
     L"AutoUpdateCheckPeriodMinutes";
@@ -770,6 +773,30 @@ bool GoogleUpdateSettings::ReenableAutoupdates() {
   return true;
 }
 
+// Reads and sanitizes the value of
+// "HKLM\SOFTWARE\Policies\Google\Update\DownloadPreference". A valid
+// group policy option must be a single alpha numeric word of up to 32
+// characters.
+base::string16 GoogleUpdateSettings::GetDownloadPreference() {
+  RegKey policy_key;
+  base::string16 value;
+  if (policy_key.Open(HKEY_LOCAL_MACHINE, kPoliciesKey, KEY_QUERY_VALUE) ==
+          ERROR_SUCCESS &&
+      policy_key.ReadValue(kDownloadPreferencePolicyValue, &value) ==
+          ERROR_SUCCESS) {
+    // Validates that |value| matches `[a-zA-z]{0-32}`.
+    const size_t kMaxValueLength = 32;
+    if (value.size() > kMaxValueLength)
+      return base::string16();
+    for (auto ch : value) {
+      if (!base::IsAsciiAlpha(ch))
+        return base::string16();
+    }
+    return value;
+  }
+  return base::string16();
+}
+
 void GoogleUpdateSettings::RecordChromeUpdatePolicyHistograms() {
   const bool is_multi_install = InstallUtil::IsMultiInstall(
       BrowserDistribution::GetDistribution(), IsSystemInstall());

chrome/installer/util/google_update_settings.h

@@ -26,9 +26,11 @@ class ChannelInfo;
 class InstallationState;
 }
 
-// This class provides accessors to the Google Update 'ClientState' information
-// that recorded when the user downloads the chrome installer. It is
-// google_update.exe responsibility to write the initial values.
+// This class provides accessors to the Google Update group policies and
+// 'ClientState' information. The group policies are set using specific
+// administrative templates. The 'ClientState' information is recorded when the
+// user downloads the Chrome installer. It is google_update.exe responsibility
+// to write the initial values.
 class GoogleUpdateSettings {
  public:
   // Update policy constants defined by Google Update; do not change these.
@@ -44,6 +46,7 @@ class GoogleUpdateSettings {
   static const wchar_t kUpdatePolicyValue[];
   static const wchar_t kUpdateOverrideValuePrefix[];
   static const wchar_t kCheckPeriodOverrideMinutes[];
+  static const wchar_t kDownloadPreferencePolicyValue[];
   static const int kCheckPeriodOverrideMinutesDefault;
   static const int kCheckPeriodOverrideMinutesMax;
   static const GoogleUpdateSettings::UpdatePolicy kDefaultUpdatePolicy;
@@ -266,6 +269,14 @@ class GoogleUpdateSettings {
   // is assumed not to autoupdate.
   static bool ReenableAutoupdates();
 
+  // Returns a string if the corresponding Google Update group policy is set.
+  // Returns an empty string if no policy or an invalid policy is set.
+  // A valid policy for DownloadPreference is a string that matches the
+  // following regex:  `[a-zA-z]{0-32}`. The actual values for this policy
+  // are specific to Google Update and documented as part of the Google Update
+  // protocol.
+  static base::string16 GetDownloadPreference();
+
   // Records UMA stats about Chrome's update policy.
   static void RecordChromeUpdatePolicyHistograms();
 

chrome/installer/util/google_update_settings_unittest.cc

@@ -990,6 +990,58 @@ TEST_F(GoogleUpdateSettingsTest, ExperimentsLabelHelperUser) {
 
 #endif  // defined(GOOGLE_CHROME_BUILD)
 
+TEST_F(GoogleUpdateSettingsTest, GetDownloadPreference) {
+  RegKey policy_key;
+
+  if (policy_key.Open(HKEY_LOCAL_MACHINE, GoogleUpdateSettings::kPoliciesKey,
+                      KEY_SET_VALUE) == ERROR_SUCCESS) {
+    policy_key.DeleteValue(
+        GoogleUpdateSettings::kDownloadPreferencePolicyValue);
+  }
+  policy_key.Close();
+
+  // When no policy is present expect to return an empty string.
+  EXPECT_TRUE(GoogleUpdateSettings::GetDownloadPreference().empty());
+
+  // Expect "cacheable" when the correct policy is present.
+  EXPECT_EQ(ERROR_SUCCESS, policy_key.Create(HKEY_LOCAL_MACHINE,
+                                             GoogleUpdateSettings::kPoliciesKey,
+                                             KEY_SET_VALUE));
+  EXPECT_EQ(
+      ERROR_SUCCESS,
+      policy_key.WriteValue(
+          GoogleUpdateSettings::kDownloadPreferencePolicyValue, L"cacheable"));
+  EXPECT_STREQ(L"cacheable",
+               GoogleUpdateSettings::GetDownloadPreference().c_str());
+
+  EXPECT_EQ(ERROR_SUCCESS,
+            policy_key.WriteValue(
+                GoogleUpdateSettings::kDownloadPreferencePolicyValue,
+                base::string16(32, L'a').c_str()));
+  EXPECT_STREQ(base::string16(32, L'a').c_str(),
+               GoogleUpdateSettings::GetDownloadPreference().c_str());
+
+  // Expect an empty string when an unsupported policy is set.
+  // It contains spaces.
+  EXPECT_EQ(ERROR_SUCCESS,
+            policy_key.WriteValue(
+                GoogleUpdateSettings::kDownloadPreferencePolicyValue, L"a b"));
+  EXPECT_TRUE(GoogleUpdateSettings::GetDownloadPreference().empty());
+
+  // It contains non alpha-numeric characters.
+  EXPECT_EQ(ERROR_SUCCESS,
+            policy_key.WriteValue(
+                GoogleUpdateSettings::kDownloadPreferencePolicyValue, L"<a>"));
+  EXPECT_TRUE(GoogleUpdateSettings::GetDownloadPreference().empty());
+
+  // It is too long.
+  EXPECT_EQ(ERROR_SUCCESS,
+            policy_key.WriteValue(
+                GoogleUpdateSettings::kDownloadPreferencePolicyValue,
+                base::string16(33, L'a').c_str()));
+  EXPECT_TRUE(GoogleUpdateSettings::GetDownloadPreference().empty());
+}
+
 // Test GoogleUpdateSettings::GetUninstallCommandLine at system- or user-level,
 // according to the param.
 class GetUninstallCommandLine : public GoogleUpdateSettingsTest,

components/component_updater/configurator_impl.cc

@@ -163,6 +163,10 @@ std::string ConfiguratorImpl::ExtraRequestParams() const {
   return extra_info_;
 }
 
+std::string ConfiguratorImpl::GetDownloadPreference() const {
+  return std::string();
+}
+
 net::URLRequestContextGetter* ConfiguratorImpl::RequestContext() const {
   return url_request_getter_;
 }

components/component_updater/configurator_impl.h

@@ -67,6 +67,10 @@ class ConfiguratorImpl {
   // XML element.
   std::string ExtraRequestParams() const;
 
+  // Provides a hint for the server to control the order in which multiple
+  // download urls are returned.
+  std::string GetDownloadPreference() const;
+
   // The source of contexts for all the url requests.
   net::URLRequestContextGetter* RequestContext() const;
 

components/components_tests.gyp

@@ -787,6 +787,7 @@
       'update_client/update_client_unittest.cc',
       'update_client/update_query_params_unittest.cc',
       'update_client/update_response_unittest.cc',
+      'update_client/utils_unittest.cc',
     ],
     'upload_list_unittest_sources': [
       'upload_list/upload_list_unittest.cc',

components/update_client/BUILD.gn

@@ -96,6 +96,7 @@ source_set("unit_tests") {
     "update_client_unittest.cc",
     "update_query_params_unittest.cc",
     "update_response_unittest.cc",
+    "utils_unittest.cc",
   ]
 
   deps = [

components/update_client/configurator.h

@@ -77,6 +77,12 @@ class Configurator : public base::RefCountedThreadSafe<Configurator> {
   // XML element.
   virtual std::string ExtraRequestParams() const = 0;
 
+  // Provides a hint for the server to control the order in which multiple
+  // download urls are returned. The hint may or may not be honored in the
+  // response returned by the server.
+  // Returns an empty string if no policy is in effect.
+  virtual std::string GetDownloadPreference() const = 0;
+
   // The source of contexts for all the url requests.
   virtual net::URLRequestContextGetter* RequestContext() const = 0;
 

components/update_client/ping_manager.cc

@@ -164,7 +164,8 @@ std::string BuildPing(const Configurator& config, const CrxUpdateItem* item) {
 
   return BuildProtocolRequest(config.GetBrowserVersion().GetString(),
                               config.GetChannel(), config.GetLang(),
-                              config.GetOSLongName(), app_element, "");
+                              config.GetOSLongName(),
+                              config.GetDownloadPreference(), app_element, "");
 }
 
 // Sends a fire and forget ping. The instances of this class have no

components/update_client/test_configurator.cc

@@ -82,6 +82,10 @@ std::string TestConfigurator::ExtraRequestParams() const {
   return "extra=\"foo\"";
 }
 
+std::string TestConfigurator::GetDownloadPreference() const {
+  return download_preference_;
+}
+
 net::URLRequestContextGetter* TestConfigurator::RequestContext() const {
   return context_.get();
 }
@@ -107,6 +111,11 @@ void TestConfigurator::SetInitialDelay(int seconds) {
   initial_time_ = seconds;
 }
 
+void TestConfigurator::SetDownloadPreference(
+    const std::string& download_preference) {
+  download_preference_ = download_preference;
+}
+
 scoped_refptr<base::SequencedTaskRunner>
 TestConfigurator::GetSequencedTaskRunner() const {
   DCHECK(worker_task_runner_.get());

components/update_client/test_configurator.h

@@ -68,6 +68,7 @@ class TestConfigurator : public Configurator {
   std::string GetLang() const override;
   std::string GetOSLongName() const override;
   std::string ExtraRequestParams() const override;
+  std::string GetDownloadPreference() const override;
   net::URLRequestContextGetter* RequestContext() const override;
   scoped_refptr<OutOfProcessPatcher> CreateOutOfProcessPatcher() const override;
   bool DeltasEnabled() const override;
@@ -77,6 +78,7 @@ class TestConfigurator : public Configurator {
 
   void SetOnDemandTime(int seconds);
   void SetInitialDelay(int seconds);
+  void SetDownloadPreference(const std::string& download_preference);
 
  private:
   friend class base::RefCountedThreadSafe<TestConfigurator>;
@@ -87,6 +89,7 @@ class TestConfigurator : public Configurator {
 
   int initial_time_;
   int ondemand_time_;
+  std::string download_preference_;
 
   scoped_refptr<net::TestURLRequestContextGetter> context_;
 

components/update_client/update_checker.cc

@@ -69,10 +69,10 @@ std::string BuildUpdateCheckRequest(const Configurator& config,
     VLOG(1) << "Appending to update request: " << app;
   }
 
-  return BuildProtocolRequest(config.GetBrowserVersion().GetString(),
-                              config.GetChannel(), config.GetLang(),
-                              config.GetOSLongName(), app_elements,
-                              additional_attributes);
+  return BuildProtocolRequest(
+      config.GetBrowserVersion().GetString(), config.GetChannel(),
+      config.GetLang(), config.GetOSLongName(), config.GetDownloadPreference(),
+      app_elements, additional_attributes);
 }
 
 class UpdateCheckerImpl : public UpdateChecker {

components/update_client/update_checker_unittest.cc

@@ -187,6 +187,9 @@ TEST_F(UpdateCheckerTest, UpdateCheckSuccess) {
   // Sanity check the request.
   EXPECT_NE(string::npos, post_interceptor_->GetRequests()[0].find(
                               "request protocol=\"3.0\" extra=\"params\""));
+  // The request must not contain any "dlpref" in the default case.
+  EXPECT_EQ(string::npos,
+            post_interceptor_->GetRequests()[0].find(" dlpref=\""));
   EXPECT_NE(
       string::npos,
       post_interceptor_->GetRequests()[0].find(
@@ -236,4 +239,28 @@ TEST_F(UpdateCheckerTest, UpdateCheckError) {
   EXPECT_EQ(0ul, results_.list.size());
 }
 
+TEST_F(UpdateCheckerTest, UpdateCheckDownloadPreference) {
+  EXPECT_TRUE(post_interceptor_->ExpectRequest(
+      new PartialMatch("updatecheck"), test_file("updatecheck_reply_1.xml")));
+
+  config_->SetDownloadPreference(string("cacheable"));
+
+  update_checker_ = UpdateChecker::Create(config_);
+
+  CrxUpdateItem item(BuildCrxUpdateItem());
+  std::vector<CrxUpdateItem*> items_to_check;
+  items_to_check.push_back(&item);
+
+  update_checker_->CheckForUpdates(
+      items_to_check, "extra=\"params\"",
+      base::Bind(&UpdateCheckerTest::UpdateCheckComplete,
+                 base::Unretained(this)));
+
+  RunThreads();
+
+  // The request must contain dlpref="cacheable".
+  EXPECT_NE(string::npos,
+            post_interceptor_->GetRequests()[0].find(" dlpref=\"cacheable\""));
+}
+
 }  // namespace update_client

components/update_client/utils.cc

@@ -64,6 +64,7 @@ std::string BuildProtocolRequest(const std::string& browser_version,
                                  const std::string& channel,
                                  const std::string& lang,
                                  const std::string& os_long_name,
+                                 const std::string& download_preference,
                                  const std::string& request_body,
                                  const std::string& additional_attributes) {
   const std::string prod_id(
@@ -98,6 +99,9 @@ std::string BuildProtocolRequest(const std::string& browser_version,
   if (is_wow64)
     base::StringAppendF(&request, " wow64=\"1\"");
 #endif
+  if (!download_preference.empty())
+    base::StringAppendF(&request, " dlpref=\"%s\"",
+                        download_preference.c_str());
   base::StringAppendF(&request, ">");
 
   // HW platform information.

components/update_client/utils.h

@@ -44,6 +44,8 @@ struct CrxUpdateItem;
 
 // Builds a protocol request string by creating the outer envelope for
 // the request and including the request body specified as a parameter.
+// If present, the |download_preference| specifies a group policy that
+// affects the list of download URLs returned in the update response.
 // If specified, |additional_attributes| are appended as attributes of the
 // request element. The additional attributes have to be well-formed for
 // insertion in the request element.
@@ -51,6 +53,7 @@ std::string BuildProtocolRequest(const std::string& browser_version,
                                  const std::string& channel,
                                  const std::string& lang,
                                  const std::string& os_long_name,
+                                 const std::string& download_preference,
                                  const std::string& request_body,
                                  const std::string& additional_attributes);
 

components/update_client/utils_unittest.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/update_client/utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+using std::string;
+
+namespace update_client {
+
+TEST(BuildProtocolRequest, DownloadPreference) {
+  const string emptystr;
+
+  // Verifies that an empty |download_preference| is not serialized.
+  const string request_no_dlpref = BuildProtocolRequest(
+      emptystr, emptystr, emptystr, emptystr, emptystr, emptystr, emptystr);
+  EXPECT_EQ(string::npos, request_no_dlpref.find(" dlpref="));
+
+  // Verifies that |download_preference| is serialized.
+  const string request_with_dlpref = BuildProtocolRequest(
+      emptystr, emptystr, emptystr, emptystr, "some pref", emptystr, emptystr);
+  EXPECT_NE(string::npos, request_with_dlpref.find(" dlpref=\"some pref\""));
+}
+
+}  // namespace update_client

ios/chrome/browser/component_updater/ios_component_updater_configurator.cc

@@ -36,6 +36,7 @@ class IOSConfigurator : public update_client::Configurator {
   std::string GetLang() const override;
   std::string GetOSLongName() const override;
   std::string ExtraRequestParams() const override;
+  std::string GetDownloadPreference() const override;
   net::URLRequestContextGetter* RequestContext() const override;
   scoped_refptr<update_client::OutOfProcessPatcher> CreateOutOfProcessPatcher()
       const override;
@@ -105,6 +106,10 @@ std::string IOSConfigurator::ExtraRequestParams() const {
   return configurator_impl_.ExtraRequestParams();
 }
 
+std::string IOSConfigurator::GetDownloadPreference() const {
+  return configurator_impl_.GetDownloadPreference();
+}
+
 net::URLRequestContextGetter* IOSConfigurator::RequestContext() const {
   return configurator_impl_.RequestContext();
 }