pull up r23712, r23714 from trunk

 ------------------------------------------------------------------------
 r23714 | ghudson | 2010-02-09 20:55:36 -0500 (Tue, 09 Feb 2010) | 13 lines

 ticket: 6656

 Followon fixes to r23712:
 * A few formatting fixes.
 * Fix unlikely leak in kdc_handle_protected_negotiation: if
   add_pa_data_element with copy == FALSE fails, it's still the
   caller's responsibility to free pa.contents.
 * Fix pre-existing (since r23465) leak of reply_encpart.enc_padata in
   process_as_req.
 * Call add_pa_data_element with copy == TRUE in
   return_referral_enc_padata since we are passing memory owned by the
   database entry.

 ------------------------------------------------------------------------
 r23712 | hartmans | 2010-02-09 14:15:07 -0500 (Tue, 09 Feb 2010) | 14 lines

 subject: enc_padata can include empty sequence
 ticket: 6656
 target_version: 1.8
 tags: pullup

 There are two issues with return_enc_padata.
 1)  It often will return an empty sequence of enc_padata rather than not including the field
 2) FAST negotiation is double supported in the referral tgs path and not supported in the non-referral path

 Rewrite the return_enc_padata logic to:

 * Split  out referral interactions with kdb into its own function
 * Use add_pa_data_element

ticket: 6656
version_fixed: 1.8
status: resolved

git-svn-id: svn://anonsvn.mit.edu/krb5/branches/krb5-1-8@23718 dc483132-0cff-0310-8789-dd5450dbe970

5 files changed, 67 insertions, 85 deletions

diff --git a/src/kdc/do_as_req.c b/src/kdc/do_as_req.c
index 83d3101..b183dcf 100644
--- a/src/kdc/do_as_req.c
+++ b/src/kdc/do_as_req.c
@@ -133,6 +133,7 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt,
     server_keyblock.contents = NULL;
     client_keyblock.contents = NULL;
     reply.padata = 0;
+    reply_encpart.enc_padata = 0;
     memset(&reply, 0, sizeof(reply));
 
     session_key.contents = 0;
@@ -623,7 +624,8 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt,
         goto errout;
     }
     errcode = return_enc_padata(kdc_context, req_pkt, request,
-                                as_encrypting_key, &server, &reply_encpart);
+                                as_encrypting_key, &server, &reply_encpart,
+                                FALSE);
     if (errcode) {
         status = "KDC_RETURN_ENC_PADATA";
         goto errout;
@@ -689,6 +691,8 @@ egress:
         krb5_free_keyblock_contents(kdc_context, &client_keyblock);
     if (reply.padata != NULL)
         krb5_free_pa_data(kdc_context, reply.padata);
+    if (reply_encpart.enc_padata)
+        krb5_free_pa_data(kdc_context, reply_encpart.enc_padata);
 
     if (cname != NULL)
         free(cname);
diff --git a/src/kdc/do_tgs_req.c b/src/kdc/do_tgs_req.c
index 26fde1e..cb0496f 100644
--- a/src/kdc/do_tgs_req.c
+++ b/src/kdc/do_tgs_req.c
@@ -948,29 +948,14 @@ tgt_again:
         status  = "generating reply key";
         goto cleanup;
     }
