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".
Loup Vaillant [Mon, 9 Oct 2017 21:42:30 +0000 (23:42 +0200)]
Added crypto_sign 2 pass interface
Also refined the crypto_check incremental interface (again). Having
to pass arguments in the final() function we already passed to init()
is cumbersome and error prone. I removed this nonsense.
Loup Vaillant [Sun, 8 Oct 2017 20:19:45 +0000 (22:19 +0200)]
moved SHA-512 source files to src/optional
There are 2 reasons behind this change:
- The primary way to install Monocypher is to copy the source files
into one's own project. But it wasn't clear whether `sha512.c` and
`sha512.h` are meant to be copied as well.
- Monocypher is advertised as a single source file library (or a 2
files library if you count the header), and a casual glance may
disagree.
Now things should be much clearer.
---
I made another slight change to the vector generation process: I
removed the optimisation options, which in conjunction with `-std=c99`
seem to trigger a bug in GCC 5.4.0 (it can't find a type definition).
Clang works.
Those optimisation options slowed down the whole process anyway, so no
loss there.
Loup Vaillant [Wed, 4 Oct 2017 21:28:53 +0000 (23:28 +0200)]
Added a make tarball rule to generate an archive
Also updated the README.md a little: added "manual" and "contributor
notes" sections, expanded the installation section, and a couple minor
other edits.
CuleX [Wed, 4 Oct 2017 18:53:48 +0000 (20:53 +0200)]
Fix mandoc invocation for recent mandoc versions
A commit in mandoc earlier this year subtly broke the -O parsing.
Multiple instances of -O do not get parsed, so all options have to be
passed into the same -O with comma separation as intended.
CuleX [Mon, 2 Oct 2017 04:14:06 +0000 (06:14 +0200)]
Improve the man page for incremental crypto_lock
This fixes the function types in the SYNOPSIS section and removes a
stray macro.
This adds information about the incremental interface to the DESCRIPTION
section. In particular, it documents the tradeoff (convenience of the
interface vs. performance loss on forged messages).
INCREMENTAL INTERFACE, which seemed to just be a subset to the EXAMPLES
section, got lowered into a second-level heading.
Added a `check` target to the makefile, that means the same as `test`
Automake specifies that `make check` runs the test suite. We should
respect such conventions. `make test` still works ("test" is a good
name for such a target).
EdDSA works. SHA-512 is properly tested. Replacing Blake2b by
SHA-512 is only a pre-processor directive away —it's foolproof. We
don't need specific Ed-25519 tests.
Since Monocypher can be used without any installation (just copy the
source files to your project), some users may want the man pages
without an actual installation.
`make install` still installs everything, documentation included.
Man pages belong to $(DESTDIR)/$(PREFIX)/share/man/man3 folder
They were originally sent to the man3monocypher folder instead, but we
don't need that: their .3monocypher extension already takes care of
the disambiguation. It also has the advantage of allowing the user to
search for the man page in section 3 directly.
Chacha20 plaintext and cypher text memory buffers may be the same
(they cannot be different *and* overlaping).
Poly1305 input and tag buffer may overlap.
Blake2b input and hash buffers may overlap.
SHA-512 input and hash buffers may overlap.
Argon2i input and hash buffers may overlap.
EdDSA message and signature buffers may overlap.
Defines and uses the $DESTDIR and $PREFIX variables. They can be
overriden from the command line. By default, they are set to "" and
"usr/local" respectively.
Defines and uses a $PKGCONFIG variable to set the location of the
pkgconfig configuration file (monocypher.pc). That variable depends
on $PREFIX.
Copies libmonocypher.a, libmonocypher.so and monocypher.h to their
respective destinations, and creates the pkgconfig configuration file.
Note: I noticed something iffy about comparing against all zeroes: for
big buffers, the timings were way off (small buffers were okay). This
suggest they were *not* constant time, which is worrying.
The generated assembly is too big for me to review. I can't tell
whether there's a variable time optimisation in there. Thankfully, we
rarely use crypto_memcmp() to compare big zeroed buffers in practice.
Instead, we compare small, pseudo random data such as hashes or
authentication tags. So I used pseudo-random data for the tests.
While we should be good in practice, I'm a bit worried. Someone may
want to check that compilers haven't become too clever.
Users who try to `make test` without the test vectors will have a nice
error message explaining how to actually perform the tests (either
generate those test vectors, or grab an official release.
It should thus be clear what libsodium is for, and why end users don't
need it.