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.
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 <[email protected]>
---
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
From: Steve Dickson <[email protected]>
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 <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
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
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
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 <[email protected]>
---
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
We treat every object as a mountpoint and pretend it doesn't exist if
it isn't exported.
Signed-off-by: J. Bruce Fields <[email protected]>
---
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
From: Steve Dickson <[email protected]>
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 <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
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
From: Steve Dickson <[email protected]>
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 <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
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
On 12/01/2009 07:39 PM, J. Bruce Fields wrote:
> From: Steve Dickson <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: J. Bruce Fields <[email protected]>
> ---
> 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.
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.