From b7ec53fc471dec725b0a4910e936592d0e60de75 Mon Sep 17 00:00:00 2001 From: Omer Preminger Date: Fri, 15 Sep 2023 13:28:09 -0400 Subject: [PATCH 1/9] replace call to sif's pkg/user code in squashfuseMount() with call to CE's squashfs code --- .../runtime/launcher/native/launcher_linux.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/internal/pkg/runtime/launcher/native/launcher_linux.go b/internal/pkg/runtime/launcher/native/launcher_linux.go index 57a6798728..f6ea6b714e 100644 --- a/internal/pkg/runtime/launcher/native/launcher_linux.go +++ b/internal/pkg/runtime/launcher/native/launcher_linux.go @@ -20,7 +20,7 @@ import ( lccgroups "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runtime-spec/specs-go" - sifuser "github.com/sylabs/sif/v2/pkg/user" + "github.com/sylabs/sif/v2/pkg/sif" "github.com/sylabs/singularity/v4/internal/pkg/buildcfg" "github.com/sylabs/singularity/v4/internal/pkg/cgroups" "github.com/sylabs/singularity/v4/internal/pkg/image/unpacker" @@ -33,6 +33,7 @@ import ( "github.com/sylabs/singularity/v4/internal/pkg/util/bin" "github.com/sylabs/singularity/v4/internal/pkg/util/env" "github.com/sylabs/singularity/v4/internal/pkg/util/fs" + "github.com/sylabs/singularity/v4/internal/pkg/util/fs/squashfs" "github.com/sylabs/singularity/v4/internal/pkg/util/gpu" "github.com/sylabs/singularity/v4/internal/pkg/util/starter" "github.com/sylabs/singularity/v4/internal/pkg/util/user" @@ -1156,18 +1157,18 @@ func squashfuseMount(ctx context.Context, img *imgutil.Image, imageDir string) ( } sylog.Infof("Mounting SIF with FUSE...") - squashfusePath, err := bin.FindBin("squashfuse") + f, err := sif.LoadContainerFromPath(img.Path, sif.OptLoadWithFlag(os.O_RDONLY)) if err != nil { - return fmt.Errorf("squashfuse is required: %w", err) + return fmt.Errorf("failed to load image: %w", err) } - if _, err := bin.FindBin("fusermount"); err != nil { - return fmt.Errorf("fusermount is required: %w", err) + defer func() { _ = f.UnloadContainer() }() + + d, err := f.GetDescriptor(sif.WithPartitionType(sif.PartPrimSys)) + if err != nil { + return fmt.Errorf("failed to get partition descriptor: %w", err) } - return sifuser.Mount(ctx, img.Path, imageDir, - sifuser.OptMountStdout(os.Stdout), - sifuser.OptMountStderr(os.Stderr), - sifuser.OptMountSquashfusePath(squashfusePath)) + return squashfs.FUSEMount(ctx, uint64(d.Offset()), img.Path, imageDir) } // starterInteractive executes the starter binary to run an image interactively, given the supplied engineConfig From 6e2819cbf6114dbc20b254441f3c94be2fb98f5a Mon Sep 17 00:00:00 2001 From: Omer Preminger Date: Fri, 15 Sep 2023 14:06:33 -0400 Subject: [PATCH 2/9] changed internal/pkg/util/fs/squashfs to use internal/pkg/util/fs/squashfs/fuse; add ctx args as needed --- .../runtime/launcher/oci/launcher_linux.go | 8 +-- .../pkg/runtime/launcher/oci/oci_overlay.go | 23 ++++---- internal/pkg/util/fs/fuse/fuse_mount_linux.go | 18 ++++-- .../pkg/util/fs/overlay/overlay_item_linux.go | 23 ++++---- .../fs/overlay/overlay_item_linux_test.go | 17 ++++-- internal/pkg/util/fs/overlay/overlay_linux.go | 5 +- .../pkg/util/fs/overlay/overlay_set_linux.go | 27 ++++----- .../util/fs/overlay/overlay_set_linux_test.go | 35 +++++++----- internal/pkg/util/fs/squashfs/squashfs.go | 56 +++++-------------- pkg/ocibundle/sif/bundle_linux.go | 4 +- pkg/ocibundle/tools/overlay_linux.go | 19 ++++--- 11 files changed, 116 insertions(+), 119 deletions(-) diff --git a/internal/pkg/runtime/launcher/oci/launcher_linux.go b/internal/pkg/runtime/launcher/oci/launcher_linux.go index 6fd52767f3..c59ab5171c 100644 --- a/internal/pkg/runtime/launcher/oci/launcher_linux.go +++ b/internal/pkg/runtime/launcher/oci/launcher_linux.go @@ -765,7 +765,7 @@ func (l *Launcher) RunWrapped(ctx context.Context, containerID, bundlePath, pidF if err := os.MkdirAll(im.GetMountPoint(), 0o755); err != nil { return err } - if err := im.Mount(); err != nil { + if err := im.Mount(ctx); err != nil { return err } } @@ -784,17 +784,17 @@ func (l *Launcher) RunWrapped(ctx context.Context, containerID, bundlePath, pidF err = Run(ctx, containerID, absBundle, pidFile, systemdCgroups) for _, im := range l.imageMountsByMountpoint { - im.Unmount() + im.Unmount(ctx) } return err } if len(l.cfg.OverlayPaths) > 0 { - return WrapWithOverlays(runFunc, absBundle, l.cfg.OverlayPaths, l.cfg.AllowSUID) + return WrapWithOverlays(ctx, runFunc, absBundle, l.cfg.OverlayPaths, l.cfg.AllowSUID) } - return WrapWithWritableTmpFs(runFunc, absBundle, l.cfg.AllowSUID) + return WrapWithWritableTmpFs(ctx, runFunc, absBundle, l.cfg.AllowSUID) } // getCgroup will return a cgroup path and resources for the runtime to create. diff --git a/internal/pkg/runtime/launcher/oci/oci_overlay.go b/internal/pkg/runtime/launcher/oci/oci_overlay.go index e1e241cfc7..368ed5ec00 100644 --- a/internal/pkg/runtime/launcher/oci/oci_overlay.go +++ b/internal/pkg/runtime/launcher/oci/oci_overlay.go @@ -6,6 +6,7 @@ package oci import ( + "context" "fmt" "github.com/sylabs/singularity/v4/internal/pkg/util/fs/overlay" @@ -18,8 +19,8 @@ import ( // tmpfs. This tmpfs is always writable so that the launcher and runtime are // able to add content to the container. Whether it is writable from inside the // container is controlled by the runtime config. -func WrapWithWritableTmpFs(f func() error, bundleDir string, allowSetuid bool) error { - overlayDir, err := prepareWritableTmpfs(bundleDir, allowSetuid) +func WrapWithWritableTmpFs(ctx context.Context, f func() error, bundleDir string, allowSetuid bool) error { + overlayDir, err := prepareWritableTmpfs(ctx, bundleDir, allowSetuid) sylog.Debugf("Done with prepareWritableTmpfs; overlayDir is: %q", overlayDir) if err != nil { return err @@ -28,7 +29,7 @@ func WrapWithWritableTmpFs(f func() error, bundleDir string, allowSetuid bool) e err = f() // Cleanup actions log errors, but don't return - so we get as much cleanup done as possible. - if cleanupErr := cleanupWritableTmpfs(bundleDir, overlayDir); cleanupErr != nil { + if cleanupErr := cleanupWritableTmpfs(ctx, bundleDir, overlayDir); cleanupErr != nil { sylog.Errorf("While cleaning up writable tmpfs: %v", cleanupErr) } @@ -37,7 +38,7 @@ func WrapWithWritableTmpFs(f func() error, bundleDir string, allowSetuid bool) e } // WrapWithOverlays runs a function wrapped with prep / cleanup steps for overlays. -func WrapWithOverlays(f func() error, bundleDir string, overlayPaths []string, allowSetuid bool) error { +func WrapWithOverlays(ctx context.Context, f func() error, bundleDir string, overlayPaths []string, allowSetuid bool) error { writableOverlayFound := false s := overlay.Set{} for _, p := range overlayPaths { @@ -64,7 +65,7 @@ func WrapWithOverlays(f func() error, bundleDir string, overlayPaths []string, a } rootFsDir := tools.RootFs(bundleDir).Path() - err := s.Mount(rootFsDir) + err := s.Mount(ctx, rootFsDir) if err != nil { return err } @@ -72,11 +73,11 @@ func WrapWithOverlays(f func() error, bundleDir string, overlayPaths []string, a if writableOverlayFound { err = f() } else { - err = WrapWithWritableTmpFs(f, bundleDir, allowSetuid) + err = WrapWithWritableTmpFs(ctx, f, bundleDir, allowSetuid) } // Cleanup actions log errors, but don't return - so we get as much cleanup done as possible. - if cleanupErr := s.Unmount(rootFsDir); cleanupErr != nil { + if cleanupErr := s.Unmount(ctx, rootFsDir); cleanupErr != nil { sylog.Errorf("While unmounting rootfs overlay: %v", cleanupErr) } @@ -84,16 +85,16 @@ func WrapWithOverlays(f func() error, bundleDir string, overlayPaths []string, a return err } -func prepareWritableTmpfs(bundleDir string, allowSetuid bool) (string, error) { +func prepareWritableTmpfs(ctx context.Context, bundleDir string, allowSetuid bool) (string, error) { sylog.Debugf("Configuring writable tmpfs overlay for %s", bundleDir) c := singularityconf.GetCurrentConfig() if c == nil { return "", fmt.Errorf("singularity configuration is not initialized") } - return tools.CreateOverlayTmpfs(bundleDir, int(c.SessiondirMaxSize), allowSetuid) + return tools.CreateOverlayTmpfs(ctx, bundleDir, int(c.SessiondirMaxSize), allowSetuid) } -func cleanupWritableTmpfs(bundleDir, overlayDir string) error { +func cleanupWritableTmpfs(ctx context.Context, bundleDir, overlayDir string) error { sylog.Debugf("Cleaning up writable tmpfs overlay for %s", bundleDir) - return tools.DeleteOverlayTmpfs(bundleDir, overlayDir) + return tools.DeleteOverlayTmpfs(ctx, bundleDir, overlayDir) } diff --git a/internal/pkg/util/fs/fuse/fuse_mount_linux.go b/internal/pkg/util/fs/fuse/fuse_mount_linux.go index 17f4dc48d7..07f29a8486 100644 --- a/internal/pkg/util/fs/fuse/fuse_mount_linux.go +++ b/internal/pkg/util/fs/fuse/fuse_mount_linux.go @@ -6,6 +6,7 @@ package fuse import ( + "context" "fmt" "os" "os/exec" @@ -43,11 +44,15 @@ type ImageMount struct { // AllowOther is set to true to mount the image with the "allow_other" option. AllowOther bool + + // ExtraMountOpts are options to be passed to the mount command (in the "-o" + // argument) beyond the ones autogenerated from other ImageMount fields. + ExtraMountOpts []string } // Mount mounts an image to a temporary directory. It also verifies that // the fusermount utility is present before performing the mount. -func (i *ImageMount) Mount() (err error) { +func (i *ImageMount) Mount(ctx context.Context) (err error) { fuseMountCmd, err := i.determineMountCmd() if err != nil { return err @@ -60,7 +65,7 @@ func (i *ImageMount) Mount() (err error) { fuseCmdLine := fmt.Sprintf("%s %s", fuseMountCmd, strings.Join(args, " ")) sylog.Debugf("Executing FUSE mount command: %q", fuseCmdLine) - execCmd := exec.Command(fuseMountCmd, args...) + execCmd := exec.CommandContext(ctx, fuseMountCmd, args...) execCmd.Stderr = os.Stderr _, err = execCmd.Output() if err != nil { @@ -142,6 +147,7 @@ func (i *ImageMount) generateCmdArgs() ([]string, error) { if i.AllowOther { opts = append(opts, "allow_other") } + opts = append(opts, i.ExtraMountOpts...) if len(opts) > 0 { args = append(args, "-o", strings.Join(opts, ",")) @@ -161,13 +167,13 @@ func (i *ImageMount) SetMountPoint(mountpoint string) { i.mountpoint = mountpoint } -func (i ImageMount) Unmount() error { - return UnmountWithFuse(i.GetMountPoint()) +func (i ImageMount) Unmount(ctx context.Context) error { + return UnmountWithFuse(ctx, i.GetMountPoint()) } // UnmountWithFuse performs an unmount on the specified directory using // fusermount -u. -func UnmountWithFuse(dir string) error { +func UnmountWithFuse(ctx context.Context, dir string) error { fusermountCmd, err := bin.FindBin("fusermount") if err != nil { // We should not be creating FUSE-based mounts in the first place @@ -175,7 +181,7 @@ func UnmountWithFuse(dir string) error { return fmt.Errorf("fusermount not available while trying to perform unmount: %w", err) } sylog.Debugf("Executing FUSE unmount command: %s -u %s", fusermountCmd, dir) - execCmd := exec.Command(fusermountCmd, "-u", dir) + execCmd := exec.CommandContext(ctx, fusermountCmd, "-u", dir) execCmd.Stderr = os.Stderr _, err = execCmd.Output() return err diff --git a/internal/pkg/util/fs/overlay/overlay_item_linux.go b/internal/pkg/util/fs/overlay/overlay_item_linux.go index 43f6bdafc6..a234547efe 100644 --- a/internal/pkg/util/fs/overlay/overlay_item_linux.go +++ b/internal/pkg/util/fs/overlay/overlay_item_linux.go @@ -6,6 +6,7 @@ package overlay import ( + "context" "fmt" "os" "path/filepath" @@ -143,14 +144,14 @@ func (i *Item) GetParentDir() (string, error) { // Mount performs the necessary steps to mount an individual Item. Note that // this method does not mount the assembled overlay itself. That happens in // Set.Mount(). -func (i *Item) Mount() error { +func (i *Item) Mount(ctx context.Context) error { var err error switch i.Type { case image.SANDBOX: err = i.mountDir() case image.SQUASHFS, image.EXT3: - err = i.mountWithFuse() + err = i.mountWithFuse(ctx) default: return fmt.Errorf("internal error: unrecognized image type in overlay.Item.Mount() (type: %v)", i.Type) @@ -236,7 +237,7 @@ func (i *Item) mountDir() error { } // mountWithFuse mounts an image to a temporary directory -func (i *Item) mountWithFuse() error { +func (i *Item) mountWithFuse(ctx context.Context) error { parentDir, err := i.GetParentDir() if err != nil { return err @@ -251,7 +252,7 @@ func (i *Item) mountWithFuse() error { AllowDev: i.allowDev, } - if err := im.Mount(); err != nil { + if err := im.Mount(ctx); err != nil { return err } @@ -263,13 +264,13 @@ func (i *Item) mountWithFuse() error { // Unmount performs the necessary steps to unmount an individual Item. Note that // this method does not unmount the overlay itself. That happens in // Set.Unmount(). -func (i Item) Unmount() error { +func (i Item) Unmount(ctx context.Context) error { switch i.Type { case image.SANDBOX: - return i.unmountDir() + return i.unmountDir(ctx) case image.SQUASHFS, image.EXT3: - return i.unmountFuse() + return i.unmountFuse(ctx) default: return fmt.Errorf("internal error: unrecognized image type in overlay.Item.Unmount() (type: %v)", i.Type) @@ -277,14 +278,14 @@ func (i Item) Unmount() error { } // unmountDir unmounts directory-based Items. -func (i Item) unmountDir() error { - return DetachMount(i.StagingDir) +func (i Item) unmountDir(ctx context.Context) error { + return DetachMount(ctx, i.StagingDir) } // unmountFuse unmounts FUSE-based Items. -func (i Item) unmountFuse() error { +func (i Item) unmountFuse(ctx context.Context) error { defer os.Remove(i.StagingDir) - err := fsfuse.UnmountWithFuse(i.StagingDir) + err := fsfuse.UnmountWithFuse(ctx, i.StagingDir) if err != nil { return fmt.Errorf("error while trying to unmount image %q from %s: %w", i.SourcePath, i.StagingDir, err) } diff --git a/internal/pkg/util/fs/overlay/overlay_item_linux_test.go b/internal/pkg/util/fs/overlay/overlay_item_linux_test.go index ba5d5ebb3d..159dc91f98 100644 --- a/internal/pkg/util/fs/overlay/overlay_item_linux_test.go +++ b/internal/pkg/util/fs/overlay/overlay_item_linux_test.go @@ -6,6 +6,7 @@ package overlay import ( + "context" "os" "path/filepath" "strings" @@ -189,6 +190,8 @@ func TestUpperAndWorkCreation(t *testing.T) { } func TestDirMounts(t *testing.T) { + ctx := context.Background() + tests := []struct { name string olStr string @@ -243,13 +246,13 @@ func TestDirMounts(t *testing.T) { item.SetAllowSetuid(tt.allowSetUID) item.SetAllowDev(tt.allowDev) - if err := item.Mount(); err != nil { + if err := item.Mount(ctx); err != nil { t.Fatalf("while trying to mount dir %q: %s", tt.olStr, err) } checkMountOpts(t, item.StagingDir, tt.expectMountOpts) - if err := item.Unmount(); err != nil { + if err := item.Unmount(ctx); err != nil { t.Errorf("while trying to unmount dir %q: %s", tt.olStr, err) } }) @@ -258,6 +261,7 @@ func TestDirMounts(t *testing.T) { func TestImageRO(t *testing.T) { require.Command(t, "fusermount") + ctx := context.Background() tests := []struct { name string @@ -340,11 +344,11 @@ func TestImageRO(t *testing.T) { t.Errorf("item.Type is %v (should be %v)", item.Type, tt.typeCode) } - if err := item.Mount(); err != nil { + if err := item.Mount(ctx); err != nil { t.Fatalf("unable to mount image for reading: %s", err) } t.Cleanup(func() { - item.Unmount() + item.Unmount(ctx) }) testFileStagedPath := filepath.Join(item.GetMountDir(), testFilePath) @@ -359,6 +363,7 @@ func TestExtfsRW(t *testing.T) { require.Command(t, "fuse-overlayfs") require.Command(t, "fusermount") tmpDir := mkTempDirOrFatal(t) + ctx := context.Background() // Create a copy of the extfs test image to be used for testing writable // extfs image overlays @@ -377,11 +382,11 @@ func TestExtfsRW(t *testing.T) { t.Errorf("item.Type is %v (should be %v)", item.Type, image.EXT3) } - if err := item.Mount(); err != nil { + if err := item.Mount(ctx); err != nil { t.Fatalf("unable to mount extfs image for reading & writing: %s", err) } t.Cleanup(func() { - item.Unmount() + item.Unmount(ctx) }) testFileStagedPath := filepath.Join(item.GetMountDir(), testFilePath) diff --git a/internal/pkg/util/fs/overlay/overlay_linux.go b/internal/pkg/util/fs/overlay/overlay_linux.go index c440e46227..a0fbd2c50c 100644 --- a/internal/pkg/util/fs/overlay/overlay_linux.go +++ b/internal/pkg/util/fs/overlay/overlay_linux.go @@ -6,6 +6,7 @@ package overlay import ( + "context" "errors" "fmt" "os" @@ -281,7 +282,9 @@ func DetachAndDelete(overlayDir string) error { } // DetachMount performs an unmount system call on the specified directory. -func DetachMount(dir string) error { +// +//nolint:revive,nolintlint +func DetachMount(ctx context.Context, dir string) error { sylog.Debugf("Calling syscall.Unmount() to detach %q", dir) if err := syscall.Unmount(dir, syscall.MNT_DETACH); err != nil { return fmt.Errorf("failed to detach %s: %w", dir, err) diff --git a/internal/pkg/util/fs/overlay/overlay_set_linux.go b/internal/pkg/util/fs/overlay/overlay_set_linux.go index ff93bc9b42..7d1608d11d 100644 --- a/internal/pkg/util/fs/overlay/overlay_set_linux.go +++ b/internal/pkg/util/fs/overlay/overlay_set_linux.go @@ -8,6 +8,7 @@ package overlay import ( + "context" "fmt" "os" "os/exec" @@ -44,7 +45,7 @@ type Set struct { // Mount prepares and mounts the entire Set onto the specified rootfs // directory. -func (s Set) Mount(rootFsDir string) error { +func (s Set) Mount(ctx context.Context, rootFsDir string) error { // Perform identity mounts for this Set dups := lo.FindDuplicatesBy(s.ReadonlyOverlays, func(item *Item) string { return item.SourcePath @@ -55,16 +56,16 @@ func (s Set) Mount(rootFsDir string) error { })) } - if err := s.performIndividualMounts(); err != nil { + if err := s.performIndividualMounts(ctx); err != nil { return err } // Perform actual overlay mount - return s.performFinalMount(rootFsDir) + return s.performFinalMount(ctx, rootFsDir) } // UnmountOverlay ummounts a Set from a specified rootfs directory. -func (s Set) Unmount(rootFsDir string) error { +func (s Set) Unmount(ctx context.Context, rootFsDir string) error { unprivOls, err := UnprivOverlaysSupported() if err != nil { return fmt.Errorf("while checking for unprivileged overlay support in kernel: %w", err) @@ -72,21 +73,21 @@ func (s Set) Unmount(rootFsDir string) error { useKernelMount := unprivOls && !s.hasWritableExtfsImg() if useKernelMount { - err = DetachMount(rootFsDir) + err = DetachMount(ctx, rootFsDir) } else { - err = fsfuse.UnmountWithFuse(rootFsDir) + err = fsfuse.UnmountWithFuse(ctx, rootFsDir) } if err != nil { return err } - return s.detachIndividualMounts() + return s.detachIndividualMounts(ctx) } // performIndividualMounts creates the mounts that furnish the individual // elements of the Set. -func (s Set) performIndividualMounts() error { +func (s Set) performIndividualMounts(ctx context.Context) error { overlaysToBind := s.ReadonlyOverlays if s.WritableOverlay != nil { overlaysToBind = append(overlaysToBind, s.WritableOverlay) @@ -94,7 +95,7 @@ func (s Set) performIndividualMounts() error { // Try to do initial bind-mounts for _, o := range overlaysToBind { - if err := o.Mount(); err != nil { + if err := o.Mount(ctx); err != nil { return err } } @@ -105,7 +106,7 @@ func (s Set) performIndividualMounts() error { // performFinalMount performs the final step in mounting a Set, namely mounting // of the overlay with its full-fledged options string, representing all the // individual Items (writable and read-only) that comprise the Set. -func (s Set) performFinalMount(rootFsDir string) error { +func (s Set) performFinalMount(ctx context.Context, rootFsDir string) error { // Try to perform actual mount options := s.options(rootFsDir) unprivOls, err := UnprivOverlaysSupported() @@ -143,7 +144,7 @@ func (s Set) performFinalMount(rootFsDir string) error { } sylog.Debugf("Mounting overlay (via fuse-overlayfs) with rootFsDir %q, options: %q", rootFsDir, options) - execCmd := exec.Command(fuseOlFsCmd, "-o", options, rootFsDir) + execCmd := exec.CommandContext(ctx, fuseOlFsCmd, "-o", options, rootFsDir) execCmd.Stderr = os.Stderr _, err = execCmd.Output() if err != nil { @@ -182,7 +183,7 @@ func (s Set) hasWritableExtfsImg() bool { // detachIndividualMounts detaches the bind mounts & remounts created by // performIndividualMounts, above. -func (s Set) detachIndividualMounts() error { +func (s Set) detachIndividualMounts(ctx context.Context) error { overlaysToDetach := s.ReadonlyOverlays if s.WritableOverlay != nil { overlaysToDetach = append(overlaysToDetach, s.WritableOverlay) @@ -192,7 +193,7 @@ func (s Set) detachIndividualMounts() error { // then return the first error encountered. errors := []error{} for _, overlay := range overlaysToDetach { - err := overlay.Unmount() + err := overlay.Unmount(ctx) if err != nil { sylog.Errorf("Error encountered trying to detach identity mount %s: %s", overlay.StagingDir, err) errors = append(errors, err) diff --git a/internal/pkg/util/fs/overlay/overlay_set_linux_test.go b/internal/pkg/util/fs/overlay/overlay_set_linux_test.go index a6ea5860b8..e0984d2dfa 100644 --- a/internal/pkg/util/fs/overlay/overlay_set_linux_test.go +++ b/internal/pkg/util/fs/overlay/overlay_set_linux_test.go @@ -6,6 +6,7 @@ package overlay import ( + "context" "os" "os/exec" "path/filepath" @@ -60,6 +61,7 @@ func wrapOverlayTest(f func(t *testing.T)) func(t *testing.T) { func TestAllTypesAtOnce(t *testing.T) { wrapOverlayTest(func(t *testing.T) { + ctx := context.Background() s := Set{} tmpRoOlDir := mkTempOlDirOrFatal(t) @@ -90,12 +92,12 @@ func TestAllTypesAtOnce(t *testing.T) { s.WritableOverlay = i rootfsDir := mkTempDirOrFatal(t) - if err := s.Mount(rootfsDir); err != nil { + if err := s.Mount(ctx, rootfsDir); err != nil { t.Fatalf("failed to mount overlay set: %s", err) } t.Cleanup(func() { if t.Failed() { - s.Unmount(rootfsDir) + s.Unmount(ctx, rootfsDir) } }) @@ -118,7 +120,7 @@ func TestAllTypesAtOnce(t *testing.T) { } } - if err := s.Unmount(rootfsDir); err != nil { + if err := s.Unmount(ctx, rootfsDir); err != nil { t.Errorf("while trying to unmount overlay set: %s", err) } })(t) @@ -127,13 +129,14 @@ func TestAllTypesAtOnce(t *testing.T) { func TestPersistentWriteToDir(t *testing.T) { wrapOverlayTest(func(t *testing.T) { tmpRwOlDir := mkTempOlDirOrFatal(t) + ctx := context.Background() i, err := NewItemFromString(tmpRwOlDir) if err != nil { t.Fatalf("failed to create writable-dir overlay item (%q): %s", tmpRwOlDir, err) } s := Set{WritableOverlay: i} - performPersistentWriteTest(t, s) + performPersistentWriteTest(ctx, t, s) })(t) } @@ -142,6 +145,7 @@ func TestPersistentWriteToExtfsImg(t *testing.T) { require.Command(t, "fuse-overlayfs") require.Command(t, "fusermount") tmpDir := mkTempDirOrFatal(t) + ctx := context.Background() // Create a copy of the extfs test image to be used for testing writable // extfs image overlays @@ -157,10 +161,10 @@ func TestPersistentWriteToExtfsImg(t *testing.T) { } s := Set{WritableOverlay: i} - performPersistentWriteTest(t, s) + performPersistentWriteTest(ctx, t, s) } -func performPersistentWriteTest(t *testing.T, s Set) { +func performPersistentWriteTest(ctx context.Context, t *testing.T, s Set) { rootfsDir := mkTempDirOrFatal(t) // This cleanup will serve adequately for both iterations of the overlay-set @@ -171,11 +175,11 @@ func performPersistentWriteTest(t *testing.T, s Set) { // expected contents. t.Cleanup(func() { if t.Failed() { - s.Unmount(rootfsDir) + s.Unmount(ctx, rootfsDir) } }) - if err := s.Mount(rootfsDir); err != nil { + if err := s.Mount(ctx, rootfsDir); err != nil { t.Fatalf("failed to mount overlay set: %s", err) } expectStr := "my_test_string" @@ -186,11 +190,11 @@ func performPersistentWriteTest(t *testing.T, s Set) { t.Fatalf("while trying to write file inside mounted overlay-set: %s", err) } - if err := s.Unmount(rootfsDir); err != nil { + if err := s.Unmount(ctx, rootfsDir); err != nil { t.Fatalf("while trying to unmount overlay set: %s", err) } - if err := s.Mount(rootfsDir); err != nil { + if err := s.Mount(ctx, rootfsDir); err != nil { t.Fatalf("failed to mount overlay set: %s", err) } data, err := os.ReadFile(testFileMountedPath) @@ -201,7 +205,7 @@ func performPersistentWriteTest(t *testing.T, s Set) { if foundStr != expectStr { t.Errorf("incorrect file contents in mounted overlay set: expected %q, but found: %q", expectStr, foundStr) } - if err := s.Unmount(rootfsDir); err != nil { + if err := s.Unmount(ctx, rootfsDir); err != nil { t.Errorf("while trying to unmount overlay set: %s", err) } } @@ -212,6 +216,7 @@ func TestDuplicateItemsInSet(t *testing.T) { var rwI *Item var err error + ctx := context.Background() s := Set{} // First, test mounting of an overlay set with only readonly items, one of @@ -223,9 +228,9 @@ func TestDuplicateItemsInSet(t *testing.T) { addROItemOrFatal(t, &s, mkTempOlDirOrFatal(t)+":ro") rootfsDir = mkTempDirOrFatal(t) - if err := s.Mount(rootfsDir); err == nil { + if err := s.Mount(ctx, rootfsDir); err == nil { t.Errorf("unexpected success: Mounting overlay.Set with duplicate (%q) should have failed", roI2.SourcePath) - if err := s.Unmount(rootfsDir); err != nil { + if err := s.Unmount(ctx, rootfsDir); err != nil { t.Fatalf("could not unmount erroneous successful mount of overlay set: %s", err) } } @@ -240,9 +245,9 @@ func TestDuplicateItemsInSet(t *testing.T) { s.WritableOverlay = rwI rootfsDir = mkTempDirOrFatal(t) - if err := s.Mount(rootfsDir); err == nil { + if err := s.Mount(ctx, rootfsDir); err == nil { t.Errorf("unexpected success: Mounting overlay.Set with duplicate (%q) should have failed", roI2.SourcePath) - if err := s.Unmount(rootfsDir); err != nil { + if err := s.Unmount(ctx, rootfsDir); err != nil { t.Fatalf("could not unmount erroneous successful mount of overlay set: %s", err) } } diff --git a/internal/pkg/util/fs/squashfs/squashfs.go b/internal/pkg/util/fs/squashfs/squashfs.go index 78502b4167..7f6434d2be 100644 --- a/internal/pkg/util/fs/squashfs/squashfs.go +++ b/internal/pkg/util/fs/squashfs/squashfs.go @@ -9,55 +9,29 @@ import ( "context" "fmt" "os" - "os/exec" "path/filepath" - "strings" - "github.com/sylabs/singularity/v4/internal/pkg/util/bin" - "github.com/sylabs/singularity/v4/pkg/sylog" + "github.com/sylabs/singularity/v4/internal/pkg/util/fs/fuse" + "github.com/sylabs/singularity/v4/pkg/image" ) func FUSEMount(ctx context.Context, offset uint64, path, mountPath string) error { - args := []string{ - "-o", fmt.Sprintf("ro,offset=%d,uid=%d,gid=%d", offset, os.Getuid(), os.Getgid()), - filepath.Clean(path), - filepath.Clean(mountPath), + im := fuse.ImageMount{ + Type: image.SQUASHFS, + Readonly: true, + SourcePath: filepath.Clean(path), + ExtraMountOpts: []string{ + "ro", + fmt.Sprintf("offset=%d", offset), + fmt.Sprintf("uid=%d", os.Getuid()), + fmt.Sprintf("gid=%d", os.Getgid()), + }, } + im.SetMountPoint(filepath.Clean(mountPath)) - squashfuse, err := bin.FindBin("squashfuse") - if err != nil { - return err - } - - cmd := exec.CommandContext(ctx, squashfuse, args...) - if outputBytes, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf( - "failed to mount: %w (cmdline: %q; output: %q)", err, - strings.Join(append([]string{squashfuse}, args...), " "), - string(outputBytes), - ) - } - - return nil + return im.Mount(ctx) } func FUSEUnmount(ctx context.Context, mountPath string) error { - args := []string{ - "-u", - filepath.Clean(mountPath), - } - - fusermount, err := bin.FindBin("fusermount") - if err != nil { - return err - } - cmd := exec.CommandContext(ctx, fusermount, args...) - - sylog.Debugf("Executing %s %s", fusermount, strings.Join(args, " ")) - - if err := cmd.Run(); err != nil { - return fmt.Errorf("failed to unmount: %w", err) - } - - return nil + return fuse.UnmountWithFuse(ctx, mountPath) } diff --git a/pkg/ocibundle/sif/bundle_linux.go b/pkg/ocibundle/sif/bundle_linux.go index 778ff0a6f9..9fafa67284 100644 --- a/pkg/ocibundle/sif/bundle_linux.go +++ b/pkg/ocibundle/sif/bundle_linux.go @@ -137,7 +137,7 @@ func (s *sifBundle) Create(ctx context.Context, ociConfig *specs.Spec) error { } if s.writable { - if err := tools.CreateOverlay(s.bundlePath); err != nil { + if err := tools.CreateOverlay(ctx, s.bundlePath); err != nil { // best effort to release FUSE mount squashfs.FUSEUnmount(ctx, rootFs) tools.DeleteBundle(s.bundlePath) @@ -161,7 +161,7 @@ func (s *sifBundle) Update(_ context.Context, ociConfig *specs.Spec) error { func (s *sifBundle) Delete(ctx context.Context) error { overlayExists := fs.IsDir(filepath.Join(s.bundlePath, "overlay")) if s.writable && overlayExists { - if err := tools.DeleteOverlay(s.bundlePath); err != nil { + if err := tools.DeleteOverlay(ctx, s.bundlePath); err != nil { return fmt.Errorf("delete error: %s", err) } } diff --git a/pkg/ocibundle/tools/overlay_linux.go b/pkg/ocibundle/tools/overlay_linux.go index 3fa298c933..574affc080 100644 --- a/pkg/ocibundle/tools/overlay_linux.go +++ b/pkg/ocibundle/tools/overlay_linux.go @@ -6,6 +6,7 @@ package tools import ( + "context" "fmt" "os" "path/filepath" @@ -18,7 +19,7 @@ import ( // CreateOverlay creates a writable overlay using a directory inside the OCI // bundle. -func CreateOverlay(bundlePath string) error { +func CreateOverlay(ctx context.Context, bundlePath string) error { oldumask := syscall.Umask(0) defer syscall.Umask(oldumask) @@ -41,16 +42,16 @@ func CreateOverlay(bundlePath string) error { Readonly: false, }} - return olSet.Mount(RootFs(bundlePath).Path()) + return olSet.Mount(ctx, RootFs(bundlePath).Path()) } // DeleteOverlay deletes an overlay previously created using a directory inside // the OCI bundle. -func DeleteOverlay(bundlePath string) error { +func DeleteOverlay(ctx context.Context, bundlePath string) error { olDir := filepath.Join(bundlePath, "overlay") rootFsDir := RootFs(bundlePath).Path() - if err := overlay.DetachMount(rootFsDir); err != nil { + if err := overlay.DetachMount(ctx, rootFsDir); err != nil { return err } @@ -58,7 +59,7 @@ func DeleteOverlay(bundlePath string) error { } // CreateOverlay creates a writable overlay using tmpfs. -func CreateOverlayTmpfs(bundlePath string, sizeMiB int, allowSetuid bool) (string, error) { +func CreateOverlayTmpfs(ctx context.Context, bundlePath string, sizeMiB int, allowSetuid bool) (string, error) { var err error oldumask := syscall.Umask(0) @@ -104,7 +105,7 @@ func CreateOverlayTmpfs(bundlePath string, sizeMiB int, allowSetuid bool) (strin oi.SetAllowSetuid(allowSetuid) olSet := overlay.Set{WritableOverlay: &oi} - err = olSet.Mount(RootFs(bundlePath).Path()) + err = olSet.Mount(ctx, RootFs(bundlePath).Path()) if err != nil { return "", err } @@ -113,16 +114,16 @@ func CreateOverlayTmpfs(bundlePath string, sizeMiB int, allowSetuid bool) (strin } // DeleteOverlayTmpfs deletes an overlay previously created using tmpfs. -func DeleteOverlayTmpfs(bundlePath, olDir string) error { +func DeleteOverlayTmpfs(ctx context.Context, bundlePath, olDir string) error { rootFsDir := RootFs(bundlePath).Path() - if err := overlay.DetachMount(rootFsDir); err != nil { + if err := overlay.DetachMount(ctx, rootFsDir); err != nil { return err } // Because CreateOverlayTmpfs() mounts the tmpfs on olDir, and then // calls ApplyOverlay(), there needs to be an extra unmount in the this case - if err := overlay.DetachMount(olDir); err != nil { + if err := overlay.DetachMount(ctx, olDir); err != nil { return err } From ba722adb2c29a77963dd9fdde5d520b54d838d0e Mon Sep 17 00:00:00 2001 From: Omer Preminger Date: Fri, 15 Sep 2023 14:44:19 -0400 Subject: [PATCH 3/9] replace call to sif's pkg/user code in CleanupHost() with call to CE's squashfs code --- .../engine/singularity/cleanup_host_linux.go | 16 ++-------------- .../runtime/launcher/native/launcher_linux.go | 4 +++- internal/pkg/util/fs/squashfs/squashfs.go | 4 ++-- pkg/ocibundle/ocisif/bundle_linux.go | 5 ++++- pkg/ocibundle/sif/bundle_linux.go | 2 +- 5 files changed, 12 insertions(+), 19 deletions(-) diff --git a/internal/pkg/runtime/engine/singularity/cleanup_host_linux.go b/internal/pkg/runtime/engine/singularity/cleanup_host_linux.go index d59283ba8b..b65e7efe0a 100644 --- a/internal/pkg/runtime/engine/singularity/cleanup_host_linux.go +++ b/internal/pkg/runtime/engine/singularity/cleanup_host_linux.go @@ -10,8 +10,7 @@ import ( "fmt" "os" - sifuser "github.com/sylabs/sif/v2/pkg/user" - "github.com/sylabs/singularity/v4/internal/pkg/util/bin" + "github.com/sylabs/singularity/v4/internal/pkg/util/fs/squashfs" "github.com/sylabs/singularity/v4/pkg/sylog" ) @@ -20,18 +19,7 @@ import ( func (e *EngineOperations) CleanupHost(ctx context.Context) (err error) { if e.EngineConfig.GetImageFuse() { sylog.Infof("Unmounting SIF with FUSE...") - fusermountPath, err := bin.FindBin("fusermount") - if err != nil { - return fmt.Errorf("while unmounting fuse directory: %s: %w", e.EngineConfig.GetImage(), err) - } - - err = sifuser.Unmount(ctx, e.EngineConfig.GetImage(), - sifuser.OptUnmountStdout(os.Stdout), - sifuser.OptUnmountStderr(os.Stderr), - sifuser.OptUnmountFusermountPath(fusermountPath)) - if err != nil { - return fmt.Errorf("while unmounting fuse directory: %s: %w", e.EngineConfig.GetImage(), err) - } + squashfs.FUSEUnmount(ctx, e.EngineConfig.GetImage()) if tempDir := e.EngineConfig.GetDeleteTempDir(); tempDir != "" { sylog.Infof("Removing image tempDir %s", tempDir) diff --git a/internal/pkg/runtime/launcher/native/launcher_linux.go b/internal/pkg/runtime/launcher/native/launcher_linux.go index f6ea6b714e..e6a092e890 100644 --- a/internal/pkg/runtime/launcher/native/launcher_linux.go +++ b/internal/pkg/runtime/launcher/native/launcher_linux.go @@ -1168,7 +1168,9 @@ func squashfuseMount(ctx context.Context, img *imgutil.Image, imageDir string) ( return fmt.Errorf("failed to get partition descriptor: %w", err) } - return squashfs.FUSEMount(ctx, uint64(d.Offset()), img.Path, imageDir) + _, err = squashfs.FUSEMount(ctx, uint64(d.Offset()), img.Path, imageDir) + + return err } // starterInteractive executes the starter binary to run an image interactively, given the supplied engineConfig diff --git a/internal/pkg/util/fs/squashfs/squashfs.go b/internal/pkg/util/fs/squashfs/squashfs.go index 7f6434d2be..dca74fa310 100644 --- a/internal/pkg/util/fs/squashfs/squashfs.go +++ b/internal/pkg/util/fs/squashfs/squashfs.go @@ -15,7 +15,7 @@ import ( "github.com/sylabs/singularity/v4/pkg/image" ) -func FUSEMount(ctx context.Context, offset uint64, path, mountPath string) error { +func FUSEMount(ctx context.Context, offset uint64, path, mountPath string) (*fuse.ImageMount, error) { im := fuse.ImageMount{ Type: image.SQUASHFS, Readonly: true, @@ -29,7 +29,7 @@ func FUSEMount(ctx context.Context, offset uint64, path, mountPath string) error } im.SetMountPoint(filepath.Clean(mountPath)) - return im.Mount(ctx) + return &im, im.Mount(ctx) } func FUSEUnmount(ctx context.Context, mountPath string) error { diff --git a/pkg/ocibundle/ocisif/bundle_linux.go b/pkg/ocibundle/ocisif/bundle_linux.go index ae1910a8e2..d75eb8ff19 100644 --- a/pkg/ocibundle/ocisif/bundle_linux.go +++ b/pkg/ocibundle/ocisif/bundle_linux.go @@ -228,5 +228,8 @@ func mount(ctx context.Context, path, mountPath string, digest v1.Hash) error { if err != nil { return fmt.Errorf("failed to get partition descriptor: %w", err) } - return squashfs.FUSEMount(ctx, uint64(d.Offset()), path, mountPath) + + _, err = squashfs.FUSEMount(ctx, uint64(d.Offset()), path, mountPath) + + return err } diff --git a/pkg/ocibundle/sif/bundle_linux.go b/pkg/ocibundle/sif/bundle_linux.go index 9fafa67284..3a12ad8504 100644 --- a/pkg/ocibundle/sif/bundle_linux.go +++ b/pkg/ocibundle/sif/bundle_linux.go @@ -124,7 +124,7 @@ func (s *sifBundle) Create(ctx context.Context, ociConfig *specs.Spec) error { } rootFs := tools.RootFs(s.bundlePath).Path() - if err := squashfs.FUSEMount(ctx, offset, s.image, rootFs); err != nil { + if _, err := squashfs.FUSEMount(ctx, offset, s.image, rootFs); err != nil { tools.DeleteBundle(s.bundlePath) return fmt.Errorf("failed to mount SIF partition: %s", err) } From de466b7abd1fb5bb6ca1a9134dd4502665883d98 Mon Sep 17 00:00:00 2001 From: Omer Preminger Date: Mon, 18 Sep 2023 11:04:04 -0400 Subject: [PATCH 4/9] address review comments --- LICENSE_DEPENDENCIES.md | 30 +++++ go.mod | 1 + go.sum | 2 + .../engine/singularity/cleanup_host_linux.go | 4 +- .../runtime/launcher/native/launcher_linux.go | 1 - internal/pkg/util/fs/fuse/fuse_mount_linux.go | 62 ++++++++-- .../pkg/util/fs/fuse/fuse_mount_linux_test.go | 117 ++++++++++++++++++ internal/pkg/util/fs/squashfs/squashfs.go | 2 +- pkg/util/maps/maps.go | 12 ++ 9 files changed, 219 insertions(+), 12 deletions(-) create mode 100644 internal/pkg/util/fs/fuse/fuse_mount_linux_test.go create mode 100644 pkg/util/maps/maps.go diff --git a/LICENSE_DEPENDENCIES.md b/LICENSE_DEPENDENCIES.md index f6f5082038..ee4e6ff4b0 100644 --- a/LICENSE_DEPENDENCIES.md +++ b/LICENSE_DEPENDENCIES.md @@ -14926,6 +14926,36 @@ SOFTWARE. ``` +## github.com/samber/mo + +**License:** MIT + +``` +MIT License + +Copyright (c) 2022 Samuel Berthe + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + +``` + + ## github.com/secure-systems-lab/go-securesystemslib **License:** MIT diff --git a/go.mod b/go.mod index fb9d45e274..d1be083609 100644 --- a/go.mod +++ b/go.mod @@ -37,6 +37,7 @@ require ( github.com/pelletier/go-toml/v2 v2.1.0 github.com/pkg/errors v0.9.1 github.com/samber/lo v1.38.1 + github.com/samber/mo v1.8.0 github.com/seccomp/libseccomp-golang v0.10.0 github.com/shopspring/decimal v1.3.1 github.com/sigstore/sigstore v1.7.3 diff --git a/go.sum b/go.sum index a339388c64..34413a42fa 100644 --- a/go.sum +++ b/go.sum @@ -460,6 +460,8 @@ github.com/safchain/ethtool v0.3.0 h1:gimQJpsI6sc1yIqP/y8GYgiXn/NjgvpM0RNoWLVVmP github.com/safchain/ethtool v0.3.0/go.mod h1:SA9BwrgyAqNo7M+uaL6IYbxpm5wk3L7Mm6ocLW+CJUs= github.com/samber/lo v1.38.1 h1:j2XEAqXKb09Am4ebOg31SpvzUTTs6EN3VfgeLUhPdXM= github.com/samber/lo v1.38.1/go.mod h1:+m/ZKRl6ClXCE2Lgf3MsQlWfh4bn1bz6CXEOxnEXnEA= +github.com/samber/mo v1.8.0 h1:vYjHTfg14JF9tD2NLhpoUsRi9bjyRoYwa4+do0nvbVw= +github.com/samber/mo v1.8.0/go.mod h1:BfkrCPuYzVG3ZljnZB783WIJIGk1mcZr9c9CPf8tAxs= github.com/sebdah/goldie/v2 v2.5.3 h1:9ES/mNN+HNUbNWpVAlrzuZ7jE+Nrczbj8uFRjM7624Y= github.com/seccomp/libseccomp-golang v0.10.0 h1:aA4bp+/Zzi0BnWZ2F1wgNBs5gTpm+na2rWM6M9YjLpY= github.com/seccomp/libseccomp-golang v0.10.0/go.mod h1:JA8cRccbGaA1s33RQf7Y1+q9gHmZX1yB/z9WDN1C6fg= diff --git a/internal/pkg/runtime/engine/singularity/cleanup_host_linux.go b/internal/pkg/runtime/engine/singularity/cleanup_host_linux.go index b65e7efe0a..1f3a284c27 100644 --- a/internal/pkg/runtime/engine/singularity/cleanup_host_linux.go +++ b/internal/pkg/runtime/engine/singularity/cleanup_host_linux.go @@ -19,7 +19,9 @@ import ( func (e *EngineOperations) CleanupHost(ctx context.Context) (err error) { if e.EngineConfig.GetImageFuse() { sylog.Infof("Unmounting SIF with FUSE...") - squashfs.FUSEUnmount(ctx, e.EngineConfig.GetImage()) + if err := squashfs.FUSEUnmount(ctx, e.EngineConfig.GetImage()); err != nil { + return fmt.Errorf("while unmounting fuse directory: %s: %w", e.EngineConfig.GetImage(), err) + } if tempDir := e.EngineConfig.GetDeleteTempDir(); tempDir != "" { sylog.Infof("Removing image tempDir %s", tempDir) diff --git a/internal/pkg/runtime/launcher/native/launcher_linux.go b/internal/pkg/runtime/launcher/native/launcher_linux.go index e6a092e890..9ed0fcbfab 100644 --- a/internal/pkg/runtime/launcher/native/launcher_linux.go +++ b/internal/pkg/runtime/launcher/native/launcher_linux.go @@ -1161,7 +1161,6 @@ func squashfuseMount(ctx context.Context, img *imgutil.Image, imageDir string) ( if err != nil { return fmt.Errorf("failed to load image: %w", err) } - defer func() { _ = f.UnloadContainer() }() d, err := f.GetDescriptor(sif.WithPartitionType(sif.PartPrimSys)) if err != nil { diff --git a/internal/pkg/util/fs/fuse/fuse_mount_linux.go b/internal/pkg/util/fs/fuse/fuse_mount_linux.go index 07f29a8486..9b64411d12 100644 --- a/internal/pkg/util/fs/fuse/fuse_mount_linux.go +++ b/internal/pkg/util/fs/fuse/fuse_mount_linux.go @@ -12,9 +12,12 @@ import ( "os/exec" "strings" + "github.com/samber/lo" + "github.com/samber/mo" "github.com/sylabs/singularity/v4/internal/pkg/util/bin" "github.com/sylabs/singularity/v4/pkg/image" "github.com/sylabs/singularity/v4/pkg/sylog" + "github.com/sylabs/singularity/v4/pkg/util/maps" ) type ImageMount struct { @@ -45,9 +48,9 @@ type ImageMount struct { // AllowOther is set to true to mount the image with the "allow_other" option. AllowOther bool - // ExtraMountOpts are options to be passed to the mount command (in the "-o" + // ExtraOpts are options to be passed to the mount command (in the "-o" // argument) beyond the ones autogenerated from other ImageMount fields. - ExtraMountOpts []string + ExtraOpts []string } // Mount mounts an image to a temporary directory. It also verifies that @@ -129,34 +132,75 @@ func (i *ImageMount) generateCmdArgs() ([]string, error) { } }() + opts := i.generateMountOpts() + + if len(opts) > 0 { + args = append(args, "-o", strings.Join(opts, ",")) + } + + args = append(args, i.SourcePath) + args = append(args, i.mountpoint) + + return args, nil +} + +func (i ImageMount) generateMountOpts() []string { // TODO: Think through what makes sense for file ownership in FUSE-mounted // images, vis a vis id-mappings and user-namespaces. opts := []string{"uid=0", "gid=0"} + + // Create a map of the extra mount options that have been requested, so we + // can weed out attempts to overwrite builtin struct fields. + extraOptsMap := lo.SliceToMap(i.ExtraOpts, func(s string) (string, mo.Option[string]) { + splitted := strings.SplitN(s, "=", 2) + if len(splitted) < 2 { + return strings.ToLower(s), mo.None[string]() + } + + return strings.ToLower(splitted[0]), mo.Some(splitted[1]) + }) + + weedOutMountOpt(&extraOptsMap, "ro") + weedOutMountOpt(&extraOptsMap, "rw") if i.Readonly { // Not strictly necessary as will be read-only in assembled overlay, // however this stops any erroneous writes through the stagingDir. opts = append(opts, "ro") } + // FUSE defaults to nosuid,nodev - attempt to reverse if AllowDev/Setuid requested. + weedOutMountOpt(&extraOptsMap, "dev") if i.AllowDev { opts = append(opts, "dev") } + weedOutMountOpt(&extraOptsMap, "suid") if i.AllowSetuid { opts = append(opts, "suid") } + + weedOutMountOpt(&extraOptsMap, "allow_other") if i.AllowOther { opts = append(opts, "allow_other") } - opts = append(opts, i.ExtraMountOpts...) - if len(opts) > 0 { - args = append(args, "-o", strings.Join(opts, ",")) - } + filteredExtraOpts := lo.MapToSlice(extraOptsMap, rebuildOpt) + opts = append(opts, filteredExtraOpts...) - args = append(args, i.SourcePath) - args = append(args, i.mountpoint) + return opts +} - return args, nil +func weedOutMountOpt(extraOptsMap *map[string]mo.Option[string], k string) { + if maps.HasKey(*extraOptsMap, k) { + sylog.Warningf("Passing extra FUSE-mount option %q would override built-in field; ignoring", rebuildOpt(k, (*extraOptsMap)[k])) + delete(*extraOptsMap, k) + } +} + +func rebuildOpt(k string, v mo.Option[string]) string { + if v.IsAbsent() { + return k + } + return k + "=" + v.MustGet() } func (i ImageMount) GetMountPoint() string { diff --git a/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go b/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go new file mode 100644 index 0000000000..31d0b415ac --- /dev/null +++ b/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go @@ -0,0 +1,117 @@ +// Copyright (c) 2023, Sylabs Inc. All rights reserved. +// This software is licensed under a 3-clause BSD license. Please consult the +// LICENSE.md file distributed with the sources of this project regarding your +// rights to use or distribute this software. + +package fuse + +import ( + "strings" + "testing" + + "github.com/samber/lo" + "github.com/sylabs/singularity/v4/pkg/image" +) + +func TestReadonlyOverride(t *testing.T) { + m1 := ImageMount{ + Type: image.SQUASHFS, + Readonly: true, + ExtraOpts: []string{"rw"}, + } + + opts := m1.generateMountOpts() + if lo.ContainsBy(opts, func(s string) bool { + splitted := strings.SplitN(s, "=", 2) + return (strings.ToLower(splitted[0]) == "rw") + }) { + t.Errorf("Failed to weed out 'rw' mount option; opts: %#v", opts) + } + + m2 := ImageMount{ + Type: image.SQUASHFS, + Readonly: false, + ExtraOpts: []string{"ro"}, + } + + opts = m2.generateMountOpts() + if lo.ContainsBy(opts, func(s string) bool { + splitted := strings.SplitN(s, "=", 2) + return (strings.ToLower(splitted[0]) == "ro") + }) { + t.Errorf("Failed to weed out 'ro' mount option; opts: %#v", opts) + } +} + +func TestDevOverride(t *testing.T) { + m := ImageMount{ + Type: image.SQUASHFS, + AllowDev: false, + ExtraOpts: []string{"dev"}, + } + + opts := m.generateMountOpts() + if lo.ContainsBy(opts, func(s string) bool { + splitted := strings.SplitN(s, "=", 2) + return (strings.ToLower(splitted[0]) == "dev") + }) { + t.Errorf("Failed to weed out 'dev' mount option; opts: %#v", opts) + } +} + +func TestSetuidOverride(t *testing.T) { + m := ImageMount{ + Type: image.SQUASHFS, + AllowSetuid: false, + ExtraOpts: []string{"suid"}, + } + + opts := m.generateMountOpts() + if lo.ContainsBy(opts, func(s string) bool { + splitted := strings.SplitN(s, "=", 2) + return (strings.ToLower(splitted[0]) == "suid") + }) { + t.Errorf("Failed to weed out 'suid' mount option; opts: %#v", opts) + } +} + +func TestAllowOtherOverride(t *testing.T) { + m := ImageMount{ + Type: image.SQUASHFS, + AllowOther: false, + ExtraOpts: []string{"allow_other"}, + } + + opts := m.generateMountOpts() + if lo.ContainsBy(opts, func(s string) bool { + splitted := strings.SplitN(s, "=", 2) + return (strings.ToLower(splitted[0]) == "allow_other") + }) { + t.Errorf("Failed to weed out 'allow_other' mount option; opts: %#v", opts) + } +} + +func TestAllOverridesAtOnce(t *testing.T) { + m := ImageMount{ + Type: image.SQUASHFS, + Readonly: true, + AllowDev: false, + AllowSetuid: false, + AllowOther: false, + ExtraOpts: []string{"suid", "allow_other", "rw", "dev"}, + } + + opts := m.generateMountOpts() + offendingOpts := lo.Filter(opts, func(s string, _ int) bool { + splitted := strings.SplitN(s, "=", 2) + switch splitted[0] { + case "rw", "dev", "suid", "allow_other": + return true + default: + return false + } + }) + if len(offendingOpts) > 0 { + t.Errorf("Failed to properly filter mount options; opts: %#v (offending options: %#v)", opts, offendingOpts) + } +} diff --git a/internal/pkg/util/fs/squashfs/squashfs.go b/internal/pkg/util/fs/squashfs/squashfs.go index dca74fa310..75423e7465 100644 --- a/internal/pkg/util/fs/squashfs/squashfs.go +++ b/internal/pkg/util/fs/squashfs/squashfs.go @@ -20,7 +20,7 @@ func FUSEMount(ctx context.Context, offset uint64, path, mountPath string) (*fus Type: image.SQUASHFS, Readonly: true, SourcePath: filepath.Clean(path), - ExtraMountOpts: []string{ + ExtraOpts: []string{ "ro", fmt.Sprintf("offset=%d", offset), fmt.Sprintf("uid=%d", os.Getuid()), diff --git a/pkg/util/maps/maps.go b/pkg/util/maps/maps.go new file mode 100644 index 0000000000..e4d8fb107b --- /dev/null +++ b/pkg/util/maps/maps.go @@ -0,0 +1,12 @@ +// Copyright (c) 2023, Sylabs Inc. All rights reserved. +// This software is licensed under a 3-clause BSD license. Please consult the +// LICENSE.md file distributed with the sources of this project regarding your +// rights to use or distribute this software. + +package maps + +func HasKey[K comparable, V any](m map[K]V, k K) bool { + _, ok := m[k] + + return ok +} From 21f0f57d7fd9f8ed9535ae3e1ce89ba42c954481 Mon Sep 17 00:00:00 2001 From: Omer Preminger Date: Mon, 18 Sep 2023 11:28:54 -0400 Subject: [PATCH 5/9] error out instead of warning on invalid FUSE mount extra opt --- internal/pkg/util/fs/fuse/fuse_mount_linux.go | 38 ++++--- .../pkg/util/fs/fuse/fuse_mount_linux_test.go | 107 +++--------------- internal/pkg/util/fs/squashfs/squashfs.go | 1 - 3 files changed, 37 insertions(+), 109 deletions(-) diff --git a/internal/pkg/util/fs/fuse/fuse_mount_linux.go b/internal/pkg/util/fs/fuse/fuse_mount_linux.go index 9b64411d12..b3fbfce599 100644 --- a/internal/pkg/util/fs/fuse/fuse_mount_linux.go +++ b/internal/pkg/util/fs/fuse/fuse_mount_linux.go @@ -132,7 +132,10 @@ func (i *ImageMount) generateCmdArgs() ([]string, error) { } }() - opts := i.generateMountOpts() + opts, err := i.generateMountOpts() + if err != nil { + return args, err + } if len(opts) > 0 { args = append(args, "-o", strings.Join(opts, ",")) @@ -144,13 +147,13 @@ func (i *ImageMount) generateCmdArgs() ([]string, error) { return args, nil } -func (i ImageMount) generateMountOpts() []string { +func (i ImageMount) generateMountOpts() ([]string, error) { // TODO: Think through what makes sense for file ownership in FUSE-mounted // images, vis a vis id-mappings and user-namespaces. opts := []string{"uid=0", "gid=0"} // Create a map of the extra mount options that have been requested, so we - // can weed out attempts to overwrite builtin struct fields. + // can catch attempts to overwrite builtin struct fields. extraOptsMap := lo.SliceToMap(i.ExtraOpts, func(s string) (string, mo.Option[string]) { splitted := strings.SplitN(s, "=", 2) if len(splitted) < 2 { @@ -160,8 +163,12 @@ func (i ImageMount) generateMountOpts() []string { return strings.ToLower(splitted[0]), mo.Some(splitted[1]) }) - weedOutMountOpt(&extraOptsMap, "ro") - weedOutMountOpt(&extraOptsMap, "rw") + if maps.HasKey(extraOptsMap, "ro") { + return opts, fmt.Errorf("cannot pass 'ro' as extra FUSE-mount option, as it is handled by an internal field") + } + if maps.HasKey(extraOptsMap, "rw") { + return opts, fmt.Errorf("cannot pass 'rw' as extra FUSE-mount option, as it is handled by an internal field") + } if i.Readonly { // Not strictly necessary as will be read-only in assembled overlay, // however this stops any erroneous writes through the stagingDir. @@ -169,16 +176,22 @@ func (i ImageMount) generateMountOpts() []string { } // FUSE defaults to nosuid,nodev - attempt to reverse if AllowDev/Setuid requested. - weedOutMountOpt(&extraOptsMap, "dev") + if maps.HasKey(extraOptsMap, "dev") { + return opts, fmt.Errorf("cannot pass 'dev' as extra FUSE-mount option, as it is handled by an internal field") + } if i.AllowDev { opts = append(opts, "dev") } - weedOutMountOpt(&extraOptsMap, "suid") + if maps.HasKey(extraOptsMap, "suid") { + return opts, fmt.Errorf("cannot pass 'suid' as extra FUSE-mount option, as it is handled by an internal field") + } if i.AllowSetuid { opts = append(opts, "suid") } - weedOutMountOpt(&extraOptsMap, "allow_other") + if maps.HasKey(extraOptsMap, "allow_other") { + return opts, fmt.Errorf("cannot pass 'allow_other' as extra FUSE-mount option, as it is handled by an internal field") + } if i.AllowOther { opts = append(opts, "allow_other") } @@ -186,14 +199,7 @@ func (i ImageMount) generateMountOpts() []string { filteredExtraOpts := lo.MapToSlice(extraOptsMap, rebuildOpt) opts = append(opts, filteredExtraOpts...) - return opts -} - -func weedOutMountOpt(extraOptsMap *map[string]mo.Option[string], k string) { - if maps.HasKey(*extraOptsMap, k) { - sylog.Warningf("Passing extra FUSE-mount option %q would override built-in field; ignoring", rebuildOpt(k, (*extraOptsMap)[k])) - delete(*extraOptsMap, k) - } + return opts, nil } func rebuildOpt(k string, v mo.Option[string]) string { diff --git a/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go b/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go index 31d0b415ac..3ba02a18a3 100644 --- a/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go +++ b/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go @@ -6,112 +6,35 @@ package fuse import ( - "strings" "testing" - - "github.com/samber/lo" - "github.com/sylabs/singularity/v4/pkg/image" ) -func TestReadonlyOverride(t *testing.T) { - m1 := ImageMount{ - Type: image.SQUASHFS, - Readonly: true, - ExtraOpts: []string{"rw"}, - } - - opts := m1.generateMountOpts() - if lo.ContainsBy(opts, func(s string) bool { - splitted := strings.SplitN(s, "=", 2) - return (strings.ToLower(splitted[0]) == "rw") - }) { - t.Errorf("Failed to weed out 'rw' mount option; opts: %#v", opts) - } - - m2 := ImageMount{ - Type: image.SQUASHFS, - Readonly: false, - ExtraOpts: []string{"ro"}, - } - - opts = m2.generateMountOpts() - if lo.ContainsBy(opts, func(s string) bool { - splitted := strings.SplitN(s, "=", 2) - return (strings.ToLower(splitted[0]) == "ro") - }) { - t.Errorf("Failed to weed out 'ro' mount option; opts: %#v", opts) - } -} - -func TestDevOverride(t *testing.T) { - m := ImageMount{ - Type: image.SQUASHFS, - AllowDev: false, - ExtraOpts: []string{"dev"}, - } - - opts := m.generateMountOpts() - if lo.ContainsBy(opts, func(s string) bool { - splitted := strings.SplitN(s, "=", 2) - return (strings.ToLower(splitted[0]) == "dev") - }) { - t.Errorf("Failed to weed out 'dev' mount option; opts: %#v", opts) - } -} - -func TestSetuidOverride(t *testing.T) { - m := ImageMount{ - Type: image.SQUASHFS, - AllowSetuid: false, - ExtraOpts: []string{"suid"}, - } - - opts := m.generateMountOpts() - if lo.ContainsBy(opts, func(s string) bool { - splitted := strings.SplitN(s, "=", 2) - return (strings.ToLower(splitted[0]) == "suid") - }) { - t.Errorf("Failed to weed out 'suid' mount option; opts: %#v", opts) - } +func TestExtraOptOverrides(t *testing.T) { + testOneOverride(t, "rw") + testOneOverride(t, "ro") + testOneOverride(t, "dev") + testOneOverride(t, "suid") + testOneOverride(t, "allow_other") } -func TestAllowOtherOverride(t *testing.T) { +func testOneOverride(t *testing.T, s string) { m := ImageMount{ - Type: image.SQUASHFS, - AllowOther: false, - ExtraOpts: []string{"allow_other"}, + ExtraOpts: []string{s}, } - opts := m.generateMountOpts() - if lo.ContainsBy(opts, func(s string) bool { - splitted := strings.SplitN(s, "=", 2) - return (strings.ToLower(splitted[0]) == "allow_other") - }) { - t.Errorf("Failed to weed out 'allow_other' mount option; opts: %#v", opts) + opts, err := m.generateMountOpts() + if err == nil { + t.Errorf("Failed to weed out %q mount option; opts: %#v", s, opts) } } func TestAllOverridesAtOnce(t *testing.T) { m := ImageMount{ - Type: image.SQUASHFS, - Readonly: true, - AllowDev: false, - AllowSetuid: false, - AllowOther: false, - ExtraOpts: []string{"suid", "allow_other", "rw", "dev"}, + ExtraOpts: []string{"suid", "allow_other", "rw", "dev"}, } - opts := m.generateMountOpts() - offendingOpts := lo.Filter(opts, func(s string, _ int) bool { - splitted := strings.SplitN(s, "=", 2) - switch splitted[0] { - case "rw", "dev", "suid", "allow_other": - return true - default: - return false - } - }) - if len(offendingOpts) > 0 { - t.Errorf("Failed to properly filter mount options; opts: %#v (offending options: %#v)", opts, offendingOpts) + opts, err := m.generateMountOpts() + if err == nil { + t.Errorf("Failed to properly filter mount options; opts: %#v", opts) } } diff --git a/internal/pkg/util/fs/squashfs/squashfs.go b/internal/pkg/util/fs/squashfs/squashfs.go index 75423e7465..82607e18ee 100644 --- a/internal/pkg/util/fs/squashfs/squashfs.go +++ b/internal/pkg/util/fs/squashfs/squashfs.go @@ -21,7 +21,6 @@ func FUSEMount(ctx context.Context, offset uint64, path, mountPath string) (*fus Readonly: true, SourcePath: filepath.Clean(path), ExtraOpts: []string{ - "ro", fmt.Sprintf("offset=%d", offset), fmt.Sprintf("uid=%d", os.Getuid()), fmt.Sprintf("gid=%d", os.Getgid()), From a54a580a6cc754b9ec5e7bac1b4b2d521cfb57ac Mon Sep 17 00:00:00 2001 From: Omer Preminger Date: Mon, 18 Sep 2023 11:38:22 -0400 Subject: [PATCH 6/9] drop samber/mo dependency --- LICENSE_DEPENDENCIES.md | 30 ------------------- go.mod | 1 - go.sum | 2 -- internal/pkg/util/fs/fuse/fuse_mount_linux.go | 13 ++++---- 4 files changed, 6 insertions(+), 40 deletions(-) diff --git a/LICENSE_DEPENDENCIES.md b/LICENSE_DEPENDENCIES.md index ee4e6ff4b0..f6f5082038 100644 --- a/LICENSE_DEPENDENCIES.md +++ b/LICENSE_DEPENDENCIES.md @@ -14926,36 +14926,6 @@ SOFTWARE. ``` -## github.com/samber/mo - -**License:** MIT - -``` -MIT License - -Copyright (c) 2022 Samuel Berthe - -Permission is hereby granted, free of charge, to any person obtaining a copy -of this software and associated documentation files (the "Software"), to deal -in the Software without restriction, including without limitation the rights -to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -copies of the Software, and to permit persons to whom the Software is -furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be included in all -copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE. - -``` - - ## github.com/secure-systems-lab/go-securesystemslib **License:** MIT diff --git a/go.mod b/go.mod index d1be083609..fb9d45e274 100644 --- a/go.mod +++ b/go.mod @@ -37,7 +37,6 @@ require ( github.com/pelletier/go-toml/v2 v2.1.0 github.com/pkg/errors v0.9.1 github.com/samber/lo v1.38.1 - github.com/samber/mo v1.8.0 github.com/seccomp/libseccomp-golang v0.10.0 github.com/shopspring/decimal v1.3.1 github.com/sigstore/sigstore v1.7.3 diff --git a/go.sum b/go.sum index 34413a42fa..a339388c64 100644 --- a/go.sum +++ b/go.sum @@ -460,8 +460,6 @@ github.com/safchain/ethtool v0.3.0 h1:gimQJpsI6sc1yIqP/y8GYgiXn/NjgvpM0RNoWLVVmP github.com/safchain/ethtool v0.3.0/go.mod h1:SA9BwrgyAqNo7M+uaL6IYbxpm5wk3L7Mm6ocLW+CJUs= github.com/samber/lo v1.38.1 h1:j2XEAqXKb09Am4ebOg31SpvzUTTs6EN3VfgeLUhPdXM= github.com/samber/lo v1.38.1/go.mod h1:+m/ZKRl6ClXCE2Lgf3MsQlWfh4bn1bz6CXEOxnEXnEA= -github.com/samber/mo v1.8.0 h1:vYjHTfg14JF9tD2NLhpoUsRi9bjyRoYwa4+do0nvbVw= -github.com/samber/mo v1.8.0/go.mod h1:BfkrCPuYzVG3ZljnZB783WIJIGk1mcZr9c9CPf8tAxs= github.com/sebdah/goldie/v2 v2.5.3 h1:9ES/mNN+HNUbNWpVAlrzuZ7jE+Nrczbj8uFRjM7624Y= github.com/seccomp/libseccomp-golang v0.10.0 h1:aA4bp+/Zzi0BnWZ2F1wgNBs5gTpm+na2rWM6M9YjLpY= github.com/seccomp/libseccomp-golang v0.10.0/go.mod h1:JA8cRccbGaA1s33RQf7Y1+q9gHmZX1yB/z9WDN1C6fg= diff --git a/internal/pkg/util/fs/fuse/fuse_mount_linux.go b/internal/pkg/util/fs/fuse/fuse_mount_linux.go index b3fbfce599..10dcd51de2 100644 --- a/internal/pkg/util/fs/fuse/fuse_mount_linux.go +++ b/internal/pkg/util/fs/fuse/fuse_mount_linux.go @@ -13,7 +13,6 @@ import ( "strings" "github.com/samber/lo" - "github.com/samber/mo" "github.com/sylabs/singularity/v4/internal/pkg/util/bin" "github.com/sylabs/singularity/v4/pkg/image" "github.com/sylabs/singularity/v4/pkg/sylog" @@ -154,13 +153,13 @@ func (i ImageMount) generateMountOpts() ([]string, error) { // Create a map of the extra mount options that have been requested, so we // can catch attempts to overwrite builtin struct fields. - extraOptsMap := lo.SliceToMap(i.ExtraOpts, func(s string) (string, mo.Option[string]) { + extraOptsMap := lo.SliceToMap(i.ExtraOpts, func(s string) (string, *string) { splitted := strings.SplitN(s, "=", 2) if len(splitted) < 2 { - return strings.ToLower(s), mo.None[string]() + return strings.ToLower(s), nil } - return strings.ToLower(splitted[0]), mo.Some(splitted[1]) + return strings.ToLower(splitted[0]), &splitted[1] }) if maps.HasKey(extraOptsMap, "ro") { @@ -202,11 +201,11 @@ func (i ImageMount) generateMountOpts() ([]string, error) { return opts, nil } -func rebuildOpt(k string, v mo.Option[string]) string { - if v.IsAbsent() { +func rebuildOpt(k string, v *string) string { + if v == nil { return k } - return k + "=" + v.MustGet() + return k + "=" + *v } func (i ImageMount) GetMountPoint() string { From d1ce366f44a0bb72b0302a61b7babd374738cc14 Mon Sep 17 00:00:00 2001 From: Omer Preminger Date: Mon, 18 Sep 2023 11:59:47 -0400 Subject: [PATCH 7/9] unit-test at Mount()/Unmount() level rather than pkg internals --- .../pkg/util/fs/fuse/fuse_mount_linux_test.go | 50 ++++++++++++++++--- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go b/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go index 3ba02a18a3..2d1cffd6b3 100644 --- a/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go +++ b/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go @@ -6,9 +6,15 @@ package fuse import ( + "context" + "path/filepath" "testing" + + "github.com/sylabs/singularity/v4/pkg/image" ) +var squashfsImgPath = filepath.Join("..", "..", "..", "..", "..", "test", "images", "squashfs-for-overlay.img") + func TestExtraOptOverrides(t *testing.T) { testOneOverride(t, "rw") testOneOverride(t, "ro") @@ -18,23 +24,51 @@ func TestExtraOptOverrides(t *testing.T) { } func testOneOverride(t *testing.T, s string) { + ctx := context.Background() + m := ImageMount{ - ExtraOpts: []string{s}, + Type: image.SQUASHFS, + SourcePath: squashfsImgPath, + } + + if err := m.Mount(ctx); err != nil { + t.Fatalf("Baseline mount of %q failed: %v", m.SourcePath, err) + } + if err := m.Unmount(ctx); err != nil { + t.Fatalf("Baseline unmount of %q failed: %v", m.SourcePath, err) } - opts, err := m.generateMountOpts() - if err == nil { - t.Errorf("Failed to weed out %q mount option; opts: %#v", s, opts) + m.ExtraOpts = []string{s} + + if err := m.Mount(ctx); err == nil { + t.Errorf("Failed to block %q mount option.", s) + if err := m.Unmount(ctx); err != nil { + t.Fatalf("Post-test unmount of %q failed: %v", m.SourcePath, err) + } } } func TestAllOverridesAtOnce(t *testing.T) { + ctx := context.Background() + m := ImageMount{ - ExtraOpts: []string{"suid", "allow_other", "rw", "dev"}, + Type: image.SQUASHFS, + SourcePath: squashfsImgPath, + } + + if err := m.Mount(ctx); err != nil { + t.Fatalf("Baseline mount of %q failed: %v", m.SourcePath, err) + } + if err := m.Unmount(ctx); err != nil { + t.Fatalf("Baseline unmount of %q failed: %v", m.SourcePath, err) } - opts, err := m.generateMountOpts() - if err == nil { - t.Errorf("Failed to properly filter mount options; opts: %#v", opts) + m.ExtraOpts = []string{"suid", "allow_other", "rw", "dev"} + + if err := m.Mount(ctx); err == nil { + t.Errorf("Failed to block mount options (%q).", m.ExtraOpts) + if err := m.Unmount(ctx); err != nil { + t.Fatalf("Post-test unmount of %q failed: %v", m.SourcePath, err) + } } } From 50ff89d76ca95a83066a691a481b4c05c592e55c Mon Sep 17 00:00:00 2001 From: Omer Preminger Date: Mon, 18 Sep 2023 12:21:32 -0400 Subject: [PATCH 8/9] improve checking of extra opts: add "nosiud", "nodev"; some refactoring --- internal/pkg/util/fs/fuse/fuse_mount_linux.go | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/internal/pkg/util/fs/fuse/fuse_mount_linux.go b/internal/pkg/util/fs/fuse/fuse_mount_linux.go index 10dcd51de2..a9ccb32b4b 100644 --- a/internal/pkg/util/fs/fuse/fuse_mount_linux.go +++ b/internal/pkg/util/fs/fuse/fuse_mount_linux.go @@ -162,11 +162,11 @@ func (i ImageMount) generateMountOpts() ([]string, error) { return strings.ToLower(splitted[0]), &splitted[1] }) - if maps.HasKey(extraOptsMap, "ro") { - return opts, fmt.Errorf("cannot pass 'ro' as extra FUSE-mount option, as it is handled by an internal field") + if err := checkProhibitedOpt(extraOptsMap, "ro"); err != nil { + return opts, err } - if maps.HasKey(extraOptsMap, "rw") { - return opts, fmt.Errorf("cannot pass 'rw' as extra FUSE-mount option, as it is handled by an internal field") + if err := checkProhibitedOpt(extraOptsMap, "rw"); err != nil { + return opts, err } if i.Readonly { // Not strictly necessary as will be read-only in assembled overlay, @@ -175,21 +175,27 @@ func (i ImageMount) generateMountOpts() ([]string, error) { } // FUSE defaults to nosuid,nodev - attempt to reverse if AllowDev/Setuid requested. - if maps.HasKey(extraOptsMap, "dev") { - return opts, fmt.Errorf("cannot pass 'dev' as extra FUSE-mount option, as it is handled by an internal field") + if err := checkProhibitedOpt(extraOptsMap, "dev"); err != nil { + return opts, err + } + if err := checkProhibitedOpt(extraOptsMap, "nodev"); err != nil { + return opts, err } if i.AllowDev { opts = append(opts, "dev") } - if maps.HasKey(extraOptsMap, "suid") { - return opts, fmt.Errorf("cannot pass 'suid' as extra FUSE-mount option, as it is handled by an internal field") + if err := checkProhibitedOpt(extraOptsMap, "suid"); err != nil { + return opts, err + } + if err := checkProhibitedOpt(extraOptsMap, "nosuid"); err != nil { + return opts, err } if i.AllowSetuid { opts = append(opts, "suid") } - if maps.HasKey(extraOptsMap, "allow_other") { - return opts, fmt.Errorf("cannot pass 'allow_other' as extra FUSE-mount option, as it is handled by an internal field") + if err := checkProhibitedOpt(extraOptsMap, "allow_other"); err != nil { + return opts, err } if i.AllowOther { opts = append(opts, "allow_other") @@ -201,6 +207,14 @@ func (i ImageMount) generateMountOpts() ([]string, error) { return opts, nil } +func checkProhibitedOpt(extraOptsMap map[string]*string, opt string) error { + if maps.HasKey(extraOptsMap, opt) { + return fmt.Errorf("cannot pass %q as extra FUSE-mount option, as it is handled by an internal field", opt) + } + + return nil +} + func rebuildOpt(k string, v *string) string { if v == nil { return k From 8ced169a97b1dc11fe3d197b85b86e3abcc9c852 Mon Sep 17 00:00:00 2001 From: Omer Preminger Date: Mon, 18 Sep 2023 13:00:25 -0400 Subject: [PATCH 9/9] incorporate UID, GID as struct-fields; improve unit-test --- internal/pkg/util/fs/fuse/fuse_mount_linux.go | 22 +++++++++++++--- .../pkg/util/fs/fuse/fuse_mount_linux_test.go | 26 ++++++++++++++----- internal/pkg/util/fs/squashfs/squashfs.go | 4 +-- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/internal/pkg/util/fs/fuse/fuse_mount_linux.go b/internal/pkg/util/fs/fuse/fuse_mount_linux.go index a9ccb32b4b..7053a1f59b 100644 --- a/internal/pkg/util/fs/fuse/fuse_mount_linux.go +++ b/internal/pkg/util/fs/fuse/fuse_mount_linux.go @@ -24,6 +24,12 @@ type ImageMount struct { // values in pkg/image) Type int + // UID is the value to pass to the uid option when mounting + UID int + + // GID is the value to pass to the gid option when mounting + GID int + // Readonly represents whether this is a Readonly overlay Readonly bool @@ -147,10 +153,6 @@ func (i *ImageMount) generateCmdArgs() ([]string, error) { } func (i ImageMount) generateMountOpts() ([]string, error) { - // TODO: Think through what makes sense for file ownership in FUSE-mounted - // images, vis a vis id-mappings and user-namespaces. - opts := []string{"uid=0", "gid=0"} - // Create a map of the extra mount options that have been requested, so we // can catch attempts to overwrite builtin struct fields. extraOptsMap := lo.SliceToMap(i.ExtraOpts, func(s string) (string, *string) { @@ -162,6 +164,18 @@ func (i ImageMount) generateMountOpts() ([]string, error) { return strings.ToLower(splitted[0]), &splitted[1] }) + opts := []string{} + + if err := checkProhibitedOpt(extraOptsMap, "uid"); err != nil { + return opts, err + } + opts = append(opts, fmt.Sprintf("uid=%d", i.UID)) + + if err := checkProhibitedOpt(extraOptsMap, "gid"); err != nil { + return opts, err + } + opts = append(opts, fmt.Sprintf("gid=%d", i.GID)) + if err := checkProhibitedOpt(extraOptsMap, "ro"); err != nil { return opts, err } diff --git a/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go b/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go index 2d1cffd6b3..8a3153ea5d 100644 --- a/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go +++ b/internal/pkg/util/fs/fuse/fuse_mount_linux_test.go @@ -15,12 +15,26 @@ import ( var squashfsImgPath = filepath.Join("..", "..", "..", "..", "..", "test", "images", "squashfs-for-overlay.img") +var optsToTest = []string{ + "uid=0", + "gid=0", + "uid=23", + "gid=67", + "uid=2345", + "gid=6789", + "rw", + "ro", + "dev", + "nodev", + "suid", + "nosuid", + "allow_other", +} + func TestExtraOptOverrides(t *testing.T) { - testOneOverride(t, "rw") - testOneOverride(t, "ro") - testOneOverride(t, "dev") - testOneOverride(t, "suid") - testOneOverride(t, "allow_other") + for _, opt := range optsToTest { + testOneOverride(t, opt) + } } func testOneOverride(t *testing.T, s string) { @@ -63,7 +77,7 @@ func TestAllOverridesAtOnce(t *testing.T) { t.Fatalf("Baseline unmount of %q failed: %v", m.SourcePath, err) } - m.ExtraOpts = []string{"suid", "allow_other", "rw", "dev"} + m.ExtraOpts = optsToTest[:] if err := m.Mount(ctx); err == nil { t.Errorf("Failed to block mount options (%q).", m.ExtraOpts) diff --git a/internal/pkg/util/fs/squashfs/squashfs.go b/internal/pkg/util/fs/squashfs/squashfs.go index 82607e18ee..4b0124baf4 100644 --- a/internal/pkg/util/fs/squashfs/squashfs.go +++ b/internal/pkg/util/fs/squashfs/squashfs.go @@ -18,12 +18,12 @@ import ( func FUSEMount(ctx context.Context, offset uint64, path, mountPath string) (*fuse.ImageMount, error) { im := fuse.ImageMount{ Type: image.SQUASHFS, + UID: os.Getuid(), + GID: os.Getgid(), Readonly: true, SourcePath: filepath.Clean(path), ExtraOpts: []string{ fmt.Sprintf("offset=%d", offset), - fmt.Sprintf("uid=%d", os.Getuid()), - fmt.Sprintf("gid=%d", os.Getgid()), }, } im.SetMountPoint(filepath.Clean(mountPath))