2023-04-17 00:40:45

by NeilBrown

Subject: [PATCH nfs-utils] mountd: don't advertise krb5 for v4root when not configured.

If /etc/krb5.keytab does not exist, then krb5 cannot work, so
advertising it as an option for v4root is pointless.
Since linux commit 676e4ebd5f2c ("NFSD: SECINFO doesn't handle
unsupported pseudoflavors correctly") this can result in an unhelpful
warning if the krb5 code is not built, or built as a module which is not
installed.

[ 161.668635] NFS: SECINFO: security flavor 390003 is not supported
[ 161.668655] NFS: SECINFO: security flavor 390004 is not supported
[ 161.668670] NFS: SECINFO: security flavor 390005 is not supported

So avoid advertising krb5 security options when krb5.keytab cannot be
found.

Link: https://lore.kernel.org/linux-nfs/[email protected]/
Signed-off-by: NeilBrown <[email protected]>
---
support/export/v4root.c | 2 ++
support/include/pseudoflavors.h | 1 +
support/nfs/exports.c | 14 +++++++-------
3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/support/export/v4root.c b/support/export/v4root.c
index fbb0ad5f5b81..3e049582d7c1 100644
--- a/support/export/v4root.c
+++ b/support/export/v4root.c
@@ -66,6 +66,8 @@ set_pseudofs_security(struct exportent *pseudo)

if (!flav->fnum)
continue;
+ if (flav->need_krb5 && !access("/etc/krb5.keytab", F_OK))
+ continue;

i = secinfo_addflavor(flav, pseudo);
new = &pseudo->e_secinfo[i];
diff --git a/support/include/pseudoflavors.h b/support/include/pseudoflavors.h
index deb052b130e6..1f16f3f796f3 100644
--- a/support/include/pseudoflavors.h
+++ b/support/include/pseudoflavors.h
@@ -8,6 +8,7 @@
struct flav_info {
char *flavour;
int fnum;
+ int need_krb5;
};

extern struct flav_info flav_map[];
diff --git a/support/nfs/exports.c b/support/nfs/exports.c
index 2c8f0752ad9d..010dfe423d6f 100644
--- a/support/nfs/exports.c
+++ b/support/nfs/exports.c
@@ -36,13 +36,13 @@
(NFSEXP_READONLY|NFSEXP_ROOTSQUASH|NFSEXP_GATHERED_WRITES|NFSEXP_NOSUBTREECHECK)

struct flav_info flav_map[] = {
- { "krb5", RPC_AUTH_GSS_KRB5 },
- { "krb5i", RPC_AUTH_GSS_KRB5I },
- { "krb5p", RPC_AUTH_GSS_KRB5P },
- { "unix", AUTH_UNIX },
- { "sys", AUTH_SYS },
- { "null", AUTH_NULL },
- { "none", AUTH_NONE },
+ { "krb5", RPC_AUTH_GSS_KRB5, 1},
+ { "krb5i", RPC_AUTH_GSS_KRB5I, 1},
+ { "krb5p", RPC_AUTH_GSS_KRB5P, 1},
+ { "unix", AUTH_UNIX, 0},
+ { "sys", AUTH_SYS, 0},
+ { "null", AUTH_NULL, 0},
+ { "none", AUTH_NONE, 0},
};

const int flav_map_size = sizeof(flav_map)/sizeof(flav_map[0]);
--
2.40.0

2023-04-17 02:07:10

by Wang Yugui

Subject: Re: [PATCH nfs-utils] mountd: don't advertise krb5 for v4root when not configured.

Hi,

>
> If /etc/krb5.keytab does not exist, then krb5 cannot work, so
> advertising it as an option for v4root is pointless.
> Since linux commit 676e4ebd5f2c ("NFSD: SECINFO doesn't handle
> unsupported pseudoflavors correctly") this can result in an unhelpful
> warning if the krb5 code is not built, or built as a module which is not
> installed.
>
> [ 161.668635] NFS: SECINFO: security flavor 390003 is not supported
> [ 161.668655] NFS: SECINFO: security flavor 390004 is not supported
> [ 161.668670] NFS: SECINFO: security flavor 390005 is not supported
>
> So avoid advertising krb5 security options when krb5.keytab cannot be
> found.
>
> Link: https://lore.kernel.org/linux-nfs/[email protected]/
> Signed-off-by: NeilBrown <[email protected]>
> ---
> support/export/v4root.c | 2 ++
> support/include/pseudoflavors.h | 1 +
> support/nfs/exports.c | 14 +++++++-------
> 3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/support/export/v4root.c b/support/export/v4root.c
> index fbb0ad5f5b81..3e049582d7c1 100644
> --- a/support/export/v4root.c
> +++ b/support/export/v4root.c
> @@ -66,6 +66,8 @@ set_pseudofs_security(struct exportent *pseudo)
>
> if (!flav->fnum)
> continue;
> + if (flav->need_krb5 && !access("/etc/krb5.keytab", F_OK))
> + continue;

Could we replace "/etc/krb5.keytab" with krb5_kt_default_name()?

Best Regards
Wang Yugui ([email protected])
2023/04/17

2023-04-17 02:25:00

by NeilBrown

Subject: Re: [PATCH nfs-utils] mountd: don't advertise krb5 for v4root when not configured.

On Mon, 17 Apr 2023, Wang Yugui wrote:
> Hi,
>
> >
> > If /etc/krb5.keytab does not exist, then krb5 cannot work, so
> > advertising it as an option for v4root is pointless.
> > Since linux commit 676e4ebd5f2c ("NFSD: SECINFO doesn't handle
> > unsupported pseudoflavors correctly") this can result in an unhelpful
> > warning if the krb5 code is not built, or built as a module which is not
> > installed.
> >
> > [ 161.668635] NFS: SECINFO: security flavor 390003 is not supported
> > [ 161.668655] NFS: SECINFO: security flavor 390004 is not supported
> > [ 161.668670] NFS: SECINFO: security flavor 390005 is not supported
> >
> > So avoid advertising krb5 security options when krb5.keytab cannot be
> > found.
> >
> > Link: https://lore.kernel.org/linux-nfs/[email protected]/
> > Signed-off-by: NeilBrown <[email protected]>
> > ---
> > support/export/v4root.c | 2 ++
> > support/include/pseudoflavors.h | 1 +
> > support/nfs/exports.c | 14 +++++++-------
> > 3 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/support/export/v4root.c b/support/export/v4root.c
> > index fbb0ad5f5b81..3e049582d7c1 100644
> > --- a/support/export/v4root.c
> > +++ b/support/export/v4root.c
> > @@ -66,6 +66,8 @@ set_pseudofs_security(struct exportent *pseudo)
> >
> > if (!flav->fnum)
> > continue;
> > + if (flav->need_krb5 && !access("/etc/krb5.keytab", F_OK))
> > + continue;
>
> Could we replace "/etc/krb5.keytab" with krb5_kt_default_name()?

Maybe? Why would we want to?

The presence of /etc/krb5.keytab is what we already use in a couple of
systemd unit files to determine if krb5 is configured. Why not just use
the same here?

NeilBrown

2023-04-17 02:34:42

by Wang Yugui

Subject: Re: [PATCH nfs-utils] mountd: don't advertise krb5 for v4root when not configured.

Hi,

> On Mon, 17 Apr 2023, Wang Yugui wrote:
> > Hi,
> >
> > >
> > > If /etc/krb5.keytab does not exist, then krb5 cannot work, so
> > > advertising it as an option for v4root is pointless.
> > > Since linux commit 676e4ebd5f2c ("NFSD: SECINFO doesn't handle
> > > unsupported pseudoflavors correctly") this can result in an unhelpful
> > > warning if the krb5 code is not built, or built as a module which is not
> > > installed.
> > >
> > > [ 161.668635] NFS: SECINFO: security flavor 390003 is not supported
> > > [ 161.668655] NFS: SECINFO: security flavor 390004 is not supported
> > > [ 161.668670] NFS: SECINFO: security flavor 390005 is not supported
> > >
> > > So avoid advertising krb5 security options when krb5.keytab cannot be
> > > found.
> > >
> > > Link: https://lore.kernel.org/linux-nfs/[email protected]/
> > > Signed-off-by: NeilBrown <[email protected]>
> > > ---
> > > support/export/v4root.c | 2 ++
> > > support/include/pseudoflavors.h | 1 +
> > > support/nfs/exports.c | 14 +++++++-------
> > > 3 files changed, 10 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/support/export/v4root.c b/support/export/v4root.c
> > > index fbb0ad5f5b81..3e049582d7c1 100644
> > > --- a/support/export/v4root.c
> > > +++ b/support/export/v4root.c
> > > @@ -66,6 +66,8 @@ set_pseudofs_security(struct exportent *pseudo)
> > >
> > > if (!flav->fnum)
> > > continue;
> > > + if (flav->need_krb5 && !access("/etc/krb5.keytab", F_OK))
> > > + continue;
> >
> > Could we replace "/etc/krb5.keytab" with krb5_kt_default_name()?
>
> Maybe? Why would we want to?
>
> The presence of /etc/krb5.keytab is what we already use in a couple of
> systemd unit files to determine if krb5 is configured. Why not just use
> the same here?

OK to just same as other files.

Best Regards
Wang Yugui ([email protected])
2023/04/17

2023-04-19 00:05:47

by NeilBrown

Subject: Re: [PATCH nfs-utils] mountd: don't advertise krb5 for v4root when not configured.

On Mon, 17 Apr 2023, NeilBrown wrote:
> If /etc/krb5.keytab does not exist, then krb5 cannot work, so
> advertising it as an option for v4root is pointless.
> Since linux commit 676e4ebd5f2c ("NFSD: SECINFO doesn't handle
> unsupported pseudoflavors correctly") this can result in an unhelpful
> warning if the krb5 code is not built, or built as a module which is not
> installed.
>
> [ 161.668635] NFS: SECINFO: security flavor 390003 is not supported
> [ 161.668655] NFS: SECINFO: security flavor 390004 is not supported
> [ 161.668670] NFS: SECINFO: security flavor 390005 is not supported
>
> So avoid advertising krb5 security options when krb5.keytab cannot be
> found.
>
> Link: https://lore.kernel.org/linux-nfs/[email protected]/
> Signed-off-by: NeilBrown <[email protected]>
> ---
> support/export/v4root.c | 2 ++
> support/include/pseudoflavors.h | 1 +
> support/nfs/exports.c | 14 +++++++-------
> 3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/support/export/v4root.c b/support/export/v4root.c
> index fbb0ad5f5b81..3e049582d7c1 100644
> --- a/support/export/v4root.c
> +++ b/support/export/v4root.c
> @@ -66,6 +66,8 @@ set_pseudofs_security(struct exportent *pseudo)
>
> if (!flav->fnum)
> continue;
> + if (flav->need_krb5 && !access("/etc/krb5.keytab", F_OK))
> + continue;

This is "obviously" wrong - thanks to Petr for testing more thoroughly
than I did (hint: you need to "rmmod nfsd" or reboot between tests, as
the messages are only reported once).
access() returns 0 on success, negative on failure. It doesn't return
bool like the above suggests.

I will repost with a fixed version.

NeilBrown


>
> i = secinfo_addflavor(flav, pseudo);
> new = &pseudo->e_secinfo[i];
> diff --git a/support/include/pseudoflavors.h b/support/include/pseudoflavors.h
> index deb052b130e6..1f16f3f796f3 100644
> --- a/support/include/pseudoflavors.h
> +++ b/support/include/pseudoflavors.h
> @@ -8,6 +8,7 @@
> struct flav_info {
> char *flavour;
> int fnum;
> + int need_krb5;
> };
>
> extern struct flav_info flav_map[];
> diff --git a/support/nfs/exports.c b/support/nfs/exports.c
> index 2c8f0752ad9d..010dfe423d6f 100644
> --- a/support/nfs/exports.c
> +++ b/support/nfs/exports.c
> @@ -36,13 +36,13 @@
> (NFSEXP_READONLY|NFSEXP_ROOTSQUASH|NFSEXP_GATHERED_WRITES|NFSEXP_NOSUBTREECHECK)
>
> struct flav_info flav_map[] = {
> - { "krb5", RPC_AUTH_GSS_KRB5 },
> - { "krb5i", RPC_AUTH_GSS_KRB5I },
> - { "krb5p", RPC_AUTH_GSS_KRB5P },
> - { "unix", AUTH_UNIX },
> - { "sys", AUTH_SYS },
> - { "null", AUTH_NULL },
> - { "none", AUTH_NONE },
> + { "krb5", RPC_AUTH_GSS_KRB5, 1},
> + { "krb5i", RPC_AUTH_GSS_KRB5I, 1},
> + { "krb5p", RPC_AUTH_GSS_KRB5P, 1},
> + { "unix", AUTH_UNIX, 0},
> + { "sys", AUTH_SYS, 0},
> + { "null", AUTH_NULL, 0},
> + { "none", AUTH_NONE, 0},
> };
>
> const int flav_map_size = sizeof(flav_map)/sizeof(flav_map[0]);
> --
> 2.40.0
>
>