From 4c7c6631cc5a27cc6b4431f5ad3805a2d624c5f5 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sun, 19 Dec 2021 21:19:25 -0600 Subject: Set owner of login protectors to correct user When the root user creates a login protector for a non-root user, make sure to chown() the protector file to make it owned by the user. Without this, the protector cannot be updated by the user, which causes it to get out of sync if the user changes their login passphrase. Fixes https://github.com/google/fscrypt/issues/319 --- cli-tests/t_encrypt_login.out | 2 ++ cli-tests/t_encrypt_login.sh | 11 ++++++++++- filesystem/filesystem.go | 33 ++++++++++++++++++++++++++------- util/util.go | 14 +++++++++++++- 4 files changed, 51 insertions(+), 9 deletions(-) diff --git a/cli-tests/t_encrypt_login.out b/cli-tests/t_encrypt_login.out index c531f73..220d901 100644 --- a/cli-tests/t_encrypt_login.out +++ b/cli-tests/t_encrypt_login.out @@ -111,6 +111,8 @@ PROTECTOR LINKED DESCRIPTION desc19 Yes (MNT_ROOT) login protector for fscrypt-test-user desc20 No custom protector "Recovery passphrase for dir" +Protector is owned by fscrypt-test-user:fscrypt-test-user + # Encrypt with login protector with --no-recovery ext4 filesystem "MNT" has 1 protector and 1 policy diff --git a/cli-tests/t_encrypt_login.sh b/cli-tests/t_encrypt_login.sh index 652d860..e03122d 100755 --- a/cli-tests/t_encrypt_login.sh +++ b/cli-tests/t_encrypt_login.sh @@ -27,13 +27,18 @@ show_status() fi } +get_login_protector() +{ + fscrypt status "$dir" | awk '/login protector/{print $1}' +} + begin "Encrypt with login protector" chown "$TEST_USER" "$dir" _user_do "echo TEST_USER_PASS | fscrypt encrypt --quiet --source=pam_passphrase '$dir'" show_status true recovery_passphrase=$(grep -E '^ +[a-z]{20}$' "$dir/fscrypt_recovery_readme.txt" | sed 's/^ +//') recovery_protector=$(fscrypt status "$dir" | awk '/Recovery passphrase/{print $1}') -login_protector=$(fscrypt status "$dir" | awk '/login protector/{print $1}') +login_protector=$(get_login_protector) _print_header "=> Lock, then unlock with login passphrase" _user_do "fscrypt lock '$dir'" # FIXME: should we be able to use $MNT:$login_protector here? @@ -57,6 +62,10 @@ show_status true begin "Encrypt with login protector as root" echo TEST_USER_PASS | fscrypt encrypt --quiet --source=pam_passphrase --user="$TEST_USER" "$dir" show_status true +# The newly-created login protector should be owned by the user, not root. +login_protector=$(get_login_protector) +owner=$(stat -c "%U:%G" "$MNT_ROOT/.fscrypt/protectors/$login_protector") +echo -e "\nProtector is owned by $owner" begin "Encrypt with login protector with --no-recovery" chown "$TEST_USER" "$dir" diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go index 9b5b7e2..456a4fc 100644 --- a/filesystem/filesystem.go +++ b/filesystem/filesystem.go @@ -37,6 +37,7 @@ import ( "io/ioutil" "log" "os" + "os/user" "path/filepath" "sort" "strings" @@ -47,6 +48,7 @@ import ( "golang.org/x/sys/unix" "github.com/google/fscrypt/metadata" + "github.com/google/fscrypt/util" ) // ErrAlreadySetup indicates that a filesystem is already setup for fscrypt. @@ -392,7 +394,7 @@ func syncDirectory(dirPath string) error { // writeDataAtomic writes the data to the path such that the data is either // written to stable storage or an error is returned. -func (m *Mount) writeDataAtomic(path string, data []byte) error { +func (m *Mount) writeDataAtomic(path string, data []byte, owner *user.User) error { // Write the data to a temporary file, sync it, then rename into place // so that the operation will be atomic. dirPath := filepath.Dir(path) @@ -407,6 +409,14 @@ func (m *Mount) writeDataAtomic(path string, data []byte) error { tempFile.Close() return err } + if owner != nil { + if err = util.Chown(tempFile, owner); err != nil { + log.Printf("could not set owner of %q to %v: %v", + path, owner.Username, err) + tempFile.Close() + return err + } + } if _, err = tempFile.Write(data); err != nil { tempFile.Close() return err @@ -428,7 +438,7 @@ func (m *Mount) writeDataAtomic(path string, data []byte) error { // addMetadata writes the metadata structure to the file with the specified // path. This will overwrite any existing data. The operation is atomic. -func (m *Mount) addMetadata(path string, md metadata.Metadata) error { +func (m *Mount) addMetadata(path string, md metadata.Metadata, owner *user.User) error { if err := md.CheckValidity(); err != nil { return errors.Wrap(err, "provided metadata is invalid") } @@ -439,7 +449,7 @@ func (m *Mount) addMetadata(path string, md metadata.Metadata) error { } log.Printf("writing metadata to %q", path) - return m.writeDataAtomic(path, data) + return m.writeDataAtomic(path, data, owner) } // getMetadata reads the metadata structure from the file with the specified @@ -480,7 +490,8 @@ func (m *Mount) removeMetadata(path string) error { // will fail with ErrLinkedProtector if a linked protector with this descriptor // already exists on the filesystem. func (m *Mount) AddProtector(data *metadata.ProtectorData) error { - if err := m.CheckSetup(); err != nil { + var err error + if err = m.CheckSetup(); err != nil { return err } if isRegularFile(m.linkedProtectorPath(data.ProtectorDescriptor)) { @@ -488,7 +499,15 @@ func (m *Mount) AddProtector(data *metadata.ProtectorData) error { data.ProtectorDescriptor, m.Path) } path := m.protectorPath(data.ProtectorDescriptor) - return m.addMetadata(path, data) + + var owner *user.User + if data.Source == metadata.SourceType_pam_passphrase && util.IsUserRoot() { + owner, err = util.UserFromUID(data.Uid) + if err != nil { + return err + } + } + return m.addMetadata(path, data, owner) } // AddLinkedProtector adds a link in this filesystem to the protector metadata @@ -528,7 +547,7 @@ func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount) (bool, error) if err != nil { return false, err } - return true, m.writeDataAtomic(linkPath, []byte(newLink)) + return true, m.writeDataAtomic(linkPath, []byte(newLink), nil) } // GetRegularProtector looks up the protector metadata by descriptor. This will @@ -609,7 +628,7 @@ func (m *Mount) AddPolicy(data *metadata.PolicyData) error { return err } - return m.addMetadata(m.PolicyPath(data.KeyDescriptor), data) + return m.addMetadata(m.PolicyPath(data.KeyDescriptor), data, nil) } // GetPolicy looks up the policy metadata by descriptor. diff --git a/util/util.go b/util/util.go index d97a7ae..1dab335 100644 --- a/util/util.go +++ b/util/util.go @@ -121,9 +121,14 @@ func AtoiOrPanic(input string) int { return i } +// UserFromUID returns the User corresponding to the given user id. +func UserFromUID(uid int64) (*user.User, error) { + return user.LookupId(strconv.FormatInt(uid, 10)) +} + // EffectiveUser returns the user entry corresponding to the effective user. func EffectiveUser() (*user.User, error) { - return user.LookupId(strconv.Itoa(os.Geteuid())) + return UserFromUID(int64(os.Geteuid())) } // IsUserRoot checks if the effective user is root. @@ -131,6 +136,13 @@ func IsUserRoot() bool { return os.Geteuid() == 0 } +// Chown changes the owner of a File to a User. +func Chown(file *os.File, user *user.User) error { + uid := AtoiOrPanic(user.Uid) + gid := AtoiOrPanic(user.Gid) + return file.Chown(uid, gid) +} + // IsKernelVersionAtLeast returns true if the Linux kernel version is at least // major.minor. If something goes wrong it assumes false. func IsKernelVersionAtLeast(major, minor int) bool { -- cgit v1.2.3