From: "bfields@fieldses.org" <bfields@fieldses.org>
To: Trond Myklebust <trondmy@hammerspace.com>
Cc: "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: Thu, 14 Jan 2021 13:07:58 -0500 [thread overview]
Message-ID: <20210114180758.GB3914@fieldses.org> (raw)
In-Reply-To: <74269493859fa65a7f594dadd5d86c74bd910e66.camel@hammerspace.com>
On Wed, Jan 13, 2021 at 02:25:25PM +0000, Trond Myklebust wrote:
> BTW: you just 'disclosed the attack method to the public' since
> linux-nfs@vger.kernel.org is a public archived mailing list. However so
> far, you've said absolutely nothing that hasn't already been known and
> discussed for over 20 years.
I dug around a bit and couldn't find the idea of using filehandle
guessing plus mountd's following of symlinks to get access to other
filesystems. That's kind of interesting.
It's not a huge surprise either, and doesn't change our basic
recommendation (normally you should only export the roots of
filesystems). Which is why I asked the reporter to move the discussion
to the public list.
I think we could do better here.
--b.
>
> >
> > 在 2021年1月13日星期三,Trond Myklebust <trondmy@hammerspace.com> 写道:
> > > On Wed, 2021-01-13 at 01:13 +0800, 吴异 wrote:
> > > > Hello,
> > > >
> > > > Well,maybe the best method is to prohibit exporting
> > > > subdirectiries,and I don't know how difficult it will be.
> > >
> > >
> > > So, there is a discussion of all this in the 'exports' manpage both
> > in
> > > the description of the 'no_subtree_check' option, and in the
> > section on
> > > 'Subdirectory Exports'.
> > > In particular, the latter section does describe the issue that you
> > are
> > > raising here:
> > >
> > > Subdirectory Exports
> > > Normally you should only export only the root of a
> > filesystem. The NFS
> > > server will also allow you to export a subdirectory of a
> > filesystem,
> > > however, this has drawbacks:
> > >
> > > First, it may be possible for a malicious user to access
> > files on the
> > > filesystem outside of the exported subdirectory, by
> > guessing filehan‐
> > > dles for those other files. The only way to prevent this
> > is by using
> > > the no_subtree_check option, which can cause other problems.
> > >
> > > Second, export options may not be enforced in the way that
> > you would
> > > expect. For example, the security_label option will not
> > work on subdi‐
> > > rectory exports, and if nested subdirectory exports change
> > the secu‐
> > > rity_label or sec= options, NFSv4 clients will normally
> > see only the
> > > options on the parent export. Also, where security options
> > differ, a
> > > malicious client may use filehandle-guessing attacks to
> > access the
> > > files from one subdirectory using the options from another.
> > >
> > >
> > > Why do we need to go further than this, when there are clearly also
> > a
> > > load of scenarios where the clients are all trusted, and the
> > security
> > > issue is moot?
> > >
> > >
> > > >
> > > > Thanks,
> > > >
> > > > 在 2021年1月13日星期三,Trond Myklebust <trondmy@hammerspace.com> 写道:
> > > > > 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
> > > > >
> > > > >
> > > > >
> > >
> > > --
> > > Trond Myklebust
> > > CTO, Hammerspace Inc
> > > 4984 El Camino Real, Suite 208
> > > Los Altos, CA 94022
> > >
> > > www.hammer.space
> > >
> > >
>
> --
> Trond Myklebust
> CTO, Hammerspace Inc
> 4984 El Camino Real, Suite 208
> Los Altos, CA 94022
>
> www.hammer.space
>
next prev parent reply other threads:[~2021-01-14 18:10 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
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 [this message]
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=20210114180758.GB3914@fieldses.org \
--to=bfields@fieldses.org \
--cc=chuck.lever@oracle.com \
--cc=greg@kroah.com \
--cc=linux-nfs@vger.kernel.org \
--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