summaryrefslogtreecommitdiffstats
path: root/sandbox
diff options
context:
space:
mode:
authormdempsky <mdempsky@chromium.org>2015-03-07 15:00:59 -0800
committerCommit bot <commit-bot@chromium.org>2015-03-07 23:02:19 +0000
commit0b7c0c442edced1ab10b0c5208d5d0998923cc17 (patch)
tree177fe4af14ee266be076fc88588c4cbbd18c514a /sandbox
parentb297f8ae256ddc2c8740e18f1d48229eb7f16449 (diff)
downloadchromium_src-0b7c0c442edced1ab10b0c5208d5d0998923cc17.zip
chromium_src-0b7c0c442edced1ab10b0c5208d5d0998923cc17.tar.gz
chromium_src-0b7c0c442edced1ab10b0c5208d5d0998923cc17.tar.bz2
bpf_dsl: switch PolicyCompiler from seccomp-bpf/die.h to base/logging.h
[Reland of https://codereview.chromium.org/945743002] PolicyCompiler runs in a normal process context, so it doesn't require the extra low-levelness of SANDBOX_DIE. BUG=449357 Review URL: https://codereview.chromium.org/985933002 Cr-Commit-Position: refs/heads/master@{#319563}
Diffstat (limited to 'sandbox')
-rw-r--r--sandbox/linux/bpf_dsl/policy_compiler.cc72
-rw-r--r--sandbox/linux/bpf_dsl/trap_registry.h5
2 files changed, 33 insertions, 44 deletions
diff --git a/sandbox/linux/bpf_dsl/policy_compiler.cc b/sandbox/linux/bpf_dsl/policy_compiler.cc
index bcfd21b..9d94968 100644
--- a/sandbox/linux/bpf_dsl/policy_compiler.cc
+++ b/sandbox/linux/bpf_dsl/policy_compiler.cc
@@ -18,7 +18,6 @@
#include "sandbox/linux/bpf_dsl/policy.h"
#include "sandbox/linux/bpf_dsl/seccomp_macros.h"
#include "sandbox/linux/bpf_dsl/syscall_set.h"
-#include "sandbox/linux/seccomp-bpf/die.h"
#include "sandbox/linux/seccomp-bpf/errorcode.h"
#include "sandbox/linux/system_headers/linux_seccomp.h"
@@ -96,35 +95,21 @@ PolicyCompiler::~PolicyCompiler() {
}
scoped_ptr<CodeGen::Program> PolicyCompiler::Compile() {
- if (!policy_->InvalidSyscall()->IsDeny()) {
- SANDBOX_DIE("Policies should deny invalid system calls.");
- }
+ CHECK(policy_->InvalidSyscall()->IsDeny())
+ << "Policies should deny invalid system calls";
// If our BPF program has unsafe traps, enable support for them.
if (has_unsafe_traps_) {
- // As support for unsafe jumps essentially defeats all the security
- // measures that the sandbox provides, we print a big warning message --
- // and of course, we make sure to only ever enable this feature if it
- // is actually requested by the sandbox policy.
-
CHECK_NE(0U, escapepc_) << "UnsafeTrap() requires a valid escape PC";
for (int sysnum : kSyscallsRequiredForUnsafeTraps) {
- if (!policy_->EvaluateSyscall(sysnum)->IsAllow()) {
- SANDBOX_DIE(
- "Policies that use UnsafeTrap() must unconditionally allow all "
- "required system calls");
- }
+ CHECK(policy_->EvaluateSyscall(sysnum)->IsAllow())
+ << "Policies that use UnsafeTrap() must unconditionally allow all "
+ "required system calls";
}
- if (!registry_->EnableUnsafeTraps()) {
- // We should never be able to get here, as UnsafeTrap() should never
- // actually return a valid ErrorCode object unless the user set the
- // CHROME_SANDBOX_DEBUGGING environment variable; and therefore,
- // "has_unsafe_traps" would always be false. But better double-check
- // than enabling dangerous code.
- SANDBOX_DIE("We'd rather die than enable unsafe traps");
- }
+ CHECK(registry_->EnableUnsafeTraps())
+ << "We'd rather die than enable unsafe traps";
}
// Assemble the BPF filter program.
@@ -261,9 +246,9 @@ CodeGen::Node PolicyCompiler::AssembleJumpTable(Ranges::const_iterator start,
// a binary search over the ranges.
// As a sanity check, we need to have at least one distinct ranges for us
// to be able to build a jump table.
- if (stop - start <= 0) {
- SANDBOX_DIE("Invalid set of system call ranges");
- } else if (stop - start == 1) {
+ CHECK(start < stop) << "Invalid iterator range";
+ const auto n = stop - start;
+ if (n == 1) {
// If we have narrowed things down to a single range object, we can
// return from the BPF filter program.
return start->node;
@@ -273,7 +258,7 @@ CodeGen::Node PolicyCompiler::AssembleJumpTable(Ranges::const_iterator start,
// We compare our system call number against the lowest valid system call
// number in this range object. If our number is lower, it is outside of
// this range object. If it is greater or equal, it might be inside.
- Ranges::const_iterator mid = start + (stop - start) / 2;
+ Ranges::const_iterator mid = start + n / 2;
// Sub-divide the list of ranges and continue recursively.
CodeGen::Node jf = AssembleJumpTable(start, mid);
@@ -293,31 +278,30 @@ CodeGen::Node PolicyCompiler::RetExpression(const ErrorCode& err) {
case ErrorCode::ET_TRAP:
return gen_.MakeInstruction(BPF_RET + BPF_K, err.err());
default:
- SANDBOX_DIE("ErrorCode is not suitable for returning from a BPF program");
+ LOG(FATAL)
+ << "ErrorCode is not suitable for returning from a BPF program";
+ return CodeGen::kNullNode;
}
}
CodeGen::Node PolicyCompiler::CondExpression(const ErrorCode& cond) {
// Sanity check that |cond| makes sense.
- if (cond.argno_ < 0 || cond.argno_ >= 6) {
- SANDBOX_DIE("sandbox_bpf: invalid argument number");
- }
- if (cond.width_ != ErrorCode::TP_32BIT &&
- cond.width_ != ErrorCode::TP_64BIT) {
- SANDBOX_DIE("sandbox_bpf: invalid argument width");
- }
- if (cond.mask_ == 0) {
- SANDBOX_DIE("sandbox_bpf: zero mask is invalid");
- }
- if ((cond.value_ & cond.mask_) != cond.value_) {
- SANDBOX_DIE("sandbox_bpf: value contains masked out bits");
+ CHECK(cond.argno_ >= 0 && cond.argno_ < 6) << "Invalid argument number "
+ << cond.argno_;
+ CHECK(cond.width_ == ErrorCode::TP_32BIT ||
+ cond.width_ == ErrorCode::TP_64BIT)
+ << "Invalid argument width " << cond.width_;
+ CHECK_NE(0U, cond.mask_) << "Zero mask is invalid";
+ CHECK_EQ(cond.value_, cond.value_ & cond.mask_)
+ << "Value contains masked out bits";
+ if (sizeof(void*) == 4) {
+ CHECK_EQ(ErrorCode::TP_32BIT, cond.width_)
+ << "Invalid width on 32-bit platform";
}
- if (cond.width_ == ErrorCode::TP_32BIT &&
- ((cond.mask_ >> 32) != 0 || (cond.value_ >> 32) != 0)) {
- SANDBOX_DIE("sandbox_bpf: test exceeds argument size");
+ if (cond.width_ == ErrorCode::TP_32BIT) {
+ CHECK_EQ(0U, cond.mask_ >> 32) << "Mask exceeds argument size";
+ CHECK_EQ(0U, cond.value_ >> 32) << "Value exceeds argument size";
}
- // TODO(mdempsky): Reject TP_64BIT on 32-bit platforms. For now we allow it
- // because some SandboxBPF unit tests exercise it.
CodeGen::Node passed = RetExpression(*cond.passed_);
CodeGen::Node failed = RetExpression(*cond.failed_);
diff --git a/sandbox/linux/bpf_dsl/trap_registry.h b/sandbox/linux/bpf_dsl/trap_registry.h
index 6f8d37a..0a5d2f1 100644
--- a/sandbox/linux/bpf_dsl/trap_registry.h
+++ b/sandbox/linux/bpf_dsl/trap_registry.h
@@ -49,6 +49,11 @@ class SANDBOX_EXPORT TrapRegistry {
// EnableUnsafeTraps tries to enable unsafe traps and returns
// whether it was successful. This is a one-way operation.
+ //
+ // CAUTION: Enabling unsafe traps effectively defeats the security
+ // guarantees provided by the sandbox policy. TrapRegistry
+ // implementations should ensure unsafe traps are only enabled
+ // during testing.
virtual bool EnableUnsafeTraps() = 0;
protected: