Skip to content

Commit f9eb70f

Browse files
happyCoder92copybara-github
authored andcommitted
policy: Condition execveat rule on Executor::enable_sandboxing_pre_execve_
PiperOrigin-RevId: 733695333 Change-Id: Iaf978bcdb65387e6d7005f20224933aacdc6b747
1 parent 2a921e2 commit f9eb70f

File tree

9 files changed

+227
-112
lines changed

9 files changed

+227
-112
lines changed

sandboxed_api/sandbox2/BUILD

+1
Original file line numberDiff line numberDiff line change
@@ -941,6 +941,7 @@ cc_test(
941941
copts = sapi_platform_copts(),
942942
data = [
943943
"@com_google_sandboxed_api//sandboxed_api/sandbox2/testcases:add_policy_on_syscalls",
944+
"@com_google_sandboxed_api//sandboxed_api/sandbox2/testcases:execveat",
944945
"@com_google_sandboxed_api//sandboxed_api/sandbox2/testcases:malloc_system",
945946
"@com_google_sandboxed_api//sandboxed_api/sandbox2/testcases:minimal",
946947
"@com_google_sandboxed_api//sandboxed_api/sandbox2/testcases:minimal_dynamic",

sandboxed_api/sandbox2/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -1025,6 +1025,7 @@ if(BUILD_TESTING AND SAPI_BUILD_TESTING)
10251025
)
10261026
add_dependencies(sandbox2_policy_test
10271027
sandbox2::testcase_add_policy_on_syscalls
1028+
sandbox2::testcase_execveat
10281029
sandbox2::testcase_malloc_system
10291030
sandbox2::testcase_minimal
10301031
sandbox2::testcase_minimal_dynamic

sandboxed_api/sandbox2/monitor_base.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,8 @@ absl::Status MonitorBase::SendPolicy(const std::vector<sock_filter>& policy) {
279279

280280
bool MonitorBase::InitSendPolicy() {
281281
bool user_notif = type_ == FORKSERVER_MONITOR_UNOTIFY;
282-
auto policy = policy_->GetPolicy(user_notif);
282+
auto policy =
283+
policy_->GetPolicy(user_notif, executor_->enable_sandboxing_pre_execve_);
283284
absl::Status status = SendPolicy(std::move(policy));
284285
if (!status.ok()) {
285286
LOG(ERROR) << "Couldn't send policy: " << status;

sandboxed_api/sandbox2/policy.cc

+39-27
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,16 @@ namespace sandbox2 {
7575
// 1. default policy (GetDefaultPolicy, private),
7676
// 2. user policy (user_policy_, public),
7777
// 3. default KILL action (avoid failing open if user policy did not do it).
78-
std::vector<sock_filter> Policy::GetPolicy(bool user_notif) const {
78+
std::vector<sock_filter> Policy::GetPolicy(
79+
bool user_notif, bool enable_sandboxing_pre_execve) const {
7980
if (absl::GetFlag(FLAGS_sandbox2_danger_danger_permit_all) ||
8081
!absl::GetFlag(FLAGS_sandbox2_danger_danger_permit_all_and_log).empty()) {
8182
return GetTrackingPolicy();
8283
}
8384

8485
// Now we can start building the policy.
8586
// 1. Start with the default policy (e.g. syscall architecture checks).
86-
auto policy = GetDefaultPolicy(user_notif);
87+
auto policy = GetDefaultPolicy(user_notif, enable_sandboxing_pre_execve);
8788
VLOG(3) << "Default policy:\n" << bpf::Disasm(policy);
8889

8990
// 2. Append user policy.
@@ -105,7 +106,8 @@ std::vector<sock_filter> Policy::GetPolicy(bool user_notif) const {
105106
// Produces a policy which returns SECCOMP_RET_TRACE instead of SECCOMP_RET_KILL
106107
// for the __NR_execve syscall, so the tracer can make a decision to allow or
107108
// disallow it depending on which occurrence of __NR_execve it was.
108-
std::vector<sock_filter> Policy::GetDefaultPolicy(bool user_notif) const {
109+
std::vector<sock_filter> Policy::GetDefaultPolicy(
110+
bool user_notif, bool enable_sandboxing_pre_execve) const {
109111
bpf_labels l = {0};
110112

111113
std::vector<sock_filter> policy;
@@ -122,16 +124,21 @@ std::vector<sock_filter> Policy::GetDefaultPolicy(bool user_notif) const {
122124
ALLOW,
123125
LABEL(&l, past_seccomp_l),
124126
LOAD_SYSCALL_NR,
125-
JNE32(__NR_execveat, JUMP(&l, past_execveat_l)),
126-
ARG_32(4),
127-
JNE32(AT_EMPTY_PATH, JUMP(&l, past_execveat_l)),
128-
ARG_32(5),
129-
JNE32(internal::kExecveMagic, JUMP(&l, past_execveat_l)),
130-
ALLOW,
131-
LABEL(&l, past_execveat_l),
132-
133-
LOAD_SYSCALL_NR,
134127
};
128+
if (enable_sandboxing_pre_execve) {
129+
policy.insert(
130+
policy.end(),
131+
{
132+
JNE32(__NR_execveat, JUMP(&l, past_execveat_l)),
133+
ARG_32(4),
134+
JNE32(AT_EMPTY_PATH, JUMP(&l, past_execveat_l)),
135+
ARG_32(5),
136+
JNE32(internal::kExecveMagic, JUMP(&l, past_execveat_l)),
137+
ALLOW,
138+
LABEL(&l, past_execveat_l),
139+
LOAD_SYSCALL_NR,
140+
});
141+
}
135142
} else {
136143
policy = {
137144
// If compiled arch is different from the runtime one, inform the
@@ -144,23 +151,28 @@ std::vector<sock_filter> Policy::GetDefaultPolicy(bool user_notif) const {
144151
TRACE(sapi::cpu::kUnknown),
145152
LABEL(&l, past_arch_check_l),
146153

147-
// After the policy is uploaded, forkserver will execve the sandboxee.
148-
// We need to allow this execve but not others. Since BPF does not have
149-
// state, we need to inform the Monitor to decide, and for that we use a
150-
// magic value in syscall args 5. Note that this value is not supposed
151-
// to be secret, but just an optimization so that the monitor is not
152-
// triggered on every call to execveat.
153-
LOAD_SYSCALL_NR,
154-
JNE32(__NR_execveat, JUMP(&l, past_execveat_l)),
155-
ARG_32(4),
156-
JNE32(AT_EMPTY_PATH, JUMP(&l, past_execveat_l)),
157-
ARG_32(5),
158-
JNE32(internal::kExecveMagic, JUMP(&l, past_execveat_l)),
159-
SANDBOX2_TRACE,
160-
LABEL(&l, past_execveat_l),
161-
162154
LOAD_SYSCALL_NR,
163155
};
156+
if (enable_sandboxing_pre_execve) {
157+
// After the policy is uploaded, forkserver will execve the sandboxee.
158+
// We need to allow this execve but not others. Since BPF does not have
159+
// state, we need to inform the Monitor to decide, and for that we use a
160+
// magic value in syscall args 5. Note that this value is not supposed
161+
// to be secret, but just an optimization so that the monitor is not
162+
// triggered on every call to execveat.
163+
policy.insert(
164+
policy.end(),
165+
{
166+
JNE32(__NR_execveat, JUMP(&l, past_execveat_l)),
167+
ARG_32(4),
168+
JNE32(AT_EMPTY_PATH, JUMP(&l, past_execveat_l)),
169+
ARG_32(5),
170+
JNE32(internal::kExecveMagic, JUMP(&l, past_execveat_l)),
171+
SANDBOX2_TRACE,
172+
LABEL(&l, past_execveat_l),
173+
LOAD_SYSCALL_NR,
174+
});
175+
}
164176
}
165177

166178
// Insert a custom syscall to signal the sandboxee it's running inside a

sandboxed_api/sandbox2/policy.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ class Policy final {
5656

5757
// Returns the policy, but modifies it according to FLAGS and internal
5858
// requirements (message passing via Comms, Executor::WaitForExecve etc.).
59-
std::vector<sock_filter> GetPolicy(bool user_notif) const;
59+
std::vector<sock_filter> GetPolicy(bool user_notif,
60+
bool enable_sandboxing_pre_execve) const;
6061

6162
const std::optional<Namespace>& GetNamespace() const { return namespace_; }
6263
const Namespace* GetNamespaceOrNull() const {
@@ -65,7 +66,8 @@ class Policy final {
6566

6667
// Returns the default policy, which blocks certain dangerous syscalls and
6768
// mismatched syscall tables.
68-
std::vector<sock_filter> GetDefaultPolicy(bool user_notif) const;
69+
std::vector<sock_filter> GetDefaultPolicy(
70+
bool user_notif, bool enable_sandboxing_pre_execve) const;
6971
// Returns a policy allowing the Monitor module to track all syscalls.
7072
std::vector<sock_filter> GetTrackingPolicy() const;
7173

0 commit comments

Comments
 (0)