[PATCH] NFSv3: match sec= flavor against server list

[PATCH] NFSv3: match sec= flavor against server list

[Weston Andros Adamson]

Older linux clients match the 'sec=' mount option flavor against the server's
flavor list (if available) and return EPERM if the specified flavor or AUTH_NULL
(which "matches" any flavor) is not found.

Recent changes skip this step and allow the vfs mount even though no operations
will succeed, creating a 'dud' mount.

This patch reverts back to the old behavior of matching specified flavors
against the server list and also returns EPERM when no sec= is specified and
none of the flavors returned by the server are supported by the client.

Example of behavior change:

the server's /etc/exports:

/export/krb5      *(sec=krb5,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
$

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

Version 2:
 - now just uses specified flavor when:
   - AUTH_NULL is in the server list
   - flavor is in the server list

 - now returns EPERM when no sec= specified and no client supported flavors are
   in the server list

 - includes example in the changelog

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

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index eb494f6..0e36807 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,21 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
 		goto out_default;
 
 	/*
+	 * If the sec= mount option is used, the specified flavor or AUTH_NULL
+	 * must be in the list returned by the server.
+	 */
+	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
+		for (i = 0; i < count; i++) {
+			if (args->auth_flavors[0] == request->auth_flavs[i] ||
+			    request->auth_flavs[i] == RPC_AUTH_NULL)
+				goto out;
+		}
+		dfprintk(MOUNT, "NFS: auth flavor %d not supported server\n",
+			args->auth_flavors[0]);
+		goto out_err;
+	}
+
+	/*
 	 * 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.
@@ -1646,6 +1659,7 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
 		case RPC_AUTH_UNIX:
 			goto out_set;
 		case RPC_AUTH_NULL:
+			auth_null_seen = true;
 			continue;
 		default:
 			if (rpcauth_get_gssinfo(flavor, &info) == 0)
@@ -1653,12 +1667,22 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
 		}
 	}
 
+	if (!auth_null_seen) {
+		dfprintk(MOUNT, "NFS: no auth flavors in common with server\n");
+		goto out_err;
+	}
+
 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_err:
+	return -EPERM;
 }
 
 /*
@@ -1721,8 +1745,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)

Re: [PATCH] NFSv3: match sec= flavor against server list

[Myklebust, Trond]

T24gTW9uLCAyMDEzLTA1LTA2IGF0IDE1OjQxIC0wNDAwLCBXZXN0b24gQW5kcm9zIEFkYW1zb24g
d3JvdGU6DQo+IE9sZGVyIGxpbnV4IGNsaWVudHMgbWF0Y2ggdGhlICdzZWM9JyBtb3VudCBvcHRp
b24gZmxhdm9yIGFnYWluc3QgdGhlIHNlcnZlcidzDQo+IGZsYXZvciBsaXN0IChpZiBhdmFpbGFi
bGUpIGFuZCByZXR1cm4gRVBFUk0gaWYgdGhlIHNwZWNpZmllZCBmbGF2b3Igb3IgQVVUSF9OVUxM
DQo+ICh3aGljaCAibWF0Y2hlcyIgYW55IGZsYXZvcikgaXMgbm90IGZvdW5kLg0KPiANCj4gUmVj
ZW50IGNoYW5nZXMgc2tpcCB0aGlzIHN0ZXAgYW5kIGFsbG93IHRoZSB2ZnMgbW91bnQgZXZlbiB0
aG91Z2ggbm8gb3BlcmF0aW9ucw0KPiB3aWxsIHN1Y2NlZWQsIGNyZWF0aW5nIGEgJ2R1ZCcgbW91
bnQuDQo+IA0KPiBUaGlzIHBhdGNoIHJldmVydHMgYmFjayB0byB0aGUgb2xkIGJlaGF2aW9yIG9m
IG1hdGNoaW5nIHNwZWNpZmllZCBmbGF2b3JzDQo+IGFnYWluc3QgdGhlIHNlcnZlciBsaXN0IGFu
ZCBhbHNvIHJldHVybnMgRVBFUk0gd2hlbiBubyBzZWM9IGlzIHNwZWNpZmllZCBhbmQNCj4gbm9u
ZSBvZiB0aGUgZmxhdm9ycyByZXR1cm5lZCBieSB0aGUgc2VydmVyIGFyZSBzdXBwb3J0ZWQgYnkg
dGhlIGNsaWVudC4NCj4gDQo+IEV4YW1wbGUgb2YgYmVoYXZpb3IgY2hhbmdlOg0KPiANCj4gdGhl
IHNlcnZlcidzIC9ldGMvZXhwb3J0czoNCj4gDQo+IC9leHBvcnQva3JiNSAgICAgICooc2VjPWty
YjUscncsbm9fcm9vdF9zcXVhc2gpDQo+IA0KPiBvbGQgY2xpZW50IGJlaGF2aW9yOg0KPiANCj4g
JCB1bmFtZSAtYQ0KPiBMaW51eCBvbmUuYXBpa2lhLmZha2UgMy44LjgtMjAyLmZjMTgueDg2XzY0
ICMxIFNNUCBXZWQgQXByIDE3IDIzOjI1OjE3IFVUQyAyMDEzIHg4Nl82NCB4ODZfNjQgeDg2XzY0
IEdOVS9MaW51eA0KPiAkIHN1ZG8gbW91bnQgLXYgLW8gc2VjPXN5cyx2ZXJzPTMgemVybzovZXhw
b3J0L2tyYjUgL21udA0KPiBtb3VudC5uZnM6IHRpbWVvdXQgc2V0IGZvciBTdW4gTWF5ICA1IDE3
OjMyOjA0IDIwMTMNCj4gbW91bnQubmZzOiB0cnlpbmcgdGV4dC1iYXNlZCBvcHRpb25zICdzZWM9
c3lzLHZlcnM9MyxhZGRyPTE5Mi4xNjguMTAwLjEwJw0KPiBtb3VudC5uZnM6IHByb2cgMTAwMDAz
LCB0cnlpbmcgdmVycz0zLCBwcm90PTYNCj4gbW91bnQubmZzOiB0cnlpbmcgMTkyLjE2OC4xMDAu
MTAgcHJvZyAxMDAwMDMgdmVycyAzIHByb3QgVENQIHBvcnQgMjA0OQ0KPiBtb3VudC5uZnM6IHBy
b2cgMTAwMDA1LCB0cnlpbmcgdmVycz0zLCBwcm90PTE3DQo+IG1vdW50Lm5mczogdHJ5aW5nIDE5
Mi4xNjguMTAwLjEwIHByb2cgMTAwMDA1IHZlcnMgMyBwcm90IFVEUCBwb3J0IDIwMDQ4DQo+IG1v
dW50Lm5mczogbW91bnQoMik6IFBlcm1pc3Npb24gZGVuaWVkDQo+IG1vdW50Lm5mczogYWNjZXNz
IGRlbmllZCBieSBzZXJ2ZXIgd2hpbGUgbW91bnRpbmcgemVybzovZXhwb3J0L2tyYjUNCj4gDQo+
IHJlY2VudGx5IGNoYW5nZWQgYmVoYXZpb3I6DQo+IA0KPiAkIHVuYW1lIC1hDQo+IExpbnV4IG9u
ZS5hcGlraWEuZmFrZSAzLjkuMC10ZXN0aW5nKyAjMiBTTVAgRnJpIE1heSAzIDIwOjI5OjMyIEVE
VCAyMDEzIHg4Nl82NCB4ODZfNjQgeDg2XzY0IEdOVS9MaW51eA0KPiAkIHN1ZG8gbW91bnQgLXYg
LW8gc2VjPXN5cyx2ZXJzPTMgemVybzovZXhwb3J0L2tyYjUgL21udA0KPiBtb3VudC5uZnM6IHRp
bWVvdXQgc2V0IGZvciBTdW4gTWF5ICA1IDE3OjM3OjE3IDIwMTMNCj4gbW91bnQubmZzOiB0cnlp
bmcgdGV4dC1iYXNlZCBvcHRpb25zICdzZWM9c3lzLHZlcnM9MyxhZGRyPTE5Mi4xNjguMTAwLjEw
Jw0KPiBtb3VudC5uZnM6IHByb2cgMTAwMDAzLCB0cnlpbmcgdmVycz0zLCBwcm90PTYNCj4gbW91
bnQubmZzOiB0cnlpbmcgMTkyLjE2OC4xMDAuMTAgcHJvZyAxMDAwMDMgdmVycyAzIHByb3QgVENQ
IHBvcnQgMjA0OQ0KPiBtb3VudC5uZnM6IHByb2cgMTAwMDA1LCB0cnlpbmcgdmVycz0zLCBwcm90
PTE3DQo+IG1vdW50Lm5mczogdHJ5aW5nIDE5Mi4xNjguMTAwLjEwIHByb2cgMTAwMDA1IHZlcnMg
MyBwcm90IFVEUCBwb3J0IDIwMDQ4DQo+ICQgbHMgL21udA0KPiBsczogY2Fubm90IG9wZW4gZGly
ZWN0b3J5IC9tbnQ6IFBlcm1pc3Npb24gZGVuaWVkDQo+ICQgc3VkbyBscyAvbW50DQo+IGxzOiBj
YW5ub3Qgb3BlbiBkaXJlY3RvcnkgL21udDogUGVybWlzc2lvbiBkZW5pZWQNCj4gJCBzdWRvIGRm
IC9tbnQNCj4gZGY6IOKAmC9tbnTigJk6IFBlcm1pc3Npb24gZGVuaWVkDQo+IGRmOiBubyBmaWxl
IHN5c3RlbXMgcHJvY2Vzc2VkDQo+ICQgc3VkbyB1bW91bnQgL21udA0KPiAkDQo+IA0KPiBTaWdu
ZWQtb2ZmLWJ5OiBXZXN0b24gQW5kcm9zIEFkYW1zb24gPGRyb3NAbmV0YXBwLmNvbT4NCj4gLS0t
DQo+IA0KPiBWZXJzaW9uIDI6DQo+ICAtIG5vdyBqdXN0IHVzZXMgc3BlY2lmaWVkIGZsYXZvciB3
aGVuOg0KPiAgICAtIEFVVEhfTlVMTCBpcyBpbiB0aGUgc2VydmVyIGxpc3QNCj4gICAgLSBmbGF2
b3IgaXMgaW4gdGhlIHNlcnZlciBsaXN0DQo+IA0KPiAgLSBub3cgcmV0dXJucyBFUEVSTSB3aGVu
IG5vIHNlYz0gc3BlY2lmaWVkIGFuZCBubyBjbGllbnQgc3VwcG9ydGVkIGZsYXZvcnMgYXJlDQo+
ICAgIGluIHRoZSBzZXJ2ZXIgbGlzdA0KPiANCg0KU2hvdWxkbid0IHRoYXQgYmUgRUFDQ0VTPw0K
DQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5l
dEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQo=

Re: [PATCH] NFSv3: match sec= flavor against server list

[Adamson, Dros]

On May 6, 2013, at 3:43 PM, "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> On Mon, 2013-05-06 at 15:41 -0400, Weston Andros Adamson wrote:
>> Older linux clients match the 'sec=' mount option flavor against the server's
>> flavor list (if available) and return EPERM if the specified flavor or AUTH_NULL
>> (which "matches" any flavor) is not found.
>> 
>> Recent changes skip this step and allow the vfs mount even though no operations
>> will succeed, creating a 'dud' mount.
>> 
>> This patch reverts back to the old behavior of matching specified flavors
>> against the server list and also returns EPERM when no sec= is specified and
>> none of the flavors returned by the server are supported by the client.
>> 
>> Example of behavior change:
>> 
>> the server's /etc/exports:
>> 
>> /export/krb5      *(sec=krb5,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
>> $
>> 
>> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
>> ---
>> 
>> Version 2:
>> - now just uses specified flavor when:
>>   - AUTH_NULL is in the server list
>>   - flavor is in the server list
>> 
>> - now returns EPERM when no sec= specified and no client supported flavors are
>>   in the server list
>> 
> 
> Shouldn't that be EACCES?

Yes. I think both cases should be EACCES.

Will test and repost

-dros

> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer
> 
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com

Re: [PATCH] NFSv3: match sec= flavor against server list

[Chuck Lever]

On May 6, 2013, at 3:41 PM, Weston Andros Adamson <dros@netapp.com> wrote:

> Older linux clients match the 'sec=' mount option flavor against the server's
> flavor list (if available) and return EPERM if the specified flavor or AUTH_NULL
> (which "matches" any flavor) is not found.
> 
> Recent changes skip this step and allow the vfs mount even though no operations
> will succeed, creating a 'dud' mount.
> 
> This patch reverts back to the old behavior of matching specified flavors
> against the server list and also returns EPERM when no sec= is specified and
> none of the flavors returned by the server are supported by the client.
> 
> Example of behavior change:
> 
> the server's /etc/exports:
> 
> /export/krb5      *(sec=krb5,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
> $
> 
> Signed-off-by: Weston Andros Adamson <dros@netapp.com>

I don't have any hard objections to this patch, so barring the change to return EACCES,

  Acked-by: Chuck Lever <chuck.lever@oracle.com>

Just a nit, perhaps, but...

I think that the optimization of setting the boolean, so that only one walk through the server list is needed, is premature.  It might be more clear if each test was done independently of the next to make it easier to reorder the tests, fix bugs in each test, and for readers to see exactly what each of these tests do.

This code is really our only documentation of the security flavor selection process, so making it as clear as possible should be a priority, IMO, over saving a few CPU cycles.


> ---
> 
> Version 2:
> - now just uses specified flavor when:
>   - AUTH_NULL is in the server list
>   - flavor is in the server list
> 
> - now returns EPERM when no sec= specified and no client supported flavors are
>   in the server list
> 
> - includes example in the changelog
> 
> fs/nfs/super.c | 37 ++++++++++++++++++++++++++++++-------
> 1 file changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index eb494f6..0e36807 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,21 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
> 		goto out_default;
> 
> 	/*
> +	 * If the sec= mount option is used, the specified flavor or AUTH_NULL
> +	 * must be in the list returned by the server.
> +	 */
> +	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
> +		for (i = 0; i < count; i++) {
> +			if (args->auth_flavors[0] == request->auth_flavs[i] ||
> +			    request->auth_flavs[i] == RPC_AUTH_NULL)
> +				goto out;
> +		}
> +		dfprintk(MOUNT, "NFS: auth flavor %d not supported server\n",
> +			args->auth_flavors[0]);
> +		goto out_err;
> +	}
> +
> +	/*
> 	 * 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.
> @@ -1646,6 +1659,7 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
> 		case RPC_AUTH_UNIX:
> 			goto out_set;
> 		case RPC_AUTH_NULL:
> +			auth_null_seen = true;
> 			continue;
> 		default:
> 			if (rpcauth_get_gssinfo(flavor, &info) == 0)
> @@ -1653,12 +1667,22 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
> 		}
> 	}
> 
> +	if (!auth_null_seen) {
> +		dfprintk(MOUNT, "NFS: no auth flavors in common with server\n");
> +		goto out_err;
> +	}
> +
> 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_err:
> +	return -EPERM;
> }
> 
> /*
> @@ -1721,8 +1745,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

Re: [PATCH] NFSv3: match sec= flavor against server list

[Adamson, Dros]

On May 6, 2013, at 4:44 PM, Chuck Lever <chuck.lever@oracle.com> wrote:

> 
> On May 6, 2013, at 3:41 PM, Weston Andros Adamson <dros@netapp.com> wrote:
> 
>> Older linux clients match the 'sec=' mount option flavor against the server's
>> flavor list (if available) and return EPERM if the specified flavor or AUTH_NULL
>> (which "matches" any flavor) is not found.
>> 
>> Recent changes skip this step and allow the vfs mount even though no operations
>> will succeed, creating a 'dud' mount.
>> 
>> This patch reverts back to the old behavior of matching specified flavors
>> against the server list and also returns EPERM when no sec= is specified and
>> none of the flavors returned by the server are supported by the client.
>> 
>> Example of behavior change:
>> 
>> the server's /etc/exports:
>> 
>> /export/krb5      *(sec=krb5,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
>> $
>> 
>> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
> 
> I don't have any hard objections to this patch, so barring the change to return EACCES,
> 
>  Acked-by: Chuck Lever <chuck.lever@oracle.com>
> 
> Just a nit, perhaps, but...
> 
> I think that the optimization of setting the boolean, so that only one walk through the server list is needed, is premature.  It might be more clear if each test was done independently of the next to make it easier to reorder the tests, fix bugs in each test, and for readers to see exactly what each of these tests do.
> 
> This code is really our only documentation of the security flavor selection process, so making it as clear as possible should be a priority, IMO, over saving a few CPU cycles.
> 

I have no problem with this.  Will post a v4 patch.

-dros

> 
>> ---
>> 
>> Version 2:
>> - now just uses specified flavor when:
>>  - AUTH_NULL is in the server list
>>  - flavor is in the server list
>> 
>> - now returns EPERM when no sec= specified and no client supported flavors are
>>  in the server list
>> 
>> - includes example in the changelog
>> 
>> fs/nfs/super.c | 37 ++++++++++++++++++++++++++++++-------
>> 1 file changed, 30 insertions(+), 7 deletions(-)
>> 
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index eb494f6..0e36807 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,21 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>> 		goto out_default;
>> 
>> 	/*
>> +	 * If the sec= mount option is used, the specified flavor or AUTH_NULL
>> +	 * must be in the list returned by the server.
>> +	 */
>> +	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
>> +		for (i = 0; i < count; i++) {
>> +			if (args->auth_flavors[0] == request->auth_flavs[i] ||
>> +			    request->auth_flavs[i] == RPC_AUTH_NULL)
>> +				goto out;
>> +		}
>> +		dfprintk(MOUNT, "NFS: auth flavor %d not supported server\n",
>> +			args->auth_flavors[0]);
>> +		goto out_err;
>> +	}
>> +
>> +	/*
>> 	 * 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.
>> @@ -1646,6 +1659,7 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>> 		case RPC_AUTH_UNIX:
>> 			goto out_set;
>> 		case RPC_AUTH_NULL:
>> +			auth_null_seen = true;
>> 			continue;
>> 		default:
>> 			if (rpcauth_get_gssinfo(flavor, &info) == 0)
>> @@ -1653,12 +1667,22 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
>> 		}
>> 	}
>> 
>> +	if (!auth_null_seen) {
>> +		dfprintk(MOUNT, "NFS: no auth flavors in common with server\n");
>> +		goto out_err;
>> +	}
>> +
>> 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_err:
>> +	return -EPERM;
>> }
>> 
>> /*
>> @@ -1721,8 +1745,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
> 
> 
> 
> 

[PATCH] NFSv3: match sec= flavor against server list

[Weston Andros Adamson]

Older linux clients match the 'sec=' mount option flavor against the server's
flavor list (if available) and return EPERM if the specified flavor or AUTH_NULL
(which "matches" any flavor) is not found.

Recent changes skip this step and allow the vfs mount even though no operations
will succeed, creating a 'dud' mount.

This patch reverts back to the old behavior of matching specified flavors
against the server list and also returns EPERM when no sec= is specified and
none of the flavors returned by the server are supported by the client.

Example of behavior change:

the server's /etc/exports:

/export/krb5      *(sec=krb5,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
$

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

V4 - better readability, better comments

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

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index eb494f6..53c2657 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1610,16 +1610,15 @@ out_security_failure:
 /*
  * Select a security flavor for this mount.  The selected flavor
  * is planted in args->auth_flavors[0].
+ *
+ * Returns 0 on success, -EACCES on failure.
  */
