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. 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 16bb3aa15a5378e8319f75f4b6b72b39177828f4..5a64220aaf1309832dc0ad543e353de67fe0a779 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 @@ -118,6 +125,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 @@ -173,6 +182,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. @@ -222,6 +233,8 @@ class ProcessSingleton { // because it posts messages between threads. class LinuxWatcher; scoped_refptr watcher_; + int sock_ = -1; + bool listen_on_ready_ = false; #endif #if BUILDFLAG(IS_MAC) diff --git a/chrome/browser/process_singleton_posix.cc b/chrome/browser/process_singleton_posix.cc index 3bf385781635bf6e7ccf97abdca43befd29e8e95..9d6ef0e143bdaf4a5043ebbb57d282d72d847433 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" @@ -96,9 +97,11 @@ #include "net/base/network_interfaces.h" #include "ui/base/l10n/l10n_util.h" +#if 0 #if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) #include "chrome/browser/ui/process_singleton_dialog_linux.h" #endif +#endif using content::BrowserThread; @@ -342,6 +345,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)); @@ -361,6 +367,7 @@ bool DisplayProfileInUseError(const base::FilePath& lock_path, NOTREACHED(); return false; +#endif } bool IsChromeProcess(pid_t pid) { @@ -401,6 +408,21 @@ bool CheckCookie(const base::FilePath& path, const base::FilePath& cookie) { return (cookie == ReadLink(path)); } +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) +} + bool ConnectSocket(ScopedSocket* socket, const base::FilePath& socket_path, const base::FilePath& cookie_path) { @@ -768,6 +790,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; + std::ignore = socket_dir_.Delete(); } ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() { @@ -932,6 +958,20 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate() { base::Seconds(kTimeoutInSeconds)); } +void ProcessSingleton::StartListeningOnSocket() { + watcher_ = base::MakeRefCounted(this); + content::GetIOThreadTaskRunner({})->PostTask(FROM_HERE, + 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, @@ -1031,14 +1071,32 @@ 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."; + 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) && @@ -1081,10 +1139,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 363c18e4d9d5c0fe30f843d32df67e97e3d73111..0c87fc8ccb4511904f19b76ae5e03a5df6664391 100644 --- a/chrome/browser/process_singleton_win.cc +++ b/chrome/browser/process_singleton_win.cc @@ -29,7 +29,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,11 +165,16 @@ bool ProcessLaunchNotification( } bool DisplayShouldKillMessageBox() { +#if 0 TRACE_EVENT0("startup", "ProcessSingleton:DisplayShouldKillMessageBox"); 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; } // Function was copied from Process::Terminate. @@ -252,9 +259,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), @@ -368,13 +379,16 @@ 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() { 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_)) { @@ -383,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; @@ -422,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()); } }