beenull/moby
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge pull request #43993 from cpuguy83/22.06_backport_43978_default_runtime

Browse source 
[22.06] Allow containerd shim refs in default-runtime
This commit is contained in:
Sebastiaan van Stijn 2022-08-18 23:00:34 +02:00 committed by GitHub
commit 1e6029e81e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 83 additions and 70 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
daemon/config/config.go
View file

@ -7,9 +7,11 @@ import (
"net"
"net/url"
"os"
"path/filepath"
"strings"
"sync"
"github.com/containerd/containerd/runtime/v2/shim"
"github.com/docker/docker/opts"
"github.com/docker/docker/pkg/authorization"
"github.com/docker/docker/registry"
@ -630,7 +632,7 @@ func Validate(config *Config) error {
if defaultRuntime := config.GetDefaultRuntimeName(); defaultRuntime != "" {
if !builtinRuntimes[defaultRuntime] {
runtimes := config.GetAllRuntimes()
if _, ok := runtimes[defaultRuntime]; !ok {
if _, ok := runtimes[defaultRuntime]; !ok && !IsPermissibleC8dRuntimeName(defaultRuntime) {
return fmt.Errorf("specified default runtime '%s' does not exist", defaultRuntime)
}
}
@ -664,3 +666,37 @@ func MaskCredentials(rawURL string) string {
parsedURL.User = url.UserPassword("xxxxx", "xxxxx")
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) != ""
}

12
daemon/config/config_linux_test.go
View file

@ -139,18 +139,6 @@ func TestUnixValidateConfigurationErrors(t *testing.T) {
},
expectedErr: `runtime name 'runc' is reserved`,
},
{
doc: `default runtime should be present in runtimes`,
config: &Config{
Runtimes: map[string]types.Runtime{
"foo": {},
},
CommonConfig: CommonConfig{
DefaultRuntime: "bar",
},
},
expectedErr: `specified default runtime 'bar' does not exist`,
},
}
for _, tc := range testCases {
tc := tc

4
daemon/daemon_unix.go
View file

@ -764,7 +764,9 @@ func verifyDaemonSettings(conf *config.Config) error {
configureRuntimes(conf)
if rtName := conf.GetDefaultRuntimeName(); rtName != "" {
if conf.GetRuntime(rtName) == nil {
return fmt.Errorf("specified default runtime '%s' does not exist", rtName)
if !config.IsPermissibleC8dRuntimeName(rtName) {
return fmt.Errorf("specified default runtime '%s' does not exist", rtName)
}
}
}
return nil

42
daemon/info_unix.go
View file

@ -45,15 +45,16 @@ func (daemon *Daemon) fillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo)
v.ContainerdCommit.ID = "N/A"
v.InitCommit.ID = "N/A"
defaultRuntimeBinary := daemon.configStore.GetRuntime(v.DefaultRuntime).Path
if rv, err := exec.Command(defaultRuntimeBinary, "--version").Output(); err == nil {
if _, _, commit, err := parseRuntimeVersion(string(rv)); err != nil {
logrus.Warnf("failed to parse %s version: %v", defaultRuntimeBinary, err)
if rt := daemon.configStore.GetRuntime(v.DefaultRuntime); rt != nil {
if rv, err := exec.Command(rt.Path, "--version").Output(); err == nil {
if _, _, commit, err := parseRuntimeVersion(string(rv)); err != nil {
logrus.Warnf("failed to parse %s version: %v", rt.Path, err)
} else {
v.RuncCommit.ID = commit
}
} else {
v.RuncCommit.ID = commit
logrus.Warnf("failed to retrieve %s version: %v", rt.Path, err)
}
} else {
logrus.Warnf("failed to retrieve %s version: %v", defaultRuntimeBinary, err)
}
if rv, err := daemon.containerd.Version(context.Background()); err == nil {
@ -176,21 +177,22 @@ func (daemon *Daemon) fillPlatformVersion(v *types.Version) {
}
defaultRuntime := daemon.configStore.GetDefaultRuntimeName()
defaultRuntimeBinary := daemon.configStore.GetRuntime(defaultRuntime).Path
if rv, err := exec.Command(defaultRuntimeBinary, "--version").Output(); err == nil {
if _, ver, commit, err := parseRuntimeVersion(string(rv)); err != nil {
logrus.Warnf("failed to parse %s version: %v", defaultRuntimeBinary, err)
if rt := daemon.configStore.GetRuntime(defaultRuntime); rt != nil {
if rv, err := exec.Command(rt.Path, "--version").Output(); err == nil {
if _, ver, commit, err := parseRuntimeVersion(string(rv)); err != nil {
logrus.Warnf("failed to parse %s version: %v", rt.Path, err)
} else {
v.Components = append(v.Components, types.ComponentVersion{
Name: defaultRuntime,
Version: ver,
Details: map[string]string{
"GitCommit": commit,
},
})
}
} else {
v.Components = append(v.Components, types.ComponentVersion{
Name: defaultRuntime,
Version: ver,
Details: map[string]string{
"GitCommit": commit,
},
})
logrus.Warnf("failed to retrieve %s version: %v", rt.Path, err)
}
} else {
logrus.Warnf("failed to retrieve %s version: %v", defaultRuntimeBinary, err)
}
defaultInitBinary := daemon.configStore.GetInitPath()

37
daemon/runtime_unix.go
View file

@ -11,7 +11,6 @@ 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"
@ -117,7 +116,7 @@ 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 {
if !isPermissibleC8dRuntimeName(name) {
if !config.IsPermissibleC8dRuntimeName(name) {
return nil, errdefs.InvalidParameter(errors.Errorf("unknown or invalid runtime name: %s", name))
}
return &types.Runtime{Shim: &types.ShimConfig{Binary: name}}, nil
@ -138,37 +137,3 @@ 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) != ""
}

20
integration/container/run_linux_test.go
View file

@ -270,4 +270,24 @@ func TestRunWithAlternativeContainerdShim(t *testing.T) {
assert.NilError(t, err)
assert.Equal(t, strings.TrimSpace(b.String()), "Hello, world!")
d.Stop(t)
d.Start(t, "--default-runtime="+"io.containerd.realfake.v42")
cID = container.Run(ctx, t, client,
container.WithImage("busybox"),
container.WithCmd("sh", "-c", `echo 'Hello, world!'`),
)
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()
b.Reset()
_, err = stdcopy.StdCopy(&b, io.Discard, out)
assert.NilError(t, err)
assert.Equal(t, strings.TrimSpace(b.String()), "Hello, world!")
}