From: Greg Hudson <ghudson@mit.edu>
Date: Mon, 25 Aug 2014 23:14:50 +0000 (-0400)
Subject: Simplify kdb_cpw.c
X-Git-Tag: krb5-1.14-alpha1~251
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=759a8e1001ac21161d26eb1dc80c6601fb379964;p=thirdparty%2Fkrb5.git

Simplify kdb_cpw.c

In add_key_rnd, stop looking up the krbtgt DB entry; we have not used
it since 1.1.

Use copy_key_data in add_key_rnd and add_key_pwd.

krb5_dbe_crk, krb5_dbe_ark, krb5_dbe_def_cpw, and krb5_dbe_apw all
contained similar logic.  Consolidate all of them into a static helper
function which does the work of all four.  The ark/apw variants had
slightly different behavior then crk/cpw with keepold=true, so
introduce a three-value enum to express all three behaviors.
---

diff --git a/src/lib/kdb/kdb_cpw.c b/src/lib/kdb/kdb_cpw.c
index d45e251c01..a9ec509260 100644
--- a/src/lib/kdb/kdb_cpw.c
+++ b/src/lib/kdb/kdb_cpw.c
@@ -54,6 +54,8 @@
 #include <stdio.h>
 #include <errno.h>
 
+enum save { DISCARD_ALL, KEEP_LAST_KVNO, KEEP_ALL };
+
 int
 krb5_db_get_key_data_kvno(context, count, data)
     krb5_context          context;
@@ -195,40 +197,10 @@ add_key_rnd(context, master_key, ks_tuple, ks_tuple_count, db_entry, kvno)
     krb5_db_entry       * db_entry;
     int                   kvno;
 {
-    krb5_principal        krbtgt_princ;
     krb5_keyblock         key;
-    krb5_db_entry         *krbtgt_entry;
-    int                   max_kvno, i, j, k;
+    int                   i, j;
     krb5_error_code       retval;
     krb5_key_data         tmp_key_data;
-    krb5_key_data        *tptr;
-
-    memset( &tmp_key_data, 0, sizeof(tmp_key_data));
-
-
-    retval = krb5_build_principal_ext(context, &krbtgt_princ,
-                                      db_entry->princ->realm.length,
-                                      db_entry->princ->realm.data,
-                                      KRB5_TGS_NAME_SIZE,
-                                      KRB5_TGS_NAME,
-                                      db_entry->princ->realm.length,
-                                      db_entry->princ->realm.data,
-                                      0);
-    if (retval)
-        return retval;
-
-    /* Get tgt from database */
-    retval = krb5_db_get_principal(context, krbtgt_princ, 0, &krbtgt_entry);
-    krb5_free_principal(context, krbtgt_princ); /* don't need it anymore */
-    if (retval)
-        return(retval);
-
-    /* Get max kvno */
-    for (max_kvno = j = 0; j < krbtgt_entry->n_key_data; j++) {
-        if (max_kvno < krbtgt_entry->key_data[j].key_data_kvno) {
-            max_kvno = krbtgt_entry->key_data[j].key_data_kvno;
-        }
-    }
 
     for (i = 0; i < ks_tuple_count; i++) {
         krb5_boolean similar;
@@ -266,10 +238,7 @@ add_key_rnd(context, master_key, ks_tuple, ks_tuple_count, db_entry, kvno)
                                              &key)))
             goto add_key_rnd_err;
 
-
-        /* db library will free this. Since, its a so, it could actually be using different memory management
-           function. So, its better if the memory is allocated by the db's malloc. So, a temporary memory is used
-           here which will later be copied to the db_entry */
+        memset( &tmp_key_data, 0, sizeof(tmp_key_data));
         retval = krb5_dbe_encrypt_key_data(context, master_key, &key, NULL,
                                            kvno, &tmp_key_data);
 
@@ -277,143 +246,18 @@ add_key_rnd(context, master_key, ks_tuple, ks_tuple_count, db_entry, kvno)
         if( retval )
             goto add_key_rnd_err;
 
