All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] NFS: Fix SECINFO processing regression
@ 2014-06-09 19:33 andros
  2014-06-09 19:33 ` [PATCH 1/3] NFS check the return of nfs4_negotiate_security in nfs4_submount andros
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: andros @ 2014-06-09 19:33 UTC (permalink / raw)
  To: trond.myklebust; +Cc: linux-nfs, Andy Adamson

From: Andy Adamson <andros@netapp.com>

Iterate over multiple flavors returned by SECINFO. If RPC_AUTH_GSS is in the
secinfo list, fully test (get and auth, credential, and gss_context)

Fix some error paths.

Testing: Minimal testing of the nfs_test_gss_flavor. Will run more tests after
the June 2014 bakeathon

Note: Can supply a patch for SECINFO_NO_name to use nfs4_negotiate_security.

Andy Adamson (3):
  NFS check the return of nfs4_negotiate_security in nfs4_submount
  NFS Return -EPERM if no supported or matching SECINFO flavor
  NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support

 fs/nfs/nfs4namespace.c | 66 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 55 insertions(+), 11 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] NFS check the return of nfs4_negotiate_security in nfs4_submount
  2014-06-09 19:33 [PATCH 0/3] NFS: Fix SECINFO processing regression andros
@ 2014-06-09 19:33 ` andros
  2014-06-09 19:33 ` [PATCH 2/3] NFS Return -EPERM if no supported or matching SECINFO flavor andros
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: andros @ 2014-06-09 19:33 UTC (permalink / raw)
  To: trond.myklebust; +Cc: linux-nfs, Andy Adamson

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4namespace.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 3d5dbf8..1b86fef 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -399,8 +399,11 @@ struct vfsmount *nfs4_submount(struct nfs_server *server, struct dentry *dentry,
 		flavor = client->cl_auth->au_flavor;
 	else {
 		rpc_authflavor_t new = nfs4_negotiate_security(dir, name);
-		if ((int)new >= 0)
-			flavor = new;
+		if ((int)new < 0) {
+			mnt = ERR_PTR((int)new);
+			goto out;
+		}
+		flavor = new;
 	}
 	mnt = nfs_do_submount(dentry, fh, fattr, flavor);
 out:
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] NFS Return -EPERM if no supported or matching SECINFO flavor
  2014-06-09 19:33 [PATCH 0/3] NFS: Fix SECINFO processing regression andros
  2014-06-09 19:33 ` [PATCH 1/3] NFS check the return of nfs4_negotiate_security in nfs4_submount andros
@ 2014-06-09 19:33 ` andros
  2014-06-09 19:33 ` [PATCH 3/3] NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support andros
  2014-06-10 15:04 ` [PATCH 0/3] NFS: Fix SECINFO processing regression Steve Dickson
  3 siblings, 0 replies; 9+ messages in thread
From: andros @ 2014-06-09 19:33 UTC (permalink / raw)
  To: trond.myklebust; +Cc: linux-nfs, Andy Adamson

From: Andy Adamson <andros@netapp.com>

Do not return RPC_AUTH_UNIX if SEINFO reply tests fail. This
prevents an infinite loop of NFS4ERR_WRONGSEC for non RPC_AUTH_UNIX mounts.

Without this patch, a mount with no sec= option to a server
that does not include RPC_AUTH_UNIX in the
SECINFO return can be presented with an attemtp to use RPC_AUTH_UNIX
which will result in an NFS4ERR_WRONG_SEC which will prompt the SECINFO
call which will again try RPC_AUTH_UNIX....

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4namespace.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 1b86fef..fd4dcb6 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -140,10 +140,11 @@ static size_t nfs_parse_server_name(char *string, size_t len,
  * @flavors: List of security tuples returned by SECINFO procedure
  *
  * Return the pseudoflavor of the first security mechanism in
- * "flavors" that is locally supported.  Return RPC_AUTH_UNIX if
- * no matching flavor is found in the array.  The "flavors" array
+ * "flavors" that is locally supported. The "flavors" array
  * is searched in the order returned from the server, per RFC 3530
  * recommendation.
+ *
+ * Return -EPERM if no matching flavor is found in the array.
  */
 static rpc_authflavor_t nfs_find_best_sec(struct nfs_server *server,
 					  struct nfs4_secinfo_flavors *flavors)
@@ -170,11 +171,7 @@ static rpc_authflavor_t nfs_find_best_sec(struct nfs_server *server,
 		}
 	}
 
-	/* if there were any sec= options then nothing matched */
-	if (server->auth_info.flavor_len > 0)
-		return -EPERM;
-
-	return RPC_AUTH_UNIX;
+	return -EPERM;
 }
 
 static rpc_authflavor_t nfs4_negotiate_security(struct inode *inode, struct qstr *name)
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support
  2014-06-09 19:33 [PATCH 0/3] NFS: Fix SECINFO processing regression andros
  2014-06-09 19:33 ` [PATCH 1/3] NFS check the return of nfs4_negotiate_security in nfs4_submount andros
  2014-06-09 19:33 ` [PATCH 2/3] NFS Return -EPERM if no supported or matching SECINFO flavor andros
@ 2014-06-09 19:33 ` andros
  2014-06-10 16:21   ` Trond Myklebust
  2014-06-10 15:04 ` [PATCH 0/3] NFS: Fix SECINFO processing regression Steve Dickson
  3 siblings, 1 reply; 9+ messages in thread
