Drop --clearenv; control the sandbox's environment at exec time
Moves responsibility for the sandboxed command's environment out of bwrap's own --clearenv/--setenv flags and into the process-launching code itself: build_sandbox_env() (bwrap.cpp) constructs the exact environment (PATH, HOME, PWD, TERM if present), and run_bwrap() passes it to run_process_foreground()'s new optional env override, which replaces the forked child's entire environment via clearenv()/setenv() (plain POSIX, not the GNU-only execvpe(), since the target platform includes musl) right before exec. bwrap, nsenter (when interposed), and slocker-lite-priv-drop all just forward whatever environment they're launched with, so controlling it once at the outermost exec is sufficient. Also adds PWD=/ to the constructed environment: per bwrap's own man page, --clearenv never actually unset PWD in the first place (bwrap manages it separately, alongside --chdir), so the old --clearenv/ --setenv sequence was leaving it unset by omission rather than by choice. Hardcoded to "/" to match --chdir's own value. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz
This commit is contained in:
@@ -140,6 +140,19 @@ Source layout (all under `src/`):
|
|||||||
`release_session_lock()` once `run_process_foreground()` returns (covering
|
`release_session_lock()` once `run_process_foreground()` returns (covering
|
||||||
every exit path — normal, nonzero, or a forwarded-signal exit — since that call
|
every exit path — normal, nonzero, or a forwarded-signal exit — since that call
|
||||||
always blocks until the child has actually exited).
|
always blocks until the child has actually exited).
|
||||||
|
`build_bwrap_args()` no longer passes `--clearenv`/`--setenv` to `bwrap` itself;
|
||||||
|
instead, `build_sandbox_env()` builds the sandboxed command's exact environment
|
||||||
|
(`PATH`, `HOME`, `PWD` — hardcoded to `"/"`, matching `--chdir`'s own value; note
|
||||||
|
per bwrap's own man page `--clearenv` never actually unset `PWD` in the first
|
||||||
|
place, so this isn't a straight port of a prior `--setenv` — and `TERM`, only
|
||||||
|
if the host process has one) and `run_bwrap()` passes it straight to
|
||||||
|
`run_process_foreground()`'s own `env` override (see `process.{h,cpp}` below).
|
||||||
|
This works because `bwrap` (and `nsenter`, when interposed via
|
||||||
|
`wrap_for_root_namespace()`) doesn't alter its own inherited environment unless
|
||||||
|
told to, and neither does `slocker-lite-priv-drop` (just
|
||||||
|
`setgroups()`/`setgid()`/`setuid()`/`execvp()`, no env manipulation) — so
|
||||||
|
controlling it once, at the outermost exec, is sufficient for it to reach the
|
||||||
|
final sandboxed command unchanged.
|
||||||
- `priv_drop_helper.cpp` → the separate `slocker-lite-priv-drop` binary (its own
|
- `priv_drop_helper.cpp` → the separate `slocker-lite-priv-drop` binary (its own
|
||||||
`executable()` target in `meson.build`, **built with `-static`**). Deliberately
|
`executable()` target in `meson.build`, **built with `-static`**). Deliberately
|
||||||
has zero dependencies on the rest of this project (no fmt/spdlog/etc.) and is
|
has zero dependencies on the rest of this project (no fmt/spdlog/etc.) and is
|
||||||
@@ -163,9 +176,9 @@ Source layout (all under `src/`):
|
|||||||
carries `home`, looked up by the final resolved uid's `/etc/passwd` entry (field 5)
|
carries `home`, looked up by the final resolved uid's `/etc/passwd` entry (field 5)
|
||||||
regardless of whether `user` was given as a name or a number; falls back to
|
regardless of whether `user` was given as a name or a number; falls back to
|
||||||
`"/root"` for uid 0 or `"/"` otherwise when there's no matching row.
|
`"/root"` for uid 0 or `"/"` otherwise when there's no matching row.
|
||||||
`build_bwrap_args()` (`bwrap.cpp`) sets the sandboxed process's `HOME` from this —
|
`build_sandbox_env()` (`bwrap.cpp`) sets the sandboxed process's `HOME` from
|
||||||
`"/root"` only when no user override applies at all (no `--user`, no image-declared
|
this — `"/root"` only when no user override applies at all (no `--user`, no
|
||||||
`config.User`). `run_container()` (`main.cpp`) calls `resolve_user_and_group()`
|
image-declared `config.User`). `run_container()` (`main.cpp`) calls `resolve_user_and_group()`
|
||||||
with either the explicit `--user`/`--group` flags, or, when `--user` wasn't given,
|
with either the explicit `--user`/`--group` flags, or, when `--user` wasn't given,
|
||||||
the image's own declared `config.User` (`OciImageConfig::user`/`group`) — so a
|
the image's own declared `config.User` (`OciImageConfig::user`/`group`) — so a
|
||||||
container defaults to running as whatever user the image itself declares, not
|
container defaults to running as whatever user the image itself declares, not
|
||||||
@@ -185,7 +198,13 @@ Source layout (all under `src/`):
|
|||||||
itself execs into something else first (e.g. `nsenter` handing off to the final
|
itself execs into something else first (e.g. `nsenter` handing off to the final
|
||||||
command via its own in-place `execvp()` — a pid never changes across `exec()`).
|
command via its own in-place `execvp()` — a pid never changes across `exec()`).
|
||||||
`run_bwrap()` (`bwrap.cpp`) is the one caller that uses it, for session pid-file
|
`run_bwrap()` (`bwrap.cpp`) is the one caller that uses it, for session pid-file
|
||||||
tracking (see `pid_file.{h,cpp}` below).
|
tracking (see `pid_file.{h,cpp}` below). `run_process_foreground()` also takes
|
||||||
|
an optional `env` (list of key/value pairs): when set, the forked child
|
||||||
|
replaces its entire environment via `clearenv()`/`setenv()` (plain POSIX, not
|
||||||
|
the GNU-only `execvpe()` — the target platform includes musl) before `execvp()`,
|
||||||
|
instead of inheriting this process's own. `nullopt` (the default) leaves the
|
||||||
|
child's environment untouched. `run_bwrap()` is again the one caller that uses
|
||||||
|
this, via `build_sandbox_env()` (`bwrap.cpp`) — see there.
|
||||||
- `pid_file.{h,cpp}` — tracks one running `-r/--run` session (a live `bwrap`
|
- `pid_file.{h,cpp}` — tracks one running `-r/--run` session (a live `bwrap`
|
||||||
process) as a locked pid file, so an outside process (or a later
|
process) as a locked pid file, so an outside process (or a later
|
||||||
`slocker-lite` invocation) can tell whether it's still running.
|
`slocker-lite` invocation) can tell whether it's still running.
|
||||||
|
|||||||
+25
-10
@@ -64,6 +64,29 @@ std::optional<std::filesystem::path> find_priv_drop_helper() {
|
|||||||
return candidate;
|
return candidate;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Builds the exact environment the sandboxed command should see. Passed directly
|
||||||
|
// to run_process_foreground() as the environment to exec bwrap with, rather than
|
||||||
|
// relying on bwrap's own --clearenv/--setenv (which run_bwrap() no longer uses) --
|
||||||
|
// bwrap, and any nsenter/priv-drop-helper interposed ahead of it, all just forward
|
||||||
|
// whatever environment they were themselves launched with, so controlling it once
|
||||||
|
// here is sufficient. PWD is set to match --chdir's own value in
|
||||||
|
// build_bwrap_args() ("/") -- per bwrap's own man page, --clearenv never actually
|
||||||
|
// unset PWD in the first place (bwrap manages it separately, alongside --chdir),
|
||||||
|
// so this isn't a straight port of a prior --setenv, just keeping the explicitly
|
||||||
|
// constructed environment a complete, accurate match for what the sandbox should
|
||||||
|
// see.
|
||||||
|
std::vector<std::pair<std::string, std::string>> build_sandbox_env(std::optional<ResolvedUser> user) {
|
||||||
|
std::vector<std::pair<std::string, std::string>> env = {
|
||||||
|
{"PATH", "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"},
|
||||||
|
{"HOME", user ? user->home : "/root"},
|
||||||
|
{"PWD", "/"},
|
||||||
|
};
|
||||||
|
if (const char* term = std::getenv("TERM")) {
|
||||||
|
env.emplace_back("TERM", term);
|
||||||
|
}
|
||||||
|
return env;
|
||||||
|
}
|
||||||
|
|
||||||
struct NamespaceProbe {
|
struct NamespaceProbe {
|
||||||
int clone_flag;
|
int clone_flag;
|
||||||
const char* bwrap_arg;
|
const char* bwrap_arg;
|
||||||
@@ -234,22 +257,13 @@ std::vector<std::string> build_bwrap_args(const std::string& root,
|
|||||||
"/tmp",
|
"/tmp",
|
||||||
"--chdir",
|
"--chdir",
|
||||||
"/",
|
"/",
|
||||||
"--clearenv",
|
|
||||||
"--setenv",
|
|
||||||
"PATH",
|
|
||||||
"/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
|
|
||||||
};
|
};
|
||||||
args.insert(args.end(), filesystem_args.begin(), filesystem_args.end());
|
args.insert(args.end(), filesystem_args.begin(), filesystem_args.end());
|
||||||
args.insert(args.end(), {"--setenv", "HOME", user ? user->home : "/root"});
|
|
||||||
|
|
||||||
for (const auto& volume : volumes) {
|
for (const auto& volume : volumes) {
|
||||||
args.insert(args.end(), {"--bind", volume.host_directory, volume.container_path});
|
args.insert(args.end(), {"--bind", volume.host_directory, volume.container_path});
|
||||||
}
|
}
|
||||||
|
|
||||||
if (const char* term = std::getenv("TERM")) {
|
|
||||||
args.insert(args.end(), {"--setenv", "TERM", term});
|
|
||||||
}
|
|
||||||
|
|
||||||
bool bound_priv_drop_helper = false;
|
bool bound_priv_drop_helper = false;
|
||||||
if (user) {
|
if (user) {
|
||||||
auto helper_path = find_priv_drop_helper();
|
auto helper_path = find_priv_drop_helper();
|
||||||
@@ -315,7 +329,8 @@ int run_bwrap(const std::string& root, const std::vector<std::string>& command,
|
|||||||
|
|
||||||
std::optional<SessionLock> session_lock;
|
std::optional<SessionLock> session_lock;
|
||||||
int exit_code = run_process_foreground(
|
int exit_code = run_process_foreground(
|
||||||
*argv, [&](pid_t pid) { session_lock = create_session_lock(container_name, pid); });
|
*argv, [&](pid_t pid) { session_lock = create_session_lock(container_name, pid); },
|
||||||
|
build_sandbox_env(user));
|
||||||
|
|
||||||
if (session_lock) {
|
if (session_lock) {
|
||||||
release_session_lock(*session_lock);
|
release_session_lock(*session_lock);
|
||||||
|
|||||||
@@ -78,6 +78,9 @@ std::vector<std::string> build_bwrap_args(const std::string& root,
|
|||||||
// see there for when it does/doesn't take effect. While bwrap is running,
|
// see there for when it does/doesn't take effect. While bwrap is running,
|
||||||
// `container_name` (paired with its actual pid) is recorded as a locked session
|
// `container_name` (paired with its actual pid) is recorded as a locked session
|
||||||
// pid file under $XDG_STATE_HOME (see pid_file.h) -- removed again once it exits.
|
// pid file under $XDG_STATE_HOME (see pid_file.h) -- removed again once it exits.
|
||||||
|
// The sandboxed command's environment is built directly here (build_sandbox_env())
|
||||||
|
// and passed to run_process_foreground()'s own env override, rather than relying
|
||||||
|
// on bwrap's own --clearenv/--setenv (which build_bwrap_args() no longer uses).
|
||||||
// Returns bwrap's exit code, or -1 on failure to launch.
|
// Returns bwrap's exit code, or -1 on failure to launch.
|
||||||
int run_bwrap(const std::string& root, const std::vector<std::string>& command, bool use_nsenter,
|
int run_bwrap(const std::string& root, const std::vector<std::string>& command, bool use_nsenter,
|
||||||
const std::vector<ResolvedVolumeMount>& volumes, std::optional<ResolvedUser> user,
|
const std::vector<ResolvedVolumeMount>& volumes, std::optional<ResolvedUser> user,
|
||||||
|
|||||||
+8
-2
@@ -108,8 +108,8 @@ ProcessResult run_process(const std::vector<std::string>& argv) {
|
|||||||
return {exit_code, output};
|
return {exit_code, output};
|
||||||
}
|
}
|
||||||
|
|
||||||
int run_process_foreground(const std::vector<std::string>& argv,
|
int run_process_foreground(const std::vector<std::string>& argv, const std::function<void(pid_t)>& on_start,
|
||||||
const std::function<void(pid_t)>& on_start) {
|
const std::optional<std::vector<std::pair<std::string, std::string>>>& env) {
|
||||||
spdlog::debug("running external command: {}", fmt::join(argv, " "));
|
spdlog::debug("running external command: {}", fmt::join(argv, " "));
|
||||||
|
|
||||||
pid_t pid = fork();
|
pid_t pid = fork();
|
||||||
@@ -118,6 +118,12 @@ int run_process_foreground(const std::vector<std::string>& argv,
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (pid == 0) {
|
if (pid == 0) {
|
||||||
|
if (env) {
|
||||||
|
clearenv();
|
||||||
|
for (const auto& [key, value] : *env) {
|
||||||
|
setenv(key.c_str(), value.c_str(), 1);
|
||||||
|
}
|
||||||
|
}
|
||||||
auto c_argv = to_c_argv(argv);
|
auto c_argv = to_c_argv(argv);
|
||||||
execvp(c_argv[0], c_argv.data());
|
execvp(c_argv[0], c_argv.data());
|
||||||
const char* msg = "run_process_foreground: execvp failed\n";
|
const char* msg = "run_process_foreground: execvp failed\n";
|
||||||
|
|||||||
+10
-3
@@ -21,6 +21,7 @@
|
|||||||
#include <optional>
|
#include <optional>
|
||||||
#include <string>
|
#include <string>
|
||||||
#include <string_view>
|
#include <string_view>
|
||||||
|
#include <utility>
|
||||||
#include <vector>
|
#include <vector>
|
||||||
|
|
||||||
#include <sys/types.h>
|
#include <sys/types.h>
|
||||||
@@ -40,10 +41,16 @@ ProcessResult run_process(const std::vector<std::string>& argv);
|
|||||||
// succeeds and before this blocks in waitpid() -- e.g. so a caller can record the
|
// succeeds and before this blocks in waitpid() -- e.g. so a caller can record the
|
||||||
// real pid of what it just launched. This stays accurate even when argv itself
|
// real pid of what it just launched. This stays accurate even when argv itself
|
||||||
// execs into something else before the real target (e.g. nsenter handing off to
|
// execs into something else before the real target (e.g. nsenter handing off to
|
||||||
// the final command), since exec() never changes the pid. Returns the exit code,
|
// the final command), since exec() never changes the pid. If `env` is set, the
|
||||||
// or -1 if fork or exec failed (on_start is not called in that case).
|
// child replaces its entire environment with exactly these key/value pairs (via
|
||||||
|
// clearenv()/setenv(), before exec) instead of inheriting this process's own --
|
||||||
|
// e.g. run_bwrap() uses this to give the sandboxed command a minimal, controlled
|
||||||
|
// environment without relying on bwrap's own --clearenv/--setenv. Returns the
|
||||||
|
// exit code, or -1 if fork or exec failed (on_start is not called in that case).
|
||||||
int run_process_foreground(const std::vector<std::string>& argv,
|
int run_process_foreground(const std::vector<std::string>& argv,
|
||||||
const std::function<void(pid_t)>& on_start = nullptr);
|
const std::function<void(pid_t)>& on_start = nullptr,
|
||||||
|
const std::optional<std::vector<std::pair<std::string, std::string>>>& env =
|
||||||
|
std::nullopt);
|
||||||
|
|
||||||
// Searches $PATH for an executable regular file named `name`, in PATH order.
|
// Searches $PATH for an executable regular file named `name`, in PATH order.
|
||||||
// Returns its full path, or nullopt if not found.
|
// Returns its full path, or nullopt if not found.
|
||||||
|
|||||||
Reference in New Issue
Block a user