Add replace_reply_key kdcpreauth callback

Provide an explicit way for kdcpreauth modules to replace the reply
key, and internally track when the reply key is fully replaced (as
opposed to strengthened by replacing it with a derivative of the
client long-term key).  Use this facility in the FAST OTP, PKINIT, and
SPAKE kdcpreauth modules.

ticket: 9049 (new)

7 files changed, 92 insertions, 81 deletions

diff --git a/src/include/krb5/kdcpreauth_plugin.h b/src/include/krb5/kdcpreauth_plugin.h
index b0daae1..f40e368 100644
--- a/src/include/krb5/kdcpreauth_plugin.h
+++ b/src/include/krb5/kdcpreauth_plugin.h
@@ -182,12 +182,12 @@ typedef struct krb5_kdcpreauth_callbacks_st {
     /* End of version 2 kdcpreauth callbacks. */
 
     /*
-     * Get the decrypted client long-term key chosen according to the request
-     * enctype list, or NULL if no matching key was found.  The returned
-     * pointer is an alias and should not be freed.  If invoked from
-     * return_padata, the result will be the same as the encrypting_key
-     * parameter if it is not NULL, and will therefore reflect the modified
-     * reply key if a return_padata handler has replaced the reply key.
+     * Get the current reply key.  Initially the reply key is the decrypted
+     * client long-term key chosen according to the request enctype list, or
+     * NULL if no matching key was found.  The value may be changed by the
+     * replace_reply_key callback or a return_padata method modifying
+     * encrypting_key.  The returned pointer is an alias and should not be
+     * freed.
      */
     const krb5_keyblock *(*client_keyblock)(krb5_context context,
                                             krb5_kdcpreauth_rock rock);
@@ -257,6 +257,20 @@ typedef struct krb5_kdcpreauth_callbacks_st {
 
     /* End of version 5 kdcpreauth callbacks. */
 
+    /*
+     * Replace the reply key with key.  If is_strengthen is true, key must be a
+     * derivative of the client long-term key.  This callback may be invoked
+     * from the verify or return_padata methods.  If it is invoked from the
+     * verify method, the new key will appear as the encrypting_key input to
+     * return_padata.
+     */
+    krb5_error_code (*replace_reply_key)(krb5_context context,
+                                         krb5_kdcpreauth_rock rock,
+                                         const krb5_keyblock *key,
+                                         krb5_boolean is_strengthen);
+
+    /* End of version 6 kdcpreauth callbacks. */
+
 } *krb5_kdcpreauth_callbacks;
 
 /* Optional: preauth plugin initialization function. */
@@ -350,7 +364,8 @@ typedef void
 /*
  * Optional: generate preauthentication response data to send to the client as
  * part of the AS-REP.  If it needs to override the key which is used to
- * encrypt the response, it can do so.
+ * encrypt the response, it can do so by modifying encrypting_key, but it is
+ * preferrable to use the replace_reply_key callback.
  */
 typedef krb5_error_code
 (*krb5_kdcpreauth_return_fn)(krb5_context context,
diff --git a/src/kdc/do_as_req.c b/src/kdc/do_as_req.c
index 5e966de..34723fa 100644
--- a/src/kdc/do_as_req.c
+++ b/src/kdc/do_as_req.c
@@ -743,10 +743,9 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt,
         state->status = "DECRYPT_CLIENT_KEY";
         goto errout;
     }
-    if (state->client_key != NULL) {
+    if (state->client_key != NULL)
         state->rock.client_key = state->client_key;
-        state->rock.client_keyblock = &state->client_keyblock;
-    }
+    state->rock.client_keyblock = &state->client_keyblock;
 
     errcode = kdc_fast_read_cookie(kdc_context, state->rstate, state->request,
                                    state->local_tgt, &state->local_tgt_key);
diff --git a/src/kdc/kdc_preauth.c b/src/kdc/kdc_preauth.c
index 8d83274..e132390 100644
--- a/src/kdc/kdc_preauth.c
+++ b/src/kdc/kdc_preauth.c
@@ -449,6 +449,8 @@ have_client_keys(krb5_context context, krb5_kdcpreauth_rock rock)
 static const krb5_keyblock *
 client_keyblock(krb5_context context, krb5_kdcpreauth_rock rock)
 {
+    if (rock->client_keyblock->enctype == ENCTYPE_NULL)
+        return NULL;
     return rock->client_keyblock;
 }
 
@@ -562,8 +564,23 @@ cleanup:
     return valid ? 0 : KRB5KDC_ERR_PREAUTH_EXPIRED;
 }
 
