krb5 commit: Preserve method data in get_in_tkt.c

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

    Preserve method data in get_in_tkt.c
    
    To continue after preauth failures, we need a persistent field in
    krb5_init_creds_context containing the METHOD-DATA from a
    KDC_PREAUTH_REQUIRED or KDC_PREAUTH_FAILED error.  If we overwrite
    this field with the padata in a KDC_MORE_PREAUTH_DATA_REQUIRED error,
    or conflate it with an optimistic padata list, we won't be able to
    correctly continue after a preauth failure.
    
    In krb5_init_creds_context, split the preauth_to_use field into
    optimistic_padata, method_padata, and more_padata.  Separately handle
    KDC_ERR_MORE_PREAUTH_DATA_REQUIRED in init_creds_step_request() and
    init_creds_step_reply(), and separately handle optimistic preauth in
    init_creds_step_request().  Do not call k5_preauth() if none of the
    padata lists are set.
    
    Also stop clearing ctx->err_reply when processing a
    KDC_ERR_PREAUTH_REQUIRED response.  Instead look for that error code
    in init_creds_step_request().  Eliminate the preauth_required field of
    krb5_init_creds_context as it can be inferred from whether we are
    performing optimistic preauth.
    
    ticket: 8537

 src/include/k5-trace.h            |   11 ++++++
 src/lib/krb5/krb/get_in_tkt.c     |   71 +++++++++++++++++++++++++------------
 src/lib/krb5/krb/init_creds_ctx.h |    5 ++-
 3 files changed, 62 insertions(+), 25 deletions(-)

diff --git a/src/include/k5-trace.h b/src/include/k5-trace.h
index 199527f..0a08c06 100644
--- a/src/include/k5-trace.h
+++ b/src/include/k5-trace.h
@@ -227,8 +227,19 @@ void krb5int_trace(krb5_context context, const char *fmt, ...);
     TRACE(c, "Looked up etypes in keytab: {etypes}", etypes)
 #define TRACE_INIT_CREDS_KEYTAB_LOOKUP_FAILED(c, code)          \
     TRACE(c, "Couldn't lookup etypes in keytab: {kerr}", code)
+#define TRACE_INIT_CREDS_PREAUTH(c)                     \
+    TRACE(c, "Preauthenticating using KDC method data")
 #define TRACE_INIT_CREDS_PREAUTH_DECRYPT_FAIL(c, code)                  \
     TRACE(c, "Decrypt with preauth AS key failed: {kerr}", code)
+#define TRACE_INIT_CREDS_PREAUTH_MORE(c, patype)                \
+    TRACE(c, "Continuing preauth mech {int}", (int)patype)
+#define TRACE_INIT_CREDS_PREAUTH_NONE(c)        \
+    TRACE(c, "Sending unauthenticated request")
+#define TRACE_INIT_CREDS_PREAUTH_OPTIMISTIC(c)  \
+    TRACE(c, "Attempting optimistic preauth")
+#define TRACE_INIT_CREDS_PREAUTH_TRYAGAIN(c, patype, code)              \
+    TRACE(c, "Recovering from KDC error {int} using preauth mech {int}", \
+          (int)patype, (int)code)
 #define TRACE_INIT_CREDS_RESTART_FAST(c)        \
     TRACE(c, "Restarting to upgrade to FAST")
 #define TRACE_INIT_CREDS_RESTART_PREAUTH_FAILED(c)                      \
diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c
index 99d843e..dc4943b 100644
--- a/src/lib/krb5/krb/get_in_tkt.c
+++ b/src/lib/krb5/krb/get_in_tkt.c
@@ -593,7 +593,9 @@ krb5_init_creds_free(krb5_context context,
     krb5_free_data(context, ctx->inner_request_body);
     krb5_free_data(context, ctx->encoded_previous_request);
     krb5int_fast_free_state(context, ctx->fast_state);
-    krb5_free_pa_data(context, ctx->preauth_to_use);
+    krb5_free_pa_data(context, ctx->optimistic_padata);
+    krb5_free_pa_data(context, ctx->method_padata);
+    krb5_free_pa_data(context, ctx->more_padata);
     krb5_free_data_contents(context, &ctx->salt);
     krb5_free_data_contents(context, &ctx->s2kparams);
     krb5_free_keyblock_contents(context, &ctx->as_key);
@@ -845,10 +847,13 @@ restart_init_creds_loop(krb5_context context, krb5_init_creds_context ctx,
 {
     krb5_error_code code = 0;
 
-    krb5_free_pa_data(context, ctx->preauth_to_use);
+    krb5_free_pa_data(context, ctx->optimistic_padata);
+    krb5_free_pa_data(context, ctx->method_padata);
+    krb5_free_pa_data(context, ctx->more_padata);
     krb5_free_pa_data(context, ctx->err_padata);
     krb5_free_error(context, ctx->err_reply);
-    ctx->preauth_to_use = ctx->err_padata = NULL;
+    ctx->optimistic_padata = ctx->method_padata = ctx->more_padata = NULL;
+    ctx->err_padata = NULL;
     ctx->err_reply = NULL;
     ctx->selected_preauth_type = KRB5_PADATA_NONE;
 
@@ -867,7 +872,7 @@ restart_init_creds_loop(krb5_context context, krb5_init_creds_context ctx,
     if (ctx->opt->flags & KRB5_GET_INIT_CREDS_OPT_PREAUTH_LIST) {
         code = make_preauth_list(context, ctx->opt->preauth_list,
                                  ctx->opt->preauth_list_length,
-                                 &ctx->preauth_to_use);
+                                 &ctx->optimistic_padata);
         if (code)
             goto cleanup;
     }
@@ -1319,6 +1324,7 @@ init_creds_step_request(krb5_context context,
                         krb5_data *out)
 {
     krb5_error_code code;
+    krb5_preauthtype pa_type;
 
     if (ctx->loopcount >= MAX_IN_TKT_LOOPS) {
         code = KRB5_GET_IN_TKT_LOOP;
@@ -1349,17 +1355,36 @@ init_creds_step_request(krb5_context context,
     read_cc_config_in_data(context, ctx);
     clear_cc_config_out_data(context, ctx);
 
-    if (ctx->err_reply == NULL) {
-        /* Either our first attempt, or retrying after KDC_ERR_PREAUTH_REQUIRED
-         * or KDC_ERR_MORE_PREAUTH_DATA_REQUIRED. */
-        code = k5_preauth(context, ctx, ctx->preauth_to_use,
-                          ctx->preauth_required, &ctx->request->padata,
-                          &ctx->selected_preauth_type);
+    ctx->request->padata = NULL;
+    if (ctx->optimistic_padata != NULL) {
+        /* Our first attempt, using an optimistic padata list. */
+        TRACE_INIT_CREDS_PREAUTH_OPTIMISTIC(context);
+        code = k5_preauth(context, ctx, ctx->optimistic_padata, FALSE,
+                          &ctx->request->padata, &ctx->selected_preauth_type);
+        krb5_free_pa_data(context, ctx->optimistic_padata);
+        ctx->optimistic_padata = NULL;
         if (code != 0)
             goto cleanup;
-    } else {
-        /* Retry after an error other than PREAUTH_NEEDED, using error padata
+    } if (ctx->more_padata != NULL) {
+        /* Continuing after KDC_ERR_MORE_PREAUTH_DATA_REQUIRED. */
+        TRACE_INIT_CREDS_PREAUTH_MORE(context, ctx->selected_preauth_type);
+        code = k5_preauth(context, ctx, ctx->more_padata, TRUE,
+                          &ctx->request->padata, &pa_type);
+        if (code != 0)
+            goto cleanup;
+    } else if (ctx->err_reply != NULL &&
+               ctx->err_reply->error == KDC_ERR_PREAUTH_REQUIRED) {
+        /* Continuing after KDC_ERR_PREAUTH_REQUIRED, using method data. */
+        TRACE_INIT_CREDS_PREAUTH(context);
+        code = k5_preauth(context, ctx, ctx->method_padata, TRUE,
+                          &ctx->request->padata, &ctx->selected_preauth_type);
+        if (code != 0)
+            goto cleanup;
+    } else if (ctx->err_reply != NULL) {
+        /* Retry after an error other than PREAUTH_REQUIRED, using error padata
          * to figure out what to change. */
+        TRACE_INIT_CREDS_PREAUTH_TRYAGAIN(context, ctx->err_reply->error,
+                                          ctx->selected_preauth_type);
         code = k5_preauth_tryagain(context, ctx, ctx->selected_preauth_type,
                                    ctx->err_reply, ctx->err_padata,
                                    &ctx->request->padata);
@@ -1369,6 +1394,8 @@ init_creds_step_request(krb5_context context,
             goto cleanup;
         }
     }
+    if (ctx->request->padata == NULL)
+        TRACE_INIT_CREDS_PREAUTH_NONE(context);
 
     /* Remember when we sent this request (after any preauth delay). */
     ctx->request_time = time(NULL);
@@ -1485,8 +1512,9 @@ init_creds_step_reply(krb5_context context,
         ctx->request->client->type == KRB5_NT_ENTERPRISE_PRINCIPAL;
 
     if (ctx->err_reply != NULL) {
+        krb5_free_pa_data(context, ctx->more_padata);
         krb5_free_pa_data(context, ctx->err_padata);
-        ctx->err_padata = NULL;
+        ctx->more_padata = ctx->err_padata = NULL;
         code = krb5int_fast_process_error(context, ctx->fast_state,
                                           &ctx->err_reply, &ctx->err_padata,
                                           &retry);
@@ -1512,21 +1540,18 @@ init_creds_step_reply(krb5_context context,
              * FAST upgrade. */
             ctx->restarted = FALSE;
             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) {
-            krb5_free_pa_data(context, ctx->preauth_to_use);
-            ctx->preauth_to_use = ctx->err_padata;
+        } else if (reply_code == KDC_ERR_PREAUTH_REQUIRED && retry) {
+            krb5_free_pa_data(context, ctx->method_padata);
+            ctx->method_padata = ctx->err_padata;
             ctx->err_padata = NULL;
             note_req_timestamp(context, ctx, ctx->err_reply->stime,
                                ctx->err_reply->susec);
-            /* This will trigger a new call to k5_preauth(). */
-            krb5_free_error(context, ctx->err_reply);
-            ctx->err_reply = NULL;
             code = sort_krb5_padata_sequence(context,
                                              &ctx->request->client->realm,
-                                             ctx->preauth_to_use);
-            ctx->preauth_required = TRUE;
-
+                                             ctx->method_padata);
+        } else if (reply_code == KDC_ERR_MORE_PREAUTH_DATA_REQUIRED && retry) {
+            ctx->more_padata = ctx->err_padata;
+            ctx->err_padata = NULL;
         } else if (canon_flag && is_referral(context, ctx->err_reply,
                                              ctx->request->client)) {
             TRACE_INIT_CREDS_REFERRAL(context, &ctx->err_reply->client->realm);
diff --git a/src/lib/krb5/krb/init_creds_ctx.h b/src/lib/krb5/krb/init_creds_ctx.h
index 8c8b749..fe76968 100644
--- a/src/lib/krb5/krb/init_creds_ctx.h
+++ b/src/lib/krb5/krb/init_creds_ctx.h
@@ -50,7 +50,9 @@ struct _krb5_init_creds_context {
     krb5_data *inner_request_body; /**< For preauth */
     krb5_data *encoded_previous_request;
     struct krb5int_fast_request_state *fast_state;
-    krb5_pa_data **preauth_to_use;
+    krb5_pa_data **optimistic_padata; /* from gic options */
+    krb5_pa_data **method_padata; /* from PREAUTH_REQUIRED or PREAUTH_FAILED */
+    krb5_pa_data **more_padata; /* from MORE_PREAUTH_DATA_REQUIRED */
     krb5_boolean default_salt;
     krb5_data salt;
     krb5_data s2kparams;
@@ -58,7 +60,6 @@ struct _krb5_init_creds_context {
     krb5_enctype etype;
     krb5_boolean enc_pa_rep_permitted;
     krb5_boolean restarted;
-    krb5_boolean preauth_required;
     struct krb5_responder_context_st rctx;
     krb5_preauthtype selected_preauth_type;
     krb5_preauthtype allowed_preauth_type;
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.