linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "bfields@fieldses.org" <bfields@fieldses.org>
To: Patrick Goetz <pgoetz@math.utexas.edu>
Cc: Trond Myklebust <trondmy@hammerspace.com>,
	"wangzhibei1999@gmail.com" <wangzhibei1999@gmail.com>,
	"security@kernel.org" <security@kernel.org>,
	"w@1wt.eu" <w@1wt.eu>, "greg@kroah.com" <greg@kroah.com>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	"chuck.lever@oracle.com" <chuck.lever@oracle.com>
Subject: Re: nfsd vurlerability submit
Date: Tue, 12 Jan 2021 13:03:26 -0500	[thread overview]
Message-ID: <20210112180326.GI9248@fieldses.org> (raw)
In-Reply-To: <42fcbc42-f1b3-5d99-c507-e1b579f5a37a@math.utexas.edu>

On Tue, Jan 12, 2021 at 11:20:28AM -0600, Patrick Goetz wrote:
> 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

You can do that if you'd like.  I doesn't make much difference here.

You can think of a filehandle as just a (device number, inode number)
pair.  (It's actually more complicated, but ignore that for now.)

So if the server's given a filehandle, it can easily determine the
filehandle is for an object on /dev/sda2.  It *cannot* easily determine
whether that object is somewhere underneath /some/directory.

So in your example, if /data2/xray is on the same filesystem as /data2,
then the server will happily allow operations on filehandles anywhere in
/data2.

Every export point should be the root of a filesystem.

--b.

> 
> 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.
> >

  reply	other threads:[~2021-01-12 18:04 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAHxDmpTKJfnhGY9CVupyVYhNCTDVKBB6KRwh-E6u_XEPJq4WJQ@mail.gmail.com>
     [not found] ` <20210105165633.GC14893@fieldses.org>
     [not found]   ` <X/hEB8awvGyMKi6x@kroah.com>
     [not found]     ` <20210108152017.GA4183@fieldses.org>
     [not found]       ` <CAHxDmpSp1LHzKD5uqbfi+jcnb+nFaAZbc5++E0oOvLsYvyYDpw@mail.gmail.com>
     [not found]         ` <20210108164433.GB8699@fieldses.org>
     [not found]           ` <CAHxDmpSjwrcr_fqLJa5=Zo=xmbt2Eo9dcy6TQuoU8+F3yVVNhw@mail.gmail.com>
     [not found]             ` <20210110201740.GA8789@fieldses.org>
     [not found]               ` <20210110202815.GB8789@fieldses.org>
     [not found]                 ` <CAHxDmpR8S7NR8OU2nWJmWBdFU9a7wDuDnxviQ2E9RDOeW9fExg@mail.gmail.com>
2021-01-11 19:25                   ` nfsd vurlerability submit J. Bruce Fields
2021-01-11 21:01                     ` [PATCH] nfsd4: readdirplus shouldn't return parent of export J. Bruce Fields
2021-01-12 13:31                       ` Chuck Lever
2021-01-12 13:50                         ` Bruce Fields
     [not found]       ` <20210108152607.GA950@1wt.eu>
     [not found]         ` <20210108153237.GB4183@fieldses.org>
     [not found]           ` <20210108154230.GB950@1wt.eu>
     [not found]             ` <20210111193655.GC2600@fieldses.org>
     [not found]               ` <CAHxDmpR1zG25ADfK2jat4VKGbAOCg6YM_0WA+a_jQE82hbnMjA@mail.gmail.com>
     [not found]                 ` <CAHxDmpRfmVukMR_yF4coioiuzrsp72zBraHWZ8gaMydUuLwKFg@mail.gmail.com>
2021-01-12 15:32                   ` nfsd vurlerability submit J. Bruce Fields
2021-01-12 16:53                     ` Trond Myklebust
2021-01-12 17:20                       ` Patrick Goetz
2021-01-12 18:03                         ` bfields [this message]
2021-01-13  8:12                           ` Christoph Hellwig
2021-01-13 14:34                             ` Trond Myklebust
2021-01-13 14:40                               ` hch
2021-01-13 15:16                                 ` Trond Myklebust
2021-01-13 15:30                                   ` hch
2021-01-13 15:45                                     ` Frank Filz
2021-01-21 20:01                           ` Patrick Goetz
2021-01-21 22:04                             ` bfields
2021-01-21 23:19                               ` Patrick Goetz
2021-01-22  1:30                                 ` bfields
2021-01-22 13:20                                   ` Patrick Goetz
2021-01-22 14:48                                     ` Tom Talpey
     [not found]                       ` <CAHxDmpTEBJ1jd_fr3GJ4k7KgzaBpe1LwKgyZn0AJ0D1ESK12fQ@mail.gmail.com>
2021-01-12 17:47                         ` Trond Myklebust
     [not found]                           ` <CAHxDmpTyrG74hOkzmDK834t+JiQduWHVWxCf_7nrDVa++EK2mA@mail.gmail.com>
2021-01-13 14:25                             ` Trond Myklebust
2021-01-14 18:07                               ` bfields
2021-01-14 18:29                                 ` Linus Torvalds
2021-01-14 18:35                                   ` Chuck Lever
2021-01-14 18:37                                     ` Linus Torvalds
2021-01-18 16:29                       ` 吴异
2021-01-18 22:55                         ` bfields
2021-01-19  2:48                           ` 吴异
2021-01-19  3:46                             ` bfields

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210112180326.GI9248@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=chuck.lever@oracle.com \
    --cc=greg@kroah.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=pgoetz@math.utexas.edu \
    --cc=security@kernel.org \
    --cc=trondmy@hammerspace.com \
    --cc=w@1wt.eu \
    --cc=wangzhibei1999@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).