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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
TLS code specific to SHA-384 was gated on MBEDTLS_SHA512_C. But SHA-384 also
requires that MBEDTLS_SHA512_NO_SHA384 is not defined. This lead to dead
code in TLS when MBEDTLS_SHA512_C and MBEDTLS_SHA512_NO_SHA384 were both
defined (i.e. when SHA-512 was enabled but not SHA-384).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
They depended on MBEDTLS_SHA512_C only. A check for !MBEDTLS_SHA512_NO_SHA384
was missing.
Fix#4499.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Session-ID based session resumption requires that the resumed session
is consistent with the client's ClientHello in terms of choice of
ciphersuite and choice of compression.
This check was previously assumed to be performed in the session cache
implementation, which seems wrong: The session cache should be an id-based
lookup only, and protocol specific checks should be left to Mbed TLS.
This commit
- adds an explicit ciphersuite and compression consistency check after
the SSL session cache has been queried
- removes the ciphersuite and compression consistency check from
Mbed TLS' session cache reference implementation.
Don't use ssl_check_xxx() for functions with void return
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
Prepare to isolate the Montgomery and short Weierstrass
implementations of mbedtls_ecp_gen_privkey into their own function.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
A previous fix in d596ca8a1e worked with
beta versions of GCC 11, but not with the final 11.1 release.
This time, just disable the warning locally.
Fix#4130
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Zephyr's native posix port define _POSIX_C_SOURCE with a higher value
during the build, so when mbedTLS defines it with a different value
breaks the build.
As Zephyr is already defining a higher value is guaranteed that mbedTLS
required features will be available. So, just define it in case it was
not defined before.
[taken from Zephyr mbedtls module:
76dcd6eeca]
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
Signed-off-by: David Brown <david.brown@linaro.org>