-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;
-
 	/*
 	 * The NFSv2 MNT operation does not return a flavor list.
 	 */
@@ -1634,6 +1633,25 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
 		goto out_default;
 
 	/*
+	 * If the sec= mount option is used, the specified flavor or AUTH_NULL
+	 * must be in the list returned by the server.
+	 *
+	 * AUTH_NULL has a special meaning when it's in the server list - it
+	 * means that the server will ignore the rpc creds, so any flavor
+	 * can be used.
+	 */
+	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
+		for (i = 0; i < count; i++) {
+			if (args->auth_flavors[0] == request->auth_flavs[i] ||
+			    request->auth_flavs[i] == RPC_AUTH_NULL)
+				goto out;
+		}
+		dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n",
+			args->auth_flavors[0]);
+		goto out_err;
+	}
+
+	/*
 	 * 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.
@@ -1653,12 +1671,29 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
 		}
 	}
 
+	/*
+	 * As a last chance, see if the server list contains AUTH_NULL -
+	 * if it does, use the default flavor.
+	 */
+	for (i = 0; i < count; i++) {
+		if (request->auth_flavs[i] == RPC_AUTH_NULL)
+			goto out_default;
+	}
+
+	dfprintk(MOUNT, "NFS: no auth flavors in common with server\n");
+	goto out_err;
+
 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_err:
