diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index b12404012afe..4410d2885ffe 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1590,15 +1590,16 @@ pref("browser.bookmarks.editDialog.maxRecentFolders", 7); // exotic configurations we can't reasonably support out of the box. // pref("security.sandbox.content.level", 6); - // Introduced as part of bug 1608558. Linux is currently the only platform - // that uses a sandbox level for the socket process. There are currently - // only 2 levels: - // 0 -> "no sandbox" - // 1 -> "sandboxed, allows socket operations and reading necessary certs" - pref("security.sandbox.socket.process.level", 1); pref("security.sandbox.content.write_path_whitelist", ""); pref("security.sandbox.content.read_path_whitelist", ""); pref("security.sandbox.content.syscall_whitelist", ""); + // Introduced as part of bug 1608558. Linux is currently the only platform + // that uses a sandbox level for the socket process. The following levels + // are currently defined: + // 0 -> "no sandbox" + // 1 -> "sandboxed, allows socket operations and reading necessary certs" + // 2 -> default-deny for ioctl + pref("security.sandbox.socket.process.level", 2); #endif #if defined(XP_OPENBSD) && defined(MOZ_SANDBOX) diff --git a/netwerk/ipc/SocketProcessChild.cpp b/netwerk/ipc/SocketProcessChild.cpp index 86c68b4829de..b3ee32bc2092 100644 --- a/netwerk/ipc/SocketProcessChild.cpp +++ b/netwerk/ipc/SocketProcessChild.cpp @@ -346,12 +346,9 @@ mozilla::ipc::IPCResult SocketProcessChild::RecvSetConnectivity( mozilla::ipc::IPCResult SocketProcessChild::RecvInitLinuxSandbox( const Maybe& aBrokerFd) { #if defined(XP_LINUX) && defined(MOZ_SANDBOX) - int fd = -1; - if (aBrokerFd.isSome()) { - fd = aBrokerFd.value().ClonePlatformHandle().release(); - } RegisterProfilerObserversForSandboxProfiler(); - SetSocketProcessSandbox(fd); + SetSocketProcessSandbox( + SocketProcessSandboxParams::ForThisProcess(aBrokerFd)); #endif // XP_LINUX && MOZ_SANDBOX return IPC_OK(); } diff --git a/security/sandbox/common/SandboxSettings.cpp b/security/sandbox/common/SandboxSettings.cpp index 300318f80263..1e89dc0d5543 100644 --- a/security/sandbox/common/SandboxSettings.cpp +++ b/security/sandbox/common/SandboxSettings.cpp @@ -207,6 +207,13 @@ int GetEffectiveSocketProcessSandboxLevel() { int level = StaticPrefs::security_sandbox_socket_process_level_DoNotUseDirectly(); +#ifdef XP_LINUX + // Turn off ioctl lockdown in safe mode, until it's gotten more testing. + if (level > 1 && gSafeMode) { + level = 1; + } +#endif + return level; } diff --git a/security/sandbox/common/test/SandboxTestingChildTests.h b/security/sandbox/common/test/SandboxTestingChildTests.h index 1c87edab29f7..8ab35c066937 100644 --- a/security/sandbox/common/test/SandboxTestingChildTests.h +++ b/security/sandbox/common/test/SandboxTestingChildTests.h @@ -721,6 +721,13 @@ void RunTestsSocket(SandboxTestingChild* child) { "are available"); return 0; }); + + child->ErrnoValueTest("ioctl_dma_buf"_ns, ENOSYS, [] { + // Attempt an arbitrary non-tty ioctl, on the wrong type of fd; if + // allowed it would fail with ENOTTY (see the RDD tests) but in + // this sandbox it should be blocked (ENOSYS). + return ioctl(0, _IOW('b', 0, uint64_t), nullptr); + }); # endif // XP_LINUX #elif XP_MACOSX RunMacTestLaunchProcess(child); diff --git a/security/sandbox/linux/Sandbox.cpp b/security/sandbox/linux/Sandbox.cpp index d33cd5d825d5..564a03285950 100644 --- a/security/sandbox/linux/Sandbox.cpp +++ b/security/sandbox/linux/Sandbox.cpp @@ -804,24 +804,24 @@ void SetRemoteDataDecoderSandbox(int aBroker) { SetCurrentProcessSandbox(GetDecoderSandboxPolicy(sBroker)); } -void SetSocketProcessSandbox(int aBroker) { +void SetSocketProcessSandbox(SocketProcessSandboxParams&& aParams) { if (!SandboxInfo::Get().Test(SandboxInfo::kHasSeccompBPF) || PR_GetEnv("MOZ_DISABLE_SOCKET_PROCESS_SANDBOX")) { - if (aBroker >= 0) { - close(aBroker); - } return; } gSandboxReporterClient = new SandboxReporterClient( SandboxReport::ProcType::SOCKET_PROCESS, TakeSandboxReporterFd()); + // FIXME(bug 1513773): merge this with the ones for content and RDD? static SandboxBrokerClient* sBroker; - if (aBroker >= 0) { - sBroker = new SandboxBrokerClient(aBroker); + MOZ_ASSERT(!sBroker); // This should only ever be called once. + if (aParams.mBroker) { + sBroker = new SandboxBrokerClient(aParams.mBroker.release()); } - SetCurrentProcessSandbox(GetSocketProcessSandboxPolicy(sBroker)); + SetCurrentProcessSandbox( + GetSocketProcessSandboxPolicy(sBroker, std::move(aParams))); } void SetUtilitySandbox(int aBroker, ipc::SandboxingKind aKind) { diff --git a/security/sandbox/linux/Sandbox.h b/security/sandbox/linux/Sandbox.h index 2b892d21668b..8c62e306a17b 100644 --- a/security/sandbox/linux/Sandbox.h +++ b/security/sandbox/linux/Sandbox.h @@ -54,6 +54,23 @@ struct ContentProcessSandboxParams { const Maybe& aBroker); }; +// Similarly to ContentProcessSandboxParams, a collection of +// parameters for the socket process. Currently this is just the +// level (and the broker), but in the future there could be more. +struct SocketProcessSandboxParams { + // Socket process sandbox level; see also GetEffectiveSandboxLevel + // and the comments for "security.sandbox.socket.process.level" in + // browser/app/profile/firefox.js + int mLevel = 0; + + // The filesystem broker client fd; this *is* a RAII class so it + // needs to be `release()`d or moved to consume it. + mozilla::UniqueFileHandle mBroker; + + static SocketProcessSandboxParams ForThisProcess( + const Maybe& aBroker); +}; + // Call only if SandboxInfo::CanSandboxContent() returns true. // (No-op if the sandbox is disabled.) // isFileProcess determines whether we allow system wide file reads. @@ -66,7 +83,7 @@ MOZ_EXPORT void SetMediaPluginSandbox(const char* aFilePath); MOZ_EXPORT void SetRemoteDataDecoderSandbox(int aBroker); -MOZ_EXPORT void SetSocketProcessSandbox(int aBroker); +MOZ_EXPORT void SetSocketProcessSandbox(SocketProcessSandboxParams&& aParams); MOZ_EXPORT void SetUtilitySandbox(int aBroker, ipc::SandboxingKind aKind); diff --git a/security/sandbox/linux/SandboxFilter.cpp b/security/sandbox/linux/SandboxFilter.cpp index ab30818c9193..a3fc8a1288dd 100644 --- a/security/sandbox/linux/SandboxFilter.cpp +++ b/security/sandbox/linux/SandboxFilter.cpp @@ -2056,8 +2056,15 @@ UniquePtr GetDecoderSandboxPolicy( // Basically a clone of RDDSandboxPolicy until we know exactly what // the SocketProcess sandbox looks like. class SocketProcessSandboxPolicy final : public SandboxPolicyCommon { + private: + SocketProcessSandboxParams mParams; + + bool BelowLevel(int aLevel) const { return mParams.mLevel < aLevel; } + public: - explicit SocketProcessSandboxPolicy(SandboxBrokerClient* aBroker) { + explicit SocketProcessSandboxPolicy(SandboxBrokerClient* aBroker, + SocketProcessSandboxParams&& aParams) + : mParams(std::move(aParams)) { mBroker = aBroker; mMayCreateShmem = true; } @@ -2139,9 +2146,10 @@ class SocketProcessSandboxPolicy final : public SandboxPolicyCommon { .ElseIf(request == FIONBIO, Allow()) // This is used by PR_Available in nsSocketInputStream::Available. .ElseIf(request == FIONREAD, Allow()) - // Allow anything that isn't a tty ioctl, for now; bug 1302711 - // will cover changing this to a default-deny policy. - .ElseIf(shifted_type != kTtyIoctls, Allow()) + // Allow anything that isn't a tty ioctl (if level < 2) + .ElseIf( + BelowLevel(2) ? shifted_type != kTtyIoctls : BoolConst(false), + Allow()) .Else(SandboxPolicyCommon::EvaluateSyscall(sysno)); } @@ -2193,9 +2201,9 @@ class SocketProcessSandboxPolicy final : public SandboxPolicyCommon { }; UniquePtr GetSocketProcessSandboxPolicy( - SandboxBrokerClient* aMaybeBroker) { + SandboxBrokerClient* aMaybeBroker, SocketProcessSandboxParams&& aParams) { return UniquePtr( - new SocketProcessSandboxPolicy(aMaybeBroker)); + new SocketProcessSandboxPolicy(aMaybeBroker, std::move(aParams))); } class UtilitySandboxPolicy : public SandboxPolicyCommon { diff --git a/security/sandbox/linux/SandboxFilter.h b/security/sandbox/linux/SandboxFilter.h index 04a37a32d438..34f682a8e46d 100644 --- a/security/sandbox/linux/SandboxFilter.h +++ b/security/sandbox/linux/SandboxFilter.h @@ -22,6 +22,7 @@ namespace mozilla { class SandboxBrokerClient; struct ContentProcessSandboxParams; +struct SocketProcessSandboxParams; UniquePtr GetContentSandboxPolicy( SandboxBrokerClient* aMaybeBroker, ContentProcessSandboxParams&& aParams); @@ -36,7 +37,7 @@ UniquePtr GetDecoderSandboxPolicy( SandboxBrokerClient* aMaybeBroker); UniquePtr GetSocketProcessSandboxPolicy( - SandboxBrokerClient* aMaybeBroker); + SandboxBrokerClient* aMaybeBroker, SocketProcessSandboxParams&& aParams); UniquePtr GetUtilitySandboxPolicy( SandboxBrokerClient* aMaybeBroker); diff --git a/security/sandbox/linux/glue/SandboxPrefBridge.cpp b/security/sandbox/linux/glue/SandboxPrefBridge.cpp index 9782e58817df..9de9655de192 100644 --- a/security/sandbox/linux/glue/SandboxPrefBridge.cpp +++ b/security/sandbox/linux/glue/SandboxPrefBridge.cpp @@ -47,4 +47,18 @@ ContentProcessSandboxParams::ForThisProcess( return params; } +/* static */ SocketProcessSandboxParams +SocketProcessSandboxParams::ForThisProcess( + const Maybe& aBroker) { + SocketProcessSandboxParams self; + + if (aBroker.isSome()) { + self.mBroker = aBroker->ClonePlatformHandle(); + MOZ_RELEASE_ASSERT(self.mBroker); + } + + self.mLevel = GetEffectiveSocketProcessSandboxLevel(); + return self; +} + } // namespace mozilla