Re: [fuse-devel] read processed when permission should be denied

[Goswin v. B. <gos...@we...>]

Miklos Szeredi <mi...@sz...> writes:

> On Wed, 23 Jun 2010, Goswin von Brederlow wrote:
>> Matej Križan <kr...@es...> writes:
>> 
>> > Hello,
>> >
>> > I think I found a hole according to your security requirements for 
>> > non-privileged mount in kernel.txt:
>> >
>> > Permissions are not checked at least for the read system call, which is 
>> > then processed in userspace. Thus the mount owner can induce undesired 
>> 
>> desired behaviour.
>> 
>> > behaviour in other users' processes, if they somehow got a valid 
>> > descriptor to a file on fuse-mounted filesystem and read from it. Note 
>> > that fstat (or open) in such processes won't be processed in userspace 
>> > and will correctly return EACCES.
>> >
>> > You can use the test case program from 
>> > https://bugzilla.gnome.org/show_bug.cgi?id=621978 which works with your 
>> > example fusexmp, too. The idea is to transport a descriptor through 
>> > execve to a setuid program.
>> >
>> > Matej Krizan
>> 
>> I think you have the problem reversed. I don't see why the fstat should
>> fail. Opening a file with higher cedentials, dropping the credentials
>> and using the already open file is an essential security feature. You
>> can also FDs over unix sockets and so on. It is essential that
>> read/write work that way.
>> 
>> I think the bug is in gvfs (and fusexmp). The getattr() callback has
>> fi->fh set to the value set by the open method (see
>> /usr/include/fuse/fuse_lowlevel.h for specifics) and fstat() should not
>> fail on opened files no matter what the permissions.
>> 
>> In fusexmp you have only:
>> 
>> static int xmp_getattr(const char *path, struct stat *stbuf)
>> 
>> while fusexmp_fh has two functions:
>> 
>> static int xmp_getattr(const char *path, struct stat *stbuf)
>> static int xmp_fgetattr(const char *path, struct stat *stbuf,
>>                         struct fuse_file_info *fi)
>> 
>> Can you reproduce your problem with fusexmp_fh?
>
> The only problem is, the kernel will not supply the file descriptor in
> case of an fstat() call.
>
> So yeah, it would be more consistent if fstat() would not fail in that
> case but it does.
>
> Thanks,
> Miklos

Oh, I see that I misremembered the docs for the fgetattr() callback. It
actually says specifically:

         * This method is called instead of the getattr() method if the
         * file information is available.
         *
         * Currently this is only called after the create() method if that
         * is implemented (see above).  Later it may be called for
         * invocations of fstat() too.

Is there any plan of fixing that in the kernel?

MfG
        Goswin