krb5 commit: Properly scope per-request preauth data

Greg Hudson ghudson at mit.edu
Thu Jan 26 18:34:08 EST 2017


https://github.com/krb5/krb5/commit/b061f419cfc9653b7549b905e54fbbd78deea49e
commit b061f419cfc9653b7549b905e54fbbd78deea49e
Author: Greg Hudson <ghudson at mit.edu>
Date:   Tue Dec 20 16:06:24 2016 -0500

    Properly scope per-request preauth data
    
    It should be possible to successfully use multiple initial credentials
    contexts with the same library context.  Create a new internal type
    krb5_preauth_req_context containing per-request preauth state,
    including the clpreauth modreq handles and the list of preauth types
    already tried.  Remove this state from clpreauth_handle and
    krb5_preauth_context.
    
    ticket: 7877

 src/include/k5-trace.h            |    3 +
 src/lib/krb5/krb/get_in_tkt.c     |   12 +-
 src/lib/krb5/krb/init_creds_ctx.h |    3 +
 src/lib/krb5/krb/int-proto.h      |    8 +-
 src/lib/krb5/krb/preauth2.c       |  190 +++++++++++++++++++++++--------------
 5 files changed, 135 insertions(+), 81 deletions(-)

diff --git a/src/include/k5-trace.h b/src/include/k5-trace.h
index 15b7e32..f2075ad 100644
--- a/src/include/k5-trace.h
+++ b/src/include/k5-trace.h
@@ -305,6 +305,9 @@ void krb5int_trace(krb5_context context, const char *fmt, ...);
     TRACE(c, "Preauth tryagain input types: {patypes}", padata)
 #define TRACE_PREAUTH_TRYAGAIN_OUTPUT(c, padata)                        \
     TRACE(c, "Followup preauth for next request: {patypes}", padata)
