Commit graph

106 commits

Author SHA1 Message Date
Gilles Peskine 2a3041f74e Fix copypasta in test data
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:47:21 +02:00
Gilles Peskine bd5d6ef8de Fix copypasta in test cases
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:47:21 +02:00
Gilles Peskine 8998770771 Annotate the choice of representation of 0 in more places
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:47:21 +02:00
Gilles Peskine 8f70d3f4e2 Improve coverage of mbedtls_mpi_cmp_mpi
Test with and without leading zeros on each side.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:47:21 +02:00
Gilles Peskine d96b3650ec In test cases where the result is 0, express it as "0", not ""
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:47:21 +02:00
Gilles Peskine 997be0aba3 Fix multiplication with negative result and a low-order 0 limb
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>
2021-06-22 12:47:21 +02:00
Gilles Peskine 3f1c3c9605 Tweak grouping of GCD test cases
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:47:21 +02:00
Gilles Peskine 9466d81fe8 Make GCD test descriptions more uniform
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:47:21 +02:00
Gilles Peskine 7cba859235 mbedtls_mpi_read_string: make an empty bignum for an empty string
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>
2021-06-22 12:47:21 +02:00
Gilles Peskine f4998b0a20 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:47:21 +02:00
Gilles Peskine 3da1a8ff39 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:44:05 +02:00
Gilles Peskine 3637488eb4 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:44:05 +02:00
Gilles Peskine 424390c517 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:44:05 +02:00
Gilles Peskine 9e9314ff93 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:44:05 +02:00
Gilles Peskine d0722f8e71 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:44:05 +02:00
Gilles Peskine fc1eeefc99 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:44:05 +02:00
Gilles Peskine 90ec8e857c 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:44:05 +02:00
Gilles Peskine 5fb6f700ba Clarify a few test descriptions (mostly involving 0)
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 12:43:00 +02:00
TRodziewicz 0730cd5d9e Merge branch 'development' into Remove__CHECK_PARAMS_option 2021-06-07 15:41:49 +02:00
Gilles Peskine 9077e435c6 Fix mistakes in test case descriptions
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-03 18:10:04 +02:00
Gilles Peskine 0cb493d239 Note that the "0 limb in ..." tests rely on undocumented behavior
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-03 18:10:04 +02:00
Gilles Peskine 0ad640ab83 MPI random test: Add test cases with lower_bound > upper_bound
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-03 18:10:04 +02:00
Gilles Peskine 951b5695e3 MPI random test: Add a few more small-range tests
Do more iterations with small values. This makes it more likely that a
mistake on bounds will be detected.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-03 18:10:04 +02:00
Gilles Peskine fbb90098e8 Fix copypasta in test case description
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-03 18:10:04 +02:00
Gilles Peskine 422e867acb MPI random: add unit tests with a previously nonzero value
Add unit tests for mbedtls_mpi_fill_random() and mbedtls_mpi_random()
when the resulting MPI object previously had a nonzero value. I wrote
those to catch a bug that I introduced during the development of
mbedtls_mpi_random() (but does not appear in a committed version).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-03 18:10:04 +02:00
Gilles Peskine 1a7df4eda0 Fix mbedtls_mpi_random when N has leading zeros
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>
2021-06-03 18:10:04 +02:00
Gilles Peskine 1e918f44c9 mbedtls_mpi_random: check for invalid arguments
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-03 18:10:04 +02:00
Gilles Peskine 02ac93a1a3 Move mbedtls_mpi_random to the bignum module
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>
2021-06-03 18:10:04 +02:00
TRodziewicz 7019955c76 Removal of the TEST_VALID_PARAM macro and its usages
Signed-off-by: TRodziewicz <tomasz.rodziewicz@mobica.com>
2021-05-27 17:35:04 +02:00
Ronald Cron 875b5fb7fa Refactor optional parameter check tests
Remove tests related to NULL pointers,
keep tests related to invalid enum values.
Remove test code related to MBEDTLS_CHECK_PARAMS.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Signed-off-by: TRodziewicz <tomasz.rodziewicz@mobica.com>
2021-05-27 17:27:14 +02:00
Gilles Peskine bd43f67a9b Fix copypasta in test case description
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-04-09 15:46:40 +02:00
Gilles Peskine b13a26cd8c Add a few unit tests for mbedtls_mpi_read_string with leading zeros
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-04-03 18:25:29 +02:00
Gilles Peskine 43e89e1b15 Add mpi_sub_abs negative tests with a larger-in-size second operand
Add test cases for mbedtls_mpi_sub_abs() where the second operand has
more limbs than the first operand (which, if the extra limbs are not
all zero, implies that the function returns
MBEDTLS_ERR_MPI_NEGATIVE_VALUE).

