From d1936790a9c023f7751aabba5b79684185f4f527 Mon Sep 17 00:00:00 2001 From: "Joe Richey joerichey@google.com" Date: Fri, 17 Aug 2018 20:26:27 -0700 Subject: [PATCH] Cleanup privilege dropping code 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. We still need to keep the 3 calls to setUids() in userKeyringIDLookup() in order to work around flaws in the kernel keyring API. --- pam/pam.go | 37 +++++------ security/privileges.go | 143 +++++++++++++++++++++-------------------- 2 files changed, 90 insertions(+), 90 deletions(-) diff --git a/pam/pam.go b/pam/pam.go index ba254c8..479d082 100644 --- 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,18 +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) - if err != nil { - log.Print(err) - } - return err + return security.SetProcessPrivileges(h.origPrivs) } func (h *Handle) err() error { diff --git a/security/privileges.go b/security/privileges.go index 24cd345..1f7bd36 100644 --- a/security/privileges.go +++ b/security/privileges.go @@ -44,29 +44,13 @@ package security /* #include -#include // setreuid, setregid -#include // 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 // setreuid, setregid +#include // setgroups */ import "C" import ( "log" - "os" "os/user" "syscall" @@ -75,72 +59,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 +// 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 +} + +// ProcessPrivileges returns the process's current effective privileges. +func ProcessPrivileges() (*Privileges, error) { + euid := C.geteuid() + egid := C.getegid() + + var groups []C.gid_t + n, err := C.getgroups(0, nil) + if n < 0 { + return nil, 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 &Privileges{euid, egid, groups}, 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)), + } + userGroups, err := user.GroupIds() + if err != nil { + return nil, util.SystemError(err.Error()) } - log.Printf("Setting privileges to %q", target.Username) + 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 +} + +// 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 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 privs.euid == 0 { + if res, err := C.seteuid(privs.euid); res < 0 { + return errors.Wrapf(err.(syscall.Errno), "setting euid") } } - if err := setGids(-1, egid); err != nil { - return err + if res, err := C.setegid(privs.egid); res < 0 { + return errors.Wrapf(err.(syscall.Errno), "setting egid") + } + + // 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] } - if err := setGroups(target); err != nil { - return err + + if res, err := C.setgroups(numGroups, groupsPtr); res < 0 { + return errors.Wrapf(err.(syscall.Errno), "setting groups") } // 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 privs.euid != 0 { + if res, err := C.seteuid(privs.euid); res < 0 { + return errors.Wrapf(err.(syscall.Errno), "setting euid") } } return 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 - } - return errors.Wrapf(err.(syscall.Errno), "setting uids") -} - -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 - } - 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()) - } - gids := make([]C.gid_t, len(groupStrings)) - for i, groupString := range groupStrings { - gids[i] = C.gid_t(util.AtoiOrPanic(groupString)) + if res, err := C.setreuid(C.uid_t(ruid), C.uid_t(euid)); res < 0 { + return errors.Wrapf(err.(syscall.Errno), "setting uids") } - res, err := C.my_setgroups(C.size_t(len(groupStrings)), &gids[0]) - log.Printf("setgroups(%v) = %d (errno %v)", gids, res, err) - if res == 0 { - return nil - } - return errors.Wrapf(err.(syscall.Errno), "setting groups") + return nil } -- 2.17.1