فهرست منبع

daemon: consolidate runtimes config validation

The daemon has made a habit of mutating the DefaultRuntime and Runtimes
values in the Config struct to merge defaults. This would be fine if it
was a part of the regular configuration loading and merging process,
as is done with other config options. The trouble is it does so in
surprising places, such as in functions with 'verify' or 'validate' in
their name. It has been necessary in order to validate that the user has
not defined a custom runtime named "runc" which would shadow the
built-in runtime of the same name. Other daemon code depends on the
runtime named "runc" always being defined in the config, but merging it
with the user config at the same time as the other defaults are merged
would trip the validation. The root of the issue is that the daemon has
used the same config values for both validating the daemon runtime
configuration as supplied by the user and for keeping track of which
runtimes have been set up by the daemon. Now that a completely separate
value is used for the latter purpose, surprising contortions are no
longer required to make the validation work as intended.

Consolidate the validation of the runtimes config and merging of the
built-in runtimes into the daemon.setupRuntimes() function. Set the
result of merging the built-in runtimes config and default default
runtime on the returned runtimes struct, without back-propagating it
onto the config.Config argument.

Signed-off-by: Cory Snider <csnider@mirantis.com>
Cory Snider 2 سال پیش
والد
کامیت
0f6eeecac0

+ 0 - 60
daemon/config/config.go

