Remove AllowUnsafeKeepCapabilities()

PiperOrigin-RevId: 506586347
Change-Id: I859a1f695ffbcf3b982a26df425c6b4e03c62da1
This commit is contained in:
Wiktor Garbacz 2023-02-02 04:46:19 -08:00 committed by Copybara-Service
parent 8f24f2a4f0
commit 34b2f6bc90
8 changed files with 3 additions and 45 deletions

View File

@ -83,7 +83,7 @@ std::vector<std::string> Executor::CopyEnviron() {
} }
absl::StatusOr<Executor::Process> Executor::StartSubProcess( absl::StatusOr<Executor::Process> Executor::StartSubProcess(
int32_t clone_flags, const Namespace* ns, const std::vector<int>& caps) { int32_t clone_flags, const Namespace* ns) {
if (started_) { if (started_) {
return absl::FailedPreconditionError( return absl::FailedPreconditionError(
"This executor has already been started"); "This executor has already been started");
@ -150,10 +150,6 @@ absl::StatusOr<Executor::Process> Executor::StartSubProcess(
request.set_clone_flags(clone_flags); request.set_clone_flags(clone_flags);
for (auto cap : caps) {
request.add_capabilities(cap);
}
Process process; Process process;
if (fork_client_) { if (fork_client_) {

View File

@ -124,12 +124,8 @@ class Executor final {
// Starts a new process which is connected with this Executor instance via a // Starts a new process which is connected with this Executor instance via a
// Comms channel. // Comms channel.
// For clone_flags refer to Linux' 'man 2 clone'. // For clone_flags refer to Linux' 'man 2 clone'.
//
// caps is a vector of capabilities that are kept in the permitted set after
// the clone, use with caution.
absl::StatusOr<Process> StartSubProcess(int clone_flags, absl::StatusOr<Process> StartSubProcess(int clone_flags,
const Namespace* ns = nullptr, const Namespace* ns = nullptr);
const std::vector<int>& caps = {});
// Whether the Executor has been started yet // Whether the Executor has been started yet
bool started_ = false; bool started_ = false;

View File

@ -300,15 +300,6 @@ void ForkServer::LaunchChild(const ForkRequest& request, int execve_fd,
InitializeNamespaces(request, uid, gid, avoid_pivot_root); InitializeNamespaces(request, uid, gid, avoid_pivot_root);
auto caps = cap_init(); auto caps = cap_init();
for (auto cap : request.capabilities()) {
SAPI_RAW_CHECK(cap_set_flag(caps, CAP_PERMITTED, 1, &cap, CAP_SET) == 0,
absl::StrCat("setting capability ", cap).c_str());
SAPI_RAW_CHECK(cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap, CAP_SET) == 0,
absl::StrCat("setting capability ", cap).c_str());
SAPI_RAW_CHECK(cap_set_flag(caps, CAP_INHERITABLE, 1, &cap, CAP_SET) == 0,
absl::StrCat("setting capability ", cap).c_str());
}
SAPI_RAW_CHECK(cap_set_proc(caps) == 0, "while dropping capabilities"); SAPI_RAW_CHECK(cap_set_proc(caps) == 0, "while dropping capabilities");
cap_free(caps); cap_free(caps);

View File

@ -192,7 +192,7 @@ void MonitorBase::Launch() {
// Get PID of the sandboxee. // Get PID of the sandboxee.
bool should_have_init = ns && (ns->GetCloneFlags() & CLONE_NEWPID); bool should_have_init = ns && (ns->GetCloneFlags() & CLONE_NEWPID);
absl::StatusOr<Executor::Process> process = absl::StatusOr<Executor::Process> process =
executor_->StartSubProcess(clone_flags, ns, policy_->capabilities()); executor_->StartSubProcess(clone_flags, ns);
if (!process.ok()) { if (!process.ok()) {
LOG(ERROR) << "Starting sandboxed subprocess failed: " << process.status(); LOG(ERROR) << "Starting sandboxed subprocess failed: " << process.status();

View File

@ -208,8 +208,6 @@ Namespace::Namespace(bool allow_unrestricted_networking, Mounts mounts,
} }
} }
void Namespace::DisableUserNamespace() { clone_flags_ &= ~CLONE_NEWUSER; }
int32_t Namespace::GetCloneFlags() const { return clone_flags_; } int32_t Namespace::GetCloneFlags() const { return clone_flags_; }
void Namespace::InitializeNamespaces(uid_t uid, gid_t gid, int32_t clone_flags, void Namespace::InitializeNamespaces(uid_t uid, gid_t gid, int32_t clone_flags,

View File

@ -47,8 +47,6 @@ class Namespace final {
Namespace(bool allow_unrestricted_networking, Mounts mounts, Namespace(bool allow_unrestricted_networking, Mounts mounts,
std::string hostname, bool allow_mount_propagation); std::string hostname, bool allow_mount_propagation);
void DisableUserNamespace();
// Returns all needed CLONE_NEW* flags. // Returns all needed CLONE_NEW* flags.
int32_t GetCloneFlags() const; int32_t GetCloneFlags() const;

View File

@ -181,13 +181,6 @@ bool Policy::SendPolicy(Comms* comms) const {
return true; return true;
} }
void Policy::AllowUnsafeKeepCapabilities(std::vector<int> caps) {
if (namespace_) {
namespace_->DisableUserNamespace();
}
capabilities_ = std::move(caps);
}
void Policy::GetPolicyDescription(PolicyDescription* policy) const { void Policy::GetPolicyDescription(PolicyDescription* policy) const {
policy->set_user_bpf_policy(user_policy_.data(), policy->set_user_bpf_policy(user_policy_.data(),
user_policy_.size() * sizeof(sock_filter)); user_policy_.size() * sizeof(sock_filter));
@ -200,10 +193,6 @@ void Policy::GetPolicyDescription(PolicyDescription* policy) const {
namespace_->GetNamespaceDescription( namespace_->GetNamespaceDescription(
policy->mutable_namespace_description()); policy->mutable_namespace_description());
} }
for (const auto& cap : capabilities_) {
policy->add_capabilities(cap);
}
} }
} // namespace sandbox2 } // namespace sandbox2

View File

@ -49,11 +49,6 @@ class Comms;
class Policy final { class Policy final {
public: public:
// Skips creation of a user namespace and keep capabilities in the global
// namespace. This only makes sense in some rare cases where the sandbox is
// started as root, please talk to sandbox-team@ before using this function.
void AllowUnsafeKeepCapabilities(std::vector<int> caps);
// Stores information about the policy (and the policy builder if existing) // Stores information about the policy (and the policy builder if existing)
// in the protobuf structure. // in the protobuf structure.
void GetPolicyDescription(PolicyDescription* policy) const; void GetPolicyDescription(PolicyDescription* policy) const;
@ -80,8 +75,6 @@ class Policy final {
namespace_ = std::move(ns); namespace_ = std::move(ns);
} }
const std::vector<int>& capabilities() const { return capabilities_; }
// Returns the default policy, which blocks certain dangerous syscalls and // Returns the default policy, which blocks certain dangerous syscalls and
// mismatched syscall tables. // mismatched syscall tables.
std::vector<sock_filter> GetDefaultPolicy() const; std::vector<sock_filter> GetDefaultPolicy() const;
@ -99,9 +92,6 @@ class Policy final {
bool collect_stacktrace_on_kill_ = true; bool collect_stacktrace_on_kill_ = true;
bool collect_stacktrace_on_exit_ = false; bool collect_stacktrace_on_exit_ = false;
// The capabilities to keep in the sandboxee.
std::vector<int> capabilities_;
// Optional pointer to a PolicyBuilder description pb object. // Optional pointer to a PolicyBuilder description pb object.
std::unique_ptr<PolicyBuilderDescription> policy_builder_description_; std::unique_ptr<PolicyBuilderDescription> policy_builder_description_;