From bd380777d68816b55da85a42d4cdf7fb262b4ba2 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 23 Feb 2022 12:35:04 -0800
Subject: [PATCH 01/13] Make the output of 'fscrypt status' unambiguous

Following the example of /proc/self/mountinfo, replace the space,
newline, tab, and backslash characters with octal escape sequences so
that the output can be parsed unambiguously.
---
 cmd/fscrypt/status.go         |  7 +++++--
 filesystem/mountpoint.go      | 15 +++++++++++++++
 filesystem/mountpoint_test.go | 15 +++++++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/cmd/fscrypt/status.go b/cmd/fscrypt/status.go
index 255bb2be..d10dfd8c 100644
--- a/cmd/fscrypt/status.go
+++ b/cmd/fscrypt/status.go
@@ -114,8 +114,11 @@ func writeGlobalStatus(w io.Writer) error {
 			continue
 		}
 
-		fmt.Fprintf(t, "%s\t%s\t%s\t%s\t%s\n", mount.Path, mount.Device,
-			mount.FilesystemType, supportString, yesNoString(usingFscrypt))
+		fmt.Fprintf(t, "%s\t%s\t%s\t%s\t%s\n",
+			filesystem.EscapeString(mount.Path),
+			filesystem.EscapeString(mount.Device),
+			filesystem.EscapeString(mount.FilesystemType),
+			supportString, yesNoString(usingFscrypt))
 
 		if supportErr == nil {
 			supportCount++
diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go
index 182cafa1..0b0693b2 100644
--- a/filesystem/mountpoint.go
+++ b/filesystem/mountpoint.go
@@ -77,6 +77,21 @@ func unescapeString(str string) string {
 	return sb.String()
 }
 
+// EscapeString is the reverse of unescapeString.  Use this to avoid injecting
+// spaces or newlines into output that uses these characters as separators.
+func EscapeString(str string) string {
+	var sb strings.Builder
+	for _, b := range []byte(str) {
+		switch b {
+		case ' ', '\t', '\n', '\\':
+			sb.WriteString(fmt.Sprintf("\\%03o", b))
+		default:
+			sb.WriteByte(b)
+		}
+	}
+	return sb.String()
+}
+
 // We get the device name via the device number rather than use the mount source
 // field directly.  This is necessary to handle a rootfs that was mounted via
 // the kernel command line, since mountinfo always shows /dev/root for that.
diff --git a/filesystem/mountpoint_test.go b/filesystem/mountpoint_test.go
index 749e5e3b..99d4b55d 100644
--- a/filesystem/mountpoint_test.go
+++ b/filesystem/mountpoint_test.go
@@ -179,6 +179,21 @@ func TestLoadMountWithSpecialCharacters(t *testing.T) {
 	}
 }
 