@@ -7,7 +7,6 @@ import (
 	"net"
 	"net"
 	"net/url"
 	"net/url"
 	"os"
 	"os"
-	"path/filepath"
 	"strings"
 	"strings"
 
 
 	"golang.org/x/text/encoding"
 	"golang.org/x/text/encoding"
@@ -15,7 +14,6 @@ import (
 	"golang.org/x/text/transform"
 	"golang.org/x/text/transform"
 
 
 	"github.com/container-orchestrated-devices/container-device-interface/pkg/cdi"
 	"github.com/container-orchestrated-devices/container-device-interface/pkg/cdi"
-	"github.com/containerd/containerd/runtime/v2/shim"
 	"github.com/docker/docker/opts"
 	"github.com/docker/docker/opts"
 	"github.com/docker/docker/registry"
 	"github.com/docker/docker/registry"
 	"github.com/imdario/mergo"
 	"github.com/imdario/mergo"
@@ -56,9 +54,6 @@ const (
 	// DefaultPluginNamespace is the name of the default containerd namespace used for plugins.
 	// DefaultPluginNamespace is the name of the default containerd namespace used for plugins.
 	DefaultPluginNamespace = "plugins.moby"
 	DefaultPluginNamespace = "plugins.moby"
 
 
-	// LinuxV2RuntimeName is the runtime used to specify the containerd v2 runc shim
-	LinuxV2RuntimeName = "io.containerd.runc.v2"
-
 	// SeccompProfileDefault is the built-in default seccomp profile.
 	// SeccompProfileDefault is the built-in default seccomp profile.
 	SeccompProfileDefault = "builtin"
 	SeccompProfileDefault = "builtin"
 	// SeccompProfileUnconfined is a special profile name for seccomp to use an
 	// SeccompProfileUnconfined is a special profile name for seccomp to use an
@@ -66,11 +61,6 @@ const (
 	SeccompProfileUnconfined = "unconfined"
 	SeccompProfileUnconfined = "unconfined"
 )
 )
 
 
-var builtinRuntimes = map[string]bool{
-	StockRuntimeName:   true,
-	LinuxV2RuntimeName: true,
-}
-
 // flatOptions contains configuration keys
 // flatOptions contains configuration keys
 // that MUST NOT be parsed as deep structures.
 // that MUST NOT be parsed as deep structures.
 // Use this to differentiate these options
 // Use this to differentiate these options
@@ -637,26 +627,10 @@ func Validate(config *Config) error {
 		return errors.Errorf("invalid max download attempts: %d", config.MaxDownloadAttempts)
 		return errors.Errorf("invalid max download attempts: %d", config.MaxDownloadAttempts)
 	}
 	}
 
 
-	// validate that "default" runtime is not reset
-	if runtimes := config.GetAllRuntimes(); len(runtimes) > 0 {
-		if _, ok := runtimes[StockRuntimeName]; ok {
-			return errors.Errorf("runtime name '%s' is reserved", StockRuntimeName)
-		}
-	}
-
 	if _, err := ParseGenericResources(config.NodeGenericResources); err != nil {
 	if _, err := ParseGenericResources(config.NodeGenericResources); err != nil {
 		return err
 		return err
 	}
 	}
 
 
-	if config.DefaultRuntime != "" {
-		if !builtinRuntimes[config.DefaultRuntime] {
-			runtimes := config.GetAllRuntimes()
-			if _, ok := runtimes[config.DefaultRuntime]; !ok && !IsPermissibleC8dRuntimeName(config.DefaultRuntime) {
-				return fmt.Errorf("specified default runtime '%s' does not exist", config.DefaultRuntime)
-			}
-		}
-	}
-
 	for _, h := range config.Hosts {
 	for _, h := range config.Hosts {
 		if _, err := opts.ValidateHost(h); err != nil {
 		if _, err := opts.ValidateHost(h); err != nil {
 			return err
 			return err
@@ -676,37 +650,3 @@ func MaskCredentials(rawURL string) string {
 	parsedURL.User = url.UserPassword("xxxxx", "xxxxx")
 	parsedURL.User = url.UserPassword("xxxxx", "xxxxx")
 	return parsedURL.String()
 	return parsedURL.String()
 }
 }
-
-// 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) != ""
-}

+ 0 - 5
daemon/config/config_linux.go

@@ -81,11 +81,6 @@ type Config struct {
 	Rootless   bool   `json:"rootless,omitempty"`
 	Rootless   bool   `json:"rootless,omitempty"`
 }
 }
 
 
-// GetAllRuntimes returns a copy of the runtimes map
-func (conf *Config) GetAllRuntimes() map[string]types.Runtime {
-	return conf.Runtimes
-}
-
 // GetExecRoot returns the user configured Exec-root
 // GetExecRoot returns the user configured Exec-root
 func (conf *Config) GetExecRoot() string {
 func (conf *Config) GetExecRoot() string {
 	return conf.ExecRoot
 	return conf.ExecRoot

+ 0 - 30
daemon/config/config_linux_test.go

@@ -3,10 +3,8 @@ package config // import "github.com/docker/docker/daemon/config"
 import (
 import (
 	"testing"
 	"testing"
 
 
-	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/opts"
 	"github.com/docker/docker/opts"
 	units "github.com/docker/go-units"
 	units "github.com/docker/go-units"
-	"github.com/imdario/mergo"
 	"github.com/spf13/pflag"
 	"github.com/spf13/pflag"
 	"gotest.tools/v3/assert"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	is "gotest.tools/v3/assert/cmp"
@@ -123,34 +121,6 @@ func TestDaemonConfigurationMergeShmSize(t *testing.T) {
 	assert.Check(t, is.Equal(int64(expectedValue), cc.ShmSize.Value()))
 	assert.Check(t, is.Equal(int64(expectedValue), cc.ShmSize.Value()))
 }
 }
 
 
-func TestUnixValidateConfigurationErrors(t *testing.T) {
-	testCases := []struct {
-		doc         string
-		config      *Config
-		expectedErr string
-	}{
-		{
-			doc: `cannot override the stock runtime`,
-			config: &Config{
-				Runtimes: map[string]types.Runtime{
-					StockRuntimeName: {},
-				},
-			},
-			expectedErr: `runtime name 'runc' is reserved`,
-		},
-	}
-	for _, tc := range testCases {
-		tc := tc
-		t.Run(tc.doc, func(t *testing.T) {
-			cfg, err := New()
-			assert.NilError(t, err)
-			assert.Check(t, mergo.Merge(cfg, tc.config, mergo.WithOverride))
-			err = Validate(cfg)
-			assert.ErrorContains(t, err, tc.expectedErr)
-		})
-	}
-}
-
 func TestUnixGetInitPath(t *testing.T) {
 func TestUnixGetInitPath(t *testing.T) {
 	testCases := []struct {
 	testCases := []struct {
 		config           *Config
 		config           *Config

+ 0 - 7
daemon/config/config_windows.go

@@ -3,8 +3,6 @@ package config // import "github.com/docker/docker/daemon/config"
 import (
 import (
 	"os"
 	"os"
 	"path/filepath"
 	"path/filepath"
-
-	"github.com/docker/docker/api/types"
 )
 )
 
 
 const (
 const (
@@ -30,11 +28,6 @@ type Config struct {
 	// for the Windows daemon.)
 	// for the Windows daemon.)
 }
 }
 
 
-// GetAllRuntimes returns a copy of the runtimes map
-func (conf *Config) GetAllRuntimes() map[string]types.Runtime {
-	return map[string]types.Runtime{}
-}
-
 // GetExecRoot returns the user configured Exec-root
 // GetExecRoot returns the user configured Exec-root
 func (conf *Config) GetExecRoot() string {
 func (conf *Config) GetExecRoot() string {
 	return ""
 	return ""

+ 0 - 1
daemon/container_unix_test.go

@@ -34,7 +34,6 @@ func TestContainerWarningHostAndPublishPorts(t *testing.T) {
 		d := &Daemon{}
 		d := &Daemon{}
 		cfg, err := config.New()
 		cfg, err := config.New()
 		assert.NilError(t, err)
 		assert.NilError(t, err)
-		configureRuntimes(cfg)
 		runtimes, err := setupRuntimes(cfg)
 		runtimes, err := setupRuntimes(cfg)
 		assert.NilError(t, err)
 		assert.NilError(t, err)
 		daemonCfg := &configStore{Config: *cfg, Runtimes: runtimes}
 		daemonCfg := &configStore{Config: *cfg, Runtimes: runtimes}

+ 1 - 1
daemon/daemon.go

@@ -960,7 +960,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 			shimOpts interface{}
 			shimOpts interface{}
 		)
 		)
 		if runtime.GOOS != "windows" {
 		if runtime.GOOS != "windows" {
-			shim, shimOpts, err = runtimes.Get(configStore.DefaultRuntime)
+			shim, shimOpts, err = runtimes.Get("")
 			if err != nil {
 			if err != nil {
 				return nil, err
 				return nil, err
 			}
 			}

+ 1 - 1
daemon/daemon_linux.go

@@ -244,7 +244,7 @@ func supportsRecursivelyReadOnly(cfg *configStore, runtime string) error {
 		return fmt.Errorf("rro is not supported: %w (kernel is older than 5.12?)", err)
 		return fmt.Errorf("rro is not supported: %w (kernel is older than 5.12?)", err)
 	}
 	}
 	if runtime == "" {
 	if runtime == "" {
-		runtime = cfg.DefaultRuntime
+		runtime = cfg.Runtimes.Default
 	}
 	}
 	features := cfg.Runtimes.Features(runtime)
 	features := cfg.Runtimes.Features(runtime)
 	if features == nil {
 	if features == nil {

+ 1 - 10
daemon/daemon_unix.go

@@ -698,7 +698,7 @@ func verifyPlatformContainerSettings(daemon *Daemon, daemonCfg *configStore, hos
 		}
 		}
 	}
 	}
 	if hostConfig.Runtime == "" {
 	if hostConfig.Runtime == "" {
-		hostConfig.Runtime = daemonCfg.DefaultRuntime
+		hostConfig.Runtime = daemonCfg.Runtimes.Default
 	}
 	}
 
 
 	if _, _, err := daemonCfg.Runtimes.Get(hostConfig.Runtime); err != nil {
 	if _, _, err := daemonCfg.Runtimes.Get(hostConfig.Runtime); err != nil {
@@ -754,15 +754,6 @@ func verifyDaemonSettings(conf *config.Config) error {
 	if conf.Rootless && UsingSystemd(conf) && cgroups.Mode() != cgroups.Unified {
 	if conf.Rootless && UsingSystemd(conf) && cgroups.Mode() != cgroups.Unified {
 		return fmt.Errorf("exec-opt native.cgroupdriver=systemd requires cgroup v2 for rootless mode")
 		return fmt.Errorf("exec-opt native.cgroupdriver=systemd requires cgroup v2 for rootless mode")
 	}
 	}
-
-	configureRuntimes(conf)
-	if rtName := conf.DefaultRuntime; rtName != "" {
-		if _, ok := conf.Runtimes[rtName]; !ok {
-			if !config.IsPermissibleC8dRuntimeName(rtName) {
-				return fmt.Errorf("specified default runtime '%s' does not exist", rtName)
-			}
-		}
-	}
 	return nil
 	return nil
 }
 }
 
 

+ 2 - 2
daemon/info.go

@@ -66,7 +66,7 @@ func (daemon *Daemon) SystemInfo() *types.Info {
 	daemon.fillDebugInfo(v)
 	daemon.fillDebugInfo(v)
 	daemon.fillAPIInfo(v, &cfg.Config)
 	daemon.fillAPIInfo(v, &cfg.Config)
 	// Retrieve platform specific info
 	// Retrieve platform specific info
-	daemon.fillPlatformInfo(v, sysInfo, &cfg.Config)
+	daemon.fillPlatformInfo(v, sysInfo, cfg)
 	daemon.fillDriverInfo(v)
 	daemon.fillDriverInfo(v)
 	daemon.fillPluginsInfo(v, &cfg.Config)
 	daemon.fillPluginsInfo(v, &cfg.Config)
 	daemon.fillSecurityOptions(v, sysInfo, &cfg.Config)
 	daemon.fillSecurityOptions(v, sysInfo, &cfg.Config)
@@ -117,7 +117,7 @@ func (daemon *Daemon) SystemVersion() types.Version {
 
 
 	v.Platform.Name = dockerversion.PlatformName
 	v.Platform.Name = dockerversion.PlatformName
 
 
-	daemon.fillPlatformVersion(&v, &cfg.Config)
+	daemon.fillPlatformVersion(&v, cfg)
 	return v
 	return v
 }
 }
 
 

+ 33 - 20
daemon/info_unix.go

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"path/filepath"
 	"strings"
 	"strings"
 
 
+	v2runcoptions "github.com/containerd/containerd/runtime/v2/runc/options"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types"
 	containertypes "github.com/docker/docker/api/types/container"
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/daemon/config"
@@ -19,8 +20,8 @@ import (
 )
 )
 
 
 // fillPlatformInfo fills the platform related info.
 // fillPlatformInfo fills the platform related info.
-func (daemon *Daemon) fillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo, cfg *config.Config) {
-	v.CgroupDriver = cgroupDriver(cfg)
+func (daemon *Daemon) fillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo, cfg *configStore) {
+	v.CgroupDriver = cgroupDriver(&cfg.Config)
 	v.CgroupVersion = "1"
 	v.CgroupVersion = "1"
 	if sysInfo.CgroupUnified {
 	if sysInfo.CgroupUnified {
 		v.CgroupVersion = "2"
 		v.CgroupVersion = "2"
@@ -39,18 +40,21 @@ func (daemon *Daemon) fillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo,
 		v.PidsLimit = sysInfo.PidsLimit
 		v.PidsLimit = sysInfo.PidsLimit
 	}
 	}
 	v.Runtimes = make(map[string]types.Runtime)
 	v.Runtimes = make(map[string]types.Runtime)
-	for n, r := range cfg.Runtimes {
+	for n, p := range stockRuntimes() {
+		v.Runtimes[n] = types.Runtime{Path: p}
+	}
+	for n, r := range cfg.Config.Runtimes {
 		v.Runtimes[n] = types.Runtime{
 		v.Runtimes[n] = types.Runtime{
 			Path: r.Path,
 			Path: r.Path,
 			Args: append([]string(nil), r.Args...),
 			Args: append([]string(nil), r.Args...),
 		}
 		}
 	}
 	}
-	v.DefaultRuntime = cfg.DefaultRuntime
+	v.DefaultRuntime = cfg.Runtimes.Default
 	v.RuncCommit.ID = "N/A"
 	v.RuncCommit.ID = "N/A"
 	v.ContainerdCommit.ID = "N/A"
 	v.ContainerdCommit.ID = "N/A"
 	v.InitCommit.ID = "N/A"
 	v.InitCommit.ID = "N/A"
 
 
-	if _, _, commit, err := parseDefaultRuntimeVersion(cfg); err != nil {
+	if _, _, commit, err := parseDefaultRuntimeVersion(&cfg.Runtimes); err != nil {
 		logrus.Warnf(err.Error())
 		logrus.Warnf(err.Error())
 	} else {
 	} else {
 		v.RuncCommit.ID = commit
 		v.RuncCommit.ID = commit
@@ -166,7 +170,7 @@ func (daemon *Daemon) fillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo,
 	}
 	}
 }
 }
 
 
-func (daemon *Daemon) fillPlatformVersion(v *types.Version, cfg *config.Config) {
+func (daemon *Daemon) fillPlatformVersion(v *types.Version, cfg *configStore) {
 	if rv, err := daemon.containerd.Version(context.Background()); err == nil {
 	if rv, err := daemon.containerd.Version(context.Background()); err == nil {
 		v.Components = append(v.Components, types.ComponentVersion{
 		v.Components = append(v.Components, types.ComponentVersion{
 			Name:    "containerd",
 			Name:    "containerd",
@@ -177,11 +181,11 @@ func (daemon *Daemon) fillPlatformVersion(v *types.Version, cfg *config.Config)
 		})
 		})
 	}
 	}
 
 
-	if _, ver, commit, err := parseDefaultRuntimeVersion(cfg); err != nil {
+	if _, ver, commit, err := parseDefaultRuntimeVersion(&cfg.Runtimes); err != nil {
 		logrus.Warnf(err.Error())
 		logrus.Warnf(err.Error())
 	} else {
 	} else {
 		v.Components = append(v.Components, types.ComponentVersion{
 		v.Components = append(v.Components, types.ComponentVersion{
-			Name:    cfg.DefaultRuntime,
+			Name:    cfg.Runtimes.Default,
 			Version: ver,
 			Version: ver,
 			Details: map[string]string{
 			Details: map[string]string{
 				"GitCommit": commit,
 				"GitCommit": commit,
@@ -331,19 +335,28 @@ func parseRuntimeVersion(v string) (runtime, version, commit string, err error)
 	return runtime, version, commit, err
 	return runtime, version, commit, err
 }
 }
 
 
-func parseDefaultRuntimeVersion(cfg *config.Config) (runtime, version, commit string, err error) {
-	if rt, ok := cfg.Runtimes[cfg.DefaultRuntime]; ok {
-		rv, err := exec.Command(rt.Path, "--version").Output()
-		if err != nil {
-			return "", "", "", fmt.Errorf("failed to retrieve %s version: %w", rt.Path, err)
-		}
-		runtime, version, commit, err := parseRuntimeVersion(string(rv))
-		if err != nil {
-			return "", "", "", fmt.Errorf("failed to parse %s version: %w", rt.Path, err)
-		}
-		return runtime, version, commit, err
+func parseDefaultRuntimeVersion(rts *runtimes) (runtime, version, commit string, err error) {
+	shim, opts, err := rts.Get(rts.Default)
+	if err != nil {
+		return "", "", "", err
+	}
+	shimopts, ok := opts.(*v2runcoptions.Options)
+	if !ok {
+		return "", "", "", fmt.Errorf("%s: retrieving version not supported", shim)
+	}
+	rt := shimopts.BinaryName
+	if rt == "" {
+		rt = defaultRuntimeName
 	}
 	}
-	return "", "", "", nil
+	rv, err := exec.Command(rt, "--version").Output()
+	if err != nil {
+		return "", "", "", fmt.Errorf("failed to retrieve %s version: %w", rt, err)
+	}
+	runtime, version, commit, err = parseRuntimeVersion(string(rv))
+	if err != nil {
+		return "", "", "", fmt.Errorf("failed to parse %s version: %w", rt, err)
+	}
+	return runtime, version, commit, err
 }
 }
 
 
 func cgroupNamespacesEnabled(sysInfo *sysinfo.SysInfo, cfg *config.Config) bool {
 func cgroupNamespacesEnabled(sysInfo *sysinfo.SysInfo, cfg *config.Config) bool {

+ 2 - 2
daemon/info_windows.go

@@ -7,10 +7,10 @@ import (
 )
 )
 
 
 // fillPlatformInfo fills the platform related info.
 // fillPlatformInfo fills the platform related info.
-func (daemon *Daemon) fillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo, cfg *config.Config) {
+func (daemon *Daemon) fillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo, cfg *configStore) {
 }
 }
 
 
-func (daemon *Daemon) fillPlatformVersion(v *types.Version, cfg *config.Config) {}
+func (daemon *Daemon) fillPlatformVersion(v *types.Version, cfg *configStore) {}
 
 
 func fillDriverWarnings(v *types.Info) {
 func fillDriverWarnings(v *types.Info) {
 }
 }

+ 0 - 1
daemon/reload_unix.go

@@ -18,7 +18,6 @@ func (daemon *Daemon) reloadPlatform(txn *reloadTxn, newCfg *configStore, conf *
 	if conf.IsValueSet("runtimes") {
 	if conf.IsValueSet("runtimes") {
 		newCfg.Config.Runtimes = conf.Runtimes
 		newCfg.Config.Runtimes = conf.Runtimes
 	}
 	}
-	configureRuntimes(&newCfg.Config)
 	var err error
 	var err error
 	newCfg.Runtimes, err = setupRuntimes(&newCfg.Config)
 	newCfg.Runtimes, err = setupRuntimes(&newCfg.Config)
 	if err != nil {
 	if err != nil {

+ 73 - 10
daemon/runtime_unix.go

@@ -16,7 +16,7 @@ import (
 
 
 	"github.com/containerd/containerd/plugin"
 	"github.com/containerd/containerd/plugin"
 	v2runcoptions "github.com/containerd/containerd/runtime/v2/runc/options"
 	v2runcoptions "github.com/containerd/containerd/runtime/v2/runc/options"
-	"github.com/docker/docker/api/types"
+	"github.com/containerd/containerd/runtime/v2/shim"
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/libcontainerd/shimopts"
 	"github.com/docker/docker/libcontainerd/shimopts"
@@ -29,6 +29,9 @@ import (
 
 
 const (
 const (
 	defaultRuntimeName = "runc"
 	defaultRuntimeName = "runc"
+
+	// The runtime used to specify the containerd v2 runc shim
+	linuxV2RuntimeName = "io.containerd.runc.v2"
 )
 )
 
 
 type shimConfig struct {
 type shimConfig struct {
@@ -41,18 +44,15 @@ type shimConfig struct {
 }
 }
 
 
 type runtimes struct {
 type runtimes struct {
+	Default    string
 	configured map[string]*shimConfig
 	configured map[string]*shimConfig
 }
 }
 
 
-func configureRuntimes(conf *config.Config) {
-	if conf.DefaultRuntime == "" {
-		conf.DefaultRuntime = config.StockRuntimeName
-	}
-	if conf.Runtimes == nil {
-		conf.Runtimes = make(map[string]types.Runtime)
+func stockRuntimes() map[string]string {
+	return map[string]string{
+		linuxV2RuntimeName:      defaultRuntimeName,
+		config.StockRuntimeName: defaultRuntimeName,
 	}
 	}
-	conf.Runtimes[config.LinuxV2RuntimeName] = types.Runtime{Path: defaultRuntimeName}
-	conf.Runtimes[config.StockRuntimeName] = conf.Runtimes[config.LinuxV2RuntimeName]
 }
 }
 
 
 func defaultV2ShimConfig(conf *config.Config, runtimePath string) *shimConfig {
 func defaultV2ShimConfig(conf *config.Config, runtimePath string) *shimConfig {
@@ -98,9 +98,27 @@ func initRuntimesDir(cfg *config.Config) error {
 }
 }
 
 
 func setupRuntimes(cfg *config.Config) (runtimes, error) {
 func setupRuntimes(cfg *config.Config) (runtimes, error) {
+	if _, ok := cfg.Runtimes[config.StockRuntimeName]; ok {
+		return runtimes{}, errors.Errorf("runtime name '%s' is reserved", config.StockRuntimeName)
+	}
+
 	newrt := runtimes{
 	newrt := runtimes{
+		Default:    cfg.DefaultRuntime,
 		configured: make(map[string]*shimConfig),
 		configured: make(map[string]*shimConfig),
 	}
 	}
+	for name, path := range stockRuntimes() {
+		newrt.configured[name] = defaultV2ShimConfig(cfg, path)
+	}
+
+	if newrt.Default != "" {
+		_, isStock := newrt.configured[newrt.Default]
+		_, isConfigured := cfg.Runtimes[newrt.Default]
+		if !isStock && !isConfigured && !isPermissibleC8dRuntimeName(newrt.Default) {
+			return runtimes{}, errors.Errorf("specified default runtime '%s' does not exist", newrt.Default)
+		}
+	} else {
+		newrt.Default = config.StockRuntimeName
+	}
 
 
 	dir := runtimeScriptsDir(cfg)
 	dir := runtimeScriptsDir(cfg)
 	for name, rt := range cfg.Runtimes {
 	for name, rt := range cfg.Runtimes {
@@ -180,7 +198,14 @@ func wrapRuntime(dir, name, binary string, args []string) (string, error) {
 	return scriptPath, nil
 	return scriptPath, nil
 }
 }
 
 
+// Get returns the containerd runtime and options for name, suitable to pass
+// into containerd.WithRuntime(). The runtime and options for the default
+// runtime are returned when name is the empty string.
 func (r *runtimes) Get(name string) (string, interface{}, error) {
 func (r *runtimes) Get(name string) (string, interface{}, error) {
+	if name == "" {
+		name = r.Default
+	}
+
 	rt := r.configured[name]
 	rt := r.configured[name]
 	if rt != nil {
 	if rt != nil {
 		if rt.PreflightCheck != nil {
 		if rt.PreflightCheck != nil {
@@ -191,16 +216,54 @@ func (r *runtimes) Get(name string) (string, interface{}, error) {
 		return rt.Shim, rt.Opts, nil
 		return rt.Shim, rt.Opts, nil
 	}
 	}
 
 
-	if !config.IsPermissibleC8dRuntimeName(name) {
+	if !isPermissibleC8dRuntimeName(name) {
 		return "", nil, errdefs.InvalidParameter(errors.Errorf("unknown or invalid runtime name: %s", name))
 		return "", nil, errdefs.InvalidParameter(errors.Errorf("unknown or invalid runtime name: %s", name))
 	}
 	}
 	return name, nil, nil
 	return name, nil, nil
 }
 }
 
 
 func (r *runtimes) Features(name string) *features.Features {
 func (r *runtimes) Features(name string) *features.Features {
+	if name == "" {
+		name = r.Default
+	}
+
 	rt := r.configured[name]
 	rt := r.configured[name]
 	if rt != nil {
 	if rt != nil {
 		return rt.Features
 		return rt.Features
 	}
 	}
 	return nil
 	return 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) != ""
+}

+ 121 - 32
daemon/runtime_unix_test.go

@@ -10,6 +10,7 @@ import (
 
 
 	"github.com/containerd/containerd/plugin"
 	"github.com/containerd/containerd/plugin"
 	v2runcoptions "github.com/containerd/containerd/runtime/v2/runc/options"
 	v2runcoptions "github.com/containerd/containerd/runtime/v2/runc/options"
+	"github.com/imdario/mergo"
 	"gotest.tools/v3/assert"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	is "gotest.tools/v3/assert/cmp"
 
 
@@ -18,81 +19,169 @@ import (
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/errdefs"
 )
 )
 
 
-func TestInitRuntimes_InvalidConfigs(t *testing.T) {
+func TestSetupRuntimes(t *testing.T) {
 	cases := []struct {
 	cases := []struct {
 		name      string
 		name      string
-		runtime   types.Runtime
+		config    *config.Config
 		expectErr string
 		expectErr string
 	}{
 	}{
 		{
 		{
-			name:      "Empty",
+			name: "Empty",
+			config: &config.Config{
+				Runtimes: map[string]types.Runtime{
+					"myruntime": {},
+				},
+			},
 			expectErr: "either a runtimeType or a path must be configured",
 			expectErr: "either a runtimeType or a path must be configured",
 		},
 		},
 		{
 		{
-			name:      "ArgsOnly",
-			runtime:   types.Runtime{Args: []string{"foo", "bar"}},
+			name: "ArgsOnly",
+			config: &config.Config{
+				Runtimes: map[string]types.Runtime{
+					"myruntime": {Args: []string{"foo", "bar"}},
+				},
+			},
 			expectErr: "either a runtimeType or a path must be configured",
 			expectErr: "either a runtimeType or a path must be configured",
 		},
 		},
 		{
 		{
-			name:      "OptionsOnly",
-			runtime:   types.Runtime{Options: map[string]interface{}{"hello": "world"}},
+			name: "OptionsOnly",
+			config: &config.Config{
+				Runtimes: map[string]types.Runtime{
+					"myruntime": {Options: map[string]interface{}{"hello": "world"}},
+				},
+			},
 			expectErr: "either a runtimeType or a path must be configured",
 			expectErr: "either a runtimeType or a path must be configured",
 		},
 		},
 		{
 		{
-			name:      "PathAndType",
-			runtime:   types.Runtime{Path: "/bin/true", Type: "io.containerd.runsc.v1"},
+			name: "PathAndType",
+			config: &config.Config{
+				Runtimes: map[string]types.Runtime{
+					"myruntime": {Path: "/bin/true", Type: "io.containerd.runsc.v1"},
+				},
+			},
 			expectErr: "cannot configure both",
 			expectErr: "cannot configure both",
 		},
 		},
 		{
 		{
-			name:      "PathAndOptions",
-			runtime:   types.Runtime{Path: "/bin/true", Options: map[string]interface{}{"a": "b"}},
+			name: "PathAndOptions",
+			config: &config.Config{
+				Runtimes: map[string]types.Runtime{
+					"myruntime": {Path: "/bin/true", Options: map[string]interface{}{"a": "b"}},
+				},
+			},
 			expectErr: "options cannot be used with a path runtime",
 			expectErr: "options cannot be used with a path runtime",
 		},
 		},
 		{
 		{
-			name:      "TypeAndArgs",
-			runtime:   types.Runtime{Type: "io.containerd.runsc.v1", Args: []string{"--version"}},
+			name: "TypeAndArgs",
+			config: &config.Config{
+				Runtimes: map[string]types.Runtime{
+					"myruntime": {Type: "io.containerd.runsc.v1", Args: []string{"--version"}},
+				},
+			},
 			expectErr: "args cannot be used with a runtimeType runtime",
 			expectErr: "args cannot be used with a runtimeType runtime",
 		},
 		},
 		{
 		{
 			name: "PathArgsOptions",
 			name: "PathArgsOptions",
-			runtime: types.Runtime{
-				Path:    "/bin/true",
-				Args:    []string{"--version"},
-				Options: map[string]interface{}{"hmm": 3},
+			config: &config.Config{
+				Runtimes: map[string]types.Runtime{
+					"myruntime": {
+						Path:    "/bin/true",
+						Args:    []string{"--version"},
+						Options: map[string]interface{}{"hmm": 3},
+					},
+				},
 			},
 			},
 			expectErr: "options cannot be used with a path runtime",
 			expectErr: "options cannot be used with a path runtime",
 		},
 		},
 		{
 		{
 			name: "TypeOptionsArgs",
 			name: "TypeOptionsArgs",
-			runtime: types.Runtime{
-				Type:    "io.containerd.kata.v2",
-				Options: map[string]interface{}{"a": "b"},
-				Args:    []string{"--help"},
+			config: &config.Config{
+				Runtimes: map[string]types.Runtime{
+					"myruntime": {
+						Type:    "io.containerd.kata.v2",
+						Options: map[string]interface{}{"a": "b"},
+						Args:    []string{"--help"},
+					},
+				},
 			},
 			},
 			expectErr: "args cannot be used with a runtimeType runtime",
 			expectErr: "args cannot be used with a runtimeType runtime",
 		},
 		},
 		{
 		{
 			name: "PathArgsTypeOptions",
 			name: "PathArgsTypeOptions",
-			runtime: types.Runtime{
-				Path:    "/bin/true",
-				Args:    []string{"foo"},
-				Type:    "io.containerd.runsc.v1",
-				Options: map[string]interface{}{"a": "b"},
+			config: &config.Config{
+				Runtimes: map[string]types.Runtime{
+					"myruntime": {
+						Path:    "/bin/true",
+						Args:    []string{"foo"},
+						Type:    "io.containerd.runsc.v1",
+						Options: map[string]interface{}{"a": "b"},
+					},
+				},
 			},
 			},
 			expectErr: "cannot configure both",
 			expectErr: "cannot configure both",
 		},
 		},
+		{
+			name: "CannotOverrideStockRuntime",
+			config: &config.Config{
+				Runtimes: map[string]types.Runtime{
+					config.StockRuntimeName: {},
+				},
+			},
+			expectErr: `runtime name 'runc' is reserved`,
+		},
+		{
+			name: "SetStockRuntimeAsDefault",
+			config: &config.Config{
+				CommonConfig: config.CommonConfig{
+					DefaultRuntime: config.StockRuntimeName,
+				},
+			},
+		},
+		{
+			name: "SetLinuxRuntimeAsDefault",
+			config: &config.Config{
+				CommonConfig: config.CommonConfig{
+					DefaultRuntime: linuxV2RuntimeName,
+				},
+			},
+		},
+		{
+			name: "CannotSetBogusRuntimeAsDefault",
+			config: &config.Config{
+				CommonConfig: config.CommonConfig{
+					DefaultRuntime: "notdefined",
+				},
+			},
+			expectErr: "specified default runtime 'notdefined' does not exist",
+		},
+		{
+			name: "SetDefinedRuntimeAsDefault",
+			config: &config.Config{
+				Runtimes: map[string]types.Runtime{
+					"some-runtime": {
+						Path: "/usr/local/bin/file-not-found",
+					},
+				},
+				CommonConfig: config.CommonConfig{
+					DefaultRuntime: "some-runtime",
+				},
+			},
+		},
 	}
 	}
-
-	for _, tt := range cases {
-		t.Run(tt.name, func(t *testing.T) {
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
 			cfg, err := config.New()
 			cfg, err := config.New()
 			assert.NilError(t, err)
 			assert.NilError(t, err)
 			cfg.Root = t.TempDir()
 			cfg.Root = t.TempDir()
-			cfg.Runtimes["myruntime"] = tt.runtime
+			assert.NilError(t, mergo.Merge(cfg, tc.config, mergo.WithOverride))
 			assert.Assert(t, initRuntimesDir(cfg))
 			assert.Assert(t, initRuntimesDir(cfg))
 
 
 			_, err = setupRuntimes(cfg)
 			_, err = setupRuntimes(cfg)
-			assert.Check(t, is.ErrorContains(err, tt.expectErr))
+			if tc.expectErr == "" {
+				assert.NilError(t, err)
+			} else {
+				assert.ErrorContains(t, err, tc.expectErr)
+			}
 		})
 		})
 	}
 	}
 }
 }
@@ -133,7 +222,6 @@ func TestGetRuntime(t *testing.T) {
 		shimAliasName:            shimAlias,
 		shimAliasName:            shimAlias,
 		configuredShimByPathName: configuredShimByPath,
 		configuredShimByPathName: configuredShimByPath,
 	}
 	}
-	configureRuntimes(cfg)
 	assert.NilError(t, initRuntimesDir(cfg))
 	assert.NilError(t, initRuntimesDir(cfg))
 	runtimes, err := setupRuntimes(cfg)
 	runtimes, err := setupRuntimes(cfg)
 	assert.NilError(t, err)
 	assert.NilError(t, err)
@@ -179,6 +267,7 @@ func TestGetRuntime(t *testing.T) {
 		{
 		{
 			name:    "EmptyString",
 			name:    "EmptyString",
 			runtime: "",
 			runtime: "",
+			want:    stockRuntime,
 		},
 		},
 		{
 		{
 			name:    "PathToShim",
 			name:    "PathToShim",

+ 1 - 1
daemon/start_unix.go

@@ -10,7 +10,7 @@ import (
 func (daemon *Daemon) getLibcontainerdCreateOptions(daemonCfg *configStore, container *container.Container) (string, interface{}, error) {
 func (daemon *Daemon) getLibcontainerdCreateOptions(daemonCfg *configStore, container *container.Container) (string, interface{}, error) {
 	// Ensure a runtime has been assigned to this container
 	// Ensure a runtime has been assigned to this container
 	if container.HostConfig.Runtime == "" {
 	if container.HostConfig.Runtime == "" {
-		container.HostConfig.Runtime = daemonCfg.DefaultRuntime
+		container.HostConfig.Runtime = daemonCfg.Runtimes.Default
 		container.CheckpointTo(daemon.containersReplica)
 		container.CheckpointTo(daemon.containersReplica)
 	}
 	}
 
 

+ 1 - 1
integration-cli/docker_cli_daemon_test.go

@@ -2275,7 +2275,7 @@ func (s *DockerDaemonSuite) TestRunWithRuntimeFromConfigFile(c *testing.T) {
 
 
 	content, err := s.d.ReadLogFile()
 	content, err := s.d.ReadLogFile()
 	assert.NilError(c, err)
 	assert.NilError(c, err)
-	assert.Assert(c, is.Contains(string(content), `file configuration validation failed: 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
 	// Check that we can select a default runtime
 	config = `
 	config = `
 {
 {