aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--actions/context.go2
-rw-r--r--cmd/fscrypt/setup.go2
-rw-r--r--cmd/fscrypt/status.go2
-rw-r--r--filesystem/filesystem.go127
-rw-r--r--filesystem/filesystem_test.go37
-rw-r--r--filesystem/path.go29
6 files changed, 144 insertions, 55 deletions
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)
generated by cgit v1.3 (git 2.53.0) at 2026-05-10 23:37:18 +0000