Commit graph

367 commits

Author SHA1 Message Date
Steven Cooreman b74b5602b7 Add missing parenthesis
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
2021-04-27 17:08:26 +01:00
Manuel Pégourié-Gonnard 0bbb38c67e
Merge pull request #4199 from TRodziewicz/mul_shortcut_fix
Fix ECDSA failing when the hash is all-bits-zero
2021-04-19 09:54:12 +02:00
Gilles Peskine 80ba850e27 Create a header file for ECP internal functions
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>
2021-04-04 01:10:41 +02:00
TRodziewicz 782a7eab14 ecjpake_zkp_read() now returns ...BAD_INPUT_DATA when r len == 0 and test follows that
Signed-off-by: TRodziewicz <tomasz.rodziewicz@mobica.com>
2021-03-17 11:36:31 +01:00
TRodziewicz 9edff740e1 Fix EC J-PAKE failing when the payload is all-bits-zero
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>
2021-03-04 18:19:48 +01:00
Steven Cooreman e538896ad8 Remove unreferenced static functions when ECP_NO_FALLBACK is used
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
2021-03-02 10:14:24 +01:00
Steven Cooreman 7eb2aa0dc1 Reworked NO_FALLBACK logic according to review feedback
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
2021-01-22 09:43:59 +01:00
Steven Cooreman 97b4984657 Add a flag for disabling fallback in ecp.c
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
2021-01-08 16:43:43 +01:00
Gilles Peskine 360e2c41d8 List Montgomery curves in mbedtls_ecp_curve_list()
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>
2020-09-30 17:02:25 +02:00
Bence Szépkúti 1e14827beb Update copyright notices to use Linux Foundation guidance
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>
2020-08-19 10:35:41 +02:00
Gilles Peskine 6d9c8d7b2d Minor documentation improvements
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-22 03:17:25 +02:00
Gilles Peskine 5997005611 Fix unused variables in Montgomery-only configuration
Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:24 +02:00
Gilles Peskine a088c81fcb Adjust ECP self-test to support Curve448
Adjust the Montgomery self-test to use Curve448 in builds without
Curve25519.

Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:24 +02:00
Gilles Peskine 24666795e4 ECP self test: add self-test step for Montgomery curves
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>
2020-07-22 03:17:24 +02:00
Gilles Peskine c95696fec4 Factor common code in mbedtls_ecp_self_test
No intended behavior change.

Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:24 +02:00
Gilles Peskine d9767a5799 Tweak ECP self-test to work with secp192k1
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>
2020-07-22 03:17:24 +02:00
Gilles Peskine e8c04fed51 Replace ECP_xxx by MBEDTLS_ECP__xxx_ENABLED
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>
2020-07-22 03:17:22 +02:00
Gilles Peskine 9b99a8942f mbedtls_ecp_muladd is only for short Weierstrass curves
Document that mbedtls_ecp_muladd and mbedtls_ecp_muladd_restartable
are only implemented on short Weierstrass curves.

Exclude these functions at build time if no short Weierstrass curve
is included in the build. Before, these functions failed to compile in
such a configuration.

Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 02:48:31 +02:00
Steven Cooreman aec44e9fe8 Merge remote-tracking branch 'mbedtls/development' into montgomery-keys-clarification 2020-07-13 11:48:21 +02:00
Steven Cooreman 0024df6b37 Remove superfluous argument to ecp_write_key
Removed after feedback from PR review.

Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
2020-07-13 10:59:40 +02:00
Steven Cooreman c9b7f78647 Rework mbedtls_ecp_write_key to remove unnecessary output parameter
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
2020-07-06 10:52:32 +02:00
Steven Cooreman de8593f2fe Implement and test mbedtls_ecp_write_key
mbedtls_ecp_write_key is a mirror function to mbedtls_ecp_read_key, which
writes a private key back into a byte buffer in the correct format.
This is a helpful convenience function, since the byte order is defined
differently between Montgomery and Weierstrass curves. Since this difference
is accounted for in mbedtls_ecp_read_key, it made sense to add
mbedtls_ecp_write_key for the purpose of abstracting this away such that
psa_export_key doesn't need to take byte order into account.

Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
2020-07-06 10:42:38 +02:00
Manuel Pégourié-Gonnard 527b87890d
Merge pull request #3454 from gilles-peskine-arm/include-common-h-development
Include common.h from all library source files
2020-07-03 09:44:18 +02:00
Gilles Peskine db09ef6d22 Include common.h instead of config.h in library source files
In library source files, include "common.h", which takes care of
including "mbedtls/config.h" (or the alternative MBEDTLS_CONFIG_FILE)
and other things that are used throughout the library.

