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 <[email protected]>
---
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);
+
switch (err) {
case 0:
case -NFS4ERR_WRONGSEC:
--
1.7.12.4 (Apple Git-37)
On Sep 4, 2013, at 12:24 PM, "Myklebust, Trond" <[email protected]> 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 <[email protected]>
>> ---
>>
>> 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).
-dros
>
>> +
>> switch (err) {
>> case 0:
>> case -NFS4ERR_WRONGSEC:
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> [email protected]
> http://www.netapp.com
Hi,
----- "Dros Adamson" <[email protected]> wrote:
> On Sep 4, 2013, at 12:29 PM, Matt W. Benjamin <[email protected]>
> wrote:
>
> > Hi
> >
> > It honestly feels quite odd to me for sec=sys to actually connote
> krb5i.
>
> I should point out that my patches don't introduce the use of krb5i
> here, they just fix it.
Ack.
>
> I personally don't think it's weird for the client to use a *more*
> secure flavor for certain (infrequent) operations when it makes sense.
> What worries me that currently sec=krb5p can cross a SECINFO boundary
> and suddenly be using sec=sys!
I think the behavior is obviously reasonable, but giving that policy a
different name would allow sec=sys to continue mean what it says.
>
> I'm testing patches that fix that now and also allow multiple sec=
> options (in the same form as nfsd exports, i.e. sec=krb5:krb5i, but
> I'm trying to fix all the recent regressions surrounding auth flavors
> / SECINFO first...
That sounds great.
>
> -dros
>
> >
Thanks,
Matt
--
Matt Benjamin
The Linux Box
206 South Fifth Ave. Suite 150
Ann Arbor, MI 48104
http://linuxbox.com
tel. 734-761-4689
fax. 734-769-8938
cel. 734-216-5309
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 <[email protected]> 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" <[email protected]> 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 <[email protected]>
> >>>>> ---
> >>>>>
> >>>>> 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.
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" <[email protected]> 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 <[email protected]>
> > >> ---
> > >>
> > >> 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.
On Sep 5, 2013, at 11:31 AM, Dr James Bruce Fields <[email protected]> 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 <[email protected]> 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" <[email protected]> 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 <[email protected]>
>>>>>>> ---
>>>>>>>
>>>>>>> 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
T24gV2VkLCAyMDEzLTA5LTA0IGF0IDEyOjEzIC0wNDAwLCBXZXN0b24gQW5kcm9zIEFkYW1zb24g
d3JvdGU6DQo+IENvbW1pdCA5NzQzMTIwNGVhMDA1ZWM4MDcwYWM5NGJjMzI1MWU4MzZkYWE3Y2E3
IGludHJvZHVjZWQgYSByZWdyZXNzaW9uDQo+IHRoYXQgY2F1c2VzIFNFQ0lORk9fTk9fTkFNRSB0
byBmYWlsIHdpdGhvdXQgc2VuZGluZyBhbiBSUEMgaWY6DQo+IA0KPiAgMSkgdGhlIG5mc19jbGll
bnQncyBycGNfY2xpZW50IGlzIHVzaW5nIGtyYjVpL3AgKG5vdyB0cmllZCBieSBkZWZhdWx0KQ0K
PiAgMikgdGhlIGN1cnJlbnQgdXNlciBkb2Vzbid0IGhhdmUgdmFsaWQga2VyYmVyb3MgY3JlZGVu
dGlhbHMNCj4gDQo+IFRoaXMgc2l0dWF0aW9uIGlzIHF1aXRlIGNvbW1vbiAtIGFzIG9mIG5vdyBh
IHNlYz1zeXMgbW91bnQgd291bGQgdXNlDQo+IGtyYjVpIGZvciB0aGUgbmZzX2NsaWVudCdzIHJw
Y19jbGllbnQgYW5kIGEgdXNlciB3b3VsZCBoYXJkbHkgYmUgZmF1bHRlZA0KPiBmb3Igbm90IGhh
dmluZyBydW4ga2luaXQuDQo+IA0KPiBUaGUgc29sdXRpb24gaXMgdG8gdXNlIHRoZSBtYWNoaW5l
IGNyZWQgd2hlbiB0cnlpbmcgdG8gdXNlIGFuIGludGVncml0eQ0KPiBwcm90ZWN0ZWQgYXV0aCBm
bGF2b3IgZm9yIFNFQ0lORk9fTk9fTkFNRS4NCj4gDQo+IE9sZGVyIHNlcnZlcnMgbWF5IG5vdCBz
dXBwb3J0IHVzaW5nIHRoZSBtYWNoaW5lIGNyZWQgb3IgYW4gaW50ZWdyaXR5DQo+IHByb3RlY3Rl
ZCBhdXRoIGZsYXZvciBmb3IgU0VDSU5GT19OT19OQU1FIGluIGV2ZXJ5IGNpcmN1bXN0YW5jZSwg
c28gd2UgZmFsbA0KPiBiYWNrIHRvIHVzaW5nIHRoZSB1c2VyJ3MgY3JlZCBhbmQgdGhlIGZpbGVz
eXN0ZW0ncyBhdXRoIGZsYXZvciBpbiB0aGlzIGNhc2UuDQo+IA0KPiBXZSBydW4gaW50byBhbm90
aGVyIHByb2JsZW0gd2hlbiBydW5uaW5nIGFnYWluc3QgbGludXggbmZzIHNlcnZlcnMgLQ0KPiB0
aGV5IHJldHVybiBORlM0RVJSX1dST05HU0VDIHdoZW4gdXNpbmcgaW50ZWdyaXR5IGF1dGggZmxh
dm9yICh1bmxlc3MgdGhlDQo+IG1vdW50IGlzIGFsc28gdGhhdCBmbGF2b3IpIGV2ZW4gdGhvdWdo
IHRoYXQgaXMgbm90IGEgdmFsaWQgZXJyb3IgZm9yDQo+IFNFQ0lORk8qLiAgRXZlbiB0aG91Z2gg
aXQncyBhZ2FpbnN0IHNwZWMsIGhhbmRsZSBXUk9OR1NFQyBlcnJvcnMgb24NCj4gU0VDSU5GT19O
T19OQU1FIGJ5IGZhbGxpbmcgYmFjayB0byB1c2luZyB0aGUgdXNlciBjcmVkIGFuZCB0aGUNCj4g
ZmlsZXN5c3RlbSdzIGF1dGggZmxhdm9yLg0KPiANCj4gU2lnbmVkLW9mZi1ieTogV2VzdG9uIEFu
ZHJvcyBBZGFtc29uIDxkcm9zQG5ldGFwcC5jb20+DQo+IC0tLQ0KPiANCj4gVGhpcyBwYXRjaCBn
b2VzIGFsb25nIHdpdGggeWVzdGVyZGF5J3MgU0VDSU5GTyBwYXRjaA0KPiANCj4gIGZzL25mcy9u
ZnM0cHJvYy5jIHwgNDEgKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKy0tLS0N
Cj4gIDEgZmlsZSBjaGFuZ2VkLCAzNyBpbnNlcnRpb25zKCspLCA0IGRlbGV0aW9ucygtKQ0KPiAN
Cj4gZGlmZiAtLWdpdCBhL2ZzL25mcy9uZnM0cHJvYy5jIGIvZnMvbmZzL25mczRwcm9jLmMNCj4g
aW5kZXggYWIxNDYxZS4uNzRiMzdmNSAxMDA2NDQNCj4gLS0tIGEvZnMvbmZzL25mczRwcm9jLmMN
Cj4gKysrIGIvZnMvbmZzL25mczRwcm9jLmMNCj4gQEAgLTcyOTEsNyArNzI5MSw4IEBAIG91dDoN
Cj4gICAqLw0KPiAgc3RhdGljIGludA0KPiAgX25mczQxX3Byb2Nfc2VjaW5mb19ub19uYW1lKHN0
cnVjdCBuZnNfc2VydmVyICpzZXJ2ZXIsIHN0cnVjdCBuZnNfZmggKmZoYW5kbGUsDQo+IC0JCSAg
ICBzdHJ1Y3QgbmZzX2ZzaW5mbyAqaW5mbywgc3RydWN0IG5mczRfc2VjaW5mb19mbGF2b3JzICpm
bGF2b3JzKQ0KPiArCQkgICAgc3RydWN0IG5mc19mc2luZm8gKmluZm8sDQo+ICsJCSAgICBzdHJ1
Y3QgbmZzNF9zZWNpbmZvX2ZsYXZvcnMgKmZsYXZvcnMsIGJvb2wgdXNlX2ludGVncml0eSkNCj4g
IHsNCj4gIAlzdHJ1Y3QgbmZzNDFfc2VjaW5mb19ub19uYW1lX2FyZ3MgYXJncyA9IHsNCj4gIAkJ
LnN0eWxlID0gU0VDSU5GT19TVFlMRV9DVVJSRU5UX0ZILA0KPiBAQCAtNzMwNCw4ICs3MzA1LDIz
IEBAIF9uZnM0MV9wcm9jX3NlY2luZm9fbm9fbmFtZShzdHJ1Y3QgbmZzX3NlcnZlciAqc2VydmVy
LCBzdHJ1Y3QgbmZzX2ZoICpmaGFuZGxlLA0KPiAgCQkucnBjX2FyZ3AgPSAmYXJncywNCj4gIAkJ
LnJwY19yZXNwID0gJnJlcywNCj4gIAl9Ow0KPiAtCXJldHVybiBuZnM0X2NhbGxfc3luYyhzZXJ2
ZXItPm5mc19jbGllbnQtPmNsX3JwY2NsaWVudCwgc2VydmVyLCAmbXNnLA0KPiAtCQkJCSZhcmdz
LnNlcV9hcmdzLCAmcmVzLnNlcV9yZXMsIDApOw0KPiArCXN0cnVjdCBycGNfY2xudCAqY2xudCA9
IHNlcnZlci0+Y2xpZW50Ow0KPiArCWludCBzdGF0dXM7DQo+ICsNCj4gKwlpZiAodXNlX2ludGVn
cml0eSkgew0KPiArCQljbG50ID0gc2VydmVyLT5uZnNfY2xpZW50LT5jbF9ycGNjbGllbnQ7DQo+
ICsJCW1zZy5ycGNfY3JlZCA9IG5mczRfZ2V0X2NsaWRfY3JlZChzZXJ2ZXItPm5mc19jbGllbnQp
Ow0KPiArCX0NCj4gKw0KPiArCWRwcmludGsoIi0tPiAlc1xuIiwgX19mdW5jX18pOw0KPiArCXN0
YXR1cyA9IG5mczRfY2FsbF9zeW5jKGNsbnQsIHNlcnZlciwgJm1zZywgJmFyZ3Muc2VxX2FyZ3Ms
DQo+ICsJCQkJJnJlcy5zZXFfcmVzLCAwKTsNCj4gKwlkcHJpbnRrKCI8LS0gJXMgc3RhdHVzPSVk
XG4iLCBfX2Z1bmNfXywgc3RhdHVzKTsNCj4gKw0KPiArCWlmIChtc2cucnBjX2NyZWQpDQo+ICsJ
CXB1dF9ycGNjcmVkKG1zZy5ycGNfY3JlZCk7DQo+ICsNCj4gKwlyZXR1cm4gc3RhdHVzOw0KPiAg
fQ0KPiAgDQo+ICBzdGF0aWMgaW50DQo+IEBAIC03MzE1LDcgKzczMzEsMjQgQEAgbmZzNDFfcHJv
Y19zZWNpbmZvX25vX25hbWUoc3RydWN0IG5mc19zZXJ2ZXIgKnNlcnZlciwgc3RydWN0IG5mc19m
aCAqZmhhbmRsZSwNCj4gIAlzdHJ1Y3QgbmZzNF9leGNlcHRpb24gZXhjZXB0aW9uID0geyB9Ow0K
PiAgCWludCBlcnI7DQo+ICAJZG8gew0KPiAtCQllcnIgPSBfbmZzNDFfcHJvY19zZWNpbmZvX25v
X25hbWUoc2VydmVyLCBmaGFuZGxlLCBpbmZvLCBmbGF2b3JzKTsNCj4gKwkJLyogZmlyc3QgdHJ5
IHVzaW5nIGludGVncml0eSBwcm90ZWN0aW9uICovDQo+ICsJCWVyciA9IC1ORlM0RVJSX1dST05H
U0VDOw0KPiArDQo+ICsJCS8qIHRyeSB0byB1c2UgaW50ZWdyaXR5IHByb3RlY3Rpb24gd2l0aCBt
YWNoaW5lIGNyZWQgKi8NCj4gKwkJaWYgKF9uZnM0X2lzX2ludGVncml0eV9wcm90ZWN0ZWQoc2Vy
dmVyLT5uZnNfY2xpZW50KSkNCj4gKwkJCWVyciA9IF9uZnM0MV9wcm9jX3NlY2luZm9fbm9fbmFt
ZShzZXJ2ZXIsIGZoYW5kbGUsIGluZm8sDQo+ICsJCQkJCQkJICBmbGF2b3JzLCB0cnVlKTsNCj4g
Kw0KPiArCQkvKg0KPiArCQkgKiBpZiB1bmFibGUgdG8gdXNlIGludGVncml0eSBwcm90ZWN0aW9u
LCBvciBTRUNJTkZPIHdpdGgNCj4gKwkJICogaW50ZWdyaXR5IHByb3RlY3Rpb24gcmV0dXJucyBO
RlM0RVJSX1dST05HU0VDICh3aGljaCBpcw0KPiArCQkgKiBkaXNhbGxvd2VkIGJ5IHNwZWMsIGJ1
dCBleGlzdHMgaW4gZGVwbG95ZWQgc2VydmVycykgdXNlDQo+ICsJCSAqIHRoZSBjdXJyZW50IGZp
bGVzeXN0ZW0ncyBycGNfY2xpZW50IGFuZCB0aGUgdXNlciBjcmVkLg0KPiArCQkgKi8NCj4gKwkJ
aWYgKGVyciA9PSAtTkZTNEVSUl9XUk9OR1NFQykNCj4gKwkJCWVyciA9IF9uZnM0MV9wcm9jX3Nl
Y2luZm9fbm9fbmFtZShzZXJ2ZXIsIGZoYW5kbGUsIGluZm8sDQo+ICsJCQkJCQkJICBmbGF2b3Jz
LCBmYWxzZSk7DQoNCkFzIEkgc2FpZCB5ZXN0ZXJkYXksIFJGQzU2NjEgZm9yYmlkcyBTRUNJTkZP
X05PX05BTUUgZnJvbSByZXR1cm5pbmcNCk5GUzRFUlJfV1JPTkdTRUMsIHNvIHRoaXMgaXMgMTAw
JSBlcXVpdmFsZW50IHRvDQoNCglpZiAoIV9uZnM0X2lzX2ludGVncml0eV9wcm90ZWN0ZWQoKSkN
CgkJZXJyID0gLi4uLg0KDQo+ICsNCj4gIAkJc3dpdGNoIChlcnIpIHsNCj4gIAkJY2FzZSAwOg0K
PiAgCQljYXNlIC1ORlM0RVJSX1dST05HU0VDOg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGlu
dXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFw
cC5jb20NCnd3dy5uZXRhcHAuY29tDQo=
On Sep 5, 2013, at 8:50 AM, Matt W. Benjamin <[email protected]> wrote:
> Hi,
>
> ----- "Dros Adamson" <[email protected]> wrote:
>
>> On Sep 4, 2013, at 12:29 PM, Matt W. Benjamin <[email protected]>
>> wrote:
>>
>>> Hi
>>>
>>> It honestly feels quite odd to me for sec=sys to actually connote
>> krb5i.
>>
>> I should point out that my patches don't introduce the use of krb5i
>> here, they just fix it.
>
> Ack.
>
>>
>> I personally don't think it's weird for the client to use a *more*
>> secure flavor for certain (infrequent) operations when it makes sense.
>> What worries me that currently sec=krb5p can cross a SECINFO boundary
>> and suddenly be using sec=sys!
>
> I think the behavior is obviously reasonable, but giving that policy a
> different name would allow sec=sys to continue mean what it says.
>
I think there is definitely room for discussion on how sec= behavior has changed and how this will affect users, especially when I add the patches mentioned below.
-dros
>>
>> I'm testing patches that fix that now and also allow multiple sec=
>> options (in the same form as nfsd exports, i.e. sec=krb5:krb5i, but
>> I'm trying to fix all the recent regressions surrounding auth flavors
>> / SECINFO first...
>
> That sounds great.
>
>>
>> -dros
>>
>>>
>
> Thanks,
>
> Matt
>
> --
> Matt Benjamin
> The Linux Box
> 206 South Fifth Ave. Suite 150
> Ann Arbor, MI 48104
>
> http://linuxbox.com
>
> tel. 734-761-4689
> fax. 734-769-8938
> cel. 734-216-5309
On Sep 5, 2013, at 10:07 AM, Dr James Bruce Fields <[email protected]> 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" <[email protected]> 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 <[email protected]>
>>>>> ---
>>>>>
>>>>> 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.
-dros
Hi
It honestly feels quite odd to me for sec=sys to actually connote krb5i.
Matt
----- "Weston Andros Adamson" <[email protected]> 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 <[email protected]>
> ---
>
> 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);
> +
> switch (err) {
> case 0:
> case -NFS4ERR_WRONGSEC:
> --
> 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
--
Matt Benjamin
The Linux Box
206 South Fifth Ave. Suite 150
Ann Arbor, MI 48104
http://linuxbox.com
tel. 734-761-4689
fax. 734-769-8938
cel. 734-216-5309
On Sep 5, 2013, at 1:25 PM, "Myklebust, Trond" <[email protected]> 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 <[email protected]>
>> ---
>>
>> 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;
>> }
>
> I have a question: if we use the machine credential, don't we risk a
> NFS4ERR_ACCESS due to the directory permission settings?
>
> I don't see anything in the spec about what permissions you do need for
> a SECINFO_NO_NAME, but since NFS4ERR_ACCESS is listed as a valid return
> code, I'm assuming that the server may enforce lookup permissions?
>
This is a good question. Looking into it?
-dros
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> [email protected]
> http://www.netapp.com
On Thu, Sep 05, 2013 at 05:05:09PM +0000, Adamson, Dros wrote:
>
> On Sep 5, 2013, at 11:31 AM, Dr James Bruce Fields <[email protected]> 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 <[email protected]> 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" <[email protected]> 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 <[email protected]>
> >>>>>>> ---
> >>>>>>>
> >>>>>>> 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.
OK, thanks.--b.
T24gV2VkLCAyMDEzLTA5LTA0IGF0IDEyOjEzIC0wNDAwLCBXZXN0b24gQW5kcm9zIEFkYW1zb24g
d3JvdGU6DQo+IENvbW1pdCA5NzQzMTIwNGVhMDA1ZWM4MDcwYWM5NGJjMzI1MWU4MzZkYWE3Y2E3
IGludHJvZHVjZWQgYSByZWdyZXNzaW9uDQo+IHRoYXQgY2F1c2VzIFNFQ0lORk9fTk9fTkFNRSB0
byBmYWlsIHdpdGhvdXQgc2VuZGluZyBhbiBSUEMgaWY6DQo+IA0KPiAgMSkgdGhlIG5mc19jbGll
bnQncyBycGNfY2xpZW50IGlzIHVzaW5nIGtyYjVpL3AgKG5vdyB0cmllZCBieSBkZWZhdWx0KQ0K
PiAgMikgdGhlIGN1cnJlbnQgdXNlciBkb2Vzbid0IGhhdmUgdmFsaWQga2VyYmVyb3MgY3JlZGVu
dGlhbHMNCj4gDQo+IFRoaXMgc2l0dWF0aW9uIGlzIHF1aXRlIGNvbW1vbiAtIGFzIG9mIG5vdyBh
IHNlYz1zeXMgbW91bnQgd291bGQgdXNlDQo+IGtyYjVpIGZvciB0aGUgbmZzX2NsaWVudCdzIHJw
Y19jbGllbnQgYW5kIGEgdXNlciB3b3VsZCBoYXJkbHkgYmUgZmF1bHRlZA0KPiBmb3Igbm90IGhh
dmluZyBydW4ga2luaXQuDQo+IA0KPiBUaGUgc29sdXRpb24gaXMgdG8gdXNlIHRoZSBtYWNoaW5l
IGNyZWQgd2hlbiB0cnlpbmcgdG8gdXNlIGFuIGludGVncml0eQ0KPiBwcm90ZWN0ZWQgYXV0aCBm
bGF2b3IgZm9yIFNFQ0lORk9fTk9fTkFNRS4NCj4gDQo+IE9sZGVyIHNlcnZlcnMgbWF5IG5vdCBz
dXBwb3J0IHVzaW5nIHRoZSBtYWNoaW5lIGNyZWQgb3IgYW4gaW50ZWdyaXR5DQo+IHByb3RlY3Rl
ZCBhdXRoIGZsYXZvciBmb3IgU0VDSU5GT19OT19OQU1FIGluIGV2ZXJ5IGNpcmN1bXN0YW5jZSwg
c28gd2UgZmFsbA0KPiBiYWNrIHRvIHVzaW5nIHRoZSB1c2VyJ3MgY3JlZCBhbmQgdGhlIGZpbGVz
eXN0ZW0ncyBhdXRoIGZsYXZvciBpbiB0aGlzIGNhc2UuDQo+IA0KPiBXZSBydW4gaW50byBhbm90
aGVyIHByb2JsZW0gd2hlbiBydW5uaW5nIGFnYWluc3QgbGludXggbmZzIHNlcnZlcnMgLQ0KPiB0
aGV5IHJldHVybiBORlM0RVJSX1dST05HU0VDIHdoZW4gdXNpbmcgaW50ZWdyaXR5IGF1dGggZmxh
dm9yICh1bmxlc3MgdGhlDQo+IG1vdW50IGlzIGFsc28gdGhhdCBmbGF2b3IpIGV2ZW4gdGhvdWdo
IHRoYXQgaXMgbm90IGEgdmFsaWQgZXJyb3IgZm9yDQo+IFNFQ0lORk8qLiAgRXZlbiB0aG91Z2gg
aXQncyBhZ2FpbnN0IHNwZWMsIGhhbmRsZSBXUk9OR1NFQyBlcnJvcnMgb24NCj4gU0VDSU5GT19O
T19OQU1FIGJ5IGZhbGxpbmcgYmFjayB0byB1c2luZyB0aGUgdXNlciBjcmVkIGFuZCB0aGUNCj4g
ZmlsZXN5c3RlbSdzIGF1dGggZmxhdm9yLg0KPiANCj4gU2lnbmVkLW9mZi1ieTogV2VzdG9uIEFu
ZHJvcyBBZGFtc29uIDxkcm9zQG5ldGFwcC5jb20+DQo+IC0tLQ0KPiANCj4gVGhpcyBwYXRjaCBn
b2VzIGFsb25nIHdpdGggeWVzdGVyZGF5J3MgU0VDSU5GTyBwYXRjaA0KPiANCj4gIGZzL25mcy9u
ZnM0cHJvYy5jIHwgNDEgKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKy0tLS0N
Cj4gIDEgZmlsZSBjaGFuZ2VkLCAzNyBpbnNlcnRpb25zKCspLCA0IGRlbGV0aW9ucygtKQ0KPiAN
Cj4gZGlmZiAtLWdpdCBhL2ZzL25mcy9uZnM0cHJvYy5jIGIvZnMvbmZzL25mczRwcm9jLmMNCj4g
aW5kZXggYWIxNDYxZS4uNzRiMzdmNSAxMDA2NDQNCj4gLS0tIGEvZnMvbmZzL25mczRwcm9jLmMN
Cj4gKysrIGIvZnMvbmZzL25mczRwcm9jLmMNCj4gQEAgLTcyOTEsNyArNzI5MSw4IEBAIG91dDoN
Cj4gICAqLw0KPiAgc3RhdGljIGludA0KPiAgX25mczQxX3Byb2Nfc2VjaW5mb19ub19uYW1lKHN0
cnVjdCBuZnNfc2VydmVyICpzZXJ2ZXIsIHN0cnVjdCBuZnNfZmggKmZoYW5kbGUsDQo+IC0JCSAg
ICBzdHJ1Y3QgbmZzX2ZzaW5mbyAqaW5mbywgc3RydWN0IG5mczRfc2VjaW5mb19mbGF2b3JzICpm
bGF2b3JzKQ0KPiArCQkgICAgc3RydWN0IG5mc19mc2luZm8gKmluZm8sDQo+ICsJCSAgICBzdHJ1
Y3QgbmZzNF9zZWNpbmZvX2ZsYXZvcnMgKmZsYXZvcnMsIGJvb2wgdXNlX2ludGVncml0eSkNCj4g
IHsNCj4gIAlzdHJ1Y3QgbmZzNDFfc2VjaW5mb19ub19uYW1lX2FyZ3MgYXJncyA9IHsNCj4gIAkJ
LnN0eWxlID0gU0VDSU5GT19TVFlMRV9DVVJSRU5UX0ZILA0KPiBAQCAtNzMwNCw4ICs3MzA1LDIz
IEBAIF9uZnM0MV9wcm9jX3NlY2luZm9fbm9fbmFtZShzdHJ1Y3QgbmZzX3NlcnZlciAqc2VydmVy
LCBzdHJ1Y3QgbmZzX2ZoICpmaGFuZGxlLA0KPiAgCQkucnBjX2FyZ3AgPSAmYXJncywNCj4gIAkJ
LnJwY19yZXNwID0gJnJlcywNCj4gIAl9Ow0KPiAtCXJldHVybiBuZnM0X2NhbGxfc3luYyhzZXJ2
ZXItPm5mc19jbGllbnQtPmNsX3JwY2NsaWVudCwgc2VydmVyLCAmbXNnLA0KPiAtCQkJCSZhcmdz
LnNlcV9hcmdzLCAmcmVzLnNlcV9yZXMsIDApOw0KPiArCXN0cnVjdCBycGNfY2xudCAqY2xudCA9
IHNlcnZlci0+Y2xpZW50Ow0KPiArCWludCBzdGF0dXM7DQo+ICsNCj4gKwlpZiAodXNlX2ludGVn
cml0eSkgew0KPiArCQljbG50ID0gc2VydmVyLT5uZnNfY2xpZW50LT5jbF9ycGNjbGllbnQ7DQo+
ICsJCW1zZy5ycGNfY3JlZCA9IG5mczRfZ2V0X2NsaWRfY3JlZChzZXJ2ZXItPm5mc19jbGllbnQp
Ow0KPiArCX0NCj4gKw0KPiArCWRwcmludGsoIi0tPiAlc1xuIiwgX19mdW5jX18pOw0KPiArCXN0
YXR1cyA9IG5mczRfY2FsbF9zeW5jKGNsbnQsIHNlcnZlciwgJm1zZywgJmFyZ3Muc2VxX2FyZ3Ms
DQo+ICsJCQkJJnJlcy5zZXFfcmVzLCAwKTsNCj4gKwlkcHJpbnRrKCI8LS0gJXMgc3RhdHVzPSVk
XG4iLCBfX2Z1bmNfXywgc3RhdHVzKTsNCj4gKw0KPiArCWlmIChtc2cucnBjX2NyZWQpDQo+ICsJ
CXB1dF9ycGNjcmVkKG1zZy5ycGNfY3JlZCk7DQo+ICsNCj4gKwlyZXR1cm4gc3RhdHVzOw0KPiAg
fQ0KDQpJIGhhdmUgYSBxdWVzdGlvbjogaWYgd2UgdXNlIHRoZSBtYWNoaW5lIGNyZWRlbnRpYWws
IGRvbid0IHdlIHJpc2sgYQ0KTkZTNEVSUl9BQ0NFU1MgZHVlIHRvIHRoZSBkaXJlY3RvcnkgcGVy
bWlzc2lvbiBzZXR0aW5ncz8NCg0KSSBkb24ndCBzZWUgYW55dGhpbmcgaW4gdGhlIHNwZWMgYWJv
dXQgd2hhdCBwZXJtaXNzaW9ucyB5b3UgZG8gbmVlZCBmb3INCmEgU0VDSU5GT19OT19OQU1FLCBi
dXQgc2luY2UgTkZTNEVSUl9BQ0NFU1MgaXMgbGlzdGVkIGFzIGEgdmFsaWQgcmV0dXJuDQpjb2Rl
LCBJJ20gYXNzdW1pbmcgdGhhdCB0aGUgc2VydmVyIG1heSBlbmZvcmNlIGxvb2t1cCBwZXJtaXNz
aW9ucy4uLg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFp
bmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29t
DQo=
On Thu, Sep 05, 2013 at 05:25:48PM +0000, 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 <[email protected]>
> > ---
> >
> > 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;
> > }
>
> I have a question: if we use the machine credential, don't we risk a
> NFS4ERR_ACCESS due to the directory permission settings?
>
> I don't see anything in the spec about what permissions you do need for
> a SECINFO_NO_NAME, but since NFS4ERR_ACCESS is listed as a valid return
> code, I'm assuming that the server may enforce lookup permissions...
It needs to in the STYLE4_PARENT case, but it would seem very strange in
the CURRENT_FH case where there's no real "lookup".
(If anyone's allowed SECINFO_NO_NAME with CURRENT_FH, would that give
away too much information? I'm not sure. I don't think many
administrators would be surprised by the fact that anyone could discover
the set of exported filesystems and their security flavors. People are
used to that being visible through showmount, for example.
Maybe there's some scenario where somebody would care just about the
ability to probe whether a given filehandle is valid or not, e.g. to
see whether a given file had been deleted or not. But you can already
get that information by sending a bare PUTFH and checking the error.
We could close that minor information leak by checking the export flavor
as soon as we've parsed enough of the filehandle to identify the export,
before trying to find the actual file. And SECINFO_NO_NAME with
CURRENT_FH could similarly be allowed to succeed as soon as we've
identified the export (we don't need any more as long as security
flavors are set only per-export). I'm not sure anyone cares.)
--b.
T24gV2VkLCAyMDEzLTA5LTA0IGF0IDE2OjQ4ICswMDAwLCBBZGFtc29uLCBEcm9zIHdyb3RlOg0K
PiBPbiBTZXAgNCwgMjAxMywgYXQgMTI6MjQgUE0sICJNeWtsZWJ1c3QsIFRyb25kIiA8VHJvbmQu
TXlrbGVidXN0QG5ldGFwcC5jb20+IHdyb3RlOg0KPiANCj4gPiBPbiBXZWQsIDIwMTMtMDktMDQg
YXQgMTI6MTMgLTA0MDAsIFdlc3RvbiBBbmRyb3MgQWRhbXNvbiB3cm90ZToNCj4gPj4gQ29tbWl0
IDk3NDMxMjA0ZWEwMDVlYzgwNzBhYzk0YmMzMjUxZTgzNmRhYTdjYTcgaW50cm9kdWNlZCBhIHJl
Z3Jlc3Npb24NCj4gPj4gdGhhdCBjYXVzZXMgU0VDSU5GT19OT19OQU1FIHRvIGZhaWwgd2l0aG91
dCBzZW5kaW5nIGFuIFJQQyBpZjoNCj4gPj4gDQo+ID4+IDEpIHRoZSBuZnNfY2xpZW50J3MgcnBj
X2NsaWVudCBpcyB1c2luZyBrcmI1aS9wIChub3cgdHJpZWQgYnkgZGVmYXVsdCkNCj4gPj4gMikg
dGhlIGN1cnJlbnQgdXNlciBkb2Vzbid0IGhhdmUgdmFsaWQga2VyYmVyb3MgY3JlZGVudGlhbHMN
Cj4gPj4gDQo+ID4+IFRoaXMgc2l0dWF0aW9uIGlzIHF1aXRlIGNvbW1vbiAtIGFzIG9mIG5vdyBh
IHNlYz1zeXMgbW91bnQgd291bGQgdXNlDQo+ID4+IGtyYjVpIGZvciB0aGUgbmZzX2NsaWVudCdz
IHJwY19jbGllbnQgYW5kIGEgdXNlciB3b3VsZCBoYXJkbHkgYmUgZmF1bHRlZA0KPiA+PiBmb3Ig
bm90IGhhdmluZyBydW4ga2luaXQuDQo+ID4+IA0KPiA+PiBUaGUgc29sdXRpb24gaXMgdG8gdXNl
IHRoZSBtYWNoaW5lIGNyZWQgd2hlbiB0cnlpbmcgdG8gdXNlIGFuIGludGVncml0eQ0KPiA+PiBw
cm90ZWN0ZWQgYXV0aCBmbGF2b3IgZm9yIFNFQ0lORk9fTk9fTkFNRS4NCj4gPj4gDQo+ID4+IE9s
ZGVyIHNlcnZlcnMgbWF5IG5vdCBzdXBwb3J0IHVzaW5nIHRoZSBtYWNoaW5lIGNyZWQgb3IgYW4g
aW50ZWdyaXR5DQo+ID4+IHByb3RlY3RlZCBhdXRoIGZsYXZvciBmb3IgU0VDSU5GT19OT19OQU1F
IGluIGV2ZXJ5IGNpcmN1bXN0YW5jZSwgc28gd2UgZmFsbA0KPiA+PiBiYWNrIHRvIHVzaW5nIHRo
ZSB1c2VyJ3MgY3JlZCBhbmQgdGhlIGZpbGVzeXN0ZW0ncyBhdXRoIGZsYXZvciBpbiB0aGlzIGNh
c2UuDQo+ID4+IA0KPiA+PiBXZSBydW4gaW50byBhbm90aGVyIHByb2JsZW0gd2hlbiBydW5uaW5n
IGFnYWluc3QgbGludXggbmZzIHNlcnZlcnMgLQ0KPiA+PiB0aGV5IHJldHVybiBORlM0RVJSX1dS
T05HU0VDIHdoZW4gdXNpbmcgaW50ZWdyaXR5IGF1dGggZmxhdm9yICh1bmxlc3MgdGhlDQo+ID4+
IG1vdW50IGlzIGFsc28gdGhhdCBmbGF2b3IpIGV2ZW4gdGhvdWdoIHRoYXQgaXMgbm90IGEgdmFs
aWQgZXJyb3IgZm9yDQo+ID4+IFNFQ0lORk8qLiAgRXZlbiB0aG91Z2ggaXQncyBhZ2FpbnN0IHNw
ZWMsIGhhbmRsZSBXUk9OR1NFQyBlcnJvcnMgb24NCj4gPj4gU0VDSU5GT19OT19OQU1FIGJ5IGZh
bGxpbmcgYmFjayB0byB1c2luZyB0aGUgdXNlciBjcmVkIGFuZCB0aGUNCj4gPj4gZmlsZXN5c3Rl
bSdzIGF1dGggZmxhdm9yLg0KPiA+PiANCj4gPj4gU2lnbmVkLW9mZi1ieTogV2VzdG9uIEFuZHJv
cyBBZGFtc29uIDxkcm9zQG5ldGFwcC5jb20+DQo+ID4+IC0tLQ0KPiA+PiANCj4gPj4gVGhpcyBw
YXRjaCBnb2VzIGFsb25nIHdpdGggeWVzdGVyZGF5J3MgU0VDSU5GTyBwYXRjaA0KPiA+PiANCj4g
Pj4gZnMvbmZzL25mczRwcm9jLmMgfCA0MSArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr
KysrKysrLS0tLQ0KPiA+PiAxIGZpbGUgY2hhbmdlZCwgMzcgaW5zZXJ0aW9ucygrKSwgNCBkZWxl
dGlvbnMoLSkNCj4gPj4gDQo+ID4+IGRpZmYgLS1naXQgYS9mcy9uZnMvbmZzNHByb2MuYyBiL2Zz
L25mcy9uZnM0cHJvYy5jDQo+ID4+IGluZGV4IGFiMTQ2MWUuLjc0YjM3ZjUgMTAwNjQ0DQo+ID4+
IC0tLSBhL2ZzL25mcy9uZnM0cHJvYy5jDQo+ID4+ICsrKyBiL2ZzL25mcy9uZnM0cHJvYy5jDQo+
ID4+IEBAIC03MjkxLDcgKzcyOTEsOCBAQCBvdXQ6DQo+ID4+ICAqLw0KPiA+PiBzdGF0aWMgaW50
DQo+ID4+IF9uZnM0MV9wcm9jX3NlY2luZm9fbm9fbmFtZShzdHJ1Y3QgbmZzX3NlcnZlciAqc2Vy
dmVyLCBzdHJ1Y3QgbmZzX2ZoICpmaGFuZGxlLA0KPiA+PiAtCQkgICAgc3RydWN0IG5mc19mc2lu
Zm8gKmluZm8sIHN0cnVjdCBuZnM0X3NlY2luZm9fZmxhdm9ycyAqZmxhdm9ycykNCj4gPj4gKwkJ
ICAgIHN0cnVjdCBuZnNfZnNpbmZvICppbmZvLA0KPiA+PiArCQkgICAgc3RydWN0IG5mczRfc2Vj
aW5mb19mbGF2b3JzICpmbGF2b3JzLCBib29sIHVzZV9pbnRlZ3JpdHkpDQo+ID4+IHsNCj4gPj4g
CXN0cnVjdCBuZnM0MV9zZWNpbmZvX25vX25hbWVfYXJncyBhcmdzID0gew0KPiA+PiAJCS5zdHls
ZSA9IFNFQ0lORk9fU1RZTEVfQ1VSUkVOVF9GSCwNCj4gPj4gQEAgLTczMDQsOCArNzMwNSwyMyBA
QCBfbmZzNDFfcHJvY19zZWNpbmZvX25vX25hbWUoc3RydWN0IG5mc19zZXJ2ZXIgKnNlcnZlciwg
c3RydWN0IG5mc19maCAqZmhhbmRsZSwNCj4gPj4gCQkucnBjX2FyZ3AgPSAmYXJncywNCj4gPj4g
CQkucnBjX3Jlc3AgPSAmcmVzLA0KPiA+PiAJfTsNCj4gPj4gLQlyZXR1cm4gbmZzNF9jYWxsX3N5
bmMoc2VydmVyLT5uZnNfY2xpZW50LT5jbF9ycGNjbGllbnQsIHNlcnZlciwgJm1zZywNCj4gPj4g
LQkJCQkmYXJncy5zZXFfYXJncywgJnJlcy5zZXFfcmVzLCAwKTsNCj4gPj4gKwlzdHJ1Y3QgcnBj
X2NsbnQgKmNsbnQgPSBzZXJ2ZXItPmNsaWVudDsNCj4gPj4gKwlpbnQgc3RhdHVzOw0KPiA+PiAr
DQo+ID4+ICsJaWYgKHVzZV9pbnRlZ3JpdHkpIHsNCj4gPj4gKwkJY2xudCA9IHNlcnZlci0+bmZz
X2NsaWVudC0+Y2xfcnBjY2xpZW50Ow0KPiA+PiArCQltc2cucnBjX2NyZWQgPSBuZnM0X2dldF9j
bGlkX2NyZWQoc2VydmVyLT5uZnNfY2xpZW50KTsNCj4gPj4gKwl9DQo+ID4+ICsNCj4gPj4gKwlk
cHJpbnRrKCItLT4gJXNcbiIsIF9fZnVuY19fKTsNCj4gPj4gKwlzdGF0dXMgPSBuZnM0X2NhbGxf
c3luYyhjbG50LCBzZXJ2ZXIsICZtc2csICZhcmdzLnNlcV9hcmdzLA0KPiA+PiArCQkJCSZyZXMu
c2VxX3JlcywgMCk7DQo+ID4+ICsJZHByaW50aygiPC0tICVzIHN0YXR1cz0lZFxuIiwgX19mdW5j
X18sIHN0YXR1cyk7DQo+ID4+ICsNCj4gPj4gKwlpZiAobXNnLnJwY19jcmVkKQ0KPiA+PiArCQlw
dXRfcnBjY3JlZChtc2cucnBjX2NyZWQpOw0KPiA+PiArDQo+ID4+ICsJcmV0dXJuIHN0YXR1czsN
Cj4gPj4gfQ0KPiA+PiANCj4gPj4gc3RhdGljIGludA0KPiA+PiBAQCAtNzMxNSw3ICs3MzMxLDI0
IEBAIG5mczQxX3Byb2Nfc2VjaW5mb19ub19uYW1lKHN0cnVjdCBuZnNfc2VydmVyICpzZXJ2ZXIs
IHN0cnVjdCBuZnNfZmggKmZoYW5kbGUsDQo+ID4+IAlzdHJ1Y3QgbmZzNF9leGNlcHRpb24gZXhj
ZXB0aW9uID0geyB9Ow0KPiA+PiAJaW50IGVycjsNCj4gPj4gCWRvIHsNCj4gPj4gLQkJZXJyID0g
X25mczQxX3Byb2Nfc2VjaW5mb19ub19uYW1lKHNlcnZlciwgZmhhbmRsZSwgaW5mbywgZmxhdm9y
cyk7DQo+ID4+ICsJCS8qIGZpcnN0IHRyeSB1c2luZyBpbnRlZ3JpdHkgcHJvdGVjdGlvbiAqLw0K
PiA+PiArCQllcnIgPSAtTkZTNEVSUl9XUk9OR1NFQzsNCj4gPj4gKw0KPiA+PiArCQkvKiB0cnkg
dG8gdXNlIGludGVncml0eSBwcm90ZWN0aW9uIHdpdGggbWFjaGluZSBjcmVkICovDQo+ID4+ICsJ
CWlmIChfbmZzNF9pc19pbnRlZ3JpdHlfcHJvdGVjdGVkKHNlcnZlci0+bmZzX2NsaWVudCkpDQo+
ID4+ICsJCQllcnIgPSBfbmZzNDFfcHJvY19zZWNpbmZvX25vX25hbWUoc2VydmVyLCBmaGFuZGxl
LCBpbmZvLA0KPiA+PiArCQkJCQkJCSAgZmxhdm9ycywgdHJ1ZSk7DQo+ID4+ICsNCj4gPj4gKwkJ
LyoNCj4gPj4gKwkJICogaWYgdW5hYmxlIHRvIHVzZSBpbnRlZ3JpdHkgcHJvdGVjdGlvbiwgb3Ig
U0VDSU5GTyB3aXRoDQo+ID4+ICsJCSAqIGludGVncml0eSBwcm90ZWN0aW9uIHJldHVybnMgTkZT
NEVSUl9XUk9OR1NFQyAod2hpY2ggaXMNCj4gPj4gKwkJICogZGlzYWxsb3dlZCBieSBzcGVjLCBi
dXQgZXhpc3RzIGluIGRlcGxveWVkIHNlcnZlcnMpIHVzZQ0KPiA+PiArCQkgKiB0aGUgY3VycmVu
dCBmaWxlc3lzdGVtJ3MgcnBjX2NsaWVudCBhbmQgdGhlIHVzZXIgY3JlZC4NCj4gPj4gKwkJICov
DQo+ID4+ICsJCWlmIChlcnIgPT0gLU5GUzRFUlJfV1JPTkdTRUMpDQo+ID4+ICsJCQllcnIgPSBf
bmZzNDFfcHJvY19zZWNpbmZvX25vX25hbWUoc2VydmVyLCBmaGFuZGxlLCBpbmZvLA0KPiA+PiAr
CQkJCQkJCSAgZmxhdm9ycywgZmFsc2UpOw0KPiA+IA0KPiA+IEFzIEkgc2FpZCB5ZXN0ZXJkYXks
IFJGQzU2NjEgZm9yYmlkcyBTRUNJTkZPX05PX05BTUUgZnJvbSByZXR1cm5pbmcNCj4gPiBORlM0
RVJSX1dST05HU0VDLCBzbyB0aGlzIGlzIDEwMCUgZXF1aXZhbGVudCB0bw0KPiA+IA0KPiA+IAlp
ZiAoIV9uZnM0X2lzX2ludGVncml0eV9wcm90ZWN0ZWQoKSkNCj4gPiAJCWVyciA9IOKApi4NCj4g
DQo+IFJpZ2h0LCBidXQgSSB0aG91Z2h0IHdlIHdlcmUgZG9pbmcgdGhpcyB0byBzdXBwb3J0IHNl
cnZlciBpbXBsZW1lbnRhdGlvbnMgbGlrZSBsaW51eCB0aGF0ICpkbyogcmV0dXJuIE5GUzRFUlJf
V1JPTkdTRUMgb24gU0VDSU5GT19OT19OQU1FIGV2ZW4gdGhvdWdoIGl0J3MgZm9yYmlkZGVuLiAg
SSBrbm93IHdlIG5vcm1hbGx5IGRvbid0IHdvcmsgYXJvdW5kIHNlcnZlciBidWdzLCBidXQgdGhp
cyBzZWVtcyBwcmV0dHkgc2ltcGxlLg0KPiANCj4gSWYgd2UgZG9uJ3QgZG8gdGhpcywgdGhlbiBT
RUNJTkZPX05PX05BTUUgd2lsbCBhbHdheXMgZmFpbCBhZ2FpbnN0IGN1cnJlbnQgbGludXggc2V2
ZXJzIG5vIG1hdHRlciB3aGF0IHRoZSBtb3VudCBvcHRpb25zIC0gdW5sZXNzIGtyYjVpL3AgaXMg
dW51c2FibGUgKG5vdCBjb25maWd1cmVkLCB0aW1lIHNrZXcsIG5vIG1hY2hpbmUgY3JlZCwgZXRj
KS4NCg0KQnJ1Y2UsIHlvdSdyZSBpdDogd2hhdCdzIHRoZSBkZWFsIGhlcmU/DQoNCi0tIA0KVHJv
bmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9u
ZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg==
On Sep 4, 2013, at 12:29 PM, Matt W. Benjamin <[email protected]> wrote:
> Hi
>
> It honestly feels quite odd to me for sec=sys to actually connote krb5i.
I should point out that my patches don't introduce the use of krb5i here, they just fix it.
I personally don't think it's weird for the client to use a *more* secure flavor for certain (infrequent) operations when it makes sense. What worries me that currently sec=krb5p can cross a SECINFO boundary and suddenly be using sec=sys!
I'm testing patches that fix that now and also allow multiple sec= options (in the same form as nfsd exports, i.e. sec=krb5:krb5i, but I'm trying to fix all the recent regressions surrounding auth flavors / SECINFO first...
-dros
>
> Matt
>
> ----- "Weston Andros Adamson" <[email protected]> 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 <[email protected]>
>> ---
>>
>> 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);
>> +
>> switch (err) {
>> case 0:
>> case -NFS4ERR_WRONGSEC:
>> --
>> 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
>
> --
> Matt Benjamin
> The Linux Box
> 206 South Fifth Ave. Suite 150
> Ann Arbor, MI 48104
>
> http://linuxbox.com
>
> tel. 734-761-4689
> fax. 734-769-8938
> cel. 734-216-5309