From 668c4dc439608868b530afe0f9a7592ee32854d6 Mon Sep 17 00:00:00 2001 From: "grt@chromium.org" Date: Thu, 16 Jun 2011 19:51:09 +0000 Subject: Return a new error code multi-install updates if Group Policy settings blocking updates for individual products are conflicting with the effective update policy for the multi-install binaries. In so doing, I moved the GP update policy checking code that was in browser into installer_util. BUG=none TEST=Install multi-install Chrome version N-1, disable updates for Chrome, then try to update to version N. With luck, you'll find that the update failed on account of inconsistent GP settings. R=robertshield@chromium.org,pastarmovj@chromium.org,gwilson@chromium.org Review URL: http://codereview.chromium.org/7111012 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@89384 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/installer/setup/setup_main.cc | 88 ++++++++++++- chrome/installer/util/google_update_settings.cc | 70 ++++++++++ chrome/installer/util/google_update_settings.h | 13 ++ .../util/google_update_settings_unittest.cc | 141 +++++++++++++++++++++ chrome/installer/util/prebuild/create_string_rc.py | 1 + chrome/installer/util/util_constants.h | 3 +- 6 files changed, 312 insertions(+), 4 deletions(-) (limited to 'chrome/installer') diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc index 7e4c4ad..8c61727 100644 --- a/chrome/installer/setup/setup_main.cc +++ b/chrome/installer/setup/setup_main.cc @@ -252,6 +252,83 @@ installer::InstallStatus RenameChromeExecutables( return ret; } +// For each product that is being updated (i.e., already installed at an earlier +// version), see if that product has an update policy override that differs from +// that for the binaries. If any are found, fail with an error indicating that +// the Group Policy settings are in an inconsistent state. Do not do this test +// for same-version installs, since it would be unkind to block attempts to +// repair a corrupt installation. This function returns false when installation +// should be halted, in which case |status| contains the relevant exit code and +// the proper installer result has been written to the registry. +bool CheckGroupPolicySettings(const InstallationState& original_state, + const InstallerState& installer_state, + const Version& new_version, + installer::InstallStatus* status) { +#if !defined(GOOGLE_CHROME_BUILD) + // Chromium builds are not updated via Google Update, so there are no + // Group Policy settings to consult. + return true; +#else + DCHECK(status); + + // Single installs are always in good shape. + if (!installer_state.is_multi_install()) + return true; + + bool settings_are_valid = true; + const bool is_system_install = installer_state.system_install(); + BrowserDistribution* const binaries_dist = + installer_state.multi_package_binaries_distribution(); + + // Get the update policy for the binaries. + const GoogleUpdateSettings::UpdatePolicy binaries_policy = + GoogleUpdateSettings::GetAppUpdatePolicy(binaries_dist->GetAppGuid(), + NULL); + + // Check for differing update policies for all of the products being updated. + const Products& products = installer_state.products(); + Products::const_iterator scan = products.begin(); + for (Products::const_iterator end = products.end(); scan != end; ++scan) { + BrowserDistribution* dist = (*scan)->distribution(); + const ProductState* product_state = + original_state.GetProductState(is_system_install, dist->GetType()); + // Is an earlier version of this product already installed? + if (product_state != NULL && + product_state->version().CompareTo(new_version) < 0) { + bool is_overridden = false; + GoogleUpdateSettings::UpdatePolicy app_policy = + GoogleUpdateSettings::GetAppUpdatePolicy(dist->GetAppGuid(), + &is_overridden); + if (is_overridden && app_policy != binaries_policy) { + LOG(ERROR) << "Found legacy Group Policy setting for " + << dist->GetAppShortCutName() << " (value: " << app_policy + << ") that does not match the setting for " + << binaries_dist->GetAppShortCutName() + << " (value: " << binaries_policy << ")."; + settings_are_valid = false; + } + } + } + + if (!settings_are_valid) { + // TODO(grt): add " See http://goo.gl/+++ for details." to the end of this + // log message and to the IDS_INSTALL_INCONSISTENT_UPDATE_POLICY string once + // we have a help center article that explains why this error is being + // reported and how to resolve it. + LOG(ERROR) << "Cannot apply update on account of inconsistent " + "Google Update Group Policy settings. Use the Group Policy " + "Editor to set the update policy override for the " + << binaries_dist->GetAppShortCutName() + << " application and try again."; + *status = installer::INCONSISTENT_UPDATE_POLICY; + installer_state.WriteInstallerResult( + *status, IDS_INSTALL_INCONSISTENT_UPDATE_POLICY_BASE, NULL); + } + + return settings_are_valid; +#endif // defined(GOOGLE_CHROME_BUILD) +} + // The supported multi-install modes are: // --multi-install --chrome --chrome-frame --ready-mode // - If a non-multi Chrome Frame installation is present, Chrome Frame is @@ -526,7 +603,7 @@ installer::InstallStatus InstallProductsHelper( // currently installed product for the shared binary installation, should // (or rather must) be upgraded. VLOG(1) << "version to install: " << installer_version->GetString(); - bool higher_version_installed = false; + bool proceed_with_installation = true; for (size_t i = 0; i < installer_state.products().size(); ++i) { const Product* product = installer_state.products()[i]; const ProductState* product_state = @@ -535,7 +612,7 @@ installer::InstallStatus InstallProductsHelper( if (product_state != NULL && (product_state->version().CompareTo(*installer_version) > 0)) { LOG(ERROR) << "Higher version is already installed."; - higher_version_installed = true; + proceed_with_installation = false; install_status = installer::HIGHER_VERSION_EXISTS; if (product->is_chrome()) { @@ -550,7 +627,12 @@ installer::InstallStatus InstallProductsHelper( } } - if (!higher_version_installed) { + proceed_with_installation = + proceed_with_installation && + CheckGroupPolicySettings(original_state, installer_state, + *installer_version, &install_status); + + if (proceed_with_installation) { // We want to keep uncompressed archive (chrome.7z) that we get after // uncompressing and binary patching. Get the location for this file. FilePath archive_to_copy( diff --git a/chrome/installer/util/google_update_settings.cc b/chrome/installer/util/google_update_settings.cc index e0947d1..a1cd48e 100644 --- a/chrome/installer/util/google_update_settings.cc +++ b/chrome/installer/util/google_update_settings.cc @@ -26,6 +26,17 @@ using installer::InstallerState; namespace { +const wchar_t kGoogleUpdatePoliciesKey[] = + L"SOFTWARE\\Policies\\Google\\Update"; +const wchar_t kGoogleUpdateUpdatePolicyValue[] = L"UpdateDefault"; +const wchar_t kGoogleUpdateUpdateOverrideValuePrefix[] = L"Update"; +const GoogleUpdateSettings::UpdatePolicy kGoogleUpdateDefaultUpdatePolicy = +#if defined(GOOGLE_CHROME_BUILD) + GoogleUpdateSettings::AUTOMATIC_UPDATES; +#else + GoogleUpdateSettings::UPDATES_DISABLED; +#endif + // An list of search results in increasing order of desirability. enum EulaSearchResult { NO_SETTING, @@ -143,6 +154,24 @@ bool GetChromeChannelInternal(bool system_install, return true; } +// Populates |update_policy| with the UpdatePolicy enum value corresponding to a +// DWORD read from the registry and returns true if |value| is within range. +// If |value| is out of range, returns false without modifying |update_policy|. +bool GetUpdatePolicyFromDword( + const DWORD value, + GoogleUpdateSettings::UpdatePolicy* update_policy) { + switch (value) { + case GoogleUpdateSettings::UPDATES_DISABLED: + case GoogleUpdateSettings::AUTOMATIC_UPDATES: + case GoogleUpdateSettings::MANUAL_UPDATES_ONLY: + *update_policy = static_cast(value); + return true; + default: + LOG(WARNING) << "Unexpected update policy override value: " << value; + } + return false; +} + } // namespace // Older versions of Chrome unconditionally read from HKCU\...\ClientState\... @@ -486,3 +515,44 @@ bool GoogleUpdateSettings::IsOrganicFirstRun(const std::wstring& brand) { return (StartsWith(brand, L"GG", true) || StartsWith(brand, L"EU", true)); } + +GoogleUpdateSettings::UpdatePolicy GoogleUpdateSettings::GetAppUpdatePolicy( + const std::wstring& app_guid, + bool* is_overridden) { + bool found_override = false; + UpdatePolicy update_policy = kGoogleUpdateDefaultUpdatePolicy; + +#if defined(GOOGLE_CHROME_BUILD) + DCHECK(!app_guid.empty()); + RegKey policy_key; + + // Google Update Group Policy settings are always in HKLM. + if (policy_key.Open(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_QUERY_VALUE) == ERROR_SUCCESS) { + static const size_t kPrefixLen = + arraysize(kGoogleUpdateUpdateOverrideValuePrefix) - 1; + DWORD value; + std::wstring app_update_override; + app_update_override.reserve(kPrefixLen + app_guid.size()); + app_update_override.append(kGoogleUpdateUpdateOverrideValuePrefix, + kPrefixLen); + app_update_override.append(app_guid); + // First try to read and comprehend the app-specific override. + found_override = (policy_key.ReadValueDW(app_update_override.c_str(), + &value) == ERROR_SUCCESS && + GetUpdatePolicyFromDword(value, &update_policy)); + + // Failing that, try to read and comprehend the default override. + if (!found_override && + policy_key.ReadValueDW(kGoogleUpdateUpdatePolicyValue, + &value) == ERROR_SUCCESS) { + GetUpdatePolicyFromDword(value, &update_policy); + } + } +#endif // defined(GOOGLE_CHROME_BUILD) + + if (is_overridden != NULL) + *is_overridden = found_override; + + return update_policy; +} diff --git a/chrome/installer/util/google_update_settings.h b/chrome/installer/util/google_update_settings.h index f677a2b..cc331bd 100644 --- a/chrome/installer/util/google_update_settings.h +++ b/chrome/installer/util/google_update_settings.h @@ -21,6 +21,13 @@ class InstallerState; // google_update.exe responsability to write the initial values. class GoogleUpdateSettings { public: + // Update policy constants defined by Google Update; do not change these. + enum UpdatePolicy { + UPDATES_DISABLED = 0, + AUTOMATIC_UPDATES = 1, + MANUAL_UPDATES_ONLY = 2, + }; + // Returns whether the user has given consent to collect UMA data and send // crash dumps to Google. This information is collected by the web server // used to download the chrome installer. @@ -160,6 +167,12 @@ class GoogleUpdateSettings { // method. static bool IsOrganicFirstRun(const std::wstring& brand); + // Returns the effective update policy for |app_guid| as dictated by + // Group Policy settings. |is_overridden|, if non-NULL, is populated with + // true if an app-specific policy override is in force, or false otherwise. + static UpdatePolicy GetAppUpdatePolicy(const std::wstring& app_guid, + bool* is_overridden); + private: DISALLOW_IMPLICIT_CONSTRUCTORS(GoogleUpdateSettings); }; diff --git a/chrome/installer/util/google_update_settings_unittest.cc b/chrome/installer/util/google_update_settings_unittest.cc index c70e3a4..e27309d 100644 --- a/chrome/installer/util/google_update_settings_unittest.cc +++ b/chrome/installer/util/google_update_settings_unittest.cc @@ -31,6 +31,16 @@ const wchar_t kHKCUReplacement[] = L"Software\\Google\\InstallUtilUnittest\\HKCU"; const wchar_t kHKLMReplacement[] = L"Software\\Google\\InstallUtilUnittest\\HKLM"; +const wchar_t kGoogleUpdatePoliciesKey[] = + L"SOFTWARE\\Policies\\Google\\Update"; +const wchar_t kGoogleUpdateUpdateDefault[] = L"UpdateDefault"; +const wchar_t kGoogleUpdateUpdatePrefix[] = L"Update"; +const GoogleUpdateSettings::UpdatePolicy kDefaultUpdatePolicy = +#if defined(GOOGLE_CHROME_BUILD) + GoogleUpdateSettings::AUTOMATIC_UPDATES; +#else + GoogleUpdateSettings::UPDATES_DISABLED; +#endif const wchar_t kTestProductGuid[] = L"{89F1B351-B15D-48D4-8F10-1298721CF13D}"; @@ -465,3 +475,134 @@ TEST_F(GoogleUpdateSettingsTest, SetEULAConsent) { key.ReadValueDW(google_update::kRegEULAAceptedField, &value)); EXPECT_EQ(1U, value); } + +// Test that the appropriate default is returned if no update override is +// present. +TEST_F(GoogleUpdateSettingsTest, GetAppUpdatePolicyNoOverride) { + // There are no policies at all. + EXPECT_EQ(ERROR_FILE_NOT_FOUND, + RegKey().Open(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_QUERY_VALUE)); + bool is_overridden = true; + EXPECT_EQ(kDefaultUpdatePolicy, + GoogleUpdateSettings::GetAppUpdatePolicy(kTestProductGuid, + &is_overridden)); + EXPECT_FALSE(is_overridden); + + // The policy key exists, but there are no values of interest present. + EXPECT_EQ(ERROR_SUCCESS, + RegKey().Create(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_SET_VALUE)); + EXPECT_EQ(ERROR_SUCCESS, + RegKey().Open(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_QUERY_VALUE)); + is_overridden = true; + EXPECT_EQ(kDefaultUpdatePolicy, + GoogleUpdateSettings::GetAppUpdatePolicy(kTestProductGuid, + &is_overridden)); + EXPECT_FALSE(is_overridden); +} + +#if defined(GOOGLE_CHROME_BUILD) + +// Test that the default override is returned if no app-specific override is +// present. +TEST_F(GoogleUpdateSettingsTest, GetAppUpdatePolicyDefaultOverride) { + EXPECT_EQ(ERROR_SUCCESS, + RegKey(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_SET_VALUE).WriteValue(kGoogleUpdateUpdateDefault, + static_cast(0))); + bool is_overridden = true; + EXPECT_EQ(GoogleUpdateSettings::UPDATES_DISABLED, + GoogleUpdateSettings::GetAppUpdatePolicy(kTestProductGuid, + &is_overridden)); + EXPECT_FALSE(is_overridden); + + EXPECT_EQ(ERROR_SUCCESS, + RegKey(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_SET_VALUE).WriteValue(kGoogleUpdateUpdateDefault, + static_cast(1))); + is_overridden = true; + EXPECT_EQ(GoogleUpdateSettings::AUTOMATIC_UPDATES, + GoogleUpdateSettings::GetAppUpdatePolicy(kTestProductGuid, + &is_overridden)); + EXPECT_FALSE(is_overridden); + + EXPECT_EQ(ERROR_SUCCESS, + RegKey(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_SET_VALUE).WriteValue(kGoogleUpdateUpdateDefault, + static_cast(2))); + is_overridden = true; + EXPECT_EQ(GoogleUpdateSettings::MANUAL_UPDATES_ONLY, + GoogleUpdateSettings::GetAppUpdatePolicy(kTestProductGuid, + &is_overridden)); + EXPECT_FALSE(is_overridden); + + // The default policy should be in force for bogus values. + EXPECT_EQ(ERROR_SUCCESS, + RegKey(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_SET_VALUE).WriteValue(kGoogleUpdateUpdateDefault, + static_cast(3))); + is_overridden = true; + EXPECT_EQ(kDefaultUpdatePolicy, + GoogleUpdateSettings::GetAppUpdatePolicy(kTestProductGuid, + &is_overridden)); + EXPECT_FALSE(is_overridden); +} + +// Test that an app-specific override is used if present. +TEST_F(GoogleUpdateSettingsTest, GetAppUpdatePolicyAppOverride) { + std::wstring app_policy_value(kGoogleUpdateUpdatePrefix); + app_policy_value.append(kTestProductGuid); + + EXPECT_EQ(ERROR_SUCCESS, + RegKey(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_SET_VALUE).WriteValue(kGoogleUpdateUpdateDefault, + static_cast(1))); + EXPECT_EQ(ERROR_SUCCESS, + RegKey(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_SET_VALUE).WriteValue(app_policy_value.c_str(), + static_cast(0))); + bool is_overridden = false; + EXPECT_EQ(GoogleUpdateSettings::UPDATES_DISABLED, + GoogleUpdateSettings::GetAppUpdatePolicy(kTestProductGuid, + &is_overridden)); + EXPECT_TRUE(is_overridden); + + EXPECT_EQ(ERROR_SUCCESS, + RegKey(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_SET_VALUE).WriteValue(kGoogleUpdateUpdateDefault, + static_cast(0))); + EXPECT_EQ(ERROR_SUCCESS, + RegKey(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_SET_VALUE).WriteValue(app_policy_value.c_str(), + static_cast(1))); + is_overridden = false; + EXPECT_EQ(GoogleUpdateSettings::AUTOMATIC_UPDATES, + GoogleUpdateSettings::GetAppUpdatePolicy(kTestProductGuid, + &is_overridden)); + EXPECT_TRUE(is_overridden); + + EXPECT_EQ(ERROR_SUCCESS, + RegKey(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_SET_VALUE).WriteValue(app_policy_value.c_str(), + static_cast(2))); + is_overridden = false; + EXPECT_EQ(GoogleUpdateSettings::MANUAL_UPDATES_ONLY, + GoogleUpdateSettings::GetAppUpdatePolicy(kTestProductGuid, + &is_overridden)); + EXPECT_TRUE(is_overridden); + + // The default policy should be in force for bogus values. + EXPECT_EQ(ERROR_SUCCESS, + RegKey(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey, + KEY_SET_VALUE).WriteValue(app_policy_value.c_str(), + static_cast(3))); + is_overridden = true; + EXPECT_EQ(GoogleUpdateSettings::UPDATES_DISABLED, + GoogleUpdateSettings::GetAppUpdatePolicy(kTestProductGuid, + &is_overridden)); + EXPECT_FALSE(is_overridden); +} + +#endif // defined(GOOGLE_CHROME_BUILD) diff --git a/chrome/installer/util/prebuild/create_string_rc.py b/chrome/installer/util/prebuild/create_string_rc.py index 3a0e14f..f7bb453 100755 --- a/chrome/installer/util/prebuild/create_string_rc.py +++ b/chrome/installer/util/prebuild/create_string_rc.py @@ -69,6 +69,7 @@ kStringIds = [ 'IDS_INSTALL_MULTI_INSTALLATION_EXISTS', 'IDS_INSTALL_CONFLICTING_CHANNEL_EXISTS', 'IDS_INSTALL_READY_MODE_REQUIRES_CHROME', + 'IDS_INSTALL_INCONSISTENT_UPDATE_POLICY', 'IDS_OEM_MAIN_SHORTCUT_NAME', 'IDS_SHORTCUT_TOOLTIP', ] diff --git a/chrome/installer/util/util_constants.h b/chrome/installer/util/util_constants.h index 3aced6c..d2c20f4 100644 --- a/chrome/installer/util/util_constants.h +++ b/chrome/installer/util/util_constants.h @@ -74,13 +74,14 @@ enum InstallStatus { REQUIRES_MULTI_INSTALL, // 41. --multi-install was missing from the // command line. APPLY_DIFF_PATCH_FAILED, // 42. Failed to apply a diff patch. + INCONSISTENT_UPDATE_POLICY, // 43. Inconsistent update policy GP settings. }; // If the following compile assert fires it means that the InstallStatus // enumeration changed which will break the contract between the old // chrome installed and the new setup.exe that is trying to upgrade. -COMPILE_ASSERT(installer::CONFLICTING_CHANNEL_EXISTS == 39, +COMPILE_ASSERT(installer::INCONSISTENT_UPDATE_POLICY == 43, dont_change_enum); // The type of an update archive. -- cgit v1.1