+	return -EACCES;
 }
 
 /*
@@ -1721,8 +1756,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)

Re: [PATCH] NFSv3: match sec= flavor against server list

[Myklebust, Trond]

T24gTW9uLCAyMDEzLTA1LTA2IGF0IDE3OjEyIC0wNDAwLCBXZXN0b24gQW5kcm9zIEFkYW1zb24g
d3JvdGU6DQo+IE9sZGVyIGxpbnV4IGNsaWVudHMgbWF0Y2ggdGhlICdzZWM9JyBtb3VudCBvcHRp
b24gZmxhdm9yIGFnYWluc3QgdGhlIHNlcnZlcidzDQo+IGZsYXZvciBsaXN0IChpZiBhdmFpbGFi
bGUpIGFuZCByZXR1cm4gRVBFUk0gaWYgdGhlIHNwZWNpZmllZCBmbGF2b3Igb3IgQVVUSF9OVUxM
DQo+ICh3aGljaCAibWF0Y2hlcyIgYW55IGZsYXZvcikgaXMgbm90IGZvdW5kLg0KPiANCj4gUmVj
ZW50IGNoYW5nZXMgc2tpcCB0aGlzIHN0ZXAgYW5kIGFsbG93IHRoZSB2ZnMgbW91bnQgZXZlbiB0
aG91Z2ggbm8gb3BlcmF0aW9ucw0KPiB3aWxsIHN1Y2NlZWQsIGNyZWF0aW5nIGEgJ2R1ZCcgbW91
bnQuDQo+IA0KPiBUaGlzIHBhdGNoIHJldmVydHMgYmFjayB0byB0aGUgb2xkIGJlaGF2aW9yIG9m
IG1hdGNoaW5nIHNwZWNpZmllZCBmbGF2b3JzDQo+IGFnYWluc3QgdGhlIHNlcnZlciBsaXN0IGFu
ZCBhbHNvIHJldHVybnMgRVBFUk0gd2hlbiBubyBzZWM9IGlzIHNwZWNpZmllZCBhbmQNCj4gbm9u
ZSBvZiB0aGUgZmxhdm9ycyByZXR1cm5lZCBieSB0aGUgc2VydmVyIGFyZSBzdXBwb3J0ZWQgYnkg
dGhlIGNsaWVudC4NCj4gDQo+IEV4YW1wbGUgb2YgYmVoYXZpb3IgY2hhbmdlOg0KPiANCj4gdGhl
IHNlcnZlcidzIC9ldGMvZXhwb3J0czoNCj4gDQo+IC9leHBvcnQva3JiNSAgICAgICooc2VjPWty
YjUscncsbm9fcm9vdF9zcXVhc2gpDQo+IA0KPiBvbGQgY2xpZW50IGJlaGF2aW9yOg0KPiANCj4g
JCB1bmFtZSAtYQ0KPiBMaW51eCBvbmUuYXBpa2lhLmZha2UgMy44LjgtMjAyLmZjMTgueDg2XzY0
ICMxIFNNUCBXZWQgQXByIDE3IDIzOjI1OjE3IFVUQyAyMDEzIHg4Nl82NCB4ODZfNjQgeDg2XzY0
IEdOVS9MaW51eA0KPiAkIHN1ZG8gbW91bnQgLXYgLW8gc2VjPXN5cyx2ZXJzPTMgemVybzovZXhw
b3J0L2tyYjUgL21udA0KPiBtb3VudC5uZnM6IHRpbWVvdXQgc2V0IGZvciBTdW4gTWF5ICA1IDE3
OjMyOjA0IDIwMTMNCj4gbW91bnQubmZzOiB0cnlpbmcgdGV4dC1iYXNlZCBvcHRpb25zICdzZWM9
c3lzLHZlcnM9MyxhZGRyPTE5Mi4xNjguMTAwLjEwJw0KPiBtb3VudC5uZnM6IHByb2cgMTAwMDAz
LCB0cnlpbmcgdmVycz0zLCBwcm90PTYNCj4gbW91bnQubmZzOiB0cnlpbmcgMTkyLjE2OC4xMDAu
MTAgcHJvZyAxMDAwMDMgdmVycyAzIHByb3QgVENQIHBvcnQgMjA0OQ0KPiBtb3VudC5uZnM6IHBy
b2cgMTAwMDA1LCB0cnlpbmcgdmVycz0zLCBwcm90PTE3DQo+IG1vdW50Lm5mczogdHJ5aW5nIDE5
Mi4xNjguMTAwLjEwIHByb2cgMTAwMDA1IHZlcnMgMyBwcm90IFVEUCBwb3J0IDIwMDQ4DQo+IG1v
dW50Lm5mczogbW91bnQoMik6IFBlcm1pc3Npb24gZGVuaWVkDQo+IG1vdW50Lm5mczogYWNjZXNz
IGRlbmllZCBieSBzZXJ2ZXIgd2hpbGUgbW91bnRpbmcgemVybzovZXhwb3J0L2tyYjUNCj4gDQo+
IHJlY2VudGx5IGNoYW5nZWQgYmVoYXZpb3I6DQo+IA0KPiAkIHVuYW1lIC1hDQo+IExpbnV4IG9u
ZS5hcGlraWEuZmFrZSAzLjkuMC10ZXN0aW5nKyAjMiBTTVAgRnJpIE1heSAzIDIwOjI5OjMyIEVE
VCAyMDEzIHg4Nl82NCB4ODZfNjQgeDg2XzY0IEdOVS9MaW51eA0KPiAkIHN1ZG8gbW91bnQgLXYg
LW8gc2VjPXN5cyx2ZXJzPTMgemVybzovZXhwb3J0L2tyYjUgL21udA0KPiBtb3VudC5uZnM6IHRp
bWVvdXQgc2V0IGZvciBTdW4gTWF5ICA1IDE3OjM3OjE3IDIwMTMNCj4gbW91bnQubmZzOiB0cnlp
bmcgdGV4dC1iYXNlZCBvcHRpb25zICdzZWM9c3lzLHZlcnM9MyxhZGRyPTE5Mi4xNjguMTAwLjEw
Jw0KPiBtb3VudC5uZnM6IHByb2cgMTAwMDAzLCB0cnlpbmcgdmVycz0zLCBwcm90PTYNCj4gbW91
bnQubmZzOiB0cnlpbmcgMTkyLjE2OC4xMDAuMTAgcHJvZyAxMDAwMDMgdmVycyAzIHByb3QgVENQ
IHBvcnQgMjA0OQ0KPiBtb3VudC5uZnM6IHByb2cgMTAwMDA1LCB0cnlpbmcgdmVycz0zLCBwcm90
PTE3DQo+IG1vdW50Lm5mczogdHJ5aW5nIDE5Mi4xNjguMTAwLjEwIHByb2cgMTAwMDA1IHZlcnMg
MyBwcm90IFVEUCBwb3J0IDIwMDQ4DQo+ICQgbHMgL21udA0KPiBsczogY2Fubm90IG9wZW4gZGly
ZWN0b3J5IC9tbnQ6IFBlcm1pc3Npb24gZGVuaWVkDQo+ICQgc3VkbyBscyAvbW50DQo+IGxzOiBj
YW5ub3Qgb3BlbiBkaXJlY3RvcnkgL21udDogUGVybWlzc2lvbiBkZW5pZWQNCj4gJCBzdWRvIGRm
IC9tbnQNCj4gZGY6IOKAmC9tbnTigJk6IFBlcm1pc3Npb24gZGVuaWVkDQo+IGRmOiBubyBmaWxl
IHN5c3RlbXMgcHJvY2Vzc2VkDQo+ICQgc3VkbyB1bW91bnQgL21udA0KPiAkDQo+IA0KPiBTaWdu
ZWQtb2ZmLWJ5OiBXZXN0b24gQW5kcm9zIEFkYW1zb24gPGRyb3NAbmV0YXBwLmNvbT4NCj4gLS0t
DQo+IA0KPiBWNCAtIGJldHRlciByZWFkYWJpbGl0eSwgYmV0dGVyIGNvbW1lbnRzDQo+IA0KPiAg
ZnMvbmZzL3N1cGVyLmMgfCA0OCArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr
KysrKy0tLS0tLS0NCj4gIDEgZmlsZSBjaGFuZ2VkLCA0MSBpbnNlcnRpb25zKCspLCA3IGRlbGV0
aW9ucygtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL2ZzL25mcy9zdXBlci5jIGIvZnMvbmZzL3N1cGVy
LmMNCj4gaW5kZXggZWI0OTRmNi4uNTNjMjY1NyAxMDA2NDQNCj4gLS0tIGEvZnMvbmZzL3N1cGVy
LmMNCj4gKysrIGIvZnMvbmZzL3N1cGVyLmMNCj4gQEAgLTE2MTAsMTYgKzE2MTAsMTUgQEAgb3V0
X3NlY3VyaXR5X2ZhaWx1cmU6DQo+ICAvKg0KPiAgICogU2VsZWN0IGEgc2VjdXJpdHkgZmxhdm9y
IGZvciB0aGlzIG1vdW50LiAgVGhlIHNlbGVjdGVkIGZsYXZvcg0KPiAgICogaXMgcGxhbnRlZCBp
biBhcmdzLT5hdXRoX2ZsYXZvcnNbMF0uDQo+ICsgKg0KPiArICogUmV0dXJucyAwIG9uIHN1Y2Nl
c3MsIC1FQUNDRVMgb24gZmFpbHVyZS4NCj4gICAqLw0KPiAtc3RhdGljIHZvaWQgbmZzX3NlbGVj
dF9mbGF2b3Ioc3RydWN0IG5mc19wYXJzZWRfbW91bnRfZGF0YSAqYXJncywNCj4gK3N0YXRpYyBp
bnQgbmZzX3NlbGVjdF9mbGF2b3Ioc3RydWN0IG5mc19wYXJzZWRfbW91bnRfZGF0YSAqYXJncywN
Cj4gIAkJCSAgICAgIHN0cnVjdCBuZnNfbW91bnRfcmVxdWVzdCAqcmVxdWVzdCkNCj4gIHsNCj4g
IAl1bnNpZ25lZCBpbnQgaSwgY291bnQgPSAqKHJlcXVlc3QtPmF1dGhfZmxhdl9sZW4pOw0KPiAg
CXJwY19hdXRoZmxhdm9yX3QgZmxhdm9yOw0KPiAgDQo+IC0JaWYgKGFyZ3MtPmF1dGhfZmxhdm9y
c1swXSAhPSBSUENfQVVUSF9NQVhGTEFWT1IpDQo+IC0JCWdvdG8gb3V0Ow0KPiAtDQo+ICAJLyoN
Cj4gIAkgKiBUaGUgTkZTdjIgTU5UIG9wZXJhdGlvbiBkb2VzIG5vdCByZXR1cm4gYSBmbGF2b3Ig
bGlzdC4NCj4gIAkgKi8NCj4gQEAgLTE2MzQsNiArMTYzMywyNSBAQCBzdGF0aWMgdm9pZCBuZnNf
c2VsZWN0X2ZsYXZvcihzdHJ1Y3QgbmZzX3BhcnNlZF9tb3VudF9kYXRhICphcmdzLA0KPiAgCQln
b3RvIG91dF9kZWZhdWx0Ow0KPiAgDQo+ICAJLyoNCj4gKwkgKiBJZiB0aGUgc2VjPSBtb3VudCBv
cHRpb24gaXMgdXNlZCwgdGhlIHNwZWNpZmllZCBmbGF2b3Igb3IgQVVUSF9OVUxMDQo+ICsJICog
bXVzdCBiZSBpbiB0aGUgbGlzdCByZXR1cm5lZCBieSB0aGUgc2VydmVyLg0KPiArCSAqDQo+ICsJ
ICogQVVUSF9OVUxMIGhhcyBhIHNwZWNpYWwgbWVhbmluZyB3aGVuIGl0J3MgaW4gdGhlIHNlcnZl
ciBsaXN0IC0gaXQNCj4gKwkgKiBtZWFucyB0aGF0IHRoZSBzZXJ2ZXIgd2lsbCBpZ25vcmUgdGhl
IHJwYyBjcmVkcywgc28gYW55IGZsYXZvcg0KPiArCSAqIGNhbiBiZSB1c2VkLg0KPiArCSAqLw0K
PiArCWlmIChhcmdzLT5hdXRoX2ZsYXZvcnNbMF0gIT0gUlBDX0FVVEhfTUFYRkxBVk9SKSB7DQo+
ICsJCWZvciAoaSA9IDA7IGkgPCBjb3VudDsgaSsrKSB7DQo+ICsJCQlpZiAoYXJncy0+YXV0aF9m
bGF2b3JzWzBdID09IHJlcXVlc3QtPmF1dGhfZmxhdnNbaV0gfHwNCj4gKwkJCSAgICByZXF1ZXN0
LT5hdXRoX2ZsYXZzW2ldID09IFJQQ19BVVRIX05VTEwpDQo+ICsJCQkJZ290byBvdXQ7DQo+ICsJ
CX0NCj4gKwkJZGZwcmludGsoTU9VTlQsICJORlM6IGF1dGggZmxhdm9yICVkIG5vdCBzdXBwb3J0
ZWQgYnkgc2VydmVyXG4iLA0KPiArCQkJYXJncy0+YXV0aF9mbGF2b3JzWzBdKTsNCj4gKwkJZ290
byBvdXRfZXJyOw0KPiArCX0NCj4gKw0KPiArCS8qDQo+ICAJICogUkZDIDI2MjMsIHNlY3Rpb24g
Mi43IHN1Z2dlc3RzIHdlIFNIT1VMRCBwcmVmZXIgdGhlDQo+ICAJICogZmxhdm9yIGxpc3RlZCBm
aXJzdC4gIEhvd2V2ZXIsIHNvbWUgc2VydmVycyBsaXN0DQo+ICAJICogQVVUSF9OVUxMIGZpcnN0
LiAgQXZvaWQgZXZlciBjaG9vc2luZyBBVVRIX05VTEwuDQo+IEBAIC0xNjUzLDEyICsxNjcxLDI5
IEBAIHN0YXRpYyB2b2lkIG5mc19zZWxlY3RfZmxhdm9yKHN0cnVjdCBuZnNfcGFyc2VkX21vdW50
X2RhdGEgKmFyZ3MsDQo+ICAJCX0NCj4gIAl9DQo+ICANCj4gKwkvKg0KPiArCSAqIEFzIGEgbGFz
dCBjaGFuY2UsIHNlZSBpZiB0aGUgc2VydmVyIGxpc3QgY29udGFpbnMgQVVUSF9OVUxMIC0NCj4g
KwkgKiBpZiBpdCBkb2VzLCB1c2UgdGhlIGRlZmF1bHQgZmxhdm9yLg0KPiArCSAqLw0KPiArCWZv
ciAoaSA9IDA7IGkgPCBjb3VudDsgaSsrKSB7DQo+ICsJCWlmIChyZXF1ZXN0LT5hdXRoX2ZsYXZz
W2ldID09IFJQQ19BVVRIX05VTEwpDQo+ICsJCQlnb3RvIG91dF9kZWZhdWx0Ow0KPiArCX0NCj4g
Kw0KPiArCWRmcHJpbnRrKE1PVU5ULCAiTkZTOiBubyBhdXRoIGZsYXZvcnMgaW4gY29tbW9uIHdp
dGggc2VydmVyXG4iKTsNCj4gKwlnb3RvIG91dF9lcnI7DQo+ICsNCj4gIG91dF9kZWZhdWx0Og0K
PiAtCWZsYXZvciA9IFJQQ19BVVRIX1VOSVg7DQo+ICsJLyogdXNlIGRlZmF1bHQgaWYgZmxhdm9y
IG5vdCBhbHJlYWR5IHNldCAqLw0KPiArCWZsYXZvciA9IChhcmdzLT5hdXRoX2ZsYXZvcnNbMF0g
PT0gUlBDX0FVVEhfTUFYRkxBVk9SKSA/DQo+ICsJCVJQQ19BVVRIX1VOSVggOiBhcmdzLT5hdXRo
X2ZsYXZvcnNbMF07DQo+ICBvdXRfc2V0Og0KPiAgCWFyZ3MtPmF1dGhfZmxhdm9yc1swXSA9IGZs
YXZvcjsNCj4gIG91dDoNCj4gIAlkZnByaW50ayhNT1VOVCwgIk5GUzogdXNpbmcgYXV0aCBmbGF2
b3IgJWRcbiIsIGFyZ3MtPmF1dGhfZmxhdm9yc1swXSk7DQo+ICsJcmV0dXJuIDA7DQo+ICtvdXRf
ZXJyOg0KPiArCXJldHVybiAtRUFDQ0VTOw0KPiAgfQ0KPiAgDQo+ICAvKg0KPiBAQCAtMTcyMSw4
ICsxNzU2LDcgQEAgc3RhdGljIGludCBuZnNfcmVxdWVzdF9tb3VudChzdHJ1Y3QgbmZzX3BhcnNl
ZF9tb3VudF9kYXRhICphcmdzLA0KPiAgCQlyZXR1cm4gc3RhdHVzOw0KPiAgCX0NCj4gIA0KPiAt
CW5mc19zZWxlY3RfZmxhdm9yKGFyZ3MsICZyZXF1ZXN0KTsNCj4gLQlyZXR1cm4gMDsNCj4gKwly
ZXR1cm4gbmZzX3NlbGVjdF9mbGF2b3IoYXJncywgJnJlcXVlc3QpOw0KPiAgfQ0KPiAgDQo+ICBz
dHJ1Y3QgZGVudHJ5ICpuZnNfdHJ5X21vdW50KGludCBmbGFncywgY29uc3QgY2hhciAqZGV2X25h
bWUsDQoNClRoYW5rcyEgQXBwbGllZC4uLg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5G
UyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29t
DQp3d3cubmV0YXBwLmNvbQ0K

[PATCH] NFSv3: match sec= flavor against server list

[Weston Andros Adamson]

Older linux clients match the 'sec=' mount option flavor against the server's
flavor list (if available) and return EPERM if the specified flavor or AUTH_NULL
(which "matches" any flavor) is not found.

Recent changes skip this step and allow the vfs mount even though no operations
will succeed, creating a 'dud' mount.

This patch reverts back to the old behavior of matching specified flavors
against the server list and also returns EPERM when no sec= is specified and
none of the flavors returned by the server are supported by the client.

Example of behavior change:

the server's /etc/exports:

/export/krb5      *(sec=krb5,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
$

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

V3 - change EPERM to EACCES

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

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index eb494f6..f16a4b3 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,21 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
 		goto out_default;
 
 	/*
+	 * If the sec= mount option is used, the specified flavor or AUTH_NULL
+	 * must be in the list returned by the server.
+	 */
+	if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
+		for (i = 0; i < count; i++) {
+			if (args->auth_flavors[0] == request->auth_flavs[i] ||
+			    request->auth_flavs[i] == RPC_AUTH_NULL)
+				goto out;
+		}
+		dfprintk(MOUNT, "NFS: auth flavor %d not supported server\n",
+			args->auth_flavors[0]);
+		goto out_err;
+	}
+
+	/*
 	 * 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.
@@ -1646,6 +1659,7 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
 		case RPC_AUTH_UNIX:
 			goto out_set;
 		case RPC_AUTH_NULL:
+			auth_null_seen = true;
 			continue;
 		default:
 			if (rpcauth_get_gssinfo(flavor, &info) == 0)
@@ -1653,12 +1667,22 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args,
 		}
 	}
 
+	if (!auth_null_seen) {
+		dfprintk(MOUNT, "NFS: no auth flavors in common with server\n");
+		goto out_err;
+	}
+
 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_err:
+	return -EACCES;
 }
 
 /*
@@ -1721,8 +1745,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)

[PATCH] NFSv3: match sec= flavor against server list

[Weston Andros Adamson]

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(-)

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)

Re: [PATCH] NFSv3: match sec= flavor against server list

[Chuck Lever]

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

Re: [PATCH] NFSv3: match sec= flavor against server list

[Adamson, Dros]

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
> 
> 
> 

Re: [PATCH] NFSv3: match sec= flavor against server list

[Myklebust, Trond]

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

Re: [PATCH] NFSv3: match sec= flavor against server list

[Chuck Lever]

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

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

Re: [PATCH] NFSv3: match sec= flavor against server list

[Adamson, Dros]

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
> 
> 
> 
> 

Re: [PATCH] NFSv3: match sec= flavor against server list

[Chuck Lever]

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

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

Re: [PATCH] NFSv3: match sec= flavor against server list

[J. Bruce Fields]

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

Re: [PATCH] NFSv3: match sec= flavor against server list

[Adamson, Dros]

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