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: 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 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(limited to 'cmd')

diff --git a/cmd/fscrypt/status.go b/cmd/fscrypt/status.go
index 255bb2b..d10dfd8 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++
-- 
cgit v1.3


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: 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(-)

(limited to 'cmd')

diff --git a/cmd/fscrypt/fscrypt_bash_completion b/cmd/fscrypt/fscrypt_bash_completion
index 00ee490..110d2d4 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
 
-- 
cgit v1.3


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: 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

(limited to 'cmd')

diff --git a/README.md b/README.md
index 75b3d62..26fd084 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 7b56d92..6e28857 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 dc17b5b..f6a4868 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 f067fc0..b92c9d9 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 8dd15e3..ac53d6f 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 269f597..b84216a 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 1f51dc0..4cfc050 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 fba816a..bbcc0f2 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 9ff90e1..8b52392 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 943a781..6ea03e3 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 a8a62a3..f7e302d 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 0000000..e788b3e
--- /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 0000000..c569f20
--- /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 0d478b5..eb425d0 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 9adb00a..1f4f9d7 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 023c0fa..30aa3a7 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 bcf5b59..2d76792 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 044b71e..1b41839 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 7b9bebb..1da0d16 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 d10dfd8..54c6f1f 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 c39514a..1450f0f 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 7aa97cb..365c5cb 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
 }
 
-- 
cgit v1.3


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: 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(-)

(limited to 'cmd')

diff --git a/README.md b/README.md
index 26fd084..f1803b4 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 26295ec..1ee0d60 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 7204380..3b8eb30 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 3278e63..1171c83 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 b92c9d9..ecdc46b 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 e788b3e..d038d52 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 eb425d0..058c62c 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 1f4f9d7..2353527 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 54c6f1f..ed5a764 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 1450f0f..6567dd6 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 365c5cb..d4ef826 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 52f83f2..2874bb8 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 a2148ce..6709804 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 8ffb4f6..84245e0 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 ef7ff92..8c640ce 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
-- 
cgit v1.3


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: 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(-)

(limited to 'cmd')

diff --git a/actions/context.go b/actions/context.go
index 1ee0d60..ac3f6d3 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 1da0d16..b9a16e8 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 ed5a764..bc8f1ee 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 6567dd6..6e4f2c6 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 d4ef826..92e113b 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 fa38701..8cfb235 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)
-- 
cgit v1.3


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: 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(-)

(limited to 'cmd')

diff --git a/actions/policy.go b/actions/policy.go
index 3b8eb30..3b20176 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 07943b8..8248862 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 1171c83..b986eb0 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 6c81d49..f20dbcf 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 f533906..8a769cc 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 b84216a..bb91a46 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 225a47d..b6ae2d8 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 ac864dd..186ca7a 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 6e4f2c6..1877b1b 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 92e113b..f74078d 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 {
-- 
cgit v1.3