From 26f8925aabcb4feb89b0bc468e41cec89e963d76 Mon Sep 17 00:00:00 2001 From: Loup Vaillant Date: Wed, 1 Nov 2017 21:07:14 +0100 Subject: [PATCH] Automatically wipe Argon2i work area crypto_wipe() wipes byte by byte. This is fine for small buffers, but for the Argon2i work area, it means losing about 20% performance. This has a direct impact on security: users are advised to chose the highest settings they are comfortable with. A 20% slow down will mean a 20% edge for the attacker.) Users must then chose between sacrificing 20% of security, or exposing themselves to side channel attacks. --- There is a faster way to wipe that work area: word by word. Since it is already required to be aligned for 8-byte words, we can wipe it in 8-bytes chunks. This is much faster than crypto_wipe, and slows down the whole process by only 2-3%. This is a bit ad-hoc, though, and it wouldn't make much sense to add a crypto_wipe_fast() function or something to handle that special case. Instead, I've chosen to integrate it in Argon2i itself. Now users don't have to wipe the work area any more. The drawback is, the output hash buffer must not overlap with the work area, or it will be wiped with it. This shouldn't be a problem in practice. --- doc/man/man3/crypto_argon2i.3monocypher | 12 +++++++----- src/monocypher.c | 10 ++++++---- tests/test.c | 20 ++++++++++---------- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/doc/man/man3/crypto_argon2i.3monocypher b/doc/man/man3/crypto_argon2i.3monocypher index a1c005f..109955d 100644 --- a/doc/man/man3/crypto_argon2i.3monocypher +++ b/doc/man/man3/crypto_argon2i.3monocypher @@ -54,7 +54,8 @@ Must be .Fn malloc should yield a suitable chunk of memory. .Pp -The work area should be wiped after use. +The work area is automatically wiped by +.Fn crypto_argon2i . .It Fa nb_blocks The number of blocks for the work area. Must be at least 8. @@ -97,7 +98,7 @@ supposed to be unknown to the attacker. In the context of password derivation, it would stay unknown .Em even if an attacker steals the password database . This may be possible if that key is stored on a separate server. -Note: changing the key, requires hashing the user's password, +Note: changing the key requires hashing the user's password, which is only possible upon user login. .It Fa key_size the length of the key, in bytes. @@ -119,7 +120,9 @@ the length of .Fa ad . .El .Pp -All pointer arguments may overlap. +The output hash must not overlap with the work area, or it will be +wiped along with it. +Any other overlap is permitted. .Pp Use .Xr crypto_verify16 3monocypher , @@ -167,8 +170,7 @@ crypto_argon2i(hash, 32, 0, 0, /* no key */ 0, 0); /* no additional data */ /* Wipe secrets if they are no longer needed */ -crypto_wipe(password , password_size ); -crypto_wipe(work_area, nb_blocks * 1024); +crypto_wipe(password, password_size); .Ed .Sh SEE ALSO .Xr crypto_lock 3monocypher , diff --git a/src/monocypher.c b/src/monocypher.c index 0a847ea..744ac1a 100644 --- a/src/monocypher.c +++ b/src/monocypher.c @@ -962,10 +962,12 @@ void crypto_argon2i(u8 *hash, u32 hash_size, store_block(final_block, blocks + (nb_blocks - 1)); extended_hash(hash, hash_size, final_block, 1024); - crypto_wipe(final_block , 1024); - // Note: the work area is *not* wiped by default. It would erase - // the final hash if the user wanted to use it to store the final - // hash, for instance to conserve stack space. + // wipe final block and work area + crypto_wipe(final_block, 1024); + volatile u64 *p = (u64*)work_area; + FOR (i, 0, 128 * nb_blocks) { + p[i] = 0; + } } //////////////////////////////////// diff --git a/tests/test.c b/tests/test.c index 2808b83..de98836 100644 --- a/tests/test.c +++ b/tests/test.c @@ -492,26 +492,26 @@ static int p_argon2i_overlap() int status = 0; FOR (i, 0, 128) { RANDOM_INPUT(work_area, 1024 * 8); - u32 hash_offset = rand64() % 128; u32 pass_offset = rand64() % 128; u32 salt_offset = rand64() % 128; u32 key_offset = rand64() % 128; u32 ad_offset = rand64() % 128; u8 clean_work_area[1024 * 8]; - u8 hash[32]; - u8 pass[16]; FOR (i, 0, 16) { pass[i] = work_area[i + pass_offset]; } - u8 salt[16]; FOR (i, 0, 16) { salt[i] = work_area[i + salt_offset]; } - u8 key [32]; FOR (i, 0, 32) { key [i] = work_area[i + key_offset]; } - u8 ad [32]; FOR (i, 0, 32) { ad [i] = work_area[i + ad_offset]; } - - crypto_argon2i(hash, 32, clean_work_area, 8, 1, + u8 hash1[32]; + u8 hash2[32]; + u8 pass [16]; FOR (i, 0, 16) { pass[i] = work_area[i + pass_offset]; } + u8 salt [16]; FOR (i, 0, 16) { salt[i] = work_area[i + salt_offset]; } + u8 key [32]; FOR (i, 0, 32) { key [i] = work_area[i + key_offset]; } + u8 ad [32]; FOR (i, 0, 32) { ad [i] = work_area[i + ad_offset]; } + + crypto_argon2i(hash1, 32, clean_work_area, 8, 1, pass, 16, salt, 16, key, 32, ad, 32); - crypto_argon2i(work_area + hash_offset, 32, work_area, 8, 1, + crypto_argon2i(hash2, 32, work_area, 8, 1, work_area + pass_offset, 16, work_area + salt_offset, 16, work_area + key_offset, 32, work_area + ad_offset, 32); - status |= memcmp(hash, work_area + hash_offset, 32); + status |= memcmp(hash1, hash2, 32); } printf("%s: Argon2i (overlaping i/o)\n", status != 0 ? "FAILED" : "OK"); return status; -- 2.47.3