CVE-2018-6558: Insufficient Information (NVD-CWE-noinfo)
value: The pam_fscrypt module in fscrypt before 0.2.4 may incorrectly restore primary and supplementary group IDs to the values associated with the root user, which allows attackers to gain privileges via a successful login through certain applications that use Linux-PAM (aka pam).
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 6.5
Published: 2018-08-23T19:29Z
Repository: https://github.com/google/fscrypt
Language: Go
Files changed: security/keyring.go, security/privileges.go
Commit date: 2018-08-23 11:00:34 -0700
Commit message: Ensure keyring privilege changes are reversible

This change makes sure that, when we set the ruid and euid in order to
get the user keyring linked into the current process keyring, we will
always be able to reverse these changes (using a suid of 0).

This fixes an issue where "su <user>" would result in a system error
when called by an unprivileged user. It also explains exactly how and
why we are making these privilege changes.

Diff: diff --git a/security/keyring.go b/security/keyring.go
index ab65631..c4603bc 100644
--- a/security/keyring.go
+++ b/security/keyring.go
@@ -20,11 +20,10 @@
 package security
 
 import (
 	"fmt"
 	"log"
-	"os"
 	"os/user"
 	"sync"
 
 	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
@@ -136,46 +135,51 @@ func UserKeyringID(target *user.User, checkSession bool) (int, error) {
 		}
 	}
 	return targetKeyring, nil
 }
 
