diff mbox

NFSv3: match sec= flavor against server list

Message ID 1367846874-1234-1-git-send-email-dros@netapp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Weston Andros Adamson May 6, 2013, 1:27 p.m. UTC
Older clients match the 'sec=' mount option flavor against the server's
flavor list (if available) and return EPERM if the specified flavor is not
found. Recent changes would skip this step and allow the vfs mount even
though no operations will succeed.

Signed-off-by: Weston Andros Adamson <dros@netapp.com>
---

Hey Chuck,

This fixes a regression that was introduced with the recent nfs_select_flavor 
changes - I'm pretty sure we want to match the specified flavor instead of just
using it.

Example of the regression:

the server's /etc/exports:

/export/krb5      *(sec=krb5,sec=none,rw,no_root_squash)

old client behavior:

$ uname -a
Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
$ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
mount.nfs: timeout set for Sun May  5 17:32:04 2013
mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
mount.nfs: prog 100003, trying vers=3, prot=6
mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
mount.nfs: prog 100005, trying vers=3, prot=17
mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
mount.nfs: mount(2): Permission denied
mount.nfs: access denied by server while mounting zero:/export/krb5

recently changed behavior:

$ uname -a
Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux
$ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
mount.nfs: timeout set for Sun May  5 17:37:17 2013
mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
mount.nfs: prog 100003, trying vers=3, prot=6
mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
mount.nfs: prog 100005, trying vers=3, prot=17
mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
$ ls /mnt
ls: cannot open directory /mnt: Permission denied
$ sudo ls /mnt
ls: cannot open directory /mnt: Permission denied
$ sudo df /mnt
df: ‘/mnt’: Permission denied
df: no file systems processed
$ sudo umount /mnt
$

I also made an attempt to support "AUTH_NULL means the server will ignore the
cred, so just use the specified flavor" behavior from much older kernels, but 
I'm not sure if we want this logic.

I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some 
older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified,
server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are
ignored by the server), while newer kernels do not. The workaround in newer
kernels is to specify sec=none.

Thoughts?

 -dros

 fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

Comments

Chuck Lever III May 6, 2013, 1:52 p.m. UTC | #1
Hi-

On May 6, 2013, at 9:27 AM, Weston Andros Adamson <dros@netapp.com> wrote:

> Older clients match the 'sec=' mount option flavor against the server's
> flavor list (if available) and return EPERM if the specified flavor is not
> found. Recent changes would skip this step and allow the vfs mount even
> though no operations will succeed.
> 
> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
> ---
> 
> Hey Chuck,
> 
> This fixes a regression that was introduced with the recent nfs_select_flavor 
> changes - I'm pretty sure we want to match the specified flavor instead of just
> using it.
> 
> Example of the regression:
> 
> the server's /etc/exports:
> 
> /export/krb5      *(sec=krb5,sec=none,rw,no_root_squash)
> 
> old client behavior:
> 
> $ uname -a
> Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
> mount.nfs: timeout set for Sun May  5 17:32:04 2013
> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
> mount.nfs: prog 100003, trying vers=3, prot=6
> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
> mount.nfs: prog 100005, trying vers=3, prot=17
> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
> mount.nfs: mount(2): Permission denied
> mount.nfs: access denied by server while mounting zero:/export/krb5

This is wrong behavior, and is fixed by my patch.

> recently changed behavior:
> 
> $ uname -a
> Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux
> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
> mount.nfs: timeout set for Sun May  5 17:37:17 2013
> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
> mount.nfs: prog 100003, trying vers=3, prot=6
> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
> mount.nfs: prog 100005, trying vers=3, prot=17
> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
> $ ls /mnt
> ls: cannot open directory /mnt: Permission denied
> $ sudo ls /mnt
> ls: cannot open directory /mnt: Permission denied
> $ sudo df /mnt
> df: ‘/mnt’: Permission denied
> df: no file systems processed
> $ sudo umount /mnt
> $

This is correct behavior.  The server should map the user's AUTH_SYS credential to anonymous.  If anonymous does not have permission on /export/krb5, then the server should prevent its access to the export.

I assume this is a Linux NFS server.  Does the Linux server support sec=none listed in the export options in the same way that NetApp and Oracle Solaris do?

