public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
* pseudoroot kernel patches
@ 2009-12-02  0:39 J. Bruce Fields
  2009-12-02  0:39 ` [PATCH 1/7] nfsd: introduce export flag for v4 pseudoroot J. Bruce Fields
  0 siblings, 1 reply; 10+ messages in thread
From: J. Bruce Fields @ 2009-12-02  0:39 UTC (permalink / raw)
  To: linux-nfs, nfsv4; +Cc: Steve Dickson

This is my revision of Steve Dickson's series of patches that allow
automatically constructing the NFSv4 psuedoroot.  I think it's close to
a final version.

The basic idea is for mountd to automatically export all of the
filesystems which must be traversed to reach any exported filesystem.
This raises obvious security concerns.  Steved's solution is to greatly
restrict access to those exports, by adding a new export flag
(NFSEXP_V4ROOT), which tells the kernel that *only* the single object at
the given path is meant to be exported, *not* the rest of the filesystem
underneath it.  Thus mountd actually generates a separate export for
every directory along the path to a real export.

Changes since the last version steved posted:
	- fix nfsd_verify to prevent filehandle-guessing attacks from
	  allowing access to unexported objects on V4ROOT filesystems.
	- fix a bug which could cause NFSv4's readdir to return bad
	  filehandles for directory entries.
	- Allow V4ROOT exports of symlink objects (to allow the path
	  listed in /etc/exports to be a symlink, as has traditionally
	  been permitted with v2/v3).  The mountd side of this is not
	  yet written.
	- Simplify the code somewhat by moving most of the readdir and
	  lookup checks into nfsd_crossmnt.

Some problems will remain:

The exported v4 namespace will still not be entirely identical with the
v2/v3 namespace; to some degree this is inevitable:

	- If /export and /export/foo are two different filesystems, both
	  exported, then a v2/v3 client that mounts /export will not see
	  the filesystem /foo.

	  This is an inherent limitation of the v2/v3 protocols, which
	  don't require clients to know how to traverse mountpoints.
	  
	  The server's current behavior in this case is simply to show
	  the contents of the directory named "foo" on the filesystem
	  /export.   This may allow the client to see (even create)
	  directory objects on /export which are invisible to users on
	  the server, because the filesystem mounted on top of
	  /export/foo hides them.

	  We could instead modify the server to hide the contents of
	  foo/ from the client somehow.  This must be done carefully:
	  the directory "foo" itself must still be present in case the
	  client wants to mount something there itself.

	- Nested exports on the same filesystem also pose a problem;
	  given:
		/export		*(rw)
		/export/foo	*(ro)
	  with foo on the same filesystem as /export,
	  	mount -tnfs server:/export/foo /mnt
	  will give a read-only filesystem,
		mount -nfs4 server:/export/foo /mnt
	  will give a read-write filesystem.  A v4 client won't see
	  any change in export options as it traverses into foo.

There are also some potential drawbacks to this approach, which we can
probably live with:

	- It requires creating an entry in the export cache for any
	  directory that is an ancestor of a directory, and for every
	  entry of each directory (a negative cache entry in the case of
	  entries that don't lead to exports).  Improvements in the
	  cache management may be sufficient to mitigate problems that
	  show up.
	- Handling filehandle->export mapping will require stat'ing all
	  the parents of exports, as well as exports, possibility
	  exacerbating problems previously raised on this list with
	  spinning up idle disks unnecessarily.
	- It will never work in cases where "/" and other filesystems
	  which must be traversed on the way to an export are not
	  themselves nfs-exportable.  This is probably a rare corner
	  case, but we've seen at least one example of somebody doing
	  embedded work who does this.  We should at least make sure we
	  fail gracefully in this case (e.g. by turning off attempts to
	  build pseudofilesystem).
	- It doesn't provide a simple upgrade path for anyone using the
	  current manual pseudoroot-construction who would also like
	  paths consistent with v2/v3.  That problem is actually very
	  simple to solve in nfs-utils, and I'll post patches for that.
	- The additional automatic exports may raise security risks.  I
	  believe we're now restricting access correctly, but additional
	  eyes here wouldn't hurt.

Before we merge this, I'd also like to look a little more into which
interfaces V4ROOT exports should be visible (versus which they should be
hidden from--as they're not quite "real" exports).  And we have a few
problems on the nfs-utils side which I believe Steve is resolving (if he
hasn't already).

--b.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/7] nfsd: introduce export flag for v4 pseudoroot
  2009-12-02  0:39 pseudoroot kernel patches J. Bruce Fields
@ 2009-12-02  0:39 ` J. Bruce Fields
  2009-12-02  0:39   ` [PATCH 2/7] nfsd4: don't continue "under" mounts in V4ROOT case J. Bruce Fields
  0 siblings, 1 reply; 10+ messages in thread
From: J. Bruce Fields @ 2009-12-02  0:39 UTC (permalink / raw)
  To: linux-nfs, nfsv4; +Cc: Steve Dickson, Steve Dickson, J. Bruce Fields

From: Steve Dickson <SteveD@redhat.com>

NFSv4 differs from v2 and v3 in that it presents a single unified
filesystem tree, whereas v2 and v3 exported multiple filesystem (whose
roots could be found using a separate mount protocol).

Our original NFSv4 server implementation asked the administrator to
designate a single filesystem as the NFSv4 root, then to mount
filesystems they wished to export underneath.  (Often using bind mounts
of already-existing filesystems.)

This was conceptually simple, and allowed easy implementation, but
created a serious obstacle to upgrading between v2/v3: since the paths
to v4 filesystems were different, administrators would have to adjust
all the paths in client-side mount commands when switching to v4.

Various workarounds are possible.  For example, the administrator could
export "/" and designate it as the v4 root.  However, the security risks
of that approach are obvious, and in any case we shouldn't be requiring
the administrator to take extra steps to fix this problem; instead, the
server should present consistent paths across different versions by
default.

These patches take a modified version of that approach: we provide a new
export option which exports only a subset of a filesystem.  With this
flag, it becomes safe for mountd to export "/" by default, with no need
for additional configuration.

We begin just by defining the new flag.

Signed-Off-By: Steve Dickson <steved@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/nfsd/export.c            |    1 +
 include/linux/nfsd/export.h |   12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index b73baba..b9e4977 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1439,6 +1439,7 @@ static struct flags {
 	{ NFSEXP_CROSSMOUNT, {"crossmnt", ""}},
 	{ NFSEXP_NOSUBTREECHECK, {"no_subtree_check", ""}},
 	{ NFSEXP_NOAUTHNLM, {"insecure_locks", ""}},
+	{ NFSEXP_V4ROOT, {"v4root", ""}},
 #ifdef MSNFS
 	{ NFSEXP_MSNFS, {"msnfs", ""}},
 #endif
diff --git a/include/linux/nfsd/export.h b/include/linux/nfsd/export.h
index a6d9ef2..e963ba9 100644
--- a/include/linux/nfsd/export.h
+++ b/include/linux/nfsd/export.h
@@ -39,7 +39,17 @@
 #define NFSEXP_FSID		0x2000
 #define	NFSEXP_CROSSMOUNT	0x4000
 #define	NFSEXP_NOACL		0x8000	/* reserved for possible ACL related use */
-#define NFSEXP_ALLFLAGS		0xFE3F
+/*
+ * The NFSEXP_V4ROOT flag causes the kernel to give access only to NFSv4
+ * clients, and only to the single directory that is the root of the
+ * export; further lookup and readdir operations are treated as if every
+ * subdirectory was a mountpoint, and ignored if they are not themselves
+ * exported.  This is used by nfsd and mountd to construct the NFSv4
+ * pseudofilesystem, which provides access only to paths leading to each
+ * exported filesystem.
+ */
+#define	NFSEXP_V4ROOT		0x10000
+#define NFSEXP_ALLFLAGS		0x1FE3F
 
 /* The flags that may vary depending on security flavor: */
 #define NFSEXP_SECINFO_FLAGS	(NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/7] nfsd4: don't continue "under" mounts in V4ROOT case
  2009-12-02  0:39 ` [PATCH 1/7] nfsd: introduce export flag for v4 pseudoroot J. Bruce Fields
