krb5 commit: Simplify null salt handling in string-to-key
Greg Hudson <[email protected]>
| Newsgroups | gmane.comp.encryption.kerberos.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://github.com/krb5/krb5/commit/d9afa34fdc08798ccbdc6b8234122f0bdbb754d3 commit d9afa34fdc08798ccbdc6b8234122f0bdbb754d3 Author: Greg Hudson <[email protected]> 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 = ∅ + /* 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);