2013-09-24 17:58:07

by Weston Andros Adamson

Subject: [PATCH] NFSv4.1: try SECINFO_NO_NAME flavs until one works

Call nfs4_lookup_root_sec for each flavor returned by SECINFO_NO_NAME until
one works.

One example of a situation this fixes:

- server configured for krb5
- server principal somehow gets deleted from KDC
- server still thinking krb is good, sends krb5 as first entry in
SECINFO_NO_NAME response
- client tries krb5, but this fails without even sending an RPC because
gssd's requests to the KDC can't find the server's principal

Signed-off-by: Weston Andros Adamson <[email protected]>
---
fs/nfs/nfs4proc.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)

This is version 2 of the patch.

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 989bb9d..a288f15 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7566,6 +7566,8 @@ nfs41_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle,
struct page *page;
rpc_authflavor_t flavor;
struct nfs4_secinfo_flavors *flavors;
+ struct nfs4_secinfo4 *secinfo;
+ int i;

page = alloc_page(GFP_KERNEL);
if (!page) {
@@ -7587,9 +7589,31 @@ nfs41_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle,
if (err)
goto out_freepage;

- flavor = nfs_find_best_sec(flavors);
- if (err == 0)
- err = nfs4_lookup_root_sec(server, fhandle, info, flavor);
+ for (i = 0; i < flavors->num_flavors; i++) {
+ secinfo = &flavors->flavors[i];
+
+ switch (secinfo->flavor) {
+ case RPC_AUTH_NULL:
+ case RPC_AUTH_UNIX:
+ case RPC_AUTH_GSS:
+ flavor = rpcauth_get_pseudoflavor(secinfo->flavor,
+ &secinfo->flavor_info);
+ break;
+ default:
+ flavor = RPC_AUTH_MAXFLAVOR;
+ break;
+ }
+
+ if (flavor != RPC_AUTH_MAXFLAVOR) {
+ err = nfs4_lookup_root_sec(server, fhandle,
+ info, flavor);
+ if (!err)
+ break;
+ }
+ }
+
+ if (flavor == RPC_AUTH_MAXFLAVOR)
+ err = -EPERM;

out_freepage:
put_page(page);
--
1.7.12.4 (Apple Git-37)

2013-09-24 18:38:06

by Adamson, Dros

Subject: Re: [PATCH] NFSv4.1: try SECINFO_NO_NAME flavs until one works

On Sep 24, 2013, at 2:31 PM, Anna Schumaker <[email protected]> wrote:

> Hi Dros,
>
>
> On Tue, Sep 24, 2013 at 1:58 PM, Weston Andros Adamson <[email protected]> wrote:
>>
>> Call nfs4_lookup_root_sec for each flavor returned by SECINFO_NO_NAME until
>> one works.
>>
>> One example of a situation this fixes:
>>
>> - server configured for krb5
>> - server principal somehow gets deleted from KDC
>> - server still thinking krb is good, sends krb5 as first entry in
>> SECINFO_NO_NAME response
>> - client tries krb5, but this fails without even sending an RPC because
>> gssd's requests to the KDC can't find the server's principal
>>
>> Signed-off-by: Weston Andros Adamson <[email protected]>
>> ---
>> fs/nfs/nfs4proc.c | 30 +++++++++++++++++++++++++++---
>> 1 file changed, 27 insertions(+), 3 deletions(-)
>>
>> This is version 2 of the patch.
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 989bb9d..a288f15 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -7566,6 +7566,8 @@ nfs41_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle,
>> struct page *page;
>> rpc_authflavor_t flavor;
>> struct nfs4_secinfo_flavors *flavors;
>> + struct nfs4_secinfo4 *secinfo;
>> + int i;
>>
>> page = alloc_page(GFP_KERNEL);
>> if (!page) {
>> @@ -7587,9 +7589,31 @@ nfs41_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle,
>> if (err)
>> goto out_freepage;
>>
>> - flavor = nfs_find_best_sec(flavors);
>> - if (err == 0)
>> - err = nfs4_lookup_root_sec(server, fhandle, info, flavor);
>> + for (i = 0; i < flavors->num_flavors; i++) {
>> + secinfo = &flavors->flavors[i];
>> +
>> + switch (secinfo->flavor) {
>> + case RPC_AUTH_NULL:
>> + case RPC_AUTH_UNIX:
>> + case RPC_AUTH_GSS:
>> + flavor = rpcauth_get_pseudoflavor(secinfo->flavor,
>> + &secinfo->flavor_info);
>> + break;
>> + default:
>> + flavor = RPC_AUTH_MAXFLAVOR;
>> + break;
>> + }
>> +
>> + if (flavor != RPC_AUTH_MAXFLAVOR) {
>> + err = nfs4_lookup_root_sec(server, fhandle,
>> + info, flavor);
>> + if (!err)
>> + break;
>
>
> I think we should only try the next sec flavor if
> nfs4_lookup_root_sec() returns -EACCESS, since this error means there
> was a problem with rpcauth_create() for the given flavor. Other
> errors should be passed on to the user, since they happen farther on
> in the mount process.
>
> Anna
>

Yeah, I thought about that, but it seemed to me that it'd be nice to try the next flavor no matter what the error was. The other option is the mount just failing. That said, I don't feel strongly either way.

Anyone else want to weigh in?

-dros

>>
>> + }
>> + }
>> +
>> + if (flavor == RPC_AUTH_MAXFLAVOR)
>> + err = -EPERM;
>>
>> out_freepage:
>> put_page(page);
>> --
>> 1.7.12.4 (Apple Git-37)
>>
> --
> 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

2013-09-24 18:32:10

by Anna Schumaker

Subject: Re: [PATCH] NFSv4.1: try SECINFO_NO_NAME flavs until one works

Hi Dros,


On Tue, Sep 24, 2013 at 1:58 PM, Weston Andros Adamson <[email protected]> wrote:
>
> Call nfs4_lookup_root_sec for each flavor returned by SECINFO_NO_NAME until
> one works.
>
> One example of a situation this fixes:
>
> - server configured for krb5
> - server principal somehow gets deleted from KDC
> - server still thinking krb is good, sends krb5 as first entry in
> SECINFO_NO_NAME response
> - client tries krb5, but this fails without even sending an RPC because
> gssd's requests to the KDC can't find the server's principal
>
> Signed-off-by: Weston Andros Adamson <[email protected]>
> ---
> fs/nfs/nfs4proc.c | 30 +++++++++++++++++++++++++++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> This is version 2 of the patch.
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 989bb9d..a288f15 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -7566,6 +7566,8 @@ nfs41_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle,
> struct page *page;
> rpc_authflavor_t flavor;
> struct nfs4_secinfo_flavors *flavors;
> + struct nfs4_secinfo4 *secinfo;
> + int i;
>
> page = alloc_page(GFP_KERNEL);
> if (!page) {
> @@ -7587,9 +7589,31 @@ nfs41_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle,
> if (err)
> goto out_freepage;
>
> - flavor = nfs_find_best_sec(flavors);
> - if (err == 0)
> - err = nfs4_lookup_root_sec(server, fhandle, info, flavor);
> + for (i = 0; i < flavors->num_flavors; i++) {
> + secinfo = &flavors->flavors[i];
> +
> + switch (secinfo->flavor) {
> + case RPC_AUTH_NULL:
> + case RPC_AUTH_UNIX:
> + case RPC_AUTH_GSS:
> + flavor = rpcauth_get_pseudoflavor(secinfo->flavor,
> + &secinfo->flavor_info);
> + break;
> + default:
> + flavor = RPC_AUTH_MAXFLAVOR;
> + break;
> + }
> +
> + if (flavor != RPC_AUTH_MAXFLAVOR) {
> + err = nfs4_lookup_root_sec(server, fhandle,
> + info, flavor);
> + if (!err)
> + break;


I think we should only try the next sec flavor if
nfs4_lookup_root_sec() returns -EACCESS, since this error means there
was a problem with rpcauth_create() for the given flavor. Other
errors should be passed on to the user, since they happen farther on
in the mount process.

Anna

>
> + }
> + }
> +
> + if (flavor == RPC_AUTH_MAXFLAVOR)
> + err = -EPERM;
>
> out_freepage:
> put_page(page);
> --
> 1.7.12.4 (Apple Git-37)
>