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>
(cherry picked from commit 8a094fe609)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Cory Snider 2023-06-06 12:57:38 -04:00 committed by Sebastiaan van Stijn
parent 4949db7f62
commit 5cc1736418
No known key found for this signature in database
GPG key ID: 76698F39D527CE8C
7 changed files with 108 additions and 33 deletions

View file

@ -54,6 +54,9 @@ func WithRlimits(daemon *Daemon, c *container.Container) coci.SpecOpts {
})
}
if s.Process == nil {
s.Process = &specs.Process{}
}
s.Process.Rlimits = rlimits
return nil
}
@ -114,6 +117,9 @@ func WithRootless(daemon *Daemon) coci.SpecOpts {
// WithOOMScore sets the oom score
func WithOOMScore(score *int) coci.SpecOpts {
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
return nil
}
@ -122,6 +128,12 @@ func WithOOMScore(score *int) coci.SpecOpts {
// WithSelinux sets the selinux labels
func WithSelinux(c *container.Container) coci.SpecOpts {
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.Linux.MountLabel = c.MountLabel
return nil
@ -152,6 +164,9 @@ func WithApparmor(c *container.Container) coci.SpecOpts {
return err
}
}
if s.Process == nil {
s.Process = &specs.Process{}
}
s.Process.ApparmorProfile = appArmorProfile
}
return nil
@ -214,6 +229,10 @@ func getUser(c *container.Container, username string) (specs.User, error) {
}
func setNamespace(s *specs.Spec, ns specs.LinuxNamespace) {
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
for i, n := range s.Linux.Namespaces {
if n.Type == ns.Type {
s.Linux.Namespaces[i] = ns
@ -608,6 +627,9 @@ func WithMounts(daemon *Daemon, c *container.Container) coci.SpecOpts {
}
rootpg := mountPropagationMap[s.Linux.RootfsPropagation]
if rootpg != mount.SHARED && rootpg != mount.RSHARED {
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
s.Linux.RootfsPropagation = mountPropagationReverseMap[mount.SHARED]
}
case mount.SLAVE, mount.RSLAVE:
@ -636,6 +658,9 @@ func WithMounts(daemon *Daemon, c *container.Container) coci.SpecOpts {
if !fallback {
rootpg := mountPropagationMap[s.Linux.RootfsPropagation]
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]
}
}
@ -691,8 +716,10 @@ func WithMounts(daemon *Daemon, c *container.Container) coci.SpecOpts {
clearReadOnly(&s.Mounts[i])
}
}
s.Linux.ReadonlyPaths = nil
s.Linux.MaskedPaths = nil
if s.Linux != nil {
s.Linux.ReadonlyPaths = nil
s.Linux.MaskedPaths = nil
}
}
// TODO: until a kernel/mount solution exists for handling remount in a user namespace,
@ -738,6 +765,9 @@ func WithCommonOptions(daemon *Daemon, c *container.Container) coci.SpecOpts {
if len(cwd) == 0 {
cwd = "/"
}
if s.Process == nil {
s.Process = &specs.Process{}
}
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
@ -817,6 +847,9 @@ func WithCgroups(daemon *Daemon, c *container.Container) coci.SpecOpts {
} else {
cgroupsPath = filepath.Join(parent, c.ID)
}
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
s.Linux.CgroupsPath = cgroupsPath
// the rest is only needed for CPU RT controller
@ -917,8 +950,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.Resources.Devices = devPermissions
s.Linux.Resources.Devices = append(s.Linux.Resources.Devices, devPermissions...)
for _, req := range c.HostConfig.DeviceRequests {
if err := daemon.handleDevice(req, s); err != nil {
@ -960,28 +999,27 @@ func WithResources(c *container.Container) coci.SpecOpts {
return err
}
specResources := &specs.LinuxResources{
Memory: memoryRes,
CPU: cpuRes,
BlockIO: &specs.LinuxBlockIO{
WeightDevice: weightDevices,
ThrottleReadBpsDevice: readBpsDevice,
ThrottleWriteBpsDevice: writeBpsDevice,
ThrottleReadIOPSDevice: readIOpsDevice,
ThrottleWriteIOPSDevice: writeIOpsDevice,
},
Pids: getPidsLimit(r),
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
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,
ThrottleReadBpsDevice: readBpsDevice,
ThrottleWriteBpsDevice: writeBpsDevice,
ThrottleReadIOPSDevice: readIOpsDevice,
ThrottleWriteIOPSDevice: writeIOpsDevice,
}
if r.BlkioWeight != 0 {
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
}
}
@ -989,6 +1027,15 @@ func WithResources(c *container.Container) coci.SpecOpts {
// WithSysctls sets the container's sysctls
func WithSysctls(c *container.Container) coci.SpecOpts {
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
// precedence for backwards-compatibility reasons).
for k, v := range c.HostConfig.Sysctls {
@ -1001,6 +1048,9 @@ func WithSysctls(c *container.Container) coci.SpecOpts {
// WithUser sets the container's user
func WithUser(c *container.Container) coci.SpecOpts {
return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
if s.Process == nil {
s.Process = &specs.Process{}
}
var err error
s.Process.User, err = getUser(c, c.Config.User)
return err

View file

@ -13,6 +13,9 @@ import (
func WithConsoleSize(c *container.Container) coci.SpecOpts {
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 s.Process == nil {
s.Process = &specs.Process{}
}
s.Process.ConsoleSize = &specs.Box{
Height: c.HostConfig.ConsoleSize[0],
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
// is a sysctl which has an identical effect to setdomainname(2) so there's
// no explicit need for runtime support.
s.Linux.Sysctl = make(map[string]string)
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
if s.Linux.Sysctl == nil {
s.Linux.Sysctl = make(map[string]string)
}
if c.Config.Domainname != "" {
s.Linux.Sysctl["kernel.domainname"] = c.Config.Domainname
}

View file

@ -9,6 +9,7 @@ import (
"github.com/docker/docker/container"
dconfig "github.com/docker/docker/daemon/config"
"github.com/docker/docker/profiles/seccomp"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/sirupsen/logrus"
)
@ -31,6 +32,9 @@ func WithSeccomp(daemon *Daemon, c *container.Container) coci.SpecOpts {
c.SeccompProfile = dconfig.SeccompProfileUnconfined
return nil
}
if s.Linux == nil {
s.Linux = &specs.Linux{}
}
var err error
switch {
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`
func RemoveNamespace(s *specs.Spec, nsType specs.LinuxNamespaceType) {
if s.Linux == nil {
return
}
for i, n := range s.Linux.Namespaces {
if n.Type == nsType {
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
// All capabilities are added if privileged is true.
func SetCapabilities(s *specs.Spec, caplist []string) error {
if s.Process == nil {
s.Process = &specs.Process{}
}
// setUser has already been executed here
if s.Process.User.UID == 0 {
s.Process.Capabilities = &specs.LinuxCapabilities{

View file

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