2014-01-24 12:46:14

by Jeff Layton

[permalink] [raw]
Subject: [PATCH v2] nfs: don't attempt auth methods that require upcall unless we know that gssd is running

Currently, if the server lists krb5 as an allowed auth method, but gssd isn't
running, you'll get the infamous "AUTH_GSS upcall failed" message, even if
you didn't request sec=krb5.

This is because nfs4_find_root_sec() establishes a default list of auth
methods to try when the admin didn't supply one, and that list contains
AUTH_GSS methods first. Skip those methods if gssd isn't running since
they won't succeed anyway.

Cc: Weston Andros Adamson <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfs/nfs4proc.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 15052b8..937a05a 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2900,6 +2900,14 @@ static int nfs4_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle,
} else {
/* no flavors specified by user, try default list */
for (i = 0; i < ARRAY_SIZE(flav_array); i++) {
+ /*
+ * Don't attempt to upcall with the default list
+ * unless we know that gssd is running.
+ */
+ if (flav_array[i] > RPC_AUTH_MAXFLAVOR &&
+ !gssd_running(server->nfs_client->cl_net))
+ continue;
+
status = nfs4_lookup_root_sec(server, fhandle, info,
flav_array[i]);
if (status == -NFS4ERR_WRONGSEC || status == -EACCES)
--
1.8.5.3



2014-01-24 17:02:46

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH v2] nfs: don't attempt auth methods that require upcall unless we know that gssd is running


On Jan 24, 2014, at 5:46, Jeff Layton <[email protected]> wrote:

> Currently, if the server lists krb5 as an allowed auth method, but gssd isn't
> running, you'll get the infamous "AUTH_GSS upcall failed" message, even if
> you didn't request sec=krb5.
>
> This is because nfs4_find_root_sec() establishes a default list of auth
> methods to try when the admin didn't supply one, and that list contains
> AUTH_GSS methods first. Skip those methods if gssd isn't running since
> they won't succeed anyway.
>
> Cc: Weston Andros Adamson <[email protected]>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/nfs/nfs4proc.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 15052b8..937a05a 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2900,6 +2900,14 @@ static int nfs4_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle,
> } else {
> /* no flavors specified by user, try default list */
> for (i = 0; i < ARRAY_SIZE(flav_array); i++) {
> + /*
> + * Don't attempt to upcall with the default list
> + * unless we know that gssd is running.
> + */
> + if (flav_array[i] > RPC_AUTH_MAXFLAVOR &&
> + !gssd_running(server->nfs_client->cl_net))
> + continue;

Big effing NACK.

This is exactly the kind of layering violation creep whereby the NFS layer ends up tracking more and more of the implementation details in the RPC authentication which made me hesitant about allowing gssd_running to be exported in the first place.

--
Trond Myklebust
Linux NFS client maintainer


2014-01-24 18:54:37

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH v2] nfs: don't attempt auth methods that require upcall unless we know that gssd is running

On Fri, 24 Jan 2014 10:02:38 -0700
Trond Myklebust <[email protected]> wrote:

>
> On Jan 24, 2014, at 5:46, Jeff Layton <[email protected]> wrote:
>
> > Currently, if the server lists krb5 as an allowed auth method, but gssd isn't
> > running, you'll get the infamous "AUTH_GSS upcall failed" message, even if
> > you didn't request sec=krb5.
> >
> > This is because nfs4_find_root_sec() establishes a default list of auth
> > methods to try when the admin didn't supply one, and that list contains
> > AUTH_GSS methods first. Skip those methods if gssd isn't running since
> > they won't succeed anyway.
> >
> > Cc: Weston Andros Adamson <[email protected]>
> > Signed-off-by: Jeff Layton <[email protected]>
> > ---
> > fs/nfs/nfs4proc.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 15052b8..937a05a 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -2900,6 +2900,14 @@ static int nfs4_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle,
> > } else {
> > /* no flavors specified by user, try default list */
> > for (i = 0; i < ARRAY_SIZE(flav_array); i++) {
> > + /*
> > + * Don't attempt to upcall with the default list
> > + * unless we know that gssd is running.
> > + */
> > + if (flav_array[i] > RPC_AUTH_MAXFLAVOR &&
> > + !gssd_running(server->nfs_client->cl_net))
> > + continue;
>
> Big effing NACK.
>
> This is exactly the kind of layering violation creep whereby the NFS layer ends up tracking more and more of the implementation details in the RPC authentication which made me hesitant about allowing gssd_running to be exported in the first place.
>

I'm not happy with the layering violations either. In this case the
problem is not serious -- just a spurious error message, but these sorts
of error messages tend to drive admins mad and cause them to call their
support vendors. Having to run gssd just to get rid of an error message
seems rather lame.

I guess the only semi-clean way to clean up the layering violation is
to somehow pass a "user intent" flag down to the lower layers. Then if
gssd isn't running, we'd just suppress the warning message. That's an
awful lot of code churn though, since the place where the log messages
occur is currently a long way away from the place where we actually
know what the intent was.

--
Jeff Layton <[email protected]>

2014-01-24 15:09:18

by Weston Andros Adamson

[permalink] [raw]
Subject: Re: [PATCH v2] nfs: don't attempt auth methods that require upcall unless we know that gssd is running

Thanks Jeff,

This makes sense to me.

-dros


On Jan 24, 2014, at 7:46 AM, Jeff Layton <[email protected]> wrote:

> Currently, if the server lists krb5 as an allowed auth method, but gssd isn't
> running, you'll get the infamous "AUTH_GSS upcall failed" message, even if
> you didn't request sec=krb5.
>
> This is because nfs4_find_root_sec() establishes a default list of auth
> methods to try when the admin didn't supply one, and that list contains
> AUTH_GSS methods first. Skip those methods if gssd isn't running since
> they won't succeed anyway.
>
> Cc: Weston Andros Adamson <[email protected]>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/nfs/nfs4proc.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 15052b8..937a05a 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2900,6 +2900,14 @@ static int nfs4_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle,
> } else {
> /* no flavors specified by user, try default list */
> for (i = 0; i < ARRAY_SIZE(flav_array); i++) {
> + /*
> + * Don't attempt to upcall with the default list
> + * unless we know that gssd is running.
> + */
> + if (flav_array[i] > RPC_AUTH_MAXFLAVOR &&
> + !gssd_running(server->nfs_client->cl_net))
> + continue;
> +
> status = nfs4_lookup_root_sec(server, fhandle, info,
> flav_array[i]);
> if (status == -NFS4ERR_WRONGSEC || status == -EACCES)
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html