On Wed, 26 Jun 2013 11:15:08 -0400
Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
>
> On Jun 26, 2013, at 10:36 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> > Currently, when using NFSv3 the mount will fail if the server happens to
> > have AUTH_GSS flavors in the returned authlist before AUTH_UNIX. This
> > seems to have been a deliberate change in commit 4580a92 (NFS: Use
> > server-recommended security flavor by default (NFSv3)).
>
> As an aside, this (from the patch description for 4580a92):
>
> > If a server lists Kerberos pseudoflavors before "sys" in its export
> > options, our client now chooses Kerberos over AUTH_UNIX for mount
> > points, when no security flavor is specified by the mount command.
> > This could be surprising to some administrators or users, who would
> > then need to have Kerberos credentials to access the export.
>
>
> is a description of side-effects of the changes in 4580a92. This text is intended as a warning that behavior could change after 4580a92, not as a statement of purpose. It describes a known limitation of the approach introduced in 4580a92.
>
Ok, good to know...
> > While the workarounds are fine, I think we can do better here and allow
> > this to keep "just working". Allow the client to fall back to
> > automatically trying AUTH_UNIX under if the following are all true:
> >
> > - the server return -EACCES from ->create_server call
> > - the client had to do a MNT request (i.e. no binary options)
> > - we didn't just try to use AUTH_UNIX
> > - the admin did not explcitly specify a sec= option
> >
> > At that point, try to use AUTH_UNIX, if the server listed it.
>
> During these checks, how do you know the server specified AUTH_SYS in its list? It seems to me you want to retry with the next flavor in server_authlist until you've exhausted the list.
>
> Thus, IMO, the code you want to emulate is not nfs4_discover_server_trunking(), but rather nfs4_find_root_sec(), substituting the flavor list returned by the server for the static array of flavors.
>
Possibly. Often, we get a list that looks something like this:
[ 379.237691] NFS: received 4 auth flavors
[ 379.257005] NFS: auth flavor[0]: 390005
[ 379.278296] NFS: auth flavor[1]: 390004
[ 379.298526] NFS: auth flavor[2]: 390003
[ 379.318313] NFS: auth flavor[3]: 1
So we could instead walk down the list and try each in turn, but how
likely is it that one of the other AUTH_GSS flavors will work once the
first one failed? I'm not sure it's worth adding the extra complexity
to do that when it's likely that the only one that will eventually work
is the AUTH_SYS one anyway.
> > Cc: Chuck Lever <chuck.lever@xxxxxxxxxx>
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > fs/nfs/super.c | 39 +++++++++++++++++++++++++++++++--------
> > 1 file changed, 31 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 5538bcc..6df327e 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -1701,10 +1701,10 @@ out_err:
> > * corresponding to the provided path.
> > */
> > static int nfs_request_mount(struct nfs_parsed_mount_data *args,
> > - struct nfs_fh *root_fh)
> > + struct nfs_fh *root_fh,
> > + unsigned int *server_authlist_len,
> > + rpc_authflavor_t *server_authlist)
> > {
> > - rpc_authflavor_t server_authlist[NFS_MAX_SECFLAVORS];
> > - unsigned int server_authlist_len = ARRAY_SIZE(server_authlist);
> > struct nfs_mount_request request = {
> > .sap = (struct sockaddr *)
> > &args->mount_server.address,
> > @@ -1712,7 +1712,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args,
> > .protocol = args->mount_server.protocol,
> > .fh = root_fh,
> > .noresvport = args->flags & NFS_MOUNT_NORESVPORT,
> > - .auth_flav_len = &server_authlist_len,
> > + .auth_flav_len = server_authlist_len,
> > .auth_flavs = server_authlist,
> > .net = args->net,
> > };
> > @@ -1756,7 +1756,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args,
> > return status;
> > }
> >
> > - return nfs_select_flavor(args, server_authlist_len, server_authlist);
> > + return 0;
> > }
> >
> > struct dentry *nfs_try_mount(int flags, const char *dev_name,
> > @@ -1765,17 +1765,40 @@ struct dentry *nfs_try_mount(int flags, const char *dev_name,
> > {
> > int status;
> > struct nfs_server *server;
> > + struct nfs_parsed_mount_data *args = mount_info->parsed;
> > + rpc_authflavor_t requested_authflavor = args->auth_flavors[0];
> > + rpc_authflavor_t server_authlist[NFS_MAX_SECFLAVORS];
> > + unsigned int server_authlist_len = ARRAY_SIZE(server_authlist);
> >
> > - if (mount_info->parsed->need_mount) {
> > - status = nfs_request_mount(mount_info->parsed, mount_info->mntfh);
> > + if (args->need_mount) {
> > + status = nfs_request_mount(args, mount_info->mntfh,
> > + &server_authlist_len, server_authlist);
> > + if (status)
> > + return ERR_PTR(status);
> > +retry:
> > + status = nfs_select_flavor(args, server_authlist_len,
> > + server_authlist);
> > if (status)
> > return ERR_PTR(status);
> > }
> >
> > /* Get a volume representation */
> > server = nfs_mod->rpc_ops->create_server(mount_info, nfs_mod);
> > - if (IS_ERR(server))
> > + if (IS_ERR(server)) {
> > + /*
> > + * If the initial attempt fails with -EACCES, then that likely
> > + * means that we couldn't get proper credentials to talk to the
> > + * server. If an authflavor wasn't specified in the options,
> > + * and we didn't try to use it already, then try AUTH_UNIX.
> > + */
> > + if (PTR_ERR(server) == -EACCES && args->need_mount &&
> > + args->auth_flavors[0] != RPC_AUTH_UNIX &&
> > + requested_authflavor == RPC_AUTH_MAXFLAVOR) {
> > + args->auth_flavors[0] = RPC_AUTH_UNIX;
>
> This might be a lot easier to read if you refactored nfs_try_mount() into two subfunctions: one which handles the args->need_mount case, and the other which handles the opposite case.
>
Good point. I'll look into doing that.
> > + goto retry;
> > + }
> > return ERR_CAST(server);
> > + }
> >
> > return nfs_fs_mount_common(server, flags, dev_name, mount_info, nfs_mod);
> > }
> > --
> > 1.8.1.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Thanks,
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