From: andros @ 2014-06-09 19:33 UTC (permalink / raw)
  To: trond.myklebust; +Cc: linux-nfs, Andy Adamson

From: Andy Adamson <andros@netapp.com>

The current code returns an RPC_AUTH_GSS pseudoflavor without testing to see
if it is configured properly. If an RPC_AUTH_GSS pseudoflavor fails then the
next SECINFO flavor should be tried.

Create an rpc_auth, rpc_cred, and initialize the cred (e.g. get a GSS Context)
using the short-lived SECINFO rpc client to test if the use of the RPC_AUTH_GSS
pseudoflavor succeeds.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4namespace.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index fd4dcb6..e0a5491 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -135,6 +135,39 @@ static size_t nfs_parse_server_name(char *string, size_t len,
 }
 
 /**
+ * nfs_test_gss - Test client support of pseudoflavor
+ * @server: NFS server struct
+ * @flavor: RPC_AUTH_GSS pseudoflavor
+ */
+
+static int nfs_test_gss_flavor(struct nfs_server *server,
+			       rpc_authflavor_t pseudoflavor)
+{
+	struct rpc_auth_create_args auth_args = {
+		.pseudoflavor = pseudoflavor,
+	};
+	struct rpc_auth *auth;
+	struct rpc_cred *rcred;
+	const struct cred *cred = current_cred();
+	struct auth_cred acred = {
+		.uid = cred->fsuid,
+		.gid = cred->fsgid,
+		.group_info = get_group_info(((struct cred *)cred)->group_info),
+	};
+
+	auth = rpcauth_create(&auth_args, server->client);
+	if (IS_ERR(auth))
+		return -EACCES;
+
+	/* This will call cr_init to create a gss context */
+	rcred = rpcauth_lookup_credcache(auth, &acred, 0);
+	if (IS_ERR(cred))
+		return -EACCES;
+
+	return 0;
+}
+
+/**
  * nfs_find_best_sec - Find a security mechanism supported locally
  * @server: NFS server struct
  * @flavors: List of security tuples returned by SECINFO procedure
@@ -152,21 +185,32 @@ static rpc_authflavor_t nfs_find_best_sec(struct nfs_server *server,
 	rpc_authflavor_t pseudoflavor;
 	struct nfs4_secinfo4 *secinfo;
 	unsigned int i;
+	int err = 0;
 
 	for (i = 0; i < flavors->num_flavors; i++) {
+		bool gss = false;
+
 		secinfo = &flavors->flavors[i];
 
 		switch (secinfo->flavor) {
+		case RPC_AUTH_GSS:
+			gss = true;
 		case RPC_AUTH_NULL:
 		case RPC_AUTH_UNIX:
-		case RPC_AUTH_GSS:
 			pseudoflavor = rpcauth_get_pseudoflavor(secinfo->flavor,
 							&secinfo->flavor_info);
 			/* make sure pseudoflavor matches sec= mount opt */
 			if (pseudoflavor != RPC_AUTH_MAXFLAVOR &&
 			    nfs_auth_info_match(&server->auth_info,
-						pseudoflavor))
+						pseudoflavor)) {
+				if (gss) {
+					err = nfs_test_gss_flavor(server,
+								  pseudoflavor);
+					if (err) /* try the next flavor */
+						continue;
+				}
 				return pseudoflavor;
+			}
 			break;
 		}
 	}
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] NFS: Fix SECINFO processing regression
  2014-06-09 19:33 [PATCH 0/3] NFS: Fix SECINFO processing regression andros
                   ` (2 preceding siblings ...)
  2014-06-09 19:33 ` [PATCH 3/3] NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support andros
