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
not equal to 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>
Backport 2.x: Fix and test the MBEDTLS_PSA_CRYPTO_SPM build
Straightforward backport from development to developement_2.x plus one trivial commit, only one approval is enough.
Fix initialization of mbedtls_psa_cipher_operation_t by not initializing the mbedtls_cipher_context_t typed field completely.
Signed-off-by: gabor-mezei-arm <gabor.mezei@arm.com>
This makes it easier to ensure that crypto_spe.h is included everywhere it
needs to be, and that it's included early enough to do its job (it must be
included before any mention of psa_xxx() functions with external linkage,
because it defines macros to rename these functions).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Introduce psa_mac_compute_internal with an
additional `is_sign` parameter compared to
the psa_mac_compute API. The intent is to
call psa_mac_compute_internal() from
psa_mac_verify() as well to compute the
message MAC.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Re-organize psa_mac_setup() to prepare the move
to a dedicated function of the additional checks
on the algorithm and the key attributes done by
this function. We want to move those checks in
a dedicated function to be able to do them
without duplicating them in psa_mac_compute().
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Implement one-shot MAC APIs, psa_mac_compute and psa_mac_verify, introduced in PSA Crypto API 1.0.
Signed-off-by: gabor-mezei-arm <gabor.mezei@arm.com>
Remove a case that cannot be triggered as PSA_ALG_SIGN_GET_HASH always
returns 0 for raw algorithms.
Signed-off-by: Janos Follath <janos.follath@arm.com>
PSA Crypto always passed MBEDTLS_MD_NONE to Mbed TLS, which worked well
as Mbed TLS does not use this parameter for anything beyond determining
the input lengths.
Some alternative implementations however check the consistency of the
algorithm used for pre-hash and for other uses in verification (verify
operation and mask generation) and fail if they don't match. This makes
all such verifications fail.
Furthermore, the PSA Crypto API mandates that the pre-hash and internal
uses are aligned as well.
Fixes#3990.
Signed-off-by: Janos Follath <janos.follath@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>
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>
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>