FROM=$'#if !defined(MBEDTLS_CONFIG_FILE)\n#include "mbedtls/config.h"\n#else\n#include MBEDTLS_CONFIG_FILE\n#endif' perl -i -0777 -pe 's~\Q$ENV{FROM}~#include "common.h"~' library/*.c 3rdparty/*/library/*.c scripts/data_files/error.fmt scripts/data_files/version_features.fmt

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-02 11:26:57 +02:00
Manuel Pégourié-Gonnard 1215c54754 Add length check in ecp_drbg_seed()
While this is a static function, so right now we know we don't need the check,
things may change in the future, so better be on the safe side.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-19 11:59:49 +02:00
Manuel Pégourié-Gonnard 4539a45cbf Use fixed-length encoding for internal RNG seed
CTR-DRBG and HMAC-DRBG may used the seed differently depending on its length.
To avoid leaks, pass them a constant-length seed.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-18 12:27:56 +02:00
Manuel Pégourié-Gonnard 25705e6757
Fix typo in a comment
Co-authored-by: Janos Follath <janos.follath@arm.com>
2020-06-10 09:18:25 +02:00
Manuel Pégourié-Gonnard 9b8d34edd4 Avoid superflous randomization with restartable
Checking the budget only after the randomization is done means sometimes we
were randomizing first, then noticing we ran out of budget, return, come back
and randomize again before we finally normalize.

While this is fine from a correctness and security perspective, it's a minor
inefficiency, and can also be disconcerting while debugging, so we might as
well avoid it.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-08 09:53:20 +02:00
Manuel Pégourié-Gonnard d53ef2ffd1 Use HMAC_DRBG by default for ECP internal DRBG
It results in smaller code than using CTR_DRBG (64 bytes smaller on ARMv6-M
with arm-none-eabi-gcc 7.3.1), so let's use this by default when both are
available.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-08 09:10:11 +02:00
Manuel Pégourié-Gonnard 22b1de3097 Skip redundant checks for NULL f_rng
Unless MBEDTLS_ECP_NO_INTERNAL_RNG is defined, it's no longer possible for
f_rng to be NULL at the places that randomize coordinates.

Eliminate the NULL check in this case:
- it makes it clearer to reviewers that randomization always happens (unless
  the user opted out at compile time)
- a NULL check in a place where it's easy to prove the value is never NULL
  might upset or confuse static analyzers (including humans)
- removing the check saves a bit of code size

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-08 09:09:20 +02:00
Manuel Pégourié-Gonnard 53fb66db12 Add support for RESTARTABLE with internal RNG
Currently we draw pseudo-random numbers at the beginning and end of the main
loop. With ECP_RESTARTABLE, it's possible that between those two occasions we
returned from the multiplication function, hence lost our internal DRBG
context that lives in this function's stack frame. This would result in the
same pseudo-random numbers being used for blinding in multiple places. While
it's not immediately clear that this would give rise to an attack, it's also
absolutely not clear that it doesn't. So let's avoid that by using a DRBG
context that lives inside the restart context and persists across
return/resume cycles. That way the RESTARTABLE case uses exactly the
same pseudo-random numbers as the non-restartable case.

Testing and compile-time options:

- The case ECP_RESTARTABLE && !ECP_NO_INTERNAL_RNG is already tested by
  component_test_no_use_psa_crypto_full_cmake_asan.
- The case ECP_RESTARTABLE && ECP_NO_INTERNAL_RNG didn't have a pre-existing
  test so a component is added.

Testing and runtime options: when ECP_RESTARTABLE is enabled, the test suites
already contain cases where restart happens and cases where it doesn't
(because the operation is short enough or because restart is disabled (NULL
restart context)).

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-08 09:09:20 +02:00
Manuel Pégourié-Gonnard f2a9fcff62 Move internal drbg init to specific mul functions
While it seems cleaner and more convenient to set it in the top-level
mbedtls_ecp_mul() function, the existence of the restartable option changes
things - when it's enabled the drbg context needs to be saved in the restart
context (more precisely in the restart_mul sub-context), which can only be
done when it's allocated, which is in the curve-specific mul function.

This commit only internal drbg management from mbedtls_ecp_mul() to
ecp_mul_mxz() and ecp_mul_comb(), without modifying behaviour (even internal),
and a future commit will modify the ecp_mul_comb() version to handle restart
properly.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-08 09:09:20 +02:00
Manuel Pégourié-Gonnard c52a43c2bd Implement use of internal DRBG for ecp_mul()
The case of MBEDTLS_ECP_RESTARTABLE isn't handled correctly yet: in that case
the DRBG instance should persist when resuming the operation. This will be
addressed in the next commit.

When both CTR_DRBG and HMAC_DRBG are available, CTR_DRBG is preferred since
both are suitable but CTR_DRBG tends to be faster and I needed a tie-breaker.

There are currently three possible cases to test:

- NO_INTERNAL_RNG is set -> tested in test_ecp_no_internal_rng
- it's unset and CTR_DRBG is available -> tested in the default config
- it's unset and CTR_DRBG is disabled -> tested in
  test_ecp_internal_rng_no_ctr_drbg

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-08 09:09:20 +02:00
Jonas b246214ade Fix Changelag PR number and uniformize code when prng fails
Signed-off-by: Jonas <jonas.lejeune4420@gmail.com>
2020-05-28 20:02:40 +09:00
Jonas 6692a064e3 Fix potential memory leak in EC multiplication
Signed-off-by: Jonas <jonas.lejeune4420@gmail.com>
2020-05-08 16:58:47 +09:00
Manuel Pégourié-Gonnard b1c8e41ae3
Merge pull request #3209 from aurel32/fix-ecp_double_add_mxz
Fix wrong modulo call in ecp_double_add_mxz
2020-04-24 09:28:17 +02:00
Aurelien Jarno 66deb38d64 Fix wrong modulo call in ecp_double_add_mxz
ecp_double_add_mxz wrongly does an MPI addition followed by a call to
MOD_MUL instead of MOD_ADD. This is more visible since the
mbedtls_mpi_xxx_mod functions have been added in commit 3b3b34f608
("Replace some macros by functions").

Fix that by using mbedtls_mpi_add_mod instead. The testsuite still
passes after that change.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
2020-04-23 23:16:11 +02:00
Kenneth Soerensen 518d435e7b Fix GCC format-signedness warnings
Signed-off-by: Kenneth Soerensen <knnthsrnsn@gmail.com>
2020-04-22 16:01:48 +02:00
Manuel Pégourié-Gonnard a4aa89b16e Fix leakage of projective coordinates in ECC
See the comments in the code for how an attack would go, and the ChangeLog
entry for an impact assessment. (For ECDSA, leaking a few bits of the scalar
over several signatures translates to full private key recovery using a
lattice attack.)

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-03-27 09:43:34 +01:00
Gilles Peskine 1174db5bac Revert "ecp: Remove dependency on TLS and X.509"
This reverts commit de0a41b716.
2020-03-19 14:23:45 +01:00
Janos Follath d27a88438f Merge branch 'development' into development-restricted 2020-01-15 15:55:11 +00:00
Janos Follath 24eed8d2d2 Initialise return values to an error
Initialising the return values to and error is best practice and makes
the library more robust.
2019-12-03 16:07:18 +00:00
Janos Follath 0e5532d6cf Change mbedtls_mpi_cmp_mpi_ct to check less than
The signature of mbedtls_mpi_cmp_mpi_ct() meant to support using it in
place of mbedtls_mpi_cmp_mpi(). This meant full comparison functionality
and a signed result.

To make the function more universal and friendly to constant time
coding, we change the result type to unsigned. Theoretically, we could
encode the comparison result in an unsigned value, but it would be less
intuitive.

Therefore we won't be able to represent the result as unsigned anymore
and the functionality will be constrained to checking if the first
operand is less than the second. This is sufficient to support the
current use case and to check any relationship between MPIs.

The only drawback is that we need to call the function twice when
checking for equality, but this can be optimised later if an when it is
needed.
2019-11-04 10:39:20 +00:00
Janos Follath d80080c884 Remove excess vertical space 2019-11-04 10:39:20 +00:00
Janos Follath a779b4601e Fix side channel vulnerability in ECDSA 2019-11-04 10:39:20 +00:00
Christoph M. Wintersteiger 2c69d10bac 3rdparty: Adjust use of Everest in ecp_supported_curves 2019-08-19 13:37:46 +01:00
Christoph M. Wintersteiger cb31073e1c ECP: add Curve448 to ecp_supported_curves 2019-08-19 13:37:46 +01:00
Christoph M. Wintersteiger c3cbddecb5 ECDH: Fix whitespace, permissions 2019-08-19 13:36:44 +01:00
Christoph M. Wintersteiger 86e36c4c2b ECDH: Replace hex literal with decimal in ecp.c 2019-08-19 13:36:44 +01:00
Christoph M. Wintersteiger c9f737b4ba ECDH: Enable Everest Curve25519 in ECDH/ECDSA/ECP 2019-08-19 13:36:44 +01:00