diff --git a/daemon/containerd/image_commit.go b/daemon/containerd/image_commit.go index 85be475393..c6f08fd253 100644 --- a/daemon/containerd/image_commit.go +++ b/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 } diff --git a/daemon/containerd/image_snapshot_unix.go b/daemon/containerd/image_snapshot_unix.go index d14b90e625..491034aabc 100644 --- a/daemon/containerd/image_snapshot_unix.go +++ b/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) + }) + }) +} diff --git a/daemon/containerd/image_snapshot_windows.go b/daemon/containerd/image_snapshot_windows.go index bb4ed3ac12..d81be30b3d 100644 --- a/daemon/containerd/image_snapshot_windows.go +++ b/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 +} diff --git a/integration/image/commit_test.go b/integration/image/commit_test.go index 1852a72c30..2efa1a25b3 100644 --- a/integration/image/commit_test.go +++ b/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")) +} diff --git a/testutil/daemon/daemon.go b/testutil/daemon/daemon.go index 65b9cba62e..280f0dc7b2 100644 --- a/testutil/daemon/daemon.go +++ b/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) } diff --git a/testutil/daemon/ops.go b/testutil/daemon/ops.go index 61676f78e0..d1c78886d8 100644 --- a/testutil/daemon/ops.go +++ b/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) {