@ 2014-06-10 15:04 ` Steve Dickson
  3 siblings, 0 replies; 9+ messages in thread
From: Steve Dickson @ 2014-06-10 15:04 UTC (permalink / raw)
  To: andros, trond.myklebust; +Cc: linux-nfs



On 06/09/2014 03:33 PM, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> Iterate over multiple flavors returned by SECINFO. If RPC_AUTH_GSS is in the
> secinfo list, fully test (get and auth, credential, and gss_context)
> 
> Fix some error paths.
> 
> Testing: Minimal testing of the nfs_test_gss_flavor. Will run more tests after
> the June 2014 bakeathon
> 
> Note: Can supply a patch for SECINFO_NO_name to use nfs4_negotiate_security.
> 
> Andy Adamson (3):
>   NFS check the return of nfs4_negotiate_security in nfs4_submount
>   NFS Return -EPERM if no supported or matching SECINFO flavor
>   NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support
> 
>  fs/nfs/nfs4namespace.c | 66 +++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 55 insertions(+), 11 deletions(-)
> 
Tested-By: Steve Dickson <steved@redhat.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support
  2014-06-09 19:33 ` [PATCH 3/3] NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support andros
@ 2014-06-10 16:21   ` Trond Myklebust
  2014-06-10 18:38     ` Adamson, Andy
  0 siblings, 1 reply; 9+ messages in thread
From: Trond Myklebust @ 2014-06-10 16:21 UTC (permalink / raw)
  To: andros; +Cc: linux-nfs

