From 41b2c91fd3c33d9b852ee715ca6254e468a5d564 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 | 30 +++++----- security/privileges.go | 126 +++++++++++++++++++---------------------- 2 files changed, 72 insertions(+), 84 deletions(-) diff --git a/pam/pam.go b/pam/pam.go index ba254c8..04d9026 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,14 +133,21 @@ 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. 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 24cd345..e364d88 100644 --- a/security/privileges.go +++ b/security/privileges.go @@ -44,29 +44,14 @@ 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 +#include // NGROUPS_MAX */ import "C" import ( "log" - "os" "os/user" "syscall" @@ -75,72 +60,75 @@ 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() + groups := make([]C.gid_t, C.NGROUPS_MAX) + n, err := C.getgroups(C.int(len(groups)), &groups[0]) + if n < 0 { + return nil, err + } + return &Privileges{euid, egid, groups[:n]}, 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 by first saving the output +// of ProcessPrivileges, calling SetProcessPrivileges with the desired privs, +// then finally 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") } - if err := setGroups(target); err != nil { - return err + if res, err := C.setgroups(C.size_t(len(privs.groups)), &privs.groups[0]); 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 + if res, err := C.setreuid(C.uid_t(ruid), C.uid_t(euid)); res < 0 { + return errors.Wrapf(err.(syscall.Errno), "setting uids") } - 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)) - } - 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