From cf38a00ecc034fb40b8d75874745abe5dd6c777b Mon Sep 17 00:00:00 2001 From: Ophestra Date: Mon, 18 Aug 2025 02:24:56 +0900 Subject: [PATCH] app: set up acl on X11 socket The socket is typically owned by the priv-user, and inaccessible by the target user, so just allowing access to the directory is not enough. This change fixes this oversight and add checks that will also be useful for merging https://git.gensokyo.uk/security/hakurei/pulls/1. Signed-off-by: Ophestra --- internal/app/seal_linux.go | 17 +++++++++++++ system/acl.go | 26 +++++++++++++++++--- system/acl_test.go | 4 +-- test/sandbox/assert.go | 46 +++++++++++++++++++++++++++++++++++ test/sandbox/case/default.nix | 3 +++ test/sandbox/case/device.nix | 11 ++++++++- test/sandbox/case/mapuid.nix | 5 ++++ test/sandbox/case/pdlike.nix | 5 ++++ test/sandbox/case/preset.nix | 5 ++++ test/sandbox/case/tty.nix | 11 ++++++++- 10 files changed, 126 insertions(+), 7 deletions(-) diff --git a/internal/app/seal_linux.go b/internal/app/seal_linux.go index 49ef78f7..53bdced0 100644 --- a/internal/app/seal_linux.go +++ b/internal/app/seal_linux.go @@ -12,6 +12,7 @@ import ( "path" "regexp" "slices" + "strconv" "strings" "sync/atomic" "syscall" @@ -390,6 +391,22 @@ func (seal *outcome) finalise(ctx context.Context, sys sys.State, config *hst.Co seal.env[display] = d socketDir := container.AbsFHSTmp.Append(".X11-unix") seal.container.Bind(socketDir, socketDir, 0) + + // the socket file at `/tmp/.X11-unix/X%d` is typically owned by the priv user + // and not accessible by the target user + var socketPath *container.Absolute + if len(d) > 1 && d[0] == ':' { // `:%d` + if n, err := strconv.Atoi(d[1:]); err == nil && n >= 0 { + socketPath = socketDir.Append("X" + strconv.Itoa(n)) + } + } else if len(d) > 5 && strings.HasPrefix(d, "unix:") { // `unix:%s` + if a, err := container.NewAbs(d[5:]); err == nil { + socketPath = a + } + } + if socketPath != nil { + seal.sys.UpdatePermTypeOptional(system.EX11, socketPath.String(), acl.Read, acl.Write, acl.Execute) + } } } diff --git a/system/acl.go b/system/acl.go index 5d48c6c1..4f6385ec 100644 --- a/system/acl.go +++ b/system/acl.go @@ -21,7 +21,17 @@ func (sys *I) UpdatePermType(et Enablement, path string, perms ...acl.Perm) *I { sys.lock.Lock() defer sys.lock.Unlock() - sys.ops = append(sys.ops, &ACL{et, path, perms}) + sys.ops = append(sys.ops, &ACL{et, path, perms, false}) + + return sys +} + +// UpdatePermTypeOptional appends an acl update Op that silently continues if the target does not exist. +func (sys *I) UpdatePermTypeOptional(et Enablement, path string, perms ...acl.Perm) *I { + sys.lock.Lock() + defer sys.lock.Unlock() + + sys.ops = append(sys.ops, &ACL{et, path, perms, true}) return sys } @@ -30,14 +40,24 @@ type ACL struct { et Enablement path string perms acl.Perms + + // since revert operations are cross-process, the success of apply must not affect the outcome of revert + skipNotExist bool } func (a *ACL) Type() Enablement { return a.et } func (a *ACL) apply(sys *I) error { msg.Verbose("applying ACL", a) - return wrapErrSuffix(acl.Update(a.path, sys.uid, a.perms...), - fmt.Sprintf("cannot apply ACL entry to %q:", a.path)) + if err := acl.Update(a.path, sys.uid, a.perms...); err != nil { + if !a.skipNotExist || !os.IsNotExist(err) { + return wrapErrSuffix(err, + fmt.Sprintf("cannot apply ACL entry to %q:", a.path)) + } + msg.Verbosef("path %q does not exist", a.path) + return nil + } + return nil } func (a *ACL) revert(sys *I, ec *Criteria) error { diff --git a/system/acl_test.go b/system/acl_test.go index 886a62e4..33c56f1c 100644 --- a/system/acl_test.go +++ b/system/acl_test.go @@ -20,7 +20,7 @@ func TestUpdatePerm(t *testing.T) { t.Run(tc.path+permSubTestSuffix(tc.perms), func(t *testing.T) { sys := New(150) sys.UpdatePerm(tc.path, tc.perms...) - (&tcOp{Process, tc.path}).test(t, sys.ops, []Op{&ACL{Process, tc.path, tc.perms}}, "UpdatePerm") + (&tcOp{Process, tc.path}).test(t, sys.ops, []Op{&ACL{Process, tc.path, tc.perms, false}}, "UpdatePerm") }) } } @@ -42,7 +42,7 @@ func TestUpdatePermType(t *testing.T) { t.Run(tc.path+"_"+TypeString(tc.et)+permSubTestSuffix(tc.perms), func(t *testing.T) { sys := New(150) sys.UpdatePermType(tc.et, tc.path, tc.perms...) - tc.test(t, sys.ops, []Op{&ACL{tc.et, tc.path, tc.perms}}, "UpdatePermType") + tc.test(t, sys.ops, []Op{&ACL{tc.et, tc.path, tc.perms, false}}, "UpdatePermType") }) } } diff --git a/test/sandbox/assert.go b/test/sandbox/assert.go index 57d17039..b108269f 100644 --- a/test/sandbox/assert.go +++ b/test/sandbox/assert.go @@ -15,6 +15,7 @@ import ( "errors" "io/fs" "log" + "net" "os" "syscall" ) @@ -33,6 +34,10 @@ type TestCase struct { FS *FS `json:"fs"` Mount []*MountinfoEntry `json:"mount"` Seccomp bool `json:"seccomp"` + + TrySocket string `json:"try_socket,omitempty"` + SocketAbstract bool `json:"socket_abstract,omitempty"` + SocketPathname bool `json:"socket_pathname,omitempty"` } type T struct { @@ -125,6 +130,47 @@ func (t *T) MustCheck(want *TestCase) { } else { printf("[SKIP] skipping seccomp check") } + + if want.TrySocket != "" { + abstractConn, abstractErr := net.Dial("unix", "@"+want.TrySocket) + pathnameConn, pathnameErr := net.Dial("unix", want.TrySocket) + ok := true + + if abstractErr == nil { + if err := abstractConn.Close(); err != nil { + ok = false + log.Printf("Close: %v", err) + } + } + if pathnameErr == nil { + if err := pathnameConn.Close(); err != nil { + ok = false + log.Printf("Close: %v", err) + } + } + + abstractWantErr := error(syscall.ECONNREFUSED) + pathnameWantErr := error(syscall.ENOENT) + if want.SocketAbstract { + abstractWantErr = nil + } + if want.SocketPathname { + pathnameWantErr = nil + } + + if !errors.Is(abstractErr, abstractWantErr) { + ok = false + log.Printf("abstractErr: %v, want %v", abstractErr, abstractWantErr) + } + if !errors.Is(pathnameErr, pathnameWantErr) { + ok = false + log.Printf("pathnameErr: %v, want %v", pathnameErr, pathnameWantErr) + } + + if !ok { + os.Exit(1) + } + } } func MustCheckFilter(pid int, want string) { diff --git a/test/sandbox/case/default.nix b/test/sandbox/case/default.nix index 6db1ff45..ec814aab 100644 --- a/test/sandbox/case/default.nix +++ b/test/sandbox/case/default.nix @@ -50,6 +50,9 @@ let useCommonPaths userns ; + enablements = { + inherit (tc) x11; + }; share = testProgram; packages = [ ]; path = "${testProgram}/bin/hakurei-test"; diff --git a/test/sandbox/case/device.nix b/test/sandbox/case/device.nix index fb2a54f2..10011d74 100644 --- a/test/sandbox/case/device.nix +++ b/test/sandbox/case/device.nix @@ -25,6 +25,7 @@ in mapRealUid = false; useCommonPaths = true; userns = false; + x11 = true; # 0, PresetStrict expectedFilter = { @@ -35,6 +36,7 @@ in want = { env = [ "DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/65534/bus" + "DISPLAY=:0" "HOME=/var/lib/hakurei/u0/a4" "PULSE_SERVER=unix:/run/user/65534/pulse/native" "SHELL=/run/current-system/sw/bin/bash" @@ -161,7 +163,9 @@ in } null; devices = fs "800001ed" null null; } null; - tmp = fs "800001f8" { } null; + tmp = fs "800001f8" { + ".X11-unix" = fs "801001ff" { X0 = fs "10001fd" null null; } null; + } null; usr = fs "800001c0" { bin = fs "800001ed" { env = fs "80001ff" null null; } null; } null; var = fs "800001c0" { lib = fs "800001c0" { @@ -231,10 +235,15 @@ in (ent ignore "/etc/passwd" "ro,nosuid,nodev,relatime" "tmpfs" "rootfs" "rw,uid=1000004,gid=1000004") (ent ignore "/etc/group" "ro,nosuid,nodev,relatime" "tmpfs" "rootfs" "rw,uid=1000004,gid=1000004") (ent ignore "/run/user/65534/wayland-0" "ro,nosuid,nodev,relatime" "ext4" "/dev/disk/by-label/nixos" "rw") + (ent "/tmp/.X11-unix" "/tmp/.X11-unix" "ro,nosuid,nodev,relatime" "ext4" "/dev/disk/by-label/nixos" "rw") (ent ignore "/run/user/65534/pulse/native" "ro,nosuid,nodev,relatime" "tmpfs" "tmpfs" ignore) (ent ignore "/run/user/65534/bus" "ro,nosuid,nodev,relatime" "ext4" "/dev/disk/by-label/nixos" "rw") ]; seccomp = true; + + try_socket = "/tmp/.X11-unix/X0"; + socket_abstract = true; + socket_pathname = true; }; } diff --git a/test/sandbox/case/mapuid.nix b/test/sandbox/case/mapuid.nix index 1069f9fa..18d82e77 100644 --- a/test/sandbox/case/mapuid.nix +++ b/test/sandbox/case/mapuid.nix @@ -34,6 +34,7 @@ in mapRealUid = true; useCommonPaths = true; userns = false; + x11 = false; # 0, PresetStrict expectedFilter = { @@ -266,5 +267,9 @@ in ]; seccomp = true; + + try_socket = "/tmp/.X11-unix/X0"; + socket_abstract = true; + socket_pathname = false; }; } diff --git a/test/sandbox/case/pdlike.nix b/test/sandbox/case/pdlike.nix index 4f0e71c4..da6753f8 100644 --- a/test/sandbox/case/pdlike.nix +++ b/test/sandbox/case/pdlike.nix @@ -34,6 +34,7 @@ in mapRealUid = false; useCommonPaths = false; userns = true; + x11 = false; # 0, PresetExt | PresetDenyDevel expectedFilter = { @@ -261,5 +262,9 @@ in ]; seccomp = true; + + try_socket = "/tmp/.X11-unix/X0"; + socket_abstract = true; + socket_pathname = false; }; } diff --git a/test/sandbox/case/preset.nix b/test/sandbox/case/preset.nix index 60257ad0..be376a17 100644 --- a/test/sandbox/case/preset.nix +++ b/test/sandbox/case/preset.nix @@ -34,6 +34,7 @@ in mapRealUid = false; useCommonPaths = false; userns = false; + x11 = false; # 0, PresetStrict expectedFilter = { @@ -259,5 +260,9 @@ in ]; seccomp = true; + + try_socket = "/tmp/.X11-unix/X0"; + socket_abstract = true; + socket_pathname = false; }; } diff --git a/test/sandbox/case/tty.nix b/test/sandbox/case/tty.nix index 267ac661..b94350eb 100644 --- a/test/sandbox/case/tty.nix +++ b/test/sandbox/case/tty.nix @@ -34,6 +34,7 @@ in mapRealUid = false; useCommonPaths = true; userns = false; + x11 = true; # 0, PresetExt | PresetDenyNS | PresetDenyDevel expectedFilter = { @@ -44,6 +45,7 @@ in want = { env = [ "DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/65534/bus" + "DISPLAY=:0" "HOME=/var/lib/hakurei/u0/a2" "PULSE_SERVER=unix:/run/user/65534/pulse/native" "SHELL=/run/current-system/sw/bin/bash" @@ -188,7 +190,9 @@ in } null; devices = fs "800001ed" null null; } null; - tmp = fs "800001f8" { } null; + tmp = fs "800001f8" { + ".X11-unix" = fs "801001ff" { X0 = fs "10001fd" null null; } null; + } null; usr = fs "800001c0" { bin = fs "800001ed" { env = fs "80001ff" null null; } null; } null; var = fs "800001c0" { lib = fs "800001c0" { @@ -263,10 +267,15 @@ in (ent ignore "/etc/passwd" "ro,nosuid,nodev,relatime" "tmpfs" "rootfs" "rw,uid=1000002,gid=1000002") (ent ignore "/etc/group" "ro,nosuid,nodev,relatime" "tmpfs" "rootfs" "rw,uid=1000002,gid=1000002") (ent ignore "/run/user/65534/wayland-0" "ro,nosuid,nodev,relatime" "ext4" "/dev/disk/by-label/nixos" "rw") + (ent "/tmp/.X11-unix" "/tmp/.X11-unix" "ro,nosuid,nodev,relatime" "ext4" "/dev/disk/by-label/nixos" "rw") (ent ignore "/run/user/65534/pulse/native" "ro,nosuid,nodev,relatime" "tmpfs" "tmpfs" ignore) (ent ignore "/run/user/65534/bus" "ro,nosuid,nodev,relatime" "ext4" "/dev/disk/by-label/nixos" "rw") ]; seccomp = true; + + try_socket = "/tmp/.X11-unix/X0"; + socket_abstract = true; + socket_pathname = true; }; }