From 2f1d42c8dd49a4c3f23568ab8b993473cbc5f7ab 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 # Conflicts: # test/sandbox/case/device.nix # test/sandbox/case/tty.nix --- internal/app/seal_linux.go | 16 ++++++++++++++++ system/acl.go | 26 +++++++++++++++++++++++--- system/acl_test.go | 4 ++-- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/internal/app/seal_linux.go b/internal/app/seal_linux.go index c729372..498ae52 100644 --- a/internal/app/seal_linux.go +++ b/internal/app/seal_linux.go @@ -416,6 +416,22 @@ func (seal *outcome) finalise(ctx context.Context, sys sys.State, config *hst.Co seal.sys.ChangeHosts("#" + seal.user.uid.String()) seal.env[display] = d 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 5d48c6c..4f6385e 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 886a62e..33c56f1 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") }) } }