daemon, polkit: update based on John Lenton's review

* rename polkit.CheckAuthorization to CheckAuthorizationForPid, and make
  it return (bool, error).  If we need to check other subject types in
  future, we can add extra entry points.

* If the response says the user dismissed the auth dialog, return
  ErrDismissed.  In daemon, don't log failures due to this error.

* If the response says a challenge is required, rewturn
  ErrInteractionRequired.

* Use uint32 to represent process IDs: this is what is used at the D-Bus
  level, and even 64-bit Linux limits IDs to 2^22.  This leads to fewer
  casts.

daemon/api_test.go

@@ -4547,7 +4547,7 @@ func (s *postCreateUserSuite) SetUpTest(c *check.C) {
 	s.apiBaseSuite.SetUpTest(c)
 
 	s.daemon(c)
-	postCreateUserUcrednetGet = func(string) (uint64, uint32, error) {
+	postCreateUserUcrednetGet = func(string) (uint32, uint32, error) {
 		return 100, 0, nil
 	}
 	s.mockUserHome = c.MkDir()
@@ -4907,7 +4907,7 @@ func (s *postCreateUserSuite) TestPostCreateUserFromAssertionAllKnownClassicErro
 
 	s.makeSystemUsers(c, []map[string]interface{}{goodUser})
 
-	postCreateUserUcrednetGet = func(string) (uint64, uint32, error) {
+	postCreateUserUcrednetGet = func(string) (uint32, uint32, error) {
 		return 100, 0, nil
 	}
 	defer func() {

daemon/daemon.go

@@ -84,7 +84,7 @@ type Command struct {
 	d *Daemon
 }
 
-var polkitCheckAuthorization = polkit.CheckAuthorization
+var polkitCheckAuthorizationForPid = polkit.CheckAuthorizationForPid
 
 func (c *Command) canAccess(r *http.Request, user *auth.UserState) bool {
 	if user != nil {
@@ -101,13 +101,12 @@ func (c *Command) canAccess(r *http.Request, user *auth.UserState) bool {
 		}
 
 		if c.ActionID != "" {
-			subject := polkit.ProcessSubject{Pid: int(pid)}
-			if result, err := polkitCheckAuthorization(subject, c.ActionID, nil, polkit.CheckAuthorizationAllowUserInteraction); err == nil {
-				if result.IsAuthorized {
+			if authorized, err := polkitCheckAuthorizationForPid(pid, c.ActionID, nil, polkit.CheckAuthorizationAllowUserInteraction); err == nil {
+				if authorized {
 					// polkit says user is authorised
 					return true
 				}
-			} else {
+			} else if err != polkit.ErrDismissed {
 				logger.Noticef("polkit error: %s", err)
 			}
 		}

daemon/daemon_test.go

@@ -47,19 +47,19 @@ import (
 func Test(t *testing.T) { check.TestingT(t) }
 
 type daemonSuite struct {
-	authResult polkit.AuthorizationResult
+	authorized bool
 	err        error
 }
 
 var _ = check.Suite(&daemonSuite{})
 
-func (s *daemonSuite) checkAuthorization(subject polkit.Subject, actionId string, details map[string]string, flags polkit.CheckAuthorizationFlags) (polkit.AuthorizationResult, error) {
-	return s.authResult, s.err
+func (s *daemonSuite) checkAuthorizationForPid(pid uint32, actionId string, details map[string]string, flags polkit.CheckAuthorizationFlags) (bool, error) {
+	return s.authorized, s.err
 }
 
 func (s *daemonSuite) SetUpSuite(c *check.C) {
 	snapstate.CanAutoRefresh = nil
-	polkitCheckAuthorization = s.checkAuthorization
+	polkitCheckAuthorizationForPid = s.checkAuthorizationForPid
 }
 
 func (s *daemonSuite) SetUpTest(c *check.C) {
@@ -70,12 +70,12 @@ func (s *daemonSuite) SetUpTest(c *check.C) {
 
 func (s *daemonSuite) TearDownTest(c *check.C) {
 	dirs.SetRootDir("")
-	s.authResult = polkit.AuthorizationResult{}
+	s.authorized = false
 	s.err = nil
 }
 
 func (s *daemonSuite) TearDownSuite(c *check.C) {
-	polkitCheckAuthorization = polkit.CheckAuthorization
+	polkitCheckAuthorizationForPid = polkit.CheckAuthorizationForPid
 }
 
 // build a new daemon, with only a little of Init(), suitable for the tests
@@ -222,11 +222,11 @@ func (s *daemonSuite) TestPolkitAccess(c *check.C) {
 	cmd := &Command{d: newTestDaemon(c), ActionID: "polkit.action"}
 
 	// polkit says user is not authorised
-	s.authResult.IsAuthorized = false
+	s.authorized = false
 	c.Check(cmd.canAccess(put, nil), check.Equals, false)
 
 	// polkit grants authorisation
-	s.authResult.IsAuthorized = true
+	s.authorized = true
 	c.Check(cmd.canAccess(put, nil), check.Equals, true)
 
 	// an error occurs communicating with polkit

daemon/ucrednet.go

@@ -31,18 +31,18 @@ import (
 var errNoID = errors.New("no pid/uid found")
 
 const (
-	ucrednetNoProcess = uint64(0)
+	ucrednetNoProcess = uint32(0)
 	ucrednetNobody    = uint32((1 << 32) - 1)
 )
 
-func ucrednetGet(remoteAddr string) (pid uint64, uid uint32, err error) {
+func ucrednetGet(remoteAddr string) (pid uint32, uid uint32, err error) {
 	pid = ucrednetNoProcess
 	uid = ucrednetNobody
 	for _, token := range strings.Split(remoteAddr, ";") {
 		var v uint64
 		if strings.HasPrefix(token, "pid=") {
-			if v, err = strconv.ParseUint(token[4:], 10, 64); err == nil {
-				pid = v
+			if v, err = strconv.ParseUint(token[4:], 10, 32); err == nil {
+				pid = uint32(v)
 			} else {
 				break
 			}

daemon/ucrednet_test.go

@@ -75,7 +75,7 @@ func (s *ucrednetSuite) TestAcceptConnRemoteAddrString(c *check.C) {
 	remoteAddr := conn.RemoteAddr().String()
 	c.Check(remoteAddr, check.Matches, "pid=100;uid=42;.*")
 	pid, uid, err := ucrednetGet(remoteAddr)
-	c.Check(pid, check.Equals, uint64(100))
+	c.Check(pid, check.Equals, uint32(100))
 	c.Check(uid, check.Equals, uint32(42))
 	c.Check(err, check.IsNil)
 }
@@ -146,14 +146,14 @@ func (s *ucrednetSuite) TestUcredErrors(c *check.C) {
 func (s *ucrednetSuite) TestGetNoUid(c *check.C) {
 	pid, uid, err := ucrednetGet("pid=100;uid=;")
 	c.Check(err, check.Equals, errNoID)
-	c.Check(pid, check.Equals, uint64(100))
+	c.Check(pid, check.Equals, uint32(100))
 	c.Check(uid, check.Equals, ucrednetNobody)
 }
 
 func (s *ucrednetSuite) TestGetBadUid(c *check.C) {
 	pid, uid, err := ucrednetGet("pid=100;uid=hello;")
 	c.Check(err, check.NotNil)
-	c.Check(pid, check.Equals, uint64(100))
+	c.Check(pid, check.Equals, uint32(100))
 	c.Check(uid, check.Equals, ucrednetNobody)
 }
 
@@ -174,6 +174,6 @@ func (s *ucrednetSuite) TestGetNothing(c *check.C) {
 func (s *ucrednetSuite) TestGet(c *check.C) {
 	pid, uid, err := ucrednetGet("pid=100;uid=42;")
 	c.Check(err, check.IsNil)
-	c.Check(pid, check.Equals, uint64(100))
+	c.Check(pid, check.Equals, uint32(100))
 	c.Check(uid, check.Equals, uint32(42))
 }

polkit/authority.go

@@ -20,6 +20,8 @@
 package polkit
 
 import (
+	"errors"
+
 	"github.com/godbus/dbus"
 )
 
@@ -30,54 +32,55 @@ const (
 	CheckAuthorizationAllowUserInteraction CheckAuthorizationFlags = 0x01
 )
 
-// CheckAuthorization queries polkit to determine whether a subject is
-// authorized to perform an action.
-func CheckAuthorization(subject Subject, actionId string, details map[string]string, flags CheckAuthorizationFlags) (result AuthorizationResult, err error) {
-	s, err := subject.serialize()
-	if err != nil {
-		return
-	}
+var (
+	ErrDismissed           = errors.New("Authorization request dismissed")
+	ErrInteractionRequired = errors.New("Authorization requires interaction")
+)
+
+func checkAuthorization(subject subject, actionId string, details map[string]string, flags CheckAuthorizationFlags) (bool, error) {
 	bus, err := dbus.SystemBus()
 	if err != nil {
-		return
+		return false, err
 	}
 	authority := bus.Object("org.freedesktop.PolicyKit1",
 		"/org/freedesktop/PolicyKit1/Authority")
 
+	var result authorizationResult
 	err = authority.Call(
 		"org.freedesktop.PolicyKit1.Authority.CheckAuthorization", 0,
-		s, actionId, details, flags, "").Store(&result)
-	return
-}
-
-type serializedSubject struct {
-	Kind    string
-	Details map[string]dbus.Variant
-}
-
-type Subject interface {
-	serialize() (serializedSubject, error)
+		subject, actionId, details, flags, "").Store(&result)
+	if err != nil && !result.IsAuthorized {
+		if result.IsChallenge {
+			err = ErrInteractionRequired
+		} else if result.Details["polkit.dismissed"] != "" {
+			err = ErrDismissed
+		}
+	}
+	return result.IsAuthorized, err
 }
 
-type ProcessSubject struct {
-	Pid       int
-	StartTime uint64
+// CheckAuthorizationForPid queries polkit to determine whether a process is
+// authorized to perform an action.
+func CheckAuthorizationForPid(pid uint32, actionId string, details map[string]string, flags CheckAuthorizationFlags) (bool, error) {
+	subject := subject{
+		Kind:    "unix-process",
+		Details: make(map[string]dbus.Variant),
+	}
+	subject.Details["pid"] = dbus.MakeVariant(pid)
+	if startTime, err := getStartTimeForPid(pid); err == nil {
+		subject.Details["start-time"] = dbus.MakeVariant(startTime)
+	} else {
+		return false, err
+	}
+	return checkAuthorization(subject, actionId, details, flags)
 }
 
-func (s ProcessSubject) serialize() (serializedSubject, error) {
-	details := make(map[string]dbus.Variant)
-	details["pid"] = dbus.MakeVariant(uint32(s.Pid))
-	if s.StartTime == 0 {
-		var err error
-		if s.StartTime, err = getStartTimeForPid(s.Pid); err != nil {
-			return serializedSubject{}, err
-		}
-	}
-	details["start-time"] = dbus.MakeVariant(s.StartTime)
-	return serializedSubject{"unix-process", details}, nil
+type subject struct {
+	Kind    string
+	Details map[string]dbus.Variant
 }
 
-type AuthorizationResult struct {
+type authorizationResult struct {
 	IsAuthorized bool
 	IsChallenge  bool
 	Details      map[string]string

polkit/pid_start_time.go

@@ -32,7 +32,7 @@ import (
 //
 // The implementation is intended to be compatible with polkit:
 //    https://cgit.freedesktop.org/polkit/tree/src/polkit/polkitunixprocess.c
-func getStartTimeForPid(pid int) (uint64, error) {
+func getStartTimeForPid(pid uint32) (uint64, error) {
 	filename := fmt.Sprintf("/proc/%d/stat", pid)
 	file, err := os.Open(filename)
 	if err != nil {