From c2bf3dd76e1ccd6b0cf9e85e0fed61f763284757 Mon Sep 17 00:00:00 2001 From: Thomas Perl Date: Thu, 15 Feb 2024 07:50:27 +0100 Subject: [PATCH] chore(daemon): Remove untrusted socket --- internals/daemon/daemon.go | 80 +++++++--------------- internals/daemon/daemon_test.go | 118 -------------------------------- 2 files changed, 25 insertions(+), 173 deletions(-) diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go index fef76dd7..53d13c2d 100644 --- a/internals/daemon/daemon.go +++ b/internals/daemon/daemon.go @@ -81,22 +81,20 @@ type Options struct { // A Daemon listens for requests and routes them to the right command type Daemon struct { - Version string - StartTime time.Time - pebbleDir string - normalSocketPath string - untrustedSocketPath string - httpAddress string - overlord *overlord.Overlord - state *state.State - generalListener net.Listener - untrustedListener net.Listener - httpListener net.Listener - connTracker *connTracker - serve *http.Server - tomb tomb.Tomb - router *mux.Router - standbyOpinions *standby.StandbyOpinions + Version string + StartTime time.Time + pebbleDir string + normalSocketPath string + httpAddress string + overlord *overlord.Overlord + state *state.State + generalListener net.Listener + httpListener net.Listener + connTracker *connTracker + serve *http.Server + tomb tomb.Tomb + router *mux.Router + standbyOpinions *standby.StandbyOpinions // set to what kind of restart was requested (if any) requestedRestart restart.RestartType @@ -123,14 +121,13 @@ type Command struct { Path string PathPrefix string // - GET ResponseFunc - PUT ResponseFunc - POST ResponseFunc - DELETE ResponseFunc - GuestOK bool - UserOK bool - UntrustedOK bool - AdminOnly bool + GET ResponseFunc + PUT ResponseFunc + POST ResponseFunc + DELETE ResponseFunc + GuestOK bool + UserOK bool + AdminOnly bool d *Daemon } @@ -153,9 +150,8 @@ const ( // - GuestOK: anyone can access GET // - UserOK: any uid on the local system can access GET // - AdminOnly: only the administrator can access this -// - UntrustedOK: can access this via the untrusted socket func (c *Command) canAccess(r *http.Request, user *UserState) accessResult { - if c.AdminOnly && (c.UserOK || c.GuestOK || c.UntrustedOK) { + if c.AdminOnly && (c.UserOK || c.GuestOK) { logger.Panicf("internal error: command cannot have AdminOnly together with any *OK flag") } @@ -174,15 +170,6 @@ func (c *Command) canAccess(r *http.Request, user *UserState) accessResult { return accessForbidden } - isUntrusted := (ucred != nil && ucred.Socket == c.d.untrustedSocketPath) - - if isUntrusted { - if c.UntrustedOK { - return accessOK - } - return accessUnauthorized - } - // the !AdminOnly check is redundant, but belt-and-suspenders if r.Method == "GET" && !c.AdminOnly { // Guest and user access restricted to GET requests @@ -369,14 +356,6 @@ func (d *Daemon) Init() error { return fmt.Errorf("when trying to listen on %s: %v", d.normalSocketPath, err) } - if listener, err := getListener(d.untrustedSocketPath, listenerMap); err == nil { - // This listener may also be nil if that socket wasn't among - // the listeners, so check it before using it. - d.untrustedListener = &ucrednetListener{Listener: listener} - } else { - logger.Debugf("cannot get listener for %q: %v", d.untrustedSocketPath, err) - } - d.addRoutes() if d.httpAddress != "" { @@ -489,14 +468,6 @@ func (d *Daemon) Start() error { d.overlord.Loop() d.tomb.Go(func() error { - if d.untrustedListener != nil { - d.tomb.Go(func() error { - if err := d.serve.Serve(d.untrustedListener); err != http.ErrServerClosed && d.tomb.Err() == tomb.ErrStillAlive { - return err - } - return nil - }) - } if err := d.serve.Serve(d.generalListener); err != http.ErrServerClosed && d.tomb.Err() == tomb.ErrStillAlive { return err } @@ -853,10 +824,9 @@ func (d *Daemon) SetServiceArgs(serviceArgs map[string][]string) error { func New(opts *Options) (*Daemon, error) { d := &Daemon{ - pebbleDir: opts.Dir, - normalSocketPath: opts.SocketPath, - untrustedSocketPath: opts.SocketPath + ".untrusted", - httpAddress: opts.HTTPAddress, + pebbleDir: opts.Dir, + normalSocketPath: opts.SocketPath, + httpAddress: opts.HTTPAddress, } ovldOptions := overlord.Options{ diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go index e3784dc9..b04134a5 100644 --- a/internals/daemon/daemon_test.go +++ b/internals/daemon/daemon_test.go @@ -204,10 +204,6 @@ func (s *daemonSuite) TestExplicitPaths(c *C) { info, err := os.Stat(s.socketPath) c.Assert(err, IsNil) c.Assert(info.Mode(), Equals, os.ModeSocket|0666) - - info, err = os.Stat(s.socketPath + ".untrusted") - c.Assert(err, IsNil) - c.Assert(info.Mode(), Equals, os.ModeSocket|0666) } func (s *daemonSuite) TestCommandMethodDispatch(c *C) { @@ -370,38 +366,6 @@ func (s *daemonSuite) TestGuestAccess(c *C) { c.Check(cmd.canAccess(del, nil), Equals, accessUnauthorized) } -func (s *daemonSuite) TestUntrustedAccessUntrustedOKWithUser(c *C) { - d := s.newDaemon(c) - - remoteAddr := "pid=100;uid=1000;socket=" + d.untrustedSocketPath + ";" - get := &http.Request{Method: "GET", RemoteAddr: remoteAddr} - put := &http.Request{Method: "PUT", RemoteAddr: remoteAddr} - pst := &http.Request{Method: "POST", RemoteAddr: remoteAddr} - del := &http.Request{Method: "DELETE", RemoteAddr: remoteAddr} - - cmd := &Command{d: d, UntrustedOK: true} - c.Check(cmd.canAccess(get, nil), Equals, accessOK) - c.Check(cmd.canAccess(put, nil), Equals, accessOK) - c.Check(cmd.canAccess(pst, nil), Equals, accessOK) - c.Check(cmd.canAccess(del, nil), Equals, accessOK) -} - -func (s *daemonSuite) TestUntrustedAccessUntrustedOKWithRoot(c *C) { - d := s.newDaemon(c) - - remoteAddr := "pid=100;uid=0;socket=" + d.untrustedSocketPath + ";" - get := &http.Request{Method: "GET", RemoteAddr: remoteAddr} - put := &http.Request{Method: "PUT", RemoteAddr: remoteAddr} - pst := &http.Request{Method: "POST", RemoteAddr: remoteAddr} - del := &http.Request{Method: "DELETE", RemoteAddr: remoteAddr} - - cmd := &Command{d: d, UntrustedOK: true} - c.Check(cmd.canAccess(get, nil), Equals, accessOK) - c.Check(cmd.canAccess(put, nil), Equals, accessOK) - c.Check(cmd.canAccess(pst, nil), Equals, accessOK) - c.Check(cmd.canAccess(del, nil), Equals, accessOK) -} - func (s *daemonSuite) TestUserAccess(c *C) { d := s.newDaemon(c) @@ -423,13 +387,6 @@ func (s *daemonSuite) TestUserAccess(c *C) { cmd = &Command{d: d, GuestOK: true} c.Check(cmd.canAccess(get, nil), Equals, accessOK) c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized) - - // Since this request has a RemoteAddr, it must be coming from the pebble server - // socket instead of the pebble one. In that case, UntrustedOK should have no - // bearing on the default behavior, which is to deny access. - cmd = &Command{d: d, UntrustedOK: true} - c.Check(cmd.canAccess(get, nil), Equals, accessUnauthorized) - c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized) } func (s *daemonSuite) TestLoggedInUserAccess(c *C) { @@ -454,10 +411,6 @@ func (s *daemonSuite) TestLoggedInUserAccess(c *C) { cmd = &Command{d: d, GuestOK: true} c.Check(cmd.canAccess(get, user), Equals, accessOK) c.Check(cmd.canAccess(put, user), Equals, accessOK) - - cmd = &Command{d: d, UntrustedOK: true} - c.Check(cmd.canAccess(get, user), Equals, accessOK) - c.Check(cmd.canAccess(put, user), Equals, accessOK) } func (s *daemonSuite) TestSuperAccess(c *C) { @@ -483,10 +436,6 @@ func (s *daemonSuite) TestSuperAccess(c *C) { cmd = &Command{d: d, GuestOK: true} c.Check(cmd.canAccess(get, nil), Equals, accessOK) c.Check(cmd.canAccess(put, nil), Equals, accessOK) - - cmd = &Command{d: d, UntrustedOK: true} - c.Check(cmd.canAccess(get, nil), Equals, accessOK) - c.Check(cmd.canAccess(put, nil), Equals, accessOK) } } @@ -545,15 +494,10 @@ func (s *daemonSuite) TestStartStop(c *C) { l1, err := net.Listen("tcp", "127.0.0.1:0") c.Assert(err, IsNil) - l2, err := net.Listen("tcp", "127.0.0.1:0") - c.Assert(err, IsNil) generalAccept := make(chan struct{}) d.generalListener = &witnessAcceptListener{Listener: l1, accept: generalAccept} - untrustedAccept := make(chan struct{}) - d.untrustedListener = &witnessAcceptListener{Listener: l2, accept: untrustedAccept} - c.Assert(d.Start(), IsNil) generalDone := make(chan struct{}) @@ -566,18 +510,7 @@ func (s *daemonSuite) TestStartStop(c *C) { close(generalDone) }() - untrustedDone := make(chan struct{}) - go func() { - select { - case <-untrustedAccept: - case <-time.After(2 * time.Second): - c.Fatal("untrusted listener accept was not called") - } - close(untrustedDone) - }() - <-generalDone - <-untrustedDone err = d.Stop(nil) c.Check(err, IsNil) @@ -592,9 +525,6 @@ func (s *daemonSuite) TestRestartWiring(c *C) { generalAccept := make(chan struct{}) d.generalListener = &witnessAcceptListener{Listener: l, accept: generalAccept} - untrustedAccept := make(chan struct{}) - d.untrustedListener = &witnessAcceptListener{Listener: l, accept: untrustedAccept} - c.Assert(d.Start(), IsNil) defer d.Stop(nil) @@ -608,18 +538,7 @@ func (s *daemonSuite) TestRestartWiring(c *C) { close(generalDone) }() - untrustedDone := make(chan struct{}) - go func() { - select { - case <-untrustedAccept: - case <-time.After(2 * time.Second): - c.Fatal("untrusted accept was not called") - } - close(untrustedDone) - }() - <-generalDone - <-untrustedDone st := d.overlord.State() st.Lock() @@ -652,16 +571,10 @@ func (s *daemonSuite) TestGracefulStop(c *C) { generalL, err := net.Listen("tcp", "127.0.0.1:0") c.Assert(err, IsNil) - untrustedL, err := net.Listen("tcp", "127.0.0.1:0") - c.Assert(err, IsNil) - generalAccept := make(chan struct{}) generalClosed := make(chan struct{}) d.generalListener = &witnessAcceptListener{Listener: generalL, accept: generalAccept, closed: generalClosed} - untrustedAccept := make(chan struct{}) - d.untrustedListener = &witnessAcceptListener{Listener: untrustedL, accept: untrustedAccept} - c.Assert(d.Start(), IsNil) generalAccepting := make(chan struct{}) @@ -674,18 +587,7 @@ func (s *daemonSuite) TestGracefulStop(c *C) { close(generalAccepting) }() - untrustedAccepting := make(chan struct{}) - go func() { - select { - case <-untrustedAccept: - case <-time.After(2 * time.Second): - c.Fatal("general accept was not called") - } - close(untrustedAccepting) - }() - <-generalAccepting - <-untrustedAccepting alright := make(chan struct{}) @@ -726,9 +628,6 @@ func (s *daemonSuite) TestRestartSystemWiring(c *C) { generalAccept := make(chan struct{}) d.generalListener = &witnessAcceptListener{Listener: l, accept: generalAccept} - untrustedAccept := make(chan struct{}) - d.untrustedListener = &witnessAcceptListener{Listener: l, accept: untrustedAccept} - c.Assert(d.Start(), IsNil) defer d.Stop(nil) @@ -744,18 +643,7 @@ func (s *daemonSuite) TestRestartSystemWiring(c *C) { close(generalDone) }() - untrustedDone := make(chan struct{}) - go func() { - select { - case <-untrustedAccept: - case <-time.After(2 * time.Second): - c.Fatal("untrusted accept was not called") - } - close(untrustedDone) - }() - <-generalDone - <-untrustedDone oldRebootNoticeWait := rebootNoticeWait oldRebootWaitTimeout := rebootWaitTimeout @@ -849,15 +737,9 @@ func makeDaemonListeners(c *C, d *Daemon) { generalL, err := net.Listen("tcp", "127.0.0.1:0") c.Assert(err, IsNil) - untrustedL, err := net.Listen("tcp", "127.0.0.1:0") - c.Assert(err, IsNil) - generalAccept := make(chan struct{}) generalClosed := make(chan struct{}) d.generalListener = &witnessAcceptListener{Listener: generalL, accept: generalAccept, closed: generalClosed} - - untrustedAccept := make(chan struct{}) - d.untrustedListener = &witnessAcceptListener{Listener: untrustedL, accept: untrustedAccept} } // This test tests that when a restart of the system is called