krb5 commit: Track preauth failures instead of tries

https://github.com/krb5/krb5/commit/a1dc81d22304e77edaa8388c7d7d75cade81dc80
commit a1dc81d22304e77edaa8388c7d7d75cade81dc80
Author: Greg Hudson <[email protected]>
Date:   Fri Jan 13 12:16:04 2017 -0500

    Track preauth failures instead of tries
    
    In preauth2.c, instead of noting whenever we try a real preauth mech,
    note when a mechanism fails on our side.  Tracking only failures
    eliminates the need to reset the list for multi-step preauth exchanges
    or for processing padata in the AS-REP, but we will need the function
    later for continuing after optimistic preauth failures.
    
    ticket: 8537

 src/lib/krb5/krb/get_in_tkt.c |    3 --
 src/lib/krb5/krb/int-proto.h  |    3 ++
 src/lib/krb5/krb/preauth2.c   |   65 +++++++++++++++++++++++-----------------
 3 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c
index e48ade1..99d843e 100644
--- a/src/lib/krb5/krb/get_in_tkt.c
+++ b/src/lib/krb5/krb/get_in_tkt.c
@@ -1514,8 +1514,6 @@ init_creds_step_reply(krb5_context context,
             code = restart_init_creds_loop(context, ctx, FALSE);
         } 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(ctx);
             krb5_free_pa_data(context, ctx->preauth_to_use);
             ctx->preauth_to_use = ctx->err_padata;
             ctx->err_padata = NULL;
