Stop using SALT_TYPE_AFS_LENGTH

In krb5_init_creds_ctx and krb5_clpreauth_rock_st, use a boolean to
track whether we're still using the default salt instead of
overloading salt.length.  In preauth2.c, process afs3 salt values like
we would in krb5int_des_string_to_key, and set an s2kparams indicator
instead of overloading salt.length.  Also use an s2kparams indicator
in kdb_cpw.c's add_key_pwd.  Remove the s2k code to handle overloaded
salt lengths, except for a sanity check.

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@25837 dc483132-0cff-0310-8789-dd5450dbe970

10 files changed, 51 insertions, 54 deletions

diff --git a/src/include/k5-int.h b/src/include/k5-int.h
index 752b40efc..23869c79c 100644
--- a/src/include/k5-int.h
+++ b/src/include/k5-int.h
@@ -754,6 +754,7 @@ struct krb5_clpreauth_rock_st {
     krb5_keyblock *as_key;
     krb5_gic_get_as_key_fct *gak_fct;
     void **gak_data;
+    krb5_boolean *default_salt;
     krb5_data *salt;
     krb5_data *s2kparams;
     krb5_principal client;
diff --git a/src/include/krb5/krb5.hin b/src/include/krb5/krb5.hin
index bdd42063e..94a78a004 100644
--- a/src/include/krb5/krb5.hin
+++ b/src/include/krb5/krb5.hin
@@ -218,12 +218,7 @@ typedef struct _krb5_octet_data {
     krb5_octet *data;
 } krb5_octet_data;
 
-/*
- * Hack length for crypto library to use the afs_string_to_key It is
- * equivalent to -1 without possible sign extension
- * We also overload for an unset salt type length - which is also -1, but
- * hey, why not....
- */
+/* Originally used to recognize AFS and default salts.  No longer used. */
 #define SALT_TYPE_AFS_LENGTH UINT_MAX
 #define SALT_TYPE_NO_LENGTH  UINT_MAX
 
diff --git a/src/lib/crypto/krb/s2k_des.c b/src/lib/crypto/krb/s2k_des.c
index fd2143054..61b3c0f01 100644
--- a/src/lib/crypto/krb/s2k_des.c
+++ b/src/lib/crypto/krb/s2k_des.c
@@ -670,7 +670,6 @@ krb5int_des_string_to_key(const struct krb5_keytypes *ktp,
                           const krb5_data *parm, krb5_keyblock *keyblock)
 {
     int type;
-    krb5_data afssalt;
 
     if (parm != NULL) {
         if (parm->length != 1)
@@ -685,12 +684,5 @@ krb5int_des_string_to_key(const struct krb5_keytypes *ktp,
     if (type == 1)
         return afs_s2k(string, salt, keyblock->contents);
 
-    /* Also use AFS string to key if the salt indicates it. */
-    if (salt != NULL && (salt->length == SALT_TYPE_AFS_LENGTH
-                         || salt->length == (unsigned)-1)) {
-        afssalt = make_data(salt->data, strcspn(salt->data, "@"));
-        return afs_s2k(string, &afssalt, keyblock->contents);
-    }
-
     return des_s2k(string, salt, keyblock->contents);
 }
diff --git a/src/lib/crypto/krb/string_to_key.c b/src/lib/crypto/krb/string_to_key.c
index cd46d454a..b55ee75d2 100644
--- a/src/lib/crypto/krb/string_to_key.c
+++ b/src/lib/crypto/krb/string_to_key.c
@@ -51,21 +51,9 @@ krb5_c_string_to_key_with_params(krb5_context context, krb5_enctype enctype,
         return KRB5_BAD_ENCTYPE;
     keylength = ktp->enc->keylength;
 
-    /*
-     * xxx AFS string2key function is indicated by a special length  in
-     * the salt in much of the code.  However only the DES enctypes can
-     * deal with this.  Using s2kparams would be a much better solution.
-     */
-    if (salt && salt->length == SALT_TYPE_AFS_LENGTH) {
-        switch (enctype) {
-        case ENCTYPE_DES_CBC_CRC:
-        case ENCTYPE_DES_CBC_MD4:
-        case ENCTYPE_DES_CBC_MD5:
-            break;
-        default:
-            return KRB5_CRYPTO_INTERNAL;
-        }
-    }
+    /* Fail gracefully if someone is using the old AFS string-to-key hack. */
+    if (salt != NULL && salt->length == SALT_TYPE_AFS_LENGTH)
+        return EINVAL;
 
     key->contents = malloc(keylength);
     if (key->contents == NULL)
diff --git a/src/lib/kdb/kdb_cpw.c b/src/lib/kdb/kdb_cpw.c
index abaae4f7c..7b00fcf5f 100644
--- a/src/lib/kdb/kdb_cpw.c
+++ b/src/lib/kdb/kdb_cpw.c
@@ -389,6 +389,7 @@ add_key_pwd(context, master_key, ks_tuple, ks_tuple_count, passwd,
     krb5_keysalt          key_salt;
     krb5_keyblock         key;
     krb5_data             pwd;
+    krb5_data             afs_params = string2data("\1"), *s2k_params = NULL;
     int                   i, j, k;
     krb5_key_data         tmp_key_data;
     krb5_key_data        *tptr;
@@ -452,15 +453,12 @@ add_key_pwd(context, master_key, ks_tuple, ks_tuple_count, passwd,
             key_salt.data.data = 0;
             break;
         case KRB5_KDB_SALTTYPE_AFS3:
-            /* The afs_mit_string_to_key needs to use strlen, and the
-               realm field is not (necessarily) NULL terminated.  */
-            retval = krb5int_copy_data_contents_add0(context,
-                                                     krb5_princ_realm(context,
-                                                                      db_entry->princ),
-                                                     &key_salt.data);
+            retval = krb5int_copy_data_contents(context,
+                                                &db_entry->princ->realm,
+                                                &key_salt.data);
             if (retval)
                 return retval;
-            key_salt.data.length = SALT_TYPE_AFS_LENGTH; /*length actually used below...*/
+            s2k_params = &afs_params;
             break;
         case KRB5_KDB_SALTTYPE_SPECIAL:
             retval = make_random_salt(context, &key_salt);
@@ -474,18 +472,15 @@ add_key_pwd(context, master_key, ks_tuple, ks_tuple_count, passwd,
         pwd.data = passwd;
         pwd.length = strlen(passwd);
 
-        /* AFS string to key will happen here */
-        if ((retval = krb5_c_string_to_key(context, ks_tuple[i].ks_enctype,
-                                           &pwd, &key_salt.data, &key))) {
-            if (key_salt.data.data)
-                free(key_salt.data.data);
-            return(retval);
+        retval = krb5_c_string_to_key_with_params(context,
+                                                  ks_tuple[i].ks_enctype,
+                                                  &pwd, &key_salt.data,
+                                                  s2k_params, &key);
+        if (retval) {
+            free(key_salt.data.data);
+            return retval;
         }
 
-        if (key_salt.data.length == SALT_TYPE_AFS_LENGTH)
-            key_salt.data.length =
-                krb5_princ_realm(context, db_entry->princ)->length;
-
         /* memory allocation to be done by db. So, use temporary block and later copy
            it to the memory allocated by db */
         retval = krb5_dbe_encrypt_key_data(context, master_key, &key,
diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c
index 471834611..738bd9c37 100644
--- a/src/lib/krb5/krb/get_in_tkt.c
+++ b/src/lib/krb5/krb/get_in_tkt.c
@@ -822,6 +822,7 @@ krb5_init_creds_init(krb5_context context,
     ctx->preauth_rock.as_key = &ctx->as_key;
     ctx->preauth_rock.gak_fct = &ctx->gak_fct;
     ctx->preauth_rock.gak_data = &ctx->gak_data;
+    ctx->preauth_rock.default_salt = &ctx->default_salt;
     ctx->preauth_rock.salt = &ctx->salt;
     ctx->preauth_rock.s2kparams = &ctx->s2kparams;
     ctx->preauth_rock.client = client;
@@ -944,9 +945,10 @@ krb5_init_creds_init(krb5_context context,
         code = krb5int_copy_data_contents(context, opte->salt, &ctx->salt);
         if (code != 0)
             goto cleanup;
+        ctx->default_salt = FALSE;
     } else {
-        ctx->salt.length = SALT_TYPE_AFS_LENGTH;
-        ctx->salt.data = NULL;
+        ctx->salt = empty_data();
+        ctx->default_salt = TRUE;
     }
 
     /* Anonymous. */
@@ -1416,7 +1418,7 @@ init_creds_step_reply(krb5_context context,
      * salt.  local_as_reply->client will be checked later on in
      * verify_as_reply.
      */
-    if (ctx->salt.length == SALT_TYPE_AFS_LENGTH && ctx->salt.data == NULL) {
+    if (ctx->default_salt) {
         code = krb5_principal2salt(context, ctx->reply->client, &ctx->salt);
         TRACE_INIT_CREDS_SALT_PRINC(context, &ctx->salt);
         if (code != 0)
diff --git a/src/lib/krb5/krb/gic_pwd.c b/src/lib/krb5/krb/gic_pwd.c
index 40b448d15..68d28fe9d 100644
--- a/src/lib/krb5/krb/gic_pwd.c
+++ b/src/lib/krb5/krb/gic_pwd.c
@@ -63,7 +63,7 @@ krb5_get_as_key_password(krb5_context context,
             return(ret);
     }
 
-    if (salt->length == SALT_TYPE_AFS_LENGTH && salt->data == NULL) {
+    if (salt == NULL) {
         if ((ret = krb5_principal2salt(context, client, &defsalt)))
             return(ret);
 
diff --git a/src/lib/krb5/krb/init_creds_ctx.h b/src/lib/krb5/krb/init_creds_ctx.h
index 48376fccd..2653ee161 100644
--- a/src/lib/krb5/krb/init_creds_ctx.h
+++ b/src/lib/krb5/krb/init_creds_ctx.h
@@ -36,6 +36,7 @@ struct _krb5_init_creds_context {
     krb5_data *encoded_previous_request;
     struct krb5int_fast_request_state *fast_state;
     krb5_pa_data **preauth_to_use;
+    krb5_boolean default_salt;
     krb5_data salt;
     krb5_data s2kparams;
     krb5_keyblock as_key;
diff --git a/src/lib/krb5/krb/preauth2.c b/src/lib/krb5/krb/preauth2.c
index 2ff862409..7c5452790 100644
--- a/src/lib/krb5/krb/preauth2.c
+++ b/src/lib/krb5/krb/preauth2.c
@@ -391,10 +391,12 @@ get_as_key(krb5_context context, krb5_clpreauth_rock rock,
            krb5_keyblock **keyblock)
 {
     krb5_error_code ret;
+    krb5_data *salt;
 
     if (rock->as_key->length == 0) {
+        salt = (*rock->default_salt) ? NULL : rock->salt;
         ret = (*rock->gak_fct)(context, rock->client, *rock->etype,
-                               rock->prompter, rock->prompter_data, rock->salt,
+                               rock->prompter, rock->prompter_data, salt,
                                rock->s2kparams, rock->as_key, *rock->gak_data);
         if (ret)
             return ret;
@@ -565,6 +567,7 @@ get_etype_info(krb5_context context, krb5_pa_data **padata,
     krb5_etype_info etype_info = NULL, e;
     krb5_etype_info_entry *entry;
     krb5_boolean valid_found;
+    const char *p;
     int i;
 
     /* Find an etype-info2 or etype-info element in padata. */
@@ -604,6 +607,10 @@ get_etype_info(krb5_context context, krb5_pa_data **padata,
         if (entry->length != KRB5_ETYPE_NO_SALT) {
             *rock->salt = make_data(entry->salt, entry->length);
             entry->salt = NULL;
+            *rock->default_salt = FALSE;
+        } else {
+            *rock->salt = empty_data();
+            *rock->default_salt = TRUE;
         }
         krb5_free_data_contents(context, rock->s2kparams);
         *rock->s2kparams = entry->s2kparams;
@@ -619,12 +626,27 @@ get_etype_info(krb5_context context, krb5_pa_data **padata,
             /* Set rock->salt based on the element we found. */
             krb5_free_data_contents(context, rock->salt);
             d = padata2data(*pa);
-            ret = krb5int_copy_data_contents_add0(context, &d, rock->salt);
+            ret = krb5int_copy_data_contents(context, &d, rock->salt);
             if (ret)
                 goto cleanup;
+            if (pa->pa_type == KRB5_PADATA_AFS3_SALT) {
+                /* Work around a (possible) old Heimdal KDC foible. */
+                p = memchr(rock->salt->data, '@', rock->salt->length);
+                if (p != NULL)
+                    rock->salt->length = p - rock->salt->data;
+                /* Tolerate extra null in MIT KDC afs3-salt value. */
+                if (rock->salt->length > 0 &&
+                    rock->salt->data[rock->salt->length - 1] == '\0')
+                    rock->salt->length--;
+                /* Set an s2kparams value to indicate AFS string-to-key. */
+                krb5_free_data_contents(context, rock->s2kparams);
+                ret = alloc_data(rock->s2kparams, 1);
+                if (ret)
+                    goto cleanup;
+                rock->s2kparams->data[0] = '\1';
+            }
+            *rock->default_salt = FALSE;
             TRACE_PREAUTH_SALT(context, rock->salt, pa->pa_type);
-            if (pa->pa_type == KRB5_PADATA_AFS3_SALT)
-                rock->salt->length = SALT_TYPE_AFS_LENGTH;
         }
     }
 
diff --git a/src/lib/krb5/libkrb5.exports b/src/lib/krb5/libkrb5.exports
index 04a09eb45..cb73a1471 100644
--- a/src/lib/krb5/libkrb5.exports
+++ b/src/lib/krb5/libkrb5.exports
@@ -577,6 +577,7 @@ krb5int_cc_default
 krb5int_cleanup_library
 krb5int_clean_hostname
 krb5int_cm_call_select
+krb5int_copy_data_contents
 krb5int_copy_data_contents_add0
 krb5int_find_pa_data
 krb5int_foreach_localaddr