d738b9
From 6a69660d9415bc49948143109759f36b2ad70d1b Mon Sep 17 00:00:00 2001
d738b9
From: Greg Hudson <ghudson@mit.edu>
d738b9
Date: Fri, 13 Jan 2017 12:16:04 -0500
d738b9
Subject: [PATCH] Track preauth failures instead of tries
d738b9
d738b9
In preauth2.c, instead of noting whenever we try a real preauth mech,
d738b9
note when a mechanism fails on our side.  Tracking only failures
d738b9
eliminates the need to reset the list for multi-step preauth exchanges
d738b9
or for processing padata in the AS-REP, but we will need the function
d738b9
later for continuing after optimistic preauth failures.
d738b9
d738b9
ticket: 8537
d738b9
(cherry picked from commit a1dc81d22304e77edaa8388c7d7d75cade81dc80)
d738b9
---
d738b9
 src/lib/krb5/krb/get_in_tkt.c |  3 --
d738b9
 src/lib/krb5/krb/int-proto.h  |  3 ++
d738b9
 src/lib/krb5/krb/preauth2.c   | 65 ++++++++++++++++++++---------------
d738b9
 3 files changed, 40 insertions(+), 31 deletions(-)
d738b9
d738b9
diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c
d738b9
index 48dc00ea6..bc903b6e9 100644
d738b9
--- a/src/lib/krb5/krb/get_in_tkt.c
d738b9
+++ b/src/lib/krb5/krb/get_in_tkt.c
d738b9
@@ -1496,8 +1496,6 @@ init_creds_step_reply(krb5_context context,
d738b9
             code = restart_init_creds_loop(context, ctx, FALSE);
d738b9
         } else if ((reply_code == KDC_ERR_MORE_PREAUTH_DATA_REQUIRED ||
d738b9
                     reply_code == KDC_ERR_PREAUTH_REQUIRED) && retry) {
d738b9
-            /* reset the list of preauth types to try */
d738b9
-            k5_reset_preauth_types_tried(ctx);
d738b9
             krb5_free_pa_data(context, ctx->preauth_to_use);
d738b9
             ctx->preauth_to_use = ctx->err_padata;
d738b9
             ctx->err_padata = NULL;
d738b9
@@ -1547,7 +1545,6 @@ init_creds_step_reply(krb5_context context,
d738b9
         goto cleanup;
d738b9
 
d738b9
     /* process any preauth data in the as_reply */
d738b9
-    k5_reset_preauth_types_tried(ctx);
d738b9
     code = krb5int_fast_process_response(context, ctx->fast_state,
d738b9
                                          ctx->reply, &strengthen_key);
d738b9
     if (code != 0)
d738b9
diff --git a/src/lib/krb5/krb/int-proto.h b/src/lib/krb5/krb/int-proto.h
d738b9
index 8903df232..41a69c207 100644
d738b9
--- a/src/lib/krb5/krb/int-proto.h
d738b9
+++ b/src/lib/krb5/krb/int-proto.h
d738b9
@@ -197,6 +197,9 @@ k5_free_preauth_context(krb5_context context);
d738b9
 void
d738b9
 k5_reset_preauth_types_tried(krb5_init_creds_context ctx);
d738b9
 
d738b9
+krb5_error_code
d738b9
+k5_preauth_note_failed(krb5_init_creds_context ctx, krb5_preauthtype pa_type);
d738b9
+
d738b9
 void
d738b9
 k5_preauth_prepare_request(krb5_context context, krb5_get_init_creds_opt *opt,
d738b9
                            krb5_kdc_req *request);
d738b9
diff --git a/src/lib/krb5/krb/preauth2.c b/src/lib/krb5/krb/preauth2.c
d738b9
index 354234a93..17f2133b1 100644
d738b9
--- a/src/lib/krb5/krb/preauth2.c
d738b9
+++ b/src/lib/krb5/krb/preauth2.c
d738b9
@@ -54,7 +54,7 @@ struct krb5_preauth_context_st {
d738b9
 
d738b9
 struct krb5_preauth_req_context_st {
d738b9
     krb5_context orig_context;
d738b9
-    krb5_preauthtype *tried;
d738b9
+    krb5_preauthtype *failed;
d738b9
     krb5_clpreauth_modreq *modreqs;
d738b9
 };
d738b9
 
d738b9
@@ -201,11 +201,7 @@ cleanup:
d738b9
     free_handles(context, list);
d738b9
 }
d738b9
 
d738b9
-/*
d738b9
- * Reset the memory of which preauth types we have already tried, because we
d738b9
- * are entering a new phase of padata processing (such as the padata in an
d738b9
- * AS-REP).
d738b9
- */
d738b9
+/* Reset the memory of which preauth types we have already tried. */
d738b9
 void
d738b9
 k5_reset_preauth_types_tried(krb5_init_creds_context ctx)
d738b9
 {
d738b9
@@ -213,10 +209,27 @@ k5_reset_preauth_types_tried(krb5_init_creds_context ctx)
d738b9
 
d738b9
     if (reqctx == NULL)
d738b9
         return;
d738b9
-    free(reqctx->tried);
d738b9
-    reqctx->tried = NULL;
d738b9
+    free(reqctx->failed);
d738b9
+    reqctx->failed = NULL;
d738b9
 }
d738b9
 
d738b9
+/* Add pa_type to the list of types which has previously failed. */
d738b9
+krb5_error_code
d738b9
+k5_preauth_note_failed(krb5_init_creds_context ctx, krb5_preauthtype pa_type)
d738b9
+{
d738b9
+    krb5_preauth_req_context reqctx = ctx->preauth_reqctx;
d738b9
+    krb5_preauthtype *newptr;
d738b9
+    size_t i;
d738b9
+
d738b9
+    for (i = 0; reqctx->failed != NULL && reqctx->failed[i] != 0; i++);
d738b9
+    newptr = realloc(reqctx->failed, (i + 2) * sizeof(*newptr));
d738b9
+    if (newptr == NULL)
d738b9
+        return ENOMEM;
d738b9
+    reqctx->failed = newptr;
d738b9
+    reqctx->failed[i] = pa_type;
d738b9
+    reqctx->failed[i + 1] = 0;
d738b9
+    return 0;
d738b9
+}
d738b9
 
d738b9
 /* Free the per-krb5_context preauth_context. This means clearing any
d738b9
  * plugin-specific context which may have been created, and then
d738b9
@@ -291,7 +304,7 @@ k5_preauth_request_context_fini(krb5_context context,
d738b9
         TRACE_PREAUTH_WRONG_CONTEXT(context);
d738b9
     }
d738b9
     free(reqctx->modreqs);
d738b9
-    free(reqctx->tried);
d738b9
+    free(reqctx->failed);
d738b9
     free(reqctx);
d738b9
     ctx->preauth_reqctx = NULL;
d738b9
 }
d738b9
@@ -612,28 +625,17 @@ pa_type_allowed(krb5_init_creds_context ctx, krb5_preauthtype pa_type)
d738b9
         pa_type == ctx->allowed_preauth_type;
d738b9
 }
d738b9
 
d738b9
-/*
d738b9
- * If pa_type has already been tried as a real preauth type for this
d738b9
- * authentication, return true.  Otherwise ass pa_type to the list of tried
d738b9
- * types and return false.
d738b9
- */
d738b9
+/* Return true if pa_type previously failed during this authentication. */
d738b9
 static krb5_boolean
d738b9
-already_tried(krb5_init_creds_context ctx, krb5_preauthtype pa_type)
d738b9
+previously_failed(krb5_init_creds_context ctx, krb5_preauthtype pa_type)
d738b9
 {
d738b9
     krb5_preauth_req_context reqctx = ctx->preauth_reqctx;
d738b9
     size_t i;
d738b9
-    krb5_preauthtype *newptr;
d738b9
 
d738b9
-    for (i = 0; reqctx->tried != NULL && reqctx->tried[i] != 0; i++) {
d738b9
-        if (reqctx->tried[i] == pa_type)
d738b9
+    for (i = 0; reqctx->failed != NULL && reqctx->failed[i] != 0; i++) {
d738b9
+        if (reqctx->failed[i] == pa_type)
d738b9
             return TRUE;
d738b9
     }
d738b9
-    newptr = realloc(reqctx->tried, (i + 2) * sizeof(*newptr));
d738b9
-    if (newptr == NULL)
d738b9
-        return FALSE;
d738b9
-    reqctx->tried = newptr;
d738b9
-    reqctx->tried[i] = pa_type;
d738b9
-    reqctx->tried[i + 1] = ENCTYPE_NULL;
d738b9
     return FALSE;
d738b9
 }
d738b9
 
d738b9
@@ -665,8 +667,8 @@ process_pa_data(krb5_context context, krb5_init_creds_context ctx,
d738b9
             /* Make sure this type is for the current pass. */
d738b9
             if (clpreauth_is_real(context, h, pa->pa_type) != real)
d738b9
                 continue;
d738b9
-            /* Only try a real mechanism once per authentication. */
d738b9
-            if (real && already_tried(ctx, pa->pa_type))
d738b9
+            /* Don't try a real mechanism again after failure. */
d738b9
+            if (real && previously_failed(ctx, pa->pa_type))
d738b9
                 continue;
d738b9
             mod_pa = NULL;
d738b9
             ret = clpreauth_process(context, h, modreq, ctx->opt, &callbacks,
d738b9
@@ -694,6 +696,12 @@ process_pa_data(krb5_context context, krb5_init_creds_context ctx,
d738b9
                 /* Save the first error we get from a real preauth type. */
d738b9
                 k5_save_ctx_error(context, ret, &save);
d738b9
             }
d738b9
+            if (real && ret) {
d738b9
+                /* Don't try this mechanism again for this authentication. */
d738b9
+                ret = k5_preauth_note_failed(ctx, pa->pa_type);
d738b9
+                if (ret)
d738b9
+                    goto cleanup;
d738b9
+            }
d738b9
         }
d738b9
     }
d738b9
 
d738b9
@@ -944,9 +952,10 @@ k5_preauth_tryagain(krb5_context context, krb5_init_creds_context ctx,
d738b9
     TRACE_PREAUTH_TRYAGAIN(context, h->vt.name, pa_type, ret);
d738b9
     if (!ret && mod_pa == NULL)
d738b9
         ret = KRB5KRB_ERR_GENERIC;
d738b9
-    if (ret)
d738b9
+    if (ret) {
d738b9
+        k5_preauth_note_failed(ctx, pa_type);
d738b9
         return ret;
d738b9
-
d738b9
+    }
d738b9
 
d738b9
     for (count = 0; mod_pa[count] != NULL; count++);
d738b9
     ret = copy_cookie(context, err_padata, &mod_pa, &count);