Without `default_permissions`, cached permissions are only checked on first access
jpandre opened this issue · 21 comments
When the inodes are cached by fuse (non-zero attr_timeout), and the standard_permissions are used, the following sequence :
chmod 755 trydir
rm -rf trydir
mkdir trydir
echo file > trydir/file
ls -li trydir/file
chmod 000 trydir
chmod 444 trydir/file
ls -li trydir/file
outputs :
293 -rw-rw-r-- 1 linux linux 5 2009-08-25 17:50 trydir/file
293 -r--r--r-- 1 linux linux 5 2009-08-25 17:50 trydir/file
The "chmod 000" should have led to an EACCES error for the subsequent commands.
This was reported in 2009 (http://article.gmane.org/gmane.comp.file-systems.fuse.devel/8099), and according to http://article.gmane.org/gmane.comp.file-systems.fuse.devel/15300 some work is still needed on it.
This means that for being Posix compliant, fuse file systems cannot either use the cache or have the permissions checked by the kernel.
Would this fix http://sourceforge.net/p/fuse/mailman/message/34801725/ too. Because the attr-timeout has nothing to do with the issue reported (the entry timeout has to be > 0). By standard permissions do you mean default permissions turned off ?
Hi,
Satya G S wrote:
Would this fix http://sourceforge.net/p/fuse/mailman/message/34801725/
too. Because the attr-timeout has nothing to do with the issue reported
(the entry timeout has to be > 0). By standard permissions do you mean
default permissions turned off ?
I do not fully understand what you mean (what has
nothing to do with what ?, what fix are you mentioning ?),
so I explain my point of view on the issue :
When the kernel needs to dig into a directory on
behalf of some user (typically in a readdir()), it
makes a lookup on the directory, and the result (either
successful or failed because of permissions) is saved
in some cached structure. Now if the cached structure
is invalid either because the permissions have changed
or the request is done by another user with different
rights, the kernel is expected to update its own data.
Your results show this is not yet the case.
attr-timeout (I mean the last argument of fuse_reply_attr())
does not need to be positive. If it is zero, the kernel
is not allowed to use cached data and it has to redo
the lookups on the full path to check what is allowed
to the requesting user.
On Linux, ntfs-3g usually uses default_permissions (yes,
that is what I meant) but it uses a zero attr-timeout
for the above reason, and repeating the posted test
sequence yields the expected result :
[linux@dimension linux]$ ./try.sh
136 -rw-r--r-- 1 linux linux 5 Jan 31 11:06 trydir/file
chmod: cannot access ‘trydir/file’: Permission denied
ls: cannot access trydir/file: Permission denied
On OpenIndiana (same fuse library), ntfs-3g can use
the fuse cache, and the cache is compatible with using
hard links at the high level interface.
Note : the proper behavior of permissions is checked by
the Posix compliance test :
http://sourceforge.net/p/ntfs-3g/pjd-fstest/ci/master/tree/
Regards
Jean-Pierre
ls -l seems to working fine (doesn't display children attributes) if default permissions is turned on. When default permissions is turned off and entry timeout is > 0 ls -l display the children attributes for the duration of entry timeout which is a bug.
Thanks,
Satya.
Hum, yes you are right. The issue occurs with default_permissions not enabled, and with permissions checked by the user-level file system. I have recompiled ntfs-3g with a timeout of 10 seconds (both attr and entry timeouts), and executed as root the following script :
rm -rf trydir
umask 077
mkdir trydir
umask 022
echo file > trydir/file
echo "ls -li trydir/file" > user.sh
chmod 755 user.sh
ls -ldi trydir trydir/file user.sh
su linux -c ./user.sh
Note that this script never changes the permissions on the directory "trydir". The expected output is :
[root@dimension linux]# ./try2.sh
144 drwx------ 1 root root 144 Jan 31 21:26 trydir
146 -rw-r--r-- 1 root root 5 Jan 31 21:26 trydir/file
126 -rwxr-xr-x 1 root root 19 Jan 31 21:26 user.sh
trydir/file not found
The error on last line is caused by the user ("linux") not being allowed to list "trydir".
With the timeouts enabled, I get :
[root@dimension linux]# ./try2.sh
123 drwx------ 1 root root 144 Jan 31 21:47 trydir
127 -rw-r--r-- 1 root root 5 Jan 31 21:47 trydir/file
126 -rwxr-xr-x 1 root root 19 Jan 31 21:47 user.sh
127 r-- 31 Jan 16 21:47 5 trydir/file
And trydir could be listed...
So this is not a bug for not refreshing the cache (the permissions on trydir are never changed), it is a bug for not checking the permissions on the cached directory when the current user is not the one for which the directory was cached in the first place. In the debug output, I can see that there is no lookup from directory tryfile after the inner script user.sh is opened, so the user-level file system has no opportunity to check whether a lookup by the non-root user would be valid.
Also note the user-level file system has no opportunity to invalidate an entry. There is no action triggering a change.
I have replayed the shell script shown in the original post, and I can confirm the bug is still present, but only when the option default_permissions is NOT used.
for Nikolaus : maybe you can change the title to "The directory entries cached by fuse are not checked against the current user permissions"
Sorry, I wanted to close the comment, ... not the issue.
Jean,
Thank you for confirming the behaviour. This issue hurts every filesystem that has support for ACLs. Setting the entry timeout to zero will result in too many up calls to filesystem.
Isn't this a place to track library issues. This looks like a kernel bug to me.
Thanks,
Satya.
I agree, not using the cache is inefficient because it implies making more calls to the user-level file system. The same goes for not using default_permissions.
So I am not insisting on this bug to be fixed, I would rather call for default_permissions to support the same permission checks as ext2/3/4.
I also call for the cache to support hard links, and this is specific to the high-level library interface, there is no problem with the low-level one.
These are the two reasons why ntfs-3g gave up using the cache on Linux (except when using the low-level interface and not using the Posix ACLs).
So this is not a bug for not refreshing the cache (the permissions on trydir are never changed), it is a bug for not checking the permissions on the cached directory when the current user is not the one for which the directory was cached in the first place
Yes, when writing the kernel code there was probably an implicit assumption that allow_other" would not be used (or maybe it didn't exist at that point). With allow_other, this becomes a security bug.
Isn't this a place to track library issues. This looks like a kernel bug to me.
You are right. I asked Jean-Pierre to file it here so that there's at least a permanent record (give that this was first reported on the kernel ml in 2006).
Nikolaus Rath wrote:
I've added this to the README, see commit 43138fb
43138fb+bug #15: the
+permission to access a cached directory entry is only checked for the
+first user that accesses it. As long as the directory entry is cached,
+accesses by other users are made with the permissions of the first
+user.
I think this is not correct : in my first example and
the situation met by Satya the initial access and the
second one are done by the same user (with permissions
changed), and yes, in my second example there is a user
change with permissions unchanged.
So my feeling is that there is no check at all.
My text change proposal :
+bug #15: the
+permission to access a cached directory entry rely on the check
+done for the current user at the moment it was cached.
+As long as the directory entry is cached, subsequent
+accesses by the same user or by other users are not checked.—
Reply to this email directly or view it on GitHub
#15 (comment).
Nikolaus Rath wrote:
Thanks for re-checking. How do you like the version in commit c41b?
Looks good to me.
Jean-Pierre
gocryptfs developer here. I don't understand why this is declared a kernel bug (but on the other hand I can't click the gmane links because it has been shut down).
Isn't just that the user-space filesystem fails to check the permissions on READDIR? (This is the opportunity to check the permissions, there is no need for an extra LOOKUP)
@rfjakob : see my example try2.sh, there is no readdir at all (no wildcards). Creating "trydir/file" is just a create() in write-only directory "trydir" (which is allowed, though directory is unreadable). Later on, for executing "trydir/file", there is no lookup from "trydir", and the user space has no chance of checking whether "trydir" is readable. When "file" is opened, the user space does not know from which directory it is opened (a file may be referenced in several directories with different permissions).
The fuse kernel module has been said to accept the lookup because it does not know who is the user requesting the file... which can be translated to a bad design of the interface between the fuse kernel module and the kernel proper.
Ok, I think I get it. When there are multiple hard links of one file in different directories, user-space cannot decide which directory to check. It's quite a corner-case, but still.
Note that the kernel knows about the user and this info is also passed to user-space in every request:
Line 695 in d6d3e50
@rfjakob
In "the kernel knows about the user and this info is also passed to user-space in every request" you miss the point. We are in the situation where user-space does not receive any request at all for checking whether directory is readable. This has been said to be a consequence of the fuse kernel module not getting the user description in some particular situation.
AFAIK the only way for the user space to get around this is by setting a null duration for the attributes cached in the kernel. This forces a lookup to user-space.
I'm trying to compile under Windows using сygwin, but eventually do make-j8 command error occurs
> ../include/fuse_kernel.h:759:41: error: expected expression before 'uint32_t'
> #define FUSE_DEV_IOC_CLONE _IOR (229, 0, uint32_t).
Please help deal with this problem, which is already just trying to build library, but this error does not allow it to make
Is this still a problem with the latest releases of libfuse and the Linux kernel?
I have no information to the contrary, so I'd say yes, this has not been fixed yet.
It would be great to at least get the docs/config/defaults into a non-confusing state, see the link directly above this comment.