Updater: ExistenceCheckerPath - check for ownership of installed app.

Bug: 1147094
Change-Id: I46e84ea854d6719a56885227bf19d4d39d337449
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2495332
Commit-Queue: Mila Green <milagreen@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827621}

chrome/updater/constants.h

@@ -204,6 +204,9 @@ constexpr int kPolicyAutomaticUpdatesOnly = 3;
 constexpr bool kInstallPolicyDefault = kPolicyEnabled;
 constexpr bool kUpdatePolicyDefault = kPolicyEnabled;
 
+constexpr int kUninstallPingReasonUninstalled = 0;
+constexpr int kUninstallPingReasonUserNotAnOwner = 1;
+
 }  // namespace updater
 
 #endif  // CHROME_UPDATER_CONSTANTS_H_

chrome/updater/control_service_impl.cc

@@ -13,6 +13,8 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
+#include "base/task/post_task.h"
+#include "base/task/thread_pool.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "base/time/time.h"
 #include "chrome/updater/configurator.h"
@@ -20,7 +22,9 @@
 #include "chrome/updater/persisted_data.h"
 #include "chrome/updater/prefs.h"
 #include "chrome/updater/update_service_impl.h"
+#include "chrome/updater/util.h"
 #include "components/prefs/pref_service.h"
+#include "components/update_client/update_client.h"
 
 namespace updater {
 
@@ -28,24 +32,49 @@ ControlServiceImpl::ControlServiceImpl(
     scoped_refptr<updater::Configurator> config)
     : config_(config),
       persisted_data_(
-          base::MakeRefCounted<PersistedData>(config_->GetPrefService())) {}
+          base::MakeRefCounted<PersistedData>(config_->GetPrefService())),
+      update_client_(update_client::UpdateClientFactory(config_)),
+      number_of_pings_remaining_(0) {}
 
 void ControlServiceImpl::Run(base::OnceClosure callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
-  UnregisterMissingApps();
-  MaybeCheckForUpdates(std::move(callback));
+  callback_ = std::move(callback);
+  UnregisterMissingApps(GetRegisteredApps());
 }
 
 void ControlServiceImpl::InitializeUpdateService(base::OnceClosure callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  VLOG(1) << __func__;
   std::move(callback).Run();
 }
 
-void ControlServiceImpl::MaybeCheckForUpdates(base::OnceClosure callback) {
+std::vector<ControlServiceImpl::AppInfo>
+ControlServiceImpl::GetRegisteredApps() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  std::vector<AppInfo> apps_to_unregister;
+  for (const std::string& app_id : persisted_data_->GetAppIds()) {
+    if (app_id == kUpdaterAppId)
+      continue;
+
+    const base::FilePath ecp = persisted_data_->GetExistenceCheckerPath(app_id);
+    if (!ecp.empty()) {
+      apps_to_unregister.push_back(
+          AppInfo(app_id, persisted_data_->GetProductVersion(app_id), ecp));
+    }
+  }
+  return apps_to_unregister;
+}
+
+bool ControlServiceImpl::WaitingOnUninstallPings() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return number_of_pings_remaining_ > 0;
+}
+
+void ControlServiceImpl::MaybeCheckForUpdates() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  scoped_refptr<UpdateServiceImpl> update_service =
+      base::MakeRefCounted<UpdateServiceImpl>(config_);
+
   const base::Time lastUpdateTime =
       config_->GetPrefService()->GetTime(kPrefUpdateTime);
 
@@ -57,13 +86,10 @@ void ControlServiceImpl::MaybeCheckForUpdates(base::OnceClosure callback) {
     VLOG(0) << "Skipping checking for updates:  "
             << timeSinceUpdate.InMinutes();
     base::SequencedTaskRunnerHandle::Get()->PostTask(FROM_HERE,
-                                                     std::move(callback));
+                                                     std::move(callback_));
     return;
   }
 
-  scoped_refptr<UpdateServiceImpl> update_service =
-      base::MakeRefCounted<UpdateServiceImpl>(config_);
-
   update_service->UpdateAll(
       base::DoNothing(),
       base::BindOnce(
@@ -78,24 +104,83 @@ void ControlServiceImpl::MaybeCheckForUpdates(base::OnceClosure callback) {
             }
             std::move(closure).Run();
           },
-          base::BindOnce(std::move(callback)), config_));
+          base::BindOnce(std::move(callback_)), config_));
 }
 
