Ensure setting user privileges is reversible

This change makes sure after dropping then elevating privileges for a
process, the euid, guid, and groups are all the same as they were
originally. This significantly simplifies the privilege logic.

This fixes CVE-2018-6558, which allowed an unprivleged user to gain
membership in the root group (gid 0) due to the groups not being
properly reset in the process.

diff --git a/pam/pam.go b/pam/pam.go
index ba254c8e71de9ef37b71d13662ef55f365ce222a..c48dd134726d7777805bcb0aa587000aeac2ba46 100644 (file)
--- a/pam/pam.go
+++ b/pam/pam.go
@@ -35,16 +35,14 @@ import (
        "unsafe"
 
        "github.com/google/fscrypt/security"
-       "github.com/google/fscrypt/util"
 )
 
 // Handle wraps the C pam_handle_t type. This is used from within modules.
 type Handle struct {
-       handle *C.pam_handle_t
-       status C.int
-       // OrigUser is the user who invoked the PAM module (usually root)
-       OrigUser *user.User
-       // PamUser is the user who the PAM module is for
+       handle    *C.pam_handle_t
+       status    C.int
+       origPrivs *security.Privileges
+       // PamUser is the user for whom the PAM module is running.
        PamUser *user.User
 }
 
@@ -62,13 +60,8 @@ func NewHandle(pamh unsafe.Pointer) (*Handle, error) {
                return nil, err
        }
 
