From 44fdcedd2a61cd40fe68aef533c878b5f2f665a8 Mon Sep 17 00:00:00 2001

From: Greg Hudson <ghudson@mit.edu>

Date: Tue, 20 Dec 2016 16:06:24 -0500

Subject: [PATCH] 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

(cherry picked from commit b061f419cfc9653b7549b905e54fbbd78deea49e)

---

 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 2885408a2..f44f162d3 100644

--- a/src/include/k5-trace.h

+++ b/src/include/k5-trace.h

@@ -291,6 +291,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 ed15550f0..80f5e1870 100644

--- a/src/lib/krb5/krb/get_in_tkt.c

+++ b/src/lib/krb5/krb/get_in_tkt.c

@@ -565,7 +565,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);

@@ -816,8 +816,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) {

@@ -1504,7 +1504,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;

@@ -1555,7 +1555,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)

@@ -1640,7 +1640,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 38c01c775..a7cded942 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 9c746d05b..f1667c238 100644

--- a/src/lib/krb5/krb/int-proto.h

+++ b/src/lib/krb5/krb/int-proto.h

@@ -194,17 +194,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 b04d14829..9a178f4e3 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)