-void ControlServiceImpl::UnregisterMissingApps() {
+void ControlServiceImpl::UnregisterMissingApps(std::vector<AppInfo> apps) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  for (const auto& app_id : persisted_data_->GetAppIds()) {
+
+  base::ThreadPool::PostTaskAndReplyWithResult(
+      FROM_HERE, {base::MayBlock()},
+      base::BindOnce(&ControlServiceImpl::GetAppIDsToRemove, this, apps),
+      base::BindOnce(&ControlServiceImpl::RemoveAppIDsAndSendUninstallPings,
+                     this));
+}
+
+void ControlServiceImpl::UnregisterMissingAppsDone() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  MaybeCheckForUpdates();
+}
+
+std::vector<ControlServiceImpl::PingInfo> ControlServiceImpl::GetAppIDsToRemove(
+    std::vector<AppInfo> apps) {
+  std::vector<PingInfo> app_ids_to_remove;
+  for (const auto& app : apps) {
     // Skip if app_id is equal to updater app id.
-    if (app_id == kUpdaterAppId)
+    if (app.app_id_ == kUpdaterAppId)
       continue;
 
-    const base::FilePath ecp = persisted_data_->GetExistenceCheckerPath(app_id);
-    if (!ecp.empty() && !base::PathExists(ecp)) {
-      if (!persisted_data_->RemoveApp(app_id))
-        VLOG(0) << "Could not remove registration of app " << app_id;
+    if (!base::PathExists(app.ecp_)) {
+      app_ids_to_remove.push_back(PingInfo(app.app_id_, app.app_version_,
+                                           kUninstallPingReasonUninstalled));
+    } else if (!PathOwnedByUser(app.ecp_)) {
+      app_ids_to_remove.push_back(PingInfo(app.app_id_, app.app_version_,
+                                           kUninstallPingReasonUserNotAnOwner));
+    }
+  }
+
+  return app_ids_to_remove;
+}
+
+void ControlServiceImpl::RemoveAppIDsAndSendUninstallPings(
+    std::vector<PingInfo> app_ids_to_remove) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  if (app_ids_to_remove.empty()) {
+    UnregisterMissingAppsDone();
+    return;
+  }
+
+  for (const auto& app_id_to_remove : app_ids_to_remove) {
+    const auto app_id = app_id_to_remove.app_id_;
+    const int ping_reason = app_id_to_remove.ping_reason_;
+    const base::Version app_version = app_id_to_remove.app_version_;
+
+    if (persisted_data_->RemoveApp(app_id)) {
+      VLOG(1) << "Uninstall ping for app id: " << app_id
+              << ". Ping reason: " << ping_reason;
+      number_of_pings_remaining_++;
+      update_client_->SendUninstallPing(
+          app_id, app_version, ping_reason,
+          base::BindOnce(&ControlServiceImpl::UninstallPingSent, this));
+    } else {
+      VLOG(0) << "Could not remove registration of app " << app_id;
     }
   }
 }
 
+void ControlServiceImpl::UninstallPingSent(update_client::Error error) {
+  number_of_pings_remaining_--;
+
+  if (error != update_client::Error::NONE)
+    VLOG(0) << __func__ << ": Error: " << static_cast<int>(error);
+
+  if (!WaitingOnUninstallPings())
+    std::move(
+        base::BindOnce(&ControlServiceImpl::UnregisterMissingAppsDone, this))
+        .Run();
+}
+
 void ControlServiceImpl::Uninitialize() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   PrefsCommitPendingWrites(config_->GetPrefService());

chrome/updater/control_service_impl.h

@@ -5,11 +5,22 @@
 #ifndef CHROME_UPDATER_CONTROL_SERVICE_IMPL_H_
 #define CHROME_UPDATER_CONTROL_SERVICE_IMPL_H_
 
+#include <string>
+#include <vector>
+
+#include "base/callback.h"
 #include "base/callback_forward.h"
+#include "base/files/file_path.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/sequence_checker.h"
+#include "base/version.h"
 #include "chrome/updater/control_service.h"
 
