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.
Loup Vaillant [Mon, 30 Mar 2020 13:28:43 +0000 (15:28 +0200)]
Added constant time tests with Valgrind
The trick is to call Monocypher API with uninitialised buffers.
If Valgrind complains about uninitialised something, that means an array
index or a conditional jump depends on secret data.
Note that crypto_check() is not tested: that's because it doesn't even
try to be constant time.
Note that a couple tested functions do have secret dependent conditional
jumps. Those jumps however are just final checks, that just reveal
success or failure (and those are revealed anyway, as part as the
semantics of the function being tested).
Note that optimisations are disabled for the compilation of `ctgrind.c`
and the linking of `ctgrind.out`. This is an attempt to maximise
Valgrind's findings.
Also note that Valgrind seems to miss a secret dependent conditional
jump (it finds only one where we should have 2). But that may just be
Valgrind squashing the error report, instead of an actual miss.
Fabio Scotoni [Fri, 27 Mar 2020 06:29:47 +0000 (07:29 +0100)]
doc: more details on mitigating power side channels in EdDSA
While already there, add a very sternly worded warning about omitting
the first pass that will *appear* to work but will, in fact, just repeat
the Sony PlayStation 3 ECDSA nonce disaster with EdDSA instead.
RFC 8032 § 8.7 already hates Monocypher's guts for providing this risky
interface at all, so we might as well use it for good:
By showing how it can be used to mitigate power analysis attacks.
The wording is such that crypto_sign.3monocypher redirects to
crypto_sign_init_first_pass.3monocypher for how to mitigate
power-related side channels;
crypto_ed25519_sign_init_first_pass.3monocypher already points to
crypto_sign_init_first_pass.3monocypher wholesale anyway.
I've intentionally broken the rule that
crypto_sign_init_first_pass.3monocypher *only* talks about BLAKE2b in
this specific instance because of the redirect on the Ed25519 page so
that this content doesn't need to be duplicated.
There's no issue doing this with the example code because both hash
functions call their internal compression functions.
While I could've just *described* what to do,
I'd feel uneasy leaving implementers just guessing what it is that we
mean and overshoot or undershoot by 32 bytes (undershooting being
particularly fatal) or just be too scared to try at all,
so I've added example code nonetheless.
It's been adorned with the bare minimum of an explanation about the
magic number 128-32.
Ideally, I'd have a good place to go on at length about EdDSA nonces,
but there really isn't.
On the other hand, I have very much *intentionally* omitted the fact
that you could be okay just hashing a random nonce in (which then should
be preferably at least 32 bytes, though you might be able to get away with
less as well, I don't think there's a well-defined threshold for
randomness with hash->reduce) or other kinds of nonces in the first pass
of EdDSA in particular.
While this is interesting and sometimes very much useful knowledge,
it's also a large footgun and the whole reason why RFC 8032 § 8.7
recommends against init-update-final interfaces in APIs (unless using
Ed25519ph, but that means you need a collision-resistant hash function
as the prehash, losing the security benefits of *not* requiring
collision resistance from the hash function in EdDSA in the first
place).
Fabio Scotoni [Tue, 24 Mar 2020 07:41:48 +0000 (08:41 +0100)]
doc: contributory behavior may be required sometimes
While already there, hoist the explanation about contributory behavior
from RETURN VALUES to the main DESCRIPTION section.
The only reason it was in RETURN VALUES is because of historical
reasons; we used to justify why the return value was deprecated there,
so the position of the explanation made sense before removal of the
return value.
Loup Vaillant [Wed, 18 Mar 2020 14:40:04 +0000 (15:40 +0100)]
Elligator: take cofactor from secret key instead of tweak
This allows the simplification of the implementation of higher level
interfaces.
The idea is, only the scalar and cofactor have any influence over
whether the inverse map succeeds or fail. This means that when it fails,
the padding & sign have not be used at all, and can be "reused" to
generate another random seed.
In practice, this means we can use Chacha20 or Blake2, or any hash that
outputs 64 random bytes from 32 random bytes, use 32 bytes to make an
attempt, then use the *other* 32 bytes to either generate more random
bytes (if we failed), or to use the tweak (if we succeed).
The tweak has also been modified to simplify the implementation. The
sign bit is now the least significant bit, and the padding bits are the
most significant bits. The computational savings are negligible, but
this allows neat micro-simplifications.
Loup Vaillant [Wed, 18 Mar 2020 11:27:31 +0000 (12:27 +0100)]
Added easy interface for Elligator
Note a small problem in the implementation: we are reusing one byte for
both the tweak and the next random seed. This makes them *not*
independent, and a possible source of vulnerability.
In practice, this is only a problem for the 3 bits comprising the
cofactor, since the sign and the padding do not play a role in deciding
whether the mapping fails or succeeds.
TODO: take the cofactor from the clamped bits of the scalar, instead of
the tweak. This will ensure proper independence, while keeping the high
level code simple and maximally efficient.
projects / Monocypher.git / log