mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1698778 - Move several pieces of the seccomp-bpf policies into SandboxPolicyCommon. r=gcp 

Minor functional changes:

1. `fcntl` `F_DUPFD_CLOEXEC` is now allowed everywhere instead of
   just content.  It's the obvious (and maybe only? and probably
   only portable) way for a library to `dup` and atomically set the
   close-on-exec flag, and appears harmless.

2. `ioctl`s used by the `isatty` function are denied with `ENOTTY` by
   default in all processes, instead of being treated as an invalid
   syscall, and this now applies to `TIOCGWINSZ` (used by musl) as well
   as `TCGETS` (used by glibc).  Nothing new is allowed here; it's just
   that this is treated as an expected denial.

3. Getting the real or effective user or group ID is allowed everywhere.
   Every process type except RDD previously did, and RDD soon will.  See
   also the new comment about why GMP may not always need it, but that
   it's not very meaningful to block.

Refactoring, no functional change intended:

1. The policy for the `kcmp` syscall as used by Mesa's `amdgpu` driver
   is now in a protected method of SandboxPolicyCommon, but is used only
   in the content process as previously.  A later patch will also apply
   it to the RDD process, so this avoids code duplication.

Differential Revision: https://phabricator.services.mozilla.com/D131679
This commit is contained in:
Jed Davis 2021-11-23 01:14:41 +00:00
Родитель ac85827e83
Коммит 158787246c
1 изменённых файлов: 49 добавлений и 48 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

97
security/sandbox/linux/SandboxFilter.cpp
Просмотреть файл

