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/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc index aca2f19f53657fc2b944b69418e099b056c8e734..e43b0d89e2e6ad0bcb2c34bfacc4ef3b59eaaab1 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc @@ -1424,7 +1424,6 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { switch (notify_result_) { case ProcessSingleton::PROCESS_NONE: // No process already running, fall through to starting a new one. - process_singleton_->StartWatching(); g_browser_process->platform_part()->PlatformSpecificCommandLineProcessing( *base::CommandLine::ForCurrentProcess()); break; diff --git a/chrome/browser/chrome_process_singleton.cc b/chrome/browser/chrome_process_singleton.cc index d97fa8a96c110acc25b0ef46d7a4ac1c708f7c76..0919af8e8b0a51ef8e8dd28f2f07139d197a7384 100644 --- a/chrome/browser/chrome_process_singleton.cc +++ b/chrome/browser/chrome_process_singleton.cc @@ -31,10 +31,6 @@ ProcessSingleton::NotifyResult return process_singleton_.NotifyOtherProcessOrCreate(); } -void ChromeProcessSingleton::StartWatching() { - process_singleton_.StartWatching(); -} - void ChromeProcessSingleton::Cleanup() { process_singleton_.Cleanup(); } diff --git a/chrome/browser/process_singleton.h b/chrome/browser/process_singleton.h index 7cd82d27a741f194da5d0b3fcfd9c15c8ea1fa5c..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 @@ -127,9 +136,6 @@ class ProcessSingleton { // another process should call this directly. bool Create(); - // Start watching for notifications from other processes. - void StartWatching(); - // Clear any lock state during shutdown. void Cleanup(); @@ -176,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. @@ -220,12 +228,13 @@ class ProcessSingleton { // Temporary directory to hold the socket. base::ScopedTempDir socket_dir_; - int sock_ = -1; // Helper class for linux specific messages. LinuxWatcher is ref counted // 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 7e4cc9cdd350e814c20feccdcc8d70b1080e60a1..2de07460462632680f9b16a019744527dcee5125 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" @@ -97,9 +98,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; @@ -343,6 +346,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)); @@ -362,6 +368,7 @@ bool DisplayProfileInUseError(const base::FilePath& lock_path, NOTREACHED(); return false; +#endif } bool IsChromeProcess(pid_t pid) { @@ -402,6 +409,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) { @@ -757,7 +779,8 @@ ProcessSingleton::ProcessSingleton( const base::FilePath& user_data_dir, const NotificationCallback& notification_callback) : notification_callback_(notification_callback), - current_pid_(base::GetCurrentProcId()) { + current_pid_(base::GetCurrentProcId()), + watcher_(new LinuxWatcher(this)) { socket_path_ = user_data_dir.Append(chrome::kSingletonSocketFilename); lock_path_ = user_data_dir.Append(chrome::kSingletonLockFilename); cookie_path_ = user_data_dir.Append(chrome::kSingletonCookieFilename); @@ -768,6 +791,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 +959,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 +1072,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) && @@ -1050,9 +1109,10 @@ bool ProcessSingleton::Create() { // leaving a dangling symlink. base::FilePath socket_target_path = socket_dir_.GetPath().Append(chrome::kSingletonSocketFilename); + int sock; SockaddrUn addr; socklen_t socklen; - SetupSocket(socket_target_path.value(), &sock_, &addr, &socklen); + SetupSocket(socket_target_path.value(), &sock, &addr, &socklen); // Setup the socket symlink and the two cookies. base::FilePath cookie(GenerateCookie()); @@ -1071,26 +1131,24 @@ bool ProcessSingleton::Create() { return false; } - if (bind(sock_, reinterpret_cast(&addr), socklen) < 0) { + if (bind(sock, reinterpret_cast(&addr), socklen) < 0) { PLOG(ERROR) << "Failed to bind() " << socket_target_path.value(); - CloseSocket(sock_); + CloseSocket(sock); return false; } - if (listen(sock_, 5) < 0) + if (listen(sock, 5) < 0) NOTREACHED() << "listen failed: " << base::safe_strerror(errno); - return true; -} + sock_ = sock; -void ProcessSingleton::StartWatching() { - DCHECK_GE(sock_, 0); - DCHECK(!watcher_); - watcher_ = new LinuxWatcher(this); - DCHECK(BrowserThread::IsThreadInitialized(BrowserThread::IO)); - content::GetIOThreadTaskRunner({})->PostTask( - FROM_HERE, base::BindOnce(&ProcessSingleton::LinuxWatcher::StartListening, - watcher_, sock_)); + if (BrowserThread::IsThreadInitialized(BrowserThread::IO)) { + StartListeningOnSocket(); + } else { + listen_on_ready_ = true; + } + + return true; } void ProcessSingleton::Cleanup() { diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc index 41bc176510f93ef667e4f1373eab61142f3e264f..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()); } } @@ -430,8 +455,6 @@ bool ProcessSingleton::Create() { return window_.hwnd() != NULL; } -void ProcessSingleton::StartWatching() {} - void ProcessSingleton::Cleanup() { }