From 0f51aa874dbaeb15ea1885f67a20939380e7aa08 Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Fri, 9 Sep 2022 15:46:18 -0700 Subject: [PATCH] Add NoSameOwner option when unpacking tars When unpacking a TAR archive, containerd preserves file's owner: https://github.com/containerd/containerd/blob/main/archive/tar.go#L384 In some cases this behavior is not desired. In current implementation we avoid `Lchown` on Windows. Another case when this should be skipped is when using native snapshotter on darwin and running as non-root user. This PR extracts a generic option - `WithNoSameOwner` (same as `tar --no-same-owner`) to skip `Lchown` when its not required. Signed-off-by: Maksym Pavlenko --- archive/issues_test.go | 2 +- archive/tar.go | 8 +++----- archive/tar_opts.go | 10 ++++++++++ diff/apply/apply_darwin.go | 9 ++++++++- diff/windows/windows.go | 12 +++++++++--- install.go | 13 +++++++++++-- 6 files changed, 42 insertions(+), 12 deletions(-) diff --git a/archive/issues_test.go b/archive/issues_test.go index de6d090cb..7c4729aad 100644 --- a/archive/issues_test.go +++ b/archive/issues_test.go @@ -43,7 +43,7 @@ func TestPrefixHeaderReadable(t *testing.T) { t.Fatal(err) } defer r.Close() - _, err = Apply(context.Background(), tmpDir, r) + _, err = Apply(context.Background(), tmpDir, r, WithNoSameOwner()) if err != nil { t.Fatal(err) } diff --git a/archive/tar.go b/archive/tar.go index 1ea14d31d..59266b367 100644 --- a/archive/tar.go +++ b/archive/tar.go @@ -24,7 +24,6 @@ import ( "io" "os" "path/filepath" - "runtime" "strings" "sync" "syscall" @@ -290,7 +289,7 @@ func applyNaive(ctx context.Context, root string, r io.Reader, options ApplyOpti srcData := io.Reader(tr) srcHdr := hdr - if err := createTarFile(ctx, path, root, srcHdr, srcData); err != nil { + if err := createTarFile(ctx, path, root, srcHdr, srcData, options.NoSameOwner); err != nil { return 0, err } @@ -315,7 +314,7 @@ func applyNaive(ctx context.Context, root string, r io.Reader, options ApplyOpti return size, nil } -func createTarFile(ctx context.Context, path, extractDir string, hdr *tar.Header, reader io.Reader) error { +func createTarFile(ctx context.Context, path, extractDir string, hdr *tar.Header, reader io.Reader, noSameOwner bool) error { // hdr.Mode is in linux format, which we can use for syscalls, // but for os.Foo() calls we need the mode converted to os.FileMode, // so use hdrInfo.Mode() (they differ for e.g. setuid bits) @@ -380,8 +379,7 @@ func createTarFile(ctx context.Context, path, extractDir string, hdr *tar.Header return fmt.Errorf("unhandled tar header type %d", hdr.Typeflag) } - // Lchown is not supported on Windows. - if runtime.GOOS != "windows" { + if !noSameOwner { if err := os.Lchown(path, hdr.Uid, hdr.Gid); err != nil { err = fmt.Errorf("failed to Lchown %q for UID %d, GID %d: %w", path, hdr.Uid, hdr.Gid, err) if errors.Is(err, syscall.EINVAL) && userns.RunningInUserNS() { diff --git a/archive/tar_opts.go b/archive/tar_opts.go index 58985555a..cd6932070 100644 --- a/archive/tar_opts.go +++ b/archive/tar_opts.go @@ -27,6 +27,7 @@ type ApplyOptions struct { Filter Filter // Filter tar headers ConvertWhiteout ConvertWhiteout // Convert whiteout files Parents []string // Parent directories to handle inherited attributes without CoW + NoSameOwner bool // NoSameOwner will not attempt to preserve the owner specified in the tar archive. applyFunc func(context.Context, string, io.Reader, ApplyOptions) (int64, error) } @@ -61,6 +62,15 @@ func WithConvertWhiteout(c ConvertWhiteout) ApplyOpt { } } +// WithNoSameOwner is same as '--no-same-owner` in 'tar' command. +// It'll skip attempt to preserve the owner specified in the tar archive. +func WithNoSameOwner() ApplyOpt { + return func(options *ApplyOptions) error { + options.NoSameOwner = true + return nil + } +} + // WithParents provides parent directories for resolving inherited attributes // directory from the filesystem. // Inherited attributes are searched from first to last, making the first diff --git a/diff/apply/apply_darwin.go b/diff/apply/apply_darwin.go index 88654023c..dd93d3107 100644 --- a/diff/apply/apply_darwin.go +++ b/diff/apply/apply_darwin.go @@ -19,6 +19,7 @@ package apply import ( "context" "io" + "os" "github.com/containerd/containerd/archive" "github.com/containerd/containerd/mount" @@ -28,8 +29,14 @@ func apply(ctx context.Context, mounts []mount.Mount, r io.Reader) error { // We currently do not support mounts nor bind mounts on MacOS in the containerd daemon. // Using this as an exception to enable native snapshotter and allow further research. if len(mounts) == 1 && mounts[0].Type == "bind" { + opts := []archive.ApplyOpt{} + + if os.Getuid() != 0 { + opts = append(opts, archive.WithNoSameOwner()) + } + path := mounts[0].Source - _, err := archive.Apply(ctx, path, r) + _, err := archive.Apply(ctx, path, r, opts...) return err } diff --git a/diff/windows/windows.go b/diff/windows/windows.go index 97afb5a5f..904e27a4a 100644 --- a/diff/windows/windows.go +++ b/diff/windows/windows.go @@ -27,7 +27,7 @@ import ( "io" "time" - winio "github.com/Microsoft/go-winio" + "github.com/Microsoft/go-winio" "github.com/containerd/containerd/archive" "github.com/containerd/containerd/archive/compression" "github.com/containerd/containerd/content" @@ -38,7 +38,7 @@ import ( "github.com/containerd/containerd/mount" "github.com/containerd/containerd/platforms" "github.com/containerd/containerd/plugin" - digest "github.com/opencontainers/go-digest" + "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" ) @@ -144,7 +144,13 @@ func (s windowsDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts return emptyDesc, err } - if _, err := archive.Apply(ctx, layer, rc, archive.WithParents(parentLayerPaths), archive.AsWindowsContainerLayer()); err != nil { + archiveOpts := []archive.ApplyOpt{ + archive.WithParents(parentLayerPaths), + archive.AsWindowsContainerLayer(), + archive.WithNoSameOwner(), // Lchown is not supported on Windows + } + + if _, err := archive.Apply(ctx, layer, rc, archiveOpts...); err != nil { return emptyDesc, err } diff --git a/install.go b/install.go index 16cff08d2..c3ea82384 100644 --- a/install.go +++ b/install.go @@ -70,7 +70,8 @@ func (c *Client) Install(ctx context.Context, image Image, opts ...InstallOpts) ra.Close() return err } - if _, err := archive.Apply(ctx, path, r, archive.WithFilter(func(hdr *tar.Header) (bool, error) { + + filter := archive.WithFilter(func(hdr *tar.Header) (bool, error) { d := filepath.Dir(hdr.Name) result := d == binDir @@ -87,7 +88,15 @@ func (c *Client) Install(ctx context.Context, image Image, opts ...InstallOpts) } } return result, nil - })); err != nil { + }) + + opts := []archive.ApplyOpt{filter} + + if runtime.GOOS == "windows" { + opts = append(opts, archive.WithNoSameOwner()) + } + + if _, err := archive.Apply(ctx, path, r, opts...); err != nil { r.Close() ra.Close() return err