Does Landlock not work with eCryptfs?

[Mickaël Salaün]

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.


On 20/03/2023 18:21, Mickaël Salaün wrote:
> 
> On 20/03/2023 18:15, Günther Noack wrote:
>> Hello!
>>
>> On Sun, Mar 19, 2023 at 10:00:46PM +0100, Mickaël Salaün wrote:
>>> Hi Günther,
>>>
>>> Thanks for the report, I confirm there is indeed a bug. I tested with a
>>> Debian distro:
>>>
>>> ecryptfs-setup-private --nopwcheck --noautomount
>>> ecryptfs-mount-private
>>> # And then with the kernel's sample/landlock/sandboxer:
>>> LL_FS_RO="/usr" LL_FS_RW="${HOME}/Private" sandboxer ls ~/Private
>>> ls: cannot open directory '/home/user/Private': Permission denied
>>>
>>> Actions other than listing a directory (e.g. creating files/directories,
>>> reading/writing to files) are controlled as expected. The issue might be
>>> that directories' inodes are not the same when listing the content of a
>>> directory or when creating new files/directories (which is weird). My
>>> hypothesis is that Landlock would then deny directory reading because the
>>> directory's inode doesn't match any rule. It might be related to the overlay
>>> nature of ecryptfs.
>>>
>>> Tyler, do you have some idea?
>>
>> I had a hunch, and found out that the example can be made to work by
>> granting the LANDLOCK_ACCESS_FS_READ_DIR right on the place where the
>> *encrypted* version of that home directory lives:
>>
>>     err := landlock.V1.RestrictPaths(
>>             landlock.RODirs(dir),
>>             landlock.PathAccess(llsys.AccessFSReadDir, "/home/.ecryptfs/gnoack/.Private"),
>>     )
>>
>> It does seem a bit like eCryptfs it calling security_file_open() under
>> the hood for the encrypted version of that file? Is that correct?
> 
> Yes, that's right, the lower directory is used to list the content of
> the ecryptfs directory:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ecryptfs/file.c#n112
> iterate_dir(lower_file, …)