+namespace update_client {
+enum class Error;
+class UpdateClient;
+}  // namespace update_client
+
 namespace updater {
 
 class Configurator;
@@ -31,16 +42,63 @@ class ControlServiceImpl : public ControlService {
 
   SEQUENCE_CHECKER(sequence_checker_);
 
+  struct AppInfo {
+    AppInfo(const std::string& app_id,
+            const base::Version& app_version,
+            const base::FilePath& ecp)
+        : app_id_(app_id), app_version_(app_version), ecp_(ecp) {}
+    std::string app_id_;
+    base::Version app_version_;
+    base::FilePath ecp_;
+  };
+
+  struct PingInfo {
+    PingInfo(const std::string& app_id,
+             const base::Version& app_version,
+             int ping_reason)
+        : app_id_(app_id),
+          app_version_(app_version),
+          ping_reason_(ping_reason) {}
+    std::string app_id_;
+    base::Version app_version_;
+    int ping_reason_;
+  };
+
   // Checks for updates of all registered applications if it has been longer
   // than the last check time by NextCheckDelay() amount defined in the config.
-  void MaybeCheckForUpdates(base::OnceClosure callback);
+  void MaybeCheckForUpdates();
+
+  // Returns a list of apps registered with the updater.
+  std::vector<AppInfo> GetRegisteredApps();
+
+  void UnregisterMissingAppsDone();
 
   // Provides a way to remove apps from the persisted data if the app is no
   // longer installed on the machine.
-  void UnregisterMissingApps();
+  void UnregisterMissingApps(std::vector<AppInfo> apps);
+
+  // After an uninstall ping has been processed, reduces the number of pings
+  // that we need to wait on before checking for updates.
+  void UninstallPingSent(update_client::Error error);
+
+  // Returns true if there are uninstall ping tasks which haven't finished.
+  // Returns false if |number_of_pings_remaining_| is 0.
+  bool WaitingOnUninstallPings() const;
+
+  // Returns a list of apps that need to be unregistered.
+  std::vector<ControlServiceImpl::PingInfo> GetAppIDsToRemove(
+      std::vector<AppInfo> apps);
+
+  // Unregisters the apps in |app_ids_to_remove| and starts an update check
+  // if necessary.
+  void RemoveAppIDsAndSendUninstallPings(
+      std::vector<PingInfo> app_ids_to_remove);
 
   scoped_refptr<updater::Configurator> config_;
   scoped_refptr<updater::PersistedData> persisted_data_;
+  scoped_refptr<update_client::UpdateClient> update_client_;
+  base::OnceClosure callback_;
+  int number_of_pings_remaining_;
 };
 
 }  // namespace updater

chrome/updater/mac/BUILD.gn

