Commit graph

6770 commits

Author SHA1 Message Date
Gilles Peskine 70a7dcda3f Fix multiplication producing a negative zero
Fix mbedtls_mpi_mul_mpi() when one of the operands is zero and the
other is negative. The sign of the result must be 1, since some
library functions do not treat {-1, 0, NULL} or {-1, n, {0}} as
representing the value 0.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine c86acc5434 mbedtls_mpi_gcd: small optimization
Shifting TA and TB before the loop is not necessary. If A != 0, it will be
done at the start of the loop iteration. If A == 0, then lz==0 and G is
correctly set to B after 0 loop iterations.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine b5e56ec5fd mbedtls_mpi_gcd: fix the case B==0
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine f643e8e8a9 Fix null pointer dereference in mbedtls_mpi_exp_mod
Fix a null pointer dereference in mbedtls_mpi_exp_mod(X, A, N, E, _RR) when
A is the value 0 represented with 0 limbs.

Make the code a little more robust against similar bugs.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Manuel Pégourié-Gonnard 6a55de9057
Merge pull request #4623 from gilles-peskine-arm/debug-print-mpi-null-2.x
Backport 2.x: Fix mbedtls_debug_print_mpi crash on 0
2021-06-22 12:08:57 +02:00
Manuel Pégourié-Gonnard 9a11ac9cc1
Merge pull request #4621 from gilles-peskine-arm/default-hashes-curves-2.x
Backport 2.x: Curve and hash selection for X.509 and TLS
2021-06-22 12:08:43 +02:00
Gilles Peskine 51859aaff2 Fix fd range for select on Windows
Fix mbedtls_net_poll() and mbedtls_net_recv_timeout() often failing with
MBEDTLS_ERR_NET_POLL_FAILED on Windows: they were testing that the file
descriptor is in range for fd_set, but on Windows socket descriptors are not
limited to a small range. Fixes #4465.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-20 23:17:39 +02:00
Gilles Peskine 0f6351f8a9 Refactor file descriptor checks into a common function
This will make it easier to change the behavior uniformly.

No behavior change.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-20 23:17:39 +02:00
Gilles Peskine f216f0d5d4 Fix missing state check for tls12_prf output
Fix PSA_ALG_TLS12_PRF and PSA_ALG_TLS12_PSK_TO_MS being too permissive
about missing inputs.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-14 18:08:26 +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 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 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 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 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 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 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 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 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 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 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 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 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 3838f28c33 mbedtls_ecp_gen_privkey_mx: rename n_bits to high_bit
For Montgomery keys, n_bits is actually the position of the highest
bit and not the number of bits, which would be 1 more (fence vs
posts). Rename the variable accordingly to lessen the confusion.

No semantic change.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine de33213f23 mbedtls_ecp_gen_privkey: create subfunctions for each curve type
Put the Montgomery and short Weierstrass implementations of
mbedtls_ecp_gen_privkey into their own function which can be tested
independently, but will not be part of the public ABI/API.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-02 21:31:24 +02:00
Gilles Peskine 570a8cd056 Fix null pointer arithmetic in error case
When mbedtls_nist_kw_wrap was called with output=NULL and out_size=0, it
performed arithmetic on the null pointer before detecting that the output
buffer is too small and returning an error code. This was unlikely to have
consequences on real-world hardware today, but it is undefined behavior and
UBSan with Clang 10 flagged it. So fix it (fix #4025).

Fix a similar-looking pattern in unwrap, though I haven't verified that it's
reachable there.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-01 11:57:18 +02:00
Ronald Cron 9df7209bf6
Merge pull request #4546 from Patater/psa-without-genprime-fix-2.x
[Backport 2.x] psa: Support RSA signature without MBEDTLS_GENPRIME
2021-05-27 14:19:14 +02:00
Gilles Peskine 66c616a393 CAMELLIA: add missing context init/free
This fixes the self-test with alternative implementations.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-05-25 18:45:04 +02:00
Gilles Peskine ccbbb2c501 ARIA: add missing context init/free
This fixes the self-test with alternative implementations.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-05-25 18:45:04 +02:00
Jaeden Amero c17f29309f psa: Support RSA signature without MBEDTLS_GENPRIME
On space-constrained platforms, it is a useful configuration to be able
to import/export and perform RSA key pair operations, but to exclude RSA
key generation, potentially saving flash space. It is not possible to
express this with the PSA_WANT_ configuration system at the present
time. However, in previous versions of Mbed TLS (v2.24.0 and earlier) it
was possible to configure a software PSA implementation which was
capable of making RSA signatures but not capable of generating RSA keys.
To do this, one unset MBEDTLS_GENPRIME.

Since the addition of MBEDTLS_PSA_BUILTIN_KEY_TYPE_RSA_KEY_PAIR, this
expressivity was lost. Expressing that you wanted to work with RSA key
pairs forced you to include the ability to generate key pairs as well.

Change psa_crypto_rsa.c to only call mbedtls_rsa_gen_key() if
MBEDTLS_GENPRIME is also set. This restores the configuration behavior
present in Mbed TLS v2.24.0 and earlier versions.

It left as a future exercise to add the ability to PSA to be able to
express a desire for a software or accelerator configuration that
includes RSA key pair operations, like signature, but excludes key pair
generation.

Without this change, linker errors will occur when attempts to call,
which doesn't exist when MBEDTLS_GENPRIME is unset.
    psa_crypto_rsa.c.obj: in function `rsa_generate_key':
    psa_crypto_rsa.c:320: undefined reference to `mbedtls_rsa_gen_key'

Fixes #4512

Signed-off-by: Jaeden Amero <jaeden.amero@arm.com>
2021-05-21 10:21:27 +01:00
Janos Follath d76f7ba2e1
Merge pull request #4529 from hanno-arm/ssl_session_cache_fix_backport_2x
Backport 2.x: Add session ID as an explicit parameter to SSL session cache API
2021-05-21 08:49:11 +01:00
Gilles Peskine d135b57e8c
Merge pull request #4412 from gilles-peskine-arm/undefined-reference-2.27
Backport 2.x: Fix missing compilation guard around psa_crypto_driver_wrappers.c
2021-05-20 17:20:36 +02:00
Gilles Peskine a33cb76820
Merge pull request #4493 from netfoundry/gcc11.fixes_2.x
Backport 2.x: build with gcc11
2021-05-20 15:54:20 +02:00
Gilles Peskine 05c11e3dd5
Merge pull request #4503 from gilles-peskine-arm/ciphersuite-sha384-guard-2.x
Backport 2.x: fix SHA384 guards in TLS
2021-05-19 21:13:08 +02:00
Gilles Peskine ad0e01248d Fix missing compilation guard around psa_crypto_driver_wrappers.c
Fix #4411.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-05-19 21:03:46 +02:00