| Message ID | 1367846874-1234-1-git-send-email-dros@netapp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi- On May 6, 2013, at 9:27 AM, Weston Andros Adamson <dros@netapp.com> wrote: > Older clients match the 'sec=' mount option flavor against the server's > flavor list (if available) and return EPERM if the specified flavor is not > found. Recent changes would skip this step and allow the vfs mount even > though no operations will succeed. > > Signed-off-by: Weston Andros Adamson <dros@netapp.com> > --- > > Hey Chuck, > > This fixes a regression that was introduced with the recent nfs_select_flavor > changes - I'm pretty sure we want to match the specified flavor instead of just > using it. > > Example of the regression: > > the server's /etc/exports: > > /export/krb5 *(sec=krb5,sec=none,rw,no_root_squash) > > old client behavior: > > $ uname -a > Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux > $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt > mount.nfs: timeout set for Sun May 5 17:32:04 2013 > mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10' > mount.nfs: prog 100003, trying vers=3, prot=6 > mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049 > mount.nfs: prog 100005, trying vers=3, prot=17 > mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048 > mount.nfs: mount(2): Permission denied > mount.nfs: access denied by server while mounting zero:/export/krb5 This is wrong behavior, and is fixed by my patch. > recently changed behavior: > > $ uname -a > Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux > $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt > mount.nfs: timeout set for Sun May 5 17:37:17 2013 > mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10' > mount.nfs: prog 100003, trying vers=3, prot=6 > mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049 > mount.nfs: prog 100005, trying vers=3, prot=17 > mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048 > $ ls /mnt > ls: cannot open directory /mnt: Permission denied > $ sudo ls /mnt > ls: cannot open directory /mnt: Permission denied > $ sudo df /mnt > df: ‘/mnt’: Permission denied > df: no file systems processed > $ sudo umount /mnt > $ This is correct behavior. The server should map the user's AUTH_SYS credential to anonymous. If anonymous does not have permission on /export/krb5, then the server should prevent its access to the export. I assume this is a Linux NFS server. Does the Linux server support sec=none listed in the export options in the same way that NetApp and Oracle Solaris do? > I also made an attempt to support "AUTH_NULL means the server will ignore the > cred, so just use the specified flavor" behavior from much older kernels, but > I'm not sure if we want this logic. > > I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some > older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified, > server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are > ignored by the server), while newer kernels do not. The workaround in newer > kernels is to specify sec=none. > > Thoughts? > > -dros > > fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++------- > 1 file changed, 32 insertions(+), 7 deletions(-) > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c > index eb494f6..6476b5d 100644 > --- a/fs/nfs/super.c > +++ b/fs/nfs/super.c > @@ -1611,14 +1611,12 @@ out_security_failure: > * Select a security flavor for this mount. The selected flavor > * is planted in args->auth_flavors[0]. > */ > -static void nfs_select_flavor(struct nfs_parsed_mount_data *args, > +static int nfs_select_flavor(struct nfs_parsed_mount_data *args, > struct nfs_mount_request *request) > { > unsigned int i, count = *(request->auth_flav_len); > rpc_authflavor_t flavor; > - > - if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) > - goto out; > + bool auth_null_seen = false; > > /* > * The NFSv2 MNT operation does not return a flavor list. > @@ -1634,6 +1632,26 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args, > goto out_default; > > /* > + * If the sec= mount option is used, the flavor must be in the list > + * returned by the server. > + * > + * One exception is when the server's flavor list includes AUTH_NULL: > + * Some servers implement AUTH_NULL by completely ignoring the rpc > + * cred, so the client can use any flavor. > + */ > + if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) { > + for (i = 0; i < count; i++) { > + if (args->auth_flavors[0] == request->auth_flavs[i]) > + goto out; > + if (request->auth_flavs[i] == RPC_AUTH_NULL) > + auth_null_seen = true; > + } > + if (auth_null_seen) > + goto out; > + goto out_nomatch; > + } > + > + /* > * RFC 2623, section 2.7 suggests we SHOULD prefer the > * flavor listed first. However, some servers list > * AUTH_NULL first. Avoid ever choosing AUTH_NULL. > @@ -1654,11 +1672,19 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args, > } > > out_default: > - flavor = RPC_AUTH_UNIX; > + /* use default if flavor not already set */ > + flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ? > + RPC_AUTH_UNIX : args->auth_flavors[0]; > out_set: > args->auth_flavors[0] = flavor; > out: > dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]); > + return 0; > + > +out_nomatch: > + dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n", > + args->auth_flavors[0]); > + return -EPERM; > } > > /* > @@ -1721,8 +1747,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args, > return status; > } > > - nfs_select_flavor(args, &request); > - return 0; > + return nfs_select_flavor(args, &request); > } > > struct dentry *nfs_try_mount(int flags, const char *dev_name, > -- > 1.7.12.4 (Apple Git-37) > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Chuck Lever chuck[dot]lever[at]oracle[dot]com -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On May 6, 2013, at 9:52 AM, Chuck Lever <chuck.lever@oracle.com> wrote: > Hi- > > On May 6, 2013, at 9:27 AM, Weston Andros Adamson <dros@netapp.com> wrote: > >> Older clients match the 'sec=' mount option flavor against the server's >> flavor list (if available) and return EPERM if the specified flavor is not >> found. Recent changes would skip this step and allow the vfs mount even >> though no operations will succeed. >> >> Signed-off-by: Weston Andros Adamson <dros@netapp.com> >> --- >> >> Hey Chuck, >> >> This fixes a regression that was introduced with the recent nfs_select_flavor >> changes - I'm pretty sure we want to match the specified flavor instead of just >> using it. >> >> Example of the regression: >> >> the server's /etc/exports: >> >> /export/krb5 *(sec=krb5,sec=none,rw,no_root_squash) >> >> old client behavior: >> >> $ uname -a >> Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux >> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt >> mount.nfs: timeout set for Sun May 5 17:32:04 2013 >> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10' >> mount.nfs: prog 100003, trying vers=3, prot=6 >> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049 >> mount.nfs: prog 100005, trying vers=3, prot=17 >> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048 >> mount.nfs: mount(2): Permission denied >> mount.nfs: access denied by server while mounting zero:/export/krb5 > > This is wrong behavior, and is fixed by my patch. > >> recently changed behavior: >> >> $ uname -a >> Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux >> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt >> mount.nfs: timeout set for Sun May 5 17:37:17 2013 >> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10' >> mount.nfs: prog 100003, trying vers=3, prot=6 >> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049 >> mount.nfs: prog 100005, trying vers=3, prot=17 >> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048 >> $ ls /mnt >> ls: cannot open directory /mnt: Permission denied >> $ sudo ls /mnt >> ls: cannot open directory /mnt: Permission denied >> $ sudo df /mnt >> df: ‘/mnt’: Permission denied >> df: no file systems processed >> $ sudo umount /mnt >> $ > > This is correct behavior. The server should map the user's AUTH_SYS credential to anonymous. If anonymous does not have permission on /export/krb5, then the server should prevent its access to the export. You're talking about AUTH_NULL behavior here, right? > > I assume this is a Linux NFS server. Does the Linux server support sec=none listed in the export options in the same way that NetApp and Oracle Solaris do? So there are two issues here and I think they're getting confused. The example is of the first issue: mounting a krb5 only export with sec=sys - client mounts sec=sys - the server list is [krb5] - the client completely ignores this list and just uses sys. - no operations work, it's a 'dud mount' The second issue is the AUTH_NULL stuff - lets just ignore that for now. Are you saying that the client should just use whatever sec= is specified and never try to match against the server list? A little background - I ran into this implementing a different patch that makes sure v4.x mounts are actually using the specified flavor (if one exists) to mount the export -- that is, it can follow SECINFOs to do initial lookup, but must be using the specified flavor on the export path passed to mount. After the initial mount lookup, SECINFOs will be used normally. Trond thought that this was a bug that should be fixed - that it's wrong when client reports that it's using one flavor (as seen in mount output, etc) when it's really using another. This (other) patch works, but when no version is specified, it always falls back to v3 and the mount will always succeed - even if the auth flavor isn't supported by the export. -dros > >> I also made an attempt to support "AUTH_NULL means the server will ignore the >> cred, so just use the specified flavor" behavior from much older kernels, but >> I'm not sure if we want this logic. >> >> I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some >> older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified, >> server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are >> ignored by the server), while newer kernels do not. The workaround in newer >> kernels is to specify sec=none. >> >> Thoughts? >> >> -dros >> >> fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++------- >> 1 file changed, 32 insertions(+), 7 deletions(-) >> >> diff --git a/fs/nfs/super.c b/fs/nfs/super.c >> index eb494f6..6476b5d 100644 >> --- a/fs/nfs/super.c >> +++ b/fs/nfs/super.c >> @@ -1611,14 +1611,12 @@ out_security_failure: >> * Select a security flavor for this mount. The selected flavor >> * is planted in args->auth_flavors[0]. >> */ >> -static void nfs_select_flavor(struct nfs_parsed_mount_data *args, >> +static int nfs_select_flavor(struct nfs_parsed_mount_data *args, >> struct nfs_mount_request *request) >> { >> unsigned int i, count = *(request->auth_flav_len); >> rpc_authflavor_t flavor; >> - >> - if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) >> - goto out; >> + bool auth_null_seen = false; >> >> /* >> * The NFSv2 MNT operation does not return a flavor list. >> @@ -1634,6 +1632,26 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args, >> goto out_default; >> >> /* >> + * If the sec= mount option is used, the flavor must be in the list >> + * returned by the server. >> + * >> + * One exception is when the server's flavor list includes AUTH_NULL: >> + * Some servers implement AUTH_NULL by completely ignoring the rpc >> + * cred, so the client can use any flavor. >> + */ >> + if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) { >> + for (i = 0; i < count; i++) { >> + if (args->auth_flavors[0] == request->auth_flavs[i]) >> + goto out; >> + if (request->auth_flavs[i] == RPC_AUTH_NULL) >> + auth_null_seen = true; >> + } >> + if (auth_null_seen) >> + goto out; >> + goto out_nomatch; >> + } >> + >> + /* >> * RFC 2623, section 2.7 suggests we SHOULD prefer the >> * flavor listed first. However, some servers list >> * AUTH_NULL first. Avoid ever choosing AUTH_NULL. >> @@ -1654,11 +1672,19 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args, >> } >> >> out_default: >> - flavor = RPC_AUTH_UNIX; >> + /* use default if flavor not already set */ >> + flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ? >> + RPC_AUTH_UNIX : args->auth_flavors[0]; >> out_set: >> args->auth_flavors[0] = flavor; >> out: >> dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]); >> + return 0; >> + >> +out_nomatch: >> + dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n", >> + args->auth_flavors[0]); >> + return -EPERM; >> } >> >> /* >> @@ -1721,8 +1747,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args, >> return status; >> } >> >> - nfs_select_flavor(args, &request); >> - return 0; >> + return nfs_select_flavor(args, &request); >> } >> >> struct dentry *nfs_try_mount(int flags, const char *dev_name, >> -- >> 1.7.12.4 (Apple Git-37) >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > Chuck Lever > chuck[dot]lever[at]oracle[dot]com > > > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
T24gTW9uLCAyMDEzLTA1LTA2IGF0IDE0OjQ5ICswMDAwLCBBZGFtc29uLCBEcm9zIHdyb3RlOg0K PiBPbiBNYXkgNiwgMjAxMywgYXQgOTo1MiBBTSwgQ2h1Y2sgTGV2ZXIgPGNodWNrLmxldmVyQG9y YWNsZS5jb20+IHdyb3RlOg0KPiANCj4gPiBIaS0NCj4gPiANCj4gPiBPbiBNYXkgNiwgMjAxMywg YXQgOToyNyBBTSwgV2VzdG9uIEFuZHJvcyBBZGFtc29uIDxkcm9zQG5ldGFwcC5jb20+IHdyb3Rl Og0KPiA+IA0KPiA+PiBPbGRlciBjbGllbnRzIG1hdGNoIHRoZSAnc2VjPScgbW91bnQgb3B0aW9u IGZsYXZvciBhZ2FpbnN0IHRoZSBzZXJ2ZXIncw0KPiA+PiBmbGF2b3IgbGlzdCAoaWYgYXZhaWxh YmxlKSBhbmQgcmV0dXJuIEVQRVJNIGlmIHRoZSBzcGVjaWZpZWQgZmxhdm9yIGlzIG5vdA0KPiA+ PiBmb3VuZC4gUmVjZW50IGNoYW5nZXMgd291bGQgc2tpcCB0aGlzIHN0ZXAgYW5kIGFsbG93IHRo ZSB2ZnMgbW91bnQgZXZlbg0KPiA+PiB0aG91Z2ggbm8gb3BlcmF0aW9ucyB3aWxsIHN1Y2NlZWQu DQo+ID4+IA0KPiA+PiBTaWduZWQtb2ZmLWJ5OiBXZXN0b24gQW5kcm9zIEFkYW1zb24gPGRyb3NA bmV0YXBwLmNvbT4NCj4gPj4gLS0tDQo+ID4+IA0KPiA+PiBIZXkgQ2h1Y2ssDQo+ID4+IA0KPiA+ PiBUaGlzIGZpeGVzIGEgcmVncmVzc2lvbiB0aGF0IHdhcyBpbnRyb2R1Y2VkIHdpdGggdGhlIHJl Y2VudCBuZnNfc2VsZWN0X2ZsYXZvciANCj4gPj4gY2hhbmdlcyAtIEknbSBwcmV0dHkgc3VyZSB3 ZSB3YW50IHRvIG1hdGNoIHRoZSBzcGVjaWZpZWQgZmxhdm9yIGluc3RlYWQgb2YganVzdA0KPiA+ PiB1c2luZyBpdC4NCj4gPj4gDQo+ID4+IEV4YW1wbGUgb2YgdGhlIHJlZ3Jlc3Npb246DQo+ID4+ IA0KPiA+PiB0aGUgc2VydmVyJ3MgL2V0Yy9leHBvcnRzOg0KPiA+PiANCj4gPj4gL2V4cG9ydC9r cmI1ICAgICAgKihzZWM9a3JiNSxzZWM9bm9uZSxydyxub19yb290X3NxdWFzaCkNCj4gPj4gDQo+ ID4+IG9sZCBjbGllbnQgYmVoYXZpb3I6DQo+ID4+IA0KPiA+PiAkIHVuYW1lIC1hDQo+ID4+IExp bnV4IG9uZS5hcGlraWEuZmFrZSAzLjguOC0yMDIuZmMxOC54ODZfNjQgIzEgU01QIFdlZCBBcHIg MTcgMjM6MjU6MTcgVVRDIDIwMTMgeDg2XzY0IHg4Nl82NCB4ODZfNjQgR05VL0xpbnV4DQo+ID4+ ICQgc3VkbyBtb3VudCAtdiAtbyBzZWM9c3lzLHZlcnM9MyB6ZXJvOi9leHBvcnQva3JiNSAvbW50 DQo+ID4+IG1vdW50Lm5mczogdGltZW91dCBzZXQgZm9yIFN1biBNYXkgIDUgMTc6MzI6MDQgMjAx Mw0KPiA+PiBtb3VudC5uZnM6IHRyeWluZyB0ZXh0LWJhc2VkIG9wdGlvbnMgJ3NlYz1zeXMsdmVy cz0zLGFkZHI9MTkyLjE2OC4xMDAuMTAnDQo+ID4+IG1vdW50Lm5mczogcHJvZyAxMDAwMDMsIHRy eWluZyB2ZXJzPTMsIHByb3Q9Ng0KPiA+PiBtb3VudC5uZnM6IHRyeWluZyAxOTIuMTY4LjEwMC4x MCBwcm9nIDEwMDAwMyB2ZXJzIDMgcHJvdCBUQ1AgcG9ydCAyMDQ5DQo+ID4+IG1vdW50Lm5mczog cHJvZyAxMDAwMDUsIHRyeWluZyB2ZXJzPTMsIHByb3Q9MTcNCj4gPj4gbW91bnQubmZzOiB0cnlp bmcgMTkyLjE2OC4xMDAuMTAgcHJvZyAxMDAwMDUgdmVycyAzIHByb3QgVURQIHBvcnQgMjAwNDgN Cj4gPj4gbW91bnQubmZzOiBtb3VudCgyKTogUGVybWlzc2lvbiBkZW5pZWQNCj4gPj4gbW91bnQu bmZzOiBhY2Nlc3MgZGVuaWVkIGJ5IHNlcnZlciB3aGlsZSBtb3VudGluZyB6ZXJvOi9leHBvcnQv a3JiNQ0KPiA+IA0KPiA+IFRoaXMgaXMgd3JvbmcgYmVoYXZpb3IsIGFuZCBpcyBmaXhlZCBieSBt eSBwYXRjaC4NCj4gPiANCj4gPj4gcmVjZW50bHkgY2hhbmdlZCBiZWhhdmlvcjoNCj4gPj4gDQo+ ID4+ICQgdW5hbWUgLWENCj4gPj4gTGludXggb25lLmFwaWtpYS5mYWtlIDMuOS4wLXRlc3Rpbmcr ICMyIFNNUCBGcmkgTWF5IDMgMjA6Mjk6MzIgRURUIDIwMTMgeDg2XzY0IHg4Nl82NCB4ODZfNjQg R05VL0xpbnV4DQo+ID4+ICQgc3VkbyBtb3VudCAtdiAtbyBzZWM9c3lzLHZlcnM9MyB6ZXJvOi9l eHBvcnQva3JiNSAvbW50DQo+ID4+IG1vdW50Lm5mczogdGltZW91dCBzZXQgZm9yIFN1biBNYXkg IDUgMTc6Mzc6MTcgMjAxMw0KPiA+PiBtb3VudC5uZnM6IHRyeWluZyB0ZXh0LWJhc2VkIG9wdGlv bnMgJ3NlYz1zeXMsdmVycz0zLGFkZHI9MTkyLjE2OC4xMDAuMTAnDQo+ID4+IG1vdW50Lm5mczog cHJvZyAxMDAwMDMsIHRyeWluZyB2ZXJzPTMsIHByb3Q9Ng0KPiA+PiBtb3VudC5uZnM6IHRyeWlu ZyAxOTIuMTY4LjEwMC4xMCBwcm9nIDEwMDAwMyB2ZXJzIDMgcHJvdCBUQ1AgcG9ydCAyMDQ5DQo+ ID4+IG1vdW50Lm5mczogcHJvZyAxMDAwMDUsIHRyeWluZyB2ZXJzPTMsIHByb3Q9MTcNCj4gPj4g bW91bnQubmZzOiB0cnlpbmcgMTkyLjE2OC4xMDAuMTAgcHJvZyAxMDAwMDUgdmVycyAzIHByb3Qg VURQIHBvcnQgMjAwNDgNCj4gPj4gJCBscyAvbW50DQo+ID4+IGxzOiBjYW5ub3Qgb3BlbiBkaXJl Y3RvcnkgL21udDogUGVybWlzc2lvbiBkZW5pZWQNCj4gPj4gJCBzdWRvIGxzIC9tbnQNCj4gPj4g bHM6IGNhbm5vdCBvcGVuIGRpcmVjdG9yeSAvbW50OiBQZXJtaXNzaW9uIGRlbmllZA0KPiA+PiAk IHN1ZG8gZGYgL21udA0KPiA+PiBkZjog4oCYL21udOKAmTogUGVybWlzc2lvbiBkZW5pZWQNCj4g Pj4gZGY6IG5vIGZpbGUgc3lzdGVtcyBwcm9jZXNzZWQNCj4gPj4gJCBzdWRvIHVtb3VudCAvbW50 DQo+ID4+ICQNCj4gPiANCj4gPiBUaGlzIGlzIGNvcnJlY3QgYmVoYXZpb3IuICBUaGUgc2VydmVy IHNob3VsZCBtYXAgdGhlIHVzZXIncyBBVVRIX1NZUyBjcmVkZW50aWFsIHRvIGFub255bW91cy4g IElmIGFub255bW91cyBkb2VzIG5vdCBoYXZlIHBlcm1pc3Npb24gb24gL2V4cG9ydC9rcmI1LCB0 aGVuIHRoZSBzZXJ2ZXIgc2hvdWxkIHByZXZlbnQgaXRzIGFjY2VzcyB0byB0aGUgZXhwb3J0Lg0K PiANCj4gWW91J3JlIHRhbGtpbmcgYWJvdXQgQVVUSF9OVUxMIGJlaGF2aW9yIGhlcmUsIHJpZ2h0 Pw0KPiANCj4gPiANCj4gPiBJIGFzc3VtZSB0aGlzIGlzIGEgTGludXggTkZTIHNlcnZlci4gIERv ZXMgdGhlIExpbnV4IHNlcnZlciBzdXBwb3J0IHNlYz1ub25lIGxpc3RlZCBpbiB0aGUgZXhwb3J0 IG9wdGlvbnMgaW4gdGhlIHNhbWUgd2F5IHRoYXQgTmV0QXBwIGFuZCBPcmFjbGUgU29sYXJpcyBk bz8NCj4gDQo+IA0KPiBTbyB0aGVyZSBhcmUgdHdvIGlzc3VlcyBoZXJlIGFuZCBJIHRoaW5rIHRo ZXkncmUgZ2V0dGluZyBjb25mdXNlZC4NCj4gDQo+IFRoZSBleGFtcGxlIGlzIG9mIHRoZSBmaXJz dCBpc3N1ZTogbW91bnRpbmcgYSBrcmI1IG9ubHkgZXhwb3J0IHdpdGggc2VjPXN5cw0KPiAgLSBj bGllbnQgbW91bnRzIHNlYz1zeXMNCj4gIC0gdGhlIHNlcnZlciBsaXN0IGlzIFtrcmI1XQ0KPiAg LSB0aGUgY2xpZW50IGNvbXBsZXRlbHkgaWdub3JlcyB0aGlzIGxpc3QgYW5kIGp1c3QgdXNlcyBz eXMuDQo+ICAtIG5vIG9wZXJhdGlvbnMgd29yaywgaXQncyBhICdkdWQgbW91bnQnDQo+IA0KPiBU aGUgc2Vjb25kIGlzc3VlIGlzIHRoZSBBVVRIX05VTEwgc3R1ZmYgLSBsZXRzIGp1c3QgaWdub3Jl IHRoYXQgZm9yIG5vdy4NCj4gDQo+IEFyZSB5b3Ugc2F5aW5nIHRoYXQgdGhlIGNsaWVudCBzaG91 bGQganVzdCB1c2Ugd2hhdGV2ZXIgc2VjPSBpcyBzcGVjaWZpZWQgYW5kIG5ldmVyIHRyeSB0byBt YXRjaCBhZ2FpbnN0IHRoZSBzZXJ2ZXIgbGlzdD8NCj4gDQo+IEEgbGl0dGxlIGJhY2tncm91bmQg LSBJIHJhbiBpbnRvIHRoaXMgaW1wbGVtZW50aW5nIGEgZGlmZmVyZW50IHBhdGNoIHRoYXQgbWFr ZXMgc3VyZSB2NC54IG1vdW50cyBhcmUgYWN0dWFsbHkgdXNpbmcgdGhlIHNwZWNpZmllZCBmbGF2 b3IgKGlmIG9uZSBleGlzdHMpIHRvIG1vdW50IHRoZSBleHBvcnQgLS0gdGhhdCBpcywgaXQgY2Fu IGZvbGxvdyBTRUNJTkZPcyB0byBkbyBpbml0aWFsIGxvb2t1cCwgYnV0IG11c3QgYmUgdXNpbmcg dGhlIHNwZWNpZmllZCBmbGF2b3Igb24gdGhlIGV4cG9ydCBwYXRoIHBhc3NlZCB0byBtb3VudC4g IEFmdGVyIHRoZSBpbml0aWFsIG1vdW50IGxvb2t1cCwgU0VDSU5GT3Mgd2lsbCBiZSB1c2VkIG5v cm1hbGx5LiBUcm9uZCB0aG91Z2h0IHRoYXQgdGhpcyB3YXMgYSBidWcgdGhhdCBzaG91bGQgYmUg Zml4ZWQgLSB0aGF0IGl0J3Mgd3Jvbmcgd2hlbiBjbGllbnQgcmVwb3J0cyB0aGF0IGl0J3MgdXNp bmcgb25lIGZsYXZvciAoYXMgc2VlbiBpbiBtb3VudCBvdXRwdXQsIGV0Yykgd2hlbiBpdCdzIHJl YWxseSB1c2luZyBhbm90aGVyLiBUaGlzIChvdGhlcikgcGF0Y2ggd29ya3MsIGJ1dCB3aGVuIG5v IHZlcnNpb24gaXMgc3BlY2lmaWVkLCBpdCBhbHdheXMgZmFsbHMgYmFjayB0byB2MyBhbmQgdGhl IG1vdW50IHdpbGwgYWx3YXlzIHN1Y2NlZWQgLSBldmVuIGlmIHRoZSBhdXRoIGZsYXZvciBpc24n dCBzdXBwb3J0ZWQgYnkgdGhlIGV4cG9ydC4NCg0KSW4gdGhlIE5GU3Y0IGNhc2UsIHdlIHNob3Vs ZCBkZWZpbml0ZWx5IGJlIHJldHVybmluZyBFQUNDRVMgaWYgdGhlIHVzZXINCnRyaWVzIHRvIHNw ZWNpZnkgYSBzZWN1cml0eSBmbGF2b3VyIHRoYXQgZG9lc24ndCBhbGxvdyB0aGUgbW91bnQgdG8N CnN1Y2NlZWQsIGluc3RlYWQgb2YgaGF2aW5nIGF1dG8tbmVnb3RpYXRpb24gb3ZlcnJpZGUgaXQu DQoNCldpdGggdGhhdCBpbiBtaW5kLCBJIGFncmVlIHRoYXQgd2Ugc2hvdWxkIGRvIHRoZSBzYW1l IHdpdGggTkZTdjMgaW4NCm9yZGVyIHRvIGF2b2lkIHRoZSBraW5kIG9mIG1vdW50IGZhaWxvdmVy IHNpdHVhdGlvbiB0aGF0IERyb3MgZGVzY3JpYmVzDQphYm92ZS4gSGF2aW5nIHRoZSBtb3VudCAn c3VjY2VlZCcgeWV0IG5vdCBhY3R1YWxseSBiZSB1c2FibGUgYnkgYW55b25lDQppcyBqdXN0IHdy b25nIChldmVuIGlmIHRoYXQgaXMgd2hhdCB3ZSB1c2VkIHRvIGRvIGJlZm9yZSBzZWN1cml0eQ0K bmVnb3RpYXRpb24gd2FzIGFkZGVkKS4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5G UyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29t DQp3d3cubmV0YXBwLmNvbQ0K -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
On Mon, May 06, 2013 at 09:27:54AM -0400, Weston Andros Adamson wrote: > Older clients match the 'sec=' mount option flavor against the server's > flavor list (if available) and return EPERM if the specified flavor is not > found. Recent changes would skip this step and allow the vfs mount even > though no operations will succeed. > > Signed-off-by: Weston Andros Adamson <dros@netapp.com> > --- > > Hey Chuck, > > This fixes a regression that was introduced with the recent nfs_select_flavor > changes - I'm pretty sure we want to match the specified flavor instead of just > using it. > > Example of the regression: > > the server's /etc/exports: > > /export/krb5 *(sec=krb5,sec=none,rw,no_root_squash) > > old client behavior: > > $ uname -a > Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux > $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt > mount.nfs: timeout set for Sun May 5 17:32:04 2013 > mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10' > mount.nfs: prog 100003, trying vers=3, prot=6 > mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049 > mount.nfs: prog 100005, trying vers=3, prot=17 > mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048 > mount.nfs: mount(2): Permission denied > mount.nfs: access denied by server while mounting zero:/export/krb5 > > recently changed behavior: > > $ uname -a > Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux > $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt > mount.nfs: timeout set for Sun May 5 17:37:17 2013 > mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10' > mount.nfs: prog 100003, trying vers=3, prot=6 > mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049 > mount.nfs: prog 100005, trying vers=3, prot=17 > mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048 > $ ls /mnt > ls: cannot open directory /mnt: Permission denied > $ sudo ls /mnt > ls: cannot open directory /mnt: Permission denied > $ sudo df /mnt > df: ‘/mnt’: Permission denied > df: no file systems processed > $ sudo umount /mnt > $ > > I also made an attempt to support "AUTH_NULL means the server will ignore the > cred, so just use the specified flavor" behavior from much older kernels, but > I'm not sure if we want this logic. > > I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some > older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified, > server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are > ignored by the server), while newer kernels do not. The workaround in newer > kernels is to specify sec=none. Nit: the above examples would actually be really useful, for example, to a distribution maintainer trying to figure out whether this patch would solve a given user problem. Could you include them in the changelog? --b. > > Thoughts? > > -dros > > fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++------- > 1 file changed, 32 insertions(+), 7 deletions(-) > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c > index eb494f6..6476b5d 100644 > --- a/fs/nfs/super.c > +++ b/fs/nfs/super.c > @@ -1611,14 +1611,12 @@ out_security_failure: > * Select a security flavor for this mount. The selected flavor > * is planted in args->auth_flavors[0]. > */ > -static void nfs_select_flavor(struct nfs_parsed_mount_data *args, > +static int nfs_select_flavor(struct nfs_parsed_mount_data *args, > struct nfs_mount_request *request) > { > unsigned int i, count = *(request->auth_flav_len); > rpc_authflavor_t flavor; > - > - if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) > - goto out; > + bool auth_null_seen = false; > > /* > * The NFSv2 MNT operation does not return a flavor list. > @@ -1634,6 +1632,26 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args, > goto out_default; > > /* > + * If the sec= mount option is used, the flavor must be in the list > + * returned by the server. > + * > + * One exception is when the server's flavor list includes AUTH_NULL: > + * Some servers implement AUTH_NULL by completely ignoring the rpc > + * cred, so the client can use any flavor. > + */ > + if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) { > + for (i = 0; i < count; i++) { > + if (args->auth_flavors[0] == request->auth_flavs[i]) > + goto out; > + if (request->auth_flavs[i] == RPC_AUTH_NULL) > + auth_null_seen = true; > + } > + if (auth_null_seen) > + goto out; > + goto out_nomatch; > + } > + > + /* > * RFC 2623, section 2.7 suggests we SHOULD prefer the > * flavor listed first. However, some servers list > * AUTH_NULL first. Avoid ever choosing AUTH_NULL. > @@ -1654,11 +1672,19 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args, > } > > out_default: > - flavor = RPC_AUTH_UNIX; > + /* use default if flavor not already set */ > + flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ? > + RPC_AUTH_UNIX : args->auth_flavors[0]; > out_set: > args->auth_flavors[0] = flavor; > out: > dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]); > + return 0; > + > +out_nomatch: > + dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n", > + args->auth_flavors[0]); > + return -EPERM; > } > > /* > @@ -1721,8 +1747,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args, > return status; > } > > - nfs_select_flavor(args, &request); > - return 0; > + return nfs_select_flavor(args, &request); > } > > struct dentry *nfs_try_mount(int flags, const char *dev_name, > -- > 1.7.12.4 (Apple Git-37) > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On May 6, 2013, at 11:17 AM, Chuck Lever <chuck.lever@oracle.com> wrote: > > On May 6, 2013, at 10:49 AM, "Adamson, Dros" <Weston.Adamson@netapp.com> wrote: > >> >> On May 6, 2013, at 9:52 AM, Chuck Lever <chuck.lever@oracle.com> wrote: >> >>> Hi- >>> >>> On May 6, 2013, at 9:27 AM, Weston Andros Adamson <dros@netapp.com> wrote: >>> >>>> Older clients match the 'sec=' mount option flavor against the server's >>>> flavor list (if available) and return EPERM if the specified flavor is not >>>> found. Recent changes would skip this step and allow the vfs mount even >>>> though no operations will succeed. >>>> >>>> Signed-off-by: Weston Andros Adamson <dros@netapp.com> >>>> --- >>>> >>>> Hey Chuck, >>>> >>>> This fixes a regression that was introduced with the recent nfs_select_flavor >>>> changes - I'm pretty sure we want to match the specified flavor instead of just >>>> using it. >>>> >>>> Example of the regression: >>>> >>>> the server's /etc/exports: >>>> >>>> /export/krb5 *(sec=krb5,sec=none,rw,no_root_squash) >>>> >>>> old client behavior: >>>> >>>> $ uname -a >>>> Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux >>>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt >>>> mount.nfs: timeout set for Sun May 5 17:32:04 2013 >>>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10' >>>> mount.nfs: prog 100003, trying vers=3, prot=6 >>>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049 >>>> mount.nfs: prog 100005, trying vers=3, prot=17 >>>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048 >>>> mount.nfs: mount(2): Permission denied >>>> mount.nfs: access denied by server while mounting zero:/export/krb5 >>> >>> This is wrong behavior, and is fixed by my patch. >>> >>>> recently changed behavior: >>>> >>>> $ uname -a >>>> Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux >>>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt >>>> mount.nfs: timeout set for Sun May 5 17:37:17 2013 >>>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10' >>>> mount.nfs: prog 100003, trying vers=3, prot=6 >>>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049 >>>> mount.nfs: prog 100005, trying vers=3, prot=17 >>>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048 >>>> $ ls /mnt >>>> ls: cannot open directory /mnt: Permission denied >>>> $ sudo ls /mnt >>>> ls: cannot open directory /mnt: Permission denied >>>> $ sudo df /mnt >>>> df: ‘/mnt’: Permission denied >>>> df: no file systems processed >>>> $ sudo umount /mnt >>>> $ >>> >>> This is correct behavior. The server should map the user's AUTH_SYS credential to anonymous. If anonymous does not have permission on /export/krb5, then the server should prevent its access to the export. >> >> You're talking about AUTH_NULL behavior here, right? >> >>> >>> I assume this is a Linux NFS server. Does the Linux server support sec=none listed in the export options in the same way that NetApp and Oracle Solaris do? >> >> >> So there are two issues here and I think they're getting confused. >> >> The example is of the first issue: mounting a krb5 only export with sec=sys >> - client mounts sec=sys >> - the server list is [krb5] >> - the client completely ignores this list and just uses sys. >> - no operations work, it's a 'dud mount' >> >> The second issue is the AUTH_NULL stuff - lets just ignore that for now. >> >> Are you saying that the client should just use whatever sec= is specified and never try to match against the server list? > > That's what my patch does. The reason for this is that the server can interpret the mount's security flavor any way it likes. > > Perhaps we should take that fallback position only if the server lists AUTH_NULL in the flavor list. If AUTH_NULL is not listed, ensure the flavor requested on the mount command line is available on the server (the most recent previous behavior); fail if no matching flavor is found. > > If the mount command line does not specify a flavor, we could look in the server's list for a flavor we support, then fail if none is found. If I'm reading this right, that's what this patch does. - if no sec= is specified, we look through the servers list for a supported flavor (same behavior as without this patch). - if sec= is specified, check the server list - if flavor is found, use it - else if AUTH_NULL is in the list, just use the user specified sec= flavor - else return -EPERM Also: if we don't get a server list for whatever reason, just use the specified sec= flavor. Does that sound right? If so, does the patch look good? -dros > >> A little background - I ran into this implementing a different patch that makes sure v4.x mounts are actually using the specified flavor (if one exists) to mount the export -- that is, it can follow SECINFOs to do initial lookup, but must be using the specified flavor on the export path passed to mount. After the initial mount lookup, SECINFOs will be used normally. Trond thought that this was a bug that should be fixed - that it's wrong when client reports that it's using one flavor (as seen in mount output, etc) when it's really using another. This (other) patch works, but when no version is specified, it always falls back to v3 and the mount will always succeed - even if the auth flavor isn't supported by the export. >> >> -dros >> >> >>> >>>> I also made an attempt to support "AUTH_NULL means the server will ignore the >>>> cred, so just use the specified flavor" behavior from much older kernels, but >>>> I'm not sure if we want this logic. >>>> >>>> I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some >>>> older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified, >>>> server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are >>>> ignored by the server), while newer kernels do not. The workaround in newer >>>> kernels is to specify sec=none. >>>> >>>> Thoughts? >>>> >>>> -dros >>>> >>>> fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++------- >>>> 1 file changed, 32 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c >>>> index eb494f6..6476b5d 100644 >>>> --- a/fs/nfs/super.c >>>> +++ b/fs/nfs/super.c >>>> @@ -1611,14 +1611,12 @@ out_security_failure: >>>> * Select a security flavor for this mount. The selected flavor >>>> * is planted in args->auth_flavors[0]. >>>> */ >>>> -static void nfs_select_flavor(struct nfs_parsed_mount_data *args, >>>> +static int nfs_select_flavor(struct nfs_parsed_mount_data *args, >>>> struct nfs_mount_request *request) >>>> { >>>> unsigned int i, count = *(request->auth_flav_len); >>>> rpc_authflavor_t flavor; >>>> - >>>> - if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) >>>> - goto out; >>>> + bool auth_null_seen = false; >>>> >>>> /* >>>> * The NFSv2 MNT operation does not return a flavor list. >>>> @@ -1634,6 +1632,26 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args, >>>> goto out_default; >>>> >>>> /* >>>> + * If the sec= mount option is used, the flavor must be in the list >>>> + * returned by the server. >>>> + * >>>> + * One exception is when the server's flavor list includes AUTH_NULL: >>>> + * Some servers implement AUTH_NULL by completely ignoring the rpc >>>> + * cred, so the client can use any flavor. >>>> + */ >>>> + if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) { >>>> + for (i = 0; i < count; i++) { >>>> + if (args->auth_flavors[0] == request->auth_flavs[i]) >>>> + goto out; >>>> + if (request->auth_flavs[i] == RPC_AUTH_NULL) >>>> + auth_null_seen = true; >>>> + } >>>> + if (auth_null_seen) >>>> + goto out; >>>> + goto out_nomatch; >>>> + } >>>> + >>>> + /* >>>> * RFC 2623, section 2.7 suggests we SHOULD prefer the >>>> * flavor listed first. However, some servers list >>>> * AUTH_NULL first. Avoid ever choosing AUTH_NULL. >>>> @@ -1654,11 +1672,19 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args, >>>> } >>>> >>>> out_default: >>>> - flavor = RPC_AUTH_UNIX; >>>> + /* use default if flavor not already set */ >>>> + flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ? >>>> + RPC_AUTH_UNIX : args->auth_flavors[0]; >>>> out_set: >>>> args->auth_flavors[0] = flavor; >>>> out: >>>> dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]); >>>> + return 0; >>>> + >>>> +out_nomatch: >>>> + dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n", >>>> + args->auth_flavors[0]); >>>> + return -EPERM; >>>> } >>>> >>>> /* >>>> @@ -1721,8 +1747,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args, >>>> return status; >>>> } >>>> >>>> - nfs_select_flavor(args, &request); >>>> - return 0; >>>> + return nfs_select_flavor(args, &request); >>>> } >>>> >>>> struct dentry *nfs_try_mount(int flags, const char *dev_name, >>>> -- >>>> 1.7.12.4 (Apple Git-37) >>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >>> -- >>> Chuck Lever >>> chuck[dot]lever[at]oracle[dot]com >>> >>> >>> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > Chuck Lever > chuck[dot]lever[at]oracle[dot]com > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On May 6, 2013, at 11:32 AM, "J. Bruce Fields" <bfields@fieldses.org> wrote: > On Mon, May 06, 2013 at 09:27:54AM -0400, Weston Andros Adamson wrote: >> Older clients match the 'sec=' mount option flavor against the server's >> flavor list (if available) and return EPERM if the specified flavor is not >> found. Recent changes would skip this step and allow the vfs mount even >> though no operations will succeed. >> >> Signed-off-by: Weston Andros Adamson <dros@netapp.com> >> --- >> >> Hey Chuck, >> >> This fixes a regression that was introduced with the recent nfs_select_flavor >> changes - I'm pretty sure we want to match the specified flavor instead of just >> using it. >> >> Example of the regression: >> >> the server's /etc/exports: >> >> /export/krb5 *(sec=krb5,sec=none,rw,no_root_squash) >> >> old client behavior: >> >> $ uname -a >> Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux >> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt >> mount.nfs: timeout set for Sun May 5 17:32:04 2013 >> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10' >> mount.nfs: prog 100003, trying vers=3, prot=6 >> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049 >> mount.nfs: prog 100005, trying vers=3, prot=17 >> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048 >> mount.nfs: mount(2): Permission denied >> mount.nfs: access denied by server while mounting zero:/export/krb5 >> >> recently changed behavior: >> >> $ uname -a >> Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux >> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt >> mount.nfs: timeout set for Sun May 5 17:37:17 2013 >> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10' >> mount.nfs: prog 100003, trying vers=3, prot=6 >> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049 >> mount.nfs: prog 100005, trying vers=3, prot=17 >> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048 >> $ ls /mnt >> ls: cannot open directory /mnt: Permission denied >> $ sudo ls /mnt >> ls: cannot open directory /mnt: Permission denied >> $ sudo df /mnt >> df: ‘/mnt’: Permission denied >> df: no file systems processed >> $ sudo umount /mnt >> $ >> >> I also made an attempt to support "AUTH_NULL means the server will ignore the >> cred, so just use the specified flavor" behavior from much older kernels, but >> I'm not sure if we want this logic. >> >> I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some >> older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified, >> server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are >> ignored by the server), while newer kernels do not. The workaround in newer >> kernels is to specify sec=none. > > Nit: the above examples would actually be really useful, for example, to > a distribution maintainer trying to figure out whether this patch would > solve a given user problem. > > Could you include them in the change log? Yes, great suggestion. If/when we agree on this behavior, I'll repost. -dros > > --b. > >> >> Thoughts? >> >> -dros >> >> fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++------- >> 1 file changed, 32 insertions(+), 7 deletions(-) >> >> diff --git a/fs/nfs/super.c b/fs/nfs/super.c >> index eb494f6..6476b5d 100644 >> --- a/fs/nfs/super.c >> +++ b/fs/nfs/super.c >> @@ -1611,14 +1611,12 @@ out_security_failure: >> * Select a security flavor for this mount. The selected flavor >> * is planted in args->auth_flavors[0]. >> */ >> -static void nfs_select_flavor(struct nfs_parsed_mount_data *args, >> +static int nfs_select_flavor(struct nfs_parsed_mount_data *args, >> struct nfs_mount_request *request) >> { >> unsigned int i, count = *(request->auth_flav_len); >> rpc_authflavor_t flavor; >> - >> - if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) >> - goto out; >> + bool auth_null_seen = false; >> >> /* >> * The NFSv2 MNT operation does not return a flavor list. >> @@ -1634,6 +1632,26 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args, >> goto out_default; >> >> /* >> + * If the sec= mount option is used, the flavor must be in the list >> + * returned by the server. >> + * >> + * One exception is when the server's flavor list includes AUTH_NULL: >> + * Some servers implement AUTH_NULL by completely ignoring the rpc >> + * cred, so the client can use any flavor. >> + */ >> + if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) { >> + for (i = 0; i < count; i++) { >> + if (args->auth_flavors[0] == request->auth_flavs[i]) >> + goto out; >> + if (request->auth_flavs[i] == RPC_AUTH_NULL) >> + auth_null_seen = true; >> + } >> + if (auth_null_seen) >> + goto out; >> + goto out_nomatch; >> + } >> + >> + /* >> * RFC 2623, section 2.7 suggests we SHOULD prefer the >> * flavor listed first. However, some servers list >> * AUTH_NULL first. Avoid ever choosing AUTH_NULL. >> @@ -1654,11 +1672,19 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args, >> } >> >> out_default: >> - flavor = RPC_AUTH_UNIX; >> + /* use default if flavor not already set */ >> + flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ? >> + RPC_AUTH_UNIX : args->auth_flavors[0]; >> out_set: >> args->auth_flavors[0] = flavor; >> out: >> dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]); >> + return 0; >> + >> +out_nomatch: >> + dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n", >> + args->auth_flavors[0]); >> + return -EPERM; >> } >> >> /* >> @@ -1721,8 +1747,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args, >> return status; >> } >> >> - nfs_select_flavor(args, &request); >> - return 0; >> + return nfs_select_flavor(args, &request); >> } >> >> struct dentry *nfs_try_mount(int flags, const char *dev_name, >> -- >> 1.7.12.4 (Apple Git-37) >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On May 6, 2013, at 11:34 AM, "Adamson, Dros" <Weston.Adamson@netapp.com> wrote: > > On May 6, 2013, at 11:17 AM, Chuck Lever <chuck.lever@oracle.com> > wrote: > >> >> On May 6, 2013, at 10:49 AM, "Adamson, Dros" <Weston.Adamson@netapp.com> wrote: >> >>> >>> On May 6, 2013, at 9:52 AM, Chuck Lever <chuck.lever@oracle.com> wrote: >>> >>>> Hi- >>>> >>>> On May 6, 2013, at 9:27 AM, Weston Andros Adamson <dros@netapp.com> wrote: >>>> >>>>> Older clients match the 'sec=' mount option flavor against the server's >>>>> flavor list (if available) and return EPERM if the specified flavor is not >>>>> found. Recent changes would skip this step and allow the vfs mount even >>>>> though no operations will succeed. >>>>> >>>>> Signed-off-by: Weston Andros Adamson <dros@netapp.com> >>>>> --- >>>>> >>>>> Hey Chuck, >>>>> >>>>> This fixes a regression that was introduced with the recent nfs_select_flavor >>>>> changes - I'm pretty sure we want to match the specified flavor instead of just >>>>> using it. >>>>> >>>>> Example of the regression: >>>>> >>>>> the server's /etc/exports: >>>>> >>>>> /export/krb5 *(sec=krb5,sec=none,rw,no_root_squash) >>>>> >>>>> old client behavior: >>>>> >>>>> $ uname -a >>>>> Linux one.apikia.fake 3.8.8-202.fc18.x86_64 #1 SMP Wed Apr 17 23:25:17 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux >>>>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt >>>>> mount.nfs: timeout set for Sun May 5 17:32:04 2013 >>>>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10' >>>>> mount.nfs: prog 100003, trying vers=3, prot=6 >>>>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049 >>>>> mount.nfs: prog 100005, trying vers=3, prot=17 >>>>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048 >>>>> mount.nfs: mount(2): Permission denied >>>>> mount.nfs: access denied by server while mounting zero:/export/krb5 >>>> >>>> This is wrong behavior, and is fixed by my patch. >>>> >>>>> recently changed behavior: >>>>> >>>>> $ uname -a >>>>> Linux one.apikia.fake 3.9.0-testing+ #2 SMP Fri May 3 20:29:32 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux >>>>> $ sudo mount -v -o sec=sys,vers=3 zero:/export/krb5 /mnt >>>>> mount.nfs: timeout set for Sun May 5 17:37:17 2013 >>>>> mount.nfs: trying text-based options 'sec=sys,vers=3,addr=192.168.100.10' >>>>> mount.nfs: prog 100003, trying vers=3, prot=6 >>>>> mount.nfs: trying 192.168.100.10 prog 100003 vers 3 prot TCP port 2049 >>>>> mount.nfs: prog 100005, trying vers=3, prot=17 >>>>> mount.nfs: trying 192.168.100.10 prog 100005 vers 3 prot UDP port 20048 >>>>> $ ls /mnt >>>>> ls: cannot open directory /mnt: Permission denied >>>>> $ sudo ls /mnt >>>>> ls: cannot open directory /mnt: Permission denied >>>>> $ sudo df /mnt >>>>> df: ‘/mnt’: Permission denied >>>>> df: no file systems processed >>>>> $ sudo umount /mnt >>>>> $ >>>> >>>> This is correct behavior. The server should map the user's AUTH_SYS credential to anonymous. If anonymous does not have permission on /export/krb5, then the server should prevent its access to the export. >>> >>> You're talking about AUTH_NULL behavior here, right? >>> >>>> >>>> I assume this is a Linux NFS server. Does the Linux server support sec=none listed in the export options in the same way that NetApp and Oracle Solaris do? >>> >>> >>> So there are two issues here and I think they're getting confused. >>> >>> The example is of the first issue: mounting a krb5 only export with sec=sys >>> - client mounts sec=sys >>> - the server list is [krb5] >>> - the client completely ignores this list and just uses sys. >>> - no operations work, it's a 'dud mount' >>> >>> The second issue is the AUTH_NULL stuff - lets just ignore that for now. >>> >>> Are you saying that the client should just use whatever sec= is specified and never try to match against the server list? >> >> That's what my patch does. The reason for this is that the server can interpret the mount's security flavor any way it likes. >> >> Perhaps we should take that fallback position only if the server lists AUTH_NULL in the flavor list. If AUTH_NULL is not listed, ensure the flavor requested on the mount command line is available on the server (the most recent previous behavior); fail if no matching flavor is found. >> >> If the mount command line does not specify a flavor, we could look in the server's list for a flavor we support, then fail if none is found. > > If I'm reading this right, that's what this patch does. > > - if no sec= is specified, we look through the servers list for a supported flavor (same behavior as without this patch). If not found, return -EPERM. > - if sec= is specified, check the server list > - if flavor is found, use it > - else if AUTH_NULL is in the list, just use the user specified sec= flavor > - else return -EPERM Unconditionally use the user's flavor if AUTH_NULL is in the list. So reverse the order of the first two checks. > Also: if we don't get a server list for whatever reason, just use the specified sec= flavor. Leave the first checks in nfs_select_flavor() intact, in other words. > > Does that sound right? If so, does the patch look good? > > -dros > >> >>> A little background - I ran into this implementing a different patch that makes sure v4.x mounts are actually using the specified flavor (if one exists) to mount the export -- that is, it can follow SECINFOs to do initial lookup, but must be using the specified flavor on the export path passed to mount. After the initial mount lookup, SECINFOs will be used normally. Trond thought that this was a bug that should be fixed - that it's wrong when client reports that it's using one flavor (as seen in mount output, etc) when it's really using another. This (other) patch works, but when no version is specified, it always falls back to v3 and the mount will always succeed - even if the auth flavor isn't supported by the export. >>> >>> -dros >>> >>> >>>> >>>>> I also made an attempt to support "AUTH_NULL means the server will ignore the >>>>> cred, so just use the specified flavor" behavior from much older kernels, but >>>>> I'm not sure if we want this logic. >>>>> >>>>> I'd actually prefer getting rid of this AUTH_NULL logic, it's just that some >>>>> older clients (RHEL 5, etc) would succeed in mounting when sec=sys specified, >>>>> server list = [AUTH_NULL] (the client ends up sending AUTH_UNIX creds that are >>>>> ignored by the server), while newer kernels do not. The workaround in newer >>>>> kernels is to specify sec=none. >>>>> >>>>> Thoughts? >>>>> >>>>> -dros >>>>> >>>>> fs/nfs/super.c | 39 ++++++++++++++++++++++++++++++++------- >>>>> 1 file changed, 32 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c >>>>> index eb494f6..6476b5d 100644 >>>>> --- a/fs/nfs/super.c >>>>> +++ b/fs/nfs/super.c >>>>> @@ -1611,14 +1611,12 @@ out_security_failure: >>>>> * Select a security flavor for this mount. The selected flavor >>>>> * is planted in args->auth_flavors[0]. >>>>> */ >>>>> -static void nfs_select_flavor(struct nfs_parsed_mount_data *args, >>>>> +static int nfs_select_flavor(struct nfs_parsed_mount_data *args, >>>>> struct nfs_mount_request *request) >>>>> { >>>>> unsigned int i, count = *(request->auth_flav_len); >>>>> rpc_authflavor_t flavor; >>>>> - >>>>> - if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) >>>>> - goto out; >>>>> + bool auth_null_seen = false; >>>>> >>>>> /* >>>>> * The NFSv2 MNT operation does not return a flavor list. >>>>> @@ -1634,6 +1632,26 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args, >>>>> goto out_default; >>>>> >>>>> /* >>>>> + * If the sec= mount option is used, the flavor must be in the list >>>>> + * returned by the server. >>>>> + * >>>>> + * One exception is when the server's flavor list includes AUTH_NULL: >>>>> + * Some servers implement AUTH_NULL by completely ignoring the rpc >>>>> + * cred, so the client can use any flavor. >>>>> + */ >>>>> + if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) { >>>>> + for (i = 0; i < count; i++) { >>>>> + if (args->auth_flavors[0] == request->auth_flavs[i]) >>>>> + goto out; >>>>> + if (request->auth_flavs[i] == RPC_AUTH_NULL) >>>>> + auth_null_seen = true; >>>>> + } >>>>> + if (auth_null_seen) >>>>> + goto out; >>>>> + goto out_nomatch; >>>>> + } >>>>> + >>>>> + /* >>>>> * RFC 2623, section 2.7 suggests we SHOULD prefer the >>>>> * flavor listed first. However, some servers list >>>>> * AUTH_NULL first. Avoid ever choosing AUTH_NULL. >>>>> @@ -1654,11 +1672,19 @@ static void nfs_select_flavor(struct nfs_parsed_mount_data *args, >>>>> } >>>>> >>>>> out_default: >>>>> - flavor = RPC_AUTH_UNIX; >>>>> + /* use default if flavor not already set */ >>>>> + flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ? >>>>> + RPC_AUTH_UNIX : args->auth_flavors[0]; >>>>> out_set: >>>>> args->auth_flavors[0] = flavor; >>>>> out: >>>>> dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]); >>>>> + return 0; >>>>> + >>>>> +out_nomatch: >>>>> + dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n", >>>>> + args->auth_flavors[0]); >>>>> + return -EPERM; >>>>> } >>>>> >>>>> /* >>>>> @@ -1721,8 +1747,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args, >>>>> return status; >>>>> } >>>>> >>>>> - nfs_select_flavor(args, &request); >>>>> - return 0; >>>>> + return nfs_select_flavor(args, &request); >>>>> } >>>>> >>>>> struct dentry *nfs_try_mount(int flags, const char *dev_name, >>>>> -- >>>>> 1.7.12.4 (Apple Git-37) >>>>> >>>>> -- >>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>>> the body of a message to majordomo@vger.kernel.org >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> >>>> -- >>>> Chuck Lever >>>> chuck[dot]lever[at]oracle[dot]com >>>> >>>> >>>> >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> -- >> Chuck Lever >> chuck[dot]lever[at]oracle[dot]com >> >> >> >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --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,
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(-)