2020-12-02 22:58:02

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 1/2] mountd: allow high ports on all pseudofs exports

From: "J. Bruce Fields" <[email protected]>

We originally tried to grant permissions on the v4 pseudoroot filesystem
that were the absolute minimum required for a client to reach a given
export. This turns out to be complicated, and we've never gotten it
quite right. Also, the tradition from the MNT protocol was to allow
anyone to browse the list of exports.

So, do as we already did with security flavors and just allow clients
from high ports to access the whole pseudofilesystem.

Signed-off-by: J. Bruce Fields <[email protected]>
---
utils/mountd/v4root.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/utils/mountd/v4root.c b/utils/mountd/v4root.c
index a9ea167a07e0..2ac4e87898c0 100644
--- a/utils/mountd/v4root.c
+++ b/utils/mountd/v4root.c
@@ -36,7 +36,7 @@ static nfs_export pseudo_root = {
.e_path = "/",
.e_flags = NFSEXP_READONLY | NFSEXP_ROOTSQUASH
| NFSEXP_NOSUBTREECHECK | NFSEXP_FSID
- | NFSEXP_V4ROOT,
+ | NFSEXP_V4ROOT | NFSEXP_INSECURE_PORT,
.e_anonuid = 65534,
.e_anongid = 65534,
.e_squids = NULL,
@@ -60,8 +60,6 @@ set_pseudofs_security(struct exportent *pseudo, int flags)
struct flav_info *flav;
int i;

- if (flags & NFSEXP_INSECURE_PORT)
- pseudo->e_flags |= NFSEXP_INSECURE_PORT;
if ((flags & NFSEXP_ROOTSQUASH) == 0)
pseudo->e_flags &= ~NFSEXP_ROOTSQUASH;
for (flav = flav_map; flav < flav_map + flav_map_size; flav++) {
@@ -70,8 +68,7 @@ set_pseudofs_security(struct exportent *pseudo, int flags)
i = secinfo_addflavor(flav, pseudo);
new = &pseudo->e_secinfo[i];

- if (flags & NFSEXP_INSECURE_PORT)
- new->flags |= NFSEXP_INSECURE_PORT;
+ new->flags |= NFSEXP_INSECURE_PORT;
}
}

--
2.28.0


2020-12-02 22:58:52

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 2/2] mountd: always root squash on the pseudofs

From: "J. Bruce Fields" <[email protected]>

As with security flavors and "secure" ports, we tried to code this so
that pseudofs directories would inherit root squashing from their
children, but it doesn't really work as coded and I'm not sure it's
useful.

Just root squash always. If it turns out somebody's exporting
directories that are only readable by root, I guess we can try to do
something else here, but frankly that sounds like a pretty weird
configuration.

Signed-off-by: J. Bruce Fields <[email protected]>
---
utils/mountd/v4root.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/utils/mountd/v4root.c b/utils/mountd/v4root.c
index 2ac4e87898c0..36543401f296 100644
--- a/utils/mountd/v4root.c
+++ b/utils/mountd/v4root.c
@@ -60,8 +60,6 @@ set_pseudofs_security(struct exportent *pseudo, int flags)
struct flav_info *flav;
int i;

- if ((flags & NFSEXP_ROOTSQUASH) == 0)
- pseudo->e_flags &= ~NFSEXP_ROOTSQUASH;
for (flav = flav_map; flav < flav_map + flav_map_size; flav++) {
struct sec_entry *new;

--
2.28.0

2020-12-02 23:05:10

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 1/2] mountd: allow high ports on all pseudofs exports

On Wed, Dec 02, 2020 at 05:56:43PM -0500, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <[email protected]>
>
> We originally tried to grant permissions on the v4 pseudoroot filesystem
> that were the absolute minimum required for a client to reach a given
> export. This turns out to be complicated, and we've never gotten it
> quite right. Also, the tradition from the MNT protocol was to allow
> anyone to browse the list of exports.
>
> So, do as we already did with security flavors and just allow clients
> from high ports to access the whole pseudofilesystem.

Oh, except then we may as well also remove this "flags" parameter.

--b.

