All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trondmy@hammerspace.com>
To: "wangzhibei1999@gmail.com" <wangzhibei1999@gmail.com>
Cc: "bfields@fieldses.org" <bfields@fieldses.org>,
	"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 17:47:35 +0000	[thread overview]
Message-ID: <96aea58bde3fe4c09cccd9ead2a1c85eb887d276.camel@hammerspace.com> (raw)
In-Reply-To: <CAHxDmpTEBJ1jd_fr3GJ4k7KgzaBpe1LwKgyZn0AJ0D1ESK12fQ@mail.gmail.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


  parent reply	other threads:[~2021-01-12 17:48 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 [this message]
     [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=96aea58bde3fe4c09cccd9ead2a1c85eb887d276.camel@hammerspace.com \
    --to=trondmy@hammerspace.com \
    --cc=bfields@fieldses.org \
    --cc=chuck.lever@oracle.com \
    --cc=greg@kroah.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=security@kernel.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.