Commit graph

15622 commits

Author SHA1 Message Date
Jaeden Amero 128c94dd87 config: Allow Mbed to implement TIMING_C
Mbed OS now provides POSIX-like time functions, although not alarm() nor
signal(). It is possible to implement MBEDTLS_TIMING_ALT on Mbed OS, so
we should not artificially prevent this in check-config. Remove the the
check that prevents implementing MBEDTLS_TIMING_ALT on Mbed OS.

Note that this limitation originally was added in the following commit,
although there isn't much context around why the restriction was
imposed: 63e7ebaaa1 ("Add material for generating yotta module"). In
2015, Mbed OS was quite a different thing: no RTOS, no threads, just an
asynchronous event loop model. I'd suppose the asynchronous event loop
model made it difficult before to implement MBEDTLS_TIMING_C on Mbed OS,
but that is no longer the case.

Fixes #4633

Signed-off-by: Jaeden Amero <jaeden.amero@arm.com>
2021-06-09 14:09:11 +01:00
Gilles Peskine 7a4c7589c8
Merge pull request #4541 from mpg/fix-ssl-cf-hmac-alt-2.x
[Backport 2.x] Fix misuse of MD API in SSL constant-flow HMAC
2021-06-07 20:53:48 +02:00
Manuel Pégourié-Gonnard 8639048631
Merge pull request #831 from gilles-peskine-arm/mpi_fill_random-constant_time_comparison-development_2.x-restricted
Backport 2.x: Fix small timing side channel in ECDSA ephemeral key generation
2021-06-07 09:10:03 +02:00
Gilles Peskine 74f66bb5c3 Fix non-constant-time comparison in mbedtls_mpi_random
Calling mbedtls_mpi_cmp_int reveals the number of leading zero limbs
to an adversary who is capable of very fine-grained timing
measurements. This is very little information, but could be practical
with secp521r1 (1/512 chance of the leading limb being 0) if the
adversary can measure the precise timing of a large number of
signature operations.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-04 14:50:23 +02:00
Manuel Pégourié-Gonnard 62da8ac37a
Merge pull request #4276 from gilles-peskine-arm/random-range-uniformity
Backport 2.x: Fix non-uniform random generation in a range
2021-06-04 10:43:25 +02:00
Gilles Peskine 23422e424c Note that the byte order in mpi_fill_random_internal() is deliberate
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-03 11:51:09 +02:00
Gilles Peskine c0b68bf03a Use MBEDTLS_MPI_CHK where warranted
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-03 11:38:26 +02:00
Gilles Peskine 3130ce24f4 New internal function mbedtls_mpi_resize_clear
The idiom "resize an mpi to a given size" appeared 4 times. Unify it
in a single function. Guarantee that the value is set to 0, which is
required by some of the callers and not a significant expense where
not required.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 23:48:07 +02:00
Gilles Peskine e4f937f5d3 Lift function call out of inner loop
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 22:28:27 +02:00
Gilles Peskine f37b9f73c7 Fix mistakes in test case descriptions
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 22:28:14 +02:00
Gilles Peskine 11779077a0 Use ternary operator with the most common case first
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine b72b7e6b9d Fix long-standing obsolete comment
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine 3f61363f8d Correct some comments about ECC in mbedtls_mpi_random
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine 346d20d209 DHM: add test case with x_size < 0
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine 9e96679548 DHM tests: add some explanations
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine 104eb82ec1 DHM: add notes about leading zeros
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine a16001e2d9 mpi_fill_random_internal: remove spurious grow() call
Since the internal function mpi_fill_random_internal() assumes that X
has the right size, there is no need to call grow().

To further simplify the function, set the sign outside, and zero out
the non-randomized part directly.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine 3d60ece16c Note that the "0 limb in ..." tests rely on undocumented behavior
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine 33701a62b2 mbedtls_mpi_random: document MBEDTLS_ERR_MPI_NOT_ACCEPTABLE
Note that this error has a negligible probability with a "crypto-sized"
bound, but macroscopic probability with a small bound.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine e39ee8e0a2 MPI random test: use more iterations for small numbers
In real life, min << N and the probability that mbedtls_mpi_random()
fails to find a suitable value after 30 iterations is less than one in
a billion. But at least for testing purposes, it's useful to not
outright reject "silly" small values of N, and for such values, 30
iterations is not enough to have a good probability of success.

Pick 250 iterations, which is enough for cases like (min=3, N=4), but
not for cases like (min=255, N=256).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:31 +02:00
Gilles Peskine 38de7ee176 MPI random test: Add test cases with lower_bound > upper_bound
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:30 +02:00
Gilles Peskine c520d7ab59 MPI random test: fix small-range test stats check when min > 1
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:30 +02:00
Gilles Peskine 8190d3129d MPI random test: Add a few more small-range tests
Do more iterations with small values. This makes it more likely that a
mistake on bounds will be detected.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:30 +02:00
Gilles Peskine b66cc7d31f Fix copypasta in test case description
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:30 +02:00
Gilles Peskine ef1325134f Contextualize comment about mbedtls_mpi_random retries
This comment is no longer in the specific context of generating a
random point on an elliptic curve.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:30 +02:00
Gilles Peskine 3b05615e5b Better document and slightly simplify >>2^n heuristic
Slightly simplify is_significantly_above_a_power_of_2() to make it
easier to understand:
* Remove the explicit negative answer for x <= 4. The only functional
  difference this makes is that is_significantly_above_a_power_of_2(3)
  is now true.
* Shift the most significant bit of x to position 8 rather than 15.
  This makes the final comparison easier to explain.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:30 +02:00
Gilles Peskine f467e1a114 MPI random: add unit tests with a previously nonzero value
Add unit tests for mbedtls_mpi_fill_random() and mbedtls_mpi_random()
when the resulting MPI object previously had a nonzero value. I wrote
those to catch a bug that I introduced during the development of
mbedtls_mpi_random() (but does not appear in a committed version).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:30 +02:00
Gilles Peskine 8f45470515 Fix mbedtls_mpi_random when N has leading zeros
mbedtls_mpi_random() uses mbedtls_mpi_cmp_mpi_ct(), which requires its
two arguments to have the same storage size. This was not the case
when the upper bound passed to mbedtls_mpi_random() had leading zero
limbs.

Fix this by forcing the result MPI to the desired size. Since this is
not what mbedtls_mpi_fill_random() does, don't call it from
mbedtls_mpi_random(), but instead call a new auxiliary function.

Add tests to cover this and other conditions with varying sizes for
the two arguments.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:30 +02:00
Gilles Peskine be4b5dd8c1 Add changelog entry for non-uniform MPI random generation
Fix #4245.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:30 +02:00
Gilles Peskine 16e3668d14 DHM: use mbedtls_mpi_random for blinding and key generation
Instead of generating blinding values and keys in a not-quite-uniform way
(https://github.com/ARMmbed/mbedtls/issues/4245) with copy-pasted code,
use mbedtls_mpi_random().

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:39:03 +02:00
Gilles Peskine 58df4c9098 dhm_check_range: microoptimization
No need to build a bignum for the value 2.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:37:43 +02:00
Gilles Peskine 87fdb1f872 DHM refactoring: use dhm_random_below in dhm_make_common
dhm_make_common includes a piece of code that is identical to
dhm_random_below except for returning a different error code in one
case. Call dhm_random_below instead of repeating the code.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:37:40 +02:00
Gilles Peskine b4e815f638 DHM blinding: don't accept P-1 as a blinding value
P-1 is as bad as 1 as a blinding value. Don't accept it.

The chance that P-1 would be randomly generated is infinitesimal, so
this is not a practical issue, but it makes the code cleaner. It was
inconsistent to accept P-1 as a blinding value but not as a private key.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:36:26 +02:00
Gilles Peskine 0853bb2bea DHM refactoring: unify mbedtls_dhm_make_{params,public}
Unify the common parts of mbedtls_dhm_make_params and mbedtls_dhm_make_public.

No intended behavior change, except that the exact error code may
change in some corner cases which are too exotic for the existing unit
tests.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine 33ec863570 Test mbedtls_dhm_make_params with different x_size
mbedtls_dhm_make_params() with x_size != size of P is not likely to be
useful, but it's supported, so test it.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine a2ce04e0eb Repeat a few DH tests
Repeat a few tests that use random data. This way the code is
exercised with a few different random values.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine dc0b6e44b0 Test range and format of dhm_make_params output
Improve the validation of the output from mbedtls_dhm_make_params:
* Test that the output in the byte buffer matches the value in the
  context structure.
* Test that the calculated values are in the desired range.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine 6466d3461e ECP: use mbedtls_mpi_random for blinding
Instead of generating blinding values in a not-quite-uniform way
(https://github.com/ARMmbed/mbedtls/issues/4245) with copy-pasted
code, use mbedtls_mpi_random().

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine aeab0fbd73 Preserve MBEDTLS_ERR_ECP_RANDOM_FAILED in case of a hostile RNG
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine cba4b35fcb Changelog entry for adding mbedtls_mpi_random()
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine 9312ba5304 mbedtls_mpi_random: check for invalid arguments
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine 4699fa47d2 Move mbedtls_mpi_random to the bignum module
Since mbedtls_mpi_random() is not specific to ECC code, move it from
the ECP module to the bignum module.

This increases the code size in builds without short Weierstrass
curves (including builds without ECC at all) that do not optimize out
unused functions.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine 7967ec5d25 mbedtls_ecp_gen_privkey_sw: generalize to mbedtls_mpi_random
Rename mbedtls_ecp_gen_privkey_sw to mbedtls_mpi_random since it has
no particular connection to elliptic curves beyond the fact that its
operation is defined by the deterministic ECDSA specification. This is
a generic function that generates a random MPI between 1 inclusive and
N exclusive.

Slightly generalize the function to accept a different lower bound,
which adds a negligible amount of complexity.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine 6373fab865 mbedtls_ecp_gen_privkey_sw: range and coverage tests
Add unit tests for private key generation on short Weierstrass curves.
These tests validate that the result is within the desired range.
Additionally, they validate that after performing many iterations, the
range is covered to an acceptable extent: for tiny ranges, all values
must be reached; for larger ranges, all value bits must reach both 0
and 1.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine eadf31d56a mbedtls_ecp_gen_privkey_mx: simplify the size calculation logic
mbedtls_ecp_gen_privkey_mx generates a random number with a certain
top bit set. Depending on the size, it would either generate a number
with that top bit being random, then forcibly set the top bit to
1 (when high_bit is not a multiple of 8); or generate a number with
that top bit being 0, then set the top bit to 1 (when high_bit is a
multiple of 8). Change it to always generate the top bit randomly
first.

This doesn't make any difference in practice: the probability
distribution is the same either way, and no supported or plausible
curve has a size of the form 8n+1 anyway. But it slightly simplifies
reasoning about the behavior of this function.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine 4f7767445b mbedtls_ecp_gen_privkey_mx: make bit manipulations unconditional
Don't calculate the bit-size of the initially generated random number.
This is not necessary to reach the desired distribution of private
keys, and creates a (tiny) side channel opportunity.

This changes the way the result is derived from the random number, but
does not affect the resulting distribution.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine 6acfc9cb4c mbedtls_ecp_gen_privkey_mx: remove the exception for all-zero
The library rejected an RNG input of all-bits-zero, which led to the
key 2^{254} (for Curve25519) having a 31/32 chance of being generated
compared to other keys. This had no practical impact because the
probability of non-compliance was 2^{-256}, but needlessly
complicated the code.

The exception was added in 98e28a74e3 to
avoid the case where b - 1 wraps because b is 0. Instead, change the
comparison code to avoid calculating b - 1.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine 188828525d Add unit tests for mbedtls_ecp_gen_privkey_mx
Test the exact output from known RNG input. This is overly
constraining, but ensures that the code has good properties.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine bef3019ed5 Make the fallback behavior of mbedtls_test_rnd_buffer_rand optional
If a fallback is not explicitly configured in the
mbedtls_test_rnd_buf_info structure, fail after the buffer is
exhausted.

There is no intended behavior change in this commit: all existing uses
of mbedtls_test_rnd_buffer_rand() have been updated to set
mbedtls_test_rnd_std_rand as the fallback.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine ebf3a4b80f Update references in some test function documentation
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00