Blob Blame History Raw
From e443dfe315a38607e7c9dcba219f73253d17032b Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
Date: Fri, 13 Jan 2017 20:45:48 -0500
Subject: [PATCH] Simplify k5_preauth_tryagain()

When retrying pre-authentication for an error, try only the module for
the selected preauth type, not all preauth types in the original
method data.  Pass the error and its padata to k5_preauth_tryagain()
explicitly, so that those fields of krb5_init_creds_context are only
referenced in get_in_tkt.c.  Handle a degenerate case in
init_creds_step_reply() to simplify the code in
init_creds_step_request().

ticket: 8537
(cherry picked from commit 27628e5d9d5e6fcfa73276106edbd8149d134dc0)
---
 src/include/k5-trace.h        |  7 ++--
 src/lib/krb5/krb/get_in_tkt.c | 20 ++++-------
 src/lib/krb5/krb/int-proto.h  |  3 +-
 src/lib/krb5/krb/preauth2.c   | 64 +++++++++++++++++++----------------
 4 files changed, 48 insertions(+), 46 deletions(-)

diff --git a/src/include/k5-trace.h b/src/include/k5-trace.h
index f44f162d3..814da3195 100644
--- a/src/include/k5-trace.h
+++ b/src/include/k5-trace.h
@@ -287,8 +287,11 @@ void krb5int_trace(krb5_context context, const char *fmt, ...);
 #define TRACE_PREAUTH_SKIP(c, name, patype)                           \
     TRACE(c, "Skipping previously used preauth module {str} ({int})", \
           name, (int) patype)
-#define TRACE_PREAUTH_TRYAGAIN_INPUT(c, padata)                 \
-    TRACE(c, "Preauth tryagain input types: {patypes}", padata)
+#define TRACE_PREAUTH_TRYAGAIN_INPUT(c, patype, padata)                 \
+    TRACE(c, "Preauth tryagain input types ({int}): {patypes}", patype, padata)
+#define TRACE_PREAUTH_TRYAGAIN(c, name, patype, code)                   \
+    TRACE(c, "Preauth module {str} ({int}) tryagain returned: {kerr}",  \
+          name, (int)patype, code)
 #define TRACE_PREAUTH_TRYAGAIN_OUTPUT(c, padata)                        \
     TRACE(c, "Followup preauth for next request: {patypes}", padata)
 #define TRACE_PREAUTH_WRONG_CONTEXT(c)                                  \
diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c
index da12204ac..988fca233 100644
--- a/src/lib/krb5/krb/get_in_tkt.c
+++ b/src/lib/krb5/krb/get_in_tkt.c
@@ -1340,17 +1340,11 @@ init_creds_step_request(krb5_context context,
         if (code != 0)
             goto cleanup;
     } else {
-        if (ctx->preauth_to_use != NULL) {
-            /*
-             * Retry after an error other than PREAUTH_NEEDED,
-             * using ctx->err_padata to figure out what to change.
-             */
-            code = k5_preauth_tryagain(context, ctx, ctx->preauth_to_use,
-                                       &ctx->request->padata);
-        } else {
-            /* No preauth supplied, so can't query the plugins. */
-            code = KRB5KRB_ERR_GENERIC;
-        }
+        /* Retry after an error other than PREAUTH_NEEDED, using error padata
+         * to figure out what to change. */
+        code = k5_preauth_tryagain(context, ctx, ctx->selected_preauth_type,
+                                   ctx->err_reply, ctx->err_padata,
+                                   &ctx->request->padata);
         if (code != 0) {
             /* couldn't come up with anything better */
             code = ctx->err_reply->error + ERROR_TABLE_BASE_krb5;
@@ -1535,10 +1529,10 @@ init_creds_step_reply(krb5_context context,
             ctx->enc_pa_rep_permitted = TRUE;
             code = restart_init_creds_loop(context, ctx, FALSE);
         } else {
-            if (retry) {
+            if (retry && ctx->selected_preauth_type != KRB5_PADATA_NONE) {
                 code = 0;
             } else {
-                /* error + no hints = give up */
+                /* error + no hints (or no preauth mech) = give up */
                 code = (krb5_error_code)reply_code + ERROR_TABLE_BASE_krb5;
             }
         }
diff --git a/src/lib/krb5/krb/int-proto.h b/src/lib/krb5/krb/int-proto.h
index 628f0baa8..8903df232 100644
--- a/src/lib/krb5/krb/int-proto.h
+++ b/src/lib/krb5/krb/int-proto.h
@@ -185,7 +185,8 @@ k5_preauth(krb5_context context, krb5_init_creds_context ctx,
 
 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_preauthtype pa_type, krb5_error *err,
+                    krb5_pa_data **err_padata, krb5_pa_data ***padata_out);
 
 void
 k5_init_preauth_context(krb5_context context);
diff --git a/src/lib/krb5/krb/preauth2.c b/src/lib/krb5/krb/preauth2.c
index cfe3dd5b0..354234a93 100644
--- a/src/lib/krb5/krb/preauth2.c
+++ b/src/lib/krb5/krb/preauth2.c
@@ -911,49 +911,53 @@ add_s4u_x509_user_padata(krb5_context context, krb5_s4u_userid *userid,
 }
 
 /*
- * If one of the modules can adjust its AS_REQ data using the contents of the
- * err_reply, return 0.  If it's the sort of correction which requires that we
- * ask the user another question, we let the calling application deal with it.
+ * If the module for pa_type can adjust its AS_REQ data using the contents of
+ * err and err_padata, return 0 with *padata_out set to a padata list for the
+ * next request.  If it's the sort of correction which requires that we ask the
+ * user another question, we let the calling application deal with it.
  */
 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_preauthtype pa_type, krb5_error *err,
+                    krb5_pa_data **err_padata, krb5_pa_data ***padata_out)
 {
     krb5_error_code ret;
     krb5_pa_data **mod_pa;
     krb5_clpreauth_modreq modreq;
     clpreauth_handle h;
-    int i, count;
+    int count;
 
     *padata_out = NULL;
 
-    TRACE_PREAUTH_TRYAGAIN_INPUT(context, in_padata);
+    TRACE_PREAUTH_TRYAGAIN_INPUT(context, pa_type, err_padata);
 
-    for (i = 0; in_padata[i] != NULL; i++) {
-        h = find_module(context, ctx, in_padata[i]->pa_type, &modreq);
-        if (h == NULL)
-            continue;
-        mod_pa = NULL;
-        ret = clpreauth_tryagain(context, h, modreq, ctx->opt, &callbacks,
-                                 (krb5_clpreauth_rock)ctx, ctx->request,
-                                 ctx->inner_request_body,
-                                 ctx->encoded_previous_request,
-                                 in_padata[i]->pa_type,
-                                 ctx->err_reply, ctx->err_padata,
-                                 ctx->prompter, ctx->prompter_data, &mod_pa);
-        if (ret == 0 && mod_pa != NULL) {
-            for (count = 0; mod_pa[count] != NULL; count++);
-            ret = copy_cookie(context, ctx->err_padata, &mod_pa, &count);
-            if (ret) {
-                krb5_free_pa_data(context, mod_pa);
-                return ret;
-            }
-            TRACE_PREAUTH_TRYAGAIN_OUTPUT(context, mod_pa);
-            *padata_out = mod_pa;
-            return 0;
-        }
+    h = find_module(context, ctx, pa_type, &modreq);
+    if (h == NULL)
+        return KRB5KRB_ERR_GENERIC;
+    mod_pa = NULL;
+    ret = clpreauth_tryagain(context, h, modreq, ctx->opt, &callbacks,
+                             (krb5_clpreauth_rock)ctx, ctx->request,
+                             ctx->inner_request_body,
+                             ctx->encoded_previous_request, pa_type, err,
+                             err_padata, ctx->prompter, ctx->prompter_data,
+                             &mod_pa);
+    TRACE_PREAUTH_TRYAGAIN(context, h->vt.name, pa_type, ret);
+    if (!ret && mod_pa == NULL)
+        ret = KRB5KRB_ERR_GENERIC;
+    if (ret)
+        return ret;
+
+
+    for (count = 0; mod_pa[count] != NULL; count++);
+    ret = copy_cookie(context, err_padata, &mod_pa, &count);
+    if (ret) {
+        krb5_free_pa_data(context, mod_pa);
+        return ret;
     }
-    return KRB5KRB_ERR_GENERIC;
+
+    TRACE_PREAUTH_TRYAGAIN_OUTPUT(context, mod_pa);
+    *padata_out = mod_pa;
+    return 0;
 }
 
 /* Compile the set of response items for in_padata by invoke each module's