On Mon, 2014-06-09 at 15:33 -0400, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> The current code returns an RPC_AUTH_GSS pseudoflavor without testing to see
> if it is configured properly. If an RPC_AUTH_GSS pseudoflavor fails then the
> next SECINFO flavor should be tried.
> 
> Create an rpc_auth, rpc_cred, and initialize the cred (e.g. get a GSS Context)
> using the short-lived SECINFO rpc client to test if the use of the RPC_AUTH_GSS
> pseudoflavor succeeds.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfs/nfs4namespace.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
> index fd4dcb6..e0a5491 100644
> --- a/fs/nfs/nfs4namespace.c
> +++ b/fs/nfs/nfs4namespace.c
> @@ -135,6 +135,39 @@ static size_t nfs_parse_server_name(char *string, size_t len,
>  }
>  
>  /**
> + * nfs_test_gss - Test client support of pseudoflavor
> + * @server: NFS server struct
> + * @flavor: RPC_AUTH_GSS pseudoflavor
> + */
> +
> +static int nfs_test_gss_flavor(struct nfs_server *server,
> +			       rpc_authflavor_t pseudoflavor)
> +{
> +	struct rpc_auth_create_args auth_args = {
> +		.pseudoflavor = pseudoflavor,
> +	};
> +	struct rpc_auth *auth;
> +	struct rpc_cred *rcred;
> +	const struct cred *cred = current_cred();
> +	struct auth_cred acred = {
> +		.uid = cred->fsuid,
> +		.gid = cred->fsgid,
> +		.group_info = get_group_info(((struct cred *)cred)->group_info),
> +	};
> +
> +	auth = rpcauth_create(&auth_args, server->client);

This call has the side-effect of changing the value of
server->client->cl_auth. Not sure that we want that here.

> +	if (IS_ERR(auth))
> +		return -EACCES;
> +
> +	/* This will call cr_init to create a gss context */
> +	rcred = rpcauth_lookup_credcache(auth, &acred, 0);

Why not call rpcauth_lookupcred() instead of open-coding?

Also note that there is a credential refcount leak here (and a
group_info refcount leak).

> +	if (IS_ERR(cred))
> +		return -EACCES;
> +
> +	return 0;
> +}
> +
> +/**
>   * nfs_find_best_sec - Find a security mechanism supported locally
>   * @server: NFS server struct
>   * @flavors: List of security tuples returned by SECINFO procedure
> @@ -152,21 +185,32 @@ static rpc_authflavor_t nfs_find_best_sec(struct nfs_server *server,
>  	rpc_authflavor_t pseudoflavor;
>  	struct nfs4_secinfo4 *secinfo;
>  	unsigned int i;
> +	int err = 0;
>  
>  	for (i = 0; i < flavors->num_flavors; i++) {
> +		bool gss = false;
> +
>  		secinfo = &flavors->flavors[i];
>  
>  		switch (secinfo->flavor) {
> +		case RPC_AUTH_GSS:
> +			gss = true;
>  		case RPC_AUTH_NULL:
>  		case RPC_AUTH_UNIX:
> -		case RPC_AUTH_GSS:
>  			pseudoflavor = rpcauth_get_pseudoflavor(secinfo->flavor,
>  							&secinfo->flavor_info);
>  			/* make sure pseudoflavor matches sec= mount opt */
>  			if (pseudoflavor != RPC_AUTH_MAXFLAVOR &&
>  			    nfs_auth_info_match(&server->auth_info,
> -						pseudoflavor))
> +						pseudoflavor)) {
> +				if (gss) {
> +					err = nfs_test_gss_flavor(server,
> +								  pseudoflavor);
> +					if (err) /* try the next flavor */
> +						continue;
> +				}
>  				return pseudoflavor;
> +			}
>  			break;
>  		}
>  	}

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support
  2014-06-10 16:21   ` Trond Myklebust
@ 2014-06-10 18:38     ` Adamson, Andy
  2014-06-10 19:29       ` Trond Myklebust
  0 siblings, 1 reply; 9+ messages in thread
From: Adamson, Andy @ 2014-06-10 18:38 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: linux-nfs


On Jun 10, 2014, at 12:21 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:

> On Mon, 2014-06-09 at 15:33 -0400, andros@netapp.com wrote:
>> From: Andy Adamson <andros@netapp.com>
>> 
>> The current code returns an RPC_AUTH_GSS pseudo-flavor without testing to see
>> if it is configured properly. If an RPC_AUTH_GSS pseudoflavor fails then the
>> next SECINFO flavor should be tried.
>> 
>> Create an rpc_auth, rpc_cred, and initialize the cred (e.g. get a GSS Context)
>> using the short-lived SECINFO rpc client to test if the use of the RPC_AUTH_GSS
>> pseudoflavor succeeds.
>> 
>> Signed-off-by: Andy Adamson <andros@netapp.com>
>> ---
>> fs/nfs/nfs4namespace.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 46 insertions(+), 2 deletions(-)
>> 
>> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
>> index fd4dcb6..e0a5491 100644
>> --- a/fs/nfs/nfs4namespace.c
>> +++ b/fs/nfs/nfs4namespace.c
>> @@ -135,6 +135,39 @@ static size_t nfs_parse_server_name(char *string, size_t len,
>> }
>> 
>> /**
>> + * nfs_test_gss - Test client support of pseudoflavor
>> + * @server: NFS server struct
>> + * @flavor: RPC_AUTH_GSS pseudoflavor
>> + */
>> +
>> +static int nfs_test_gss_flavor(struct nfs_server *server,
>> +			       rpc_authflavor_t pseudoflavor)
>> +{
>> +	struct rpc_auth_create_args auth_args = {
>> +		.pseudoflavor = pseudoflavor,
>> +	};
>> +	struct rpc_auth *auth;
>> +	struct rpc_cred *rcred;
>> +	const struct cred *cred = current_cred();
>> +	struct auth_cred acred = {
>> +		.uid = cred->fsuid,
>> +		.gid = cred->fsgid,
>> +		.group_info = get_group_info(((struct cred *)cred)->group_info),
>> +	};
>> +
>> +	auth = rpcauth_create(&auth_args, server->client);
> 
> This call has the side-effect of changing the value of
> server->client->cl_auth. Not sure that we want that here.

I don't see any other interface to get a gss_auth struct to pass to rpcauth_lookupcredcache.

If the gss_cred/gss_context creation works, then the cl_auth being set is OK as it would have been set anyway by the callers of nfs4_negotiate_security (nfs4_submount or nfs4_create_sec_client so far) if we simply passed the flavor to those functions to “test” if RPC_AUTH_GSS can be used.

But on failure, you’re right, the cl_auth needs to be reaped. I’ll add a call to rpcauth_release() if nfs_test_gss_flavor() fails and set the cl_auth to NULL - and check that it is actually reaped. Since failure means no gss_context was created it is more simple than otherwise.

> 
>> +	if (IS_ERR(auth))
>> +		return -EACCES;
>> +
>> +	/* This will call cr_init to create a gss context */
>> +	rcred = rpcauth_lookup_credcache(auth, &acred, 0);
> 
> Why not call rpcauth_lookupcred() instead of open-coding?

I see - it will call rpcauth_lookupcredcache for me (gssand do the put of the group_info as well. Good - I’ll use it.

> 
> Also note that there is a credential refcount leak here

I’ll make sure this is addressed

Thanks for the review :)

—>Andy



> (and a
> group_info refcount leak).
> 
>> +	if (IS_ERR(cred))
>> +		return -EACCES;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>>  * nfs_find_best_sec - Find a security mechanism supported locally
>>  * @server: NFS server struct
>>  * @flavors: List of security tuples returned by SECINFO procedure
>> @@ -152,21 +185,32 @@ static rpc_authflavor_t nfs_find_best_sec(struct nfs_server *server,
>> 	rpc_authflavor_t pseudoflavor;
>> 	struct nfs4_secinfo4 *secinfo;
>> 	unsigned int i;
>> +	int err = 0;
>> 
>> 	for (i = 0; i < flavors->num_flavors; i++) {
>> +		bool gss = false;
>> +
>> 		secinfo = &flavors->flavors[i];
>> 
>> 		switch (secinfo->flavor) {
>> +		case RPC_AUTH_GSS:
>> +			gss = true;
>> 		case RPC_AUTH_NULL:
>> 		case RPC_AUTH_UNIX:
>> -		case RPC_AUTH_GSS:
>> 			pseudoflavor = rpcauth_get_pseudoflavor(secinfo->flavor,
>> 							&secinfo->flavor_info);
>> 			/* make sure pseudoflavor matches sec= mount opt */
>> 			if (pseudoflavor != RPC_AUTH_MAXFLAVOR &&
>> 			    nfs_auth_info_match(&server->auth_info,
>> -						pseudoflavor))
>> +						pseudoflavor)) {
>> +				if (gss) {
>> +					err = nfs_test_gss_flavor(server,
>> +								  pseudoflavor);
>> +					if (err) /* try the next flavor */
>> +						continue;
>> +				}
>> 				return pseudoflavor;
>> +			}
>> 			break;
>> 		}
>> 	}
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer, PrimaryData
> trond.myklebust@primarydata.com


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support
  2014-06-10 18:38     ` Adamson, Andy