+static krb5_error_code
+replace_reply_key(krb5_context context, krb5_kdcpreauth_rock rock,
+                  const krb5_keyblock *key, krb5_boolean is_strengthen)
+{
+    krb5_keyblock copy;
+
+    if (krb5_copy_keyblock_contents(context, key, &copy) != 0)
+        return ENOMEM;
+    krb5_free_keyblock_contents(context, rock->client_keyblock);
+    *rock->client_keyblock = copy;
+    if (!is_strengthen)
+        rock->replaced_reply_key = TRUE;
+    return 0;
+}
+
 static struct krb5_kdcpreauth_callbacks_st callbacks = {
-    5,
+    6,
     max_time_skew,
     client_keys,
     free_keys,
@@ -581,7 +598,8 @@ static struct krb5_kdcpreauth_callbacks_st callbacks = {
     match_client,
     client_name,
     send_freshness_token,
-    check_freshness_token
+    check_freshness_token,
+    replace_reply_key
 };
 
 static krb5_error_code
diff --git a/src/kdc/kdc_util.h b/src/kdc/kdc_util.h
index 45683ec..ded2912 100644
--- a/src/kdc/kdc_util.h
+++ b/src/kdc/kdc_util.h
@@ -447,6 +447,7 @@ struct krb5_kdcpreauth_rock_st {
     verto_ctx *vctx;
     krb5_data ***auth_indicators;
     krb5_boolean send_freshness_token;
+    krb5_boolean replaced_reply_key;
 };
 
 #define isflagset(flagfield, flag) (flagfield & (flag))
diff --git a/src/plugins/preauth/otp/main.c b/src/plugins/preauth/otp/main.c
index a1b6816..119714f 100644
--- a/src/plugins/preauth/otp/main.c
+++ b/src/plugins/preauth/otp/main.c
@@ -158,19 +158,36 @@ on_response(void *data, krb5_error_code retval, otp_response response,
             char *const *indicators)
 {
     struct request_state rs = *(struct request_state *)data;
+    krb5_context context = rs.context;
+    krb5_keyblock *armor_key;
     char *const *ind;
 
     free(data);
 
     if (retval == 0 && response != otp_response_success)
         retval = KRB5_PREAUTH_FAILED;
+    if (retval)
+        goto done;
 
-    if (retval == 0)
-        rs.enc_tkt_reply->flags |= TKT_FLG_PRE_AUTH;
+    rs.enc_tkt_reply->flags |= TKT_FLG_PRE_AUTH;
+    armor_key = rs.preauth_cb->fast_armor(context, rs.rock);
+    if (armor_key == NULL) {
+        retval = ENOENT;
+        goto done;
+    }
 
-    for (ind = indicators; ind != NULL && *ind != NULL && retval == 0; ind++)
-        retval = rs.preauth_cb->add_auth_indicator(rs.context, rs.rock, *ind);
+    retval = rs.preauth_cb->replace_reply_key(context, rs.rock, armor_key,
+                                              FALSE);
+    if (retval)
+        goto done;
 
+    for (ind = indicators; ind != NULL && *ind != NULL; ind++) {
+        retval = rs.preauth_cb->add_auth_indicator(context, rs.rock, *ind);
+        if (retval)
+            goto done;
+    }
+
+done:
     rs.respond(rs.arg, retval, NULL, NULL, NULL);
 }
 
@@ -343,31 +360,6 @@ error:
     (*respond)(arg, retval, NULL, NULL, NULL);
 }
 
-static krb5_error_code
-otp_return_padata(krb5_context context, krb5_pa_data *padata,
-                  krb5_data *req_pkt, krb5_kdc_req *request,
-                  krb5_kdc_rep *reply, krb5_keyblock *encrypting_key,
-                  krb5_pa_data **send_pa_out, krb5_kdcpreauth_callbacks cb,
-                  krb5_kdcpreauth_rock rock, krb5_kdcpreauth_moddata moddata,
-                  krb5_kdcpreauth_modreq modreq)
-{
-    krb5_keyblock *armor_key = NULL;
-
-    if (padata->length == 0)
-        return 0;
-
-    /* Get the armor key. */
-    armor_key = cb->fast_armor(context, rock);
-    if (!armor_key) {
-      com_err("otp", ENOENT, "No armor key found when returning padata");
-      return ENOENT;
-    }
-
-    /* Replace the reply key with the FAST armor key. */
-    krb5_free_keyblock_contents(context, encrypting_key);
-    return krb5_copy_keyblock_contents(context, armor_key, encrypting_key);
-}
-
 krb5_error_code
 kdcpreauth_otp_initvt(krb5_context context, int maj_ver, int min_ver,
                       krb5_plugin_vtable vtable);
