From d9d2b32f9fa9e39b154b71b2abc9eda43d5aaa3c Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 29 Oct 2019 00:04:39 -0700 Subject: filesystem: add device number utilities Add a utility type and functions for handling device numbers. --- filesystem/path.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'filesystem/path.go') diff --git a/filesystem/path.go b/filesystem/path.go index cfc3dc0..a99b743 100644 --- a/filesystem/path.go +++ b/filesystem/path.go @@ -20,6 +20,7 @@ package filesystem import ( + "fmt" "log" "os" "path/filepath" @@ -99,3 +100,28 @@ func isRegularFile(path string) bool { info, err := loggedStat(path) return err == nil && info.Mode().IsRegular() } + +// DeviceNumber represents a combined major:minor device number. +type DeviceNumber uint64 + +func (num DeviceNumber) String() string { + return fmt.Sprintf("%d:%d", unix.Major(uint64(num)), unix.Minor(uint64(num))) +} + +func newDeviceNumberFromString(str string) (DeviceNumber, error) { + var major, minor uint32 + if count, _ := fmt.Sscanf(str, "%d:%d", &major, &minor); count != 2 { + return 0, errors.Errorf("invalid device number string %q", str) + } + return DeviceNumber(unix.Mkdev(major, minor)), nil +} + +// getDeviceNumber returns the device number of the device node at the given +// path. If there is a symlink at the path, it is dereferenced. +func getDeviceNumber(path string) (DeviceNumber, error) { + var stat unix.Stat_t + if err := unix.Stat(path, &stat); err != nil { + return 0, err + } + return DeviceNumber(stat.Rdev), nil +} -- cgit v1.3 From c7da2443d6ffa51727db09f8ef1df6aea8c7612c Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 29 Oct 2019 00:04:39 -0700 Subject: filesystem: get correct device for kernel-mounted rootfs A root filesystem mounted via the kernel command line always has a source of "/dev/root", which isn't a real device node. This makes fscrypt think this filesystem doesn't have a source device, which breaks creating login passphrase-protected directories on other filesystems: fscrypt encrypt: filesystem /: no device for mount "/": system error: cannot create filesystem link This also makes 'fscrypt status' show a blank source device: MOUNTPOINT DEVICE FILESYSTEM ENCRYPTION FSCRYPT / ext4 supported Yes To fix this case, update loadMountInfo() to map the device number to the device name via sysfs rather than use the mount source field. --- filesystem/filesystem.go | 4 ++++ filesystem/mountpoint.go | 26 +++++++++++++++++++------- filesystem/path.go | 6 ------ 3 files changed, 23 insertions(+), 13 deletions(-) (limited to 'filesystem/path.go') diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go index 2b3383c..c37962a 100644 --- a/filesystem/filesystem.go +++ b/filesystem/filesystem.go @@ -67,6 +67,9 @@ var ( // Path - Absolute path where the directory is mounted // FilesystemType - Type of the mounted filesystem, e.g. "ext4" // Device - Device for filesystem (empty string if we cannot find one) +// DeviceNumber - Device number of the filesystem. This is set even if +// Device isn't, since all filesystems have a device +// number assigned by the kernel, even pseudo-filesystems. // // In order to use a Mount to store fscrypt metadata, some directories must be // setup first. Specifically, the directories created look like: @@ -92,6 +95,7 @@ type Mount struct { Path string FilesystemType string Device string + DeviceNumber DeviceNumber } // PathSorter allows mounts to be sorted by Path. diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go index 861f5b1..da6a69a 100644 --- a/filesystem/mountpoint.go +++ b/filesystem/mountpoint.go @@ -69,6 +69,18 @@ func unescapeString(str string) string { return sb.String() } +// We get the device name via the device number rather than use the mount source +// field directly. This is necessary to handle a rootfs that was mounted via +// the kernel command line, since mountinfo always shows /dev/root for that. +// This assumes that the device nodes are in the standard location. +func getDeviceName(num DeviceNumber) string { + linkPath := fmt.Sprintf("/sys/dev/block/%v", num) + if target, err := os.Readlink(linkPath); err == nil { + return fmt.Sprintf("/dev/%s", filepath.Base(target)) + } + return "" +} + // Parse one line of /proc/self/mountinfo. // // The line contains the following space-separated fields: @@ -105,9 +117,14 @@ func parseMountInfoLine(line string) *Mount { } var mnt *Mount = &Mount{} + var err error + mnt.DeviceNumber, err = newDeviceNumberFromString(fields[2]) + if err != nil { + return nil + } mnt.Path = unescapeString(fields[4]) mnt.FilesystemType = unescapeString(fields[n+1]) - mnt.Device = unescapeString(fields[n+2]) + mnt.Device = getDeviceName(mnt.DeviceNumber) return mnt } @@ -145,13 +162,8 @@ func loadMountInfo() error { // filesystems are listed in mount order. mountsByPath[mnt.Path] = mnt - var err error - mnt.Device, err = canonicalizePath(mnt.Device) - // Only use real valid devices (unlike cgroups, tmpfs, ...) - if err == nil && isDevice(mnt.Device) { + if mnt.Device != "" { mountsByDevice[mnt.Device] = append(mountsByDevice[mnt.Device], mnt) - } else { - mnt.Device = "" } } mountsInitialized = true diff --git a/filesystem/path.go b/filesystem/path.go index a99b743..e421783 100644 --- a/filesystem/path.go +++ b/filesystem/path.go @@ -73,12 +73,6 @@ func isDir(path string) bool { return err == nil && info.IsDir() } -// isDevice returns true if the path exists and is that of a device. -func isDevice(path string) bool { - info, err := loggedStat(path) - return err == nil && info.Mode()&os.ModeDevice != 0 -} - // 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. -- cgit v1.3 From dbafdbaa9b0767f71affaf15fb8c626f64e27122 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 29 Oct 2019 00:33:54 -0700 Subject: filesystem: handle bind mounts properly Currently, fscrypt treats bind mounts as separate filesystems. This is broken because fscrypt will look for a directory's encryption policy in different places depending on which mount it's accessed through. This forces users to create an fscrypt metadata directory at every bind mount, and to copy fscrypt metadata around between mounts. Fix this by storing fscrypt metadata only at the root of the filesystem. To accomplish this: - Make mountsByDevice store only a single Mount per filesystem, rather than multiple. For this Mount, choose a mount of the full filesystem if available, preferably a read-write mount. If the filesystem has only bind mounts, store a nil entry in mountsByDevice so we can show a proper error message later. - Change FindMount() and GetMount() to look up the Mount by device number rather than by path, so that they don't return different Mounts depending on which path is used. - Change AllFilesystems() to not return bind mounts. - Due to the above changes, the mountsByPath map is no longer needed outside of loadMountInfo(). So make it a local variable there. Resolves https://github.com/google/fscrypt/issues/59 --- filesystem/filesystem.go | 22 +++---- filesystem/mountpoint.go | 147 +++++++++++++++++++++++++++-------------------- filesystem/path.go | 10 ++++ 3 files changed, 107 insertions(+), 72 deletions(-) (limited to 'filesystem/path.go') diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go index c37962a..9bae72b 100644 --- a/filesystem/filesystem.go +++ b/filesystem/filesystem.go @@ -70,6 +70,9 @@ var ( // DeviceNumber - Device number of the filesystem. This is set even if // Device isn't, since all filesystems have a device // number assigned by the kernel, even pseudo-filesystems. +// BindMnt - True if this mount is not for the full filesystem but +// rather is only for a subtree. +// ReadOnly - True if this is a read-only mount // // In order to use a Mount to store fscrypt metadata, some directories must be // setup first. Specifically, the directories created look like: @@ -96,6 +99,8 @@ type Mount struct { FilesystemType string Device string DeviceNumber DeviceNumber + BindMnt bool + ReadOnly bool } // PathSorter allows mounts to be sorted by Path. @@ -437,21 +442,16 @@ func (m *Mount) GetProtector(descriptor string) (*Mount, *metadata.ProtectorData return nil, nil, m.err(err) } - // As the link could refer to multiple filesystems, we check each one - // for valid metadata. - mnts, err := getMountsFromLink(string(link)) + linkedMnt, err := getMountFromLink(string(link)) if err != nil { return nil, nil, m.err(err) } - - for _, mnt := range mnts { - if data, err := mnt.GetRegularProtector(descriptor); err != nil { - log.Print(err) - } else { - return mnt, data, nil - } + data, err := linkedMnt.GetRegularProtector(descriptor) + if err != nil { + log.Print(err) + return nil, nil, m.err(errors.Wrapf(ErrLinkExpired, "protector %s", descriptor)) } - return nil, nil, m.err(errors.Wrapf(ErrLinkExpired, "protector %s", descriptor)) + return linkedMnt, data, nil } // RemoveProtector deletes the protector metadata (or a link to another diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go index d40f864..50389c0 100644 --- a/filesystem/mountpoint.go +++ b/filesystem/mountpoint.go @@ -37,9 +37,8 @@ import ( ) var ( - // These maps hold data about the state of the system's mountpoints. - mountsByPath map[string]*Mount - mountsByDevice map[DeviceNumber][]*Mount + // This map holds data about the state of the system's filesystems. + mountsByDevice map[DeviceNumber]*Mount // Used to make the mount functions thread safe mountMutex sync.Mutex // True if the maps have been successfully initialized. @@ -122,7 +121,13 @@ func parseMountInfoLine(line string) *Mount { if err != nil { return nil } + mnt.BindMnt = unescapeString(fields[3]) != "/" mnt.Path = unescapeString(fields[4]) + for _, opt := range strings.Split(fields[5], ",") { + if opt == "ro" { + mnt.ReadOnly = true + } + } mnt.FilesystemType = unescapeString(fields[n+1]) mnt.Device = getDeviceName(mnt.DeviceNumber) return mnt @@ -134,8 +139,8 @@ func loadMountInfo() error { if mountsInitialized { return nil } - mountsByPath = make(map[string]*Mount) - mountsByDevice = make(map[DeviceNumber][]*Mount) + mountsByPath := make(map[string]*Mount) + mountsByDevice = make(map[DeviceNumber]*Mount) file, err := os.Open("/proc/self/mountinfo") if err != nil { @@ -159,17 +164,38 @@ func loadMountInfo() error { // Note this overrides the info if we have seen the mountpoint // earlier in the file. This is correct behavior because the - // filesystems are listed in mount order. + // mountpoints are listed in mount order. mountsByPath[mnt.Path] = mnt - - mountsByDevice[mnt.DeviceNumber] = append(mountsByDevice[mnt.DeviceNumber], mnt) + } + // fscrypt only really cares about the root directory of each + // filesystem, because that's where the fscrypt metadata is stored. So + // keep just one Mount per filesystem, ignoring bind mounts. Store that + // Mount in mountsByDevice so that it can be found later from the device + // number. Also, prefer a read-write mount to a read-only one. + // + // If the filesystem has *only* bind mounts, store an explicit nil entry + // so that we can show a useful error message later. + for _, mnt := range mountsByPath { + existingMnt, ok := mountsByDevice[mnt.DeviceNumber] + if mnt.BindMnt { + if !ok { + mountsByDevice[mnt.DeviceNumber] = nil + } + } else if existingMnt == nil || (existingMnt.ReadOnly && !mnt.ReadOnly) { + mountsByDevice[mnt.DeviceNumber] = mnt + } } mountsInitialized = true return nil } -// AllFilesystems lists all the Mounts on the current system ordered by path. -// Use CheckSetup() to see if they are used with fscrypt. +func filesystemRootDirNotVisibleError(deviceNumber DeviceNumber) error { + return errors.Errorf("root of filesystem on device %q (%v) is not visible in the current mount namespace", + getDeviceName(deviceNumber), deviceNumber) +} + +// AllFilesystems lists all non-bind Mounts on the current system ordered by +// path. Use CheckSetup() to see if they are used with fscrypt. func AllFilesystems() ([]*Mount, error) { mountMutex.Lock() defer mountMutex.Unlock() @@ -177,9 +203,11 @@ func AllFilesystems() ([]*Mount, error) { return nil, err } - mounts := make([]*Mount, 0, len(mountsByPath)) - for _, mount := range mountsByPath { - mounts = append(mounts, mount) + mounts := make([]*Mount, 0, len(mountsByDevice)) + for _, mount := range mountsByDevice { + if mount != nil { + mounts = append(mounts, mount) + } } sort.Sort(PathSorter(mounts)) @@ -195,70 +223,64 @@ func UpdateMountInfo() error { return loadMountInfo() } -// FindMount returns the corresponding Mount object for some path in a -// filesystem. Note that in the case of a bind mounts there may be two Mount -// objects for the same underlying filesystem. An error is returned if the path -// is invalid or we cannot load the required mount data. If a filesystem has -// been updated since the last call to one of the mount functions, run -// UpdateMountInfo to see changes. +// FindMount returns the main Mount object for the filesystem which contains the +// file at the specified path. An error is returned if the path is invalid or if +// we cannot load the required mount data. If a mount has been updated since the +// last call to one of the mount functions, run UpdateMountInfo to see changes. func FindMount(path string) (*Mount, error) { - path, err := canonicalizePath(path) - if err != nil { - return nil, err - } - mountMutex.Lock() defer mountMutex.Unlock() - if err = loadMountInfo(); err != nil { + if err := loadMountInfo(); err != nil { return nil, err } - - // Traverse up the directory tree until we find a mountpoint - for { - if mnt, ok := mountsByPath[path]; ok { - return mnt, nil - } - - // Move to the parent directory unless we have reached the root. - parent := filepath.Dir(path) - if parent == path { - return nil, errors.Wrap(ErrNotAMountpoint, path) - } - path = parent + deviceNumber, err := getNumberOfContainingDevice(path) + if err != nil { + return nil, err } + mnt, ok := mountsByDevice[deviceNumber] + if !ok { + return nil, errors.Errorf("couldn't find mountpoint containing %q", path) + } + if mnt == nil { + return nil, filesystemRootDirNotVisibleError(deviceNumber) + } + return mnt, nil } -// GetMount returns the Mount object with a matching mountpoint. An error is -// returned if the path is invalid or we cannot load the required mount data. If -// a filesystem has been updated since the last call to one of the mount -// functions, run UpdateMountInfo to see changes. +// GetMount is like FindMount, except GetMount also returns an error if the path +// isn't the root directory of a filesystem. For example, if a filesystem is +// mounted at "/mnt" and the file "/mnt/a" exists, FindMount("/mnt/a") will +// succeed whereas GetMount("/mnt/a") will fail. func GetMount(mountpoint string) (*Mount, error) { - mountpoint, err := canonicalizePath(mountpoint) + mnt, err := FindMount(mountpoint) + if err != nil { + return nil, errors.Wrap(ErrNotAMountpoint, mountpoint) + } + // Check whether 'mountpoint' is the root directory of the filesystem, + // i.e. is the same directory as 'mnt.Path'. Use os.SameFile() (i.e., + // compare inode numbers) rather than compare canonical paths, since the + // filesystem might be fully mounted in multiple places. + fi1, err := os.Stat(mountpoint) if err != nil { return nil, err } - - mountMutex.Lock() - defer mountMutex.Unlock() - if err = loadMountInfo(); err != nil { + fi2, err := os.Stat(mnt.Path) + if err != nil { return nil, err } - - if mnt, ok := mountsByPath[mountpoint]; ok { - return mnt, nil + if !os.SameFile(fi1, fi2) { + return nil, errors.Wrap(ErrNotAMountpoint, mountpoint) } - - return nil, errors.Wrap(ErrNotAMountpoint, mountpoint) + return mnt, nil } -// getMountsFromLink returns the Mount objects which match the provided link. +// getMountsFromLink returns the Mount object which matches the provided link. // This link is formatted as a tag (e.g. =) similar to how they -// appear in "/etc/fstab". Currently, only "UUID" tokens are supported. Note -// that this can match multiple Mounts (due to the existence of bind mounts). An -// error is returned if the link is invalid or we cannot load the required mount -// data. If a filesystem has been updated since the last call to one of the -// mount functions, run UpdateMountInfo to see the change. -func getMountsFromLink(link string) ([]*Mount, error) { +// appear in "/etc/fstab". Currently, only "UUID" tokens are supported. An error +// is returned if the link is invalid or we cannot load the required mount data. +// If a mount has been updated since the last call to one of the mount +// functions, run UpdateMountInfo to see the change. +func getMountFromLink(link string) (*Mount, error) { // Parse the link linkComponents := strings.Split(link, "=") if len(linkComponents) != 2 { @@ -286,13 +308,16 @@ func getMountsFromLink(link string) ([]*Mount, error) { if err := loadMountInfo(); err != nil { return nil, err } - mnts, ok := mountsByDevice[deviceNumber] + mnt, ok := mountsByDevice[deviceNumber] if !ok { devicePath, _ := canonicalizePath(searchPath) return nil, errors.Wrapf(ErrFollowLink, "no mounts for device %q (%v)", devicePath, deviceNumber) } - return mnts, nil + if mnt == nil { + return nil, filesystemRootDirNotVisibleError(deviceNumber) + } + return mnt, nil } // makeLink returns a link of the form = where value is the tag diff --git a/filesystem/path.go b/filesystem/path.go index e421783..376daf0 100644 --- a/filesystem/path.go +++ b/filesystem/path.go @@ -119,3 +119,13 @@ func getDeviceNumber(path string) (DeviceNumber, error) { } return DeviceNumber(stat.Rdev), nil } + +// getNumberOfContainingDevice returns the device number of the filesystem which +// contains the given file. If the file is a symlink, it is not dereferenced. +func getNumberOfContainingDevice(path string) (DeviceNumber, error) { + var stat unix.Stat_t + if err := unix.Lstat(path, &stat); err != nil { + return 0, err + } + return DeviceNumber(stat.Dev), nil +} -- cgit v1.3