-        tptr = &db_entry->key_data[db_entry->n_key_data-1];
-
-        tptr->key_data_ver = tmp_key_data.key_data_ver;
-        tptr->key_data_kvno = tmp_key_data.key_data_kvno;
-
-        for( k = 0; k < tmp_key_data.key_data_ver; k++ )
-        {
-            tptr->key_data_type[k] = tmp_key_data.key_data_type[k];
-            tptr->key_data_length[k] = tmp_key_data.key_data_length[k];
-            if( tmp_key_data.key_data_contents[k] )
-            {
-                tptr->key_data_contents[k] = krb5_db_alloc(context, NULL, tmp_key_data.key_data_length[k]);
-                if( tptr->key_data_contents[k] == NULL )
-                {
-                    cleanup_key_data(context, db_entry->n_key_data, db_entry->key_data);
-                    db_entry->key_data = NULL;
-                    db_entry->n_key_data = 0;
-                    retval = ENOMEM;
-                    goto add_key_rnd_err;
-                }
-                memcpy( tptr->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;
-            }
-        }
-
+        /* 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)
+            goto add_key_rnd_err;
     }
 
 add_key_rnd_err:
-    krb5_db_free_principal(context, krbtgt_entry);
-
-    for( i = 0; i < tmp_key_data.key_data_ver; i++ )
-    {
-        if( tmp_key_data.key_data_contents[i] )
-        {
-            memset( tmp_key_data.key_data_contents[i], 0, tmp_key_data.key_data_length[i]);
-            free( tmp_key_data.key_data_contents[i] );
-        }
-    }
     return(retval);
 }
 
-/*
- * Change random key for a krb5_db_entry
- * Assumes the max kvno
- *
- * As a side effect all old keys are nuked if keepold is false.
- */
-krb5_error_code
-krb5_dbe_crk(context, master_key, ks_tuple, ks_tuple_count, keepold, db_entry)
-    krb5_context          context;
-    krb5_keyblock       * master_key;
-    krb5_key_salt_tuple * ks_tuple;
-    int                   ks_tuple_count;
-    krb5_boolean          keepold;
-    krb5_db_entry       * db_entry;
-{
-    int                   key_data_count;
-    krb5_key_data       * key_data;
-    krb5_error_code       retval;
-    int                   kvno;
-
-    /* First save the old keydata */
-    kvno = krb5_db_get_key_data_kvno(context, db_entry->n_key_data,
-                                     db_entry->key_data);
-    key_data_count = db_entry->n_key_data;
-    key_data = db_entry->key_data;
-    db_entry->key_data = NULL;
-    db_entry->n_key_data = 0;
-
-    /* increment the kvno */
-    kvno++;
-
-    retval = add_key_rnd(context, master_key, ks_tuple,
-                         ks_tuple_count, db_entry, kvno);
-    if (retval) {
-        cleanup_key_data(context, db_entry->n_key_data, db_entry->key_data);
-        db_entry->n_key_data = key_data_count;
-        db_entry->key_data = key_data;
-        return retval;
-    }
-
-    if (keepold) {
-        retval = preserve_old_keys(context, master_key, db_entry, 0,
-                                   key_data_count, key_data);
-    }
-    cleanup_key_data(context, key_data_count, key_data);
-    return retval;
-}
-
-/*
- * Add random key for a krb5_db_entry
- * Assumes the max kvno
- *
- * As a side effect all old keys older than the max kvno are nuked.
- */
-krb5_error_code
-krb5_dbe_ark(context, master_key, ks_tuple, ks_tuple_count, db_entry)
-    krb5_context          context;
-    krb5_keyblock       * master_key;
-    krb5_key_salt_tuple * ks_tuple;
-    int                   ks_tuple_count;
-    krb5_db_entry       * db_entry;
-{
-    int                   key_data_count;
-    krb5_key_data       * key_data;
-    krb5_error_code       retval;
-    int                   kvno;
-
-    /* First save the old keydata */
-    kvno = krb5_db_get_key_data_kvno(context, db_entry->n_key_data,
-                                     db_entry->key_data);
-    key_data_count = db_entry->n_key_data;
-    key_data = db_entry->key_data;
-    db_entry->key_data = NULL;
-    db_entry->n_key_data = 0;
-
-    /* increment the kvno */
-    kvno++;
-
-    if ((retval = add_key_rnd(context, master_key, ks_tuple,
-                              ks_tuple_count, db_entry, kvno))) {
-        cleanup_key_data(context, db_entry->n_key_data, db_entry->key_data);
-        db_entry->n_key_data = key_data_count;
-        db_entry->key_data = key_data;
-        return retval;
-    }
-
-    /* Preserve only the most recent kvno. */
-    retval = preserve_old_keys(context, master_key, db_entry, kvno - 1,
-                               key_data_count, key_data);
-    cleanup_key_data(context, key_data_count, key_data);
-    return retval;
-}
-
 /* Construct a random explicit salt. */
 static krb5_error_code
 make_random_salt(krb5_context context, krb5_keysalt *salt_out)
@@ -465,13 +309,8 @@ add_key_pwd(context, master_key, ks_tuple, ks_tuple_count, passwd,
     krb5_keyblock         key;
     krb5_data             pwd;
     krb5_data             afs_params = string2data("\1"), *s2k_params;
-    int                   i, j, k;
+    int                   i, j;
     krb5_key_data         tmp_key_data;
-    krb5_key_data        *tptr;
-
-    memset( &tmp_key_data, 0, sizeof(tmp_key_data));
-
-    retval = 0;
 
     for (i = 0; i < ks_tuple_count; i++) {
         krb5_boolean similar;
@@ -557,8 +396,7 @@ add_key_pwd(context, master_key, ks_tuple, ks_tuple_count, passwd,
             return retval;
         }
 
-        /* memory allocation to be done by db. So, use temporary block and later copy
-           it to the memory allocated by db */
+        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);
@@ -569,142 +407,120 @@ add_key_pwd(context, master_key, ks_tuple, ks_tuple_count, passwd,
         if( retval )
             return retval;
 
-        tptr = &db_entry->key_data[db_entry->n_key_data-1];
-
-        tptr->key_data_ver = tmp_key_data.key_data_ver;
-        tptr->key_data_kvno = tmp_key_data.key_data_kvno;
-
-        for( k = 0; k < tmp_key_data.key_data_ver; k++ )
-        {
-            tptr->key_data_type[k] = tmp_key_data.key_data_type[k];
-            tptr->key_data_length[k] = tmp_key_data.key_data_length[k];
-            if( tmp_key_data.key_data_contents[k] )
-            {
-                tptr->key_data_contents[k] = krb5_db_alloc(context, NULL, tmp_key_data.key_data_length[k]);
-                if( tptr->key_data_contents[k] == NULL )
-                {
-                    cleanup_key_data(context, db_entry->n_key_data, db_entry->key_data);
-                    db_entry->key_data = NULL;
-                    db_entry->n_key_data = 0;
-                    retval = ENOMEM;
-                    goto add_key_pwd_err;
-                }
-                memcpy( tptr->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;
-            }
-        }
-    }
-add_key_pwd_err:
-    for( i = 0; i < tmp_key_data.key_data_ver; i++ )
-    {
-        if( tmp_key_data.key_data_contents[i] )
-        {
-            memset( tmp_key_data.key_data_contents[i], 0, tmp_key_data.key_data_length[i]);
-            free( tmp_key_data.key_data_contents[i] );
-        }
+        /* 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(retval);
+    return 0;
 }
 
-/*
- * Change password for a krb5_db_entry
- * Assumes the max kvno
- *
- * As a side effect all old keys are nuked if keepold is false.
- */
-krb5_error_code
-krb5_dbe_def_cpw(context, master_key, ks_tuple, ks_tuple_count, passwd,
-                 new_kvno, keepold, db_entry)
-    krb5_context          context;
-    krb5_keyblock       * master_key;
-    krb5_key_salt_tuple * ks_tuple;
-    int                   ks_tuple_count;
-    char                * passwd;
-    int                   new_kvno;
-    krb5_boolean          keepold;
-    krb5_db_entry       * db_entry;
+static krb5_error_code
+rekey(krb5_context context, krb5_keyblock *mkey, krb5_key_salt_tuple *ks_tuple,
+      int ks_tuple_count, const char *password, int new_kvno,
+      enum save savekeys, krb5_db_entry *db_entry)
 {
-    int                   key_data_count;
-    krb5_key_data       * key_data;
-    krb5_error_code       retval;
-    int                   old_kvno;
+    krb5_error_code ret;
+    krb5_key_data *key_data;
+    int n_key_data, old_kvno, save_kvno;
 
-    /* First save the old keydata */
-    old_kvno = krb5_db_get_key_data_kvno(context, db_entry->n_key_data,
-                                         db_entry->key_data);
-    key_data_count = db_entry->n_key_data;
+    /* Save aside the old key data. */
+    n_key_data = db_entry->n_key_data;
     key_data = db_entry->key_data;
-    db_entry->key_data = NULL;
     db_entry->n_key_data = 0;
+    db_entry->key_data = NULL;
 
-    /* increment the kvno.  if the requested kvno is too small,
-       increment the old kvno */
-    if (new_kvno < old_kvno+1)
-        new_kvno = old_kvno+1;
-
-    retval = add_key_pwd(context, master_key, ks_tuple, ks_tuple_count,
-                         passwd, db_entry, new_kvno);
-    if (retval) {
+    /* Make sure the new kvno is greater than the old largest kvno. */
+    old_kvno = krb5_db_get_key_data_kvno(context, n_key_data, key_data);
+    if (new_kvno < old_kvno + 1)
+        new_kvno = old_kvno + 1;
+
+    /* Add new keys to the front of the list. */
+    if (password != NULL) {
+        ret = add_key_pwd(context, mkey, ks_tuple, ks_tuple_count, password,
+                          db_entry, new_kvno);
+    } else {
+        ret = add_key_rnd(context, mkey, ks_tuple, ks_tuple_count, db_entry,
+                          new_kvno);
+    }
+    if (ret) {
         cleanup_key_data(context, db_entry->n_key_data, db_entry->key_data);
-        db_entry->n_key_data = key_data_count;
+        db_entry->n_key_data = n_key_data;
         db_entry->key_data = key_data;
-        return retval;
+        return ret;
     }
 
-    if (keepold) {
-        retval = preserve_old_keys(context, master_key, db_entry, 0,
-                                   key_data_count, key_data);
+    /* Possibly add some or all of the old keys to the back of the list. */
+    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);
     }
-    cleanup_key_data(context, key_data_count, key_data);
-    return retval;
+
+    cleanup_key_data(context, n_key_data, key_data);
+    return ret;
 }
 
 /*
- * Add password for a krb5_db_entry
+ * Change random key for a krb5_db_entry
  * Assumes the max kvno
  *
- * As a side effect all old keys older than the max kvno are nuked.
+ * As a side effect all old keys are nuked if keepold is false.
  */
 krb5_error_code
-krb5_dbe_apw(context, master_key, ks_tuple, ks_tuple_count, passwd, db_entry)
-    krb5_context          context;
-    krb5_keyblock       * master_key;
-    krb5_key_salt_tuple * ks_tuple;
-    int                   ks_tuple_count;
-    char                * passwd;
-    krb5_db_entry       * db_entry;
+krb5_dbe_crk(krb5_context context, krb5_keyblock *mkey,
+             krb5_key_salt_tuple *ks_tuple, int ks_tuple_count,
+             krb5_boolean keepold, krb5_db_entry *dbent)
 {
-    int                   key_data_count;
-    krb5_key_data       * key_data;
-    krb5_error_code       retval;
-    int                   old_kvno, new_kvno;
-
-    /* First save the old keydata */
-    old_kvno = krb5_db_get_key_data_kvno(context, db_entry->n_key_data,
-                                         db_entry->key_data);
-    key_data_count = db_entry->n_key_data;
-    key_data = db_entry->key_data;
-    db_entry->key_data = NULL;
-    db_entry->n_key_data = 0;
+    return rekey(context, mkey, ks_tuple, ks_tuple_count, NULL, 0,
+                 keepold ? KEEP_ALL : DISCARD_ALL, dbent);
+}
 
-    /* increment the kvno */
-    new_kvno = old_kvno+1;
+/*
+ * Add random key for a krb5_db_entry
+ * Assumes the max kvno
+ *
+ * As a side effect all old keys older than the max kvno are nuked.
+ */
+krb5_error_code
+krb5_dbe_ark(krb5_context context, krb5_keyblock *mkey,
+             krb5_key_salt_tuple *ks_tuple, int ks_tuple_count,
+             krb5_db_entry *dbent)
+{
+    return rekey(context, mkey, ks_tuple, ks_tuple_count, NULL, 0,
+                 KEEP_LAST_KVNO, dbent);
+}
 
-    if ((retval = add_key_pwd(context, master_key, ks_tuple, ks_tuple_count,
-                              passwd, db_entry, new_kvno))) {
-        cleanup_key_data(context, db_entry->n_key_data, db_entry->key_data);
-        db_entry->n_key_data = key_data_count;
-        db_entry->key_data = key_data;
-        return retval;
-    }
+/*
+ * Change password for a krb5_db_entry
+ * Assumes the max kvno
+ *
+ * As a side effect all old keys are nuked if keepold is false.
+ */
+krb5_error_code
+krb5_dbe_def_cpw(krb5_context context, krb5_keyblock *mkey,
+                 krb5_key_salt_tuple *ks_tuple, int ks_tuple_count,
+                 char *password, int new_kvno, krb5_boolean keepold,
+                 krb5_db_entry *dbent)
+{
+    return rekey(context, mkey, ks_tuple, ks_tuple_count, password, new_kvno,
+                 keepold ? KEEP_ALL : DISCARD_ALL, dbent);
+}
 
-    /* Preserve only the most recent kvno. */
-    retval = preserve_old_keys(context, master_key, db_entry, old_kvno,
-                               key_data_count, key_data);
-    cleanup_key_data(context, key_data_count, key_data);
-    return retval;
+/*
+ * Add password for a krb5_db_entry
+ * Assumes the max kvno
+ *
+ * As a side effect all old keys older than the max kvno are nuked.
+ */
+krb5_error_code
+krb5_dbe_apw(krb5_context context, krb5_keyblock *mkey,
+             krb5_key_salt_tuple *ks_tuple, int ks_tuple_count, char *password,
+             krb5_db_entry *dbent)
+{
+    return rekey(context, mkey, ks_tuple, ks_tuple_count, password, 0,
+                 KEEP_LAST_KVNO, dbent);
 }