The loop exits early iff there is a nonzero limb, so i==0 means that
all limbs are 0, whether the number of limbs is 0 or not.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Fix a bug introduced in "Fix multiplication producing a negative zero" that
caused the sign to be forced to +1 when A > 0, B < 0 and B's low-order limb
is 0.
Add a non-regression test. More generally, systematically test combinations
of leading zeros, trailing zeros and signs.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
In mbedtls_mpi_read_string, if the string is empty, return an empty bignum
rather than a bignum with one limb with the value 0.
Both representations are correct, so this is not, in principle, a
user-visible change. The change does leak however through
mbedtls_mpi_write_string in base 16 (but not in other bases), as it writes a
bignum with 0 limbs as "" but a bignum with the value 0 and at least one
limb as "00".
This change makes it possible to construct an empty bignum through
mbedtls_mpi_read_string, which is especially useful to construct test
cases (a common use of mbedtls_mpi_read_string, as most formats use in
production encode numbers in binary, to be read with mbedtls_mpi_read_binary
or mbedtls_mpi_read_binary_le).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
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>
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>
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>
* development_2.x:
Reword changelog - Test Resource Leak
Fix fd range for select on Windows
Refactor file descriptor checks into a common function
Update changelog formatting - Missing Free Context
Update changelog formatting Missing Free Context
Update changelog formatting - Missing Free Context
Changelog entry for Free Context in test_suite_aes fix
Free context in at the end of aes_crypt_xts_size()
Fix copypasta in test data
Use UNUSED wherever applicable in derive_input tests
Fix missing state check for tls12_prf output
Key derivation: add test cases where the secret is missing
Add bad-workflow key derivation tests
More explicit names for some bad-workflow key derivation tests
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>
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>
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>
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>
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>
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>
