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.

diff --git a/src/lib/crypto/krb/s2k_des.c b/src/lib/crypto/krb/s2k_des.c
index 31a613bebc6185f387bbbec0c75cb6f450ef0be6..d5c29befcb2e104f2aa97bdd7c6ff75bcb83ce09 100644 (file)
--- 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 ec5856c2be79b9664e06b08dcb6c0d5d0f33c4cb..1fea03408c76e92dc1b784fdb72663f567dab5c1 100644 (file)
--- 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 b55ee75d2f343ccb7cf543e90ce8136ecae4d429..352a8e8dcce2d1a7e85dfd184f7429940de69576 100644 (file)
--- 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 = ∅
+
     /* 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);