Implement the first stage of Bazel's "Sandbox 2.0" for Linux.
This has the following improvements upon the older one:
- Uses PID namespaces, PR_SET_PDEATHSIG and a number of other tricks for
further process isolation and 100% reliable killing of child processes.
- Uses clone() instead of unshare() to work around a Linux kernel bug that
made creating a sandbox unreliable.
- Instead of mounting a hardcoded list of paths + whatever you add with
--sandbox_add_path, this sandbox instead mounts all of /, except for what
you make inaccessible via --sandbox_block_path. This should solve the
majority of "Sandboxing breaks my build, because my compiler is installed
in /opt or /usr/local" issues that users have seen.
- Instead of doing magic with bind mounts, we create a separate execroot for
each process containing symlinks to the input files. This is simpler and
gives more predictable performance.
- Actually makes everything except the working directory read-only
(fixes #1364). This means that a running process can no longer accidentally
modify your source code (yay!).
- Prevents a number of additional "attacks" or leaks, like accidentally
inheriting file handles from the parent.
- Simpler command-line interface.
- We can provide the same semantics in a Mac OS X sandbox, which will come in
a separate code review from yueg@.
It has the following caveats / known issues:
- The "fallback to /bin/bash on error" feature is gone, but now that the
sandbox mounts everything by default, the main use-case for this is no
longer needed.
The following improvements are planned:
- Use a FUSE filesystem if possible for the new execroot, instead of creating
symlinks.
- Mount a base image instead of "/".
FAQ:
Q: Why is mounting all of "/" okay, doesn't this make the whole sandbox
useless?
A: This is still a reasonable behavior, because the sandbox never tried to
isolate your build from the operating system it runs in. Instead it is
supposed to protect your data from a test running "rm -rf $HOME" and to
make it difficult / impossible for actions to use input files that are not
declared dependencies. For even more isolation the sandbox will support
mounting a base image as its root in a future version (similar to Docker
images).
Q: Let's say my process-specific execroot contains a symlink to an input file
"good.h", can't the process just resolve the symlink, strip off the file
name and then look around in the workspace?
A: Yes. Unfortunately we could not find any way on Linux to make a file appear
in a different directory with *all* of the semantics we would like. The
options investigated were:
1) Copying input files, which is much too slow.
2) Hard linking input files, which is fast, but doesn't work cross-
filesystems and it's also not possible to make them read-only.
3) Bind mounts, which don't scale once you're up in the thousands of input
files (across all actions) - it seems like the kernel has some
non-linear performance behavior when the mount table grows too much,
resulting in the mount syscall taking more time the more mounts you
have.
4) FUSE filesystem, good in theory, but wasn't ready for the first
iteration.
RELNOTES: New sandboxing implementation for Linux in which all actions run in a separate execroot that contains input files as symlinks back to the originals in the workspace. The running action now has read-write access to its execroot and /tmp only and can no longer write in arbitrary other places in the file system.
--
Change-Id: Ic91386fc92f8eef727ed6d22e6bd0f357d145063
Reviewed-on: https://bazel-review.googlesource.com/#/c/4053
MOS_MIGRATED_REVID=130638204
diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategyTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategyTest.java
index b2fe0e6..182a62d 100644
--- a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategyTest.java
@@ -14,220 +14,27 @@
package com.google.devtools.build.lib.sandbox;
import static com.google.common.truth.Truth.assertThat;
-import static org.junit.Assert.fail;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
-
+import java.nio.charset.Charset;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.TreeMap;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-import java.io.IOException;
-import java.nio.charset.Charset;
-import java.util.ArrayList;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
-
/**
* Tests for {@code LinuxSandboxedStrategy}.
- *
- * <p>The general idea for each test is to provide a file tree consisting of symlinks, directories
- * and empty files and then handing that together with an arbitrary number of input files (what
- * would be specified in the "srcs" attribute, for example) to the LinuxSandboxedStrategy.
- *
- * <p>The algorithm that processes the mounts must then always find (and thus mount) the expected
- * tree of files given only the set of input files.
*/
@RunWith(JUnit4.class)
public class LinuxSandboxedStrategyTest extends LinuxSandboxedStrategyTestCase {
- /**
- * Strips the working directory (which can be very long) from the file names in the input map, to
- * make assertion failures easier to read.
- */
- private ImmutableMap<String, String> userFriendlyMap(Map<Path, Path> input) {
- ImmutableMap.Builder<String, String> userFriendlyMap = ImmutableMap.builder();
- for (Entry<Path, Path> entry : input.entrySet()) {
- String key = entry.getKey().getPathString().replace(workspaceDir.getPathString(), "");
- String value = entry.getValue().getPathString().replace(workspaceDir.getPathString(), "");
- userFriendlyMap.put(key, value);
- }
- return userFriendlyMap.build();
- }
-
- /**
- * Takes a map of file specifications, creates the necessary files / symlinks / dirs,
- * mounts files listed in customMount at their canonical location in the sandbox and returns the
- * output of {@code LinuxSandboxedStrategy#fixMounts} for it.
- */
- private ImmutableMap<String, String> userFriendlyMounts(
- Map<String, String> linksAndFiles, List<String> customMounts) throws Exception {
- return userFriendlyMap(mounts(linksAndFiles, customMounts));
- }
-
- private ImmutableMap<Path, Path> mounts(
- Map<String, String> linksAndFiles, List<String> customMounts) throws Exception {
- createTreeStructure(linksAndFiles);
-
- ImmutableMap.Builder<Path, Path> mounts = ImmutableMap.builder();
- for (String customMount : customMounts) {
- Path customMountPath = workspaceDir.getRelative(customMount);
- mounts.put(customMountPath, customMountPath);
- }
- return ImmutableMap.copyOf(LinuxSandboxedStrategy.finalizeMounts(mounts.build()));
- }
-
- /**
- * Takes a map of file specifications, creates the necessary files / symlinks / dirs,
- * mounts the first file of the specification at its canonical location in the sandbox and returns
- * the output of {@code LinuxSandboxedStrategy#fixMounts} for it.
- */
- private Map<String, String> userFriendlyMounts(Map<String, String> linksAndFiles)
- throws Exception {
- return userFriendlyMap(mounts(linksAndFiles));
- }
-
- private Map<Path, Path> mounts(Map<String, String> linksAndFiles) throws Exception {
- return mounts(
- linksAndFiles, ImmutableList.of(Iterables.getFirst(linksAndFiles.keySet(), null)));
- }
-
- /**
- * Returns a map of mount entries for a list files, which can be used to assert that all
- * expected mounts have been made by the LinuxSandboxedStrategy.
- */
- private ImmutableMap<String, String> userFriendlyAsserts(List<String> asserts) {
- return userFriendlyMap(asserts(asserts));
- }
-
- private ImmutableMap<Path, Path> asserts(List<String> asserts) {
- ImmutableMap.Builder<Path, Path> pathifiedAsserts = ImmutableMap.builder();
- for (String fileName : asserts) {
- Path inputPath = workspaceDir.getRelative(fileName);
- pathifiedAsserts.put(inputPath, inputPath);
- }
- return pathifiedAsserts.build();
- }
-
- private void createTreeStructure(Map<String, String> linksAndFiles) throws Exception {
- for (Entry<String, String> entry : linksAndFiles.entrySet()) {
- Path filePath = workspaceDir.getRelative(entry.getKey());
- String linkTarget = entry.getValue();
-
- FileSystemUtils.createDirectoryAndParents(filePath.getParentDirectory());
-
- if (!linkTarget.isEmpty()) {
- filePath.createSymbolicLink(new PathFragment(linkTarget));
- } else if (filePath.getPathString().endsWith("/")) {
- filePath.createDirectory();
- } else {
- FileSystemUtils.createEmptyFile(filePath);
- }
- }
- }
-
- @Test
- public void testResolvesRelativeFileToFileSymlinkInSameDir() throws Exception {
- Map<String, String> testFiles = new LinkedHashMap<>();
- testFiles.put("symlink.txt", "goal.txt");
- testFiles.put("goal.txt", "");
-
- List<String> assertMounts = new ArrayList<>();
- assertMounts.add("symlink.txt");
- assertMounts.add("goal.txt");
-
- assertThat(userFriendlyMounts(testFiles)).isEqualTo(userFriendlyAsserts(assertMounts));
- }
-
- @Test
- public void testResolvesRelativeFileToFileSymlinkInSubDir() throws Exception {
- Map<String, String> testFiles =
- ImmutableMap.of(
- "symlink.txt", "x/goal.txt",
- "x/goal.txt", "");
-
- List<String> assertMounts = ImmutableList.of("symlink.txt", "x/goal.txt");
- assertThat(userFriendlyMounts(testFiles)).isEqualTo(userFriendlyAsserts(assertMounts));
- }
-
- @Test
- public void testResolvesRelativeFileToFileSymlinkInParentDir() throws Exception {
- Map<String, String> testFiles =
- ImmutableMap.of(
- "x/symlink.txt", "../goal.txt",
- "goal.txt", "");
-
- List<String> assertMounts = ImmutableList.of("x/symlink.txt", "goal.txt");
-
- assertThat(userFriendlyMounts(testFiles)).isEqualTo(userFriendlyAsserts(assertMounts));
- }
-
- @Test
- public void testRecursesSubDirs() throws Exception {
- ImmutableList<String> inputFile = ImmutableList.of("a/b");
-
- Map<String, String> testFiles =
- ImmutableMap.of(
- "a/b/x.txt", "",
- "a/b/y.txt", "z.txt",
- "a/b/z.txt", "");
-
- List<String> assertMounts = ImmutableList.of("a/b/x.txt", "a/b/y.txt", "a/b/z.txt");
-
- assertThat(userFriendlyMounts(testFiles, inputFile))
- .isEqualTo(userFriendlyAsserts(assertMounts));
- }
-
- /**
- * Test that the algorithm correctly identifies and refuses symlink loops.
- */
- @Test
- public void testCatchesSymlinkLoop() throws Exception {
- try {
- mounts(
- ImmutableMap.of(
- "a", "b",
- "b", "a"));
- fail();
- } catch (IOException e) {
- assertThat(e)
- .hasMessage(
- String.format(
- "%s (Too many levels of symbolic links)",
- workspaceDir.getRelative("a").getPathString()));
- }
- }
-
- /**
- * Test that the algorithm correctly detects and refuses symlinks whose subcomponents are not all
- * directories (e.g. "a -> dir/file/file").
- */
- @Test
- public void testCatchesIllegalSymlink() throws Exception {
- try {
- mounts(
- ImmutableMap.of(
- "b", "a/c",
- "a", ""));
- fail();
- } catch (IOException e) {
- assertThat(e)
- .hasMessage(
- String.format(
- "%s (Not a directory)", workspaceDir.getRelative("a/c").getPathString()));
- }
- }
-
@Test
public void testParseManifestFile() throws Exception {
- Path targetDir = workspaceDir.getRelative("runfiles");
- targetDir.createDirectory();
+ PathFragment targetDir = new PathFragment("runfiles");
Path testFile = workspaceDir.getRelative("testfile");
FileSystemUtils.createEmptyFile(testFile);
@@ -238,23 +45,22 @@
Charset.defaultCharset(),
String.format("x/testfile %s\nx/emptyfile \n", testFile.getPathString()));
- Map mounts =
- LinuxSandboxedStrategy.parseManifestFile(targetDir, manifestFile.getPathFile(), false, "");
+ Map<PathFragment, Path> mounts = new TreeMap<>();
+ LinuxSandboxedStrategy.parseManifestFile(
+ fileSystem, mounts, targetDir, manifestFile.getPathFile(), false, "");
- assertThat(userFriendlyMap(mounts))
+ assertThat(mounts)
.isEqualTo(
- userFriendlyMap(
- ImmutableMap.of(
- fileSystem.getPath("/runfiles/x/testfile"),
- testFile,
- fileSystem.getPath("/runfiles/x/emptyfile"),
- fileSystem.getPath("/dev/null"))));
+ ImmutableMap.of(
+ new PathFragment("runfiles/x/testfile"),
+ testFile,
+ new PathFragment("runfiles/x/emptyfile"),
+ fileSystem.getPath("/dev/null")));
}
@Test
public void testParseFilesetManifestFile() throws Exception {
- Path targetDir = workspaceDir.getRelative("fileset");
- targetDir.createDirectory();
+ PathFragment targetDir = new PathFragment("fileset");
Path testFile = workspaceDir.getRelative("testfile");
FileSystemUtils.createEmptyFile(testFile);
@@ -265,12 +71,10 @@
Charset.defaultCharset(),
String.format("workspace/x/testfile %s\n0\n", testFile.getPathString()));
- Map mounts =
- LinuxSandboxedStrategy.parseManifestFile(
- targetDir, manifestFile.getPathFile(), true, "workspace");
+ Map<PathFragment, Path> mounts = new HashMap<>();
+ LinuxSandboxedStrategy.parseManifestFile(
+ fileSystem, mounts, targetDir, manifestFile.getPathFile(), true, "workspace");
- assertThat(userFriendlyMap(mounts))
- .isEqualTo(
- userFriendlyMap(ImmutableMap.of(fileSystem.getPath("/fileset/x/testfile"), testFile)));
+ assertThat(mounts).isEqualTo(ImmutableMap.of(new PathFragment("fileset/x/testfile"), testFile));
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategyTestCase.java b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategyTestCase.java
index b97e396..a80074a 100644
--- a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategyTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategyTestCase.java
@@ -36,10 +36,8 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.util.FileSystems;
import com.google.devtools.common.options.OptionsParser;
-
-import org.junit.Before;
-
import java.io.IOException;
+import org.junit.Before;
/**
* Common parts of all {@link LinuxSandboxedStrategy} tests.
@@ -50,7 +48,6 @@
protected FileSystem fileSystem;
protected Path workspaceDir;
- protected Path fakeSandboxDir;
protected BlazeExecutor executor;
protected BlazeDirectories blazeDirs;
@@ -75,9 +72,6 @@
outputBase = testRoot.getRelative("outputBase");
outputBase.createDirectory();
- fakeSandboxDir = testRoot.getRelative("sandbox");
- fakeSandboxDir.createDirectory();
-
blazeDirs = new BlazeDirectories(outputBase, outputBase, workspaceDir, "mock-product-name");
BlazeTestUtils.getIntegrationBinTools(blazeDirs);
@@ -101,7 +95,6 @@
"",
new LinuxSandboxedStrategy(
optionsParser.getOptions(SandboxOptions.class),
- ImmutableMap.<String, String>of(),
blazeDirs,
MoreExecutors.newDirectExecutorService(),
true,
diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/MountMapTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/MountMapTest.java
deleted file mode 100644
index 060eb6f..0000000
--- a/src/test/java/com/google/devtools/build/lib/sandbox/MountMapTest.java
+++ /dev/null
@@ -1,110 +0,0 @@
-// Copyright 2015 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.lib.sandbox;
-
-import static com.google.common.truth.Truth.assertThat;
-import static org.junit.Assert.fail;
-
-import com.google.common.collect.ImmutableMap;
-
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-import java.io.IOException;
-
-/**
- * Tests for {@code MountMap}.
- */
-@RunWith(JUnit4.class)
-public class MountMapTest extends LinuxSandboxedStrategyTestCase {
- @Test
- public void testMountMapWithNormalMounts() throws IOException {
- // Allowed: Just two normal mounts (a -> sandbox/a, b -> sandbox/b)
- MountMap mounts = new MountMap();
- mounts.put(fileSystem.getPath("/a"), workspaceDir.getRelative("a"));
- mounts.put(fileSystem.getPath("/b"), workspaceDir.getRelative("b"));
- assertThat(mounts)
- .isEqualTo(
- ImmutableMap.of(
- fileSystem.getPath("/a"), workspaceDir.getRelative("a"),
- fileSystem.getPath("/b"), workspaceDir.getRelative("b")));
- }
-
- @Test
- public void testMountMapWithSameMountTwice() throws IOException {
- // Allowed: Mount same thing twice (a -> sandbox/a, a -> sandbox/a, b -> sandbox/b)
- MountMap mounts = new MountMap();
- mounts.put(fileSystem.getPath("/a"), workspaceDir.getRelative("a"));
- mounts.put(fileSystem.getPath("/a"), workspaceDir.getRelative("a"));
- mounts.put(fileSystem.getPath("/b"), workspaceDir.getRelative("b"));
- assertThat(mounts)
- .isEqualTo(
- ImmutableMap.of(
- fileSystem.getPath("/a"), workspaceDir.getRelative("a"),
- fileSystem.getPath("/b"), workspaceDir.getRelative("b")));
- }
-
- @Test
- public void testMountMapWithOneThingTwoTargets() throws IOException {
- // Allowed: Mount one thing in two targets (x -> sandbox/a, x -> sandbox/b)
- MountMap mounts = new MountMap();
- mounts.put(fileSystem.getPath("/a"), workspaceDir.getRelative("x"));
- mounts.put(fileSystem.getPath("/b"), workspaceDir.getRelative("x"));
- assertThat(mounts)
- .isEqualTo(
- ImmutableMap.of(
- fileSystem.getPath("/a"), workspaceDir.getRelative("x"),
- fileSystem.getPath("/b"), workspaceDir.getRelative("x")));
- }
-
- @Test
- public void testMountMapWithTwoThingsOneTarget() throws IOException {
- // Forbidden: Mount two things onto the same target (x -> sandbox/a, y -> sandbox/a)
- try {
- MountMap mounts = new MountMap();
- mounts.put(fileSystem.getPath("/x"), workspaceDir.getRelative("a"));
- mounts.put(fileSystem.getPath("/x"), workspaceDir.getRelative("b"));
- fail();
- } catch (IllegalArgumentException e) {
- assertThat(e)
- .hasMessage(
- String.format(
- "Cannot mount both '%s' and '%s' onto '%s'",
- workspaceDir.getRelative("a"),
- workspaceDir.getRelative("b"),
- fileSystem.getPath("/x")));
- }
- }
-
- @Test
- public void testMountMapGuaranteesOrdering() {
- MountMap mounts = new MountMap();
- mounts.put(fileSystem.getPath("/a/c"), workspaceDir.getRelative("x"));
- mounts.put(fileSystem.getPath("/b"), workspaceDir.getRelative("x"));
- mounts.put(fileSystem.getPath("/a/b"), workspaceDir.getRelative("x"));
- mounts.put(fileSystem.getPath("/a"), workspaceDir.getRelative("x"));
-
- assertThat(mounts.entrySet())
- .containsExactlyElementsIn(
- ImmutableMap.builder()
- .put(fileSystem.getPath("/a"), workspaceDir.getRelative("x"))
- .put(fileSystem.getPath("/a/b"), workspaceDir.getRelative("x"))
- .put(fileSystem.getPath("/a/c"), workspaceDir.getRelative("x"))
- .put(fileSystem.getPath("/b"), workspaceDir.getRelative("x"))
- .build()
- .entrySet())
- .inOrder();
- }
-}
diff --git a/src/test/java/com/google/devtools/build/lib/unix/NativePosixFilesTest.java b/src/test/java/com/google/devtools/build/lib/unix/NativePosixFilesTest.java
index 46522a7..9620220 100644
--- a/src/test/java/com/google/devtools/build/lib/unix/NativePosixFilesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/unix/NativePosixFilesTest.java
@@ -23,15 +23,14 @@
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.UnixFileSystem;
-
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.IOException;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-import java.io.File;
-import java.io.FileNotFoundException;
-
/**
* This class tests the FilesystemUtils class.
*/
@@ -104,9 +103,12 @@
File foo = new File("/bin");
try {
NativePosixFiles.setWritable(foo);
- fail("Expected FilePermissionException, but wasn't thrown.");
+ fail("Expected FilePermissionException or IOException, but wasn't thrown.");
} catch (FilePermissionException e) {
assertThat(e).hasMessage(foo + " (Operation not permitted)");
+ } catch (IOException e) {
+ // When running in a sandbox, /bin might actually be a read-only file system.
+ assertThat(e).hasMessage(foo + " (Read-only file system)");
}
}
}
