Permit sandboxee's bpf() to fail

The default policy causes immediate termination of a sandboxee that
calls `bpf`(2).

This does not allow for try-call use of `bpf()` to test for optional
features.

To support such try-call use cases, sandboxes would like to say:

```
  sandbox2::PolicyBuilder builder;
  builder.BlockSyscallWithErrno(__NR_bpf, EPERM);
```

but this doesn't work because the default policy unconditionally treats
`bpf()` as a sandbox violation.

Remove the bpf violation check from the policy if `bpf()` is explicitly
blocked with an errno.

PiperOrigin-RevId: 345239389
Change-Id: I7fcfd3a938c610c8679edf8e1fa0238b32cc9db4
This commit is contained in:
Sandboxed API Team 2020-12-02 08:37:55 -08:00 committed by Copybara-Service
parent da64459e3f
commit 3323ddc129
6 changed files with 56 additions and 12 deletions

View File

@ -106,18 +106,26 @@ std::vector<sock_filter> Policy::GetDefaultPolicy() const {
SANDBOX2_TRACE, SANDBOX2_TRACE,
LABEL(&l, past_execveat_l), LABEL(&l, past_execveat_l),
// Forbid some syscalls because unsafe or too risky. // Forbid ptrace because it's unsafe or too risky.
LOAD_SYSCALL_NR, LOAD_SYSCALL_NR,
JEQ32(__NR_ptrace, DENY), JEQ32(__NR_ptrace, DENY),
JEQ32(__NR_bpf, DENY),
// Disallow clone with CLONE_UNTRACED flag.
JNE32(__NR_clone, JUMP(&l, past_clone_untraced_l)),
// Regardless of arch, we only care about the lower 32-bits of the flags.
ARG_32(0),
JA32(CLONE_UNTRACED, DENY),
LABEL(&l, past_clone_untraced_l),
}; };
// 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_) {
policy.insert(policy.end(), {JEQ32(__NR_bpf, DENY)});
}
policy.insert(policy.end(),
{
// Disallow clone with CLONE_UNTRACED flag. This uses
// LOAD_SYSCALL_NR from above.
JNE32(__NR_clone, JUMP(&l, past_clone_untraced_l)),
// Regardless of arch, we only care about the lower 32-bits
// of the flags.
ARG_32(0),
JA32(CLONE_UNTRACED, DENY),
LABEL(&l, past_clone_untraced_l),
});
if (bpf_resolve_jumps(&l, policy.data(), policy.size()) != 0) { if (bpf_resolve_jumps(&l, policy.data(), policy.size()) != 0) {
LOG(FATAL) << "Cannot resolve bpf jumps"; LOG(FATAL) << "Cannot resolve bpf jumps";

View File

@ -96,6 +96,7 @@ class Policy final {
// The policy set by the user. // The policy set by the user.
std::vector<sock_filter> user_policy_; std::vector<sock_filter> user_policy_;
bool user_policy_handles_bpf_ = false;
// Get the default policy, which blocks certain dangerous syscalls and // Get the default policy, which blocks certain dangerous syscalls and
// mismatched syscall tables. // mismatched syscall tables.

View File

@ -40,7 +40,7 @@ using ::testing::Eq;
namespace sandbox2 { namespace sandbox2 {
namespace { namespace {
std::unique_ptr<Policy> PolicyTestcasePolicy() { PolicyBuilder CreatePolicyTestPolicyBuilder() {
return PolicyBuilder() return PolicyBuilder()
.DisableNamespaces() .DisableNamespaces()
.AllowStaticStartup() .AllowStaticStartup()
@ -60,8 +60,11 @@ std::unique_ptr<Policy> PolicyTestcasePolicy() {
#ifdef __NR_faccessat #ifdef __NR_faccessat
.BlockSyscallWithErrno(__NR_faccessat, ENOENT) .BlockSyscallWithErrno(__NR_faccessat, ENOENT)
#endif #endif
.BlockSyscallWithErrno(__NR_prlimit64, EPERM) .BlockSyscallWithErrno(__NR_prlimit64, EPERM);
.BuildOrDie(); }
std::unique_ptr<Policy> PolicyTestcasePolicy() {
return CreatePolicyTestPolicyBuilder().BuildOrDie();
} }
#ifdef SAPI_X86_64 #ifdef SAPI_X86_64
@ -150,6 +153,25 @@ TEST(PolicyTest, BpfDisallowed) {
EXPECT_THAT(result.reason_code(), Eq(__NR_bpf)); EXPECT_THAT(result.reason_code(), Eq(__NR_bpf));
} }
// Test that bpf(2) can return EPERM.
TEST(PolicyTest, BpfPermissionDenied) {
SKIP_SANITIZERS_AND_COVERAGE;
const std::string path = GetTestSourcePath("sandbox2/testcases/policy");
std::vector<std::string> args = {path, "7"};
auto executor = absl::make_unique<Executor>(path, args);
auto policy = CreatePolicyTestPolicyBuilder()
.BlockSyscallWithErrno(__NR_bpf, EPERM)
.BuildOrDie();
Sandbox2 s2(std::move(executor), std::move(policy));
auto result = s2.Run();
// bpf(2) is not a violation due to explicit policy. EPERM is expected.
ASSERT_THAT(result.final_status(), Eq(Result::OK));
EXPECT_THAT(result.reason_code(), Eq(EPERM));
}
TEST(PolicyTest, IsattyAllowed) { TEST(PolicyTest, IsattyAllowed) {
SKIP_SANITIZERS_AND_COVERAGE; SKIP_SANITIZERS_AND_COVERAGE;
const std::string path = GetTestSourcePath("sandbox2/testcases/policy"); const std::string path = GetTestSourcePath("sandbox2/testcases/policy");

View File

@ -82,6 +82,9 @@ PolicyBuilder& PolicyBuilder::BlockSyscallWithErrno(unsigned int num,
int error) { int error) {
if (handled_syscalls_.insert(num).second) { if (handled_syscalls_.insert(num).second) {
user_policy_.insert(user_policy_.end(), {SYSCALL(num, ERRNO(error))}); user_policy_.insert(user_policy_.end(), {SYSCALL(num, ERRNO(error))});
if (num == __NR_bpf) {
user_policy_handles_bpf_ = true;
}
} }
return *this; return *this;
} }
@ -728,6 +731,7 @@ absl::StatusOr<std::unique_ptr<Policy>> PolicyBuilder::TryBuild() {
output->collect_stacktrace_on_timeout_ = collect_stacktrace_on_timeout_; output->collect_stacktrace_on_timeout_ = collect_stacktrace_on_timeout_;
output->collect_stacktrace_on_kill_ = collect_stacktrace_on_kill_; output->collect_stacktrace_on_kill_ = collect_stacktrace_on_kill_;
output->user_policy_ = std::move(user_policy_); output->user_policy_ = std::move(user_policy_);
output->user_policy_handles_bpf_ = user_policy_handles_bpf_;
auto pb_description = absl::make_unique<PolicyBuilderDescription>(); auto pb_description = absl::make_unique<PolicyBuilderDescription>();

View File

@ -554,6 +554,7 @@ class PolicyBuilder final {
// Seccomp fields // Seccomp fields
std::vector<sock_filter> user_policy_; std::vector<sock_filter> user_policy_;
bool user_policy_handles_bpf_ = false;
std::set<unsigned int> handled_syscalls_; std::set<unsigned int> handled_syscalls_;
// Error handling // Error handling

View File

@ -19,6 +19,7 @@
#include <syscall.h> #include <syscall.h>
#include <unistd.h> #include <unistd.h>
#include <cerrno>
#include <cstdint> #include <cstdint>
#include <cstdio> #include <cstdio>
#include <cstdlib> #include <cstdlib>
@ -79,6 +80,10 @@ void TestBpf() {
exit(EXIT_FAILURE); exit(EXIT_FAILURE);
} }
void TestBpfError() {
exit(syscall(__NR_bpf, 0, nullptr, 0) == -1 ? errno : 0);
}
void TestIsatty() { void TestIsatty() {
isatty(0); isatty(0);
@ -119,6 +124,9 @@ int main(int argc, char** argv) {
case 6: case 6:
TestIsatty(); TestIsatty();
break; break;
case 7:
TestBpfError();
break;
default: default:
printf("Unknown test: %d\n", testno); printf("Unknown test: %d\n", testno);
return EXIT_FAILURE; return EXIT_FAILURE;