On Jun 10, 2014, at 12:21 PM, Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> wrote:
> On Mon, 2014-06-09 at 15:33 -0400, andros@xxxxxxxxxx wrote:
>> From: Andy Adamson <andros@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
