From 82a072f641b2b8eb4f966a01056283f93b57342f Mon Sep 17 00:00:00 2001 From: Ophestra Date: Mon, 17 Feb 2025 00:07:52 +0900 Subject: [PATCH] system/tmpfiles: implement private tmpfiles These are only available within the mount namespace and should significantly reduce attack surface. Signed-off-by: Ophestra --- internal/app/app_nixos_test.go | 4 +- internal/app/app_pd_test.go | 4 +- internal/app/share.go | 6 +- internal/system/link.go | 2 +- internal/system/op_test.go | 2 +- internal/system/tmpfiles.go | 109 ++++++++++++------------------- internal/system/tmpfiles_test.go | 64 ++++-------------- 7 files changed, 66 insertions(+), 125 deletions(-) diff --git a/internal/app/app_nixos_test.go b/internal/app/app_nixos_test.go index 18a42e1..8923e9b 100644 --- a/internal/app/app_nixos_test.go +++ b/internal/app/app_nixos_test.go @@ -64,7 +64,7 @@ var testCasesNixos = []sealTestCase{ Ephemeral(system.Process, "/run/user/1971/fortify/8e2c76b066dabe574cf073bdb46eb5c1", 0700).UpdatePermType(system.Process, "/run/user/1971/fortify/8e2c76b066dabe574cf073bdb46eb5c1", acl.Execute). UpdatePermType(system.EWayland, "/run/user/1971/wayland-0", acl.Read, acl.Write, acl.Execute). Link("/run/user/1971/pulse/native", "/run/user/1971/fortify/8e2c76b066dabe574cf073bdb46eb5c1/pulse"). - CopyFile("/tmp/fortify.1971/8e2c76b066dabe574cf073bdb46eb5c1/pulse-cookie", "/home/ophestra/xdg/config/pulse/cookie"). + CopyFile(nil, "/home/ophestra/xdg/config/pulse/cookie", 256, 256). MustProxyDBus("/tmp/fortify.1971/8e2c76b066dabe574cf073bdb46eb5c1/bus", &dbus.Config{ Talk: []string{ "org.freedesktop.FileManager1", "org.freedesktop.Notifications", @@ -213,7 +213,7 @@ var testCasesNixos = []sealTestCase{ CopyBind("/etc/group", []byte("fortify:x:1971:\n")). Bind("/run/user/1971/wayland-0", "/run/user/1971/wayland-0"). Bind("/run/user/1971/fortify/8e2c76b066dabe574cf073bdb46eb5c1/pulse", "/run/user/1971/pulse/native"). - Bind("/tmp/fortify.1971/8e2c76b066dabe574cf073bdb46eb5c1/pulse-cookie", fst.Tmp+"/pulse-cookie"). + CopyBind(fst.Tmp+"/pulse-cookie", nil). Bind("/tmp/fortify.1971/8e2c76b066dabe574cf073bdb46eb5c1/bus", "/run/user/1971/bus"). Bind("/tmp/fortify.1971/8e2c76b066dabe574cf073bdb46eb5c1/system_bus_socket", "/run/dbus/system_bus_socket"). Tmpfs("/var/run/nscd", 8192). diff --git a/internal/app/app_pd_test.go b/internal/app/app_pd_test.go index 269c8aa..4d90d7f 100644 --- a/internal/app/app_pd_test.go +++ b/internal/app/app_pd_test.go @@ -219,7 +219,7 @@ var testCasesPd = []sealTestCase{ Ensure("/tmp/fortify.1971/wayland", 0711). Wayland("/tmp/fortify.1971/wayland/ebf083d1b175911782d413369b64ce7c", "/run/user/1971/wayland-0", "org.chromium.Chromium", "ebf083d1b175911782d413369b64ce7c"). Link("/run/user/1971/pulse/native", "/run/user/1971/fortify/ebf083d1b175911782d413369b64ce7c/pulse"). - CopyFile("/tmp/fortify.1971/ebf083d1b175911782d413369b64ce7c/pulse-cookie", "/home/ophestra/xdg/config/pulse/cookie"). + CopyFile(new([]byte), "/home/ophestra/xdg/config/pulse/cookie", 256, 256). MustProxyDBus("/tmp/fortify.1971/ebf083d1b175911782d413369b64ce7c/bus", &dbus.Config{ Talk: []string{ "org.freedesktop.Notifications", @@ -382,7 +382,7 @@ var testCasesPd = []sealTestCase{ CopyBind("/etc/group", []byte("fortify:x:65534:\n")). Bind("/tmp/fortify.1971/wayland/ebf083d1b175911782d413369b64ce7c", "/run/user/65534/wayland-0"). Bind("/run/user/1971/fortify/ebf083d1b175911782d413369b64ce7c/pulse", "/run/user/65534/pulse/native"). - Bind("/tmp/fortify.1971/ebf083d1b175911782d413369b64ce7c/pulse-cookie", fst.Tmp+"/pulse-cookie"). + CopyBind(fst.Tmp+"/pulse-cookie", nil). Bind("/tmp/fortify.1971/ebf083d1b175911782d413369b64ce7c/bus", "/run/user/65534/bus"). Bind("/tmp/fortify.1971/ebf083d1b175911782d413369b64ce7c/system_bus_socket", "/run/dbus/system_bus_socket"). Tmpfs("/var/run/nscd", 8192). diff --git a/internal/app/share.go b/internal/app/share.go index c7f0c7a..1862fb2 100644 --- a/internal/app/share.go +++ b/internal/app/share.go @@ -231,11 +231,11 @@ func (seal *appSeal) setupShares(bus [2]*dbus.Config, os linux.System) error { // not fatal fmsg.Verbose(strings.TrimSpace(err.(*fmsg.BaseError).Message())) } else { - dst := path.Join(seal.share, "pulse-cookie") innerDst := fst.Tmp + "/pulse-cookie" seal.sys.bwrap.SetEnv[pulseCookie] = innerDst - seal.sys.CopyFile(dst, src) - seal.sys.bwrap.Bind(dst, innerDst) + payload := new([]byte) + seal.sys.bwrap.CopyBindRef(innerDst, &payload) + seal.sys.CopyFile(payload, src, 256, 256) } } diff --git a/internal/system/link.go b/internal/system/link.go index 7e690c7..9689e25 100644 --- a/internal/system/link.go +++ b/internal/system/link.go @@ -28,7 +28,7 @@ type Hardlink struct { func (l *Hardlink) Type() Enablement { return l.et } func (l *Hardlink) apply(_ *I) error { - fmsg.Verbose("linking ", l) + fmsg.Verbose("linking", l) return fmsg.WrapErrorSuffix(os.Link(l.src, l.dst), fmt.Sprintf("cannot link %q:", l.dst)) } diff --git a/internal/system/op_test.go b/internal/system/op_test.go index 28929e6..3e8c4d2 100644 --- a/internal/system/op_test.go +++ b/internal/system/op_test.go @@ -100,7 +100,7 @@ func TestI_Equal(t *testing.T) { "op type mismatch", system.New(150). ChangeHosts("chronos"). - CopyFile("/tmp/fortify.1971/30c9543e0a2c9621a8bfecb9d874c347/pulse-cookie", "/home/ophestra/xdg/config/pulse/cookie"), + CopyFile(new([]byte), "/home/ophestra/xdg/config/pulse/cookie", 0, 256), system.New(150). ChangeHosts("chronos"). Ensure("/run", 0755), diff --git a/internal/system/tmpfiles.go b/internal/system/tmpfiles.go index 8208e03..5a2307f 100644 --- a/internal/system/tmpfiles.go +++ b/internal/system/tmpfiles.go @@ -1,95 +1,72 @@ package system import ( - "errors" + "bytes" "fmt" "io" "os" - "strconv" + "syscall" - "git.gensokyo.uk/security/fortify/acl" "git.gensokyo.uk/security/fortify/internal/fmsg" ) -// CopyFile registers an Op that copies path dst from src. -func (sys *I) CopyFile(dst, src string) *I { - return sys.CopyFileType(Process, dst, src) -} +// CopyFile registers an Op that copies from src. +// A buffer is initialised with size cap and the Op faults if bytes read exceed n. +func (sys *I) CopyFile(payload *[]byte, src string, cap int, n int64) *I { + buf := new(bytes.Buffer) + buf.Grow(cap) -// CopyFileType registers a file copying Op labelled with type et. -func (sys *I) CopyFileType(et Enablement, dst, src string) *I { sys.lock.Lock() - sys.ops = append(sys.ops, &Tmpfile{et, tmpfileCopy, dst, src}) + sys.ops = append(sys.ops, &Tmpfile{payload, src, n, buf}) sys.lock.Unlock() - sys.UpdatePermType(et, dst, acl.Read) - return sys } -const ( - tmpfileCopy uint8 = iota -) - type Tmpfile struct { - et Enablement - method uint8 - dst, src string -} - -func (t *Tmpfile) Type() Enablement { - return t.et + payload *[]byte + src string + + n int64 + buf *bytes.Buffer } +func (t *Tmpfile) Type() Enablement { return Process } func (t *Tmpfile) apply(_ *I) error { - switch t.method { - case tmpfileCopy: - fmsg.Verbose("publishing tmpfile", t) - return fmsg.WrapErrorSuffix(copyFile(t.dst, t.src), - fmt.Sprintf("cannot copy tmpfile %q:", t.dst)) - default: - panic("invalid tmpfile method " + strconv.Itoa(int(t.method))) - } -} + fmsg.Verbose("copying", t) -func (t *Tmpfile) revert(_ *I, ec *Criteria) error { - if ec.hasType(t) { - fmsg.Verbosef("removing tmpfile %q", t.dst) - return fmsg.WrapErrorSuffix(os.Remove(t.dst), - fmt.Sprintf("cannot remove tmpfile %q:", t.dst)) + if b, err := os.Stat(t.src); err != nil { + return fmsg.WrapErrorSuffix(err, + fmt.Sprintf("cannot stat %q:", t.src)) } else { - fmsg.Verbosef("skipping tmpfile %q", t.dst) - return nil + if b.IsDir() { + return fmsg.WrapErrorSuffix(syscall.EISDIR, + fmt.Sprintf("%q is a directory", t.src)) + } + if s := b.Size(); s > t.n { + return fmsg.WrapErrorSuffix(syscall.ENOMEM, + fmt.Sprintf("file %q is too long: %d > %d", + t.src, s, t.n)) + } } + + if f, err := os.Open(t.src); err != nil { + return fmsg.WrapErrorSuffix(err, + fmt.Sprintf("cannot open %q:", t.src)) + } else if _, err = io.CopyN(t.buf, f, t.n); err != nil { + return fmsg.WrapErrorSuffix(err, + fmt.Sprintf("cannot read from %q:", t.src)) + } + + *t.payload = t.buf.Bytes() + return nil } +func (t *Tmpfile) revert(*I, *Criteria) error { t.buf.Reset(); return nil } func (t *Tmpfile) Is(o Op) bool { t0, ok := o.(*Tmpfile) - return ok && t0 != nil && *t == *t0 -} - -func (t *Tmpfile) Path() string { return t.src } - -func (t *Tmpfile) String() string { - switch t.method { - case tmpfileCopy: - return fmt.Sprintf("%q from %q", t.dst, t.src) - default: - panic("invalid tmpfile method " + strconv.Itoa(int(t.method))) - } -} - -func copyFile(dst, src string) error { - dstD, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) - if err != nil { - return err - } - - srcD, err := os.Open(src) - if err != nil { - return errors.Join(err, dstD.Close()) - } - - _, err = io.Copy(dstD, srcD) - return errors.Join(err, dstD.Close(), srcD.Close()) + return ok && t0 != nil && + t.src == t0.src && t.n == t0.n } +func (t *Tmpfile) Path() string { return t.src } +func (t *Tmpfile) String() string { return fmt.Sprintf("up to %d bytes from %q", t.n, t.src) } diff --git a/internal/system/tmpfiles_test.go b/internal/system/tmpfiles_test.go index 254c873..c7402f5 100644 --- a/internal/system/tmpfiles_test.go +++ b/internal/system/tmpfiles_test.go @@ -1,46 +1,25 @@ package system import ( + "strconv" "testing" - - "git.gensokyo.uk/security/fortify/acl" ) func TestCopyFile(t *testing.T) { - testCases := []struct { - dst, src string - }{ - {"/tmp/fortify.1971/f587afe9fce3c8e1ad5b64deb6c41ad5/pulse-cookie", "/home/ophestra/xdg/config/pulse/cookie"}, - {"/tmp/fortify.1971/62154f708b5184ab01f9dcc2bbe7a33b/pulse-cookie", "/home/ophestra/xdg/config/pulse/cookie"}, - } - for _, tc := range testCases { - t.Run("copy file "+tc.dst+" from "+tc.src, func(t *testing.T) { - sys := New(150) - sys.CopyFile(tc.dst, tc.src) - (&tcOp{Process, tc.src}).test(t, sys.ops, []Op{ - &Tmpfile{Process, tmpfileCopy, tc.dst, tc.src}, - &ACL{Process, tc.dst, []acl.Perm{acl.Read}}, - }, "CopyFile") - }) - } -} - -func TestCopyFileType(t *testing.T) { testCases := []struct { tcOp - dst string + cap int + n int64 }{ - {tcOp{User, "/tmp/fortify.1971/f587afe9fce3c8e1ad5b64deb6c41ad5/pulse-cookie"}, "/home/ophestra/xdg/config/pulse/cookie"}, - {tcOp{Process, "/tmp/fortify.1971/62154f708b5184ab01f9dcc2bbe7a33b/pulse-cookie"}, "/home/ophestra/xdg/config/pulse/cookie"}, + {tcOp{Process, "/home/ophestra/xdg/config/pulse/cookie"}, 256, 256}, } for _, tc := range testCases { - t.Run("copy file "+tc.dst+" from "+tc.path+" with type "+TypeString(tc.et), func(t *testing.T) { + t.Run("copy file "+tc.path+" with cap = "+strconv.Itoa(tc.cap)+" n = "+strconv.Itoa(int(tc.n)), func(t *testing.T) { sys := New(150) - sys.CopyFileType(tc.et, tc.dst, tc.path) + sys.CopyFile(new([]byte), tc.path, tc.cap, tc.n) tc.test(t, sys.ops, []Op{ - &Tmpfile{tc.et, tmpfileCopy, tc.dst, tc.path}, - &ACL{tc.et, tc.dst, []acl.Perm{acl.Read}}, - }, "CopyFileType") + &Tmpfile{nil, tc.path, tc.n, nil}, + }, "CopyFile") }) } } @@ -83,33 +62,18 @@ func TestLinkFileType(t *testing.T) { } func TestTmpfile_String(t *testing.T) { - t.Run("invalid method panic", func(t *testing.T) { - defer func() { - wantPanic := "invalid tmpfile method 255" - if r := recover(); r != wantPanic { - t.Errorf("String() panic = %v, want %v", - r, wantPanic) - } - }() - _ = (&Tmpfile{method: 255}).String() - }) - testCases := []struct { - method uint8 - dst, src string - want string + src string + n int64 + want string }{ - {tmpfileCopy, "/tmp/fortify.1971/4b6bdc9182fb2f1d3a965c5fa8b9b66e/pulse-cookie", "/home/ophestra/xdg/config/pulse/cookie", - `"/tmp/fortify.1971/4b6bdc9182fb2f1d3a965c5fa8b9b66e/pulse-cookie" from "/home/ophestra/xdg/config/pulse/cookie"`}, + {"/home/ophestra/xdg/config/pulse/cookie", 256, + `up to 256 bytes from "/home/ophestra/xdg/config/pulse/cookie"`}, } for _, tc := range testCases { t.Run(tc.want, func(t *testing.T) { - if got := (&Tmpfile{ - method: tc.method, - dst: tc.dst, - src: tc.src, - }).String(); got != tc.want { + if got := (&Tmpfile{src: tc.src, n: tc.n}).String(); got != tc.want { t.Errorf("String() = %v, want %v", got, tc.want) } })