-       if h.PamUser, err = user.Lookup(C.GoString(pamUsername)); err != nil {
-               return nil, err
-       }
-       if h.OrigUser, err = util.EffectiveUser(); err != nil {
-               return nil, err
-       }
-       return h, nil
+       h.PamUser, err = user.Lookup(C.GoString(pamUsername))
+       return h, err
 }
 
 func (h *Handle) setData(name string, data unsafe.Pointer, cleanup C.CleanupFunc) error {
@@ -140,14 +133,20 @@ func (h *Handle) StartAsPamUser() error {
        if _, err := security.UserKeyringID(h.PamUser, true); err != nil {
                log.Printf("Setting up keyrings in PAM: %v", err)
        }
-       return security.SetProcessPrivileges(h.PamUser)
+       userPrivs, err := security.UserPrivileges(h.PamUser)
+       if err != nil {
+               return err
+       }
+       if h.origPrivs, err = security.ProcessPrivileges(); err != nil {
+               return err
+       }
+       return security.SetProcessPrivileges(userPrivs)
 }
 
 // StopAsPamUser restores the original privileges that were running the
-// PAM module (this is usually root). As this error is often ignored in a defer
-// statement, any error is also logged.
+// PAM module (this is usually root).
 func (h *Handle) StopAsPamUser() error {
-       err := security.SetProcessPrivileges(h.OrigUser)
+       err := security.SetProcessPrivileges(h.origPrivs)
        if err != nil {
                log.Print(err)
        }

diff --git a/security/privileges.go b/security/privileges.go
index 24cd345664ae809a9e53ec975abc34119337022c..eaee808235e74cc9588578f6718a5b23f4ed8951 100644 (file)
--- a/security/privileges.go
+++ b/security/privileges.go
@@ -43,30 +43,15 @@ package security
 // cgo maps them to different Go types.
 
 /*
+#define _GNU_SOURCE    // for getresuid and setresuid
 #include <sys/types.h>
-#include <unistd.h>    // setreuid, setregid
-#include <grp.h>       // setgroups
-
-static int my_setreuid(uid_t ruid, uid_t euid)
-{
-       return setreuid(ruid, euid);
-}
-
-static int my_setregid(gid_t rgid, gid_t egid)
-{
-       return setregid(rgid, egid);
-}
-
-static int my_setgroups(size_t size, const gid_t *list)
-{
-       return setgroups(size, list);
-}
+#include <unistd.h>    // getting and setting uids and gids
+#include <grp.h>       // setgroups
 */
 import "C"
 
 import (
        "log"
-       "os"
        "os/user"
        "syscall"
 
@@ -75,72 +60,93 @@ import (
        "github.com/google/fscrypt/util"
 )
 
-// SetProcessPrivileges temporarily drops the privileges of the current process
-// to have the effective uid/gid of the target user. The privileges can be
-// changed again with another call to SetProcessPrivileges.
-func SetProcessPrivileges(target *user.User) error {
-       euid := util.AtoiOrPanic(target.Uid)
-       egid := util.AtoiOrPanic(target.Gid)
-       if os.Geteuid() == euid {
-               log.Printf("Privileges already set to %q", target.Username)
-               return nil
-       }
-       log.Printf("Setting privileges to %q", target.Username)
+// Privileges encapulate the effective uid/gid and groups of a process.
+type Privileges struct {
+       euid   C.uid_t
+       egid   C.gid_t
+       groups []C.gid_t
+}
 
-       // If setting privs to root, we want to set the uid first, so we will
-       // then have the necessary permissions to perform the other actions.
-       if euid == 0 {
-               if err := setUids(-1, euid); err != nil {
-                       return err
-               }
-       }
-       if err := setGids(-1, egid); err != nil {
-               return err
-       }
-       if err := setGroups(target); err != nil {
-               return err
+// ProcessPrivileges returns the process's current effective privileges.
+func ProcessPrivileges() (*Privileges, error) {
+       ruid := C.getuid()
+       euid := C.geteuid()
+       rgid := C.getgid()
+       egid := C.getegid()
+
+       var groups []C.gid_t
+       n, err := C.getgroups(0, nil)
+       if n < 0 {
+               return nil, err
        }
-       // If not setting privs to root, we want to avoid dropping the uid
-       // util the very end.
-       if euid != 0 {
-               if err := setUids(-1, euid); err != nil {
-                       return err
+       // If n == 0, the user isn't in any groups, so groups == nil is fine.
+       if n > 0 {
+               groups = make([]C.gid_t, n)
+               n, err = C.getgroups(n, &groups[0])
+               if n < 0 {
+                       return nil, err
                }
+               groups = groups[:n]
        }
-       return nil
+       log.Printf("Current privs (real, effective): uid=(%d,%d) gid=(%d,%d) groups=%v",
+               ruid, euid, rgid, egid, groups)
+       return &Privileges{euid, egid, groups}, nil
 }
 
-func setUids(ruid, euid int) error {
-       res, err := C.my_setreuid(C.uid_t(ruid), C.uid_t(euid))
-       log.Printf("setreuid(%d, %d) = %d (errno %v)", ruid, euid, res, err)
-       if res == 0 {
-               return nil
+// UserPrivileges returns the defualt privileges for the specified user.
+func UserPrivileges(user *user.User) (*Privileges, error) {
+       privs := &Privileges{
+               euid: C.uid_t(util.AtoiOrPanic(user.Uid)),
+               egid: C.gid_t(util.AtoiOrPanic(user.Gid)),
        }
-       return errors.Wrapf(err.(syscall.Errno), "setting uids")
+       userGroups, err := user.GroupIds()
+       if err != nil {
+               return nil, util.SystemError(err.Error())
+       }
+       privs.groups = make([]C.gid_t, len(userGroups))
+       for i, group := range userGroups {
+               privs.groups[i] = C.gid_t(util.AtoiOrPanic(group))
+       }
+       return privs, nil
 }
 
-func setGids(rgid, egid int) error {
-       res, err := C.my_setregid(C.gid_t(rgid), C.gid_t(egid))
-       log.Printf("setregid(%d, %d) = %d (errno %v)", rgid, egid, res, err)
-       if res == 0 {
-               return nil
+// SetProcessPrivileges sets the privileges of the current process to have those
+// specified by privs. The original privileges can be obtained by first saving
+// the output of ProcessPrivileges, calling SetProcessPrivileges with the
+// desired privs, then calling SetProcessPrivileges with the saved privs.
+func SetProcessPrivileges(privs *Privileges) error {
+       log.Printf("Setting euid=%d egid=%d groups=%v", privs.euid, privs.egid, privs.groups)
+
+       // If setting privs as root, we need to set the euid to 0 first, so that
+       // we will have the necessary permissions to make the other changes to
+       // the groups/egid/euid, regardless of our original euid.
+       C.seteuid(0)
+
+       // Seperately handle the case where the user is in no groups.
+       numGroups := C.size_t(len(privs.groups))
+       groupsPtr := (*C.gid_t)(nil)
+       if numGroups > 0 {
+               groupsPtr = &privs.groups[0]
        }
-       return errors.Wrapf(err.(syscall.Errno), "setting gids")
-}
 
-func setGroups(target *user.User) error {
-       groupStrings, err := target.GroupIds()
-       if err != nil {
-               return util.SystemError(err.Error())
+       if res, err := C.setgroups(numGroups, groupsPtr); res < 0 {
+               return errors.Wrapf(err.(syscall.Errno), "setting groups")
+       }
+       if res, err := C.setegid(privs.egid); res < 0 {
+               return errors.Wrapf(err.(syscall.Errno), "setting egid")
        }
-       gids := make([]C.gid_t, len(groupStrings))
-       for i, groupString := range groupStrings {
-               gids[i] = C.gid_t(util.AtoiOrPanic(groupString))
+       if res, err := C.seteuid(privs.euid); res < 0 {
+               return errors.Wrapf(err.(syscall.Errno), "setting euid")
        }
-       res, err := C.my_setgroups(C.size_t(len(groupStrings)), &gids[0])
-       log.Printf("setgroups(%v) = %d (errno %v)", gids, res, err)
+       ProcessPrivileges()
+       return nil
+}
+
+func setUids(ruid, euid int) error {
+       res, err := C.setreuid(C.uid_t(ruid), C.uid_t(euid))
+       log.Printf("setreuid(%d, %d) = %d (errno %v)", ruid, euid, res, err)
        if res == 0 {
                return nil
        }
-       return errors.Wrapf(err.(syscall.Errno), "setting groups")
+       return errors.Wrapf(err.(syscall.Errno), "setting uids")
 }