krb5 commit: Simplify k5_preauth_tryagain()

Greg Hudson <[email protected]>
Newsgroups gmane.comp.encryption.kerberos.cvs
Message-ID <[email protected]>
https://github.com/krb5/krb5/commit/27628e5d9d5e6fcfa73276106edbd8149d134dc0
commit 27628e5d9d5e6fcfa73276106edbd8149d134dc0
Author: Greg Hudson <[email protected]>
Date:   Fri Jan 13 20:45:48 2017 -0500

    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

 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 f2075ad..199527f 100644
--- a/src/include/k5-trace.h
+++ b/src/include/k5-trace.h
@@ -301,8 +301,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 4150d60..b3cea6b 100644
--- a/src/lib/krb5/krb/get_in_tkt.c
+++ b/src/lib/krb5/krb/get_in_tkt.c
@@ -1358,17 +1358,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;
@@ -1553,10 +1547,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 40955ba..f6f4f2f 100644
--- a/src/lib/krb5/krb/int-proto.h
+++ b/src/lib/krb5/krb/int-proto.h
@@ -187,7 +187,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 cfe3dd5..354234a 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);
+
+    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 (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;
-        }
+
+    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
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.