@@ -87,6 +87,7 @@ source_set("util") {
 
   deps = [
     "//base",
+    "//chrome/updater:base",
     "//chrome/updater:version_header",
   ]
 

chrome/updater/mac/util.mm

@@ -4,13 +4,16 @@
 
 #import "chrome/updater/mac/util.h"
 
+#include <pwd.h>
 #include <unistd.h>
 
+#include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/mac/foundation_util.h"
 #include "base/version.h"
 #include "chrome/updater/updater_version.h"
+#include "chrome/updater/util.h"
 
 namespace updater {
 namespace {
@@ -68,4 +71,41 @@ base::FilePath GetUpdaterExecutablePath() {
       .Append(FILE_PATH_LITERAL(PRODUCT_FULLNAME_STRING));
 }
 
+bool PathOwnedByUser(const base::FilePath& path) {
+  struct passwd* result = nullptr;
+  struct passwd user_info = {};
+  char pwbuf[2048] = {};
+  uid_t user_uid = geteuid();
+
+  int error = getpwuid_r(user_uid, &user_info, pwbuf, sizeof(pwbuf), &result);
+
+  if (error) {
+    VLOG(1) << "Failed to get user info.";
+    return true;
+  }
+
+  if (result == nullptr) {
+    VLOG(1) << "No entry for user.";
+    return true;
+  }
+
+  base::stat_wrapper_t stat_info = {};
+  if (base::File::Lstat(path.value().c_str(), &stat_info) != 0) {
+    DPLOG(ERROR) << "Failed to get information on path " << path.value();
+    return false;
+  }
+
+  if (S_ISLNK(stat_info.st_mode)) {
+    DLOG(ERROR) << "Path " << path.value() << " is a symbolic link.";
+    return false;
+  }
+
+  if (stat_info.st_uid != user_uid) {
+    DLOG(ERROR) << "Path " << path.value() << " is owned by the wrong user.";
+    return false;
+  }
+
+  return true;
+}
+
 }  // namespace updater

chrome/updater/persisted_data.cc

@@ -67,7 +67,14 @@ void PersistedData::SetFingerprint(const std::string& id,
 base::FilePath PersistedData::GetExistenceCheckerPath(
     const std::string& id) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return base::FilePath().AppendASCII(GetString(id, kECP));
+#if defined(OS_WIN)
+  base::FilePath::StringType ecp;
+  const std::string str = GetString(id, kECP);
+  return base::UTF8ToWide(str.c_str(), str.size(), &ecp) ? base::FilePath(ecp)
+                                                         : base::FilePath();
+#else
+  return base::FilePath(GetString(id, kECP));
+#endif  // OS_WIN
 }
 
 void PersistedData::SetExistenceCheckerPath(const std::string& id,

chrome/updater/test/integration_tests.cc

@@ -18,6 +18,7 @@
 #include "chrome/updater/constants.h"
 #include "chrome/updater/persisted_data.h"
 #include "chrome/updater/prefs.h"
+#include "chrome/updater/registration_data.h"
 #include "chrome/updater/test/test_app/constants.h"
 #include "chrome/updater/test/test_app/test_app_version.h"
 #include "chrome/updater/updater_version.h"
@@ -181,6 +182,75 @@ TEST_F(IntegrationTest, RegisterTestApp) {
   ExpectActive();
   Uninstall();
 }
+
+TEST_F(IntegrationTest, UnregisterUninstalledApp) {
+  RegisterTestApp();
+  ExpectInstalled();
+  ExpectActiveVersion(UPDATER_VERSION_STRING);
+  ExpectActive();
+
+  {
+    std::unique_ptr<GlobalPrefs> global_prefs = CreateGlobalPrefs();
+    auto persisted_data =
+        base::MakeRefCounted<PersistedData>(global_prefs->GetPrefService());
+    base::FilePath fake_ecp =
+        persisted_data->GetExistenceCheckerPath(kTestAppId)
+            .Append(FILE_PATH_LITERAL("NOT_THERE"));
+    persisted_data->SetExistenceCheckerPath(kTestAppId, fake_ecp);
+
+    PrefsCommitPendingWrites(global_prefs->GetPrefService());
+
+    EXPECT_EQ(fake_ecp.value(),
+              persisted_data->GetExistenceCheckerPath(kTestAppId).value());
+  }
+
+  RunWake(0);
+
+  {
+    std::unique_ptr<GlobalPrefs> global_prefs = CreateGlobalPrefs();
+    auto persisted_data =
+        base::MakeRefCounted<PersistedData>(global_prefs->GetPrefService());
+    EXPECT_EQ(base::FilePath(FILE_PATH_LITERAL("")).value(),
+              persisted_data->GetExistenceCheckerPath(kTestAppId).value());
+  }
+
+  Uninstall();
+  Clean();
+}
+
+TEST_F(IntegrationTest, UnregisterUnownedApp) {
+  RegisterTestApp();
+  ExpectInstalled();
+  ExpectActiveVersion(UPDATER_VERSION_STRING);
+  ExpectActive();
+
+  {
+    std::unique_ptr<GlobalPrefs> global_prefs = CreateGlobalPrefs();
+    auto persisted_data =
+        base::MakeRefCounted<PersistedData>(global_prefs->GetPrefService());
+    base::FilePath fake_ecp{FILE_PATH_LITERAL("/Library")};
+    persisted_data->SetExistenceCheckerPath(kTestAppId, fake_ecp);
+
+    PrefsCommitPendingWrites(global_prefs->GetPrefService());
+
+    EXPECT_EQ(fake_ecp.value(),
+              persisted_data->GetExistenceCheckerPath(kTestAppId).value());
+  }
+
+  RunWake(0);
+
+  {
+    std::unique_ptr<GlobalPrefs> global_prefs = CreateGlobalPrefs();
+    auto persisted_data =
+        base::MakeRefCounted<PersistedData>(global_prefs->GetPrefService());
+    EXPECT_EQ(base::FilePath(FILE_PATH_LITERAL("")).value(),
+              persisted_data->GetExistenceCheckerPath(kTestAppId).value());
+  }
+
+  Uninstall();
+  Clean();
+}
+
 #endif  // OS_MAC
 
 #if defined(OS_WIN)

chrome/updater/util.h

@@ -37,6 +37,9 @@ bool GetBaseDirectory(base::FilePath* path);
 // %localappdata%\Chromium\ChromiumUpdater\1.2.3.4 for a User install.
 bool GetVersionedDirectory(base::FilePath* path);
 
+// Returns true if the user running the updater also owns the |path|.
+bool PathOwnedByUser(const base::FilePath& path);
+
 // Initializes logging for an executable.
 void InitLogging(const base::FilePath::StringType& filename);
 

chrome/updater/win/util.cc

@@ -387,4 +387,10 @@ base::win::ScopedHandle GetUserTokenFromCurrentSessionId() {
   return token_handle;
 }
 
+bool PathOwnedByUser(const base::FilePath& path) {
+  // TODO(crbug.com/1147094): Implement for Win.
+
+  return true;
+}
+
 }  // namespace updater