> I also made an attempt to support "AUTH_NULL means the server will ignore the
> cred, so just use the specified flavor" behavior from much older kernels, but 
> I'm not sure if we want this logic.
> 
> I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some 
> older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified,
> server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are
> ignored by the server), while newer kernels do not. The workaround in newer
> kernels is to specify sec=none.
> 
> Thoughts?
> 
> -dros
> 
> fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++-------
> 1 file changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index eb494f6..6476b5d 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1611,14 +1611,12 @@ out_security_failure:
>  * Select a security flavor for this mount.  The selected flavor
>  * is planted in args->auth_flavors[0].
>  */
> -static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
> +static int nfs_select_flavor(struct nfs_parsed_mount_data *args,
> 			      struct nfs_mount_request *request)
> {
> 	unsigned int i, count = *(request->auth_flav_len);
> 	rpc_authflavor_t flavor;
> -
> -	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR)
> -		goto out;
> +	bool auth_null_seen = false;
> 
> 	/*
> 	 * The NFSv2 MNT operation does not return a flavor list.
> @@ -1634,6 +1632,26 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
> 		goto out_default;
> 
> 	/*
> +	 * If the sec= mount option is used, the flavor must be in the list
> +	 * returned by the server.
> +	 *
> +	 * One exception is when the server's flavor list includes AUTH_NULL:
> +	 * Some servers implement AUTH_NULL by completely ignoring the rpc
> +	 * cred, so the client can use any flavor.
> +	 */
> +	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
> +		for (i = 0; i < count; i++) {
> +			if (args->auth_flavors[0] == request->auth_flavs[i])
> +				goto out;
> +			if (request->auth_flavs[i] == RPC_AUTH_NULL)
> +				auth_null_seen = true;
> +		}
> +		if (auth_null_seen)
> +			goto out;
> +		goto out_nomatch;
> +	}
> +
> +	/*
> 	 * RFC 2623, section 2.7 suggests we SHOULD prefer the
> 	 * flavor listed first.  However, some servers list
> 	 * AUTH_NULL first.  Avoid ever choosing AUTH_NULL.
> @@ -1654,11 +1672,19 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
> 	}
> 
> out_default:
> -	flavor = RPC_AUTH_UNIX;
> +	/* use default if flavor not already set */
> +	flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ?
> +		RPC_AUTH_UNIX : args->auth_flavors[0];
> out_set:
> 	args->auth_flavors[0] = flavor;
> out:
> 	dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]);
> +	return 0;
> +
> +out_nomatch:
> +	dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n",
> +		args->auth_flavors[0]);
> +	return -EPERM;
> }
> 
> /*
> @@ -1721,8 +1747,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args,
> 		return status;
> 	}
> 
> -	nfs_select_flavor(args, &request);
> -	return 0;
> +	return nfs_select_flavor(args, &request);
> }
> 
> struct dentry *nfs_try_mount(int flags, const char *dev_name,
> -- 
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adamson, Dros May 6, 2013, 2:49 p.m. UTC | #2
On May 6, 2013, at 9:52 AM, Chuck Lever <chuck.lever@oracle.com> wrote:

> Hi-
> 
> On May 6, 2013, at 9:27 AM, Weston Andros Adamson <dros@netapp.com> wrote:
> 
>> Older clients match the 'sec=' mount option flavor against the server's
>> flavor list (if available) and return EPERM if the specified flavor is not
>> found. Recent changes would skip this step and allow the vfs mount even
>> though no operations will succeed.
>> 
>> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
>> ---
>> 
>> Hey Chuck,
>> 
>> This fixes a regression that was introduced with the recent nfs_select_flavor 
>> changes - I'm pretty sure we want to match the specified flavor instead of just
>> using it.
>> 
>> Example of the regression:
>> 
>> the server's /etc/exports:
>> 
>> /export/krb5      *(sec=krb5,sec=none,rw,no_root_squash)
>> 
>> old client behavior:
>> 
>> $ uname -a
>> Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
>> mount.nfs: timeout set for Sun May  5 17:32:04 2013
>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
>> mount.nfs: prog 100003, trying vers=3, prot=6
>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
>> mount.nfs: prog 100005, trying vers=3, prot=17
>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
>> mount.nfs: mount(2): Permission denied
>> mount.nfs: access denied by server while mounting zero:/export/krb5
> 
> This is wrong behavior, and is fixed by my patch.
> 
>> recently changed behavior:
>> 
>> $ uname -a
>> Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux
>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
>> mount.nfs: timeout set for Sun May  5 17:37:17 2013
>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
>> mount.nfs: prog 100003, trying vers=3, prot=6
>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
>> mount.nfs: prog 100005, trying vers=3, prot=17
>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
>> $ ls /mnt
>> ls: cannot open directory /mnt: Permission denied
>> $ sudo ls /mnt
>> ls: cannot open directory /mnt: Permission denied
>> $ sudo df /mnt
>> df: ‘/mnt’: Permission denied
>> df: no file systems processed
>> $ sudo umount /mnt
>> $
> 
> This is correct behavior.  The server should map the user's AUTH_SYS credential to anonymous.  If anonymous does not have permission on /export/krb5, then the server should prevent its access to the export.

You're talking about AUTH_NULL behavior here, right?

> 
> I assume this is a Linux NFS server.  Does the Linux server support sec=none listed in the export options in the same way that NetApp and Oracle Solaris do?


So there are two issues here and I think they're getting confused.

The example is of the first issue: mounting a krb5 only export with sec=sys
 - client mounts sec=sys
 - the server list is [krb5]
 - the client completely ignores this list and just uses sys.
 - no operations work, it's a 'dud mount'

The second issue is the AUTH_NULL stuff - lets just ignore that for now.

Are you saying that the client should just use whatever sec= is specified and never try to match against the server list?

A little background - I ran into this implementing a different patch that makes sure v4.x mounts are actually using the specified flavor (if one exists) to mount the export -- that is, it can follow SECINFOs to do initial lookup, but must be using the specified flavor on the export path passed to mount.  After the initial mount lookup, SECINFOs will be used normally. Trond thought that this was a bug that should be fixed - that it's wrong when client reports that it's using one flavor (as seen in mount output, etc) when it's really using another. This (other) patch works, but when no version is specified, it always falls back to v3 and the mount will always succeed - even if the auth flavor isn't supported by the export.

-dros


> 
>> I also made an attempt to support "AUTH_NULL means the server will ignore the
>> cred, so just use the specified flavor" behavior from much older kernels, but 
>> I'm not sure if we want this logic.
>> 
>> I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some 
>> older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified,
>> server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are
>> ignored by the server), while newer kernels do not. The workaround in newer
>> kernels is to specify sec=none.
>> 
>> Thoughts?
>> 
>> -dros
>> 
>> fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++-------
>> 1 file changed, 32 insertions(+), 7 deletions(-)
>> 
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index eb494f6..6476b5d 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -1611,14 +1611,12 @@ out_security_failure:
>> * Select a security flavor for this mount.  The selected flavor
>> * is planted in args->auth_flavors[0].
>> */
>> -static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>> +static int nfs_select_flavor(struct nfs_parsed_mount_data *args,
>> 			      struct nfs_mount_request *request)
>> {
>> 	unsigned int i, count = *(request->auth_flav_len);
>> 	rpc_authflavor_t flavor;
>> -
>> -	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR)
>> -		goto out;
>> +	bool auth_null_seen = false;
>> 
>> 	/*
>> 	 * The NFSv2 MNT operation does not return a flavor list.
>> @@ -1634,6 +1632,26 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>> 		goto out_default;
>> 
>> 	/*
>> +	 * If the sec= mount option is used, the flavor must be in the list
>> +	 * returned by the server.
>> +	 *
>> +	 * One exception is when the server's flavor list includes AUTH_NULL:
>> +	 * Some servers implement AUTH_NULL by completely ignoring the rpc
>> +	 * cred, so the client can use any flavor.
>> +	 */
>> +	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
>> +		for (i = 0; i < count; i++) {
>> +			if (args->auth_flavors[0] == request->auth_flavs[i])
>> +				goto out;
>> +			if (request->auth_flavs[i] == RPC_AUTH_NULL)
>> +				auth_null_seen = true;
>> +		}
>> +		if (auth_null_seen)
>> +			goto out;
>> +		goto out_nomatch;
>> +	}
>> +
>> +	/*
>> 	 * RFC 2623, section 2.7 suggests we SHOULD prefer the
>> 	 * flavor listed first.  However, some servers list
>> 	 * AUTH_NULL first.  Avoid ever choosing AUTH_NULL.
>> @@ -1654,11 +1672,19 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>> 	}
>> 
>> out_default:
>> -	flavor = RPC_AUTH_UNIX;
>> +	/* use default if flavor not already set */
>> +	flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ?
>> +		RPC_AUTH_UNIX : args->auth_flavors[0];
>> out_set:
>> 	args->auth_flavors[0] = flavor;
>> out:
>> 	dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]);
>> +	return 0;
>> +
>> +out_nomatch:
>> +	dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n",
>> +		args->auth_flavors[0]);
>> +	return -EPERM;
>> }
>> 
>> /*
>> @@ -1721,8 +1747,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args,
>> 		return status;
>> 	}
>> 
>> -	nfs_select_flavor(args, &request);
>> -	return 0;
>> +	return nfs_select_flavor(args, &request);
>> }
>> 
>> struct dentry *nfs_try_mount(int flags, const char *dev_name,
>> -- 
>> 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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Trond Myklebust May 6, 2013, 3:08 p.m. UTC | #3
T24gTW9uLCAyMDEzLTA1LTA2IGF0IDE0OjQ5ICswMDAwLCBBZGFtc29uLCBEcm9zIHdyb3RlOg0K
PiBPbiBNYXkgNiwgMjAxMywgYXQgOTo1MiBBTSwgQ2h1Y2sgTGV2ZXIgPGNodWNrLmxldmVyQG9y
YWNsZS5jb20+IHdyb3RlOg0KPiANCj4gPiBIaS0NCj4gPiANCj4gPiBPbiBNYXkgNiwgMjAxMywg
YXQgOToyNyBBTSwgV2VzdG9uIEFuZHJvcyBBZGFtc29uIDxkcm9zQG5ldGFwcC5jb20+IHdyb3Rl
Og0KPiA+IA0KPiA+PiBPbGRlciBjbGllbnRzIG1hdGNoIHRoZSAnc2VjPScgbW91bnQgb3B0aW9u
IGZsYXZvciBhZ2FpbnN0IHRoZSBzZXJ2ZXIncw0KPiA+PiBmbGF2b3IgbGlzdCAoaWYgYXZhaWxh
YmxlKSBhbmQgcmV0dXJuIEVQRVJNIGlmIHRoZSBzcGVjaWZpZWQgZmxhdm9yIGlzIG5vdA0KPiA+
PiBmb3VuZC4gUmVjZW50IGNoYW5nZXMgd291bGQgc2tpcCB0aGlzIHN0ZXAgYW5kIGFsbG93IHRo
ZSB2ZnMgbW91bnQgZXZlbg0KPiA+PiB0aG91Z2ggbm8gb3BlcmF0aW9ucyB3aWxsIHN1Y2NlZWQu
DQo+ID4+IA0KPiA+PiBTaWduZWQtb2ZmLWJ5OiBXZXN0b24gQW5kcm9zIEFkYW1zb24gPGRyb3NA
bmV0YXBwLmNvbT4NCj4gPj4gLS0tDQo+ID4+IA0KPiA+PiBIZXkgQ2h1Y2ssDQo+ID4+IA0KPiA+
PiBUaGlzIGZpeGVzIGEgcmVncmVzc2lvbiB0aGF0IHdhcyBpbnRyb2R1Y2VkIHdpdGggdGhlIHJl
Y2VudCBuZnNfc2VsZWN0X2ZsYXZvciANCj4gPj4gY2hhbmdlcyAtIEknbSBwcmV0dHkgc3VyZSB3
ZSB3YW50IHRvIG1hdGNoIHRoZSBzcGVjaWZpZWQgZmxhdm9yIGluc3RlYWQgb2YganVzdA0KPiA+
PiB1c2luZyBpdC4NCj4gPj4gDQo+ID4+IEV4YW1wbGUgb2YgdGhlIHJlZ3Jlc3Npb246DQo+ID4+
IA0KPiA+PiB0aGUgc2VydmVyJ3MgL2V0Yy9leHBvcnRzOg0KPiA+PiANCj4gPj4gL2V4cG9ydC9r
cmI1ICAgICAgKihzZWM9a3JiNSxzZWM9bm9uZSxydyxub19yb290X3NxdWFzaCkNCj4gPj4gDQo+
ID4+IG9sZCBjbGllbnQgYmVoYXZpb3I6DQo+ID4+IA0KPiA+PiAkIHVuYW1lIC1hDQo+ID4+IExp
bnV4IG9uZS5hcGlraWEuZmFrZSAzLjguOC0yMDIuZmMxOC54ODZfNjQgIzEgU01QIFdlZCBBcHIg
MTcgMjM6MjU6MTcgVVRDIDIwMTMgeDg2XzY0IHg4Nl82NCB4ODZfNjQgR05VL0xpbnV4DQo+ID4+
ICQgc3VkbyBtb3VudCAtdiAtbyBzZWM9c3lzLHZlcnM9MyB6ZXJvOi9leHBvcnQva3JiNSAvbW50
DQo+ID4+IG1vdW50Lm5mczogdGltZW91dCBzZXQgZm9yIFN1biBNYXkgIDUgMTc6MzI6MDQgMjAx
Mw0KPiA+PiBtb3VudC5uZnM6IHRyeWluZyB0ZXh0LWJhc2VkIG9wdGlvbnMgJ3NlYz1zeXMsdmVy
cz0zLGFkZHI9MTkyLjE2OC4xMDAuMTAnDQo+ID4+IG1vdW50Lm5mczogcHJvZyAxMDAwMDMsIHRy
eWluZyB2ZXJzPTMsIHByb3Q9Ng0KPiA+PiBtb3VudC5uZnM6IHRyeWluZyAxOTIuMTY4LjEwMC4x
MCBwcm9nIDEwMDAwMyB2ZXJzIDMgcHJvdCBUQ1AgcG9ydCAyMDQ5DQo+ID4+IG1vdW50Lm5mczog
cHJvZyAxMDAwMDUsIHRyeWluZyB2ZXJzPTMsIHByb3Q9MTcNCj4gPj4gbW91bnQubmZzOiB0cnlp
bmcgMTkyLjE2OC4xMDAuMTAgcHJvZyAxMDAwMDUgdmVycyAzIHByb3QgVURQIHBvcnQgMjAwNDgN
Cj4gPj4gbW91bnQubmZzOiBtb3VudCgyKTogUGVybWlzc2lvbiBkZW5pZWQNCj4gPj4gbW91bnQu
bmZzOiBhY2Nlc3MgZGVuaWVkIGJ5IHNlcnZlciB3aGlsZSBtb3VudGluZyB6ZXJvOi9leHBvcnQv
a3JiNQ0KPiA+IA0KPiA+IFRoaXMgaXMgd3JvbmcgYmVoYXZpb3IsIGFuZCBpcyBmaXhlZCBieSBt
eSBwYXRjaC4NCj4gPiANCj4gPj4gcmVjZW50bHkgY2hhbmdlZCBiZWhhdmlvcjoNCj4gPj4gDQo+
ID4+ICQgdW5hbWUgLWENCj4gPj4gTGludXggb25lLmFwaWtpYS5mYWtlIDMuOS4wLXRlc3Rpbmcr
ICMyIFNNUCBGcmkgTWF5IDMgMjA6Mjk6MzIgRURUIDIwMTMgeDg2XzY0IHg4Nl82NCB4ODZfNjQg
R05VL0xpbnV4DQo+ID4+ICQgc3VkbyBtb3VudCAtdiAtbyBzZWM9c3lzLHZlcnM9MyB6ZXJvOi9l
eHBvcnQva3JiNSAvbW50DQo+ID4+IG1vdW50Lm5mczogdGltZW91dCBzZXQgZm9yIFN1biBNYXkg
IDUgMTc6Mzc6MTcgMjAxMw0KPiA+PiBtb3VudC5uZnM6IHRyeWluZyB0ZXh0LWJhc2VkIG9wdGlv
bnMgJ3NlYz1zeXMsdmVycz0zLGFkZHI9MTkyLjE2OC4xMDAuMTAnDQo+ID4+IG1vdW50Lm5mczog
cHJvZyAxMDAwMDMsIHRyeWluZyB2ZXJzPTMsIHByb3Q9Ng0KPiA+PiBtb3VudC5uZnM6IHRyeWlu
ZyAxOTIuMTY4LjEwMC4xMCBwcm9nIDEwMDAwMyB2ZXJzIDMgcHJvdCBUQ1AgcG9ydCAyMDQ5DQo+
ID4+IG1vdW50Lm5mczogcHJvZyAxMDAwMDUsIHRyeWluZyB2ZXJzPTMsIHByb3Q9MTcNCj4gPj4g
bW91bnQubmZzOiB0cnlpbmcgMTkyLjE2OC4xMDAuMTAgcHJvZyAxMDAwMDUgdmVycyAzIHByb3Qg
VURQIHBvcnQgMjAwNDgNCj4gPj4gJCBscyAvbW50DQo+ID4+IGxzOiBjYW5ub3Qgb3BlbiBkaXJl
Y3RvcnkgL21udDogUGVybWlzc2lvbiBkZW5pZWQNCj4gPj4gJCBzdWRvIGxzIC9tbnQNCj4gPj4g
bHM6IGNhbm5vdCBvcGVuIGRpcmVjdG9yeSAvbW50OiBQZXJtaXNzaW9uIGRlbmllZA0KPiA+PiAk
IHN1ZG8gZGYgL21udA0KPiA+PiBkZjog4oCYL21udOKAmTogUGVybWlzc2lvbiBkZW5pZWQNCj4g
Pj4gZGY6IG5vIGZpbGUgc3lzdGVtcyBwcm9jZXNzZWQNCj4gPj4gJCBzdWRvIHVtb3VudCAvbW50
DQo+ID4+ICQNCj4gPiANCj4gPiBUaGlzIGlzIGNvcnJlY3QgYmVoYXZpb3IuICBUaGUgc2VydmVy
IHNob3VsZCBtYXAgdGhlIHVzZXIncyBBVVRIX1NZUyBjcmVkZW50aWFsIHRvIGFub255bW91cy4g
IElmIGFub255bW91cyBkb2VzIG5vdCBoYXZlIHBlcm1pc3Npb24gb24gL2V4cG9ydC9rcmI1LCB0
aGVuIHRoZSBzZXJ2ZXIgc2hvdWxkIHByZXZlbnQgaXRzIGFjY2VzcyB0byB0aGUgZXhwb3J0Lg0K
PiANCj4gWW91J3JlIHRhbGtpbmcgYWJvdXQgQVVUSF9OVUxMIGJlaGF2aW9yIGhlcmUsIHJpZ2h0
Pw0KPiANCj4gPiANCj4gPiBJIGFzc3VtZSB0aGlzIGlzIGEgTGludXggTkZTIHNlcnZlci4gIERv
ZXMgdGhlIExpbnV4IHNlcnZlciBzdXBwb3J0IHNlYz1ub25lIGxpc3RlZCBpbiB0aGUgZXhwb3J0
IG9wdGlvbnMgaW4gdGhlIHNhbWUgd2F5IHRoYXQgTmV0QXBwIGFuZCBPcmFjbGUgU29sYXJpcyBk
bz8NCj4gDQo+IA0KPiBTbyB0aGVyZSBhcmUgdHdvIGlzc3VlcyBoZXJlIGFuZCBJIHRoaW5rIHRo
ZXkncmUgZ2V0dGluZyBjb25mdXNlZC4NCj4gDQo+IFRoZSBleGFtcGxlIGlzIG9mIHRoZSBmaXJz
dCBpc3N1ZTogbW91bnRpbmcgYSBrcmI1IG9ubHkgZXhwb3J0IHdpdGggc2VjPXN5cw0KPiAgLSBj
bGllbnQgbW91bnRzIHNlYz1zeXMNCj4gIC0gdGhlIHNlcnZlciBsaXN0IGlzIFtrcmI1XQ0KPiAg
LSB0aGUgY2xpZW50IGNvbXBsZXRlbHkgaWdub3JlcyB0aGlzIGxpc3QgYW5kIGp1c3QgdXNlcyBz
eXMuDQo+ICAtIG5vIG9wZXJhdGlvbnMgd29yaywgaXQncyBhICdkdWQgbW91bnQnDQo+IA0KPiBU
aGUgc2Vjb25kIGlzc3VlIGlzIHRoZSBBVVRIX05VTEwgc3R1ZmYgLSBsZXRzIGp1c3QgaWdub3Jl
IHRoYXQgZm9yIG5vdy4NCj4gDQo+IEFyZSB5b3Ugc2F5aW5nIHRoYXQgdGhlIGNsaWVudCBzaG91
bGQganVzdCB1c2Ugd2hhdGV2ZXIgc2VjPSBpcyBzcGVjaWZpZWQgYW5kIG5ldmVyIHRyeSB0byBt
YXRjaCBhZ2FpbnN0IHRoZSBzZXJ2ZXIgbGlzdD8NCj4gDQo+IEEgbGl0dGxlIGJhY2tncm91bmQg
LSBJIHJhbiBpbnRvIHRoaXMgaW1wbGVtZW50aW5nIGEgZGlmZmVyZW50IHBhdGNoIHRoYXQgbWFr
ZXMgc3VyZSB2NC54IG1vdW50cyBhcmUgYWN0dWFsbHkgdXNpbmcgdGhlIHNwZWNpZmllZCBmbGF2
b3IgKGlmIG9uZSBleGlzdHMpIHRvIG1vdW50IHRoZSBleHBvcnQgLS0gdGhhdCBpcywgaXQgY2Fu
IGZvbGxvdyBTRUNJTkZPcyB0byBkbyBpbml0aWFsIGxvb2t1cCwgYnV0IG11c3QgYmUgdXNpbmcg
dGhlIHNwZWNpZmllZCBmbGF2b3Igb24gdGhlIGV4cG9ydCBwYXRoIHBhc3NlZCB0byBtb3VudC4g
IEFmdGVyIHRoZSBpbml0aWFsIG1vdW50IGxvb2t1cCwgU0VDSU5GT3Mgd2lsbCBiZSB1c2VkIG5v
cm1hbGx5LiBUcm9uZCB0aG91Z2h0IHRoYXQgdGhpcyB3YXMgYSBidWcgdGhhdCBzaG91bGQgYmUg
Zml4ZWQgLSB0aGF0IGl0J3Mgd3Jvbmcgd2hlbiBjbGllbnQgcmVwb3J0cyB0aGF0IGl0J3MgdXNp
bmcgb25lIGZsYXZvciAoYXMgc2VlbiBpbiBtb3VudCBvdXRwdXQsIGV0Yykgd2hlbiBpdCdzIHJl
YWxseSB1c2luZyBhbm90aGVyLiBUaGlzIChvdGhlcikgcGF0Y2ggd29ya3MsIGJ1dCB3aGVuIG5v
IHZlcnNpb24gaXMgc3BlY2lmaWVkLCBpdCBhbHdheXMgZmFsbHMgYmFjayB0byB2MyBhbmQgdGhl
IG1vdW50IHdpbGwgYWx3YXlzIHN1Y2NlZWQgLSBldmVuIGlmIHRoZSBhdXRoIGZsYXZvciBpc24n
dCBzdXBwb3J0ZWQgYnkgdGhlIGV4cG9ydC4NCg0KSW4gdGhlIE5GU3Y0IGNhc2UsIHdlIHNob3Vs
ZCBkZWZpbml0ZWx5IGJlIHJldHVybmluZyBFQUNDRVMgaWYgdGhlIHVzZXINCnRyaWVzIHRvIHNw
ZWNpZnkgYSBzZWN1cml0eSBmbGF2b3VyIHRoYXQgZG9lc24ndCBhbGxvdyB0aGUgbW91bnQgdG8N
CnN1Y2NlZWQsIGluc3RlYWQgb2YgaGF2aW5nIGF1dG8tbmVnb3RpYXRpb24gb3ZlcnJpZGUgaXQu
DQoNCldpdGggdGhhdCBpbiBtaW5kLCBJIGFncmVlIHRoYXQgd2Ugc2hvdWxkIGRvIHRoZSBzYW1l
IHdpdGggTkZTdjMgaW4NCm9yZGVyIHRvIGF2b2lkIHRoZSBraW5kIG9mIG1vdW50IGZhaWxvdmVy
IHNpdHVhdGlvbiB0aGF0IERyb3MgZGVzY3JpYmVzDQphYm92ZS4gSGF2aW5nIHRoZSBtb3VudCAn
c3VjY2VlZCcgeWV0IG5vdCBhY3R1YWxseSBiZSB1c2FibGUgYnkgYW55b25lDQppcyBqdXN0IHdy
b25nIChldmVuIGlmIHRoYXQgaXMgd2hhdCB3ZSB1c2VkIHRvIGRvIGJlZm9yZSBzZWN1cml0eQ0K
bmVnb3RpYXRpb24gd2FzIGFkZGVkKS4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5G
UyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29t
DQp3d3cubmV0YXBwLmNvbQ0K
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever III May 6, 2013, 3:17 p.m. UTC | #4
On May 6, 2013, at 10:49 AM, "Adamson, Dros" <Weston.Adamson@netapp.com> wrote:

> 
> On May 6, 2013, at 9:52 AM, Chuck Lever <chuck.lever@oracle.com> wrote:
> 
>> Hi-
>> 
>> On May 6, 2013, at 9:27 AM, Weston Andros Adamson <dros@netapp.com> wrote:
>> 
>>> Older clients match the 'sec=' mount option flavor against the server's
>>> flavor list (if available) and return EPERM if the specified flavor is not
>>> found. Recent changes would skip this step and allow the vfs mount even
>>> though no operations will succeed.
>>> 
>>> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
>>> ---
>>> 
>>> Hey Chuck,
>>> 
>>> This fixes a regression that was introduced with the recent nfs_select_flavor 
>>> changes - I'm pretty sure we want to match the specified flavor instead of just
>>> using it.
>>> 
>>> Example of the regression:
>>> 
>>> the server's /etc/exports:
>>> 
>>> /export/krb5      *(sec=krb5,sec=none,rw,no_root_squash)
>>> 
>>> old client behavior:
>>> 
>>> $ uname -a
>>> Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
>>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
>>> mount.nfs: timeout set for Sun May  5 17:32:04 2013
>>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
>>> mount.nfs: prog 100003, trying vers=3, prot=6
>>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
>>> mount.nfs: prog 100005, trying vers=3, prot=17
>>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
>>> mount.nfs: mount(2): Permission denied
>>> mount.nfs: access denied by server while mounting zero:/export/krb5
>> 
>> This is wrong behavior, and is fixed by my patch.
>> 
>>> recently changed behavior:
>>> 
>>> $ uname -a
>>> Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux
>>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
>>> mount.nfs: timeout set for Sun May  5 17:37:17 2013
>>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
>>> mount.nfs: prog 100003, trying vers=3, prot=6
>>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
>>> mount.nfs: prog 100005, trying vers=3, prot=17
>>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
>>> $ ls /mnt
>>> ls: cannot open directory /mnt: Permission denied
>>> $ sudo ls /mnt
>>> ls: cannot open directory /mnt: Permission denied
>>> $ sudo df /mnt
>>> df: ‘/mnt’: Permission denied
>>> df: no file systems processed
>>> $ sudo umount /mnt
>>> $
>> 
>> This is correct behavior.  The server should map the user's AUTH_SYS credential to anonymous.  If anonymous does not have permission on /export/krb5, then the server should prevent its access to the export.
> 
> You're talking about AUTH_NULL behavior here, right?
> 
>> 
>> I assume this is a Linux NFS server.  Does the Linux server support sec=none listed in the export options in the same way that NetApp and Oracle Solaris do?
> 
> 
> So there are two issues here and I think they're getting confused.
> 
> The example is of the first issue: mounting a krb5 only export with sec=sys
> - client mounts sec=sys
> - the server list is [krb5]
> - the client completely ignores this list and just uses sys.
> - no operations work, it's a 'dud mount'
> 
> The second issue is the AUTH_NULL stuff - lets just ignore that for now.
> 
> Are you saying that the client should just use whatever sec= is specified and never try to match against the server list?

That's what my patch does.  The reason for this is that the server can interpret the mount's security flavor any way it likes.

Perhaps we should take that fallback position only if the server lists AUTH_NULL in the flavor list.  If AUTH_NULL is not listed, ensure the flavor requested on the mount command line is available on the server (the most recent previous behavior); fail if no matching flavor is found.

If the mount command line does not specify a flavor, we could look in the server's list for a flavor we support, then fail if none is found.

> A little background - I ran into this implementing a different patch that makes sure v4.x mounts are actually using the specified flavor (if one exists) to mount the export -- that is, it can follow SECINFOs to do initial lookup, but must be using the specified flavor on the export path passed to mount.  After the initial mount lookup, SECINFOs will be used normally. Trond thought that this was a bug that should be fixed - that it's wrong when client reports that it's using one flavor (as seen in mount output, etc) when it's really using another. This (other) patch works, but when no version is specified, it always falls back to v3 and the mount will always succeed - even if the auth flavor isn't supported by the export.
> 
> -dros
> 
> 
>> 
>>> I also made an attempt to support "AUTH_NULL means the server will ignore the
>>> cred, so just use the specified flavor" behavior from much older kernels, but 
>>> I'm not sure if we want this logic.
>>> 
>>> I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some 
>>> older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified,
>>> server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are
>>> ignored by the server), while newer kernels do not. The workaround in newer
>>> kernels is to specify sec=none.
>>> 
>>> Thoughts?
>>> 
>>> -dros
>>> 
>>> fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++-------
>>> 1 file changed, 32 insertions(+), 7 deletions(-)
>>> 
>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>> index eb494f6..6476b5d 100644
>>> --- a/fs/nfs/super.c
>>> +++ b/fs/nfs/super.c
>>> @@ -1611,14 +1611,12 @@ out_security_failure:
>>> * Select a security flavor for this mount.  The selected flavor
>>> * is planted in args->auth_flavors[0].
>>> */
>>> -static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>>> +static int nfs_select_flavor(struct nfs_parsed_mount_data *args,
>>> 			      struct nfs_mount_request *request)
>>> {
>>> 	unsigned int i, count = *(request->auth_flav_len);
>>> 	rpc_authflavor_t flavor;
>>> -
>>> -	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR)
>>> -		goto out;
>>> +	bool auth_null_seen = false;
>>> 
>>> 	/*
>>> 	 * The NFSv2 MNT operation does not return a flavor list.
>>> @@ -1634,6 +1632,26 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>>> 		goto out_default;
>>> 
>>> 	/*
>>> +	 * If the sec= mount option is used, the flavor must be in the list
>>> +	 * returned by the server.
>>> +	 *
>>> +	 * One exception is when the server's flavor list includes AUTH_NULL:
>>> +	 * Some servers implement AUTH_NULL by completely ignoring the rpc
>>> +	 * cred, so the client can use any flavor.
>>> +	 */
>>> +	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
>>> +		for (i = 0; i < count; i++) {
>>> +			if (args->auth_flavors[0] == request->auth_flavs[i])
>>> +				goto out;
>>> +			if (request->auth_flavs[i] == RPC_AUTH_NULL)
>>> +				auth_null_seen = true;
>>> +		}
>>> +		if (auth_null_seen)
>>> +			goto out;
>>> +		goto out_nomatch;
>>> +	}
>>> +
>>> +	/*
>>> 	 * RFC 2623, section 2.7 suggests we SHOULD prefer the
>>> 	 * flavor listed first.  However, some servers list
>>> 	 * AUTH_NULL first.  Avoid ever choosing AUTH_NULL.
>>> @@ -1654,11 +1672,19 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>>> 	}
>>> 
>>> out_default:
>>> -	flavor = RPC_AUTH_UNIX;
>>> +	/* use default if flavor not already set */
>>> +	flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ?
>>> +		RPC_AUTH_UNIX : args->auth_flavors[0];
>>> out_set:
>>> 	args->auth_flavors[0] = flavor;
>>> out:
>>> 	dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]);
>>> +	return 0;
>>> +
>>> +out_nomatch:
>>> +	dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n",
>>> +		args->auth_flavors[0]);
>>> +	return -EPERM;
>>> }
>>> 
>>> /*
>>> @@ -1721,8 +1747,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args,
>>> 		return status;
>>> 	}
>>> 
>>> -	nfs_select_flavor(args, &request);
>>> -	return 0;
>>> +	return nfs_select_flavor(args, &request);
>>> }
>>> 
>>> struct dentry *nfs_try_mount(int flags, const char *dev_name,
>>> -- 
>>> 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 majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
>> --
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com
>> 
>> 
>> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
bfields@fieldses.org May 6, 2013, 3:32 p.m. UTC | #5
On Mon, May 06, 2013 at 09:27:54AM -0400, Weston Andros Adamson wrote:
> Older clients match the 'sec=' mount option flavor against the server's
> flavor list (if available) and return EPERM if the specified flavor is not
> found. Recent changes would skip this step and allow the vfs mount even
> though no operations will succeed.
> 
> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
> ---
> 
> Hey Chuck,
> 
> This fixes a regression that was introduced with the recent nfs_select_flavor 
> changes - I'm pretty sure we want to match the specified flavor instead of just
> using it.
> 
> Example of the regression:
> 
> the server's /etc/exports:
> 
> /export/krb5      *(sec=krb5,sec=none,rw,no_root_squash)
> 
> old client behavior:
> 
> $ uname -a
> Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
> mount.nfs: timeout set for Sun May  5 17:32:04 2013
> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
> mount.nfs: prog 100003, trying vers=3, prot=6
> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
> mount.nfs: prog 100005, trying vers=3, prot=17
> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
> mount.nfs: mount(2): Permission denied
> mount.nfs: access denied by server while mounting zero:/export/krb5
> 
> recently changed behavior:
> 
> $ uname -a
> Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux
> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
> mount.nfs: timeout set for Sun May  5 17:37:17 2013
> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
> mount.nfs: prog 100003, trying vers=3, prot=6
> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
> mount.nfs: prog 100005, trying vers=3, prot=17
> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
> $ ls /mnt
> ls: cannot open directory /mnt: Permission denied
> $ sudo ls /mnt
> ls: cannot open directory /mnt: Permission denied
> $ sudo df /mnt
> df: ‘/mnt’: Permission denied
> df: no file systems processed
> $ sudo umount /mnt
> $
> 
> I also made an attempt to support "AUTH_NULL means the server will ignore the
> cred, so just use the specified flavor" behavior from much older kernels, but 
> I'm not sure if we want this logic.
> 
> I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some 
> older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified,
> server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are
> ignored by the server), while newer kernels do not. The workaround in newer
> kernels is to specify sec=none.

Nit: the above examples would actually be really useful, for example, to
a distribution maintainer trying to figure out whether this patch would
solve a given user problem.

Could you include them in the changelog?

--b.

> 
> Thoughts?
> 
>  -dros
> 
>  fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index eb494f6..6476b5d 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1611,14 +1611,12 @@ out_security_failure:
>   * Select a security flavor for this mount.  The selected flavor
>   * is planted in args->auth_flavors[0].
>   */
> -static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
> +static int nfs_select_flavor(struct nfs_parsed_mount_data *args,
>  			      struct nfs_mount_request *request)
>  {
>  	unsigned int i, count = *(request->auth_flav_len);
>  	rpc_authflavor_t flavor;
> -
> -	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR)
> -		goto out;
> +	bool auth_null_seen = false;
>  
>  	/*
>  	 * The NFSv2 MNT operation does not return a flavor list.
> @@ -1634,6 +1632,26 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>  		goto out_default;
>  
>  	/*
> +	 * If the sec= mount option is used, the flavor must be in the list
> +	 * returned by the server.
> +	 *
> +	 * One exception is when the server's flavor list includes AUTH_NULL:
> +	 * Some servers implement AUTH_NULL by completely ignoring the rpc
> +	 * cred, so the client can use any flavor.
> +	 */
> +	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
> +		for (i = 0; i < count; i++) {
> +			if (args->auth_flavors[0] == request->auth_flavs[i])
> +				goto out;
> +			if (request->auth_flavs[i] == RPC_AUTH_NULL)
> +				auth_null_seen = true;
> +		}
> +		if (auth_null_seen)
> +			goto out;
> +		goto out_nomatch;
> +	}
> +
> +	/*
>  	 * RFC 2623, section 2.7 suggests we SHOULD prefer the
>  	 * flavor listed first.  However, some servers list
>  	 * AUTH_NULL first.  Avoid ever choosing AUTH_NULL.
> @@ -1654,11 +1672,19 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>  	}
>  
>  out_default:
> -	flavor = RPC_AUTH_UNIX;
> +	/* use default if flavor not already set */
> +	flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ?
> +		RPC_AUTH_UNIX : args->auth_flavors[0];
>  out_set:
>  	args->auth_flavors[0] = flavor;
>  out:
>  	dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]);
> +	return 0;
> +
> +out_nomatch:
> +	dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n",
> +		args->auth_flavors[0]);
> +	return -EPERM;
>  }
>  
>  /*
> @@ -1721,8 +1747,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args,
>  		return status;
>  	}
>  
> -	nfs_select_flavor(args, &request);
> -	return 0;
> +	return nfs_select_flavor(args, &request);
>  }
>  
>  struct dentry *nfs_try_mount(int flags, const char *dev_name,
> -- 
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adamson, Dros May 6, 2013, 3:34 p.m. UTC | #6
On May 6, 2013, at 11:17 AM, Chuck Lever <chuck.lever@oracle.com>
 wrote:

> 
> On May 6, 2013, at 10:49 AM, "Adamson, Dros" <Weston.Adamson@netapp.com> wrote:
> 
>> 
>> On May 6, 2013, at 9:52 AM, Chuck Lever <chuck.lever@oracle.com> wrote:
>> 
>>> Hi-
>>> 
>>> On May 6, 2013, at 9:27 AM, Weston Andros Adamson <dros@netapp.com> wrote:
>>> 
>>>> Older clients match the 'sec=' mount option flavor against the server's
>>>> flavor list (if available) and return EPERM if the specified flavor is not
>>>> found. Recent changes would skip this step and allow the vfs mount even
>>>> though no operations will succeed.
>>>> 
>>>> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
>>>> ---
>>>> 
>>>> Hey Chuck,
>>>> 
>>>> This fixes a regression that was introduced with the recent nfs_select_flavor 
>>>> changes - I'm pretty sure we want to match the specified flavor instead of just
>>>> using it.
>>>> 
>>>> Example of the regression:
>>>> 
>>>> the server's /etc/exports:
>>>> 
>>>> /export/krb5      *(sec=krb5,sec=none,rw,no_root_squash)
>>>> 
>>>> old client behavior:
>>>> 
>>>> $ uname -a
>>>> Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
>>>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
>>>> mount.nfs: timeout set for Sun May  5 17:32:04 2013
>>>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
>>>> mount.nfs: prog 100003, trying vers=3, prot=6
>>>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
>>>> mount.nfs: prog 100005, trying vers=3, prot=17
>>>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
>>>> mount.nfs: mount(2): Permission denied
>>>> mount.nfs: access denied by server while mounting zero:/export/krb5
>>> 
>>> This is wrong behavior, and is fixed by my patch.
>>> 
>>>> recently changed behavior:
>>>> 
>>>> $ uname -a
>>>> Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux
>>>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
>>>> mount.nfs: timeout set for Sun May  5 17:37:17 2013
>>>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
>>>> mount.nfs: prog 100003, trying vers=3, prot=6
>>>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
>>>> mount.nfs: prog 100005, trying vers=3, prot=17
>>>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
>>>> $ ls /mnt
>>>> ls: cannot open directory /mnt: Permission denied
>>>> $ sudo ls /mnt
>>>> ls: cannot open directory /mnt: Permission denied
>>>> $ sudo df /mnt
>>>> df: ‘/mnt’: Permission denied
>>>> df: no file systems processed
>>>> $ sudo umount /mnt
>>>> $
>>> 
>>> This is correct behavior.  The server should map the user's AUTH_SYS credential to anonymous.  If anonymous does not have permission on /export/krb5, then the server should prevent its access to the export.
>> 
>> You're talking about AUTH_NULL behavior here, right?
>> 
>>> 
>>> I assume this is a Linux NFS server.  Does the Linux server support sec=none listed in the export options in the same way that NetApp and Oracle Solaris do?
>> 
>> 
>> So there are two issues here and I think they're getting confused.
>> 
>> The example is of the first issue: mounting a krb5 only export with sec=sys
>> - client mounts sec=sys
>> - the server list is [krb5]
>> - the client completely ignores this list and just uses sys.
>> - no operations work, it's a 'dud mount'
>> 
>> The second issue is the AUTH_NULL stuff - lets just ignore that for now.
>> 
>> Are you saying that the client should just use whatever sec= is specified and never try to match against the server list?
> 
> That's what my patch does.  The reason for this is that the server can interpret the mount's security flavor any way it likes.
> 
> Perhaps we should take that fallback position only if the server lists AUTH_NULL in the flavor list.  If AUTH_NULL is not listed, ensure the flavor requested on the mount command line is available on the server (the most recent previous behavior); fail if no matching flavor is found.
> 
> If the mount command line does not specify a flavor, we could look in the server's list for a flavor we support, then fail if none is found.

If I'm reading this right, that's what this patch does.

 - if no sec= is specified, we look through the servers list for a supported flavor (same behavior as without this patch).

 - if sec= is specified, check the server list
   - if flavor is found, use it
   - else if AUTH_NULL is in the list, just use the user specified sec= flavor
   - else return -EPERM

Also: if we don't get a server list for whatever reason, just use the specified sec= flavor.

Does that sound right? If so, does the patch look good?

-dros

> 
>> A little background - I ran into this implementing a different patch that makes sure v4.x mounts are actually using the specified flavor (if one exists) to mount the export -- that is, it can follow SECINFOs to do initial lookup, but must be using the specified flavor on the export path passed to mount.  After the initial mount lookup, SECINFOs will be used normally. Trond thought that this was a bug that should be fixed - that it's wrong when client reports that it's using one flavor (as seen in mount output, etc) when it's really using another. This (other) patch works, but when no version is specified, it always falls back to v3 and the mount will always succeed - even if the auth flavor isn't supported by the export.
>> 
>> -dros
>> 
>> 
>>> 
>>>> I also made an attempt to support "AUTH_NULL means the server will ignore the
>>>> cred, so just use the specified flavor" behavior from much older kernels, but 
>>>> I'm not sure if we want this logic.
>>>> 
>>>> I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some 
>>>> older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified,
>>>> server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are
>>>> ignored by the server), while newer kernels do not. The workaround in newer
>>>> kernels is to specify sec=none.
>>>> 
>>>> Thoughts?
>>>> 
>>>> -dros
>>>> 
>>>> fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++-------
>>>> 1 file changed, 32 insertions(+), 7 deletions(-)
>>>> 
>>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>>> index eb494f6..6476b5d 100644
>>>> --- a/fs/nfs/super.c
>>>> +++ b/fs/nfs/super.c
>>>> @@ -1611,14 +1611,12 @@ out_security_failure:
>>>> * Select a security flavor for this mount.  The selected flavor
>>>> * is planted in args->auth_flavors[0].
>>>> */
>>>> -static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>>>> +static int nfs_select_flavor(struct nfs_parsed_mount_data *args,
>>>> 			      struct nfs_mount_request *request)
>>>> {
>>>> 	unsigned int i, count = *(request->auth_flav_len);
>>>> 	rpc_authflavor_t flavor;
>>>> -
>>>> -	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR)
>>>> -		goto out;
>>>> +	bool auth_null_seen = false;
>>>> 
>>>> 	/*
>>>> 	 * The NFSv2 MNT operation does not return a flavor list.
>>>> @@ -1634,6 +1632,26 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>>>> 		goto out_default;
>>>> 
>>>> 	/*
>>>> +	 * If the sec= mount option is used, the flavor must be in the list
>>>> +	 * returned by the server.
>>>> +	 *
>>>> +	 * One exception is when the server's flavor list includes AUTH_NULL:
>>>> +	 * Some servers implement AUTH_NULL by completely ignoring the rpc
>>>> +	 * cred, so the client can use any flavor.
>>>> +	 */
>>>> +	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
>>>> +		for (i = 0; i < count; i++) {
>>>> +			if (args->auth_flavors[0] == request->auth_flavs[i])
>>>> +				goto out;
>>>> +			if (request->auth_flavs[i] == RPC_AUTH_NULL)
>>>> +				auth_null_seen = true;
>>>> +		}
>>>> +		if (auth_null_seen)
>>>> +			goto out;
>>>> +		goto out_nomatch;
>>>> +	}
>>>> +
>>>> +	/*
>>>> 	 * RFC 2623, section 2.7 suggests we SHOULD prefer the
>>>> 	 * flavor listed first.  However, some servers list
>>>> 	 * AUTH_NULL first.  Avoid ever choosing AUTH_NULL.
>>>> @@ -1654,11 +1672,19 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>>>> 	}
>>>> 
>>>> out_default:
>>>> -	flavor = RPC_AUTH_UNIX;
>>>> +	/* use default if flavor not already set */
>>>> +	flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ?
>>>> +		RPC_AUTH_UNIX : args->auth_flavors[0];
>>>> out_set:
>>>> 	args->auth_flavors[0] = flavor;
>>>> out:
>>>> 	dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]);
>>>> +	return 0;
>>>> +
>>>> +out_nomatch:
>>>> +	dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n",
>>>> +		args->auth_flavors[0]);
>>>> +	return -EPERM;
>>>> }
>>>> 
>>>> /*
>>>> @@ -1721,8 +1747,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args,
>>>> 		return status;
>>>> 	}
>>>> 
>>>> -	nfs_select_flavor(args, &request);
>>>> -	return 0;
>>>> +	return nfs_select_flavor(args, &request);
>>>> }
>>>> 
>>>> struct dentry *nfs_try_mount(int flags, const char *dev_name,
>>>> -- 
>>>> 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 majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> 
>>> --
>>> Chuck Lever
>>> chuck[dot]lever[at]oracle[dot]com
>>> 
>>> 
>>> 
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> -- 
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adamson, Dros May 6, 2013, 3:35 p.m. UTC | #7
On May 6, 2013, at 11:32 AM, "J. Bruce Fields" <bfields@fieldses.org>
 wrote:

> On Mon, May 06, 2013 at 09:27:54AM -0400, Weston Andros Adamson wrote:
>> Older clients match the 'sec=' mount option flavor against the server's
>> flavor list (if available) and return EPERM if the specified flavor is not
>> found. Recent changes would skip this step and allow the vfs mount even
>> though no operations will succeed.
>> 
>> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
>> ---
>> 
>> Hey Chuck,
>> 
>> This fixes a regression that was introduced with the recent nfs_select_flavor 
>> changes - I'm pretty sure we want to match the specified flavor instead of just
>> using it.
>> 
>> Example of the regression:
>> 
>> the server's /etc/exports:
>> 
>> /export/krb5      *(sec=krb5,sec=none,rw,no_root_squash)
>> 
>> old client behavior:
>> 
>> $ uname -a
>> Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
>> mount.nfs: timeout set for Sun May  5 17:32:04 2013
>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
>> mount.nfs: prog 100003, trying vers=3, prot=6
>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
>> mount.nfs: prog 100005, trying vers=3, prot=17
>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
>> mount.nfs: mount(2): Permission denied
>> mount.nfs: access denied by server while mounting zero:/export/krb5
>> 
>> recently changed behavior:
>> 
>> $ uname -a
>> Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux
>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
>> mount.nfs: timeout set for Sun May  5 17:37:17 2013
>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
>> mount.nfs: prog 100003, trying vers=3, prot=6
>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
>> mount.nfs: prog 100005, trying vers=3, prot=17
>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
>> $ ls /mnt
>> ls: cannot open directory /mnt: Permission denied
>> $ sudo ls /mnt
>> ls: cannot open directory /mnt: Permission denied
>> $ sudo df /mnt
>> df: ‘/mnt’: Permission denied
>> df: no file systems processed
>> $ sudo umount /mnt
>> $
>> 
>> I also made an attempt to support "AUTH_NULL means the server will ignore the
>> cred, so just use the specified flavor" behavior from much older kernels, but 
>> I'm not sure if we want this logic.
>> 
>> I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some 
>> older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified,
>> server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are
>> ignored by the server), while newer kernels do not. The workaround in newer
>> kernels is to specify sec=none.
> 
> Nit: the above examples would actually be really useful, for example, to
> a distribution maintainer trying to figure out whether this patch would
> solve a given user problem.
> 
> Could you include them in the change log?

Yes, great suggestion. If/when we agree on this behavior, I'll repost.

-dros

> 
> --b.
> 
>> 
>> Thoughts?
>> 
>> -dros
>> 
>> fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++-------
>> 1 file changed, 32 insertions(+), 7 deletions(-)
>> 
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index eb494f6..6476b5d 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -1611,14 +1611,12 @@ out_security_failure:
>>  * Select a security flavor for this mount.  The selected flavor
>>  * is planted in args->auth_flavors[0].
>>  */
>> -static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>> +static int nfs_select_flavor(struct nfs_parsed_mount_data *args,
>> 			      struct nfs_mount_request *request)
>> {
>> 	unsigned int i, count = *(request->auth_flav_len);
>> 	rpc_authflavor_t flavor;
>> -
>> -	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR)
>> -		goto out;
>> +	bool auth_null_seen = false;
>> 
>> 	/*
>> 	 * The NFSv2 MNT operation does not return a flavor list.
>> @@ -1634,6 +1632,26 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>> 		goto out_default;
>> 
>> 	/*
>> +	 * If the sec= mount option is used, the flavor must be in the list
>> +	 * returned by the server.
>> +	 *
>> +	 * One exception is when the server's flavor list includes AUTH_NULL:
>> +	 * Some servers implement AUTH_NULL by completely ignoring the rpc
>> +	 * cred, so the client can use any flavor.
>> +	 */
>> +	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
>> +		for (i = 0; i < count; i++) {
>> +			if (args->auth_flavors[0] == request->auth_flavs[i])
>> +				goto out;
>> +			if (request->auth_flavs[i] == RPC_AUTH_NULL)
>> +				auth_null_seen = true;
>> +		}
>> +		if (auth_null_seen)
>> +			goto out;
>> +		goto out_nomatch;
>> +	}
>> +
>> +	/*
>> 	 * RFC 2623, section 2.7 suggests we SHOULD prefer the
>> 	 * flavor listed first.  However, some servers list
>> 	 * AUTH_NULL first.  Avoid ever choosing AUTH_NULL.
>> @@ -1654,11 +1672,19 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>> 	}
>> 
>> out_default:
>> -	flavor = RPC_AUTH_UNIX;
>> +	/* use default if flavor not already set */
>> +	flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ?
>> +		RPC_AUTH_UNIX : args->auth_flavors[0];
>> out_set:
>> 	args->auth_flavors[0] = flavor;
>> out:
>> 	dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]);
>> +	return 0;
>> +
>> +out_nomatch:
>> +	dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n",
>> +		args->auth_flavors[0]);
>> +	return -EPERM;
>> }
>> 
>> /*
>> @@ -1721,8 +1747,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args,
>> 		return status;
>> 	}
>> 
>> -	nfs_select_flavor(args, &request);
>> -	return 0;
>> +	return nfs_select_flavor(args, &request);
>> }
>> 
>> struct dentry *nfs_try_mount(int flags, const char *dev_name,
>> -- 
>> 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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever III May 6, 2013, 3:38 p.m. UTC | #8
On May 6, 2013, at 11:34 AM, "Adamson, Dros" <Weston.Adamson@netapp.com> wrote:

> 
> On May 6, 2013, at 11:17 AM, Chuck Lever <chuck.lever@oracle.com>
> wrote:
> 
>> 
>> On May 6, 2013, at 10:49 AM, "Adamson, Dros" <Weston.Adamson@netapp.com> wrote:
>> 
>>> 
>>> On May 6, 2013, at 9:52 AM, Chuck Lever <chuck.lever@oracle.com> wrote:
>>> 
>>>> Hi-
>>>> 
>>>> On May 6, 2013, at 9:27 AM, Weston Andros Adamson <dros@netapp.com> wrote:
>>>> 
>>>>> Older clients match the 'sec=' mount option flavor against the server's
>>>>> flavor list (if available) and return EPERM if the specified flavor is not
>>>>> found. Recent changes would skip this step and allow the vfs mount even
>>>>> though no operations will succeed.
>>>>> 
>>>>> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
>>>>> ---
>>>>> 
>>>>> Hey Chuck,
>>>>> 
>>>>> This fixes a regression that was introduced with the recent nfs_select_flavor 
>>>>> changes - I'm pretty sure we want to match the specified flavor instead of just
>>>>> using it.
>>>>> 
>>>>> Example of the regression:
>>>>> 
>>>>> the server's /etc/exports:
>>>>> 
>>>>> /export/krb5      *(sec=krb5,sec=none,rw,no_root_squash)
>>>>> 
>>>>> old client behavior:
>>>>> 
>>>>> $ uname -a
>>>>> Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
>>>>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
>>>>> mount.nfs: timeout set for Sun May  5 17:32:04 2013
>>>>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
>>>>> mount.nfs: prog 100003, trying vers=3, prot=6
>>>>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
>>>>> mount.nfs: prog 100005, trying vers=3, prot=17
>>>>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
>>>>> mount.nfs: mount(2): Permission denied
>>>>> mount.nfs: access denied by server while mounting zero:/export/krb5
>>>> 
>>>> This is wrong behavior, and is fixed by my patch.
>>>> 
>>>>> recently changed behavior:
>>>>> 
>>>>> $ uname -a
>>>>> Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux
>>>>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt
>>>>> mount.nfs: timeout set for Sun May  5 17:37:17 2013
>>>>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10'
>>>>> mount.nfs: prog 100003, trying vers=3, prot=6
>>>>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049
>>>>> mount.nfs: prog 100005, trying vers=3, prot=17
>>>>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048
>>>>> $ ls /mnt
>>>>> ls: cannot open directory /mnt: Permission denied
>>>>> $ sudo ls /mnt
>>>>> ls: cannot open directory /mnt: Permission denied
>>>>> $ sudo df /mnt
>>>>> df: ‘/mnt’: Permission denied
>>>>> df: no file systems processed
>>>>> $ sudo umount /mnt
>>>>> $
>>>> 
>>>> This is correct behavior.  The server should map the user's AUTH_SYS credential to anonymous.  If anonymous does not have permission on /export/krb5, then the server should prevent its access to the export.
>>> 
>>> You're talking about AUTH_NULL behavior here, right?
>>> 
>>>> 
>>>> I assume this is a Linux NFS server.  Does the Linux server support sec=none listed in the export options in the same way that NetApp and Oracle Solaris do?
>>> 
>>> 
>>> So there are two issues here and I think they're getting confused.
>>> 
>>> The example is of the first issue: mounting a krb5 only export with sec=sys
>>> - client mounts sec=sys
>>> - the server list is [krb5]
>>> - the client completely ignores this list and just uses sys.
>>> - no operations work, it's a 'dud mount'
>>> 
>>> The second issue is the AUTH_NULL stuff - lets just ignore that for now.
>>> 
>>> Are you saying that the client should just use whatever sec= is specified and never try to match against the server list?
>> 
>> That's what my patch does.  The reason for this is that the server can interpret the mount's security flavor any way it likes.
>> 
>> Perhaps we should take that fallback position only if the server lists AUTH_NULL in the flavor list.  If AUTH_NULL is not listed, ensure the flavor requested on the mount command line is available on the server (the most recent previous behavior); fail if no matching flavor is found.
>> 
>> If the mount command line does not specify a flavor, we could look in the server's list for a flavor we support, then fail if none is found.
> 
> If I'm reading this right, that's what this patch does.
> 
> - if no sec= is specified, we look through the servers list for a supported flavor (same behavior as without this patch).

If not found, return -EPERM.

> - if sec= is specified, check the server list
>   - if flavor is found, use it
>   - else if AUTH_NULL is in the list, just use the user specified sec= flavor
>   - else return -EPERM

Unconditionally use the user's flavor if AUTH_NULL is in the list.  So reverse the order of the first two checks.

> Also: if we don't get a server list for whatever reason, just use the specified sec= flavor.

Leave the first checks in nfs_select_flavor() intact, in other words.

> 
> Does that sound right? If so, does the patch look good?
> 
> -dros
> 
>> 
>>> A little background - I ran into this implementing a different patch that makes sure v4.x mounts are actually using the specified flavor (if one exists) to mount the export -- that is, it can follow SECINFOs to do initial lookup, but must be using the specified flavor on the export path passed to mount.  After the initial mount lookup, SECINFOs will be used normally. Trond thought that this was a bug that should be fixed - that it's wrong when client reports that it's using one flavor (as seen in mount output, etc) when it's really using another. This (other) patch works, but when no version is specified, it always falls back to v3 and the mount will always succeed - even if the auth flavor isn't supported by the export.
>>> 
>>> -dros
>>> 
>>> 
>>>> 
>>>>> I also made an attempt to support "AUTH_NULL means the server will ignore the
>>>>> cred, so just use the specified flavor" behavior from much older kernels, but 
>>>>> I'm not sure if we want this logic.
>>>>> 
>>>>> I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some 
>>>>> older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified,
>>>>> server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are
>>>>> ignored by the server), while newer kernels do not. The workaround in newer
>>>>> kernels is to specify sec=none.
>>>>> 
>>>>> Thoughts?
>>>>> 
>>>>> -dros
>>>>> 
>>>>> fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++-------
>>>>> 1 file changed, 32 insertions(+), 7 deletions(-)
>>>>> 
>>>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>>>> index eb494f6..6476b5d 100644
>>>>> --- a/fs/nfs/super.c
>>>>> +++ b/fs/nfs/super.c
>>>>> @@ -1611,14 +1611,12 @@ out_security_failure:
>>>>> * Select a security flavor for this mount.  The selected flavor
>>>>> * is planted in args->auth_flavors[0].
>>>>> */
>>>>> -static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>>>>> +static int nfs_select_flavor(struct nfs_parsed_mount_data *args,
>>>>> 			      struct nfs_mount_request *request)
>>>>> {
>>>>> 	unsigned int i, count = *(request->auth_flav_len);
>>>>> 	rpc_authflavor_t flavor;
>>>>> -
>>>>> -	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR)
>>>>> -		goto out;
>>>>> +	bool auth_null_seen = false;
>>>>> 
>>>>> 	/*
>>>>> 	 * The NFSv2 MNT operation does not return a flavor list.
>>>>> @@ -1634,6 +1632,26 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>>>>> 		goto out_default;
>>>>> 
>>>>> 	/*
>>>>> +	 * If the sec= mount option is used, the flavor must be in the list
>>>>> +	 * returned by the server.
>>>>> +	 *
>>>>> +	 * One exception is when the server's flavor list includes AUTH_NULL:
>>>>> +	 * Some servers implement AUTH_NULL by completely ignoring the rpc
>>>>> +	 * cred, so the client can use any flavor.
>>>>> +	 */
>>>>> +	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
>>>>> +		for (i = 0; i < count; i++) {
>>>>> +			if (args->auth_flavors[0] == request->auth_flavs[i])
>>>>> +				goto out;
>>>>> +			if (request->auth_flavs[i] == RPC_AUTH_NULL)
>>>>> +				auth_null_seen = true;
>>>>> +		}
>>>>> +		if (auth_null_seen)
>>>>> +			goto out;
>>>>> +		goto out_nomatch;
>>>>> +	}
>>>>> +
>>>>> +	/*
>>>>> 	 * RFC 2623, section 2.7 suggests we SHOULD prefer the
>>>>> 	 * flavor listed first.  However, some servers list
>>>>> 	 * AUTH_NULL first.  Avoid ever choosing AUTH_NULL.
>>>>> @@ -1654,11 +1672,19 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>>>>> 	}
>>>>> 
>>>>> out_default:
>>>>> -	flavor = RPC_AUTH_UNIX;
>>>>> +	/* use default if flavor not already set */
>>>>> +	flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ?
>>>>> +		RPC_AUTH_UNIX : args->auth_flavors[0];
>>>>> out_set:
>>>>> 	args->auth_flavors[0] = flavor;
>>>>> out:
>>>>> 	dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]);
>>>>> +	return 0;
>>>>> +
>>>>> +out_nomatch:
>>>>> +	dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n",
>>>>> +		args->auth_flavors[0]);
>>>>> +	return -EPERM;
>>>>> }
>>>>> 
>>>>> /*
>>>>> @@ -1721,8 +1747,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args,
>>>>> 		return status;
>>>>> 	}
>>>>> 
>>>>> -	nfs_select_flavor(args, &request);
>>>>> -	return 0;
>>>>> +	return nfs_select_flavor(args, &request);
>>>>> }
>>>>> 
>>>>> struct dentry *nfs_try_mount(int flags, const char *dev_name,
>>>>> -- 
>>>>> 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 majordomo@vger.kernel.org
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>> 
>>>> --
>>>> Chuck Lever
>>>> chuck[dot]lever[at]oracle[dot]com
>>>> 
>>>> 
>>>> 
>>> 
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
>> -- 
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com
>> 
>> 
>> 
>> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index eb494f6..6476b5d 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1611,14 +1611,12 @@  out_security_failure:
  * Select a security flavor for this mount.  The selected flavor
  * is planted in args->auth_flavors[0].
  */