+// Tests the EscapeString() and unescapeString() functions.
+func TestStringEscaping(t *testing.T) {
+	charsNeedEscaping := " \t\n\\"
+	charsDontNeedEscaping := "ABCDEF\u2603\xff\xff\v"
+
+	orig := charsNeedEscaping + charsDontNeedEscaping
+	escaped := `\040\011\012\134` + charsDontNeedEscaping
+	if EscapeString(orig) != escaped {
+		t.Fatal("EscapeString gave wrong result")
+	}
+	if unescapeString(escaped) != orig {
+		t.Fatal("unescapeString gave wrong result")
+	}
+}
+
 // Test parsing some invalid mountinfo lines.
 func TestLoadBadMountInfo(t *testing.T) {
 	mountinfos := []string{"a",

From fa1a1fdbdea65829ce24a6b6f86ce2961e465b02 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 23 Feb 2022 12:35:04 -0800
Subject: [PATCH 02/13] bash_completion: fix command injection and incorrect
 completions

Mountpoint paths might be untrusted arbitrary strings; the fscrypt bash
completion script might need to complete to such strings.
Unfortunately, the design of bash completion places some major footguns
in the way of doing this correctly and securely:

   - "compgen -W" expands anything passed to it, so the argument to -W
     must be single-quoted to avoid an extra level of expansion.

   - The backslashes needed to escape meta-characters in the completed
     text aren't added automatically; they must be explicitly added.

Note that the completion script for 'umount' used to have these same
bugs (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=892179,
https://github.com/util-linux/util-linux/issues/539).

Fix these bugs in roughly the same way that 'umount' fixed them.
---
 cmd/fscrypt/fscrypt_bash_completion | 78 +++++++++++++++++++++++------
 1 file changed, 62 insertions(+), 16 deletions(-)

diff --git a/cmd/fscrypt/fscrypt_bash_completion b/cmd/fscrypt/fscrypt_bash_completion
index 00ee490c..110d2d45 100644
--- a/cmd/fscrypt/fscrypt_bash_completion
+++ b/cmd/fscrypt/fscrypt_bash_completion
@@ -15,25 +15,71 @@
 # License for the specific language governing permissions and limitations under
 # the License.
 
-
-# Prefer completion script style COMPREPLY=($(...)) assignment.
+#
+# bash completion scripts require exercising some unusual shell script
+# features/quirks, so we have to disable some shellcheck warnings:
+#
+# Disable SC2016 ("Expressions don't expand in single quotes, use double quotes
+# for that") because the 'compgen' built-in expands the argument passed to -W,
+# so that argument *must* be single-quoted to avoid command injection.
+# shellcheck disable=SC2016
+#
+# Disable SC2034 ("{Variable} appears unused. Verify use (or export if used
+# externally)") because of the single quoting mentioned above as well as the
+# fact that we have to declare "local" variables used only by a called function
+# (_init_completion()) and not by the function itself.
+# shellcheck disable=SC2034
+#
+# Disable SC2207 ("Prefer mapfile or read -a to split command output (or quote
+# to avoid splitting)") because bash completion scripts conventionally use
+# COMPREPLY=($(...)) assignments.
 # shellcheck disable=SC2207
-true  # To apply shellcheck directive to all file
+#
+true  # To apply the above shellcheck directives to the entire file
 
 
-# Output list of possible mount points
-_fscrypt_mountpoints()
+# Generate the completion list for possible mountpoints.
+#
+# We need to be super careful here because mountpoints can contain whitespace
+# and shell meta-characters.  To avoid most problems, we do the following:
+#
+#   1.) To avoid parsing ambiguities, 'fscrypt status' replaces the space, tab,
+#       newline, and backslash characters with octal escape sequences -- like
+#       what /proc/self/mountinfo does.  To properly process its output, we need
+#       to split lines on space only (and not on other whitespace which might
+#       not be escaped), and unescape these characters.  Exception: we don't
+#       unescape newlines, as we need to reserve newline as the separator for
+#       the words passed to compgen.  (This causes mountpoints containing
+#       newlines to not be completed correctly, which we have to tolerate.)
+#
+#   2.) We backslash-escape all shell meta-characters, and single-quote the
+#       argument passed to compgen -W.  Without either step, command injection
+#       would be possible.  Without both steps, completions would be incorrect.
+#       The list of shell meta-characters used comes from that used by the
+#       completion script for umount, which has to solve this same problem.
+#
+_fscrypt_compgen_mountpoints()
 {
-    # shellcheck disable=SC2016
-    fscrypt status 2>/dev/null | \
-        command awk 'substr($0, 1, 1) == "/" && $5 == "Yes" { print $1 }'
+    local IFS=$'\n'
+    compgen -W '$(_fscrypt_mountpoints_internal)' -- "${cur}"
 }
 
+_fscrypt_mountpoints_internal()
+{
+    fscrypt status 2>/dev/null | command awk -F " " \
+    'substr($0, 1, 1) == "/" && $5 == "Yes" {
+        gsub(/\\040/, " ", $1)
+        gsub(/\\011/, "\t", $1)
+        gsub(/\\134/, "\\", $1)
+        gsub(/[\]\[(){}<>",:;^&!$=?`|'\''\\ \t\f\n\r\v]/, "\\\\&", $1)
+        print $1
+    }'
+}
 
 # Complete with all possible mountpoints
 _fscrypt_complete_mountpoint()
 {
-    COMPREPLY=($(compgen -W "$(_fscrypt_mountpoints)" -- "${cur}"))
+    COMPREPLY=($(_fscrypt_compgen_mountpoints))
 }
 
 
@@ -43,7 +89,6 @@ _fscrypt_complete_mountpoint()
 _fscrypt_status_section()
 {
     local section=${2^^}
-    # shellcheck disable=SC2016
     fscrypt status "$1" 2>/dev/null | \
         command awk '/^[[:xdigit:]]{16}/ && section == "'"$section"'" { print $1; next; }
                      { section = $1 }'
@@ -57,13 +102,13 @@ _fscrypt_complete_policy_or_protector()
     if [[ $cur = *:* ]]; then
         # Complete with IDs of the given mountpoint
         local mountpoint="${cur%:*}" id="${cur#*:}"
+        # Note: compgen expands the argument to -W, so it *must* be single-quoted.
         COMPREPLY=($(compgen \
-            -W "$(_fscrypt_status_section "${mountpoint}" "${status_section}")" \
+            -W '$(_fscrypt_status_section "${mountpoint}" "${status_section}")' \
             -- "${id}"))
     else
         # Complete with mountpoints, with colon and without ending space
-        COMPREPLY=($(compgen -W "$(_fscrypt_mountpoints)" \
-                     -- "${cur}" | sed s/\$/:/))
+        COMPREPLY=($(_fscrypt_compgen_mountpoints | sed s/\$/:/))
         compopt -o nospace
     fi
 }
@@ -72,7 +117,8 @@ _fscrypt_complete_policy_or_protector()
 # Complete with all arguments of that function
 _fscrypt_complete_word()
 {
-    COMPREPLY=($(compgen -W "$*" -- "${cur}"))
+    # Note: compgen expands the argument to -W, so it *must* be single-quoted.
+    COMPREPLY=($(compgen -W '$*' -- "${cur}"))
 }
 
 
@@ -82,7 +128,8 @@ _fscrypt_complete_option()
     local additional_opts=( "$@" )
     # Add global options, always correct
     additional_opts+=( --verbose --quiet --help )
-    COMPREPLY=($(compgen -W "${additional_opts[*]}" -- "${cur}"))
+    # Note: compgen expands the argument to -W, so it *must* be single-quoted.
+    COMPREPLY=($(compgen -W '${additional_opts[*]}' -- "${cur}"))
 }
 
 
@@ -95,7 +142,6 @@ _fscrypt()
     #
     # `split` is set by `_init_completion -s`, we must declare it local
     # even if we don't use it, not to modify the environment.
-    # shellcheck disable=SC2034
     local cur prev words cword split
     _init_completion -s -n : || return
 

From 1a47718420317f893831b0223153d56005d5b02b Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 23 Feb 2022 12:35:04 -0800
Subject: [PATCH 03/13] filesystem: validate size and type of metadata files

Don't allow reading metadata files that are very large, as they can
crash the program due to the memory required.  Similarly, don't allow
reading metadata files that aren't regular files, such as FIFOs, or
symlinks (which could point to a device node like /dev/zero), as that
can hang the program.  Both issues were particularly problematic for
pam_fscrypt, as they could prevent users from being able to log in.

Note: these checks are arguably unneeded if we strictly check the file
ownership too, which a later commit will do.  But there's no reason not
to do these basic checks too.
---
 filesystem/filesystem.go      | 65 ++++++++++++++++++++++++++++++++---
 filesystem/filesystem_test.go | 63 +++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+), 4 deletions(-)

diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go
index da6b9cc0..3c44160a 100644
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -34,6 +34,7 @@ package filesystem
 
 import (
 	"fmt"
+	"io"
 	"io/ioutil"
 	"log"
 	"os"
@@ -213,6 +214,12 @@ const (
 	// The metadata files are globally visible, but can only be deleted by
 	// the user that created them
 	filePermissions = 0644
+
+	// Maximum size of a metadata file.  This value is arbitrary, and it can
+	// be changed.  We just set a reasonable limit that shouldn't be reached
+	// in practice, except by users trying to cause havoc by creating
+	// extremely large files in the metadata directories.
+	maxMetadataFileSize = 16384
 )
 
 func (m *Mount) String() string {
@@ -496,10 +503,60 @@ func (m *Mount) addMetadata(path string, md metadata.Metadata, owner *user.User)
 	return m.writeDataAtomic(path, data, owner)
 }
 
+// readMetadataFileSafe gets the contents of a metadata file extra-carefully,
+// considering that it could be a malicious file created to cause a
+// denial-of-service.  Specifically, the following checks are done:
+//
+// - It must be a regular file, not another type of file like a symlink or FIFO.
+//   (Symlinks aren't bad by themselves, but given that a malicious user could
+//   point one to absolutely anywhere, and there is no known use case for the
+//   metadata files themselves being symlinks, it seems best to disallow them.)
+// - It must have a reasonable size (<= maxMetadataFileSize).
+//
+// Take care to avoid TOCTOU (time-of-check-time-of-use) bugs when doing these
+// tests.  Notably, we must open the file before checking the file type, as the
+// file type could change between any previous checks and the open.  When doing
+// this, O_NOFOLLOW is needed to avoid following a symlink (this applies to the
+// last path component only), and O_NONBLOCK is needed to avoid blocking if the
+// file is a FIFO.
+//
+// This function returns the data read.
+func readMetadataFileSafe(path string) ([]byte, error) {
+	file, err := os.OpenFile(path, os.O_RDONLY|unix.O_NOFOLLOW|unix.O_NONBLOCK, 0)
+	if err != nil {
+		return nil, err
+	}
+	defer file.Close()
+
+	info, err := file.Stat()
+	if err != nil {
+		return nil, err
+	}
+	if !info.Mode().IsRegular() {
+		return nil, &ErrCorruptMetadata{path, errors.New("not a regular file")}
+	}
+	// Clear O_NONBLOCK, since it has served its purpose when opening the
+	// file, and the behavior of reading from a regular file with O_NONBLOCK
+	// is technically unspecified.
+	if _, err = unix.FcntlInt(file.Fd(), unix.F_SETFL, 0); err != nil {
+		return nil, &os.PathError{Op: "clearing O_NONBLOCK", Path: path, Err: err}
+	}
+	// Read the file contents, allowing at most maxMetadataFileSize bytes.
+	reader := &io.LimitedReader{R: file, N: maxMetadataFileSize + 1}
+	data, err := ioutil.ReadAll(reader)
+	if err != nil {
+		return nil, err
+	}
+	if reader.N == 0 {
+		return nil, &ErrCorruptMetadata{path, errors.New("metadata file size limit exceeded")}
+	}
+	return data, nil
+}
+
 // getMetadata reads the metadata structure from the file with the specified
 // path. Only reads normal metadata files, not linked metadata.
 func (m *Mount) getMetadata(path string, md metadata.Metadata) error {
-	data, err := ioutil.ReadFile(path)
+	data, err := readMetadataFileSafe(path)
 	if err != nil {
 		log.Printf("could not read metadata from %q: %v", path, err)
 		return err
@@ -569,7 +626,7 @@ func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount) (bool, error)
 	linkPath := m.linkedProtectorPath(descriptor)
 
 	// Check whether the link already exists.
-	existingLink, err := ioutil.ReadFile(linkPath)
+	existingLink, err := readMetadataFileSafe(linkPath)
 	if err == nil {
 		existingLinkedMnt, err := getMountFromLink(string(existingLink))
 		if err != nil {
@@ -594,7 +651,7 @@ func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount) (bool, error)
 }
 
 // GetRegularProtector looks up the protector metadata by descriptor. This will
-// fail with ErrNoMetadata if the descriptor is a linked protector.
+// fail with ErrProtectorNotFound if the descriptor is a linked protector.
 func (m *Mount) GetRegularProtector(descriptor string) (*metadata.ProtectorData, error) {
 	if err := m.CheckSetup(); err != nil {
 		return nil, err
@@ -617,7 +674,7 @@ func (m *Mount) GetProtector(descriptor string) (*Mount, *metadata.ProtectorData
 	}
 	// Get the link data from the link file
 	path := m.linkedProtectorPath(descriptor)
-	link, err := ioutil.ReadFile(path)
+	link, err := readMetadataFileSafe(path)
 	if err != nil {
 		// If the link doesn't exist, try for a regular protector.
 		if os.IsNotExist(err) {
diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go
index 92726b28..71724ae1 100644
--- a/filesystem/filesystem_test.go
+++ b/filesystem/filesystem_test.go
@@ -24,9 +24,11 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+	"syscall"
 	"testing"
 
 	"github.com/golang/protobuf/proto"
+	"golang.org/x/sys/unix"
 
 	"github.com/google/fscrypt/crypto"
 	"github.com/google/fscrypt/metadata"
@@ -382,3 +384,64 @@ func TestLinkedProtector(t *testing.T) {
 		t.Errorf("protector %+v does not equal expected protector %+v", retProtector, protector)
 	}
 }
+
+func createFile(path string, size int64) error {
+	if err := ioutil.WriteFile(path, []byte{}, 0600); err != nil {
+		return err
+	}
+	return os.Truncate(path, size)
+}
+
+// Tests the readMetadataFileSafe() function.
+func TestReadMetadataFileSafe(t *testing.T) {
+	tempDir, err := ioutil.TempDir("", "fscrypt")
+	if err != nil {
+		t.Fatal(err)
+	}
+	filePath := filepath.Join(tempDir, "file")
+	defer os.RemoveAll(tempDir)
+
+	// Good file (control case)
+	if err = createFile(filePath, 1000); err != nil {
+		t.Fatal(err)
+	}
+	if _, err = readMetadataFileSafe(filePath); err != nil {
+		t.Fatal("failed to read file")
+	}
+	os.Remove(filePath)
+
+	// Nonexistent file
+	_, err = readMetadataFileSafe(filePath)
+	if !os.IsNotExist(err) {
+		t.Fatal("trying to read nonexistent file didn't fail with expected error")
+	}
+
+	// Symlink
+	if err = os.Symlink("target", filePath); err != nil {
+		t.Fatal(err)
+	}
+	_, err = readMetadataFileSafe(filePath)
+	if err.(*os.PathError).Err != syscall.ELOOP {
+		t.Fatal("trying to read symlink didn't fail with ELOOP")
+	}
+	os.Remove(filePath)
+
+	// FIFO
+	if err = unix.Mkfifo(filePath, 0600); err != nil {
+		t.Fatal(err)
+	}
+	_, err = readMetadataFileSafe(filePath)
+	if _, ok := err.(*ErrCorruptMetadata); !ok {
+		t.Fatal("trying to read FIFO didn't fail with expected error")
+	}
+	os.Remove(filePath)
+
+	// Very large file
+	if err = createFile(filePath, 1000000); err != nil {
+		t.Fatal(err)
+	}
+	_, err = readMetadataFileSafe(filePath)
+	if _, ok := err.(*ErrCorruptMetadata); !ok {
+		t.Fatal("trying to read very large file didn't fail with expected error")
+	}
+}

From b44fbe71e1e93c47050322af51725bac997641e0 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 23 Feb 2022 12:35:04 -0800
Subject: [PATCH 04/13] filesystem: reject spoofed login protectors

If a login protector contains a UID that differs from the file owner
(and the file owner is not root), it might be a spoofed file that was
created maliciously, so make sure to consider such files to be invalid.
---
 filesystem/filesystem.go      | 57 ++++++++++++++++++----------
 filesystem/filesystem_test.go | 71 ++++++++++++++++++++++++++++++++---
 2 files changed, 103 insertions(+), 25 deletions(-)

diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go
index 3c44160a..8ce8bde5 100644
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -42,6 +42,7 @@ import (
 	"path/filepath"
 	"sort"
 	"strings"
+	"syscall"
 	"time"
 
 	"github.com/golang/protobuf/proto"
@@ -520,58 +521,60 @@ func (m *Mount) addMetadata(path string, md metadata.Metadata, owner *user.User)
 // last path component only), and O_NONBLOCK is needed to avoid blocking if the
 // file is a FIFO.
 //
-// This function returns the data read.
-func readMetadataFileSafe(path string) ([]byte, error) {
+// This function returns the data read as well as the UID of the user who owns
+// the file.  The returned UID is needed for login protectors, where the UID
+// needs to be cross-checked with the UID stored in the file itself.
+func readMetadataFileSafe(path string) ([]byte, int64, error) {
 	file, err := os.OpenFile(path, os.O_RDONLY|unix.O_NOFOLLOW|unix.O_NONBLOCK, 0)
 	if err != nil {
-		return nil, err
+		return nil, -1, err
 	}
 	defer file.Close()
 
 	info, err := file.Stat()
 	if err != nil {
-		return nil, err
+		return nil, -1, err
 	}
 	if !info.Mode().IsRegular() {
-		return nil, &ErrCorruptMetadata{path, errors.New("not a regular file")}
+		return nil, -1, &ErrCorruptMetadata{path, errors.New("not a regular file")}
 	}
 	// Clear O_NONBLOCK, since it has served its purpose when opening the
 	// file, and the behavior of reading from a regular file with O_NONBLOCK
 	// is technically unspecified.
 	if _, err = unix.FcntlInt(file.Fd(), unix.F_SETFL, 0); err != nil {
-		return nil, &os.PathError{Op: "clearing O_NONBLOCK", Path: path, Err: err}
+		return nil, -1, &os.PathError{Op: "clearing O_NONBLOCK", Path: path, Err: err}
 	}
 	// Read the file contents, allowing at most maxMetadataFileSize bytes.
 	reader := &io.LimitedReader{R: file, N: maxMetadataFileSize + 1}
 	data, err := ioutil.ReadAll(reader)
 	if err != nil {
-		return nil, err
+		return nil, -1, err
 	}
 	if reader.N == 0 {
-		return nil, &ErrCorruptMetadata{path, errors.New("metadata file size limit exceeded")}
+		return nil, -1, &ErrCorruptMetadata{path, errors.New("metadata file size limit exceeded")}
 	}
-	return data, nil
+	return data, int64(info.Sys().(*syscall.Stat_t).Uid), nil
 }
 
 // getMetadata reads the metadata structure from the file with the specified
 // path. Only reads normal metadata files, not linked metadata.
-func (m *Mount) getMetadata(path string, md metadata.Metadata) error {
-	data, err := readMetadataFileSafe(path)
+func (m *Mount) getMetadata(path string, md metadata.Metadata) (int64, error) {
+	data, owner, err := readMetadataFileSafe(path)
 	if err != nil {
 		log.Printf("could not read metadata from %q: %v", path, err)
-		return err
+		return -1, err
 	}
 
 	if err := proto.Unmarshal(data, md); err != nil {
-		return &ErrCorruptMetadata{path, err}
+		return -1, &ErrCorruptMetadata{path, err}
 	}
 
 	if err := md.CheckValidity(); err != nil {
-		return &ErrCorruptMetadata{path, err}
+		return -1, &ErrCorruptMetadata{path, err}
 	}
 
 	log.Printf("successfully read metadata from %q", path)
-	return nil
+	return owner, nil
 }
 
 // removeMetadata deletes the metadata struct from the file with the specified
@@ -626,7 +629,7 @@ func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount) (bool, error)
 	linkPath := m.linkedProtectorPath(descriptor)
 
 	// Check whether the link already exists.
-	existingLink, err := readMetadataFileSafe(linkPath)
+	existingLink, _, err := readMetadataFileSafe(linkPath)
 	if err == nil {
 		existingLinkedMnt, err := getMountFromLink(string(existingLink))
 		if err != nil {
@@ -658,11 +661,25 @@ func (m *Mount) GetRegularProtector(descriptor string) (*metadata.ProtectorData,
 	}
 	data := new(metadata.ProtectorData)
 	path := m.protectorPath(descriptor)
-	err := m.getMetadata(path, data)
+	owner, err := m.getMetadata(path, data)
 	if os.IsNotExist(err) {
 		err = &ErrProtectorNotFound{descriptor, m}
 	}
-	return data, err
+	if err != nil {
+		return nil, err
+	}
+	// Login protectors have their UID stored in the file.  Since normally
+	// any user can create files in the fscrypt metadata directories, for a
+	// login protector to be considered valid it *must* be owned by the
+	// claimed user or by root.  Note: fscrypt v0.3.2 and later always makes
+	// login protectors owned by the user, but previous versions could
+	// create them owned by root -- that is the main reason we allow root.
+	if data.Source == metadata.SourceType_pam_passphrase && owner != 0 && owner != data.Uid {
+		log.Printf("WARNING: %q claims to be the login protector for uid %d, but it is owned by uid %d.  Needs to be %d or 0.",
+			path, data.Uid, owner, data.Uid)
+		return nil, &ErrCorruptMetadata{path, errors.New("login protector belongs to wrong user")}
+	}
+	return data, nil
 }
 
 // GetProtector returns the Mount of the filesystem containing the information
@@ -674,7 +691,7 @@ func (m *Mount) GetProtector(descriptor string) (*Mount, *metadata.ProtectorData
 	}
 	// Get the link data from the link file
 	path := m.linkedProtectorPath(descriptor)
-	link, err := readMetadataFileSafe(path)
+	link, _, err := readMetadataFileSafe(path)
 	if err != nil {
 		// If the link doesn't exist, try for a regular protector.
 		if os.IsNotExist(err) {
@@ -737,7 +754,7 @@ func (m *Mount) GetPolicy(descriptor string) (*metadata.PolicyData, error) {
 		return nil, err
 	}
 	data := new(metadata.PolicyData)
-	err := m.getMetadata(m.PolicyPath(descriptor), data)
+	_, err := m.getMetadata(m.PolicyPath(descriptor), data)
 	if os.IsNotExist(err) {
 		err = &ErrPolicyNotFound{descriptor, m}
 	}
diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go
index 71724ae1..7aa97cb4 100644
--- a/filesystem/filesystem_test.go
+++ b/filesystem/filesystem_test.go
@@ -60,6 +60,19 @@ func getFakeProtector() *metadata.ProtectorData {
 	}
 }
 
+func getFakeLoginProtector(uid int64) *metadata.ProtectorData {
+	protector := getFakeProtector()
+	protector.Source = metadata.SourceType_pam_passphrase
+	protector.Uid = uid
+	protector.Costs = &metadata.HashingCosts{
+		Time:        1,
+		Memory:      1 << 8,
+		Parallelism: 1,
+	}
+	protector.Salt = make([]byte, 16)
+	return protector
+}
+
 func getFakePolicy() *metadata.PolicyData {
 	return &metadata.PolicyData{
 		KeyDescriptor: "0123456789abcdef",
@@ -315,6 +328,50 @@ func TestSetProtector(t *testing.T) {
 	}
 }
 
+// Tests that a login protector whose embedded UID doesn't match the file owner
+// is considered invalid.  (Such a file could be created by a malicious user to
+// try to confuse fscrypt into processing the wrong file.)
+func TestSpoofedLoginProtector(t *testing.T) {
+	myUID := int64(os.Geteuid())
+	badUID := myUID + 1 // anything different from myUID
+	mnt, err := getSetupMount(t)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer mnt.RemoveAllMetadata()
+
+	// Control case: protector with matching UID should be accepted.
+	protector := getFakeLoginProtector(myUID)
+	if err = mnt.AddProtector(protector); err != nil {
+		t.Fatal(err)
+	}
+	_, err = mnt.GetRegularProtector(protector.ProtectorDescriptor)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err = mnt.RemoveProtector(protector.ProtectorDescriptor); err != nil {
+		t.Fatal(err)
+	}
+
+	// The real test: protector with mismatching UID should rejected,
+	// *unless* the process running the tests (and hence the file owner) is
+	// root in which case it should be accepted.
+	protector = getFakeLoginProtector(badUID)
+	if err = mnt.AddProtector(protector); err != nil {
+		t.Fatal(err)
+	}
+	_, err = mnt.GetRegularProtector(protector.ProtectorDescriptor)
+	if myUID == 0 {
+		if err != nil {
+			t.Fatal(err)
+		}
+	} else {
+		if err == nil {
+			t.Fatal("reading protector with bad UID unexpectedly succeeded")
+		}
+	}
+}
+
 // Gets a setup mount and a fake second mount
 func getTwoSetupMounts(t *testing.T) (realMnt, fakeMnt *Mount, err error) {
 	if realMnt, err = getSetupMount(t); err != nil {
@@ -405,13 +462,17 @@ func TestReadMetadataFileSafe(t *testing.T) {
 	if err = createFile(filePath, 1000); err != nil {
 		t.Fatal(err)
 	}
-	if _, err = readMetadataFileSafe(filePath); err != nil {
+	_, owner, err := readMetadataFileSafe(filePath)
+	if err != nil {
 		t.Fatal("failed to read file")
 	}
+	if owner != int64(os.Geteuid()) {
+		t.Fatal("got wrong owner")
+	}
 	os.Remove(filePath)
 
 	// Nonexistent file
-	_, err = readMetadataFileSafe(filePath)
+	_, _, err = readMetadataFileSafe(filePath)
 	if !os.IsNotExist(err) {
 		t.Fatal("trying to read nonexistent file didn't fail with expected error")
 	}
@@ -420,7 +481,7 @@ func TestReadMetadataFileSafe(t *testing.T) {
 	if err = os.Symlink("target", filePath); err != nil {
 		t.Fatal(err)
 	}
-	_, err = readMetadataFileSafe(filePath)
+	_, _, err = readMetadataFileSafe(filePath)
 	if err.(*os.PathError).Err != syscall.ELOOP {
 		t.Fatal("trying to read symlink didn't fail with ELOOP")
 	}
@@ -430,7 +491,7 @@ func TestReadMetadataFileSafe(t *testing.T) {
 	if err = unix.Mkfifo(filePath, 0600); err != nil {
 		t.Fatal(err)
 	}
-	_, err = readMetadataFileSafe(filePath)
+	_, _, err = readMetadataFileSafe(filePath)
 	if _, ok := err.(*ErrCorruptMetadata); !ok {
 		t.Fatal("trying to read FIFO didn't fail with expected error")
 	}
@@ -440,7 +501,7 @@ func TestReadMetadataFileSafe(t *testing.T) {
 	if err = createFile(filePath, 1000000); err != nil {
 		t.Fatal(err)
 	}
-	_, err = readMetadataFileSafe(filePath)
+	_, _, err = readMetadataFileSafe(filePath)
 	if _, ok := err.(*ErrCorruptMetadata); !ok {
 		t.Fatal("trying to read very large file didn't fail with expected error")
 	}

From 45599bdfad300f1a034c70dd70b4bd180d66f52c Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 23 Feb 2022 12:35:04 -0800
Subject: [PATCH 05/13] filesystem: fall back to non-atomic overwrites when
 required

To allow users to update fscrypt metadata they own in
single-user-writable metadata directories (introduced by the next
commit), fall back to non-atomic overwrites when atomic ones can't be
done due to not having write access to the directory.
---
 filesystem/filesystem.go | 43 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go
index 8ce8bde5..c39514ac 100644
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -444,14 +444,47 @@ func syncDirectory(dirPath string) error {
 	return dirFile.Close()
 }
 
-// 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, owner *user.User) error {
+func (m *Mount) overwriteDataNonAtomic(path string, data []byte) error {
+	file, err := os.OpenFile(path, os.O_WRONLY|os.O_TRUNC|unix.O_NOFOLLOW, 0)
+	if err != nil {
+		return err
+	}
+	if _, err = file.Write(data); err != nil {
+		log.Printf("WARNING: overwrite of %q failed; file will be corrupted!", path)
+		file.Close()
+		return err
+	}
+	if err = file.Sync(); err != nil {
+		file.Close()
+		return err
+	}
+	if err = file.Close(); err != nil {
+		return err
+	}
+	log.Printf("successfully overwrote %q non-atomically", path)
+	return nil
+}
+
+// writeData writes the given data to the given path such that, if possible, the
+// data is either written to stable storage or an error is returned.  If a file
+// already exists at the path, it will be replaced.
+//
+// However, if the process doesn't have write permission to the directory but
+// does have write permission to the file itself, then as a fallback the file is
+// overwritten in-place rather than replaced.  Note that this may be non-atomic.
+func (m *Mount) writeData(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)
 	tempFile, err := ioutil.TempFile(dirPath, tempPrefix)
 	if err != nil {
+		log.Print(err)
+		if os.IsPermission(err) {
+			if _, err = os.Lstat(path); err == nil {
+				log.Printf("trying non-atomic overwrite of %q", path)
+				return m.overwriteDataNonAtomic(path, data)
+			}
+		}
 		return err
 	}
 	defer os.Remove(tempFile.Name())
@@ -501,7 +534,7 @@ func (m *Mount) addMetadata(path string, md metadata.Metadata, owner *user.User)
 	}
 
 	log.Printf("writing metadata to %q", path)
-	return m.writeDataAtomic(path, data, owner)
+	return m.writeData(path, data, owner)
 }
 
 // readMetadataFileSafe gets the contents of a metadata file extra-carefully,
@@ -650,7 +683,7 @@ func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount) (bool, error)
 	if err != nil {
 		return false, err
 	}
-	return true, m.writeDataAtomic(linkPath, []byte(newLink), nil)
+	return true, m.writeData(linkPath, []byte(newLink), nil)
 }
 
 // GetRegularProtector looks up the protector metadata by descriptor. This will

From 6e355131670ad014e45f879475ddf800f0080d41 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 23 Feb 2022 12:35:04 -0800
Subject: [PATCH 06/13] Make 'fscrypt setup' offer a choice of directory modes

World-writable directories are not appropriate for some systems, so
offer a choice of single-user-writable and world-writable modes, with
single-user-writable being the default.  Add a new documentation section
to help users decide which one to use.
---
 README.md                       | 53 ++++++++++++++++++++---
 actions/context_test.go         |  3 +-
 cli-tests/run.sh                |  4 +-
 cli-tests/t_encrypt.out         | 18 +++++---
 cli-tests/t_encrypt_custom.out  |  9 ++--
 cli-tests/t_encrypt_login.out   | 42 ++++++++++++-------
 cli-tests/t_encrypt_raw_key.out | 15 ++++---
 cli-tests/t_metadata.out        |  6 ++-
 cli-tests/t_not_supported.sh    |  2 +-
 cli-tests/t_setup.out           |  6 ++-
 cli-tests/t_setup.sh            |  4 +-
 cli-tests/t_single_user.out     | 30 +++++++++++++
 cli-tests/t_single_user.sh      | 55 ++++++++++++++++++++++++
 cli-tests/t_status.out          |  6 ++-
 cli-tests/t_v1_policy.out       |  3 +-
 cmd/fscrypt/commands.go         |  6 +--
 cmd/fscrypt/errors.go           |  4 ++
 cmd/fscrypt/flags.go            | 14 ++++++-
 cmd/fscrypt/setup.go            | 41 +++++++++++++++++-
 cmd/fscrypt/status.go           | 11 ++++-
 filesystem/filesystem.go        | 74 ++++++++++++++++++++++++++++-----
 filesystem/filesystem_test.go   | 35 ++++++++++++++--
 22 files changed, 374 insertions(+), 67 deletions(-)
 create mode 100644 cli-tests/t_single_user.out
 create mode 100755 cli-tests/t_single_user.sh

diff --git a/README.md b/README.md
index 75b3d621..26fd0840 100644
--- a/README.md
+++ b/README.md
@@ -37,6 +37,7 @@ dependencies](#runtime-dependencies).
 - [Building and installing](#building-and-installing)
 - [Runtime dependencies](#runtime-dependencies)
 - [Configuration file](#configuration-file)
+- [Setting up `fscrypt` on a filesystem](#setting-up-fscrypt-on-a-filesystem)
 - [Setting up for login protectors](#setting-up-for-login-protectors)
   - [Securing your login passphrase](#securing-your-login-passphrase)
   - [Enabling the PAM module](#enabling-the-pam-module)
@@ -377,6 +378,44 @@ The fields are:
   kernels, it's better to not use this setting and instead (re-)create your
   encrypted directories with `"policy_version": "2"`.
 
+## Setting up `fscrypt` on a filesystem
+
+`fscrypt` needs some directories to exist on the filesystem on which encryption
+will be used:
+
+* `MOUNTPOINT/.fscrypt/policies`
+* `MOUNTPOINT/.fscrypt/protectors`
+
+(If login protectors are used, these must also exist on the root filesystem.)
+
+To create these directories, run `fscrypt setup MOUNTPOINT`.  If MOUNTPOINT is
+owned by root, as is usually the case, then this command will require root.
+
+There will be one decision you'll need to make: whether non-root users will be
+allowed to create `fscrypt` metadata (policies and protectors).
+
+If you say `y`, then these directories will be made world-writable, with the
+sticky bit set so that users can't delete each other's files -- just like
+`/tmp`.  If you say `N`, then these directories will be writable only by root.
+
+Saying `y` maximizes the usability of `fscrypt`, and on most systems it's fine
+to say `y`.  However, on some systems this may be inappropriate, as it will
+allow malicious users to fill the entire filesystem unless filesystem quotas
+have been configured -- similar to problems that have historically existed with
+other world-writable directories, e.g. `/tmp`.  If you are concerned about this,
+say `N`.  If you say `N`, then you'll only be able to run `fscrypt` as root to
+set up encryption on users' behalf, unless you manually set custom permissions
+on the metadata directories to grant write access to specific users or groups.
+
+If you chose the wrong mode at `fscrypt setup` time, you can change the
+directory permissions at any time.  To enable single-user writable mode, run:
+
+    sudo chmod 0755 MOUNTPOINT/.fscrypt/*
+
+To enable world-writable mode, run:
+
+    sudo chmod 1777 MOUNTPOINT/.fscrypt/*
+
 ## Setting up for login protectors
 
 If you want any encrypted directories to be protected by your login passphrase,
@@ -646,11 +685,15 @@ MOUNTPOINT  DEVICE     FILESYSTEM  ENCRYPTION   FSCRYPT
 Defaulting to policy_version 2 because kernel supports it.
 Customizing passphrase hashing difficulty for this system...
 Created global config file at "/etc/fscrypt.conf".
-Metadata directories created at "/.fscrypt".
+Allow users other than root to create fscrypt metadata on the root filesystem?
+(See https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem) [y/N] y
+Metadata directories created at "/.fscrypt", writable by everyone.
 
 # Start using fscrypt with our filesystem
->>>>> fscrypt setup /mnt/disk
-Metadata directories created at "/mnt/disk/.fscrypt".
+>>>>> sudo fscrypt setup /mnt/disk
+Allow users other than root to create fscrypt metadata on this filesystem? (See
+https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem) [y/N] y
+Metadata directories created at "/mnt/disk/.fscrypt", writable by everyone.
 
 # Initialize encryption on a new empty directory
 >>>>> mkdir /mnt/disk/dir1
@@ -678,8 +721,8 @@ POLICY                            UNLOCKED  PROTECTORS
 
 #### Quiet version
 ```bash
->>>>> sudo fscrypt setup --quiet --force
->>>>> fscrypt setup /mnt/disk --quiet
+>>>>> sudo fscrypt setup --quiet --force --all-users
+>>>>> sudo fscrypt setup /mnt/disk --quiet --all-users
 >>>>> echo "hunter2" | fscrypt encrypt /mnt/disk/dir1 --quiet --source=custom_passphrase  --name="Super Secret"
 ```
 
diff --git a/actions/context_test.go b/actions/context_test.go
index 7b56d92c..6e288576 100644
--- a/actions/context_test.go
+++ b/actions/context_test.go
@@ -27,6 +27,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/google/fscrypt/filesystem"
 	"github.com/google/fscrypt/util"
 	"github.com/pkg/errors"
 )
@@ -67,7 +68,7 @@ func setupContext() (ctx *Context, err error) {
 		return nil, err
 	}
 
-	return ctx, ctx.Mount.Setup()
+	return ctx, ctx.Mount.Setup(filesystem.WorldWritable)
 }
 
 // Cleans up the testing config file and testing filesystem data.
diff --git a/cli-tests/run.sh b/cli-tests/run.sh
index dc17b5b9..f6a48682 100755
--- a/cli-tests/run.sh
+++ b/cli-tests/run.sh
@@ -159,7 +159,7 @@ setup_for_test()
 
 	# Give the tests their own fscrypt.conf.
 	export FSCRYPT_CONF="$TMPDIR/fscrypt.conf"
-	fscrypt setup --time=1ms > /dev/null
+	fscrypt setup --time=1ms --quiet --all-users > /dev/null
 
 	# The tests assume kernel support for v2 policies.
 	if ! grep -q '"policy_version": "2"' "$FSCRYPT_CONF"; then
@@ -171,7 +171,7 @@ EOF
 	fi
 
 	# Set up the test filesystems that aren't already set up.
-	fscrypt setup "$MNT" > /dev/null
+	fscrypt setup --quiet --all-users "$MNT" > /dev/null
 }
 
 run_test()
diff --git a/cli-tests/t_encrypt.out b/cli-tests/t_encrypt.out
index f067fc0e..b92c9d98 100644
--- a/cli-tests/t_encrypt.out
+++ b/cli-tests/t_encrypt.out
@@ -1,7 +1,8 @@
 
 # Try to encrypt a nonexistent directory
 [ERROR] fscrypt encrypt: no such file or directory
-ext4 filesystem "MNT" has 0 protectors and 0 policies
+ext4 filesystem "MNT" has 0 protectors and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 [ERROR] fscrypt status: file or directory "MNT/dir" is not
                         encrypted
@@ -23,7 +24,8 @@ files into it, and securely delete the original directory. For example:
 Caution: due to the nature of modern storage devices and filesystems, the
 original data may still be recoverable from disk. It's much better to encrypt
 your files from the start.
-ext4 filesystem "MNT" has 0 protectors and 0 policies
+ext4 filesystem "MNT" has 0 protectors and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 [ERROR] fscrypt status: file or directory "MNT/dir" is not
                         encrypted
@@ -45,13 +47,15 @@ files into it, and securely delete the original directory. For example:
 Caution: due to the nature of modern storage devices and filesystems, the
 original data may still be recoverable from disk. It's much better to encrypt
 your files from the start.
-ext4 filesystem "MNT" has 0 protectors and 0 policies
+ext4 filesystem "MNT" has 0 protectors and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 [ERROR] fscrypt status: file or directory "MNT/dir" is not
                         encrypted
 
 # Encrypt a directory as non-root user
-ext4 filesystem "MNT" has 1 protector and 1 policy
+ext4 filesystem "MNT" has 1 protector and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc1  No      custom protector "prot"
@@ -67,7 +71,8 @@ Unlocked: Yes
 Protected with 1 protector:
 PROTECTOR         LINKED  DESCRIPTION
 desc1  No      custom protector "prot"
-ext4 filesystem "MNT" has 1 protector and 1 policy
+ext4 filesystem "MNT" has 1 protector and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc1  No      custom protector "prot"
@@ -94,7 +99,8 @@ desc1  No      custom protector "prot"
 
                          Encryption can only be enabled on a directory you own,
                          even if you have write access to the directory.
-ext4 filesystem "MNT" has 0 protectors and 0 policies
+ext4 filesystem "MNT" has 0 protectors and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 [ERROR] fscrypt status: file or directory "MNT/dir" is not
                         encrypted
diff --git a/cli-tests/t_encrypt_custom.out b/cli-tests/t_encrypt_custom.out
index 8dd15e3a..ac53d6fd 100644
--- a/cli-tests/t_encrypt_custom.out
+++ b/cli-tests/t_encrypt_custom.out
@@ -1,6 +1,7 @@
 
 # Encrypt with custom passphrase protector
-ext4 filesystem "MNT" has 1 protector and 1 policy
+ext4 filesystem "MNT" has 1 protector and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc1  No      custom protector "prot"
@@ -28,7 +29,8 @@ Enter a name for the new protector: prot
 Enter custom passphrase for protector "prot": 
 Confirm passphrase: 
 "MNT/dir" is now encrypted, unlocked, and ready for use.
-ext4 filesystem "MNT" has 1 protector and 1 policy
+ext4 filesystem "MNT" has 1 protector and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc6  No      custom protector "prot"
@@ -49,7 +51,8 @@ desc6  No      custom protector "prot"
 [ERROR] fscrypt encrypt: custom_passphrase protectors must be named
 
 Use --name=PROTECTOR_NAME to specify a protector name.
-ext4 filesystem "MNT" has 0 protectors and 0 policies
+ext4 filesystem "MNT" has 0 protectors and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 [ERROR] fscrypt status: file or directory "MNT/dir" is not
                         encrypted
diff --git a/cli-tests/t_encrypt_login.out b/cli-tests/t_encrypt_login.out
index 269f5978..b84216a1 100644
--- a/cli-tests/t_encrypt_login.out
+++ b/cli-tests/t_encrypt_login.out
@@ -7,7 +7,8 @@ IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for
            will lose access to this directory if you reinstall the operating
            system or move this filesystem to another system.
 
-ext4 filesystem "MNT" has 2 protectors and 1 policy
+ext4 filesystem "MNT" has 2 protectors and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED                              DESCRIPTION
 desc1  Yes (MNT_ROOT)  login protector for fscrypt-test-user
@@ -15,7 +16,8 @@ desc2  No                                  custom protector "Recovery passphrase
 
 POLICY                            UNLOCKED  PROTECTORS
 desc3  Yes       desc1, desc2
-ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies
+ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc1  No      login protector for fscrypt-test-user
@@ -57,7 +59,8 @@ IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for
            system or move this filesystem to another system.
 
 "MNT/dir" is now encrypted, unlocked, and ready for use.
-ext4 filesystem "MNT" has 2 protectors and 1 policy
+ext4 filesystem "MNT" has 2 protectors and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED                              DESCRIPTION
 desc10  Yes (MNT_ROOT)  login protector for fscrypt-test-user
@@ -65,7 +68,8 @@ desc11  No                                  custom protector "Recovery passphras
 
 POLICY                            UNLOCKED  PROTECTORS
 desc12  Yes       desc10, desc11
-ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies
+ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc10  No      login protector for fscrypt-test-user
@@ -88,7 +92,8 @@ IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for
            will lose access to this directory if you reinstall the operating
            system or move this filesystem to another system.
 
-ext4 filesystem "MNT" has 2 protectors and 1 policy
+ext4 filesystem "MNT" has 2 protectors and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED                              DESCRIPTION
 desc19  Yes (MNT_ROOT)  login protector for fscrypt-test-user
@@ -96,7 +101,8 @@ desc20  No                                  custom protector "Recovery passphras
 
 POLICY                            UNLOCKED  PROTECTORS
 desc21  Yes       desc19, desc20
-ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies
+ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc19  No      login protector for fscrypt-test-user
@@ -114,14 +120,16 @@ desc20  No                                  custom protector "Recovery passphras
 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
+ext4 filesystem "MNT" has 1 protector and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED                              DESCRIPTION
 desc28  Yes (MNT_ROOT)  login protector for fscrypt-test-user
 
 POLICY                            UNLOCKED  PROTECTORS
 desc29  Yes       desc28
-ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies
+ext4 filesystem "MNT_ROOT" has 1 protector and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc28  No      login protector for fscrypt-test-user
@@ -145,7 +153,8 @@ Unlocked: Yes
 Protected with 1 protector:
 PROTECTOR         LINKED  DESCRIPTION
 desc35  No      login protector for fscrypt-test-user
-ext4 filesystem "MNT_ROOT" has 1 protector and 1 policy
+ext4 filesystem "MNT_ROOT" has 1 protector and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc35  No      login protector for fscrypt-test-user
@@ -159,18 +168,22 @@ desc34  Yes       desc35
                          identified by user, not by name.
 
 To fix this, don't specify the --name=PROTECTOR_NAME option.
-ext4 filesystem "MNT" has 0 protectors and 0 policies
+ext4 filesystem "MNT" has 0 protectors and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
-ext4 filesystem "MNT_ROOT" has 0 protectors and 0 policies
+ext4 filesystem "MNT_ROOT" has 0 protectors and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 [ERROR] fscrypt status: file or directory "MNT/dir" is not
                         encrypted
 
 # Try to use the wrong login passphrase
 [ERROR] fscrypt encrypt: incorrect login passphrase
-ext4 filesystem "MNT" has 0 protectors and 0 policies
+ext4 filesystem "MNT" has 0 protectors and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
-ext4 filesystem "MNT_ROOT" has 0 protectors and 0 policies
+ext4 filesystem "MNT_ROOT" has 0 protectors and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 [ERROR] fscrypt status: file or directory "MNT/dir" is not
                         encrypted
@@ -183,7 +196,8 @@ IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for
            will lose access to this directory if you reinstall the operating
            system or move this filesystem to another system.
 
-ext4 filesystem "MNT" has 2 protectors and 1 policy
+ext4 filesystem "MNT" has 2 protectors and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED                              DESCRIPTION
 desc39  No                                  custom protector "Recovery passphrase for dir"
diff --git a/cli-tests/t_encrypt_raw_key.out b/cli-tests/t_encrypt_raw_key.out
index 1f51dc0d..4cfc0500 100644
--- a/cli-tests/t_encrypt_raw_key.out
+++ b/cli-tests/t_encrypt_raw_key.out
@@ -1,6 +1,7 @@
 
 # Encrypt with raw_key protector from file
-ext4 filesystem "MNT" has 1 protector and 1 policy
+ext4 filesystem "MNT" has 1 protector and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc1  No      raw key protector "prot"
@@ -18,7 +19,8 @@ PROTECTOR         LINKED  DESCRIPTION
 desc1  No      raw key protector "prot"
 
 # Encrypt with raw_key protector from stdin
-ext4 filesystem "MNT" has 1 protector and 1 policy
+ext4 filesystem "MNT" has 1 protector and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc6  No      raw key protector "prot"
@@ -37,21 +39,24 @@ desc6  No      raw key protector "prot"
 
 # Try to encrypt with raw_key protector from file, using wrong key length
 [ERROR] fscrypt encrypt: TMPDIR/raw_key: key file must be 32 bytes
-ext4 filesystem "MNT" has 0 protectors and 0 policies
+ext4 filesystem "MNT" has 0 protectors and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 [ERROR] fscrypt status: file or directory "MNT/dir" is not
                         encrypted
 
 # Try to encrypt with raw_key protector from stdin, using wrong key length
 [ERROR] fscrypt encrypt: unexpected EOF
-ext4 filesystem "MNT" has 0 protectors and 0 policies
+ext4 filesystem "MNT" has 0 protectors and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 [ERROR] fscrypt status: file or directory "MNT/dir" is not
                         encrypted
 
 # Encrypt with raw_key protector from file, unlock from stdin
 "MNT/dir" is now locked.
-ext4 filesystem "MNT" has 1 protector and 1 policy
+ext4 filesystem "MNT" has 1 protector and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc11  No      raw key protector "prot"
diff --git a/cli-tests/t_metadata.out b/cli-tests/t_metadata.out
index fba816af..bbcc0f29 100644
--- a/cli-tests/t_metadata.out
+++ b/cli-tests/t_metadata.out
@@ -1,4 +1,5 @@
-ext4 filesystem "MNT" has 3 protectors and 1 policy
+ext4 filesystem "MNT" has 3 protectors and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc1  No      custom protector "foo"
@@ -7,7 +8,8 @@ desc3  No      custom protector "baz"
 
 POLICY                            UNLOCKED  PROTECTORS
 desc4  No        desc1, desc2, desc3
-ext4 filesystem "MNT" has 2 protectors and 1 policy
+ext4 filesystem "MNT" has 2 protectors and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc1  No      custom protector "foo"
diff --git a/cli-tests/t_not_supported.sh b/cli-tests/t_not_supported.sh
index 9ff90e17..8b52392d 100755
--- a/cli-tests/t_not_supported.sh
+++ b/cli-tests/t_not_supported.sh
@@ -10,7 +10,7 @@ umount "$MNT"
 mount tmpfs -t tmpfs -o size=128m "$MNT"
 
 _print_header "Try to create fscrypt metadata on tmpfs"
-_expect_failure "fscrypt setup '$MNT'"
+_expect_failure "fscrypt setup --quiet '$MNT'"
 
 _print_header "Try to encrypt a directory on tmpfs"
 mkdir "$MNT/dir"
diff --git a/cli-tests/t_setup.out b/cli-tests/t_setup.out
index 943a7813..6ea03e3f 100644
--- a/cli-tests/t_setup.out
+++ b/cli-tests/t_setup.out
@@ -9,7 +9,8 @@ Skipping creating MNT_ROOT/.fscrypt because it already exists.
 Defaulting to policy_version 2 because kernel supports it.
 Customizing passphrase hashing difficulty for this system...
 Created global config file at "FSCRYPT_CONF".
-Metadata directories created at "MNT_ROOT/.fscrypt".
+Allow users other than root to create fscrypt metadata on this filesystem? (See
+https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem) [y/N] Metadata directories created at "MNT_ROOT/.fscrypt", writable by everyone.
 
 # fscrypt setup when fscrypt.conf already exists (cancel)
 Replace "FSCRYPT_CONF"? [y/N] [ERROR] fscrypt setup: operation canceled
@@ -31,7 +32,8 @@ If desired, use --force to automatically run destructive operations.
 # fscrypt setup --quiet --force when fscrypt.conf already exists
 
 # fscrypt setup filesystem
-Metadata directories created at "MNT/.fscrypt".
+Allow users other than root to create fscrypt metadata on this filesystem? (See
+https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem) [y/N] Metadata directories created at "MNT/.fscrypt", writable by everyone.
 
 # fscrypt setup filesystem (already set up)
 [ERROR] fscrypt setup: filesystem MNT is already setup for
diff --git a/cli-tests/t_setup.sh b/cli-tests/t_setup.sh
index a8a62a37..f7e302d8 100755
--- a/cli-tests/t_setup.sh
+++ b/cli-tests/t_setup.sh
@@ -14,7 +14,7 @@ fscrypt setup --time=1ms
 _print_header "fscrypt setup creates fscrypt.conf and /.fscrypt"
 _rm_metadata "$MNT_ROOT"
 rm -f "$FSCRYPT_CONF"
-fscrypt setup --time=1ms
+echo y | fscrypt setup --time=1ms
 [ -e "$MNT_ROOT/.fscrypt" ]
 
 _print_header "fscrypt setup when fscrypt.conf already exists (cancel)"
@@ -37,7 +37,7 @@ fscrypt setup --quiet --force --time=1ms
 
 _print_header "fscrypt setup filesystem"
 _rm_metadata "$MNT"
-fscrypt setup "$MNT"
+echo y | fscrypt setup "$MNT"
 [ -e "$MNT/.fscrypt" ]
 
 _print_header "fscrypt setup filesystem (already set up)"
diff --git a/cli-tests/t_single_user.out b/cli-tests/t_single_user.out
new file mode 100644
index 00000000..e788b3e7
--- /dev/null
+++ b/cli-tests/t_single_user.out
@@ -0,0 +1,30 @@
+ext4 filesystem "MNT" has 0 protectors and 0 policies.
+Only root can create fscrypt metadata on this filesystem.
+
+ext4 filesystem "MNT" has 0 protectors and 0 policies.
+Only root can create fscrypt metadata on this filesystem.
+
+
+# Encrypt, lock, and unlock as root
+"MNT/dir" is now locked.
+
+# Encrypt as root with user's login protector
+
+IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for
+           important recovery instructions. It is *strongly recommended* to
+           record the recovery passphrase in a secure location; otherwise you
+           will lose access to this directory if you reinstall the operating
+           system or move this filesystem to another system.
+
+Protector desc1 no longer protecting policy desc2.
+"MNT/dir" is now locked.
+Enter login passphrase for fscrypt-test-user: "MNT/dir" is now unlocked and ready for use.
+
+# Encrypt as user (should fail)
+[ERROR] fscrypt encrypt: user lacks permission to create fscrypt metadata on
+                         MNT
+
+For how to allow users to create fscrypt metadata on a filesystem, refer to
+https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem
+
+# Encrypt as user if they set up filesystem (should succeed)
diff --git a/cli-tests/t_single_user.sh b/cli-tests/t_single_user.sh
new file mode 100755
index 00000000..c569f201
--- /dev/null
+++ b/cli-tests/t_single_user.sh
@@ -0,0 +1,55 @@
+#!/bin/bash
+
+# Test 'fscrypt setup' without --all-users.
+
+cd "$(dirname "$0")"
+. common.sh
+
+_rm_metadata "$MNT_ROOT"
+_rm_metadata "$MNT"
+rm "$FSCRYPT_CONF"
+fscrypt setup --time=1ms --quiet
+fscrypt setup --time=1ms --quiet "$MNT"
+fscrypt status "$MNT"
+_user_do "fscrypt status \"$MNT\""
+
+dir=$MNT/dir
+
+begin()
+{
+	_reset_filesystems
+	mkdir "$dir"
+	_print_header "$1"
+}
+
+begin "Encrypt, lock, and unlock as root"
+echo hunter2 | fscrypt encrypt --quiet --name=dir --skip-unlock "$dir"
+echo hunter2 | fscrypt unlock --quiet "$dir"
+fscrypt lock "$dir"
+
+begin "Encrypt as root with user's login protector"
+echo TEST_USER_PASS | fscrypt encrypt --quiet --source=pam_passphrase --user="$TEST_USER" "$dir"
+# The user should be able to update the policy and protectors created by the
+# above command themselves.  The easiest way to test this is by updating the
+# policy to remove the auto-generated recovery protector.  This verifies that
+# (a) the policy was made owned by the user, and that (b) policy updates fall
+# back to overwrites when the process cannot write to the containing directory.
+# (It would be better to test updating the protectors too, but this is the
+# easiest test to do here.)
+policy=$(fscrypt status "$dir" | awk '/Policy/{print $2}')
+recovery_protector=$(_get_protector_descriptor "$MNT" custom 'Recovery passphrase for dir')
+_user_do "fscrypt metadata remove-protector-from-policy --force --protector=$MNT:$recovery_protector --policy=$MNT:$policy"
+chown "$TEST_USER" "$dir"
+_user_do "fscrypt lock $dir"
+_user_do "echo TEST_USER_PASS | fscrypt unlock $dir"
+
+begin "Encrypt as user (should fail)"
+chown "$TEST_USER" "$dir"
+_user_do_and_expect_failure "echo hunter2 | fscrypt encrypt --quiet --name=dir --skip-unlock \"$dir\""
+
+begin "Encrypt as user if they set up filesystem (should succeed)"
+_rm_metadata "$MNT"
+chown "$TEST_USER" "$MNT"
+chown "$TEST_USER" "$dir"
+_user_do "fscrypt setup --time=1ms --quiet $MNT"
+_user_do "echo hunter2 | fscrypt encrypt --quiet --name=dir3 --skip-unlock \"$dir\""
diff --git a/cli-tests/t_status.out b/cli-tests/t_status.out
index 0d478b5a..eb425d0e 100644
--- a/cli-tests/t_status.out
+++ b/cli-tests/t_status.out
@@ -4,9 +4,11 @@ ext4 supported Yes
 ext4 supported Yes
 
 # Get status of setup mountpoint
-ext4 filesystem "MNT" has 0 protectors and 0 policies
+ext4 filesystem "MNT" has 0 protectors and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
-ext4 filesystem "MNT" has 0 protectors and 0 policies
+ext4 filesystem "MNT" has 0 protectors and 0 policies.
+All users can create fscrypt metadata on this filesystem.
 
 
 # Get status of unencrypted directory on setup mountpoint
diff --git a/cli-tests/t_v1_policy.out b/cli-tests/t_v1_policy.out
index 9adb00ae..1f4f9d77 100644
--- a/cli-tests/t_v1_policy.out
+++ b/cli-tests/t_v1_policy.out
@@ -120,7 +120,8 @@ Unlocked: Partially (incompletely locked, or unlocked by another user)
 Protected with 1 protector:
 PROTECTOR         LINKED  DESCRIPTION
 desc2  No      custom protector "prot"
-ext4 filesystem "MNT" has 1 protector and 1 policy
+ext4 filesystem "MNT" has 1 protector and 1 policy.
+All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
 desc2  No      custom protector "prot"
diff --git a/cmd/fscrypt/commands.go b/cmd/fscrypt/commands.go
index 023c0fa2..30aa3a7a 100644
--- a/cmd/fscrypt/commands.go
+++ b/cmd/fscrypt/commands.go
@@ -63,7 +63,7 @@ var Setup = cli.Command{
 		the README). This may require root privileges.`,
 		mountpointArg, actions.ConfigFileLocation,
 		shortDisplay(timeTargetFlag)),
-	Flags:  []cli.Flag{timeTargetFlag, forceFlag},
+	Flags:  []cli.Flag{timeTargetFlag, forceFlag, allUsersSetupFlag},
 	Action: setupAction,
 }
 
@@ -468,7 +468,7 @@ var Lock = cli.Command{
 		recoverable by an attacker who compromises system memory. To be
 		fully safe, you must reboot with a power cycle.`,
 		directoryArg, shortDisplay(dropCachesFlag)),
-	Flags:  []cli.Flag{dropCachesFlag, userFlag, allUsersFlag},
+	Flags:  []cli.Flag{dropCachesFlag, userFlag, allUsersLockFlag},
 	Action: lockAction,
 }
 
@@ -502,7 +502,7 @@ func lockAction(c *cli.Context) error {
 		return newExitError(c, ErrDropCachesPerm)
 	}
 
-	if err = policy.Deprovision(allUsersFlag.Value); err != nil {
+	if err = policy.Deprovision(allUsersLockFlag.Value); err != nil {
 		switch err {
 		case keyring.ErrKeyNotPresent:
 			break
diff --git a/cmd/fscrypt/errors.go b/cmd/fscrypt/errors.go
index bcf5b595..2d76792d 100644
--- a/cmd/fscrypt/errors.go
+++ b/cmd/fscrypt/errors.go
@@ -232,6 +232,10 @@ func getErrorSuggestions(err error) string {
 			}
 		}
 		return ""
+	case *filesystem.ErrNoCreatePermission:
+		return `For how to allow users to create fscrypt metadata on a
+			filesystem, refer to
+			https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem`
 	case *filesystem.ErrNotSetup:
 		return fmt.Sprintf(`Run "sudo fscrypt setup %s" to use fscrypt
 		        on this filesystem.`, e.Mount.Path)
diff --git a/cmd/fscrypt/flags.go b/cmd/fscrypt/flags.go
index 044b71e8..1b41839d 100644
--- a/cmd/fscrypt/flags.go
+++ b/cmd/fscrypt/flags.go
@@ -116,7 +116,8 @@ var (
 	allFlags = []prettyFlag{helpFlag, versionFlag, verboseFlag, quietFlag,
 		forceFlag, skipUnlockFlag, timeTargetFlag,
 		sourceFlag, nameFlag, keyFileFlag, protectorFlag,
-		unlockWithFlag, policyFlag, allUsersFlag, noRecoveryFlag}
+		unlockWithFlag, policyFlag, allUsersLockFlag, allUsersSetupFlag,
+		noRecoveryFlag}
 	// universalFlags contains flags that should be on every command
 	universalFlags = []cli.Flag{verboseFlag, quietFlag, helpFlag}
 )
@@ -164,7 +165,7 @@ var (
 			privileges.`,
 		Default: true,
 	}
-	allUsersFlag = &boolFlag{
+	allUsersLockFlag = &boolFlag{
 		Name: "all-users",
 		Usage: `Lock the directory no matter which user(s) have unlocked
 			it. Requires root privileges. This flag is only
@@ -172,6 +173,15 @@ var (
 			different from the one you're locking it as. This flag
 			is only implemented for v2 encryption policies.`,
 	}
+	allUsersSetupFlag = &boolFlag{
+		Name: "all-users",
+		Usage: `When setting up a filesystem for fscrypt, allow users
+			other than the calling user (typically root) to create
+			fscrypt policies and protectors on the filesystem. Note
+			that this will create a world-writable directory, which
+			users could use to fill up the entire filesystem. Hence,
+			this option may not be appropriate for some systems.`,
+	}
 	noRecoveryFlag = &boolFlag{
 		Name:  "no-recovery",
 		Usage: `Don't generate a recovery passphrase.`,
diff --git a/cmd/fscrypt/setup.go b/cmd/fscrypt/setup.go
index 7b9bebbb..1da0d163 100644
--- a/cmd/fscrypt/setup.go
+++ b/cmd/fscrypt/setup.go
@@ -26,6 +26,7 @@ import (
 	"os"
 
 	"github.com/google/fscrypt/actions"
+	"github.com/google/fscrypt/filesystem"
 	"github.com/google/fscrypt/util"
 )
 
@@ -80,11 +81,47 @@ func setupFilesystem(w io.Writer, path string) error {
 	if err != nil {
 		return err
 	}
+	username := ctx.TargetUser.Username
 
-	if err = ctx.Mount.Setup(); err != nil {
+	err = ctx.Mount.CheckSetup()
+	if err == nil {
+		return &filesystem.ErrAlreadySetup{Mount: ctx.Mount}
+	}
+	if _, ok := err.(*filesystem.ErrNotSetup); !ok {
 		return err
 	}
 
-	fmt.Fprintf(w, "Metadata directories created at %q.\n", ctx.Mount.BaseDir())
+	allUsers := allUsersSetupFlag.Value
+	if !allUsers {
+		thisFilesystem := "this filesystem"
+		if ctx.Mount.Path == "/" {
+			thisFilesystem = "the root filesystem"
+		}
+		prompt := fmt.Sprintf(`Allow users other than %s to create
+fscrypt metadata on %s? (See
+https://github.com/google/fscrypt#setting-up-fscrypt-on-a-filesystem)`,
+			username, thisFilesystem)
+		allUsers, err = askQuestion(wrapText(prompt, 0), false)
+		if err != nil {
+			return err
+		}
+	}
+	var setupMode filesystem.SetupMode
+	if allUsers {
+		setupMode = filesystem.WorldWritable
+	} else {
+		setupMode = filesystem.SingleUserWritable
+	}
+	if err = ctx.Mount.Setup(setupMode); err != nil {
+		return err
+	}
+
+	if allUsers {
+		fmt.Fprintf(w, "Metadata directories created at %q, writable by everyone.\n",
+			ctx.Mount.BaseDir())
+	} else {
+		fmt.Fprintf(w, "Metadata directories created at %q, writable by %s only.\n",
+			ctx.Mount.BaseDir(), username)
+	}
 	return nil
 }
diff --git a/cmd/fscrypt/status.go b/cmd/fscrypt/status.go
index d10dfd8c..54c6f1f3 100644
--- a/cmd/fscrypt/status.go
+++ b/cmd/fscrypt/status.go
@@ -165,9 +165,18 @@ func writeFilesystemStatus(w io.Writer, ctx *actions.Context) error {
 		return err
 	}
 
-	fmt.Fprintf(w, "%s filesystem %q has %s and %s\n\n", ctx.Mount.FilesystemType,
+	fmt.Fprintf(w, "%s filesystem %q has %s and %s.\n", ctx.Mount.FilesystemType,
 		ctx.Mount.Path, pluralize(len(options), "protector"),
 		pluralize(len(policyDescriptors), "policy"))
+	if setupMode, user, err := ctx.Mount.GetSetupMode(); err == nil {
+		switch setupMode {
+		case filesystem.WorldWritable:
+			fmt.Fprintf(w, "All users can create fscrypt metadata on this filesystem.\n")
+		case filesystem.SingleUserWritable:
+			fmt.Fprintf(w, "Only %s can create fscrypt metadata on this filesystem.\n", user.Username)
+		}
+	}
+	fmt.Fprintf(w, "\n")
 
 	if len(options) > 0 {
 		writeOptions(w, options)
diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go
index c39514ac..1450f0f0 100644
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -96,6 +96,16 @@ func (err *ErrMakeLink) Error() string {
 		err.Target.Path, err.UnderlyingError)
 }
 
+// ErrNoCreatePermission indicates that the current user lacks permission to
+// create fscrypt metadata on the given filesystem.
+type ErrNoCreatePermission struct {
+	Mount *Mount
+}
+
+func (err *ErrNoCreatePermission) Error() string {
+	return fmt.Sprintf("user lacks permission to create fscrypt metadata on %s", err.Mount.Path)
+}
+
 // ErrNotAMountpoint indicates that a path is not a mountpoint.
 type ErrNotAMountpoint struct {
 	Path string
@@ -209,9 +219,6 @@ const (
 
 	// The base directory should be read-only (except for the creator)
 	basePermissions = 0755
-	// The subdirectories should be writable to everyone, but they have the
-	// sticky bit set so users cannot delete other users' metadata.
-	dirPermissions = os.ModeSticky | 0777
 	// The metadata files are globally visible, but can only be deleted by
 	// the user that created them
 	filePermissions = 0644
@@ -223,6 +230,18 @@ const (
 	maxMetadataFileSize = 16384
 )
 
+// SetupMode is a mode for creating the fscrypt metadata directories.
+type SetupMode int
+
+const (
+	// SingleUserWritable specifies to make the fscrypt metadata directories
+	// writable by a single user (usually root) only.
+	SingleUserWritable SetupMode = iota
+	// WorldWritable specifies to make the fscrypt metadata directories
+	// world-writable (with the sticky bit set).
+	WorldWritable
+)
+
 func (m *Mount) String() string {
 	return fmt.Sprintf(`%s
 	FilesystemType: %s
@@ -359,8 +378,8 @@ func (m *Mount) CheckSetup() error {
 	}
 	// Run all the checks so we will always get all the warnings
 	baseGood := isDirCheckPerm(m.BaseDir(), basePermissions)
-	policyGood := isDirCheckPerm(m.PolicyDir(), dirPermissions)
-	protectorGood := isDirCheckPerm(m.ProtectorDir(), dirPermissions)
+	policyGood := isDir(m.PolicyDir())
+	protectorGood := isDir(m.ProtectorDir())
 
 	if baseGood && policyGood && protectorGood {
 		return nil
@@ -370,7 +389,7 @@ func (m *Mount) CheckSetup() error {
 
 // makeDirectories creates the three metadata directories with the correct
 // permissions. Note that this function overrides the umask.
-func (m *Mount) makeDirectories() error {
+func (m *Mount) makeDirectories(setupMode SetupMode) error {
 	// Zero the umask so we get the permissions we want
 	oldMask := unix.Umask(0)
 	defer func() {
@@ -380,17 +399,51 @@ func (m *Mount) makeDirectories() error {
 	if err := os.Mkdir(m.BaseDir(), basePermissions); err != nil {
 		return err
 	}
-	if err := os.Mkdir(m.PolicyDir(), dirPermissions); err != nil {
+
+	var dirMode os.FileMode
+	switch setupMode {
+	case SingleUserWritable:
+		dirMode = 0755
+	case WorldWritable:
+		dirMode = os.ModeSticky | 0777
+	}
+	if err := os.Mkdir(m.PolicyDir(), dirMode); err != nil {
 		return err
 	}
-	return os.Mkdir(m.ProtectorDir(), dirPermissions)
+	return os.Mkdir(m.ProtectorDir(), dirMode)
+}
+
+// GetSetupMode returns the current mode for fscrypt metadata creation on this
+// filesystem.
+func (m *Mount) GetSetupMode() (SetupMode, *user.User, error) {
+	info1, err1 := os.Stat(m.PolicyDir())
+	info2, err2 := os.Stat(m.ProtectorDir())
+
+	if err1 == nil && err2 == nil {
+		mask := os.ModeSticky | 0777
+		mode1 := info1.Mode() & mask
+		mode2 := info2.Mode() & mask
+		uid1 := info1.Sys().(*syscall.Stat_t).Uid
+		uid2 := info2.Sys().(*syscall.Stat_t).Uid
+		user, err := util.UserFromUID(int64(uid1))
+		if err == nil && mode1 == mode2 && uid1 == uid2 {
+			switch mode1 {
+			case mask:
+				return WorldWritable, nil, nil
+			case 0755:
+				return SingleUserWritable, user, nil
+			}
+		}
+		log.Printf("filesystem %s uses custom permissions on metadata directories", m.Path)
+	}
+	return -1, nil, errors.New("unable to determine setup mode")
 }
 
 // Setup sets up the filesystem for use with fscrypt. Note that this merely
 // creates the appropriate files on the filesystem. It does not actually modify
 // the filesystem's feature flags. This operation is atomic; it either succeeds
 // or no files in the baseDir are created.
-func (m *Mount) Setup() error {
+func (m *Mount) Setup(mode SetupMode) error {
 	if m.CheckSetup() == nil {
 		return &ErrAlreadySetup{m}
 	}
@@ -404,7 +457,7 @@ func (m *Mount) Setup() error {
 	}
 	defer os.RemoveAll(temp.Path)
 
-	if err = temp.makeDirectories(); err != nil {
+	if err = temp.makeDirectories(mode); err != nil {
 		return err
 	}
 
@@ -484,6 +537,7 @@ func (m *Mount) writeData(path string, data []byte, owner *user.User) error {
 				log.Printf("trying non-atomic overwrite of %q", path)
 				return m.overwriteDataNonAtomic(path, data)
 			}
+			return &ErrNoCreatePermission{m}
 		}
 		return err
 	}
diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go
index 7aa97cb4..365c5cbc 100644
--- a/filesystem/filesystem_test.go
+++ b/filesystem/filesystem_test.go
@@ -92,7 +92,7 @@ func getSetupMount(t *testing.T) (*Mount, error) {
 	if err != nil {
 		return nil, err
 	}
-	return mnt, mnt.Setup()
+	return mnt, mnt.Setup(WorldWritable)
 }
 
 // Tests that the setup works and creates the correct files
@@ -153,7 +153,7 @@ func testSetupWithSymlink(t *testing.T, mnt *Mount, symlinkTarget string, realDi
 	}
 	defer os.Remove(rawBaseDir)
 
-	if err := mnt.Setup(); err != nil {
+	if err := mnt.Setup(WorldWritable); err != nil {
 		t.Fatal(err)
 	}
 	defer mnt.RemoveAllMetadata()
@@ -203,6 +203,35 @@ func TestSetupWithRelativeSymlink(t *testing.T) {
 	testSetupWithSymlink(t, mnt, ".fscrypt-real", realDir)
 }
 
+func testSetupMode(t *testing.T, mnt *Mount, setupMode SetupMode, expectedPerms os.FileMode) {
+	mnt.RemoveAllMetadata()
+	if err := mnt.Setup(setupMode); err != nil {
+		t.Fatal(err)
+	}
+	dirNames := []string{"policies", "protectors"}
+	for _, dirName := range dirNames {
+		fi, err := os.Stat(filepath.Join(mnt.Path, ".fscrypt", dirName))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if fi.Mode()&(os.ModeSticky|0777) != expectedPerms {
+			t.Errorf("directory %s doesn't have permissions %o", dirName, expectedPerms)
+		}
+	}
+}
+
+// Tests that the supported setup modes (WorldWritable and SingleUserWritable)
+// work as intended.
+func TestSetupModes(t *testing.T) {
+	mnt, err := getTestMount(t)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer mnt.RemoveAllMetadata()
+	testSetupMode(t, mnt, WorldWritable, os.ModeSticky|0777)
+	testSetupMode(t, mnt, SingleUserWritable, 0755)
+}
+
 // Adding a good Protector should succeed, adding a bad one should fail
 func TestAddProtector(t *testing.T) {
 	mnt, err := getSetupMount(t)
@@ -384,7 +413,7 @@ func getTwoSetupMounts(t *testing.T) (realMnt, fakeMnt *Mount, err error) {
 		return
 	}
 	fakeMnt = &Mount{Path: fakeMountpoint, FilesystemType: realMnt.FilesystemType}
-	err = fakeMnt.Setup()
+	err = fakeMnt.Setup(WorldWritable)
 	return
 }
 

From 74e870b7bd1585b4b509da47e0e75db66336e576 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 23 Feb 2022 12:35:04 -0800
Subject: [PATCH 07/13] Strictly validate metadata file ownership by default

The metadata validation checks introduced by the previous commits are
good, but to reduce the attack surface it would be much better to avoid
reading and parsing files owned by other users in the first place.

There are some possible use cases for users sharing fscrypt metadata
files, but I think that for the vast majority of users it is unneeded
and just opens up attack surface.  Thus, make fscrypt (and pam_fscrypt)
not process policies or protectors owned by other users by default.
Specifically,

   * If fscrypt or pam_fscrypt is running as a non-root user, only
     policies and protectors owned by the user or by root can be used.

   * If fscrypt is running as root, any policy or protector can be used.
     (This is to match user expectations -- starting a sudo session
     should gain rights, not remove rights.)

   * If pam_fscrypt is running as root, only policies and protectors
     owned by root can be used.  Note that this only applies when the
     root user themselves has an fscrypt login protector, which is rare.

Add an option 'allow_cross_user_metadata' to /etc/fscrypt.conf which
allows restoring the old behavior for anyone who really needs it.
---
 README.md                     |  11 ++-
 actions/context.go            |  20 +++++-
 actions/policy.go             |   8 +--
 actions/protector.go          |   4 +-
 cli-tests/t_encrypt.out       |   2 +-
 cli-tests/t_single_user.out   |   2 +-
 cli-tests/t_status.out        |   2 +-
 cli-tests/t_v1_policy.out     |   2 +-
 cmd/fscrypt/status.go         |  10 ++-
 filesystem/filesystem.go      | 107 +++++++++++++++++++++--------
 filesystem/filesystem_test.go |  54 +++++++++++----
 metadata/config_test.go       |   3 +-
 metadata/metadata.pb.go       | 126 ++++++++++++++++++----------------
 metadata/metadata.proto       |   1 +
 pam_fscrypt/run_fscrypt.go    |  12 +++-
 15 files changed, 245 insertions(+), 119 deletions(-)

diff --git a/README.md b/README.md
index 26fd0840..f1803b46 100644
--- a/README.md
+++ b/README.md
@@ -320,7 +320,8 @@ that looks like the following:
 		"filenames": "AES_256_CTS",
 		"policy_version": "2"
 	},
-	"use_fs_keyring_for_v1_policies": false
+	"use_fs_keyring_for_v1_policies": false,
+	"allow_cross_user_metadata": false
 }
 ```
 
@@ -378,6 +379,14 @@ The fields are:
   kernels, it's better to not use this setting and instead (re-)create your
   encrypted directories with `"policy_version": "2"`.
 
+* "allow\_cross\_user\_metadata" specifies whether `fscrypt` will allow
+  protectors and policies from other non-root users to be read, e.g. to be
+  offered as options by `fscrypt encrypt`.  The default value is `false`, since
+  other users might be untrusted and could create malicious files.  This can be
+  set to `true` to restore the old behavior on systems where `fscrypt` metadata
+  needs to be shared between multiple users.  Note that this option is
+  independent from the permissions on the metadata files themselves.
+
 ## Setting up `fscrypt` on a filesystem
 
 `fscrypt` needs some directories to exist on the filesystem on which encryption
diff --git a/actions/context.go b/actions/context.go
index 26295ec5..1ee0d600 100644
--- a/actions/context.go
+++ b/actions/context.go
@@ -58,6 +58,12 @@ type Context struct {
 	// the user for whom the keys are claimed in the filesystem keyring when
 	// v2 policies are provisioned.
 	TargetUser *user.User
+	// TrustedUser is the user for whom policies and protectors are allowed
+	// to be read.  Specifically, if TrustedUser is set, then only
+	// policies and protectors owned by TrustedUser or by root will be
+	// allowed to be read.  If it's nil, then all policies and protectors
+	// the process has filesystem-level read access to will be allowed.
+	TrustedUser *user.User
 }
 
 // NewContextFromPath makes a context for the filesystem containing the
@@ -112,6 +118,16 @@ func newContextFromUser(targetUser *user.User) (*Context, error) {
 		return nil, err
 	}
 
+	// By default, when running as a non-root user we only read policies and
+	// protectors owned by the user or root.  When running as root, we allow
+	// reading all policies and protectors.
+	if !ctx.Config.GetAllowCrossUserMetadata() && !util.IsUserRoot() {
+		ctx.TrustedUser, err = util.EffectiveUser()
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	log.Printf("creating context for user %q", targetUser.Username)
 	return ctx, nil
 }
@@ -136,7 +152,7 @@ func (ctx *Context) getKeyringOptions() *keyring.Options {
 // getProtectorOption returns the ProtectorOption for the protector on the
 // context's mountpoint with the specified descriptor.
 func (ctx *Context) getProtectorOption(protectorDescriptor string) *ProtectorOption {
-	mnt, data, err := ctx.Mount.GetProtector(protectorDescriptor)
+	mnt, data, err := ctx.Mount.GetProtector(protectorDescriptor, ctx.TrustedUser)
 	if err != nil {
 		return &ProtectorOption{ProtectorInfo{}, nil, err}
 	}
@@ -155,7 +171,7 @@ func (ctx *Context) ProtectorOptions() ([]*ProtectorOption, error) {
 	if err := ctx.checkContext(); err != nil {
 		return nil, err
 	}
-	descriptors, err := ctx.Mount.ListProtectors()
+	descriptors, err := ctx.Mount.ListProtectors(ctx.TrustedUser)
 	if err != nil {
 		return nil, err
 	}
diff --git a/actions/policy.go b/actions/policy.go
index 7204380b..3b8eb30c 100644
--- a/actions/policy.go
+++ b/actions/policy.go
@@ -145,7 +145,7 @@ func PurgeAllPolicies(ctx *Context) error {
 	if err := ctx.checkContext(); err != nil {
 		return err
 	}
-	policies, err := ctx.Mount.ListPolicies()
+	policies, err := ctx.Mount.ListPolicies(nil)
 	if err != nil {
 		return err
 	}
@@ -225,7 +225,7 @@ func GetPolicy(ctx *Context, descriptor string) (*Policy, error) {
 	if err := ctx.checkContext(); err != nil {
 		return nil, err
 	}
-	data, err := ctx.Mount.GetPolicy(descriptor)
+	data, err := ctx.Mount.GetPolicy(descriptor, ctx.TrustedUser)
 	if err != nil {
 		return nil, err
 	}
@@ -262,7 +262,7 @@ func GetPolicyFromPath(ctx *Context, path string) (*Policy, error) {
 	descriptor := pathData.KeyDescriptor
 	log.Printf("found policy %s for %q", descriptor, path)
 
-	mountData, err := ctx.Mount.GetPolicy(descriptor)
+	mountData, err := ctx.Mount.GetPolicy(descriptor, ctx.TrustedUser)
 	if err != nil {
 		log.Printf("getting policy metadata: %v", err)
 		if _, ok := err.(*filesystem.ErrPolicyNotFound); ok {
@@ -428,7 +428,7 @@ func (policy *Policy) AddProtector(protector *Protector) error {
 	if policy.Context.Mount != protector.Context.Mount {
 		log.Printf("policy on %s\n protector on %s\n", policy.Context.Mount, protector.Context.Mount)
 		isNewLink, err := policy.Context.Mount.AddLinkedProtector(
-			protector.Descriptor(), protector.Context.Mount)
+			protector.Descriptor(), protector.Context.Mount, protector.Context.TrustedUser)
 		if err != nil {
 			return err
 		}
diff --git a/actions/protector.go b/actions/protector.go
index 3278e631..1171c837 100644
--- a/actions/protector.go
+++ b/actions/protector.go
@@ -199,7 +199,7 @@ func GetProtector(ctx *Context, descriptor string) (*Protector, error) {
 	}
 
 	protector := &Protector{Context: ctx}
-	protector.data, err = ctx.Mount.GetRegularProtector(descriptor)
+	protector.data, err = ctx.Mount.GetRegularProtector(descriptor, ctx.TrustedUser)
 	return protector, err
 }
 
@@ -218,7 +218,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.TargetUser}
+		ctx = &Context{ctx.Config, option.LinkedMount, ctx.TargetUser, ctx.TrustedUser}
 	}
 	return &Protector{Context: ctx, data: option.data}, nil
 }
diff --git a/cli-tests/t_encrypt.out b/cli-tests/t_encrypt.out
index b92c9d98..ecdc46bb 100644
--- a/cli-tests/t_encrypt.out
+++ b/cli-tests/t_encrypt.out
@@ -71,7 +71,7 @@ Unlocked: Yes
 Protected with 1 protector:
 PROTECTOR         LINKED  DESCRIPTION
 desc1  No      custom protector "prot"
-ext4 filesystem "MNT" has 1 protector and 1 policy.
+ext4 filesystem "MNT" has 1 protector and 1 policy (only including ones owned by fscrypt-test-user or root).
 All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
diff --git a/cli-tests/t_single_user.out b/cli-tests/t_single_user.out
index e788b3e7..d038d524 100644
--- a/cli-tests/t_single_user.out
+++ b/cli-tests/t_single_user.out
@@ -1,7 +1,7 @@
 ext4 filesystem "MNT" has 0 protectors and 0 policies.
 Only root can create fscrypt metadata on this filesystem.
 
-ext4 filesystem "MNT" has 0 protectors and 0 policies.
+ext4 filesystem "MNT" has 0 protectors and 0 policies (only including ones owned by fscrypt-test-user or root).
 Only root can create fscrypt metadata on this filesystem.
 
 
diff --git a/cli-tests/t_status.out b/cli-tests/t_status.out
index eb425d0e..058c62c9 100644
--- a/cli-tests/t_status.out
+++ b/cli-tests/t_status.out
@@ -7,7 +7,7 @@ ext4 supported Yes
 ext4 filesystem "MNT" has 0 protectors and 0 policies.
 All users can create fscrypt metadata on this filesystem.
 
-ext4 filesystem "MNT" has 0 protectors and 0 policies.
+ext4 filesystem "MNT" has 0 protectors and 0 policies (only including ones owned by fscrypt-test-user or root).
 All users can create fscrypt metadata on this filesystem.
 
 
diff --git a/cli-tests/t_v1_policy.out b/cli-tests/t_v1_policy.out
index 1f4f9d77..23535278 100644
--- a/cli-tests/t_v1_policy.out
+++ b/cli-tests/t_v1_policy.out
@@ -120,7 +120,7 @@ Unlocked: Partially (incompletely locked, or unlocked by another user)
 Protected with 1 protector:
 PROTECTOR         LINKED  DESCRIPTION
 desc2  No      custom protector "prot"
-ext4 filesystem "MNT" has 1 protector and 1 policy.
+ext4 filesystem "MNT" has 1 protector and 1 policy (only including ones owned by fscrypt-test-user or root).
 All users can create fscrypt metadata on this filesystem.
 
 PROTECTOR         LINKED  DESCRIPTION
diff --git a/cmd/fscrypt/status.go b/cmd/fscrypt/status.go
index 54c6f1f3..ed5a764e 100644
--- a/cmd/fscrypt/status.go
+++ b/cmd/fscrypt/status.go
@@ -160,14 +160,18 @@ func writeFilesystemStatus(w io.Writer, ctx *actions.Context) error {
 		return err
 	}
 
-	policyDescriptors, err := ctx.Mount.ListPolicies()
+	policyDescriptors, err := ctx.Mount.ListPolicies(ctx.TrustedUser)
 	if err != nil {
 		return err
 	}
 
-	fmt.Fprintf(w, "%s filesystem %q has %s and %s.\n", ctx.Mount.FilesystemType,
+	filterDescription := ""
+	if ctx.TrustedUser != nil {
+		filterDescription = fmt.Sprintf(" (only including ones owned by %s or root)", ctx.TrustedUser.Username)
+	}
+	fmt.Fprintf(w, "%s filesystem %q has %s and %s%s.\n", ctx.Mount.FilesystemType,
 		ctx.Mount.Path, pluralize(len(options), "protector"),
-		pluralize(len(policyDescriptors), "policy"))
+		pluralize(len(policyDescriptors), "policy"), filterDescription)
 	if setupMode, user, err := ctx.Mount.GetSetupMode(); err == nil {
 		switch setupMode {
 		case filesystem.WorldWritable:
diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go
index 1450f0f0..6567dd66 100644
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -370,6 +370,20 @@ func (m *Mount) CheckSupport() error {
 	return m.EncryptionSupportError(metadata.CheckSupport(m.Path))
 }
 
+func checkOwnership(path string, info os.FileInfo, trustedUser *user.User) bool {
+	if trustedUser == nil {
+		return true
+	}
+	trustedUID := uint32(util.AtoiOrPanic(trustedUser.Uid))
+	actualUID := info.Sys().(*syscall.Stat_t).Uid
+	if actualUID != 0 && actualUID != trustedUID {
+		log.Printf("WARNING: %q is owned by uid %d, but expected %d or 0",
+			path, actualUID, trustedUID)
+		return false
+	}
+	return true
+}
+
 // CheckSetup returns an error if all the fscrypt metadata directories do not
 // exist. Will log any unexpected errors or incorrect permissions.
 func (m *Mount) CheckSetup() error {
@@ -600,6 +614,8 @@ func (m *Mount) addMetadata(path string, md metadata.Metadata, owner *user.User)
 //   point one to absolutely anywhere, and there is no known use case for the
 //   metadata files themselves being symlinks, it seems best to disallow them.)
 // - It must have a reasonable size (<= maxMetadataFileSize).
+// - If trustedUser is non-nil, then the file must be owned by the given user
+//   or by root.
 //
 // Take care to avoid TOCTOU (time-of-check-time-of-use) bugs when doing these
 // tests.  Notably, we must open the file before checking the file type, as the
@@ -611,7 +627,7 @@ func (m *Mount) addMetadata(path string, md metadata.Metadata, owner *user.User)
 // This function returns the data read as well as the UID of the user who owns
 // the file.  The returned UID is needed for login protectors, where the UID
 // needs to be cross-checked with the UID stored in the file itself.
-func readMetadataFileSafe(path string) ([]byte, int64, error) {
+func readMetadataFileSafe(path string, trustedUser *user.User) ([]byte, int64, error) {
 	file, err := os.OpenFile(path, os.O_RDONLY|unix.O_NOFOLLOW|unix.O_NONBLOCK, 0)
 	if err != nil {
 		return nil, -1, err
@@ -625,6 +641,9 @@ func readMetadataFileSafe(path string) ([]byte, int64, error) {
 	if !info.Mode().IsRegular() {
 		return nil, -1, &ErrCorruptMetadata{path, errors.New("not a regular file")}
 	}
+	if !checkOwnership(path, info, trustedUser) {
+		return nil, -1, &ErrCorruptMetadata{path, errors.New("metadata file belongs to another user")}
+	}
 	// Clear O_NONBLOCK, since it has served its purpose when opening the
 	// file, and the behavior of reading from a regular file with O_NONBLOCK
 	// is technically unspecified.
@@ -645,8 +664,8 @@ func readMetadataFileSafe(path string) ([]byte, int64, error) {
 
 // getMetadata reads the metadata structure from the file with the specified
 // path. Only reads normal metadata files, not linked metadata.
-func (m *Mount) getMetadata(path string, md metadata.Metadata) (int64, error) {
-	data, owner, err := readMetadataFileSafe(path)
+func (m *Mount) getMetadata(path string, trustedUser *user.User, md metadata.Metadata) (int64, error) {
+	data, owner, err := readMetadataFileSafe(path, trustedUser)
 	if err != nil {
 		log.Printf("could not read metadata from %q: %v", path, err)
 		return -1, err
@@ -704,19 +723,19 @@ func (m *Mount) AddProtector(data *metadata.ProtectorData) error {
 // AddLinkedProtector adds a link in this filesystem to the protector metadata
 // in the dest filesystem, if one doesn't already exist.  On success, the return
 // value is a nil error and a bool that is true iff the link is newly created.
-func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount) (bool, error) {
+func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount, trustedUser *user.User) (bool, error) {
 	if err := m.CheckSetup(); err != nil {
 		return false, err
 	}
 	// Check that the link is good (descriptor exists, filesystem has UUID).
-	if _, err := dest.GetRegularProtector(descriptor); err != nil {
+	if _, err := dest.GetRegularProtector(descriptor, trustedUser); err != nil {
 		return false, err
 	}
 
 	linkPath := m.linkedProtectorPath(descriptor)
 
 	// Check whether the link already exists.
-	existingLink, _, err := readMetadataFileSafe(linkPath)
+	existingLink, _, err := readMetadataFileSafe(linkPath, trustedUser)
 	if err == nil {
 		existingLinkedMnt, err := getMountFromLink(string(existingLink))
 		if err != nil {
@@ -742,13 +761,13 @@ func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount) (bool, error)
 
 // GetRegularProtector looks up the protector metadata by descriptor. This will
 // fail with ErrProtectorNotFound if the descriptor is a linked protector.
-func (m *Mount) GetRegularProtector(descriptor string) (*metadata.ProtectorData, error) {
+func (m *Mount) GetRegularProtector(descriptor string, trustedUser *user.User) (*metadata.ProtectorData, error) {
 	if err := m.CheckSetup(); err != nil {
 		return nil, err
 	}
 	data := new(metadata.ProtectorData)
 	path := m.protectorPath(descriptor)
-	owner, err := m.getMetadata(path, data)
+	owner, err := m.getMetadata(path, trustedUser, data)
 	if os.IsNotExist(err) {
 		err = &ErrProtectorNotFound{descriptor, m}
 	}
@@ -772,17 +791,17 @@ func (m *Mount) GetRegularProtector(descriptor string) (*metadata.ProtectorData,
 // GetProtector returns the Mount of the filesystem containing the information
 // and that protector's data. If the descriptor is a regular (not linked)
 // protector, the mount will return itself.
-func (m *Mount) GetProtector(descriptor string) (*Mount, *metadata.ProtectorData, error) {
+func (m *Mount) GetProtector(descriptor string, trustedUser *user.User) (*Mount, *metadata.ProtectorData, error) {
 	if err := m.CheckSetup(); err != nil {
 		return nil, nil, err
 	}
 	// Get the link data from the link file
 	path := m.linkedProtectorPath(descriptor)
-	link, _, err := readMetadataFileSafe(path)
+	link, _, err := readMetadataFileSafe(path, trustedUser)
 	if err != nil {
 		// If the link doesn't exist, try for a regular protector.
 		if os.IsNotExist(err) {
-			data, err := m.GetRegularProtector(descriptor)
+			data, err := m.GetRegularProtector(descriptor, trustedUser)
 			return m, data, err
 		}
 		return nil, nil, err
@@ -792,7 +811,7 @@ func (m *Mount) GetProtector(descriptor string) (*Mount, *metadata.ProtectorData
 	if err != nil {
 		return nil, nil, errors.Wrap(err, path)
 	}
-	data, err := linkedMnt.GetRegularProtector(descriptor)
+	data, err := linkedMnt.GetRegularProtector(descriptor, trustedUser)
 	if err != nil {
 		return nil, nil, &ErrFollowLink{string(link), err}
 	}
@@ -818,12 +837,10 @@ func (m *Mount) RemoveProtector(descriptor string) error {
 }
 
 // ListProtectors lists the descriptors of all protectors on this filesystem.
-// This does not include linked protectors.
-func (m *Mount) ListProtectors() ([]string, error) {
-	if err := m.CheckSetup(); err != nil {
-		return nil, err
-	}
-	return m.listDirectory(m.ProtectorDir())
+// This does not include linked protectors.  If trustedUser is non-nil, then
+// the protectors are restricted to those owned by the given user or by root.
+func (m *Mount) ListProtectors(trustedUser *user.User) ([]string, error) {
+	return m.listMetadata(m.ProtectorDir(), "protectors", trustedUser)
 }
 
 // AddPolicy adds the policy metadata to the filesystem storage.
@@ -836,12 +853,12 @@ func (m *Mount) AddPolicy(data *metadata.PolicyData) error {
 }
 
 // GetPolicy looks up the policy metadata by descriptor.
-func (m *Mount) GetPolicy(descriptor string) (*metadata.PolicyData, error) {
+func (m *Mount) GetPolicy(descriptor string, trustedUser *user.User) (*metadata.PolicyData, error) {
 	if err := m.CheckSetup(); err != nil {
 		return nil, err
 	}
 	data := new(metadata.PolicyData)
-	_, err := m.getMetadata(m.PolicyPath(descriptor), data)
+	_, err := m.getMetadata(m.PolicyPath(descriptor), trustedUser, data)
 	if os.IsNotExist(err) {
 		err = &ErrPolicyNotFound{descriptor, m}
 	}
@@ -860,12 +877,11 @@ func (m *Mount) RemovePolicy(descriptor string) error {
 	return err
 }
 
-// ListPolicies lists the descriptors of all policies on this filesystem.
-func (m *Mount) ListPolicies() ([]string, error) {
-	if err := m.CheckSetup(); err != nil {
-		return nil, err
-	}
-	return m.listDirectory(m.PolicyDir())
+// ListPolicies lists the descriptors of all policies on this filesystem.  If
+// trustedUser is non-nil, then the policies are restricted to those owned by
+// the given user or by root.
+func (m *Mount) ListPolicies(trustedUser *user.User) ([]string, error) {
+	return m.listMetadata(m.PolicyDir(), "policies", trustedUser)
 }
 
 type namesAndTimes struct {
@@ -902,7 +918,6 @@ func sortFileListByLastMtime(directoryPath string, names []string) error {
 // listDirectory returns a list of descriptors for a metadata directory,
 // including files which are links to other filesystem's metadata.
 func (m *Mount) listDirectory(directoryPath string) ([]string, error) {
-	log.Printf("listing descriptors in %q", directoryPath)
 	dir, err := os.Open(directoryPath)
 	if err != nil {
 		return nil, err
@@ -925,7 +940,41 @@ func (m *Mount) listDirectory(directoryPath string) ([]string, error) {
 		// Be sure to include links as well
 		descriptors = append(descriptors, strings.TrimSuffix(name, linkFileExtension))
 	}
-
-	log.Printf("found %d descriptor(s)", len(descriptors))
 	return descriptors, nil
 }
+
+func (m *Mount) listMetadata(dirPath string, metadataType string, owner *user.User) ([]string, error) {
+	log.Printf("listing %s in %q", metadataType, dirPath)
+	if err := m.CheckSetup(); err != nil {
+		return nil, err
+	}
+	names, err := m.listDirectory(dirPath)
+	if err != nil {
+		return nil, err
+	}
+	filesIgnoredDescription := ""
+	if owner != nil {
+		filteredNames := make([]string, 0, len(names))
+		uid := uint32(util.AtoiOrPanic(owner.Uid))
+		for _, name := range names {
+			info, err := os.Lstat(filepath.Join(dirPath, name))
+			if err != nil {
+				continue
+			}
+			fileUID := info.Sys().(*syscall.Stat_t).Uid
+			if fileUID != uid && fileUID != 0 {
+				continue
+			}
+			filteredNames = append(filteredNames, name)
+		}
+		numIgnored := len(names) - len(filteredNames)
+		if numIgnored != 0 {
+			filesIgnoredDescription =
+				fmt.Sprintf(" (ignored %d %s not owned by %s or root)",
+					numIgnored, metadataType, owner.Username)
+		}
+		names = filteredNames
+	}
+	log.Printf("found %d %s%s", len(names), metadataType, filesIgnoredDescription)
+	return names, nil
+}
diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go
index 365c5cbc..d4ef826d 100644
--- a/filesystem/filesystem_test.go
+++ b/filesystem/filesystem_test.go
@@ -23,6 +23,7 @@ import (
 	"io/ioutil"
 	"log"
 	"os"
+	"os/user"
 	"path/filepath"
 	"syscall"
 	"testing"
@@ -323,7 +324,7 @@ func TestSetPolicy(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	realPolicy, err := mnt.GetPolicy(policy.KeyDescriptor)
+	realPolicy, err := mnt.GetPolicy(policy.KeyDescriptor, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -347,7 +348,7 @@ func TestSetProtector(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	realProtector, err := mnt.GetRegularProtector(protector.ProtectorDescriptor)
+	realProtector, err := mnt.GetRegularProtector(protector.ProtectorDescriptor, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -374,7 +375,7 @@ func TestSpoofedLoginProtector(t *testing.T) {
 	if err = mnt.AddProtector(protector); err != nil {
 		t.Fatal(err)
 	}
-	_, err = mnt.GetRegularProtector(protector.ProtectorDescriptor)
+	_, err = mnt.GetRegularProtector(protector.ProtectorDescriptor, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -389,7 +390,7 @@ func TestSpoofedLoginProtector(t *testing.T) {
 	if err = mnt.AddProtector(protector); err != nil {
 		t.Fatal(err)
 	}
-	_, err = mnt.GetRegularProtector(protector.ProtectorDescriptor)
+	_, err = mnt.GetRegularProtector(protector.ProtectorDescriptor, nil)
 	if myUID == 0 {
 		if err != nil {
 			t.Fatal(err)
@@ -439,13 +440,13 @@ func TestLinkedProtector(t *testing.T) {
 
 	// Add the link to the second filesystem
 	var isNewLink bool
-	if isNewLink, err = fakeMnt.AddLinkedProtector(protector.ProtectorDescriptor, realMnt); err != nil {
+	if isNewLink, err = fakeMnt.AddLinkedProtector(protector.ProtectorDescriptor, realMnt, nil); err != nil {
 		t.Fatal(err)
 	}
 	if !isNewLink {
 		t.Fatal("Link was not new")
 	}
-	if isNewLink, err = fakeMnt.AddLinkedProtector(protector.ProtectorDescriptor, realMnt); err != nil {
+	if isNewLink, err = fakeMnt.AddLinkedProtector(protector.ProtectorDescriptor, realMnt, nil); err != nil {
 		t.Fatal(err)
 	}
 	if isNewLink {
@@ -453,12 +454,12 @@ func TestLinkedProtector(t *testing.T) {
 	}
 
 	// Get the protector though the second system
-	_, err = fakeMnt.GetRegularProtector(protector.ProtectorDescriptor)
+	_, err = fakeMnt.GetRegularProtector(protector.ProtectorDescriptor, nil)
 	if _, ok := err.(*ErrProtectorNotFound); !ok {
 		t.Fatal(err)
 	}
 
-	retMnt, retProtector, err := fakeMnt.GetProtector(protector.ProtectorDescriptor)
+	retMnt, retProtector, err := fakeMnt.GetProtector(protector.ProtectorDescriptor, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -480,6 +481,11 @@ func createFile(path string, size int64) error {
 
 // Tests the readMetadataFileSafe() function.
 func TestReadMetadataFileSafe(t *testing.T) {
+	currentUser, err := util.EffectiveUser()
+	otherUser := &user.User{Uid: "-1"}
+	if err != nil {
+		t.Fatal(err)
+	}
 	tempDir, err := ioutil.TempDir("", "fscrypt")
 	if err != nil {
 		t.Fatal(err)
@@ -491,17 +497,39 @@ func TestReadMetadataFileSafe(t *testing.T) {
 	if err = createFile(filePath, 1000); err != nil {
 		t.Fatal(err)
 	}
-	_, owner, err := readMetadataFileSafe(filePath)
+	_, owner, err := readMetadataFileSafe(filePath, nil)
 	if err != nil {
 		t.Fatal("failed to read file")
 	}
 	if owner != int64(os.Geteuid()) {
 		t.Fatal("got wrong owner")
 	}
+	// Also try it with the trustedUser argument set to the current user.
+	if _, _, err = readMetadataFileSafe(filePath, currentUser); err != nil {
+		t.Fatal("failed to read file")
+	}
+	os.Remove(filePath)
+
+	// File owned by another user.  We might not have permission to actually
+	// change the file's ownership, so we simulate this by passing in a bad
+	// value for the trustedUser argument.
+	if err = createFile(filePath, 1000); err != nil {
+		t.Fatal(err)
+	}
+	_, _, err = readMetadataFileSafe(filePath, otherUser)
+	if util.IsUserRoot() {
+		if err != nil {
+			t.Fatal("root-owned file didn't pass owner validation")
+		}
+	} else {
+		if err == nil {
+			t.Fatal("unexpectedly could read file owned by another user")
+		}
+	}
 	os.Remove(filePath)
 
 	// Nonexistent file
-	_, _, err = readMetadataFileSafe(filePath)
+	_, _, err = readMetadataFileSafe(filePath, nil)
 	if !os.IsNotExist(err) {
 		t.Fatal("trying to read nonexistent file didn't fail with expected error")
 	}
@@ -510,7 +538,7 @@ func TestReadMetadataFileSafe(t *testing.T) {
 	if err = os.Symlink("target", filePath); err != nil {
 		t.Fatal(err)
 	}
-	_, _, err = readMetadataFileSafe(filePath)
+	_, _, err = readMetadataFileSafe(filePath, nil)
 	if err.(*os.PathError).Err != syscall.ELOOP {
 		t.Fatal("trying to read symlink didn't fail with ELOOP")
 	}
@@ -520,7 +548,7 @@ func TestReadMetadataFileSafe(t *testing.T) {
 	if err = unix.Mkfifo(filePath, 0600); err != nil {
 		t.Fatal(err)
 	}
-	_, _, err = readMetadataFileSafe(filePath)
+	_, _, err = readMetadataFileSafe(filePath, nil)
 	if _, ok := err.(*ErrCorruptMetadata); !ok {
 		t.Fatal("trying to read FIFO didn't fail with expected error")
 	}
@@ -530,7 +558,7 @@ func TestReadMetadataFileSafe(t *testing.T) {
 	if err = createFile(filePath, 1000000); err != nil {
 		t.Fatal(err)
 	}
-	_, _, err = readMetadataFileSafe(filePath)
+	_, _, err = readMetadataFileSafe(filePath, nil)
 	if _, ok := err.(*ErrCorruptMetadata); !ok {
 		t.Fatal("trying to read very large file didn't fail with expected error")
 	}
diff --git a/metadata/config_test.go b/metadata/config_test.go
index 52f83f2b..2874bb89 100644
--- a/metadata/config_test.go
+++ b/metadata/config_test.go
@@ -49,7 +49,8 @@ var testConfigString = `{
 		"filenames": "AES_256_CTS",
 		"policy_version": "1"
 	},
-	"use_fs_keyring_for_v1_policies": false
+	"use_fs_keyring_for_v1_policies": false,
+	"allow_cross_user_metadata": false
 }
 `
 
diff --git a/metadata/metadata.pb.go b/metadata/metadata.pb.go
index a2148cef..67098043 100644
--- a/metadata/metadata.pb.go
+++ b/metadata/metadata.pb.go
@@ -45,7 +45,7 @@ func (x SourceType) String() string {
 	return proto.EnumName(SourceType_name, int32(x))
 }
 func (SourceType) EnumDescriptor() ([]byte, []int) {
-	return fileDescriptor_metadata_20fa0d9b7a38c428, []int{0}
+	return fileDescriptor_metadata_31965d2849cb292a, []int{0}
 }
 
 // Type of encryption; should match declarations of unix.FSCRYPT_MODE
@@ -87,7 +87,7 @@ func (x EncryptionOptions_Mode) String() string {
 	return proto.EnumName(EncryptionOptions_Mode_name, int32(x))
 }
 func (EncryptionOptions_Mode) EnumDescriptor() ([]byte, []int) {
-	return fileDescriptor_metadata_20fa0d9b7a38c428, []int{3, 0}
+	return fileDescriptor_metadata_31965d2849cb292a, []int{3, 0}
 }
 
 // Cost parameters to be used in our hashing functions.
@@ -104,7 +104,7 @@ func (m *HashingCosts) Reset()         { *m = HashingCosts{} }
 func (m *HashingCosts) String() string { return proto.CompactTextString(m) }
 func (*HashingCosts) ProtoMessage()    {}
 func (*HashingCosts) Descriptor() ([]byte, []int) {
-	return fileDescriptor_metadata_20fa0d9b7a38c428, []int{0}
+	return fileDescriptor_metadata_31965d2849cb292a, []int{0}
 }
 func (m *HashingCosts) XXX_Unmarshal(b []byte) error {
 	return xxx_messageInfo_HashingCosts.Unmarshal(m, b)
@@ -159,7 +159,7 @@ func (m *WrappedKeyData) Reset()         { *m = WrappedKeyData{} }
 func (m *WrappedKeyData) String() string { return proto.CompactTextString(m) }
 func (*WrappedKeyData) ProtoMessage()    {}
 func (*WrappedKeyData) Descriptor() ([]byte, []int) {
-	return fileDescriptor_metadata_20fa0d9b7a38c428, []int{1}
+	return fileDescriptor_metadata_31965d2849cb292a, []int{1}
 }
 func (m *WrappedKeyData) XXX_Unmarshal(b []byte) error {
 	return xxx_messageInfo_WrappedKeyData.Unmarshal(m, b)
@@ -219,7 +219,7 @@ func (m *ProtectorData) Reset()         { *m = ProtectorData{} }
 func (m *ProtectorData) String() string { return proto.CompactTextString(m) }
 func (*ProtectorData) ProtoMessage()    {}
 func (*ProtectorData) Descriptor() ([]byte, []int) {
-	return fileDescriptor_metadata_20fa0d9b7a38c428, []int{2}
+	return fileDescriptor_metadata_31965d2849cb292a, []int{2}
 }
 func (m *ProtectorData) XXX_Unmarshal(b []byte) error {
 	return xxx_messageInfo_ProtectorData.Unmarshal(m, b)
@@ -303,7 +303,7 @@ func (m *EncryptionOptions) Reset()         { *m = EncryptionOptions{} }
 func (m *EncryptionOptions) String() string { return proto.CompactTextString(m) }
 func (*EncryptionOptions) ProtoMessage()    {}
 func (*EncryptionOptions) Descriptor() ([]byte, []int) {
-	return fileDescriptor_metadata_20fa0d9b7a38c428, []int{3}
+	return fileDescriptor_metadata_31965d2849cb292a, []int{3}
 }
 func (m *EncryptionOptions) XXX_Unmarshal(b []byte) error {
 	return xxx_messageInfo_EncryptionOptions.Unmarshal(m, b)
@@ -363,7 +363,7 @@ func (m *WrappedPolicyKey) Reset()         { *m = WrappedPolicyKey{} }
 func (m *WrappedPolicyKey) String() string { return proto.CompactTextString(m) }
 func (*WrappedPolicyKey) ProtoMessage()    {}
 func (*WrappedPolicyKey) Descriptor() ([]byte, []int) {
-	return fileDescriptor_metadata_20fa0d9b7a38c428, []int{4}
+	return fileDescriptor_metadata_31965d2849cb292a, []int{4}
 }
 func (m *WrappedPolicyKey) XXX_Unmarshal(b []byte) error {
 	return xxx_messageInfo_WrappedPolicyKey.Unmarshal(m, b)
@@ -411,7 +411,7 @@ func (m *PolicyData) Reset()         { *m = PolicyData{} }
 func (m *PolicyData) String() string { return proto.CompactTextString(m) }
 func (*PolicyData) ProtoMessage()    {}
 func (*PolicyData) Descriptor() ([]byte, []int) {
-	return fileDescriptor_metadata_20fa0d9b7a38c428, []int{5}
+	return fileDescriptor_metadata_31965d2849cb292a, []int{5}
 }
 func (m *PolicyData) XXX_Unmarshal(b []byte) error {
 	return xxx_messageInfo_PolicyData.Unmarshal(m, b)
@@ -458,6 +458,7 @@ type Config struct {
 	HashCosts                 *HashingCosts      `protobuf:"bytes,2,opt,name=hash_costs,json=hashCosts,proto3" json:"hash_costs,omitempty"`
 	Options                   *EncryptionOptions `protobuf:"bytes,4,opt,name=options,proto3" json:"options,omitempty"`
 	UseFsKeyringForV1Policies bool               `protobuf:"varint,5,opt,name=use_fs_keyring_for_v1_policies,json=useFsKeyringForV1Policies,proto3" json:"use_fs_keyring_for_v1_policies,omitempty"`
+	AllowCrossUserMetadata    bool               `protobuf:"varint,6,opt,name=allow_cross_user_metadata,json=allowCrossUserMetadata,proto3" json:"allow_cross_user_metadata,omitempty"`
 	XXX_NoUnkeyedLiteral      struct{}           `json:"-"`
 	XXX_unrecognized          []byte             `json:"-"`
 	XXX_sizecache             int32              `json:"-"`
@@ -467,7 +468,7 @@ func (m *Config) Reset()         { *m = Config{} }
 func (m *Config) String() string { return proto.CompactTextString(m) }
 func (*Config) ProtoMessage()    {}
 func (*Config) Descriptor() ([]byte, []int) {
-	return fileDescriptor_metadata_20fa0d9b7a38c428, []int{6}
+	return fileDescriptor_metadata_31965d2849cb292a, []int{6}
 }
 func (m *Config) XXX_Unmarshal(b []byte) error {
 	return xxx_messageInfo_Config.Unmarshal(m, b)
@@ -515,6 +516,13 @@ func (m *Config) GetUseFsKeyringForV1Policies() bool {
 	return false
 }
 
+func (m *Config) GetAllowCrossUserMetadata() bool {
+	if m != nil {
+		return m.AllowCrossUserMetadata
+	}
+	return false
+}
+
 func init() {
 	proto.RegisterType((*HashingCosts)(nil), "metadata.HashingCosts")
 	proto.RegisterType((*WrappedKeyData)(nil), "metadata.WrappedKeyData")
@@ -527,53 +535,55 @@ func init() {
 	proto.RegisterEnum("metadata.EncryptionOptions_Mode", EncryptionOptions_Mode_name, EncryptionOptions_Mode_value)
 }
 
-func init() { proto.RegisterFile("metadata/metadata.proto", fileDescriptor_metadata_20fa0d9b7a38c428) }
-
-var fileDescriptor_metadata_20fa0d9b7a38c428 = []byte{
-	// 716 bytes of a gzipped FileDescriptorProto
-	0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x9c, 0x54, 0xdd, 0x6a, 0xdb, 0x48,
-	0x14, 0x5e, 0x49, 0x8e, 0x7f, 0x8e, 0x7f, 0x56, 0x99, 0x64, 0xb3, 0xda, 0x5d, 0x58, 0x8c, 0x97,
-	0x40, 0x58, 0x42, 0x16, 0x7b, 0x49, 0x69, 0xa1, 0x14, 0x52, 0x27, 0x69, 0x93, 0x10, 0x9a, 0x8e,
-	0x8d, 0xdb, 0x42, 0x41, 0x4c, 0xa4, 0xb1, 0x3d, 0x58, 0xd2, 0x88, 0x99, 0x71, 0x8c, 0xee, 0x7a,
-	0xd7, 0x07, 0xe8, 0xbb, 0xf4, 0x69, 0xfa, 0x28, 0xbd, 0x28, 0x1a, 0xc9, 0x7f, 0x09, 0x84, 0xa4,
-	0x37, 0xe2, 0x9c, 0x6f, 0xce, 0xef, 0x77, 0xce, 0x11, 0xfc, 0x1e, 0x52, 0x45, 0x7c, 0xa2, 0xc8,
-	0x7f, 0x73, 0xe1, 0x20, 0x16, 0x5c, 0x71, 0x54, 0x9e, 0xeb, 0xad, 0x8f, 0x50, 0x7b, 0x4d, 0xe4,
-	0x98, 0x45, 0xa3, 0x2e, 0x97, 0x4a, 0x22, 0x04, 0x05, 0xc5, 0x42, 0xea, 0x98, 0x4d, 0x63, 0xcf,
-	0xc2, 0x5a, 0x46, 0x3b, 0x50, 0x0c, 0x69, 0xc8, 0x45, 0xe2, 0x58, 0x1a, 0xcd, 0x35, 0xd4, 0x84,
-	0x6a, 0x4c, 0x04, 0x09, 0x02, 0x1a, 0x30, 0x19, 0x3a, 0x05, 0xfd, 0xb8, 0x0a, 0xb5, 0x3e, 0x40,
-	0xe3, 0x9d, 0x20, 0x71, 0x4c, 0xfd, 0x0b, 0x9a, 0x1c, 0x13, 0x45, 0x50, 0x03, 0xcc, 0xb3, 0x81,
-	0x63, 0x34, 0x8d, 0xbd, 0x1a, 0x36, 0xcf, 0x06, 0xe8, 0x1f, 0xa8, 0xd3, 0xc8, 0x13, 0x49, 0xac,
-	0xa8, 0xef, 0x4e, 0x68, 0xa2, 0x13, 0xd7, 0x70, 0x6d, 0x01, 0x5e, 0xd0, 0x24, 0x2d, 0x6a, 0x1c,
-	0x12, 0x4f, 0xa7, 0xaf, 0x61, 0x2d, 0xb7, 0xbe, 0x98, 0x50, 0xbf, 0x12, 0x5c, 0x51, 0x4f, 0x71,
-	0xa1, 0x43, 0xb7, 0x61, 0x3b, 0x9e, 0x03, 0xae, 0x4f, 0xa5, 0x27, 0x58, 0xac, 0xb8, 0xd0, 0xc9,
-	0x2a, 0x78, 0x6b, 0xf1, 0x76, 0xbc, 0x78, 0x42, 0xfb, 0x50, 0x94, 0x7c, 0x2a, 0xbc, 0xac, 0xdf,
-	0x46, 0x67, 0xfb, 0x60, 0x41, 0x54, 0x4f, 0xe3, 0xfd, 0x24, 0xa6, 0x38, 0xb7, 0x49, 0xcb, 0x88,
-	0x48, 0x48, 0x75, 0x19, 0x15, 0xac, 0x65, 0xb4, 0x0f, 0x1b, 0x5e, 0x4a, 0x9c, 0xee, 0xbe, 0xda,
-	0xd9, 0x59, 0x06, 0x58, 0xa5, 0x15, 0x67, 0x46, 0x69, 0x04, 0x49, 0x02, 0xe5, 0x6c, 0x64, 0x8d,
-	0xa4, 0x32, 0xb2, 0xc1, 0x9a, 0x32, 0xdf, 0x29, 0x6a, 0xf6, 0x52, 0x11, 0x3d, 0x83, 0xea, 0x2c,
-	0x63, 0x4d, 0x33, 0x52, 0xd2, 0x91, 0x9d, 0x65, 0xe4, 0x75, 0x4a, 0x31, 0xcc, 0x16, 0x7a, 0xeb,
-	0x9b, 0x09, 0x9b, 0x27, 0x19, 0x75, 0x8c, 0x47, 0x6f, 0xf4, 0x57, 0x22, 0x07, 0x4a, 0x31, 0xf1,
-	0x7d, 0x16, 0x8d, 0x34, 0x19, 0x16, 0x9e, 0xab, 0xe8, 0x39, 0x94, 0x3d, 0x1e, 0x29, 0x1a, 0x29,
-	0x99, 0x53, 0xd0, 0x5c, 0xe6, 0xb9, 0x13, 0xe8, 0xe0, 0x92, 0xfb, 0x14, 0x2f, 0x3c, 0xd0, 0x0b,
-	0xa8, 0x0c, 0x59, 0x40, 0x53, 0x22, 0xa4, 0x66, 0xe5, 0x21, 0xee, 0x4b, 0x17, 0xb4, 0x0b, 0x8d,
-	0x98, 0x07, 0xcc, 0x4b, 0xdc, 0x1b, 0x2a, 0x24, 0xe3, 0x51, 0xbe, 0x43, 0xf5, 0x0c, 0x1d, 0x64,
-	0x60, 0xeb, 0xb3, 0x01, 0x85, 0xd4, 0x15, 0x55, 0xa1, 0xe4, 0xd3, 0x21, 0x99, 0x06, 0xca, 0xfe,
-	0x05, 0xfd, 0x0a, 0xd5, 0xa3, 0x93, 0x9e, 0xdb, 0x39, 0x7c, 0xe2, 0xbe, 0xef, 0xf7, 0x6c, 0x63,
-	0x15, 0x78, 0xd5, 0xbd, 0xb4, 0xcd, 0x55, 0xa0, 0xfb, 0xb2, 0x6b, 0x5b, 0x6b, 0x40, 0xbf, 0x67,
-	0x17, 0xe6, 0x40, 0xbb, 0xf3, 0x54, 0x5b, 0x6c, 0xac, 0x01, 0xfd, 0x9e, 0x5d, 0x44, 0x35, 0x28,
-	0x1f, 0xf9, 0x8c, 0x44, 0x6a, 0x1a, 0xda, 0x95, 0xd6, 0x27, 0x03, 0xec, 0x9c, 0xfd, 0x2b, 0x5d,
-	0x62, 0xba, 0x9d, 0x3f, 0xb1, 0x77, 0xb7, 0x26, 0x6c, 0x3e, 0x62, 0xc2, 0x5f, 0x0d, 0x80, 0x2c,
-	0xb7, 0x5e, 0xfa, 0x5d, 0x68, 0x4c, 0x68, 0x72, 0x37, 0x6d, 0x7d, 0x42, 0x93, 0x95, 0x84, 0x87,
-	0x50, 0xe2, 0xd9, 0x10, 0xf2, 0x64, 0x7f, 0xdd, 0x33, 0x27, 0x3c, 0xb7, 0x45, 0xe7, 0xb0, 0x35,
-	0xaf, 0x33, 0x1f, 0xd4, 0x84, 0x26, 0xe9, 0xa8, 0xad, 0xbd, 0x6a, 0xe7, 0xcf, 0x3b, 0xf5, 0x2e,
-	0x38, 0xc1, 0x9b, 0xb3, 0x5b, 0x88, 0x6c, 0x7d, 0x37, 0xa0, 0xd8, 0xe5, 0xd1, 0x90, 0x8d, 0x56,
-	0xce, 0xce, 0x78, 0xc0, 0xd9, 0x1d, 0x02, 0x8c, 0x89, 0x1c, 0xbb, 0xd9, 0x9d, 0x99, 0xf7, 0xde,
-	0x59, 0x25, 0xb5, 0xcc, 0xfe, 0x64, 0x2b, 0x2d, 0x17, 0x1e, 0xd1, 0xf2, 0x11, 0xfc, 0x3d, 0x95,
-	0xd4, 0x1d, 0xca, 0xb4, 0x55, 0xc1, 0xa2, 0x91, 0x3b, 0xe4, 0xc2, 0xbd, 0x69, 0x67, 0x04, 0x30,
-	0x2a, 0xf5, 0xf1, 0x96, 0xf1, 0x1f, 0x53, 0x49, 0x4f, 0xe5, 0x45, 0x66, 0x73, 0xca, 0xc5, 0xa0,
-	0x7d, 0x95, 0x1b, 0x9c, 0x17, 0xca, 0x96, 0x5d, 0xc0, 0x75, 0x8f, 0x87, 0x31, 0x51, 0xec, 0x9a,
-	0x05, 0x4c, 0x25, 0xff, 0xbe, 0x05, 0x58, 0xf6, 0xb6, 0xbe, 0xc9, 0x08, 0x1a, 0x31, 0x09, 0xdd,
-	0x98, 0x48, 0x19, 0x8f, 0x05, 0x91, 0xd4, 0x36, 0xd0, 0x6f, 0xb0, 0xe9, 0x4d, 0xa5, 0xe2, 0x6b,
-	0xb0, 0x99, 0xfa, 0x09, 0x32, 0x4b, 0x4b, 0xb3, 0xad, 0xeb, 0xa2, 0xfe, 0x99, 0xff, 0xff, 0x23,
-	0x00, 0x00, 0xff, 0xff, 0x3d, 0x33, 0x9f, 0x0d, 0xe7, 0x05, 0x00, 0x00,
+func init() { proto.RegisterFile("metadata/metadata.proto", fileDescriptor_metadata_31965d2849cb292a) }
+
+var fileDescriptor_metadata_31965d2849cb292a = []byte{
+	// 748 bytes of a gzipped FileDescriptorProto
+	0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x9c, 0x55, 0xdb, 0x6a, 0xf3, 0x46,
+	0x10, 0xae, 0x24, 0xc7, 0x87, 0xf1, 0xa1, 0xca, 0xfe, 0x69, 0xaa, 0xb4, 0x50, 0x8c, 0x4b, 0x20,
+	0x94, 0x90, 0x62, 0x97, 0x94, 0x06, 0x4a, 0x21, 0x75, 0x92, 0x36, 0x09, 0xa1, 0xe9, 0xda, 0x75,
+	0x5b, 0x28, 0x88, 0x8d, 0xb4, 0xb6, 0x17, 0x4b, 0x5a, 0xb1, 0xbb, 0x8a, 0xd1, 0x5d, 0xef, 0xfa,
+	0x00, 0x7d, 0x97, 0xf6, 0x65, 0xfa, 0x30, 0x45, 0x2b, 0xc9, 0x87, 0x04, 0x42, 0xf2, 0xdf, 0x98,
+	0xd9, 0x6f, 0x67, 0xe6, 0x9b, 0xf9, 0x66, 0xc7, 0x82, 0x8f, 0x43, 0xaa, 0x88, 0x4f, 0x14, 0xf9,
+	0xb2, 0x34, 0x4e, 0x62, 0xc1, 0x15, 0x47, 0xf5, 0xf2, 0xdc, 0xfb, 0x03, 0x5a, 0x3f, 0x12, 0x39,
+	0x67, 0xd1, 0x6c, 0xc8, 0xa5, 0x92, 0x08, 0x41, 0x45, 0xb1, 0x90, 0x3a, 0x66, 0xd7, 0x38, 0xb2,
+	0xb0, 0xb6, 0xd1, 0x3e, 0x54, 0x43, 0x1a, 0x72, 0x91, 0x3a, 0x96, 0x46, 0x8b, 0x13, 0xea, 0x42,
+	0x33, 0x26, 0x82, 0x04, 0x01, 0x0d, 0x98, 0x0c, 0x9d, 0x8a, 0xbe, 0xdc, 0x84, 0x7a, 0xbf, 0x43,
+	0xe7, 0x57, 0x41, 0xe2, 0x98, 0xfa, 0xb7, 0x34, 0xbd, 0x20, 0x8a, 0xa0, 0x0e, 0x98, 0xd7, 0x13,
+	0xc7, 0xe8, 0x1a, 0x47, 0x2d, 0x6c, 0x5e, 0x4f, 0xd0, 0xe7, 0xd0, 0xa6, 0x91, 0x27, 0xd2, 0x58,
+	0x51, 0xdf, 0x5d, 0xd0, 0x54, 0x13, 0xb7, 0x70, 0x6b, 0x05, 0xde, 0xd2, 0x34, 0x2b, 0x6a, 0x1e,
+	0x12, 0x4f, 0xd3, 0xb7, 0xb0, 0xb6, 0x7b, 0x7f, 0x9b, 0xd0, 0xbe, 0x17, 0x5c, 0x51, 0x4f, 0x71,
+	0xa1, 0x53, 0xf7, 0x61, 0x2f, 0x2e, 0x01, 0xd7, 0xa7, 0xd2, 0x13, 0x2c, 0x56, 0x5c, 0x68, 0xb2,
+	0x06, 0x7e, 0xb7, 0xba, 0xbb, 0x58, 0x5d, 0xa1, 0x63, 0xa8, 0x4a, 0x9e, 0x08, 0x2f, 0xef, 0xb7,
+	0x33, 0xd8, 0x3b, 0x59, 0x09, 0x35, 0xd2, 0xf8, 0x38, 0x8d, 0x29, 0x2e, 0x7c, 0xb2, 0x32, 0x22,
+	0x12, 0x52, 0x5d, 0x46, 0x03, 0x6b, 0x1b, 0x1d, 0xc3, 0x8e, 0x97, 0x09, 0xa7, 0xbb, 0x6f, 0x0e,
+	0xf6, 0xd7, 0x09, 0x36, 0x65, 0xc5, 0xb9, 0x53, 0x96, 0x41, 0x92, 0x40, 0x39, 0x3b, 0x79, 0x23,
+	0x99, 0x8d, 0x6c, 0xb0, 0x12, 0xe6, 0x3b, 0x55, 0xad, 0x5e, 0x66, 0xa2, 0x33, 0x68, 0x2e, 0x73,
+	0xd5, 0xb4, 0x22, 0x35, 0x9d, 0xd9, 0x59, 0x67, 0xde, 0x96, 0x14, 0xc3, 0x72, 0x75, 0xee, 0xfd,
+	0x67, 0xc2, 0xee, 0x65, 0x2e, 0x1d, 0xe3, 0xd1, 0x4f, 0xfa, 0x57, 0x22, 0x07, 0x6a, 0x31, 0xf1,
+	0x7d, 0x16, 0xcd, 0xb4, 0x18, 0x16, 0x2e, 0x8f, 0xe8, 0x5b, 0xa8, 0x7b, 0x3c, 0x52, 0x34, 0x52,
+	0xb2, 0x90, 0xa0, 0xbb, 0xe6, 0x79, 0x96, 0xe8, 0xe4, 0x8e, 0xfb, 0x14, 0xaf, 0x22, 0xd0, 0x77,
+	0xd0, 0x98, 0xb2, 0x80, 0x66, 0x42, 0x48, 0xad, 0xca, 0x6b, 0xc2, 0xd7, 0x21, 0xe8, 0x10, 0x3a,
+	0x31, 0x0f, 0x98, 0x97, 0xba, 0x8f, 0x54, 0x48, 0xc6, 0xa3, 0xe2, 0x0d, 0xb5, 0x73, 0x74, 0x92,
+	0x83, 0xbd, 0xbf, 0x0c, 0xa8, 0x64, 0xa1, 0xa8, 0x09, 0x35, 0x9f, 0x4e, 0x49, 0x12, 0x28, 0xfb,
+	0x03, 0xf4, 0x21, 0x34, 0xcf, 0x2f, 0x47, 0xee, 0xe0, 0xf4, 0x6b, 0xf7, 0xb7, 0xf1, 0xc8, 0x36,
+	0x36, 0x81, 0x1f, 0x86, 0x77, 0xb6, 0xb9, 0x09, 0x0c, 0xbf, 0x1f, 0xda, 0xd6, 0x16, 0x30, 0x1e,
+	0xd9, 0x95, 0x12, 0xe8, 0x0f, 0xbe, 0xd1, 0x1e, 0x3b, 0x5b, 0xc0, 0x78, 0x64, 0x57, 0x51, 0x0b,
+	0xea, 0xe7, 0x3e, 0x23, 0x91, 0x4a, 0x42, 0xbb, 0xd1, 0xfb, 0xd3, 0x00, 0xbb, 0x50, 0xff, 0x5e,
+	0x97, 0x98, 0xbd, 0xce, 0xf7, 0x78, 0x77, 0x4f, 0x26, 0x6c, 0xbe, 0x61, 0xc2, 0xff, 0x18, 0x00,
+	0x39, 0xb7, 0x7e, 0xf4, 0x87, 0xd0, 0x59, 0xd0, 0xf4, 0x39, 0x6d, 0x7b, 0x41, 0xd3, 0x0d, 0xc2,
+	0x53, 0xa8, 0xf1, 0x7c, 0x08, 0x05, 0xd9, 0xa7, 0x2f, 0xcc, 0x09, 0x97, 0xbe, 0xe8, 0x06, 0xde,
+	0x95, 0x75, 0x16, 0x83, 0x5a, 0xd0, 0x34, 0x1b, 0xb5, 0x75, 0xd4, 0x1c, 0x7c, 0xf2, 0xac, 0xde,
+	0x95, 0x26, 0x78, 0x77, 0xf9, 0x04, 0x91, 0xbd, 0x7f, 0x4d, 0xa8, 0x0e, 0x79, 0x34, 0x65, 0xb3,
+	0x8d, 0xb5, 0x33, 0x5e, 0xb1, 0x76, 0xa7, 0x00, 0x73, 0x22, 0xe7, 0x6e, 0xbe, 0x67, 0xe6, 0x8b,
+	0x7b, 0xd6, 0xc8, 0x3c, 0xf3, 0x7f, 0xb2, 0x8d, 0x96, 0x2b, 0x6f, 0x68, 0xf9, 0x1c, 0x3e, 0x4b,
+	0x24, 0x75, 0xa7, 0x32, 0x6b, 0x55, 0xb0, 0x68, 0xe6, 0x4e, 0xb9, 0x70, 0x1f, 0xfb, 0xb9, 0x00,
+	0x8c, 0x4a, 0xbd, 0xbc, 0x75, 0x7c, 0x90, 0x48, 0x7a, 0x25, 0x6f, 0x73, 0x9f, 0x2b, 0x2e, 0x26,
+	0xfd, 0xfb, 0xc2, 0x01, 0x9d, 0xc1, 0x01, 0x09, 0x02, 0xbe, 0x74, 0x3d, 0xc1, 0xa5, 0x74, 0x13,
+	0x49, 0x85, 0x5b, 0x52, 0xeb, 0x3d, 0xaf, 0xe3, 0x7d, 0xed, 0x30, 0xcc, 0xee, 0x7f, 0x91, 0x54,
+	0xdc, 0x15, 0xb7, 0x37, 0x95, 0xba, 0x65, 0x57, 0x70, 0xdb, 0xe3, 0x61, 0x4c, 0x14, 0x7b, 0x60,
+	0x01, 0x53, 0xe9, 0x17, 0x3f, 0x03, 0xac, 0x65, 0xd9, 0x5e, 0x02, 0x04, 0x9d, 0x98, 0x84, 0x6e,
+	0x4c, 0xa4, 0x8c, 0xe7, 0x82, 0x48, 0x6a, 0x1b, 0xe8, 0x23, 0xd8, 0xf5, 0x12, 0xa9, 0xf8, 0x16,
+	0x6c, 0x66, 0x71, 0x82, 0x2c, 0xb3, 0xae, 0x6c, 0xeb, 0xa1, 0xaa, 0xbf, 0x03, 0x5f, 0xfd, 0x1f,
+	0x00, 0x00, 0xff, 0xff, 0xe2, 0x78, 0x9e, 0x2e, 0x22, 0x06, 0x00, 0x00,
 }
diff --git a/metadata/metadata.proto b/metadata/metadata.proto
index 8ffb4f6e..84245e02 100644
--- a/metadata/metadata.proto
+++ b/metadata/metadata.proto
@@ -99,6 +99,7 @@ message Config {
   HashingCosts hash_costs = 2;
   EncryptionOptions options = 4;
   bool use_fs_keyring_for_v1_policies = 5;
+  bool allow_cross_user_metadata = 6;
 
   // reserve the removed field 'string compatibility = 3;'
   reserved 3;
diff --git a/pam_fscrypt/run_fscrypt.go b/pam_fscrypt/run_fscrypt.go
index ef7ff924..8c640ced 100644
--- a/pam_fscrypt/run_fscrypt.go
+++ b/pam_fscrypt/run_fscrypt.go
@@ -137,6 +137,13 @@ func loginProtector(handle *pam.Handle) (*actions.Protector, error) {
 	if err != nil {
 		return nil, err
 	}
+	// Ensure that pam_fscrypt only processes metadata files owned by the
+	// user or root, even if the user is root themselves.  (Normally, when
+	// fscrypt is run as root it is allowed to process all metadata files.
+	// This implements stricter behavior for pam_fscrypt.)
+	if !ctx.Config.GetAllowCrossUserMetadata() {
+		ctx.TrustedUser = handle.PamUser
+	}
 
 	// Find the user's PAM protector.
 	options, err := ctx.ProtectorOptions()
@@ -164,10 +171,11 @@ func policiesUsingProtector(protector *actions.Protector) []*actions.Policy {
 	var policies []*actions.Policy
 	for _, mount := range mounts {
 		// Skip mountpoints that do not use the protector.
-		if _, _, err := mount.GetProtector(protector.Descriptor()); err != nil {
+		if _, _, err := mount.GetProtector(protector.Descriptor(),
+			protector.Context.TrustedUser); err != nil {
 			continue
 		}
-		policyDescriptors, err := mount.ListPolicies()
+		policyDescriptors, err := mount.ListPolicies(protector.Context.TrustedUser)
 		if err != nil {
 			log.Printf("listing policies: %s", err)
 			continue

From 85a747493ff368a72f511619ecd391016ecb933c Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 23 Feb 2022 12:35:04 -0800
Subject: [PATCH 08/13] Extend ownership validation to entire directory
 structure

A previous commit extended file ownership validation to policy and
protector files (by default -- there's an opt-out in /etc/fscrypt.conf).

However, that didn't apply to the parent directories:

	MOUNTPOINT
	MOUNTPOINT/.fscrypt
	MOUNTPOINT/.fscrypt/policies
	MOUNTPOINT/.fscrypt/protectors

The problem is that if the parent directories aren't trusted (owned by
another non-root user), then untrusted changes to their contents can be
made at any time, including the introduction of symlinks and so on.

While it's debatable how much of a problem this really is, given the
other validations that are done, it seems to be appropriate to validate
the parent directories too.

Therefore, this commit applies the same ownership validations to the
above four directories as are done on the metadata files themselves.

In addition, it is validated that none of these directories are symlinks
except for ".fscrypt" where this is explicitly supported.
---
 actions/context.go            |   2 +-
 cmd/fscrypt/setup.go          |   2 +-
 cmd/fscrypt/status.go         |   2 +-
 filesystem/filesystem.go      | 127 ++++++++++++++++++++++++++++------
 filesystem/filesystem_test.go |  37 ++++++----
 filesystem/path.go            |  29 +++-----
 6 files changed, 144 insertions(+), 55 deletions(-)

diff --git a/actions/context.go b/actions/context.go
index 1ee0d600..ac3f6d30 100644
--- a/actions/context.go
+++ b/actions/context.go
@@ -138,7 +138,7 @@ func (ctx *Context) checkContext() error {
 	if err := ctx.Config.CheckValidity(); err != nil {
 		return &ErrBadConfig{ctx.Config, err}
 	}
-	return ctx.Mount.CheckSetup()
+	return ctx.Mount.CheckSetup(ctx.TrustedUser)
 }
 
 func (ctx *Context) getKeyringOptions() *keyring.Options {
diff --git a/cmd/fscrypt/setup.go b/cmd/fscrypt/setup.go
index 1da0d163..b9a16e81 100644
--- a/cmd/fscrypt/setup.go
+++ b/cmd/fscrypt/setup.go
@@ -83,7 +83,7 @@ func setupFilesystem(w io.Writer, path string) error {
 	}
 	username := ctx.TargetUser.Username
 
-	err = ctx.Mount.CheckSetup()
+	err = ctx.Mount.CheckSetup(ctx.TrustedUser)
 	if err == nil {
 		return &filesystem.ErrAlreadySetup{Mount: ctx.Mount}
 	}
diff --git a/cmd/fscrypt/status.go b/cmd/fscrypt/status.go
index ed5a764e..bc8f1eed 100644
--- a/cmd/fscrypt/status.go
+++ b/cmd/fscrypt/status.go
@@ -101,7 +101,7 @@ func writeGlobalStatus(w io.Writer) error {
 	t := makeTableWriter(w, "MOUNTPOINT\tDEVICE\tFILESYSTEM\tENCRYPTION\tFSCRYPT")
 	for _, mount := range mounts {
 		// Only print mountpoints backed by devices or using fscrypt.
-		usingFscrypt := mount.CheckSetup() == nil
+		usingFscrypt := mount.CheckSetup(nil) == nil
 		if !usingFscrypt && mount.Device == "" {
 			continue
 		}
diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go
index 6567dd66..6e4f2c6f 100644
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -85,6 +85,17 @@ func (err *ErrFollowLink) Error() string {
 		err.Link, err.UnderlyingError)
 }
 
+// ErrInsecurePermissions indicates that a filesystem is not considered to be
+// setup for fscrypt because a metadata directory has insecure permissions.
+type ErrInsecurePermissions struct {
+	Path string
+}
+
+func (err *ErrInsecurePermissions) Error() string {
+	return fmt.Sprintf("%q has insecure permissions (world-writable without sticky bit)",
+		err.Path)
+}
+
 // ErrMakeLink indicates that a protector link can't be created.
 type ErrMakeLink struct {
 	Target          *Mount
@@ -96,6 +107,17 @@ func (err *ErrMakeLink) Error() string {
 		err.Target.Path, err.UnderlyingError)
 }
 
+// ErrMountOwnedByAnotherUser indicates that the mountpoint root directory is
+// owned by a user that isn't trusted in the current context, so we don't
+// consider fscrypt to be properly setup on the filesystem.
+type ErrMountOwnedByAnotherUser struct {
+	Mount *Mount
+}
+
+func (err *ErrMountOwnedByAnotherUser) Error() string {
+	return fmt.Sprintf("another non-root user owns the root directory of %s", err.Mount.Path)
+}
+
 // ErrNoCreatePermission indicates that the current user lacks permission to
 // create fscrypt metadata on the given filesystem.
 type ErrNoCreatePermission struct {
@@ -124,6 +146,17 @@ func (err *ErrNotSetup) Error() string {
 	return fmt.Sprintf("filesystem %s is not setup for use with fscrypt", err.Mount.Path)
 }
 
+// ErrSetupByAnotherUser indicates that one or more of the fscrypt metadata
+// directories is owned by a user that isn't trusted in the current context, so
+// we don't consider fscrypt to be properly setup on the filesystem.
+type ErrSetupByAnotherUser struct {
+	Mount *Mount
+}
+
+func (err *ErrSetupByAnotherUser) Error() string {
+	return fmt.Sprintf("another non-root user owns fscrypt metadata directories on %s", err.Mount.Path)
+}
+
 // ErrSetupNotSupported indicates that the given filesystem type is not
 // supported for fscrypt setup.
 type ErrSetupNotSupported struct {
@@ -384,21 +417,75 @@ func checkOwnership(path string, info os.FileInfo, trustedUser *user.User) bool
 	return true
 }
 
-// CheckSetup returns an error if all the fscrypt metadata directories do not
+// CheckSetup returns an error if any of the fscrypt metadata directories do not
 // exist. Will log any unexpected errors or incorrect permissions.
-func (m *Mount) CheckSetup() error {
+func (m *Mount) CheckSetup(trustedUser *user.User) error {
 	if !m.isFscryptSetupAllowed() {
 		return &ErrNotSetup{m}
 	}
-	// Run all the checks so we will always get all the warnings
-	baseGood := isDirCheckPerm(m.BaseDir(), basePermissions)
-	policyGood := isDir(m.PolicyDir())
-	protectorGood := isDir(m.ProtectorDir())
+	// Check that the mountpoint directory itself is not a symlink and has
+	// proper ownership, as otherwise we can't trust anything beneath it.
+	info, err := loggedLstat(m.Path)
+	if err != nil {
+		return &ErrNotSetup{m}
+	}
+	if (info.Mode() & os.ModeSymlink) != 0 {
+		log.Printf("mountpoint directory %q cannot be a symlink", m.Path)
+		return &ErrNotSetup{m}
+	}
+	if !info.IsDir() {
+		log.Printf("mountpoint %q is not a directory", m.Path)
+		return &ErrNotSetup{m}
+	}
+	if !checkOwnership(m.Path, info, trustedUser) {
+		return &ErrMountOwnedByAnotherUser{m}
+	}
+
+	// Check BaseDir similarly.  However, unlike the other directories, we
+	// allow BaseDir to be a symlink, to support the use case of metadata
+	// for a read-only filesystem being redirected to a writable location.
+	info, err = loggedStat(m.BaseDir())
+	if err != nil {
+		return &ErrNotSetup{m}
+	}
+	if !info.IsDir() {
+		log.Printf("%q is not a directory", m.BaseDir())
+		return &ErrNotSetup{m}
+	}
+	if !checkOwnership(m.Path, info, trustedUser) {
+		return &ErrMountOwnedByAnotherUser{m}
+	}
 
-	if baseGood && policyGood && protectorGood {
-		return nil
+	// Check that the policies and protectors directories aren't symlinks and
+	// have proper ownership.
+	subdirs := []string{m.PolicyDir(), m.ProtectorDir()}
+	for _, path := range subdirs {
+		info, err := loggedLstat(path)
+		if err != nil {
+			return &ErrNotSetup{m}
+		}
+		if (info.Mode() & os.ModeSymlink) != 0 {
+			log.Printf("directory %q cannot be a symlink", path)
+			return &ErrNotSetup{m}
+		}
+		if !info.IsDir() {
+			log.Printf("%q is not a directory", path)
+			return &ErrNotSetup{m}
+		}
+		// We are no longer too picky about the mode, given that
+		// 'fscrypt setup' now offers a choice of two different modes,
+		// and system administrators could customize it further.
+		// However, we can at least verify that if the directory is
+		// world-writable, then the sticky bit is also set.
+		if info.Mode()&(os.ModeSticky|0002) == 0002 {
+			log.Printf("%q is world-writable but doesn't have sticky bit set", path)
+			return &ErrInsecurePermissions{path}
+		}
+		if !checkOwnership(path, info, trustedUser) {
+			return &ErrSetupByAnotherUser{m}
+		}
 	}
-	return &ErrNotSetup{m}
+	return nil
 }
 
 // makeDirectories creates the three metadata directories with the correct
@@ -458,7 +545,7 @@ func (m *Mount) GetSetupMode() (SetupMode, *user.User, error) {
 // the filesystem's feature flags. This operation is atomic; it either succeeds
 // or no files in the baseDir are created.
 func (m *Mount) Setup(mode SetupMode) error {
-	if m.CheckSetup() == nil {
+	if m.CheckSetup(nil) == nil {
 		return &ErrAlreadySetup{m}
 	}
 	if !m.isFscryptSetupAllowed() {
@@ -485,7 +572,7 @@ func (m *Mount) Setup(mode SetupMode) error {
 // WARNING: Will cause data loss if the metadata is used to encrypt
 // directories (this could include directories on other filesystems).
 func (m *Mount) RemoveAllMetadata() error {
-	if err := m.CheckSetup(); err != nil {
+	if err := m.CheckSetup(nil); err != nil {
 		return err
 	}
 	// temp will hold the old metadata temporarily
@@ -701,7 +788,7 @@ func (m *Mount) removeMetadata(path string) error {
 // already exists on the filesystem.
 func (m *Mount) AddProtector(data *metadata.ProtectorData) error {
 	var err error
-	if err = m.CheckSetup(); err != nil {
+	if err = m.CheckSetup(nil); err != nil {
 		return err
 	}
 	if isRegularFile(m.linkedProtectorPath(data.ProtectorDescriptor)) {
@@ -724,7 +811,7 @@ func (m *Mount) AddProtector(data *metadata.ProtectorData) error {
 // in the dest filesystem, if one doesn't already exist.  On success, the return
 // value is a nil error and a bool that is true iff the link is newly created.
 func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount, trustedUser *user.User) (bool, error) {
-	if err := m.CheckSetup(); err != nil {
+	if err := m.CheckSetup(trustedUser); err != nil {
 		return false, err
 	}
 	// Check that the link is good (descriptor exists, filesystem has UUID).
@@ -762,7 +849,7 @@ func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount, trustedUser *
 // GetRegularProtector looks up the protector metadata by descriptor. This will
 // fail with ErrProtectorNotFound if the descriptor is a linked protector.
 func (m *Mount) GetRegularProtector(descriptor string, trustedUser *user.User) (*metadata.ProtectorData, error) {
-	if err := m.CheckSetup(); err != nil {
+	if err := m.CheckSetup(trustedUser); err != nil {
 		return nil, err
 	}
 	data := new(metadata.ProtectorData)
@@ -792,7 +879,7 @@ func (m *Mount) GetRegularProtector(descriptor string, trustedUser *user.User) (
 // and that protector's data. If the descriptor is a regular (not linked)
 // protector, the mount will return itself.
 func (m *Mount) GetProtector(descriptor string, trustedUser *user.User) (*Mount, *metadata.ProtectorData, error) {
-	if err := m.CheckSetup(); err != nil {
+	if err := m.CheckSetup(trustedUser); err != nil {
 		return nil, nil, err
 	}
 	// Get the link data from the link file
@@ -821,7 +908,7 @@ func (m *Mount) GetProtector(descriptor string, trustedUser *user.User) (*Mount,
 // RemoveProtector deletes the protector metadata (or a link to another
 // filesystem's metadata) from the filesystem storage.
 func (m *Mount) RemoveProtector(descriptor string) error {
-	if err := m.CheckSetup(); err != nil {
+	if err := m.CheckSetup(nil); err != nil {
 		return err
 	}
 	// We first try to remove the linkedProtector. If that metadata does not
@@ -845,7 +932,7 @@ func (m *Mount) ListProtectors(trustedUser *user.User) ([]string, error) {
 
 // AddPolicy adds the policy metadata to the filesystem storage.
 func (m *Mount) AddPolicy(data *metadata.PolicyData) error {
-	if err := m.CheckSetup(); err != nil {
+	if err := m.CheckSetup(nil); err != nil {
 		return err
 	}
 
@@ -854,7 +941,7 @@ func (m *Mount) AddPolicy(data *metadata.PolicyData) error {
 
 // GetPolicy looks up the policy metadata by descriptor.
 func (m *Mount) GetPolicy(descriptor string, trustedUser *user.User) (*metadata.PolicyData, error) {
-	if err := m.CheckSetup(); err != nil {
+	if err := m.CheckSetup(trustedUser); err != nil {
 		return nil, err
 	}
 	data := new(metadata.PolicyData)
@@ -867,7 +954,7 @@ func (m *Mount) GetPolicy(descriptor string, trustedUser *user.User) (*metadata.
 
 // RemovePolicy deletes the policy metadata from the filesystem storage.
 func (m *Mount) RemovePolicy(descriptor string) error {
-	if err := m.CheckSetup(); err != nil {
+	if err := m.CheckSetup(nil); err != nil {
 		return err
 	}
 	err := m.removeMetadata(m.PolicyPath(descriptor))
@@ -945,7 +1032,7 @@ func (m *Mount) listDirectory(directoryPath string) ([]string, error) {
 
 func (m *Mount) listMetadata(dirPath string, metadataType string, owner *user.User) ([]string, error) {
 	log.Printf("listing %s in %q", metadataType, dirPath)
-	if err := m.CheckSetup(); err != nil {
+	if err := m.CheckSetup(owner); err != nil {
 		return nil, err
 	}
 	names, err := m.listDirectory(dirPath)
diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go
index d4ef826d..92e113b2 100644
--- a/filesystem/filesystem_test.go
+++ b/filesystem/filesystem_test.go
@@ -21,7 +21,6 @@ package filesystem
 
 import (
 	"io/ioutil"
-	"log"
 	"os"
 	"os/user"
 	"path/filepath"
@@ -103,7 +102,7 @@ func TestSetup(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if err := mnt.CheckSetup(); err != nil {
+	if err := mnt.CheckSetup(nil); err != nil {
 		t.Error(err)
 	}
 
@@ -126,16 +125,6 @@ func TestRemoveAllMetadata(t *testing.T) {
 	}
 }
 
-// loggedLstat runs os.Lstat (doesn't dereference trailing symlink), but it logs
-// the error if lstat returns any error other than nil or IsNotExist.
-func loggedLstat(name string) (os.FileInfo, error) {
-	info, err := os.Lstat(name)
-	if err != nil && !os.IsNotExist(err) {
-		log.Print(err)
-	}
-	return info, err
-}
-
 // isSymlink returns true if the path exists and is that of a symlink.
 func isSymlink(path string) bool {
 	info, err := loggedLstat(path)
@@ -158,7 +147,7 @@ func testSetupWithSymlink(t *testing.T, mnt *Mount, symlinkTarget string, realDi
 		t.Fatal(err)
 	}
 	defer mnt.RemoveAllMetadata()
-	if err := mnt.CheckSetup(); err != nil {
+	if err := mnt.CheckSetup(nil); err != nil {
 		t.Fatal(err)
 	}
 	if !isSymlink(rawBaseDir) {
@@ -233,6 +222,28 @@ func TestSetupModes(t *testing.T) {
 	testSetupMode(t, mnt, SingleUserWritable, 0755)
 }
 
+// Tests that fscrypt refuses to use metadata directories that are
+// world-writable but don't have the sticky bit set.
+func TestInsecurePermissions(t *testing.T) {
+	mnt, err := getTestMount(t)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer mnt.RemoveAllMetadata()
+
+	if err = mnt.Setup(WorldWritable); err != nil {
+		t.Fatal(err)
+	}
+	if err = os.Chmod(mnt.PolicyDir(), 0777); err != nil {
+		t.Fatal(err)
+	}
+	defer os.Chmod(mnt.PolicyDir(), os.ModeSticky|0777)
+	err = mnt.CheckSetup(nil)
+	if _, ok := err.(*ErrInsecurePermissions); !ok {
+		t.Fatal("expected ErrInsecurePermissions")
+	}
+}
+
 // Adding a good Protector should succeed, adding a bad one should fail
 func TestAddProtector(t *testing.T) {
 	mnt, err := getSetupMount(t)
diff --git a/filesystem/path.go b/filesystem/path.go
index fa387013..8cfb2357 100644
--- a/filesystem/path.go
+++ b/filesystem/path.go
@@ -38,9 +38,6 @@ func OpenFileOverridingUmask(name string, flag int, perm os.FileMode) (*os.File,
 	return os.OpenFile(name, flag, perm)
 }
 
-// We only check the unix permissions and the sticky bit
-const permMask = os.ModeSticky | os.ModePerm
-
 // canonicalizePath turns path into an absolute path without symlinks.
 func canonicalizePath(path string) (string, error) {
 	path, err := filepath.Abs(path)
@@ -67,28 +64,22 @@ func loggedStat(name string) (os.FileInfo, error) {
 	return info, err
 }
 
+// loggedLstat runs os.Lstat (doesn't dereference trailing symlink), but it logs
+// the error if lstat returns any error other than nil or IsNotExist.
+func loggedLstat(name string) (os.FileInfo, error) {
+	info, err := os.Lstat(name)
+	if err != nil && !os.IsNotExist(err) {
+		log.Print(err)
+	}
+	return info, err
+}
+
 // isDir returns true if the path exists and is that of a directory.
 func isDir(path string) bool {
 	info, err := loggedStat(path)
 	return err == nil && info.IsDir()
 }
 
-// isDirCheckPerm returns true if the path exists and is a directory. If the
-// specified permissions and sticky bit of mode do not match the path, an error
-// is logged.
-func isDirCheckPerm(path string, mode os.FileMode) bool {
-	info, err := loggedStat(path)
-	// Check if directory
-	if err != nil || !info.IsDir() {
-		return false
-	}
-	// Check for bad permissions
-	if info.Mode()&permMask != mode&permMask {
-		log.Printf("directory %s has incorrect permissions", path)
-	}
-	return true
-}
-
 // isRegularFile returns true if the path exists and is that of a regular file.
 func isRegularFile(path string) bool {
 	info, err := loggedStat(path)

From d4ce0b892cbe68db9f90f4015342e6a9069b079c Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 23 Feb 2022 12:35:04 -0800
Subject: [PATCH 09/13] Make all new metadata files owned by user when needed

Since commit 4c7c6631cc5a ("Set owner of login protectors to correct
user"), login protectors are made owned by the user when root creates
one on a user's behalf.  That's good, but the same isn't true of other
files that get created at the same time:

- The policy protecting the directory
- The protector link file, if the policy is on a different filesystem
- The recovery protector, if the policy is on a different filesystem
- The recovery instructions file

In preparation for setting all metadata files to mode 0600, start making
all these files owned by the user in this scenario as well.
---
 actions/policy.go             | 36 +++++++++++++++++++++++++++++++++--
 actions/policy_test.go        |  8 ++++----
 actions/protector.go          | 16 +++++++++-------
 actions/protector_test.go     |  4 ++--
 actions/recovery.go           |  8 +++++++-
 cli-tests/t_encrypt_login.out |  2 ++
 cli-tests/t_encrypt_login.sh  |  8 ++++++++
 cmd/fscrypt/protector.go      |  8 ++++++--
 filesystem/filesystem.go      | 21 ++++++++------------
 filesystem/filesystem_test.go | 34 ++++++++++++++++-----------------
 10 files changed, 97 insertions(+), 48 deletions(-)

diff --git a/actions/policy.go b/actions/policy.go
index 3b8eb30c..3b201769 100644
--- a/actions/policy.go
+++ b/actions/policy.go
@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"os/user"
 
 	"github.com/golang/protobuf/proto"
 	"github.com/pkg/errors"
@@ -178,6 +179,7 @@ type Policy struct {
 	data                *metadata.PolicyData
 	key                 *crypto.Key
 	created             bool
+	ownerIfCreating     *user.User
 	newLinkedProtectors []string
 }
 
@@ -210,6 +212,12 @@ func CreatePolicy(ctx *Context, protector *Protector) (*Policy, error) {
 		created: true,
 	}
 
+	policy.ownerIfCreating, err = getOwnerOfMetadataForProtector(protector)
+	if err != nil {
+		policy.Lock()
+		return nil, err
+	}
+
 	if err = policy.AddProtector(protector); err != nil {
 		policy.Lock()
 		return nil, err
@@ -410,6 +418,25 @@ func (policy *Policy) UsesProtector(protector *Protector) bool {
 	return ok
 }
 
+// getOwnerOfMetadataForProtector returns the User to whom the owner of any new
+// policies or protector links for the given protector should be set.
+//
+// This will return a non-nil value only when the protector is a login protector
+// and the process is running as root.  In this scenario, root is setting up
+// encryption on the user's behalf, so we need to make new policies and
+// protector links owned by the user (rather than root) to allow them to be read
+// by the user, just like the login protector itself which is handled elsewhere.
+func getOwnerOfMetadataForProtector(protector *Protector) (*user.User, error) {
+	if protector.data.Source == metadata.SourceType_pam_passphrase && util.IsUserRoot() {
+		owner, err := util.UserFromUID(protector.data.Uid)
+		if err != nil {
+			return nil, err
+		}
+		return owner, nil
+	}
+	return nil, nil
+}
+
 // AddProtector updates the data that is wrapping the Policy Key so that the
 // provided Protector is now protecting the specified Policy. If an error is
 // returned, no data has been changed. If the policy and protector are on
@@ -427,8 +454,13 @@ func (policy *Policy) AddProtector(protector *Protector) error {
 	// to it on the policy's filesystem.
 	if policy.Context.Mount != protector.Context.Mount {
 		log.Printf("policy on %s\n protector on %s\n", policy.Context.Mount, protector.Context.Mount)
+		ownerIfCreating, err := getOwnerOfMetadataForProtector(protector)
+		if err != nil {
+			return err
+		}
 		isNewLink, err := policy.Context.Mount.AddLinkedProtector(
-			protector.Descriptor(), protector.Context.Mount, protector.Context.TrustedUser)
+			protector.Descriptor(), protector.Context.Mount,
+			protector.Context.TrustedUser, ownerIfCreating)
 		if err != nil {
 			return err
 		}
@@ -554,7 +586,7 @@ func (policy *Policy) CanBeAppliedWithoutProvisioning() bool {
 
 // commitData writes the Policy's current data to the filesystem.
 func (policy *Policy) commitData() error {
-	return policy.Context.Mount.AddPolicy(policy.data)
+	return policy.Context.Mount.AddPolicy(policy.data, policy.ownerIfCreating)
 }
 
 // findWrappedPolicyKey returns the index of the wrapped policy key
diff --git a/actions/policy_test.go b/actions/policy_test.go
index 07943b88..8248862e 100644
--- a/actions/policy_test.go
+++ b/actions/policy_test.go
@@ -27,7 +27,7 @@ import (
 
 // Makes a protector and policy
 func makeBoth() (*Protector, *Policy, error) {
-	protector, err := CreateProtector(testContext, testProtectorName, goodCallback)
+	protector, err := CreateProtector(testContext, testProtectorName, goodCallback, nil)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -68,7 +68,7 @@ func TestPolicyGoodAddProtector(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	pro2, err := CreateProtector(testContext, testProtectorName2, goodCallback)
+	pro2, err := CreateProtector(testContext, testProtectorName2, goodCallback, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -103,7 +103,7 @@ func TestPolicyGoodRemoveProtector(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	pro2, err := CreateProtector(testContext, testProtectorName2, goodCallback)
+	pro2, err := CreateProtector(testContext, testProtectorName2, goodCallback, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -129,7 +129,7 @@ func TestPolicyBadRemoveProtector(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	pro2, err := CreateProtector(testContext, testProtectorName2, goodCallback)
+	pro2, err := CreateProtector(testContext, testProtectorName2, goodCallback, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/actions/protector.go b/actions/protector.go
index 1171c837..b986eb02 100644
--- a/actions/protector.go
+++ b/actions/protector.go
@@ -109,17 +109,18 @@ func checkIfUserHasLoginProtector(ctx *Context, uid int64) error {
 // to unlock policies and create new polices. As with the key struct, a
 // Protector should be wiped after use.
 type Protector struct {
-	Context *Context
-	data    *metadata.ProtectorData
-	key     *crypto.Key
-	created bool
+	Context         *Context
+	data            *metadata.ProtectorData
+	key             *crypto.Key
+	created         bool
+	ownerIfCreating *user.User
 }
 
 // CreateProtector creates an unlocked protector with a given name (name only
 // needed for custom and raw protector types). The keyFn provided to create the
 // Protector key will only be called once. If an error is returned, no data has
 // been changed on the filesystem.
-func CreateProtector(ctx *Context, name string, keyFn KeyFunc) (*Protector, error) {
+func CreateProtector(ctx *Context, name string, keyFn KeyFunc, owner *user.User) (*Protector, error) {
 	if err := ctx.checkContext(); err != nil {
 		return nil, err
 	}
@@ -147,7 +148,8 @@ func CreateProtector(ctx *Context, name string, keyFn KeyFunc) (*Protector, erro
 			Name:   name,
 			Source: ctx.Config.Source,
 		},
-		created: true,
+		created:         true,
+		ownerIfCreating: owner,
 	}
 
 	// Extra data is needed for some SourceTypes
@@ -294,5 +296,5 @@ func (protector *Protector) Rewrap(keyFn KeyFunc) error {
 		return err
 	}
 
-	return protector.Context.Mount.AddProtector(protector.data)
+	return protector.Context.Mount.AddProtector(protector.data, protector.ownerIfCreating)
 }
diff --git a/actions/protector_test.go b/actions/protector_test.go
index 6c81d499..f20dbcfb 100644
--- a/actions/protector_test.go
+++ b/actions/protector_test.go
@@ -43,7 +43,7 @@ func badCallback(info ProtectorInfo, retry bool) (*crypto.Key, error) {
 
 // Tests that we can create a valid protector.
 func TestCreateProtector(t *testing.T) {
-	p, err := CreateProtector(testContext, testProtectorName, goodCallback)
+	p, err := CreateProtector(testContext, testProtectorName, goodCallback, nil)
 	if err != nil {
 		t.Error(err)
 	} else {
@@ -54,7 +54,7 @@ func TestCreateProtector(t *testing.T) {
 
 // Tests that a failure in the callback is relayed back to the caller.
 func TestBadCallback(t *testing.T) {
-	p, err := CreateProtector(testContext, testProtectorName, badCallback)
+	p, err := CreateProtector(testContext, testProtectorName, badCallback, nil)
 	if err == nil {
 		p.Lock()
 		p.Destroy()
diff --git a/actions/recovery.go b/actions/recovery.go
index f5339062..8a769cc7 100644
--- a/actions/recovery.go
+++ b/actions/recovery.go
@@ -25,6 +25,7 @@ import (
 
 	"github.com/google/fscrypt/crypto"
 	"github.com/google/fscrypt/metadata"
+	"github.com/google/fscrypt/util"
 )
 
 // modifiedContextWithSource returns a copy of ctx with the protector source
@@ -66,7 +67,7 @@ func AddRecoveryPassphrase(policy *Policy, dirname string) (*crypto.Key, *Protec
 		if seq != 1 {
 			name += " (" + strconv.Itoa(seq) + ")"
 		}
-		recoveryProtector, err = CreateProtector(customCtx, name, getPassphraseFn)
+		recoveryProtector, err = CreateProtector(customCtx, name, getPassphraseFn, policy.ownerIfCreating)
 		if err == nil {
 			break
 		}
@@ -121,5 +122,10 @@ It is safe to keep it around though, as the recovery passphrase is high-entropy.
 	if _, err = file.WriteString(str); err != nil {
 		return err
 	}
+	if recoveryProtector.ownerIfCreating != nil {
+		if err = util.Chown(file, recoveryProtector.ownerIfCreating); err != nil {
+			return err
+		}
+	}
 	return file.Sync()
 }
diff --git a/cli-tests/t_encrypt_login.out b/cli-tests/t_encrypt_login.out
index b84216a1..bb91a464 100644
--- a/cli-tests/t_encrypt_login.out
+++ b/cli-tests/t_encrypt_login.out
@@ -118,6 +118,8 @@ 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
+"MNT/dir" is now locked.
+"MNT/dir" is now locked.
 
 # 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 225a47d3..b6ae2d8d 100755
--- a/cli-tests/t_encrypt_login.sh
+++ b/cli-tests/t_encrypt_login.sh
@@ -58,9 +58,17 @@ 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.
+# This is partly redundant with the below check, but we might as well test both.
 login_protector=$(_get_login_descriptor)
 owner=$(stat -c "%U:%G" "$MNT_ROOT/.fscrypt/protectors/$login_protector")
 echo -e "\nProtector is owned by $owner"
+# The user should be able to lock and unlock the directory themselves.  This
+# tests that the fscrypt metadata file permissions got set appropriately when
+# root set up the encryption on the user's behalf.
+chown "$TEST_USER" "$dir"
+_user_do "fscrypt lock $dir"
+_user_do "echo TEST_USER_PASS | fscrypt unlock $dir --quiet --unlock-with=$MNT_ROOT:$login_protector"
+_user_do "fscrypt lock $dir"
 
 begin "Encrypt with login protector with --no-recovery"
 chown "$TEST_USER" "$dir"
diff --git a/cmd/fscrypt/protector.go b/cmd/fscrypt/protector.go
index ac864dd4..186ca7ac 100644
--- a/cmd/fscrypt/protector.go
+++ b/cmd/fscrypt/protector.go
@@ -23,6 +23,7 @@ package main
 import (
 	"fmt"
 	"log"
+	"os/user"
 
 	"github.com/google/fscrypt/actions"
 	"github.com/google/fscrypt/filesystem"
@@ -38,7 +39,6 @@ func createProtectorFromContext(ctx *actions.Context) (*actions.Protector, error
 		return nil, err
 	}
 	log.Printf("using source: %s", ctx.Config.Source.String())
-
 	if ctx.Config.Source == metadata.SourceType_pam_passphrase {
 		if userFlag.Value == "" && util.IsUserRoot() {
 			return nil, ErrSpecifyUser
@@ -70,7 +70,11 @@ IMPORTANT: Before continuing, ensure you have properly set up your system for
 		}
 	}
 
-	return actions.CreateProtector(ctx, name, createKeyFn)
+	var owner *user.User
+	if ctx.Config.Source == metadata.SourceType_pam_passphrase && util.IsUserRoot() {
+		owner = ctx.TargetUser
+	}
+	return actions.CreateProtector(ctx, name, createKeyFn, owner)
 }
 
 // selectExistingProtector returns a locked Protector which corresponds to an
diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go
index 6e4f2c6f..1877b1b2 100644
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -649,6 +649,8 @@ func (m *Mount) writeData(path string, data []byte, owner *user.User) error {
 		tempFile.Close()
 		return err
 	}
+	// Override the file owner if one was specified.  This happens when root
+	// needs to create files owned by a particular user.
 	if owner != nil {
 		if err = util.Chown(tempFile, owner); err != nil {
 			log.Printf("could not set owner of %q to %v: %v",
@@ -786,7 +788,7 @@ func (m *Mount) removeMetadata(path string) error {
 // will overwrite the value of an existing protector with this descriptor. This
 // will fail with ErrLinkedProtector if a linked protector with this descriptor
 // already exists on the filesystem.
-func (m *Mount) AddProtector(data *metadata.ProtectorData) error {
+func (m *Mount) AddProtector(data *metadata.ProtectorData, owner *user.User) error {
 	var err error
 	if err = m.CheckSetup(nil); err != nil {
 		return err
@@ -796,21 +798,14 @@ func (m *Mount) AddProtector(data *metadata.ProtectorData) error {
 			data.ProtectorDescriptor, m.Path)
 	}
 	path := m.protectorPath(data.ProtectorDescriptor)
-
-	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
 // in the dest filesystem, if one doesn't already exist.  On success, the return
 // value is a nil error and a bool that is true iff the link is newly created.
-func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount, trustedUser *user.User) (bool, error) {
+func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount, trustedUser *user.User,
+	ownerIfCreating *user.User) (bool, error) {
 	if err := m.CheckSetup(trustedUser); err != nil {
 		return false, err
 	}
@@ -843,7 +838,7 @@ func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount, trustedUser *
 	if err != nil {
 		return false, err
 	}
-	return true, m.writeData(linkPath, []byte(newLink), nil)
+	return true, m.writeData(linkPath, []byte(newLink), ownerIfCreating)
 }
 
 // GetRegularProtector looks up the protector metadata by descriptor. This will
@@ -931,12 +926,12 @@ func (m *Mount) ListProtectors(trustedUser *user.User) ([]string, error) {
 }
 
 // AddPolicy adds the policy metadata to the filesystem storage.
-func (m *Mount) AddPolicy(data *metadata.PolicyData) error {
+func (m *Mount) AddPolicy(data *metadata.PolicyData, owner *user.User) error {
 	if err := m.CheckSetup(nil); err != nil {
 		return err
 	}
 
-	return m.addMetadata(m.PolicyPath(data.KeyDescriptor), data, nil)
+	return m.addMetadata(m.PolicyPath(data.KeyDescriptor), data, owner)
 }
 
 // GetPolicy looks up the policy metadata by descriptor.
diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go
index 92e113b2..f74078d6 100644
--- a/filesystem/filesystem_test.go
+++ b/filesystem/filesystem_test.go
@@ -253,31 +253,31 @@ func TestAddProtector(t *testing.T) {
 	defer mnt.RemoveAllMetadata()
 
 	protector := getFakeProtector()
-	if err = mnt.AddProtector(protector); err != nil {
+	if err = mnt.AddProtector(protector, nil); err != nil {
 		t.Error(err)
 	}
 
 	// Change the source to bad one, or one that requires hashing costs
 	protector.Source = metadata.SourceType_default
-	if mnt.AddProtector(protector) == nil {
+	if mnt.AddProtector(protector, nil) == nil {
 		t.Error("bad source for a descriptor should make metadata invalid")
 	}
 	protector.Source = metadata.SourceType_custom_passphrase
-	if mnt.AddProtector(protector) == nil {
+	if mnt.AddProtector(protector, nil) == nil {
 		t.Error("protectors using passphrases should require hashing costs")
 	}
 	protector.Source = metadata.SourceType_raw_key
 
 	// Use a bad wrapped key
 	protector.WrappedKey = wrappedPolicyKey
-	if mnt.AddProtector(protector) == nil {
+	if mnt.AddProtector(protector, nil) == nil {
 		t.Error("bad length for protector keys should make metadata invalid")
 	}
 	protector.WrappedKey = wrappedProtectorKey
 
 	// Change the descriptor (to a bad length)
 	protector.ProtectorDescriptor = "abcde"
-	if mnt.AddProtector(protector) == nil {
+	if mnt.AddProtector(protector, nil) == nil {
 		t.Error("bad descriptor length should make metadata invalid")
 	}
 
@@ -292,32 +292,32 @@ func TestAddPolicy(t *testing.T) {
 	defer mnt.RemoveAllMetadata()
 
 	policy := getFakePolicy()
-	if err = mnt.AddPolicy(policy); err != nil {
+	if err = mnt.AddPolicy(policy, nil); err != nil {
 		t.Error(err)
 	}
 
 	// Bad encryption options should make policy invalid
 	policy.Options.Padding = 7
-	if mnt.AddPolicy(policy) == nil {
+	if mnt.AddPolicy(policy, nil) == nil {
 		t.Error("padding not a power of 2 should make metadata invalid")
 	}
 	policy.Options.Padding = 16
 	policy.Options.Filenames = metadata.EncryptionOptions_default
-	if mnt.AddPolicy(policy) == nil {
+	if mnt.AddPolicy(policy, nil) == nil {
 		t.Error("encryption mode not set should make metadata invalid")
 	}
 	policy.Options.Filenames = metadata.EncryptionOptions_AES_256_CTS
 
 	// Use a bad wrapped key
 	policy.WrappedPolicyKeys[0].WrappedKey = wrappedProtectorKey
-	if mnt.AddPolicy(policy) == nil {
+	if mnt.AddPolicy(policy, nil) == nil {
 		t.Error("bad length for policy keys should make metadata invalid")
 	}
 	policy.WrappedPolicyKeys[0].WrappedKey = wrappedPolicyKey
 
 	// Change the descriptor (to a bad length)
 	policy.KeyDescriptor = "abcde"
-	if mnt.AddPolicy(policy) == nil {
+	if mnt.AddPolicy(policy, nil) == nil {
 		t.Error("bad descriptor length should make metadata invalid")
 	}
 }
@@ -331,7 +331,7 @@ func TestSetPolicy(t *testing.T) {
 	defer mnt.RemoveAllMetadata()
 
 	policy := getFakePolicy()
-	if err = mnt.AddPolicy(policy); err != nil {
+	if err = mnt.AddPolicy(policy, nil); err != nil {
 		t.Fatal(err)
 	}
 
@@ -355,7 +355,7 @@ func TestSetProtector(t *testing.T) {
 	defer mnt.RemoveAllMetadata()
 
 	protector := getFakeProtector()
-	if err = mnt.AddProtector(protector); err != nil {
+	if err = mnt.AddProtector(protector, nil); err != nil {
 		t.Fatal(err)
 	}
 
@@ -383,7 +383,7 @@ func TestSpoofedLoginProtector(t *testing.T) {
 
 	// Control case: protector with matching UID should be accepted.
 	protector := getFakeLoginProtector(myUID)
-	if err = mnt.AddProtector(protector); err != nil {
+	if err = mnt.AddProtector(protector, nil); err != nil {
 		t.Fatal(err)
 	}
 	_, err = mnt.GetRegularProtector(protector.ProtectorDescriptor, nil)
@@ -398,7 +398,7 @@ func TestSpoofedLoginProtector(t *testing.T) {
 	// *unless* the process running the tests (and hence the file owner) is
 	// root in which case it should be accepted.
 	protector = getFakeLoginProtector(badUID)
-	if err = mnt.AddProtector(protector); err != nil {
+	if err = mnt.AddProtector(protector, nil); err != nil {
 		t.Fatal(err)
 	}
 	_, err = mnt.GetRegularProtector(protector.ProtectorDescriptor, nil)
@@ -445,19 +445,19 @@ func TestLinkedProtector(t *testing.T) {
 
 	// Add the protector to the first filesystem
 	protector := getFakeProtector()
-	if err = realMnt.AddProtector(protector); err != nil {
+	if err = realMnt.AddProtector(protector, nil); err != nil {
 		t.Fatal(err)
 	}
 
 	// Add the link to the second filesystem
 	var isNewLink bool
-	if isNewLink, err = fakeMnt.AddLinkedProtector(protector.ProtectorDescriptor, realMnt, nil); err != nil {
+	if isNewLink, err = fakeMnt.AddLinkedProtector(protector.ProtectorDescriptor, realMnt, nil, nil); err != nil {
 		t.Fatal(err)
 	}
 	if !isNewLink {
 		t.Fatal("Link was not new")
 	}
-	if isNewLink, err = fakeMnt.AddLinkedProtector(protector.ProtectorDescriptor, realMnt, nil); err != nil {
+	if isNewLink, err = fakeMnt.AddLinkedProtector(protector.ProtectorDescriptor, realMnt, nil, nil); err != nil {
 		t.Fatal(err)
 	}
 	if isNewLink {

From 312bc381a3751e397995eeb2e63e66856912fafb Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 23 Feb 2022 12:35:04 -0800
Subject: [PATCH 10/13] filesystem: preserve metadata file permissions on
 updates

Since fscrypt replaces metadata files rather than overwrites them (to
get atomicity), their owner will change to root if root makes a change.
That isn't too much of an issue when the files have mode 0644.  However,
it will become a much bigger issue when the files have mode 0600,
especially because existing files with mode 0644 would also get changed
to have mode 0600.

In preparation for this, start preserving the previous owner and mode of
policy and protector files when they are updated.
---
 filesystem/filesystem.go | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go
index 1877b1b2..70076b72 100644
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -254,7 +254,7 @@ const (
 	basePermissions = 0755
 	// The metadata files are globally visible, but can only be deleted by
 	// the user that created them
-	filePermissions = 0644
+	filePermissions = os.FileMode(0644)
 
 	// Maximum size of a metadata file.  This value is arbitrary, and it can
 	// be changed.  We just set a reasonable limit that shouldn't be reached
@@ -626,7 +626,7 @@ func (m *Mount) overwriteDataNonAtomic(path string, data []byte) error {
 // However, if the process doesn't have write permission to the directory but
 // does have write permission to the file itself, then as a fallback the file is
 // overwritten in-place rather than replaced.  Note that this may be non-atomic.
-func (m *Mount) writeData(path string, data []byte, owner *user.User) error {
+func (m *Mount) writeData(path string, data []byte, owner *user.User, mode os.FileMode) 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)
@@ -644,8 +644,8 @@ func (m *Mount) writeData(path string, data []byte, owner *user.User) error {
 	}
 	defer os.Remove(tempFile.Name())
 
-	// TempFile() creates the file with mode 0600.  Change it to 0644.
-	if err = tempFile.Chmod(filePermissions); err != nil {
+	// Ensure the new file has the right permissions mask.
+	if err = tempFile.Chmod(mode); err != nil {
 		tempFile.Close()
 		return err
 	}
@@ -690,8 +690,29 @@ func (m *Mount) addMetadata(path string, md metadata.Metadata, owner *user.User)
 		return err
 	}
 
-	log.Printf("writing metadata to %q", path)
-	return m.writeData(path, data, owner)
+	mode := filePermissions
+	// If the file already exists, then preserve its owner and mode if
+	// possible.  This is necessary because by default, for atomicity
+	// reasons we'll replace the file rather than overwrite it.
+	info, err := os.Lstat(path)
+	if err == nil {
+		if owner == nil && util.IsUserRoot() {
+			uid := info.Sys().(*syscall.Stat_t).Uid
+			if owner, err = util.UserFromUID(int64(uid)); err != nil {
+				log.Print(err)
+			}
+		}
+		mode = info.Mode() & 0777
+	} else if !os.IsNotExist(err) {
+		log.Print(err)
+	}
+
+	if owner != nil {
+		log.Printf("writing metadata to %q and setting owner to %s", path, owner.Username)
+	} else {
+		log.Printf("writing metadata to %q", path)
+	}
+	return m.writeData(path, data, owner, mode)
 }
 
 // readMetadataFileSafe gets the contents of a metadata file extra-carefully,
@@ -838,7 +859,7 @@ func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount, trustedUser *
 	if err != nil {
 		return false, err
 	}
-	return true, m.writeData(linkPath, []byte(newLink), ownerIfCreating)
+	return true, m.writeData(linkPath, []byte(newLink), ownerIfCreating, filePermissions)
 }
 
 // GetRegularProtector looks up the protector metadata by descriptor. This will

From 06c989df4e31dd9f172f94fbd6243f49d4dd0b92 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 23 Feb 2022 12:35:04 -0800
Subject: [PATCH 11/13] filesystem: create metadata files with mode 0600

Currently, fscrypt policies and protectors are world readable, as they
are created with mode 0644.  While this can be nice for use cases where
users share these files, those use cases seem to be quite rare, and it's
not a great default security-wise since it exposes password hashes to
all users.  While fscrypt uses a very strong password hash algorithm, it
would still be best to follow the lead of /etc/shadow and keep this
information non-world-readable.

Therefore, start creating these files with mode 0600.

Of course, if users do actually want to share these files, they have the
option of simply chmod'ing them to a less restrictive mode.  An option
could also be added to make fscrypt use the old mode 0644; however, the
need for that is currently unclear.
---
 README.md                     |  4 +++-
 cli-tests/t_lock.out          |  1 -
 cli-tests/t_lock.sh           |  5 ++++-
 filesystem/filesystem.go      | 11 ++++++++---
 filesystem/filesystem_test.go | 35 +++++++++++++++++++++++++++++++++++
 5 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/README.md b/README.md
index f1803b46..eff9ecf5 100644
--- a/README.md
+++ b/README.md
@@ -385,7 +385,9 @@ The fields are:
   other users might be untrusted and could create malicious files.  This can be
   set to `true` to restore the old behavior on systems where `fscrypt` metadata
   needs to be shared between multiple users.  Note that this option is
-  independent from the permissions on the metadata files themselves.
+  independent from the permissions on the metadata files themselves, which are
+  set to 0600 by default; users who wish to share their metadata files with
+  other users would also need to explicitly change their mode to 0644.
 
 ## Setting up `fscrypt` on a filesystem
 
diff --git a/cli-tests/t_lock.out b/cli-tests/t_lock.out
index b8c8dcbb..0da8964f 100644
--- a/cli-tests/t_lock.out
+++ b/cli-tests/t_lock.out
@@ -76,7 +76,6 @@ cat: MNT/dir/file: No such file or directory
 mkdir: cannot create directory 'MNT/dir/subdir': Required key not available
 
 # Try to lock directory while other user has unlocked
-Enter custom passphrase for protector "prot": "MNT/dir" is now unlocked and ready for use.
 [ERROR] fscrypt lock: Directory "MNT/dir" couldn't be fully
                       locked because other user(s) have unlocked it.
 
diff --git a/cli-tests/t_lock.sh b/cli-tests/t_lock.sh
index 7ac17274..9b193fdd 100755
--- a/cli-tests/t_lock.sh
+++ b/cli-tests/t_lock.sh
@@ -43,8 +43,11 @@ _expect_failure "cat '$dir/file'"
 _expect_failure "mkdir '$dir/subdir'"
 
 _print_header "Try to lock directory while other user has unlocked"
+rm -rf "$dir"
+mkdir "$dir"
 chown "$TEST_USER" "$dir"
-_user_do "echo hunter2 | fscrypt unlock '$dir'"
+_user_do "echo hunter2 | fscrypt encrypt --quiet --name=prot '$dir'"
+_user_do "echo contents > $dir/file"
 _expect_failure "fscrypt lock '$dir'"
 cat "$dir/file"
 fscrypt lock --all-users "$dir"
diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go
index 70076b72..27bfa241 100644
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -252,9 +252,14 @@ const (
 
 	// The base directory should be read-only (except for the creator)
 	basePermissions = 0755
-	// The metadata files are globally visible, but can only be deleted by
-	// the user that created them
-	filePermissions = os.FileMode(0644)
+
+	// The metadata files shouldn't be readable or writable by other users.
+	// Having them be world-readable wouldn't necessarily be a huge issue,
+	// but given that some of these files contain (strong) password hashes,
+	// we error on the side of caution -- similar to /etc/shadow.
+	// Note: existing files on-disk might have mode 0644, as that was the
+	// mode used by fscrypt v0.3.2 and earlier.
+	filePermissions = os.FileMode(0600)
 
 	// Maximum size of a metadata file.  This value is arbitrary, and it can
 	// be changed.  We just set a reasonable limit that shouldn't be reached
diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go
index f74078d6..0e15256e 100644
--- a/filesystem/filesystem_test.go
+++ b/filesystem/filesystem_test.go
@@ -413,6 +413,41 @@ func TestSpoofedLoginProtector(t *testing.T) {
 	}
 }
 
+// Tests that the fscrypt metadata files are given mode 0600.
+func TestMetadataFileMode(t *testing.T) {
+	mnt, err := getSetupMount(t)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer mnt.RemoveAllMetadata()
+
+	// Policy
+	policy := getFakePolicy()
+	if err = mnt.AddPolicy(policy, nil); err != nil {
+		t.Fatal(err)
+	}
+	fi, err := os.Stat(filepath.Join(mnt.Path, ".fscrypt/policies/", policy.KeyDescriptor))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if fi.Mode()&0777 != 0600 {
+		t.Error("Policy file has wrong mode")
+	}
+
+	// Protector
+	protector := getFakeProtector()
+	if err = mnt.AddProtector(protector, nil); err != nil {
+		t.Fatal(err)
+	}
+	fi, err = os.Stat(filepath.Join(mnt.Path, ".fscrypt/protectors", protector.ProtectorDescriptor))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if fi.Mode()&0777 != 0600 {
+		t.Error("Protector file has wrong mode")
+	}
+}
+
 // Gets a setup mount and a fake second mount
 func getTwoSetupMounts(t *testing.T) (realMnt, fakeMnt *Mount, err error) {
 	if realMnt, err = getSetupMount(t); err != nil {

From 9871a39409222a80b4c4c22cbaab17bae84f1712 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 23 Feb 2022 12:35:04 -0800
Subject: [PATCH 12/13] pam_fscrypt: log errors getting protector in
 policiesUsingProtector()

If the error is anything other than ErrNotSetup, it might be helpful to
know what is going on.
---
 pam_fscrypt/run_fscrypt.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/pam_fscrypt/run_fscrypt.go b/pam_fscrypt/run_fscrypt.go
index 8c640ced..a563ab5c 100644
--- a/pam_fscrypt/run_fscrypt.go
+++ b/pam_fscrypt/run_fscrypt.go
@@ -173,6 +173,9 @@ func policiesUsingProtector(protector *actions.Protector) []*actions.Policy {
 		// Skip mountpoints that do not use the protector.
 		if _, _, err := mount.GetProtector(protector.Descriptor(),
 			protector.Context.TrustedUser); err != nil {
+			if _, ok := err.(*filesystem.ErrNotSetup); !ok {
+				log.Print(err)
+			}
 			continue
 		}
 		policyDescriptors, err := mount.ListPolicies(protector.Context.TrustedUser)

From 97700817e737eabf45033cdb4a42fa5c6e74f877 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 23 Feb 2022 12:35:04 -0800
Subject: [PATCH 13/13] pam_fscrypt: ignore system users

pam_fscrypt should never need to do anything for system users, so detect
them early so that we can avoid wasting any resources looking for their
login protector.
---
 pam_fscrypt/run_fscrypt.go | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/pam_fscrypt/run_fscrypt.go b/pam_fscrypt/run_fscrypt.go
index a563ab5c..6b40854c 100644
--- a/pam_fscrypt/run_fscrypt.go
+++ b/pam_fscrypt/run_fscrypt.go
@@ -35,6 +35,7 @@ import (
 	"log"
 	"log/syslog"
 	"os"
+	"os/user"
 	"path/filepath"
 	"runtime/debug"
 	"unsafe"
@@ -57,6 +58,10 @@ const (
 	countDirectoryPermissions = 0700
 	countFilePermissions      = 0600
 	countFileFormat           = "%d\n"
+	// uidMin is the first UID that can be used for a regular user (as
+	// opposed to a system user or root).  This value is fairly standard
+	// across Linux distros, but it can be adjusted if needed.
+	uidMin = 1000
 )
 
 // PamFunc is used to define the various actions in the PAM module.
@@ -67,6 +72,14 @@ type PamFunc struct {
 	impl func(handle *pam.Handle, args map[string]bool) error
 }
 
+// isSystemUser checks if a user is a system user.  pam_fscrypt should never
+// need to do anything for system users since they should never have login
+// protectors.  Therefore, we detect them early to avoid wasting resources.
+func isSystemUser(user *user.User) bool {
+	uid := util.AtoiOrPanic(user.Uid)
+	return uid < uidMin && uid != 0
+}
+
 // Run is used to convert between the Go functions and exported C funcs.
 func (f *PamFunc) Run(pamh unsafe.Pointer, argc C.int, argv **C.char) (ret C.int) {
 	args := parseArgs(argc, argv)
@@ -85,7 +98,13 @@ func (f *PamFunc) Run(pamh unsafe.Pointer, argc C.int, argv **C.char) (ret C.int
 	log.Printf("%s(%v) starting", f.name, args)
 	handle, err := pam.NewHandle(pamh)
 	if err == nil {
-		err = f.impl(handle, args)
+		if isSystemUser(handle.PamUser) {
+			log.Printf("invoked for system user %q (%s), doing nothing",
+				handle.PamUser.Username, handle.PamUser.Uid)
+			err = nil
+		} else {
+			err = f.impl(handle, args)
+		}
 	}
 	if err != nil {
 		fmt.Fprintf(errorWriter, "%s(%v) failed: %s", f.name, args, err)