Blob Blame History Raw
From 5b52d5b4b1e65699dac6a53f0b6dbe545af4f689 Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
Date: Fri, 13 Jan 2017 15:35:48 -0500
Subject: [PATCH] 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
(cherry picked from commit 97a9b0c4ef3fc7b20e6ae592201bcb132d58bbe5)
---
 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 814da3195..e60ee0b75 100644
--- a/src/include/k5-trace.h
+++ b/src/include/k5-trace.h
@@ -213,8 +213,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 bc903b6e9..8c7919e65 100644
--- a/src/lib/krb5/krb/get_in_tkt.c
+++ b/src/lib/krb5/krb/get_in_tkt.c
@@ -575,7 +575,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);
@@ -827,10 +829,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;
 
@@ -849,7 +854,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;
     }
@@ -1301,6 +1306,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;
@@ -1331,17 +1337,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);
@@ -1351,6 +1376,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);
@@ -1467,8 +1494,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);
@@ -1494,21 +1522,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 8c8b7494b..fe769685b 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;