We don't really need a secure hash for that, something like CRC32 would
probably be enough - but we have SHA-256 handy, not CRC32, so use that for the
sake of simplicity.
By semi-internal I mean functions that are only public because they're used in
more than once compilation unit in the library (for example in ecc.c and
ecc_dsa.c) but should not really be part of the public-facing API.
Same motivation as for the other parameters. This is the last one, making the
curve structure empty, so it's left with a dummy parameter for legal reasons.
This commit removes from the TinyCrypt header and source code files, the
configuration condition on MBEDTLS_USE_TINYCRYPT to include the file
contents.
This is to allow use of the library by the Factory Tool without enabling
MBEDTLS_USE_TINYCRYPT, and also removes a modification we've made to make the
code closer to the upstream TinyCrypt making it easier to maintain.
Validating the input is always a good idea. Validating the output protects
against some fault injections that would make the result invalid.
Note: valid_point() implies that the point is not zero.
Adding validation to mult_safer() makes it redundant in
compute_shared_secret().
This will make easier to add future counter-measures in a single place.
In practice this change means that:
- compute_public_key() now uses projective coordinate randomisation, which it
should as this is a protection against Template Attacks for example.
- mult_safer() now checks that the result is not the point at infinity, which
it can as the result is indeed never expected to be that
Inspection of the generated assembly showed that before this commit, armcc 5
was optimizing away the successive reads to the volatile local variable that's
used for double-checks. Inspection also reveals that inserting a call to an
external function is enough to prevent it from doing that.
The tested versions of ARM-GCC, Clang and Armcc 6 (aka armclang) all keep the
double read, with our without a call to an external function in the middle.
The inserted function can also be changed to insert a random delay if
desired in the future, as it is appropriately places between the reads.
This hardens against attacks that glitch the conditional branch by making it
necessary for the attacker to inject two consecutive faults instead of one. If
desired, we could insert a random delay in order to further protect against
double-glitch attacks.
Also, when a single glitch is detected we report it.
Previously it was returning 0 or 1, so flipping a single bit in the return
value reversed its meaning. Now it's returning the diff itself.
This is safe because in the two places it's used (signature verification and
point validation), invalid values will have a large number of bits differing
from the expected value, so diff will have a large Hamming weight.
An alternative would be to return for example -!(diff == 0), but the
comparison itself is prone to attacks (glitching the appropriate flag in the
CPU flags register, or the conditional branch if the comparison uses one). So
we'd need to protect the comparison, and it's simpler to just skip it and
return diff itself.
This is a first step in protecting against fault injection attacks: the
attacker can no longer change failure into success by flipping a single bit.
Additional steps are needed to prevent other attacks (instruction skip etc)
and will be the object of future commits.
The return value of uECC_vli_equal() should be protected as well, which will
be done in a future commit as well.
This avoids the need for each calling site to manually regularize the scalar
and randomize coordinates, which makes for simpler safe use and saves 50 bytes
of code size in the library.
Even though this is type name is purely internal to a single C file, let's
reduce the potential for clashes with other wait state types which might be
added elsewhere in the library and become visible here (for example through
platform_util.h).
Previous size was 3584 bytes which is not acceptable on constrained systems
(especially on the stack). This was a misguided attempt at minimizing the
number of calls to the RNG function in order to minimize impact on
performance, but clearly this does not justify using that much RAM and a
compromise had to be found.
While at it, loose the 'curve' argument in internal randomized functions, for
the same reasons we lost 'num_words' in uECC_vli_mult_rnd(): we only have one
curve so we don't need this, and hardcoding it saves a bit of code size and
speed, which is welcome to slightly reduce the impact of the counter-measure
on both of them.
This is a counter-measure to make horizontal attacks harder. Horizontal
attacks work with a single trace by noticing when intermediate computations
within that trace happen on the same operands.
We'll try to make that harder for an attacker to achieve that by introducing
random delays based on extra computation and extra random accesses to input in
the multi-precision multiplication (which is the dominant operation and the target of
horizontal attacks known so far). This should make it hard for the attacker to
compare two multiplications.
This first commit introduces the new function for multiplication with random
delay - future commits will ensure it is used all the way up to the top-level
scalar multiplication routine.
Why: this protects against potential side-channels attacks. This
counter-measure is for example effective against Template SPA. Also, the
bignum arithmetic as implemented in TinyCrypt isn't entirely regular, which
could in principle be exploited by an attacker; randomizing the coordinates
makes this less likely to happen.
Randomizing projective coordinates is also a well-known countermeasure to DPA.
In the context of the scalar multiplication in ECDSA, DPA isn't a concern
since it requires multiple measurements with various base points and the same
scalar, and the scalar mult in ECDSA is the opposite: the base point's always
the same and the scalar is always unique. But we want protection against the
other attacks as well.
How: we use the same code fragment as in uECC_shared_secret in ecc_dh.c,
adapted as follows: (1) replace p2 with k2 as that's how it's called in this
function; (2) adjust how errors are handled.
The code might not be immediately clear so here are a few more details:
regularize_k() takes two arrays as outputs, and the return value says which one
should be passed to ECCPoint_mult(). The other one is free for us to re-use to
generate a random number to be used as the initial Z value for randomizing
coordinates (otherwise the initial Z value is 1), thus avoiding the use of an
extra stack buffer.
Steps:
1. sed -i 's/\bmemset(\([^)]\)/mbedtls_platform_memset(\1/g' library/*.c tinycrypt/*.c include/mbedtls/*.h scripts/data_files/*.fmt
2. Manually edit library/platform_util.c to revert to memset() in the
implementations of mbedtls_platform_memset() and mbedtls_platform_memcpy()
3. egrep -n '\<memset\>' library/*.c include/mbedtls/*.h tinycrypt/*.c
The remaining occurrences are in three categories:
a. From point 2 above.
b. In comments.
c. In the initialisation of memset_func, to be changed in a future commit.
This commit adds a LICENSE file and README file to tinycrypt, to help auditing
of the source code for licenses and also to indicate the origin of the work.
We called in tinycrypt in the file names, but uecc in config.h, all.sh and
other places, which could be confusing. Just use tinycrypt everywhere because
that's the name of the project and repo where we took the files.
The changes were made using the following commands (with GNU sed and zsh):
sed -i 's/uecc/tinycrypt/g' **/*.[ch] tests/scripts/all.sh
sed -i 's/MBEDTLS_USE_UECC/MBEDTLS_USE_TINYCRYPT/g' **/*.[ch] tests/scripts/all.sh scripts/config.pl
