Merge pull request #46785 from rumpl/c8d-userns-commit

c8d: handle user namespace remapping on commit

daemon/containerd/image_commit.go

@@ -16,6 +16,7 @@ import (
 	cerrdefs "github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/leases"
+	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/rootfs"
 	"github.com/containerd/containerd/snapshots"
 	"github.com/containerd/log"
@@ -90,7 +91,7 @@ func (i *ImageService) CommitImage(ctx context.Context, cc backend.CommitConfig)
 	if diffLayerDesc != nil {
 		rootfsID := identity.ChainID(imageConfig.RootFS.DiffIDs).String()
 
-		if err := applyDiffLayer(ctx, rootfsID, parentImage, sn, differ, *diffLayerDesc); err != nil {
+		if err := i.applyDiffLayer(ctx, rootfsID, cc.ContainerID, sn, differ, *diffLayerDesc); err != nil {
 			return "", fmt.Errorf("failed to apply diff: %w", err)
 		}
 
@@ -291,13 +292,19 @@ func createDiff(ctx context.Context, name string, sn snapshots.Snapshotter, cs c
 }
 
 // applyDiffLayer will apply diff layer content created by createDiff into the snapshotter.
-func applyDiffLayer(ctx context.Context, name string, baseImg imagespec.DockerOCIImage, sn snapshots.Snapshotter, differ diff.Applier, diffDesc ocispec.Descriptor) (retErr error) {
+func (i *ImageService) applyDiffLayer(ctx context.Context, name string, containerID string, sn snapshots.Snapshotter, differ diff.Applier, diffDesc ocispec.Descriptor) (retErr error) {
 	var (
 		key    = uniquePart() + "-" + name
-		parent = identity.ChainID(baseImg.RootFS.DiffIDs).String()
+		mounts []mount.Mount
+		err    error
 	)
 
-	mount, err := sn.Prepare(ctx, key, parent)
+	info, err := sn.Stat(ctx, containerID)
+	if err != nil {
+		return err
+	}
+
+	mounts, err = sn.Prepare(ctx, key, info.Parent)
 	if err != nil {
 		return fmt.Errorf("failed to prepare snapshot: %w", err)
 	}
@@ -312,16 +319,46 @@ func applyDiffLayer(ctx context.Context, name string, baseImg imagespec.DockerOC
 		}
 	}()
 
-	if _, err = differ.Apply(ctx, diffDesc, mount); err != nil {
+	if _, err = differ.Apply(ctx, diffDesc, mounts); err != nil {
 		return err
 	}
 
+	if !i.idMapping.Empty() {
+		// The rootfs of the container is remapped if an id mapping exists, we
+		// need to "unremap" it before committing the snapshot
+		rootPair := i.idMapping.RootPair()
+		usernsID := fmt.Sprintf("%s-%d-%d", key, rootPair.UID, rootPair.GID)
+		remappedID := usernsID + remapSuffix
+
+		if err = sn.Commit(ctx, name+"-pre", key); err != nil {
+			if cerrdefs.IsAlreadyExists(err) {
+				return nil
+			}
+			return err
+		}
+
+		mounts, err = sn.Prepare(ctx, remappedID, name+"-pre")
+		if err != nil {
+			return err
+		}
+
+		if err := i.unremapRootFS(ctx, mounts); err != nil {
+			return err
+		}
+
+		if err := sn.Commit(ctx, name, remappedID); err != nil {
+			return err
+		}
+		key = remappedID
+	}
+
 	if err = sn.Commit(ctx, name, key); err != nil {
 		if cerrdefs.IsAlreadyExists(err) {
 			return nil
 		}
 		return err
 	}
+
 	return nil
 }
 

daemon/containerd/image_snapshot_unix.go

@@ -67,3 +67,25 @@ func (i *ImageService) remapRootFS(ctx context.Context, mounts []mount.Mount) er
 		})
 	})
 }