+#define TRACE_PREAUTH_WRONG_CONTEXT(c)                                  \
+    TRACE(c, "Wrong context passed to krb5_init_creds_free(); leaking " \
+          "modreq objects")
 
 #define TRACE_PROFILE_ERR(c,subsection, section, retval)             \
     TRACE(c, "Bad value of {str} from [{str}] in conf file: {kerr}", \
diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c
index 54badbb..e422a04 100644
--- a/src/lib/krb5/krb/get_in_tkt.c
+++ b/src/lib/krb5/krb/get_in_tkt.c
@@ -583,7 +583,7 @@ krb5_init_creds_free(krb5_context context,
     k5_response_items_free(ctx->rctx.items);
     free(ctx->in_tkt_service);
     zapfree(ctx->gakpw.storage.data, ctx->gakpw.storage.length);
-    k5_preauth_request_context_fini(context);
+    k5_preauth_request_context_fini(context, ctx);
     krb5_free_error(context, ctx->err_reply);
     krb5_free_pa_data(context, ctx->err_padata);
     krb5_free_cred_contents(context, &ctx->cred);
@@ -834,8 +834,8 @@ restart_init_creds_loop(krb5_context context, krb5_init_creds_context ctx,
     if (fast_upgrade)
         ctx->fast_state->fast_state_flags |= KRB5INT_FAST_DO_FAST;
 
-    k5_preauth_request_context_fini(context);
-    k5_preauth_request_context_init(context);
+    k5_preauth_request_context_fini(context, ctx);
+    k5_preauth_request_context_init(context, ctx);
     krb5_free_data(context, ctx->outer_request_body);
     ctx->outer_request_body = NULL;
     if (ctx->opt->flags & KRB5_GET_INIT_CREDS_OPT_PREAUTH_LIST) {
@@ -1522,7 +1522,7 @@ init_creds_step_reply(krb5_context context,
         } else if ((reply_code == KDC_ERR_MORE_PREAUTH_DATA_REQUIRED ||
                     reply_code == KDC_ERR_PREAUTH_REQUIRED) && retry) {
             /* reset the list of preauth types to try */
-            k5_reset_preauth_types_tried(context);
+            k5_reset_preauth_types_tried(ctx);
             krb5_free_pa_data(context, ctx->preauth_to_use);
             ctx->preauth_to_use = ctx->err_padata;
             ctx->err_padata = NULL;
@@ -1573,7 +1573,7 @@ init_creds_step_reply(krb5_context context,
         goto cleanup;
 
     /* process any preauth data in the as_reply */
-    k5_reset_preauth_types_tried(context);
+    k5_reset_preauth_types_tried(ctx);
     code = krb5int_fast_process_response(context, ctx->fast_state,
                                          ctx->reply, &strengthen_key);
     if (code != 0)
@@ -1658,7 +1658,7 @@ init_creds_step_reply(krb5_context context,
             k5_prependmsg(context, code, _("Failed to store credentials"));
     }
 
-    k5_preauth_request_context_fini(context);
+    k5_preauth_request_context_fini(context, ctx);
 
     /* success */
     ctx->complete = TRUE;
diff --git a/src/lib/krb5/krb/init_creds_ctx.h b/src/lib/krb5/krb/init_creds_ctx.h
index 38c01c7..a7cded9 100644
--- a/src/lib/krb5/krb/init_creds_ctx.h
+++ b/src/lib/krb5/krb/init_creds_ctx.h
@@ -6,6 +6,8 @@
 #include "k5-json.h"
 #include "int-proto.h"
 
+typedef struct krb5_preauth_req_context_st *krb5_preauth_req_context;
+
 struct krb5_responder_context_st {
     k5_response_items *items;
 };
@@ -67,6 +69,7 @@ struct _krb5_init_creds_context {
     krb5_timestamp pa_offset;
     krb5_int32 pa_offset_usec;
     enum { NO_OFFSET = 0, UNAUTH_OFFSET, AUTH_OFFSET } pa_offset_state;
+    krb5_preauth_req_context preauth_reqctx;
 };
 
 krb5_error_code
diff --git a/src/lib/krb5/krb/int-proto.h b/src/lib/krb5/krb/int-proto.h
index 6da7485..c506253 100644
--- a/src/lib/krb5/krb/int-proto.h
+++ b/src/lib/krb5/krb/int-proto.h
@@ -196,17 +196,19 @@ void
 k5_free_preauth_context(krb5_context context);
 
 void
-k5_reset_preauth_types_tried(krb5_context context);
+k5_reset_preauth_types_tried(krb5_init_creds_context ctx);
 
 void
 k5_preauth_prepare_request(krb5_context context, krb5_get_init_creds_opt *opt,
                            krb5_kdc_req *request);
 
 void
-k5_preauth_request_context_init(krb5_context context);
+k5_preauth_request_context_init(krb5_context context,
+                                krb5_init_creds_context ctx);
 
 void
-k5_preauth_request_context_fini(krb5_context context);
+k5_preauth_request_context_fini(krb5_context context,
+                                krb5_init_creds_context ctx);
 
 krb5_error_code
 k5_response_items_new(k5_response_items **ri_out);
diff --git a/src/lib/krb5/krb/preauth2.c b/src/lib/krb5/krb/preauth2.c
index b04d148..9a178f4 100644
--- a/src/lib/krb5/krb/preauth2.c
+++ b/src/lib/krb5/krb/preauth2.c
@@ -46,14 +46,18 @@
 typedef struct {
     struct krb5_clpreauth_vtable_st vt;
     krb5_clpreauth_moddata data;
-    krb5_clpreauth_modreq req;
 } *clpreauth_handle;
 
 struct krb5_preauth_context_st {
-    krb5_preauthtype *tried;
     clpreauth_handle *handles;
 };
 
+struct krb5_preauth_req_context_st {
+    krb5_context orig_context;
+    krb5_preauthtype *tried;
+    krb5_clpreauth_modreq *modreqs;
+};
+
 /* Release the memory used by a list of handles. */
 static void
 free_handles(krb5_context context, clpreauth_handle *handles)
@@ -71,21 +75,44 @@ free_handles(krb5_context context, clpreauth_handle *handles)
     free(handles);
 }
 
-/* Find the handle in handles which can process pa_type. */
-static clpreauth_handle
-find_module(clpreauth_handle *handles, krb5_preauthtype pa_type)
+/* Return an index into handles which can process pa_type, or -1 if none is
+ * found found. */
+static int
+search_module_list(clpreauth_handle *handles, krb5_preauthtype pa_type)
 {
-    clpreauth_handle *hp, h;
-    krb5_preauthtype *tp;
+    clpreauth_handle h;
+    int i, j;
 
-    for (hp = handles; *hp != NULL; hp++) {
-        h = *hp;
-        for (tp = h->vt.pa_type_list; *tp != 0; tp++) {
-            if (*tp == pa_type)
-                return h;
+    for (i = 0; handles[i] != NULL; i++) {
+        h = handles[i];
+        for (j = 0; h->vt.pa_type_list[j] != 0; j++) {
+            if (h->vt.pa_type_list[j] == pa_type)
+                return i;
         }
     }
-    return FALSE;
+    return -1;
+}
+
+/* Find the handle which can process pa_type, or NULL if none is found.  On
+ * success, set *modreq_out to the corresponding per-request module data. */
+static clpreauth_handle
+find_module(krb5_context context, krb5_init_creds_context ctx,
+            krb5_preauthtype pa_type, krb5_clpreauth_modreq *modreq_out)
+{
+    krb5_preauth_context pctx = context->preauth_context;
+    krb5_preauth_req_context reqctx = ctx->preauth_reqctx;
+    int i;
+
+    *modreq_out = NULL;
+    if (pctx == NULL || reqctx == NULL)
+        return NULL;
+
+    i = search_module_list(pctx->handles, pa_type);
+    if (i == -1)
+        return NULL;
+
+    *modreq_out = reqctx->modreqs[i];
+    return pctx->handles[i];
 }
 
 /* Initialize the preauth state for a krb5 context. */
@@ -93,7 +120,8 @@ void
 k5_init_preauth_context(krb5_context context)
 {
     krb5_plugin_initvt_fn *modules = NULL, *mod;
-    clpreauth_handle *list = NULL, h, h2;
+    clpreauth_handle *list = NULL, h;
+    int i;
     size_t count;
     krb5_preauthtype *tp;
 
@@ -140,9 +168,10 @@ k5_init_preauth_context(krb5_context context)
 
         /* Check for a preauth type conflict with an existing module. */
         for (tp = h->vt.pa_type_list; *tp != 0; tp++) {
-            h2 = find_module(list, *tp);
-            if (h2 != NULL) {
-                TRACE_PREAUTH_CONFLICT(context, h->vt.name, h2->vt.name, *tp);
+            i = search_module_list(list, *tp);
+            if (i != -1) {
+                TRACE_PREAUTH_CONFLICT(context, h->vt.name, list[i]->vt.name,
+                                       *tp);
                 break;
             }
         }
@@ -164,7 +193,6 @@ k5_init_preauth_context(krb5_context context)
     context->preauth_context = malloc(sizeof(*context->preauth_context));
     if (context->preauth_context == NULL)
         goto cleanup;
-    context->preauth_context->tried = NULL;
     context->preauth_context->handles = list;
     list = NULL;
 
@@ -179,14 +207,14 @@ cleanup:
  * AS-REP).
  */
 void
-k5_reset_preauth_types_tried(krb5_context context)
+k5_reset_preauth_types_tried(krb5_init_creds_context ctx)
 {
-    krb5_preauth_context pctx = context->preauth_context;
+    krb5_preauth_req_context reqctx = ctx->preauth_reqctx;
 
-    if (pctx == NULL)
+    if (reqctx == NULL)
         return;
-    free(pctx->tried);
-    pctx->tried = NULL;
+    free(reqctx->tried);
+    reqctx->tried = NULL;
 }
 
 
@@ -200,7 +228,6 @@ k5_free_preauth_context(krb5_context context)
 
     if (pctx == NULL)
         return;
-    free(pctx->tried);
     free_handles(context, pctx->handles);
     free(pctx);
     context->preauth_context = NULL;
@@ -209,10 +236,13 @@ k5_free_preauth_context(krb5_context context)
 /* Initialize the per-AS-REQ context. This means calling the client_req_init
  * function to give the plugin a chance to allocate a per-request context. */
 void
-k5_preauth_request_context_init(krb5_context context)
+k5_preauth_request_context_init(krb5_context context,
+                                krb5_init_creds_context ctx)
 {
     krb5_preauth_context pctx = context->preauth_context;
-    clpreauth_handle *hp, h;
+    clpreauth_handle h;
+    krb5_preauth_req_context reqctx;
+    size_t count, i;
 
     if (pctx == NULL) {
         k5_init_preauth_context(context);
@@ -220,30 +250,50 @@ k5_preauth_request_context_init(krb5_context context)
         if (pctx == NULL)
             return;
     }
-    k5_reset_preauth_types_tried(context);
-    for (hp = pctx->handles; *hp != NULL; hp++) {
-        h = *hp;
+
+    reqctx = calloc(1, sizeof(*reqctx));
+    if (reqctx == NULL)
+        return;
+    reqctx->orig_context = context;
+
+    /* Create an array of per-request module data objects corresponding to the
+     * preauth context's array of handles. */
+    for (count = 0; pctx->handles[count] != NULL; count++);
+    reqctx->modreqs = calloc(count, sizeof(*reqctx->modreqs));
+    for (i = 0; i < count; i++) {
+        h = pctx->handles[i];
         if (h->vt.request_init != NULL)
-            h->vt.request_init(context, h->data, &h->req);
+            h->vt.request_init(context, h->data, &reqctx->modreqs[i]);
     }
+    ctx->preauth_reqctx = reqctx;
 }
 
 /* Free the per-AS-REQ context. This means clearing any request-specific
  * context which the plugin may have created. */
 void
-k5_preauth_request_context_fini(krb5_context context)
+k5_preauth_request_context_fini(krb5_context context,
+                                krb5_init_creds_context ctx)
 {
     krb5_preauth_context pctx = context->preauth_context;
-    clpreauth_handle *hp, h;
+    krb5_preauth_req_context reqctx = ctx->preauth_reqctx;
+    size_t i;
+    clpreauth_handle h;
 
-    if (pctx == NULL)
+    if (reqctx == NULL)
         return;
-    for (hp = pctx->handles; *hp != NULL; hp++) {
-        h = *hp;
-        if (h->req != NULL && h->vt.request_fini != NULL)
-            h->vt.request_fini(context, h->data, h->req);
-        h->req = NULL;
+    if (reqctx->orig_context == context && pctx != NULL) {
+        for (i = 0; pctx->handles[i] != NULL; i++) {
+            h = pctx->handles[i];
+            if (reqctx->modreqs[i] != NULL && h->vt.request_fini != NULL)
+                h->vt.request_fini(context, h->data, reqctx->modreqs[i]);
+        }
+    } else {
+        TRACE_PREAUTH_WRONG_CONTEXT(context);
     }
+    free(reqctx->modreqs);
+    free(reqctx->tried);
+    free(reqctx);
+    ctx->preauth_reqctx = NULL;
 }
 
 /* Return 1 if pa_type is a real preauthentication mechanism according to the
@@ -259,6 +309,7 @@ clpreauth_is_real(krb5_context context, clpreauth_handle h,
 
 static krb5_error_code
 clpreauth_prep_questions(krb5_context context, clpreauth_handle h,
+                         krb5_clpreauth_modreq modreq,
                          krb5_get_init_creds_opt *opt,
                          krb5_clpreauth_callbacks cb, krb5_clpreauth_rock rock,
                          krb5_kdc_req *req, krb5_data *req_body,
@@ -266,35 +317,35 @@ clpreauth_prep_questions(krb5_context context, clpreauth_handle h,
 {
     if (h->vt.prep_questions == NULL)
         return 0;
-    return h->vt.prep_questions(context, h->data, h->req, opt, cb, rock, req,
+    return h->vt.prep_questions(context, h->data, modreq, opt, cb, rock, req,
                                 req_body, prev_req, pa_data);
 }
 
 static krb5_error_code
 clpreauth_process(krb5_context context, clpreauth_handle h,
-                  krb5_get_init_creds_opt *opt, krb5_clpreauth_callbacks cb,
-                  krb5_clpreauth_rock rock, krb5_kdc_req *req,
-                  krb5_data *req_body, krb5_data *prev_req,
+                  krb5_clpreauth_modreq modreq, krb5_get_init_creds_opt *opt,
+                  krb5_clpreauth_callbacks cb, krb5_clpreauth_rock rock,
+                  krb5_kdc_req *req, krb5_data *req_body, krb5_data *prev_req,
                   krb5_pa_data *pa_data, krb5_prompter_fct prompter,
                   void *prompter_data, krb5_pa_data ***pa_data_out)
 {
-    return h->vt.process(context, h->data, h->req, opt, cb, rock, req,
+    return h->vt.process(context, h->data, modreq, opt, cb, rock, req,
                          req_body, prev_req, pa_data, prompter, prompter_data,
                          pa_data_out);
 }
 
 static krb5_error_code
 clpreauth_tryagain(krb5_context context, clpreauth_handle h,
-                   krb5_get_init_creds_opt *opt, krb5_clpreauth_callbacks cb,
-                   krb5_clpreauth_rock rock, krb5_kdc_req *req,
-                   krb5_data *req_body, krb5_data *prev_req,
+                   krb5_clpreauth_modreq modreq, krb5_get_init_creds_opt *opt,
+                   krb5_clpreauth_callbacks cb, krb5_clpreauth_rock rock,
+                   krb5_kdc_req *req, krb5_data *req_body, krb5_data *prev_req,
                    krb5_preauthtype pa_type, krb5_error *error,
                    krb5_pa_data **error_padata, krb5_prompter_fct prompter,
                    void *prompter_data, krb5_pa_data ***pa_data_out)
 {
     if (h->vt.tryagain == NULL)
         return 0;
-    return h->vt.tryagain(context, h->data, h->req, opt, cb, rock, req,
+    return h->vt.tryagain(context, h->data, modreq, opt, cb, rock, req,
                           req_body, prev_req, pa_type, error, error_padata,
                           prompter, prompter_data, pa_data_out);
 }
@@ -554,22 +605,22 @@ pa_type_allowed(krb5_init_creds_context ctx, krb5_preauthtype pa_type)
  * types and return false.
  */
 static krb5_boolean
-already_tried(krb5_context context, krb5_preauthtype pa_type)
+already_tried(krb5_init_creds_context ctx, krb5_preauthtype pa_type)
 {
-    krb5_preauth_context pctx = context->preauth_context;
-    size_t count;
+    krb5_preauth_req_context reqctx = ctx->preauth_reqctx;
+    size_t i;
     krb5_preauthtype *newptr;
 
-    for (count = 0; pctx->tried != NULL && pctx->tried[count] != 0; count++) {
-        if (pctx->tried[count] == pa_type)
+    for (i = 0; reqctx->tried != NULL && reqctx->tried[i] != 0; i++) {
+        if (reqctx->tried[i] == pa_type)
             return TRUE;
     }
-    newptr = realloc(pctx->tried, (count + 2) * sizeof(*newptr));
+    newptr = realloc(reqctx->tried, (i + 2) * sizeof(*newptr));
     if (newptr == NULL)
         return FALSE;
-    pctx->tried = newptr;
-    pctx->tried[count] = pa_type;
-    pctx->tried[count + 1] = ENCTYPE_NULL;
+    reqctx->tried = newptr;
+    reqctx->tried[i] = pa_type;
+    reqctx->tried[i + 1] = ENCTYPE_NULL;
     return FALSE;
 }
 
@@ -580,16 +631,13 @@ process_pa_data(krb5_context context, krb5_init_creds_context ctx,
                 krb5_pa_data ***out_pa_list, int *out_pa_list_size,
                 krb5_preauthtype *out_type)
 {
-    krb5_preauth_context pctx = context->preauth_context;
     struct errinfo save = EMPTY_ERRINFO;
     krb5_pa_data *pa, **pa_ptr, **mod_pa;
     krb5_error_code ret = 0;
+    krb5_clpreauth_modreq modreq;
     clpreauth_handle h;
     int real, i;
 
-    if (pctx == NULL)
-        return ENOENT;
-
     /* Process all informational padata types, then the first real preauth type
      * we succeed on. */
     for (real = 0; real <= 1; real++) {
@@ -598,17 +646,17 @@ process_pa_data(krb5_context context, krb5_init_creds_context ctx,
             /* Restrict real mechanisms to the chosen one if we have one. */
             if (real && !pa_type_allowed(ctx, pa->pa_type))
                 continue;
-            h = find_module(pctx->handles, pa->pa_type);
+            h = find_module(context, ctx, pa->pa_type, &modreq);
             if (h == NULL)
                 continue;
             /* Make sure this type is for the current pass. */
             if (clpreauth_is_real(context, h, pa->pa_type) != real)
                 continue;
             /* Only try a real mechanism once per authentication. */
-            if (real && already_tried(context, pa->pa_type))
+            if (real && already_tried(ctx, pa->pa_type))
                 continue;
             mod_pa = NULL;
-            ret = clpreauth_process(context, h, ctx->opt, &callbacks,
+            ret = clpreauth_process(context, h, modreq, ctx->opt, &callbacks,
                                     (krb5_clpreauth_rock)ctx, ctx->request,
                                     ctx->inner_request_body,
                                     ctx->encoded_previous_request, pa,
@@ -858,24 +906,22 @@ krb5_error_code
 k5_preauth_tryagain(krb5_context context, krb5_init_creds_context ctx,
                     krb5_pa_data **in_padata, krb5_pa_data ***padata_out)
 {
-    krb5_preauth_context pctx = context->preauth_context;
     krb5_error_code ret;
     krb5_pa_data **mod_pa;
+    krb5_clpreauth_modreq modreq;
     clpreauth_handle h;
     int i;
 
     *padata_out = NULL;
-    if (pctx == NULL)
-        return KRB5KRB_ERR_GENERIC;
 
     TRACE_PREAUTH_TRYAGAIN_INPUT(context, in_padata);
 
     for (i = 0; in_padata[i] != NULL; i++) {
-        h = find_module(pctx->handles, in_padata[i]->pa_type);
+        h = find_module(context, ctx, in_padata[i]->pa_type, &modreq);
         if (h == NULL)
             continue;
         mod_pa = NULL;
-        ret = clpreauth_tryagain(context, h, ctx->opt, &callbacks,
+        ret = clpreauth_tryagain(context, h, modreq, ctx->opt, &callbacks,
                                  (krb5_clpreauth_rock)ctx, ctx->request,
                                  ctx->inner_request_body,
                                  ctx->encoded_previous_request,
@@ -897,9 +943,9 @@ static krb5_error_code
 fill_response_items(krb5_context context, krb5_init_creds_context ctx,
                     krb5_pa_data **in_padata)
 {
-    krb5_preauth_context pctx = context->preauth_context;
     krb5_error_code ret;
     krb5_pa_data *pa;
+    krb5_clpreauth_modreq modreq;
     clpreauth_handle h;
     int i;
 
@@ -908,11 +954,11 @@ fill_response_items(krb5_context context, krb5_init_creds_context ctx,
         pa = in_padata[i];
         if (!pa_type_allowed(ctx, pa->pa_type))
             continue;
-        h = find_module(pctx->handles, pa->pa_type);
+        h = find_module(context, ctx, pa->pa_type, &modreq);
         if (h == NULL)
             continue;
-        ret = clpreauth_prep_questions(context, h, ctx->opt, &callbacks,
-                                       (krb5_clpreauth_rock)ctx,
+        ret = clpreauth_prep_questions(context, h, modreq, ctx->opt,
+                                       &callbacks, (krb5_clpreauth_rock)ctx,
                                        ctx->request, ctx->inner_request_body,
                                        ctx->encoded_previous_request, pa);
         if (ret)


More information about the cvs-krb5 mailing list