Re: nfsd vurlerability submit

Re: nfsd vurlerability submit

[J. Bruce Fields]

On Mon, Jan 11, 2021 at 08:55:43PM +0800, 吴异 wrote:
> Yes, the safest way for users is that the first exported file system is on
> a different hard disk with "\", and then does not contain subdirectories,
> so I seem to have nothing to do. However, I did not find these tips in the
> configuration files related and man exports . It is best to recommend the
> best configuration method to users if it is updated in the future.

I agree that the documentation needs improvement.

I also think we should warn by default when a user tries to export
something with no_subtree_check that's not at a mountpoint.

(Really, "at a mountpoint" should be "at the root of a filesystem", but
I don't know how to check for that, at least not from userspace.  I'm
not sure whether the warning belongs in the kernel or elsewhere.)

> Back to the present situation, have you considered removing the following
> piece of code in the nfsd_acceptable function：
>  if (exp->ex_flags & NFSEXP_NOSUBTREECHECK)
> return 1;

That would turn on subtree_check unconditionally.  Currently we default
to no_subtree_check, and for good reason: subtree_check causes
unexpected behavior, like ESTALE on attempts to access open files after
cross-directory rename.

> In this way, all problems seem to be solved. Even if an attacker can get or
> guess some file handles, these handles will not actually work.Fix
> compose_entry_fh and nfsd_lookup_dentry functions is actually not that
> urgent.
> 
> One more question, do I need to copy all the previous emails to
> linux-nfs@vger.kernel.org, this is a bit too much, right?

I don't think that's necessary.  To summarize issue you've raised; if a
client exports only a subdirectory of a filesystem:

	- NFSv2/v3 clients can discover the filehandle of the parent of
	  the export point using readdir_plus.  I agree that we should
	  fix this; I'll follow up with a patch.  (It's already not too
	  hard to guess filehandles, though.)

	- Once a single filehandle outside the export directory is
	  found, it's easy to access the rest of the filesystem.  This
	  issue has been known for decades.  But I don't think it's
	  well-enough understood by users.

	- If only a subdirectory of a filesystem is exported, an
	  attacker might be able to gain access to *other* filesystems
	  as well, by replacing some component along the exported path
	  by a symlink and waiting for the server to restart.  (In the
	  v2/v3 case, export path symlinks are followed on the server
	  side.)

The main defense against all of this is to *only* export the root of a
filesystem.  If you're exporting "/export/home/", then you should create
a separate filesystem and mount that at "/export/home/".

But this isn't as well understood as it should be, and we should do a
better job of preventing users from choosing dangerous configurations.
And then mitigate the danger where we can.

--b.

