| 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 |
| |
