2011-06-28 18:25:24

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 0/2] Allow mounting exports that advertise NULL security

The Linux NFS client fails to mount exports that advertise only
AUTH_NULL. This small patch set, against 3.0-rc4, addresses the
problem, and corrects the behavior of the client when AUTH_NULL
appears anywhere in the returned authflavor list.

This behavioral requirement is not documented in any NFS protocol
specification. Tom is considering some language for the active NFSv4
specifications to provide some guidance for client implementors on how
to interpret authflavor lists.

Eventually there should be a bugzilla filed against RHEL 6 by the
reporter. When that happens, the bugzilla number should be documented
in the patch description of the second patch.

This problem has existed since at least 2.6.32. This fix may be
appropriate for stable kernels.

---

Chuck Lever (2):
NFS: Allow sec=none mounts in certain cases
NFS: Clean up nfs_walk_authlist()


fs/nfs/super.c | 54 +++++++++++++++++++++++++++++++-----------------------
1 files changed, 31 insertions(+), 23 deletions(-)

--
Chuck Lever


2011-06-28 19:06:40

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFS: Allow sec=none mounts in certain cases

On Tue, Jun 28, 2011 at 02:25:41PM -0400, Chuck Lever wrote:
> There is an undocumented convention (verified by reviewing network
> traces from a NetApp filer and a Solaris NFS server) where a server
> that returns a mount authflavor list containing an AUTH_NULL entry
> is actually indicating it will accept any security flavor for the
> export being mounted.

This is only in the case of NLM? (Not v4 secinfo?)

--b.

>
> This might be used when the server maps all security flavors into the
> same security mode. For example, the server treats all accessors as,
> say, UID 17.
>
> Essentially, AUTH_NULL is treated as a wildcard that matches the
> flavor the mounter requested.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>
> fs/nfs/super.c | 15 +++++++++++----
> 1 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 4625a4c..543cf9f 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1570,13 +1570,20 @@ static int nfs_walk_authlist(struct nfs_parsed_mount_data *args,
> * the first flavor in the list that it supports, on the
> * assumption that the best access is provided by the first
> * flavor."
> + *
> + * By convention we treat AUTH_NULL in the returned list as
> + * a wild card. The server will map our requested flavor to
> + * something else.
> */
> - for (i = 0; i < args->auth_flavor_len; i++)
> - for (j = 0; j < server_authlist_len; j++)
> - if (args->auth_flavors[i] == server->auth_flavs[j]) {
> - args->auth_flavors[0] = server->auth_flavs[j];
> + for (i = 0; i < server_authlist_len; i++) {
> + if (server->auth_flavs[i] == RPC_AUTH_NULL)
> + goto out;
> + for (j = 0; j < args->auth_flavor_len; j++)
> + if (server->auth_flavs[i] == args->auth_flavors[j]) {
> + args->auth_flavors[0] = server->auth_flavs[i];
> goto out;
> }
> + }
>
> dfprintk(MOUNT, "NFS: server does not support requested auth flavor\n");
> nfs_umount(server);
>
> --
> 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

2011-06-28 18:25:35

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 1/2] NFS: Clean up nfs_walk_authlist()

Clean up: Add common exit labels in preparation for the following
patch. Clarify comments. Rename variable for clarity.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/nfs/super.c | 45 +++++++++++++++++++++++----------------------
1 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index ce40e5c..4625a4c 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1541,15 +1541,19 @@ out_security_failure:
}