> 
> Best regards,
> 
> J. Bruce Fields <bfields@fieldses.org> 于2021年1月11日周一 上午4:28写道：
> 
> > On Sun, Jan 10, 2021 at 03:17:40PM -0500, J. Bruce Fields wrote:
> > > On Sat, Jan 09, 2021 at 10:52:33PM +0800, 吴异 wrote:
> > > > I do try it,because of the default no_subtreecheck in nfsd, I can
> > delete
> > > > /nfs as long as I can get the handle of /fs. This takes advantage of
> > the
> > > > vulnerability I submitted before. I have added my poc and traffic
> > > > information in the attachment.
> > > >
> > > > My server filesystem configuration is as follows：
> > > >
> > > > /dev/sda3 on / type ext4 (rw,relatime,errors=remount-ro)
> > > >
> > > > /dev/sda1 on /fs type ext4 (rw,relatime)
> > > >
> > > > The configuration of /etc/export is
> > > >
> > > > /fs/nfs      *(rw,sync,no_root_squash,insecure)
> > >
> > > Again, you're exporting a subdirectory, which we discourage.  The safe
> > > and recommended configuration would be to mount /dev/sda1 on /fs/nfs and
> > > export /fs/nfs.
> > >
> > > > As you can see, the file system where the shared directory is located
> > has
> > > > been separated from the “\” directory.What I did was to delete the
> > mount
> > > > point directory
> > >
> > > It should not be possible to delete /fs/nfs/ while you had something
> > > mounted there.
> >
> > Whoops, I'm sorry, there's only a mountpoint at /fs/, so, yes, you can
> > probably delete /fs/nfs and replace it by a symlink, got it.
> >
> > Sure, that's interesting.  But, again, we don't recommend exporting
> > filesystem subdirectories.
> >
> > Good to keep the public nfs list (linux-nfs@vger.kernel.org) cc'd on
> > these things, by the way.
> >
> > --b.
> >
> > >
> > > --b.
> > >
> > > > and restart the server-side nfsd service. At this time, the
> > > > server-side shared directory became "/", and the configuration
> > information
> > > > did not change.
> > > >
> > > >
> > > > For the convenience I set no_root_squash on the server, but in fact, as
> > > > long as the server gives the user the permission to write in the /fs
> > > > directory, the above operation can be completed.
> > > >
> > > >
> > > > Best regards,
> > > >
> > > >
> > > >
> > > > J. Bruce Fields <bfields@fieldses.org> 于2021年1月9日周六 上午12:44写道：
> > > >
> > > > > On Sat, Jan 09, 2021 at 12:30:58AM +0800, 吴异 wrote:
> > > > > >  when the shared directory /fs/nfs of the server is mounted on the
> > device
> > > > > > /dev/sda3, and the "/" directory is mounted on the device
> > /dev/sda1, then
> > > > > > the file system of the shared directory and the "/" file system
> > are In
> > > > > > isolation, an attacker can obtain the ability of any file in the
> > > > > /dev/sda3
> > > > > > file system by using a file handle, but cannot access the "/" file
> > system
> > > > > > by using an NFS file handle, even if he already knows the "/"
> > handle is
> > > > > > "0100010000000000", which will return the error code nfserr_stale.
> > > > > >
> > > > > > In this case, the attacker deletes the mount point  directory
> > /fs/nfs in
> > > > > > the file system under its control., and create a symbolic link
> > with the
> > > > > > same name pointing to the "/" directory.
> > > > >
> > > > > Access to the filesystem mounted at /fs/nfs/ should not give you the
> > > > > ability to delete the directory /fs/nfs.
> > > > >
> > > > > Have you actually tried this?
> > > > >
> > > > > Please explain what access you assume the attacker starts with, what
> > > > > they're able to do it, why you think that isn't expected, and please
> > > > > test to make sure your idea works.
> > > > >
> > > > > --b.
> > > > >
> > > > >
> > > > > > After nfsd re-parses the
> > > > > > configuration file /etc/exports, the attacker can remount it to
> > achieve
> > > > > > real file system escape.
> > > > > >
> > > > > > It seems that nfsd should strengthen the restrictions on symbolic
> > link
> > > > > > resolution in configuration files.
> > > > > >
> > > > > > Best regards,
> > > > > >
> > > > > > J. Bruce Fields <bfields@fieldses.org> 于2021年1月8日周五 下午11:20写道：
> > > > > >
> > > > > > > On Fri, Jan 08, 2021 at 12:37:43PM +0100, Greg KH wrote:
> > > > > > > > On Tue, Jan 05, 2021 at 11:56:33AM -0500, J. Bruce Fields
> > wrote:
> > > > > > > > > On Tue, Jan 05, 2021 at 11:37:26PM +0800, 吴异 wrote:
> > > > > > > > > > It is not safe to export only the root of the file system,
> > > > > > > > > > Attacker can use lookup ".." to get the file handle to the
> > ".."
> > > > > > > directory，
> > > > > > > > >
> > > > > > > > > You have claimed the ability to read and write any file on
> > the file
> > > > > > > > > server.  But if you put /fs/nfs/ on a separate filesystem
> > then
> > > > > that is
> > > > > > > > > no longer possible.  Perhaps you can learn the value of the
> > > > > filehandle
> > > > > > > > > /fs/, but you can't use that filehandle to gain additional
> > access
> > > > > > > unless
> > > > > > > > > something on that filesystem is already exported.
> > > > > > > > >
> > > > > > > > > So, there may be some information disclosure here, I agree,
> > but
> > > > > it's
> > > > > > > > > relatively mild, given that filehandles are not well-kept
> > secrets
> > > > > > > > > anyway.
> > > > > > > >
> > > > > > > > Ok, to clarify, you are saying that this really isn't an issue
> > at all
> > > > > > > > with the kernel code, so there's nothing to do here?
> > > > > > > >
> > > > > > > > Just trying to figure out if there's anything to actually work
> > on
> > > > > here
> > > > > > > > or not.
> > > > > > >
> > > > > > > I think there's one possibly interesting issue, but it doesn't
> > require
> > > > > > > special handling.  Reporter has raised a few issues:
> > > > > > >
> > > > > > >         - The one that might be interesting: clients may be able
> > to
> > > > > > >           discover filehandles that they shouldn't be able to
> > using
> > > > > > >           readdir_plus.  On review of the code, I agree it's odd
> > that
> > > > > > >           the logic in compose_entry_fh() is different from that
> > in
> > > > > > >           nfsd_lookup_dentry().  I'd also prefer to take that
> > > > > discussion
> > > > > > >           to linux-nfs@vger.kernel.org, as this seems at most a
> > mild
> > > > > > >           information disclosure.
> > > > > > >
> > > > > > >         - If only part of a filesystem is exported, an attacker
> > can
> > > > > gain
> > > > > > >           access to files outside the exported directory by
> > guessing
> > > > > > >           filehandles.  This issue has been known for decades.
> > If we
> > > > > > >           want to discuss mitigation strategies, I believe the
> > correct
> > > > > > >           forum is again linux-nfs@vger.kernel.org, not
> > > > > > >           security@kernel.org.
> > > > > > >
> > > > > > >         - An attacker might replace an exported mountpoint by a
> > symlink
> > > > > > >           pointing elsewhere.  That seems to assume the attacker
> > > > > already
> > > > > > >           has a lot of access, so I'm not clear what advantage
> > they
> > > > > gain
> > > > > > >           from this.
> > > > > > >
> > > > > > > --b.
> > > > > > >
> > > > >
> > >
> > >
> >

