krb5/Refactor-KDC-authdata-list-management-helpers.patch

From 00245d789edc6cf6263540d7c9d7ee45bbac58ce Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
Date: Wed, 5 Feb 2020 18:46:11 -0500
Subject: [PATCH] Refactor KDC authdata list management helpers

Remove the unused concat_authorization_data().  Split merge_authdata()
into two helpers, one to destructively merge without filtering and one
to add copied elements while filtering out KDC-only authdata types.
Remove context parameters where they aren't needed (taking advantage
of knowledge that some libkrb5 functions don't use their context
parameters).

(cherry picked from commit b2190fdc253de6024001e0f1ff9fe56c31042bb7)
---
 src/kdc/kdc_authdata.c | 138 +++++++++++++++++++----------------------
 src/kdc/kdc_util.c     |  50 ---------------
 src/kdc/kdc_util.h     |   5 --
 3 files changed, 64 insertions(+), 129 deletions(-)

diff --git a/src/kdc/kdc_authdata.c b/src/kdc/kdc_authdata.c
index 1ebe87246..010922c27 100644
--- a/src/kdc/kdc_authdata.c
+++ b/src/kdc/kdc_authdata.c
@@ -108,7 +108,7 @@ unload_authdata_plugins(krb5_context context)
 /* Return true if authdata should be filtered when copying from untrusted
  * authdata.  If desired_type is non-zero, look only for that type. */
 static krb5_boolean
