krb5 commit: Simplify null salt handling in string-to-key

[Greg Hudson]

https://github.com/krb5/krb5/commit/d9afa34fdc08798ccbdc6b8234122f0bdbb754d3
commit d9afa34fdc08798ccbdc6b8234122f0bdbb754d3
Author: Greg Hudson <ghudson at mit.edu>
Date:   Mon Mar 27 15:40:08 2017 -0400

    Simplify null salt handling in string-to-key
    
    The per-enctype string_to_key implementations are inconsistent about
    whether a null salt is treated as empty or results in a null
    dereference.  Since the original DES string-to-key allowed a null
    salt, substitute an empty salt in krb5_c_string_to_key_with_params().
    Eliminate conditionals on accessing salt in the per-enctype
    implementations as they are no longer needed.  Based on a patch by
    Martin Kittel.

 src/lib/crypto/krb/s2k_des.c       |    4 ++--
 src/lib/crypto/krb/s2k_pbkdf2.c    |    4 ++--
 src/lib/crypto/krb/string_to_key.c |    7 ++++++-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/lib/crypto/krb/s2k_des.c b/src/lib/crypto/krb/s2k_des.c
index 31a613b..d5c29be 100644
--- a/src/lib/crypto/krb/s2k_des.c
+++ b/src/lib/crypto/krb/s2k_des.c
@@ -509,7 +509,7 @@ des_s2k(const krb5_data *pw, const krb5_data *salt, unsigned char *key_out)
 #define FETCH4(VAR, IDX)        VAR = temp.ui[IDX/4]
 #define PUT4(VAR, IDX)          temp.ui[IDX/4] = VAR
 
-    copylen = pw->length + (salt ? salt->length : 0);
+    copylen = pw->length + salt->length;
     /* Don't need NUL termination, at this point we're treating it as
        a byte array, not a string.  */
     copy = malloc(copylen);
@@ -517,7 +517,7 @@ des_s2k(const krb5_data *pw, const krb5_data *salt, unsigned char *key_out)
         return ENOMEM;
     if (pw->length > 0)
         memcpy(copy, pw->data, pw->length);
-    if (salt != NULL && salt->length > 0)
+    if (salt->length > 0)
         memcpy(copy + pw->length, salt->data, salt->length);
 
     memset(&temp, 0, sizeof(temp));
diff --git a/src/lib/crypto/krb/s2k_pbkdf2.c b/src/lib/crypto/krb/s2k_pbkdf2.c
index ec5856c..1fea034 100644
--- a/src/lib/crypto/krb/s2k_pbkdf2.c
+++ b/src/lib/crypto/krb/s2k_pbkdf2.c
@@ -47,7 +47,7 @@ krb5int_dk_string_to_key(const struct krb5_keytypes *ktp,
     keybytes = ktp->enc->keybytes;
     keylength = ktp->enc->keylength;
 
-    concatlen = string->length + (salt ? salt->length : 0);
+    concatlen = string->length + salt->length;
 
     concat = k5alloc(concatlen, &ret);
     if (ret != 0)
@@ -63,7 +63,7 @@ krb5int_dk_string_to_key(const struct krb5_keytypes *ktp,
 
     if (string->length > 0)
         memcpy(concat, string->data, string->length);
-    if (salt != NULL && salt->length > 0)
+    if (salt->length > 0)
         memcpy(concat + string->length, salt->data, salt->length);
 
     krb5int_nfold(concatlen*8, concat, keybytes*8, foldstring);
diff --git a/src/lib/crypto/krb/string_to_key.c b/src/lib/crypto/krb/string_to_key.c
index b55ee75..352a8e8 100644
--- a/src/lib/crypto/krb/string_to_key.c
+++ b/src/lib/crypto/krb/string_to_key.c
@@ -43,6 +43,7 @@ krb5_c_string_to_key_with_params(krb5_context context, krb5_enctype enctype,
                                  const krb5_data *params, krb5_keyblock *key)
 {
     krb5_error_code ret;
+    krb5_data empty = empty_data();
     const struct krb5_keytypes *ktp;
     size_t keylength;
 
@@ -51,8 +52,12 @@ krb5_c_string_to_key_with_params(krb5_context context, krb5_enctype enctype,
         return KRB5_BAD_ENCTYPE;
     keylength = ktp->enc->keylength;
 
+    /* For compatibility with past behavior, treat a null salt as empty. */
+    if (salt == NULL)
+        salt = &empty;
+
     /* Fail gracefully if someone is using the old AFS string-to-key hack. */
-    if (salt != NULL && salt->length == SALT_TYPE_AFS_LENGTH)
+    if (salt->length == SALT_TYPE_AFS_LENGTH)
         return EINVAL;
 
     key->contents = malloc(keylength);