Merge pull request #46972 from dmcgowan/fix-userns-capabilities

c8d: Fix image commit with userns mapping (carry)

daemon/containerd/image_commit.go

@@ -17,7 +17,7 @@ import (
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/leases"
 	"github.com/containerd/containerd/mount"
-	"github.com/containerd/containerd/rootfs"
+	"github.com/containerd/containerd/pkg/cleanup"
 	"github.com/containerd/containerd/snapshots"
 	"github.com/containerd/log"
 	"github.com/docker/docker/api/types/backend"
@@ -29,6 +29,7 @@ import (
 	"github.com/opencontainers/image-spec/identity"
 	"github.com/opencontainers/image-spec/specs-go"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/pkg/errors"
 )
 
 /*
@@ -81,7 +82,7 @@ func (i *ImageService) CommitImage(ctx context.Context, cc backend.CommitConfig)
 		}
 	}()
 
-	diffLayerDesc, diffID, err := createDiff(ctx, cc.ContainerID, sn, cs, differ)
+	diffLayerDesc, diffID, err := i.createDiff(ctx, cc.ContainerID, sn, cs, differ)
 	if err != nil {
 		return "", fmt.Errorf("failed to export layer: %w", err)
 	}
@@ -249,12 +250,82 @@ func writeContentsForImage(ctx context.Context, snName string, cs content.Store,
 
 // createDiff creates a layer diff into containerd's content store.
 // If the diff is empty it returns nil empty digest and no error.
-func createDiff(ctx context.Context, name string, sn snapshots.Snapshotter, cs content.Store, comparer diff.Comparer) (*ocispec.Descriptor, digest.Digest, error) {
-	newDesc, err := rootfs.CreateDiff(ctx, name, sn, comparer)
+func (i *ImageService) createDiff(ctx context.Context, name string, sn snapshots.Snapshotter, cs content.Store, comparer diff.Comparer) (*ocispec.Descriptor, digest.Digest, error) {
+	info, err := sn.Stat(ctx, name)
 	if err != nil {
 		return nil, "", err
 	}
 
+	var upper []mount.Mount
+	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-%s", name, rootPair.UID, rootPair.GID, uniquePart())
+		remappedID := usernsID + remapSuffix
+		baseName := name
+
+		if info.Kind == snapshots.KindActive {
+			source, err := sn.Mounts(ctx, name)
+			if err != nil {
+				return nil, "", err
+			}
+
+			// No need to use parent since the whole snapshot is copied.
+			// Using parent would require doing diff/apply while starting
+			// from empty can just copy the whole snapshot.
+			// TODO: Optimize this for overlay mounts, can use parent
+			// and just copy upper directories without mounting
+			upper, err = sn.Prepare(ctx, remappedID, "")
+			if err != nil {
+				return nil, "", err
+			}
+
+			if err := i.copyAndUnremapRootFS(ctx, upper, source); err != nil {
+				return nil, "", err
+			}
+		} else {
+			upper, err = sn.Prepare(ctx, remappedID, baseName)
+			if err != nil {
+				return nil, "", err
+			}
+
+			if err := i.unremapRootFS(ctx, upper); err != nil {
+				return nil, "", err
+			}
+		}
+	} else {
+		if info.Kind == snapshots.KindActive {
+			upper, err = sn.Mounts(ctx, name)
+			if err != nil {
+				return nil, "", err
+			}
+		} else {
+			upperKey := fmt.Sprintf("%s-view-%s", name, uniquePart())
+			upper, err = sn.View(ctx, upperKey, name)
+			if err != nil {
+				return nil, "", err
+			}
+			defer cleanup.Do(ctx, func(ctx context.Context) {
+				sn.Remove(ctx, upperKey)
+			})
+		}
+	}
+
+	lowerKey := fmt.Sprintf("%s-parent-view-%s", info.Parent, uniquePart())
+	lower, err := sn.View(ctx, lowerKey, info.Parent)
+	if err != nil {
+		return nil, "", err
+	}
+	defer cleanup.Do(ctx, func(ctx context.Context) {
+		sn.Remove(ctx, lowerKey)
+	})
+
+	newDesc, err := comparer.Compare(ctx, lower, upper)
+	if err != nil {
+		return nil, "", errors.Wrap(err, "CreateDiff")
+	}
+
 	ra, err := cs.ReaderAt(ctx, newDesc)
 	if err != nil {
 		return nil, "", fmt.Errorf("failed to read diff archive: %w", err)
@@ -269,12 +340,12 @@ func createDiff(ctx context.Context, name string, sn snapshots.Snapshotter, cs c
 		return nil, "", nil
 	}
 
-	info, err := cs.Info(ctx, newDesc.Digest)
+	cinfo, err := cs.Info(ctx, newDesc.Digest)
 	if err != nil {
 		return nil, "", fmt.Errorf("failed to get content info: %w", err)
 	}
 
-	diffIDStr, ok := info.Labels["containerd.io/uncompressed"]
+	diffIDStr, ok := cinfo.Labels["containerd.io/uncompressed"]
 	if !ok {
 		return nil, "", fmt.Errorf("invalid differ response with no diffID")
 	}
@@ -287,7 +358,7 @@ func createDiff(ctx context.Context, name string, sn snapshots.Snapshotter, cs c
 	return &ocispec.Descriptor{
 		MediaType: ocispec.MediaTypeImageLayerGzip,
 		Digest:    newDesc.Digest,
-		Size:      info.Size,
+		Size:      cinfo.Size,
 	}, diffID, nil
 }
 
@@ -311,7 +382,7 @@ func (i *ImageService) applyDiffLayer(ctx context.Context, name string, containe
 
 	defer func() {
 		if retErr != nil {
-			// NOTE: the snapshotter should be hold by lease. Even
+			// NOTE: the snapshotter should be held by lease. Even
 			// if the cleanup fails, the containerd gc can delete it.
 			if err := sn.Remove(ctx, key); err != nil {
 				log.G(ctx).Warnf("failed to cleanup aborted apply %s: %s", key, err)
@@ -323,35 +394,6 @@ func (i *ImageService) applyDiffLayer(ctx context.Context, name string, containe
 		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

daemon/containerd/image_snapshot.go

@@ -17,8 +17,6 @@ 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, setupInit func(string) error) error {
 	var parentSnapshot string

daemon/containerd/image_snapshot_unix.go

@@ -11,38 +11,34 @@ import (
 
 	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/snapshots"
-	"github.com/containerd/log"
+	"github.com/containerd/continuity/fs"
+	"github.com/containerd/continuity/sysx"
 	"github.com/docker/docker/pkg/idtools"
 )
 
-func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string) error {
-	rootPair := i.idMapping.RootPair()
-	usernsID := fmt.Sprintf("%s-%d-%d", parentSnapshot, rootPair.UID, rootPair.GID)
-	remappedID := usernsID + remapSuffix
+const (
+	// Values based on linux/include/uapi/linux/capability.h
+	xattrCapsSz2    = 20
+	versionOffset   = 3
+	vfsCapRevision2 = 2
+	vfsCapRevision3 = 3
+	remapSuffix     = "-remap"
+)
 
-	// 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)
+func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string) error {
+	_, err := snapshotter.Prepare(ctx, id, parentSnapshot)
+	if err != nil {
 		return err
 	}
-
-	mounts, err := snapshotter.Prepare(ctx, remappedID, parentSnapshot)
+	mounts, err := snapshotter.Mounts(ctx, id)
 	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
 }
 
@@ -63,7 +59,36 @@ func (i *ImageService) remapRootFS(ctx context.Context, mounts []mount.Mount) er
 				return err
 			}
 
-			return os.Lchown(path, ids.UID, ids.GID)
+			return chownWithCaps(path, ids.UID, ids.GID)
+		})
+	})
+}
+
+func (i *ImageService) copyAndUnremapRootFS(ctx context.Context, dst, src []mount.Mount) error {
+	return mount.WithTempMount(ctx, src, func(source string) error {
+		return mount.WithTempMount(ctx, dst, func(root string) error {
+			// TODO: Update CopyDir to support remap directly
+			if err := fs.CopyDir(root, source); err != nil {
+				return fmt.Errorf("failed to copy: %w", err)
+			}
+
+			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 chownWithCaps(path, uid, gid)
+			})
 		})
 	})
 }
@@ -85,7 +110,48 @@ func (i *ImageService) unremapRootFS(ctx context.Context, mounts []mount.Mount)
 				return err
 			}
 
-			return os.Lchown(path, uid, gid)
+			return chownWithCaps(path, uid, gid)
 		})
 	})
 }
+
+// chownWithCaps will chown path and preserve the extended attributes.
+// chowning a file will remove the capabilities, so we need to first get all of
+// them, chown the file, and then set the extended attributes
+func chownWithCaps(path string, uid int, gid int) error {
+	xattrKeys, err := sysx.LListxattr(path)
+	if err != nil {
+		return err
+	}
+
+	xattrs := make(map[string][]byte, len(xattrKeys))
+
+	for _, xattr := range xattrKeys {
+		data, err := sysx.LGetxattr(path, xattr)
+		if err != nil {
+			return err
+		}
+		xattrs[xattr] = data
+	}
+
+	if err := os.Lchown(path, uid, gid); err != nil {
+		return err
+	}
+
+	for xattrKey, xattrValue := range xattrs {
+		length := len(xattrValue)
+		// make sure the capabilities are version 2,
+		// capabilities version 3 also store the root uid of the namespace,
+		// we don't want this when we are in userns-remap mode
+		// see: https://github.com/moby/moby/pull/41724
+		if xattrKey == "security.capability" && xattrValue[versionOffset] == vfsCapRevision3 {
+			xattrValue[versionOffset] = vfsCapRevision2
+			length = xattrCapsSz2
+		}
+		if err := sysx.LSetxattr(path, xattrKey, xattrValue[:length], 0); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}

daemon/containerd/image_snapshot_windows.go

@@ -7,6 +7,12 @@ import (
 	"github.com/containerd/containerd/snapshots"
 )
 
+const remapSuffix = "-remap"
+
+func (i *ImageService) copyAndUnremapRootFS(ctx context.Context, dst, src []mount.Mount) error {
+	return nil
+}
+
 func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string) error {
 	return nil
 }