From 0313544f4a9370cb18cc65916e66a3d840191edf Mon Sep 17 00:00:00 2001 From: Djordje Lukic Date: Wed, 30 Aug 2023 15:47:00 +0200 Subject: [PATCH] c8d: Handle userns properly If the daemon is run with --userns-remap we need to chown the prepared snapshot Signed-off-by: Djordje Lukic --- daemon/containerd/image_snapshot.go | 12 ++- daemon/containerd/image_snapshot_unix.go | 84 +++++++++++++++++++++ daemon/containerd/image_snapshot_windows.go | 12 +++ daemon/containerd/service.go | 4 + daemon/daemon.go | 1 + integration-cli/docker_cli_userns_test.go | 7 +- 6 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 daemon/containerd/image_snapshot_unix.go create mode 100644 daemon/containerd/image_snapshot_windows.go diff --git a/daemon/containerd/image_snapshot.go b/daemon/containerd/image_snapshot.go index 21e36a4f7c..40eeccdd0c 100644 --- a/daemon/containerd/image_snapshot.go +++ b/daemon/containerd/image_snapshot.go @@ -16,6 +16,8 @@ import ( "github.com/pkg/errors" ) +const remapSuffix = "-remap" + // PrepareSnapshot prepares a snapshot from a parent image for a container func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentImage string, platform *ocispec.Platform) error { var parentSnapshot string @@ -63,7 +65,6 @@ func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentIma if err != nil { return err } - if err := ls.AddResource(ctx, lease, leases.Resource{ ID: id, Type: "snapshots/" + i.StorageDriver(), @@ -71,8 +72,13 @@ func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentIma return err } - s := i.client.SnapshotService(i.StorageDriver()) - _, err = s.Prepare(ctx, id, parentSnapshot) + snapshotter := i.client.SnapshotService(i.StorageDriver()) + + if !i.idMapping.Empty() { + return i.remapSnapshot(ctx, snapshotter, id, parentSnapshot, lease) + } + + _, err = snapshotter.Prepare(ctx, id, parentSnapshot) return err } diff --git a/daemon/containerd/image_snapshot_unix.go b/daemon/containerd/image_snapshot_unix.go new file mode 100644 index 0000000000..d5e2977601 --- /dev/null +++ b/daemon/containerd/image_snapshot_unix.go @@ -0,0 +1,84 @@ +//go:build !windows + +package containerd + +import ( + "context" + "fmt" + "os" + "path/filepath" + "syscall" + + "github.com/containerd/containerd/leases" + "github.com/containerd/containerd/log" + "github.com/containerd/containerd/mount" + "github.com/containerd/containerd/snapshots" + "github.com/docker/docker/pkg/idtools" +) + +func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string, lease leases.Lease) error { + ls := i.client.LeasesService() + rootPair := i.idMapping.RootPair() + usernsID := fmt.Sprintf("%s-%d-%d", parentSnapshot, rootPair.UID, rootPair.GID) + remappedID := usernsID + remapSuffix + + // If the remapped snapshot already exist we only need to prepare the new snapshot + if _, err := snapshotter.Stat(ctx, usernsID); err == nil { + _, err = snapshotter.Prepare(ctx, id, usernsID) + return err + } + + if err := ls.AddResource(ctx, lease, leases.Resource{ + ID: remappedID, + Type: "snapshots/" + i.StorageDriver(), + }); err != nil { + return err + } + if err := ls.AddResource(ctx, lease, leases.Resource{ + ID: usernsID, + Type: "snapshots/" + i.StorageDriver(), + }); err != nil { + return err + } + + mounts, err := snapshotter.Prepare(ctx, remappedID, parentSnapshot) + if err != nil { + return err + } + + if err := i.remapRootFS(ctx, mounts); err != nil { + if rmErr := snapshotter.Remove(ctx, usernsID); rmErr != nil { + log.G(ctx).WithError(rmErr).Warn("failed to remove snapshot after remap error") + } + return err + } + + if err := snapshotter.Commit(ctx, usernsID, remappedID); err != nil { + return err + } + + _, err = snapshotter.Prepare(ctx, id, usernsID) + return err +} + +func (i *ImageService) remapRootFS(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) + } + + ids, err := i.idMapping.ToHost(idtools.Identity{UID: int(stat.Uid), GID: int(stat.Gid)}) + if err != nil { + return err + } + + return os.Lchown(path, ids.UID, ids.GID) + }) + }) +} diff --git a/daemon/containerd/image_snapshot_windows.go b/daemon/containerd/image_snapshot_windows.go new file mode 100644 index 0000000000..7330efd17f --- /dev/null +++ b/daemon/containerd/image_snapshot_windows.go @@ -0,0 +1,12 @@ +package containerd + +import ( + "context" + + "github.com/containerd/containerd/leases" + "github.com/containerd/containerd/snapshots" +) + +func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string, lease leases.Lease) error { + return nil +} diff --git a/daemon/containerd/service.go b/daemon/containerd/service.go index 04ae26abab..c751c81f96 100644 --- a/daemon/containerd/service.go +++ b/daemon/containerd/service.go @@ -19,6 +19,7 @@ import ( "github.com/docker/docker/errdefs" "github.com/docker/docker/image" "github.com/docker/docker/layer" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/registry" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" @@ -34,6 +35,7 @@ type ImageService struct { eventsService *daemonevents.Events pruneRunning atomic.Bool refCountMounter snapshotter.Mounter + idMapping idtools.IdentityMapping } type RegistryConfigProvider interface { @@ -49,6 +51,7 @@ type ImageServiceConfig struct { Registry RegistryConfigProvider EventsService *daemonevents.Events RefCountMounter snapshotter.Mounter + IDMapping idtools.IdentityMapping } // NewService creates a new ImageService. @@ -61,6 +64,7 @@ func NewService(config ImageServiceConfig) *ImageService { registryService: config.Registry, eventsService: config.EventsService, refCountMounter: config.RefCountMounter, + idMapping: config.IDMapping, } } diff --git a/daemon/daemon.go b/daemon/daemon.go index f93788d692..0e27dd02a6 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1072,6 +1072,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S RegistryHosts: d.RegistryHosts, Registry: d.registryService, EventsService: d.EventsService, + IDMapping: idMapping, RefCountMounter: snapshotter.NewMounter(config.Root, driverName, idMapping), }) } else { diff --git a/integration-cli/docker_cli_userns_test.go b/integration-cli/docker_cli_userns_test.go index 054eac9726..327eb79bd1 100644 --- a/integration-cli/docker_cli_userns_test.go +++ b/integration-cli/docker_cli_userns_test.go @@ -16,6 +16,7 @@ import ( "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/testutil" "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" ) // user namespaces test: run daemon with remapped root setting @@ -27,6 +28,10 @@ func (s *DockerDaemonSuite) TestDaemonUserNamespaceRootSetting(c *testing.T) { ctx := testutil.GetContext(c) s.d.StartWithBusybox(ctx, c, "--userns-remap", "default") + out, err := s.d.Cmd("run", "busybox", "stat", "-c", "%u:%g", "/bin/cat") + assert.Check(c, err) + assert.Assert(c, is.Equal(strings.TrimSpace(out), "0:0")) + tmpDir, err := os.MkdirTemp("", "userns") assert.NilError(c, err) @@ -47,7 +52,7 @@ func (s *DockerDaemonSuite) TestDaemonUserNamespaceRootSetting(c *testing.T) { // writable by the remapped root UID/GID pair assert.NilError(c, os.Chown(tmpDir, uid, gid)) - out, err := s.d.Cmd("run", "-d", "--name", "userns", "-v", tmpDir+":/goofy", "-v", tmpDirNotExists+":/donald", "busybox", "sh", "-c", "touch /goofy/testfile; exec top") + out, err = s.d.Cmd("run", "-d", "--name", "userns", "-v", tmpDir+":/goofy", "-v", tmpDirNotExists+":/donald", "busybox", "sh", "-c", "touch /goofy/testfile; exec top") assert.NilError(c, err, "Output: %s", out) user := s.findUser(c, "userns")