daemon: ensure OCI options play nicely together

Audit the OCI spec options used for Linux containers to ensure they are
less order-dependent. Ensure they don't assume that any pointer fields
are non-nil and that they don't unintentionally clobber mutations to the
spec applied by other options.

Signed-off-by: Cory Snider <csnider@mirantis.com>
This commit is contained in:
Cory Snider 2023-06-06 12:57:38 -04:00
parent dea870f4ea
commit 8a094fe609
7 changed files with 108 additions and 33 deletions

View file

@ -53,6 +53,9 @@ func withRlimits(daemon *Daemon, daemonCfg *dconfig.Config, c *container.Contain
}) })
} }
if s.Process == nil {
s.Process = &specs.Process{}
}
s.Process.Rlimits = rlimits s.Process.Rlimits = rlimits
return nil return nil
} }
@ -113,6 +116,9 @@ func withRootless(daemon *Daemon, daemonCfg *dconfig.Config) coci.SpecOpts {
// WithOOMScore sets the oom score // WithOOMScore sets the oom score
func WithOOMScore(score *int) coci.SpecOpts { func WithOOMScore(score *int) coci.SpecOpts {
return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error { return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
if s.Process == nil {
s.Process = &specs.Process{}
}
s.Process.OOMScoreAdj = score s.Process.OOMScoreAdj = score
return nil return nil
} }
@ -121,6 +127,12 @@ func WithOOMScore(score *int) coci.SpecOpts {
// WithSelinux sets the selinux labels // WithSelinux sets the selinux labels
func WithSelinux(c *container.Container) coci.SpecOpts { func WithSelinux(c *container.Container) coci.SpecOpts {
return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error { return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
if s.Process == nil {
s.Process = &specs.Process{}
}
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
s.Process.SelinuxLabel = c.GetProcessLabel() s.Process.SelinuxLabel = c.GetProcessLabel()
s.Linux.MountLabel = c.MountLabel s.Linux.MountLabel = c.MountLabel
return nil return nil
@ -151,6 +163,9 @@ func WithApparmor(c *container.Container) coci.SpecOpts {
return err return err
} }
} }
if s.Process == nil {
s.Process = &specs.Process{}
}
s.Process.ApparmorProfile = appArmorProfile s.Process.ApparmorProfile = appArmorProfile
} }
return nil return nil
@ -213,6 +228,10 @@ func getUser(c *container.Container, username string) (specs.User, error) {
} }
func setNamespace(s *specs.Spec, ns specs.LinuxNamespace) { func setNamespace(s *specs.Spec, ns specs.LinuxNamespace) {
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
for i, n := range s.Linux.Namespaces { for i, n := range s.Linux.Namespaces {
if n.Type == ns.Type { if n.Type == ns.Type {
s.Linux.Namespaces[i] = ns s.Linux.Namespaces[i] = ns
@ -606,6 +625,9 @@ func withMounts(daemon *Daemon, daemonCfg *configStore, c *container.Container)
} }
rootpg := mountPropagationMap[s.Linux.RootfsPropagation] rootpg := mountPropagationMap[s.Linux.RootfsPropagation]
if rootpg != mount.SHARED && rootpg != mount.RSHARED { if rootpg != mount.SHARED && rootpg != mount.RSHARED {
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
s.Linux.RootfsPropagation = mountPropagationReverseMap[mount.SHARED] s.Linux.RootfsPropagation = mountPropagationReverseMap[mount.SHARED]
} }
case mount.SLAVE, mount.RSLAVE: case mount.SLAVE, mount.RSLAVE:
@ -634,6 +656,9 @@ func withMounts(daemon *Daemon, daemonCfg *configStore, c *container.Container)
if !fallback { if !fallback {
rootpg := mountPropagationMap[s.Linux.RootfsPropagation] rootpg := mountPropagationMap[s.Linux.RootfsPropagation]
if rootpg != mount.SHARED && rootpg != mount.RSHARED && rootpg != mount.SLAVE && rootpg != mount.RSLAVE { if rootpg != mount.SHARED && rootpg != mount.RSHARED && rootpg != mount.SLAVE && rootpg != mount.RSLAVE {
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
s.Linux.RootfsPropagation = mountPropagationReverseMap[mount.RSLAVE] s.Linux.RootfsPropagation = mountPropagationReverseMap[mount.RSLAVE]
} }
} }
@ -706,9 +731,11 @@ func withMounts(daemon *Daemon, daemonCfg *configStore, c *container.Container)
clearReadOnly(&s.Mounts[i]) clearReadOnly(&s.Mounts[i])
} }
} }
if s.Linux != nil {
s.Linux.ReadonlyPaths = nil s.Linux.ReadonlyPaths = nil
s.Linux.MaskedPaths = nil s.Linux.MaskedPaths = nil
} }
}
// TODO: until a kernel/mount solution exists for handling remount in a user namespace, // TODO: until a kernel/mount solution exists for handling remount in a user namespace,
// we must clear the readonly flag for the cgroups mount (@mrunalp concurs) // we must clear the readonly flag for the cgroups mount (@mrunalp concurs)
@ -755,6 +782,9 @@ func withCommonOptions(daemon *Daemon, daemonCfg *dconfig.Config, c *container.C
if len(cwd) == 0 { if len(cwd) == 0 {
cwd = "/" cwd = "/"
} }
if s.Process == nil {
s.Process = &specs.Process{}
}
s.Process.Args = append([]string{c.Path}, c.Args...) s.Process.Args = append([]string{c.Path}, c.Args...)
// only add the custom init if it is specified and the container is running in its // only add the custom init if it is specified and the container is running in its
@ -831,6 +861,9 @@ func withCgroups(daemon *Daemon, daemonCfg *dconfig.Config, c *container.Contain
} else { } else {
cgroupsPath = filepath.Join(parent, c.ID) cgroupsPath = filepath.Join(parent, c.ID)
} }
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
s.Linux.CgroupsPath = cgroupsPath s.Linux.CgroupsPath = cgroupsPath
// the rest is only needed for CPU RT controller // the rest is only needed for CPU RT controller
@ -931,8 +964,14 @@ func WithDevices(daemon *Daemon, c *container.Container) coci.SpecOpts {
} }
} }
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
if s.Linux.Resources == nil {
s.Linux.Resources = &specs.LinuxResources{}
}
s.Linux.Devices = append(s.Linux.Devices, devs...) s.Linux.Devices = append(s.Linux.Devices, devs...)
s.Linux.Resources.Devices = devPermissions s.Linux.Resources.Devices = append(s.Linux.Resources.Devices, devPermissions...)
for _, req := range c.HostConfig.DeviceRequests { for _, req := range c.HostConfig.DeviceRequests {
if err := daemon.handleDevice(req, s); err != nil { if err := daemon.handleDevice(req, s); err != nil {
@ -974,28 +1013,27 @@ func WithResources(c *container.Container) coci.SpecOpts {
return err return err
} }
specResources := &specs.LinuxResources{ if s.Linux == nil {
Memory: memoryRes, s.Linux = &specs.Linux{}
CPU: cpuRes, }
BlockIO: &specs.LinuxBlockIO{ if s.Linux.Resources == nil {
s.Linux.Resources = &specs.LinuxResources{}
}
s.Linux.Resources.Memory = memoryRes
s.Linux.Resources.CPU = cpuRes
s.Linux.Resources.BlockIO = &specs.LinuxBlockIO{
WeightDevice: weightDevices, WeightDevice: weightDevices,
ThrottleReadBpsDevice: readBpsDevice, ThrottleReadBpsDevice: readBpsDevice,
ThrottleWriteBpsDevice: writeBpsDevice, ThrottleWriteBpsDevice: writeBpsDevice,
ThrottleReadIOPSDevice: readIOpsDevice, ThrottleReadIOPSDevice: readIOpsDevice,
ThrottleWriteIOPSDevice: writeIOpsDevice, ThrottleWriteIOPSDevice: writeIOpsDevice,
},
Pids: getPidsLimit(r),
} }
if r.BlkioWeight != 0 { if r.BlkioWeight != 0 {
w := r.BlkioWeight w := r.BlkioWeight
specResources.BlockIO.Weight = &w s.Linux.Resources.BlockIO.Weight = &w
} }
s.Linux.Resources.Pids = getPidsLimit(r)
if s.Linux.Resources != nil && len(s.Linux.Resources.Devices) > 0 {
specResources.Devices = s.Linux.Resources.Devices
}
s.Linux.Resources = specResources
return nil return nil
} }
} }
@ -1003,6 +1041,15 @@ func WithResources(c *container.Container) coci.SpecOpts {
// WithSysctls sets the container's sysctls // WithSysctls sets the container's sysctls
func WithSysctls(c *container.Container) coci.SpecOpts { func WithSysctls(c *container.Container) coci.SpecOpts {
return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error { return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
if len(c.HostConfig.Sysctls) == 0 {
return nil
}
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
if s.Linux.Sysctl == nil {
s.Linux.Sysctl = make(map[string]string)
}
// We merge the sysctls injected above with the HostConfig (latter takes // We merge the sysctls injected above with the HostConfig (latter takes
// precedence for backwards-compatibility reasons). // precedence for backwards-compatibility reasons).
for k, v := range c.HostConfig.Sysctls { for k, v := range c.HostConfig.Sysctls {
@ -1015,6 +1062,9 @@ func WithSysctls(c *container.Container) coci.SpecOpts {
// WithUser sets the container's user // WithUser sets the container's user
func WithUser(c *container.Container) coci.SpecOpts { func WithUser(c *container.Container) coci.SpecOpts {
return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error { return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
if s.Process == nil {
s.Process = &specs.Process{}
}
var err error var err error
s.Process.User, err = getUser(c, c.Config.User) s.Process.User, err = getUser(c, c.Config.User)
return err return err

View file

@ -13,6 +13,9 @@ import (
func WithConsoleSize(c *container.Container) coci.SpecOpts { func WithConsoleSize(c *container.Container) coci.SpecOpts {
return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error { return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
if c.HostConfig.ConsoleSize[0] > 0 || c.HostConfig.ConsoleSize[1] > 0 { if c.HostConfig.ConsoleSize[0] > 0 || c.HostConfig.ConsoleSize[1] > 0 {
if s.Process == nil {
s.Process = &specs.Process{}
}
s.Process.ConsoleSize = &specs.Box{ s.Process.ConsoleSize = &specs.Box{
Height: c.HostConfig.ConsoleSize[0], Height: c.HostConfig.ConsoleSize[0],
Width: c.HostConfig.ConsoleSize[1], Width: c.HostConfig.ConsoleSize[1],

View file

@ -9,7 +9,12 @@ func setLinuxDomainname(c *container.Container, s *specs.Spec) {
// There isn't a field in the OCI for the NIS domainname, but luckily there // There isn't a field in the OCI for the NIS domainname, but luckily there
// is a sysctl which has an identical effect to setdomainname(2) so there's // is a sysctl which has an identical effect to setdomainname(2) so there's
// no explicit need for runtime support. // no explicit need for runtime support.
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
if s.Linux.Sysctl == nil {
s.Linux.Sysctl = make(map[string]string) s.Linux.Sysctl = make(map[string]string)
}
if c.Config.Domainname != "" { if c.Config.Domainname != "" {
s.Linux.Sysctl["kernel.domainname"] = c.Config.Domainname s.Linux.Sysctl["kernel.domainname"] = c.Config.Domainname
} }

View file

@ -9,6 +9,7 @@ import (
"github.com/docker/docker/container" "github.com/docker/docker/container"
dconfig "github.com/docker/docker/daemon/config" dconfig "github.com/docker/docker/daemon/config"
"github.com/docker/docker/profiles/seccomp" "github.com/docker/docker/profiles/seccomp"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
) )
@ -31,6 +32,9 @@ func WithSeccomp(daemon *Daemon, c *container.Container) coci.SpecOpts {
c.SeccompProfile = dconfig.SeccompProfileUnconfined c.SeccompProfile = dconfig.SeccompProfileUnconfined
return nil return nil
} }
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
var err error var err error
switch { switch {
case c.SeccompProfile == dconfig.SeccompProfileDefault: case c.SeccompProfile == dconfig.SeccompProfileDefault:

View file

@ -4,6 +4,9 @@ import specs "github.com/opencontainers/runtime-spec/specs-go"
// RemoveNamespace removes the `nsType` namespace from OCI spec `s` // RemoveNamespace removes the `nsType` namespace from OCI spec `s`
func RemoveNamespace(s *specs.Spec, nsType specs.LinuxNamespaceType) { func RemoveNamespace(s *specs.Spec, nsType specs.LinuxNamespaceType) {
if s.Linux == nil {
return
}
for i, n := range s.Linux.Namespaces { for i, n := range s.Linux.Namespaces {
if n.Type == nsType { if n.Type == nsType {
s.Linux.Namespaces = append(s.Linux.Namespaces[:i], s.Linux.Namespaces[i+1:]...) s.Linux.Namespaces = append(s.Linux.Namespaces[:i], s.Linux.Namespaces[i+1:]...)

View file

@ -20,6 +20,9 @@ var deviceCgroupRuleRegex = regexp.MustCompile("^([acb]) ([0-9]+|\\*):([0-9]+|\\
// SetCapabilities sets the provided capabilities on the spec // SetCapabilities sets the provided capabilities on the spec
// All capabilities are added if privileged is true. // All capabilities are added if privileged is true.
func SetCapabilities(s *specs.Spec, caplist []string) error { func SetCapabilities(s *specs.Spec, caplist []string) error {
if s.Process == nil {
s.Process = &specs.Process{}
}
// setUser has already been executed here // setUser has already been executed here
if s.Process.User.UID == 0 { if s.Process.User.UID == 0 {
s.Process.Capabilities = &specs.LinuxCapabilities{ s.Process.Capabilities = &specs.LinuxCapabilities{

View file

@ -40,11 +40,13 @@ func getCurrentOOMScoreAdj() int {
func toRootless(spec *specs.Spec, v2Controllers []string, currentOOMScoreAdj int) error { func toRootless(spec *specs.Spec, v2Controllers []string, currentOOMScoreAdj int) error {
if len(v2Controllers) == 0 { if len(v2Controllers) == 0 {
if spec.Linux != nil {
// Remove cgroup settings. // Remove cgroup settings.
spec.Linux.Resources = nil spec.Linux.Resources = nil
spec.Linux.CgroupsPath = "" spec.Linux.CgroupsPath = ""
}
} else { } else {
if spec.Linux.Resources != nil { if spec.Linux != nil && spec.Linux.Resources != nil {
m := make(map[string]struct{}) m := make(map[string]struct{})
for _, s := range v2Controllers { for _, s := range v2Controllers {
m[s] = struct{}{} m[s] = struct{}{}
@ -77,7 +79,7 @@ func toRootless(spec *specs.Spec, v2Controllers []string, currentOOMScoreAdj int
} }
} }
if spec.Process.OOMScoreAdj != nil && *spec.Process.OOMScoreAdj < currentOOMScoreAdj { if spec.Process != nil && spec.Process.OOMScoreAdj != nil && *spec.Process.OOMScoreAdj < currentOOMScoreAdj {
*spec.Process.OOMScoreAdj = currentOOMScoreAdj *spec.Process.OOMScoreAdj = currentOOMScoreAdj
} }
@ -110,6 +112,9 @@ func isHostNS(spec *specs.Spec, nsType specs.LinuxNamespaceType) (bool, error) {
if strings.Contains(string(nsType), string(os.PathSeparator)) { if strings.Contains(string(nsType), string(os.PathSeparator)) {
return false, fmt.Errorf("unexpected namespace type %q", nsType) return false, fmt.Errorf("unexpected namespace type %q", nsType)
} }
if spec.Linux == nil {
return false, nil
}
for _, ns := range spec.Linux.Namespaces { for _, ns := range spec.Linux.Namespaces {
if ns.Type == nsType { if ns.Type == nsType {
if ns.Path == "" { if ns.Path == "" {
@ -144,6 +149,7 @@ func bindMountHostProcfs(spec *specs.Spec) error {
} }
} }
if spec.Linux != nil {
// Remove ReadonlyPaths for /proc/* // Remove ReadonlyPaths for /proc/*
newROP := spec.Linux.ReadonlyPaths[:0] newROP := spec.Linux.ReadonlyPaths[:0]
for _, s := range spec.Linux.ReadonlyPaths { for _, s := range spec.Linux.ReadonlyPaths {
@ -153,6 +159,7 @@ func bindMountHostProcfs(spec *specs.Spec) error {
} }
} }
spec.Linux.ReadonlyPaths = newROP spec.Linux.ReadonlyPaths = newROP
}
return nil return nil
} }