@@ -389,7 +381,6 @@ kdcpreauth_otp_initvt(krb5_context context, int maj_ver, int min_ver,
     vt->flags = otp_flags;
     vt->edata = otp_edata;
     vt->verify = otp_verify;
-    vt->return_padata = otp_return_padata;
 
     com_err("otp", 0, "Loaded");
 
diff --git a/src/plugins/preauth/pkinit/pkinit_srv.c b/src/plugins/preauth/pkinit/pkinit_srv.c
index 81e9656..1147a8f 100644
--- a/src/plugins/preauth/pkinit/pkinit_srv.c
+++ b/src/plugins/preauth/pkinit/pkinit_srv.c
@@ -763,6 +763,7 @@ pkinit_server_return_padata(krb5_context context,
 
     unsigned char *dh_pubkey = NULL, *server_key = NULL;
     unsigned int server_key_len = 0, dh_pubkey_len = 0;
+    krb5_keyblock reply_key = { 0 };
 
     krb5_kdc_dh_key_info dhkey_info;
     krb5_data *encoded_dhkey_info = NULL;
@@ -800,12 +801,6 @@ pkinit_server_return_padata(krb5_context context,
     TRACE_PKINIT_SERVER_RETURN_PADATA(context);
     reqctx = (pkinit_kdc_req_context)modreq;
 
-    if (encrypting_key->contents) {
-        free(encrypting_key->contents);
-        encrypting_key->length = 0;
-        encrypting_key->contents = NULL;
-    }
-
     for(i = 0; i < request->nktypes; i++) {
         enctype = request->ktype[i];
         if (!krb5_c_valid_enctype(enctype))
@@ -883,19 +878,19 @@ pkinit_server_return_padata(krb5_context context,
     } else {
         pkiDebug("received RSA key delivery AS REQ\n");
 
-        retval = krb5_c_make_random_key(context, enctype, encrypting_key);
-        if (retval) {
-            pkiDebug("unable to make a session key\n");
-            goto cleanup;
-        }
-
         init_krb5_reply_key_pack(&key_pack);
         if (key_pack == NULL) {
             retval = ENOMEM;
             goto cleanup;
         }
 
-        retval = krb5_c_make_checksum(context, 0, encrypting_key,
+        retval = krb5_c_make_random_key(context, enctype, &key_pack->replyKey);
+        if (retval) {
+            pkiDebug("unable to make a session key\n");
+            goto cleanup;
+        }
+
+        retval = krb5_c_make_checksum(context, 0, &key_pack->replyKey,
                                       KRB5_KEYUSAGE_TGS_REQ_AUTH_CKSUM,
                                       req_pkt, &key_pack->asChecksum);
         if (retval) {
@@ -908,13 +903,10 @@ pkinit_server_return_padata(krb5_context context,
         pkiDebug("checksum size = %d\n", key_pack->asChecksum.length);
         print_buffer(key_pack->asChecksum.contents,
                      key_pack->asChecksum.length);
-        pkiDebug("encrypting key (%d)\n", encrypting_key->length);
-        print_buffer(encrypting_key->contents, encrypting_key->length);
+        pkiDebug("encrypting key (%d)\n", key_pack->replyKey.length);
+        print_buffer(key_pack->replyKey.contents, key_pack->replyKey.length);
 #endif
 
-        krb5_copy_keyblock_contents(context, encrypting_key,
-                                    &key_pack->replyKey);
-
         retval = k5int_encode_krb5_reply_key_pack(key_pack,
                                                   &encoded_key_pack);
         if (retval) {
@@ -944,6 +936,11 @@ pkinit_server_return_padata(krb5_context context,
         print_buffer_bin(rep->u.encKeyPack.data, rep->u.encKeyPack.length,
                          "/tmp/kdc_enc_key_pack");
 #endif
+
+        retval = cb->replace_reply_key(context, rock, &key_pack->replyKey,
+                                       FALSE);
+        if (retval)
+            goto cleanup;
     }
 
     if (rep->choice == choice_pa_pk_as_rep_dhInfo &&
@@ -989,7 +986,7 @@ pkinit_server_return_padata(krb5_context context,
                                             rep->u.dh_Info.kdfID,
                                             request->client, request->server,
                                             enctype, req_pkt, out_data,
-                                            encrypting_key);
+                                            &reply_key);
             if (retval) {
                 pkiDebug("pkinit_alg_agility_kdf failed: %s\n",
                          error_message(retval));
@@ -999,13 +996,16 @@ pkinit_server_return_padata(krb5_context context,
             /* Otherwise, use the older octetstring2key() function */
         } else {
             retval = pkinit_octetstring2key(context, enctype, server_key,
-                                            server_key_len, encrypting_key);
+                                            server_key_len, &reply_key);
             if (retval) {
                 pkiDebug("pkinit_octetstring2key failed: %s\n",
                          error_message(retval));
                 goto cleanup;
             }
         }
+        retval = cb->replace_reply_key(context, rock, &reply_key, FALSE);
+        if (retval)
+            goto cleanup;
     }
 
     *send_pa = malloc(sizeof(krb5_pa_data));
@@ -1034,6 +1034,7 @@ cleanup:
     free_krb5_pa_pk_as_req(&reqp);
     free_krb5_pa_pk_as_rep(&rep);
     free_krb5_reply_key_pack(&key_pack);
+    krb5_free_keyblock_contents(context, &reply_key);
 
     if (retval)
         pkiDebug("pkinit_verify_padata failure");
diff --git a/src/plugins/preauth/spake/spake_kdc.c b/src/plugins/preauth/spake/spake_kdc.c
index 88c964c..95cdb55 100644
--- a/src/plugins/preauth/spake/spake_kdc.c
+++ b/src/plugins/preauth/spake/spake_kdc.c
@@ -458,6 +458,10 @@ verify_response(krb5_context context, groupstate *gstate,
 
     ret = derive_key(context, gstate, group, ikey, &wbytes, &spakeresult,
                      &thash, der_req, 0, &reply_key);
+    if (ret)
+        goto cleanup;
+
+    ret = cb->replace_reply_key(context, rock, reply_key, TRUE);
 
 cleanup:
     zapfree(wbytes.data, wbytes.length);
@@ -466,7 +470,7 @@ cleanup:
     krb5_free_data_contents(context, &thash);
     krb5_free_keyblock(context, k1);
     k5_free_spake_factor(context, factor);
-    (*respond)(arg, ret, (krb5_kdcpreauth_modreq)reply_key, NULL, NULL);
+    (*respond)(arg, ret, NULL, NULL, NULL);
 }
 
 /*
@@ -533,23 +537,6 @@ spake_verify(krb5_context context, krb5_data *req_pkt, krb5_kdc_req *request,
     k5_free_pa_spake(context, pa_spake);
 }
 
-/* If a key was set in the per-request module data, replace the reply key.  Do
- * not generate any pa-data to include with the KDC reply. */
-static krb5_error_code
-spake_return(krb5_context context, krb5_pa_data *padata, krb5_data *req_pkt,
-             krb5_kdc_req *request, krb5_kdc_rep *reply,
-             krb5_keyblock *encrypting_key, krb5_pa_data **send_pa_out,
-             krb5_kdcpreauth_callbacks cb, krb5_kdcpreauth_rock rock,
-             krb5_kdcpreauth_moddata moddata, krb5_kdcpreauth_modreq modreq)
-{
-    krb5_keyblock *reply_key = (krb5_keyblock *)modreq;
-
-    if (reply_key == NULL)
-        return 0;
-    krb5_free_keyblock_contents(context, encrypting_key);
-    return krb5_copy_keyblock_contents(context, reply_key, encrypting_key);
-}
-
 /* Release a per-request module data object. */
 static void
 spake_free_modreq(krb5_context context, krb5_kdcpreauth_moddata moddata,
@@ -578,7 +565,6 @@ kdcpreauth_spake_initvt(krb5_context context, int maj_ver, int min_ver,
     vt->fini = spake_fini;
     vt->edata = spake_edata;
     vt->verify = spake_verify;
-    vt->return_padata = spake_return;
     vt->free_modreq = spake_free_modreq;
     return 0;
 }