This exposes a buffer overflow (reported in #4042).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-01 17:28:03 +01:00
Janos Follath a946dcf8b2 Merge branch 'development-restricted' into mbedtls-2.25.0r0-pr 2020-12-08 20:59:45 +00:00
Janos Follath 7973e2b358
Merge pull request #772 from chris-jones-arm/development-restricted
Fix Diffie-Hellman large key size DoS
2020-12-07 09:28:55 +00:00
Gilles Peskine 56f943a184 Always test in-place addition and subtraction
Run all the addition and subtraction tests with the result aliased to
the first operand and with the result aliased to the second operand.
Before, only some of the aliasing possibilities were tested, for only
some of the functions, with only some inputs.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-12-06 22:32:02 +01:00
Chris Jones e64a46f466 Fix exponentiation tests with MBEDTLS_MPI_MAX_BITS larger than 256
Fixes an issue where configs that had `MBEDTLS_MPI_MAX_BITS` greater than 256
but smaller than the test that was running (792 bits) the test would fail
incorrectly.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
2020-12-03 17:48:27 +00:00
Chris Jones 7c430305e3 Move dependancy to specific test cases
Move dependancy on `MBEDTLS_MPI_MAX_BITS` to apply to the specific test cases
which will break when `MBEDTLS_MPI_MAX_BITS` is too small. This re-enables
previous tests that were turned off accidentally.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
2020-12-03 15:22:25 +00:00
Chris Jones e146bdcff9 Fix cases where exponentiation was not fully tested
In two test cases, the exponentiation computation was not being fully tested
as when A_bytes (the base) == N_bytes (the modulus) -> A = N. When this is the
case A is reduced to 0 and therefore the result of the computation will always
be 0.

This fixes that issue and therefore increases the test coverage to ensure
different computations are actually being run.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
2020-12-03 11:52:40 +00:00
Chris Jones aa850cdfdc Reword test cases
Reword test cases to be easier to read and understand.
Adds comments to better explain what the test is doing.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
2020-12-03 11:35:41 +00:00
Chris Jones a03f56fe8f Extend exponentiation test coverage
Add two further boundary tests for cases where both the exponent and modulus to
`mbedtls_mpi_exp_mod()` are `MBEDTLS_MPI_MAX_SIZE`, or longer, bytes long.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
2020-12-03 11:20:33 +00:00
Chris Jones d10b331574 Test that overly large Diffie-Hellman keys are rejected
Adds test cases to ensure that `mbedtls_mpi_exp_mod` will return an error with
an exponent or modulus that is greater than `MBEDTLS_MPI_MAX_SIZE` in size.

Adds test cases to ensure that Diffie-Hellman will fail to make a key pair
(using `mbedtls_dhm_make_public`) when the prime modulus is greater than
`MBEDTLS_MPI_MAX_SIZE` in size.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
2020-12-02 15:25:27 +00:00
Chris Jones 0c5875fd0c Test that overly large Diffie-Hellman keys are rejected
Add a test case to ensure `mbedtls_mpi_exp_mod` fails when using a key size
larger than MBEDTLS_MPI_MAX_SIZE.
Add a test case to ensure that Diffie-Hellman operations fail when using a key
size larger than MBEDTLS_MPI_MAX_SIZE.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
2020-12-02 15:51:39 +00:00
Gilles Peskine 3cb1e296a4 Test mbedtls_mpi_fill_random
Positive tests: test that the RNG has the expected size, given that we
know how many leading zeros it has because we know how the function
consumes bytes and when the test RNG produces null bytes.

Negative tests: test that if the RNG is willing to emit less than the
number of wanted bytes, the function fails.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-11-25 17:30:44 +01:00
Manuel Pégourié-Gonnard 4d8c836cdc
Merge pull request #346 from gilles-peskine-arm/mpi_copy_shrink
Improve robustness and testing of mbedtls_mpi_copy
2020-02-06 09:52:01 +01:00
Gilles Peskine a9da093617 shrink tests: clearer description 2020-02-03 16:18:30 +01:00
Gilles Peskine 7428b45126 Better coverage for copy and swap
Cover more cases: different signs, different zeronesses, repeated
argument.
2020-01-20 21:01:51 +01:00
Gilles Peskine 72d7609f82 Bignum copy/shrink: More precise test case descriptions 2020-01-20 17:19:03 +01:00
Janos Follath 0b1ae0e972 mpi_lt_mpi_ct: Add further tests
The existing tests did not catch a failure that came up at integration
testing. Adding the missing test cases to trigger the bug.
2019-11-05 15:13:00 +00:00
Janos Follath 53fc7b0309 mpi_lt_mpi_ct: Fix test numbering 2019-11-05 15:13:00 +00:00