Re: nfsd vurlerability submit

bfields
2021-01-18T22:57:05Z

On Tue, Jan 19, 2021 at 12:29:28AM +0800, 吴异 wrote:
> I want to consult you on what is the original intention of designing
> subtree_check and whether it is to solve the 'I want to export a
> subtree of a filesystem' problem.
>
> As far as I know, when opening subtree_check, the folder's file
> handle does not contain the inode information of its parent directory
> and
> 'while (tdentry != exp->ex_path.dentry && !IS_ROOT(tdentry))' in
> nfsd_acceptable can work well to Intercept handles beyond the export
> point.
>
> This seems to delete code as follows in nfsfh.c could solve the 'I
> want to export a subtree of a filesystem' problem and ensure safety:
> if (exp->ex_flags & NFSEXP_NOSUBTREECHECK)
> return 1;
>
> Or replace by follow:
> if (exp->ex_path.dentry == exp->vfs_mount->mnt_root)
> return 1;
>
> When I was reading the nfsd code, I was confused about whether the
> designer used the file system as a security boundary or an export
> point.Since exporting a complete file system is the safest, why not
> directly prohibit unsafe practices, but add code like subtree_check to
> try to verify the file handle.

Sorry, I honestly don't understand the question.

If you have a specific proposal, perhaps you could send a patch.

--b.

>
> I may not understand your design ideas.
>
> Yours sincerely,
>
> Trond Myklebust <trondmy@hammerspace.com> 于2021年1月13日周三 上午12:53写道：
> >
> > 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.
> >
> > --
> > Trond Myklebust
> > Linux NFS client maintainer, Hammerspace
> > trond.myklebust@hammerspace.com
> >
> >