-is_kdc_issued_authdatum(krb5_context context, krb5_authdata *authdata,
+is_kdc_issued_authdatum(krb5_authdata *authdata,
                         krb5_authdatatype desired_type)
 {
     krb5_boolean result = FALSE;
@@ -117,7 +117,7 @@ is_kdc_issued_authdatum(krb5_context context, krb5_authdata *authdata,
     krb5_authdatatype *ad_types, *containee_types = NULL;
 
     if (authdata->ad_type == KRB5_AUTHDATA_IF_RELEVANT) {
-        if (krb5int_get_authdata_containee_types(context, authdata, &count,
+        if (krb5int_get_authdata_containee_types(NULL, authdata, &count,
                                                  &containee_types) != 0)
             goto cleanup;
         ad_types = containee_types;
@@ -152,7 +152,7 @@ cleanup:
 /* Return true if authdata contains any elements which should only come from
  * the KDC.  If desired_type is non-zero, look only for that type. */
 static krb5_boolean
-has_kdc_issued_authdata(krb5_context context, krb5_authdata **authdata,
+has_kdc_issued_authdata(krb5_authdata **authdata,
                         krb5_authdatatype desired_type)
 {
     int i;
@@ -160,7 +160,7 @@ has_kdc_issued_authdata(krb5_context context, krb5_authdata **authdata,
     if (authdata == NULL)
         return FALSE;
     for (i = 0; authdata[i] != NULL; i++) {
-        if (is_kdc_issued_authdatum(context, authdata[i], desired_type))
+        if (is_kdc_issued_authdatum(authdata[i], desired_type))
             return TRUE;
     }
     return FALSE;
@@ -181,66 +181,71 @@ has_mandatory_for_kdc_authdata(krb5_context context, krb5_authdata **authdata)
     return FALSE;
 }
 
-/*
- * Add the elements of in_authdata to out_authdata.  If copy is false,
- * in_authdata is invalid on successful return.  If ignore_kdc_issued is true,
- * KDC-issued authdata is not copied.
- */
+/* Add elements from *new_elements to *existing_list, reallocating as
+ * necessary.  On success, release *new_elements and set it to NULL. */
 static krb5_error_code
-merge_authdata(krb5_context context, krb5_authdata **in_authdata,
-               krb5_authdata ***out_authdata, krb5_boolean copy,
-               krb5_boolean ignore_kdc_issued)
+merge_authdata(krb5_authdata ***existing_list, krb5_authdata ***new_elements)
 {
-    krb5_error_code ret;
-    size_t i, j, nadata = 0;
-    krb5_authdata **in_copy = NULL, **authdata = *out_authdata;
+    size_t count = 0, ncount = 0;
+    krb5_authdata **list = *existing_list, **nlist = *new_elements;
 
-    if (in_authdata == NULL || in_authdata[0] == NULL)
+    if (nlist == NULL)
         return 0;
 
-    if (authdata != NULL) {
-        for (nadata = 0; authdata[nadata] != NULL; nadata++)
-            ;
-    }
+    for (count = 0; list != NULL && list[count] != NULL; count++);
+    for (ncount = 0; nlist[ncount] != NULL; ncount++);
 
-    for (i = 0; in_authdata[i] != NULL; i++)
-        ;
-
-    if (copy) {
-        ret = krb5_copy_authdata(context, in_authdata, &in_copy);
-        if (ret)
-            return ret;
-        in_authdata = in_copy;
-    }
-
-    authdata = realloc(authdata, (nadata + i + 1) * sizeof(krb5_authdata *));
-    if (authdata == NULL) {
-        krb5_free_authdata(context, in_copy);
+    list = realloc(list, (count + ncount + 1) * sizeof(*list));
+    if (list == NULL)
         return ENOMEM;
+
+    memcpy(list + count, nlist, ncount * sizeof(*nlist));
+    list[count + ncount] = NULL;
+    free(nlist);
+
+    if (list[0] == NULL) {
+        free(list);
+        list = NULL;
     }
 
-    for (i = 0, j = 0; in_authdata[i] != NULL; i++) {
-        if (ignore_kdc_issued &&
-            is_kdc_issued_authdatum(context, in_authdata[i], 0)) {
-            free(in_authdata[i]->contents);
-            free(in_authdata[i]);
+    *new_elements = NULL;
+    *existing_list = list;
+    return 0;
+}
+
+/* Add a copy of new_elements to *existing_list, omitting KDC-issued
+ * authdata. */
+static krb5_error_code
+add_filtered_authdata(krb5_authdata ***existing_list,
+                      krb5_authdata **new_elements)
+{
+    krb5_error_code ret;
+    krb5_authdata **copy;
+    size_t i, j;
+
+    if (new_elements == NULL)
+        return 0;
+
+    ret = krb5_copy_authdata(NULL, new_elements, &copy);
+    if (ret)
+        return ret;
+
+    /* Remove KDC-issued elements from copy. */
+    j = 0;
+    for (i = 0; copy[i] != NULL; i++) {
+        if (is_kdc_issued_authdatum(copy[i], 0)) {
+            free(copy[i]->contents);
+            free(copy[i]);
         } else {
-            authdata[nadata + j++] = in_authdata[i];
+            copy[j++] = copy[i];
         }
     }
+    copy[j] = NULL;
 
-    authdata[nadata + j] = NULL;
-
-    free(in_authdata);
-
-    if (authdata[0] == NULL) {
-        free(authdata);
-        authdata = NULL;
-    }
-
-    *out_authdata = authdata;
-
-    return 0;
+    /* Destructively merge the filtered copy into existing_list. */
+    ret = merge_authdata(existing_list, &copy);
+    krb5_free_authdata(NULL, copy);
+    return ret;
 }
 
 /* Copy TGS-REQ authorization data into the ticket authdata. */
@@ -289,10 +294,7 @@ copy_request_authdata(krb5_context context, krb5_keyblock *client_key,
         goto cleanup;
     }
 
-    /* Add a copy of the requested authdata to the ticket, ignoring KDC-issued
-     * types. */
-    ret = merge_authdata(context, req->unenc_authdata, tkt_authdata, TRUE,
-                         TRUE);
+    ret = add_filtered_authdata(tkt_authdata, req->unenc_authdata);
 
 cleanup:
     free(plaintext.data);
@@ -307,9 +309,7 @@ copy_tgt_authdata(krb5_context context, krb5_kdc_req *request,
     if (has_mandatory_for_kdc_authdata(context, tgt_authdata))
         return KRB5KDC_ERR_POLICY;
 
-    /* Add a copy of the TGT authdata to the ticket, ignoring KDC-issued
-     * types. */
-    return merge_authdata(context, tgt_authdata, tkt_authdata, TRUE, TRUE);
+    return add_filtered_authdata(tkt_authdata, tgt_authdata);
 }
 
 /* Fetch authorization data from KDB module. */
@@ -374,8 +374,7 @@ fetch_kdb_authdata(krb5_context context, unsigned int flags,
 
     /* Put the KDB authdata first in the ticket.  A successful merge places the
      * combined list in db_authdata and releases the old ticket authdata. */
-    ret = merge_authdata(context, enc_tkt_reply->authorization_data,
-                         &db_authdata, FALSE, FALSE);
+    ret = merge_authdata(&db_authdata, &enc_tkt_reply->authorization_data);
     if (ret)
         krb5_free_authdata(context, db_authdata);
     else
@@ -404,8 +403,7 @@ make_signedpath_data(krb5_context context, krb5_const_principal client,
             return ret;
 
         for (i = 0, j = 0; authdata[i] != NULL; i++) {
-            if (is_kdc_issued_authdatum(context, authdata[i],
-                                        KRB5_AUTHDATA_SIGNTICKET))
+            if (is_kdc_issued_authdatum(authdata[i], KRB5_AUTHDATA_SIGNTICKET))
                 continue;
 
             sign_authdata[j++] = authdata[i];
@@ -635,12 +633,8 @@ make_signedpath(krb5_context context, krb5_const_principal for_user_princ,
     if (ret)
         goto cleanup;
 
-    /* Add the authdata to the ticket, without copying or filtering. */
-    ret = merge_authdata(context, if_relevant,
-                         &enc_tkt_reply->authorization_data, FALSE, FALSE);
-    if (ret)
-        goto cleanup;
-    if_relevant = NULL;         /* merge_authdata() freed */
+    /* Add the signedpath authdata to the ticket. */
+    ret = merge_authdata(&enc_tkt_reply->authorization_data, &if_relevant);
 
 cleanup:
     free(sp.delegated);
@@ -665,7 +659,7 @@ free_deleg_path(krb5_context context, krb5_principal *deleg_path)
 static krb5_boolean
 has_pac(krb5_context context, krb5_authdata **authdata)
 {
-    return has_kdc_issued_authdata(context, authdata, KRB5_AUTHDATA_WIN2K_PAC);
+    return has_kdc_issued_authdata(authdata, KRB5_AUTHDATA_WIN2K_PAC);
 }
 
 /* Verify AD-SIGNTICKET authdata if we need to, and insert an AD-SIGNEDPATH
@@ -746,11 +740,7 @@ add_auth_indicators(krb5_context context, krb5_data *const *auth_indicators,
         goto cleanup;
 
     /* Add the wrapped authdata to the ticket, without copying or filtering. */
-    ret = merge_authdata(context, cammac, &enc_tkt_reply->authorization_data,
-                         FALSE, FALSE);
-    if (ret)
-        goto cleanup;
-    cammac = NULL;              /* merge_authdata() freed */
+    ret = merge_authdata(&enc_tkt_reply->authorization_data, &cammac);
 
 cleanup:
     krb5_free_data(context, der_indicators);
diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index 6330387d0..a4a05b9fa 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -78,56 +78,6 @@ static krb5_error_code find_server_key(krb5_context,
                                        krb5_kvno, krb5_keyblock **,
                                        krb5_kvno *);
 
-/*
- * concatenate first two authdata arrays, returning an allocated replacement.
- * The replacement should be freed with krb5_free_authdata().
- */
-krb5_error_code
-concat_authorization_data(krb5_context context,
-                          krb5_authdata **first, krb5_authdata **second,
-                          krb5_authdata ***output)
-{
-    int i, j;
-    krb5_authdata **ptr, **retdata;
-
-    /* count up the entries */
-    i = 0;
-    if (first)
-        for (ptr = first; *ptr; ptr++)
-            i++;
-    if (second)
-        for (ptr = second; *ptr; ptr++)
-            i++;
-
-    retdata = (krb5_authdata **)malloc((i+1)*sizeof(*retdata));
-    if (!retdata)
-        return ENOMEM;
-    retdata[i] = 0;                     /* null-terminated array */
-    for (i = 0, j = 0, ptr = first; j < 2 ; ptr = second, j++)
-        while (ptr && *ptr) {
-            /* now walk & copy */
-            retdata[i] = (krb5_authdata *)malloc(sizeof(*retdata[i]));
-            if (!retdata[i]) {
-                krb5_free_authdata(context, retdata);
-                return ENOMEM;
-            }
-            *retdata[i] = **ptr;
-            if (!(retdata[i]->contents =
-                  (krb5_octet *)malloc(retdata[i]->length))) {
-                free(retdata[i]);
-                retdata[i] = 0;
-                krb5_free_authdata(context, retdata);
-                return ENOMEM;
-            }
-            memcpy(retdata[i]->contents, (*ptr)->contents, retdata[i]->length);
-
-            ptr++;
-            i++;
-        }
-    *output = retdata;
-    return 0;
-}
-
 krb5_boolean
 is_local_principal(kdc_realm_t *kdc_active_realm, krb5_const_principal princ1)
 {
diff --git a/src/kdc/kdc_util.h b/src/kdc/kdc_util.h
index 2c9d8cf69..42b7ee208 100644
--- a/src/kdc/kdc_util.h
+++ b/src/kdc/kdc_util.h
@@ -52,11 +52,6 @@ compress_transited (krb5_data *,
                     krb5_principal,
                     krb5_data *);
 krb5_error_code
-concat_authorization_data (krb5_context,
-                           krb5_authdata **,
-                           krb5_authdata **,
-                           krb5_authdata ***);
-krb5_error_code
 fetch_last_req_info (krb5_db_entry *, krb5_last_req_entry ***);
 
 krb5_error_code