Patch 9/9: process_tgs_req() must release encrypting_key

[Alexandr Nedvedicky]

Hello,

I'm upgrading kerberos bundled with Solaris to krb5-1.16. Solaris currently
ships krb5-1.15.1. I've noticed there are some memory leaks, while running test
suite, which comes with krb-1.16 (e.g. running 'make check').  I don't think
those memory leaks are critical, though as kerberos newbie I can't be sure, so
I think I'm better to share my findings. All memory leaks were found using
'libumem', which can be found on Solaris (or its OSS sibbling illumos).
All patches are against krb5-1.16 release.

There are two ways how db_tgs_req() obtains encrypting_key:

 539     if (isflagset(request->kdc_options, KDC_OPT_ENC_TKT_IN_SKEY)) {
 540         krb5_enc_tkt_part *t2enc = request->second_ticket[st_idx]->enc_part2;
 541         encrypting_key = *(t2enc->session);
 542     } else {
 543         /*
 544          * Find the server key
 545          */
 546         if ((errcode = krb5_dbe_find_enctype(kdc_context, server,
 547                                              -1, /* ignore keytype */
 548                                              -1, /* Ignore salttype */
 549                                              0,  /* Get highest kvno */
 550                                              &server_key))) {
 551             status = "FINDING_SERVER_KEY";
 552             goto cleanup;
 553         }
 554 
 555         /*
 556          * Convert server.key into a real key
 557          * (it may be encrypted in the database)
 558          */
 559         if ((errcode = krb5_dbe_decrypt_key_data(kdc_context, NULL,
 560                                                  server_key, &encrypting_key,
 561                                                  NULL))) {
 562             status = "DECRYPT_SERVER_KEY";
 563             goto cleanup;
 564         }

it either reads it from request (lines 540-541) or gets it via call
to krb5_dbe_decrypt_key_data() at line 559. If function executes
line 559, then it must also make sure the encrypting_key is freed.

regards
sasha

--------8<---------------8<---------------8<------------------8<--------
diff --git a/src/kdc/do_tgs_req.c b/src/kdc/do_tgs_req.c
index 293791814..b845e756b 100644
--- a/src/kdc/do_tgs_req.c
+++ b/src/kdc/do_tgs_req.c
@@ -114,6 +114,7 @@ process_tgs_req(struct server_handle *handle, krb5_data *pkt,
     int newtransited = 0;
     krb5_error_code retval = 0;
     krb5_keyblock encrypting_key;
+    krb5_keyblock *cleanup_encrypting_key;
     krb5_timestamp kdc_time, authtime = 0;
     krb5_keyblock session_key;
     krb5_keyblock *reply_key = NULL;
@@ -146,6 +147,7 @@ process_tgs_req(struct server_handle *handle, krb5_data *pkt,
     memset(&enc_tkt_reply, 0, sizeof(enc_tkt_reply));
     memset(&encrypting_key, 0, sizeof(krb5_keyblock));
     memset(&session_key, 0, sizeof(krb5_keyblock));
+    cleanup_encrypting_key = NULL;
 
     retval = decode_krb5_tgs_req(pkt, &request);
     if (retval)
@@ -554,13 +556,13 @@ process_tgs_req(struct server_handle *handle, krb5_data *pkt,
          * Convert server.key into a real key
          * (it may be encrypted in the database)
          */
-	memset(&encrypting_key, 0, sizeof (encrypting_key));
         if ((errcode = krb5_dbe_decrypt_key_data(kdc_context, NULL,
                                                  server_key, &encrypting_key,
                                                  NULL))) {
             status = "DECRYPT_SERVER_KEY";
             goto cleanup;
         }
+	cleanup_encrypting_key = &encrypting_key;
     }
 
     if (isflagset(c_flags, KRB5_KDB_FLAG_CONSTRAINED_DELEGATION)) {
@@ -879,6 +881,10 @@ cleanup:
         krb5_free_pa_data(kdc_context, reply_encpart.enc_padata);
     if (enc_tkt_reply.authorization_data != NULL)
         krb5_free_authdata(kdc_context, enc_tkt_reply.authorization_data);
+    if (cleanup_encrypting_key != NULL) {
+	free(cleanup_encrypting_key->contents);
+	cleanup_encrypting_key->contents = NULL;
+    }
     krb5_free_pa_data(kdc_context, e_data);
     k5_free_data_ptr_list(auth_indicators);