-    if (is_referral && isflagset(s_flags, KRB5_KDB_FLAG_CANONICALIZE)) {
-        int idx = 0;
-
-        errcode = return_enc_padata(kdc_context, pkt, request,
-                                    reply_key, &server, &reply_encpart);
-        if (errcode) {
-            status = "KDC_RETURN_ENC_PADATA";
-            goto cleanup;
-        }
-        /* Not referral. */
-        reply_encpart.enc_padata = calloc(3, sizeof(krb5_pa_data *));
-        if (reply_encpart.enc_padata == NULL) {
-            errcode = ENOMEM;
-            status = "Allocating enc_padata";
-            goto cleanup;
-        }
-        errcode = kdc_handle_protected_negotiation(pkt, request, reply_key,
-                                                   reply_encpart.enc_padata,
-                                                   &idx);
-        if (errcode != 0) {
-            status = "protected negotiation";
-            goto cleanup;
-        }
+    errcode = return_enc_padata(kdc_context, pkt, request,
+                                reply_key, &server, &reply_encpart,
+                                is_referral &&
+                                isflagset(s_flags,
+                                          KRB5_KDB_FLAG_CANONICALIZE));
+    if (errcode) {
+        status = "KDC_RETURN_ENC_PADATA";
+        goto cleanup;
     }
 
     errcode = krb5_encode_kdc_rep(kdc_context, KRB5_TGS_REP, &reply_encpart,
diff --git a/src/kdc/kdc_preauth.c b/src/kdc/kdc_preauth.c
index c5dfb1f..00800aa 100644
--- a/src/kdc/kdc_preauth.c
+++ b/src/kdc/kdc_preauth.c
@@ -3084,48 +3084,52 @@ include_pac_p(krb5_context context, krb5_kdc_req *request)
     return retval;
 }
 
-krb5_error_code
-return_enc_padata(krb5_context context, krb5_data *req_pkt,
-                  krb5_kdc_req *request, krb5_keyblock *reply_key,
-                  krb5_db_entry *server, krb5_enc_kdc_rep_part *reply_encpart)
+static krb5_error_code
+return_referral_enc_padata( krb5_context context,
+                            krb5_enc_kdc_rep_part *reply,
+                            krb5_db_entry *server)
 {
     krb5_error_code             code;
     krb5_tl_data                tl_data;
-    krb5_pa_data                *pa_data;
-    int idx = 0;
+    krb5_pa_data                pa_data;
 
-    /* This should be initialized and only used for Win2K compat and other
-     * specific standardized uses such as FAST negotiation. */
-    assert(reply_encpart->enc_padata == NULL);
-    reply_encpart->enc_padata = calloc(4, sizeof(krb5_pa_data *));
-    if (reply_encpart->enc_padata == NULL)
-        return ENOMEM;
     tl_data.tl_data_type = KRB5_TL_SVR_REFERRAL_DATA;
     code = krb5_dbe_lookup_tl_data(context, server, &tl_data);
     if (code || tl_data.tl_data_length == 0)
-        goto negotiate; /* no server referrals to return */
-
-    pa_data = (krb5_pa_data *)malloc(sizeof(*pa_data));
-    if (pa_data == NULL)
-        return ENOMEM;
-    pa_data->magic = KV5M_PA_DATA;
-    pa_data->pa_type = KRB5_PADATA_SVR_REFERRAL_INFO;
-    pa_data->length = tl_data.tl_data_length;
-    pa_data->contents = malloc(pa_data->length);
-    if (pa_data->contents == NULL) {
-        free(pa_data);
-        return ENOMEM;
-    }
-    memcpy(pa_data->contents, tl_data.tl_data_contents, tl_data.tl_data_length);
+        return 0;
 
+    pa_data.magic = KV5M_PA_DATA;
+    pa_data.pa_type = KRB5_PADATA_SVR_REFERRAL_INFO;
+    pa_data.length = tl_data.tl_data_length;
+    pa_data.contents = tl_data.tl_data_contents;
+    return add_pa_data_element(context, &pa_data, &reply->enc_padata, TRUE);
+}
 
-    reply_encpart->enc_padata[idx++] = pa_data;
-    reply_encpart->enc_padata[1] = NULL;
-negotiate:
-    return kdc_handle_protected_negotiation(req_pkt, request, reply_key,
-                                            reply_encpart->enc_padata, &idx);
+krb5_error_code
+return_enc_padata(krb5_context context, krb5_data *req_pkt,
+                  krb5_kdc_req *request, krb5_keyblock *reply_key,
+                  krb5_db_entry *server, krb5_enc_kdc_rep_part *reply_encpart,
+                  krb5_boolean is_referral)
+{
+    krb5_error_code code = 0;
+    /* This should be initialized and only used for Win2K compat and other
+     * specific standardized uses such as FAST negotiation. */
+    assert(reply_encpart->enc_padata == NULL);
+    if (is_referral) {
+        code = return_referral_enc_padata(context, reply_encpart, server);
+        if (code)
+            return code;
+    }
+    code = kdc_handle_protected_negotiation(req_pkt, request, reply_key,
+                                            &reply_encpart->enc_padata);
+    if (code)
+        goto cleanup;
+    /*Add potentially other enc_padata providers*/
+cleanup:
+    return code;
 }
 
+
 #if 0
 static krb5_error_code return_server_referral(krb5_context context,
                                               krb5_pa_data * padata,
diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index 281bcc8..d63bba2 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -2675,23 +2675,18 @@ kdc_get_ticket_endtime(krb5_context context,
 krb5_error_code
 kdc_handle_protected_negotiation(krb5_data *req_pkt, krb5_kdc_req *request,
                                  const krb5_keyblock *reply_key,
-                                 krb5_pa_data **out_enc_padata, int *idx)
+                                 krb5_pa_data ***out_enc_padata)
 {
     krb5_error_code retval = 0;
     krb5_checksum checksum;
     krb5_data *out = NULL;
-    krb5_pa_data *pa;
-    assert(out_enc_padata != NULL);
-    pa = krb5int_find_pa_data(kdc_context, request->padata,
+    krb5_pa_data pa, *pa_in;
+    pa_in = krb5int_find_pa_data(kdc_context, request->padata,
                               KRB5_ENCPADATA_REQ_ENC_PA_REP);
-    if (pa == NULL)
+    if (pa_in == NULL)
         return 0;
-    checksum.contents = NULL;
-    pa = malloc(sizeof(krb5_pa_data));
-    if (pa == NULL)
-        return ENOMEM;
-    pa->magic = KV5M_PA_DATA;
-    pa->pa_type = KRB5_ENCPADATA_REQ_ENC_PA_REP;
+    pa.magic = KV5M_PA_DATA;
+    pa.pa_type = KRB5_ENCPADATA_REQ_ENC_PA_REP;
     retval = krb5_c_make_checksum(kdc_context,0, reply_key,
                                   KRB5_KEYUSAGE_AS_REQ, req_pkt, &checksum);
     if (retval != 0)
@@ -2699,29 +2694,22 @@ kdc_handle_protected_negotiation(krb5_data *req_pkt, krb5_kdc_req *request,
     retval = encode_krb5_checksum(&checksum, &out);
     if (retval != 0)
         goto cleanup;
-    pa->contents = (krb5_octet *) out->data;
-    pa->length = out->length;
-    out_enc_padata[(*idx)++] = pa;
-    pa = NULL;
-    out->data = NULL;
-    pa = malloc(sizeof(krb5_pa_data));
-    if (pa == NULL) {
-        retval = ENOMEM;
+    pa.contents = (krb5_octet *) out->data;
+    pa.length = out->length;
+    retval = add_pa_data_element(kdc_context, &pa, out_enc_padata, FALSE);
+    if (retval)
         goto cleanup;
-    }
-    pa->magic = KV5M_PA_DATA;
-    pa->pa_type = KRB5_PADATA_FX_FAST;
-    pa->length = 0;
-    pa->contents = NULL;
-    out_enc_padata[(*idx)++] = pa;
-    pa = NULL;
+    out->data = NULL;
+    pa.magic = KV5M_PA_DATA;
+    pa.pa_type = KRB5_PADATA_FX_FAST;
+    pa.length = 0;
+    pa.contents = NULL;
+    retval = add_pa_data_element(kdc_context, &pa, out_enc_padata, FALSE);
 cleanup:
     if (checksum.contents)
         krb5_free_checksum_contents(kdc_context, &checksum);
     if (out != NULL)
         krb5_free_data(kdc_context, out);
-    if (pa != NULL)
-        free(pa);
     return retval;
 }
 
diff --git a/src/kdc/kdc_util.h b/src/kdc/kdc_util.h
index 2583895..03ecaf7 100644
--- a/src/kdc/kdc_util.h
+++ b/src/kdc/kdc_util.h
@@ -258,7 +258,8 @@ return_enc_padata(krb5_context context,
                   krb5_data *req_pkt, krb5_kdc_req *request,
                   krb5_keyblock *reply_key,
                   krb5_db_entry *server,
-                  krb5_enc_kdc_rep_part *reply_encpart);
+                  krb5_enc_kdc_rep_part *reply_encpart,
+                  krb5_boolean is_referral);
 
 krb5_error_code
 sign_db_authdata (krb5_context context,
@@ -401,7 +402,7 @@ krb5_error_code kdc_preauth_get_cookie(struct kdc_request_state *state,
 krb5_error_code
 kdc_handle_protected_negotiation( krb5_data *req_pkt, krb5_kdc_req *request,
                                   const krb5_keyblock *reply_key,
-                                  krb5_pa_data **out_enc_padata, int *idx);
+                                  krb5_pa_data ***out_enc_padata);
 krb5_error_code
 krb5int_get_domain_realm_mapping(krb5_context context,
                                  const char *host, char ***realmsp);