From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Wed, 18 Aug 2021 08:24:10 -0700 Subject: extend ProcessSingleton This patch applies Electron ProcessSingleton changes onto the Chromium files. This patch adds a few changes to the Chromium code: 1. It adds a parameter `program_name` to the Windows constructor, making the generated mutex name on the Windows-side program-dependent, rather than shared between all Electron applications. 2. It adds an `IsAppSandboxed` check for macOS so that sandboxed applications generate shorter temp paths. 3. It adds a `ChangeWindowMessageFilterEx` call to the Windows implementation, along with a parameter `is_app_sandboxed` in the constructor, to handle the case when the primary app is run with admin permissions. diff --git a/chrome/browser/process_singleton.h b/chrome/browser/process_singleton.h index 23a8257aa2a0a671cf7af35aff9906891091606d..31f5b160e4cd755cfb56a62b04261ee1bee80277 100644 --- a/chrome/browser/process_singleton.h +++ b/chrome/browser/process_singleton.h @@ -102,12 +102,19 @@ class ProcessSingleton { base::RepeatingCallback; +#if BUILDFLAG(IS_WIN) + ProcessSingleton(const std::string& program_name, + const base::FilePath& user_data_dir, + bool is_sandboxed, + const NotificationCallback& notification_callback); +#else ProcessSingleton(const base::FilePath& user_data_dir, const NotificationCallback& notification_callback); ProcessSingleton(const ProcessSingleton&) = delete; ProcessSingleton& operator=(const ProcessSingleton&) = delete; +#endif ~ProcessSingleton(); // Notify another process, if available. Otherwise sets ourselves as the @@ -176,6 +183,8 @@ class ProcessSingleton { #if BUILDFLAG(IS_WIN) bool EscapeVirtualization(const base::FilePath& user_data_dir); + std::string program_name_; // Used for mutexName. + bool is_app_sandboxed_; // Whether the Electron app is sandboxed. HWND remote_window_; // The HWND_MESSAGE of another browser. base::win::MessageWindow window_; // The message-only window. bool is_virtualized_; // Stuck inside Microsoft Softricity VM environment. diff --git a/chrome/browser/process_singleton_posix.cc b/chrome/browser/process_singleton_posix.cc index f2913f2d78edb2547ae21816b34936d38f1870a9..298c9c81fa110ad7900d0bd6822136bb57f0382e 100644 --- a/chrome/browser/process_singleton_posix.cc +++ b/chrome/browser/process_singleton_posix.cc @@ -54,6 +54,7 @@ #include #include #include +#include #include #include "base/base_paths.h" @@ -81,6 +82,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/task/sequenced_task_runner_helpers.h" #include "base/task/single_thread_task_runner.h" +#include "base/threading/thread_restrictions.h" #include "base/threading/platform_thread.h" #include "base/time/time.h" #include "base/timer/timer.h" @@ -97,7 +99,7 @@ #include "ui/base/l10n/l10n_util.h" #include "ui/base/resource/scoped_startup_resource_bundle.h" -#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) +#if 0 #include "chrome/browser/ui/process_singleton_dialog_linux.h" #endif @@ -343,6 +345,8 @@ bool SymlinkPath(const base::FilePath& target, const base::FilePath& path) { bool DisplayProfileInUseError(const base::FilePath& lock_path, const std::string& hostname, int pid) { + return true; +#if 0 // Ensure there is an instance of ResourceBundle that is initialized for // localized string resource accesses. ui::ScopedStartupResourceBundle ensure_startup_resource_bundle; @@ -366,6 +370,7 @@ bool DisplayProfileInUseError(const base::FilePath& lock_path, NOTREACHED(); return false; +#endif } bool IsChromeProcess(pid_t pid) { @@ -378,6 +383,21 @@ bool IsChromeProcess(pid_t pid) { base::FilePath(chrome::kBrowserProcessExecutableName)); } +bool IsAppSandboxed() { +#if BUILDFLAG(IS_MAC) + // NB: There is no sane API for this, we have to just guess by + // reading tea leaves + base::FilePath home_dir; + if (!base::PathService::Get(base::DIR_HOME, &home_dir)) { + return false; + } + + return home_dir.value().find("Library/Containers") != std::string::npos; +#else + return false; +#endif // BUILDFLAG(IS_MAC) +} + // A helper class to hold onto a socket. class ScopedSocket { public: @@ -781,6 +801,10 @@ ProcessSingleton::~ProcessSingleton() { if (watcher_) { watcher_->OnEminentProcessSingletonDestruction(); } + // Manually free resources with IO explicitly allowed. + base::ScopedAllowBlocking allow_blocking; + watcher_ = nullptr; + std::ignore = socket_dir_.Delete(); } ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() { @@ -1047,11 +1071,32 @@ bool ProcessSingleton::Create() { // Create the socket file somewhere in /tmp which is usually mounted as a // normal filesystem. Some network filesystems (notably AFS) are screwy and // do not support Unix domain sockets. - if (!socket_dir_.CreateUniqueTempDir()) { - LOG(ERROR) << "Failed to create socket directory."; + base::FilePath tmp_dir; + if (!base::GetTempDir(&tmp_dir)) { + LOG(ERROR) << "Failed to get temporary directory."; return false; } + if (IsAppSandboxed()) { + // For sandboxed applications, the tmp dir could be too long to fit + // addr->sun_path, so we need to make it as short as possible. + if (!socket_dir_.Set(tmp_dir.Append("S"))) { + LOG(ERROR) << "Failed to set socket directory."; + return false; + } + } else { + // Create the socket file somewhere in /tmp which is usually mounted as a + // normal filesystem. Some network filesystems (notably AFS) are screwy and + // do not support Unix domain sockets. + // Prefer CreateUniqueTempDirUnderPath rather than CreateUniqueTempDir as + // the latter will calculate unique paths based on bundle ids which can + // increase the socket path length than what is allowed. + if (!socket_dir_.CreateUniqueTempDirUnderPath(tmp_dir)) { + LOG(ERROR) << "Failed to create socket directory."; + return false; + } + } + // Check that the directory was created with the correct permissions. int dir_mode = 0; CHECK(base::GetPosixFilePermissions(socket_dir_.GetPath(), &dir_mode) && diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc index 071f3c05b8f899857531f2f08c05c7f2551eb4d0..11f35769cc53b4aa111a319d155a3916f0040fa7 100644 --- a/chrome/browser/process_singleton_win.cc +++ b/chrome/browser/process_singleton_win.cc @@ -28,7 +28,9 @@ #include "base/win/wmi.h" #include "chrome/browser/process_singleton_internal.h" #include "chrome/browser/shell_integration.h" +#if 0 #include "chrome/browser/ui/simple_message_box.h" +#endif #include "chrome/browser/win/chrome_process_finder.h" #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_paths.h" @@ -163,6 +165,7 @@ bool ProcessLaunchNotification( } bool DisplayShouldKillMessageBox() { +#if 0 TRACE_EVENT0("startup", "ProcessSingleton:DisplayShouldKillMessageBox"); // Ensure there is an instance of ResourceBundle that is initialized for @@ -173,6 +176,10 @@ bool DisplayShouldKillMessageBox() { NULL, l10n_util::GetStringUTF16(IDS_PRODUCT_NAME), l10n_util::GetStringUTF16(IDS_BROWSER_HUNGBROWSER_MESSAGE)) != chrome::MESSAGE_BOX_RESULT_NO; +#endif + // This is called when the secondary process can't ping the primary + // process. + return false; } // Function was copied from Process::Terminate. @@ -255,9 +262,13 @@ bool ProcessSingleton::EscapeVirtualization( } ProcessSingleton::ProcessSingleton( + const std::string& program_name, const base::FilePath& user_data_dir, + bool is_app_sandboxed, const NotificationCallback& notification_callback) : notification_callback_(notification_callback), + program_name_(program_name), + is_app_sandboxed_(is_app_sandboxed), is_virtualized_(false), lock_file_(INVALID_HANDLE_VALUE), user_data_dir_(user_data_dir), @@ -377,7 +388,7 @@ ProcessSingleton::NotifyOtherProcessOrCreate() { bool ProcessSingleton::Create() { TRACE_EVENT0("startup", "ProcessSingleton::Create"); - static const wchar_t kMutexName[] = L"Local\\ChromeProcessSingletonStartup!"; + std::wstring mutexName = base::UTF8ToWide("Local\\" + program_name_ + "ProcessSingletonStartup"); remote_window_ = chrome::FindRunningChromeWindow(user_data_dir_); if (!remote_window_ && !EscapeVirtualization(user_data_dir_)) { @@ -386,7 +397,7 @@ bool ProcessSingleton::Create() { // access. As documented, it's clearer to NOT request ownership on creation // since it isn't guaranteed we will get it. It is better to create it // without ownership and explicitly get the ownership afterward. - base::win::ScopedHandle only_me(::CreateMutex(NULL, FALSE, kMutexName)); + base::win::ScopedHandle only_me(::CreateMutex(NULL, FALSE, mutexName.c_str())); if (!only_me.IsValid()) { DPLOG(FATAL) << "CreateMutex failed"; return false; @@ -425,6 +436,17 @@ bool ProcessSingleton::Create() { window_.CreateNamed(base::BindRepeating(&ProcessLaunchNotification, notification_callback_), user_data_dir_.value()); + + // When the app is sandboxed, firstly, the app should not be in + // admin mode, and even if it somehow is, messages from an unelevated + // instance should not be able to be sent to it. + if (!is_app_sandboxed_) { + // NB: Ensure that if the primary app gets started as elevated + // admin inadvertently, secondary windows running not as elevated + // will still be able to send messages. + ::ChangeWindowMessageFilterEx(window_.hwnd(), WM_COPYDATA, MSGFLT_ALLOW, + NULL); + } CHECK(result && window_.hwnd()); } }