security: Rewrite of keryings and permissions

The keyring lookup functions no longer read from /proc/keys. Now they
simply spawn a thread, drop privs, and check with GetKeyringID and
KEY_SPEC_USER_KEYRING. See userKeyringID() for more info.

The privileges functions have also been changed. Now the concept of
setting privileges is seperate form the concept of setting up the
keyrings.

2 files changed, 162 insertions, 164 deletions

diff --git a/security/keyring.go b/security/keyring.go
index ef56364..8112c54 100644
--- a/security/keyring.go
+++ b/security/keyring.go
@@ -20,34 +20,55 @@
 package security
 
 import (
-	"bytes"
-	"fmt"
-	"io/ioutil"
 	"log"
-	"strconv"
+	"os"
+	"os/user"
+	"runtime"
+	"sync"
 
 	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
+
+	"github.com/google/fscrypt/util"
 )
 
-const (
-	// file which lists all visible keys
-	keyListFilename = "/proc/keys"
-	// keyType is always logon as required by filesystem encryption.
-	keyType = "logon"
+// KeyType is always logon as required by filesystem encryption.
+const KeyType = "logon"
+
+// Keyring related error values
+var (
+	ErrFindingKeyring = util.SystemError("could not find user keyring")
+	ErrKeyringInsert  = util.SystemError("could not insert key into the keyring")
+	ErrKeyringSearch  = errors.New("could not find key with descriptor")
+	ErrKeyringDelete  = util.SystemError("could not delete key from the keyring")
+	ErrKeyringLink    = util.SystemError("could not link keyring")
 )
 
+// KeyringsSetup configures the desired keyring linkage by linking the target
+// user's keying into the privileged user's keyring.
+func KeyringsSetup(target, privileged *user.User) error {
+	targetKeyringID, err := userKeyringID(target)
+	if err != nil {
+		return err
+	}
+	privilegedKeyringID, err := userKeyringID(privileged)
+	if err != nil {
+		return err
+	}
+	return keyringLink(targetKeyringID, privilegedKeyringID)
+}
+
 // FindKey tries to locate a key in the kernel keyring with the provided
-// description. The key id is returned if we can find the key. An error is
+// description. The key ID is returned if we can find the key. An error is
 // returned if the key does not exist.
-func FindKey(description string) (int, error) {
-	keyringID, err := getUserKeyringID()
+func FindKey(description string, target *user.User) (int, error) {
+	keyringID, err := userKeyringID(target)
 	if err != nil {
 		return 0, err
 	}
 
-	keyID, err := unix.KeyctlSearch(keyringID, keyType, description, 0)
-	log.Printf("KeyctlSearch(%d, %s, %s) = %d, %v", keyringID, keyType, description, keyID, err)
+	keyID, err := unix.KeyctlSearch(keyringID, KeyType, description, 0)
+	log.Printf("KeyctlSearch(%d, %s, %s) = %d, %v", keyringID, KeyType, description, keyID, err)
 	if err != nil {
 		return 0, errors.Wrap(ErrKeyringSearch, err.Error())
 	}
@@ -56,8 +77,8 @@ func FindKey(description string) (int, error) {
 
 // RemoveKey tries to remove a policy key from the kernel keyring with the
 // provided description. An error is returned if the key does not exist.
-func RemoveKey(description string) error {
-	keyID, err := FindKey(description)
+func RemoveKey(description string, target *user.User) error {
+	keyID, err := FindKey(description, target)
 	if err != nil {
 		return err
 	}
@@ -74,103 +95,106 @@ func RemoveKey(description string) error {
 
 // InsertKey puts the provided data into the kernel keyring with the provided
 // description.
-func InsertKey(data []byte, description string) error {
-	keyringID, err := getUserKeyringID()
+func InsertKey(data []byte, description string, target *user.User) error {
+	keyringID, err := userKeyringID(target)
 	if err != nil {
 		return err
 	}
 
-	keyID, err := unix.AddKey(keyType, description, data, keyringID)
+	keyID, err := unix.AddKey(KeyType, description, data, keyringID)
 	log.Printf("KeyctlAddKey(%s, %s, <data>, %d) = %d, %v",
-		keyType, description, keyringID, keyID, err)
+		KeyType, description, keyringID, keyID, err)
 	if err != nil {
 		return errors.Wrap(ErrKeyringInsert, err.Error())
 	}
 	return nil
 }
 
-var keyringIDCache = make(map[int]int)
+var (
+	keyringIDCache = make(map[int]int)
+	cacheLock      sync.Mutex
+)
 
-// getUserKeyringID returns the key id of the current user's user keyring. A
-// simpler approach would be to use
-//     unix.KeyctlGetKeyringID(unix.KEY_SPEC_USER_KEYRING, false)
-// which would work in almost all cases. However, despite the fact that the rest
-// of the keyrings API uses the _effective_ UID throughout, the translation of
-// KEY_SPEC_USER_KEYRING is done with respect to the _real_ UID. This means that
-// a simpler implementation would not respect permissions dropping.
-func getUserKeyringID() (int, error) {
+// userKeyringID returns the key id of the target user's keyring. The returned
+// keyring will also be linked into the process keyring so that it will be
+// accessible thoughout the program.
+func userKeyringID(target *user.User) (int, error) {
+	uid := util.AtoiOrPanic(target.Uid)
 	// We will cache the result of this function.
-	euid := unix.Geteuid()
-	if keyringID, ok := keyringIDCache[euid]; ok {
+	cacheLock.Lock()
+	defer cacheLock.Unlock()
+	if keyringID, ok := keyringIDCache[uid]; ok {
 		return keyringID, nil
 	}
 
-	data, err := ioutil.ReadFile(keyListFilename)
-	if err != nil {
-		log.Print(err)
-		return 0, ErrReadingKeyList
+	// The permissions of the keyrings API is a little strange. The euid is
+	// used to determine if we can access/modify a key/keyring. However, the
+	// ruid is used to determine KEY_SPEC_USER_KEYRING. This means both the
+	// ruid and euid must match the user's uid for the lookup to work.
+	if uid == os.Getuid() && uid == os.Geteuid() {
+		log.Printf("Normal keyring lookup for uid=%d", uid)
+		return userKeyringIDLookup(uid)
 	}
 
-	expectedName := fmt.Sprintf("_uid.%d:", euid)
-	for _, line := range bytes.Split(data, []byte{'\n'}) {
-		if len(line) == 0 {
-			continue
+	// We drop permissions in a separate thread (guaranteed as the main
+	// thread is locked) because we need to drop the real AND effective IDs.
+	log.Printf("Threaded keyring lookup for uid=%d", uid)
+	idChan := make(chan int, 0)
+	errChan := make(chan error, 0)
+	// OSThread locks ensure the privilege change is only for the lookup.
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+	go func() {
+		runtime.LockOSThread()
+		if err := SetThreadPrivileges(target, true); err != nil {
+			errChan <- err
+			return
 		}
-
-		// Each line in /proc/keys should have 9 columns.
-		columns := bytes.Fields(line)
-		if len(columns) < 9 {
-			return 0, ErrReadingKeyList
-		}
-		hexID := string(columns[0])
-		owner := string(columns[5])
-		name := string(columns[8])
-
-		// Our desired key is owned by the user and has the right name.
-		// The owner check is so another user cannot DOS this user by
-		// inserting a keyring with a similar name.
-		if owner != strconv.Itoa(euid) || name != expectedName {
-			continue
-		}
-
-		// The keyring's ID is encoded as hex.
-		parsedID, err := strconv.ParseInt(hexID, 16, 32)
+		keyringID, err := userKeyringIDLookup(uid)
 		if err != nil {
-			log.Print(err)
-			return 0, ErrReadingKeyList
+			errChan <- err
+			return
 		}
+		idChan <- keyringID
+		return
+	}()
 
-		keyringID := int(parsedID)
-		// For some stupid reason, a thread does not automaticaly "possess" keys
-		// in the user keyring. So we link it into the process keyring so that
-		// we will not get "permission denied" when purging or modifying keys.
-		if err := keyringLink(keyringID, unix.KEY_SPEC_PROCESS_KEYRING); err != nil {
-			return 0, err
+	// We select so the thread will have to complete
+	select {
+	case err := <-errChan:
+		return 0, err
+	case keyringID := <-idChan:
+		if uid == os.Getuid() && uid == os.Geteuid() {
+			log.Print("thread privileges now incorrect")
 		}
-
-		keyringIDCache[euid] = keyringID
 		return keyringID, nil
 	}
+}
+
+func userKeyringIDLookup(uid int) (int, error) {
+	// This will trigger the creation of the user keyring, if necessary.
+	keyringID, err := unix.KeyctlGetKeyringID(unix.KEY_SPEC_USER_KEYRING, false)
+	log.Printf("keyringID(_uid.%d) = %d, %v", uid, keyringID, err)
+	if err != nil {
+		return 0, errors.Wrap(ErrFindingKeyring, err.Error())
+	}
+
+	// For some silly reason, a thread does not automatically "possess" keys
+	// in the user keyring. So we link it into the process keyring so that
+	// we will not get "permission denied" when purging or modifying keys.
+	if err := keyringLink(keyringID, unix.KEY_SPEC_PROCESS_KEYRING); err != nil {
+		return 0, err
+	}
 
-	return 0, ErrFindingKeyring
+	keyringIDCache[uid] = keyringID
+	return keyringID, nil
 }
 
 func keyringLink(keyID int, keyringID int) error {
 	_, err := unix.KeyctlInt(unix.KEYCTL_LINK, keyID, keyringID, 0, 0)
 	log.Printf("KeyctlLink(%d, %d) = %v", keyID, keyringID, err)
-
 	if err != nil {
 		return errors.Wrap(ErrKeyringLink, err.Error())
 	}
-	return err
-}
-
-func keyringUnlink(keyID int, keyringID int) error {
-	_, err := unix.KeyctlInt(unix.KEYCTL_UNLINK, keyID, keyringID, 0, 0)
-	log.Printf("KeyctlUnlink(%d, %d) = %v", keyID, keyringID, err)
-
-	if err != nil {
-		return errors.Wrap(ErrKeyringUnlink, err.Error())
-	}
-	return err
+	return nil
 }
diff --git a/security/privileges.go b/security/privileges.go
index aff41a7..2a1bdae 100644
--- a/security/privileges.go
+++ b/security/privileges.go
@@ -1,5 +1,5 @@
 /*
- * privileges.go - Handles raising and dropping user privileges.
+ * privileges.go - Functions for managing users and privileges.
  *
  * Copyright 2017 Google Inc.
  * Author: Joe Richey (joerichey@google.com)
@@ -26,6 +26,7 @@ package security
 
 import (
 	"log"
+	"os/user"
 
 	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
@@ -33,101 +34,74 @@ import (
 	"github.com/google/fscrypt/util"
 )
 
-// Package security error values
-var (
-	ErrReadingKeyList = util.SystemError("could not read keys from " + keyListFilename)
-	ErrFindingKeyring = util.SystemError("could not find user keyring")
-	ErrKeyringInsert  = util.SystemError("could not insert key into the keyring")
-	ErrKeyringSearch  = errors.New("could not find key with descriptor")
-	ErrKeyringDelete  = util.SystemError("could not delete key from the keyring")
-	ErrKeyringLink    = util.SystemError("could not link keyring")
-	ErrKeyringUnlink  = util.SystemError("could not unlink keyring")
-)
-
-// Privileges contains the state needed to restore a user's original privileges.
-type Privileges struct {
-	euid   int
-	egid   int
-	groups []int
-}
-
-// DropThreadPrivileges temporarily drops the privileges of the current thread
-// to have the euid and egid specified. The returned opaque Privileges structure
-// should later be passed to RestoreThreadPrivileges.
-// Due to golang/go#1435, these privileges are only dropped for a single thread.
-// This function also makes sure that the appropriate user keyrings are linked.
-// This ensures that the user's keys are visible from commands like sudo.
-func DropThreadPrivileges(euid int, egid int) (privs *Privileges, err error) {
-	privs = &Privileges{
-		euid: unix.Geteuid(),
-		egid: unix.Getegid(),
-	}
-	if privs.groups, err = unix.Getgroups(); err != nil {
-		return nil, errors.Wrapf(err, "getting groups")
+// SetThreadPrivileges drops drops the privileges of the current thread to have
+// the uid/gid of the target user. If permanent is true, this operation cannot
+// be reversed in the thread (the real and effective IDs are set). If
+// permanent is false, only the effective IDs are set, allowing the privileges
+// to be changed again with another call to SetThreadPrivileges.
+func SetThreadPrivileges(target *user.User, permanent bool) error {
+	euid := util.AtoiOrPanic(target.Uid)
+	egid := util.AtoiOrPanic(target.Gid)
+	var ruid, rgid int
+	if permanent {
+		log.Printf("Permanently dropping to user %q", target.Username)
+		ruid, rgid = euid, egid
+	} else {
+		log.Printf("Temporarily dropping to user %q", target.Username)
+		// Real IDs of -1 mean they will not be changed.
+		ruid, rgid = -1, -1
 	}
 
-	// We link the privileged keyring into the thread keyring so that we
-	// can still modify it after dropping privileges.
-	privilegedUserKeyringID, err := getUserKeyringID()
-	if err != nil {
-		return
-	}
-	if err = keyringLink(privilegedUserKeyringID, unix.KEY_SPEC_THREAD_KEYRING); err != nil {
-		return
+	// 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(ruid, euid); err != nil {
+			return err
+		}
 	}
-	defer keyringUnlink(privilegedUserKeyringID, unix.KEY_SPEC_THREAD_KEYRING)
 
-	// Drop euid last so we have permissions to drop the others.
-	if err = unix.Setregid(-1, egid); err != nil {
-		err = errors.Wrapf(err, "dropping egid to %d", egid)
-		return
-	}
-	if err = unix.Setgroups([]int{egid}); err != nil {
-		err = errors.Wrapf(err, "dropping groups")
-		return
+	if err := setGids(rgid, egid); err != nil {
+		return err
 	}
-	if err = unix.Setreuid(-1, euid); err != nil {
-		err = errors.Wrapf(err, "dropping euid to %d", euid)
-		return
+
+	if err := setGroups(target); err != nil {
+		return err
 	}
-	log.Printf("privileges dropped to euid=%d, egid=%d", euid, egid)
 
-	// Failure should undo the privilege modification
-	defer func() {
-		if err != nil {
-			raiseErr := RaiseThreadPrivileges(privs)
-			log.Printf("when reverting privilege changes: %v", raiseErr)
+	// If not setting privs to root, we want to avoid dropping the uid
+	// util the very end.
+	if euid != 0 {
+		if err := setUids(ruid, euid); err != nil {
+			return err
 		}
-	}()
-
-	// If the link already exists, this linking does nothing and succeeds.
-	droppedUserKeyringID, err := getUserKeyringID()
-	if err != nil {
-		return
-	}
-	if err = keyringLink(droppedUserKeyringID, privilegedUserKeyringID); err != nil {
-		return
 	}
-	log.Printf("user keyring (%d) linked into root user keyring (%d)",
-		droppedUserKeyringID, privilegedUserKeyringID)
+	return nil
+}
 
-	return privs, nil
+func setUids(ruid, euid int) error {
+	err := unix.Setreuid(ruid, euid)
+	log.Printf("Setreuid(%d, %d) = %v", ruid, euid, err)
+	return errors.Wrapf(err, "setting uids")
 }
 
-// RaiseThreadPrivileges returns the state of a threads privileges to what it
-// was before the call to DropThreadPrivileges.
-func RaiseThreadPrivileges(privs *Privileges) error {
-	// Raise euid last so we have permissions to raise the others.
-	if err := unix.Setreuid(-1, privs.euid); err != nil {
-		return errors.Wrapf(err, "raising euid to %d", privs.euid)
-	}
-	if err := unix.Setregid(-1, privs.egid); err != nil {
-		return errors.Wrapf(err, "raising egid to %d", privs.egid)
+func setGids(rgid, egid int) error {
+	err := unix.Setregid(rgid, egid)
+	log.Printf("Setregid(%d, %d) = %v", rgid, egid, err)
+	return errors.Wrapf(err, "setting gids")
+}
+
+func setGroups(target *user.User) error {
+	groupStrings, err := target.GroupIds()
+	if err != nil {
+		return util.SystemError(err.Error())
 	}
-	if err := unix.Setgroups(privs.groups); err != nil {
-		return errors.Wrapf(err, "raising groups")
+
+	gids := make([]int, len(groupStrings))
+	for i, groupString := range groupStrings {
+		gids[i] = util.AtoiOrPanic(groupString)
 	}
 
-	log.Printf("privileges raised to euid=%d, egid=%d", privs.euid, privs.egid)
-	return nil
+	err = unix.Setgroups(gids)
+	log.Printf("Setgroups(%v) = %v", gids, err)
+	return errors.Wrapf(err, "setting groups")
 }