/*
- * Match the requested auth flavors with the list returned by
- * the server. Returns zero and sets the mount's authentication
- * flavor on success; returns -EACCES if server does not support
- * the requested flavor.
+ * Match the requested auth flavors with the list returned by the
+ * server. args->auth_flavors contains a list of security flavors
+ * to check for. The caller has already set up the list of flavors
+ * with the default (usually AUTH_SYS).
+ *
+ * Returns zero on success; the authentication flavor in args->
+ * auth_flavors[0] should be used for this mount. Otherwise, returns
+ * -EACCES if server does not support the requested flavor.
*/
static int nfs_walk_authlist(struct nfs_parsed_mount_data *args,
- struct nfs_mount_request *request)
+ struct nfs_mount_request *server)
{
- unsigned int i, j, server_authlist_len = *(request->auth_flav_len);
+ unsigned int i, j, server_authlist_len = *(server->auth_flav_len);

/*
* Certain releases of Linux's mountd return an empty
@@ -1559,31 +1563,28 @@ static int nfs_walk_authlist(struct nfs_parsed_mount_data *args,
* if the returned flavor list is empty.
*/
if (server_authlist_len == 0)
- return 0;
+ goto out;

/*
- * We avoid sophisticated negotiating here, as there are
- * plenty of cases where we can get it wrong, providing
- * either too little or too much security.
- *
- * RFC 2623, section 2.7 suggests we SHOULD prefer the
- * flavor listed first. However, some servers list
- * AUTH_NULL first. Our caller plants AUTH_SYS, the
- * preferred default, in args->auth_flavors[0] if user
- * didn't specify sec= mount option.
+ * RFC 2623 section 2.7 says "... a NFS client SHOULD use
+ * the first flavor in the list that it supports, on the
+ * assumption that the best access is provided by the first
+ * flavor."
*/
for (i = 0; i < args->auth_flavor_len; i++)
for (j = 0; j < server_authlist_len; j++)
- if (args->auth_flavors[i] == request->auth_flavs[j]) {
- dfprintk(MOUNT, "NFS: using auth flavor %d\n",
- request->auth_flavs[j]);
- args->auth_flavors[0] = request->auth_flavs[j];
- return 0;
+ if (args->auth_flavors[i] == server->auth_flavs[j]) {
+ args->auth_flavors[0] = server->auth_flavs[j];
+ goto out;
}

dfprintk(MOUNT, "NFS: server does not support requested auth flavor\n");
- nfs_umount(request);
+ nfs_umount(server);
return -EACCES;
+
+out:
+ dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]);
+ return 0;
}

