filesystem: create metadata files with mode 0600

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

Therefore, start creating these files with mode 0600.

Of course, if users do actually want to share these files, they have the
option of simply chmod'ing them to a less restrictive mode.  An option
could also be added to make fscrypt use the old mode 0644; however, the
need for that is currently unclear.

5 files changed, 50 insertions, 6 deletions

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