+
+func (i *ImageService) unremapRootFS(ctx context.Context, mounts []mount.Mount) error {
+	return mount.WithTempMount(ctx, mounts, func(root string) error {
+		return filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+			if err != nil {
+				return err
+			}
+
+			stat := info.Sys().(*syscall.Stat_t)
+			if stat == nil {
+				return fmt.Errorf("cannot get underlying data for %s", path)
+			}
+
+			uid, gid, err := i.idMapping.ToContainer(idtools.Identity{UID: int(stat.Uid), GID: int(stat.Gid)})
+			if err != nil {
+				return err
+			}
+
+			return os.Lchown(path, uid, gid)
+		})
+	})
+}

daemon/containerd/image_snapshot_windows.go

@@ -3,9 +3,14 @@ package containerd
 import (
 	"context"
 
+	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/snapshots"
 )
 
 func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string) error {
 	return nil
 }
+
+func (i *ImageService) unremapRootFS(ctx context.Context, mounts []mount.Mount) error {
+	return nil
+}

integration/image/commit_test.go

@@ -1,12 +1,14 @@
 package image // import "github.com/docker/docker/integration/image"
 
 import (
+	"context"
 	"strings"
 	"testing"
 
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/versions"
 	"github.com/docker/docker/integration/internal/container"
+	"github.com/docker/docker/testutil/daemon"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/skip"
@@ -47,3 +49,26 @@ func TestCommitInheritsEnv(t *testing.T) {
 	expectedEnv2 := []string{"PATH=/usr/bin:/bin"}
 	assert.Check(t, is.DeepEqual(expectedEnv2, image2.Config.Env))
 }
+
+// Verify that files created are owned by the remapped user even after a commit
+func TestUsernsCommit(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType != "linux")
+	skip.If(t, testEnv.IsRemoteDaemon())
+	skip.If(t, !testEnv.IsUserNamespaceInKernel())
+	skip.If(t, testEnv.IsRootless())
+
+	ctx := context.Background()
+	dUserRemap := daemon.New(t, daemon.WithUserNsRemap("default"))
+	dUserRemap.StartWithBusybox(ctx, t)
+	clientUserRemap := dUserRemap.NewClientT(t)
+	defer clientUserRemap.Close()
+
+	container.Run(ctx, t, clientUserRemap, container.WithName(t.Name()), container.WithImage("busybox"), container.WithCmd("sh", "-c", "echo hello world > /hello.txt && chown 1000:1000 /hello.txt"))
+	img, err := clientUserRemap.ContainerCommit(ctx, t.Name(), containertypes.CommitOptions{})
+	assert.NilError(t, err)
+
+	res := container.RunAttach(ctx, t, clientUserRemap, container.WithImage(img.ID), container.WithCmd("sh", "-c", "stat -c %u:%g /hello.txt"))
+	assert.Check(t, is.Equal(res.ExitCode, 0))
+	assert.Check(t, is.Equal(res.Stderr.String(), ""))
+	assert.Assert(t, is.Equal(strings.TrimSpace(res.Stdout.String()), "1000:1000"))
+}

testutil/daemon/daemon.go

@@ -83,6 +83,7 @@ type Daemon struct {
 	args                       []string
 	extraEnv                   []string
 	containerdSocket           string
+	usernsRemap                string
 	rootlessUser               *user.User
 	rootlessXDGRuntimeDir      string
 
@@ -457,6 +458,10 @@ func (d *Daemon) StartWithLogFile(out *os.File, providedArgs ...string) error {
 		d.args = append(d.args, "--containerd", d.containerdSocket)
 	}
 
+	if d.usernsRemap != "" {
+		d.args = append(d.args, "--userns-remap", d.usernsRemap)
+	}
+
 	if d.defaultCgroupNamespaceMode != "" {
 		d.args = append(d.args, "--default-cgroupns-mode", d.defaultCgroupNamespaceMode)
 	}

testutil/daemon/ops.go

@@ -19,6 +19,12 @@ func WithContainerdSocket(socket string) Option {
 	}
 }
 
+func WithUserNsRemap(remap string) Option {
+	return func(d *Daemon) {
+		d.usernsRemap = remap
+	}
+}
+
 // WithDefaultCgroupNamespaceMode sets the default cgroup namespace mode for the daemon
 func WithDefaultCgroupNamespaceMode(mode string) Option {
 	return func(d *Daemon) {