2021-01-13 14:27:06

by Trond Myklebust

[permalink] [raw]
Subject: Re: nfsd vurlerability submit

On Wed, 2021-01-13 at 17:00 +0800, 吴异 wrote:
> Linux reference manual does describe the risk of exporting
> subdirectories of a file system,but I think there are some omissions.
>
> /fs on /dev/sda1
> export /fs/nfs
>
> I can easily guess the handle of /fs,and then use it to remove the
> export point /nfs by a symlink,which is pointing to files in other
> file systems,e.g. "/" on /dev/sdb,it just like inserting a back door
> into the server.And then when nfsd service restart,I can mount to "/"
> ,this breaks file system isolation.
>
> My question is whether Linux has made users fully aware of the
> security risks posed by subdirectories.In other words,how many people
> will be attacked with the nfsd when attack method is disclosed to the
> public?

The above is precisely what the manpage warns you about and tells you
not to do as an administrator of a server.


BTW: you just 'disclosed the attack method to the public' since
[email protected] 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.

>
> 在 2021年1月13日星期三,Trond Myklebust <[email protected]> 写道:
> > 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 <[email protected]> 写道:
> > > > 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
> > > > [email protected]
> > > >
> > > >
> > > >
> >
> > --
> > Trond Myklebust
> > CTO, Hammerspace Inc
> > 4984 El Camino Real, Suite 208
> > Los Altos, CA 94022
> >
> > http://www.hammer.space
> >
> >

--
Trond Myklebust
CTO, Hammerspace Inc
4984 El Camino Real, Suite 208
Los Altos, CA 94022

http://www.hammer.space


2021-01-14 18:11:44

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfsd vurlerability submit

On Wed, Jan 13, 2021 at 02:25:25PM +0000, Trond Myklebust wrote:
> BTW: you just 'disclosed the attack method to the public' since
> [email protected] 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 <[email protected]> 写道:
> > > 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 <[email protected]> 写道:
> > > > > 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
> > > > > [email protected]
> > > > >
> > > > >
> > > > >
> > >
> > > --
> > > Trond Myklebust
> > > CTO, Hammerspace Inc
> > > 4984 El Camino Real, Suite 208
> > > Los Altos, CA 94022
> > >
> > > http://www.hammer.space
> > >
> > >
>
> --
> Trond Myklebust
> CTO, Hammerspace Inc
> 4984 El Camino Real, Suite 208
> Los Altos, CA 94022
> ​
> http://www.hammer.space
>

2021-01-14 18:31:40

by Linus Torvalds

[permalink] [raw]
Subject: Re: nfsd vurlerability submit

On Thu, Jan 14, 2021 at 10:08 AM [email protected]
<[email protected]> wrote:
>
> 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.

[ Other people removed from cc, this is just a question about nfsd cleanliness ]

I missed if Trond's suggestion to at least fix up ".." to have the
same filehandle as "." for the top export directory was done.

Because honestly, the whole "guessing file handles is easy" argument
doesn't seem to cover the case that the client just does something
wrong _by_mistake_, and this ends up then exposing the server
unnecessarily that way.

It's one thing if you have an actively malicious client that is
controlled by an attacker and that then makes up its own file handles.

It's another thing if you have a (benign) client that can be fooled to
access files on the server that it shouldn't have.

So I think that from a pure cleanliness standpoint, the server
shouldn't give the client a file handle that the client mustn't
actually ever use! It's just a recipe for "oops, I didn't mean to do
something bad, but by mistake..."

Hmm?

Linus

2021-01-14 18:37:45

by Chuck Lever III

[permalink] [raw]
Subject: Re: nfsd vurlerability submit


> On Jan 14, 2021, at 1:29 PM, Linus Torvalds <[email protected]> wrote:
>
> On Thu, Jan 14, 2021 at 10:08 AM [email protected]
> <[email protected]> wrote:
>>
>> 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.
>
> [ Other people removed from cc, this is just a question about nfsd cleanliness ]
>
> I missed if Trond's suggestion to at least fix up ".." to have the
> same filehandle as "." for the top export directory was done.

If I understand your question correctly... there is a commit in
linux-next that simply doesn't return any filehandle for ".."
in the root directory.

https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/?h=for-next&id=51b2ee7d006a736a9126e8111d1f24e4fd0afaa6

I intend to send you a PR after a few more days of soak time,
unless you'd like me to send it now.


--
Chuck Lever



2021-01-14 18:40:18

by Linus Torvalds

[permalink] [raw]
Subject: Re: nfsd vurlerability submit

On Thu, Jan 14, 2021 at 10:35 AM Chuck Lever <[email protected]> wrote:
>
> If I understand your question correctly... there is a commit in
> linux-next that simply doesn't return any filehandle for ".."
> in the root directory.

Great.

> I intend to send you a PR after a few more days of soak time,
> unless you'd like me to send it now.

No, no hurry. I was more just checking that Trond's suggestion didn't
get lost in the discussion about guessing file handles.

Linus