linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Use root user credential for lease management
@ 2013-06-07 19:44 Chuck Lever
  2013-06-07 19:44 ` [PATCH 1/3] NFS: Never use user credentials for lease renewal Chuck Lever
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Chuck Lever @ 2013-06-07 19:44 UTC (permalink / raw)
  To: linux-nfs

Hi-

This series applies to post 3.10-rc4 kernels.  The idea is to make
newer kernels behave like 3.6 and previous when it comes to mounting
sec=krb5 when the client has no keytab available.

The third patch is similar to the 3.7 patch I posted a few days ago,
but is adjusted slightly to apply after the "Use krb5i for lease
management" patch that appears in 3.10-rc.

---

Chuck Lever (3):
      NFS: Never use user credentials for lease renewal
      NFS: Partially revert commit 845cbceb
      NFS: Use root's credential for lease management when keytab is missing


 fs/nfs/nfs4proc.c  |   25 ++++++++++++++++---------
 fs/nfs/nfs4state.c |   28 ++++++++++++++++++++--------
 2 files changed, 36 insertions(+), 17 deletions(-)

-- 
Chuck Lever

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

* [PATCH 1/3] NFS: Never use user credentials for lease renewal
  2013-06-07 19:44 [PATCH 0/3] Use root user credential for lease management Chuck Lever
@ 2013-06-07 19:44 ` Chuck Lever
  2013-06-07 19:45 ` [PATCH 2/3] NFS: Partially revert commit 845cbceb Chuck Lever
  2013-06-07 19:45 ` [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing Chuck Lever
  2 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2013-06-07 19:44 UTC (permalink / raw)
  To: linux-nfs

Don't try to use a non-UID-0 user credential for lease management,
as that credential can change out from under us.  The server will
block NFSv4 lease recovery with NFS4ERR_CLID_INUSE.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfs/nfs4proc.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index d7ba561..5ba38b3 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6919,7 +6919,7 @@ static const struct nfs4_state_recovery_ops nfs40_reboot_recovery_ops = {
 	.recover_open	= nfs4_open_reclaim,
 	.recover_lock	= nfs4_lock_reclaim,
 	.establish_clid = nfs4_init_clientid,
-	.get_clid_cred	= nfs4_get_setclientid_cred,
+	.get_clid_cred	= nfs4_get_exchange_id_cred,
 	.detect_trunking = nfs40_discover_server_trunking,
 };
 
@@ -6942,7 +6942,7 @@ static const struct nfs4_state_recovery_ops nfs40_nograce_recovery_ops = {
 	.recover_open	= nfs4_open_expired,
 	.recover_lock	= nfs4_lock_expired,
 	.establish_clid = nfs4_init_clientid,
-	.get_clid_cred	= nfs4_get_setclientid_cred,
+	.get_clid_cred	= nfs4_get_exchange_id_cred,
 };
 
 #if defined(CONFIG_NFS_V4_1)


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

* [PATCH 2/3] NFS: Partially revert commit 845cbceb
  2013-06-07 19:44 [PATCH 0/3] Use root user credential for lease management Chuck Lever
  2013-06-07 19:44 ` [PATCH 1/3] NFS: Never use user credentials for lease renewal Chuck Lever
@ 2013-06-07 19:45 ` Chuck Lever
  2013-06-07 19:45 ` [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing Chuck Lever
  2 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2013-06-07 19:45 UTC (permalink / raw)
  To: linux-nfs

I'm about to add logic that invokes nfs4_clear_machine_cred(), but
we removed it in 3.10.  This is a separate commit to make it easier
to backport the next patch to a kernel that may still have
nfs4_clear_machine_cred().

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfs/nfs4state.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 1fab140..4949ce5 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -154,6 +154,18 @@ struct rpc_cred *nfs4_get_machine_cred_locked(struct nfs_client *clp)
 	return cred;
 }
 
+static void nfs4_clear_machine_cred(struct nfs_client *clp)
+{
+	struct rpc_cred *cred;
+
+	spin_lock(&clp->cl_lock);
+	cred = clp->cl_machine_cred;
+	clp->cl_machine_cred = NULL;
+	spin_unlock(&clp->cl_lock);
+	if (cred != NULL)
+		put_rpccred(cred);
+}
+
 static struct rpc_cred *
 nfs4_get_renew_cred_server_locked(struct nfs_server *server)
 {


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

* [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing
  2013-06-07 19:44 [PATCH 0/3] Use root user credential for lease management Chuck Lever
  2013-06-07 19:44 ` [PATCH 1/3] NFS: Never use user credentials for lease renewal Chuck Lever
  2013-06-07 19:45 ` [PATCH 2/3] NFS: Partially revert commit 845cbceb Chuck Lever
@ 2013-06-07 19:45 ` Chuck Lever
  2013-06-07 19:52   ` Myklebust, Trond
  2 siblings, 1 reply; 6+ messages in thread
From: Chuck Lever @ 2013-06-07 19:45 UTC (permalink / raw)
  To: linux-nfs

Commit 05f4c350 "NFS: Discover NFSv4 server trunking when mounting"
Fri Sep 14 17:24:32 2012 introduced Uniform Client String support,
which forces our NFS client to establish a client ID immediately
during a mount operation rather than waiting until a user wants to
open a file.

Normally machine credentials (eg. from a keytab) are used to perform
a mount operation that is protected by Kerberos.  Before 05fc350,
SETCLIENTID used a machine credential, or fell back to a regular
user's credential if no keytab is available.

On clients that don't have a keytab, performing SETCLIENTID early
means there's no user credential to fall back on, since no regular
user has kinit'd yet.  05f4c350 seems to have broken the ability
to mount with sec=krb5 on clients that don't have a keytab in
kernels 3.7 - 3.9.

To address this regression, commit 4edaa308 (NFS: Use "krb5i" to
establish NFSv4 state whenever possible), Sat Mar 16 15:56:20 2013,
was merged in 3.10.  This commit forces the NFS client to fall back
to AUTH_SYS for lease management operations if no keytab is
available.

Neil Brown noticed that, since root is required to kinit to do a
sec=krb5 mount when a client doesn't have a keytab, we can try to
use root's Kerberos credential before AUTH_SYS.

Now, when determining a principal and flavor to use for lease
management, the NFS client tries in this order:

  1.  Flavor: AUTH_GSS, krb5i
      Principal: service principal (via keytab)

  2.  Flavor: AUTH_GSS, krb5i
      Principal: user principal established for UID 0 (via kinit)

  3.  Flavor: AUTH_SYS
      Principal: UID 0 / GID 0

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfs/nfs4proc.c  |   21 ++++++++++++++-------
 fs/nfs/nfs4state.c |   16 ++++++++--------
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 5ba38b3..4d76fae 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4356,7 +4356,8 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 	dprintk("NFS call  setclientid auth=%s, '%.*s'\n",
 		clp->cl_rpcclient->cl_auth->au_ops->au_name,
 		setclientid.sc_name_len, setclientid.sc_name);
-	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
+	status = rpc_call_sync(clp->cl_rpcclient, &msg,
+					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
 	dprintk("NFS reply setclientid: %d\n", status);
 	return status;
 }
@@ -4383,7 +4384,8 @@ int nfs4_proc_setclientid_confirm(struct nfs_client *clp,
 	dprintk("NFS call  setclientid_confirm auth=%s, (client ID %llx)\n",
 		clp->cl_rpcclient->cl_auth->au_ops->au_name,
 		clp->cl_clientid);
-	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
+	status = rpc_call_sync(clp->cl_rpcclient, &msg,
+					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
 	dprintk("NFS reply setclientid_confirm: %d\n", status);
 	return status;
 }
@@ -5461,7 +5463,8 @@ int nfs4_proc_bind_conn_to_session(struct nfs_client *clp, struct rpc_cred *cred
 		goto out;
 	}
 
-	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
+	status = rpc_call_sync(clp->cl_rpcclient, &msg,
+					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
 	if (status == 0) {
 		if (memcmp(res.session->sess_id.data,
 		    clp->cl_session->sess_id.data, NFS4_MAX_SESSIONID_LEN)) {
@@ -5545,7 +5548,8 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
 		goto out_server_scope;
 	}
 
-	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
+	status = rpc_call_sync(clp->cl_rpcclient, &msg,
+					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
 	if (status == 0)
 		status = nfs4_check_cl_exchange_flags(res.flags);
 
@@ -5605,7 +5609,8 @@ static int _nfs4_proc_destroy_clientid(struct nfs_client *clp,
 	};
 	int status;
 
-	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
+	status = rpc_call_sync(clp->cl_rpcclient, &msg,
+					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
 	if (status)
 		dprintk("NFS: Got error %d from the server %s on "
 			"DESTROY_CLIENTID.", status, clp->cl_hostname);
@@ -5871,7 +5876,8 @@ static int _nfs4_proc_create_session(struct nfs_client *clp,
 	nfs4_init_channel_attrs(&args);
 	args.flags = (SESSION4_PERSIST | SESSION4_BACK_CHAN);
 
-	status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
+	status = rpc_call_sync(session->clp->cl_rpcclient, &msg,
+					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
 
 	if (!status) {
 		/* Verify the session's negotiated channel_attrs values */
@@ -5934,7 +5940,8 @@ int nfs4_proc_destroy_session(struct nfs4_session *session,
 	if (session->clp->cl_cons_state != NFS_CS_READY)
 		return status;
 
-	status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
+	status = rpc_call_sync(session->clp->cl_rpcclient, &msg,
+					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
 
 	if (status)
 		dprintk("NFS: Got error %d from the server on DESTROY_SESSION. "
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 4949ce5..d8068ab 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1811,10 +1811,9 @@ static int nfs4_establish_lease(struct nfs_client *clp)
 	int status;
 
 	cred = ops->get_clid_cred(clp);
-	if (cred == NULL)
-		return -ENOENT;
 	status = ops->establish_clid(clp, cred);
-	put_rpccred(cred);
+	if (cred)
+		put_rpccred(cred);
 	if (status != 0)
 		return status;
 	pnfs_destroy_all_layouts(clp);
@@ -1885,11 +1884,9 @@ int nfs4_discover_server_trunking(struct nfs_client *clp,
 again:
 	status  = -ENOENT;
 	cred = ops->get_clid_cred(clp);
-	if (cred == NULL)
-		goto out_unlock;
-
 	status = ops->detect_trunking(clp, result, cred);
-	put_rpccred(cred);
+	if (cred)
+		put_rpccred(cred);
 	switch (status) {
 	case 0:
 		break;
@@ -1902,6 +1899,10 @@ again:
 			__func__, status);
 		goto again;
 	case -EACCES:
+		if (clp->cl_machine_cred != NULL) {
+			nfs4_clear_machine_cred(clp);
+			goto again;
+		}
 		if (i++)
 			break;
 	case -NFS4ERR_CLID_INUSE:
@@ -1935,7 +1936,6 @@ again:
 		status = -EIO;
 	}
 
-out_unlock:
 	mutex_unlock(&nfs_clid_init_mutex);
 	dprintk("NFS: %s: status = %d\n", __func__, status);
 	return status;


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

* Re: [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing
  2013-06-07 19:45 ` [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing Chuck Lever
@ 2013-06-07 19:52   ` Myklebust, Trond
  2013-06-07 20:04     ` Chuck Lever
  0 siblings, 1 reply; 6+ messages in thread
From: Myklebust, Trond @ 2013-06-07 19:52 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs@vger.kernel.org

On Fri, 2013-06-07 at 15:45 -0400, Chuck Lever wrote:
> Commit 05f4c350 "NFS: Discover NFSv4 server trunking when mounting"
> Fri Sep 14 17:24:32 2012 introduced Uniform Client String support,
> which forces our NFS client to establish a client ID immediately
> during a mount operation rather than waiting until a user wants to
> open a file.
> 
> Normally machine credentials (eg. from a keytab) are used to perform
> a mount operation that is protected by Kerberos.  Before 05fc350,
> SETCLIENTID used a machine credential, or fell back to a regular
> user's credential if no keytab is available.
> 
> On clients that don't have a keytab, performing SETCLIENTID early
> means there's no user credential to fall back on, since no regular
> user has kinit'd yet.  05f4c350 seems to have broken the ability
> to mount with sec=krb5 on clients that don't have a keytab in
> kernels 3.7 - 3.9.
> 
> To address this regression, commit 4edaa308 (NFS: Use "krb5i" to
> establish NFSv4 state whenever possible), Sat Mar 16 15:56:20 2013,
> was merged in 3.10.  This commit forces the NFS client to fall back
> to AUTH_SYS for lease management operations if no keytab is
> available.
> 
> Neil Brown noticed that, since root is required to kinit to do a
> sec=krb5 mount when a client doesn't have a keytab, we can try to
> use root's Kerberos credential before AUTH_SYS.
> 
> Now, when determining a principal and flavor to use for lease
> management, the NFS client tries in this order:
> 
>   1.  Flavor: AUTH_GSS, krb5i
>       Principal: service principal (via keytab)
> 
>   2.  Flavor: AUTH_GSS, krb5i
>       Principal: user principal established for UID 0 (via kinit)
> 
>   3.  Flavor: AUTH_SYS
>       Principal: UID 0 / GID 0
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfs/nfs4proc.c  |   21 ++++++++++++++-------
>  fs/nfs/nfs4state.c |   16 ++++++++--------
>  2 files changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 5ba38b3..4d76fae 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -4356,7 +4356,8 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
>  	dprintk("NFS call  setclientid auth=%s, '%.*s'\n",
>  		clp->cl_rpcclient->cl_auth->au_ops->au_name,
>  		setclientid.sc_name_len, setclientid.sc_name);
> -	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
> +	status = rpc_call_sync(clp->cl_rpcclient, &msg,
> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);

We are certainly _not_ resurrecting the RPC_TASK_ROOTCREDS abomination.

For one thing is overrides the machine creds that are what we really
want to use when available.

NACK...

>  	dprintk("NFS reply setclientid: %d\n", status);
>  	return status;
>  }
> @@ -4383,7 +4384,8 @@ int nfs4_proc_setclientid_confirm(struct nfs_client *clp,
>  	dprintk("NFS call  setclientid_confirm auth=%s, (client ID %llx)\n",
>  		clp->cl_rpcclient->cl_auth->au_ops->au_name,
>  		clp->cl_clientid);
> -	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
> +	status = rpc_call_sync(clp->cl_rpcclient, &msg,
> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
>  	dprintk("NFS reply setclientid_confirm: %d\n", status);
>  	return status;
>  }
> @@ -5461,7 +5463,8 @@ int nfs4_proc_bind_conn_to_session(struct nfs_client *clp, struct rpc_cred *cred
>  		goto out;
>  	}
>  
> -	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
> +	status = rpc_call_sync(clp->cl_rpcclient, &msg,
> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
>  	if (status == 0) {
>  		if (memcmp(res.session->sess_id.data,
>  		    clp->cl_session->sess_id.data, NFS4_MAX_SESSIONID_LEN)) {
> @@ -5545,7 +5548,8 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
>  		goto out_server_scope;
>  	}
>  
> -	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
> +	status = rpc_call_sync(clp->cl_rpcclient, &msg,
> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
>  	if (status == 0)
>  		status = nfs4_check_cl_exchange_flags(res.flags);
>  
> @@ -5605,7 +5609,8 @@ static int _nfs4_proc_destroy_clientid(struct nfs_client *clp,
>  	};
>  	int status;
>  
> -	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
> +	status = rpc_call_sync(clp->cl_rpcclient, &msg,
> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
>  	if (status)
>  		dprintk("NFS: Got error %d from the server %s on "
>  			"DESTROY_CLIENTID.", status, clp->cl_hostname);
> @@ -5871,7 +5876,8 @@ static int _nfs4_proc_create_session(struct nfs_client *clp,
>  	nfs4_init_channel_attrs(&args);
>  	args.flags = (SESSION4_PERSIST | SESSION4_BACK_CHAN);
>  
> -	status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
> +	status = rpc_call_sync(session->clp->cl_rpcclient, &msg,
> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
>  
>  	if (!status) {
>  		/* Verify the session's negotiated channel_attrs values */
> @@ -5934,7 +5940,8 @@ int nfs4_proc_destroy_session(struct nfs4_session *session,
>  	if (session->clp->cl_cons_state != NFS_CS_READY)
>  		return status;
>  
> -	status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
> +	status = rpc_call_sync(session->clp->cl_rpcclient, &msg,
> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
>  
>  	if (status)
>  		dprintk("NFS: Got error %d from the server on DESTROY_SESSION. "
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index 4949ce5..d8068ab 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -1811,10 +1811,9 @@ static int nfs4_establish_lease(struct nfs_client *clp)
>  	int status;
>  
>  	cred = ops->get_clid_cred(clp);
> -	if (cred == NULL)
> -		return -ENOENT;
>  	status = ops->establish_clid(clp, cred);
> -	put_rpccred(cred);
> +	if (cred)
> +		put_rpccred(cred);
>  	if (status != 0)
>  		return status;
>  	pnfs_destroy_all_layouts(clp);
> @@ -1885,11 +1884,9 @@ int nfs4_discover_server_trunking(struct nfs_client *clp,
>  again:
>  	status  = -ENOENT;
>  	cred = ops->get_clid_cred(clp);
> -	if (cred == NULL)
> -		goto out_unlock;
> -
>  	status = ops->detect_trunking(clp, result, cred);
> -	put_rpccred(cred);
> +	if (cred)
> +		put_rpccred(cred);
>  	switch (status) {
>  	case 0:
>  		break;
> @@ -1902,6 +1899,10 @@ again:
>  			__func__, status);
>  		goto again;
>  	case -EACCES:
> +		if (clp->cl_machine_cred != NULL) {
> +			nfs4_clear_machine_cred(clp);
> +			goto again;
> +		}
>  		if (i++)
>  			break;
>  	case -NFS4ERR_CLID_INUSE:
> @@ -1935,7 +1936,6 @@ again:
>  		status = -EIO;
>  	}
>  
> -out_unlock:
>  	mutex_unlock(&nfs_clid_init_mutex);
>  	dprintk("NFS: %s: status = %d\n", __func__, status);
>  	return status;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

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

* Re: [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing
  2013-06-07 19:52   ` Myklebust, Trond
@ 2013-06-07 20:04     ` Chuck Lever
  0 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2013-06-07 20:04 UTC (permalink / raw)
  To: Myklebust, Trond; +Cc: linux-nfs@vger.kernel.org


On Jun 7, 2013, at 3:52 PM, "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> On Fri, 2013-06-07 at 15:45 -0400, Chuck Lever wrote:
>> Commit 05f4c350 "NFS: Discover NFSv4 server trunking when mounting"
>> Fri Sep 14 17:24:32 2012 introduced Uniform Client String support,
>> which forces our NFS client to establish a client ID immediately
>> during a mount operation rather than waiting until a user wants to
>> open a file.
>> 
>> Normally machine credentials (eg. from a keytab) are used to perform
>> a mount operation that is protected by Kerberos.  Before 05fc350,
>> SETCLIENTID used a machine credential, or fell back to a regular
>> user's credential if no keytab is available.
>> 
>> On clients that don't have a keytab, performing SETCLIENTID early
>> means there's no user credential to fall back on, since no regular
>> user has kinit'd yet.  05f4c350 seems to have broken the ability
>> to mount with sec=krb5 on clients that don't have a keytab in
>> kernels 3.7 - 3.9.
>> 
>> To address this regression, commit 4edaa308 (NFS: Use "krb5i" to
>> establish NFSv4 state whenever possible), Sat Mar 16 15:56:20 2013,
>> was merged in 3.10.  This commit forces the NFS client to fall back
>> to AUTH_SYS for lease management operations if no keytab is
>> available.
>> 
>> Neil Brown noticed that, since root is required to kinit to do a
>> sec=krb5 mount when a client doesn't have a keytab, we can try to
>> use root's Kerberos credential before AUTH_SYS.
>> 
>> Now, when determining a principal and flavor to use for lease
>> management, the NFS client tries in this order:
>> 
>>  1.  Flavor: AUTH_GSS, krb5i
>>      Principal: service principal (via keytab)
>> 
>>  2.  Flavor: AUTH_GSS, krb5i
>>      Principal: user principal established for UID 0 (via kinit)
>> 
>>  3.  Flavor: AUTH_SYS
>>      Principal: UID 0 / GID 0
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> fs/nfs/nfs4proc.c  |   21 ++++++++++++++-------
>> fs/nfs/nfs4state.c |   16 ++++++++--------
>> 2 files changed, 22 insertions(+), 15 deletions(-)
>> 
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 5ba38b3..4d76fae 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -4356,7 +4356,8 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
>> 	dprintk("NFS call  setclientid auth=%s, '%.*s'\n",
>> 		clp->cl_rpcclient->cl_auth->au_ops->au_name,
>> 		setclientid.sc_name_len, setclientid.sc_name);
>> -	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
>> +	status = rpc_call_sync(clp->cl_rpcclient, &msg,
>> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
> 
> We are certainly _not_ resurrecting the RPC_TASK_ROOTCREDS abomination.

Maybe it should be removed then?  I don't see any other consumers (even NFS SWAP).

> For one thing is overrides the machine creds that are what we really
> want to use when available.

I don't see how it would override machine creds...?

rpcauth_bind_root_cred is called _only_ if the passed-in cred is NULL.  The first SETCLIENTID try hands in an rpc_cred, so the root cred is not used.  If the first try fails, nfs4_clear_machine_cred() clears that rpc_cred, and SETCLIENTID tries with a NULL cred, which should try to bind to root's cred.

If I can figure out another way to obtain a root cred, is this general approach acceptable?  Do you have a recommended approach for obtaining the UID 0 cred?


> 
>> 	dprintk("NFS reply setclientid: %d\n", status);
>> 	return status;
>> }
>> @@ -4383,7 +4384,8 @@ int nfs4_proc_setclientid_confirm(struct nfs_client *clp,
>> 	dprintk("NFS call  setclientid_confirm auth=%s, (client ID %llx)\n",
>> 		clp->cl_rpcclient->cl_auth->au_ops->au_name,
>> 		clp->cl_clientid);
>> -	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
>> +	status = rpc_call_sync(clp->cl_rpcclient, &msg,
>> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
>> 	dprintk("NFS reply setclientid_confirm: %d\n", status);
>> 	return status;
>> }
>> @@ -5461,7 +5463,8 @@ int nfs4_proc_bind_conn_to_session(struct nfs_client *clp, struct rpc_cred *cred
>> 		goto out;
>> 	}
>> 
>> -	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
>> +	status = rpc_call_sync(clp->cl_rpcclient, &msg,
>> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
>> 	if (status == 0) {
>> 		if (memcmp(res.session->sess_id.data,
>> 		    clp->cl_session->sess_id.data, NFS4_MAX_SESSIONID_LEN)) {
>> @@ -5545,7 +5548,8 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
>> 		goto out_server_scope;
>> 	}
>> 
>> -	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
>> +	status = rpc_call_sync(clp->cl_rpcclient, &msg,
>> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
>> 	if (status == 0)
>> 		status = nfs4_check_cl_exchange_flags(res.flags);
>> 
>> @@ -5605,7 +5609,8 @@ static int _nfs4_proc_destroy_clientid(struct nfs_client *clp,
>> 	};
>> 	int status;
>> 
>> -	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
>> +	status = rpc_call_sync(clp->cl_rpcclient, &msg,
>> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
>> 	if (status)
>> 		dprintk("NFS: Got error %d from the server %s on "
>> 			"DESTROY_CLIENTID.", status, clp->cl_hostname);
>> @@ -5871,7 +5876,8 @@ static int _nfs4_proc_create_session(struct nfs_client *clp,
>> 	nfs4_init_channel_attrs(&args);
>> 	args.flags = (SESSION4_PERSIST | SESSION4_BACK_CHAN);
>> 
>> -	status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
>> +	status = rpc_call_sync(session->clp->cl_rpcclient, &msg,
>> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
>> 
>> 	if (!status) {
>> 		/* Verify the session's negotiated channel_attrs values */
>> @@ -5934,7 +5940,8 @@ int nfs4_proc_destroy_session(struct nfs4_session *session,
>> 	if (session->clp->cl_cons_state != NFS_CS_READY)
>> 		return status;
>> 
>> -	status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
>> +	status = rpc_call_sync(session->clp->cl_rpcclient, &msg,
>> +					RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS);
>> 
>> 	if (status)
>> 		dprintk("NFS: Got error %d from the server on DESTROY_SESSION. "
>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
>> index 4949ce5..d8068ab 100644
>> --- a/fs/nfs/nfs4state.c
>> +++ b/fs/nfs/nfs4state.c
>> @@ -1811,10 +1811,9 @@ static int nfs4_establish_lease(struct nfs_client *clp)
>> 	int status;
>> 
>> 	cred = ops->get_clid_cred(clp);
>> -	if (cred == NULL)
>> -		return -ENOENT;
>> 	status = ops->establish_clid(clp, cred);
>> -	put_rpccred(cred);
>> +	if (cred)
>> +		put_rpccred(cred);
>> 	if (status != 0)
>> 		return status;
>> 	pnfs_destroy_all_layouts(clp);
>> @@ -1885,11 +1884,9 @@ int nfs4_discover_server_trunking(struct nfs_client *clp,
>> again:
>> 	status  = -ENOENT;
>> 	cred = ops->get_clid_cred(clp);
>> -	if (cred == NULL)
>> -		goto out_unlock;
>> -
>> 	status = ops->detect_trunking(clp, result, cred);
>> -	put_rpccred(cred);
>> +	if (cred)
>> +		put_rpccred(cred);
>> 	switch (status) {
>> 	case 0:
>> 		break;
>> @@ -1902,6 +1899,10 @@ again:
>> 			__func__, status);
>> 		goto again;
>> 	case -EACCES:
>> +		if (clp->cl_machine_cred != NULL) {
>> +			nfs4_clear_machine_cred(clp);
>> +			goto again;
>> +		}
>> 		if (i++)
>> 			break;
>> 	case -NFS4ERR_CLID_INUSE:
>> @@ -1935,7 +1936,6 @@ again:
>> 		status = -EIO;
>> 	}
>> 
>> -out_unlock:
>> 	mutex_unlock(&nfs_clid_init_mutex);
>> 	dprintk("NFS: %s: status = %d\n", __func__, status);
>> 	return status;
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer
> 
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





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

end of thread, other threads:[~2013-06-07 20:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-07 19:44 [PATCH 0/3] Use root user credential for lease management Chuck Lever
2013-06-07 19:44 ` [PATCH 1/3] NFS: Never use user credentials for lease renewal Chuck Lever
2013-06-07 19:45 ` [PATCH 2/3] NFS: Partially revert commit 845cbceb Chuck Lever
2013-06-07 19:45 ` [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing Chuck Lever
2013-06-07 19:52   ` Myklebust, Trond
2013-06-07 20:04     ` Chuck Lever

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).