Skip to content

Commit

Permalink
daemon: ensure OCI options play nicely together
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@corhere
corhere committed Jun 6, 2023
1 parent dea870f commit 8a094fe
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 33 deletions.
90 changes: 70 additions & 20 deletions daemon/oci_linux.go
View file
Expand Up @@ -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
return nil
}
Expand Down Expand Up @@ -113,6 +116,9 @@ func withRootless(daemon *Daemon, daemonCfg *dconfig.Config) 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
}
Expand All @@ -121,6 +127,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
Expand Down Expand Up @@ -151,6 +163,9 @@ func WithApparmor(c *container.Container) coci.SpecOpts {
return err
}
}
if s.Process == nil {
s.Process = &specs.Process{}
}
s.Process.ApparmorProfile = appArmorProfile
}
return nil
Expand Down Expand Up @@ -213,6 +228,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
Expand Down Expand Up @@ -606,6 +625,9 @@ func withMounts(daemon *Daemon, daemonCfg *configStore, c *container.Container)
}
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:
Expand Down Expand Up @@ -634,6 +656,9 @@ func withMounts(daemon *Daemon, daemonCfg *configStore, c *container.Container)
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]
}
}
Expand Down Expand Up @@ -706,8 +731,10 @@ func withMounts(daemon *Daemon, daemonCfg *configStore, c *container.Container)
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,
Expand Down Expand Up @@ -755,6 +782,9 @@ func withCommonOptions(daemon *Daemon, daemonCfg *dconfig.Config, c *container.C
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
Expand Down Expand Up @@ -831,6 +861,9 @@ func withCgroups(daemon *Daemon, daemonCfg *dconfig.Config, c *container.Contain
} 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
Expand Down Expand Up @@ -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.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 {
Expand Down Expand Up @@ -974,35 +1013,43 @@ 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
}

if s.Linux.Resources != nil && len(s.Linux.Resources.Devices) > 0 {
specResources.Devices = s.Linux.Resources.Devices
s.Linux.Resources.BlockIO.Weight = &w
}
s.Linux.Resources.Pids = getPidsLimit(r)

s.Linux.Resources = specResources
return nil
}
}

// 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 {
Expand All @@ -1015,6 +1062,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
Expand Down
3 changes: 3 additions & 0 deletions daemon/oci_opts.go
View file
Expand Up @@ -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],
Expand Down
7 changes: 6 additions & 1 deletion daemon/oci_utils.go
View file
Expand Up @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions daemon/seccomp_linux.go
View file
Expand Up @@ -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"
)

Expand All @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions oci/namespaces.go
View file
Expand Up @@ -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:]...)
Expand Down
3 changes: 3 additions & 0 deletions oci/oci.go
View file
Expand Up @@ -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{
Expand Down
31 changes: 19 additions & 12 deletions pkg/rootless/specconv/specconv_linux.go
View file
Expand Up @@ -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{}{}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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 == "" {
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 8a094fe

Please sign in to comment.