diff --git a/utils/mountd/v4root.c b/utils/mountd/v4root.c
index 36543401f296..f6eb126660f3 100644
--- a/utils/mountd/v4root.c
+++ b/utils/mountd/v4root.c
@@ -55,7 +55,7 @@ static nfs_export pseudo_root = {
};

static void
-set_pseudofs_security(struct exportent *pseudo, int flags)
+set_pseudofs_security(struct exportent *pseudo)
{
struct flav_info *flav;
int i;
@@ -85,7 +85,7 @@ v4root_create(char *path, nfs_export *export)
strncpy(eep.e_path, path, sizeof(eep.e_path)-1);
if (strcmp(path, "/") != 0)
eep.e_flags &= ~NFSEXP_FSID;
- set_pseudofs_security(&eep, curexp->e_flags);
+ set_pseudofs_security(&eep);
exp = export_create(&eep, 0);
if (exp == NULL)
return NULL;
@@ -133,7 +133,7 @@ pseudofs_update(char *hostname, char *path, nfs_export *source)
return 0;
}
/* Update an existing V4ROOT export: */
- set_pseudofs_security(&exp->m_export, source->m_export.e_flags);
+ set_pseudofs_security(&exp->m_export);
return 0;
}

2020-12-03 00:57:04

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 2/2] mountd: always root squash on the pseudofs

On Wed, 2020-12-02 at 17:56 -0500, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <[email protected]>
>
> As with security flavors and "secure" ports, we tried to code this so
> that pseudofs directories would inherit root squashing from their
> children, but it doesn't really work as coded and I'm not sure it's
> useful.
>
> Just root squash always.  If it turns out somebody's exporting
> directories that are only readable by root, I guess we can try to do
> something else here, but frankly that sounds like a pretty weird
> configuration.
>
> Signed-off-by: J. Bruce Fields <[email protected]>
> ---
>  utils/mountd/v4root.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/utils/mountd/v4root.c b/utils/mountd/v4root.c
> index 2ac4e87898c0..36543401f296 100644
> --- a/utils/mountd/v4root.c
> +++ b/utils/mountd/v4root.c
> @@ -60,8 +60,6 @@ set_pseudofs_security(struct exportent *pseudo, int
> flags)
>         struct flav_info *flav;
>         int i;
>  
> -       if ((flags & NFSEXP_ROOTSQUASH) == 0)
> -               pseudo->e_flags &= ~NFSEXP_ROOTSQUASH;
>         for (flav = flav_map; flav < flav_map + flav_map_size;
> flav++) {
>                 struct sec_entry *new;
>  

Hmm... What is the harm in allowing root to be unsquashed here? Isn't
this really all about respecting lookup permissions, or could a user
actually modify something in the pseudofs? If the latter, then that
sounds like a bug (the pseudofs should always be read-only).

The consequence of not being able to look up a directory in the
pseudofs is that the NFSv4 client will be completely unable to mount
that subtree, so squashing root could make a major difference.

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]


2020-12-03 01:09:20

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 2/2] mountd: always root squash on the pseudofs

