commit 57603811195db25088f3a41bc12007f3ee1a1fe7
parent 415f58c3f99a4d451366698b58e6cf75b74e863e
Author: Yannis Juglaret <yjuglaret@mozilla.com>
Date: Wed, 17 Dec 2025 15:21:19 +0000
Bug 1957156 - Cleanup pass in utility code. r=ipc-reviewers,nika,haik
- Rewrite TestUtilityProcess to use mozilla::WaitFor for readability.
- Cover more code in TestUtilityProcess.
- Provide a header for the TestUtilityProcess test suite so that it can
be used in the upcoming utility PKCS#11 process test.
- Ensure that the success of TestUtilityProcessSandboxing doesn't depend
on the integer value of SandboxingKind::COUNT.
- Ensure that the decision to start a utility process with or without a
sandbox faithfully follows IsUtilitySandboxEnabled() on macOS.
- Remove an unnecessary listing of unsandboxed process kinds in
SandboxBroker::SetSecurityLevelForUtilityProcess to ease adding
new unsandboxed process kinds in the future (e.g. in the patch that
follows).
- Similarly, prepare support for unsandboxed kinds on Linux without
having to reenumerate them.
Differential Revision: https://phabricator.services.mozilla.com/D270689
Diffstat:
7 files changed, 115 insertions(+), 122 deletions(-)
diff --git a/ipc/glue/UtilityProcessHost.cpp b/ipc/glue/UtilityProcessHost.cpp
@@ -56,10 +56,6 @@ LazyLogModule gUtilityProcessLog("utilityproc");
("UtilityProcessHost=%p, " msg, this, ##__VA_ARGS__))
#endif
-#if defined(XP_MACOSX) && defined(MOZ_SANDBOX)
-bool UtilityProcessHost::sLaunchWithMacSandbox = false;
-#endif
-
UtilityProcessHost::UtilityProcessHost(SandboxingKind aSandbox,
RefPtr<Listener> aListener)
: GeckoChildProcessHost(GeckoProcessType_Utility),
@@ -71,10 +67,7 @@ UtilityProcessHost::UtilityProcessHost(SandboxingKind aSandbox,
this, aSandbox);
#if defined(XP_MACOSX) && defined(MOZ_SANDBOX)
- if (!sLaunchWithMacSandbox) {
- sLaunchWithMacSandbox = IsUtilitySandboxEnabled(aSandbox);
- }
- mDisableOSActivityMode = sLaunchWithMacSandbox;
+ mDisableOSActivityMode = IsUtilitySandboxEnabled(aSandbox);
#endif
#if defined(MOZ_SANDBOX)
mSandbox = aSandbox;
@@ -196,15 +189,17 @@ void UtilityProcessHost::InitAfterConnect(bool aSucceeded) {
#if defined(XP_LINUX) && defined(MOZ_SANDBOX)
UniquePtr<SandboxBroker::Policy> policy;
- switch (mSandbox) {
- case SandboxingKind::GENERIC_UTILITY:
- policy = SandboxBrokerPolicyFactory::GetUtilityProcessPolicy(
- GetActor()->OtherPid());
- break;
-
- default:
- MOZ_ASSERT(false, "Invalid SandboxingKind");
- break;
+ if (IsUtilitySandboxEnabled(mSandbox)) {
+ switch (mSandbox) {
+ case SandboxingKind::GENERIC_UTILITY:
+ policy = SandboxBrokerPolicyFactory::GetUtilityProcessPolicy(
+ GetActor()->OtherPid());
+ break;
+
+ default:
+ MOZ_ASSERT(false, "Invalid SandboxingKind");
+ break;
+ }
}
if (policy != nullptr) {
brokerFd = Some(FileDescriptor());
diff --git a/ipc/glue/UtilityProcessHost.h b/ipc/glue/UtilityProcessHost.h
@@ -108,10 +108,8 @@ class UtilityProcessHost final : public mozilla::ipc::GeckoChildProcessHost {
void DestroyProcess();
#if defined(XP_MACOSX) && defined(MOZ_SANDBOX)
- static bool sLaunchWithMacSandbox;
-
- // Sandbox the Utility process at launch for all instances
- bool IsMacSandboxLaunchEnabled() override { return sLaunchWithMacSandbox; }
+ // Sandbox utility processes based on IsUtilitySandboxEnabled()
+ bool IsMacSandboxLaunchEnabled() override { return mDisableOSActivityMode; }
// Override so we can turn on Utility process-specific sandbox logging
bool FillMacSandboxInfo(MacSandboxInfo& aInfo) override;
diff --git a/ipc/glue/test/gtest/TestUtilityProcess.cpp b/ipc/glue/test/gtest/TestUtilityProcess.cpp
@@ -3,8 +3,13 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+#include <algorithm>
+#include <type_traits>
+
#include "gtest/gtest.h"
-#include "mozilla/SpinEventLoopUntil.h"
+#include "mozilla/gtest/ipc/TestUtilityProcess.h"
+#include "mozilla/gtest/WaitFor.h"
+#include "nsThreadUtils.h"
#include "mozilla/ipc/UtilityProcessManager.h"
@@ -27,116 +32,86 @@
#endif // XP_MACOSX
using namespace mozilla;
+using namespace mozilla::gtest::ipc;
using namespace mozilla::ipc;
-#define WAIT_FOR_EVENTS \
- SpinEventLoopUntil("UtilityProcess::emptyUtil"_ns, [&]() { return done; });
-
-bool setupDone = false;
-
-class UtilityProcess : public ::testing::Test {
- protected:
- void SetUp() override {
- if (setupDone) {
- return;
- }
-
-#if defined(MOZ_WIDGET_ANDROID) || defined(XP_MACOSX)
- appShell = do_GetService(NS_APPSHELLSERVICE_CONTRACTID);
-#endif // defined(MOZ_WIDGET_ANDROID) || defined(XP_MACOSX)
-
+// Note that some test suites inherit TestUtilityProcess, so any change here
+// will propagate there. Please ensure compatibility.
+/* static */ void TestUtilityProcess::SetUpTestSuite() {
#if defined(XP_WIN) && defined(MOZ_SANDBOX)
+ // Ensure only one execution even with GTEST_REPEAT>1
+ static bool sOnce = false;
+ if (!sOnce) {
mozilla::SandboxBroker::GeckoDependentInitialize();
-#endif // defined(XP_WIN) && defined(MOZ_SANDBOX)
-
- setupDone = true;
+ sOnce = true;
}
+#endif // defined(XP_WIN) && defined(MOZ_SANDBOX)
#if defined(MOZ_WIDGET_ANDROID) || defined(XP_MACOSX)
- nsCOMPtr<nsIAppShellService> appShell;
+ // Ensure that the app shell service is running
+ nsCOMPtr<nsIAppShellService> appShell =
+ do_GetService(NS_APPSHELLSERVICE_CONTRACTID);
#endif // defined(MOZ_WIDGET_ANDROID) || defined(XP_MACOSX)
-};
-
-TEST_F(UtilityProcess, ProcessManager) {
- RefPtr<UtilityProcessManager> utilityProc =
- UtilityProcessManager::GetSingleton();
- ASSERT_NE(utilityProc, nullptr);
}
-TEST_F(UtilityProcess, NoProcess) {
- RefPtr<UtilityProcessManager> utilityProc =
- UtilityProcessManager::GetSingleton();
- EXPECT_NE(utilityProc, nullptr);
+TEST_F(TestUtilityProcess, LaunchAllKinds) {
+ using kind_t = std::underlying_type<SandboxingKind>::type;
- Maybe<int32_t> noPid =
- utilityProc->ProcessPid(SandboxingKind::GENERIC_UTILITY);
- ASSERT_TRUE(noPid.isNothing());
-}
+ auto manager = UtilityProcessManager::GetSingleton();
+ ASSERT_TRUE(manager);
-TEST_F(UtilityProcess, LaunchProcess) {
- bool done = false;
+ auto currentPid = base::GetCurrentProcId();
+ ASSERT_GE(currentPid, base::ProcessId(1));
- RefPtr<UtilityProcessManager> utilityProc =
- UtilityProcessManager::GetSingleton();
- EXPECT_NE(utilityProc, nullptr);
-
- int32_t thisPid = base::GetCurrentProcId();
- EXPECT_GE(thisPid, 1);
-
- utilityProc->LaunchProcess(SandboxingKind::GENERIC_UTILITY)
- ->Then(
- GetCurrentSerialEventTarget(), __func__,
- [&]() mutable {
- EXPECT_TRUE(true);
-
- Maybe<int32_t> utilityPid =
- utilityProc->ProcessPid(SandboxingKind::GENERIC_UTILITY);
- EXPECT_TRUE(utilityPid.isSome());
- EXPECT_GE(*utilityPid, 1);
- EXPECT_NE(*utilityPid, thisPid);
-
- printf_stderr("UtilityProcess running as %d\n", *utilityPid);
+ // Launch all kinds
+ for (kind_t i = 0; i < SandboxingKind::COUNT; ++i) {
+ auto kind = static_cast<SandboxingKind>(i);
+ auto res = WaitFor(manager->LaunchProcess(kind));
+ ASSERT_TRUE(res.isOk())
+ << "First launch LaunchError: " << res.inspectErr().FunctionName() << ", "
+ << res.inspectErr().ErrorCode();
+ }
- done = true;
- },
- [&](LaunchError const&) {
- EXPECT_TRUE(false);
- done = true;
- });
+ // Collect process identifiers
+ std::array<base::ProcessId, SandboxingKind::COUNT> pids{};
+ for (kind_t i = 0; i < SandboxingKind::COUNT; ++i) {
+ auto kind = static_cast<SandboxingKind>(i);
+ auto utilityPid = manager->ProcessPid(kind);
+ ASSERT_TRUE(utilityPid.isSome())
+ << "No PID for kind " << kind;
+ ASSERT_GE(*utilityPid, base::ProcessId(1));
+ ASSERT_NE(*utilityPid, currentPid);
- WAIT_FOR_EVENTS;
-}
+ printf_stderr("Utility process running as PID %" PRIPID "\n", *utilityPid);
-TEST_F(UtilityProcess, DestroyProcess) {
- bool done = false;
-
- RefPtr<UtilityProcessManager> utilityProc =
- UtilityProcessManager::GetSingleton();
+ pids[i] = *utilityPid;
+ }
- utilityProc->LaunchProcess(SandboxingKind::GENERIC_UTILITY)
- ->Then(
- GetCurrentSerialEventTarget(), __func__,
- [&]() {
- Maybe<int32_t> utilityPid =
- utilityProc->ProcessPid(SandboxingKind::GENERIC_UTILITY);
- EXPECT_TRUE(utilityPid.isSome());
- EXPECT_GE(*utilityPid, 1);
+ // Re-launching should resolve immediately with process identifiers unchanged
+ for (kind_t i = 0; i < SandboxingKind::COUNT; ++i) {
+ auto kind = static_cast<SandboxingKind>(i);
+ auto res = WaitFor(manager->LaunchProcess(kind));
+ ASSERT_TRUE(res.isOk())
+ << "Second launch LaunchError: " << res.inspectErr().FunctionName() << ", "
+ << res.inspectErr().ErrorCode();
- utilityProc->CleanShutdown(SandboxingKind::GENERIC_UTILITY);
+ ASSERT_TRUE(manager->ProcessPid(kind) == Some(pids[i]));
+ }
- utilityPid =
- utilityProc->ProcessPid(SandboxingKind::GENERIC_UTILITY);
- EXPECT_TRUE(utilityPid.isNothing());
+ // Check that every process identifier is unique
+ std::sort(pids.begin(), pids.end());
+ auto adjacentEqualPids = std::adjacent_find(pids.begin(), pids.end());
+ ASSERT_TRUE(adjacentEqualPids == pids.end());
- EXPECT_TRUE(true);
- done = true;
- },
- [&](LaunchError const&) {
- EXPECT_TRUE(false);
- done = true;
- });
+ // After being individually shut down, a process is no longer referenced
+ for (kind_t i = 0; i < SandboxingKind::COUNT; ++i) {
+ auto kind = static_cast<SandboxingKind>(i);
+ manager->CleanShutdown(kind);
+ ASSERT_TRUE(manager->ProcessPid(kind).isNothing());
+ }
- WAIT_FOR_EVENTS;
+ // Drain the event queue.
+ NS_ProcessPendingEvents(nullptr);
}
#if defined(XP_WIN)
@@ -147,9 +122,7 @@ static void LoadLibraryCrash_Test() {
L"2b49036e-6ba3-400c-a297-38fa1f6c5255.dll");
}
-TEST_F(UtilityProcess, LoadLibraryCrash) {
+TEST_F(TestUtilityProcess, LoadLibraryCrash) {
ASSERT_DEATH_IF_SUPPORTED(LoadLibraryCrash_Test(), "");
}
#endif // defined(XP_WIN)
-
-#undef WAIT_FOR_EVENTS
diff --git a/ipc/glue/test/gtest/TestUtilityProcess.h b/ipc/glue/test/gtest/TestUtilityProcess.h
@@ -0,0 +1,20 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_gtest_ipc_TestUtilityProcess_h
+#define mozilla_gtest_ipc_TestUtilityProcess_h
+
+#include "gtest/gtest.h"
+
+namespace mozilla::gtest::ipc {
+
+class TestUtilityProcess : public ::testing::Test {
+ protected:
+ static void SetUpTestSuite();
+};
+
+} // namespace mozilla::gtest::ipc
+
+#endif // mozilla_gtest_ipc_TestUtilityProcess_h
diff --git a/ipc/glue/test/gtest/TestUtilityProcessSandboxing.cpp b/ipc/glue/test/gtest/TestUtilityProcessSandboxing.cpp
@@ -8,6 +8,8 @@
#include "mozilla/gtest/MozHelpers.h"
#include "mozilla/ipc/UtilityProcessSandboxing.h"
+#include <sstream>
+
using namespace mozilla;
using namespace mozilla::ipc;
@@ -55,7 +57,7 @@ TEST(UtilityProcessSandboxing, ParseEnvVar_DisableWMFOnly)
}
#endif // defined(XP_WIN)
-TEST(UtilityProcessSandboxing, ParseEnvVar_DisableGenericOnly_Multiples)
+TEST(UtilityProcessSandboxing, ParseEnvVar_DisableMultiple)
{
EXPECT_FALSE(IsUtilitySandboxEnabled("utility:1,utility:0,utility:2",
SandboxingKind::GENERIC_UTILITY));
@@ -69,6 +71,9 @@ TEST(UtilityProcessSandboxing, ParseEnvVar_DisableGenericOnly_Multiples)
IsUtilitySandboxEnabled("utility:1,utility:0,utility:2",
SandboxingKind::UTILITY_AUDIO_DECODING_WMF));
#endif // XP_WIN
- EXPECT_TRUE(IsUtilitySandboxEnabled("utility:8,utility:0,utility:6",
- SandboxingKind::COUNT));
+ std::ostringstream envVar;
+ envVar << "utility:" << (SandboxingKind::COUNT + 1)
+ << ",utility:0,utility:" << (SandboxingKind::COUNT + 3);
+ EXPECT_TRUE(
+ IsUtilitySandboxEnabled(envVar.str().c_str(), SandboxingKind::COUNT));
}
diff --git a/ipc/glue/test/gtest/moz.build b/ipc/glue/test/gtest/moz.build
@@ -6,6 +6,10 @@
Library("ipcgluetest")
+EXPORTS.mozilla.gtest.ipc += [
+ "TestUtilityProcess.h",
+]
+
UNIFIED_SOURCES = [
"TestAsyncBlockers.cpp",
"TestUtilityProcess.cpp",
diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
@@ -36,6 +36,7 @@
#include "mozilla/WinDllServices.h"
#include "mozilla/WindowsVersion.h"
#include "mozilla/ipc/LaunchError.h"
+#include "mozilla/ipc/UtilityProcessSandboxing.h"
#include "nsAppDirectoryServiceDefs.h"
#include "nsCOMPtr.h"
#include "nsDirectoryServiceDefs.h"
@@ -1856,6 +1857,8 @@ bool BuildUtilitySandbox(sandbox::TargetConfig* config,
bool SandboxBroker::SetSecurityLevelForUtilityProcess(
mozilla::ipc::SandboxingKind aSandbox) {
+ MOZ_ASSERT(IsUtilitySandboxEnabled(aSandbox));
+
if (!mPolicy) {
return false;
}
@@ -1873,11 +1876,6 @@ bool SandboxBroker::SetSecurityLevelForUtilityProcess(
#endif
case mozilla::ipc::SandboxingKind::WINDOWS_UTILS:
return BuildUtilitySandbox(config, WindowsUtilitySandboxProps());
- case mozilla::ipc::SandboxingKind::WINDOWS_FILE_DIALOG:
- // This process type is not sandboxed. (See commentary in
- // `ipc::IsUtilitySandboxEnabled()`.)
- MOZ_ASSERT_UNREACHABLE("No sandboxing for this process type");
- return false;
default:
MOZ_ASSERT_UNREACHABLE("Unknown sandboxing value");
return false;
