From 6be39215a36a77ef6c1a9256cf799c5ee38b75ae Mon Sep 17 00:00:00 2001
From: Nobel Barakat <nobelbarakat@google.com>
Date: Wed, 26 Apr 2023 10:11:18 -0700
Subject: [PATCH] Revert "Add default sysctls to allow ping sockets and
 privileged ports with no capabilities"

This reverts commit dae652e2e5e47d99c8febd5bc81df0a3269beb74.
---
 daemon/oci_linux.go                         | 26 ----------
 daemon/oci_linux_test.go                    | 53 +--------------------
 integration-cli/docker_cli_run_unix_test.go | 15 +-----
 3 files changed, 3 insertions(+), 91 deletions(-)

diff --git a/daemon/oci_linux.go b/daemon/oci_linux.go
index 7b9a5dd03e..cb0571e176 100644
--- a/daemon/oci_linux.go
+++ b/daemon/oci_linux.go
@@ -700,14 +700,6 @@ func WithMounts(daemon *Daemon, c *container.Container) coci.SpecOpts {
 	}
 }
 
-// sysctlExists checks if a sysctl exists; runc will error if we add any that do not actually
-// exist, so do not add the default ones if running on an old kernel.
-func sysctlExists(s string) bool {
-	f := filepath.Join("/proc", "sys", strings.Replace(s, ".", "/", -1))
-	_, err := os.Stat(f)
-	return err == nil
-}
-
 // WithCommonOptions sets common docker options
 func WithCommonOptions(daemon *Daemon, c *container.Container) coci.SpecOpts {
 	return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
@@ -760,24 +752,6 @@ func WithCommonOptions(daemon *Daemon, c *container.Container) coci.SpecOpts {
 		s.Hostname = c.Config.Hostname
 		setLinuxDomainname(c, s)
 
-		// Add default sysctls that are generally safe and useful; currently we
-		// grant the capabilities to allow these anyway. You can override if
-		// you want to restore the original behaviour.
-		// We do not set network sysctls if network namespace is host, or if we are
-		// joining an existing namespace, only if we create a new net namespace.
-		if c.HostConfig.NetworkMode.IsPrivate() {
-			// We cannot set up ping socket support in a user namespace
-			userNS := daemon.configStore.RemappedRoot != "" && c.HostConfig.UsernsMode.IsPrivate()
-			if !userNS && !sys.RunningInUserNS() && sysctlExists("net.ipv4.ping_group_range") {
-				// allow unprivileged ICMP echo sockets without CAP_NET_RAW
-				s.Linux.Sysctl["net.ipv4.ping_group_range"] = "0 2147483647"
-			}
-			// allow opening any port less than 1024 without CAP_NET_BIND_SERVICE
-			if sysctlExists("net.ipv4.ip_unprivileged_port_start") {
-				s.Linux.Sysctl["net.ipv4.ip_unprivileged_port_start"] = "0"
-			}
-		}
-
 		return nil
 	}
 }
diff --git a/daemon/oci_linux_test.go b/daemon/oci_linux_test.go
index 37d3cb3589..fbfcad281c 100644
--- a/daemon/oci_linux_test.go
+++ b/daemon/oci_linux_test.go
@@ -117,8 +117,7 @@ func TestSysctlOverride(t *testing.T) {
 			Domainname: "baz.cyphar.com",
 		},
 		HostConfig: &containertypes.HostConfig{
-			NetworkMode: "bridge",
-			Sysctls:     map[string]string{},
+			Sysctls: map[string]string{},
 		},
 	}
 	d := setupFakeDaemon(t, c)
