krb5 commit: Properly scope per-request preauth data

Greg Hudson <[email protected]>
Newsgroups gmane.comp.encryption.kerberos.cvs
Message-ID <[email protected]>
https://github.com/krb5/krb5/commit/b061f419cfc9653b7549b905e54fbbd78deea49e
commit b061f419cfc9653b7549b905e54fbbd78deea49e
Author: Greg Hudson <[email protected]>
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)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.