From: Loup Vaillant Date: Fri, 10 Jul 2020 22:46:38 +0000 (+0200) Subject: Argon2i hash is now allowed to overlap with the work area X-Git-Url: https://git.codecow.com/?a=commitdiff_plain;h=6157b48962d95564115d50dc554034693e8e66ee;p=Monocypher.git Argon2i hash is now allowed to overlap with the work area Fixes #183 Almost all of Monocypher allows arguments to overlap. Users may come to expect it, and misuse those who don't allow such an overlap. (Chacha20 and AEAD are an exception, but (i) portability concerns prevents us to allow it properly, and (ii) disallowed overlaps tend to trigger visible corruptions immediately.) Before, having the hash coincide with the working area meant the output was always zero. All passwords have the same hash. Therefore all passwords are correct. Oops. For the record, I made the mistake, and caught the bug only days later, by pure luck. Now overlap is allowed, and gives the right result. Note that the work area is still wiped. The wipe just happens *before* the final hash is computed. --- diff --git a/doc/man/man3/crypto_argon2i.3monocypher b/doc/man/man3/crypto_argon2i.3monocypher index ac9d1c6..024832d 100644 --- a/doc/man/man3/crypto_argon2i.3monocypher +++ b/doc/man/man3/crypto_argon2i.3monocypher @@ -163,9 +163,7 @@ Must be at least 8. 16 is recommended. .El .Pp -The output hash must not overlap with the work area, or it will be -wiped along with it. -Any other overlap is permitted. +The arguments may overlap or point at the same buffer. .Pp Use .Xr crypto_verify16 3monocypher , diff --git a/src/monocypher.c b/src/monocypher.c index c37482e..dd488e0 100644 --- a/src/monocypher.c +++ b/src/monocypher.c @@ -1023,15 +1023,16 @@ void crypto_argon2i_general(u8 *hash, u32 hash_size, } } wipe_block(&tmp); - // hash the very last block with H' into the output hash u8 final_block[1024]; store_block(final_block, blocks + (nb_blocks - 1)); - extended_hash(hash, hash_size, final_block, 1024); - WIPE_BUFFER(final_block); // wipe work area volatile u64 *p = (u64*)work_area; ZERO(p, 128 * nb_blocks); + + // hash the very last block with H' into the output hash + extended_hash(hash, hash_size, final_block, 1024); + WIPE_BUFFER(final_block); } void crypto_argon2i(u8 *hash, u32 hash_size, diff --git a/tests/test.c b/tests/test.c index a6cb629..f4b974b 100644 --- a/tests/test.c +++ b/tests/test.c @@ -652,16 +652,17 @@ static int p_argon2i_overlap() u8 *clean_work_area = (u8*)alloc(8 * 1024); FOR (i, 0, 10) { p_random(work_area, 8 * 1024); + u32 hash_offset = rand64() % 64; u32 pass_offset = rand64() % 64; u32 salt_offset = rand64() % 64; u32 key_offset = rand64() % 64; u32 ad_offset = rand64() % 64; - u8 hash1[32]; - u8 hash2[32]; - u8 pass [16]; FOR (j, 0, 16) { pass[j] = work_area[j + pass_offset]; } - u8 salt [16]; FOR (j, 0, 16) { salt[j] = work_area[j + salt_offset]; } - u8 key [32]; FOR (j, 0, 32) { key [j] = work_area[j + key_offset]; } - u8 ad [32]; FOR (j, 0, 32) { ad [j] = work_area[j + ad_offset]; } + u8 hash1[32]; + u8 *hash2 = work_area + hash_offset; + u8 pass [16]; FOR (j, 0, 16) { pass[j] = work_area[j + pass_offset]; } + u8 salt [16]; FOR (j, 0, 16) { salt[j] = work_area[j + salt_offset]; } + u8 key [32]; FOR (j, 0, 32) { key [j] = work_area[j + key_offset]; } + u8 ad [32]; FOR (j, 0, 32) { ad [j] = work_area[j + ad_offset]; } crypto_argon2i_general(hash1, 32, clean_work_area, 8, 1, pass, 16, salt, 16, key, 32, ad, 32);