Re: nfsd vurlerability submit

Patrick Goetz
2021-01-12T17:28:55Z

I was under the impression that the best practice is to create something
along the lines of

/srv/nfs

and then bind mount everything you plan to export into that folder; e.g.

/etc/fstab:
/data2/xray /srv/nfs/xray none defaults,bind 0

Presumably this becomes a non-issue under these circumstances? Not sure
it's a good idea to attempt to accommodate every wacky use case someone
attempts to implement.

On 1/12/21 10:53 AM, Trond Myklebust wrote:
> On Tue, 2021-01-12 at 10:32 -0500, J. Bruce Fields wrote:
>> On Tue, Jan 12, 2021 at 10:48:00PM +0800, 吴异 wrote:
>>> Telling users how to configure the exported file system in the most
>>> secure
>>> way does
>>> mitigate the problem to some extent, but this does not seem to
>>> address the
>>> security risks posed by no_ subtree_ check in the code. In my
>>> opinion,when
>>> the generated filehandle does not contain the inode information of
>>> the
>>> parent directory,the nfsd_acceptable function can also recursively
>>> determine whether the request file exceeds the export path
>>> dentry.Enabling
>>> subtree_check to add parent directory information only brings some
>>> troubles.
>>
>> Filesystems don't necessarily provide us with an efficient way to
>> find
>> parent directories from any given file.  (And note a single file may
>> have multiple parent directories.)
>>
>> (I do wonder if we could do better in the directory case, though.  We
>> already reconnect directories all the way back up to the root.)
>>
>>> I have a bold idea, why not directly remove the file handle
>>> modification in
>>> subtree_check, and then normalize the judgment of whether dentry
>>> exceeds
>>> the export point directory in nfsd_acceptable (line 38 to 54 in
>>> /fs/nfsd/nfsfh.c) .
>>>
>>> As far as I understand it, the reason why subtree_check is not
>>> turned on by
>>> default is that it will cause problems when reading and writing
>>> files,
>>> rather than it wastes more time when nfsd_acceptable.
>>>
>>> In short,I think it's open to question whether the security of the
>>> system
>>> depends on the user's complete correct configuration(the system
>>> does not
>>> prohibit the export of a subdirectory).
>>
>>> Enabling subtree_check to add parent directoryinformation only
>>> brings
>>> some troubles.
>>>
>>> In short,I think it's open to question whether the security of the
>>> system depends on the user's complete correct configuration(the
>>> system
>>> does not prohibit the export of a subdirectory).
>>
>> I'd love to replace the export interface by one that prohibited
>> subdirectory exports (or at least made it more obvious where they're
>> being used.)
>>
>> But given the interface we already have, that would be a disruptive
>> and
>> time-consuming change.
>>
>> Another approach is to add more entropy to filehandles so they're
>> harder
>> to guess; see e.g.:
>>
>>         https://www.fsl.cs.stonybrook.edu/docs/nfscrack-tr/index.html
>>
>> In the end none of these change the fact that a filehandle has an
>> infinite lifetime, so once it's leaked, there's nothing you can do.
>> The
>> authors suggest NFSv4 volatile filehandles as a solution to that
>> problem, but I don't think they've thought through the obstacles to
>> making volatile filehandles work.
>>
>> --b.
>
> The point is that there is no good solution to the 'I want to export a
> subtree of a filesystem' problem, and so it is plainly wrong to try to
> make a default of those solutions, which break the one sane case of
> exporting the whole filesystem.
>
> Just a reminder that we kicked out subtree_check not only because a
> trivial rename of a file breaks the client's ability to perform I/O by
> invalidating the filehandle. In addition, that option causes filehandle
> aliasing (i.e. multiple filehandles pointing to the same file) which is
> a major PITA for clients to try to manage for more or less the same
> reason that it is a major PITA to try to manage these files using
> paths.
>
> The discussion on volatile filehandles in RFC5661 does try to address
> some of the above issues, but ends up concluding that you need to
> introduce POSIX-incompatible restrictions, such as trying to ban
> renames and deletions of open files in order to make it work.
>
> None of these compromises are necessary if you export a whole
> filesystem (or a hierarchy of whole filesystems). That's the sane case.
> That's the one that people should default to using.
>