Warning reported with IAR compiler:
"mbedtls\library\pkparse.c",1167 Warning[Pe550]: variable "ret" was set but never used
Signed-off-by: Kenneth Soerensen <knnthsrnsn@gmail.com>
A DES or AES block operation can fail in alternative implementations of
mbedtls_internal_aes_encrypt() (under MBEDTLS_AES_ENCRYPT_ALT),
mbedtls_internal_aes_decrypt() (under MBEDTLS_AES_DECRYPT_ALT),
mbedtls_des_crypt_ecb() (under MBEDTLS_DES_CRYPT_ECB_ALT),
mbedtls_des3_crypt_ecb() (under MBEDTLS_DES3_CRYPT_ECB_ALT).
A failure can happen if the accelerator peripheral is in a bad state.
Several block modes were not catching the error.
This commit does the following code changes:
* Fix DES and AES API calls which ignored the return values:
* In library code: on failure, goto exit and return ret.
* In pkey programs: goto exit.
* In the benchmark program: exit (not ideal since there's no error
message, but it's what the code currently does for failures).
* In test code: TEST_ASSERT.
* Changelog entry.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Mateusz Starzyk <mateusz.starzyk@mobica.com>
The function `pk_hashlen_helper` exists to ensure a valid hash_len is
used in pk_verify and pk_sign functions. This function has been
used to adjust to the corrsponding hash_len if the user passes in 0
for the hash_len argument based on the md algorithm given. If the user
does not pass in 0 as the hash_len, then it is not adjusted. This is
problematic if the user gives a hash_len and hash buffer that is less than the
associated length of the md algorithm. This error would go unchecked
and eventually lead to buffer overread when given to specific pk_sign/verify
functions, since they both ignore the hash_len argument if md_alg is not
MBEDTLS_MD_NONE.
This commit, adds a conditional to `pk_hashlen_helper` so that an
error is thrown if the user specifies a hash_length (not 0) and it is
less than the expected for the associated message digest algorithm.
This aligns better with the api documentation where it states "If
hash_len is 0, then the length associated with md_alg is used instead,
or an error returned if it is invalid"
Signed-off-by: Nick Child <nick.child@ibm.com>
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
These macros were moved into a header and now check-names.sh is failing.
Add an MBEDTLS_ prefix to the macro names to make it pass.
Signed-off-by: Janos Follath <janos.follath@arm.com>
We were already rejecting them at the end, due to the fact that with the
usual (x, z) formulas they lead to the result (0, 0) so when we want to
normalize at the end, trying to compute the modular inverse of z will
give an error.
If we wanted to support those points, we'd a special case in
ecp_normalize_mxz(). But it's actually permitted by all sources (RFC
7748 say we MAY reject 0 as a result) and recommended by some to reject
those points (either to ensure contributory behaviour, or to protect
against timing attack when the underlying field arithmetic is not
constant-time).
Since our field arithmetic is indeed not constant-time, let's reject
those points before they get mixed with sensitive data (in
ecp_mul_mxz()), in order to avoid exploitable leaks caused by the
special cases they would trigger. (See the "May the Fourth" paper
https://eprint.iacr.org/2017/806.pdf)
Signed-off-by: Janos Follath <janos.follath@arm.com>
Clang was complaining and check-names.sh too
This only duplicates macros, so no impact on code size. In 3.0 we can
probably avoid the duplication by using an internal header under
library/ but this won't work for 2.16.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
We were already rejecting them at the end, due to the fact that with the
usual (x, z) formulas they lead to the result (0, 0) so when we want to
normalize at the end, trying to compute the modular inverse of z will
give an error.
If we wanted to support those points, we'd a special case in
ecp_normalize_mxz(). But it's actually permitted by all sources
(RFC 7748 say we MAY reject 0 as a result) and recommended by some to
reject those points (either to ensure contributory behaviour, or to
protect against timing attack when the underlying field arithmetic is
not constant-time).
Since our field arithmetic is indeed not constant-time, let's reject
those points before they get mixed with sensitive data (in
ecp_mul_mxz()), in order to avoid exploitable leaks caused by the
special cases they would trigger. (See the "May the Fourth" paper
https://eprint.iacr.org/2017/806.pdf)
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
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>
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>
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>
* mbedtls-2.16: (21 commits)
Reword changelog - Test Resource Leak
Update changelog formatting - Missing Free Context
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
Changelog entry for Free Context in test_suite_aes fix
Free context at the end of aes_crypt_xts_size()
Add changelog entry for non-uniform MPI random generation
ecp: Fix bias in the generation of blinding values
DHM: add test case with x_size < 0
DHM tests: add some explanations
DHM: add notes about leading zeros
dhm: Fix bias in private key generation and blinding
dhm_check_range: microoptimization
DHM refactoring: use dhm_random_below in dhm_make_common
DHM blinding: don't accept P-1 as a blinding value
DHM refactoring: unify mbedtls_dhm_make_{params,public}
Test mbedtls_dhm_make_params with different x_size
Repeat a few DH tests
...
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>
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>
We know that we'll never call this function with T_size == 0, but the
compiler doesn't.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@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>
