ticket: new

    subject: FAST PKINIT
    target_version: 1.10
    tags: pullup

    Per RFC 6113 fast should use the inner request body for the pkinit
    checksum. We did that on the KDC; now do so on the client.  Remove
    code that explicitly blocked pkinit under FAST.

    Also, use the reply key *before* the strengthen key is applied when
    verifying the PADATA_PKINIT_KX.

    Add FAST pkinit test.

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@25486 dc483132-0cff-0310-8789-dd5450dbe970

3 files changed, 45 insertions, 21 deletions

diff --git a/src/lib/krb5/krb/fast.c b/src/lib/krb5/krb/fast.c
index a0db841d5..e5eb960be 100644
--- a/src/lib/krb5/krb/fast.c
+++ b/src/lib/krb5/krb/fast.c
@@ -207,8 +207,6 @@ krb5int_fast_prep_req(krb5_context context,
     krb5_data *encoded_fast_req = NULL;
     krb5_data *encoded_armored_req = NULL;
     krb5_data *local_encoded_result = NULL;
-    krb5_data random_data;
-    char random_buf[4];
 
     assert(state != NULL);
     assert(state->fast_outer_request.padata == NULL);
@@ -218,14 +216,7 @@ krb5int_fast_prep_req(krb5_context context,
     }
 
     TRACE_FAST_ENCODE(context);
-    /* Fill in a fresh random nonce for each inner request*/
-    random_data.length = 4;
-    random_data.data = (char *)random_buf;
-    retval = krb5_c_random_make_octets(context, &random_data);
-    if (retval == 0) {
-        request->nonce = 0x7fffffff & load_32_n(random_buf);
-        state->nonce = request->nonce;
-    }
+    state->nonce = request->nonce;
     fast_req.req_body =  request;
     if (fast_req.req_body->padata == NULL) {
         fast_req.req_body->padata = calloc(1, sizeof(krb5_pa_data *));
diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c
index 4237915d8..8351dfd30 100644
--- a/src/lib/krb5/krb/get_in_tkt.c
+++ b/src/lib/krb5/krb/get_in_tkt.c
@@ -530,7 +530,8 @@ krb5_init_creds_free(krb5_context context,
     krb5_free_cred_contents(context, &ctx->cred);
     krb5_free_kdc_req(context, ctx->request);
     krb5_free_kdc_rep(context, ctx->reply);
-    krb5_free_data(context, ctx->encoded_request_body);
+    krb5_free_data(context, ctx->outer_request_body);
+    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);
@@ -708,9 +709,9 @@ restart_init_creds_loop(krb5_context context, krb5_init_creds_context ctx,
         goto cleanup;
     ctx->preauth_rock.fast_state = ctx->fast_state;
     krb5_preauth_request_context_init(context);
-    if (ctx->encoded_request_body) {
-        krb5_free_data(context, ctx->encoded_request_body);
-        ctx->encoded_request_body = NULL;
+    if (ctx->outer_request_body) {
+        krb5_free_data(context, ctx->outer_request_body);
+        ctx->outer_request_body = NULL;
     }
     if (ctx->opte &&
         (ctx->opte->flags & KRB5_GET_INIT_CREDS_OPT_PREAUTH_LIST)) {
@@ -775,7 +776,7 @@ restart_init_creds_loop(krb5_context context, krb5_init_creds_context ctx,
         ctx->request->rtime = 0;
     code = krb5int_fast_prep_req_body(context, ctx->fast_state,
                                       ctx->request,
-                                      &ctx->encoded_request_body);
+                                      &ctx->outer_request_body);
     if (code != 0)
         goto cleanup;
 cleanup:
@@ -1104,17 +1105,39 @@ init_creds_step_request(krb5_context context,
 {
     krb5_error_code code;
     krb5_boolean got_real;
+    char random_buf[4];
+    krb5_data random_data;
 
     if (ctx->loopcount >= MAX_IN_TKT_LOOPS) {
         code = KRB5_GET_IN_TKT_LOOP;
         goto cleanup;
     }
+    /*
+     * RFC 6113 requires a new nonce for the inner request on each try. It's
+     * permitted to change the nonce even for non-FAST so we do here.
+     */
+    random_data.length = 4;
+    random_data.data = (char *)random_buf;
+    code = krb5_c_random_make_octets(context, &random_data);
+    if (code !=0)
+        goto cleanup;
+    /*
+     * See RT ticket 3196 at MIT.  If we set the high bit, we may have
+     * compatibility problems with Heimdal, because we (incorrectly) encode
+     * this value as signed.
+     */
+    ctx->request->nonce = 0x7fffffff & load_32_n(random_buf);
+    krb5_free_data(context, ctx->inner_request_body);
+    ctx->inner_request_body = NULL;
+    code = encode_krb5_kdc_req_body(ctx->request, &ctx->inner_request_body);
+    if (code)
+        goto cleanup;
 
     if (ctx->err_reply == NULL) {
         /* either our first attempt, or retrying after PREAUTH_NEEDED */
         code = krb5_do_preauth(context,
                                ctx->request,
-                               ctx->encoded_request_body,
+                               ctx->inner_request_body,
                                ctx->encoded_previous_request,
                                ctx->preauth_to_use,
                                &ctx->request->padata,
@@ -1135,7 +1158,7 @@ init_creds_step_request(krb5_context context,
              */
             code = krb5_do_preauth_tryagain(context,
                                             ctx->request,
-                                            ctx->encoded_request_body,
+                                            ctx->inner_request_body,
                                             ctx->encoded_previous_request,
                                             ctx->preauth_to_use,
                                             &ctx->request->padata,
@@ -1167,7 +1190,7 @@ init_creds_step_request(krb5_context context,
     if (code)
         goto cleanup;
     code = krb5int_fast_prep_req(context, ctx->fast_state,
-                                 ctx->request, ctx->encoded_request_body,
+                                 ctx->request, ctx->outer_request_body,
                                  encode_krb5_as_req,
                                  &ctx->encoded_previous_request);
     if (code != 0)
@@ -1366,7 +1389,7 @@ init_creds_step_reply(krb5_context context,
 
     code = krb5_do_preauth(context,
                            ctx->request,
-                           ctx->encoded_request_body,
+                           ctx->inner_request_body,
                            ctx->encoded_previous_request,
                            ctx->reply->padata,
                            &kdc_padata,
@@ -1453,7 +1476,7 @@ init_creds_step_reply(krb5_context context,
     if (code != 0)
         goto cleanup;
     code = verify_anonymous(context, ctx->request, ctx->reply,
-                            &encrypting_key);
+                            &ctx->as_key);
     if (code)
         goto cleanup;
 
diff --git a/src/lib/krb5/krb/init_creds_ctx.h b/src/lib/krb5/krb/init_creds_ctx.h
index 604f3f89a..48376fccd 100644
--- a/src/lib/krb5/krb/init_creds_ctx.h
+++ b/src/lib/krb5/krb/init_creds_ctx.h
@@ -22,7 +22,17 @@ struct _krb5_init_creds_context {
     krb5_creds cred;
     krb5_kdc_req *request;
     krb5_kdc_rep *reply;
-    krb5_data *encoded_request_body;
+    /**
+     * Stores the outer request body in order to feed into FAST for
+     * checksumming.  This is maintained even if FAST is not used. This is not
+     * used for preauth: that requires the inner request body.  For AS-only
+     * FAST it would be better for krb5int_fast_prep_req() to simply generate
+     * this.  However for TGS FAST, the client needs to supply the
+     * to_be_checksummed data. Whether this should be refactored should be
+     * revisited as TGS fast is integrated.
+     */
+    krb5_data *outer_request_body;
+    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;