Refactor KDC krb5_pa_data utility functions

Move alloc_padata from fast_util.c to kdc_util.c and make it
non-static so it can be used by other files.  Rename it to
alloc_pa_data for consistency with add_pa_data_element.  Make it
correctly handle zero length using a null contents pointer.

Make add_pa_data_element claim both the container and contents memory
from the caller, now that callers can use alloc_pa_data to simplify
allocation and copying.  Remove the copy parameter and the unused
context parameter, and put the list parameter first.  Adjust all
callers accordingly, making small simplifications to memory handling
where applicable.

diff --git a/src/kdc/fast_util.c b/src/kdc/fast_util.c
index e05107ef328d83306114150044ac980a92bcaf38..6a3fc11b98e7d3d972ca5315be0d30feb160a806 100644 (file)
--- a/src/kdc/fast_util.c
+++ b/src/kdc/fast_util.c
@@ -451,36 +451,12 @@ kdc_fast_hide_client(struct kdc_request_state *state)
     return (state->fast_options & KRB5_FAST_OPTION_HIDE_CLIENT_NAMES) != 0;
 }
 
-/* Allocate a pa-data entry with an uninitialized buffer of size len. */
-static krb5_error_code
-alloc_padata(krb5_preauthtype pa_type, size_t len, krb5_pa_data **out)
-{
-    krb5_pa_data *pa;
-    uint8_t *buf;
-
-    *out = NULL;
-    buf = malloc(len);
-    if (buf == NULL)
-        return ENOMEM;
-    pa = malloc(sizeof(*pa));
-    if (pa == NULL) {
-        free(buf);
-        return ENOMEM;
-    }
-    pa->magic = KV5M_PA_DATA;
-    pa->pa_type = pa_type;
-    pa->length = len;
-    pa->contents = buf;
-    *out = pa;
-    return 0;
-}
-
 /* Create a pa-data entry with the specified type and contents. */
 static krb5_error_code
 make_padata(krb5_preauthtype pa_type, const void *contents, size_t len,
             krb5_pa_data **out)
 {
-    if (alloc_padata(pa_type, len, out) != 0)
+    if (alloc_pa_data(pa_type, len, out) != 0)
         return ENOMEM;
     memcpy((*out)->contents, contents, len);
     return 0;
@@ -720,7 +696,7 @@ kdc_fast_make_cookie(krb5_context context, struct kdc_request_state *state,
         goto cleanup;
 
     /* Construct the cookie pa-data entry. */
-    ret = alloc_padata(KRB5_PADATA_FX_COOKIE, 8 + enc.ciphertext.length, &pa);
+    ret = alloc_pa_data(KRB5_PADATA_FX_COOKIE, 8 + enc.ciphertext.length, &pa);
     memcpy(pa->contents, "MIT1", 4);
     store_32_be(kvno, pa->contents + 4);
     memcpy(pa->contents + 8, enc.ciphertext.data, enc.ciphertext.length);

diff --git a/src/kdc/kdc_preauth.c b/src/kdc/kdc_preauth.c
index 739c5e77658fba22a8e9bc3bbd827c5ee5657f34..edc30bd83efcb71ce8e20ddb4a9421ac14baad11 100644 (file)
--- a/src/kdc/kdc_preauth.c
+++ b/src/kdc/kdc_preauth.c
@@ -1617,18 +1617,20 @@ return_referral_enc_padata( krb5_context context,
 {
     krb5_error_code             code;
     krb5_tl_data                tl_data;
-    krb5_pa_data                pa_data;
+    krb5_pa_data                *pa;
 
     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)
         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);
+    code = alloc_pa_data(KRB5_PADATA_SVR_REFERRAL_INFO, tl_data.tl_data_length,
+                         &pa);
+    if (code)
+        return code;
+    memcpy(pa->contents, tl_data.tl_data_contents, tl_data.tl_data_length);
+    /* add_pa_data_element() claims pa on success or failure. */
+    return add_pa_data_element(&reply->enc_padata, pa);
 }
 
 krb5_error_code

diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index 754570c013108330364fdfc330cd4083aaefa430..13111215d157d2421e7d2e1622e986c77a048874 100644 (file)
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -1353,9 +1353,9 @@ kdc_make_s4u2self_rep(krb5_context context,
                       krb5_enc_kdc_rep_part *reply_encpart)
 {
     krb5_error_code             code;
-    krb5_data                   *data = NULL;
+    krb5_data                   *der_user_id = NULL, *der_s4u_x509_user = NULL;
     krb5_pa_s4u_x509_user       rep_s4u_user;
-    krb5_pa_data                padata;
+    krb5_pa_data                *pa;
     krb5_enctype                enctype;
     krb5_keyusage               usage;
 
@@ -1366,7 +1366,7 @@ kdc_make_s4u2self_rep(krb5_context context,
     rep_s4u_user.user_id.options =
         req_s4u_user->user_id.options & KRB5_S4U_OPTS_USE_REPLY_KEY_USAGE;
 
-    code = encode_krb5_s4u_userid(&rep_s4u_user.user_id, &data);
+    code = encode_krb5_s4u_userid(&rep_s4u_user.user_id, &der_user_id);
     if (code != 0)
         goto cleanup;
 
@@ -1377,29 +1377,25 @@ kdc_make_s4u2self_rep(krb5_context context,
 
     code = krb5_c_make_checksum(context, req_s4u_user->cksum.checksum_type,
                                 tgs_subkey != NULL ? tgs_subkey : tgs_session,
-                                usage, data,
-                                &rep_s4u_user.cksum);
+                                usage, der_user_id, &rep_s4u_user.cksum);
     if (code != 0)
         goto cleanup;
 
-    krb5_free_data(context, data);
-    data = NULL;
-
-    code = encode_krb5_pa_s4u_x509_user(&rep_s4u_user, &data);
+    code = encode_krb5_pa_s4u_x509_user(&rep_s4u_user, &der_s4u_x509_user);
     if (code != 0)
         goto cleanup;
 
-    padata.magic = KV5M_PA_DATA;
-    padata.pa_type = KRB5_PADATA_S4U_X509_USER;
-    padata.length = data->length;
-    padata.contents = (krb5_octet *)data->data;
-
-    code = add_pa_data_element(context, &padata, &reply->padata, FALSE);
+    /* Add a padata element, stealing memory from der_s4u_x509_user. */
+    code = alloc_pa_data(KRB5_PADATA_S4U_X509_USER, 0, &pa);
+    if (code != 0)
+        goto cleanup;
+    pa->length = der_s4u_x509_user->length;
+    pa->contents = (uint8_t *)der_s4u_x509_user->data;
+    der_s4u_x509_user->data = NULL;
+    /* add_pa_data_element() claims pa on success or failure. */
+    code = add_pa_data_element(&reply->padata, pa);
     if (code != 0)
         goto cleanup;
-
-    free(data);
-    data = NULL;
 
     if (tgs_subkey != NULL)
         enctype = tgs_subkey->enctype;
@@ -1413,33 +1409,27 @@ kdc_make_s4u2self_rep(krb5_context context,
      */
     if ((req_s4u_user->user_id.options & KRB5_S4U_OPTS_USE_REPLY_KEY_USAGE) &&
         enctype_requires_etype_info_2(enctype) == FALSE) {
-        padata.length = req_s4u_user->cksum.length +
-            rep_s4u_user.cksum.length;
-        padata.contents = malloc(padata.length);
-        if (padata.contents == NULL) {
-            code = ENOMEM;
+        code = alloc_pa_data(KRB5_PADATA_S4U_X509_USER,
+                             req_s4u_user->cksum.length +
+                             rep_s4u_user.cksum.length, &pa);
+        if (code != 0)
             goto cleanup;
-        }
+        memcpy(pa->contents,
+               req_s4u_user->cksum.contents, req_s4u_user->cksum.length);
+        memcpy(&pa->contents[req_s4u_user->cksum.length],
+               rep_s4u_user.cksum.contents, rep_s4u_user.cksum.length);
 
-        memcpy(padata.contents,
-               req_s4u_user->cksum.contents,
-               req_s4u_user->cksum.length);
-        memcpy(&padata.contents[req_s4u_user->cksum.length],
-               rep_s4u_user.cksum.contents,
-               rep_s4u_user.cksum.length);
-
-        code = add_pa_data_element(context,&padata,
-                                   &reply_encpart->enc_padata, FALSE);
-        if (code != 0) {
-            free(padata.contents);
+        /* add_pa_data_element() claims pa on success or failure. */
+        code = add_pa_data_element(&reply_encpart->enc_padata, pa);
+        if (code != 0)
             goto cleanup;
-        }
     }
 
 cleanup:
     if (rep_s4u_user.cksum.contents != NULL)
         krb5_free_checksum_contents(context, &rep_s4u_user.cksum);
-    krb5_free_data(context, data);
+    krb5_free_data(context, der_user_id);
+    krb5_free_data(context, der_s4u_x509_user);
 
     return code;
 }
@@ -1707,46 +1697,50 @@ enctype_requires_etype_info_2(krb5_enctype enctype)
     }
 }
 
-/* XXX where are the generic helper routines for this? */
+/* Allocate a pa-data entry with an uninitialized buffer of size len. */
 krb5_error_code
-add_pa_data_element(krb5_context context,
-                    krb5_pa_data *padata,
-                    krb5_pa_data ***inout_padata,
-                    krb5_boolean copy)
+alloc_pa_data(krb5_preauthtype pa_type, size_t len, krb5_pa_data **out)
 {
-    int                         i;
-    krb5_pa_data                **p;
+    krb5_pa_data *pa;
+    uint8_t *buf = NULL;
 
-    if (*inout_padata != NULL) {
-        for (i = 0; (*inout_padata)[i] != NULL; i++)
-            ;
-    } else
-        i = 0;
-
-    p = realloc(*inout_padata, (i + 2) * sizeof(krb5_pa_data *));
-    if (p == NULL)
-        return ENOMEM;
-
-    *inout_padata = p;
-
-    p[i] = (krb5_pa_data *)malloc(sizeof(krb5_pa_data));
-    if (p[i] == NULL)
+    *out = NULL;
+    if (len > 0) {
+        buf = malloc(len);
+        if (buf == NULL)
+            return ENOMEM;
+    }
+    pa = malloc(sizeof(*pa));
+    if (pa == NULL) {
+        free(buf);
         return ENOMEM;
-    *(p[i]) = *padata;
+    }
+    pa->magic = KV5M_PA_DATA;
+    pa->pa_type = pa_type;
+    pa->length = len;
+    pa->contents = buf;
+    *out = pa;
+    return 0;
+}
 
-    p[i + 1] = NULL;
+/* Add pa to list, claiming its memory.  Free pa on failure. */
+krb5_error_code
+add_pa_data_element(krb5_pa_data ***list, krb5_pa_data *pa)
+{
+    size_t count;
+    krb5_pa_data **newlist;
 
-    if (copy) {
-        p[i]->contents = (krb5_octet *)malloc(padata->length);
-        if (p[i]->contents == NULL) {
-            free(p[i]);
-            p[i] = NULL;
-            return ENOMEM;
-        }
+    for (count = 0; *list != NULL && (*list)[count] != NULL; count++);
 
-        memcpy(p[i]->contents, padata->contents, padata->length);
+    newlist = realloc(*list, (count + 2) * sizeof(*newlist));
+    if (newlist == NULL) {
+        free(pa->contents);
+        free(pa);
+        return ENOMEM;
     }
-
+    newlist[count] = pa;
+    newlist[count + 1] = NULL;
+    *list = newlist;
     return 0;
 }
 
@@ -1850,38 +1844,47 @@ kdc_handle_protected_negotiation(krb5_context context,
 {
     krb5_error_code retval = 0;
     krb5_checksum checksum;
-    krb5_data *out = NULL;
-    krb5_pa_data pa, *pa_in;
+    krb5_data *der_cksum = NULL;
+    krb5_pa_data *pa, *pa_in;
+
+    memset(&checksum, 0, sizeof(checksum));
+
     pa_in = krb5int_find_pa_data(context, request->padata,
                                  KRB5_ENCPADATA_REQ_ENC_PA_REP);
     if (pa_in == NULL)
         return 0;
-    pa.magic = KV5M_PA_DATA;
-    pa.pa_type = KRB5_ENCPADATA_REQ_ENC_PA_REP;
-    memset(&checksum, 0, sizeof(checksum));
-    retval = krb5_c_make_checksum(context,0, reply_key,
-                                  KRB5_KEYUSAGE_AS_REQ, req_pkt, &checksum);
+
+    /* Compute and encode a checksum over the AS-REQ. */
+    retval = krb5_c_make_checksum(context, 0, reply_key, KRB5_KEYUSAGE_AS_REQ,
+                                  req_pkt, &checksum);
     if (retval != 0)
         goto cleanup;
-    retval = encode_krb5_checksum(&checksum, &out);
+    retval = encode_krb5_checksum(&checksum, &der_cksum);
     if (retval != 0)
         goto cleanup;
-    pa.contents = (krb5_octet *) out->data;
-    pa.length = out->length;
-    retval = add_pa_data_element(context, &pa, out_enc_padata, FALSE);
+
+    /* Add a pa-data element to the list, stealing memory from der_cksum. */
+    retval = alloc_pa_data(KRB5_ENCPADATA_REQ_ENC_PA_REP, 0, &pa);
+    if (retval)
+        goto cleanup;
+    pa->length = der_cksum->length;
+    pa->contents = (uint8_t *)der_cksum->data;
+    der_cksum->data = NULL;
+    /* add_pa_data_element() claims pa on success or failure. */
+    retval = add_pa_data_element(out_enc_padata, pa);
     if (retval)
         goto cleanup;
-    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(context, &pa, out_enc_padata, FALSE);
+
+    /* Add a zero-length PA-FX-FAST element to the list. */
+    retval = alloc_pa_data(KRB5_PADATA_FX_FAST, 0, &pa);
+    if (retval)
+        goto cleanup;
+    /* add_pa_data_element() claims pa on success or failure. */
+    retval = add_pa_data_element(out_enc_padata, pa);
+
 cleanup:
-    if (checksum.contents)
-        krb5_free_checksum_contents(context, &checksum);
-    if (out != NULL)
-        krb5_free_data(context, out);
+    krb5_free_checksum_contents(context, &checksum);
+    krb5_free_data(context, der_cksum);
     return retval;
 }
 

diff --git a/src/kdc/kdc_util.h b/src/kdc/kdc_util.h
index f99efcf505d1c3f81f88c7621a04205f3fbc7315..18649b8ad54be7a8b2cd502278ef46d650d3bdb7 100644 (file)
--- a/src/kdc/kdc_util.h
+++ b/src/kdc/kdc_util.h
@@ -203,10 +203,10 @@ void
 free_padata_context(krb5_context context, void *padata_context);
 
 krb5_error_code
-add_pa_data_element (krb5_context context,
-                     krb5_pa_data *padata,
-                     krb5_pa_data ***out_padata,
-                     krb5_boolean copy);
+alloc_pa_data(krb5_preauthtype pa_type, size_t len, krb5_pa_data **out);
+
+krb5_error_code
+add_pa_data_element(krb5_pa_data ***list, krb5_pa_data *pa);
 
 /* kdc_preauth_ec.c */
 krb5_error_code