diff options
| author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-07-16 11:45:35 +0200 |
|---|---|---|
| committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-07-17 08:59:23 +0000 |
| commit | 552906b0f222c5d5dd11b9fd73829d510980461a (patch) | |
| tree | 3a11e6ed0538a81dd83b20cf3a4783e297f26d91 /chromium/sandbox | |
| parent | 1b05827804eaf047779b597718c03e7d38344261 (diff) | |
| download | qtwebengine-chromium-552906b0f222c5d5dd11b9fd73829d510980461a.tar.gz | |
BASELINE: Update Chromium to 83.0.4103.122
Change-Id: Ie3a82f5bb0076eec2a7c6a6162326b4301ee291e
Reviewed-by: Michael Brüning <michael.bruning@qt.io>
Diffstat (limited to 'chromium/sandbox')
43 files changed, 490 insertions, 204 deletions
diff --git a/chromium/sandbox/BUILD.gn b/chromium/sandbox/BUILD.gn index 81e7aff037d..9de38b4acd5 100644 --- a/chromium/sandbox/BUILD.gn +++ b/chromium/sandbox/BUILD.gn @@ -9,23 +9,17 @@ import("//testing/libfuzzer/fuzzer_test.gni") # Several targets want to include this header file. We separate it out # here so multiple targets can depend on it. source_set("sandbox_export") { - sources = [ - "sandbox_export.h", - ] + sources = [ "sandbox_export.h" ] } source_set("common") { - sources = [ - "constants.h", - ] + sources = [ "constants.h" ] } # Meta-target that forwards to the proper platform one. group("sandbox") { if (is_win) { - public_deps = [ - "//sandbox/win:sandbox", - ] + public_deps = [ "//sandbox/win:sandbox" ] } else if (is_mac) { public_deps = [ "//sandbox/mac:seatbelt", @@ -34,9 +28,7 @@ group("sandbox") { "//sandbox/mac/mojom", ] } else if (is_linux || is_android) { - public_deps = [ - "//sandbox/linux:sandbox", - ] + public_deps = [ "//sandbox/linux:sandbox" ] } } @@ -61,9 +53,7 @@ fuzzer_test("sandbox_ipc_fuzzer") { if (!is_win) { defines = [ "SANDBOX_FUZZ_TARGET" ] } - deps = [ - "//base", - ] + deps = [ "//base" ] dict = "ipc.dict" libfuzzer_options = [ "max_len=1024" ] } diff --git a/chromium/sandbox/features.gni b/chromium/sandbox/features.gni index 89693c54c4f..09280d35f6a 100644 --- a/chromium/sandbox/features.gni +++ b/chromium/sandbox/features.gni @@ -8,10 +8,9 @@ import("//build/config/nacl/config.gni") # currently. # Do not disable seccomp_bpf anywhere without talking to # security@chromium.org! -use_seccomp_bpf = - (is_linux || is_android) && - (current_cpu == "x86" || current_cpu == "x64" || current_cpu == "arm" || - current_cpu == "arm64" || current_cpu == "mipsel" || - current_cpu == "mips64el") +use_seccomp_bpf = (is_linux || is_android) && + (current_cpu == "x86" || current_cpu == "x64" || + current_cpu == "arm" || current_cpu == "arm64" || + current_cpu == "mipsel" || current_cpu == "mips64el") use_seccomp_bpf = use_seccomp_bpf || is_nacl_nonsfi diff --git a/chromium/sandbox/linux/BUILD.gn b/chromium/sandbox/linux/BUILD.gn index b00a88cfa35..c27351f9a6a 100644 --- a/chromium/sandbox/linux/BUILD.gn +++ b/chromium/sandbox/linux/BUILD.gn @@ -34,9 +34,7 @@ if (is_nacl_nonsfi) { # the setuid sandbox and is its own target. group("sandbox") { - public_deps = [ - ":sandbox_services", - ] + public_deps = [ ":sandbox_services" ] if (compile_suid_client || is_nacl_nonsfi) { public_deps += [ ":suid_sandbox_client" ] } @@ -59,9 +57,7 @@ source_set("sandbox_linux_test_utils") { "tests/unit_tests.h", ] - deps = [ - "//testing/gtest", - ] + deps = [ "//testing/gtest" ] if (!is_nacl_nonsfi) { sources += [ @@ -191,17 +187,14 @@ action("bpf_dsl_golden") { "bpf_dsl/golden/x86-64/NegativeConstantsPolicy.txt", "bpf_dsl/golden/x86-64/SwitchPolicy.txt", ] - outputs = [ - "$target_gen_dir/bpf_dsl/golden/golden_files.h", - ] + outputs = [ "$target_gen_dir/bpf_dsl/golden/golden_files.h" ] args = rebase_path(outputs, root_build_dir) + rebase_path(inputs, root_build_dir) } test("sandbox_linux_unittests") { - deps = [ - ":sandbox_linux_unittests_sources", - ] + deps = [ ":sandbox_linux_unittests_sources" ] + data_deps = [ "//testing/buildbot/filters:sandbox_linux_unittests_filters" ] if (is_android) { use_raw_android_executable = true } @@ -358,9 +351,7 @@ component("sandbox_services") { defines = [ "SANDBOX_IMPLEMENTATION" ] - public_deps = [ - "//sandbox:sandbox_export", - ] + public_deps = [ "//sandbox:sandbox_export" ] deps = [ "//base", "//base/third_party/dynamic_annotations", @@ -445,9 +436,7 @@ if (compile_suid_client || is_nacl_nonsfi) { "suid/common/suid_unsafe_environment_variables.h", ] defines = [ "SANDBOX_IMPLEMENTATION" ] - public_deps = [ - "//sandbox:sandbox_export", - ] + public_deps = [ "//sandbox:sandbox_export" ] deps = [ ":sandbox_services", "//base", diff --git a/chromium/sandbox/linux/OWNERS b/chromium/sandbox/linux/OWNERS index 7ea24db8239..e9a367b47e1 100644 --- a/chromium/sandbox/linux/OWNERS +++ b/chromium/sandbox/linux/OWNERS @@ -1,4 +1,5 @@ jorgelo@chromium.org +mpdenton@chromium.org palmer@chromium.org rsesek@chromium.org diff --git a/chromium/sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc b/chromium/sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc index b4b64d6aa8c..d30e15560a4 100644 --- a/chromium/sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc +++ b/chromium/sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc @@ -1915,17 +1915,19 @@ BPF_TEST_C(SandboxBPF, PthreadBitMask, PthreadPolicyBitMask) { // // Depending on the architecture, this may modify regs, so the caller is // responsible for committing these changes using PTRACE_SETREGS. +#if !defined(__arm__) && !defined(__aarch64__) && !defined(__mips__) long SetSyscall(pid_t pid, regs_struct* regs, int syscall_number) { #if defined(__arm__) // On ARM, the syscall is changed using PTRACE_SET_SYSCALL. We cannot use the // libc ptrace call as the request parameter is an enum, and // PTRACE_SET_SYSCALL may not be in the enum. return syscall(__NR_ptrace, PTRACE_SET_SYSCALL, pid, NULL, syscall_number); -#endif - +#else SECCOMP_PT_SYSCALL(*regs) = syscall_number; return 0; +#endif } +#endif const uint16_t kTraceData = 0xcc; @@ -1952,16 +1954,11 @@ SANDBOX_TEST(SandboxBPF, DISABLE_ON_TSAN(SeccompRetTrace)) { // See https://code.google.com/p/chromium/issues/detail?id=383977 #if defined(__arm__) || defined(__aarch64__) printf("This test is currently disabled on ARM32/64 due to a kernel bug."); - return; -#endif - -#if defined(__mips__) +#elif defined(__mips__) // TODO: Figure out how to support specificity of handling indirect syscalls // in this test and enable it. printf("This test is currently disabled on MIPS."); - return; -#endif - +#else pid_t pid = fork(); BPF_ASSERT_NE(-1, pid); if (pid == 0) { @@ -2040,6 +2037,7 @@ SANDBOX_TEST(SandboxBPF, DISABLE_ON_TSAN(SeccompRetTrace)) { BPF_ASSERT_NE(-1, ptrace(PTRACE_CONT, pid, NULL, NULL)); } +#endif } // Android does not expose pread64 nor pwrite64. diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc index 768025ce192..712f9699a94 100644 --- a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc +++ b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc @@ -11,7 +11,7 @@ #include <sys/types.h> #include <unistd.h> -#include "base/clang_coverage_buildflags.h" +#include "base/clang_profiling_buildflags.h" #include "base/logging.h" #include "build/build_config.h" #include "sandbox/linux/bpf_dsl/bpf_dsl.h" @@ -128,7 +128,7 @@ ResultExpr EvaluateSyscallImpl(int fs_denied_errno, #endif // defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER) || // defined(MEMORY_SANITIZER) -#if BUILDFLAG(CLANG_COVERAGE) +#if BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) if (SyscallSets::IsPrctl(sysno)) { return Allow(); } diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc index c0a51dad101..fc36187c945 100644 --- a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc +++ b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc @@ -24,7 +24,7 @@ #include <time.h> #include <unistd.h> -#include "base/clang_coverage_buildflags.h" +#include "base/clang_profiling_buildflags.h" #include "base/files/scoped_file.h" #include "base/macros.h" #include "base/posix/eintr_wrapper.h" @@ -345,7 +345,7 @@ BPF_TEST_C(BaselinePolicy, PrctlDumpable, BaselinePolicy) { #define PR_CAPBSET_READ 23 #endif -#if !BUILDFLAG(CLANG_COVERAGE) +#if !BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) BPF_DEATH_TEST_C(BaselinePolicy, PrctlSigsys, DEATH_SEGV_MESSAGE(GetPrctlErrorMessageContentForTests()), diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc index d9789a7133e..ff5a1c093c7 100644 --- a/chromium/sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc +++ b/chromium/sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc @@ -405,6 +405,15 @@ ResultExpr RestrictPrlimit(pid_t target_pid) { return If(AnyOf(pid == 0, pid == target_pid), Allow()).Else(Error(EPERM)); } +ResultExpr RestrictPrlimitToGetrlimit(pid_t target_pid) { + const Arg<pid_t> pid(0); + const Arg<uintptr_t> new_limit(2); + // Only allow operations for the current process, and only with |new_limit| + // set to null. + return If(AllOf(new_limit == 0, AnyOf(pid == 0, pid == target_pid)), Allow()) + .Else(Error(EPERM)); +} + #if !defined(OS_NACL_NONSFI) ResultExpr RestrictPtrace() { const Arg<int> request(0); diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.h b/chromium/sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.h index 15442892bcb..ba4289f05be 100644 --- a/chromium/sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.h +++ b/chromium/sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.h @@ -104,6 +104,11 @@ SANDBOX_EXPORT bpf_dsl::ResultExpr RestrictGetRandom(); // gracefully; see crbug.com/160157. SANDBOX_EXPORT bpf_dsl::ResultExpr RestrictPrlimit(pid_t target_pid); +// Restrict |pid| to the calling process (or 0) for prlimit64(), and require the +// |new_limit_ argument to be null. This allows only getting limits on the +// current process. Otherwise fail gracefully. +SANDBOX_EXPORT bpf_dsl::ResultExpr RestrictPrlimitToGetrlimit(pid_t target_pid); + // Restrict ptrace() to just read operations that are needed for crash // reporting. See https://crbug.com/933418 for details. SANDBOX_EXPORT bpf_dsl::ResultExpr RestrictPtrace(); diff --git a/chromium/sandbox/linux/seccomp-bpf/syscall_unittest.cc b/chromium/sandbox/linux/seccomp-bpf/syscall_unittest.cc index 2b776d287b9..40cc9548306 100644 --- a/chromium/sandbox/linux/seccomp-bpf/syscall_unittest.cc +++ b/chromium/sandbox/linux/seccomp-bpf/syscall_unittest.cc @@ -226,6 +226,8 @@ TEST(Syscall, ComplexSyscallSixArgs) { kPageSize #endif ))); +#if !defined(MEMORY_SANITIZER) + // MSan considers the memory backing addr2 uninitialized. EXPECT_EQ(0, memcmp(addr2 + kPageSize, addr3, kPageSize)); // Just to be absolutely on the safe side, also verify that the file @@ -237,6 +239,7 @@ TEST(Syscall, ComplexSyscallSixArgs) { 2 * kPageSize ))); EXPECT_EQ(0, memcmp(addr2, buf, 2 * kPageSize)); +#endif // Clean up EXPECT_EQ(0, Syscall::Call(__NR_munmap, addr2, 2 * kPageSize)); diff --git a/chromium/sandbox/linux/seccomp-bpf/trap.cc b/chromium/sandbox/linux/seccomp-bpf/trap.cc index 003708d2c89..9884be8bb2c 100644 --- a/chromium/sandbox/linux/seccomp-bpf/trap.cc +++ b/chromium/sandbox/linux/seccomp-bpf/trap.cc @@ -164,11 +164,18 @@ void Trap::SigSys(int nr, LinuxSigInfo* info, ucontext_t* ctx) { } - // Obtain the siginfo information that is specific to SIGSYS. Unfortunately, - // most versions of glibc don't include this information in siginfo_t. So, - // we need to explicitly copy it into a arch_sigsys structure. + // Obtain the siginfo information that is specific to SIGSYS. struct arch_sigsys sigsys; +#if defined(si_call_addr) && !defined(__native_client_nonsfi__) + sigsys.ip = info->si_call_addr; + sigsys.nr = info->si_syscall; + sigsys.arch = info->si_arch; +#else + // If the version of glibc doesn't include this information in + // siginfo_t (older than 2.17), we need to explicitly copy it + // into an arch_sigsys structure. memcpy(&sigsys, &info->_sifields, sizeof(sigsys)); +#endif #if defined(__mips__) // When indirect syscall (syscall(__NR_foo, ...)) is made on Mips, the diff --git a/chromium/sandbox/linux/services/credentials.cc b/chromium/sandbox/linux/services/credentials.cc index 542567f3ee5..d7b5d8c4413 100644 --- a/chromium/sandbox/linux/services/credentials.cc +++ b/chromium/sandbox/linux/services/credentials.cc @@ -37,7 +37,9 @@ namespace sandbox { namespace { const int kExitSuccess = 0; +#if !defined(THREAD_SANITIZER) const int kExitFailure = 1; +#endif #if defined(__clang__) // Disable sanitizers that rely on TLS and may write to non-stack memory. @@ -260,8 +262,7 @@ bool Credentials::CanCreateProcessInNewUserNS() { // With TSAN, processes will always have threads running and can never // enter a new user namespace with MoveToNewUserNS(). return false; -#endif - +#else uid_t uid; gid_t gid; if (!GetRESIds(&uid, &gid)) { @@ -306,6 +307,7 @@ bool Credentials::CanCreateProcessInNewUserNS() { // clone(2) succeeded. Now return true only if the system grants // unprivileged use of CLONE_NEWUSER as well. return WIFEXITED(status) && WEXITSTATUS(status) == kExitSuccess; +#endif } bool Credentials::MoveToNewUserNS() { diff --git a/chromium/sandbox/linux/services/libc_interceptor.cc b/chromium/sandbox/linux/services/libc_interceptor.cc index cff3ea4a3bb..2f0df72c71c 100644 --- a/chromium/sandbox/linux/services/libc_interceptor.cc +++ b/chromium/sandbox/linux/services/libc_interceptor.cc @@ -37,7 +37,6 @@ namespace { // they fork. (This means that it'll be incorrect for global constructor // functions and before ZygoteMain is called - beware). bool g_am_zygote_or_renderer = false; -bool g_use_localtime_override = true; int g_backchannel_fd = -1; base::LazyInstance<std::set<std::string>>::Leaky g_timezones = @@ -114,7 +113,7 @@ void WriteTimeStruct(base::Pickle* pickle, const struct tm& time) { } // See -// https://chromium.googlesource.com/chromium/src/+/master/docs/linux_zygote.md +// https://chromium.googlesource.com/chromium/src/+/master/docs/linux/zygote.md void ProxyLocaltimeCallToBrowser(time_t input, struct tm* output, char* timezone_out, @@ -222,7 +221,7 @@ __attribute__((__visibility__("default"))) struct tm* localtime_override( NO_SANITIZE("cfi-icall") __attribute__((__visibility__("default"))) struct tm* localtime_override( const time_t* timep) { - if (g_am_zygote_or_renderer && g_use_localtime_override) { + if (g_am_zygote_or_renderer) { static struct tm time_struct; static char timezone_string[64]; ProxyLocaltimeCallToBrowser(*timep, &time_struct, timezone_string, @@ -248,7 +247,7 @@ __attribute__((__visibility__("default"))) struct tm* localtime64_override( NO_SANITIZE("cfi-icall") __attribute__((__visibility__("default"))) struct tm* localtime64_override( const time_t* timep) { - if (g_am_zygote_or_renderer && g_use_localtime_override) { + if (g_am_zygote_or_renderer) { static struct tm time_struct; static char timezone_string[64]; ProxyLocaltimeCallToBrowser(*timep, &time_struct, timezone_string, @@ -275,7 +274,7 @@ NO_SANITIZE("cfi-icall") __attribute__((__visibility__("default"))) struct tm* localtime_r_override( const time_t* timep, struct tm* result) { - if (g_am_zygote_or_renderer && g_use_localtime_override) { + if (g_am_zygote_or_renderer) { ProxyLocaltimeCallToBrowser(*timep, result, nullptr, 0); return result; } @@ -299,7 +298,7 @@ NO_SANITIZE("cfi-icall") __attribute__((__visibility__("default"))) struct tm* localtime64_r_override( const time_t* timep, struct tm* result) { - if (g_am_zygote_or_renderer && g_use_localtime_override) { + if (g_am_zygote_or_renderer) { ProxyLocaltimeCallToBrowser(*timep, result, nullptr, 0); return result; } @@ -315,10 +314,6 @@ __attribute__((__visibility__("default"))) struct tm* localtime64_r_override( return res; } -void SetUseLocaltimeOverride(bool enable) { - g_use_localtime_override = enable; -} - void SetAmZygoteOrRenderer(bool enable, int backchannel_fd) { g_am_zygote_or_renderer = enable; g_backchannel_fd = backchannel_fd; diff --git a/chromium/sandbox/linux/services/libc_interceptor.h b/chromium/sandbox/linux/services/libc_interceptor.h index be020c20179..a43460e11ab 100644 --- a/chromium/sandbox/linux/services/libc_interceptor.h +++ b/chromium/sandbox/linux/services/libc_interceptor.h @@ -32,11 +32,10 @@ namespace sandbox { // // Our replacement functions must handle both cases, and either proxy the call // to the parent over the IPC back-channel (see -// https://chromium.googlesource.com/chromium/src/+/master/docs/linux_sandbox_ipc.md) +// https://chromium.googlesource.com/chromium/src/+/master/docs/linux/sandbox_ipc.md) // or use dlsym with RTLD_NEXT to resolve the symbol, ignoring any symbols in -// the current module. Use SetUseLocaltimeOverride() and SetAmZygoteOrRenderer() -// below to control the mode of operation, which defaults using the dlsym -// approach. +// the current module. Use SetAmZygoteOrRenderer() below to control the mode of +// operation, which defaults using the dlsym approach. // // Other avenues: // @@ -61,11 +60,6 @@ SANDBOX_EXPORT bool HandleInterceptedCall( base::PickleIterator iter, const std::vector<base::ScopedFD>& fds); -// On Linux, localtime is overridden to use a synchronous IPC to the browser -// process to determine the locale. This can be disabled, which causes -// localtime to use UTC instead. https://crbug.com/772503. -SANDBOX_EXPORT void SetUseLocaltimeOverride(bool enable); - // Turns on/off the libc interception. Called by the zygote and inherited by it // children. |backchannel_fd| must be the fd to use for proxying calls. SANDBOX_EXPORT void SetAmZygoteOrRenderer(bool enable, int backchannel_fd); diff --git a/chromium/sandbox/linux/suid/client/setuid_sandbox_host.cc b/chromium/sandbox/linux/suid/client/setuid_sandbox_host.cc index a277ffa5eb5..0aaed76c1dd 100644 --- a/chromium/sandbox/linux/suid/client/setuid_sandbox_host.cc +++ b/chromium/sandbox/linux/suid/client/setuid_sandbox_host.cc @@ -128,7 +128,7 @@ base::FilePath SetuidSandboxHost::GetSandboxBinaryPath() { // In user-managed builds, including development builds, an environment // variable is required to enable the sandbox. See - // https://chromium.googlesource.com/chromium/src/+/master/docs/linux_suid_sandbox_development.md + // https://chromium.googlesource.com/chromium/src/+/master/docs/linux/suid_sandbox_development.md struct stat st; if (sandbox_binary.empty() && stat(base::kProcSelfExe, &st) == 0 && st.st_uid == getuid()) { @@ -146,10 +146,11 @@ void SetuidSandboxHost::PrependWrapper(base::CommandLine* cmd_line) { struct stat st; if (sandbox_binary.empty() || stat(sandbox_binary.c_str(), &st) != 0) { LOG(FATAL) << "The SUID sandbox helper binary is missing: " - << sandbox_binary << " Aborting now. See " - "https://chromium.googlesource.com/" - "chromium/src/+/master/docs/" - "linux_suid_sandbox_development.md."; + << sandbox_binary + << " Aborting now. See " + "https://chromium.googlesource.com/" + "chromium/src/+/master/docs/" + "linux/suid_sandbox_development.md."; } if (access(sandbox_binary.c_str(), X_OK) != 0 || (st.st_uid != 0) || diff --git a/chromium/sandbox/linux/suid/sandbox.c b/chromium/sandbox/linux/suid/sandbox.c index 854819bfbb4..5fdb4817af8 100644 --- a/chromium/sandbox/linux/suid/sandbox.c +++ b/chromium/sandbox/linux/suid/sandbox.c @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// https://chromium.googlesource.com/chromium/src/+/master/docs/linux_suid_sandbox.md +// https://chromium.googlesource.com/chromium/src/+/master/docs/linux/suid_sandbox.md #include "sandbox/linux/suid/common/sandbox.h" @@ -403,7 +403,7 @@ bool CheckAndExportApiVersion() { "The setuid sandbox provides API version %d, " "but you need %d\n" "Please read " - "https://chromium.googlesource.com/chromium/src/+/master/docs/linux_suid_sandbox_development.md." + "https://chromium.googlesource.com/chromium/src/+/master/docs/linux/suid_sandbox_development.md." "\n\n", kSUIDSandboxApiNumber, api_number); diff --git a/chromium/sandbox/linux/syscall_broker/broker_process.cc b/chromium/sandbox/linux/syscall_broker/broker_process.cc index 19b825a150d..8321d23798d 100644 --- a/chromium/sandbox/linux/syscall_broker/broker_process.cc +++ b/chromium/sandbox/linux/syscall_broker/broker_process.cc @@ -73,6 +73,9 @@ bool BrokerProcess::Init( return false; if (child_pid) { + // This string is referenced in a ChromeOS integration test; do not change. + // TODO(crbug.com/1044502): If we can fix setproctitle, the integration + // test not longer needs to look for this message. VLOG(3) << "BrokerProcess::Init(), in parent, child is " << child_pid; // We are the parent and we have just forked our broker process. ipc_reader.reset(); @@ -84,9 +87,13 @@ bool BrokerProcess::Init( return true; } + // This string is referenced in a ChromeOS integration test; do not change. + // TODO(crbug.com/1044502): If we can fix setproctitle, the integration test + // not longer needs to look for this message. + VLOG(3) << "BrokerProcess::Init(), in child"; + // We are the broker process. Make sure to close the writer's end so that // we get notified if the client disappears. - VLOG(3) << "BrokerProcess::Init(), in child"; ipc_writer.reset(); CHECK(std::move(broker_process_init_callback).Run()); BrokerHost broker_host(broker_permission_list_, allowed_command_set_, @@ -100,9 +107,6 @@ bool BrokerProcess::Init( continue; } } - _exit(1); - NOTREACHED(); - return false; } bool BrokerProcess::IsSyscallAllowed(int sysno) const { diff --git a/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc b/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc index a4fc812e71a..e1144da6e78 100644 --- a/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc +++ b/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc @@ -64,7 +64,7 @@ TEST(BrokerProcess, CreateAndDestroy) { BrokerFilePermission::ReadOnly("/proc/cpuinfo")}; BrokerProcess open_broker(kFakeErrnoSentinel, BrokerCommandSet(), permissions); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); ASSERT_TRUE(TestUtils::CurrentProcessHasChildren()); } // Destroy the broker and check it has exited properly. @@ -77,7 +77,7 @@ TEST(BrokerProcess, TestOpenAccessNull) { std::vector<BrokerFilePermission> empty; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, empty); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); int fd = open_broker.Open(NULL, O_RDONLY); ASSERT_EQ(fd, -EFAULT); @@ -105,7 +105,7 @@ void TestOpenFilePerms(bool fast_check_in_client, int denied_errno) { BrokerFilePermission::ReadWrite(kRW_WhiteListed)}; BrokerProcess open_broker(denied_errno, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); int fd = -1; fd = open_broker.Open(kR_WhiteListed, O_RDONLY); @@ -264,7 +264,7 @@ void TestBadPaths(bool fast_check_in_client) { BrokerFilePermission::ReadOnlyRecursive("/proc/")}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); // Open cpuinfo via the broker. int cpuinfo_fd = open_broker.Open(kFileCpuInfo, O_RDONLY); @@ -327,7 +327,7 @@ void TestOpenCpuinfo(bool fast_check_in_client, bool recursive) { BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); int fd = open_broker.Open(kFileCpuInfo, O_RDWR); ASSERT_EQ(fd, -kFakeErrnoSentinel); @@ -405,7 +405,7 @@ TEST(BrokerProcess, OpenFileRW) { std::vector<BrokerFilePermission> permissions = { BrokerFilePermission::ReadWrite(tempfile_name)}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); // Check we can access that file with read or write. int can_access = open_broker.Access(tempfile_name, R_OK | W_OK); @@ -444,7 +444,7 @@ SANDBOX_TEST(BrokerProcess, BrokerDied) { BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, true /* fast_check_in_client */, true /* quiet_failures_for_tests */); - SANDBOX_ASSERT(open_broker.Init(base::BindRepeating(&NoOpCallback))); + SANDBOX_ASSERT(open_broker.Init(base::BindOnce(&NoOpCallback))); const pid_t broker_pid = open_broker.broker_pid(); SANDBOX_ASSERT(kill(broker_pid, SIGKILL) == 0); @@ -472,7 +472,7 @@ void TestOpenComplexFlags(bool fast_check_in_client) { BrokerFilePermission::ReadOnly(kCpuInfo)}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); // Test that we do the right thing for O_CLOEXEC and O_NONBLOCK. int fd = -1; @@ -562,7 +562,7 @@ SANDBOX_TEST_ALLOW_NOISE(BrokerProcess, MAYBE_RecvMsgDescriptorLeak) { std::vector<BrokerFilePermission> permissions = { BrokerFilePermission::ReadOnly(kCpuInfo)}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions); - SANDBOX_ASSERT(open_broker.Init(base::BindRepeating(&NoOpCallback))); + SANDBOX_ASSERT(open_broker.Init(base::BindOnce(&NoOpCallback))); const int ipc_fd = BrokerProcessTestHelper::GetIPCDescriptor(&open_broker); SANDBOX_ASSERT(ipc_fd >= 0); @@ -615,7 +615,7 @@ TEST(BrokerProcess, BrokerDiesOnClosedChannel) { BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, true /* fast_check_in_client */, false /* quiet_failures_for_tests */); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&CloseFD, lifeline_fds[0]))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&CloseFD, lifeline_fds[0]))); // Make sure the writing end only exists in the broker process. CloseFD(lifeline_fds[1]); @@ -661,7 +661,7 @@ TEST(BrokerProcess, CreateFile) { }; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); int fd = -1; @@ -756,7 +756,7 @@ void TestStatHelper(bool fast_check_in_client, bool follow_links) { BrokerFilePermission::ReadOnly(tempfile_name)}; BrokerProcess open_broker(kFakeErrnoSentinel, BrokerCommandSet(), permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); memset(&sb, 0, sizeof(sb)); EXPECT_EQ(-kFakeErrnoSentinel, @@ -771,7 +771,7 @@ void TestStatHelper(bool fast_check_in_client, bool follow_links) { std::vector<BrokerFilePermission> permissions; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); memset(&sb, 0, sizeof(sb)); EXPECT_EQ(-kFakeErrnoSentinel, @@ -782,7 +782,7 @@ void TestStatHelper(bool fast_check_in_client, bool follow_links) { std::vector<BrokerFilePermission> permissions; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); memset(&sb, 0, sizeof(sb)); EXPECT_EQ(-kFakeErrnoSentinel, @@ -794,7 +794,7 @@ void TestStatHelper(bool fast_check_in_client, bool follow_links) { BrokerFilePermission::ReadOnly(nonesuch_name)}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); memset(&sb, 0, sizeof(sb)); EXPECT_EQ(-ENOENT, open_broker.Stat(nonesuch_name, follow_links, &sb)); @@ -827,7 +827,7 @@ void TestStatHelper(bool fast_check_in_client, bool follow_links) { BrokerFilePermission::ReadWriteCreate(nonesuch_name)}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); memset(&sb, 0, sizeof(sb)); EXPECT_EQ(-ENOENT, open_broker.Stat(nonesuch_name, follow_links, &sb)); @@ -859,7 +859,7 @@ void TestStatHelper(bool fast_check_in_client, bool follow_links) { BrokerFilePermission::ReadOnly(tempfile_name)}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); memset(&sb, 0, sizeof(sb)); EXPECT_EQ(0, open_broker.Stat(tempfile_name, follow_links, &sb)); @@ -924,7 +924,7 @@ void TestRenameHelper(bool fast_check_in_client) { // itself is not allowed. BrokerProcess open_broker(kFakeErrnoSentinel, BrokerCommandSet(), rwc_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Rename(oldpath.c_str(), newpath.c_str())); @@ -941,7 +941,7 @@ void TestRenameHelper(bool fast_check_in_client) { BrokerFilePermission::ReadWriteCreate(oldpath)}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Rename(oldpath.c_str(), newpath.c_str())); @@ -955,7 +955,7 @@ void TestRenameHelper(bool fast_check_in_client) { BrokerFilePermission::ReadWriteCreate(newpath)}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Rename(oldpath.c_str(), newpath.c_str())); @@ -970,7 +970,7 @@ void TestRenameHelper(bool fast_check_in_client) { BrokerFilePermission::ReadWriteCreate(newpath)}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Rename(oldpath.c_str(), newpath.c_str())); @@ -985,7 +985,7 @@ void TestRenameHelper(bool fast_check_in_client) { BrokerFilePermission::ReadOnly(newpath)}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Rename(oldpath.c_str(), newpath.c_str())); @@ -997,7 +997,7 @@ void TestRenameHelper(bool fast_check_in_client) { // Check rename passes with write permissions to both files. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, rwc_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(0, open_broker.Rename(oldpath.c_str(), newpath.c_str())); // ... and files were moved around. @@ -1041,7 +1041,7 @@ void TestReadlinkHelper(bool fast_check_in_client) { BrokerFilePermission::ReadOnly(newpath_name)}; BrokerProcess open_broker(kFakeErrnoSentinel, BrokerCommandSet(), permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Readlink(newpath_name, buf, sizeof(buf))); } @@ -1054,7 +1054,7 @@ void TestReadlinkHelper(bool fast_check_in_client) { std::vector<BrokerFilePermission> permissions; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Readlink(nonesuch_name, buf, sizeof(buf))); } @@ -1063,7 +1063,7 @@ void TestReadlinkHelper(bool fast_check_in_client) { std::vector<BrokerFilePermission> permissions; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Readlink(newpath_name, buf, sizeof(buf))); } @@ -1073,7 +1073,7 @@ void TestReadlinkHelper(bool fast_check_in_client) { BrokerFilePermission::ReadOnly(nonesuch_name)}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-ENOENT, open_broker.Readlink(nonesuch_name, buf, sizeof(buf))); } { @@ -1082,7 +1082,7 @@ void TestReadlinkHelper(bool fast_check_in_client) { BrokerFilePermission::ReadOnly(newpath_name)}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); ssize_t retlen = open_broker.Readlink(newpath_name, buf, sizeof(buf)); EXPECT_TRUE(retlen == static_cast<ssize_t>(strlen(oldpath_name))); EXPECT_EQ(0, memcmp(oldpath_name, buf, retlen)); @@ -1093,7 +1093,7 @@ void TestReadlinkHelper(bool fast_check_in_client) { BrokerFilePermission::ReadOnly(newpath_name)}; BrokerProcess open_broker(kFakeErrnoSentinel, command_set, permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-ENAMETOOLONG, open_broker.Readlink(newpath_name, buf, 4)); } @@ -1137,7 +1137,7 @@ void TestMkdirHelper(bool fast_check_in_client) { // Actual file with permissions to use but command itself not allowed. BrokerProcess open_broker(kFakeErrnoSentinel, BrokerCommandSet(), rw_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Mkdir(path_name, 0600)); } @@ -1147,56 +1147,56 @@ void TestMkdirHelper(bool fast_check_in_client) { // Nonexistent file with no permissions to see file. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, no_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Mkdir(nonesuch_name, 0600)); } { // Actual file with no permissions to see file. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, no_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Mkdir(path_name, 0600)); } { // Nonexistent file with insufficient permissions to see file. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, ro_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Mkdir(nonesuch_name, 0600)); } { // Actual file with insufficient permissions to see file. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, ro_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Mkdir(path_name, 0600)); } { // Nonexistent file with insufficient permissions to see file, case 2. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, rw_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Mkdir(nonesuch_name, 0600)); } { // Actual file with insufficient permissions to see file, case 2. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, rw_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Mkdir(path_name, 0600)); } { // Nonexistent file with permissions to see file. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, rwc_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-ENOENT, open_broker.Mkdir(nonesuch_name, 0600)); } { // Actual file with permissions to see file. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, rwc_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(0, open_broker.Mkdir(path_name, 0600)); } @@ -1242,7 +1242,7 @@ void TestRmdirHelper(bool fast_check_in_client) { // Actual dir with permissions to use but command itself not allowed. BrokerProcess open_broker(kFakeErrnoSentinel, BrokerCommandSet(), rw_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Rmdir(path_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1253,7 +1253,7 @@ void TestRmdirHelper(bool fast_check_in_client) { // Nonexistent dir with no permissions to see dir. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, no_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Rmdir(nonesuch_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1262,7 +1262,7 @@ void TestRmdirHelper(bool fast_check_in_client) { // Actual dir with no permissions to see dir. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, no_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Rmdir(path_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1271,7 +1271,7 @@ void TestRmdirHelper(bool fast_check_in_client) { // Nonexistent dir with insufficient permissions to see dir. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, ro_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Rmdir(nonesuch_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1280,7 +1280,7 @@ void TestRmdirHelper(bool fast_check_in_client) { // Actual dir with insufficient permissions to see dir. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, ro_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Rmdir(path_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1289,7 +1289,7 @@ void TestRmdirHelper(bool fast_check_in_client) { // Nonexistent dir with insufficient permissions to see dir, case 2. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, rw_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Rmdir(nonesuch_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1298,7 +1298,7 @@ void TestRmdirHelper(bool fast_check_in_client) { // Actual dir with insufficient permissions to see dir, case 2. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, rw_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Rmdir(path_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1307,7 +1307,7 @@ void TestRmdirHelper(bool fast_check_in_client) { // Nonexistent dir with permissions to see dir. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, rwc_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_TRUE(open_broker.Rmdir(nonesuch_name) < 0); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1316,7 +1316,7 @@ void TestRmdirHelper(bool fast_check_in_client) { // Actual dir with permissions to see dir. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, rwc_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(0, open_broker.Rmdir(path_name)); } // Confirm it was erased. @@ -1363,7 +1363,7 @@ void TestUnlinkHelper(bool fast_check_in_client) { // Actual file with permissions to use but command itself not allowed. BrokerProcess open_broker(kFakeErrnoSentinel, BrokerCommandSet(), rwc_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Unlink(path_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1374,7 +1374,7 @@ void TestUnlinkHelper(bool fast_check_in_client) { // Nonexistent file with no permissions to see file. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, no_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Unlink(nonesuch_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1383,7 +1383,7 @@ void TestUnlinkHelper(bool fast_check_in_client) { // Actual file with no permissions to see file. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, no_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Unlink(path_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1392,7 +1392,7 @@ void TestUnlinkHelper(bool fast_check_in_client) { // Nonexistent file with insufficient permissions to see file. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, ro_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Unlink(nonesuch_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1401,7 +1401,7 @@ void TestUnlinkHelper(bool fast_check_in_client) { // Actual file with insufficient permissions to see file. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, ro_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Unlink(path_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1410,7 +1410,7 @@ void TestUnlinkHelper(bool fast_check_in_client) { // Nonexistent file with insufficient permissions to see file, case 2. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, rw_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Unlink(nonesuch_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1419,7 +1419,7 @@ void TestUnlinkHelper(bool fast_check_in_client) { // Actual file with insufficient permissions to see file, case 2. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, rw_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(-kFakeErrnoSentinel, open_broker.Unlink(path_name)); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1428,7 +1428,7 @@ void TestUnlinkHelper(bool fast_check_in_client) { // Nonexistent file with permissions to see file. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, rwc_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_TRUE(open_broker.Unlink(nonesuch_name) < 0); } EXPECT_EQ(0, access(path_name, F_OK)); @@ -1437,7 +1437,7 @@ void TestUnlinkHelper(bool fast_check_in_client) { // Actual file with permissions to see file. BrokerProcess open_broker(kFakeErrnoSentinel, command_set, rwc_permissions, fast_check_in_client); - ASSERT_TRUE(open_broker.Init(base::BindRepeating(&NoOpCallback))); + ASSERT_TRUE(open_broker.Init(base::BindOnce(&NoOpCallback))); EXPECT_EQ(0, open_broker.Unlink(path_name)); } // Confirm it was erased. diff --git a/chromium/sandbox/linux/system_headers/x86_64_linux_syscalls.h b/chromium/sandbox/linux/system_headers/x86_64_linux_syscalls.h index 349504aee43..b0ae0a2edf6 100644 --- a/chromium/sandbox/linux/system_headers/x86_64_linux_syscalls.h +++ b/chromium/sandbox/linux/system_headers/x86_64_linux_syscalls.h @@ -1290,5 +1290,65 @@ #define __NR_memfd_create 319 #endif +#if !defined(__NR_kexec_file_load) +#define __NR_kexec_file_load 320 +#endif + +#if !defined(__NR_bpf) +#define __NR_bpf 321 +#endif + +#if !defined(__NR_execveat) +#define __NR_execveat 322 +#endif + +#if !defined(__NR_userfaultfd) +#define __NR_userfaultfd 323 +#endif + +#if !defined(__NR_membarrier) +#define __NR_membarrier 324 +#endif + +#if !defined(__NR_mlock2) +#define __NR_mlock2 325 +#endif + +#if !defined(__NR_copy_file_range) +#define __NR_copy_file_range 326 +#endif + +#if !defined(__NR_preadv2) +#define __NR_preadv2 327 +#endif + +#if !defined(__NR_pwritev2) +#define __NR_pwritev2 328 +#endif + +#if !defined(__NR_pkey_mprotect) +#define __NR_pkey_mprotect 329 +#endif + +#if !defined(__NR_pkey_alloc) +#define __NR_pkey_alloc 330 +#endif + +#if !defined(__NR_pkey_free) +#define __NR_pkey_free 331 +#endif + +#if !defined(__NR_statx) +#define __NR_statx 332 +#endif + +#if !defined(__NR_io_pgetevents) +#define __NR_io_pgetevents 333 +#endif + +#if !defined(__NR_rseq) +#define __NR_rseq 334 +#endif + #endif // SANDBOX_LINUX_SYSTEM_HEADERS_X86_64_LINUX_SYSCALLS_H_ diff --git a/chromium/sandbox/mac/BUILD.gn b/chromium/sandbox/mac/BUILD.gn index 1ef9664ea57..d75fce09374 100644 --- a/chromium/sandbox/mac/BUILD.gn +++ b/chromium/sandbox/mac/BUILD.gn @@ -8,9 +8,7 @@ import("//third_party/protobuf/proto_library.gni") proto_library("seatbelt_proto") { visibility = [ ":*" ] - sources = [ - "seatbelt.proto", - ] + sources = [ "seatbelt.proto" ] } component("seatbelt") { @@ -26,12 +24,8 @@ component("seatbelt") { "seatbelt_export.h", ] libs = [ "sandbox" ] - deps = [ - ":seatbelt_proto", - ] - public_deps = [ - "//third_party/protobuf:protobuf_lite", - ] + deps = [ ":seatbelt_proto" ] + public_deps = [ "//third_party/protobuf:protobuf_lite" ] defines = [ "SEATBELT_IMPLEMENTATION" ] } @@ -43,9 +37,7 @@ component("seatbelt_extension") { "seatbelt_extension_token.h", ] libs = [ "sandbox" ] - public_deps = [ - "//base", - ] + public_deps = [ "//base" ] defines = [ "SEATBELT_IMPLEMENTATION" ] } @@ -59,9 +51,7 @@ component("system_services") { "Carbon.framework", "CoreFoundation.framework", ] - public_deps = [ - "//base", - ] + public_deps = [ "//base" ] defines = [ "SEATBELT_IMPLEMENTATION" ] } diff --git a/chromium/sandbox/mac/mojom/BUILD.gn b/chromium/sandbox/mac/mojom/BUILD.gn index 30e0949ba92..0ed698ddc47 100644 --- a/chromium/sandbox/mac/mojom/BUILD.gn +++ b/chromium/sandbox/mac/mojom/BUILD.gn @@ -5,16 +5,10 @@ import("//mojo/public/tools/bindings/mojom.gni") mojom("mojom") { - sources = [ - "seatbelt_extension_token.mojom", - ] + sources = [ "seatbelt_extension_token.mojom" ] } mojom("test_interfaces") { - sources = [ - "traits_test_service.mojom", - ] - public_deps = [ - ":mojom", - ] + sources = [ "traits_test_service.mojom" ] + public_deps = [ ":mojom" ] } diff --git a/chromium/sandbox/win/BUILD.gn b/chromium/sandbox/win/BUILD.gn index 919820cd689..2557f5c3572 100644 --- a/chromium/sandbox/win/BUILD.gn +++ b/chromium/sandbox/win/BUILD.gn @@ -2,6 +2,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import("//testing/libfuzzer/fuzzer_test.gni") import("//testing/test.gni") # This needs to be a static library rather than a sources set because small @@ -240,9 +241,7 @@ test("sbox_integration_tests") { } executable("cfi_unittest_exe") { - sources = [ - "tests/integration_tests/cfi_unittest_exe.cc", - ] + sources = [ "tests/integration_tests/cfi_unittest_exe.cc" ] deps = [ "//base", "//build/win:default_exe_manifest", @@ -371,3 +370,18 @@ shared_library("pocdll") { defines = [ "POCDLL_EXPORTS" ] } + +# This fuzzer will only work on Windows, add fuzz targets which could run on Linux +# to //sandbox/ directly. +fuzzer_test("sandbox_policy_rule_fuzzer") { + set_sources_assignment_filter([]) + sources = [ + "fuzzer/fuzzer_types.h", + "fuzzer/sandbox_policy_rule_fuzzer.cc", + ] + dict = "fuzzer/sandbox_policy_rule.dict" + deps = [ + "//base", + "//sandbox", + ] +} diff --git a/chromium/sandbox/win/OWNERS b/chromium/sandbox/win/OWNERS index da58563c3d9..ac5f8853410 100644 --- a/chromium/sandbox/win/OWNERS +++ b/chromium/sandbox/win/OWNERS @@ -1,6 +1,6 @@ +ajgo@chromium.org forshaw@chromium.org jschuh@chromium.org -pennymac@chromium.org wfh@chromium.org # TEAM: security-dev@chromium.org diff --git a/chromium/sandbox/win/src/broker_services.cc b/chromium/sandbox/win/src/broker_services.cc index 96c490bad3d..f524145e901 100644 --- a/chromium/sandbox/win/src/broker_services.cc +++ b/chromium/sandbox/win/src/broker_services.cc @@ -264,9 +264,11 @@ DWORD WINAPI BrokerServicesBase::TargetEventsThread(PVOID param) { HANDLE job_handle = tracker->job.Get(); // Erase by comparing with the job handle. - jobs.erase(std::remove_if( - jobs.begin(), jobs.end(), - [&](auto&& p) -> bool { return p->job.Get() == job_handle; })); + jobs.erase(std::remove_if(jobs.begin(), jobs.end(), + [&](auto&& p) -> bool { + return p->job.Get() == job_handle; + }), + jobs.end()); break; } @@ -355,10 +357,12 @@ DWORD WINAPI BrokerServicesBase::TargetEventsThread(PVOID param) { tracker->wait_handle = INVALID_HANDLE_VALUE; // PID is unique until the process handle is closed in dtor. - processes.erase(std::remove_if( - processes.begin(), processes.end(), [&](auto&& p) -> bool { - return p->process_id == tracker->process_id; - })); + processes.erase(std::remove_if(processes.begin(), processes.end(), + [&](auto&& p) -> bool { + return p->process_id == + tracker->process_id; + }), + processes.end()); } else if (THREAD_CTRL_GET_POLICY_INFO == key) { // Clone the policies for sandbox diagnostics. diff --git a/chromium/sandbox/win/src/interception.cc b/chromium/sandbox/win/src/interception.cc index b2c0bc47eaa..966a2fab535 100644 --- a/chromium/sandbox/win/src/interception.cc +++ b/chromium/sandbox/win/src/interception.cc @@ -449,14 +449,15 @@ ResultCode InterceptionManager::PatchClientFunctions( thunk.reset(new ServiceResolverThunk(child_->Process(), relaxed_)); #else base::win::OSInfo* os_info = base::win::OSInfo::GetInstance(); + base::win::Version real_os_version = os_info->Kernel32Version(); if (os_info->wow64_status() == base::win::OSInfo::WOW64_ENABLED) { - if (os_info->version() >= base::win::Version::WIN10) + if (real_os_version >= base::win::Version::WIN10) thunk.reset(new Wow64W10ResolverThunk(child_->Process(), relaxed_)); - else if (os_info->version() >= base::win::Version::WIN8) + else if (real_os_version >= base::win::Version::WIN8) thunk.reset(new Wow64W8ResolverThunk(child_->Process(), relaxed_)); else thunk.reset(new Wow64ResolverThunk(child_->Process(), relaxed_)); - } else if (os_info->version() >= base::win::Version::WIN8) { + } else if (real_os_version >= base::win::Version::WIN8) { thunk.reset(new Win8ResolverThunk(child_->Process(), relaxed_)); } else { thunk.reset(new ServiceResolverThunk(child_->Process(), relaxed_)); diff --git a/chromium/sandbox/win/src/policy_low_level.cc b/chromium/sandbox/win/src/policy_low_level.cc index d987211c8b5..b07677ec7da 100644 --- a/chromium/sandbox/win/src/policy_low_level.cc +++ b/chromium/sandbox/win/src/policy_low_level.cc @@ -59,6 +59,10 @@ LowLevelPolicy::~LowLevelPolicy() { } } +size_t LowLevelPolicy::GetPolicyGlobalSize() { + return policy_global_size_; +} + // Here is where the heavy byte shuffling is done. We take all the rules and // 'compile' them into a single memory region. Now, the rules are in random // order so the first step is to reorganize them into a stl map that is keyed @@ -72,6 +76,8 @@ bool LowLevelPolicy::Done() { typedef std::map<IpcTag, RuleList> Mmap; Mmap mmap; + policy_global_size_ = 0; + for (RuleNodes::iterator it = rules_.begin(); it != rules_.end(); ++it) { mmap[it->service].push_back(it->rule); } @@ -119,6 +125,10 @@ bool LowLevelPolicy::Done() { current_buffer = ¤t_buffer[policy_buffers_occupied + 1]; } + // The size used to store policy rules. Must be >=0 if we got here + // or we would have bailed out for lack of space earlier. + policy_global_size_ = policy_store_->data_size - avail_size; + return true; } diff --git a/chromium/sandbox/win/src/policy_low_level.h b/chromium/sandbox/win/src/policy_low_level.h index 1586f96af90..da6410a4a6a 100644 --- a/chromium/sandbox/win/src/policy_low_level.h +++ b/chromium/sandbox/win/src/policy_low_level.h @@ -96,6 +96,10 @@ class LowLevelPolicy { // passed on the constructor. Returns false on error. bool Done(); + // Returns the size that could hold all rules, valid after Done() has + // packed them. + size_t GetPolicyGlobalSize(); + private: struct RuleNode { const PolicyRule* rule; @@ -103,6 +107,7 @@ class LowLevelPolicy { }; std::list<RuleNode> rules_; PolicyGlobal* policy_store_; + size_t policy_global_size_; DISALLOW_IMPLICIT_CONSTRUCTORS(LowLevelPolicy); }; diff --git a/chromium/sandbox/win/src/process_mitigations_dyncode_unittest.cc b/chromium/sandbox/win/src/process_mitigations_dyncode_unittest.cc index 70b20532348..cdc1363e044 100644 --- a/chromium/sandbox/win/src/process_mitigations_dyncode_unittest.cc +++ b/chromium/sandbox/win/src/process_mitigations_dyncode_unittest.cc @@ -407,10 +407,7 @@ TEST(ProcessMitigationsTest, CheckWin81DynamicCodePolicySuccess) { return; // TODO(crbug.com/805414): Windows ASan hotpatching requires dynamic code. -#if defined(ADDRESS_SANITIZER) - return; -#endif - +#if !defined(ADDRESS_SANITIZER) std::wstring test_command = L"CheckPolicy "; test_command += std::to_wstring(TESTPOLICY_DYNAMICCODE); @@ -439,6 +436,7 @@ TEST(ProcessMitigationsTest, CheckWin81DynamicCodePolicySuccess) { policy2->SetDelayedProcessMitigations(MITIGATION_DYNAMIC_CODE_DISABLE), SBOX_ALL_OK); EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner2.RunTest(test_command.c_str())); +#endif } // This test validates that we can meddle with dynamic code if the @@ -483,10 +481,7 @@ TEST(ProcessMitigationsTest, CheckWin10DynamicCodeOptOutPolicySuccess) { return; // TODO(crbug.com/805414): Windows ASan hotpatching requires dynamic code. -#if defined(ADDRESS_SANITIZER) - return; -#endif - +#if !defined(ADDRESS_SANITIZER) std::wstring test_command = L"CheckPolicy "; test_command += std::to_wstring(TESTPOLICY_DYNAMICCODEOPTOUT); @@ -516,6 +511,7 @@ TEST(ProcessMitigationsTest, CheckWin10DynamicCodeOptOutPolicySuccess) { MITIGATION_DYNAMIC_CODE_DISABLE_WITH_OPT_OUT), SBOX_ALL_OK); EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner2.RunTest(test_command.c_str())); +#endif } // This test validates that we CAN meddle with dynamic code if the diff --git a/chromium/sandbox/win/src/restricted_token.cc b/chromium/sandbox/win/src/restricted_token.cc index 8e0cb8b30dc..5b20e88264d 100644 --- a/chromium/sandbox/win/src/restricted_token.cc +++ b/chromium/sandbox/win/src/restricted_token.cc @@ -146,6 +146,14 @@ DWORD RestrictedToken::GetRestrictedToken( } } + for (const auto& default_dacl_sid : sids_for_default_dacl_) { + if (!AddSidToDefaultDacl(new_token.Get(), std::get<0>(default_dacl_sid), + std::get<1>(default_dacl_sid), + std::get<2>(default_dacl_sid))) { + return ::GetLastError(); + } + } + // Add user to default dacl. if (!AddUserSidToDefaultDacl(new_token.Get(), GENERIC_ALL)) return ::GetLastError(); @@ -410,4 +418,15 @@ void RestrictedToken::SetLockdownDefaultDacl() { lockdown_default_dacl_ = true; } +DWORD RestrictedToken::AddDefaultDaclSid(const Sid& sid, + ACCESS_MODE access_mode, + ACCESS_MASK access) { + DCHECK(init_); + if (!init_) + return ERROR_NO_TOKEN; + + sids_for_default_dacl_.push_back(std::make_tuple(sid, access_mode, access)); + return ERROR_SUCCESS; +} + } // namespace sandbox diff --git a/chromium/sandbox/win/src/restricted_token.h b/chromium/sandbox/win/src/restricted_token.h index b713f87702b..4ee90bdb93d 100644 --- a/chromium/sandbox/win/src/restricted_token.h +++ b/chromium/sandbox/win/src/restricted_token.h @@ -10,6 +10,7 @@ #include <vector> #include <string> +#include <tuple> #include "base/macros.h" #include "base/win/scoped_handle.h" @@ -174,6 +175,12 @@ class RestrictedToken { // default DACL when created. void SetLockdownDefaultDacl(); + // Add a SID to the default DACL. These SIDs are added regardless of the + // SetLockdownDefaultDacl state. + DWORD AddDefaultDaclSid(const Sid& sid, + ACCESS_MODE access_mode, + ACCESS_MASK access); + private: // The list of restricting sids in the restricted token. std::vector<Sid> sids_to_restrict_; @@ -181,6 +188,8 @@ class RestrictedToken { std::vector<LUID> privileges_to_disable_; // The list of sids to mark as Deny Only in the restricted token. std::vector<Sid> sids_for_deny_only_; + // The list of sids to add to the default DACL of the restricted token. + std::vector<std::tuple<Sid, ACCESS_MODE, ACCESS_MASK>> sids_for_default_dacl_; // The token to restrict. Can only be set in a constructor. base::win::ScopedHandle effective_token_; // The token integrity level. Only valid on Vista. diff --git a/chromium/sandbox/win/src/restricted_token_test.cc b/chromium/sandbox/win/src/restricted_token_test.cc index ca7e78f624e..e788858c506 100644 --- a/chromium/sandbox/win/src/restricted_token_test.cc +++ b/chromium/sandbox/win/src/restricted_token_test.cc @@ -42,6 +42,44 @@ int RunOpenProcessTest(bool unsandboxed, .c_str()); } +int RunRestrictedOpenProcessTest(bool unsandboxed, + bool lockdown_dacl, + DWORD access_mask) { + TestRunner runner(JOB_NONE, USER_RESTRICTED_SAME_ACCESS, USER_LIMITED); + runner.GetPolicy()->SetDelayedIntegrityLevel(INTEGRITY_LEVEL_LOW); + runner.GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW); + if (lockdown_dacl) { + runner.GetPolicy()->SetLockdownDefaultDacl(); + runner.GetPolicy()->AddRestrictingRandomSid(); + } + runner.SetAsynchronous(true); + // This spins up a GPU level process, we don't care about the result. + runner.RunTest(L"IntegrationTestsTest_args 1"); + + TestRunner runner2(JOB_NONE, USER_RESTRICTED_SAME_ACCESS, USER_LIMITED); + runner2.GetPolicy()->SetDelayedIntegrityLevel(INTEGRITY_LEVEL_LOW); + runner2.GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW); + runner2.SetUnsandboxed(unsandboxed); + return runner2.RunTest( + base::StringPrintf(L"RestrictedTokenTest_openprocess %d 0x%08X", + runner.process_id(), access_mask) + .c_str()); +} + +int RunRestrictedSelfOpenProcessTest(bool add_random_sid, DWORD access_mask) { + TestRunner runner(JOB_NONE, USER_RESTRICTED_SAME_ACCESS, USER_LIMITED); + runner.GetPolicy()->SetDelayedIntegrityLevel(INTEGRITY_LEVEL_LOW); + runner.GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW); + runner.GetPolicy()->SetLockdownDefaultDacl(); + if (add_random_sid) + runner.GetPolicy()->AddRestrictingRandomSid(); + + return runner.RunTest( + base::StringPrintf(L"RestrictedTokenTest_currentprocess_dup 0x%08X", + access_mask) + .c_str()); +} + } // namespace // Opens a process based on a PID and access mask passed on the command line. @@ -62,6 +100,31 @@ SBOX_TESTS_COMMAND int RestrictedTokenTest_openprocess(int argc, return SBOX_TEST_DENIED; } +// Opens a process through duplication. This is to avoid the OpenProcess hook. +SBOX_TESTS_COMMAND int RestrictedTokenTest_currentprocess_dup(int argc, + wchar_t** argv) { + if (argc < 1) + return SBOX_TEST_NOT_FOUND; + DWORD desired_access = wcstoul(argv[0], nullptr, 0); + + HANDLE dup_handle; + if (!::DuplicateHandle(::GetCurrentProcess(), ::GetCurrentProcess(), + ::GetCurrentProcess(), &dup_handle, 0, FALSE, 0)) { + return SBOX_TEST_FIRST_ERROR; + } + base::win::ScopedHandle process_handle(dup_handle); + if (::DuplicateHandle(::GetCurrentProcess(), process_handle.Get(), + ::GetCurrentProcess(), &dup_handle, desired_access, + FALSE, 0)) { + ::CloseHandle(dup_handle); + return SBOX_TEST_SUCCEEDED; + } + + if (::GetLastError() != ERROR_ACCESS_DENIED) + return SBOX_TEST_SECOND_ERROR; + return SBOX_TEST_DENIED; +} + // Opens a the process token and checks if it's restricted. SBOX_TESTS_COMMAND int RestrictedTokenTest_IsRestricted(int argc, wchar_t** argv) { @@ -106,4 +169,30 @@ TEST(RestrictedTokenTest, CheckNonAdminRestricted) { runner_restricted.RunTest(L"RestrictedTokenTest_IsRestricted")); } +TEST(RestrictedTokenTest, OpenProcessSameSandboxRandomSid) { + // Test process to process open when not using random SID. + ASSERT_EQ(SBOX_TEST_SUCCEEDED, + RunRestrictedOpenProcessTest(false, false, GENERIC_ALL)); + // Test process to process open when using random SID. + ASSERT_EQ(SBOX_TEST_DENIED, + RunRestrictedOpenProcessTest(false, true, MAXIMUM_ALLOWED)); + // Test process to process open when not using random SID and opening from + // unsandboxed. + ASSERT_EQ(SBOX_TEST_SUCCEEDED, + RunRestrictedOpenProcessTest(true, false, GENERIC_ALL)); + // Test process to process open when using random SID and opening from + // unsandboxed. + ASSERT_EQ(SBOX_TEST_SUCCEEDED, + RunRestrictedOpenProcessTest(true, true, GENERIC_ALL)); +} + +TEST(RestrictedTokenTest, OpenProcessSelfRandomSid) { + // Test process can't open self when not using random SID. + ASSERT_EQ(SBOX_TEST_DENIED, + RunRestrictedSelfOpenProcessTest(false, PROCESS_ALL_ACCESS)); + // Test process can open self when using random SID. + ASSERT_EQ(SBOX_TEST_SUCCEEDED, + RunRestrictedSelfOpenProcessTest(true, PROCESS_ALL_ACCESS)); +} + } // namespace sandbox diff --git a/chromium/sandbox/win/src/restricted_token_unittest.cc b/chromium/sandbox/win/src/restricted_token_unittest.cc index 1855054a7cd..3968a6d8b76 100644 --- a/chromium/sandbox/win/src/restricted_token_unittest.cc +++ b/chromium/sandbox/win/src/restricted_token_unittest.cc @@ -20,11 +20,19 @@ namespace sandbox { namespace { -void TestDefaultDalc(bool restricted_required) { +void TestDefaultDalc(bool restricted_required, bool additional_sid_required) { RestrictedToken token; ASSERT_EQ(static_cast<DWORD>(ERROR_SUCCESS), token.Init(nullptr)); if (!restricted_required) token.SetLockdownDefaultDacl(); + ATL::CSid additional_sid = ATL::Sids::Guests(); + ATL::CSid additional_sid2 = ATL::Sids::Batch(); + if (additional_sid_required) { + token.AddDefaultDaclSid(Sid(additional_sid.GetPSID()), GRANT_ACCESS, + READ_CONTROL); + token.AddDefaultDaclSid(Sid(additional_sid2.GetPSID()), DENY_ACCESS, + GENERIC_ALL); + } ASSERT_EQ(static_cast<DWORD>(ERROR_SUCCESS), token.AddRestrictingSid(ATL::Sids::World().GetPSID())); @@ -44,20 +52,31 @@ void TestDefaultDalc(bool restricted_required) { bool restricted_found = false; bool logon_sid_found = false; + bool additional_sid_found = false; + bool additional_sid2_found = false; unsigned int ace_count = dacl.GetAceCount(); for (unsigned int i = 0; i < ace_count; ++i) { ATL::CSid sid; ACCESS_MASK mask = 0; - dacl.GetAclEntry(i, &sid, &mask); + BYTE ace_type = 0; + dacl.GetAclEntry(i, &sid, &mask, &ace_type); if (sid == ATL::Sids::RestrictedCode() && mask == GENERIC_ALL) { restricted_found = true; } else if (sid == logon_sid) { logon_sid_found = true; + } else if (sid == additional_sid && mask == READ_CONTROL && + ace_type == ACCESS_ALLOWED_ACE_TYPE) { + additional_sid_found = true; + } else if (sid == additional_sid2 && mask == GENERIC_ALL && + ace_type == ACCESS_DENIED_ACE_TYPE) { + additional_sid2_found = true; } } ASSERT_EQ(restricted_required, restricted_found); + ASSERT_EQ(additional_sid_required, additional_sid_found); + ASSERT_EQ(additional_sid_required, additional_sid2_found); if (!restricted_required) ASSERT_FALSE(logon_sid_found); } @@ -325,13 +344,24 @@ TEST(RestrictedTokenTest, ResultToken) { // Verifies that the token created has "Restricted" in its default dacl. TEST(RestrictedTokenTest, DefaultDacl) { - TestDefaultDalc(true); + TestDefaultDalc(true, false); } // Verifies that the token created does not have "Restricted" in its default // dacl. TEST(RestrictedTokenTest, DefaultDaclLockdown) { - TestDefaultDalc(false); + TestDefaultDalc(false, false); +} + +// Verifies that the token created has an additional SID in its default dacl. +TEST(RestrictedTokenTest, DefaultDaclWithAddition) { + TestDefaultDalc(true, true); +} + +// Verifies that the token created does not have "Restricted" in its default +// dacl and also has an additional SID. +TEST(RestrictedTokenTest, DefaultDaclLockdownWithAddition) { + TestDefaultDalc(false, true); } // Tests the method "AddSidForDenyOnly". diff --git a/chromium/sandbox/win/src/restricted_token_utils.cc b/chromium/sandbox/win/src/restricted_token_utils.cc index 7c18f8481a2..cfe397141ae 100644 --- a/chromium/sandbox/win/src/restricted_token_utils.cc +++ b/chromium/sandbox/win/src/restricted_token_utils.cc @@ -56,11 +56,18 @@ DWORD CreateRestrictedToken(HANDLE effective_token, IntegrityLevel integrity_level, TokenType token_type, bool lockdown_default_dacl, + PSID unique_restricted_sid, base::win::ScopedHandle* token) { RestrictedToken restricted_token; restricted_token.Init(effective_token); if (lockdown_default_dacl) restricted_token.SetLockdownDefaultDacl(); + if (unique_restricted_sid) { + restricted_token.AddDefaultDaclSid(Sid(unique_restricted_sid), GRANT_ACCESS, + GENERIC_ALL); + restricted_token.AddDefaultDaclSid(Sid(WinCreatorOwnerRightsSid), + GRANT_ACCESS, READ_CONTROL); + } std::vector<std::wstring> privilege_exceptions; std::vector<Sid> sid_exceptions; @@ -105,6 +112,8 @@ DWORD CreateRestrictedToken(HANDLE effective_token, restricted_token.AddRestrictingSid(WinRestrictedCodeSid); restricted_token.AddRestrictingSidCurrentUser(); restricted_token.AddRestrictingSidLogonSession(); + if (unique_restricted_sid) + restricted_token.AddRestrictingSid(Sid(unique_restricted_sid)); break; } case USER_INTERACTIVE: { @@ -118,6 +127,8 @@ DWORD CreateRestrictedToken(HANDLE effective_token, restricted_token.AddRestrictingSid(WinRestrictedCodeSid); restricted_token.AddRestrictingSidCurrentUser(); restricted_token.AddRestrictingSidLogonSession(); + if (unique_restricted_sid) + restricted_token.AddRestrictingSid(Sid(unique_restricted_sid)); break; } case USER_LIMITED: { @@ -128,6 +139,8 @@ DWORD CreateRestrictedToken(HANDLE effective_token, restricted_token.AddRestrictingSid(WinBuiltinUsersSid); restricted_token.AddRestrictingSid(WinWorldSid); restricted_token.AddRestrictingSid(WinRestrictedCodeSid); + if (unique_restricted_sid) + restricted_token.AddRestrictingSid(Sid(unique_restricted_sid)); // This token has to be able to create objects in BNO. // Unfortunately, on Vista+, it needs the current logon sid @@ -141,11 +154,15 @@ DWORD CreateRestrictedToken(HANDLE effective_token, privilege_exceptions.push_back(SE_CHANGE_NOTIFY_NAME); restricted_token.AddUserSidForDenyOnly(); restricted_token.AddRestrictingSid(WinRestrictedCodeSid); + if (unique_restricted_sid) + restricted_token.AddRestrictingSid(Sid(unique_restricted_sid)); break; } case USER_LOCKDOWN: { restricted_token.AddUserSidForDenyOnly(); restricted_token.AddRestrictingSid(WinNullSid); + if (unique_restricted_sid) + restricted_token.AddRestrictingSid(Sid(unique_restricted_sid)); break; } default: { return ERROR_BAD_ARGUMENTS; } diff --git a/chromium/sandbox/win/src/restricted_token_utils.h b/chromium/sandbox/win/src/restricted_token_utils.h index a396687ce16..0e41e01acdd 100644 --- a/chromium/sandbox/win/src/restricted_token_utils.h +++ b/chromium/sandbox/win/src/restricted_token_utils.h @@ -38,6 +38,7 @@ DWORD CreateRestrictedToken(HANDLE effective_token, IntegrityLevel integrity_level, TokenType token_type, bool lockdown_default_dacl, + PSID unique_restricted_sid, base::win::ScopedHandle* token); // Sets the integrity label on a object handle. diff --git a/chromium/sandbox/win/src/sandbox_nt_util.cc b/chromium/sandbox/win/src/sandbox_nt_util.cc index bace3102cbc..d7f6f032eab 100644 --- a/chromium/sandbox/win/src/sandbox_nt_util.cc +++ b/chromium/sandbox/win/src/sandbox_nt_util.cc @@ -9,6 +9,7 @@ #include <string> +#include "base/compiler_specific.h" #include "base/win/pe_image.h" #include "sandbox/win/src/sandbox_factory.h" #include "sandbox/win/src/target_services.h" @@ -58,7 +59,8 @@ void* AllocateNearTo(void* source, size_t size) { const char* top_address = base + kMaxSize; while (base < top_address) { - MEMORY_BASIC_INFORMATION mem_info; + // Avoid memset inserted by -ftrivial-auto-var-init=pattern. + STACK_UNINITIALIZED MEMORY_BASIC_INFORMATION mem_info; NTSTATUS status = g_nt.QueryVirtualMemory(NtCurrentProcess, base, MemoryBasicInformation, &mem_info, sizeof(mem_info), nullptr); diff --git a/chromium/sandbox/win/src/sandbox_policy.h b/chromium/sandbox/win/src/sandbox_policy.h index 527995586c8..279e5024124 100644 --- a/chromium/sandbox/win/src/sandbox_policy.h +++ b/chromium/sandbox/win/src/sandbox_policy.h @@ -252,6 +252,10 @@ class TargetPolicy { // resources. virtual void SetLockdownDefaultDacl() = 0; + // Adds a restricting random SID to the restricted SIDs list as well as + // the default DACL. + virtual void AddRestrictingRandomSid() = 0; + // Enable OPM API redirection when in Win32k lockdown. virtual void SetEnableOPMRedirection() = 0; // Enable OPM API emulation when in Win32k lockdown. @@ -272,6 +276,9 @@ class TargetPolicy { // lifetime of the policy object. virtual void SetEffectiveToken(HANDLE token) = 0; + // Returns the size of policy memory used at process start. + virtual size_t GetPolicyGlobalSize() const = 0; + protected: ~TargetPolicy() {} }; diff --git a/chromium/sandbox/win/src/sandbox_policy_base.cc b/chromium/sandbox/win/src/sandbox_policy_base.cc index 257eb032374..fdf63d9c774 100644 --- a/chromium/sandbox/win/src/sandbox_policy_base.cc +++ b/chromium/sandbox/win/src/sandbox_policy_base.cc @@ -109,6 +109,7 @@ PolicyBase::PolicyBase() policy_(nullptr), lowbox_sid_(nullptr), lockdown_default_dacl_(false), + add_restricting_random_sid_(false), enable_opm_redirection_(false), effective_token_(nullptr) { ::InitializeCriticalSection(&lock_); @@ -389,6 +390,10 @@ void PolicyBase::SetLockdownDefaultDacl() { lockdown_default_dacl_ = true; } +void PolicyBase::AddRestrictingRandomSid() { + add_restricting_random_sid_ = true; +} + const base::HandlesToInheritVector& PolicyBase::GetHandlesBeingShared() { return handles_to_share_; } @@ -413,11 +418,16 @@ ResultCode PolicyBase::MakeJobObject(base::win::ScopedHandle* job) { ResultCode PolicyBase::MakeTokens(base::win::ScopedHandle* initial, base::win::ScopedHandle* lockdown, base::win::ScopedHandle* lowbox) { + Sid random_sid = Sid::GenerateRandomSid(); + PSID random_sid_ptr = nullptr; + if (add_restricting_random_sid_) + random_sid_ptr = random_sid.GetPSID(); + // Create the 'naked' token. This will be the permanent token associated // with the process and therefore with any thread that is not impersonating. - DWORD result = - CreateRestrictedToken(effective_token_, lockdown_level_, integrity_level_, - PRIMARY, lockdown_default_dacl_, lockdown); + DWORD result = CreateRestrictedToken( + effective_token_, lockdown_level_, integrity_level_, PRIMARY, + lockdown_default_dacl_, random_sid_ptr, lockdown); if (ERROR_SUCCESS != result) return SBOX_ERROR_CANNOT_CREATE_RESTRICTED_TOKEN; @@ -484,9 +494,9 @@ ResultCode PolicyBase::MakeTokens(base::win::ScopedHandle* initial, // Create the 'better' token. We use this token as the one that the main // thread uses when booting up the process. It should contain most of // what we need (before reaching main( )) - result = - CreateRestrictedToken(effective_token_, initial_level_, integrity_level_, - IMPERSONATION, lockdown_default_dacl_, initial); + result = CreateRestrictedToken( + effective_token_, initial_level_, integrity_level_, IMPERSONATION, + lockdown_default_dacl_, random_sid_ptr, initial); if (ERROR_SUCCESS != result) return SBOX_ERROR_CANNOT_CREATE_RESTRICTED_IMP_TOKEN; @@ -497,9 +507,18 @@ PSID PolicyBase::GetLowBoxSid() const { return lowbox_sid_; } +size_t PolicyBase::GetPolicyGlobalSize() const { + // TODO(1059129) remove when Process.Sandbox.PolicyGlobalSize expires. + if (policy_maker_) + return policy_maker_->GetPolicyGlobalSize(); + return 0; +} + ResultCode PolicyBase::AddTarget(TargetProcess* target) { - if (policy_) - policy_maker_->Done(); + if (policy_) { + if (!policy_maker_->Done()) + return SBOX_ERROR_NO_SPACE; + } if (!ApplyProcessMitigationsToSuspendedProcess(target->Process(), mitigations_)) { diff --git a/chromium/sandbox/win/src/sandbox_policy_base.h b/chromium/sandbox/win/src/sandbox_policy_base.h index a2fda7ced0d..9763311479c 100644 --- a/chromium/sandbox/win/src/sandbox_policy_base.h +++ b/chromium/sandbox/win/src/sandbox_policy_base.h @@ -73,12 +73,14 @@ class PolicyBase final : public TargetPolicy { const wchar_t* handle_name) override; void AddHandleToShare(HANDLE handle) override; void SetLockdownDefaultDacl() override; + void AddRestrictingRandomSid() override; void SetEnableOPMRedirection() override; bool GetEnableOPMRedirection() override; ResultCode AddAppContainerProfile(const wchar_t* package_name, bool create_profile) override; scoped_refptr<AppContainerProfile> GetAppContainerProfile() override; void SetEffectiveToken(HANDLE token) override; + size_t GetPolicyGlobalSize() const override; // Get the AppContainer profile as its internal type. scoped_refptr<AppContainerProfileBase> GetAppContainerProfileBase(); @@ -168,6 +170,7 @@ class PolicyBase final : public TargetPolicy { base::win::ScopedHandle lowbox_directory_; std::unique_ptr<Dispatcher> dispatcher_; bool lockdown_default_dacl_; + bool add_restricting_random_sid_; static HDESK alternate_desktop_handle_; static HWINSTA alternate_winstation_handle_; diff --git a/chromium/sandbox/win/src/sandbox_policy_diagnostic.cc b/chromium/sandbox/win/src/sandbox_policy_diagnostic.cc index de4f4a7542c..2c2c392b535 100644 --- a/chromium/sandbox/win/src/sandbox_policy_diagnostic.cc +++ b/chromium/sandbox/win/src/sandbox_policy_diagnostic.cc @@ -31,7 +31,7 @@ namespace { base::Value ProcessIdList(std::vector<uint32_t> process_ids) { base::Value results(base::Value::Type::LIST); for (const auto pid : process_ids) { - results.GetList().push_back(base::Value(base::strict_cast<double>(pid))); + results.Append(base::strict_cast<double>(pid)); } return results; } @@ -339,7 +339,7 @@ base::Value GetPolicyOpcodes(const PolicyGlobal* policy_rules, IpcTag service) { } else { cur_rule += " -> "; cur_rule += GetPolicyOpcode(opcode, false); - entry.GetList().push_back(base::Value(cur_rule)); + entry.Append(cur_rule); cur_rule.clear(); } } diff --git a/chromium/sandbox/win/src/sid.cc b/chromium/sandbox/win/src/sid.cc index 5f35f3eb6a3..a97ca340ed3 100644 --- a/chromium/sandbox/win/src/sid.cc +++ b/chromium/sandbox/win/src/sid.cc @@ -7,8 +7,10 @@ #include <memory> #include <sddl.h> +#include <stdlib.h> #include "base/logging.h" +#include "base/rand_util.h" #include "base/win/windows_version.h" #include "sandbox/win/src/win_utils.h" @@ -132,6 +134,14 @@ Sid Sid::AllRestrictedApplicationPackages() { return FromSubAuthorities(&package_authority, 2, sub_authorities); } +Sid Sid::GenerateRandomSid() { + SID_IDENTIFIER_AUTHORITY package_authority = {SECURITY_NULL_SID_AUTHORITY}; + DWORD sub_authorities[4] = {}; + base::RandBytes(&sub_authorities, sizeof(sub_authorities)); + return FromSubAuthorities(&package_authority, _countof(sub_authorities), + sub_authorities); +} + PSID Sid::GetPSID() const { return const_cast<BYTE*>(sid_); } diff --git a/chromium/sandbox/win/src/sid.h b/chromium/sandbox/win/src/sid.h index 5382160a2ff..745f4710546 100644 --- a/chromium/sandbox/win/src/sid.h +++ b/chromium/sandbox/win/src/sid.h @@ -52,6 +52,8 @@ class Sid { PDWORD sub_authorities); // Create the restricted all application packages sid. static Sid AllRestrictedApplicationPackages(); + // Generate a random SID value. + static Sid GenerateRandomSid(); // Returns sid_. PSID GetPSID() const; diff --git a/chromium/sandbox/win/src/sid_unittest.cc b/chromium/sandbox/win/src/sid_unittest.cc index 81186cd0bec..e39b47ec685 100644 --- a/chromium/sandbox/win/src/sid_unittest.cc +++ b/chromium/sandbox/win/src/sid_unittest.cc @@ -179,4 +179,12 @@ TEST(SidTest, SubAuthorities) { ASSERT_TRUE(EqualSid(sid_admin, ATL::Sids::Admins())); } +TEST(SidTest, RandomSid) { + Sid sid1 = Sid::GenerateRandomSid(); + ASSERT_TRUE(sid1.IsValid()); + Sid sid2 = Sid::GenerateRandomSid(); + ASSERT_TRUE(sid2.IsValid()); + ASSERT_FALSE(::EqualSid(sid1.GetPSID(), sid2.GetPSID())); +} + } // namespace sandbox diff --git a/chromium/sandbox/win/src/win_utils.cc b/chromium/sandbox/win/src/win_utils.cc index 85be8d16f27..0c97c31bcd0 100644 --- a/chromium/sandbox/win/src/win_utils.cc +++ b/chromium/sandbox/win/src/win_utils.cc @@ -234,7 +234,6 @@ DWORD IsReparsePoint(const std::wstring& full_path) { (path.rfind(L'\\') == kNTDotPrefixLen - 1)) { break; } - NOTREACHED_NT(); return error; } } else if (FILE_ATTRIBUTE_REPARSE_POINT & attributes) { |