-static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
+static int nfs_select_flavor(struct nfs_parsed_mount_data *args,
 			      struct nfs_mount_request *request)
 {
 	unsigned int i, count = *(request->auth_flav_len);
 	rpc_authflavor_t flavor;
-
-	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR)
-		goto out;
+	bool auth_null_seen = false;
 
 	/*
 	 * The NFSv2 MNT operation does not return a flavor list.
@@ -1634,6 +1632,26 @@  static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
 		goto out_default;
 
 	/*
+	 * If the sec= mount option is used, the flavor must be in the list
+	 * returned by the server.
+	 *
+	 * One exception is when the server's flavor list includes AUTH_NULL:
+	 * Some servers implement AUTH_NULL by completely ignoring the rpc
+	 * cred, so the client can use any flavor.
+	 */
+	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
+		for (i = 0; i < count; i++) {
+			if (args->auth_flavors[0] == request->auth_flavs[i])
+				goto out;
+			if (request->auth_flavs[i] == RPC_AUTH_NULL)
+				auth_null_seen = true;
+		}
+		if (auth_null_seen)
+			goto out;
+		goto out_nomatch;
+	}
+
+	/*
 	 * RFC 2623, section 2.7 suggests we SHOULD prefer the
 	 * flavor listed first.  However, some servers list
 	 * AUTH_NULL first.  Avoid ever choosing AUTH_NULL.
@@ -1654,11 +1672,19 @@  static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
 	}
 
 out_default:
-	flavor = RPC_AUTH_UNIX;
+	/* use default if flavor not already set */
+	flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ?
+		RPC_AUTH_UNIX : args->auth_flavors[0];
 out_set:
 	args->auth_flavors[0] = flavor;
 out:
 	dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]);
+	return 0;
+
+out_nomatch:
+	dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n",
+		args->auth_flavors[0]);
+	return -EPERM;
 }
 
 /*
@@ -1721,8 +1747,7 @@  static int nfs_request_mount(struct nfs_parsed_mount_data *args,
 		return status;
 	}
 
-	nfs_select_flavor(args, &request);
-	return 0;
+	return nfs_select_flavor(args, &request);
 }
 
 struct dentry *nfs_try_mount(int flags, const char *dev_name,