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

6 files changed, 49 insertions, 59 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;
diff --git a/src/plugins/preauth/pkinit/pkinit_clnt.c b/src/plugins/preauth/pkinit/pkinit_clnt.c
index ad354cf0b..e574c3c8f 100644
--- a/src/plugins/preauth/pkinit/pkinit_clnt.c
+++ b/src/plugins/preauth/pkinit/pkinit_clnt.c
@@ -1029,15 +1029,11 @@ pkinit_client_process(krb5_context context, krb5_clpreauth_moddata moddata,
     int processing_request = 0;
     pkinit_context plgctx = (pkinit_context)moddata;
     pkinit_req_context reqctx = (pkinit_req_context)modreq;
-    krb5_keyblock *armor_key = cb->fast_armor(context, rock), as_key;
+    krb5_keyblock as_key;
 
     pkiDebug("pkinit_client_process %p %p %p %p\n",
              context, plgctx, reqctx, request);
 
-    /* Remove (along with armor_key) when FAST PKINIT is settled. */
-    /* Don't use PKINIT if also using FAST. */
-    if (armor_key != NULL)
-        return EINVAL;
 
     if (plgctx == NULL || reqctx == NULL)
         return EINVAL;
diff --git a/src/plugins/preauth/pkinit/pkinit_srv.c b/src/plugins/preauth/pkinit/pkinit_srv.c
index 44e310cf9..3322310bf 100644
--- a/src/plugins/preauth/pkinit/pkinit_srv.c
+++ b/src/plugins/preauth/pkinit/pkinit_srv.c
@@ -107,16 +107,9 @@ pkinit_server_get_edata(krb5_context context,
 {
     krb5_error_code retval = 0;
     pkinit_kdc_context plgctx = NULL;
-    krb5_keyblock *armor_key = cb->fast_armor(context, rock);
 
     pkiDebug("pkinit_server_get_edata: entered!\n");
 
-    /* Remove (along with armor_key) when FAST PKINIT is settled. */
-    /* Don't advertise PKINIT if the client used FAST. */
-    if (armor_key != NULL) {
-        (*respond)(arg, EINVAL, NULL);
-        return;
-    }
 
     /*
      * If we don't have a realm context for the given realm,
@@ -309,7 +302,6 @@ pkinit_server_verify_padata(krb5_context context,
     krb5_kdc_req *tmp_as_req = NULL;
     krb5_data k5data;
     int is_signed = 1;
-    krb5_keyblock *armor_key = cb->fast_armor(context, rock);
     krb5_pa_data **e_data = NULL;
     krb5_kdcpreauth_modreq modreq = NULL;
 
@@ -319,12 +311,6 @@ pkinit_server_verify_padata(krb5_context context,
         return;
     }
 
-    /* Remove (along with armor_key) when FAST PKINIT is settled. */
-    /* Don't allow PKINIT if the client used FAST. */
-    if (armor_key != NULL) {
-        (*respond)(arg, EINVAL, NULL, NULL, NULL);
-        return;
-    }
 
     if (moddata == NULL) {
         (*respond)(arg, EINVAL, NULL, NULL, NULL);
@@ -462,23 +448,7 @@ pkinit_server_verify_padata(krb5_context context,
                                      "value not supported."));
             goto cleanup;
         }
-        /*
-         * The KDC may have modified the request after decoding it.
-         * We need to compute the checksum on the data that
-         * came from the client.  Therefore, we use the original
-         * packet contents.
-         */
-        retval = k5int_decode_krb5_as_req(req_pkt, &tmp_as_req);
-        if (retval) {
-            pkiDebug("decode_krb5_as_req returned %d\n", (int)retval);
-            goto cleanup;
-        }
-
-        retval = k5int_encode_krb5_kdc_req_body(tmp_as_req, &der_req);
-        if (retval) {
-            pkiDebug("encode_krb5_kdc_req_body returned %d\n", (int) retval);
-            goto cleanup;
-        }
+        der_req = cb->request_body(context, rock);
         retval = krb5_c_make_checksum(context, CKSUMTYPE_NIST_SHA, NULL,
                                       0, der_req, &cksum);
         if (retval) {
@@ -566,8 +536,6 @@ cleanup:
     case KRB5_PADATA_PK_AS_REQ:
         free_krb5_pa_pk_as_req(&reqp);
         free(cksum.contents);
-        if (der_req != NULL)
-            krb5_free_data(context, der_req);
         break;
     case KRB5_PADATA_PK_AS_REP_OLD:
     case KRB5_PADATA_PK_AS_REQ_OLD:
diff --git a/src/tests/t_anonpkinit.py b/src/tests/t_anonpkinit.py
index 5b2368e12..7ae955d17 100644
--- a/src/tests/t_anonpkinit.py
+++ b/src/tests/t_anonpkinit.py
@@ -42,6 +42,8 @@ realm = K5Realm(krb5_conf=pkinit_krb5_conf, kdc_conf=restrictive_kdc_conf,
                 create_user=False)
 realm.addprinc('WELLKNOWN/ANONYMOUS')
 realm.kinit('@%s' % realm.realm, flags=['-n'])
+# now try FAST
+realm.kinit('@%s' % realm.realm, flags=['-n', '-T', realm.ccache])
 realm.run_as_client([kvno, realm.host_princ], expected_code=1)
 
 success('Anonymous PKINIT')