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 ac9d1c6cb546429e4f40f4cbeaae0d0e06c8d57b..024832d2b8e02cf9ec08c889fde84580f55b6ac9 100644 (file)
--- 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 c37482ec5a157a95c4ee8f4dc61132df20d16020..dd488e0571d7e9420d749f803389871fa93c5a96 100644 (file)
--- 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 a6cb629ce4304281b82bef7855d86e6fa4bda43d..f4b974b3039ffcda3888a3c81add2f86e999ea99 100644 (file)
--- 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);