Loup Vaillant [Mon, 9 Nov 2020 13:04:18 +0000 (14:04 +0100)]
Added test vectors from Kleshni
Fixes #181
The MON-01-004 issue from Cure53's audit noted that Monocypher did not
compare to <https://github.com/Kleshni/Elligator-2>, which I didn't know
of at the time. Some test vectors were added back then, but full
interoperability was not yet ascertained. (Moreover, I though I'd added
vectors for the reverse map, and somehow didn't. This is now fixed.)
Now I have been able to generate decoding (direct map) test vectors from
Kleshni's implementation, that Monocypher matches perfectly. For the
inverse map however, I was not so lucky: Monocypher and Kleshni disagree
on quite a few points, including those used in Kleshni's test vectors.
Some investigation revealed that currently, Kleshni's encoding (inverse
map) is not reliable. In some cases, the round trip fails to yield the
same point we started with (and it's not just a matter of chopping off
the most significant bit).
However, Monocypher and Kleshni *do* agree on some points, which I have
added (and *checked* I have added) to the list of test vectors. There's
just one divergence left: Monocypher fails to encode the zero point,
which is a departure from the standard (we're supposed to output the
zero representative instead).
Loup Vaillant [Sun, 8 Nov 2020 21:41:58 +0000 (22:41 +0100)]
Documented 2^255-19 carry propagation
Fixes #185
Carry propagation is now justified, in a way that I can personally vouch
for (I used to rely on SUPERCOP's ref10 code and proofs).
The use of arithmetic right shifts is also documented, and a workaround
has been devised in case someone somewhere uses a platforms that does
not perform sign extension. (That will never happen.)
Loup Vaillant [Sun, 8 Nov 2020 12:17:08 +0000 (13:17 +0100)]
Fixed assumption in 2^255-19 carry propagation
Careful re-examination of the carry propagation code revealed that
SUPERCOP's invariants for fe_tobytes() were not respected: there is a
possibility that the inputs slightly outrange the set of input for which
SUPERCOP's original proof was intended.
This happens in invsqrt(), used for EdDSA verification And Elligator,
and the reverse map of Elligator. X25519 is unaffected.
Note that we were unable to produce a set of input that actually
triggers this range overflow. Moreover, careful mathematical analysis
(and tests with SAGE) showed that fe_tobytes() is actually much more
tolerant than SUPERCOP's proof let on. As far as I can tell, this
slight overflow cannot trigger any observable bug.
Still, I figured it would be a good idea to abide those invariants
anyway, if only to facilitate future audits. To this end, I made sure
the inputs of fe_tobytes() came directly from either multiplications
(which perform a carry propagation), or constants (where carry
propagation has been pre-computed).
Loup Vaillant [Sat, 7 Nov 2020 23:22:09 +0000 (00:22 +0100)]
Tests: fixed tweak coverage for Elligator.
Shifting the index by 6 caused a reuse of one bit, leading to 4
different configurations instead of 8.
Shifting by 5 means we are using the 3 least significant bits of the
index, as was always intended.
TIS-CI needs a dedicated test suite, different from the regular one. So
I'm following my Chief Testing Officer's advice, and stole his work like
I should have from the very beginning.
We'll need to refine this, but this should be a good first step.
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.
Loup Vaillant [Sun, 28 Jun 2020 22:40:50 +0000 (00:40 +0200)]
Added Kleshni/Elligator-2 test vectors
An auditor recently told me about the following repository on GitHub:
https://github.com/Kleshni/Elligator-2/
I was able to steal a couple test vectors from them. Not all of them
unfortunately:
- Some representative exceed 2^254, and Monocypher do not decode
negative representatives. Instead, it assumes it has a positive
representative, and clears the two most significant bits before
decoding.
- It is not clear yet what encoding does, and some points in the (few)
test vectors have their most significant bit set. Monocypher ignores
the most significant bit of curve point, basically assumes they are
all below 2^255 - 19. Adding those points will require tweaking
similar to the tweaking applied to the Hash to Curve RFC draft test
vectors.
Loup Vaillant [Mon, 8 Jun 2020 15:46:16 +0000 (17:46 +0200)]
Optimised key loading in Blake2b
The idea is to avoid the slow loading code in the internal
blake2b_update() function, and avoid the overhead of calling
crypto_blake2b_update().
It's a micro-optimisation that in principle shouldn't matter that much,
but it might help a bit if we repeatedly hash small messages with a key,
as can happen in authenticated key exchanges like Monokex.
Loup Vaillant [Fri, 29 May 2020 21:40:50 +0000 (23:40 +0200)]
Removed #define from vectors.h
Also reduced the size of vectors.h by removing indirections and printing
numbers in decimal form. Hopefully this will make the tarball a little
smaller.
It appears that arithmetic on NULL pointers is undefined, even when we
just add zero.
Monocypher generally allows input buffers to be NULL pointers if their
length is zero. This is because we never dereference those pointers in
this case. Likewise, we should not perform any arithmetic on them.
The fix is to return immediately when the input buffer length is zero.
MSVC issues a warning when we try to negate an unsigned number. The goal
however was to perform bit twiddling, so I used bitwise negation.
Hopefully the compiler will not complain about overflow, which is
perfectly well defined on unsigned numbers.
There used to be two ways to perform carry propagation: a "fast" one,
used after decoding and multiplication by small numbers, and a "safe"
one, more conservative, used after full multiplications or squarings.
Using the safe carry propagation simplifies the source code and
facilitates audits. The cost is a 1% performance hit for X25519.
Fabio Scotoni [Tue, 31 Mar 2020 13:11:10 +0000 (15:11 +0200)]
Address review concerns from #164.
1. Remove recommendation for 512-bit BLAKE2b.
32 bytes is enough, and it's not like we offer EC functions of a
higher security level either.
The text added in 628f027 already does enough to recommend proper
hash output lengths.
2. Bump .Dd date in crypto_poly1305.3monocypher.
3. crypto_verify16 add "byte by byte" for both accuracy of how a MAC
with a variable-time comparison function will be found and
for dramatic reasons because it sounds like doom is slowly
approaching, byte by byte.
Fabio Scotoni [Tue, 31 Mar 2020 12:08:19 +0000 (14:08 +0200)]
crypto_sign_init_first_pass: Swap emphasis
The original emphasis, when skimmed, made the message read that the
first pass *causes* loss of all security, which is the opposite of what
we want to emphasize.
Also bump authorship notice years while there and mdoc date that we
forgot to bump when we introduced the power-analysis and glitching
example.
Fabio Scotoni [Tue, 31 Mar 2020 11:49:55 +0000 (13:49 +0200)]
crypto_verify: wording nitpicks
1. s/guessed a few bytes/guessed a byte/
Nobody guesses multiple bytes per attempt except by sheer dumb luck.
2. Add missing "functions" to make one sentence not seem incomplete.