* [PATCH 1/3] auth_gss: fetch the acceptor name out of the downcall
2014-04-10 20:29 [PATCH 0/3] nfs: fix v4.0 callback channel auth failures Jeff Layton
@ 2014-04-10 20:29 ` Jeff Layton
2014-04-10 20:29 ` [PATCH 2/3] sunrpc: add a new "stringify_acceptor" rpc_credop Jeff Layton
2014-04-10 20:29 ` [PATCH 3/3] nfs4: copy acceptor name from context to nfs_client Jeff Layton
2 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2014-04-10 20:29 UTC (permalink / raw)
To: trond.myklebust; +Cc: steved, linux-nfs
If rpc.gssd sends us an acceptor name string trailing the context token,
stash it as part of the context.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
include/linux/sunrpc/auth_gss.h | 1 +
net/sunrpc/auth_gss/auth_gss.c | 20 +++++++++++++++++---
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/include/linux/sunrpc/auth_gss.h b/include/linux/sunrpc/auth_gss.h
index f1cfd4c85cd0..cbc6875fb9cf 100644
--- a/include/linux/sunrpc/auth_gss.h
+++ b/include/linux/sunrpc/auth_gss.h
@@ -71,6 +71,7 @@ struct gss_cl_ctx {
spinlock_t gc_seq_lock;
struct gss_ctx __rcu *gc_gss_ctx;
struct xdr_netobj gc_wire_ctx;
+ struct xdr_netobj gc_acceptor;
u32 gc_win;
unsigned long gc_expiry;
struct rcu_head gc_rcu;
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index 36e431ee1c90..f341aabb92db 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -262,9 +262,22 @@ gss_fill_context(const void *p, const void *end, struct gss_cl_ctx *ctx, struct
p = ERR_PTR(ret);
goto err;
}
- dprintk("RPC: %s Success. gc_expiry %lu now %lu timeout %u\n",
- __func__, ctx->gc_expiry, now, timeout);
- return q;
+
+ /* is there any trailing data? */
+ if (q == end) {
+ p = q;
+ goto done;
+ }
+
+ /* pull in acceptor name (if there is one) */
+ p = simple_get_netobj(q, end, &ctx->gc_acceptor);
+ if (IS_ERR(p))
+ goto err;
+done:
+ dprintk("RPC: %s Success. gc_expiry %lu now %lu timeout %u acceptor %.*s\n",
+ __func__, ctx->gc_expiry, now, timeout, ctx->gc_acceptor.len,
+ ctx->gc_acceptor.data);
+ return p;
err:
dprintk("RPC: %s returns error %ld\n", __func__, -PTR_ERR(p));
return p;
@@ -1225,6 +1238,7 @@ gss_do_free_ctx(struct gss_cl_ctx *ctx)
gss_delete_sec_context(&ctx->gc_gss_ctx);
kfree(ctx->gc_wire_ctx.data);
+ kfree(ctx->gc_acceptor.data);
kfree(ctx);
}
--
1.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] sunrpc: add a new "stringify_acceptor" rpc_credop
2014-04-10 20:29 [PATCH 0/3] nfs: fix v4.0 callback channel auth failures Jeff Layton
2014-04-10 20:29 ` [PATCH 1/3] auth_gss: fetch the acceptor name out of the downcall Jeff Layton
@ 2014-04-10 20:29 ` Jeff Layton
2014-04-10 20:29 ` [PATCH 3/3] nfs4: copy acceptor name from context to nfs_client Jeff Layton
2 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2014-04-10 20:29 UTC (permalink / raw)
To: trond.myklebust; +Cc: steved, linux-nfs
...and add an new rpc_auth function to call it when it exists. This
is only applicable for AUTH_GSS mechanisms, so we only specify this
for those sorts of credentials.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
include/linux/sunrpc/auth.h | 2 ++
net/sunrpc/auth.c | 9 ++++++
net/sunrpc/auth_gss/auth_gss.c | 62 ++++++++++++++++++++++++++++--------------
3 files changed, 53 insertions(+), 20 deletions(-)
diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index 790be1472792..c683b9a06913 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -140,6 +140,7 @@ struct rpc_credops {
void *, __be32 *, void *);
int (*crkey_timeout)(struct rpc_cred *);
bool (*crkey_to_expire)(struct rpc_cred *);
+ char * (*crstringify_acceptor)(struct rpc_cred *);
};
extern const struct rpc_authops authunix_ops;
@@ -182,6 +183,7 @@ void rpcauth_clear_credcache(struct rpc_cred_cache *);
int rpcauth_key_timeout_notify(struct rpc_auth *,
struct rpc_cred *);
bool rpcauth_cred_key_to_expire(struct rpc_cred *);
+char * rpcauth_stringify_acceptor(struct rpc_cred *);
static inline
struct rpc_cred * get_rpccred(struct rpc_cred *cred)
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index 5285ead196c0..ef51bb77c465 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -363,6 +363,15 @@ rpcauth_cred_key_to_expire(struct rpc_cred *cred)
}
EXPORT_SYMBOL_GPL(rpcauth_cred_key_to_expire);
+char *
+rpcauth_stringify_acceptor(struct rpc_cred *cred)
+{
+ if (!cred->cr_ops->crstringify_acceptor)
+ return NULL;
+ return cred->cr_ops->crstringify_acceptor(cred);
+}
+EXPORT_SYMBOL_GPL(rpcauth_stringify_acceptor);
+
/*
* Destroy a list of credentials
*/
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index f341aabb92db..2f5b860f3491 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -1346,6 +1346,26 @@ gss_cred_init(struct rpc_auth *auth, struct rpc_cred *cred)
return err;
}
+static char *
+gss_stringify_acceptor(struct rpc_cred *cred)
+{
+ char *string;
+ struct gss_cred *gss_cred = container_of(cred, struct gss_cred, gc_base);
+ struct xdr_netobj *acceptor = &gss_cred->gc_ctx->gc_acceptor;
+
+ /* no point if there's no string */
+ if (!acceptor->len)
+ return NULL;
+
+ string = kmalloc(acceptor->len + 1, GFP_KERNEL);
+ if (!string)
+ return string;
+
+ memcpy(string, acceptor->data, acceptor->len);
+ string[acceptor->len] = '\0';
+ return string;
+}
+
/*
* Returns -EACCES if GSS context is NULL or will expire within the
* timeout (miliseconds)
@@ -1923,29 +1943,31 @@ static const struct rpc_authops authgss_ops = {
};
static const struct rpc_credops gss_credops = {
- .cr_name = "AUTH_GSS",
- .crdestroy = gss_destroy_cred,
- .cr_init = gss_cred_init,
- .crbind = rpcauth_generic_bind_cred,
- .crmatch = gss_match,
- .crmarshal = gss_marshal,
- .crrefresh = gss_refresh,
- .crvalidate = gss_validate,
- .crwrap_req = gss_wrap_req,
- .crunwrap_resp = gss_unwrap_resp,
- .crkey_timeout = gss_key_timeout,
+ .cr_name = "AUTH_GSS",
+ .crdestroy = gss_destroy_cred,
+ .cr_init = gss_cred_init,
+ .crbind = rpcauth_generic_bind_cred,
+ .crmatch = gss_match,
+ .crmarshal = gss_marshal,
+ .crrefresh = gss_refresh,
+ .crvalidate = gss_validate,
+ .crwrap_req = gss_wrap_req,
+ .crunwrap_resp = gss_unwrap_resp,
+ .crkey_timeout = gss_key_timeout,
+ .crstringify_acceptor = gss_stringify_acceptor,
};
static const struct rpc_credops gss_nullops = {
- .cr_name = "AUTH_GSS",
- .crdestroy = gss_destroy_nullcred,
- .crbind = rpcauth_generic_bind_cred,
- .crmatch = gss_match,
- .crmarshal = gss_marshal,
- .crrefresh = gss_refresh_null,
- .crvalidate = gss_validate,
- .crwrap_req = gss_wrap_req,
- .crunwrap_resp = gss_unwrap_resp,
+ .cr_name = "AUTH_GSS",
+ .crdestroy = gss_destroy_nullcred,
+ .crbind = rpcauth_generic_bind_cred,
+ .crmatch = gss_match,
+ .crmarshal = gss_marshal,
+ .crrefresh = gss_refresh_null,
+ .crvalidate = gss_validate,
+ .crwrap_req = gss_wrap_req,
+ .crunwrap_resp = gss_unwrap_resp,
+ .crstringify_acceptor = gss_stringify_acceptor,
};
static const struct rpc_pipe_ops gss_upcall_ops_v0 = {
--
1.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] nfs4: copy acceptor name from context to nfs_client
2014-04-10 20:29 [PATCH 0/3] nfs: fix v4.0 callback channel auth failures Jeff Layton
2014-04-10 20:29 ` [PATCH 1/3] auth_gss: fetch the acceptor name out of the downcall Jeff Layton
2014-04-10 20:29 ` [PATCH 2/3] sunrpc: add a new "stringify_acceptor" rpc_credop Jeff Layton
@ 2014-04-10 20:29 ` Jeff Layton
2014-05-28 21:39 ` Trond Myklebust
2 siblings, 1 reply; 6+ messages in thread
From: Jeff Layton @ 2014-04-10 20:29 UTC (permalink / raw)
To: trond.myklebust; +Cc: steved, linux-nfs
The current CB_COMPOUND handling code tries to compare the principal
name of the request with the cl_hostname in the client. This is not
guaranteed to ever work, particularly if the client happened to mount
a CNAME of the server or a non-fqdn.
Fix this by instead comparing the cr_principal string with the acceptor
name that we get from gssd. In the event that gssd didn't send one
down (i.e. it was too old), then we fall back to trying to use the
cl_hostname as we do today.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/callback.c | 12 ++++++++++++
fs/nfs/client.c | 1 +
fs/nfs/nfs4proc.c | 30 +++++++++++++++++++++++++++++-
include/linux/nfs_fs_sb.h | 1 +
include/linux/nfs_xdr.h | 1 +
5 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 073b4cf67ed9..54de482143cc 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -428,6 +428,18 @@ check_gss_callback_principal(struct nfs_client *clp, struct svc_rqst *rqstp)
if (p == NULL)
return 0;
+ /*
+ * Did we get the acceptor from userland during the SETCLIENID
+ * negotiation?
+ */
+ if (clp->cl_acceptor)
+ return !strcmp(p, clp->cl_acceptor);
+
+ /*
+ * Otherwise try to verify it using the cl_hostname. Note that this
+ * doesn't work if a non-canonical hostname was used in the devname.
+ */
+
/* Expect a GSS_C_NT_HOSTBASED_NAME like "nfs@serverhostname" */
if (memcmp(p, "nfs@", 4) != 0)
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 1d09289c8f0e..6a461378fcf2 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -252,6 +252,7 @@ void nfs_free_client(struct nfs_client *clp)
put_net(clp->cl_net);
put_nfs_version(clp->cl_nfs_mod);
kfree(clp->cl_hostname);
+ kfree(clp->cl_acceptor);
kfree(clp);
dprintk("<-- nfs_free_client()\n");
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 397be39c6dc8..cc6c3fe3e060 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4896,6 +4896,18 @@ nfs4_init_callback_netid(const struct nfs_client *clp, char *buf, size_t len)
return scnprintf(buf, len, "tcp");
}
+static void nfs4_setclientid_done(struct rpc_task *task, void *calldata)
+{
+ struct nfs4_setclientid *sc = calldata;
+
+ if (task->tk_status == 0)
+ *sc->sc_acceptor = rpcauth_stringify_acceptor(task->tk_rqstp->rq_cred);
+}
+
+static const struct rpc_call_ops nfs4_setclientid_ops = {
+ .rpc_call_done = nfs4_setclientid_done,
+};
+
/**
* nfs4_proc_setclientid - Negotiate client ID
* @clp: state data structure
@@ -4915,6 +4927,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
.sc_verifier = &sc_verifier,
.sc_prog = program,
.sc_cb_ident = clp->cl_cb_ident,
+ .sc_acceptor = &clp->cl_acceptor,
};
struct rpc_message msg = {
.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SETCLIENTID],
@@ -4922,6 +4935,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
.rpc_resp = res,
.rpc_cred = cred,
};
+ struct rpc_task *task;
+ struct rpc_task_setup task_setup_data = {
+ .rpc_client = clp->cl_rpcclient,
+ .rpc_message = &msg,
+ .callback_ops = &nfs4_setclientid_ops,
+ .callback_data = &setclientid,
+ .flags = RPC_TASK_TIMEOUT,
+ };
int status;
/* nfs_client_id4 */
@@ -4948,7 +4969,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
dprintk("NFS call setclientid auth=%s, '%.*s'\n",
clp->cl_rpcclient->cl_auth->au_ops->au_name,
setclientid.sc_name_len, setclientid.sc_name);
- status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
+ task = rpc_run_task(&task_setup_data);
+ if (IS_ERR(task)) {
+ status = PTR_ERR(task);
+ goto out;
+ }
+ status = task->tk_status;
+ rpc_put_task(task);
+out:
trace_nfs4_setclientid(clp, status);
dprintk("NFS reply setclientid: %d\n", status);
return status;
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 1150ea41b626..922be2e050f5 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -45,6 +45,7 @@ struct nfs_client {
struct sockaddr_storage cl_addr; /* server identifier */
size_t cl_addrlen;
char * cl_hostname; /* hostname of server */
+ char * cl_acceptor; /* GSSAPI acceptor name */
struct list_head cl_share_link; /* link in global client list */
struct list_head cl_superblocks; /* List of nfs_server structs */
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 6fb5b2335b59..5945d88a26e2 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1010,6 +1010,7 @@ struct nfs4_setclientid {
unsigned int sc_uaddr_len;
char sc_uaddr[RPCBIND_MAXUADDRLEN + 1];
u32 sc_cb_ident;
+ char **sc_acceptor;
};
struct nfs4_setclientid_res {
--
1.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] nfs4: copy acceptor name from context to nfs_client
2014-04-10 20:29 ` [PATCH 3/3] nfs4: copy acceptor name from context to nfs_client Jeff Layton
@ 2014-05-28 21:39 ` Trond Myklebust
2014-05-28 23:20 ` Jeff Layton
0 siblings, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2014-05-28 21:39 UTC (permalink / raw)
To: Jeffrey Layton; +Cc: steved, linux-nfs
On Thu, 2014-04-10 at 16:29 -0400, Jeff Layton wrote:
> The current CB_COMPOUND handling code tries to compare the principal
> name of the request with the cl_hostname in the client. This is not
> guaranteed to ever work, particularly if the client happened to mount
> a CNAME of the server or a non-fqdn.
>
> Fix this by instead comparing the cr_principal string with the acceptor
> name that we get from gssd. In the event that gssd didn't send one
> down (i.e. it was too old), then we fall back to trying to use the
> cl_hostname as we do today.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> fs/nfs/callback.c | 12 ++++++++++++
> fs/nfs/client.c | 1 +
> fs/nfs/nfs4proc.c | 30 +++++++++++++++++++++++++++++-
> include/linux/nfs_fs_sb.h | 1 +
> include/linux/nfs_xdr.h | 1 +
> 5 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> index 073b4cf67ed9..54de482143cc 100644
> --- a/fs/nfs/callback.c
> +++ b/fs/nfs/callback.c
> @@ -428,6 +428,18 @@ check_gss_callback_principal(struct nfs_client *clp, struct svc_rqst *rqstp)
> if (p == NULL)
> return 0;
>
> + /*
> + * Did we get the acceptor from userland during the SETCLIENID
> + * negotiation?
> + */
> + if (clp->cl_acceptor)
> + return !strcmp(p, clp->cl_acceptor);
> +
> + /*
> + * Otherwise try to verify it using the cl_hostname. Note that this
> + * doesn't work if a non-canonical hostname was used in the devname.
> + */
> +
> /* Expect a GSS_C_NT_HOSTBASED_NAME like "nfs@serverhostname" */
>
> if (memcmp(p, "nfs@", 4) != 0)
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 1d09289c8f0e..6a461378fcf2 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -252,6 +252,7 @@ void nfs_free_client(struct nfs_client *clp)
> put_net(clp->cl_net);
> put_nfs_version(clp->cl_nfs_mod);
> kfree(clp->cl_hostname);
> + kfree(clp->cl_acceptor);
> kfree(clp);
>
> dprintk("<-- nfs_free_client()\n");
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 397be39c6dc8..cc6c3fe3e060 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -4896,6 +4896,18 @@ nfs4_init_callback_netid(const struct nfs_client *clp, char *buf, size_t len)
> return scnprintf(buf, len, "tcp");
> }
>
> +static void nfs4_setclientid_done(struct rpc_task *task, void *calldata)
> +{
> + struct nfs4_setclientid *sc = calldata;
> +
> + if (task->tk_status == 0)
> + *sc->sc_acceptor = rpcauth_stringify_acceptor(task->tk_rqstp->rq_cred);
We can't call a GFP_KERNEL memory allocator from within a RPC callback
function. This needs to be done outside the RPC call.
> +}
> +
> +static const struct rpc_call_ops nfs4_setclientid_ops = {
> + .rpc_call_done = nfs4_setclientid_done,
> +};
> +
> /**
> * nfs4_proc_setclientid - Negotiate client ID
> * @clp: state data structure
> @@ -4915,6 +4927,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
> .sc_verifier = &sc_verifier,
> .sc_prog = program,
> .sc_cb_ident = clp->cl_cb_ident,
> + .sc_acceptor = &clp->cl_acceptor,
> };
> struct rpc_message msg = {
> .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SETCLIENTID],
> @@ -4922,6 +4935,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
> .rpc_resp = res,
> .rpc_cred = cred,
> };
> + struct rpc_task *task;
> + struct rpc_task_setup task_setup_data = {
> + .rpc_client = clp->cl_rpcclient,
> + .rpc_message = &msg,
> + .callback_ops = &nfs4_setclientid_ops,
> + .callback_data = &setclientid,
> + .flags = RPC_TASK_TIMEOUT,
> + };
> int status;
>
> /* nfs_client_id4 */
> @@ -4948,7 +4969,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
> dprintk("NFS call setclientid auth=%s, '%.*s'\n",
> clp->cl_rpcclient->cl_auth->au_ops->au_name,
> setclientid.sc_name_len, setclientid.sc_name);
> - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
> + task = rpc_run_task(&task_setup_data);
> + if (IS_ERR(task)) {
> + status = PTR_ERR(task);
> + goto out;
> + }
> + status = task->tk_status;
Why not just do it here?
> + rpc_put_task(task);
> +out:
> trace_nfs4_setclientid(clp, status);
> dprintk("NFS reply setclientid: %d\n", status);
> return status;
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 1150ea41b626..922be2e050f5 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -45,6 +45,7 @@ struct nfs_client {
> struct sockaddr_storage cl_addr; /* server identifier */
> size_t cl_addrlen;
> char * cl_hostname; /* hostname of server */
> + char * cl_acceptor; /* GSSAPI acceptor name */
> struct list_head cl_share_link; /* link in global client list */
> struct list_head cl_superblocks; /* List of nfs_server structs */
>
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 6fb5b2335b59..5945d88a26e2 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -1010,6 +1010,7 @@ struct nfs4_setclientid {
> unsigned int sc_uaddr_len;
> char sc_uaddr[RPCBIND_MAXUADDRLEN + 1];
> u32 sc_cb_ident;
> + char **sc_acceptor;
> };
>
> struct nfs4_setclientid_res {
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] nfs4: copy acceptor name from context to nfs_client
2014-05-28 21:39 ` Trond Myklebust
@ 2014-05-28 23:20 ` Jeff Layton
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2014-05-28 23:20 UTC (permalink / raw)
To: Trond Myklebust; +Cc: Jeffrey Layton, steved, linux-nfs
On Wed, 28 May 2014 17:39:32 -0400
Trond Myklebust <trond.myklebust@primarydata.com> wrote:
> On Thu, 2014-04-10 at 16:29 -0400, Jeff Layton wrote:
> > The current CB_COMPOUND handling code tries to compare the principal
> > name of the request with the cl_hostname in the client. This is not
> > guaranteed to ever work, particularly if the client happened to mount
> > a CNAME of the server or a non-fqdn.
> >
> > Fix this by instead comparing the cr_principal string with the acceptor
> > name that we get from gssd. In the event that gssd didn't send one
> > down (i.e. it was too old), then we fall back to trying to use the
> > cl_hostname as we do today.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/nfs/callback.c | 12 ++++++++++++
> > fs/nfs/client.c | 1 +
> > fs/nfs/nfs4proc.c | 30 +++++++++++++++++++++++++++++-
> > include/linux/nfs_fs_sb.h | 1 +
> > include/linux/nfs_xdr.h | 1 +
> > 5 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> > index 073b4cf67ed9..54de482143cc 100644
> > --- a/fs/nfs/callback.c
> > +++ b/fs/nfs/callback.c
> > @@ -428,6 +428,18 @@ check_gss_callback_principal(struct nfs_client *clp, struct svc_rqst *rqstp)
> > if (p == NULL)
> > return 0;
> >
> > + /*
> > + * Did we get the acceptor from userland during the SETCLIENID
> > + * negotiation?
> > + */
> > + if (clp->cl_acceptor)
> > + return !strcmp(p, clp->cl_acceptor);
> > +
> > + /*
> > + * Otherwise try to verify it using the cl_hostname. Note that this
> > + * doesn't work if a non-canonical hostname was used in the devname.
> > + */
> > +
> > /* Expect a GSS_C_NT_HOSTBASED_NAME like "nfs@serverhostname" */
> >
> > if (memcmp(p, "nfs@", 4) != 0)
> > diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> > index 1d09289c8f0e..6a461378fcf2 100644
> > --- a/fs/nfs/client.c
> > +++ b/fs/nfs/client.c
> > @@ -252,6 +252,7 @@ void nfs_free_client(struct nfs_client *clp)
> > put_net(clp->cl_net);
> > put_nfs_version(clp->cl_nfs_mod);
> > kfree(clp->cl_hostname);
> > + kfree(clp->cl_acceptor);
> > kfree(clp);
> >
> > dprintk("<-- nfs_free_client()\n");
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 397be39c6dc8..cc6c3fe3e060 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -4896,6 +4896,18 @@ nfs4_init_callback_netid(const struct nfs_client *clp, char *buf, size_t len)
> > return scnprintf(buf, len, "tcp");
> > }
> >
> > +static void nfs4_setclientid_done(struct rpc_task *task, void *calldata)
> > +{
> > + struct nfs4_setclientid *sc = calldata;
> > +
> > + if (task->tk_status == 0)
> > + *sc->sc_acceptor = rpcauth_stringify_acceptor(task->tk_rqstp->rq_cred);
>
> We can't call a GFP_KERNEL memory allocator from within a RPC callback
> function. This needs to be done outside the RPC call.
>
Ahh right...I should know that by now. Sorry...
> > +}
> > +
> > +static const struct rpc_call_ops nfs4_setclientid_ops = {
> > + .rpc_call_done = nfs4_setclientid_done,
> > +};
> > +
> > /**
> > * nfs4_proc_setclientid - Negotiate client ID
> > * @clp: state data structure
> > @@ -4915,6 +4927,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
> > .sc_verifier = &sc_verifier,
> > .sc_prog = program,
> > .sc_cb_ident = clp->cl_cb_ident,
> > + .sc_acceptor = &clp->cl_acceptor,
> > };
> > struct rpc_message msg = {
> > .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SETCLIENTID],
> > @@ -4922,6 +4935,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
> > .rpc_resp = res,
> > .rpc_cred = cred,
> > };
> > + struct rpc_task *task;
> > + struct rpc_task_setup task_setup_data = {
> > + .rpc_client = clp->cl_rpcclient,
> > + .rpc_message = &msg,
> > + .callback_ops = &nfs4_setclientid_ops,
> > + .callback_data = &setclientid,
> > + .flags = RPC_TASK_TIMEOUT,
> > + };
> > int status;
> >
> > /* nfs_client_id4 */
> > @@ -4948,7 +4969,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
> > dprintk("NFS call setclientid auth=%s, '%.*s'\n",
> > clp->cl_rpcclient->cl_auth->au_ops->au_name,
> > setclientid.sc_name_len, setclientid.sc_name);
> > - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
> > + task = rpc_run_task(&task_setup_data);
> > + if (IS_ERR(task)) {
> > + status = PTR_ERR(task);
> > + goto out;
> > + }
> > + status = task->tk_status;
>
> Why not just do it here?
>
It's been a while since I've looked at this set, but I think I tried
that initially. At this point, the rq_cred has been destroyed, so it's
too late to call rpcauth_stringify_acceptor on it.
I guess what I can do is turn the sc_acceptor field into an rpc_cred
pointer, and have nfs4_setclientid_done bump the refcount on it and
set the pointer. Then I can call rpcauth_stringify_acceptor on it here,
and put the cred.
I'll do that once I have a chance to respin and retest. It may be a bit
as I have some other priorities at the moment. ;)
> > + rpc_put_task(task);
> > +out:
> > trace_nfs4_setclientid(clp, status);
> > dprintk("NFS reply setclientid: %d\n", status);
> > return status;
> > diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> > index 1150ea41b626..922be2e050f5 100644
> > --- a/include/linux/nfs_fs_sb.h
> > +++ b/include/linux/nfs_fs_sb.h
> > @@ -45,6 +45,7 @@ struct nfs_client {
> > struct sockaddr_storage cl_addr; /* server identifier */
> > size_t cl_addrlen;
> > char * cl_hostname; /* hostname of server */
> > + char * cl_acceptor; /* GSSAPI acceptor name */
> > struct list_head cl_share_link; /* link in global client list */
> > struct list_head cl_superblocks; /* List of nfs_server structs */
> >
> > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> > index 6fb5b2335b59..5945d88a26e2 100644
> > --- a/include/linux/nfs_xdr.h
> > +++ b/include/linux/nfs_xdr.h
> > @@ -1010,6 +1010,7 @@ struct nfs4_setclientid {
> > unsigned int sc_uaddr_len;
> > char sc_uaddr[RPCBIND_MAXUADDRLEN + 1];
> > u32 sc_cb_ident;
> > + char **sc_acceptor;
> > };
> >
> > struct nfs4_setclientid_res {
>
--
Jeff Layton <jlayton@primarydata.com>
^ permalink raw reply [flat|nested] 6+ messages in thread