Modernize default key data encryption functions

2 files changed, 111 insertions, 136 deletions

diff --git a/src/lib/kdb/decrypt_key.c b/src/lib/kdb/decrypt_key.c
index 541064d88..82bbed631 100644
--- a/src/lib/kdb/decrypt_key.c
+++ b/src/lib/kdb/decrypt_key.c
@@ -1,7 +1,7 @@
 /* -*- mode: c; c-basic-offset: 4; indent-tabs-mode: nil -*- */
 /* lib/kdb/decrypt_key.c */
 /*
- * Copyright 1990,1991 by the Massachusetts Institute of Technology.
+ * Copyright 1990,1991,2023 by the Massachusetts Institute of Technology.
  * All Rights Reserved.
  *
  * Export of this software from the United States of America may
@@ -52,88 +52,84 @@
 #include "k5-int.h"
 #include "kdb.h"
 
-/*
- * Decrypt a key from storage in the database.  "eblock" is used
- * to decrypt the key in "in" into "out"; the storage pointed to by "out"
- * is allocated before use.
- */
-
+/* Decrypt key_data, putting the result into dbkey_out and (if not null)
+ * keysalt_out. */
 krb5_error_code
-krb5_dbe_def_decrypt_key_data( krb5_context     context,
-                               const krb5_keyblock    * mkey,
-                               const krb5_key_data    * key_data,
-                               krb5_keyblock  * dbkey,
-                               krb5_keysalt   * keysalt)
+krb5_dbe_def_decrypt_key_data(krb5_context context, const krb5_keyblock *mkey,
+                              const krb5_key_data *kd,
+                              krb5_keyblock *dbkey_out,
+                              krb5_keysalt *keysalt_out)
 {
-    krb5_error_code       retval = 0;
-    krb5_int16            tmplen;
-    krb5_octet          * ptr;
-    krb5_enc_data         cipher;
-    krb5_data             plain;
+    krb5_error_code ret;
+    int16_t keylen;
+    krb5_enc_data cipher;
+    krb5_data plain = empty_data();
+    krb5_keyblock kb = { 0 };
+    krb5_keysalt salt = { 0 };
 
-    if (!mkey)
-        return KRB5_KDB_BADSTORED_MKEY;
-    ptr = key_data->key_data_contents[0];
+    memset(dbkey_out, 0, sizeof(*dbkey_out));
+    if (keysalt_out != NULL)
+        memset(keysalt_out, 0, sizeof(*keysalt_out));
 
-    if (ptr) {
-        krb5_kdb_decode_int16(ptr, tmplen);
-        ptr += 2;
+    if (mkey == NULL)
+        return KRB5_KDB_BADSTORED_MKEY;
 
-        if (tmplen < 0)
+    if (kd->key_data_contents[0] != NULL && kd->key_data_length[0] >= 2) {
+        keylen = load_16_le(kd->key_data_contents[0]);
+        if (keylen < 0)
             return EINVAL;
         cipher.enctype = ENCTYPE_UNKNOWN;
-        cipher.ciphertext.length = key_data->key_data_length[0]-2;
-        cipher.ciphertext.data = (char *) ptr;
-        plain.length = key_data->key_data_length[0]-2;
-        if ((plain.data = malloc(plain.length)) == NULL)
-            return(ENOMEM);
-
-        if ((retval = krb5_c_decrypt(context, mkey, 0 /* XXX */, 0,
-                                     &cipher, &plain))) {
-            free(plain.data);
-            return retval;
-        }
+        cipher.ciphertext = make_data(kd->key_data_contents[0] + 2,
+                                      kd->key_data_length[0] - 2);
+        ret = alloc_data(&plain, kd->key_data_length[0] - 2);
+        if (ret)
+            goto cleanup;
 
-        /* tmplen is the true length of the key.  plain.data is the
-           plaintext data length, but it may be padded, since the
-           old-style etypes didn't store the real length.  I can check
-           to make sure that there are enough bytes, but I can't do
-           any better than that. */
+        ret = krb5_c_decrypt(context, mkey, 0, 0, &cipher, &plain);
+        if (ret)
+            goto cleanup;
 
-        if ((unsigned int) tmplen >  plain.length) {
-            free(plain.data);
-            return(KRB5_CRYPTO_INTERNAL);
+        /* Make sure the plaintext has at least as many bytes as the true ke
+         * length (it may have more due to padding). */
+        if ((unsigned int)keylen > plain.length) {
+            ret = KRB5_CRYPTO_INTERNAL;
+            if (ret)
+                goto cleanup;
         }
 
-        dbkey->magic = KV5M_KEYBLOCK;
-        dbkey->enctype = key_data->key_data_type[0];
-        dbkey->length = tmplen;
-        dbkey->contents = (krb5_octet *) plain.data;
+        kb.magic = KV5M_KEYBLOCK;
+        kb.enctype = kd->key_data_type[0];
+        kb.length = keylen;
+        kb.contents = (uint8_t *)plain.data;
+        plain = empty_data();
     }
 
-    /* Decode salt data */
-    if (keysalt) {
-        if (key_data->key_data_ver == 2) {
-            keysalt->type = key_data->key_data_type[1];
-            if ((keysalt->data.length = key_data->key_data_length[1])) {
-                if (!(keysalt->data.data=(char *)malloc(keysalt->data.length))){
-                    if (key_data->key_data_contents[0]) {
-                        free(dbkey->contents);
-                        dbkey->contents = 0;
-                        dbkey->length = 0;
-                    }
-                    return ENOMEM;
-                }
-                memcpy(keysalt->data.data, key_data->key_data_contents[1],
-                       (size_t) keysalt->data.length);
-            } else
-                keysalt->data.data = (char *) NULL;
+    /* Decode salt data. */
+    if (keysalt_out != NULL) {
+        if (kd->key_data_ver == 2) {
+            salt.type = kd->key_data_type[1];
+            salt.data.length = kd->key_data_length[1];
+            if (kd->key_data_length[1] > 0) {
+                ret = alloc_data(&salt.data, kd->key_data_length[1]);
+                if (ret)
+                    goto cleanup;
+                memcpy(salt.data.data, kd->key_data_contents[1],
+                       salt.data.length);
+            }
         } else {
-            keysalt->type = KRB5_KDB_SALTTYPE_NORMAL;
-            keysalt->data.data = (char *) NULL;
-            keysalt->data.length = 0;
+            salt.type = KRB5_KDB_SALTTYPE_NORMAL;
         }
     }
 
-    return retval;
+    *dbkey_out = kb;
+    if (keysalt_out != NULL)
+        *keysalt_out = salt;
+    memset(&kb, 0, sizeof(kb));
+    memset(&salt, 0, sizeof(salt));
+
+cleanup:
+    zapfree(plain.data, plain.length);
+    krb5_free_keyblock_contents(context, &kb);
+    free(salt.data.data);
+    return ret;
 }
diff --git a/src/lib/kdb/encrypt_key.c b/src/lib/kdb/encrypt_key.c
index 91debea53..a05c519a0 100644
--- a/src/lib/kdb/encrypt_key.c
+++ b/src/lib/kdb/encrypt_key.c
@@ -1,7 +1,7 @@
 /* -*- mode: c; c-basic-offset: 4; indent-tabs-mode: nil -*- */
 /* lib/kdb/encrypt_key.c */
 /*
- * Copyright 1990,1991 by the Massachusetts Institute of Technology.
+ * Copyright 1990,1991,2023 by the Massachusetts Institute of Technology.
  * All Rights Reserved.
  *
  * Export of this software from the United States of America may
@@ -53,83 +53,62 @@
 #include "kdb.h"
 
 /*
- * Encrypt a key for storage in the database.  "eblock" is used
- * to encrypt the key in "in" into "out"; the storage pointed to by "out"
- * is allocated before use.
+ * Encrypt dbkey for storage in the database, putting the result into
+ * key_data_out.
  */
-
 krb5_error_code
-krb5_dbe_def_encrypt_key_data( krb5_context             context,
-                               const krb5_keyblock    * mkey,
-                               const krb5_keyblock    * dbkey,
-                               const krb5_keysalt     * keysalt,
-                               int                      keyver,
-                               krb5_key_data          * key_data)
+krb5_dbe_def_encrypt_key_data(krb5_context context, const krb5_keyblock *mkey,
+                              const krb5_keyblock *dbkey,
+                              const krb5_keysalt *keysalt, int keyver,
+                              krb5_key_data *key_data_out)
 {
-    krb5_error_code               retval;
-    krb5_octet                  * ptr;
-    size_t                        len;
-    int                           i;
-    krb5_data                     plain;
-    krb5_enc_data                 cipher;
-
-    for (i = 0; i < key_data->key_data_ver; i++) {
-        free(key_data->key_data_contents[i]);
-        key_data->key_data_contents[i] = NULL;
-    }
-
-    key_data->key_data_ver = 1;
-    key_data->key_data_kvno = keyver;
+    krb5_error_code ret;
+    size_t clen;
+    krb5_data plain;
+    krb5_enc_data cipher;
+    krb5_key_data kd = { 0 };
 
-    /*
-     * The First element of the type/length/contents
-     * fields is the key type/length/contents
-     */
-    if ((retval = krb5_c_encrypt_length(context, mkey->enctype, dbkey->length,
-                                        &len)))
-        return(retval);
+    memset(key_data_out, 0, sizeof(*key_data_out));
 
-    ptr = malloc(2 + len);
-    if (ptr == NULL)
-        return(ENOMEM);
+    kd.key_data_ver = 1;
+    kd.key_data_kvno = keyver;
 
-    key_data->key_data_type[0] = dbkey->enctype;
-    key_data->key_data_length[0] = 2 + len;
-    key_data->key_data_contents[0] = ptr;
+    ret = krb5_c_encrypt_length(context, mkey->enctype, dbkey->length, &clen);
+    if (ret)
+        goto cleanup;
 
-    krb5_kdb_encode_int16(dbkey->length, ptr);
-    ptr += 2;
+    /* The first element of the type/length/contents fields is the key
+     * type/length/contents. */
+    kd.key_data_type[0] = dbkey->enctype;
+    kd.key_data_length[0] = 2 + clen;
+    kd.key_data_contents[0] = k5alloc(kd.key_data_length[0], &ret);
+    if (kd.key_data_contents[0] == NULL)
+        goto cleanup;
+    store_16_le(dbkey->length, kd.key_data_contents[0]);
 
-    plain.length = dbkey->length;
-    plain.data = (char *) dbkey->contents;
+    plain = make_data(dbkey->contents, dbkey->length);
+    cipher.ciphertext = make_data(kd.key_data_contents[0] + 2, clen);
+    ret = krb5_c_encrypt(context, mkey, 0, 0, &plain, &cipher);
+    if (ret)
+        goto cleanup;
 
-    cipher.ciphertext.length = len;
-    cipher.ciphertext.data = (char *) ptr;
-
-    if ((retval = krb5_c_encrypt(context, mkey, /* XXX */ 0, 0,
-                                 &plain, &cipher))) {
-        free(key_data->key_data_contents[0]);
-        key_data->key_data_contents[0] = NULL;
-        return retval;
-    }
-
-    /* After key comes the salt in necessary */
-    if (keysalt) {
-        if (keysalt->type > 0) {
-            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] = malloc(keysalt->data.length);
-                if (key_data->key_data_contents[1] == NULL) {
-                    free(key_data->key_data_contents[0]);
-                    key_data->key_data_contents[0] = NULL;
-                    return ENOMEM;
-                }
-                memcpy(key_data->key_data_contents[1], keysalt->data.data,
-                       (size_t) keysalt->data.length);
-            }
+    /* The second element of each array is the salt, if necessary. */
+    if (keysalt != NULL && keysalt->type > 0) {
+        kd.key_data_ver++;
+        kd.key_data_type[1] = keysalt->type;
+        kd.key_data_length[1] = keysalt->data.length;
+        if (keysalt->data.length > 0) {
+            kd.key_data_contents[1] = k5memdup(keysalt->data.data,
+                                               keysalt->data.length, &ret);
+            if (kd.key_data_contents[1] == NULL)
+                goto cleanup;
         }
     }
 
-    return retval;
+    *key_data_out = kd;
+    memset(&kd, 0, sizeof(kd));
+
+cleanup:
+    krb5_dbe_free_key_data_contents(context, &kd);
+    return ret;
 }