@@ -1565,7 +1563,6 @@ init_creds_step_reply(krb5_context context,
         goto cleanup;
 
     /* process any preauth data in the as_reply */
-    k5_reset_preauth_types_tried(ctx);
     code = krb5int_fast_process_response(context, ctx->fast_state,
                                          ctx->reply, &strengthen_key);
     if (code != 0)
diff --git a/src/lib/krb5/krb/int-proto.h b/src/lib/krb5/krb/int-proto.h
index f6f4f2f..57411a9 100644
--- a/src/lib/krb5/krb/int-proto.h
+++ b/src/lib/krb5/krb/int-proto.h
@@ -199,6 +199,9 @@ k5_free_preauth_context(krb5_context context);
 void
 k5_reset_preauth_types_tried(krb5_init_creds_context ctx);
 
+krb5_error_code
+k5_preauth_note_failed(krb5_init_creds_context ctx, krb5_preauthtype pa_type);
+
 void
 k5_preauth_prepare_request(krb5_context context, krb5_get_init_creds_opt *opt,
                            krb5_kdc_req *request);
diff --git a/src/lib/krb5/krb/preauth2.c b/src/lib/krb5/krb/preauth2.c
index 354234a..17f2133 100644
--- a/src/lib/krb5/krb/preauth2.c
+++ b/src/lib/krb5/krb/preauth2.c
@@ -54,7 +54,7 @@ struct krb5_preauth_context_st {
 
 struct krb5_preauth_req_context_st {
     krb5_context orig_context;
-    krb5_preauthtype *tried;
+    krb5_preauthtype *failed;
     krb5_clpreauth_modreq *modreqs;
 };
 
@@ -201,11 +201,7 @@ cleanup:
     free_handles(context, list);
 }
 
-/*
- * Reset the memory of which preauth types we have already tried, because we
- * are entering a new phase of padata processing (such as the padata in an
- * AS-REP).
- */
+/* Reset the memory of which preauth types we have already tried. */
 void
 k5_reset_preauth_types_tried(krb5_init_creds_context ctx)
 {
@@ -213,10 +209,27 @@ k5_reset_preauth_types_tried(krb5_init_creds_context ctx)
 
     if (reqctx == NULL)
         return;
-    free(reqctx->tried);
-    reqctx->tried = NULL;
+    free(reqctx->failed);
+    reqctx->failed = NULL;
 }
 
+/* Add pa_type to the list of types which has previously failed. */
+krb5_error_code
+k5_preauth_note_failed(krb5_init_creds_context ctx, krb5_preauthtype pa_type)
+{
+    krb5_preauth_req_context reqctx = ctx->preauth_reqctx;
+    krb5_preauthtype *newptr;
+    size_t i;
+
+    for (i = 0; reqctx->failed != NULL && reqctx->failed[i] != 0; i++);
+    newptr = realloc(reqctx->failed, (i + 2) * sizeof(*newptr));
+    if (newptr == NULL)
+        return ENOMEM;
+    reqctx->failed = newptr;
+    reqctx->failed[i] = pa_type;
+    reqctx->failed[i + 1] = 0;
+    return 0;
+}
 
 /* Free the per-krb5_context preauth_context. This means clearing any
  * plugin-specific context which may have been created, and then
@@ -291,7 +304,7 @@ k5_preauth_request_context_fini(krb5_context context,
         TRACE_PREAUTH_WRONG_CONTEXT(context);
     }
     free(reqctx->modreqs);
-    free(reqctx->tried);
+    free(reqctx->failed);
     free(reqctx);
     ctx->preauth_reqctx = NULL;
 }
@@ -612,28 +625,17 @@ pa_type_allowed(krb5_init_creds_context ctx, krb5_preauthtype pa_type)
         pa_type == ctx->allowed_preauth_type;
 }
 
-/*
- * If pa_type has already been tried as a real preauth type for this
- * authentication, return true.  Otherwise ass pa_type to the list of tried
- * types and return false.
- */
+/* Return true if pa_type previously failed during this authentication. */
 static krb5_boolean
-already_tried(krb5_init_creds_context ctx, krb5_preauthtype pa_type)
+previously_failed(krb5_init_creds_context ctx, krb5_preauthtype pa_type)
 {
     krb5_preauth_req_context reqctx = ctx->preauth_reqctx;
     size_t i;
-    krb5_preauthtype *newptr;
 
-    for (i = 0; reqctx->tried != NULL && reqctx->tried[i] != 0; i++) {
-        if (reqctx->tried[i] == pa_type)
+    for (i = 0; reqctx->failed != NULL && reqctx->failed[i] != 0; i++) {
+        if (reqctx->failed[i] == pa_type)
             return TRUE;
     }
-    newptr = realloc(reqctx->tried, (i + 2) * sizeof(*newptr));
-    if (newptr == NULL)
-        return FALSE;
-    reqctx->tried = newptr;
-    reqctx->tried[i] = pa_type;
-    reqctx->tried[i + 1] = ENCTYPE_NULL;
     return FALSE;
 }
 
@@ -665,8 +667,8 @@ process_pa_data(krb5_context context, krb5_init_creds_context ctx,
             /* 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(ctx, pa->pa_type))
+            /* Don't try a real mechanism again after failure. */
+            if (real && previously_failed(ctx, pa->pa_type))
                 continue;
             mod_pa = NULL;
             ret = clpreauth_process(context, h, modreq, ctx->opt, &callbacks,
@@ -694,6 +696,12 @@ process_pa_data(krb5_context context, krb5_init_creds_context ctx,
                 /* Save the first error we get from a real preauth type. */
                 k5_save_ctx_error(context, ret, &save);
             }
+            if (real && ret) {
+                /* Don't try this mechanism again for this authentication. */
+                ret = k5_preauth_note_failed(ctx, pa->pa_type);
+                if (ret)
+                    goto cleanup;
+            }
         }
     }
 
@@ -944,9 +952,10 @@ k5_preauth_tryagain(krb5_context context, krb5_init_creds_context ctx,
     TRACE_PREAUTH_TRYAGAIN(context, h->vt.name, pa_type, ret);
     if (!ret && mod_pa == NULL)
         ret = KRB5KRB_ERR_GENERIC;
-    if (ret)
+    if (ret) {
+        k5_preauth_note_failed(ctx, pa_type);
         return ret;
-
+    }
 
     for (count = 0; mod_pa[count] != NULL; count++);
     ret = copy_cookie(context, err_padata, &mod_pa, &count);