@ -166,6 +166,28 @@ class SandboxPolicyCommon : public SandboxPolicyBase {
return ConvertError(syscall(nr, args...));
}
// Mesa's amdgpu driver uses kcmp with KCMP_FILE; see also bug
// 1624743. This policy restricts it to the process's own pid,
// which should be sufficient on its own if we need to remove the
// `type` restriction in the future.
//
// (Note: if we end up with more Mesa-specific hooks needed in
// several process types, we could put them into this class's
// EvaluateSyscall guarded by a boolean member variable, or
// introduce another layer of subclassing.)
ResultExpr KcmpPolicyForMesa() const {
// The real KCMP_FILE is part of an anonymous enum in
// <linux/kcmp.h>, but we can't depend on having that header,
// and it's not a #define so the usual #ifndef approach
// doesn't work.
static const int kKcmpFile = 0;
const pid_t myPid = getpid();
Arg<pid_t> pid1(0), pid2(1);
Arg<int> type(2);
return If(AllOf(pid1 == myPid, pid2 == myPid, type == kKcmpFile), Allow())
.Else(InvalidSyscall());
}
private:
// Bug 1093893: Translate tkill to tgkill for pthread_kill; fixed in
// bionic commit 10c8ce59a (in JB and up; API level 16 = Android 4.1).
@ -768,6 +790,8 @@ class SandboxPolicyCommon : public SandboxPolicyBase {
.Case(F_GETFL, Allow())
.Case(F_SETFL, If((flags & ~allowed_flags) == 0, Allow())
.Else(InvalidSyscall()))
// Not much different from other forms of dup(), and commonly used.
.Case(F_DUPFD_CLOEXEC, Allow())
.Default(SandboxPolicyBase::EvaluateSyscall(sysno));
}
@ -932,6 +956,16 @@ class SandboxPolicyCommon : public SandboxPolicyBase {
case __NR_getrandom:
return Allow();
// Used by almost every process: GMP needs them for Clearkey
// because of bug 1576006 (but may not need them for other
// plugin types; see bug 1737092). Given that fstat is
// allowed, the uid/gid are probably available anyway.
CASES_FOR_getuid:
CASES_FOR_getgid:
CASES_FOR_geteuid:
CASES_FOR_getegid:
return Allow();
#ifdef DESKTOP
// Bug 1543858: glibc's qsort calls sysinfo to check the
// memory size; it falls back to assuming there's enough RAM.
@ -946,6 +980,18 @@ class SandboxPolicyCommon : public SandboxPolicyBase {
case __NR_rseq:
return Allow();
case __NR_ioctl: {
Arg<unsigned long> request(1);
// Make isatty() return false, because none of the terminal
// ioctls will be allowed; libraries sometimes call this for
// various reasons (e.g., to decide whether to emit ANSI/VT
// color codes when logging to stderr). glibc uses TCGETS and
// musl uses TIOCGWINSZ.
return If(AnyOf(request == TCGETS, request == TIOCGWINSZ),
Error(ENOTTY))
.Else(SandboxPolicyBase::EvaluateSyscall(sysno));
}
#ifdef MOZ_ASAN
// ASAN's error reporter wants to know if stderr is a tty.
case __NR_ioctl: {
@ -1345,9 +1391,6 @@ class ContentSandboxPolicy : public SandboxPolicyCommon {
return If(request == FIOCLEX, Allow())
// Rust's stdlib also uses FIONBIO instead of equivalent fcntls.
.ElseIf(request == FIONBIO, Allow())
// ffmpeg, and anything else that calls isatty(), will be told
// that nothing is a typewriter:
.ElseIf(request == TCGETS, Error(ENOTTY))
// 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())
@ -1357,7 +1400,6 @@ class ContentSandboxPolicy : public SandboxPolicyCommon {
CASES_FOR_fcntl : {
Arg<int> cmd(1);
return Switch(cmd)
.Case(F_DUPFD_CLOEXEC, Allow())
// Nvidia GL and fontconfig (newer versions) use fcntl file locking.
.Case(F_SETLK, Allow())
#ifdef F_SETLK64
@ -1406,12 +1448,6 @@ class ContentSandboxPolicy : public SandboxPolicyCommon {
CASES_FOR_dup2: // See ConnectTrapCommon
return Allow();
CASES_FOR_getuid:
CASES_FOR_getgid:
CASES_FOR_geteuid:
CASES_FOR_getegid:
return Allow();
case __NR_fsync:
case __NR_msync:
return Allow();
@ -1526,22 +1562,8 @@ class ContentSandboxPolicy : public SandboxPolicyCommon {
case __NR_get_mempolicy:
return Allow();
// Mesa's amdgpu driver uses kcmp with KCMP_FILE; see also bug
// 1624743. The pid restriction should be sufficient on its
// own if we need to remove the type restriction in the future.
case __NR_kcmp: {
// The real KCMP_FILE is part of an anonymous enum in
// <linux/kcmp.h>, but we can't depend on having that header,
// and it's not a #define so the usual #ifndef approach
// doesn't work.
static const int kKcmpFile = 0;
const pid_t myPid = getpid();
Arg<pid_t> pid1(0), pid2(1);
Arg<int> type(2);
return If(AllOf(pid1 == myPid, pid2 == myPid, type == kKcmpFile),
Allow())
.Else(InvalidSyscall());
}
case __NR_kcmp:
return KcmpPolicyForMesa();
#endif // DESKTOP
@ -1667,13 +1689,6 @@ class GMPSandboxPolicy : public SandboxPolicyCommon {
return Trap(OpenTrap, mFiles);
case __NR_brk:
// Because Firefox on glibc resorts to the fallback implementation
// mentioned in bug 1576006, we must explicitly allow the get*id()
// functions in order to use NSS in the clearkey CDM.
CASES_FOR_getuid:
CASES_FOR_getgid:
CASES_FOR_geteuid:
CASES_FOR_getegid:
return Allow();
case __NR_sched_get_priority_min:
case __NR_sched_get_priority_max:
@ -1739,12 +1754,7 @@ class RDDSandboxPolicy final : public SandboxPolicyCommon {
switch (sysno) {
case __NR_getrusage:
return Allow();
case __NR_ioctl: {
Arg<unsigned long> request(1);
// ffmpeg, and anything else that calls isatty(), will be told
// that nothing is a typewriter:
return If(request == TCGETS, Error(ENOTTY)).Else(InvalidSyscall());
}
// Pass through the common policy.
default:
return SandboxPolicyCommon::EvaluateSyscall(sysno);
@ -1839,9 +1849,6 @@ class SocketProcessSandboxPolicy final : public SandboxPolicyCommon {
.ElseIf(request == FIONBIO, Allow())
// This is used by PR_Available in nsSocketInputStream::Available.
.ElseIf(request == FIONREAD, Allow())
// ffmpeg, and anything else that calls isatty(), will be told
// that nothing is a typewriter:
.ElseIf(request == TCGETS, Error(ENOTTY))
// 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())
@ -1885,12 +1892,6 @@ class SocketProcessSandboxPolicy final : public SandboxPolicyCommon {
}
#endif // DESKTOP
CASES_FOR_getuid:
CASES_FOR_getgid:
CASES_FOR_geteuid:
CASES_FOR_getegid:
return Allow();
// Bug 1640612
case __NR_uname:
return Allow();