Commit graph

71 commits

Author SHA1 Message Date
Jarno Lamsa 83d7881cec Make VS compiler happy
It doesn't seem to like using unary - to unsigned values.
2019-12-04 14:40:57 +02:00
Manuel Pégourié-Gonnard 231bf52691 Fix indentation level in one place 2019-11-28 12:22:43 +01:00
Manuel Pégourié-Gonnard e1cb8846e7 Add loop integrity check to curve param check
Also make the reference result static const while at it.
2019-11-28 12:21:34 +01:00
Manuel Pégourié-Gonnard 5c3066a4f6 Add double-checking in some critical places 2019-11-27 13:01:10 +01:00
Manuel Pégourié-Gonnard 98e1fe0796 Add flow control in uECC_vli_equal loop 2019-11-27 12:52:54 +01:00
Manuel Pégourié-Gonnard 9d6a535ba1 Return and propagate UECC_FAULT_DETECTED
This commit first changes the return convention of EccPoint_mult_safer() so
that it properly reports when faults are detected. Then all functions that
call it need to be changed to (1) follow the same return convention and (2)
properly propagate UECC_FAULT_DETECTED when it occurs.

Here's the reverse call graph from EccPoint_mult_safer() to the rest of the
library (where return values are translated to the MBEDTLS_ERR_ space) and test
functions (where expected return values are asserted explicitly).

EccPoint_mult_safer()
    EccPoint_compute_public_key()
        uECC_compute_public_key()
            pkparse.c
            tests/suites/test_suite_pkparse.function
        uECC_make_key_with_d()
        uECC_make_key()
            ssl_cli.c
            ssl_srv.c
            tests/suites/test_suite_pk.function
            tests/suites/test_suite_tinycrypt.function
    uECC_shared_secret()
        ssl_tls.c
        tests/suites/test_suite_tinycrypt.function
    uECC_sign_with_k()
        uECC_sign()
            pk.c
            tests/suites/test_suite_tinycrypt.function

Note: in uECC_sign_with_k() a test for uECC_vli_isZero(p) is suppressed
because it is redundant with a more thorough test (point validity) done at the
end of EccPoint_mult_safer(). This redundancy was introduced in a previous
commit but not noticed earlier.
2019-11-26 12:54:06 +01:00
Manuel Pégourié-Gonnard 4d6186beb0 Rename ATTACK_DETECTED to FAULT_DETECTED
We don't know for sure it's an attack, it could be the hardware failing
randomly as well.
2019-11-26 12:54:06 +01:00
Manuel Pégourié-Gonnard 2b90961b8d Add integrity check for curve parameters
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.
2019-11-26 12:54:06 +01:00
Manuel Pégourié-Gonnard 1a5337179f Remove curve parameter from public functions 2019-11-26 12:54:06 +01:00
Manuel Pégourié-Gonnard be5f833c9c Remove curve parameter from (semi-)internal functions
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.
2019-11-26 12:54:06 +01:00
Manuel Pégourié-Gonnard bc3f49011a Remove struct curve entirely 2019-11-26 12:54:06 +01:00
Manuel Pégourié-Gonnard ffd13996fd Move b from curve structure to its own constant
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.
2019-11-26 12:54:04 +01:00
Manuel Pégourié-Gonnard a6115087a0 Move G from struct curve to its own constant 2019-11-26 12:53:13 +01:00
Manuel Pégourié-Gonnard 356d8594d7 Move n from struct curve to its own constant 2019-11-26 12:52:57 +01:00
Manuel Pégourié-Gonnard 4d8777cbb6 Move p from curve structure to its own constant
This removes an indirection, which both makes the code smaller and decreases
the number of glitching opportunities for an attacker.
2019-11-26 12:51:58 +01:00
Manuel Pégourié-Gonnard 30833f2a07 Remove num_n_bits member from curve structure 2019-11-21 15:37:22 +01:00
Manuel Pégourié-Gonnard 72c1764c00 Remove num_bytes member from curve structure
Reduces code size and size of the structure.
2019-11-21 15:37:22 +01:00
Manuel Pégourié-Gonnard 1765933ab2 Remove num_words member from curve structure
Saves code size, and makes the curve structure simpler.
2019-11-21 15:37:22 +01:00
Manuel Pégourié-Gonnard 1c6f7eae2d Remove function pointers from curve structure
They're not needed in practice, and removing them decreases the code size
slightly and provides less opportunities for an attacker.
2019-11-21 15:37:22 +01:00
Manuel Pégourié-Gonnard e714332563 Add pre and post-validation to mult_safer()
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().
2019-11-21 15:37:22 +01:00
Manuel Pégourié-Gonnard 41ab8cb6cb Centralize everything to EccPoint_mult_safer()
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
2019-11-21 15:37:22 +01:00
Manuel Pégourié-Gonnard 72a8c9e7dc Force some compilers to respect volatile reads
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.
2019-11-21 15:14:59 +01:00
Manuel Pégourié-Gonnard e6d6f17738 Add double-checking of critical value in uECC_verify()
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.
2019-11-21 15:14:59 +01:00
Manuel Pégourié-Gonnard 2b6312b7d9 Harden return value of uECC_vli_equal()
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.
2019-11-21 15:12:44 +01:00
Manuel Pégourié-Gonnard 10d8e8ed64 Use safer return values in uECC_verify()
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.
2019-11-21 15:12:44 +01:00
Simon Butcher a3877007e6 Merge remote-tracking branch 'public/pr/2876' into baremetal 2019-11-20 12:00:18 +00:00
Manuel Pégourié-Gonnard c881486bb2 Fix off-by-one number of extra operations
This caused a performance issue.
2019-11-05 10:32:37 +01:00
Manuel Pégourié-Gonnard ad166d8db7 Also check curve in verify()
This is the only function that performs computations without calling
EccPoint_mult_safer() and that didn't have that guard yet.
2019-11-04 15:53:24 +01:00
Manuel Pégourié-Gonnard 913534837a Hardcode numwords in vli_modInv 2019-11-04 15:53:22 +01:00
Manuel Pégourié-Gonnard 3e20adf533 Hardcode numwords in vli_modMult 2019-11-04 15:53:20 +01:00
Manuel Pégourié-Gonnard 10349e4912 Hardcode numwords in vli_mmod 2019-11-04 15:53:19 +01:00
Manuel Pégourié-Gonnard 1b0875d863 Hardcode numwords in vli_modSub 2019-11-04 15:53:17 +01:00
Manuel Pégourié-Gonnard 0779be7f31 Hardcode numwords in vli_modAdd 2019-11-04 15:53:14 +01:00
Manuel Pégourié-Gonnard 5e3baf2303 Hardcode numwords in vli_rshift1 2019-11-04 15:53:12 +01:00
Manuel Pégourié-Gonnard 2cb3eea922 Hardcode numwords in vli_cmp 2019-11-04 15:53:10 +01:00
Manuel Pégourié-Gonnard 129b42ea2e Hardcode numwords in vli_sub 2019-11-04 15:53:09 +01:00
Manuel Pégourié-Gonnard 2eca3d367b Hardcode numwords in vli_equal 2019-11-04 15:53:07 +01:00
Manuel Pégourié-Gonnard a752191191 Hardcode numwords in vli_cpm_unsafe 2019-11-04 15:53:03 +01:00
Manuel Pégourié-Gonnard cbbb0f034b Hardcode numwords in vli_set() 2019-11-04 15:52:43 +01:00
Manuel Pégourié-Gonnard 2bf5a129cf Hardcode numwords in semi-internal vli_numBits() 2019-11-04 15:52:43 +01:00
Manuel Pégourié-Gonnard 94e48498ef Hardcode numwords in semi-internal vli_clear() 2019-11-04 15:52:43 +01:00
Manuel Pégourié-Gonnard f3899fc0ea hardcode numwords in semi-internal vli_isZero 2019-11-04 15:52:43 +01:00
Manuel Pégourié-Gonnard 02d9d21fd6 Hardcode numwords in internal vli_add
Saves 40 bytes
2019-11-04 15:52:37 +01:00
Manuel Pégourié-Gonnard 78a7e351fe Use macros for number of bits and words 2019-11-04 12:31:37 +01:00
Manuel Pégourié-Gonnard c3ec14c87f Harcode curve in semi-internal modMult function
Saves 80 bytes of code size.
2019-11-04 12:23:11 +01:00
Manuel Pégourié-Gonnard 3645ac93f5 Start hardcoding curve in internal functions
Saves 68 byte of code size.
2019-11-04 12:20:22 +01:00
Manuel Pégourié-Gonnard 27926d63b7 Remove less-safe mult function from public API
This doesn't change code size, but makes it easier to remove unneeded
parameters later (less possible entry points).
2019-11-04 11:26:46 +01:00
Manuel Pégourié-Gonnard ef238283d5 Add ECCPoint_mult_safer() function
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.
2019-11-04 11:19:30 +01:00
Manuel Pégourié-Gonnard c78d86b499 Remove some internal functions that aren't needed
This saves 10 bytes of code size, and makes it a bit easier to remove unused
parameters later (fewer prototypes to change).
2019-11-04 10:18:42 +01:00
Manuel Pégourié-Gonnard 86c4f81408 Improve documentation of internal function 2019-10-31 13:07:58 +01:00