Commit graph

15626 commits

Author SHA1 Message Date
Gilles Peskine 51e1936f83
Merge pull request #840 from yanesca/reject-low-order-points-early-x25519-restricted-2.x
[Backport 2.x] Reject low order points early x25519
2021-06-28 13:45:51 +02:00
Janos Follath 2667fb708e Fix unused parameter warning
Signed-off-by: Janos Follath <janos.follath@arm.com>
2021-06-25 15:36:55 +01:00
Janos Follath ef15ce502c Add ChangeLog entry
Signed-off-by: Janos Follath <janos.follath@arm.com>
2021-06-25 14:59:30 +01:00
Janos Follath bc58902a32 Add prefix to BYTES_TO_T_UINT_*
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>
2021-06-25 14:59:15 +01:00
Janos Follath 51ccd62a08 Fix ecp_check_pub() test cases
Negative x coordinate was tested with the value -1. It happens to be one
of the low order points both for Curve25519 and Curve448 and might be
rejected because of that and not because it is negative. Make sure that
x < 0 is the only plausible reason for the point to be rejected.

Signed-off-by: Janos Follath <janos.follath@arm.com>
2021-06-25 14:59:15 +01:00
Janos Follath be89c357ae Add ecp_check_pub tests for Curve 448
Signed-off-by: Janos Follath <janos.follath@arm.com>
2021-06-25 14:59:15 +01:00
Janos Follath 7d4ebddbb6 Reject low-order points on Curve448 early
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>
2021-06-25 14:59:15 +01:00
Janos Follath 701742500d Add DoS test case for ecp_check_pub
A test case for which the loop would take practically forever if it was
reached. The point would be to validate that the loop is not reached.
The test case should cause the CI to time out if starting with the
current code, ecp_check_pubkey_mx() was changed to call
ecp_check_pubkey_x25519() first and run the mbedtls_mpi_size(() test
afterwards, which would make no semantic difference in terms of memory
contents when the function returns, but would open the way for a DoS.

Signed-off-by: Janos Follath <janos.follath@arm.com>
2021-06-25 14:59:15 +01:00
Janos Follath 1c6a439783 Use mbedtls_mpi_lset() more
Signed-off-by: Janos Follath <janos.follath@arm.com>
2021-06-25 14:59:15 +01:00
Janos Follath bc96a79854 Move mpi constant macros to bn_mul.h
Signed-off-by: Janos Follath <janos.follath@arm.com>
2021-06-25 14:59:01 +01:00
Janos Follath d31a30c083 Remove redundant ecp_check_pub() tests
Signed-off-by: Janos Follath <janos.follath@arm.com>
2021-06-25 14:15:24 +01:00
Janos Follath b4c676e6b3 Prevent memory leak in ecp_check_pubkey_x25519()
Signed-off-by: Janos Follath <janos.follath@arm.com>
2021-06-25 14:15:24 +01:00
Manuel Pégourié-Gonnard 520f0a0ea0 Avoid complaints about undeclared non-static symbols
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>
2021-06-25 14:15:24 +01:00
Manuel Pégourié-Gonnard ae48111294 Use more compact encoding of Montgomery curve constants
Base 256 beats base 16.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-25 14:15:24 +01:00
Manuel Pégourié-Gonnard 10b8e5a5c9 Use a more compact encoding of bad points
Base 10 is horrible, base 256 is much better.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-25 14:15:22 +01:00
Manuel Pégourié-Gonnard 6a5f5745d0 Add test for check_pubkey for x25519
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-25 14:06:45 +01:00
Manuel Pégourié-Gonnard f2268d1c17 Reject low-order points on Curve25519 early
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>
2021-06-25 14:06:45 +01:00
Manuel Pégourié-Gonnard 82a5a9dcdd Merge branch 'development_2.x' into development_2.x-restricted
* 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
2021-06-22 10:42:04 +02:00
Dave Rodgman c158213b2e
Merge pull request #4678 from JoeSubbiani/FixedMissingContextFree-test_suite_aes
Backport 2.x: Add missing free context at the end of aes_crypt_xts_size()
2021-06-22 09:24:14 +01:00
Manuel Pégourié-Gonnard b7a87e3059
Merge pull request #835 from mpg/rsa-lookup-2.x-restricted
[Backport 2.x] Use constant-time look-up in modular exponentiation
2021-06-22 09:33:24 +02:00
Manuel Pégourié-Gonnard 3f0538d7b7
Merge pull request #4688 from gilles-peskine-arm/winsock-fd-range-2.x
Backport 2.x: Fix net_sockets regression on Windows
2021-06-22 09:29:33 +02:00
Joe Subbiani 7d5fa2be81 Reword changelog - Test Resource Leak
- “Fix an issue where X happens” → ”Fix X“
  the extra words are just a distraction.
- “resource” → “a resource”
- “where resource is never freed” has a name: it's a resource leak
- “when running one particular test suite” → “in a test suite”

Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
2021-06-21 16:57:28 +01:00
Gilles Peskine 51859aaff2 Fix fd range for select on Windows
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>
2021-06-20 23:17:39 +02:00
Gilles Peskine 0f6351f8a9 Refactor file descriptor checks into a common function
This will make it easier to change the behavior uniformly.

No behavior change.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-20 23:17:39 +02:00
Joe Subbiani 02945bcab4 Update changelog formatting - Missing Free Context
Missing trailing full stop added to the end of the fixed issue number

Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
2021-06-18 18:55:57 +01:00
Joe Subbiani 707186d179 Update changelog formatting Missing Free Context
Trailing white space causing check_files.py to fail

Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
2021-06-18 17:45:34 +01:00
Joe Subbiani 5e1fac8b28 Update changelog formatting - Missing Free Context
The original formatting was in dos and the changelog
assembler would fail. The length of the description was
too long horizontally. This has been updated.

Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
2021-06-18 15:42:42 +01:00
Joe Subbiani 2af8d04085 Changelog entry for Free Context in test_suite_aes fix
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
2021-06-18 12:06:31 +01:00
JoeSubbiani 67889a5e64 Free context in at the end of aes_crypt_xts_size()
in file tests/suite/test_suite_aes.function, aes_crypt_xts_size()
did not free the context upon the function exit.
The function now frees the context on exit.

Fixes #4176

Signed-off-by: JoeSubbiani <Joe.Subbiani@arm.com>
2021-06-17 16:15:31 +01:00
Manuel Pégourié-Gonnard c94b6b07dc Homogenize coding patterns
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-17 16:39:47 +02:00
Gilles Peskine f97a963037
Merge pull request #4656 from gilles-peskine-arm/psa_key_derivation-bad_workflow-20210527-2.x
Backport 2.x: PSA key derivation bad-workflow tests
2021-06-17 09:55:37 +02:00
Manuel Pégourié-Gonnard fbf9aff285
Merge pull request #830 from gilles-peskine-arm/ecp_max_bits-check-2.x
Backport 2.x: check MBEDTLS_ECP_MAX_BITS
2021-06-15 11:31:11 +02:00
Gilles Peskine 8d54b69c96 Fix copypasta in test data
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-14 18:08:26 +02:00
Gilles Peskine a172cf53f7 Use UNUSED wherever applicable in derive_input tests
Exhaustivity check:
```
<tests/suites/test_suite_psa_crypto.data awk -F: '$1=="derive_input" { for (step=1; step<=3; step++) { if ($(4*step-1) == "0") { if ($(4*step) != "UNUSED" || $(4*step+1) != "\"\"" || $(4*step+2) != "UNUSED") print NR, step, $(4*step), $(4*step+1), $(4*step+2) } } }'
```

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-14 18:08:26 +02:00
Gilles Peskine f216f0d5d4 Fix missing state check for tls12_prf output
Fix PSA_ALG_TLS12_PRF and PSA_ALG_TLS12_PSK_TO_MS being too permissive
about missing inputs.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-14 18:08:26 +02:00
Gilles Peskine d40a21cff1 Key derivation: add test cases where the secret is missing
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-14 18:08:26 +02:00
Gilles Peskine f627931cde Add bad-workflow key derivation tests
Add HKDF tests where the sequence of inputs differs from the nominal
case: missing step, duplicate step, step out of order, or invalid step.

There were already similar tests for TLS 1.2 PRF. Add one with a key
agreement which has slightly different code.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-14 18:08:26 +02:00
Gilles Peskine 0faba4e8c5 More explicit names for some bad-workflow key derivation tests
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-14 18:08:26 +02:00
Gilles Peskine 3223940938 Update MBEDTLS_ECP_MAX_BITS_MIN when adding a curve
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-11 21:44:58 +02:00
Gilles Peskine 33c92f01a0 Determine MBEDTLS_ECP_MAX_BITS automatically
MBEDTLS_ECP_MAX_BITS is now determined automatically from the configured
curves and no longer needs to be configured explicitly to save RAM. Setting
it explicit in config.h is still supported for backward compatibility.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-11 21:44:58 +02:00
Gilles Peskine e57bad4b42 Check MBEDTLS_ECP_MAX_xxx constants in unit tests
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-11 21:43:26 +02:00
Gilles Peskine 6dba3200d4 Fail the build if MBEDTLS_ECP_MAX_BITS is not large enough
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-11 21:43:14 +02:00
Manuel Pégourié-Gonnard 7576f55f19 Add ChangeLog entry about RSA side channel.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:22 +02:00
Manuel Pégourié-Gonnard 0b3bde57f1 Silence MSVC type conversion warnings
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:22 +02:00
Manuel Pégourié-Gonnard f10d289441 Simplify sign selection
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:22 +02:00
Manuel Pégourié-Gonnard 5325b976b9 Avoid UB caused by conversion to int
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:22 +02:00
Manuel Pégourié-Gonnard 464fe6a4d7 Use bit operations for mpi_safe_cond_swap()
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>
2021-06-11 10:13:21 +02:00
Manuel Pégourié-Gonnard c3be399591 Use bit operations for mpi_safe_cond_assign()
- copied limbs
- sign
- cleared limbs

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:21 +02:00
Manuel Pégourié-Gonnard eaafa494e1 Avoid using == for sensitive comparisons
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>
2021-06-11 10:13:21 +02:00
Manuel Pégourié-Gonnard e10e8db6d4 Use constant-time look-up for modular exponentiation
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2021-06-11 10:13:21 +02:00