Add flag to allow a subset of private IPs (#8355)

Context: https://buildbuddy-corp.slack.com/archives/C01D5GHRJ59/p1739204423929709?thread_ts=1738948035.930359&cid=C01D5GHRJ59 The new INPUT rule breaks testing connections between actions and locally running servers.
buildbuddy-io · Feb 10, 2025 · 2e4a7f4 · 2e4a7f4
1 parent 8bb802b
commit 2e4a7f4
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 6 deletions.
diff --git a/server/util/networking/BUILD b/server/util/networking/BUILD
@@ -8,6 +8,7 @@ go_library(
     deps = [
         "//server/util/alert",
         "//server/util/background",
+        "//server/util/flag",
         "//server/util/log",
         "//server/util/random",
         "//server/util/status",
@@ -33,6 +34,7 @@ go_test(
     deps = [
         ":networking",
         "//server/testutil/testnetworking",
+        "//server/util/testing/flags",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
         "@org_golang_x_sync//errgroup",

diff --git a/server/util/networking/networking.go b/server/util/networking/networking.go
@@ -3,7 +3,6 @@ package networking
 import (
 	"bufio"
 	"context"
-	"flag"
 	"fmt"
 	"net"
 	"net/netip"
@@ -19,6 +18,7 @@ import (
 
 	"github.com/buildbuddy-io/buildbuddy/server/util/alert"
 	"github.com/buildbuddy-io/buildbuddy/server/util/background"
+	"github.com/buildbuddy-io/buildbuddy/server/util/flag"
 	"github.com/buildbuddy-io/buildbuddy/server/util/log"
 	"github.com/buildbuddy-io/buildbuddy/server/util/random"
 	"github.com/buildbuddy-io/buildbuddy/server/util/status"
@@ -34,6 +34,7 @@ var (
 	natSourcePortRange            = flag.String("executor.nat_source_port_range", "", "If set, restrict the source ports for NATed traffic to this range. ")
 	networkLockDir                = flag.String("executor.network_lock_directory", "", "If set, use this directory to store lockfiles for allocated IP ranges. This is required if running multiple executors within the same networking environment.")
 	taskIPRange                   = flag.String("executor.task_ip_range", "192.168.0.0/16", "Subnet to allocate IP addresses from for actions that require network access. Must be a /16 range.")
+	taskAllowedPrivateIPs         = flag.Slice("executor.task_allowed_private_ips", []string{}, "Allowed private IPs that should be reachable from actions: either 'default', an IP address, or IP range. Private IP ranges as defined in RFC1918 are otherwise blocked.")
 
 	// Private IP ranges, as defined in RFC1918.
 	PrivateIPRanges = []string{"10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "169.254.0.0/16"}
@@ -609,6 +610,17 @@ func setupVethPair(ctx context.Context, netns *Namespace) (_ *vethPair, err erro
 	}
 
 	var iptablesRules [][]string
+	for _, allow := range *taskAllowedPrivateIPs {
+		if allow == "default" {
+			defaultIP, err := DefaultIP(ctx)
+			if err != nil {
+				return nil, fmt.Errorf("find default IP: %w", err)
+			}
+			allow = defaultIP.String()
+		}
+		iptablesRules = append(iptablesRules, []string{"FORWARD", "-i", vp.hostDevice, "-d", allow, "-j", "ACCEPT"})
+		iptablesRules = append(iptablesRules, []string{"INPUT", "-i", vp.hostDevice, "-d", allow, "-j", "ACCEPT"})
+	}
 	for _, r := range PrivateIPRanges {
 		iptablesRules = append(iptablesRules, []string{"FORWARD", "-i", vp.hostDevice, "-d", r, "-j", "REJECT"})
 		iptablesRules = append(iptablesRules, []string{"INPUT", "-i", vp.hostDevice, "-d", r, "-j", "REJECT"})

diff --git a/server/util/networking/networking_test.go b/server/util/networking/networking_test.go
@@ -14,6 +14,7 @@ import (
 
 	"github.com/buildbuddy-io/buildbuddy/server/testutil/testnetworking"
 	"github.com/buildbuddy-io/buildbuddy/server/util/networking"
+	"github.com/buildbuddy-io/buildbuddy/server/util/testing/flags"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sync/errgroup"
@@ -118,12 +119,8 @@ func TestConcurrentSetupAndCleanup(t *testing.T) {
 func TestContainerNetworking(t *testing.T) {
 	testnetworking.Setup(t)
 
-	// Enable IP forwarding and masquerading so that we can test external
-	// traffic.
 	ctx := context.Background()
-	err := os.WriteFile("/proc/sys/net/ipv4/ip_forward", []byte("1\n"), 0)
-	require.NoError(t, err)
-	err = networking.EnableMasquerading(ctx)
+	err := networking.EnableMasquerading(ctx)
 	require.NoError(t, err)
 
 	defaultIP, err := networking.DefaultIP(ctx)
@@ -189,6 +186,22 @@ func TestContainerNetworking(t *testing.T) {
 	}
 }
 
+func TestAllowTrafficToHostDefaultIP(t *testing.T) {
+	testnetworking.Setup(t)
+	flags.Set(t, "executor.task_allowed_private_ips", []string{"default"})
+	ctx := context.Background()
+	err := networking.EnableMasquerading(ctx)
+	require.NoError(t, err)
+	defaultIP, err := networking.DefaultIP(ctx)
+	require.NoError(t, err)
+
+	// Create container network
+	c1 := createContainerNetwork(ctx, t)
+
+	// Should be able to reach host's default IP when flag is enabled
+	netnsExec(t, c1.NamespacePath(), fmt.Sprintf("ping -c 1 -W 1 %s", defaultIP))
+}
+
 func TestContainerNetworkPool(t *testing.T) {
 	testnetworking.Setup(t)