diff --git a/sandboxed_api/sandbox2/policy.cc b/sandboxed_api/sandbox2/policy.cc index 8cd2878..ac37db4 100644 --- a/sandboxed_api/sandbox2/policy.cc +++ b/sandboxed_api/sandbox2/policy.cc @@ -106,10 +106,16 @@ std::vector Policy::GetDefaultPolicy() const { SANDBOX2_TRACE, LABEL(&l, past_execveat_l), - // Forbid ptrace because it's unsafe or too risky. LOAD_SYSCALL_NR, - JEQ32(__NR_ptrace, DENY), }; + + // Forbid ptrace because it's unsafe or too risky. The user policy can only + // block (i.e. return an error instead of killing the process) but not allow + // ptrace. This uses LOAD_SYSCALL_NR from above. + if (!user_policy_handles_ptrace_) { + policy.insert(policy.end(), {JEQ32(__NR_ptrace, DENY)}); + } + // If user policy doesn't mention it, then forbid bpf because it's unsafe or // too risky. This uses LOAD_SYSCALL_NR from above. if (!user_policy_handles_bpf_) { diff --git a/sandboxed_api/sandbox2/policy.h b/sandboxed_api/sandbox2/policy.h index c419e8f..3f2dcbb 100644 --- a/sandboxed_api/sandbox2/policy.h +++ b/sandboxed_api/sandbox2/policy.h @@ -107,6 +107,7 @@ class Policy final { // The policy set by the user. std::vector user_policy_; bool user_policy_handles_bpf_ = false; + bool user_policy_handles_ptrace_ = false; // Contains a list of hosts the sandboxee is allowed to connect to. absl::optional allowed_hosts_; diff --git a/sandboxed_api/sandbox2/policy_test.cc b/sandboxed_api/sandbox2/policy_test.cc index ac2cb75..b03009c 100644 --- a/sandboxed_api/sandbox2/policy_test.cc +++ b/sandboxed_api/sandbox2/policy_test.cc @@ -114,6 +114,21 @@ TEST(PolicyTest, PtraceDisallowed) { EXPECT_THAT(result.reason_code(), Eq(__NR_ptrace)); } +TEST(PolicyTest, PtraceBlocked) { + SKIP_SANITIZERS_AND_COVERAGE; + const std::string path = GetTestSourcePath("sandbox2/testcases/policy"); + std::vector args = {path, "8"}; + + Sandbox2 s2(std::make_unique(path, args), + CreatePolicyTestPolicyBuilder() + .BlockSyscallWithErrno(__NR_ptrace, EPERM) + .BuildOrDie()); + auto result = s2.Run(); + + // The policy binary fails with an error if the system call is *not* blocked. + ASSERT_THAT(result.final_status(), Eq(Result::OK)); +} + // Test that clone(2) with flag CLONE_UNTRACED is disallowed. TEST(PolicyTest, CloneUntracedDisallowed) { SKIP_SANITIZERS_AND_COVERAGE; diff --git a/sandboxed_api/sandbox2/policybuilder.cc b/sandboxed_api/sandbox2/policybuilder.cc index 6864eed..2111967 100644 --- a/sandboxed_api/sandbox2/policybuilder.cc +++ b/sandboxed_api/sandbox2/policybuilder.cc @@ -113,6 +113,9 @@ PolicyBuilder& PolicyBuilder::BlockSyscallWithErrno(uint32_t num, int error) { if (num == __NR_bpf) { user_policy_handles_bpf_ = true; } + if (num == __NR_ptrace) { + user_policy_handles_ptrace_ = true; + } } return *this; } @@ -929,6 +932,7 @@ absl::StatusOr> PolicyBuilder::TryBuild() { overridable_policy_.begin(), overridable_policy_.end()); output->user_policy_handles_bpf_ = user_policy_handles_bpf_; + output->user_policy_handles_ptrace_ = user_policy_handles_ptrace_; auto pb_description = absl::make_unique(); diff --git a/sandboxed_api/sandbox2/policybuilder.h b/sandboxed_api/sandbox2/policybuilder.h index 6e26346..7d8c1a3 100644 --- a/sandboxed_api/sandbox2/policybuilder.h +++ b/sandboxed_api/sandbox2/policybuilder.h @@ -639,6 +639,7 @@ class PolicyBuilder final { std::vector user_policy_; std::vector overridable_policy_; bool user_policy_handles_bpf_ = false; + bool user_policy_handles_ptrace_ = false; absl::flat_hash_set handled_syscalls_; // Error handling diff --git a/sandboxed_api/sandbox2/testcases/policy.cc b/sandboxed_api/sandbox2/testcases/policy.cc index 0ff38c3..12bedee 100644 --- a/sandboxed_api/sandbox2/testcases/policy.cc +++ b/sandboxed_api/sandbox2/testcases/policy.cc @@ -58,13 +58,22 @@ void TestAMD64SyscallMismatchFs() { } #endif -void TestPtrace() { +void TestPtraceDenied() { ptrace(PTRACE_SEIZE, getppid(), 0, 0); printf("Syscall violation should have been discovered by now\n"); exit(EXIT_FAILURE); } +void TestPtraceBlocked() { + int result = ptrace(PTRACE_SEIZE, getppid(), 0, 0); + + if (result != -1 || errno != EPERM) { + printf("System call should have been blocked\n"); + exit(EXIT_FAILURE); + } +} + void TestCloneUntraced() { syscall(__NR_clone, static_cast(CLONE_UNTRACED), nullptr, nullptr, nullptr, static_cast(0)); @@ -113,7 +122,7 @@ int main(int argc, char* argv[]) { break; #endif case 3: - TestPtrace(); + TestPtraceDenied(); break; case 4: TestCloneUntraced(); @@ -127,6 +136,9 @@ int main(int argc, char* argv[]) { case 7: TestBpfError(); break; + case 8: + TestPtraceBlocked(); + break; default: printf("Unknown test: %d\n", testno); return EXIT_FAILURE;