svn rev #21797: branches/mkey_migrate/src/ include/ include/krb5/ kadmin/dbutil/ ...
wfiveash@MIT.EDU
wfiveash at MIT.EDU
Mon Jan 26 14:24:06 EST 2009
http://src.mit.edu/fisheye/changelog/krb5/?cs=21797
Commit By: wfiveash
Log Message:
Work to address some of Ken's review comments. This doesn't address all
of his issues so there will be a follow up commit.
The type krb5_keylist_node shouldn't go into krb5.hin, as it's not part of
the library (or any other) public API. Maybe k5-int.h as a catch-all, if
there's not a more appropriate internal header?
Done.
Can we avoid moving krb5_free_key_data_contents, which deals with a data
structure used only in the KDC-related libraries, into libkrb5 and
k5-int.h? (Exception: The libkrb5 asn.1 code does encode/decode the data
structure and thus may allocate it. But I think we can assume the same C
runtime for kadm5srv/kdb and krb5 libs, so it's kind of okay. And the
asn.1 setup should be "modularized" at some point, so the ldap support can
move out into the ldap kdb plugin.) I think it can probably go into
libkdb?
Done.
If possible, k5-int.h shouldn't include kdb.h, so updating kdb.h doesn't
cause recompilation of (for example) all of the crypto library code.
Done.
After printing "master keys for principal", if enctype_to_string fails, we
haven't set retval to the error code but use it anyways. Later, asprintf
isn't checked for failure.
Done.
Some cases of indentation not matching MIT style, in particular,
continuation lines in function calls being indented four columns instead of
indented to make function arguments line up.
Done.
krb5_dbe_lookup_mkvno, krb5_dbe_lookup_mkey_aux, krb5_dbe_lookup_actkvno
need to verify lengths before decoding data.
Done.
kdb5_add_mkey should use the "zap" macro on key data instead of memset
before directly freeing it; some compilers (one reference I found mentions
the Microsoft C++ .NET compiler) may optimize away scribbles over storage
about to be freed, leaving the values to be retained in core dumps or
uninitialized heap allocations, and "zap" is intended to be where we dump
any necessary hacks to defeat that. Similarly for any other places where
key data is stored (e.g., within tl_data).
Done.
krb5_dbe_update_actkvno (and probably elsewhere in our existing code): Note
that failure in realloc (NULL return when size is nonzero) leaves the old
storage un-freed. So "x=realloc(x,sz)" is a good way to leak memory if
reallocation fails, since you no longer have a handle on the orignial "x".
Done.
Changed Files:
U branches/mkey_migrate/src/include/k5-int.h
U branches/mkey_migrate/src/include/kdb.h
U branches/mkey_migrate/src/include/krb5/krb5.hin
U branches/mkey_migrate/src/kadmin/dbutil/kdb5_mkey.c
U branches/mkey_migrate/src/kdc/extern.c
U branches/mkey_migrate/src/lib/kadm5/clnt/Makefile.in
U branches/mkey_migrate/src/lib/kdb/kdb5.c
U branches/mkey_migrate/src/lib/kdb/kdb_default.c
U branches/mkey_migrate/src/lib/kdb/libkdb5.exports
U branches/mkey_migrate/src/lib/krb5/krb/kfree.c
U branches/mkey_migrate/src/lib/krb5/libkrb5.exports
Modified: branches/mkey_migrate/src/include/k5-int.h
===================================================================
--- branches/mkey_migrate/src/include/k5-int.h 2009-01-26 19:06:21 UTC (rev 21796)
+++ branches/mkey_migrate/src/include/k5-int.h 2009-01-26 19:24:03 UTC (rev 21797)
@@ -1170,11 +1170,6 @@
void KRB5_CALLCONV krb5_free_etype_list
(krb5_context, krb5_etype_list * );
-#include "kdb.h"
-
-void KRB5_CALLCONV krb5_free_key_data_contents
- (krb5_context, krb5_key_data *);
-
/* #include "krb5/wordsize.h" -- comes in through base-defs.h. */
#include "com_err.h"
#include "k5-plugin.h"
Modified: branches/mkey_migrate/src/include/kdb.h
===================================================================
--- branches/mkey_migrate/src/include/kdb.h 2009-01-26 19:06:21 UTC (rev 21796)
+++ branches/mkey_migrate/src/include/kdb.h 2009-01-26 19:24:03 UTC (rev 21797)
@@ -195,6 +195,12 @@
krb5_key_data latest_mkey; /* most recent mkey */
} krb5_mkey_aux_node;
+typedef struct _krb5_keylist_node {
+ krb5_keyblock keyblock;
+ krb5_kvno kvno;
+ struct _krb5_keylist_node *next;
+} krb5_keylist_node;
+
/*
* Determines the number of failed KDC requests before DISALLOW_ALL_TIX is set
* on the principal.
@@ -640,7 +646,6 @@
osa_policy_ent_t policy);
-
krb5_error_code
krb5_db_set_context
(krb5_context, void *db_context);
@@ -649,6 +654,9 @@
krb5_db_get_context
(krb5_context, void **db_context);
+void
+krb5_free_key_data_contents(krb5_context, krb5_key_data *);
+
#define KRB5_KDB_DEF_FLAGS 0
#define KDB_MAX_DB_NAME 128
Modified: branches/mkey_migrate/src/include/krb5/krb5.hin
===================================================================
--- branches/mkey_migrate/src/include/krb5/krb5.hin 2009-01-26 19:06:21 UTC (rev 21796)
+++ branches/mkey_migrate/src/include/krb5/krb5.hin 2009-01-26 19:24:03 UTC (rev 21797)
@@ -348,12 +348,6 @@
krb5_octet *contents;
} krb5_keyblock;
-typedef struct _krb5_keylist_node {
- krb5_keyblock keyblock;
- krb5_kvno kvno;
- struct _krb5_keylist_node *next;
-} krb5_keylist_node;
-
#ifdef KRB5_OLD_CRYPTO
typedef struct _krb5_encrypt_block {
krb5_magic magic;
Modified: branches/mkey_migrate/src/kadmin/dbutil/kdb5_mkey.c
===================================================================
--- branches/mkey_migrate/src/kadmin/dbutil/kdb5_mkey.c 2009-01-26 19:06:21 UTC (rev 21796)
+++ branches/mkey_migrate/src/kadmin/dbutil/kdb5_mkey.c 2009-01-26 19:24:03 UTC (rev 21797)
@@ -1,5 +1,5 @@
/*
- * Copyright 2008 Sun Microsystems, Inc. All rights reserved.
+ * Copyright 2009 Sun Microsystems, Inc. All rights reserved.
* Use is subject to license terms.
*/
@@ -255,8 +255,6 @@
}
}
- /* XXX WAF: debug printf, remove before final commit */
- /* printf("i = %d old_key_data_count = %d\n", i, old_key_data_count); */
assert(i == old_key_data_count + 1);
if ((retval = krb5_dbe_update_mkey_aux(util_context, &master_entry,
@@ -300,19 +298,19 @@
}
/* clean up */
(void) krb5_db_fini(util_context);
- memset((char *)master_keyblock.contents, 0, master_keyblock.length);
+ zap((char *)master_keyblock.contents, master_keyblock.length);
free(master_keyblock.contents);
- memset((char *)new_master_keyblock.contents, 0, new_master_keyblock.length);
+ zap((char *)new_master_keyblock.contents, new_master_keyblock.length);
free(new_master_keyblock.contents);
if (pw_str) {
- memset(pw_str, 0, pw_size);
+ zap(pw_str, pw_size);
free(pw_str);
}
free(master_salt.data);
free(mkey_fullname);
+
for (cur_mkey_aux_data = mkey_aux_data_head; cur_mkey_aux_data != NULL;
- cur_mkey_aux_data = next_mkey_aux_data) {
-
+ cur_mkey_aux_data = next_mkey_aux_data) {
next_mkey_aux_data = cur_mkey_aux_data->next;
krb5_free_key_data_contents(util_context, &(cur_mkey_aux_data->latest_mkey));
free(cur_mkey_aux_data);
@@ -511,9 +509,9 @@
/* assemble & parse the master key name */
if ((retval = krb5_db_setup_mkey_name(util_context,
- global_params.mkey_name,
- global_params.realm,
- &mkey_fullname, &master_princ))) {
+ global_params.mkey_name,
+ global_params.realm,
+ &mkey_fullname, &master_princ))) {
com_err(progname, retval, "while setting up master key name");
exit_status++;
return;
@@ -551,8 +549,8 @@
for (cur_kb_node = master_keylist; cur_kb_node != NULL;
cur_kb_node = cur_kb_node->next) {
- if (krb5_enctype_to_string(cur_kb_node->keyblock.enctype,
- enctype, sizeof(enctype))) {
+ if ((retval = krb5_enctype_to_string(cur_kb_node->keyblock.enctype,
+ enctype, sizeof(enctype)))) {
com_err(progname, retval, "while getting enctype description");
exit_status++;
return;
@@ -580,18 +578,26 @@
}
if (cur_kb_node->kvno == act_kvno) {
- asprintf(&output_str, "KNVO: %d, Enctype: %s, Active on: %s *\n",
- cur_kb_node->kvno, enctype, strdate(act_time));
+ /* * indicates kvno is currently active */
+ retval = asprintf(&output_str, "KNVO: %d, Enctype: %s, Active on: %s *\n",
+ cur_kb_node->kvno, enctype, strdate(act_time));
} else {
if (act_time) {
- asprintf(&output_str, "KNVO: %d, Enctype: %s, Active on: %s\n",
- cur_kb_node->kvno, enctype, strdate(act_time));
+ retval = asprintf(&output_str, "KNVO: %d, Enctype: %s, Active on: %s\n",
+ cur_kb_node->kvno, enctype, strdate(act_time));
} else {
- asprintf(&output_str, "KNVO: %d, Enctype: %s, No activate time set\n",
- cur_kb_node->kvno, enctype);
+ retval = asprintf(&output_str, "KNVO: %d, Enctype: %s, No activate time set\n",
+ cur_kb_node->kvno, enctype);
}
}
+ if (retval == -1) {
+ com_err(progname, ENOMEM, "asprintf could not allocate enough memory to hold output");
+ exit_status++;
+ return;
+ }
printf("%s", output_str);
+ free(output_str);
+ output_str = NULL;
}
/* clean up */
Modified: branches/mkey_migrate/src/kdc/extern.c
===================================================================
--- branches/mkey_migrate/src/kdc/extern.c 2009-01-26 19:06:21 UTC (rev 21796)
+++ branches/mkey_migrate/src/kdc/extern.c 2009-01-26 19:24:03 UTC (rev 21797)
@@ -27,6 +27,7 @@
*/
#include "k5-int.h"
+#include "kdb.h"
#include "extern.h"
/* real declarations of KDC's externs */
Modified: branches/mkey_migrate/src/lib/kadm5/clnt/Makefile.in
===================================================================
--- branches/mkey_migrate/src/lib/kadm5/clnt/Makefile.in 2009-01-26 19:06:21 UTC (rev 21796)
+++ branches/mkey_migrate/src/lib/kadm5/clnt/Makefile.in 2009-01-26 19:24:03 UTC (rev 21797)
@@ -15,7 +15,8 @@
$(TOPLIBD)/libkrb5$(SHLIBEXT) \
$(TOPLIBD)/libk5crypto$(SHLIBEXT) \
$(COM_ERR_DEPLIB) $(SUPPORT_LIBDEP)
-SHLIB_EXPLIBS=-lgssrpc -lgssapi_krb5 -lkrb5 -lk5crypto $(SUPPORT_LIB) -lcom_err
+SHLIB_EXPLIBS=-lgssrpc -lgssapi_krb5 -lkrb5 -lkdb5 -lk5crypto $(SUPPORT_LIB) \
+ -lcom_err
SHLIB_DIRS=-L$(TOPLIBD)
SHLIB_RDIRS=$(KRB5_LIBDIR)
RELDIR=kadm5/clnt
Modified: branches/mkey_migrate/src/lib/kdb/kdb5.c
===================================================================
--- branches/mkey_migrate/src/lib/kdb/kdb5.c 2009-01-26 19:06:21 UTC (rev 21796)
+++ branches/mkey_migrate/src/lib/kdb/kdb5.c 2009-01-26 19:24:03 UTC (rev 21797)
@@ -1,5 +1,5 @@
/*
- * Copyright 2006, 2008 by the Massachusetts Institute of Technology.
+ * Copyright 2006, 2009 by the Massachusetts Institute of Technology.
* All Rights Reserved.
*
* Export of this software from the United States of America may
@@ -23,6 +23,11 @@
*/
/*
+ * Copyright 2009 Sun Microsystems, Inc. All rights reserved.
+ * Use is subject to license terms.
+ */
+
+/*
* This code was based on code donated to MIT by Novell for
* distribution under the MIT license.
*/
@@ -107,9 +112,9 @@
krb5_actkvno_node *temp, *prev;
for (temp = val; temp != NULL;) {
- prev = temp;
- temp = temp->next;
- krb5_xfree(prev);
+ prev = temp;
+ temp = temp->next;
+ krb5_xfree(prev);
}
}
@@ -119,13 +124,29 @@
krb5_mkey_aux_node *temp, *prev;
for (temp = val; temp != NULL;) {
- prev = temp;
- temp = temp->next;
- krb5_free_key_data_contents(context, &prev->latest_mkey);
- krb5_xfree(prev);
+ prev = temp;
+ temp = temp->next;
+ krb5_free_key_data_contents(context, &prev->latest_mkey);
+ krb5_xfree(prev);
}
}
+void
+krb5_free_key_data_contents(krb5_context context,
+ krb5_key_data *key)
+{
+ int i, idx;
+
+ idx = (key->key_data_ver == 1 ? 1 : 2);
+ for (i = 0; i < idx; i++) {
+ if (key->key_data_contents[i]) {
+ zap(key->key_data_contents[i], key->key_data_length[i]);
+ free(key->key_data_contents[i]);
+ }
+ }
+ return;
+}
+
#define kdb_init_lib_lock(a) 0
#define kdb_destroy_lib_lock(a) (void)0
#define kdb_lock_lib_lock(a, b) 0
@@ -1684,7 +1705,7 @@
if (!salt)
krb5_xfree(scratch.data);
- memset(password, 0, sizeof(password)); /* erase it */
+ zap(password, sizeof(password)); /* erase it */
} else {
kdb5_dal_handle *dal_handle;
@@ -1731,7 +1752,7 @@
clean_n_exit:
if (tmp_key.contents) {
- memset(tmp_key.contents, 0, tmp_key.length);
+ zap(tmp_key.contents, tmp_key.length);
krb5_db_free(context, tmp_key.contents);
}
return retval;
@@ -2211,6 +2232,8 @@
if (tl_data.tl_data_length == 0) {
*mkvno = 1; /* default for princs that lack the KRB5_TL_MKVNO data */
return (0);
+ } else if (tl_data.tl_data_length != 2) {
+ return (KRB5_KDB_TRUNCATED_RECORD);
}
krb5_kdb_decode_int16(tl_data.tl_data_contents, tmp);
@@ -2259,6 +2282,10 @@
krb5_kdb_decode_int16(tl_data.tl_data_contents, version);
if (version == KRB5_TL_MKEY_AUX_VER_1) {
+ /* variable size, must be at least 10 bytes */
+ if (tl_data.tl_data_length < 10)
+ return (KRB5_KDB_TRUNCATED_RECORD);
+
/* curloc points to first tuple entry in the tl_data_contents */
curloc = tl_data.tl_data_contents + sizeof(version);
@@ -2413,6 +2440,11 @@
/* get version to determine how to parse the data */
krb5_kdb_decode_int16(tl_data.tl_data_contents, version);
if (version == KRB5_TL_ACTKVNO_VER_1) {
+
+ /* variable size, must be at least 8 bytes */
+ if (tl_data.tl_data_length < 8)
+ return (KRB5_KDB_TRUNCATED_RECORD);
+
/*
* Find number of tuple entries, remembering to account for version
* field.
@@ -2466,6 +2498,7 @@
krb5_tl_data new_tl_data;
unsigned char *nextloc;
const krb5_actkvno_node *cur_actkvno;
+ krb5_octet *tmpptr;
if (actkvno_list == NULL) {
return (EINVAL);
@@ -2484,11 +2517,15 @@
for (cur_actkvno = actkvno_list; cur_actkvno != NULL;
cur_actkvno = cur_actkvno->next) {
+
new_tl_data.tl_data_length += ACTKVNO_TUPLE_SIZE;
- new_tl_data.tl_data_contents = (krb5_octet *) realloc(new_tl_data.tl_data_contents,
- new_tl_data.tl_data_length);
- if (new_tl_data.tl_data_contents == NULL)
+ tmpptr = realloc(new_tl_data.tl_data_contents, new_tl_data.tl_data_length);
+ if (tmpptr == NULL) {
+ free(new_tl_data.tl_data_contents);
return (ENOMEM);
+ } else {
+ new_tl_data.tl_data_contents = tmpptr;
+ }
/*
* Using realloc so tl_data_contents is required to correctly calculate
Modified: branches/mkey_migrate/src/lib/kdb/kdb_default.c
===================================================================
--- branches/mkey_migrate/src/lib/kdb/kdb_default.c 2009-01-26 19:06:21 UTC (rev 21796)
+++ branches/mkey_migrate/src/lib/kdb/kdb_default.c 2009-01-26 19:24:03 UTC (rev 21797)
@@ -1,7 +1,7 @@
/*
* lib/kdb/kdb_helper.c
*
- * Copyright 1995, 2008 by the Massachusetts Institute of Technology.
+ * Copyright 1995, 2009 by the Massachusetts Institute of Technology.
* All Rights Reserved.
*
* Export of this software from the United States of America may
Modified: branches/mkey_migrate/src/lib/kdb/libkdb5.exports
===================================================================
--- branches/mkey_migrate/src/lib/kdb/libkdb5.exports 2009-01-26 19:06:21 UTC (rev 21796)
+++ branches/mkey_migrate/src/lib/kdb/libkdb5.exports 2009-01-26 19:24:03 UTC (rev 21797)
@@ -66,6 +66,7 @@
krb5_db_free_policy
krb5_def_store_mkey
krb5_db_promote
+krb5_free_key_data_contents
ulog_map
ulog_set_role
ulog_free_entries
Modified: branches/mkey_migrate/src/lib/krb5/krb/kfree.c
===================================================================
--- branches/mkey_migrate/src/lib/krb5/krb/kfree.c 2009-01-26 19:06:21 UTC (rev 21796)
+++ branches/mkey_migrate/src/lib/krb5/krb/kfree.c 2009-01-26 19:24:03 UTC (rev 21797)
@@ -810,20 +810,3 @@
krb5_xfree(etypes);
}
}
-
-void KRB5_CALLCONV
-krb5_free_key_data_contents(krb5_context context,
- krb5_key_data *key)
-{
- int i, idx;
-
- idx = (key->key_data_ver == 1 ? 1 : 2);
- for (i = 0; i < idx; i++) {
- if (key->key_data_contents[i]) {
- memset(key->key_data_contents[i], 0, key->key_data_length[i]);
- free(key->key_data_contents[i]);
- }
- }
- return;
-}
-
Modified: branches/mkey_migrate/src/lib/krb5/libkrb5.exports
===================================================================
--- branches/mkey_migrate/src/lib/krb5/libkrb5.exports 2009-01-26 19:06:21 UTC (rev 21796)
+++ branches/mkey_migrate/src/lib/krb5/libkrb5.exports 2009-01-26 19:24:03 UTC (rev 21797)
@@ -221,7 +221,6 @@
krb5_free_host_realm
krb5_free_kdc_rep
krb5_free_kdc_req
-krb5_free_key_data_contents
krb5_free_keyblock
krb5_free_keyblock_contents
krb5_free_keytab_entry_contents
More information about the cvs-krb5
mailing list