diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxUtil.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxUtil.java index 659057424996d8..d10ab53b4640a4 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxUtil.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxUtil.java @@ -72,6 +72,7 @@ public static class CommandLineBuilder { private boolean createNetworkNamespace = false; private boolean useFakeRoot = false; private boolean useFakeUsername = false; + private boolean enablePseudoterminal = false; private boolean useDebugMode = false; private boolean sigintSendsSigterm = false; @@ -188,6 +189,16 @@ public CommandLineBuilder setUseFakeUsername(boolean useFakeUsername) { return this; } + /** + * Sets whether to set group to 'tty' and make /dev/pts writable inside the sandbox in order to + * enable the use of pseudoterminals. + */ + @CanIgnoreReturnValue + public CommandLineBuilder setEnablePseudoterminal(boolean enablePseudoterminal) { + this.enablePseudoterminal = enablePseudoterminal; + return this; + } + /** Sets whether to enable debug mode (e.g. to print debugging messages). */ @CanIgnoreReturnValue public CommandLineBuilder setUseDebugMode(boolean useDebugMode) { @@ -262,6 +273,9 @@ public ImmutableList build() { if (useFakeUsername) { commandLineBuilder.add("-U"); } + if (enablePseudoterminal) { + commandLineBuilder.add("-P"); + } if (useDebugMode) { commandLineBuilder.add("-D"); } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java index bf7c097036a3e7..1fe431d17a6685 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java @@ -229,6 +229,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context .setTmpfsDirectories(ImmutableSet.copyOf(getSandboxOptions().sandboxTmpfsPath)) .setBindMounts(getBindMounts(blazeDirs, sandboxExecRoot, sandboxTmp)) .setUseFakeHostname(getSandboxOptions().sandboxFakeHostname) + .setEnablePseudoterminal(getSandboxOptions().sandboxExplicitPseudoterminal) .setCreateNetworkNamespace( !(allowNetwork || Spawns.requiresNetwork( diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java index 2d3d05529df175..0a09fa75f77ba4 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java @@ -131,6 +131,18 @@ public String getTypeDescription() { help = "Change the current username to 'nobody' for sandboxed actions.") public boolean sandboxFakeUsername; + @Option( + name = "sandbox_explicit_pseudoterminal", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, + effectTags = {OptionEffectTag.EXECUTION}, + help = + "Explicitly enable the creation of pseudoterminals for sandboxed actions." + + " Some linux distributions require setting the group id of the process to 'tty'" + + " inside the sandbox in order for pseudoterminals to function. If this is" + + " causing issues, this flag can be disabled to enable other groups to be used.") + public boolean sandboxExplicitPseudoterminal; + @Option( name = "sandbox_block_path", allowMultiple = true, diff --git a/src/main/tools/linux-sandbox-options.cc b/src/main/tools/linux-sandbox-options.cc index a9a7ac552dc8b1..50ca4f79d7da45 100644 --- a/src/main/tools/linux-sandbox-options.cc +++ b/src/main/tools/linux-sandbox-options.cc @@ -72,6 +72,7 @@ static void Usage(char *program_name, const char *fmt, ...) { " -N if set, a new network namespace will be created\n" " -R if set, make the uid/gid be root\n" " -U if set, make the uid/gid be nobody\n" + " -P if set, make the gid be tty and make /dev/pts writable\n" " -D if set, debug info will be printed\n" " -h if set, chroot to sandbox-dir and only " " mount whats been specified with -M/-m for improved hermeticity. " @@ -97,7 +98,7 @@ static void ParseCommandLine(unique_ptr> args) { bool source_specified = false; while ((c = getopt(args->size(), args->data(), - ":W:T:t:il:L:w:e:M:m:S:h:HNRUD")) != -1) { + ":W:T:t:il:L:w:e:M:m:S:h:HNRUPD")) != -1) { if (c != 'M' && c != 'm') source_specified = false; switch (c) { case 'W': @@ -215,6 +216,9 @@ static void ParseCommandLine(unique_ptr> args) { } opt.fake_username = true; break; + case 'P': + opt.enable_pty = true; + break; case 'D': opt.debug = true; break; diff --git a/src/main/tools/linux-sandbox-options.h b/src/main/tools/linux-sandbox-options.h index 2843e9f8e3aa90..1766f463f5ba3f 100644 --- a/src/main/tools/linux-sandbox-options.h +++ b/src/main/tools/linux-sandbox-options.h @@ -52,6 +52,9 @@ struct Options { bool fake_root; // Set the username inside the sandbox to 'nobody' (-U) bool fake_username; + // Enable writing to /dev/pts and map the user's gid to tty to enable + // pseudoterminals (-P) + bool enable_pty; // Print debugging messages (-D) bool debug; // Improved hermetic build using whitelisting strategy (-h) diff --git a/src/main/tools/linux-sandbox-pid1.cc b/src/main/tools/linux-sandbox-pid1.cc index 703b8c32a53f64..cc7e465863c3f9 100644 --- a/src/main/tools/linux-sandbox-pid1.cc +++ b/src/main/tools/linux-sandbox-pid1.cc @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -242,6 +243,19 @@ static void SetupUserNamespace() { inner_uid = global_outer_uid; inner_gid = global_outer_gid; } + if (opt.enable_pty) { + // Change the group to "tty" regardless of what was previously set + struct group grp; + char buf[256]; + size_t buflen = sizeof(buf); + struct group *result; + getgrnam_r("tty", &grp, buf, buflen, &result); + if (result == nullptr) { + DIE("getgrnam_r"); + } + inner_gid = grp.gr_gid; + } + WriteFile("/proc/self/uid_map", "%u %u 1\n", inner_uid, global_outer_uid); WriteFile("/proc/self/gid_map", "%u %u 1\n", inner_gid, global_outer_gid); } @@ -325,6 +339,10 @@ static bool ShouldBeWritable(const std::string &mnt_dir) { return true; } + if (opt.enable_pty && mnt_dir == "/dev/pts") { + return true; + } + for (const std::string &writable_file : opt.writable_files) { if (mnt_dir == writable_file) { return true; diff --git a/src/test/shell/bazel/bazel_sandboxing_test.sh b/src/test/shell/bazel/bazel_sandboxing_test.sh index 44dc2a6cab0c54..d64f029bb11da8 100755 --- a/src/test/shell/bazel/bazel_sandboxing_test.sh +++ b/src/test/shell/bazel/bazel_sandboxing_test.sh @@ -645,6 +645,26 @@ EOF bazel test --nocache_test_results --sandbox_fake_username --test_output=errors :test || fail "test did not pass" } +# Tests that a pseudoterminal can be opened in linux when --sandbox_explicit_pseudoterminal is active +function test_can_enable_pseudoterminals() { + if [[ "$(uname -s)" != Linux ]]; then + echo "Skipping test: flag intended for linux systems" + return 0 + fi + + cat > test.py <<'EOF' +import pty +pty.openpty() +EOF + cat > BUILD <<'EOF' +py_test( + name = "test", + srcs = ["test.py"], +) +EOF + bazel test --sandbox_explicit_pseudoterminal :test || fail "test did not pass" +} + # Tests that /proc/self == /proc/$$. This should always be true unless the PID namespace is active without /proc being remounted correctly. function test_sandbox_proc_self() { if [[ ! -d /proc/self ]]; then