@ 2009-12-02  0:39   ` J. Bruce Fields
  2009-12-02  0:39     ` [PATCH 3/7] nfsd: filter lookup results " J. Bruce Fields
  0 siblings, 1 reply; 10+ messages in thread
From: J. Bruce Fields @ 2009-12-02  0:39 UTC (permalink / raw)
  To: linux-nfs, nfsv4; +Cc: Steve Dickson, J. Bruce Fields

If /A/mount/point/ has filesystem "B" mounted on top of it, and if "A"
is exported, but not "B", then the nfs server has always returned to the
client a filehandle for the mountpoint, instead of for the root of "B",
allowing the client to see the subtree of "A" that would otherwise be
hidden by B.

Disable this behavior in the case of V4ROOT exports; we implement the
path restrictions of V4ROOT exports by treating *every* directory as if
it were a mountpoint, and allowing traversal *only* if the new directory
is exported.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/nfsd/vfs.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index a7038ed..04d61c8 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -117,8 +117,16 @@ nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
 
 	exp2 = rqst_exp_get_by_name(rqstp, &path);
 	if (IS_ERR(exp2)) {
-		if (PTR_ERR(exp2) != -ENOENT)
-			err = PTR_ERR(exp2);
+		err = PTR_ERR(exp2);
+		/*
+		 * We normally allow NFS clients to continue
+		 * "underneath" a mountpoint that is not exported.
+		 * The exception is V4ROOT, where no traversal is ever
+		 * allowed without an explicit export of the new
+		 * directory.
+		 */
+		if (err == -ENOENT && !(exp->ex_flags & NFSEXP_V4ROOT))
+			err = 0;
 		path_put(&path);
 		goto out;
 	}
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/7] nfsd: filter lookup results in V4ROOT case
  2009-12-02  0:39   ` [PATCH 2/7] nfsd4: don't continue "under" mounts in V4ROOT case J. Bruce Fields
@ 2009-12-02  0:39     ` J. Bruce Fields
  2009-12-02  0:39       ` [PATCH 4/7] nfsd: special readdir exception for V4ROOT J. Bruce Fields
  0 siblings, 1 reply; 10+ messages in thread
From: J. Bruce Fields @ 2009-12-02  0:39 UTC (permalink / raw)
  To: linux-nfs, nfsv4; +Cc: Steve Dickson, J. Bruce Fields

We treat every object as a mountpoint and pretend it doesn't exist if
it isn't exported.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/nfsd/vfs.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 04d61c8..5136e61 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -184,6 +184,19 @@ static int nfsd_lookup_parent(struct svc_rqst *rqstp, struct dentry *dparent, st
 	return 0;
 }
 
+/*
+ * For nfsd purposes, we treat V4ROOT exports as though there was an
+ * export at *every* directory.
+ */
+static int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
+{
+	if (d_mountpoint(dentry))
+		return 1;
+	if (!(exp->ex_flags & NFSEXP_V4ROOT))
+		return 0;
+	return dentry->d_inode != NULL;
+}
+
 __be32
 nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		   const char *name, unsigned int len,
@@ -229,7 +242,7 @@ nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		/*
 		 * check if we have crossed a mount point ...
 		 */
-		if (d_mountpoint(dentry)) {
+		if (nfsd_mountpoint(dentry, exp)) {
 			if ((host_err = nfsd_cross_mnt(rqstp, &dentry, &exp))) {
 				dput(dentry);
 				goto out_nfserr;
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/7] nfsd: special readdir exception for V4ROOT
  2009-12-02  0:39     ` [PATCH 3/7] nfsd: filter lookup results " J. Bruce Fields
