From c5af2f3f2d974f7d0d84cecab6c57444b3413b01 Mon Sep 17 00:00:00 2001 From: Philipp Wollermann Date: Fri, 7 Oct 2016 13:36:04 +0000 Subject: [PATCH] sandbox: Allow network access by default, unless a target has a "block-network" tag. To block network access, you can set the "block-network" tag on a target like this: genrule( name = "no_access_to_network", cmd = "curl http://www.bazel.io/this_will_fail", tags = [ "block-network" ], ) This is needed to fix a performance issue due to a bug in the Linux kernel: https://lkml.org/lkml/2014/8/28/656 RELNOTES[INC]: Sandboxed actions can access the network by default, unless their target has a "block-network" tag. -- MOS_MIGRATED_REVID=135470811 --- .../templates/attributes/common/tags.html | 5 ++- .../build/lib/sandbox/SandboxHelpers.java | 21 ++++------ src/test/shell/bazel/BUILD | 3 -- src/test/shell/bazel/bazel_sandboxing_test.sh | 42 +++++++++---------- 4 files changed, 32 insertions(+), 39 deletions(-) diff --git a/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html b/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html index 0e7ab18d05c220..752e9d0418c7d3 100644 --- a/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html +++ b/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html @@ -29,8 +29,9 @@ attribute has the same effect. -
  • requires-network keyword allows access to the external - network from inside the sandbox. +
  • block-network keyword blocks access to the external + network from inside the sandbox. In this case, only communication + with localhost is allowed.
    • diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java index 2f6c9092384ddd..5d5f542b1d1309 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java @@ -68,18 +68,9 @@ public static ImmutableSet getOutputFiles(Spawn spawn) { } static boolean shouldAllowNetwork(BuildRequest buildRequest, Spawn spawn) { - // If we don't run tests, allow network access. - if (!buildRequest.shouldRunTests()) { - return true; - } - - // If the Spawn specifically requests network access, allow it. - if (spawn.getExecutionInfo().containsKey("requires-network")) { - return true; - } - // Allow network access, when --java_debug is specified, otherwise we can't connect to the - // remote debug server of the test. + // remote debug server of the test. This intentionally overrides the "block-network" execution + // tag. if (buildRequest .getOptions(BuildConfiguration.Options.class) .testArguments @@ -87,7 +78,13 @@ static boolean shouldAllowNetwork(BuildRequest buildRequest, Spawn spawn) { return true; } - return false; + // If the Spawn requests to block network access, do so. + if (spawn.getExecutionInfo().containsKey("block-network")) { + return false; + } + + // Network access is allowed by default. + return true; } static void postActionStatusMessage(Executor executor, Spawn spawn) { diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index ce0efd433a3c9c..2be680b2f03f4b 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -164,9 +164,6 @@ sh_test( ":test-deps", "//src/test/shell/bazel/testdata:bazel_toolchain_test_project_pkg", ], - tags = [ - "requires-network", - ], ) # TODO(bazel-team): zip is non-deterministic because of file timestamp, diff --git a/src/test/shell/bazel/bazel_sandboxing_test.sh b/src/test/shell/bazel/bazel_sandboxing_test.sh index 685bf05c5411c3..0b796683ed2e87 100755 --- a/src/test/shell/bazel/bazel_sandboxing_test.sh +++ b/src/test/shell/bazel/bazel_sandboxing_test.sh @@ -328,17 +328,15 @@ function test_sandbox_network_access() { cat << EOF >> examples/genrule/BUILD genrule( - name = "breaks4", - outs = [ "breaks4.txt" ], + name = "sandbox_network_access", + outs = [ "sandbox_network_access.txt" ], cmd = "curl -o \$@ localhost:${nc_port}", ) EOF - bazel build examples/genrule:breaks1 &> $TEST_log \ - && fail "Non-hermetic genrule succeeded: examples/genrule:breaks4" || true - [ ! -f "${BAZEL_GENFILES_DIR}/examples/genrule/breaks4.txt" ] || { - output=$(cat "${BAZEL_GENFILES_DIR}/examples/genrule/breaks4.txt") - fail "Non-hermetic genrule breaks1 succeeded with following output: $output" - } + bazel build examples/genrule:sandbox_network_access &> $TEST_log \ + || fail "genrule 'sandbox_network_access' trying to use network failed, but should have succeeded" + [ -f "${BAZEL_GENFILES_DIR}/examples/genrule/sandbox_network_access.txt" ] \ + || fail "genrule 'sandbox_network_access' did not produce output" kill_nc } @@ -347,34 +345,34 @@ function test_sandbox_network_access_with_local() { cat << EOF >> examples/genrule/BUILD genrule( - name = "breaks4_works_with_local", - outs = [ "breaks4_works_with_local.txt" ], + name = "sandbox_network_access_with_local", + outs = [ "sandbox_network_access_with_local.txt" ], cmd = "curl -o \$@ localhost:${nc_port}", tags = [ "local" ], ) EOF - bazel build examples/genrule:breaks4_works_with_local &> $TEST_log \ - || fail "Non-hermetic genrule failed even though tags=['local']: examples/genrule:breaks4_works_with_local" - [ -f "${BAZEL_GENFILES_DIR}/examples/genrule/breaks4_works_with_local.txt" ] \ - || fail "Genrule did not produce output: examples/genrule:breaks4_works_with_local" + bazel build examples/genrule:sandbox_network_access_with_local &> $TEST_log \ + || fail "genrule 'sandbox_network_access_with_local' trying to use network failed, but should have succeeded" + [ -f "${BAZEL_GENFILES_DIR}/examples/genrule/sandbox_network_access_with_local.txt" ] \ + || fail "genrule 'sandbox_network_access_with_local' did not produce output" kill_nc } -function test_sandbox_network_access_with_requires_network() { +function test_sandbox_network_access_with_block_network() { serve_file file_to_serve cat << EOF >> examples/genrule/BUILD genrule( - name = "breaks4_works_with_requires_network", - outs = [ "breaks4_works_with_requires_network.txt" ], + name = "sandbox_network_access_with_block_network", + outs = [ "sandbox_network_access_with_block_network.txt" ], cmd = "curl -o \$@ localhost:${nc_port}", - tags = [ "requires-network" ], + tags = [ "block-network" ], ) EOF - bazel build examples/genrule:breaks4_works_with_requires_network &> $TEST_log \ - || fail "Non-hermetic genrule failed even though tags=['requires-network']: examples/genrule:breaks4_works_with_requires_network" - [ -f "${BAZEL_GENFILES_DIR}/examples/genrule/breaks4_works_with_requires_network.txt" ] \ - || fail "Genrule did not produce output: examples/genrule:breaks4_works_with_requires_network" + bazel build examples/genrule:sandbox_network_access_with_block_network &> $TEST_log \ + && fail "genrule 'sandbox_network_access_with_block_network' trying to use network succeeded, but should have failed" || true + [ ! -f "${BAZEL_GENFILES_DIR}/examples/genrule/breaks4_works_with_requires_network.txt" ] \ + || fail "genrule 'sandbox_network_access_with_block_network' produced output, but was expected to fail" kill_nc }