[29415] in CVS-changelog-for-Kerberos-V5
krb5 commit: Simplify principal and policy manipulation code
daemon@ATHENA.MIT.EDU (Greg Hudson)
Thu May 26 11:45:41 2016
Date: Thu, 26 May 2016 11:45:36 -0400
From: Greg Hudson <ghudson@mit.edu>
Message-Id: <201605261545.u4QFjaQZ000992@drugstore.mit.edu>
To: cvs-krb5@mit.edu
Reply-To: krbdev@mit.edu
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: cvs-krb5-bounces@mit.edu
https://github.com/krb5/krb5/commit/d0168227a062bc70b1ec04295cdaa512c33c2233
commit d0168227a062bc70b1ec04295cdaa512c33c2233
Author: Greg Hudson <ghudson@mit.edu>
Date: Tue May 17 19:28:25 2016 -0400
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
src/kadmin/dbutil/dump.c | 3 +-
src/kadmin/dbutil/kdb5_create.c | 3 +-
src/lib/kadm5/srv/svr_policy.c | 8 +--
src/lib/kadm5/srv/svr_principal.c | 106 +++-------------------
src/lib/kdb/encrypt_key.c | 11 +--
src/lib/kdb/kdb5.c | 71 ++++++---------
src/lib/kdb/kdb_convert.c | 3 +-
src/lib/kdb/kdb_cpw.c | 103 +++++----------------
src/lib/kdb/t_stringattr.c | 3 +-
src/plugins/kdb/ldap/ldap_util/kdb5_ldap_realm.c | 14 +---
src/tests/create/kdb5_mkdums.c | 3 +-
11 files changed, 75 insertions(+), 253 deletions(-)
diff --git a/src/kadmin/dbutil/dump.c b/src/kadmin/dbutil/dump.c
index 07f62e9..e3b0f7c 100644
--- a/src/kadmin/dbutil/dump.c
+++ b/src/kadmin/dbutil/dump.c
@@ -712,10 +712,9 @@ process_k5beta7_princ(krb5_context context, const char *fname, FILE *filep,
krb5_tl_data *tl;
krb5_error_code ret;
- dbentry = krb5_db_alloc(context, NULL, sizeof(*dbentry));
+ dbentry = calloc(1, sizeof(*dbentry));
if (dbentry == NULL)
return 1;
- memset(dbentry, 0, sizeof(*dbentry));
(*linenop)++;
nread = fscanf(filep, "%u\t%u\t%u\t%u\t%u\t", &u1, &u2, &u3, &u4, &u5);
if (nread == EOF) {
diff --git a/src/kadmin/dbutil/kdb5_create.c b/src/kadmin/dbutil/kdb5_create.c
index 74e506a..9bfe201 100644
--- a/src/kadmin/dbutil/kdb5_create.c
+++ b/src/kadmin/dbutil/kdb5_create.c
@@ -423,10 +423,9 @@ add_principal(context, princ, op, pblock)
struct iterate_args iargs;
krb5_actkvno_node actkvno;
- entry = krb5_db_alloc(context, NULL, sizeof(*entry));
+ entry = calloc(1, sizeof(*entry));
if (entry == NULL)
return ENOMEM;
- memset(entry, 0, sizeof(*entry));
entry->len = KRB5_KDB_V1_BASE_LENGTH;
entry->attributes = pblock->flags;
diff --git a/src/lib/kadm5/srv/svr_policy.c b/src/lib/kadm5/srv/svr_policy.c
index dfb3183..dbf0a24 100644
--- a/src/lib/kadm5/srv/svr_policy.c
+++ b/src/lib/kadm5/srv/svr_policy.c
@@ -249,7 +249,6 @@ kadm5_modify_policy(void *server_handle, kadm5_policy_ent_t entry, long mask)
krb5_tl_data *tl;
osa_policy_ent_t p;
int ret;
- size_t len;
CHECK_HANDLE(server_handle);
@@ -329,17 +328,14 @@ kadm5_modify_policy(void *server_handle, kadm5_policy_ent_t entry, long mask)
if ((mask & KADM5_POLICY_MAX_RLIFE))
p->max_renewable_life = entry->max_renewable_life;
if ((mask & KADM5_POLICY_ALLOWED_KEYSALTS)) {
- krb5_db_free(handle->context, p->allowed_keysalts);
+ free(p->allowed_keysalts);
p->allowed_keysalts = NULL;
if (entry->allowed_keysalts != NULL) {
- len = strlen(entry->allowed_keysalts) + 1;
- p->allowed_keysalts = krb5_db_alloc(handle->context, NULL,
- len);
+ p->allowed_keysalts = strdup(entry->allowed_keysalts);
if (p->allowed_keysalts == NULL) {
ret = ENOMEM;
goto cleanup;
}
- memcpy(p->allowed_keysalts, entry->allowed_keysalts, len);
}
}
if ((mask & KADM5_POLICY_TL_DATA)) {
diff --git a/src/lib/kadm5/srv/svr_principal.c b/src/lib/kadm5/srv/svr_principal.c
index 5a340b6..466204d 100644
--- a/src/lib/kadm5/srv/svr_principal.c
+++ b/src/lib/kadm5/srv/svr_principal.c
@@ -31,61 +31,6 @@ static int decrypt_key_data(krb5_context context,
int n_key_data, krb5_key_data *key_data,
krb5_keyblock **keyblocks, int *n_keys);
-static krb5_error_code
-kadm5_copy_principal(krb5_context context, krb5_const_principal inprinc, krb5_principal *outprinc)
-{
- register krb5_principal tempprinc;
- register int i, nelems;
-
- tempprinc = (krb5_principal)krb5_db_alloc(context, NULL, sizeof(krb5_principal_data));
-
- if (tempprinc == 0)
- return ENOMEM;
-
- VALGRIND_CHECK_DEFINED(*inprinc);
- *tempprinc = *inprinc;
-
- nelems = (int) krb5_princ_size(context, inprinc);
- tempprinc->data = krb5_db_alloc(context, NULL, nelems * sizeof(krb5_data));
- if (tempprinc->data == 0) {
- krb5_db_free(context, (char *)tempprinc);
- return ENOMEM;
- }
-
- for (i = 0; i < nelems; i++) {
- unsigned int len = krb5_princ_component(context, inprinc, i)->length;
- krb5_princ_component(context, tempprinc, i)->length = len;
- if (((krb5_princ_component(context, tempprinc, i)->data =
- krb5_db_alloc(context, NULL, len)) == 0) && len) {
- while (--i >= 0)
- krb5_db_free(context, krb5_princ_component(context, tempprinc, i)->data);
- krb5_db_free (context, tempprinc->data);
- krb5_db_free (context, tempprinc);
- return ENOMEM;
- }
- if (len)
- memcpy(krb5_princ_component(context, tempprinc, i)->data,
- krb5_princ_component(context, inprinc, i)->data, len);
- krb5_princ_component(context, tempprinc, i)->magic = KV5M_DATA;
- }
-
- tempprinc->realm.data =
- krb5_db_alloc(context, NULL, tempprinc->realm.length = inprinc->realm.length);
- if (!tempprinc->realm.data && tempprinc->realm.length) {
- for (i = 0; i < nelems; i++)
- krb5_db_free(context, krb5_princ_component(context, tempprinc, i)->data);
- krb5_db_free(context, tempprinc->data);
- krb5_db_free(context, tempprinc);
- return ENOMEM;
- }
- if (tempprinc->realm.length)
- memcpy(tempprinc->realm.data, inprinc->realm.data,
- inprinc->realm.length);
-
- *outprinc = tempprinc;
- return 0;
-}
-
/*
* XXX Functions that ought to be in libkrb5.a, but aren't.
*/
@@ -144,13 +89,11 @@ static void cleanup_key_data(context, count, data)
int count;
krb5_key_data * data;
{
- int i, j;
+ int i;
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);
+ krb5_free_key_data_contents(context, &data[i]);
+ free(data);
}
/* Check whether a ks_tuple is present in an array of ks_tuples. */
@@ -408,10 +351,9 @@ kadm5_create_principal_3(void *server_handle,
return ret;
}
- kdb = krb5_db_alloc(handle->context, NULL, sizeof(*kdb));
+ kdb = calloc(1, sizeof(*kdb));
if (kdb == NULL)
return ENOMEM;
- memset(kdb, 0, sizeof(*kdb));
memset(&adb, 0, sizeof(osa_princ_ent_rec));
/*
@@ -478,8 +420,8 @@ kadm5_create_principal_3(void *server_handle,
to free the entire kdb entry, and that will try to free the
principal. */
- if ((ret = kadm5_copy_principal(handle->context,
- entry->principal, &(kdb->princ))))
+ ret = krb5_copy_principal(handle->context, entry->principal, &kdb->princ);
+ if (ret)
goto cleanup;
if ((ret = krb5_dbe_update_last_pwd_change(handle->context, kdb, now)))
@@ -1752,7 +1694,7 @@ kadm5_setv4key_principal(void *server_handle,
krb5_int32 now;
kadm5_policy_ent_rec pol;
krb5_keysalt keysalt;
- int i, k, kvno, ret;
+ int i, kvno, ret;
krb5_boolean have_pol = FALSE;
#if 0
int last_pwd;
@@ -1787,10 +1729,9 @@ kadm5_setv4key_principal(void *server_handle,
if (kdb->key_data != NULL)
cleanup_key_data(handle->context, kdb->n_key_data, kdb->key_data);
- kdb->key_data = (krb5_key_data*)krb5_db_alloc(handle->context, NULL, sizeof(krb5_key_data));
+ kdb->key_data = calloc(1, sizeof(krb5_key_data));
if (kdb->key_data == NULL)
return ENOMEM;
- memset(kdb->key_data, 0, sizeof(krb5_key_data));
kdb->n_key_data = 1;
keysalt.type = KRB5_KDB_SALTTYPE_V4;
/* XXX data.magic? */
@@ -1803,33 +1744,11 @@ kadm5_setv4key_principal(void *server_handle,
/* use tmp_key_data as temporary location and reallocate later */
ret = krb5_dbe_encrypt_key_data(handle->context, act_mkey, keyblock,
- &keysalt, kvno + 1, &tmp_key_data);
+ &keysalt, kvno + 1, kdb->key_data);
if (ret) {
goto done;
}
- for (k = 0; k < tmp_key_data.key_data_ver; k++) {
- kdb->key_data->key_data_type[k] = tmp_key_data.key_data_type[k];
- kdb->key_data->key_data_length[k] = tmp_key_data.key_data_length[k];
- if (tmp_key_data.key_data_contents[k]) {
- kdb->key_data->key_data_contents[k] = krb5_db_alloc(handle->context, NULL, tmp_key_data.key_data_length[k]);
- if (kdb->key_data->key_data_contents[k] == NULL) {
- cleanup_key_data(handle->context, kdb->n_key_data, kdb->key_data);
- kdb->key_data = NULL;
- kdb->n_key_data = 0;
- ret = ENOMEM;
- goto done;
- }
- memcpy (kdb->key_data->key_data_contents[k], tmp_key_data.key_data_contents[k], tmp_key_data.key_data_length[k]);
-
- memset (tmp_key_data.key_data_contents[k], 0, tmp_key_data.key_data_length[k]);
- free (tmp_key_data.key_data_contents[k]);
- tmp_key_data.key_data_contents[k] = NULL;
- }
- }
-
-
-
kdb->attributes &= ~KRB5_KDB_REQUIRES_PWCHANGE;
ret = krb5_timeofday(handle->context, &now);
@@ -2053,14 +1972,12 @@ kadm5_setkey_principal_4(void *server_handle, krb5_principal principal,
}
n_new_key_data = n_key_data + (keepold ? kdb->n_key_data : 0);
- new_key_data = krb5_db_alloc(handle->context, NULL,
- n_new_key_data * sizeof(krb5_key_data));
+ new_key_data = calloc(n_new_key_data, sizeof(krb5_key_data));
if (new_key_data == NULL) {
n_new_key_data = 0;
ret = ENOMEM;
goto done;
}
- memset(new_key_data, 0, n_new_key_data * sizeof(krb5_key_data));
n_new_key_data = 0;
for (i = 0; i < n_key_data; i++) {
@@ -2361,8 +2278,7 @@ kadm5_purgekeys(void *server_handle,
n_old_keydata = kdb->n_key_data;
kdb->n_key_data = 0;
/* Allocate one extra key_data to avoid allocating 0 bytes. */
- kdb->key_data = krb5_db_alloc(handle->context, NULL,
- (n_old_keydata + 1) * sizeof(krb5_key_data));
+ kdb->key_data = calloc(n_old_keydata, sizeof(krb5_key_data));
if (kdb->key_data == NULL) {
ret = ENOMEM;
goto done;
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);
diff --git a/src/plugins/kdb/ldap/ldap_util/kdb5_ldap_realm.c b/src/plugins/kdb/ldap/ldap_util/kdb5_ldap_realm.c
index 4d30700..e95791f 100644
--- a/src/plugins/kdb/ldap/ldap_util/kdb5_ldap_realm.c
+++ b/src/plugins/kdb/ldap/ldap_util/kdb5_ldap_realm.c
@@ -1129,11 +1129,6 @@ krb5_dbe_update_tl_data_new(krb5_context context, krb5_db_entry *entry,
/* 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)
-*/
if ((tmp = (krb5_octet *) malloc (new_tl_data->tl_data_length)) == NULL)
return (ENOMEM);
@@ -1150,12 +1145,6 @@ krb5_dbe_update_tl_data_new(krb5_context context, krb5_db_entry *entry,
/* if necessary, chain a new record in the beginning and point at it */
if (!tl_data) {
-/*
- if ((tl_data =
- (krb5_tl_data *) krb5_db_alloc(context, NULL,
- sizeof(krb5_tl_data)))
- == NULL) {
-*/
if ((tl_data = (krb5_tl_data *) malloc (sizeof(krb5_tl_data))) == NULL) {
free(tmp);
return (ENOMEM);
@@ -1168,8 +1157,7 @@ krb5_dbe_update_tl_data_new(krb5_context context, krb5_db_entry *entry,
/* 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;
diff --git a/src/tests/create/kdb5_mkdums.c b/src/tests/create/kdb5_mkdums.c
index 1267270..622f549 100644
--- a/src/tests/create/kdb5_mkdums.c
+++ b/src/tests/create/kdb5_mkdums.c
@@ -218,12 +218,11 @@ add_princ(context, str_newprinc)
krb5_db_entry *newentry;
char princ_name[4096];
- newentry = krb5_db_alloc(context, NULL, sizeof(*newentry));
+ newentry = calloc(1, sizeof(*newentry));
if (newentry == NULL) {
com_err(progname, ENOMEM, "while allocating DB entry");
return;
}
- memset(newentry, 0, sizeof(*newentry));
snprintf(princ_name, sizeof(princ_name), "%s@%s", str_newprinc, cur_realm);
if ((retval = krb5_parse_name(context, princ_name, &newprinc))) {
com_err(progname, retval, "while parsing '%s'", princ_name);
_______________________________________________
cvs-krb5 mailing list
cvs-krb5@mit.edu
https://mailman.mit.edu/mailman/listinfo/cvs-krb5
