Merge pull request #45267 from rumpl/c8d-fix-exec-user

c8d: Set the process user on exec

daemon/exec_linux.go

@@ -3,20 +3,69 @@ package daemon // import "github.com/docker/docker/daemon"
 import (
 	"context"
 
+	"github.com/containerd/containerd"
+	"github.com/containerd/containerd/containers"
+	"github.com/containerd/containerd/oci"
+	coci "github.com/containerd/containerd/oci"
 	"github.com/containerd/containerd/pkg/apparmor"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/oci/caps"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 )
 
+func withResetAdditionalGIDs() oci.SpecOpts {
+	return func(_ context.Context, _ oci.Client, _ *containers.Container, s *oci.Spec) error {
+		s.Process.User.AdditionalGids = nil
+		return nil
+	}
+}
+
+func getUserFromContainerd(ctx context.Context, containerdCli *containerd.Client, ec *container.ExecConfig) (specs.User, error) {
+	ctr, err := containerdCli.LoadContainer(ctx, ec.Container.ID)
+	if err != nil {
+		return specs.User{}, err
+	}
+
+	cinfo, err := ctr.Info(ctx)
+	if err != nil {
+		return specs.User{}, err
+	}
+
+	spec, err := ctr.Spec(ctx)
+	if err != nil {
+		return specs.User{}, err
+	}
+
+	opts := []oci.SpecOpts{
+		coci.WithUser(ec.User),
+		withResetAdditionalGIDs(),
+		coci.WithAdditionalGIDs(ec.User),
+	}
+	for _, opt := range opts {
+		if err := opt(ctx, containerdCli, &cinfo, spec); err != nil {
+			return specs.User{}, err
+		}
+	}
+
+	return spec.Process.User, nil
+}
+
 func (daemon *Daemon) execSetPlatformOpt(ctx context.Context, ec *container.ExecConfig, p *specs.Process) error {
 	if len(ec.User) > 0 {
 		var err error
-		p.User, err = getUser(ec.Container, ec.User)
-		if err != nil {
-			return err
+		if daemon.UsesSnapshotter() {
+			p.User, err = getUserFromContainerd(ctx, daemon.containerdCli, ec)
+			if err != nil {
+				return err
+			}
+		} else {
+			p.User, err = getUser(ec.Container, ec.User)
+			if err != nil {
+				return err
+			}
 		}
 	}
+
 	if ec.Privileged {
 		p.Capabilities = &specs.LinuxCapabilities{
 			Bounding:  caps.GetAllCapabilities(),
@@ -24,6 +73,7 @@ func (daemon *Daemon) execSetPlatformOpt(ctx context.Context, ec *container.Exec
 			Effective: caps.GetAllCapabilities(),
 		}
 	}
+
 	if apparmor.HostSupports() {
 		var appArmorProfile string
 		if ec.Container.AppArmorProfile != "" {

vendor.mod

@@ -25,7 +25,7 @@ require (
 	github.com/bsphere/le_go v0.0.0-20200109081728-fc06dab2caa8
 	github.com/cloudflare/cfssl v0.0.0-20180323000720-5d63dbd981b5
 	github.com/containerd/cgroups/v3 v3.0.1
-	github.com/containerd/containerd v1.6.20
+	github.com/containerd/containerd v1.6.21-0.20230406162538-c0efc63d3907
 	github.com/containerd/continuity v0.3.0
 	github.com/containerd/fifo v1.1.0
 	github.com/containerd/typeurl/v2 v2.1.0

vendor.sum

@@ -372,8 +372,8 @@ github.com/containerd/containerd v1.5.0-beta.4/go.mod h1:GmdgZd2zA2GYIBZ0w09Zvgq
 github.com/containerd/containerd v1.5.0-rc.0/go.mod h1:V/IXoMqNGgBlabz3tHD2TWDoTJseu1FGOKuoA4nNb2s=
 github.com/containerd/containerd v1.5.1/go.mod h1:0DOxVqwDy2iZvrZp2JUx/E+hS0UNTVn7dJnIOwtYR4g=
 github.com/containerd/containerd v1.5.7/go.mod h1:gyvv6+ugqY25TiXxcZC3L5yOeYgEw0QMhscqVp1AR9c=
-github.com/containerd/containerd v1.6.20 h1:+itjwpdqXpzHB/QAiWc/BZCjjVfcNgw69w/oIeF4Oy0=
-github.com/containerd/containerd v1.6.20/go.mod h1:apei1/i5Ux2FzrK6+DM/suEsGuK/MeVOfy8tR2q7Wnw=
+github.com/containerd/containerd v1.6.21-0.20230406162538-c0efc63d3907 h1:77Ml66lA9zDrfdVBpNTAnxko/vm5YKRlDEJwkWgxYlg=
+github.com/containerd/containerd v1.6.21-0.20230406162538-c0efc63d3907/go.mod h1:apei1/i5Ux2FzrK6+DM/suEsGuK/MeVOfy8tR2q7Wnw=
 github.com/containerd/continuity v0.0.0-20190426062206-aaeac12a7ffc/go.mod h1:GL3xCUCBDV3CZiTSEKksMWbLE66hEyuu9qyDOOqM47Y=
 github.com/containerd/continuity v0.0.0-20190815185530-f2a389ac0a02/go.mod h1:GL3xCUCBDV3CZiTSEKksMWbLE66hEyuu9qyDOOqM47Y=
 github.com/containerd/continuity v0.0.0-20191127005431-f65d91d395eb/go.mod h1:GL3xCUCBDV3CZiTSEKksMWbLE66hEyuu9qyDOOqM47Y=

vendor/github.com/containerd/containerd/Vagrantfile

@@ -93,7 +93,7 @@ EOF
   config.vm.provision "install-golang", type: "shell", run: "once" do |sh|
     sh.upload_path = "/tmp/vagrant-install-golang"
     sh.env = {
-        'GO_VERSION': ENV['GO_VERSION'] || "1.19.7",
+        'GO_VERSION': ENV['GO_VERSION'] || "1.19.8",
     }
     sh.inline = <<~SHELL
         #!/usr/bin/env bash

vendor/github.com/containerd/containerd/images/archive/exporter.go

@@ -176,7 +176,7 @@ func Export(ctx context.Context, store content.Provider, writer io.Writer, opts
 			}
 
 			name := desc.Annotations[images.AnnotationImageName]
-			if name != "" && !eo.skipDockerManifest {
+			if name != "" {
 				mt.names = append(mt.names, name)
 			}
 		case images.MediaTypeDockerSchema2ManifestList, ocispec.MediaTypeImageIndex:
@@ -215,26 +215,24 @@ func Export(ctx context.Context, store content.Provider, writer io.Writer, opts
 					records = append(records, r...)
 				}
 
-				if !eo.skipDockerManifest {
-					if len(manifests) >= 1 {
-						if len(manifests) > 1 {
-							sort.SliceStable(manifests, func(i, j int) bool {
-								if manifests[i].Platform == nil {
-									return false
-								}
-								if manifests[j].Platform == nil {
-									return true
-								}
-								return eo.platform.Less(*manifests[i].Platform, *manifests[j].Platform)
-							})
-						}
-						d = manifests[0].Digest
-						dManifests[d] = &exportManifest{
-							manifest: manifests[0],
-						}
-					} else if eo.platform != nil {
-						return fmt.Errorf("no manifest found for platform: %w", errdefs.ErrNotFound)
+				if len(manifests) >= 1 {
+					if len(manifests) > 1 {
+						sort.SliceStable(manifests, func(i, j int) bool {
+							if manifests[i].Platform == nil {
+								return false
+							}
+							if manifests[j].Platform == nil {
+								return true
+							}
+							return eo.platform.Less(*manifests[i].Platform, *manifests[j].Platform)
+						})
 					}
+					d = manifests[0].Digest
+					dManifests[d] = &exportManifest{
+						manifest: manifests[0],
+					}
+				} else if eo.platform != nil {
+					return fmt.Errorf("no manifest found for platform: %w", errdefs.ErrNotFound)
 				}
 				resolvedIndex[desc.Digest] = d
 			}
@@ -250,7 +248,7 @@ func Export(ctx context.Context, store content.Provider, writer io.Writer, opts
 		}
 	}
 
-	if len(dManifests) > 0 {
+	if !eo.skipDockerManifest && len(dManifests) > 0 {
 		tr, err := manifestsRecord(ctx, store, dManifests)
 		if err != nil {
 			return fmt.Errorf("unable to create manifests file: %w", err)

vendor/github.com/containerd/containerd/oci/spec_opts.go

@@ -663,8 +663,7 @@ func WithUser(userstr string) SpecOpts {
 				return err
 			}
 
-			mounts = tryReadonlyMounts(mounts)
-			return mount.WithTempMount(ctx, mounts, f)
+			return mount.WithReadonlyTempMount(ctx, mounts, f)
 		default:
 			return fmt.Errorf("invalid USER value %s", userstr)
 		}
@@ -724,8 +723,7 @@ func WithUserID(uid uint32) SpecOpts {
 			return err
 		}
 
-		mounts = tryReadonlyMounts(mounts)
-		return mount.WithTempMount(ctx, mounts, setUser)
+		return mount.WithReadonlyTempMount(ctx, mounts, setUser)
 	}
 }
 
@@ -769,8 +767,7 @@ func WithUsername(username string) SpecOpts {
 				return err
 			}
 
-			mounts = tryReadonlyMounts(mounts)
-			return mount.WithTempMount(ctx, mounts, setUser)
+			return mount.WithReadonlyTempMount(ctx, mounts, setUser)
 		} else if s.Windows != nil {
 			s.Process.User.Username = username
 		} else {
@@ -848,8 +845,7 @@ func WithAdditionalGIDs(userstr string) SpecOpts {
 			return err
 		}
 
-		mounts = tryReadonlyMounts(mounts)
-		return mount.WithTempMount(ctx, mounts, setAdditionalGids)
+		return mount.WithReadonlyTempMount(ctx, mounts, setAdditionalGids)
 	}
 }
 
@@ -910,8 +906,7 @@ func WithAppendAdditionalGroups(groups ...string) SpecOpts {
 			return err
 		}
 
-		mounts = tryReadonlyMounts(mounts)
-		return mount.WithTempMount(ctx, mounts, setAdditionalGids)
+		return mount.WithReadonlyTempMount(ctx, mounts, setAdditionalGids)
 	}
 }
 
@@ -1389,21 +1384,3 @@ func WithDevShmSize(kb int64) SpecOpts {
 		return ErrNoShmMount
 	}
 }
-
-// tryReadonlyMounts is used by the options which are trying to get user/group
-// information from container's rootfs. Since the option does read operation
-// only, this helper will append ReadOnly mount option to prevent linux kernel
-// from syncing whole filesystem in umount syscall.
-//
-// TODO(fuweid):
-//
-// Currently, it only works for overlayfs. I think we can apply it to other
-// kinds of filesystem. Maybe we can return `ro` option by `snapshotter.Mount`
-// API, when the caller passes that experimental annotation
-// `containerd.io/snapshot/readonly.mount` something like that.
-func tryReadonlyMounts(mounts []mount.Mount) []mount.Mount {
-	if len(mounts) == 1 && mounts[0].Type == "overlay" {
-		mounts[0].Options = append(mounts[0].Options, "ro")
-	}
-	return mounts
-}

vendor/modules.txt

@@ -216,7 +216,7 @@ github.com/containerd/cgroups/v3/cgroup2/stats
 # github.com/containerd/console v1.0.3
 ## explicit; go 1.13
 github.com/containerd/console
-# github.com/containerd/containerd v1.6.20
+# github.com/containerd/containerd v1.6.21-0.20230406162538-c0efc63d3907
 ## explicit; go 1.17
 github.com/containerd/containerd
 github.com/containerd/containerd/api/events