On 2023-03-23 18:05:38, Günther Noack wrote: > +ecryptfs mailing list FYI > > Just some additional data points on the Landlock/eCryptfs issues. > > On Tue, Mar 21, 2023 at 07:16:28PM +0100, Mickaël Salaün wrote: > > On 21/03/2023 18:24, Christian Brauner wrote: > > > On Tue, Mar 21, 2023 at 05:36:19PM +0100, Mickaël Salaün wrote: > > > > There is an inconsistency between ecryptfs_dir_open() and ecryptfs_open(). > > > > ecryptfs_dir_open() actually checks access right to the lower directory, > > > > which is why landlocked processes may not access the upper directory when > > > > reading its content. ecryptfs_open() uses a cache for upper files (which > > > > could be a problem on its own). The execution flow is: > > > > > > > > ecryptfs_open() -> ecryptfs_get_lower_file() -> ecryptfs_init_lower_file() > > > > -> ecryptfs_privileged_open() > > > > > > > > In ecryptfs_privileged_open(), the dentry_open() call failed if access to > > > > the lower file is not allowed by Landlock (or other access-control systems). > > > > Then wait_for_completion(&req.done) waits for a kernel's thread executing > > > > ecryptfs_threadfn(), which uses the kernel's credential to access the lower > > > > file. > > > > > > > > I think there are two main solutions to fix this consistency issue: > > > > - store the mounter credentials and uses them instead of the kernel's > > > > credentials for lower file and directory access checks (ecryptfs_dir_open > > > > and ecryptfs_threadfn changes); > > > > - use the kernel's credentials for all lower file/dir access check, > > > > especially in ecryptfs_dir_open(). > > > > > > > > I think using the mounter credentials makes more sense, is much safer, and > > > > fits with overlayfs. It may not work in cases where the mounter doesn't have > > > > access to the lower file hierarchy though. > > > > > > > > File creation calls vfs_*() helpers (lower directory) and there is not path > > > > nor file security hook calls for those, so it works unconditionally. > > > > > > > > From Landlock end users point of view, it makes more sense to grants access > > > > to a file hierarchy (where access is already allowed) and be allowed to > > > > access this file hierarchy, whatever it belongs to a specific filesystem > > > > (and whatever the potential lower file hierarchy, which may be unknown to > > > > users). This is how it works for overlayfs and I'd like to have the same > > > > behavior for ecryptfs. > > > > > > So given that ecryptfs is marked as "Odd Fixes" who is realistically > > > going to do the work of switching it to a mounter's credentials model, > > > making sure this doesn't regress anything, and dealing with any > > > potential bugs caused by this. It might be potentially better to just > > > refuse to combine Landlock with ecryptfs if that's possible. > > There is now a patch to mark it orphaned (independent of this thread): > https://lore.kernel.org/all/20230320182103.46350-1-frank.li@xxxxxxxx/ I have little time to devote to eCryptfs these days. I'm not sure it needs to be fully orphaned but I think deprecation and marking for removal is the responsible thing to do. > > If Tyler is OK with the proposed solutions, I'll get a closer look at it in > > a few months. If anyone is interested to work on that, I'd be happy to > > review and test (the Landlock part). > > I wonder whether this problem of calling security hooks for the > underlying directory might have been affecting AppArmor and SELinux as > well? There seem to be reports on the web, but it's possible that I > am misinterpreting some of them: Yes, this eCryptfs design problem is common for other LSMs, as well. Tyler > > https://wiki.ubuntu.com/SecurityTeam/Roadmap > mentions this "unscheduled wishlist item": > "eCryptfs + SELinux/AppArmor integration, to protect encrypted data from root" > > https://askubuntu.com/a/1195430 > reports that AppArmor does not work on an eCryptfs mount for their use case > "i tried adding the [eCryptfs] source folder as an alias in apparmor and it now works." > > —Günther > > --
