From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Wed, 18 Aug 2021 08:24:10 -0700 Subject: Convert Electron ProcessSingleton changes to patch This patch applies Electron ProcessSingleton changes onto the Chromium files, so that Electron doesn't need to maintain separate copies of those 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. 4. It adds an `OnBrowserReady` function to allow `requestSingleInstanceLock` to start listening to the socket later in the posix implementation. This function is necessary, because otherwise, users calling `requestSingleInstanceLock` create a `ProcessSingleton` instance that tries to connect to the socket before the browser thread is ready. diff --git a/chrome/browser/process_singleton.h b/chrome/browser/process_singleton.h index 0d7c1db6489d95a40c66808c3f838b0740e46ff6..eec994c4252f17d9c9c41e66d5dae6509ed98a18 100644 --- a/chrome/browser/process_singleton.h +++ b/chrome/browser/process_singleton.h @@ -103,12 +103,19 @@ class ProcessSingleton { base::RepeatingCallback; +#if defined(OS_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 @@ -119,6 +126,8 @@ class ProcessSingleton { // TODO(brettw): Make the implementation of this method non-platform-specific // by making Linux re-use the Windows implementation. NotifyResult NotifyOtherProcessOrCreate(); + void StartListeningOnSocket(); + void OnBrowserReady(); // Sets ourself up as the singleton instance. Returns true on success. If // false is returned, we are not the singleton instance and the caller must @@ -174,6 +183,8 @@ class ProcessSingleton { #if defined(OS_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. @@ -223,6 +234,8 @@ class ProcessSingleton { // because it posts messages between threads. class LinuxWatcher; scoped_refptr watcher_; + int sock_ = -1; + bool listen_on_ready_ = false; #endif #if defined(OS_MAC) diff --git a/chrome/browser/process_singleton_posix.cc b/chrome/browser/process_singleton_posix.cc index 4547eb8563e1af57aad991d9d1e2cf02c778380a..727333dd6abec99643e31bc77ed2cc8f3d5a0a0b 100644 --- a/chrome/browser/process_singleton_posix.cc +++ b/chrome/browser/process_singleton_posix.cc @@ -80,6 +80,7 @@ #include "base/strings/stringprintf.h" #include "base/strings/sys_string_conversions.h" #include "base/strings/utf_string_conversions.h" +#include "base/task/post_task.h" #include "base/threading/platform_thread.h" #include "base/threading/thread_task_runner_handle.h" #include "base/time/time.h" @@ -95,9 +96,11 @@ #include "net/base/network_interfaces.h" #include "ui/base/l10n/l10n_util.h" +#if 0 #if defined(OS_LINUX) || defined(OS_CHROMEOS) #include "chrome/browser/ui/process_singleton_dialog_linux.h" #endif +#endif #if defined(TOOLKIT_VIEWS) && \ (defined(OS_LINUX) || BUILDFLAG(IS_CHROMEOS_LACROS)) @@ -289,6 +292,9 @@ 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 std::u16string error = l10n_util::GetStringFUTF16( IDS_PROFILE_IN_USE_POSIX, base::NumberToString16(pid), base::ASCIIToUTF16(hostname)); @@ -308,6 +314,7 @@ bool DisplayProfileInUseError(const base::FilePath& lock_path, NOTREACHED(); return false; +#endif } bool IsChromeProcess(pid_t pid) { @@ -348,6 +355,21 @@ bool CheckCookie(const base::FilePath& path, const base::FilePath& cookie) { return (cookie == ReadLink(path)); } +bool IsAppSandboxed() { +#if defined(OS_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 // defined(OS_MAC) +} + bool ConnectSocket(ScopedSocket* socket, const base::FilePath& socket_path, const base::FilePath& cookie_path) { @@ -729,6 +751,10 @@ ProcessSingleton::ProcessSingleton( ProcessSingleton::~ProcessSingleton() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + // Manually free resources with IO explicitly allowed. + base::ThreadRestrictions::ScopedAllowIO allow_io; + watcher_ = nullptr; + ignore_result(socket_dir_.Delete()); } ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() { @@ -897,6 +923,20 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate() { base::Seconds(kTimeoutInSeconds)); } +void ProcessSingleton::StartListeningOnSocket() { + watcher_ = base::MakeRefCounted(this); + base::PostTask(FROM_HERE, {BrowserThread::IO}, + base::BindOnce(&ProcessSingleton::LinuxWatcher::StartListening, + watcher_, sock_)); +} + +void ProcessSingleton::OnBrowserReady() { + if (listen_on_ready_) { + StartListeningOnSocket(); + listen_on_ready_ = false; + } +} + ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeoutOrCreate( const base::CommandLine& command_line, @@ -999,12 +1039,26 @@ bool ProcessSingleton::Create() { #endif } - // 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."; - 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. + base::FilePath tmp_dir; + if (!base::GetTempDir(&tmp_dir)) { + LOG(ERROR) << "Failed to get temporary directory."; + return false; + } + 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. + if (!socket_dir_.CreateUniqueTempDir()) { + LOG(ERROR) << "Failed to create socket directory."; + return false; + } } // Check that the directory was created with the correct permissions. @@ -1046,10 +1100,13 @@ bool ProcessSingleton::Create() { if (listen(sock, 5) < 0) NOTREACHED() << "listen failed: " << base::safe_strerror(errno); - DCHECK(BrowserThread::IsThreadInitialized(BrowserThread::IO)); - content::GetIOThreadTaskRunner({})->PostTask( - FROM_HERE, base::BindOnce(&ProcessSingleton::LinuxWatcher::StartListening, - watcher_, sock)); + sock_ = sock; + + if (BrowserThread::IsThreadInitialized(BrowserThread::IO)) { + StartListeningOnSocket(); + } else { + listen_on_ready_ = true; + } return true; } diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc index e7cc3c5bc7912b27f1f8ce0fb8f691e28284e238..19d5659d665321da54e05cee01be7da02e0c283b 100644 --- a/chrome/browser/process_singleton_win.cc +++ b/chrome/browser/process_singleton_win.cc @@ -28,7 +28,9 @@ #include "base/win/windows_version.h" #include "base/win/wmi.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" @@ -180,10 +182,15 @@ bool ProcessLaunchNotification( } bool DisplayShouldKillMessageBox() { +#if 0 return chrome::ShowQuestionMessageBoxSync( 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; } void SendRemoteProcessInteractionResultHistogram( @@ -265,9 +272,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), @@ -372,11 +383,14 @@ ProcessSingleton::NotifyOtherProcessOrCreate() { return PROFILE_IN_USE; } +void ProcessSingleton::StartListeningOnSocket() {} +void ProcessSingleton::OnBrowserReady() {} + // Look for a Chrome instance that uses the same profile directory. If there // isn't one, create a message window with its title set to the profile // directory path. bool 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_)) { @@ -385,7 +399,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; @@ -424,6 +438,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()); } }