From 85a747493ff368a72f511619ecd391016ecb933c Mon Sep 17 00:00:00 2001 From: Eric Biggers 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. --- filesystem/path.go | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) (limited to 'filesystem/path.go') 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.2.3