summaryrefslogtreecommitdiff
diff options
Diffstat
-rw-r--r--cmd/snap-update-ns/main.go9
-rw-r--r--cmd/snap-update-ns/system.go7
-rw-r--r--cmd/snap-update-ns/system_test.go6
-rw-r--r--cmd/snap-update-ns/utils.go21
-rw-r--r--cmd/snap-update-ns/utils_test.go21
-rw-r--r--tests/main/interfaces-shared-memory-private/task.yaml9
-rw-r--r--tests/main/snap-mgmt/task.yaml12
7 files changed, 77 insertions, 8 deletions
diff --git a/cmd/snap-update-ns/main.go b/cmd/snap-update-ns/main.go
index 7ff0418598..f3f4ca9867 100644
--- a/cmd/snap-update-ns/main.go
+++ b/cmd/snap-update-ns/main.go
@@ -22,6 +22,7 @@ package main
import (
"fmt"
"os"
+ "syscall"
"github.com/jessevdk/go-flags"
@@ -78,6 +79,14 @@ func run() error {
if err := parseArgs(os.Args[1:]); err != nil {
return err
}
+
+ // Explicitly set the umask to 0 to prevent permission bits
+ // being masked out when creating files and directories.
+ //
+ // While snap-confine already does this for us, we inherit
+ // snapd's umask when it invokes us.
+ syscall.Umask(0)
+
var upCtx MountProfileUpdateContext
if opts.UserMounts {
upCtx = NewUserProfileUpdateContext(opts.Positionals.SnapName, opts.FromSnapConfine, os.Getuid())
diff --git a/cmd/snap-update-ns/system.go b/cmd/snap-update-ns/system.go
index c66e23003e..bc7acd1c62 100644
--- a/cmd/snap-update-ns/system.go
+++ b/cmd/snap-update-ns/system.go
@@ -21,6 +21,7 @@ package main
import (
"fmt"
+ "os"
"github.com/snapcore/snapd/dirs"
"github.com/snapcore/snapd/snap"
@@ -80,14 +81,14 @@ func (upCtx *SystemProfileUpdateContext) Assumptions() *Assumptions {
// As such, provide write access to all of /tmp.
as.AddUnrestrictedPaths("/var/lib/snapd/hostfs/tmp")
as.AddModeHint("/var/lib/snapd/hostfs/tmp/snap.*", 0700)
- as.AddModeHint("/var/lib/snapd/hostfs/tmp/snap.*/tmp", 01777)
+ as.AddModeHint("/var/lib/snapd/hostfs/tmp/snap.*/tmp", 0777|os.ModeSticky)
// This is to ensure that unprivileged users can create the socket. This
// permission only matters if the plug-side app constructs its mount
// namespace before the slot-side app is launched.
- as.AddModeHint("/var/lib/snapd/hostfs/tmp/snap.*/tmp/.X11-unix", 01777)
+ as.AddModeHint("/var/lib/snapd/hostfs/tmp/snap.*/tmp/.X11-unix", 0777|os.ModeSticky)
// This is to ensure private shared-memory directories have
// the right permissions.
- as.AddModeHint("/dev/shm/snap.*", 01777)
+ as.AddModeHint("/dev/shm/snap.*", 0777|os.ModeSticky)
return as
}
diff --git a/cmd/snap-update-ns/system_test.go b/cmd/snap-update-ns/system_test.go
index c19c3e5d6d..2d0f60a609 100644
--- a/cmd/snap-update-ns/system_test.go
+++ b/cmd/snap-update-ns/system_test.go
@@ -79,10 +79,10 @@ func (s *systemSuite) TestAssumptions(c *C) {
c.Check(as.ModeForPath("/tmp"), Equals, os.FileMode(0755))
c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp"), Equals, os.FileMode(0755))
c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap.x11-server"), Equals, os.FileMode(0700))
- c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap.x11-server/tmp"), Equals, os.FileMode(01777))
+ c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap.x11-server/tmp"), Equals, os.FileMode(0777)|os.ModeSticky)
c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap.x11-server/foo"), Equals, os.FileMode(0755))
- c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap.x11-server/tmp/.X11-unix"), Equals, os.FileMode(01777))
- c.Check(as.ModeForPath("/dev/shm/snap.some-snap"), Equals, os.FileMode(01777))
+ c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap.x11-server/tmp/.X11-unix"), Equals, os.FileMode(0777)|os.ModeSticky)
+ c.Check(as.ModeForPath("/dev/shm/snap.some-snap"), Equals, os.FileMode(0777)|os.ModeSticky)
// Instances can, in addition, access /snap/$SNAP_INSTANCE_NAME
upCtx = update.NewSystemProfileUpdateContext("foo_instance", false)
diff --git a/cmd/snap-update-ns/utils.go b/cmd/snap-update-ns/utils.go
index ba675774de..dfa93f12e0 100644
--- a/cmd/snap-update-ns/utils.go
+++ b/cmd/snap-update-ns/utils.go
@@ -128,6 +128,23 @@ func OpenPath(path string) (int, error) {
return fd, nil
}
+// syscallMode returns the syscall-specific mode bits from Go's portable mode bits.
+// This is a copy of the same helper in Go's os package.
+func syscallMode(i os.FileMode) (o uint32) {
+ o |= uint32(i.Perm())
+ if i&os.ModeSetuid != 0 {
+ o |= syscall.S_ISUID
+ }
+ if i&os.ModeSetgid != 0 {
+ o |= syscall.S_ISGID
+ }
+ if i&os.ModeSticky != 0 {
+ o |= syscall.S_ISVTX
+ }
+ // No mapping for Go's ModeTemporary (plan9 only).
+ return o
+}
+
// MkPrefix creates all the missing directories in a given base path and
// returns the file descriptor to the leaf directory as well as the restricted
// flag. This function is a base for secure variants of mkdir, touch and
@@ -180,7 +197,7 @@ func MkDir(dirFd int, dirName string, name string, perm os.FileMode, uid sys.Use
made := true
const openFlags = syscall.O_NOFOLLOW | syscall.O_CLOEXEC | syscall.O_DIRECTORY
- if err := sysMkdirat(dirFd, name, uint32(perm.Perm())); err != nil {
+ if err := sysMkdirat(dirFd, name, syscallMode(perm)); err != nil {
switch err {
case syscall.EEXIST:
made = false
@@ -239,7 +256,7 @@ func MkFile(dirFd int, dirName string, name string, perm os.FileMode, uid sys.Us
// we know if we need to chown it) but fall back to just opening an
// existing one.
- newFd, err := sysOpenat(dirFd, name, openFlags|syscall.O_CREAT|syscall.O_EXCL, uint32(perm.Perm()))
+ newFd, err := sysOpenat(dirFd, name, openFlags|syscall.O_CREAT|syscall.O_EXCL, syscallMode(perm))
if err != nil {
switch err {
case syscall.EEXIST:
diff --git a/cmd/snap-update-ns/utils_test.go b/cmd/snap-update-ns/utils_test.go
index b7d91bb49c..16701c106a 100644
--- a/cmd/snap-update-ns/utils_test.go
+++ b/cmd/snap-update-ns/utils_test.go
@@ -136,6 +136,27 @@ func (s *utilsSuite) TestSecureMkdirAllLevel3(c *C) {
})
}
+// Ensure that we are not masking out the sticky bit when creating directories
+func (s *utilsSuite) TestSecureMkdirAllAllowsStickyBit(c *C) {
+ s.sys.InsertFault(`mkdirat 3 "dev" 01777`, syscall.EEXIST)
+ s.sys.InsertFault(`mkdirat 4 "shm" 01777`, syscall.EEXIST)
+ c.Assert(update.MkdirAll("/dev/shm/snap.foo", 0777|os.ModeSticky, 0, 0, nil), IsNil)
+ c.Assert(s.sys.RCalls(), testutil.SyscallsEqual, []testutil.CallResultError{
+ {C: `open "/" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`, R: 3},
+ {C: `mkdirat 3 "dev" 01777`, E: syscall.EEXIST},
+ {C: `openat 3 "dev" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`, R: 4},
+ {C: `mkdirat 4 "shm" 01777`, E: syscall.EEXIST},
+ {C: `openat 4 "shm" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`, R: 5},
+ {C: `close 4`},
+ {C: `close 3`},
+ {C: `mkdirat 5 "snap.foo" 01777`},
+ {C: `openat 5 "snap.foo" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`, R: 3},
+ {C: `fchown 3 0 0`},
+ {C: `close 3`},
+ {C: `close 5`},
+ })
+}
+
// Ensure that trespassing for prefix is matched using clean base path.
func (s *utilsSuite) TestTrespassingMatcher(c *C) {
// We mounted tmpfs at "/path".
diff --git a/tests/main/interfaces-shared-memory-private/task.yaml b/tests/main/interfaces-shared-memory-private/task.yaml
index 1f10f59cb0..186895f405 100644
--- a/tests/main/interfaces-shared-memory-private/task.yaml
+++ b/tests/main/interfaces-shared-memory-private/task.yaml
@@ -26,6 +26,9 @@ execute: |
echo "shared-memory plugs with 'private: true' set are autoconnected"
snap connections shm-private | MATCH "shared-memory +shm-private:shmem-private +:shared-memory +-"
+ echo "The private /dev/shm directory has the correct permissions"
+ shm-private.cmd stat -c %a /dev/shm | MATCH '^1777$'
+
echo "The snap can create arbitrary named segments under /dev/shm"
shm-private.cmd touch /dev/shm/root-segment
tests.session -u test exec shm-private.cmd touch /dev/shm/user-segment
@@ -41,3 +44,9 @@ execute: |
echo "The private shm segments are stored in a subdirectory of /dev/shm"
test -f /dev/shm/snap.shm-private/root-segment
test -f /dev/shm/snap.shm-private/user-segment
+
+ echo "Mount namespace updates create directory with correct permissions"
+ snap disconnect shm-private:shmem-private
+ rm -rf /dev/shm/snap.shm-private
+ snap connect shm-private:shmem-private
+ shm-private.cmd stat -c %a /dev/shm | MATCH '^1777$'
diff --git a/tests/main/snap-mgmt/task.yaml b/tests/main/snap-mgmt/task.yaml
index a281be6237..ffbe9121b9 100644
--- a/tests/main/snap-mgmt/task.yaml
+++ b/tests/main/snap-mgmt/task.yaml
@@ -74,12 +74,24 @@ prepare: |
test "$(find /etc/systemd/system -name 'snap.*.slice' | wc -l)" -gt 0
fi
+restore: |
+ #shellcheck source=tests/lib/pkgdb.sh
+ . "$TESTSLIB/pkgdb.sh"
+ if [ -e pkg-removed ]; then
+ distro_install_build_snapd
+ rm pkg-removed
+ fi
+
+debug: |
+ systemctl --no-legend --full | grep -E 'snap\..*\.(service|timer|socket|slice)' || true
+
execute: |
echo "Stop snapd before purging"
systemctl stop snapd.service snapd.socket
echo "A purge will really purge things"
snapd.tool exec snap-mgmt --purge
+ touch pkg-removed
echo "Data directories are empty"
SNAP_MOUNT_DIR="$(os.paths snap-mount-dir)"
generated by cgit v1.2.3 (git 2.25.1) at 2025-10-27 07:21:33 +0000