Commit graph

15962 commits

Author SHA1 Message Date
Gilles Peskine 88ea3e86d7 Add RSA tests with message=0
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine 70a7dcda3f Fix multiplication producing a negative zero
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>
2021-06-22 12:39:17 +02:00
Gilles Peskine c86acc5434 mbedtls_mpi_gcd: small optimization
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>
2021-06-22 12:39:17 +02:00
Gilles Peskine b5e56ec5fd mbedtls_mpi_gcd: fix the case B==0
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine 37e7736d8e Changelog for the fix to mbedtls_mpi_exp_mod(A=0)
In Mbed TLS 2.26.0, the bug was hard to trigger, since all methods for
parsing a bignum (mbedtls_mpi_read_xxx functions) constructed an mbedtls_mpi
object with at least one limb.

In the development branch, after the commit
"New internal function mbedtls_mpi_resize_clear", this bug could be
triggered by a TLS server, by passing invalid custom Diffie-Hellman
parameters with G=0 transmitted as a 0-length byte string.

Since the behavior change in mbedtls_mpi_read_binary and
mbedtls_mpi_read_binary_le (constructing 0 limbs instead of 1 when passed
empty input) turned out to have consequences despite being in principle an
internal detail, mention it in the changelog.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine f643e8e8a9 Fix null pointer dereference in mbedtls_mpi_exp_mod
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>
2021-06-22 12:39:17 +02:00
Gilles Peskine bcfc83f7c8 Add many test cases involving 0
Test both 0 represented with 0 limbs ("0 (null)") and 0 represented
with 1 limb ("0 (1 limb)"), because occasionally there are bugs with
0-limb bignums and occasionally there are bugs with removing leading
zero limbs.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine 4cc8021a00 Test mbedtls_mpi_exp_mod both with and without _RR
mbedtls_mpi_exp_mod can be called in three ways regarding the speed-up
parameter _RR: null (unused), zero (will be updated), nonzero (will be
used). Systematically test all three.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine 1c6d6be355 mbedtls_mpi_exp_mod test: don't read RR from test data
Remove the RR parameter to the mbedtls_mpi_exp_mod test function.
It was never used in the test data, so there is no loss of functionality.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine 673d3eaa08 Add some GCD tests
Add GCD tests with negative arguments and with large non-co-prime arguments.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine 502316724f Test mbedtls_mpi_safe_cond_{assign,swap} with the basic functions
Test mbedtls_mpi_safe_cond_assign() and mbedtls_mpi_safe_cond_swap()
with their "unsafe" counterparts mbedtls_mpi_copy() and
mbedtls_mpi_swap(). This way we don't need to repeat the coverage of
test cases.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine d382c28976 Overhaul testing of mbedtls_mpi_swap
Similarly to "Overhaul testing of mbedtls_mpi_copy", simplify the code
to test mbedtls_mpi_swap to have just one function for distinct MPIs
and one function for swapping an MPI with itself, covering all cases
of size (0, 1, >1) and sign (>0, <0).

The test cases are exactly the same as for mbedtls_mpi_copy with the
following replacements:
* `Copy` -> `Swap`
* ` to ` -> ` with `
* `_copy` -> `_swap`

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine 77f55c9b00 Overhaul testing of mbedtls_mpi_copy
Replace the two test functions mbedtls_mpi_copy_sint (supporting signed
inputs but always with exactly one limb) and mbedtls_mpi_copy_binary
(supporting arbitrary-sized inputs but not negative inputs) by a single
function that supports both arbitrary-sized inputs and arbitrary-signed
inputs. This will allows testing combinations like negative source and
zero-sized destination.

Also generalize mpi_copy_self to support arbitrary inputs.

Generate a new list of test cases systematically enumerating all
possibilities among various categories: zero with 0 or 1 limb, negative or
positive with 1 limb, negative or positive with >1 limb. I used the
following Perl script:

```
sub rhs { $_ = $_[0]; s/bead/beef/; s/ca5cadedb01dfaceacc01ade/face1e55ca11ab1ecab005e5/; $_ }
%v = (
    "zero (null)" => "",
    "zero (1 limb)" => "0",
    "small positive" => "bead",
    "large positive" => "ca5cadedb01dfaceacc01ade",
    "small negative" => "-bead",
    "large negative" => "-ca5cadedb01dfaceacc01ade",
);
foreach $s (sort keys %v) {
    foreach $d (sort keys %v) {
        printf "Copy %s to %s\nmbedtls_mpi_copy:\"%s\":\"%s\"\n\n",
               $s, $d, $v{$s}, rhs($v{$d});
    }
}
foreach $s (sort keys %v) {
    printf "Copy self: %s\nmpi_copy_self:\"%s\"\n\n", $s, $v{$s};
}
```

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine b53b218bf2 Test the validity of the sign bit after constructing an MPI object
This is mostly to look for cases where the sign bit may have been left at 0
after zerozing memory, or a value of 0 with the sign bit set to -11. Both of
these mostly work fine, so they can go otherwise undetected by unit tests,
but they can break when certain combinations of functions are used.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine a0f4e10e61 Use mbedtls_test_read_mpi in test suites
Replace calls to mbedtls_mpi_read_string() with a wrapper
mbedtls_test_read_mpi() when reading test data except for the purpose
of testing mbedtls_mpi_read_string() itself. The wrapper lets the test
data control precisely how many limbs the constructed MPI has.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:39:17 +02:00
Gilles Peskine db4797198a New test helper mbedtls_test_read_mpi
This test helper reads an MPI from a string and guarantees control over the
number of limbs of the MPI, allowing test cases to construct values with or
without leading zeros, including 0 with 0 limbs.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:38:00 +02:00
Gilles Peskine 23942a4b20 Clarify a few test descriptions (mostly involving 0)
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:38:00 +02:00
Manuel Pégourié-Gonnard 6a55de9057
Merge pull request #4623 from gilles-peskine-arm/debug-print-mpi-null-2.x
Backport 2.x: Fix mbedtls_debug_print_mpi crash on 0
2021-06-22 12:08:57 +02:00
Manuel Pégourié-Gonnard 9a11ac9cc1
Merge pull request #4621 from gilles-peskine-arm/default-hashes-curves-2.x
Backport 2.x: Curve and hash selection for X.509 and TLS
2021-06-22 12:08:43 +02:00
Gilles Peskine 5ea63a31c4 Mention the Montgomery curve exception
Montgomery curves are not in the expected place in the curve list.
This is a bug (https://github.com/ARMmbed/mbedtls/issues/4698), but
until this bug is fixed, document the current behavior and indicate
that it's likely to change.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 10:50:04 +02: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 138d9f52cf SHA-1 is allowed for handshake signatures by default
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-21 09:53:25 +02: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