IAR was warning that conditional execution could bypass initialisation of
variables, although those same variables were not used uninitialised. Fix
this along with some other IAR warnings.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
The bit length of m is leaked through through timing in ecp_mul_mxz().
Initially found by Manuel Pégourié-Gonnard on ecp_mul_edxyz(), which has
been inspired from ecp_mul_mxz(), during initial review of the EdDSA PR.
See: https://github.com/Mbed-TLS/mbedtls/pull/3245#discussion_r490827996
Fix that by using grp->nbits + 1 instead, which anyway is very close to
the length of m, which means there is no significant performance impact.
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
We used to include platform.h only when MBEDTLS_PLATFORM_C was enabled, and
to define ad hoc replacements for mbedtls_xxx functions on a case-by-case
basis when MBEDTLS_PLATFORM_C was disabled. The only reason for this
complication was to allow building individual source modules without copying
platform.h. This is not something we support or recommend anymore, so get
rid of the complication: include platform.h unconditionally.
There should be no change in behavior since just including the header should
not change the behavior of a program.
This commit replaces most occurrences of conditional inclusion of
platform.h, using the following code:
```
perl -i -0777 -pe 's!#if.*\n#include "mbedtls/platform.h"\n(#else.*\n(#define (mbedtls|MBEDTLS)_.*\n|#include <(stdarg|stddef|stdio|stdlib|string|time)\.h>\n)*)?#endif.*!#include "mbedtls/platform.h"!mg' $(git grep -l '#include "mbedtls/platform.h"')
```
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
minor changes, such as improving the documentation for the byte reading
macros, and using MBEDTLS_PUT_UINT16_xy in place of byte reading
macro combinations
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
byte shifting opertations throughout library/ were only replaced with
the byte reading macros when an 0xff mask was being used.
The byte reading macros are now more widley used, however they have not
been used in all cases of a byte shift operation, as it detracted from
the immediate readability or otherwise did not seem appropriate.
Signed-off-by: Joe Subbiani <joe.subbiani@arm.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>
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>
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>
This header file will contain declarations of functions that are not
part of the public ABI/API, and must not be called from other modules,
but can be called from unit tests.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Fix function mbedtls_ecp_mul_shortcuts() to skip multiplication when m
is 0 and simply assignt 0 to R. Additionally fix ecjpake_zkp_read() to
return MBEDTLS_ERR_ECP_INVALID_KEY when the above condintion is met.
Fix#1792
Signed-off-by: TRodziewicz <rodziewicz@gmail.com>
mbedtls_ecp_curve_list() now lists Curve25519 and Curve448 under the names
"x25519" and "x448". These curves support ECDH but not ECDSA.
This was meant ever since the introduction of mbedtls_ecdsa_can_do()
in 0082f9df6f, but
2c69d10bac had removed the claim
that Montgomery curves support ECDH except through Everest.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
As a result, the copyright of contributors other than Arm is now
acknowledged, and the years of publishing are no longer tracked in the
source files.
Also remove the now-redundant lines declaring that the files are part of
MbedTLS.
This commit was generated using the following script:
# ========================
#!/bin/sh
# Find files
find '(' -path './.git' -o -path './3rdparty' ')' -prune -o -type f -print | xargs sed -bi '
# Replace copyright attribution line
s/Copyright.*Arm.*/Copyright The Mbed TLS Contributors/I
# Remove redundant declaration and the preceding line
$!N
/This file is part of Mbed TLS/Id
P
D
'
# ========================
Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
Run some self-test both for a short Weierstrass curve and for a
Montgomery curve, if the build-time configuration includes a curve of
both types. Run both because there are significant differences in the
implementation.
The test data is suitable for Curve25519.
Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
The constants used in the test worked with every supported curve
except secp192k1. For secp192k1, the "N-1" exponent was too large.
Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
Replace the now-redundant internal curve type macros ECP_xxx by the
macros MBEDTLS_ECP__xxx_ENABLED which are declared in ecp.h.
Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