/*


2011-06-28 19:10:05

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFS: Allow sec=none mounts in certain cases

On Tue, Jun 28, 2011 at 03:06:40PM -0400, bfields wrote:
> On Tue, Jun 28, 2011 at 02:25:41PM -0400, Chuck Lever wrote:
> > There is an undocumented convention (verified by reviewing network
> > traces from a NetApp filer and a Solaris NFS server) where a server
> > that returns a mount authflavor list containing an AUTH_NULL entry
> > is actually indicating it will accept any security flavor for the
> > export being mounted.
>
> This is only in the case of NLM? (Not v4 secinfo?)
^^^
(Sorry, I meant MOUNT !)
>
> --b.
>
> >
> > This might be used when the server maps all security flavors into the
> > same security mode. For example, the server treats all accessors as,
> > say, UID 17.
> >
> > Essentially, AUTH_NULL is treated as a wildcard that matches the
> > flavor the mounter requested.
> >
> > Signed-off-by: Chuck Lever <[email protected]>
> > ---
> >
> > fs/nfs/super.c | 15 +++++++++++----
> > 1 files changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 4625a4c..543cf9f 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -1570,13 +1570,20 @@ static int nfs_walk_authlist(struct nfs_parsed_mount_data *args,
> > * the first flavor in the list that it supports, on the
> > * assumption that the best access is provided by the first
> > * flavor."
> > + *
> > + * By convention we treat AUTH_NULL in the returned list as
> > + * a wild card. The server will map our requested flavor to
> > + * something else.
> > */
> > - for (i = 0; i < args->auth_flavor_len; i++)
> > - for (j = 0; j < server_authlist_len; j++)
> > - if (args->auth_flavors[i] == server->auth_flavs[j]) {
> > - args->auth_flavors[0] = server->auth_flavs[j];
> > + for (i = 0; i < server_authlist_len; i++) {
> > + if (server->auth_flavs[i] == RPC_AUTH_NULL)
> > + goto out;
> > + for (j = 0; j < args->auth_flavor_len; j++)
> > + if (server->auth_flavs[i] == args->auth_flavors[j]) {
> > + args->auth_flavors[0] = server->auth_flavs[i];
> > goto out;
> > }
> > + }
> >
> > dfprintk(MOUNT, "NFS: server does not support requested auth flavor\n");
> > nfs_umount(server);
> >
> > --
> > 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

2011-06-28 19:34:41

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFS: Allow sec=none mounts in certain cases


On Jun 28, 2011, at 3:10 PM, J. Bruce Fields wrote:

> On Tue, Jun 28, 2011 at 03:06:40PM -0400, bfields wrote:
>> On Tue, Jun 28, 2011 at 02:25:41PM -0400, Chuck Lever wrote:
>>> There is an undocumented convention (verified by reviewing network
>>> traces from a NetApp filer and a Solaris NFS server) where a server
>>> that returns a mount authflavor list containing an AUTH_NULL entry
>>> is actually indicating it will accept any security flavor for the
>>> export being mounted.
>>
>> This is only in the case of NLM? (Not v4 secinfo?)
> ^^^
> (Sorry, I meant MOUNT !)

Right, this fix is specifically for the kernel's NFSv3 mount client. I expect that SECINFO is probably the area where NFSv4 spec changes might help, but I haven't touched that in these patches.

At some point Bryan, Trond, and I should discuss how we can unify these pieces, and teach them how to determine the list of locally supported authflavors. Maybe this is done already?

>>
>> --b.
>>
>>>
>>> This might be used when the server maps all security flavors into the
>>> same security mode. For example, the server treats all accessors as,
>>> say, UID 17.
>>>
>>> Essentially, AUTH_NULL is treated as a wildcard that matches the
>>> flavor the mounter requested.
>>>
>>> Signed-off-by: Chuck Lever <[email protected]>
>>> ---
>>>
>>> fs/nfs/super.c | 15 +++++++++++----
>>> 1 files changed, 11 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>> index 4625a4c..543cf9f 100644
>>> --- a/fs/nfs/super.c
>>> +++ b/fs/nfs/super.c
>>> @@ -1570,13 +1570,20 @@ static int nfs_walk_authlist(struct nfs_parsed_mount_data *args,
>>> * the first flavor in the list that it supports, on the
>>> * assumption that the best access is provided by the first
>>> * flavor."
>>> + *
>>> + * By convention we treat AUTH_NULL in the returned list as
>>> + * a wild card. The server will map our requested flavor to
>>> + * something else.
>>> */
>>> - for (i = 0; i < args->auth_flavor_len; i++)
>>> - for (j = 0; j < server_authlist_len; j++)
>>> - if (args->auth_flavors[i] == server->auth_flavs[j]) {
>>> - args->auth_flavors[0] = server->auth_flavs[j];
>>> + for (i = 0; i < server_authlist_len; i++) {
>>> + if (server->auth_flavs[i] == RPC_AUTH_NULL)
>>> + goto out;
>>> + for (j = 0; j < args->auth_flavor_len; j++)
>>> + if (server->auth_flavs[i] == args->auth_flavors[j]) {
>>> + args->auth_flavors[0] = server->auth_flavs[i];
>>> goto out;
>>> }
>>> + }
>>>
>>> dfprintk(MOUNT, "NFS: server does not support requested auth flavor\n");
>>> nfs_umount(server);
>>>
>>> --
>>> 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

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





2011-06-28 18:41:55

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFS: Allow sec=none mounts in certain cases

I think the short patch description here is misleading. This patch does not explicitly allow the "sec=none" mount option to be used. I'll update this if/when I redrive the patches.


On Jun 28, 2011, at 2:25 PM, Chuck Lever wrote:

> There is an undocumented convention (verified by reviewing network
> traces from a NetApp filer and a Solaris NFS server) where a server
> that returns a mount authflavor list containing an AUTH_NULL entry
> is actually indicating it will accept any security flavor for the
> export being mounted.
>
> This might be used when the server maps all security flavors into the
> same security mode. For example, the server treats all accessors as,
> say, UID 17.
>
> Essentially, AUTH_NULL is treated as a wildcard that matches the
> flavor the mounter requested.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>
> fs/nfs/super.c | 15 +++++++++++----
> 1 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 4625a4c..543cf9f 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1570,13 +1570,20 @@ static int nfs_walk_authlist(struct nfs_parsed_mount_data *args,
> * the first flavor in the list that it supports, on the
> * assumption that the best access is provided by the first
> * flavor."
> + *
> + * By convention we treat AUTH_NULL in the returned list as
> + * a wild card. The server will map our requested flavor to
> + * something else.
> */
> - for (i = 0; i < args->auth_flavor_len; i++)
> - for (j = 0; j < server_authlist_len; j++)
> - if (args->auth_flavors[i] == server->auth_flavs[j]) {
> - args->auth_flavors[0] = server->auth_flavs[j];
> + for (i = 0; i < server_authlist_len; i++) {
> + if (server->auth_flavs[i] == RPC_AUTH_NULL)
> + goto out;
> + for (j = 0; j < args->auth_flavor_len; j++)
> + if (server->auth_flavs[i] == args->auth_flavors[j]) {
> + args->auth_flavors[0] = server->auth_flavs[i];
> goto out;
> }
> + }
>
> dfprintk(MOUNT, "NFS: server does not support requested auth flavor\n");
> nfs_umount(server);
>
> --
> 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

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





2011-06-28 18:25:44

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 2/2] NFS: Allow sec=none mounts in certain cases

There is an undocumented convention (verified by reviewing network
traces from a NetApp filer and a Solaris NFS server) where a server
that returns a mount authflavor list containing an AUTH_NULL entry
is actually indicating it will accept any security flavor for the
export being mounted.

This might be used when the server maps all security flavors into the
same security mode. For example, the server treats all accessors as,
say, UID 17.

Essentially, AUTH_NULL is treated as a wildcard that matches the
flavor the mounter requested.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/nfs/super.c | 15 +++++++++++----
1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 4625a4c..543cf9f 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1570,13 +1570,20 @@ static int nfs_walk_authlist(struct nfs_parsed_mount_data *args,
* the first flavor in the list that it supports, on the
* assumption that the best access is provided by the first
* flavor."
+ *
+ * By convention we treat AUTH_NULL in the returned list as
+ * a wild card. The server will map our requested flavor to
+ * something else.
*/
- for (i = 0; i < args->auth_flavor_len; i++)
- for (j = 0; j < server_authlist_len; j++)
- if (args->auth_flavors[i] == server->auth_flavs[j]) {
- args->auth_flavors[0] = server->auth_flavs[j];
+ for (i = 0; i < server_authlist_len; i++) {
+ if (server->auth_flavs[i] == RPC_AUTH_NULL)
+ goto out;
+ for (j = 0; j < args->auth_flavor_len; j++)
+ if (server->auth_flavs[i] == args->auth_flavors[j]) {
+ args->auth_flavors[0] = server->auth_flavs[i];
goto out;
}
+ }

dfprintk(MOUNT, "NFS: server does not support requested auth flavor\n");
nfs_umount(server);


2011-06-28 20:08:21

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFS: Allow sec=none mounts in certain cases

On 06/28/2011 03:34 PM, Chuck Lever wrote:
>
> On Jun 28, 2011, at 3:10 PM, J. Bruce Fields wrote:
>
>> On Tue, Jun 28, 2011 at 03:06:40PM -0400, bfields wrote:
>>> On Tue, Jun 28, 2011 at 02:25:41PM -0400, Chuck Lever wrote:
>>>> There is an undocumented convention (verified by reviewing network
>>>> traces from a NetApp filer and a Solaris NFS server) where a server
>>>> that returns a mount authflavor list containing an AUTH_NULL entry
>>>> is actually indicating it will accept any security flavor for the
>>>> export being mounted.
>>>
>>> This is only in the case of NLM? (Not v4 secinfo?)
>> ^^^
>> (Sorry, I meant MOUNT !)
>
> Right, this fix is specifically for the kernel's NFSv3 mount client. I expect that SECINFO is probably the area where NFSv4 spec changes might help, but I haven't touched that in these patches.
>
> At some point Bryan, Trond, and I should discuss how we can unify these pieces, and teach them how to determine the list of locally supported authflavors. Maybe this is done already?

Do you mean something similar to gss_mech_list_pseudoflavors() in net/sunrpc/auth_gss/gss_mech_switch.c? I use it to determine the sec flavor of an NFS v4.0 mount if AUTH_SYS fails (see nfs4_find_root_sec() in fs/nfs/nfs4proc.c)

- Bryan

>
>>>
>>> --b.
>>>
>>>>
>>>> This might be used when the server maps all security flavors into the
>>>> same security mode. For example, the server treats all accessors as,
>>>> say, UID 17.
>>>>
>>>> Essentially, AUTH_NULL is treated as a wildcard that matches the
>>>> flavor the mounter requested.
>>>>
>>>> Signed-off-by: Chuck Lever <[email protected]>
>>>> ---
>>>>
>>>> fs/nfs/super.c | 15 +++++++++++----
>>>> 1 files changed, 11 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>>> index 4625a4c..543cf9f 100644
>>>> --- a/fs/nfs/super.c
>>>> +++ b/fs/nfs/super.c
>>>> @@ -1570,13 +1570,20 @@ static int nfs_walk_authlist(struct nfs_parsed_mount_data *args,
>>>> * the first flavor in the list that it supports, on the
>>>> * assumption that the best access is provided by the first
>>>> * flavor."
>>>> + *
>>>> + * By convention we treat AUTH_NULL in the returned list as
>>>> + * a wild card. The server will map our requested flavor to
>>>> + * something else.
>>>> */
>>>> - for (i = 0; i < args->auth_flavor_len; i++)
>>>> - for (j = 0; j < server_authlist_len; j++)
>>>> - if (args->auth_flavors[i] == server->auth_flavs[j]) {
>>>> - args->auth_flavors[0] = server->auth_flavs[j];
>>>> + for (i = 0; i < server_authlist_len; i++) {
>>>> + if (server->auth_flavs[i] == RPC_AUTH_NULL)
>>>> + goto out;
>>>> + for (j = 0; j < args->auth_flavor_len; j++)
>>>> + if (server->auth_flavs[i] == args->auth_flavors[j]) {
>>>> + args->auth_flavors[0] = server->auth_flavs[i];
>>>> goto out;
>>>> }
>>>> + }
>>>>
>>>> dfprintk(MOUNT, "NFS: server does not support requested auth flavor\n");
>>>> nfs_umount(server);
>>>>
>>>> --
>>>> 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
>