Merge pull request #43641 from thaJeztah/fix_hostconfig_validation

Fix validation of IpcMode, PidMode, UTSMode, CgroupnsMode
moby · May 25, 2022 · 262f574 · 262f574
2 parents b01dd1e + c3d7a0c
commit 262f574
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 8 deletions.
diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go
@@ -660,9 +660,18 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes.
 		return warnings, err
 	}
 
+	if !hostConfig.IpcMode.Valid() {
+		return warnings, errors.Errorf("invalid IPC mode: %v", hostConfig.IpcMode)
+	}
+	if !hostConfig.PidMode.Valid() {
+		return warnings, errors.Errorf("invalid PID mode: %v", hostConfig.PidMode)
+	}
 	if hostConfig.ShmSize < 0 {
 		return warnings, fmt.Errorf("SHM size can not be less than 0")
 	}
+	if !hostConfig.UTSMode.Valid() {
+		return warnings, errors.Errorf("invalid UTS mode: %v", hostConfig.UTSMode)
+	}
 
 	if hostConfig.OomScoreAdj < -1000 || hostConfig.OomScoreAdj > 1000 {
 		return warnings, fmt.Errorf("Invalid value %d, range for oom score adj is [-1000, 1000]", hostConfig.OomScoreAdj)

diff --git a/daemon/oci_linux.go b/daemon/oci_linux.go
@@ -18,6 +18,7 @@ import (
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/container"
 	dconfig "github.com/docker/docker/daemon/config"
+	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/oci"
 	"github.com/docker/docker/oci/caps"
 	"github.com/docker/docker/pkg/idtools"
@@ -260,12 +261,15 @@ func WithNamespaces(daemon *Daemon, c *container.Container) coci.SpecOpts {
 
 		// ipc
 		ipcMode := c.HostConfig.IpcMode
+		if !ipcMode.Valid() {
+			return errdefs.InvalidParameter(errors.Errorf("invalid IPC mode: %v", ipcMode))
+		}
 		switch {
 		case ipcMode.IsContainer():
 			ns := specs.LinuxNamespace{Type: "ipc"}
 			ic, err := daemon.getIpcContainer(ipcMode.Container())
 			if err != nil {
-				return err
+				return errdefs.InvalidParameter(errors.Wrapf(err, "invalid IPC mode: %v", ipcMode))
 			}
 			ns.Path = fmt.Sprintf("/proc/%d/ns/ipc", ic.State.GetPID())
 			setNamespace(s, ns)
@@ -284,11 +288,12 @@ func WithNamespaces(daemon *Daemon, c *container.Container) coci.SpecOpts {
 		case ipcMode.IsPrivate(), ipcMode.IsShareable(), ipcMode.IsNone():
 			ns := specs.LinuxNamespace{Type: "ipc"}
 			setNamespace(s, ns)
-		default:
-			return fmt.Errorf("Invalid IPC mode: %v", ipcMode)
 		}
 
 		// pid
+		if !c.HostConfig.PidMode.Valid() {
+			return errdefs.InvalidParameter(errors.Errorf("invalid PID mode: %v", c.HostConfig.PidMode))
+		}
 		if c.HostConfig.PidMode.IsContainer() {
 			pc, err := daemon.getPidContainer(c)
 			if err != nil {
@@ -314,18 +319,20 @@ func WithNamespaces(daemon *Daemon, c *container.Container) coci.SpecOpts {
 			setNamespace(s, ns)
 		}
 		// uts
+		if !c.HostConfig.UTSMode.Valid() {
+			return errdefs.InvalidParameter(errors.Errorf("invalid UTS mode: %v", c.HostConfig.UTSMode))
+		}
 		if c.HostConfig.UTSMode.IsHost() {
 			oci.RemoveNamespace(s, "uts")
 			s.Hostname = ""
 		}
 
 		// cgroup
+		if !c.HostConfig.CgroupnsMode.Valid() {
+			return errdefs.InvalidParameter(errors.Errorf("invalid cgroup namespace mode: %v", c.HostConfig.CgroupnsMode))
+		}
 		if !c.HostConfig.CgroupnsMode.IsEmpty() {
-			cgroupNsMode := c.HostConfig.CgroupnsMode
-			if !cgroupNsMode.Valid() {
-				return fmt.Errorf("invalid cgroup namespace mode: %v", cgroupNsMode)
-			}
-			if cgroupNsMode.IsPrivate() {
+			if c.HostConfig.CgroupnsMode.IsPrivate() {
 				nsCgroup := specs.LinuxNamespace{Type: "cgroup"}
 				setNamespace(s, nsCgroup)
 			}

diff --git a/integration/container/create_test.go b/integration/container/create_test.go
@@ -528,3 +528,52 @@ func TestCreatePlatformSpecificImageNoPlatform(t *testing.T) {
 	)
 	assert.NilError(t, err)
 }
+
+func TestCreateInvalidHostConfig(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows")
+
+	defer setupTest(t)()
+	apiClient := testEnv.APIClient()
+	ctx := context.Background()
+
+	testCases := []struct {
+		doc         string
+		hc          containertypes.HostConfig
+		expectedErr string
+	}{
+		{
+			doc:         "invalid IpcMode",
+			hc:          containertypes.HostConfig{IpcMode: "invalid"},
+			expectedErr: "Error response from daemon: invalid IPC mode: invalid",
+		},
+		{
+			doc:         "invalid PidMode",
+			hc:          containertypes.HostConfig{PidMode: "invalid"},
+			expectedErr: "Error response from daemon: invalid PID mode: invalid",
+		},
+		{
+			doc:         "invalid PidMode without container ID",
+			hc:          containertypes.HostConfig{PidMode: "container"},
+			expectedErr: "Error response from daemon: invalid PID mode: container",
+		},
+		{
+			doc:         "invalid UTSMode",
+			hc:          containertypes.HostConfig{UTSMode: "invalid"},
+			expectedErr: "Error response from daemon: invalid UTS mode: invalid",
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.doc, func(t *testing.T) {
+			t.Parallel()
+			cfg := container.Config{
+				Image: "busybox",
+			}
+			resp, err := apiClient.ContainerCreate(ctx, &cfg, &tc.hc, nil, nil, "")
+			assert.Check(t, is.Equal(len(resp.Warnings), 0))
+			assert.Check(t, errdefs.IsInvalidParameter(err), "got: %T", err)
+			assert.Error(t, err, tc.expectedErr)
+		})
+	}
+}