Return-Path: linux-nfs-owner@vger.kernel.org Received: from mx12.netapp.com ([216.240.18.77]:28018 "EHLO mx12.netapp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753785Ab3IERFL convert rfc822-to-8bit (ORCPT ); Thu, 5 Sep 2013 13:05:11 -0400 From: "Adamson, Dros" To: Dr James Bruce Fields CC: "Adamson, Dros" , "Myklebust, Trond" , linux-nfs list Subject: Re: [PATCH] NFSv4: use mach cred for SECINFO_NO_NAME w/ integrity Date: Thu, 5 Sep 2013 17:05:09 +0000 Message-ID: <316C2554-07BD-4DB1-9BA2-4956C83680D0@netapp.com> References: <1378311199-1695-1-git-send-email-dros@netapp.com> <1378311850.3527.16.camel@leira.trondhjem.org> <1378341909.6143.1.camel@leira.trondhjem.org> <20130905140718.GI10232@fieldses.org> <20130905153121.GJ10232@fieldses.org> In-Reply-To: <20130905153121.GJ10232@fieldses.org> Content-Type: text/plain; charset="Windows-1252" MIME-Version: 1.0 Sender: linux-nfs-owner@vger.kernel.org List-ID: On Sep 5, 2013, at 11:31 AM, Dr James Bruce Fields wrote: > On Thu, Sep 05, 2013 at 03:17:37PM +0000, Adamson, Dros wrote: >> >> On Sep 5, 2013, at 10:07 AM, Dr James Bruce Fields wrote: >> >>> On Thu, Sep 05, 2013 at 12:45:09AM +0000, Myklebust, Trond wrote: >>>> On Wed, 2013-09-04 at 16:48 +0000, Adamson, Dros wrote: >>>>> On Sep 4, 2013, at 12:24 PM, "Myklebust, Trond" wrote: >>>>> >>>>>> On Wed, 2013-09-04 at 12:13 -0400, Weston Andros Adamson wrote: >>>>>>> Commit 97431204ea005ec8070ac94bc3251e836daa7ca7 introduced a regression >>>>>>> that causes SECINFO_NO_NAME to fail without sending an RPC if: >>>>>>> >>>>>>> 1) the nfs_client's rpc_client is using krb5i/p (now tried by default) >>>>>>> 2) the current user doesn't have valid kerberos credentials >>>>>>> >>>>>>> This situation is quite common - as of now a sec=sys mount would use >>>>>>> krb5i for the nfs_client's rpc_client and a user would hardly be faulted >>>>>>> for not having run kinit. >>>>>>> >>>>>>> The solution is to use the machine cred when trying to use an integrity >>>>>>> protected auth flavor for SECINFO_NO_NAME. >>>>>>> >>>>>>> Older servers may not support using the machine cred or an integrity >>>>>>> protected auth flavor for SECINFO_NO_NAME in every circumstance, so we fall >>>>>>> back to using the user's cred and the filesystem's auth flavor in this case. >>>>>>> >>>>>>> We run into another problem when running against linux nfs servers - >>>>>>> they return NFS4ERR_WRONGSEC when using integrity auth flavor (unless the >>>>>>> mount is also that flavor) even though that is not a valid error for >>>>>>> SECINFO*. Even though it's against spec, handle WRONGSEC errors on >>>>>>> SECINFO_NO_NAME by falling back to using the user cred and the >>>>>>> filesystem's auth flavor. >>>>>>> >>>>>>> Signed-off-by: Weston Andros Adamson >>>>>>> --- >>>>>>> >>>>>>> This patch goes along with yesterday's SECINFO patch >>>>>>> >>>>>>> fs/nfs/nfs4proc.c | 41 +++++++++++++++++++++++++++++++++++++---- >>>>>>> 1 file changed, 37 insertions(+), 4 deletions(-) >>>>>>> >>>>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >>>>>>> index ab1461e..74b37f5 100644 >>>>>>> --- a/fs/nfs/nfs4proc.c >>>>>>> +++ b/fs/nfs/nfs4proc.c >>>>>>> @@ -7291,7 +7291,8 @@ out: >>>>>>> */ >>>>>>> static int >>>>>>> _nfs41_proc_secinfo_no_name(struct nfs_server *server, struct nfs_fh *fhandle, >>>>>>> - struct nfs_fsinfo *info, struct nfs4_secinfo_flavors *flavors) >>>>>>> + struct nfs_fsinfo *info, >>>>>>> + struct nfs4_secinfo_flavors *flavors, bool use_integrity) >>>>>>> { >>>>>>> struct nfs41_secinfo_no_name_args args = { >>>>>>> .style = SECINFO_STYLE_CURRENT_FH, >>>>>>> @@ -7304,8 +7305,23 @@ _nfs41_proc_secinfo_no_name(struct nfs_server *server, struct nfs_fh *fhandle, >>>>>>> .rpc_argp = &args, >>>>>>> .rpc_resp = &res, >>>>>>> }; >>>>>>> - return nfs4_call_sync(server->nfs_client->cl_rpcclient, server, &msg, >>>>>>> - &args.seq_args, &res.seq_res, 0); >>>>>>> + struct rpc_clnt *clnt = server->client; >>>>>>> + int status; >>>>>>> + >>>>>>> + if (use_integrity) { >>>>>>> + clnt = server->nfs_client->cl_rpcclient; >>>>>>> + msg.rpc_cred = nfs4_get_clid_cred(server->nfs_client); >>>>>>> + } >>>>>>> + >>>>>>> + dprintk("--> %s\n", __func__); >>>>>>> + status = nfs4_call_sync(clnt, server, &msg, &args.seq_args, >>>>>>> + &res.seq_res, 0); >>>>>>> + dprintk("<-- %s status=%d\n", __func__, status); >>>>>>> + >>>>>>> + if (msg.rpc_cred) >>>>>>> + put_rpccred(msg.rpc_cred); >>>>>>> + >>>>>>> + return status; >>>>>>> } >>>>>>> >>>>>>> static int >>>>>>> @@ -7315,7 +7331,24 @@ nfs41_proc_secinfo_no_name(struct nfs_server *server, struct nfs_fh *fhandle, >>>>>>> struct nfs4_exception exception = { }; >>>>>>> int err; >>>>>>> do { >>>>>>> - err = _nfs41_proc_secinfo_no_name(server, fhandle, info, flavors); >>>>>>> + /* first try using integrity protection */ >>>>>>> + err = -NFS4ERR_WRONGSEC; >>>>>>> + >>>>>>> + /* try to use integrity protection with machine cred */ >>>>>>> + if (_nfs4_is_integrity_protected(server->nfs_client)) >>>>>>> + err = _nfs41_proc_secinfo_no_name(server, fhandle, info, >>>>>>> + flavors, true); >>>>>>> + >>>>>>> + /* >>>>>>> + * if unable to use integrity protection, or SECINFO with >>>>>>> + * integrity protection returns NFS4ERR_WRONGSEC (which is >>>>>>> + * disallowed by spec, but exists in deployed servers) use >>>>>>> + * the current filesystem's rpc_client and the user cred. >>>>>>> + */ >>>>>>> + if (err == -NFS4ERR_WRONGSEC) >>>>>>> + err = _nfs41_proc_secinfo_no_name(server, fhandle, info, >>>>>>> + flavors, false); >>>>>> >>>>>> As I said yesterday, RFC5661 forbids SECINFO_NO_NAME from returning >>>>>> NFS4ERR_WRONGSEC, so this is 100% equivalent to >>>>>> >>>>>> if (!_nfs4_is_integrity_protected()) >>>>>> err = ?. >>>>> >>>>> Right, but I thought we were doing this to support server implementations like linux that *do* return NFS4ERR_WRONGSEC on SECINFO_NO_NAME even though it's forbidden. I know we normally don't work around server bugs, but this seems pretty simple. >>>>> >>>>> If we don't do this, then SECINFO_NO_NAME will always fail against current linux severs no matter what the mount options - unless krb5i/p is unusable (not configured, time skew, no machine cred, etc). >>>> >>>> Bruce, you're it: what's the deal here? >>> >>> Dros, in what cases exactly do you see SECINFO_NO_NAME returning >>> WRONGSEC? >>> >>> From a quick skim of the code it looks like it shouldn't happen in the >>> CURRENT_FH case, which is the one the client uses. But I probably >>> overlooked something.... >>> >>> --b. >> >> SECINFO_NO_NAME will fail with NFS4ERR_WRONGSEC in check_nfsd_access when the rpc auth flavor is different from the export's auth flavor - in the same way as SECINFO. > > Huh. There's no check_nfsd_access call in secinfo_no_name in the > CURRENT_FH case. And any checks on the putfh op should be turned off by > the OP_HANDLES_WRONGSEC flag on secinfo_no_name. > > But I haven't actually tried it, and presumably you have (any hints on > reproducing?), so I'll take a look.... > > --b. You may be right here - I'm pretty sure I saw SECINFO_NO_NAME fail like this, but I'm not 100%. I'll try to reproduce and report back. -dros