by Steve Dickson

[permalink] [raw]

Subject: Re: [PATCH 5/5] gssd: scrape the acceptor name out of the context

Hey Jeff,

Just a couple nit....

On 04/10/2014 04:31 PM, Jeff Layton wrote:
> ...and pass it to the kernel in the downcall. Legacy kernels will just
> ignore the extra data, but with a proposed kernel patch the kernel will
> grab this info and use it to verify requests on the v4.0 callback
> channel.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> utils/gssd/gssd_proc.c | 39 ++++++++++++++++++++++++++++-----------
> 1 file changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
> index 7387cce010cf..d95e39416c28 100644
> --- a/utils/gssd/gssd_proc.c
> +++ b/utils/gssd/gssd_proc.c
> @@ -77,6 +77,7 @@
> #include "context.h"
> #include "nfsrpc.h"
> #include "nfslib.h"
> +#include "gss_names.h"
>
> /*
> * pollarray:
> @@ -683,16 +684,19 @@ parse_enctypes(char *enctypes)
>
> static void
> do_downcall(int k5_fd, uid_t uid, struct authgss_private_data *pd,
> - gss_buffer_desc *context_token, OM_uint32 lifetime_rec)
> + gss_buffer_desc *context_token, OM_uint32 lifetime_rec,
> + gss_buffer_desc *acceptor)
> {
> char *buf = NULL, *p = NULL, *end = NULL;
> unsigned int timeout = context_timeout;
> unsigned int buf_size = 0;
>
> - printerr(1, "doing downcall lifetime_rec %u\n", lifetime_rec);
> + printerr(1, "doing downcall: lifetime_rec=%u acceptor=%.*s\n",
> + lifetime_rec, acceptor->length, acceptor->value);
> buf_size = sizeof(uid) + sizeof(timeout) + sizeof(pd->pd_seq_win) +
> sizeof(pd->pd_ctx_hndl.length) + pd->pd_ctx_hndl.length +
> - sizeof(context_token->length) + context_token->length;
> + sizeof(context_token->length) + context_token->length +
> + acceptor->length;
> p = buf = malloc(buf_size);
> if (!buf)
> goto out_err;
> @@ -707,6 +711,8 @@ do_downcall(int k5_fd, uid_t uid, struct authgss_private_data *pd,
> if (WRITE_BYTES(&p, end, pd->pd_seq_win)) goto out_err;
> if (write_buffer(&p, end, &pd->pd_ctx_hndl)) goto out_err;
> if (write_buffer(&p, end, context_token)) goto out_err;
> + if (acceptor->length > 0 &&
> + write_buffer(&p, end, acceptor)) goto out_err;
>
> if (write(k5_fd, buf, p - buf) free(buf);
> @@ -1034,6 +1040,9 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
> gss_cred_id_t gss_cred;
> OM_uint32 maj_stat, min_stat, lifetime_rec;
> pid_t pid;
> + gss_name_t gacceptor;
> + gss_OID mech;
> + gss_buffer_desc acceptor = {0};
>
> pid = fork();
> switch(pid) {
> @@ -1174,15 +1183,22 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
> goto out_return_error;
> }
>
> - /* Grab the context lifetime to pass to the kernel. lifetime_rec
> - * is set to zero on error */
Why get rid of this comment instead of updating it?

> - maj_stat = gss_inquire_context(&min_stat, pd.pd_ctx, NULL, NULL,
> - &lifetime_rec, NULL, NULL, NULL, NULL);
> + maj_stat = gss_inquire_context(&min_stat, pd.pd_ctx, NULL, &gacceptor,
> + &lifetime_rec, &mech, NULL, NULL, NULL);
>
> - if (maj_stat)
> - printerr(1, "WARNING: Failed to inquire context for lifetme "
> - "maj_stat %u\n", maj_stat);
> + if (maj_stat != GSS_S_COMPLETE) {
> + printerr(1, "WARNING: Failed to inquire context "
> + "maj_stat (0x%x)\n", maj_stat);
> + } else {
> + get_hostbased_client_buffer(gacceptor, mech, &acceptor);
> + gss_release_name(&min_stat, &gacceptor);
> + }
>
> + /*
> + * The serialization can mean turning the ctx into a lucid context. If
> + * that happens then the original ctx is no longer valid, so we mustn't
> + * try to use if after this point.
I'm not sure what you are trying to say here...

steved.

> + */
> if (serialize_context_for_kernel(&pd.pd_ctx, &token, &krb5oid, NULL)) {
> printerr(0, "WARNING: Failed to serialize krb5 context for "
> "user with uid %d for server %s\n",
> @@ -1190,9 +1206,10 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
> goto out_return_error;
> }
>
> - do_downcall(fd, uid, &pd, &token, lifetime_rec);
> + do_downcall(fd, uid, &pd, &token, lifetime_rec, &acceptor);
>
> out:
> + gss_release_buffer(&min_stat, &acceptor);
> if (token.value)
> free(token.value);
> #ifdef HAVE_AUTHGSS_FREE_PRIVATE_DATA
>