Loup Vaillant [Wed, 1 Nov 2017 20:07:14 +0000 (21:07 +0100)]
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.
Loup Vaillant [Thu, 19 Oct 2017 21:27:00 +0000 (23:27 +0200)]
Added crypto_wipe() (Erases buffers)
I've been convinced that wiping secrets might be useful to mitigate
some side channel attacks where the attacker might read your memory
after you're done processing those secrets.
Loup Vaillant [Mon, 16 Oct 2017 21:46:39 +0000 (23:46 +0200)]
Manual review: Chacha20
I intended this to be a fairly light review, but this ended up being a
rather comprehensive rewrite...
I tried to follow the advice of mdoc(7) as much as possible. CuleX
did a remarkable job adapting the old manual to man pages without
butchering the original text; but I now think it has to be butchered
eventually.
Expanded the EXAMPLES section. I think it gives a clearer view of all
possible use cases that way.
Replaced STANDARDS by IMPLEMENTATION DETAILS. The choice of primitive
has implications for the end users, and thus isn't a mere
implementation detail. Also serves to emphasise that Monocypher does
implement widely reviewed standards, as opposed to home-invented
crypto.
Replaced CAVEATS by SECURITY CONSIDERATIONS. Arguably, either section
would do. I changed it because every consideration listed there would
trigger a vulnerability if not observed.
Other changes:
* crypto_lock and crypto_aead_lock pages have been merged as they are
closer to each other in complexity than the incremental and low-level
crypto_lock interface.
The crypto_memcmp and crypto_zerocmp pages have not been removed in case
people still have references to those functions in their code and are
wondering what the canonical replacement is.
Loup Vaillant [Sat, 14 Oct 2017 10:11:20 +0000 (12:11 +0200)]
Fixed bogus comparison functions
Found by michaelforney on Github.
- On neq0 , I used ^ instead of |
- On zerocmp32, I used + instead of |
Both errors lead to false negatives: you *think* all went well and the
number looks like it is indeed, zero, but it's not. This could lead
to vulnerabilities in practice, where we could use the flaws in the
comparison functions to find pseudo-collisions, that defeat the checks
without being actual collisions.
Loup Vaillant [Fri, 13 Oct 2017 20:37:34 +0000 (22:37 +0200)]
Removed crypto_memcmp and crypto_zerocmp
Fixes #38
This breaks compatibility. Users need to switch to the crypto_verify
functions. Sorry.
I do not break compatibility lightly.
Under the heaviest optimisation options (-O3), the old comparison
functions generated a huge amount of code, with quite a few
conditional branches. It wasn't clear those branches weren't input
dependent. This could lead to timing attacks down the line.
This is not just theoretical. During my tests, I have observed
suspect timings (that's why I looked at the assembly in the first
place). I tried to tweak the implementations, to no avail (some of my
tweaks actually made things worse).
Using more reasonable optimisation settings (-O2) is not an option:
the performance of `-O3` is simply too juicy to be ignored. Some
users *will* sacrifice security to use it, even if I tell them not to.
The crypto_verify functions emit very terse and clean assembly, which
contains no conditional branches, and no input dependent indices.
That I can trust.
Loup Vaillant [Wed, 11 Oct 2017 18:57:15 +0000 (20:57 +0200)]
Renamed init1 and init2 into init_first_pass and init_second_pass
The names are a bit long for my taste, but we must be absolutely clear
to the user that we need two passes. Hopefully "first" will act as a
strong enough hint that there is a "second".