From 829c9fe574f9019f5a0a2ea54b83b7bf9b6e50f5 Mon Sep 17 00:00:00 2001 From: David Trudgian Date: Fri, 1 Mar 2024 11:24:50 +0000 Subject: [PATCH] fix: honor --userns in unsquashfs wrapping If singularity is executed with `--userns/-u` then where possible it should also use a user namespace where it executes `unsquashfs` in a wrapped manner. Previously the `unsquashfs` wrapping was without `--userns/-u` in a setuid installation. This caused extraction to fail from within a non-root-mapped user namespace (e.g. `unshare -c`). Part of #2698 --- CHANGELOG.md | 5 ++++- internal/pkg/build/sources/packer_sif.go | 2 +- internal/pkg/build/sources/packer_squashfs.go | 2 +- internal/pkg/image/unpacker/squashfs.go | 21 ++++++++++++------- .../image/unpacker/squashfs_singularity.go | 15 ++++++------- internal/pkg/image/unpacker/squashfs_test.go | 2 +- .../runtime/launcher/native/launcher_linux.go | 18 +++++++++++----- pkg/image/image_test.go | 2 +- 8 files changed, 42 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49e3561860..6c7c0bbc52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,8 +19,11 @@ the original image. - Fix `target: no such file or directory` error in native mode when extracting layers from certain OCI images that manipulate hard links across layers. -- Fix extraction of OCI layers when run in a root mapped user namespace +- Fix extraction of OCI layers when run in a root mapped user namespace (e.g.. `unshare -r`). +- Use user namespace for wrapping of `unsquashfs` when singularity is run with + `--userns / -u` flag. Fixes temporary sandbox extraction of images in non-root + mapped user namespace (e.g. `unshare -c`). ## 4.1.1 \[2024-02-01\] diff --git a/internal/pkg/build/sources/packer_sif.go b/internal/pkg/build/sources/packer_sif.go index 9dfb23b044..5e047c3d3d 100644 --- a/internal/pkg/build/sources/packer_sif.go +++ b/internal/pkg/build/sources/packer_sif.go @@ -51,7 +51,7 @@ func unpackSIF(b *types.Bundle, img *image.Image) (err error) { return fmt.Errorf("could not extract root filesystem: %s", err) } - s := unpacker.NewSquashfs() + s := unpacker.NewSquashfs(false) // extract root filesystem if err := s.ExtractAll(reader, b.RootfsPath); err != nil { diff --git a/internal/pkg/build/sources/packer_squashfs.go b/internal/pkg/build/sources/packer_squashfs.go index 26610e280e..80cc02b1d0 100644 --- a/internal/pkg/build/sources/packer_squashfs.go +++ b/internal/pkg/build/sources/packer_squashfs.go @@ -29,7 +29,7 @@ func (p *SquashfsPacker) Pack(context.Context) (*types.Bundle, error) { return nil, fmt.Errorf("could not extract root filesystem: %s", err) } - s := unpacker.NewSquashfs() + s := unpacker.NewSquashfs(false) // extract root filesystem if err := s.ExtractAll(reader, p.b.RootfsPath); err != nil { diff --git a/internal/pkg/image/unpacker/squashfs.go b/internal/pkg/image/unpacker/squashfs.go index d6c875ffd7..cada217213 100644 --- a/internal/pkg/image/unpacker/squashfs.go +++ b/internal/pkg/image/unpacker/squashfs.go @@ -26,11 +26,11 @@ const ( excludeDevRegex = `^(.{0}[^d]|.{1}[^e]|.{2}[^v]|.{3}[^\x2f]).*$` ) -var cmdFunc func(unsquashfs string, dest string, filename string, filter string, opts ...string) (*exec.Cmd, error) +var cmdFunc func(squashfs *Squashfs, dest string, filename string, filter string, opts ...string) (*exec.Cmd, error) // unsquashfsCmd is the command instance for executing unsquashfs command // in a non sandboxed environment when this package is used for unit tests. -func unsquashfsCmd(unsquashfs string, dest string, filename string, filter string, opts ...string) (*exec.Cmd, error) { +func unsquashfsCmd(squashfs *Squashfs, dest string, filename string, filter string, opts ...string) (*exec.Cmd, error) { args := []string{} args = append(args, opts...) // remove the destination directory if any, if the directory is @@ -49,18 +49,23 @@ func unsquashfsCmd(unsquashfs string, dest string, filename string, filter strin args = append(args, filter) } - sylog.Debugf("Calling %s %v", unsquashfs, args) - return exec.Command(unsquashfs, args...), nil + sylog.Debugf("Calling %s %v", squashfs.UnsquashfsPath, args) + return exec.Command(squashfs.UnsquashfsPath, args...), nil } // Squashfs represents a squashfs unpacker. type Squashfs struct { + // Path to the unsquashfs executable UnsquashfsPath string + // ForceUserns sets --userns when unsquashfs is being run wrapped by singularity. + ForceUserns bool } // NewSquashfs initializes and returns a Squahfs unpacker instance -func NewSquashfs() *Squashfs { - s := &Squashfs{} +func NewSquashfs(userns bool) *Squashfs { + s := &Squashfs{ + ForceUserns: userns, + } s.UnsquashfsPath, _ = bin.FindBin("unsquashfs") return s } @@ -112,7 +117,7 @@ func (s *Squashfs) extract(files []string, reader io.Reader, dest string) (err e if err != nil { return fmt.Errorf("could not get host UID: %s", err) } - rootless := hostuid != 0 + rootless := s.ForceUserns || hostuid != 0 // Does our target filesystem support user xattrs? ok, err := TestUserXattr(filepath.Dir(dest)) @@ -155,7 +160,7 @@ func (s *Squashfs) extract(files []string, reader io.Reader, dest string) (err e // Now run unsquashfs with our 'best' options sylog.Debugf("Trying unsquashfs options: %v", opts) - cmd, err := cmdFunc(s.UnsquashfsPath, dest, filename, filter, opts...) + cmd, err := cmdFunc(s, dest, filename, filter, opts...) if err != nil { return fmt.Errorf("command error: %s", err) } diff --git a/internal/pkg/image/unpacker/squashfs_singularity.go b/internal/pkg/image/unpacker/squashfs_singularity.go index 3eb00c1bbc..3ecf1974be 100644 --- a/internal/pkg/image/unpacker/squashfs_singularity.go +++ b/internal/pkg/image/unpacker/squashfs_singularity.go @@ -144,7 +144,7 @@ func parseLibraryBinds(buf io.Reader) ([]libBind, error) { // unsquashfsSandboxCmd is the command instance for executing unsquashfs command // in a sandboxed environment with singularity. -func unsquashfsSandboxCmd(unsquashfs string, dest string, filename string, filter string, opts ...string) (*exec.Cmd, error) { +func unsquashfsSandboxCmd(squashfs *Squashfs, dest string, filename string, filter string, opts ...string) (*exec.Cmd, error) { const ( // will contain both dest and filename inside the sandbox rootfsImageDir = "/image" @@ -186,9 +186,6 @@ func unsquashfsSandboxCmd(unsquashfs string, dest string, filename string, filte } } - // the decision to use user namespace is left to singularity - // which will detect automatically depending of the configuration - // what workflow it could use args := []string{ "exec", "--no-home", @@ -200,16 +197,20 @@ func unsquashfsSandboxCmd(unsquashfs string, dest string, filename string, filte "-B", fmt.Sprintf("%s:%s", tmpdir, rootfsImageDir), } + if squashfs.ForceUserns { + args = append(args, "--userns") + } + if filename != stdinFile { filename = filepath.Join(rootfsImageDir, filepath.Base(filename)) } roFiles := []string{ - unsquashfs, + squashfs.UnsquashfsPath, } // get the library dependencies of unsquashfs - libs, err := getLibraryBinds(unsquashfs) + libs, err := getLibraryBinds(squashfs.UnsquashfsPath) if err != nil { return nil, err } @@ -262,7 +263,7 @@ func unsquashfsSandboxCmd(unsquashfs string, dest string, filename string, filte args = append(args, rootfs) // unsquashfs execution arguments - args = append(args, unsquashfs) + args = append(args, squashfs.UnsquashfsPath) args = append(args, opts...) if overwrite { diff --git a/internal/pkg/image/unpacker/squashfs_test.go b/internal/pkg/image/unpacker/squashfs_test.go index d2c4e74763..5888382559 100644 --- a/internal/pkg/image/unpacker/squashfs_test.go +++ b/internal/pkg/image/unpacker/squashfs_test.go @@ -50,7 +50,7 @@ func TestSquashfs(t *testing.T) { } func testSquashfs(t *testing.T, tmpParent string) { - s := NewSquashfs() + s := NewSquashfs(false) if !s.HasUnsquashfs() { t.Skip("unsquashfs not found") diff --git a/internal/pkg/runtime/launcher/native/launcher_linux.go b/internal/pkg/runtime/launcher/native/launcher_linux.go index f5bb47e2c4..477904b875 100644 --- a/internal/pkg/runtime/launcher/native/launcher_linux.go +++ b/internal/pkg/runtime/launcher/native/launcher_linux.go @@ -1074,7 +1074,14 @@ func (l *Launcher) prepareSquashfs(ctx context.Context, img *imgutil.Image, tryF sylog.Warningf("--writable applies to temporary sandbox only, changes will not be written to the original image.") } - err = extractImage(img, imageDir) + // Due to path traversal issues in older unsquashfs versions, we run it + // wrapped under singularity. If the user has requested --userns/-u then + // that wrapping should also use a user namespace (to support + // container/namespace nesting). An exception is when running as root. As + // root, unsquashfs would attempt chown and fail with the single uid/gid + // mapping. + extractUserns := l.cfg.Namespaces.User && os.Getuid() != 0 + err = extractImage(img, imageDir, extractUserns) if err == nil { l.engineConfig.SetImage(imageDir) l.engineConfig.SetDeleteTempDir(tempDir) @@ -1165,9 +1172,10 @@ func mkContainerDirs() (tempDir, imageDir string, err error) { } // extractImage extracts img to directory dir within a temporary directory -// tempDir. It is the caller's responsibility to remove tempDir -// when no longer needed. -func extractImage(img *imgutil.Image, imageDir string) error { +// tempDir. It is the caller's responsibility to remove tempDir when no longer +// needed. If userns is true, then where unsquashfs is wrapped with singularity, +// a user namespace will be used. +func extractImage(img *imgutil.Image, imageDir string, userns bool) error { sylog.Infof("Converting SIF file to temporary sandbox...") unsquashfsPath, err := bin.FindBin("unsquashfs") if err != nil { @@ -1179,7 +1187,7 @@ func extractImage(img *imgutil.Image, imageDir string) error { if err != nil { return fmt.Errorf("could not extract root filesystem: %s", err) } - s := unpacker.NewSquashfs() + s := unpacker.NewSquashfs(userns) if !s.HasUnsquashfs() && unsquashfsPath != "" { s.UnsquashfsPath = unsquashfsPath } diff --git a/pkg/image/image_test.go b/pkg/image/image_test.go index c4f7b5158a..c465dd8cb8 100644 --- a/pkg/image/image_test.go +++ b/pkg/image/image_test.go @@ -61,7 +61,7 @@ func checkPartition(t *testing.T, reader io.Reader) error { extracted := "/bin/busybox" dir := t.TempDir() - s := unpacker.NewSquashfs() + s := unpacker.NewSquashfs(false) if s.HasUnsquashfs() { if err := s.ExtractFiles([]string{extracted}, reader, dir); err != nil { return fmt.Errorf("extraction failed: %s", err)