[PATCH] nfsd4: readdirplus shouldn't return parent of export

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

If you export a subdirectory of a filesystem, a READDIRPLUS on the root
of that export will return the filehandle of the parent with the ".."
entry.

The filehandle is optional, so let's just not return the filehandle for
".." if we're at the root of an export.

Note that once the client learns one filehandle outside of the export,
they can trivially access the rest of the export using further lookups.

However, it is also not very difficult to guess filehandles outside of
the export.  So exporting a subdirectory of a filesystem should
considered equivalent to providing access to the entire filesystem.  To
avoid confusion, we recommend only exporting entire filesystems.

Reported-by: 吴异 <wangzhibei1999@gmail.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs3xdr.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index 821db21ba072..34b880211e5e 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -865,9 +865,14 @@ compose_entry_fh(struct nfsd3_readdirres *cd, struct svc_fh *fhp,
 	if (isdotent(name, namlen)) {
 		if (namlen == 2) {
 			dchild = dget_parent(dparent);
-			/* filesystem root - cannot return filehandle for ".." */
+			/*
+			 * Don't return filehandle for ".." if we're at
OA+			 * the filesystem or export root:
+			 */
 			if (dchild == dparent)
 				goto out;
+			if (dparent == exp->ex_path.dentry)
+				goto out;
 		} else
 			dchild = dget(dparent);
 	} else
-- 
2.29.2

Re: [PATCH] nfsd4: readdirplus shouldn't return parent of export

[Chuck Lever]

> On Jan 11, 2021, at 4:01 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> 
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> If you export a subdirectory of a filesystem, a READDIRPLUS on the root
> of that export will return the filehandle of the parent with the ".."
> entry.
> 
> The filehandle is optional, so let's just not return the filehandle for
> ".." if we're at the root of an export.
> 
> Note that once the client learns one filehandle outside of the export,
> they can trivially access the rest of the export using further lookups.
> 
> However, it is also not very difficult to guess filehandles outside of
> the export.  So exporting a subdirectory of a filesystem should
> considered equivalent to providing access to the entire filesystem.  To
> avoid confusion, we recommend only exporting entire filesystems.
> 
> Reported-by: 吴异 <wangzhibei1999@gmail.com>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
> fs/nfsd/nfs3xdr.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> index 821db21ba072..34b880211e5e 100644
> --- a/fs/nfsd/nfs3xdr.c
> +++ b/fs/nfsd/nfs3xdr.c
> @@ -865,9 +865,14 @@ compose_entry_fh(struct nfsd3_readdirres *cd, struct svc_fh *fhp,
> 	if (isdotent(name, namlen)) {
> 		if (namlen == 2) {
> 			dchild = dget_parent(dparent);
> -			/* filesystem root - cannot return filehandle for ".." */
> +			/*
> +			 * Don't return filehandle for ".." if we're at
> OA+			 * the filesystem or export root:
> +			 */
> 			if (dchild == dparent)
> 				goto out;
> +			if (dparent == exp->ex_path.dentry)
> +				goto out;
> 		} else
> 			dchild = dget(dparent);
> 	} else
> -- 
> 2.29.2

Thanks for the fix!

I've replaced the Reported-by: tag and pushed this to my
cel-next topic branch, and intend to submit it with the
next 5.11 -rc pull request. See:

https://git.linux-nfs.org/?p=cel/cel-2.6.git;a=shortlog;h=refs/heads/cel-next

Is there additional context that should be added? A Link:
tag that points to the discussion on security@ perhaps?

Note there was some damage in the patch body: there's a
spurious "OA" in the hunk that had to be removed before
the patch would apply.


--
Chuck Lever

Re: [PATCH] nfsd4: readdirplus shouldn't return parent of export

[Bruce Fields]

On Tue, Jan 12, 2021 at 08:31:59AM -0500, Chuck Lever wrote:
> 
> 
> > On Jan 11, 2021, at 4:01 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > 
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > If you export a subdirectory of a filesystem, a READDIRPLUS on the root
> > of that export will return the filehandle of the parent with the ".."
> > entry.
> > 
> > The filehandle is optional, so let's just not return the filehandle for
> > ".." if we're at the root of an export.
> > 
> > Note that once the client learns one filehandle outside of the export,
> > they can trivially access the rest of the export using further lookups.
> > 
> > However, it is also not very difficult to guess filehandles outside of
> > the export.  So exporting a subdirectory of a filesystem should
> > considered equivalent to providing access to the entire filesystem.  To
> > avoid confusion, we recommend only exporting entire filesystems.
> > 
> > Reported-by: 吴异 <wangzhibei1999@gmail.com>
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> > fs/nfsd/nfs3xdr.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> > index 821db21ba072..34b880211e5e 100644
> > --- a/fs/nfsd/nfs3xdr.c
> > +++ b/fs/nfsd/nfs3xdr.c
> > @@ -865,9 +865,14 @@ compose_entry_fh(struct nfsd3_readdirres *cd, struct svc_fh *fhp,
> > 	if (isdotent(name, namlen)) {
> > 		if (namlen == 2) {
> > 			dchild = dget_parent(dparent);
> > -			/* filesystem root - cannot return filehandle for ".." */
> > +			/*
> > +			 * Don't return filehandle for ".." if we're at
> > OA+			 * the filesystem or export root:
> > +			 */
> > 			if (dchild == dparent)
> > 				goto out;
> > +			if (dparent == exp->ex_path.dentry)
> > +				goto out;
> > 		} else
> > 			dchild = dget(dparent);
> > 	} else
> > -- 
> > 2.29.2
> 
> Thanks for the fix!
> 
> I've replaced the Reported-by: tag and pushed this to my
> cel-next topic branch, and intend to submit it with the
> next 5.11 -rc pull request. See:
> 
> https://git.linux-nfs.org/?p=cel/cel-2.6.git;a=shortlog;h=refs/heads/cel-next
> 
> Is there additional context that should be added? A Link:
> tag that points to the discussion on security@ perhaps?

I don't think so.

I guess it should get a stable cc: too.

> Note there was some damage in the patch body: there's a
> spurious "OA" in the hunk that had to be removed before
> the patch would apply.

Whoops, apologies, I'm not sure how that happened....

--b.

Re: nfsd vurlerability submit

[J. Bruce Fields]

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.

Re: nfsd vurlerability submit

[Trond Myklebust]

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

Re: nfsd vurlerability submit

[Patrick Goetz]

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

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

Re: nfsd vurlerability submit

[bfields]

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

Re: nfsd vurlerability submit

[Christoph Hellwig]

FYI, if people really want to use some sort of subtree exports, for XFS
(and probably ext4) we encode the project id into the file handle and
use the hierarchical project ID inheritance that we already use for
project quotas.

Re: nfsd vurlerability submit

[Trond Myklebust]

On Wed, 2021-01-13 at 08:12 +0000, Christoph Hellwig wrote:
> FYI, if people really want to use some sort of subtree exports, for
> XFS
> (and probably ext4) we encode the project id into the file handle and
> use the hierarchical project ID inheritance that we already use for
> project quotas.

You'd basically need something along the lines of a NetApp qtree.

i.e. a persisted tag that can label all the files and directories in a
subtree, and that is used to enforce a set of rules that are generally
normally associated with filesystems. So no renames from objects inside
the subtree to directories that lie outside it. No hard links that
cross the subtree boundary.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com

Re: nfsd vurlerability submit

[hch]

On Wed, Jan 13, 2021 at 02:34:45PM +0000, Trond Myklebust wrote:
> On Wed, 2021-01-13 at 08:12 +0000, Christoph Hellwig wrote:
> > FYI, if people really want to use some sort of subtree exports, for
> > XFS
> > (and probably ext4) we encode the project id into the file handle and
> > use the hierarchical project ID inheritance that we already use for
> > project quotas.
> 
> You'd basically need something along the lines of a NetApp qtree.
> 
> i.e. a persisted tag that can label all the files and directories in a
> subtree, and that is used to enforce a set of rules that are generally
> normally associated with filesystems. So no renames from objects inside
> the subtree to directories that lie outside it. No hard links that
> cross the subtree boundary.

That is the XFS project ID, which ext4 has also picked up a few years
ago.

Re: nfsd vurlerability submit

[Trond Myklebust]

On Wed, 2021-01-13 at 14:40 +0000, hch@infradead.org wrote:
> On Wed, Jan 13, 2021 at 02:34:45PM +0000, Trond Myklebust wrote:
> > On Wed, 2021-01-13 at 08:12 +0000, Christoph Hellwig wrote:
> > > FYI, if people really want to use some sort of subtree exports,
> > > for
> > > XFS
> > > (and probably ext4) we encode the project id into the file handle
> > > and
> > > use the hierarchical project ID inheritance that we already use
> > > for
> > > project quotas.
> > 
> > You'd basically need something along the lines of a NetApp qtree.
> > 
> > i.e. a persisted tag that can label all the files and directories
> > in a
> > subtree, and that is used to enforce a set of rules that are
> > generally
> > normally associated with filesystems. So no renames from objects
> > inside
> > the subtree to directories that lie outside it. No hard links that
> > cross the subtree boundary.
> 
> That is the XFS project ID, which ext4 has also picked up a few years
> ago.

How would that work then? Would you just look at the project ID of the
directory identified by the filehandle as the export point, and then
match to the project ID on the target inode? That sounds like it
doesn't even need to encode anything special in the filehandle.

How do you set a project ID in XFS?

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com

Re: nfsd vurlerability submit

[hch]

On Wed, Jan 13, 2021 at 03:16:52PM +0000, Trond Myklebust wrote:
> How would that work then? Would you just look at the project ID of the
> directory identified by the filehandle as the export point, and then
> match to the project ID on the target inode? That sounds like it
> doesn't even need to encode anything special in the filehandle.

True, we would not even have to encode them.

> How do you set a project ID in XFS?

With the XFS_IOC_SETXFLAGS ioctl.

On the command line side people usually do it using the xfs_quota
tool as part of setting up the tree quotas, but it can also be done
separately using the chproj subcommand of xfs_io.

RE: nfsd vurlerability submit

[Frank Filz]

> On Wed, Jan 13, 2021 at 03:16:52PM +0000, Trond Myklebust wrote:
> > How would that work then? Would you just look at the project ID of the
> > directory identified by the filehandle as the export point, and then
> > match to the project ID on the target inode? That sounds like it
> > doesn't even need to encode anything special in the filehandle.
> 
> True, we would not even have to encode them.
> 
> > How do you set a project ID in XFS?
> 
> With the XFS_IOC_SETXFLAGS ioctl.
> 
> On the command line side people usually do it using the xfs_quota tool as
part
> of setting up the tree quotas, but it can also be done separately using
the chproj
> subcommand of xfs_io.

Is this also queried via the ioctl? If this is a viable way of specifying
sub-trees, nfs-ganesha could also use it, though making an ioctl call for
each file system object would add some metadata performance hit.

Frank

Re: nfsd vurlerability submit

[Patrick Goetz]

See below.

On 1/12/21 12:03 PM, bfields@fieldses.org wrote:
> 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.
> 

I didn't respond to this message immediately, but it's been bothering me 
ever since. When I do a bind mount like this in /etc/fstab:

   /data2/xray      /srv/nfs/xray        none    defaults,bind    0

it's my understanding that the kernel keeps track of the resulting 
/srv/nfs/xray filesystem in it's vfs somehow.  Even when directly on the 
server I can't "break out" of /srv/nfs/xray to get to the other 
directories in /data.  Then how on earth would an NFS client do this?

I thought the whole point of doing a bind mount like this is to solve 
the problem of exporting leaves of a directory hierarchy. In particular,

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

Yes, sure; but I'm not exporting /data2/xray; I'm exporting 
/srv/nfs/xray, a bind mount to the preceding.  Am I missing something, 
or is NFS too insecure to use in any context requiring differentiated 
security settings on different folders in the same directory structure? 
  It's not practical to making everything you export its own partition; 
although I suppose one could do this with ZFS datasets.


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

Re: nfsd vurlerability submit

[bfields]

On Thu, Jan 21, 2021 at 02:01:13PM -0600, Patrick Goetz wrote:
> I didn't respond to this message immediately, but it's been
> bothering me ever since. When I do a bind mount like this in
> /etc/fstab:
> 
>   /data2/xray      /srv/nfs/xray        none    defaults,bind    0
> 
> it's my understanding that the kernel keeps track of the resulting
> /srv/nfs/xray filesystem in it's vfs somehow.  Even when directly on
> the server I can't "break out" of /srv/nfs/xray to get to the other
> directories in /data.  Then how on earth would an NFS client do
> this?

As I said, NFS allows you to look up objects by filehandle (so,
basically by inode number), not just by path.

Also, note, mounting something over a directory doesn't hide what's
under the mountpoint.  And it's unwise to depend on directory
permissions alone to hide contents of anything underneath that
directory.

> I thought the whole point of doing a bind mount like this is to
> solve the problem of exporting leaves of a directory hierarchy. In
> particular,
> 
>   "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."
> 
> Yes, sure; but I'm not exporting /data2/xray; I'm exporting
> /srv/nfs/xray, a bind mount to the preceding.  Am I missing
> something, or is NFS too insecure to use in any context requiring
> differentiated security settings on different folders in the same
> directory structure?

Definitely do *not* depend on NFS to enforce different export options on
different subdirectories of the same filesystem.

> It's not practical to making everything you export its own partition;
> although I suppose one could do this with ZFS datasets.

I'd be happy to hear about any use cases where that's not practical.

As Christophe pointed out, xfs/ext4 project ids are another option.

--b.

Re: nfsd vurlerability submit

[Patrick Goetz]

On 1/21/21 4:04 PM, bfields@fieldses.org wrote:
> On Thu, Jan 21, 2021 at 02:01:13PM -0600, Patrick Goetz wrote:
>> I didn't respond to this message immediately, but it's been
>> bothering me ever since. When I do a bind mount like this in
>> /etc/fstab:
>>
>>    /data2/xray      /srv/nfs/xray        none    defaults,bind    0
>>
>> it's my understanding that the kernel keeps track of the resulting
>> /srv/nfs/xray filesystem in it's vfs somehow.  Even when directly on
>> the server I can't "break out" of /srv/nfs/xray to get to the other
>> directories in /data.  Then how on earth would an NFS client do
>> this?
> 
> As I said, NFS allows you to look up objects by filehandle (so,
> basically by inode number), not just by path

Except surely this doesn't buy you much if you don't have root access to 
the system?  Is this all only an issue when the filesystems are exported 
with no_root_squash?

I feel like I must be missing something, but it seems to me that if I'm 
not root, I'm not going to be able to access inodes I don't have 
permissions to access even when directly connected to the exporting server.

> 
> Also, note, mounting something over a directory doesn't hide what's
> under the mountpoint.  And it's unwise to depend on directory
> permissions alone to hide contents of anything underneath that
> directory.

Well, I only ever bind mount over empty directories; but again, "doesn't 
hide what's under the mount point" from whom?  I'm sure root can get to 
this somehow, but can someone with ordinary user access? even if the 
user doesn't have permissions to access the stuff that's been mounted over?


> 
>> I thought the whole point of doing a bind mount like this is to
>> solve the problem of exporting leaves of a directory hierarchy. In
>> particular,
>>
>>    "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."
>>
>> Yes, sure; but I'm not exporting /data2/xray; I'm exporting
>> /srv/nfs/xray, a bind mount to the preceding.  Am I missing
>> something, or is NFS too insecure to use in any context requiring
>> differentiated security settings on different folders in the same
>> directory structure?
> 
> Definitely do *not* depend on NFS to enforce different export options on
> different subdirectories of the same filesystem.
> 
>> It's not practical to making everything you export its own partition;
>> although I suppose one could do this with ZFS datasets.
> 
> I'd be happy to hear about any use cases where that's not practical.
> 

Sure. The xray example is taken from one of my research groups which 
collects thousands of very large electron microscopy images, along with 
some xray data. I will certainly design this differently in the next 
iteration (most likely using ZFS), but our current server has a 519T 
attached storage device which presents itself as a single device: 
/dev/sdg.  Different groups need access to different classes of data, 
which I export separately and with are presented on the workstations as 
/xray, /EM, etc..

Yes, I could partition the storage device, but then I run into the usual 
issues where one partition runs out of space while others are barely 
utilized. This is one good reason to switch to ZFS datasets.  The other 
is that -- with 450T+ of ever changing data, currently rsync backups are 
almost impossible.  I'm hoping zfs send/receive is going to save me here.


> As Christophe pointed out, xfs/ext4 project ids are another option.
> 
> --b.
> 

I must have missed this one, but it just leaves me more confused. 
Project ID's are filesystem metadata, yet this affords better boundary 
enforcement than a bind mount?  Also, the only use case for Project ID's 
I was able to find are project quotas, so am not even sure how this 
would be implemented, and used by NFS.

Re: nfsd vurlerability submit

[bfields]

On Thu, Jan 21, 2021 at 05:19:32PM -0600, Patrick Goetz wrote:
> On 1/21/21 4:04 PM, bfields@fieldses.org wrote:
> >As I said, NFS allows you to look up objects by filehandle (so,
> >basically by inode number), not just by path
> 
> Except surely this doesn't buy you much if you don't have root
> access to the system?  Is this all only an issue when the
> filesystems are exported with no_root_squash?
> 
> I feel like I must be missing something, but it seems to me that if
> I'm not root, I'm not going to be able to access inodes I don't have
> permissions to access even when directly connected to the exporting
> server.

If an attacker has access to the network (so they can send their own
hand-crafted NFS requests), then filehandle guessing allows them to
bypass the normal process of looking up a file.  So if you were
depending on lookup permissions along that path, or on hiding that path
somehow, you're out of luck.

But it doesn't let them bypass the permissions on the file itself once
they get there.  If the permissions on the file don't allow read, the
server still won't let them read it.

> >>It's not practical to making everything you export its own partition;
> >>although I suppose one could do this with ZFS datasets.
> >
> >I'd be happy to hear about any use cases where that's not practical.
> 
> Sure. The xray example is taken from one of my research groups which
> collects thousands of very large electron microscopy images, along
> with some xray data. I will certainly design this differently in the
> next iteration (most likely using ZFS), but our current server has a
> 519T attached storage device which presents itself as a single
> device: /dev/sdg.  Different groups need access to different classes
> of data, which I export separately and with are presented on the
> workstations as /xray, /EM, etc..
> 
> Yes, I could partition the storage device, but then I run into the
> usual issues where one partition runs out of space while others are
> barely utilized. This is one good reason to switch to ZFS datasets.
> The other is that -- with 450T+ of ever changing data, currently
> rsync backups are almost impossible.  I'm hoping zfs send/receive is
> going to save me here.
> 
> >As Christophe pointed out, xfs/ext4 project ids are another option.
> 
> I must have missed this one, but it just leaves me more confused.
> Project ID's are filesystem metadata, yet this affords better
> boundary enforcement than a bind mount?

Right.  The project ID is stored in the inode, so it's easy to look up
from the filehandle.  (Whereas figuring out what paths might lead to
that inode is a little tricker.)

> Also, the only use case for Project ID's I was able to find are
> project quotas, so am not even sure how this would be implemented, and
> used by NFS.

Project ID's were implemented for quotas, but they also have the
characteristics to work well as NFS export boundaries.

That said, I think Christoph was suggesting this is something we *could*
support, not something that we now do.  Though looking at it quickly, I
think it shouldn't take much code at all.  I'll put it on my list....

Other options for doing this kind of thing might be btrfs subvolumes or
lvm thin provisioning.  I haven't personally used either, but they
should both work now.

--b.

Re: nfsd vurlerability submit

[Patrick Goetz]

Thanks for engaging; this has been informative.

On 1/21/21 7:30 PM, bfields@fieldses.org wrote:
> On Thu, Jan 21, 2021 at 05:19:32PM -0600, Patrick Goetz wrote:
>> On 1/21/21 4:04 PM, bfields@fieldses.org wrote:
>>> As I said, NFS allows you to look up objects by filehandle (so,
>>> basically by inode number), not just by path
>>
>> Except surely this doesn't buy you much if you don't have root
>> access to the system?  Is this all only an issue when the
>> filesystems are exported with no_root_squash?
>>
>> I feel like I must be missing something, but it seems to me that if
>> I'm not root, I'm not going to be able to access inodes I don't have
>> permissions to access even when directly connected to the exporting
>> server.
> 
> If an attacker has access to the network (so they can send their own
> hand-crafted NFS requests), then filehandle guessing allows them to
> bypass the normal process of looking up a file.  So if you were
> depending on lookup permissions along that path, or on hiding that path
> somehow, you're out of luck.
> 
> But it doesn't let them bypass the permissions on the file itself once
> they get there.  If the permissions on the file don't allow read, the
> server still won't let them read it.
>

That's probably good enough. Security through obscurity isn't a good 
idea, so file/directory level permissions should be atomically correct 
and not rely on directory hierarchies, restricted direct access by 
users, or anything like this.

I didn't not know about the filehandle guessing thing and will keep that 
in mind for the next NFS server I deploy.


>>>> It's not practical to making everything you export its own partition;
>>>> although I suppose one could do this with ZFS datasets.
>>>
>>> I'd be happy to hear about any use cases where that's not practical.
>>
>> Sure. The xray example is taken from one of my research groups which
>> collects thousands of very large electron microscopy images, along
>> with some xray data. I will certainly design this differently in the
>> next iteration (most likely using ZFS), but our current server has a
>> 519T attached storage device which presents itself as a single
>> device: /dev/sdg.  Different groups need access to different classes
>> of data, which I export separately and with are presented on the
>> workstations as /xray, /EM, etc..
>>
>> Yes, I could partition the storage device, but then I run into the
>> usual issues where one partition runs out of space while others are
>> barely utilized. This is one good reason to switch to ZFS datasets.
>> The other is that -- with 450T+ of ever changing data, currently
>> rsync backups are almost impossible.  I'm hoping zfs send/receive is
>> going to save me here.
>>
>>> As Christophe pointed out, xfs/ext4 project ids are another option.
>>
>> I must have missed this one, but it just leaves me more confused.
>> Project ID's are filesystem metadata, yet this affords better
>> boundary enforcement than a bind mount?
> 
> Right.  The project ID is stored in the inode, so it's easy to look up
> from the filehandle.  (Whereas figuring out what paths might lead to
> that inode is a little tricker.)
> 
>> Also, the only use case for Project ID's I was able to find are
>> project quotas, so am not even sure how this would be implemented, and
>> used by NFS.
> 
> Project ID's were implemented for quotas, but they also have the
> characteristics to work well as NFS export boundaries.
> 
> That said, I think Christoph was suggesting this is something we *could*
> support, not something that we now do.  Though looking at it quickly, I
> think it shouldn't take much code at all.  I'll put it on my list....
> 
> Other options for doing this kind of thing might be btrfs subvolumes or
> lvm thin provisioning.  I haven't personally used either, but they
> should both work now.
> 
> --b.
> 

Re: nfsd vurlerability submit

[Tom Talpey]

On 1/22/2021 8:20 AM, Patrick Goetz wrote:
> Thanks for engaging; this has been informative.
> 
> On 1/21/21 7:30 PM, bfields@fieldses.org wrote:
>> On Thu, Jan 21, 2021 at 05:19:32PM -0600, Patrick Goetz wrote:
>>> On 1/21/21 4:04 PM, bfields@fieldses.org wrote:
>>>> As I said, NFS allows you to look up objects by filehandle (so,
>>>> basically by inode number), not just by path
>>>
>>> Except surely this doesn't buy you much if you don't have root
>>> access to the system?  Is this all only an issue when the
>>> filesystems are exported with no_root_squash?
>>>
>>> I feel like I must be missing something, but it seems to me that if
>>> I'm not root, I'm not going to be able to access inodes I don't have
>>> permissions to access even when directly connected to the exporting
>>> server.
>>
>> If an attacker has access to the network (so they can send their own
>> hand-crafted NFS requests), then filehandle guessing allows them to
>> bypass the normal process of looking up a file.  So if you were
>> depending on lookup permissions along that path, or on hiding that path
>> somehow, you're out of luck.
>>
>> But it doesn't let them bypass the permissions on the file itself once
>> they get there.  If the permissions on the file don't allow read, the
>> server still won't let them read it.
>>
> 
> That's probably good enough. Security through obscurity isn't a good 
> idea, so file/directory level permissions should be atomically correct 
> and not rely on directory hierarchies, restricted direct access by 
> users, or anything like this.
> 
> I didn't not know about the filehandle guessing thing and will keep that 
> in mind for the next NFS server I deploy.

There are some NFS clients which implement open-by-filehandle, and don't
rely on lookup at all in normal operation. It is highly efficient, and
provides a very low latency file access. I believe Yahoo was the first
to implement it at scale in production, but it's a very straightforward
cache operation.

Tom.

>>>>> It's not practical to making everything you export its own partition;
>>>>> although I suppose one could do this with ZFS datasets.
>>>>
>>>> I'd be happy to hear about any use cases where that's not practical.
>>>
>>> Sure. The xray example is taken from one of my research groups which
>>> collects thousands of very large electron microscopy images, along
>>> with some xray data. I will certainly design this differently in the
>>> next iteration (most likely using ZFS), but our current server has a
>>> 519T attached storage device which presents itself as a single
>>> device: /dev/sdg.  Different groups need access to different classes
>>> of data, which I export separately and with are presented on the
>>> workstations as /xray, /EM, etc..
>>>
>>> Yes, I could partition the storage device, but then I run into the
>>> usual issues where one partition runs out of space while others are
>>> barely utilized. This is one good reason to switch to ZFS datasets.
>>> The other is that -- with 450T+ of ever changing data, currently
>>> rsync backups are almost impossible.  I'm hoping zfs send/receive is
>>> going to save me here.
>>>
>>>> As Christophe pointed out, xfs/ext4 project ids are another option.
>>>
>>> I must have missed this one, but it just leaves me more confused.
>>> Project ID's are filesystem metadata, yet this affords better
>>> boundary enforcement than a bind mount?
>>
>> Right.  The project ID is stored in the inode, so it's easy to look up
>> from the filehandle.  (Whereas figuring out what paths might lead to
>> that inode is a little tricker.)
>>
>>> Also, the only use case for Project ID's I was able to find are
>>> project quotas, so am not even sure how this would be implemented, and
>>> used by NFS.
>>
>> Project ID's were implemented for quotas, but they also have the
>> characteristics to work well as NFS export boundaries.
>>
>> That said, I think Christoph was suggesting this is something we *could*
>> support, not something that we now do.  Though looking at it quickly, I
>> think it shouldn't take much code at all.  I'll put it on my list....
>>
>> Other options for doing this kind of thing might be btrfs subvolumes or
>> lvm thin provisioning.  I haven't personally used either, but they
>> should both work now.
>>
>> --b.
>>
> 

Re: nfsd vurlerability submit

[Trond Myklebust]

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

Re: nfsd vurlerability submit

[Trond Myklebust]

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

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

Re: nfsd vurlerability submit

[bfields]

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
> 

Re: nfsd vurlerability submit

[Linus Torvalds]

On Thu, Jan 14, 2021 at 10:08 AM bfields@fieldses.org
<bfields@fieldses.org> 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

Re: nfsd vurlerability submit

[Chuck Lever]

> On Jan 14, 2021, at 1:29 PM, Linus Torvalds <torvalds@linuxfoundation.org> wrote:
> 
> On Thu, Jan 14, 2021 at 10:08 AM bfields@fieldses.org
> <bfields@fieldses.org> 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

Re: nfsd vurlerability submit

[Linus Torvalds]

On Thu, Jan 14, 2021 at 10:35 AM Chuck Lever <chuck.lever@oracle.com> 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

Re: nfsd vurlerability submit

[吴异]

hello,

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.

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

Re: nfsd vurlerability submit

[bfields]

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

Re: nfsd vurlerability submit

[吴异]

My patch is below:
/fs/nfsd/nfsfh.c

nfsd_acceptable(void expv,struct dentry *dentry)
{
-     if(exp->ex_flags & NFSEXP_NOSUBTREEXHECK)
-     return 1;

+     if(is_root_export(exp))
+       return 1;

+    /* If not subdirectory export, accept anything*/

}




在 2021年1月19日星期二，bfields@fieldses.org <bfields@fieldses.org> 写道：
> 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
>> >
>> >
>

Re: nfsd vurlerability submit

[bfields]

On Tue, Jan 19, 2021 at 10:48:22AM +0800, 吴异 wrote:
> My patch is below:
> /fs/nfsd/nfsfh.c
> 
> nfsd_acceptable(void expv,struct dentry *dentry)
> {
> -     if(exp->ex_flags & NFSEXP_NOSUBTREEXHECK)
> -     return 1;
> 
> +     if(is_root_export(exp))
> +       return 1;

So you end up doing the SUBTREECHECK checks in cases where the
filehandle doesn't actually have any parent information.  That means we
may be stuck with no way to connect the file back up the export point,
so this check will fail even though the file may be under the export
point.  So users probably get spurious ESTALE errors.

To reproduce you probably need to do something like open a file on the
client, reboot the server, then try to access the open file again.  The
dentry cache will no longer know how to look up the parents after the
reboot.

--b.

> 
> +    /* If not subdirectory export, accept anything*/
> 
> }
> 
> 
> 
> 
> 在 2021年1月19日星期二，bfields@fieldses.org <bfields@fieldses.org> 写道：
> > 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
> >> >
> >> >
> >