Merge pull request #43887 from thaJeztah/22.06_backport_implicit_runt…

…ime_config

[22.06 backport] daemon: support other containerd runtimes (MVP)

daemon/daemon_unix.go

@@ -707,8 +707,8 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes.
 		hostConfig.Runtime = daemon.configStore.GetDefaultRuntimeName()
 	}
 
-	if rt := daemon.configStore.GetRuntime(hostConfig.Runtime); rt == nil {
-		return warnings, fmt.Errorf("Unknown runtime specified %s", hostConfig.Runtime)
+	if _, err := daemon.getRuntime(hostConfig.Runtime); err != nil {
+		return warnings, err
 	}
 
 	parser := volumemounts.NewParser()

daemon/runtime_unix.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 
 	v2runcoptions "github.com/containerd/containerd/runtime/v2/runc/options"
+	"github.com/containerd/containerd/runtime/v2/shim"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/errdefs"
@@ -116,7 +117,10 @@ func (daemon *Daemon) rewriteRuntimePath(name, p string, args []string) (string,
 func (daemon *Daemon) getRuntime(name string) (*types.Runtime, error) {
 	rt := daemon.configStore.GetRuntime(name)
 	if rt == nil {
-		return nil, errdefs.InvalidParameter(errors.Errorf("runtime not found in config: %s", name))
+		if !isPermissibleC8dRuntimeName(name) {
+			return nil, errdefs.InvalidParameter(errors.Errorf("unknown or invalid runtime name: %s", name))
+		}
+		return &types.Runtime{Shim: &types.ShimConfig{Binary: name}}, nil
 	}
 
 	if len(rt.Args) > 0 {
@@ -134,3 +138,37 @@ func (daemon *Daemon) getRuntime(name string) (*types.Runtime, error) {
 
 	return rt, nil
 }
+
+// isPermissibleC8dRuntimeName tests whether name is safe to pass into
+// containerd as a runtime name, and whether the name is well-formed.
+// It does not check if the runtime is installed.
+//
+// A runtime name containing slash characters is interpreted by containerd as
+// the path to a runtime binary. If we allowed this, anyone with Engine API
+// access could get containerd to execute an arbitrary binary as root. Although
+// Engine API access is already equivalent to root on the host, the runtime name
+// has not historically been a vector to run arbitrary code as root so users are
+// not expecting it to become one.
+//
+// This restriction is not configurable. There are viable workarounds for
+// legitimate use cases: administrators and runtime developers can make runtimes
+// available for use with Docker by installing them onto PATH following the
+// [binary naming convention] for containerd Runtime v2.
+//
+// [binary naming convention]: https://github.com/containerd/containerd/blob/main/runtime/v2/README.md#binary-naming
+func isPermissibleC8dRuntimeName(name string) bool {
+	// containerd uses a rather permissive test to validate runtime names:
+	//
+	//   - Any name for which filepath.IsAbs(name) is interpreted as the absolute
+	//     path to a shim binary. We want to block this behaviour.
+	//   - Any name which contains at least one '.' character and no '/' characters
+	//     and does not begin with a '.' character is a valid runtime name. The shim
+	//     binary name is derived from the final two components of the name and
+	//     searched for on the PATH. The name "a.." is technically valid per
+	//     containerd's implementation: it would resolve to a binary named
+	//     "containerd-shim---".
+	//
+	// https://github.com/containerd/containerd/blob/11ded166c15f92450958078cd13c6d87131ec563/runtime/v2/manager.go#L297-L317
+	// https://github.com/containerd/containerd/blob/11ded166c15f92450958078cd13c6d87131ec563/runtime/v2/shim/util.go#L83-L93
+	return !filepath.IsAbs(name) && !strings.ContainsRune(name, '/') && shim.BinaryName(name) != ""
+}

daemon/runtime_unix_test.go

@@ -0,0 +1,107 @@
+//go:build !windows
+// +build !windows
+
+package daemon
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	v2runcoptions "github.com/containerd/containerd/runtime/v2/runc/options"
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
+
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/daemon/config"
+	"github.com/docker/docker/errdefs"
+)
+
+func TestGetRuntime(t *testing.T) {
+	// Configured runtimes can have any arbitrary name, including names
+	// which would not be allowed as implicit runtime names. Explicit takes
+	// precedence over implicit.
+	const configuredRtName = "my/custom.shim.v1"
+	configuredRuntime := types.Runtime{Path: "/bin/true"}
+
+	d := &Daemon{configStore: config.New()}
+	d.configStore.Root = t.TempDir()
+	assert.Assert(t, os.Mkdir(filepath.Join(d.configStore.Root, "runtimes"), 0700))
+	d.configStore.Runtimes = map[string]types.Runtime{
+		configuredRtName: configuredRuntime,
+	}
+	configureRuntimes(d.configStore)
+	assert.Assert(t, d.loadRuntimes())
+
+	stockRuntime, ok := d.configStore.Runtimes[config.StockRuntimeName]
+	assert.Assert(t, ok, "stock runtime could not be found (test needs to be updated)")
+
+	configdOpts := *stockRuntime.Shim.Opts.(*v2runcoptions.Options)
+	configdOpts.BinaryName = configuredRuntime.Path
+	wantConfigdRuntime := configuredRuntime
+	wantConfigdRuntime.Shim = &types.ShimConfig{
+		Binary: stockRuntime.Shim.Binary,
+		Opts:   &configdOpts,
+	}
+
+	for _, tt := range []struct {
+		name, runtime string
+		want          *types.Runtime
+	}{
+		{
+			name:    "StockRuntime",
+			runtime: config.StockRuntimeName,
+			want:    &stockRuntime,
+		},
+		{
+			name:    "ShimName",
+			runtime: "io.containerd.my-shim.v42",
+			want:    &types.Runtime{Shim: &types.ShimConfig{Binary: "io.containerd.my-shim.v42"}},
+		},
+		{
+			// containerd is pretty loose about the format of runtime names. Perhaps too
+			// loose. The only requirements are that the name contain a dot and (depending
+			// on the containerd version) not start with a dot. It does not enforce any
+			// particular format of the dot-delimited components of the name.
+			name:    "VersionlessShimName",
+			runtime: "io.containerd.my-shim",
+			want:    &types.Runtime{Shim: &types.ShimConfig{Binary: "io.containerd.my-shim"}},
+		},
+		{
+			name:    "IllformedShimName",
+			runtime: "myshim",
+		},
+		{
+			name:    "EmptyString",
+			runtime: "",
+		},
+		{
+			name:    "PathToShim",
+			runtime: "/path/to/runc",
+		},
+		{
+			name:    "PathToShimName",
+			runtime: "/path/to/io.containerd.runc.v2",
+		},
+		{
+			name:    "RelPathToShim",
+			runtime: "my/io.containerd.runc.v2",
+		},
+		{
+			name:    "ConfiguredRuntime",
+			runtime: configuredRtName,
+			want:    &wantConfigdRuntime,
+		},
+	} {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := d.getRuntime(tt.runtime)
+			assert.Check(t, is.DeepEqual(got, tt.want))
+			if tt.want != nil {
+				assert.Check(t, err)
+			} else {
+				assert.Check(t, errdefs.IsInvalidParameter(err))
+			}
+		})
+	}
+}

integration-cli/docker_cli_daemon_test.go

@@ -39,6 +39,7 @@ import (
 	"github.com/moby/sys/mount"
 	"golang.org/x/sys/unix"
 	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/icmd"
 	"gotest.tools/v3/poll"
 )
@@ -2384,7 +2385,7 @@ func (s *DockerDaemonSuite) TestRunWithRuntimeFromConfigFile(c *testing.T) {
 	// Run with "vm"
 	out, err = s.d.Cmd("run", "--rm", "--runtime=vm", "busybox", "ls")
 	assert.ErrorContains(c, err, "", out)
-	assert.Assert(c, strings.Contains(out, "/usr/local/bin/vm-manager: no such file or directory"))
+	assert.Assert(c, is.Contains(out, "/usr/local/bin/vm-manager: no such file or directory"))
 	// Reset config to only have the default
 	config = `
 {
@@ -2404,11 +2405,11 @@ func (s *DockerDaemonSuite) TestRunWithRuntimeFromConfigFile(c *testing.T) {
 	// Run with "oci"
 	out, err = s.d.Cmd("run", "--rm", "--runtime=oci", "busybox", "ls")
 	assert.ErrorContains(c, err, "", out)
-	assert.Assert(c, strings.Contains(out, "Unknown runtime specified oci"))
+	assert.Assert(c, is.Contains(out, "unknown or invalid runtime name: oci"))
 	// Start previously created container with oci
 	out, err = s.d.Cmd("start", "oci-runtime-ls")
 	assert.ErrorContains(c, err, "", out)
-	assert.Assert(c, strings.Contains(out, "Unknown runtime specified oci"))
+	assert.Assert(c, is.Contains(out, "unknown or invalid runtime name: oci"))
 	// Check that we can't override the default runtime
 	config = `
 {
@@ -2426,7 +2427,7 @@ func (s *DockerDaemonSuite) TestRunWithRuntimeFromConfigFile(c *testing.T) {
 
 	content, err := s.d.ReadLogFile()
 	assert.NilError(c, err)
-	assert.Assert(c, strings.Contains(string(content), `file configuration validation failed: runtime name 'runc' is reserved`))
+	assert.Assert(c, is.Contains(string(content), `file configuration validation failed: runtime name 'runc' is reserved`))
 	// Check that we can select a default runtime
 	config = `
 {
@@ -2451,7 +2452,7 @@ func (s *DockerDaemonSuite) TestRunWithRuntimeFromConfigFile(c *testing.T) {
 
 	out, err = s.d.Cmd("run", "--rm", "busybox", "ls")
 	assert.ErrorContains(c, err, "", out)
-	assert.Assert(c, strings.Contains(out, "/usr/local/bin/vm-manager: no such file or directory"))
+	assert.Assert(c, is.Contains(out, "/usr/local/bin/vm-manager: no such file or directory"))
 	// Run with default runtime explicitly
 	out, err = s.d.Cmd("run", "--rm", "--runtime=runc", "busybox", "ls")
 	assert.NilError(c, err, out)
@@ -2475,7 +2476,7 @@ func (s *DockerDaemonSuite) TestRunWithRuntimeFromCommandLine(c *testing.T) {
 	// Run with "vm"
 	out, err = s.d.Cmd("run", "--rm", "--runtime=vm", "busybox", "ls")
 	assert.ErrorContains(c, err, "", out)
-	assert.Assert(c, strings.Contains(out, "/usr/local/bin/vm-manager: no such file or directory"))
+	assert.Assert(c, is.Contains(out, "/usr/local/bin/vm-manager: no such file or directory"))
 	// Start a daemon without any extra runtimes
 	s.d.Stop(c)
 	s.d.StartWithBusybox(c)
@@ -2487,25 +2488,25 @@ func (s *DockerDaemonSuite) TestRunWithRuntimeFromCommandLine(c *testing.T) {
 	// Run with "oci"
 	out, err = s.d.Cmd("run", "--rm", "--runtime=oci", "busybox", "ls")
 	assert.ErrorContains(c, err, "", out)
-	assert.Assert(c, strings.Contains(out, "Unknown runtime specified oci"))
+	assert.Assert(c, is.Contains(out, "unknown or invalid runtime name: oci"))
 	// Start previously created container with oci
 	out, err = s.d.Cmd("start", "oci-runtime-ls")
 	assert.ErrorContains(c, err, "", out)
-	assert.Assert(c, strings.Contains(out, "Unknown runtime specified oci"))
+	assert.Assert(c, is.Contains(out, "unknown or invalid runtime name: oci"))
 	// Check that we can't override the default runtime
 	s.d.Stop(c)
 	assert.Assert(c, s.d.StartWithError("--add-runtime", "runc=my-runc") != nil)
 
 	content, err := s.d.ReadLogFile()
 	assert.NilError(c, err)
-	assert.Assert(c, strings.Contains(string(content), `runtime name 'runc' is reserved`))
+	assert.Assert(c, is.Contains(string(content), `runtime name 'runc' is reserved`))
 	// Check that we can select a default runtime
 	s.d.Stop(c)
 	s.d.StartWithBusybox(c, "--default-runtime=vm", "--add-runtime", "oci=runc", "--add-runtime", "vm=/usr/local/bin/vm-manager")
 
 	out, err = s.d.Cmd("run", "--rm", "busybox", "ls")
 	assert.ErrorContains(c, err, "", out)
-	assert.Assert(c, strings.Contains(out, "/usr/local/bin/vm-manager: no such file or directory"))
+	assert.Assert(c, is.Contains(out, "/usr/local/bin/vm-manager: no such file or directory"))
 	// Run with default runtime explicitly
 	out, err = s.d.Cmd("run", "--rm", "--runtime=runc", "busybox", "ls")
 	assert.NilError(c, err, out)

integration/container/run_linux_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"io"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -15,7 +16,9 @@ import (
 	"github.com/docker/docker/api/types/versions"
 	"github.com/docker/docker/integration/internal/container"
 	net "github.com/docker/docker/integration/internal/network"
+	"github.com/docker/docker/pkg/stdcopy"
 	"github.com/docker/docker/pkg/system"
+	"github.com/docker/docker/testutil/daemon"
 	"golang.org/x/sys/unix"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
@@ -214,3 +217,57 @@ func TestRunConsoleSize(t *testing.T) {
 
 	assert.Equal(t, strings.TrimSpace(b.String()), "123 57")
 }
+
+func TestRunWithAlternativeContainerdShim(t *testing.T) {
+	skip.If(t, testEnv.IsRemoteDaemon)
+	skip.If(t, testEnv.DaemonInfo.OSType != "linux")
+
+	realShimPath, err := exec.LookPath("containerd-shim-runc-v2")
+	assert.Assert(t, err)
+	realShimPath, err = filepath.Abs(realShimPath)
+	assert.Assert(t, err)
+
+	// t.TempDir() can't be used here as the temporary directory returned by
+	// that function cannot be accessed by the fake-root user for rootless
+	// Docker. It creates a nested hierarchy of directories where the
+	// outermost has permission 0700.
+	shimDir, err := os.MkdirTemp("", t.Name())
+	assert.Assert(t, err)
+	t.Cleanup(func() {
+		if err := os.RemoveAll(shimDir); err != nil {
+			t.Errorf("shimDir RemoveAll cleanup: %v", err)
+		}
+	})
+	assert.Assert(t, os.Chmod(shimDir, 0777))
+	shimDir, err = filepath.Abs(shimDir)
+	assert.Assert(t, err)
+	assert.Assert(t, os.Symlink(realShimPath, filepath.Join(shimDir, "containerd-shim-realfake-v42")))
+
+	d := daemon.New(t,
+		daemon.WithEnvVars("PATH="+shimDir+":"+os.Getenv("PATH")),
+		daemon.WithContainerdSocket(""), // A new containerd instance needs to be started which inherits the PATH env var defined above.
+	)
+	d.StartWithBusybox(t)
+	defer d.Stop(t)
+
+	client := d.NewClientT(t)
+	ctx := context.Background()
+
+	cID := container.Run(ctx, t, client,
+		container.WithImage("busybox"),
+		container.WithCmd("sh", "-c", `echo 'Hello, world!'`),
+		container.WithRuntime("io.containerd.realfake.v42"),
+	)
+
+	poll.WaitOn(t, container.IsStopped(ctx, client, cID), poll.WithDelay(100*time.Millisecond))
+
+	out, err := client.ContainerLogs(ctx, cID, types.ContainerLogsOptions{ShowStdout: true})
+	assert.NilError(t, err)
+	defer out.Close()
+
+	var b bytes.Buffer
+	_, err = stdcopy.StdCopy(&b, io.Discard, out)
+	assert.NilError(t, err)
+
+	assert.Equal(t, strings.TrimSpace(b.String()), "Hello, world!")
+}

integration/internal/container/ops.go

@@ -234,3 +234,10 @@ func WithConsoleSize(width, height uint) func(*TestContainerConfig) {
 		c.HostConfig.ConsoleSize = [2]uint{height, width}
 	}
 }
+
+// WithRuntime sets the runtime to use to start the container
+func WithRuntime(name string) func(*TestContainerConfig) {
+	return func(c *TestContainerConfig) {
+		c.HostConfig.Runtime = name
+	}
+}

testutil/daemon/daemon.go

@@ -76,6 +76,7 @@ type Daemon struct {
 	log                        LogT
 	pidFile                    string
 	args                       []string
+	extraEnv                   []string
 	containerdSocket           string
 	rootlessUser               *user.User
 	rootlessXDGRuntimeDir      string
@@ -334,9 +335,10 @@ func (d *Daemon) StartWithLogFile(out *os.File, providedArgs ...string) error {
 		dockerdBinary = "sudo"
 		d.args = append(d.args,
 			"-u", d.rootlessUser.Username,
-			"-E", "XDG_RUNTIME_DIR="+d.rootlessXDGRuntimeDir,
-			"-E", "HOME="+d.rootlessUser.HomeDir,
-			"-E", "PATH="+os.Getenv("PATH"),
+			"--preserve-env",
+			"--preserve-env=PATH", // Pass through PATH, overriding secure_path.
+			"XDG_RUNTIME_DIR="+d.rootlessXDGRuntimeDir,
+			"HOME="+d.rootlessUser.HomeDir,
 			"--",
 			defaultDockerdRootlessBinary,
 		)
@@ -392,6 +394,7 @@ func (d *Daemon) StartWithLogFile(out *os.File, providedArgs ...string) error {
 	d.args = append(d.args, providedArgs...)
 	d.cmd = exec.Command(dockerdBinary, d.args...)
 	d.cmd.Env = append(os.Environ(), "DOCKER_SERVICE_PREFER_OFFLINE_IMAGE=1")
+	d.cmd.Env = append(d.cmd.Env, d.extraEnv...)
 	d.cmd.Stdout = out
 	d.cmd.Stderr = out
 	d.logFile = out

testutil/daemon/ops.go

@@ -122,3 +122,10 @@ func WithOOMScoreAdjust(score int) Option {
 		d.OOMScoreAdjust = score
 	}
 }
+
+// WithEnvVars sets additional environment variables for the daemon
+func WithEnvVars(vars ...string) Option {
+	return func(d *Daemon) {
+		d.extraEnv = append(d.extraEnv, vars...)
+	}
+}