On Thu, Dec 03, 2020 at 12:54:53AM +0000, Trond Myklebust wrote:
> On Wed, 2020-12-02 at 17:56 -0500, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <[email protected]>
> >
> > As with security flavors and "secure" ports, we tried to code this so
> > that pseudofs directories would inherit root squashing from their
> > children, but it doesn't really work as coded and I'm not sure it's
> > useful.
> >
> > Just root squash always.? If it turns out somebody's exporting
> > directories that are only readable by root, I guess we can try to do
> > something else here, but frankly that sounds like a pretty weird
> > configuration.
> >
> > Signed-off-by: J. Bruce Fields <[email protected]>
> > ---
> > ?utils/mountd/v4root.c | 2 --
> > ?1 file changed, 2 deletions(-)
> >
> > diff --git a/utils/mountd/v4root.c b/utils/mountd/v4root.c
> > index 2ac4e87898c0..36543401f296 100644
> > --- a/utils/mountd/v4root.c
> > +++ b/utils/mountd/v4root.c
> > @@ -60,8 +60,6 @@ set_pseudofs_security(struct exportent *pseudo, int
> > flags)
> > ????????struct flav_info *flav;
> > ????????int i;
> > ?
> > -???????if ((flags & NFSEXP_ROOTSQUASH) == 0)
> > -???????????????pseudo->e_flags &= ~NFSEXP_ROOTSQUASH;
> > ????????for (flav = flav_map; flav < flav_map + flav_map_size;
> > flav++) {
> > ????????????????struct sec_entry *new;
> > ?
>
> Hmm... What is the harm in allowing root to be unsquashed here? Isn't
> this really all about respecting lookup permissions, or could a user
> actually modify something in the pseudofs? If the latter, then that
> sounds like a bug (the pseudofs should always be read-only).

Yeah, it should only be read-only.

> The consequence of not being able to look up a directory in the
> pseudofs is that the NFSv4 client will be completely unable to mount
> that subtree, so squashing root could make a major difference.

Fair enough, I'll resend.

--b.

2020-12-03 01:17:16

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 1/2] mountd: allow high ports on all pseudofs exports

From: "J. Bruce Fields" <[email protected]>

We originally tried to grant permissions on the v4 pseudoroot filesystem
that were the absolute minimum required for a client to reach a given
export. This turns out to be complicated, and we've never gotten it
quite right. Also, the tradition from the MNT protocol was to allow
anyone to browse the list of exports.

So, do as we already did with security flavors and just allow clients
from high ports to access the whole pseudofilesystem.

Signed-off-by: J. Bruce Fields <[email protected]>
---
utils/mountd/v4root.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/utils/mountd/v4root.c b/utils/mountd/v4root.c
index a9ea167a07e0..39dd87a94e59 100644
--- a/utils/mountd/v4root.c
+++ b/utils/mountd/v4root.c
@@ -36,7 +36,7 @@ static nfs_export pseudo_root = {
.e_path = "/",
.e_flags = NFSEXP_READONLY | NFSEXP_ROOTSQUASH
| NFSEXP_NOSUBTREECHECK | NFSEXP_FSID
- | NFSEXP_V4ROOT,
+ | NFSEXP_V4ROOT | NFSEXP_INSECURE_PORT,
.e_anonuid = 65534,
.e_anongid = 65534,
.e_squids = NULL,
@@ -55,13 +55,11 @@ static nfs_export pseudo_root = {
};

static void
-set_pseudofs_security(struct exportent *pseudo, int flags)
+set_pseudofs_security(struct exportent *pseudo)
{
struct flav_info *flav;
int i;

- if (flags & NFSEXP_INSECURE_PORT)
- pseudo->e_flags |= NFSEXP_INSECURE_PORT;
if ((flags & NFSEXP_ROOTSQUASH) == 0)
pseudo->e_flags &= ~NFSEXP_ROOTSQUASH;
for (flav = flav_map; flav < flav_map + flav_map_size; flav++) {
@@ -70,8 +68,7 @@ set_pseudofs_security(struct exportent *pseudo, int flags)
i = secinfo_addflavor(flav, pseudo);
new = &pseudo->e_secinfo[i];

- if (flags & NFSEXP_INSECURE_PORT)
- new->flags |= NFSEXP_INSECURE_PORT;
+ new->flags |= NFSEXP_INSECURE_PORT;
}
}

@@ -90,7 +87,7 @@ v4root_create(char *path, nfs_export *export)
strncpy(eep.e_path, path, sizeof(eep.e_path)-1);
if (strcmp(path, "/") != 0)
eep.e_flags &= ~NFSEXP_FSID;
- set_pseudofs_security(&eep, curexp->e_flags);
+ set_pseudofs_security(&eep);
exp = export_create(&eep, 0);
if (exp == NULL)
return NULL;
@@ -138,7 +135,7 @@ pseudofs_update(char *hostname, char *path, nfs_export *source)
return 0;
}
/* Update an existing V4ROOT export: */
- set_pseudofs_security(&exp->m_export, source->m_export.e_flags);
+ set_pseudofs_security(&exp->m_export);
return 0;
}

--
2.28.0