Simplify principal and policy manipulation code

Now that principal entry and policy fields are allocated using the
malloc visible to the krb5 libraries, we don't need to use
krb5_db_alloc() and krb5_db_free() when modifying them within our
code.

ticket: 8414

5 files changed, 58 insertions, 133 deletions

diff --git a/src/lib/kdb/encrypt_key.c b/src/lib/kdb/encrypt_key.c
index dafe612..dc612c8 100644
--- a/src/lib/kdb/encrypt_key.c
+++ b/src/lib/kdb/encrypt_key.c
@@ -74,7 +74,7 @@ krb5_dbe_def_encrypt_key_data( krb5_context             context,
     krb5_enc_data                 cipher;
 
     for (i = 0; i < key_data->key_data_ver; i++) {
-        krb5_db_free(context, key_data->key_data_contents[i]);
+        free(key_data->key_data_contents[i]);
         key_data->key_data_contents[i] = NULL;
     }
 
@@ -89,7 +89,7 @@ krb5_dbe_def_encrypt_key_data( krb5_context             context,
                                         &len)))
         return(retval);
 
-    ptr = krb5_db_alloc(context, NULL, 2 + len);
+    ptr = malloc(2 + len);
     if (ptr == NULL)
         return(ENOMEM);
 
@@ -108,7 +108,7 @@ krb5_dbe_def_encrypt_key_data( krb5_context             context,
 
     if ((retval = krb5_c_encrypt(context, mkey, /* XXX */ 0, 0,
                                  &plain, &cipher))) {
-        krb5_db_free(context, key_data->key_data_contents[0]);
+        free(key_data->key_data_contents[0]);
         return retval;
     }
 
@@ -118,10 +118,9 @@ krb5_dbe_def_encrypt_key_data( krb5_context             context,
             key_data->key_data_ver++;
             key_data->key_data_type[1] = keysalt->type;
             if ((key_data->key_data_length[1] = keysalt->data.length) != 0) {
-                key_data->key_data_contents[1] =
-                    krb5_db_alloc(context, NULL, keysalt->data.length);
+                key_data->key_data_contents[1] = malloc(keysalt->data.length);
                 if (key_data->key_data_contents[1] == NULL) {
-                    krb5_db_free(context, key_data->key_data_contents[0]);
+                    free(key_data->key_data_contents[0]);
                     return ENOMEM;
                 }
                 memcpy(key_data->key_data_contents[1], keysalt->data.data,
diff --git a/src/lib/kdb/kdb5.c b/src/lib/kdb/kdb5.c
index 20e8698..2886d05 100644
--- a/src/lib/kdb/kdb5.c
+++ b/src/lib/kdb/kdb5.c
@@ -114,10 +114,6 @@ logging(krb5_context context)
         log_ctx->ulog != NULL;
 }
 
-/*
- * XXX eventually this should be consolidated with krb5_free_key_data_contents
- * so there is only a single version.
- */
 void
 krb5_dbe_free_key_data_contents(krb5_context context, krb5_key_data *key)
 {
@@ -795,14 +791,12 @@ krb5_db_free_principal(krb5_context kcontext, krb5_db_entry *entry)
 }
 
 static void
-free_db_args(krb5_context kcontext, char **db_args)
+free_db_args(char **db_args)
 {
     int i;
     if (db_args) {
-        /* XXX Is this right?  Or are we borrowing storage from
-           the caller?  */
         for (i = 0; db_args[i]; i++)
-            krb5_db_free(kcontext, db_args[i]);
+            free(db_args[i]);
         free(db_args);
     }
 }
@@ -854,7 +848,7 @@ extract_db_args_from_tl_data(krb5_context kcontext, krb5_tl_data **start,
                 prev->tl_data_next = curr->tl_data_next;
             }
             (*count)--;
-            krb5_db_free(kcontext, curr);
+            free(curr);
 
             /* previous does not change */
             curr = next;
@@ -866,7 +860,7 @@ extract_db_args_from_tl_data(krb5_context kcontext, krb5_tl_data **start,
     status = 0;
 clean_n_exit:
     if (status != 0) {
-        free_db_args(kcontext, db_args);
+        free_db_args(db_args);
         db_args = NULL;
     }
     *db_argsp = db_args;
@@ -891,7 +885,7 @@ krb5int_put_principal_no_log(krb5_context kcontext, krb5_db_entry *entry)
     if (status)
         return status;
     status = v->put_principal(kcontext, entry, db_args);
-    free_db_args(kcontext, db_args);
+    free_db_args(db_args);
     return status;
 }
 
@@ -1210,10 +1204,7 @@ krb5_db_fetch_mkey(krb5_context context, krb5_principal mname,
     }
 
 clean_n_exit:
-    if (tmp_key.contents) {
-        zap(tmp_key.contents, tmp_key.length);
-        krb5_db_free(context, tmp_key.contents);
-    }
+    zapfree(tmp_key.contents, tmp_key.length);
     return retval;
 }
 
@@ -1486,11 +1477,13 @@ krb5_dbe_lookup_tl_data(krb5_context context, krb5_db_entry *entry,
 krb5_error_code
 krb5_dbe_create_key_data(krb5_context context, krb5_db_entry *entry)
 {
-    if ((entry->key_data =
-         (krb5_key_data *) krb5_db_alloc(context, entry->key_data,
-                                         (sizeof(krb5_key_data) *
-                                          (entry->n_key_data + 1)))) == NULL)
-        return (ENOMEM);
+    krb5_key_data *newptr;
+
+    newptr = realloc(entry->key_data,
+                     (entry->n_key_data + 1) * sizeof(*entry->key_data));
+    if (newptr == NULL)
+        return ENOMEM;
+    entry->key_data = newptr;
 
     memset(entry->key_data + entry->n_key_data, 0, sizeof(krb5_key_data));
     entry->n_key_data++;
@@ -2195,9 +2188,8 @@ krb5_db_update_tl_data(krb5_context context, krb5_int16 *n_tl_datap,
      * Copy the new data first, so we can fail cleanly if malloc()
      * fails.
      */
-    if ((tmp =
-         (krb5_octet *) krb5_db_alloc(context, NULL,
-                                      new_tl_data->tl_data_length)) == NULL)
+    tmp = malloc(new_tl_data->tl_data_length);
+    if (tmp == NULL)
         return (ENOMEM);
 
     /*
@@ -2215,12 +2207,11 @@ krb5_db_update_tl_data(krb5_context context, krb5_int16 *n_tl_datap,
     /* If necessary, chain a new record in the beginning and point at it.  */
 
     if (!tl_data) {
-        tl_data = krb5_db_alloc(context, NULL, sizeof(krb5_tl_data));
+        tl_data = calloc(1, sizeof(*tl_data));
         if (tl_data == NULL) {
             free(tmp);
             return (ENOMEM);
         }
-        memset(tl_data, 0, sizeof(krb5_tl_data));
         tl_data->tl_data_next = *tl_datap;
         *tl_datap = tl_data;
         (*n_tl_datap)++;
@@ -2228,8 +2219,7 @@ krb5_db_update_tl_data(krb5_context context, krb5_int16 *n_tl_datap,
 
     /* fill in the record */
 
-    if (tl_data->tl_data_contents)
-        krb5_db_free(context, tl_data->tl_data_contents);
+    free(tl_data->tl_data_contents);
 
     tl_data->tl_data_type = new_tl_data->tl_data_type;
     tl_data->tl_data_length = new_tl_data->tl_data_length;
@@ -2301,9 +2291,8 @@ krb5_error_code
 krb5_dbe_specialize_salt(krb5_context context, krb5_db_entry *entry)
 {
     krb5_int16 stype, i;
-    krb5_data *salt = NULL;
-    krb5_error_code ret = 0;
-    uint8_t *data;
+    krb5_data *salt;
+    krb5_error_code ret;
 
     if (context == NULL || entry == NULL)
         return EINVAL;
@@ -2316,27 +2305,19 @@ krb5_dbe_specialize_salt(krb5_context context, krb5_db_entry *entry)
         ret = krb5_dbe_compute_salt(context, &entry->key_data[i], entry->princ,
                                     &stype, &salt);
         if (ret)
-            goto cleanup;
-
-        data = krb5_db_alloc(context, NULL, salt->length);
-        if (data == NULL) {
-            ret = ENOMEM;
-            goto cleanup;
-        }
-        memcpy(data, salt->data, salt->length);
+            return ret;
 
+        /* Steal the data pointer from salt and free the container. */
+        if (entry->key_data[i].key_data_ver >= 2)
+            free(entry->key_data[i].key_data_contents[1]);
         entry->key_data[i].key_data_type[1] = KRB5_KDB_SALTTYPE_SPECIAL;
-        krb5_db_free(context, entry->key_data[i].key_data_contents[1]);
-        entry->key_data[i].key_data_contents[1] = data;
+        entry->key_data[i].key_data_contents[1] = (uint8_t *)salt->data;
         entry->key_data[i].key_data_length[1] = salt->length;
         entry->key_data[i].key_data_ver = 2;
-        krb5_free_data(context, salt);
-        salt = NULL;
+        free(salt);
     }
 
-cleanup:
-    krb5_free_data(context, salt);
-    return ret;
+    return 0;
 }
 
 /* change password functions */
diff --git a/src/lib/kdb/kdb_convert.c b/src/lib/kdb/kdb_convert.c
index 509016f..8172e9d 100644
--- a/src/lib/kdb/kdb_convert.c
+++ b/src/lib/kdb/kdb_convert.c
@@ -623,10 +623,9 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry,
      * Set ent->n_tl_data = 0 initially, if this is an ADD update
      */
     if (is_add) {
-        ent = krb5_db_alloc(context, NULL, sizeof(*ent));
+        ent = calloc(1, sizeof(*ent));
         if (ent == NULL)
             return (ENOMEM);
-        memset(ent, 0, sizeof(*ent));
         ent->n_tl_data = 0;
     }
 
diff --git a/src/lib/kdb/kdb_cpw.c b/src/lib/kdb/kdb_cpw.c
index 8124078..ead06ec 100644
--- a/src/lib/kdb/kdb_cpw.c
+++ b/src/lib/kdb/kdb_cpw.c
@@ -78,55 +78,18 @@ cleanup_key_data(context, count, data)
     int                   count;
     krb5_key_data       * data;
 {
-    int i, j;
+    int i;
 
     /* If data is NULL, count is always 0 */
     if (data == NULL) return;
 
-    for (i = 0; i < count; i++) {
-        for (j = 0; j < data[i].key_data_ver; j++) {
-            if (data[i].key_data_length[j]) {
-                krb5_db_free(context, data[i].key_data_contents[j]);
-            }
-        }
-    }
-    krb5_db_free(context, data);
+    for (i = 0; i < count; i++)
+        krb5_dbe_free_key_data_contents(context, &data[i]);
+    free(data);
 }
 
-/* Copy key data from in to out, using krb5_db_alloc storage for out. */
-static krb5_error_code
-copy_key_data(krb5_context context, krb5_key_data *in, krb5_key_data *out)
-{
-    int i;
-    void *copies[2] = { NULL, NULL };
-
-    memset(out, 0, sizeof(*out));
-
-    /* Copy the key data contents using krb5_db_alloc storage. */
-    for (i = 0; i < in->key_data_ver && i < 2; i++) {
-        if (in->key_data_length[i] == 0)
-            continue;
-        copies[i] = krb5_db_alloc(context, NULL, in->key_data_length[i]);
-        if (copies[i] == NULL) {
-            while (--i >= 0) {
-                zap(copies[i], in->key_data_length[i]);
-                krb5_db_free(context, copies[i]);
-            }
-            return ENOMEM;
-        }
-        memcpy(copies[i], in->key_data_contents[i], in->key_data_length[i]);
-    }
-
-    /* Copy the structure and replace the allocated fields with the copies. */
-    *out = *in;
-    for (i = 0; i < 2; i++)
-        out->key_data_contents[i] = copies[i];
-
-    return 0;
-}
-
-/* Copy key data from old_kd to new_kd.  new_kd will be encrypted with mkey and
- * will use krb5_db_alloc storage. */
+/* Transfer key data from old_kd to new_kd, making sure that new_kd is
+ * encrypted with mkey.  May steal from old_kd and zero it out. */
 static krb5_error_code
 preserve_one_old_key(krb5_context context, krb5_keyblock *mkey,
                      krb5_db_entry *dbent, krb5_key_data *old_kd,
@@ -135,16 +98,15 @@ preserve_one_old_key(krb5_context context, krb5_keyblock *mkey,
     krb5_error_code ret;
     krb5_keyblock kb;
     krb5_keysalt salt;
-    krb5_key_data kd;
 
     memset(new_kd, 0, sizeof(*new_kd));
-    memset(&kd, 0, sizeof(kd));
 
     ret = krb5_dbe_decrypt_key_data(context, mkey, old_kd, &kb, NULL);
     if (ret == 0) {
-        /* old_kd is already encrypted in mkey, so just copy it. */
-        krb5_free_keyblock_contents(context, &kb);
-        return copy_key_data(context, old_kd, new_kd);
+        /* old_kd is already encrypted in mkey, so just move it. */
+        *new_kd = *old_kd;
+        memset(old_kd, 0, sizeof(*old_kd));
+        return 0;
     }
 
     /* Decrypt and re-encrypt old_kd using mkey. */
@@ -152,20 +114,17 @@ preserve_one_old_key(krb5_context context, krb5_keyblock *mkey,
     if (ret)
         return ret;
     ret = krb5_dbe_encrypt_key_data(context, mkey, &kb, &salt,
-                                    old_kd->key_data_kvno, &kd);
+                                    old_kd->key_data_kvno, new_kd);
     krb5_free_keyblock_contents(context, &kb);
     krb5_free_data_contents(context, &salt.data);
-    if (ret)
-        return ret;
-
-    /* Copy the result to ensure new_kd uses db_alloc storage. */
-    ret = copy_key_data(context, &kd, new_kd);
-    krb5_dbe_free_key_data_contents(context, &kd);
     return ret;
 }
 
-/* Add key_data to dbent, making sure that each entry is encrypted in mkey.  If
- * kvno is non-zero, preserve only keys of that kvno. */
+/*
+ * Add key_data to dbent, making sure that each entry is encrypted in mkey.  If
+ * kvno is non-zero, preserve only keys of that kvno.  May steal some elements
+ * from key_data and zero them out.
+ */
 static krb5_error_code
 preserve_old_keys(krb5_context context, krb5_keyblock *mkey,
                   krb5_db_entry *dbent, int kvno, int n_key_data,
@@ -200,7 +159,7 @@ add_key_rnd(context, master_key, ks_tuple, ks_tuple_count, db_entry, kvno)
     krb5_keyblock         key;
     int                   i, j;
     krb5_error_code       retval;
-    krb5_key_data         tmp_key_data;
+    krb5_key_data        *kd_slot;
 
     for (i = 0; i < ks_tuple_count; i++) {
         krb5_boolean similar;
@@ -228,6 +187,7 @@ add_key_rnd(context, master_key, ks_tuple, ks_tuple_count, db_entry, kvno)
 
         if ((retval = krb5_dbe_create_key_data(context, db_entry)))
             return retval;
+        kd_slot = &db_entry->key_data[db_entry->n_key_data - 1];
 
         /* there used to be code here to extract the old key, and derive
            a new key from it.  Now that there's a unified prng, that isn't
@@ -238,20 +198,12 @@ add_key_rnd(context, master_key, ks_tuple, ks_tuple_count, db_entry, kvno)
                                              &key)))
             return retval;
 
-        memset( &tmp_key_data, 0, sizeof(tmp_key_data));
         retval = krb5_dbe_encrypt_key_data(context, master_key, &key, NULL,
-                                           kvno, &tmp_key_data);
+                                           kvno, kd_slot);
 
         krb5_free_keyblock_contents(context, &key);
         if( retval )
             return retval;
-
-        /* Copy the result to ensure we use db_alloc storage. */
-        retval = copy_key_data(context, &tmp_key_data,
-                               &db_entry->key_data[db_entry->n_key_data - 1]);
-        krb5_dbe_free_key_data_contents(context, &tmp_key_data);
-        if (retval)
-            return retval;
     }
 
     return 0;
@@ -309,7 +261,7 @@ add_key_pwd(context, master_key, ks_tuple, ks_tuple_count, passwd,
     krb5_data             pwd;
     krb5_data             afs_params = string2data("\1"), *s2k_params;
     int                   i, j;
-    krb5_key_data         tmp_key_data;
+    krb5_key_data        *kd_slot;
 
     for (i = 0; i < ks_tuple_count; i++) {
         krb5_boolean similar;
@@ -339,6 +291,7 @@ add_key_pwd(context, master_key, ks_tuple, ks_tuple_count, passwd,
 
         if ((retval = krb5_dbe_create_key_data(context, db_entry)))
             return(retval);
+        kd_slot = &db_entry->key_data[db_entry->n_key_data - 1];
 
         /* Convert password string to key using appropriate salt */
         switch (key_salt.type = ks_tuple[i].ks_salttype) {
@@ -394,23 +347,15 @@ add_key_pwd(context, master_key, ks_tuple, ks_tuple_count, passwd,
             return retval;
         }
 
-        memset(&tmp_key_data, 0, sizeof(tmp_key_data));
         retval = krb5_dbe_encrypt_key_data(context, master_key, &key,
                                            (const krb5_keysalt *)&key_salt,
-                                           kvno, &tmp_key_data);
+                                           kvno, kd_slot);
         if (key_salt.data.data)
             free(key_salt.data.data);
         free(key.contents);
 
         if( retval )
             return retval;
-
-        /* Copy the result to ensure we use db_alloc storage. */
-        retval = copy_key_data(context, &tmp_key_data,
-                               &db_entry->key_data[db_entry->n_key_data - 1]);
-        krb5_dbe_free_key_data_contents(context, &tmp_key_data);
-        if (retval)
-            return retval;
     }
 
     return 0;
@@ -455,13 +400,15 @@ rekey(krb5_context context, krb5_keyblock *mkey, krb5_key_salt_tuple *ks_tuple,
         return ret;
     }
 
-    /* Possibly add some or all of the old keys to the back of the list. */
+    /* Possibly add some or all of the old keys to the back of the list.  May
+     * steal from and zero out some of the old key data entries. */
     if (savekeys != DISCARD_ALL) {
         save_kvno = (savekeys == KEEP_LAST_KVNO) ? old_kvno : 0;
         ret = preserve_old_keys(context, mkey, db_entry, save_kvno, n_key_data,
                                 key_data);
     }
 
+    /* Free any old key data entries not stolen and zeroed out above. */
     cleanup_key_data(context, n_key_data, key_data);
     return ret;
 }
diff --git a/src/lib/kdb/t_stringattr.c b/src/lib/kdb/t_stringattr.c
index a153f7c..1174036 100644
--- a/src/lib/kdb/t_stringattr.c
+++ b/src/lib/kdb/t_stringattr.c
@@ -49,12 +49,11 @@ main()
     assert(krb5int_init_context_kdc(&context) == 0);
 
     /* Start with an empty entry. */
-    ent = krb5_db_alloc(context, NULL, sizeof(*ent));
+    ent = calloc(1, sizeof(*ent));
     if (ent == NULL) {
         fprintf(stderr, "Can't allocate memory for entry.\n");
         return 1;
     }
-    memset(ent, 0, sizeof(*ent));
 
     /* Check that the entry has no strings to start. */
     assert(krb5_dbe_get_strings(context, ent, &strings, &count) == 0);