@ 2014-06-10 19:29       ` Trond Myklebust
  2014-06-10 19:37         ` Adamson, Andy
  0 siblings, 1 reply; 9+ messages in thread
From: Trond Myklebust @ 2014-06-10 19:29 UTC (permalink / raw)
  To: Adamson, Andy; +Cc: linux-nfs

On Tue, Jun 10, 2014 at 2:38 PM, Adamson, Andy
<William.Adamson@netapp.com> wrote:
>
> On Jun 10, 2014, at 12:21 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
>
>> On Mon, 2014-06-09 at 15:33 -0400, andros@netapp.com wrote:
>>> From: Andy Adamson <andros@netapp.com>
>>>
>>> The current code returns an RPC_AUTH_GSS pseudo-flavor without testing to see
>>> if it is configured properly. If an RPC_AUTH_GSS pseudoflavor fails then the
>>> next SECINFO flavor should be tried.
>>>
>>> Create an rpc_auth, rpc_cred, and initialize the cred (e.g. get a GSS Context)
>>> using the short-lived SECINFO rpc client to test if the use of the RPC_AUTH_GSS
>>> pseudoflavor succeeds.
>>>
>>> Signed-off-by: Andy Adamson <andros@netapp.com>
>>> ---
>>> fs/nfs/nfs4namespace.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 46 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
>>> index fd4dcb6..e0a5491 100644
>>> --- a/fs/nfs/nfs4namespace.c
>>> +++ b/fs/nfs/nfs4namespace.c
>>> @@ -135,6 +135,39 @@ static size_t nfs_parse_server_name(char *string, size_t len,
>>> }
>>>
>>> /**
>>> + * nfs_test_gss - Test client support of pseudoflavor
>>> + * @server: NFS server struct
>>> + * @flavor: RPC_AUTH_GSS pseudoflavor
>>> + */
>>> +
>>> +static int nfs_test_gss_flavor(struct nfs_server *server,
>>> +                           rpc_authflavor_t pseudoflavor)
>>> +{
>>> +    struct rpc_auth_create_args auth_args = {
>>> +            .pseudoflavor = pseudoflavor,
>>> +    };
>>> +    struct rpc_auth *auth;
>>> +    struct rpc_cred *rcred;
>>> +    const struct cred *cred = current_cred();
>>> +    struct auth_cred acred = {
>>> +            .uid = cred->fsuid,
>>> +            .gid = cred->fsgid,
>>> +            .group_info = get_group_info(((struct cred *)cred)->group_info),
>>> +    };
>>> +
>>> +    auth = rpcauth_create(&auth_args, server->client);
>>
>> This call has the side-effect of changing the value of
>> server->client->cl_auth. Not sure that we want that here.
>
> I don't see any other interface to get a gss_auth struct to pass to rpcauth_lookupcredcache.
>
> If the gss_cred/gss_context creation works, then the cl_auth being set is OK as it would have been set anyway by the callers of nfs4_negotiate_security (nfs4_submount or nfs4_create_sec_client so far) if we simply passed the flavor to those functions to “test” if RPC_AUTH_GSS can be used.

Looking at the callers, I think I disagree. They are both trying to do
lookups, so the WRONGSEC error that we're handling applies to the
object being looked up, and not to the parent directory or the struct
nfs_server that is being passed as an argument to nfs_find_best_sec().

-- 
Trond Myklebust

Linux NFS client maintainer, PrimaryData

trond.myklebust@primarydata.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support
  2014-06-10 19:29       ` Trond Myklebust
@ 2014-06-10 19:37         ` Adamson, Andy
  0 siblings, 0 replies; 9+ messages in thread
From: Adamson, Andy @ 2014-06-10 19:37 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Adamson, Andy, linux-nfs


On Jun 10, 2014, at 3:29 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:

> On Tue, Jun 10, 2014 at 2:38 PM, Adamson, Andy
> <William.Adamson@netapp.com> wrote:
>> 
>> On Jun 10, 2014, at 12:21 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
>> 
>>> On Mon, 2014-06-09 at 15:33 -0400, andros@netapp.com wrote:
>>>> From: Andy Adamson <andros@netapp.com>
>>>> 
>>>> The current code returns an RPC_AUTH_GSS pseudo-flavor without testing to see
>>>> if it is configured properly. If an RPC_AUTH_GSS pseudoflavor fails then the
>>>> next SECINFO flavor should be tried.
>>>> 
>>>> Create an rpc_auth, rpc_cred, and initialize the cred (e.g. get a GSS Context)
>>>> using the short-lived SECINFO rpc client to test if the use of the RPC_AUTH_GSS
>>>> pseudoflavor succeeds.
>>>> 
>>>> Signed-off-by: Andy Adamson <andros@netapp.com>
>>>> ---
>>>> fs/nfs/nfs4namespace.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
>>>> 1 file changed, 46 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
>>>> index fd4dcb6..e0a5491 100644
>>>> --- a/fs/nfs/nfs4namespace.c
>>>> +++ b/fs/nfs/nfs4namespace.c
>>>> @@ -135,6 +135,39 @@ static size_t nfs_parse_server_name(char *string, size_t len,
>>>> }
>>>> 
>>>> /**
>>>> + * nfs_test_gss - Test client support of pseudoflavor
>>>> + * @server: NFS server struct
>>>> + * @flavor: RPC_AUTH_GSS pseudoflavor
>>>> + */
>>>> +
>>>> +static int nfs_test_gss_flavor(struct nfs_server *server,
>>>> +                           rpc_authflavor_t pseudoflavor)
>>>> +{
>>>> +    struct rpc_auth_create_args auth_args = {
>>>> +            .pseudoflavor = pseudoflavor,
>>>> +    };
>>>> +    struct rpc_auth *auth;
>>>> +    struct rpc_cred *rcred;
>>>> +    const struct cred *cred = current_cred();
>>>> +    struct auth_cred acred = {
>>>> +            .uid = cred->fsuid,
>>>> +            .gid = cred->fsgid,
>>>> +            .group_info = get_group_info(((struct cred *)cred)->group_info),
>>>> +    };
>>>> +
>>>> +    auth = rpcauth_create(&auth_args, server->client);
>>> 
>>> This call has the side-effect of changing the value of
>>> server->client->cl_auth. Not sure that we want that here.
>> 
>> I don't see any other interface to get a gss_auth struct to pass to rpcauth_lookupcredcache.
>> 
>> If the gss_cred/gss_context creation works, then the cl_auth being set is OK as it would have been set anyway by the callers of nfs4_negotiate_security (nfs4_submount or nfs4_create_sec_client so far) if we simply passed the flavor to those functions to “test” if RPC_AUTH_GSS can be used.
> 
> Looking at the callers, I think I disagree. They are both trying to do
> lookups, so the WRONGSEC error that we're handling applies to the
> object being looked up, and not to the parent directory or the struct
> nfs_server that is being passed as an argument to nfs_find_best_sec().

Ah. So I should clone a client to test the RPC_AUTH_GSS SECINFO case…

—>Andy

> 
> -- 
> Trond Myklebust
> 
> Linux NFS client maintainer, PrimaryData
> 
> trond.myklebust@primarydata.com


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-06-10 19:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-09 19:33 [PATCH 0/3] NFS: Fix SECINFO processing regression andros
2014-06-09 19:33 ` [PATCH 1/3] NFS check the return of nfs4_negotiate_security in nfs4_submount andros
2014-06-09 19:33 ` [PATCH 2/3] NFS Return -EPERM if no supported or matching SECINFO flavor andros
2014-06-09 19:33 ` [PATCH 3/3] NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support andros
2014-06-10 16:21   ` Trond Myklebust
2014-06-10 18:38     ` Adamson, Andy
2014-06-10 19:29       ` Trond Myklebust
2014-06-10 19:37         ` Adamson, Andy
2014-06-10 15:04 ` [PATCH 0/3] NFS: Fix SECINFO processing regression Steve Dickson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.