@ 2009-12-02  0:39       ` J. Bruce Fields
  2009-12-02  0:39         ` [PATCH 5/7] nfsd: allow exports of symlinks J. Bruce Fields
  0 siblings, 1 reply; 10+ messages in thread
From: J. Bruce Fields @ 2009-12-02  0:39 UTC (permalink / raw)
  To: linux-nfs, nfsv4; +Cc: Steve Dickson, J. Bruce Fields

Code here is a little ugly; maybe do some cleanup first.
---
 fs/nfsd/nfs4xdr.c |   10 +++++++---
 fs/nfsd/vfs.c     |    2 +-
 fs/nfsd/vfs.h     |    1 +
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index db0fc55..931fa96 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2205,11 +2205,14 @@ nfsd4_encode_dirent_fattr(struct nfsd4_readdir *cd,
 	 * we will not follow the cross mount and will fill the attribtutes
 	 * directly from the mountpoint dentry.
 	 */
-	if (d_mountpoint(dentry) && !attributes_need_mount(cd->rd_bmval))
-		ignore_crossmnt = 1;
-	else if (d_mountpoint(dentry)) {
+	if (nfsd_mountpoint(dentry, exp)) {
 		int err;
 
+		if (!(exp->ex_flags & NFSEXP_V4ROOT)
+				&& !attributes_need_mount(cd->rd_bmval)) {
+			ignore_crossmnt = 1;
+			goto out_encode;
+		}
 		/*
 		 * Why the heck aren't we just using nfsd_lookup??
 		 * Different "."/".." handling?  Something else?
@@ -2225,6 +2228,7 @@ nfsd4_encode_dirent_fattr(struct nfsd4_readdir *cd,
 			goto out_put;
 
 	}
+out_encode:
 	nfserr = nfsd4_encode_fattr(NULL, exp, dentry, p, buflen, cd->rd_bmval,
 					cd->rd_rqstp, ignore_crossmnt);
 out_put:
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 5136e61..caf4794 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -188,7 +188,7 @@ static int nfsd_lookup_parent(struct svc_rqst *rqstp, struct dentry *dparent, st
  * For nfsd purposes, we treat V4ROOT exports as though there was an
  * export at *every* directory.
  */
-static int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
+int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
 {
 	if (d_mountpoint(dentry))
 		return 1;
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index b8011fd..f4fa6d3 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -40,6 +40,7 @@ __be32		 nfsd_lookup_dentry(struct svc_rqst *, struct svc_fh *,
 				struct svc_export **, struct dentry **);
 __be32		nfsd_setattr(struct svc_rqst *, struct svc_fh *,
 				struct iattr *, int, time_t);
+int nfsd_mountpoint(struct dentry *, struct svc_export *);
 #ifdef CONFIG_NFSD_V4
 __be32          nfsd4_set_nfs4_acl(struct svc_rqst *, struct svc_fh *,
                     struct nfs4_acl *);
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/7] nfsd: allow exports of symlinks
  2009-12-02  0:39       ` [PATCH 4/7] nfsd: special readdir exception for V4ROOT J. Bruce Fields
@ 2009-12-02  0:39         ` J. Bruce Fields
  2009-12-02  0:39           ` [PATCH 6/7] nfsd: restrict filehandles accepted in V4ROOT case J. Bruce Fields
  0 siblings, 1 reply; 10+ messages in thread
From: J. Bruce Fields @ 2009-12-02  0:39 UTC (permalink / raw)
  To: linux-nfs, nfsv4; +Cc: Steve Dickson, J. Bruce Fields

We want to allow exports of symlinks, to allow mountd to communicate to
the kernel which symlinks lead to exports, and hence which symlinks need
to be visible on the pseudofilesystem.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/nfsd/export.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index b9e4977..0bcd10a 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -372,10 +372,12 @@ static struct svc_export *svc_export_lookup(struct svc_export *);
 static int check_export(struct inode *inode, int flags, unsigned char *uuid)
 {
 
-	/* We currently export only dirs and regular files.
-	 * This is what umountd does.
+	/*
+	 * We currently export only dirs, regular files, and (for v4
+	 * pseudoroot) symlinks.
 	 */
 	if (!S_ISDIR(inode->i_mode) &&
+	    !S_ISLNK(inode->i_mode) &&
 	    !S_ISREG(inode->i_mode))
 		return -ENOTDIR;
 
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/7] nfsd: restrict filehandles accepted in V4ROOT case
  2009-12-02  0:39         ` [PATCH 5/7] nfsd: allow exports of symlinks J. Bruce Fields
@ 2009-12-02  0:39           ` J. Bruce Fields
  2009-12-02  0:39             ` [PATCH 7/7] nfsd: increase export interface version J. Bruce Fields
  2009-12-04 15:05             ` [PATCH 6/7] nfsd: restrict filehandles accepted in V4ROOT case Steve Dickson
  0 siblings, 2 replies; 10+ messages in thread
From: J. Bruce Fields @ 2009-12-02  0:39 UTC (permalink / raw)
  To: linux-nfs, nfsv4; +Cc: Steve Dickson, Steve Dickson, J. Bruce Fields

From: Steve Dickson <SteveD@redhat.com>

On V4ROOT exports, only accept filehandles that are the *root* of some
export.  This allows mountd to allow or deny access to individual paths
and symlinks on the pseudofilesystem.

Note that the checks in readdir and lookup are not enough, since a
malicious host with access to the network could guess filehandles that
they weren't able to obtain through lookup or readdir.

Signed-Off-By: Steve Dickson <steved@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/nfsd/nfsd.h  |    4 ++++
 fs/nfsd/nfsfh.c |   35 +++++++++++++++++++++++++++++++++++
 fs/nfsd/vfs.c   |    7 +------
 3 files changed, 40 insertions(+), 6 deletions(-)
 create mode 100644 fs/nfsd/nfsd.h

diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
new file mode 100644
index 0000000..7a1ad80
--- /dev/null
+++ b/fs/nfsd/nfsd.h
@@ -0,0 +1,4 @@
+static inline int nfsd_v4client(struct svc_rqst *rq)
+{
+	return rq->rq_prog == NFS_PROGRAM && rq->rq_vers == 4;
+}
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index a77efb8..9b902c0 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -22,6 +22,7 @@
 #include <linux/sunrpc/svc.h>
 #include <linux/sunrpc/svcauth_gss.h>
 #include <linux/nfsd/nfsd.h>
+#include "nfsd.h"
 #include "vfs.h"
 #include "auth.h"
 
@@ -110,6 +111,36 @@ static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,
 	return nfserrno(nfsd_setuser(rqstp, exp));
 }
 
+static inline __be32 check_pseudo_root(struct svc_rqst *rqstp,
+	struct dentry *dentry, struct svc_export *exp)
+{
+	if (!(exp->ex_flags & NFSEXP_V4ROOT))
+		return nfs_ok;
+	/*
+	 * v2/v3 clients have no need for the V4ROOT export--they use
+	 * the mount protocl instead; also, further V4ROOT checks may be
+	 * in v4-specific code, in which case v2/v3 clients could bypass
+	 * them.
+	 */
+	if (!nfsd_v4client(rqstp))
+		return nfserr_stale;
+	/*
+	 * We're exposing only the directories and symlinks that have to be
+	 * traversed on the way to real exports:
+	 */
+	if (unlikely(!S_ISDIR(dentry->d_inode->i_mode) &&
+		     !S_ISLNK(dentry->d_inode->i_mode)))
+		return nfserr_stale;
+	/*
+	 * A pseudoroot export gives permission to access only one
+	 * single directory; the kernel has to make another upcall
+	 * before granting access to anything else under it:
+	 */
+	if (unlikely(dentry->d_parent != exp->ex_path.dentry))
+		return nfserr_stale;
+	return nfs_ok;
+}
+
 /*
  * Use the given filehandle to look up the corresponding export and
  * dentry.  On success, the results are used to set fh_export and
@@ -306,6 +337,10 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, int access)
 	 *	  (for example, if different id-squashing options are in
 	 *	  effect on the new filesystem).
 	 */
+	error = check_pseudo_root(rqstp, dentry, exp);
+	if (error)
+		goto out;
+
 	error = nfsd_setuser_and_check_port(rqstp, exp);
 	if (error)
 		goto out;
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index caf4794..f20d7b4 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -56,6 +56,7 @@
 #endif /* CONFIG_NFSD_V4 */
 #include <linux/jhash.h>
 #include <linux/ima.h>
+#include "nfsd.h"
 #include "vfs.h"
 
 #include <asm/uaccess.h>
@@ -90,12 +91,6 @@ struct raparm_hbucket {
 #define RAPARM_HASH_MASK	(RAPARM_HASH_SIZE-1)
 static struct raparm_hbucket	raparm_hash[RAPARM_HASH_SIZE];
 
-static inline int
-nfsd_v4client(struct svc_rqst *rq)
-{
-    return rq->rq_prog == NFS_PROGRAM && rq->rq_vers == 4;
-}
-
 /* 
  * Called from nfsd_lookup and encode_dirent. Check if we have crossed 
  * a mount point.
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/7] nfsd: increase export interface version
  2009-12-02  0:39           ` [PATCH 6/7] nfsd: restrict filehandles accepted in V4ROOT case J. Bruce Fields
@ 2009-12-02  0:39             ` J. Bruce Fields
  2009-12-04 15:05             ` [PATCH 6/7] nfsd: restrict filehandles accepted in V4ROOT case Steve Dickson
  1 sibling, 0 replies; 10+ messages in thread
From: J. Bruce Fields @ 2009-12-02  0:39 UTC (permalink / raw)
  To: linux-nfs, nfsv4; +Cc: Steve Dickson, Steve Dickson, J. Bruce Fields

From: Steve Dickson <SteveD@redhat.com>

The V4ROOT flag is supposed to make it safe to export filesystems that
would not ordinarily have been exported, by severely restricting which
directories on the filesystem may be accessed.

Before exporting those filesystems, we must have a way for userspace
(mainly mountd) to know whether the kernel supports the new flag.  The
kernel has always silently ignored unknown export flags.

So, we bump the export version to signal mountd that the kernel has
pseudo root support.

Signed-Off-By: Steve Dickson <steved@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/nfsd/export.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 0bcd10a..39c22d9 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1522,7 +1522,7 @@ static int e_show(struct seq_file *m, void *p)
 	struct svc_export *exp = container_of(cp, struct svc_export, h);
 
 	if (p == SEQ_START_TOKEN) {
-		seq_puts(m, "# Version 1.1\n");
+		seq_puts(m, "# Version 1.2\n");
 		seq_puts(m, "# Path Client(Flags) # IPs\n");
 		return 0;
 	}
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 6/7] nfsd: restrict filehandles accepted in V4ROOT case
  2009-12-02  0:39           ` [PATCH 6/7] nfsd: restrict filehandles accepted in V4ROOT case J. Bruce Fields
  2009-12-02  0:39             ` [PATCH 7/7] nfsd: increase export interface version J. Bruce Fields
@ 2009-12-04 15:05             ` Steve Dickson
       [not found]               ` <4B192525.4050301-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
  1 sibling, 1 reply; 10+ messages in thread
From: Steve Dickson @ 2009-12-04 15:05 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: linux-nfs, nfsv4



On 12/01/2009 07:39 PM, J. Bruce Fields wrote:
> From: Steve Dickson <SteveD@redhat.com>
> 
> On V4ROOT exports, only accept filehandles that are the *root* of some
> export.  This allows mountd to allow or deny access to individual paths
> and symlinks on the pseudofilesystem.
> 
> Note that the checks in readdir and lookup are not enough, since a
> malicious host with access to the network could guess filehandles that
> they weren't able to obtain through lookup or readdir.
> 
> Signed-Off-By: Steve Dickson <steved@redhat.com>
> Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
> ---
>  fs/nfsd/nfsd.h  |    4 ++++
>  fs/nfsd/nfsfh.c |   35 +++++++++++++++++++++++++++++++++++
>  fs/nfsd/vfs.c   |    7 +------
>  3 files changed, 40 insertions(+), 6 deletions(-)
>  create mode 100644 fs/nfsd/nfsd.h
> 
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> new file mode 100644
> index 0000000..7a1ad80
> --- /dev/null
> +++ b/fs/nfsd/nfsd.h
> @@ -0,0 +1,4 @@
> +static inline int nfsd_v4client(struct svc_rqst *rq)
> +{
> +	return rq->rq_prog == NFS_PROGRAM && rq->rq_vers == 4;
> +}
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index a77efb8..9b902c0 100644
> --- a/fs/nfsd/nfsfh.c
> +++ b/fs/nfsd/nfsfh.c
> @@ -22,6 +22,7 @@
>  #include <linux/sunrpc/svc.h>
>  #include <linux/sunrpc/svcauth_gss.h>
>  #include <linux/nfsd/nfsd.h>
> +#include "nfsd.h"
>  #include "vfs.h"
>  #include "auth.h"
>  
> @@ -110,6 +111,36 @@ static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,
>  	return nfserrno(nfsd_setuser(rqstp, exp));
>  }
>  
> +static inline __be32 check_pseudo_root(struct svc_rqst *rqstp,
> +	struct dentry *dentry, struct svc_export *exp)
> +{
> +	if (!(exp->ex_flags & NFSEXP_V4ROOT))
> +		return nfs_ok;
> +	/*
> +	 * v2/v3 clients have no need for the V4ROOT export--they use
> +	 * the mount protocl instead; also, further V4ROOT checks may be
> +	 * in v4-specific code, in which case v2/v3 clients could bypass
> +	 * them.
> +	 */
> +	if (!nfsd_v4client(rqstp))
> +		return nfserr_stale;
> +	/*
> +	 * We're exposing only the directories and symlinks that have to be
> +	 * traversed on the way to real exports:
> +	 */
> +	if (unlikely(!S_ISDIR(dentry->d_inode->i_mode) &&
> +		     !S_ISLNK(dentry->d_inode->i_mode)))
> +		return nfserr_stale;
> +	/*
> +	 * A pseudoroot export gives permission to access only one
> +	 * single directory; the kernel has to make another upcall
> +	 * before granting access to anything else under it:
> +	 */
> +	if (unlikely(dentry->d_parent != exp->ex_path.dentry))
Remember this is wrong... it needs to be 
-	if (unlikely(dentry->d_parent != exp->ex_path.dentry))
+	if (unlikely(dentry != exp->ex_path.dentry))

steved.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 6/7] nfsd: restrict filehandles accepted in V4ROOT case
       [not found]               ` <4B192525.4050301-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
@ 2009-12-04 18:49                 ` J. Bruce Fields
  0 siblings, 0 replies; 10+ messages in thread
From: J. Bruce Fields @ 2009-12-04 18:49 UTC (permalink / raw)
  To: Steve Dickson; +Cc: linux-nfs, nfsv4

On Fri, Dec 04, 2009 at 10:05:09AM -0500, Steve Dickson wrote:
> 
> 
> On 12/01/2009 07:39 PM, J. Bruce Fields wrote:
> > +	/*
> > +	 * A pseudoroot export gives permission to access only one
> > +	 * single directory; the kernel has to make another upcall
> > +	 * before granting access to anything else under it:
> > +	 */
> > +	if (unlikely(dentry->d_parent != exp->ex_path.dentry))
> Remember this is wrong... it needs to be 
> -	if (unlikely(dentry->d_parent != exp->ex_path.dentry))
> +	if (unlikely(dentry != exp->ex_path.dentry))

Oops, thanks.  Looking back through the git reflogs....  It seems that I
had this fix in a separate patch, was rebasing the series and squashing
that patch in at the same time, and missed this chunk.  Fixed in my
local version.

--b.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-12-04 18:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-02  0:39 pseudoroot kernel patches J. Bruce Fields
2009-12-02  0:39 ` [PATCH 1/7] nfsd: introduce export flag for v4 pseudoroot J. Bruce Fields
2009-12-02  0:39   ` [PATCH 2/7] nfsd4: don't continue "under" mounts in V4ROOT case J. Bruce Fields
2009-12-02  0:39     ` [PATCH 3/7] nfsd: filter lookup results " J. Bruce Fields
2009-12-02  0:39       ` [PATCH 4/7] nfsd: special readdir exception for V4ROOT J. Bruce Fields
2009-12-02  0:39         ` [PATCH 5/7] nfsd: allow exports of symlinks J. Bruce Fields
2009-12-02  0:39           ` [PATCH 6/7] nfsd: restrict filehandles accepted in V4ROOT case J. Bruce Fields
2009-12-02  0:39             ` [PATCH 7/7] nfsd: increase export interface version J. Bruce Fields
2009-12-04 15:05             ` [PATCH 6/7] nfsd: restrict filehandles accepted in V4ROOT case Steve Dickson
     [not found]               ` <4B192525.4050301-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
2009-12-04 18:49                 ` J. Bruce Fields

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox