Commit Graph

15813 Commits

Author SHA1 Message Date
Gilles Peskine
33c92f01a0 Determine MBEDTLS_ECP_MAX_BITS automatically
MBEDTLS_ECP_MAX_BITS is now determined automatically from the configured
curves and no longer needs to be configured explicitly to save RAM. Setting
it explicit in config.h is still supported for backward compatibility.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-11 21:44:58 +02:00
Gilles Peskine
e57bad4b42 Check MBEDTLS_ECP_MAX_xxx constants in unit tests
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-11 21:43:26 +02:00
Gilles Peskine
6dba3200d4 Fail the build if MBEDTLS_ECP_MAX_BITS is not large enough
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-11 21:43:14 +02:00
Manuel Pégourié-Gonnard
7576f55f19 Add ChangeLog entry about RSA side channel.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:22 +02:00
Manuel Pégourié-Gonnard
0b3bde57f1 Silence MSVC type conversion warnings
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:22 +02:00
Manuel Pégourié-Gonnard
f10d289441 Simplify sign selection
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:22 +02:00
Manuel Pégourié-Gonnard
5325b976b9 Avoid UB caused by conversion to int
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:22 +02:00
Manuel Pégourié-Gonnard
464fe6a4d7 Use bit operations for mpi_safe_cond_swap()
Unrelated to RSA (only used in ECP), but while improving one
mbedtls_safe_cond_xxx function, let's improve the other as well.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:21 +02:00
Manuel Pégourié-Gonnard
c3be399591 Use bit operations for mpi_safe_cond_assign()
- copied limbs
- sign
- cleared limbs

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:21 +02:00
Manuel Pégourié-Gonnard
eaafa494e1 Avoid using == for sensitive comparisons
mbedtls_mpi_cf_bool_eq() is a verbatim copy of mbedtls_ssl_cf_bool_eq()

Deduplication will be part of a future task.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:21 +02:00
Manuel Pégourié-Gonnard
e10e8db6d4 Use constant-time look-up for modular exponentiation
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:21 +02:00
Manuel Pégourié-Gonnard
c4c0d819ce Merge branch 'development_2.x' into development_2.x-restricted
* development_2.x:
  Disable OS X builds on Travis
  config: Allow Mbed to implement TIMING_C
  Fix misuse of MD API in SSL constant-flow HMAC
2021-06-11 10:09:53 +02:00
Ronald Cron
766edb8476
Merge pull request #4635 from Patater/mbed-can-do-timing-2.x
Backport 2.x: config: Allow Mbed to implement TIMING_C

Trivial backport, only one reviewer is ok.
2021-06-11 09:14:00 +02:00
Dave Rodgman
78719eaa7b
Merge pull request #4646 from daverodgman/travis-disable-osx-development_2.x
Backport 2.x: Disable OS X builds on Travis
2021-06-10 17:48:27 +01:00
Dave Rodgman
fcf958afc7 Disable OS X builds on Travis
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
2021-06-10 15:48:20 +01:00
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
2bfbb17f30 Remove duplicate wipe call in psa_destroy_key
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 23:27:54 +02:00
Gilles Peskine
11794b30f9 Hopefully clarify the example
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 23:21:50 +02:00
Gilles Peskine
4de5a6096b Add missing parentheses
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 21:42:15 +02:00
Gilles Peskine
e247b10cd6 Indicate that the truncation from size_t to int is deliberate
MPI sizes do fit in int. Let MSVC know this conversion is deliberate.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 21:42:08 +02:00
Gilles Peskine
2ee0bb333c Simplify mbedtls_debug_print_mpi and fix the case of empty bignums
Rewrite mbedtls_debug_print_mpi to be simpler and smaller. Leverage
mbedtls_mpi_bitlen() instead of manually looking for the leading
zeros.

Fix #4608: the old code made an invalid memory dereference when
X->n==0 (freshly initialized bignum with the value 0).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 21:41:59 +02:00
Gilles Peskine
fba257d391 Add mbedtls_debug_print_mpi test case for 0
There was already a test case for 0 but with a non-empty representation
(X->n == 1). Add a test case with X->n == 0 (freshly initialized mpi).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 21:41:53 +02:00
Gilles Peskine
b37abdcb07 Clarify test case descriptions
Reorder test cases and make their descriptions more explicit. No
change in test data.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 21:41:38 +02:00
Gilles Peskine
da728b31b0 Remove meaningless clause
We stated that curves were listed "in order of preference", but we never
explained what the preference was, so this was not meaningful.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 21:28:14 +02:00
Gilles Peskine
b3ca90bc44 Reduce the default ECP window size
MBEDTLS_ECP_WINDOW_SIZE is a compromise between memory usage (growing based
on the value) and performance (faster with larger values). There are
disminishing returns as the value grows larger. Based on Manuel's benchmarks
recorded in https://github.com/ARMmbed/mbedtls/issues/4127, 4 is a good
compromise point, with larger values bringing little advantage. So reduce
the default from 6 to 4.

Document the default value as in optimized for performance mostly, but don't
document the specific value, so we may change it later or make it
platform-dependent.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 21:27:51 +02:00
Gilles Peskine
646b78b927 Document more precisely what goes into the default preset
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 21:26:41 +02:00
Gilles Peskine
0ecd719edf Document more precisely what goes into the default profile
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-07 21:24:26 +02: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