Skip to content

Commit

Permalink
chore(daemon): Remove untrusted socket
Browse files Browse the repository at this point in the history
  • Loading branch information
@thp-canonical
thp-canonical committed Feb 15, 2024
1 parent cf052a4 commit c2bf3dd
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 173 deletions.
80 changes: 25 additions & 55 deletions internals/daemon/daemon.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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")
}

Expand All @@ -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
Expand Down Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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{
Expand Down
118 changes: 0 additions & 118 deletions internals/daemon/daemon_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)

Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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{})
Expand All @@ -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)
Expand All @@ -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)

Expand All @@ -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()
Expand Down Expand Up @@ -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{})
Expand All @@ -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{})

Expand Down Expand Up @@ -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)

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit c2bf3dd

Please sign in to comment.