-func userKeyringIDLookup(uid int) (int, error) {
+func userKeyringIDLookup(uid int) (keyringID int, err error) {
 	cacheLock.Lock()
 	defer cacheLock.Unlock()
-	if keyringID, ok := keyringIDCache[uid]; ok {
-		return keyringID, nil
-	}
-
-	ruid, euid := os.Getuid(), os.Geteuid()
-	// If all the ids do not agree, we will have to change them
-	needSetUids := uid != ruid || uid != euid
-
-	// The value of KEY_SPEC_USER_KEYRING is determined by the real uid, so
-	// we must set the ruid appropriately. Note that this will also trigger
-	// the creation of the uid keyring if it does not yet exist.
-	if needSetUids {
-		defer setUids(ruid, euid) // Always reset privileges
-		if err := setUids(uid, 0); err != nil {
-			return 0, err
+	var ok bool
+	if keyringID, ok = keyringIDCache[uid]; ok {
+		return
+	}
+
+	// Our goals here are to:
+	//    - Find the user keyring (for the provided uid)
+	//    - Link it into the current process keyring (so we can use it)
+	//    - Make no permenant changes to the process privileges
+	// Complicating this are the facts that:
+	//    - The value of KEY_SPEC_USER_KEYRING is determined by the ruid
+	//    - Keyring linking permissions use the euid
+	// So we have to change both the ruid and euid to make this work,
+	// setting the suid to 0 so that we can later switch back.
+	ruid, euid, suid := getUids()
+	if ruid != uid || euid != uid {
+		if err = setUids(uid, uid, 0); err != nil {
+			return
 		}
+		defer func() {
+			resetErr := setUids(ruid, euid, suid)
+			if resetErr != nil {
+				err = resetErr
+			}
+		}()
 	}
-	keyringID, err := unix.KeyctlGetKeyringID(unix.KEY_SPEC_USER_KEYRING, true)
+
+	// We get the value of KEY_SPEC_USER_KEYRING. Note that this will also
+	// trigger the creation of the uid keyring if it does not yet exist.
+	keyringID, err = unix.KeyctlGetKeyringID(unix.KEY_SPEC_USER_KEYRING, true)
 	log.Printf("keyringID(_uid.%d) = %d, %v", uid, keyringID, err)
 	if err != nil {
 		return 0, err
 	}
 
-	// We still want to use this key after our privileges are reset. If we
-	// link the key into the process keyring, we will possess it and still
-	// be able to use it. However, the permissions to link are based on the
-	// effective uid, so we must set the euid appropriately.
-	if needSetUids {
-		if err := setUids(0, uid); err != nil {
-			return 0, err
-		}
-	}
-	if err := keyringLink(keyringID, unix.KEY_SPEC_PROCESS_KEYRING); err != nil {
+	// We still want to use this keyring after our privileges are reset. So
+	// we link it into the process keyring, preventing a loss of access.
+	if err = keyringLink(keyringID, unix.KEY_SPEC_PROCESS_KEYRING); err != nil {
 		return 0, err
 	}
 
 	keyringIDCache[uid] = keyringID
 	return keyringID, nil
diff --git a/security/privileges.go b/security/privileges.go
index eaee808..c9bfde7 100644
--- a/security/privileges.go
+++ b/security/privileges.go
@@ -140,13 +140,24 @@ func SetProcessPrivileges(privs *Privileges) error {
 	}
 	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
+func setUids(ruid, euid, suid int) error {
+	log.Printf("Setting ruid=%d euid=%d suid=%d", ruid, euid, suid)
+	// We elevate the all the privs before setting them. This prevents
+	// issues with (ruid=1000,euid=1000,suid=0), where just a single call
+	// to setresuid might fail with permission denied.
+	if res, err := C.setresuid(0, 0, 0); res < 0 {
+		return errors.Wrapf(err.(syscall.Errno), "setting uids")
 	}
-	return errors.Wrapf(err.(syscall.Errno), "setting uids")
+	if res, err := C.setresuid(C.uid_t(ruid), C.uid_t(euid), C.uid_t(suid)); res < 0 {
+		return errors.Wrapf(err.(syscall.Errno), "setting uids")
+	}
+	return nil
+}
+
+func getUids() (int, int, int) {
+	var ruid, euid, suid C.uid_t
+	C.getresuid(&ruid, &euid, &suid)
+	return int(ruid), int(euid), int(suid)
 }

Vulnerable code: 	"os"
func userKeyringIDLookup(uid int) (int, error) {
	if keyringID, ok := keyringIDCache[uid]; ok {
		return keyringID, nil
	}

	ruid, euid := os.Getuid(), os.Geteuid()
	// If all the ids do not agree, we will have to change them
	needSetUids := uid != ruid || uid != euid

	// The value of KEY_SPEC_USER_KEYRING is determined by the real uid, so
	// we must set the ruid appropriately. Note that this will also trigger
	// the creation of the uid keyring if it does not yet exist.
	if needSetUids {
		defer setUids(ruid, euid) // Always reset privileges
		if err := setUids(uid, 0); err != nil {
			return 0, err
	keyringID, err := unix.KeyctlGetKeyringID(unix.KEY_SPEC_USER_KEYRING, true)
	// We still want to use this key after our privileges are reset. If we
	// link the key into the process keyring, we will possess it and still
	// be able to use it. However, the permissions to link are based on the
	// effective uid, so we must set the euid appropriately.
	if needSetUids {
		if err := setUids(0, uid); err != nil {
			return 0, err
		}
	}
	if err := keyringLink(keyringID, unix.KEY_SPEC_PROCESS_KEYRING); err != 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 uids")
Fixed code: func userKeyringIDLookup(uid int) (keyringID int, err error) {
	var ok bool
	if keyringID, ok = keyringIDCache[uid]; ok {
		return
	}

	// Our goals here are to:
	//    - Find the user keyring (for the provided uid)
	//    - Link it into the current process keyring (so we can use it)
	//    - Make no permenant changes to the process privileges
	// Complicating this are the facts that:
	//    - The value of KEY_SPEC_USER_KEYRING is determined by the ruid
	//    - Keyring linking permissions use the euid
	// So we have to change both the ruid and euid to make this work,
	// setting the suid to 0 so that we can later switch back.
	ruid, euid, suid := getUids()
	if ruid != uid || euid != uid {
		if err = setUids(uid, uid, 0); err != nil {
			return
		defer func() {
			resetErr := setUids(ruid, euid, suid)
			if resetErr != nil {
				err = resetErr
			}
		}()

	// We get the value of KEY_SPEC_USER_KEYRING. Note that this will also
	// trigger the creation of the uid keyring if it does not yet exist.
	keyringID, err = unix.KeyctlGetKeyringID(unix.KEY_SPEC_USER_KEYRING, true)
	// We still want to use this keyring after our privileges are reset. So
	// we link it into the process keyring, preventing a loss of access.
	if err = keyringLink(keyringID, unix.KEY_SPEC_PROCESS_KEYRING); err != nil {
func setUids(ruid, euid, suid int) error {
	log.Printf("Setting ruid=%d euid=%d suid=%d", ruid, euid, suid)
	// We elevate the all the privs before setting them. This prevents
	// issues with (ruid=1000,euid=1000,suid=0), where just a single call
	// to setresuid might fail with permission denied.
	if res, err := C.setresuid(0, 0, 0); res < 0 {
		return errors.Wrapf(err.(syscall.Errno), "setting uids")
	if res, err := C.setresuid(C.uid_t(ruid), C.uid_t(euid), C.uid_t(suid)); res < 0 {
		return errors.Wrapf(err.(syscall.Errno), "setting uids")
	}
	return nil
}

func getUids() (int, int, int) {
	var ruid, euid, suid C.uid_t
	C.getresuid(&ruid, &euid, &suid)
	return int(ruid), int(euid), int(suid)