From 52f21a19f3230e62387bebb88d45d51b6375ac8d Mon Sep 17 00:00:00 2001 From: Ophestra Umiker Date: Wed, 18 Dec 2024 19:39:25 +0900 Subject: [PATCH] cmd/fshim: switch to setup pipe The socket-based approach is no longer necessary as fsu allows extra files and sudo compatibility is no longer relevant. Signed-off-by: Ophestra Umiker --- cmd/fshim/ipc/payload.go | 14 --- cmd/fshim/ipc/shim/shim.go | 180 ++++++------------------------------- cmd/fshim/main.go | 43 ++++----- cmd/fsu/main.go | 12 +-- internal/app/start.go | 8 -- 5 files changed, 52 insertions(+), 205 deletions(-) diff --git a/cmd/fshim/ipc/payload.go b/cmd/fshim/ipc/payload.go index f6b0c92..d833073 100644 --- a/cmd/fshim/ipc/payload.go +++ b/cmd/fshim/ipc/payload.go @@ -1,11 +1,7 @@ package shim0 import ( - "encoding/gob" - "net" - "git.ophivana.moe/security/fortify/helper/bwrap" - "git.ophivana.moe/security/fortify/internal/fmsg" ) const Env = "FORTIFY_SHIM" @@ -23,13 +19,3 @@ type Payload struct { // verbosity pass through Verbose bool } - -func (p *Payload) Serve(conn *net.UnixConn) error { - if err := gob.NewEncoder(conn).Encode(*p); err != nil { - return fmsg.WrapErrorSuffix(err, - "cannot stream shim payload:") - } - - return fmsg.WrapErrorSuffix(conn.Close(), - "cannot close setup connection:") -} diff --git a/cmd/fshim/ipc/shim/shim.go b/cmd/fshim/ipc/shim/shim.go index 4b6b5f8..383f3fc 100644 --- a/cmd/fshim/ipc/shim/shim.go +++ b/cmd/fshim/ipc/shim/shim.go @@ -1,18 +1,16 @@ package shim import ( + "encoding/gob" "errors" - "net" "os" "os/exec" "os/signal" + "strconv" "strings" - "sync" - "sync/atomic" "syscall" "time" - "git.ophivana.moe/security/fortify/acl" shim0 "git.ophivana.moe/security/fortify/cmd/fshim/ipc" "git.ophivana.moe/security/fortify/internal" "git.ophivana.moe/security/fortify/internal/fmsg" @@ -32,20 +30,14 @@ type Shim struct { aid string // string representation of supplementary group ids supp []string - // path to setup socket - socket string - // shim setup abort reason and completion - abort chan error - abortErr atomic.Pointer[error] - abortOnce sync.Once // fallback exit notifier with error returned killing the process killFallback chan error // shim setup payload payload *shim0.Payload } -func New(uid uint32, aid string, supp []string, socket string, payload *shim0.Payload) *Shim { - return &Shim{uid: uid, aid: aid, supp: supp, socket: socket, payload: payload} +func New(uid uint32, aid string, supp []string, payload *shim0.Payload) *Shim { + return &Shim{uid: uid, aid: aid, supp: supp, payload: payload} } func (s *Shim) String() string { @@ -59,39 +51,11 @@ func (s *Shim) Unwrap() *exec.Cmd { return s.cmd } -func (s *Shim) Abort(err error) { - s.abortOnce.Do(func() { - s.abortErr.Store(&err) - // s.abort is buffered so this will never block - s.abort <- err - }) -} - -func (s *Shim) AbortWait(err error) { - s.Abort(err) - <-s.abort -} - func (s *Shim) WaitFallback() chan error { return s.killFallback } func (s *Shim) Start() (*time.Time, error) { - var ( - cf chan *net.UnixConn - accept func() - ) - - // listen on setup socket - if c, a, err := s.serve(); err != nil { - return nil, fmsg.WrapErrorSuffix(err, - "cannot listen on shim setup socket:") - } else { - // accepts a connection after each call to accept - // connections are sent to the channel cf - cf, accept = c, a - } - // start user switcher process and save time var fsu string if p, ok := internal.Check(internal.Fsu); !ok { @@ -101,10 +65,19 @@ func (s *Shim) Start() (*time.Time, error) { fsu = p } s.cmd = exec.Command(fsu) - s.cmd.Env = []string{ - shim0.Env + "=" + s.socket, - "FORTIFY_APP_ID=" + s.aid, + + var encoder *gob.Encoder + if fd, e, err := proc.Setup(&s.cmd.ExtraFiles); err != nil { + return nil, fmsg.WrapErrorSuffix(err, + "cannot create shim setup pipe:") + } else { + encoder = e + s.cmd.Env = []string{ + shim0.Env + "=" + strconv.Itoa(fd), + "FORTIFY_APP_ID=" + s.aid, + } } + if len(s.supp) > 0 { fmsg.VPrintf("attaching supplementary group ids %s", s.supp) s.cmd.Env = append(s.cmd.Env, "FORTIFY_GROUPS="+strings.Join(s.supp, " ")) @@ -145,117 +118,20 @@ func (s *Shim) Start() (*time.Time, error) { signal.Ignore(syscall.SIGINT, syscall.SIGTERM) }() - accept() - var conn *net.UnixConn + shimErr := make(chan error) + go func() { shimErr <- encoder.Encode(s.payload) }() + select { - case c := <-cf: - if c == nil { - return &startTime, fmsg.WrapErrorSuffix(*s.abortErr.Load(), "cannot accept call on setup socket:") - } else { - conn = c + case err := <-shimErr: + if err != nil { + return &startTime, fmsg.WrapErrorSuffix(err, + "cannot transmit shim config:") } - case <-time.After(shimSetupTimeout): - err := fmsg.WrapError(errors.New("timed out waiting for shim"), - "timed out waiting for shim to connect") - s.AbortWait(err) - return &startTime, err - } - - // authenticate against called provided uid and shim pid - if cred, err := peerCred(conn); err != nil { - return &startTime, fmsg.WrapErrorSuffix(*s.abortErr.Load(), "cannot retrieve shim credentials:") - } else if cred.Uid != s.uid { - fmsg.Printf("process %d owned by user %d tried to connect, expecting %d", - cred.Pid, cred.Uid, s.uid) - err = errors.New("compromised fortify build") - s.Abort(err) - return &startTime, err - } else if cred.Pid != int32(s.cmd.Process.Pid) { - fmsg.Printf("process %d tried to connect to shim setup socket, expecting shim %d", - cred.Pid, s.cmd.Process.Pid) - err = errors.New("compromised target user") - s.Abort(err) - return &startTime, err - } - - // serve payload - // this also closes the connection - err := s.payload.Serve(conn) - if err == nil { killShim = func() {} + case <-time.After(shimSetupTimeout): + return &startTime, fmsg.WrapError(errors.New("timed out waiting for shim"), + "timed out waiting for shim") } - s.Abort(err) // aborting with nil indicates success - return &startTime, err -} - -func (s *Shim) serve() (chan *net.UnixConn, func(), error) { - if s.abort != nil { - panic("attempted to serve shim setup twice") - } - s.abort = make(chan error, 1) - - cf := make(chan *net.UnixConn) - accept := make(chan struct{}, 1) - - if l, err := net.ListenUnix("unix", &net.UnixAddr{Name: s.socket, Net: "unix"}); err != nil { - return nil, nil, err - } else { - l.SetUnlinkOnClose(true) - - fmsg.VPrintf("listening on shim setup socket %q", s.socket) - if err = acl.UpdatePerm(s.socket, int(s.uid), acl.Read, acl.Write, acl.Execute); err != nil { - fmsg.Println("cannot append ACL entry to shim setup socket:", err) - s.Abort(err) // ensures setup socket cleanup - } - - go func() { - cfWg := new(sync.WaitGroup) - for { - select { - case err = <-s.abort: - if err != nil { - fmsg.VPrintln("aborting shim setup, reason:", err) - } - if err = l.Close(); err != nil { - fmsg.Println("cannot close setup socket:", err) - } - close(s.abort) - go func() { - cfWg.Wait() - close(cf) - }() - return - case <-accept: - cfWg.Add(1) - go func() { - defer cfWg.Done() - if conn, err0 := l.AcceptUnix(); err0 != nil { - // breaks loop - s.Abort(err0) - // receiver sees nil value and loads err0 stored during abort - cf <- nil - } else { - cf <- conn - } - }() - } - } - }() - } - - return cf, func() { accept <- struct{}{} }, nil -} - -// peerCred fetches peer credentials of conn -func peerCred(conn *net.UnixConn) (ucred *syscall.Ucred, err error) { - var raw syscall.RawConn - if raw, err = conn.SyscallConn(); err != nil { - return - } - - err0 := raw.Control(func(fd uintptr) { - ucred, err = syscall.GetsockoptUcred(int(fd), syscall.SOL_SOCKET, syscall.SO_PEERCRED) - }) - err = errors.Join(err, err0) - return + + return &startTime, nil } diff --git a/cmd/fshim/main.go b/cmd/fshim/main.go index d48a294..84ce700 100644 --- a/cmd/fshim/main.go +++ b/cmd/fshim/main.go @@ -1,8 +1,7 @@ package main import ( - "encoding/gob" - "net" + "errors" "os" "path" "strconv" @@ -38,15 +37,6 @@ func main() { } } - // lookup socket path from environment - var socketPath string - if s, ok := os.LookupEnv(shim.Env); !ok { - fmsg.Fatal("FORTIFY_SHIM not set") - panic("unreachable") - } else { - socketPath = s - } - // check path to finit var finitPath string if p, ok := internal.Path(internal.Finit); !ok { @@ -55,21 +45,24 @@ func main() { finitPath = p } - // dial setup socket - var conn *net.UnixConn - if c, err := net.DialUnix("unix", nil, &net.UnixAddr{Name: socketPath, Net: "unix"}); err != nil { - fmsg.Fatal(err.Error()) + // receive setup payload + var ( + payload shim.Payload + closeSetup func() error + ) + if f, err := proc.Receive(shim.Env, &payload); err != nil { + if errors.Is(err, proc.ErrInvalid) { + fmsg.Fatal("invalid config descriptor") + } + if errors.Is(err, proc.ErrNotSet) { + fmsg.Fatal("FORTIFY_SHIM not set") + } + + fmsg.Fatalf("cannot decode shim setup payload: %v", err) panic("unreachable") - } else { - conn = c - } - - // decode payload gob stream - var payload shim.Payload - if err := gob.NewDecoder(conn).Decode(&payload); err != nil { - fmsg.Fatalf("cannot decode shim payload: %v", err) } else { fmsg.SetVerbose(payload.Verbose) + closeSetup = f } if payload.Bwrap == nil { @@ -82,8 +75,8 @@ func main() { } // close setup socket - if err := conn.Close(); err != nil { - fmsg.Println("cannot close setup socket:", err) + if err := closeSetup(); err != nil { + fmsg.Println("cannot close setup pipe:", err) // not fatal } diff --git a/cmd/fsu/main.go b/cmd/fsu/main.go index d232818..2412a0e 100644 --- a/cmd/fsu/main.go +++ b/cmd/fsu/main.go @@ -83,17 +83,17 @@ func main() { uid += aid } - // pass through setup path to shim - var shimSetupPath string + // pass through setup fd to shim + var shimSetupFd string if s, ok := os.LookupEnv(envShim); !ok { // fortify requests target uid // print resolved uid and exit fmt.Print(uid) os.Exit(0) - } else if !path.IsAbs(s) { - log.Fatal("FORTIFY_SHIM is not absolute") + } else if len(s) != 1 || s[0] > '9' || s[0] < '3' { + log.Fatal("FORTIFY_SHIM holds an invalid value") } else { - shimSetupPath = s + shimSetupFd = s } // supplementary groups @@ -142,7 +142,7 @@ func main() { if _, _, errno := syscall.AllThreadsSyscall(syscall.SYS_PRCTL, PR_SET_NO_NEW_PRIVS, 1, 0); errno != 0 { log.Fatalf("cannot set no_new_privs flag: %s", errno.Error()) } - if err := syscall.Exec(fshim, []string{"fshim"}, []string{envShim + "=" + shimSetupPath}); err != nil { + if err := syscall.Exec(fshim, []string{"fshim"}, []string{envShim + "=" + shimSetupFd}); err != nil { log.Fatalf("cannot start shim: %v", err) } diff --git a/internal/app/start.go b/internal/app/start.go index 9186451..1ebc94f 100644 --- a/internal/app/start.go +++ b/internal/app/start.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "os/exec" - "path" "path/filepath" "strings" @@ -46,7 +45,6 @@ func (a *app) Start() error { uint32(a.seal.sys.UID()), a.seal.sys.user.as, a.seal.sys.user.supp, - path.Join(a.seal.share, "shim"), &shim0.Payload{ Argv: a.seal.command, Exec: shimExec, @@ -252,12 +250,6 @@ func (a *app) Wait() (int, error) { } } - if a.shim.Unwrap() == nil { - fmsg.VPrintln("fault before shim start") - } else { - a.shim.AbortWait(errors.New("shim exited")) - } - if a.seal.sys.needRevert { if err := a.seal.sys.Revert(ec); err != nil { return err.(RevertCompoundError)