libnm-core: relax restrictions on input arguments for crypto_md5_hash()

crypto_md5_hash() only has two users:
 (a) crypto_make_des_aes_key()
 (b) nm_utils_uuid_generate_from_string()

For (b) it is just a complicated way to compute the MD5 hash. The
restrictions on salt and password don't matter. Actually they
are harmful because we cannot compute the MD5 hash of the empty
word.
For (a), the caller should make sure to pass whatever restrictions
he wants to enforce on the data.

For example, it is counterintuitive, that crypto_md5_hash() would
require @salt_len, enforce it to be at least 8 bytes, and then just
use the first 8 bytes. If the caller (a) wants that behavior, he
should make sure that he passes in 8 bytes.
Likewise for the empty word. If the caller does not want to compute
the hash of empty passwords, he must not hash them.

Indeed, all of this was enforced by assertions, any caller already
did the right thing.
This commit is contained in:
Thomas Haller
2014-12-02 12:26:38 +01:00
committed by Dan Winship
parent 4460386800
commit ef3de46c43
3 changed files with 21 additions and 14 deletions

View File

@@ -405,7 +405,7 @@ crypto_make_des_aes_key (const char *cipher,
key = g_malloc0 (digest_len + 1); key = g_malloc0 (digest_len + 1);
crypto_md5_hash (salt, crypto_md5_hash (salt,
salt_len, 8,
password, password,
strlen (password), strlen (password),
key, key,
@@ -763,9 +763,9 @@ crypto_verify_private_key (const char *filename,
void void
crypto_md5_hash (const char *salt, crypto_md5_hash (const char *salt,
const gsize salt_len, gssize salt_len,
const char *password, const char *password,
gsize password_len, gssize password_len,
char *buffer, char *buffer,
gsize buflen) gsize buflen)
{ {
@@ -778,25 +778,28 @@ crypto_md5_hash (const char *salt,
g_assert_cmpint (g_checksum_type_get_length (G_CHECKSUM_MD5), ==, sizeof (digest)); g_assert_cmpint (g_checksum_type_get_length (G_CHECKSUM_MD5), ==, sizeof (digest));
if (salt) g_return_if_fail (password_len == 0 || password);
g_return_if_fail (salt_len >= 8);
g_return_if_fail (password != NULL);
g_return_if_fail (password_len > 0);
g_return_if_fail (buffer != NULL); g_return_if_fail (buffer != NULL);
g_return_if_fail (buflen > 0); g_return_if_fail (buflen > 0);
g_return_if_fail (salt_len == 0 || salt);
ctx = g_checksum_new (G_CHECKSUM_MD5); ctx = g_checksum_new (G_CHECKSUM_MD5);
if (salt_len < 0)
salt_len = strlen (salt);
if (password_len < 0)
password_len = strlen (password);
while (nkey > 0) { while (nkey > 0) {
int i = 0; int i = 0;
g_checksum_reset (ctx); g_checksum_reset (ctx);
if (count++) if (count++)
g_checksum_update (ctx, (const guchar *) digest, digest_len); g_checksum_update (ctx, (const guchar *) digest, digest_len);
g_checksum_update (ctx, (const guchar *) password, password_len); if (password_len > 0)
if (salt) g_checksum_update (ctx, (const guchar *) password, password_len);
g_checksum_update (ctx, (const guchar *) salt, 8); /* Only use 8 bytes of salt */ if (salt_len > 0)
g_checksum_update (ctx, (const guchar *) salt, salt_len);
g_checksum_get_digest (ctx, (guchar *) digest, &digest_len); g_checksum_get_digest (ctx, (guchar *) digest, &digest_len);
while (nkey && (i < digest_len)) { while (nkey && (i < digest_len)) {

View File

@@ -79,9 +79,9 @@ NMCryptoFileFormat crypto_verify_private_key (const char *file,
/* Internal utils API bits for crypto providers */ /* Internal utils API bits for crypto providers */
void crypto_md5_hash (const char *salt, void crypto_md5_hash (const char *salt,
const gsize salt_len, gssize salt_len,
const char *password, const char *password,
gsize password_len, gssize password_len,
char *buffer, char *buffer,
gsize buflen); gsize buflen);

View File

@@ -438,7 +438,11 @@ test_md5 (void)
for (i = 0; i < G_N_ELEMENTS (md5_tests); i++) { for (i = 0; i < G_N_ELEMENTS (md5_tests); i++) {
memset (digest, 0, sizeof (digest)); memset (digest, 0, sizeof (digest));
crypto_md5_hash (md5_tests[i].salt, crypto_md5_hash (md5_tests[i].salt,
md5_tests[i].salt ? strlen (md5_tests[i].salt) : 0, /* crypto_md5_hash() used to clamp salt_len to 8. It
* doesn't any more, so we need to do it here now to
* get output that matches md5_tests[i].result.
*/
md5_tests[i].salt ? 8 : 0,
md5_tests[i].password, md5_tests[i].password,
strlen (md5_tests[i].password), strlen (md5_tests[i].password),
digest, md5_tests[i].digest_size); digest, md5_tests[i].digest_size);