@@ -129,65 +128,15 @@ func TestSysctlOverride(t *testing.T) {
 	assert.NilError(t, err)
 	assert.Equal(t, s.Hostname, "foobar")
 	assert.Equal(t, s.Linux.Sysctl["kernel.domainname"], c.Config.Domainname)
-	if sysctlExists("net.ipv4.ip_unprivileged_port_start") {
-		assert.Equal(t, s.Linux.Sysctl["net.ipv4.ip_unprivileged_port_start"], "0")
-	}
-	if sysctlExists("net.ipv4.ping_group_range") {
-		assert.Equal(t, s.Linux.Sysctl["net.ipv4.ping_group_range"], "0 2147483647")
-	}
 
 	// Set an explicit sysctl.
 	c.HostConfig.Sysctls["kernel.domainname"] = "foobar.net"
 	assert.Assert(t, c.HostConfig.Sysctls["kernel.domainname"] != c.Config.Domainname)
-	c.HostConfig.Sysctls["net.ipv4.ip_unprivileged_port_start"] = "1024"
 
 	s, err = d.createSpec(c)
 	assert.NilError(t, err)
 	assert.Equal(t, s.Hostname, "foobar")
 	assert.Equal(t, s.Linux.Sysctl["kernel.domainname"], c.HostConfig.Sysctls["kernel.domainname"])
-	assert.Equal(t, s.Linux.Sysctl["net.ipv4.ip_unprivileged_port_start"], c.HostConfig.Sysctls["net.ipv4.ip_unprivileged_port_start"])
-
-	// Ensure the ping_group_range is not set on a daemon with user-namespaces enabled
-	d.configStore.RemappedRoot = "dummy:dummy"
-	s, err = d.createSpec(c)
-	assert.NilError(t, err)
-	_, ok := s.Linux.Sysctl["net.ipv4.ping_group_range"]
-	assert.Assert(t, !ok)
-
-	// Ensure the ping_group_range is set on a container in "host" userns mode
-	// on a daemon with user-namespaces enabled
-	c.HostConfig.UsernsMode = "host"
-	s, err = d.createSpec(c)
-	assert.NilError(t, err)
-	assert.Equal(t, s.Linux.Sysctl["net.ipv4.ping_group_range"], "0 2147483647")
-}
-
-// TestSysctlOverrideHost ensures that any implicit network sysctls are not set
-// with host networking
-func TestSysctlOverrideHost(t *testing.T) {
-	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
-	c := &container.Container{
-		Config: &containertypes.Config{},
-		HostConfig: &containertypes.HostConfig{
-			NetworkMode: "host",
-			Sysctls:     map[string]string{},
-		},
-	}
-	d := setupFakeDaemon(t, c)
-	defer cleanupFakeContainer(c)
-
-	// Ensure that the implicit sysctl is not set
-	s, err := d.createSpec(c)
-	assert.NilError(t, err)
-	assert.Equal(t, s.Linux.Sysctl["net.ipv4.ip_unprivileged_port_start"], "")
-	assert.Equal(t, s.Linux.Sysctl["net.ipv4.ping_group_range"], "")
-
-	// Set an explicit sysctl.
-	c.HostConfig.Sysctls["net.ipv4.ip_unprivileged_port_start"] = "1024"
-
-	s, err = d.createSpec(c)
-	assert.NilError(t, err)
-	assert.Equal(t, s.Linux.Sysctl["net.ipv4.ip_unprivileged_port_start"], c.HostConfig.Sysctls["net.ipv4.ip_unprivileged_port_start"])
 }
 
 func TestGetSourceMount(t *testing.T) {
diff --git a/integration-cli/docker_cli_run_unix_test.go b/integration-cli/docker_cli_run_unix_test.go
index f11a8891a3..5b69f61928 100644
--- a/integration-cli/docker_cli_run_unix_test.go
+++ b/integration-cli/docker_cli_run_unix_test.go
@@ -1233,23 +1233,12 @@ func (s *DockerSuite) TestUserNoEffectiveCapabilitiesNetBindService(c *testing.T
 	// test that a root user has default capability CAP_NET_BIND_SERVICE
 	dockerCmd(c, "run", "syscall-test", "socket-test")
 	// test that non root user does not have default capability CAP_NET_BIND_SERVICE
-	// as we allow this via sysctl, also tweak the sysctl back to default
-	args := []string{"run", "--user", "1000:1000"}
-	if sysctlExists("net.ipv4.ip_unprivileged_port_start") {
-		args = append(args, "--sysctl", "net.ipv4.ip_unprivileged_port_start=1024")
-	}
-	args = append(args, "syscall-test", "socket-test")
-	icmd.RunCommand(dockerBinary, args...).Assert(c, icmd.Expected{
+	icmd.RunCommand(dockerBinary, "run", "--user", "1000:1000", "syscall-test", "socket-test").Assert(c, icmd.Expected{
 		ExitCode: 1,
 		Err:      "Permission denied",
 	})
 	// test that root user can drop default capability CAP_NET_BIND_SERVICE
-	args = []string{"run", "--cap-drop", "net_bind_service"}
-	if sysctlExists("net.ipv4.ip_unprivileged_port_start") {
-		args = append(args, "--sysctl", "net.ipv4.ip_unprivileged_port_start=1024")
-	}
-	args = append(args, "syscall-test", "socket-test")
-	icmd.RunCommand(dockerBinary, args...).Assert(c, icmd.Expected{
+	icmd.RunCommand(dockerBinary, "run", "--cap-drop", "net_bind_service", "syscall-test", "socket-test").Assert(c, icmd.Expected{
 		ExitCode: 1,
 		Err:      "Permission denied",
 	})
-- 
2.40.1.495.gc816e09b53d-goog