From faec58eb20569513427c4defb84e54cb68e5a56a Mon Sep 17 00:00:00 2001 From: Joseph Richey Date: Tue, 29 Aug 2017 21:32:33 -0700 Subject: cmd/fscrypt: Stop dropping/raising for sudo --- cmd/fscrypt/fscrypt.go | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/cmd/fscrypt/fscrypt.go b/cmd/fscrypt/fscrypt.go index 93df341..d6162f6 100644 --- a/cmd/fscrypt/fscrypt.go +++ b/cmd/fscrypt/fscrypt.go @@ -31,12 +31,8 @@ import ( "io/ioutil" "log" "os" - "strconv" "time" - "golang.org/x/sys/unix" - - "github.com/google/fscrypt/security" "github.com/urfave/cli" ) @@ -130,27 +126,6 @@ func setupBefore(c *cli.Context) error { if !quietFlag.Value { c.App.Writer = os.Stdout } - - if unix.Geteuid() != 0 { - return nil // Must be root to setup links - } - euid, err := strconv.Atoi(os.Getenv("SUDO_UID")) - if err != nil { - return nil // Must be running with sudo - } - egid, err := strconv.Atoi(os.Getenv("SUDO_GID")) - if err != nil { - return nil // Must be running with sudo - } - - // Dropping and raising privileges checks the needed keyring link. - privs, err := security.DropThreadPrivileges(euid, egid) - if err != nil { - return newExitError(c, err) - } - if err := security.RaiseThreadPrivileges(privs); err != nil { - return newExitError(c, err) - } return nil } -- cgit v1.2.3 From 83d4b499c505d3f5841fc0b0f8f29509622e870b Mon Sep 17 00:00:00 2001 From: Joseph Richey Date: Tue, 29 Aug 2017 21:37:01 -0700 Subject: pam_fscrypt: Handle empty arguments list --- pam_fscrypt/run_fscrypt.go | 3 +++ pam_fscrypt/run_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 pam_fscrypt/run_test.go diff --git a/pam_fscrypt/run_fscrypt.go b/pam_fscrypt/run_fscrypt.go index 1527d42..3d73e87 100644 --- a/pam_fscrypt/run_fscrypt.go +++ b/pam_fscrypt/run_fscrypt.go @@ -82,6 +82,9 @@ func RunPamFunc(f PamFunc, pamh unsafe.Pointer, argc C.int, argv **C.char) C.int // where a key has a value of true if it appears in the argument list. func parseArgs(argc C.int, argv **C.char) map[string]bool { args := make(map[string]bool) + if argc == 0 || argv == nil { + return args + } for _, cString := range util.PointerSlice(unsafe.Pointer(argv))[:argc] { args[C.GoString((*C.char)(cString))] = true } diff --git a/pam_fscrypt/run_test.go b/pam_fscrypt/run_test.go new file mode 100644 index 0000000..1e74528 --- /dev/null +++ b/pam_fscrypt/run_test.go @@ -0,0 +1,35 @@ +/* + * run_test.go - tests that the PAM helper functionsd work properly + * + * Copyright 2017 Google Inc. + * Author: Joe Richey (joerichey@google.com) + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package main + +import ( + "testing" +) + +func TestParseArgsEmpty(t *testing.T) { + // An empty argv should create a map with no entries + args := parseArgs(0, nil) + if args == nil { + t.Fatal("args map should not be nil") + } + if len(args) > 0 { + t.Fatal("args map should not have any entries") + } +} -- cgit v1.2.3 From 8539c4c12ebdd6441f5d93fd499e31df2ed943c3 Mon Sep 17 00:00:00 2001 From: Joseph Richey Date: Tue, 29 Aug 2017 22:29:33 -0700 Subject: Go formatter "gofmt" -> "goimports" --- CONTRIBUTING.md | 9 +++++---- Makefile | 5 +++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e7a2a75..20fb884 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -86,10 +86,11 @@ make test-teardown ### Formatting and Linting -The `make format` command formats all the code in fscrypt with either `gofmt` -(for Go code) or [`clang-format`](https://clang.llvm.org/docs/ClangFormat.html) -(for C code). `gofmt` comes with any Go distribution, and `clang-format` can be -installed with your package manager. +The `make format` command formats all the code in fscrypt with either +[`goimports`](https://godoc.org/golang.org/x/tools/cmd/goimports) (for Go code) +or [`clang-format`](https://clang.llvm.org/docs/ClangFormat.html) (for C code). +`goimports` can be installed with `go get`; `clang-format` can be installed +with your package manager. The `make lint` command runs a series of static analysis checks on your code. This requires the diff --git a/Makefile b/Makefile index 1ec009e..af1d4ab 100644 --- a/Makefile +++ b/Makefile @@ -129,11 +129,11 @@ update: # Format all the Go and C code .PHONY: format format-check format: - @gofmt -l -s -w $(GO_FILES) + @goimports -l -w $(GO_FILES) @clang-format -i -style=Google $(C_FILES) format-check: - @gofmt -s -d $(GO_FILES) \ + @goimports -d $(GO_FILES) \ | ./input_fail.py "Incorrectly formatted Go files. Run \"make format\"." @clang-format -i -style=Google -output-replacements-xml $(C_FILES) \ | grep " Date: Wed, 30 Aug 2017 04:07:13 -0700 Subject: gitignore: Update to include VSCode files --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 34880d3..345dfa1 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ fscrypt fscrypt.* fscrypt_image pam_fscrypt.so +.vscode -- cgit v1.2.3 From e37a80cd9b601adc16894d3b6fb526ae8f4c846b Mon Sep 17 00:00:00 2001 From: Joseph Richey Date: Wed, 30 Aug 2017 04:49:39 -0700 Subject: util: Added parsing and effective user functions --- util/util.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/util/util.go b/util/util.go index c02ea0e..3de4a1a 100644 --- a/util/util.go +++ b/util/util.go @@ -27,6 +27,8 @@ import ( "bufio" "math" "os" + "os/user" + "strconv" "unsafe" ) @@ -105,3 +107,23 @@ func ReadLine() (string, error) { scanner.Scan() return scanner.Text(), scanner.Err() } + +// AtoiOrPanic converts a string to an int or it panics. Should only be used in +// situations where the input MUST be a decimal number. +func AtoiOrPanic(input string) int { + i, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + return i +} + +// EffectiveUser returns the user entry corresponding to the effective user. +func EffectiveUser() (*user.User, error) { + return user.LookupId(strconv.Itoa(os.Geteuid())) +} + +// IsUserRoot checks if the effective user is root. +func IsUserRoot() bool { + return os.Geteuid() == 0 +} -- cgit v1.2.3 From 7888645ab68ed0510ff66121f35630b11976a09f Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Wed, 30 Aug 2017 17:51:05 -0700 Subject: 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. --- security/keyring.go | 186 ++++++++++++++++++++++++++++--------------------- security/privileges.go | 140 +++++++++++++++---------------------- 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, , %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") } -- cgit v1.2.3 From 70efc397db81f3ad170e54114f3ad0a97f2ed7d0 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Wed, 30 Aug 2017 17:55:30 -0700 Subject: pam: Handle holds data for calling and PAM users The functions are now changed to (Start|Stop)AsPamUser to indicate that they handle privilege modification and keyring setup. --- pam/pam.go | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/pam/pam.go b/pam/pam.go index 12f2e97..657e9fb 100644 --- a/pam/pam.go +++ b/pam/pam.go @@ -30,26 +30,27 @@ package pam import "C" import ( "errors" - "fmt" "log" + "os/user" "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 - privs *security.Privileges - // UID of the user being authenticated - UID int - // GID of the user being authenticated - GID 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 + PamUser *user.User } // NewHandle creates a Handle from a raw pointer. func NewHandle(pamh unsafe.Pointer) (*Handle, error) { + var err error h := &Handle{ handle: (*C.pam_handle_t)(pamh), status: C.PAM_SUCCESS, @@ -61,12 +62,12 @@ func NewHandle(pamh unsafe.Pointer) (*Handle, error) { return nil, err } - pwnam := C.getpwnam(pamUsername) - if pwnam == nil { - return nil, fmt.Errorf("unknown user %q", C.GoString(pamUsername)) + 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 } - h.UID = int(pwnam.pw_uid) - h.GID = int(pwnam.pw_gid) return h, nil } @@ -127,18 +128,20 @@ func (h *Handle) GetItem(i Item) (unsafe.Pointer, error) { return data, h.err() } -// DropThreadPrivileges sets the effective privileges to that of the PAM user -func (h *Handle) DropThreadPrivileges() error { - var err error - h.privs, err = security.DropThreadPrivileges(h.UID, h.GID) - return err +// StartAsPamUser sets the effective privileges to that of the PAM user, and +// configures the PAM user's keyrings to be properly linked. +func (h *Handle) StartAsPamUser() error { + if err := security.KeyringsSetup(h.PamUser, h.OrigUser); err != nil { + return err + } + return security.SetThreadPrivileges(h.PamUser, false) } -// RaiseThreadPrivileges restores the original privileges that were running the +// 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) RaiseThreadPrivileges() error { - err := security.RaiseThreadPrivileges(h.privs) +func (h *Handle) StopAsPamUser() error { + err := security.SetThreadPrivileges(h.OrigUser, false) if err != nil { log.Print(err) } -- cgit v1.2.3 From d685f6b232485a0dc0cc8b915561b9be37d32722 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Wed, 30 Aug 2017 17:57:38 -0700 Subject: crypto: Updated to include user parameter --- crypto/crypto_test.go | 21 ++++++++++++--------- crypto/key.go | 5 +++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index 719db00..444f847 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -34,6 +34,7 @@ import ( "github.com/google/fscrypt/metadata" "github.com/google/fscrypt/security" + "github.com/google/fscrypt/util" ) // Reader that always returns the same byte @@ -60,6 +61,8 @@ var ( fakeValidPolicyKey, _ = makeKey(42, metadata.PolicyKeyLen) fakeInvalidPolicyKey, _ = makeKey(42, metadata.PolicyKeyLen-1) fakeWrappingKey, _ = makeKey(17, metadata.InternalKeyLen) + + testUser, _ = util.EffectiveUser() ) // As the passpharase hashing function clears the passphrase, we need to make @@ -243,10 +246,10 @@ func TestKeyLargeResize(t *testing.T) { func TestAddRemoveKeys(t *testing.T) { for _, service := range []string{defaultService, "ext4:", "f2fs:"} { validDescription := service + fakeValidDescriptor - if err := InsertPolicyKey(fakeValidPolicyKey, validDescription); err != nil { + if err := InsertPolicyKey(fakeValidPolicyKey, validDescription, testUser); err != nil { t.Error(err) } - if err := security.RemoveKey(validDescription); err != nil { + if err := security.RemoveKey(validDescription, testUser); err != nil { t.Error(err) } } @@ -255,23 +258,23 @@ func TestAddRemoveKeys(t *testing.T) { // Adds a key twice (both should succeed) func TestAddTwice(t *testing.T) { validDescription := defaultService + fakeValidDescriptor - InsertPolicyKey(fakeValidPolicyKey, validDescription) - if InsertPolicyKey(fakeValidPolicyKey, validDescription) != nil { + InsertPolicyKey(fakeValidPolicyKey, validDescription, testUser) + if InsertPolicyKey(fakeValidPolicyKey, validDescription, testUser) != nil { t.Error("InsertPolicyKey should not fail if key already exists") } - security.RemoveKey(validDescription) + security.RemoveKey(validDescription, testUser) } // Makes sure a key fails with bad policy or service func TestBadAddKeys(t *testing.T) { validDescription := defaultService + fakeValidDescriptor - if InsertPolicyKey(fakeInvalidPolicyKey, validDescription) == nil { - security.RemoveKey(validDescription) + if InsertPolicyKey(fakeInvalidPolicyKey, validDescription, testUser) == nil { + security.RemoveKey(validDescription, testUser) t.Error("InsertPolicyKey should fail with bad policy key") } invalidDescription := "ext4" + fakeValidDescriptor - if InsertPolicyKey(fakeValidPolicyKey, invalidDescription) == nil { - security.RemoveKey(invalidDescription) + if InsertPolicyKey(fakeValidPolicyKey, invalidDescription, testUser) == nil { + security.RemoveKey(invalidDescription, testUser) t.Error("InsertPolicyKey should fail with bad service") } } diff --git a/crypto/key.go b/crypto/key.go index ec37330..9bf9098 100644 --- a/crypto/key.go +++ b/crypto/key.go @@ -33,6 +33,7 @@ import ( "io" "log" "os" + "os/user" "runtime" "unsafe" @@ -247,7 +248,7 @@ func NewFixedLengthKeyFromReader(reader io.Reader, length int) (*Key, error) { // InsertPolicyKey puts the provided policy key into the kernel keyring with the // provided description, and type logon. The key must be a policy key. -func InsertPolicyKey(key *Key, description string) error { +func InsertPolicyKey(key *Key, description string, target *user.User) error { if err := util.CheckValidLength(metadata.PolicyKeyLen, key.Len()); err != nil { return errors.Wrap(err, "policy key") } @@ -266,7 +267,7 @@ func InsertPolicyKey(key *Key, description string) error { fscryptKey.Size = metadata.PolicyKeyLen copy(fscryptKey.Raw[:], key.data) - return security.InsertKey(payload.data, description) + return security.InsertKey(payload.data, description, target) } var ( -- cgit v1.2.3 From dad0a047cefc79cbe664afc07d69db6b8bf123bd Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Wed, 30 Aug 2017 18:00:04 -0700 Subject: actions: context now hold a target user.User This user is used with policies to interface with the keryings and with protectors to indicate which user's login passphrase should be used to protectors of type pam_passphrase. --- actions/context.go | 70 ++++++++++++++++++++++++++++++++++--------------- actions/context_test.go | 4 +-- actions/policy.go | 8 +++--- actions/protector.go | 16 +++++------ 4 files changed, 63 insertions(+), 35 deletions(-) diff --git a/actions/context.go b/actions/context.go index 7e4b64b..8ad1357 100644 --- a/actions/context.go +++ b/actions/context.go @@ -30,6 +30,7 @@ package actions import ( "log" + "os/user" "golang.org/x/sys/unix" @@ -37,6 +38,7 @@ import ( "github.com/google/fscrypt/filesystem" "github.com/google/fscrypt/metadata" + "github.com/google/fscrypt/util" ) // Errors relating to Config files or Config structures. @@ -49,47 +51,73 @@ var ( ) // Context contains the necessary global state to perform most of fscrypt's -// actions. It contains a config struct, which is loaded from the global config -// file, but can be edited manually. A context is specific to a filesystem, and -// all actions to add, edit, remove, and apply Protectors and Policies are done -// relative to that filesystem. +// actions. type Context struct { + // Config is the struct loaded from the global config file. It can be + // modified after being loaded to customise parameters. Config *metadata.Config - Mount *filesystem.Mount + // Mount is the filesystem relitive to which all Protectors and Policies + // are added, edited, removed, and applied. + Mount *filesystem.Mount + // TargetUser is the user for which protectors are created and to whose + // keyring policies are provisioned. + TargetUser *user.User } // NewContextFromPath makes a context for the filesystem containing the // specified path and whose Config is loaded from the global config file. On -// success, the Context contains a valid Config and Mount. -func NewContextFromPath(path string) (ctx *Context, err error) { - ctx = new(Context) - if ctx.Mount, err = filesystem.FindMount(path); err != nil { - return +// success, the Context contains a valid Config and Mount. The target defaults +// the the current effective user if none is specified. +func NewContextFromPath(path string, target *user.User) (*Context, error) { + ctx, err := newContextFromUser(target) + if err != nil { + return nil, err } - if ctx.Config, err = getConfig(); err != nil { - return + if ctx.Mount, err = filesystem.FindMount(path); err != nil { + return nil, err } log.Printf("%s is on %s filesystem %q (%s)", path, ctx.Mount.Filesystem, ctx.Mount.Path, ctx.Mount.Device) - return + return ctx, nil } // NewContextFromMountpoint makes a context for the filesystem at the specified // mountpoint and whose Config is loaded from the global config file. On -// success, the Context contains a valid Config and Mount. -func NewContextFromMountpoint(mountpoint string) (ctx *Context, err error) { - ctx = new(Context) - if ctx.Mount, err = filesystem.GetMount(mountpoint); err != nil { - return +// success, the Context contains a valid Config and Mount. The target defaults +// the the current effective user if none is specified. +func NewContextFromMountpoint(mountpoint string, target *user.User) (*Context, error) { + ctx, err := newContextFromUser(target) + if err != nil { + return nil, err } - if ctx.Config, err = getConfig(); err != nil { - return + if ctx.Mount, err = filesystem.GetMount(mountpoint); err != nil { + return nil, err } log.Printf("found %s filesystem %q (%s)", ctx.Mount.Filesystem, ctx.Mount.Path, ctx.Mount.Device) - return + return ctx, nil +} + +// newContextFromUser makes a context with the corresponding target user, and +// whose Config is loaded from the global config file. If the target is nil, the +// effecitive user is used. +func newContextFromUser(target *user.User) (*Context, error) { + var err error + if target == nil { + if target, err = util.EffectiveUser(); err != nil { + return nil, err + } + } + + ctx := &Context{TargetUser: target} + if ctx.Config, err = getConfig(); err != nil { + return nil, err + } + + log.Printf("creating context for %q", target.Username) + return ctx, nil } // checkContext verifies that the context contains an valid config and a mount diff --git a/actions/context_test.go b/actions/context_test.go index 4b38a33..593518f 100644 --- a/actions/context_test.go +++ b/actions/context_test.go @@ -47,7 +47,7 @@ func setupContext() (ctx *Context, err error) { ConfigFileLocation = filepath.Join(mountpoint, "test.conf") // Should not be able to setup without a config file - if badCtx, badCtxErr := NewContextFromMountpoint(mountpoint); badCtxErr == nil { + if badCtx, badCtxErr := NewContextFromMountpoint(mountpoint, nil); badCtxErr == nil { badCtx.Mount.RemoveAllMetadata() return nil, fmt.Errorf("created context at %q without config file", badCtx.Mount.Path) } @@ -61,7 +61,7 @@ func setupContext() (ctx *Context, err error) { } }() - ctx, err = NewContextFromMountpoint(mountpoint) + ctx, err = NewContextFromMountpoint(mountpoint, nil) if err != nil { return nil, err } diff --git a/actions/policy.go b/actions/policy.go index 461f8cc..510afa1 100644 --- a/actions/policy.go +++ b/actions/policy.go @@ -57,7 +57,7 @@ func PurgeAllPolicies(ctx *Context) error { for _, policyDescriptor := range policies { service := ctx.getService() - err = security.RemoveKey(service + policyDescriptor) + err = security.RemoveKey(service+policyDescriptor, ctx.TargetUser) switch errors.Cause(err) { case nil, security.ErrKeyringSearch: @@ -372,7 +372,7 @@ func (policy *Policy) Apply(path string) error { // IsProvisioned returns a boolean indicating if the policy has its key in the // keyring, meaning files and directories using this policy are accessible. func (policy *Policy) IsProvisioned() bool { - _, err := security.FindKey(policy.Description()) + _, err := security.FindKey(policy.Description(), policy.Context.TargetUser) return err == nil } @@ -382,13 +382,13 @@ func (policy *Policy) Provision() error { if policy.key == nil { return ErrLocked } - return crypto.InsertPolicyKey(policy.key, policy.Description()) + return crypto.InsertPolicyKey(policy.key, policy.Description(), policy.Context.TargetUser) } // Deprovision removes the Policy key from the kernel keyring. This prevents // reading and writing to the directory once the caches are cleared. func (policy *Policy) Deprovision() error { - return security.RemoveKey(policy.Description()) + return security.RemoveKey(policy.Description(), policy.Context.TargetUser) } // commitData writes the Policy's current data to the filesystem. diff --git a/actions/protector.go b/actions/protector.go index 5245951..ffc3c43 100644 --- a/actions/protector.go +++ b/actions/protector.go @@ -22,12 +22,12 @@ package actions import ( "fmt" "log" - "os" "github.com/pkg/errors" "github.com/google/fscrypt/crypto" "github.com/google/fscrypt/metadata" + "github.com/google/fscrypt/util" ) // Errors relating to Protectors @@ -54,17 +54,17 @@ func checkForProtectorWithName(ctx *Context, name string) error { return nil } -// checkForProtectorWithUid returns an error if there is already a login -// protector on the filesystem with a specific UID (or if we cannot read the +// checkIfUserHasLoginProtector returns an error if there is already a login +// protector on the filesystem for a specific user (or if we cannot read the // necessary data). -func checkForProtectorWithUID(ctx *Context, uid int64) error { +func checkIfUserHasLoginProtector(ctx *Context, uid int64) error { options, err := ctx.ProtectorOptions() if err != nil { return err } for _, option := range options { if option.Source() == metadata.SourceType_pam_passphrase && option.UID() == uid { - return errors.Wrapf(ErrDuplicateUID, "uid %d", uid) + return errors.Wrapf(ErrDuplicateUID, "user %q", ctx.TargetUser.Username) } } return nil @@ -121,9 +121,9 @@ func CreateProtector(ctx *Context, name string, keyFn KeyFunc) (*Protector, erro case metadata.SourceType_pam_passphrase: // As the pam passphrases are user specific, we also store the // UID for this kind of source. - protector.data.Uid = int64(os.Getuid()) + protector.data.Uid = int64(util.AtoiOrPanic(ctx.TargetUser.Uid)) // Make sure we aren't duplicating protectors - if err := checkForProtectorWithUID(ctx, protector.data.Uid); err != nil { + if err := checkIfUserHasLoginProtector(ctx, protector.data.Uid); err != nil { return nil, err } fallthrough @@ -180,7 +180,7 @@ func GetProtectorFromOption(ctx *Context, option *ProtectorOption) (*Protector, // Replace the context if this is a linked protector if option.LinkedMount != nil { - ctx = &Context{ctx.Config, option.LinkedMount} + ctx = &Context{ctx.Config, option.LinkedMount, ctx.TargetUser} } return &Protector{Context: ctx, data: option.data}, nil } -- cgit v1.2.3 From 5814155d0c0247d501f7479f760a676185cd4b6d Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Wed, 30 Aug 2017 18:03:09 -0700 Subject: pam_fscrypt: Added logging and use of new pam API --- pam_fscrypt/pam_fscrypt.go | 36 ++++++++++++++++++++---------------- pam_fscrypt/run_fscrypt.go | 18 ++++++++++-------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/pam_fscrypt/pam_fscrypt.go b/pam_fscrypt/pam_fscrypt.go index 5beb311..7eccc85 100644 --- a/pam_fscrypt/pam_fscrypt.go +++ b/pam_fscrypt/pam_fscrypt.go @@ -55,18 +55,19 @@ const ( // Authenticate copies the AUTHTOK (if necessary) into the PAM data so it can be // used in pam_sm_open_session. func Authenticate(handle *pam.Handle, _ map[string]bool) error { - if err := handle.DropThreadPrivileges(); err != nil { + log.Print("Authenticate()") + if err := handle.StartAsPamUser(); err != nil { return err } - defer handle.RaiseThreadPrivileges() + defer handle.StopAsPamUser() // If this user doesn't have a login protector, no unlocking is needed. if _, err := loginProtector(handle); err != nil { - log.Printf("no need to copy AUTHTOK: %s", err) + log.Printf("no protector, no need for AUTHTOK: %s", err) return nil } - log.Print("Authenticate: copying AUTHTOK for use in the session") + log.Print("copying AUTHTOK for use in the session open") authtok, err := handle.GetItem(pam.Authtok) if err != nil { return errors.Wrap(err, "could not get AUTHTOK") @@ -77,6 +78,7 @@ func Authenticate(handle *pam.Handle, _ map[string]bool) error { // OpenSession provisions any policies protected with the login protector. func OpenSession(handle *pam.Handle, _ map[string]bool) error { + log.Print("OpenSession()") // We will always clear the the AUTHTOK data defer handle.ClearData(authtokLabel) // Increment the count as we add a session @@ -84,15 +86,15 @@ func OpenSession(handle *pam.Handle, _ map[string]bool) error { return err } - if err := handle.DropThreadPrivileges(); err != nil { + if err := handle.StartAsPamUser(); err != nil { return err } - defer handle.RaiseThreadPrivileges() + defer handle.StopAsPamUser() // If there are no polices for the login protector, no unlocking needed. protector, err := loginProtector(handle) if err != nil { - log.Printf("nothing to unlock: %s", err) + log.Printf("no protector to unlock: %s", err) return nil } policies := policiesUsingProtector(protector) @@ -101,7 +103,7 @@ func OpenSession(handle *pam.Handle, _ map[string]bool) error { return nil } - log.Print("OpenSession: unlocking policies protected with AUTHTOK") + log.Printf("unlocking %d policies protected with AUTHTOK", len(policies)) keyFn := func(_ actions.ProtectorInfo, retry bool) (*crypto.Key, error) { if retry { // Login passphrase and login protector have diverged. @@ -150,6 +152,7 @@ func OpenSession(handle *pam.Handle, _ map[string]bool) error { // CloseSession can deprovision all keys provisioned at the start of the // session. It can also clear the cache so these changes take effect. func CloseSession(handle *pam.Handle, args map[string]bool) error { + log.Printf("CloseSession(%v)", args) // Only do stuff on session close when we are the last session if count, err := AdjustCount(handle, -1); err != nil || count != 0 { log.Printf("count is %d and we are not locking", count) @@ -159,12 +162,12 @@ func CloseSession(handle *pam.Handle, args map[string]bool) error { var errLock, errCache error // Don't automatically drop privileges, we may need them to drop caches. if args[lockFlag] { - log.Print("CloseSession: locking polices protected with login") + log.Print("locking polices protected with login protector") errLock = lockLoginPolicies(handle) } if args[cacheFlag] { - log.Print("CloseSession: dropping inode caches") + log.Print("dropping inode caches at session close") errCache = security.DropInodeCache() } @@ -177,10 +180,10 @@ func CloseSession(handle *pam.Handle, args map[string]bool) error { // lockLoginPolicies deprovisions all policy keys that are protected by // the user's login protector. func lockLoginPolicies(handle *pam.Handle) error { - if err := handle.DropThreadPrivileges(); err != nil { + if err := handle.StartAsPamUser(); err != nil { return err } - defer handle.RaiseThreadPrivileges() + defer handle.StopAsPamUser() // If there are no polices for the login protector, no locking needed. protector, err := loginProtector(handle) @@ -211,14 +214,15 @@ func lockLoginPolicies(handle *pam.Handle) error { // Chauthtok rewraps the login protector when the passphrase changes. func Chauthtok(handle *pam.Handle, _ map[string]bool) error { - if err := handle.DropThreadPrivileges(); err != nil { + log.Print("Chauthtok()") + if err := handle.StartAsPamUser(); err != nil { return err } - defer handle.RaiseThreadPrivileges() + defer handle.StopAsPamUser() protector, err := loginProtector(handle) if err != nil { - log.Printf("nothing to rewrap: %s", err) + log.Printf("no login protector to rewrap: %s", err) return nil } @@ -244,7 +248,7 @@ func Chauthtok(handle *pam.Handle, _ map[string]bool) error { return crypto.NewKeyFromCString(authtok) } - log.Print("Chauthtok: rewrapping login protector") + log.Print("rewrapping login protector") if err = protector.Unlock(oldKeyFn); err != nil { return err } diff --git a/pam_fscrypt/run_fscrypt.go b/pam_fscrypt/run_fscrypt.go index 3d73e87..c02b05f 100644 --- a/pam_fscrypt/run_fscrypt.go +++ b/pam_fscrypt/run_fscrypt.go @@ -115,7 +115,7 @@ func setupLogging(args map[string]bool) io.Writer { // one exists. This protector descriptor (if found) will be cached in the pam // data, under descriptorLabel. func loginProtector(handle *pam.Handle) (*actions.Protector, error) { - ctx, err := actions.NewContextFromMountpoint("/") + ctx, err := actions.NewContextFromMountpoint("/", handle.PamUser) if err != nil { return nil, err } @@ -125,13 +125,13 @@ func loginProtector(handle *pam.Handle) (*actions.Protector, error) { if err != nil { return nil, err } + uid := int64(util.AtoiOrPanic(handle.PamUser.Uid)) for _, option := range options { - if option.Source() == metadata.SourceType_pam_passphrase && - option.UID() == int64(handle.UID) { + if option.Source() == metadata.SourceType_pam_passphrase && option.UID() == uid { return actions.GetProtectorFromOption(ctx, option) } } - return nil, errors.Errorf("no PAM protector for UID=%d on %q", handle.UID, ctx.Mount.Path) + return nil, errors.Errorf("no PAM protector for UID=%d on %q", uid, ctx.Mount.Path) } // policiesUsingProtector searches all the mountpoints for any policies @@ -155,9 +155,11 @@ func policiesUsingProtector(protector *actions.Protector) []*actions.Policy { continue } - ctx := &actions.Context{Config: protector.Context.Config, Mount: mount} + // Clone context but modify the mountpoint + ctx := *protector.Context + ctx.Mount = mount for _, policyDescriptor := range policyDescriptors { - policy, err := actions.GetPolicy(ctx, policyDescriptor) + policy, err := actions.GetPolicy(&ctx, policyDescriptor) if err != nil { log.Printf("reading policy: %s", err) continue @@ -181,7 +183,7 @@ func AdjustCount(handle *pam.Handle, delta int) (int, error) { return 0, err } - path := filepath.Join(countDirectory, fmt.Sprintf("%d.count", handle.UID)) + path := filepath.Join(countDirectory, handle.PamUser.Uid+".count") file, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, countFilePermissions) if err != nil { return 0, err @@ -199,7 +201,7 @@ func AdjustCount(handle *pam.Handle, delta int) (int, error) { return 0, err } - log.Printf("Session count for UID=%d updated to %d", handle.UID, newCount) + log.Printf("Session count for UID=%s updated to %d", handle.PamUser.Uid, newCount) return newCount, nil } -- cgit v1.2.3 From 11b31826334bc3faa4d4c7ee05a3b2996a88c969 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Wed, 30 Aug 2017 18:16:16 -0700 Subject: cmd/fscrypt: Add --user flag for running as root The --user flag can now be used to have the targe user (the one whose keyring and password will be used in fscrypt) be different than the calling user. Very usefull for things like sudo fscrypt purge /media/joerichey/usb --user=joerichey which will now have privileges to drop caches, but will properly clear the keys from the user's keyring. --- cmd/fscrypt/commands.go | 70 +++++++++++++++++++++++++++++------------------- cmd/fscrypt/errors.go | 10 +++++++ cmd/fscrypt/flags.go | 57 +++++++++++++++++++++++++++++---------- cmd/fscrypt/prompt.go | 2 +- cmd/fscrypt/protector.go | 4 ++- cmd/fscrypt/setup.go | 5 ++-- cmd/fscrypt/status.go | 2 +- 7 files changed, 104 insertions(+), 46 deletions(-) diff --git a/cmd/fscrypt/commands.go b/cmd/fscrypt/commands.go index 3e8bc98..43c9cb0 100644 --- a/cmd/fscrypt/commands.go +++ b/cmd/fscrypt/commands.go @@ -25,8 +25,6 @@ import ( "log" "os" - "golang.org/x/sys/unix" - "github.com/pkg/errors" "github.com/urfave/cli" @@ -34,6 +32,7 @@ import ( "github.com/google/fscrypt/filesystem" "github.com/google/fscrypt/metadata" "github.com/google/fscrypt/security" + "github.com/google/fscrypt/util" ) // Setup is a command which can to global or per-filesystem initialization. @@ -60,7 +59,6 @@ var Setup = cli.Command{ func setupAction(c *cli.Context) error { var err error - switch c.NArg() { case 0: // Case (1) - global setup @@ -92,7 +90,7 @@ var Encrypt = cli.Command{ immediately be used.`, directoryArg, shortDisplay(policyFlag), shortDisplay(protectorFlag), mountpointArg), Flags: []cli.Flag{policyFlag, unlockWithFlag, protectorFlag, sourceFlag, - nameFlag, keyFileFlag, skipUnlockFlag}, + userFlag, nameFlag, keyFileFlag, skipUnlockFlag}, Action: encryptAction, } @@ -121,7 +119,11 @@ func encryptAction(c *cli.Context) error { // keyring unless --skip-unlock is used. On failure, an error is returned, any // metadata creation is reverted, and the directory is unmodified. func encryptPath(path string) (err error) { - ctx, err := actions.NewContextFromPath(path) + target, err := parseUserFlag() + if err != nil { + return + } + ctx, err := actions.NewContextFromPath(path, target) if err != nil { return } @@ -133,7 +135,7 @@ func encryptPath(path string) (err error) { if policyFlag.Value != "" { log.Printf("getting policy for %q", path) - policy, err = getPolicyFromFlag(policyFlag.Value) + policy, err = getPolicyFromFlag(policyFlag.Value, ctx.TargetUser) } else { log.Printf("creating policy for %q", path) @@ -220,7 +222,7 @@ func checkEncryptable(ctx *actions.Context, path string) error { // created a new protector. func selectOrCreateProtector(ctx *actions.Context) (*actions.Protector, bool, error) { if protectorFlag.Value != "" { - protector, err := getProtectorFromFlag(protectorFlag.Value) + protector, err := getProtectorFromFlag(protectorFlag.Value, ctx.TargetUser) return protector, false, err } @@ -263,7 +265,7 @@ var Unlock = cli.Command{ locked again upon reboot, or after running "fscrypt purge" and unmounting the corresponding filesystem.`, directoryArg, shortDisplay(unlockWithFlag)), - Flags: []cli.Flag{unlockWithFlag, keyFileFlag}, + Flags: []cli.Flag{unlockWithFlag, keyFileFlag, userFlag}, Action: unlockAction, } @@ -272,8 +274,12 @@ func unlockAction(c *cli.Context) error { return expectedArgsErr(c, 1, false) } + target, err := parseUserFlag() + if err != nil { + return newExitError(c, err) + } path := c.Args().Get(0) - ctx, err := actions.NewContextFromPath(path) + ctx, err := actions.NewContextFromPath(path, target) if err != nil { return newExitError(c, err) } @@ -336,7 +342,7 @@ var Purge = cli.Command{ can be eliminated by cycling the power or mitigated by using page cache and slab cache poisoning.`, mountpointArg, shortDisplay(dropCachesFlag)), - Flags: []cli.Flag{forceFlag, dropCachesFlag}, + Flags: []cli.Flag{forceFlag, dropCachesFlag, userFlag}, Action: purgeAction, } @@ -346,12 +352,17 @@ func purgeAction(c *cli.Context) error { } if dropCachesFlag.Value { - if unix.Geteuid() != 0 { + if !util.IsUserRoot() { return newExitError(c, ErrDropCachesPerm) } } - ctx, err := actions.NewContextFromMountpoint(c.Args().Get(0)) + target, err := parseUserFlag() + if err != nil { + return newExitError(c, err) + } + mountpoint := c.Args().Get(0) + ctx, err := actions.NewContextFromMountpoint(mountpoint, target) if err != nil { return newExitError(c, err) } @@ -417,7 +428,7 @@ func statusAction(c *cli.Context) error { err = writeGlobalStatus(c.App.Writer) case 1: path := c.Args().Get(0) - ctx, mntErr := actions.NewContextFromMountpoint(path) + ctx, mntErr := actions.NewContextFromMountpoint(path, nil) switch errors.Cause(mntErr) { case nil: @@ -487,7 +498,7 @@ var createProtector = cli.Command{ applicable). As with "fscrypt encrypt", these prompts can be disabled with the appropriate flags.`, mountpointArg, shortDisplay(protectorFlag)), - Flags: []cli.Flag{sourceFlag, nameFlag, keyFileFlag}, + Flags: []cli.Flag{sourceFlag, nameFlag, keyFileFlag, userFlag}, Action: createProtectorAction, } @@ -496,7 +507,12 @@ func createProtectorAction(c *cli.Context) error { return expectedArgsErr(c, 1, false) } - ctx, err := actions.NewContextFromMountpoint(c.Args().Get(0)) + target, err := parseUserFlag() + if err != nil { + return newExitError(c, err) + } + mountpoint := c.Args().Get(0) + ctx, err := actions.NewContextFromMountpoint(mountpoint, target) if err != nil { return newExitError(c, err) } @@ -539,7 +555,7 @@ func createPolicyAction(c *cli.Context) error { return expectedArgsErr(c, 1, false) } - ctx, err := actions.NewContextFromMountpoint(c.Args().Get(0)) + ctx, err := actions.NewContextFromMountpoint(c.Args().Get(0), nil) if err != nil { return newExitError(c, err) } @@ -547,7 +563,7 @@ func createPolicyAction(c *cli.Context) error { if err := checkRequiredFlags(c, []*stringFlag{protectorFlag}); err != nil { return err } - protector, err := getProtectorFromFlag(protectorFlag.Value) + protector, err := getProtectorFromFlag(protectorFlag.Value, ctx.TargetUser) if err != nil { return newExitError(c, err) } @@ -608,7 +624,7 @@ func destoryMetadataAction(c *cli.Context) error { switch { case protectorFlag.Value != "": // Case (1) - protector destroy - protector, err := getProtectorFromFlag(protectorFlag.Value) + protector, err := getProtectorFromFlag(protectorFlag.Value, nil) if err != nil { return newExitError(c, err) } @@ -627,7 +643,7 @@ func destoryMetadataAction(c *cli.Context) error { protector.Descriptor(), protector.Context.Mount.Path) case policyFlag.Value != "": // Case (2) - policy destroy - policy, err := getPolicyFromFlag(policyFlag.Value) + policy, err := getPolicyFromFlag(policyFlag.Value, nil) if err != nil { return newExitError(c, err) } @@ -654,7 +670,7 @@ func destoryMetadataAction(c *cli.Context) error { case 1: // Case (3) - mountpoint destroy path := c.Args().Get(0) - ctx, err := actions.NewContextFromMountpoint(path) + ctx, err := actions.NewContextFromMountpoint(path, nil) if err != nil { return newExitError(c, err) } @@ -694,7 +710,7 @@ func changePassphraseAction(c *cli.Context) error { return err } - protector, err := getProtectorFromFlag(protectorFlag.Value) + protector, err := getProtectorFromFlag(protectorFlag.Value, nil) if err != nil { return newExitError(c, err) } @@ -732,11 +748,11 @@ func addProtectorAction(c *cli.Context) error { return err } - protector, err := getProtectorFromFlag(protectorFlag.Value) + protector, err := getProtectorFromFlag(protectorFlag.Value, nil) if err != nil { return newExitError(c, err) } - policy, err := getPolicyFromFlag(policyFlag.Value) + policy, err := getPolicyFromFlag(policyFlag.Value, protector.Context.TargetUser) if err != nil { return newExitError(c, err) } @@ -790,11 +806,11 @@ func removeProtectorAction(c *cli.Context) error { } // We do not need to unlock anything for this operation - protector, err := getProtectorFromFlag(protectorFlag.Value) + protector, err := getProtectorFromFlag(protectorFlag.Value, nil) if err != nil { return newExitError(c, err) } - policy, err := getPolicyFromFlag(policyFlag.Value) + policy, err := getPolicyFromFlag(policyFlag.Value, protector.Context.TargetUser) if err != nil { return newExitError(c, err) } @@ -834,14 +850,14 @@ func dumpMetadataAction(c *cli.Context) error { switch { case protectorFlag.Value != "": // Case (1) - protector print - protector, err := getProtectorFromFlag(protectorFlag.Value) + protector, err := getProtectorFromFlag(protectorFlag.Value, nil) if err != nil { return newExitError(c, err) } fmt.Fprintln(c.App.Writer, protector) case policyFlag.Value != "": // Case (2) - policy print - policy, err := getPolicyFromFlag(policyFlag.Value) + policy, err := getPolicyFromFlag(policyFlag.Value, nil) if err != nil { return newExitError(c, err) } diff --git a/cmd/fscrypt/errors.go b/cmd/fscrypt/errors.go index b2aa57e..88525d1 100644 --- a/cmd/fscrypt/errors.go +++ b/cmd/fscrypt/errors.go @@ -60,6 +60,8 @@ var ( ErrNotPassphrase = errors.New("protector does not use a passphrase") ErrUnknownUser = errors.New("unknown user") ErrDropCachesPerm = errors.New("inode cache can only be dropped as root") + ErrSpecifyUser = errors.New("user must be specified when run as root") + ErrSpecifyNonRootUser = errors.New("non-root user must be specified") ) var loadHelpText = fmt.Sprintf("You may need to mount a linked filesystem. Run with %s for more information.", shortDisplay(verboseFlag)) @@ -125,6 +127,14 @@ func getErrorSuggestions(err error) string { properly clear the inode cache, or it should be run with %s=false (this may leave encrypted files and directories in an accessible state).`, shortDisplay(dropCachesFlag)) + case ErrSpecifyUser: + return fmt.Sprintf(`When running this command as root, you + usually still want to provision/remove keys for a normal + user's keyring and use a normal user's login passphrase + as a protector (so the corresponding files will be + accessible for that user). This can be done with %s. To + use the root user's keyring or passphrase, use + --%s=root.`, shortDisplay(userFlag), userFlag.GetName()) case ErrAllLoadsFailed: return loadHelpText default: diff --git a/cmd/fscrypt/flags.go b/cmd/fscrypt/flags.go index a06b952..e883a6d 100644 --- a/cmd/fscrypt/flags.go +++ b/cmd/fscrypt/flags.go @@ -25,6 +25,7 @@ import ( "flag" "fmt" "log" + "os/user" "regexp" "strconv" "time" @@ -32,6 +33,7 @@ import ( "github.com/urfave/cli" "github.com/google/fscrypt/actions" + "github.com/google/fscrypt/util" ) // We define the types boolFlag, durationFlag, and stringFlag here instead of @@ -204,6 +206,12 @@ var ( formatted as raw binary and should be exactly 32 bytes long.`, } + userFlag = &stringFlag{ + Name: "user", + ArgName: "USERNAME", + Usage: `Specifiy which user should be used for login passphrases + or to which user's keyring keys should be provisioned.`, + } protectorFlag = &stringFlag{ Name: "protector", ArgName: "MOUNTPOINT:ID", @@ -233,27 +241,31 @@ var ( // group is required and corresponds to the descriptor. var idFlagRegex = regexp.MustCompile("^([[:print:]]+):([[:alnum:]]+)$") +func matchMetadataFlag(flagValue string) (mountpoint, descriptor string, err error) { + matches := idFlagRegex.FindStringSubmatch(flagValue) + if matches == nil { + return "", "", fmt.Errorf("flag value %q does not have format %s", + flagValue, mountpointIDArg) + } + log.Printf("parsed flag: mountpoint=%q descriptor=%s", matches[1], matches[2]) + return matches[1], matches[2], nil +} + // parseMetadataFlag takes the value of either protectorFlag or policyFlag // formatted as MOUNTPOINT:DESCRIPTOR, and returns a context for the mountpoint // and a string for the descriptor. -func parseMetadataFlag(flagValue string) (*actions.Context, string, error) { - matches := idFlagRegex.FindStringSubmatch(flagValue) - if matches == nil { - err := fmt.Errorf("flag value %q does not have format %s", flagValue, mountpointIDArg) +func parseMetadataFlag(flagValue string, target *user.User) (*actions.Context, string, error) { + mountpoint, descriptor, err := matchMetadataFlag(flagValue) + if err != nil { return nil, "", err } - - mountpoint := matches[1] - descriptor := matches[2] - log.Printf("parsed flag: mountpoint=%q descriptor=%s", mountpoint, descriptor) - - ctx, err := actions.NewContextFromMountpoint(mountpoint) + ctx, err := actions.NewContextFromMountpoint(mountpoint, target) return ctx, descriptor, err } // getProtectorFromFlag gets an existing locked protector from protectorFlag. -func getProtectorFromFlag(flagValue string) (*actions.Protector, error) { - ctx, descriptor, err := parseMetadataFlag(flagValue) +func getProtectorFromFlag(flagValue string, target *user.User) (*actions.Protector, error) { + ctx, descriptor, err := parseMetadataFlag(flagValue, target) if err != nil { return nil, err } @@ -261,10 +273,27 @@ func getProtectorFromFlag(flagValue string) (*actions.Protector, error) { } // getPolicyFromFlag gets an existing locked policy from policyFlag. -func getPolicyFromFlag(flagValue string) (*actions.Policy, error) { - ctx, descriptor, err := parseMetadataFlag(flagValue) +func getPolicyFromFlag(flagValue string, target *user.User) (*actions.Policy, error) { + ctx, descriptor, err := parseMetadataFlag(flagValue, target) if err != nil { return nil, err } return actions.GetPolicy(ctx, descriptor) } + +// parseUserFlag returns the user specified by userFlag or the current effective +// user if the flag value is missing. If the effective user is root, however, a +// user must specified in the flag. +func parseUserFlag() (*user.User, error) { + if userFlag.Value != "" { + return user.Lookup(userFlag.Value) + } + effectiveUser, err := util.EffectiveUser() + if err != nil { + return nil, err + } + if util.IsUserRoot() { + return nil, ErrSpecifyUser + } + return effectiveUser, nil +} diff --git a/cmd/fscrypt/prompt.go b/cmd/fscrypt/prompt.go index b882c08..0031e8f 100644 --- a/cmd/fscrypt/prompt.go +++ b/cmd/fscrypt/prompt.go @@ -308,7 +308,7 @@ func optionFn(policyDescriptor string, options []*actions.ProtectorOption) (int, // protector to unlock the policy. if unlockWithFlag.Value != "" { log.Printf("optionFn(%s) w/ unlock flag", policyDescriptor) - protector, err := getProtectorFromFlag(unlockWithFlag.Value) + protector, err := getProtectorFromFlag(unlockWithFlag.Value, nil) if err != nil { return 0, err } diff --git a/cmd/fscrypt/protector.go b/cmd/fscrypt/protector.go index f54d3a4..32ba4ab 100644 --- a/cmd/fscrypt/protector.go +++ b/cmd/fscrypt/protector.go @@ -119,5 +119,7 @@ func modifiedContext(ctx *actions.Context) (*actions.Context, error) { return nil, err } - return &actions.Context{Config: ctx.Config, Mount: mnt}, nil + modifiedCtx := *ctx + modifiedCtx.Mount = mnt + return &modifiedCtx, nil } diff --git a/cmd/fscrypt/setup.go b/cmd/fscrypt/setup.go index 6433b0c..72dfbdb 100644 --- a/cmd/fscrypt/setup.go +++ b/cmd/fscrypt/setup.go @@ -26,11 +26,12 @@ import ( "os" "github.com/google/fscrypt/actions" + "github.com/google/fscrypt/util" ) // createGlobalConfig creates (or overwrites) the global config file func createGlobalConfig(w io.Writer, path string) error { - if os.Getuid() != 0 { + if !util.IsUserRoot() { return ErrMustBeRoot } @@ -61,7 +62,7 @@ func createGlobalConfig(w io.Writer, path string) error { // setupFilesystem creates the directories for a filesystem to use fscrypt. func setupFilesystem(w io.Writer, path string) error { - ctx, err := actions.NewContextFromMountpoint(path) + ctx, err := actions.NewContextFromMountpoint(path, nil) if err != nil { return err } diff --git a/cmd/fscrypt/status.go b/cmd/fscrypt/status.go index c2adad7..1465a4e 100644 --- a/cmd/fscrypt/status.go +++ b/cmd/fscrypt/status.go @@ -166,7 +166,7 @@ func writeFilesystemStatus(w io.Writer, ctx *actions.Context) error { } func writePathStatus(w io.Writer, path string) error { - ctx, err := actions.NewContextFromPath(path) + ctx, err := actions.NewContextFromPath(path, nil) if err != nil { return err } -- cgit v1.2.3 From 5586bc35fbb33f20c38f52285c19c015b804ea94 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Thu, 31 Aug 2017 11:29:30 -0700 Subject: Fixed linter issues --- cmd/fscrypt/errors.go | 1 - security/keyring.go | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/cmd/fscrypt/errors.go b/cmd/fscrypt/errors.go index 88525d1..9731efc 100644 --- a/cmd/fscrypt/errors.go +++ b/cmd/fscrypt/errors.go @@ -61,7 +61,6 @@ var ( ErrUnknownUser = errors.New("unknown user") ErrDropCachesPerm = errors.New("inode cache can only be dropped as root") ErrSpecifyUser = errors.New("user must be specified when run as root") - ErrSpecifyNonRootUser = errors.New("non-root user must be specified") ) var loadHelpText = fmt.Sprintf("You may need to mount a linked filesystem. Run with %s for more information.", shortDisplay(verboseFlag)) diff --git a/security/keyring.go b/security/keyring.go index 8112c54..f507737 100644 --- a/security/keyring.go +++ b/security/keyring.go @@ -139,8 +139,8 @@ func userKeyringID(target *user.User) (int, error) { // 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) + idChan := make(chan int) + errChan := make(chan error) // OSThread locks ensure the privilege change is only for the lookup. runtime.LockOSThread() defer runtime.UnlockOSThread() @@ -156,7 +156,6 @@ func userKeyringID(target *user.User) (int, error) { return } idChan <- keyringID - return }() // We select so the thread will have to complete -- cgit v1.2.3 From f1bd511fff8e411687001bd8e76e8a41c9f5ff41 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Thu, 31 Aug 2017 12:09:26 -0700 Subject: security: Error if privilege reset goes wrong --- security/keyring.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/keyring.go b/security/keyring.go index f507737..4cfb4af 100644 --- a/security/keyring.go +++ b/security/keyring.go @@ -164,7 +164,7 @@ func userKeyringID(target *user.User) (int, error) { return 0, err case keyringID := <-idChan: if uid == os.Getuid() && uid == os.Geteuid() { - log.Print("thread privileges now incorrect") + return 0, util.SystemError("thread privileges now incorrect") } return keyringID, nil } -- cgit v1.2.3