Commit Graph

9659 Commits

Author SHA1 Message Date
Gilles Peskine
f95d433655 Clarification in a comment
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
1d6b1dc955 Simplify is-zero check
The loop exits early iff there is a nonzero limb, so i==0 means that
all limbs are 0, whether the number of limbs is 0 or not.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
55e6abc99f Fix copypasta in test data
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
afbf191b17 Write a proof of correctness for mbedtls_mpi_gcd
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
2949d3ac1b Explain how the code relates to the description in HAC
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
ead6660d8c Fix copypasta in test cases
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
3cfb7be72f Annotate the choice of representation of 0 in more places
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
8c68c97db7 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 18:48:37 +02:00
Gilles Peskine
5ce7cb3d3c Fix copypasta in test function argument name
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
14357a35f7 Unify G=1 and G=-1 test cases
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
05a1af6d0f 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 18:48:37 +02:00
Gilles Peskine
44e6bb6b38 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 18:48:37 +02:00
Gilles Peskine
266275e924 mpi_shrink test: just set the top bit
No need to bypass the API to fill limbs. It's a better test to just
set the top bit that we want to have set, and it's one less bypass of
the API.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
81a6743cfb Tweak grouping of GCD test cases
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
d57f403f0c Make GCD test descriptions more uniform
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
0d3bc852b7 DHM: test some edge cases for the generator
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
4d106c1306 Add RSA tests with message=0
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
ab6ab6aaf0 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 18:48:37 +02:00
Gilles Peskine
5504d1725b mbedtls_mpi_gcd: fix the case B==0
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:48:37 +02:00
Gilles Peskine
c559eac574 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 18:48:37 +02:00
Gilles Peskine
bc781eab47 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 18:48:37 +02:00
Gilles Peskine
efc3fd4c03 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 18:48:37 +02:00
Gilles Peskine
cca6bb909d 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 18:48:37 +02:00
Gilles Peskine
9e8316e6eb 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 18:48:37 +02:00
Gilles Peskine
4cbb1c9cc9 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 18:48:37 +02:00
Gilles Peskine
1e914269b0 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 18:48:37 +02:00
Gilles Peskine
8e1aa66479 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 18:48:37 +02:00
Gilles Peskine
8854c5d450 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 18:48:37 +02:00
Gilles Peskine
b8e1534a0d 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 18:48:37 +02:00
Gilles Peskine
bbc4b8d2be 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 18:47:44 +02:00
Gilles Peskine
aa9d9ac598 Clarify a few test descriptions (mostly involving 0)
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-22 18:47:44 +02:00
Manuel Pégourié-Gonnard
07941f45e6 Merge pull request #4690 from gilles-peskine-arm/debug-print-mpi-null-2.16
Backport 2.16: Fix mbedtls_debug_print_mpi crash on 0
2021-06-22 12:09:05 +02:00
Manuel Pégourié-Gonnard
c9807ea0cc Merge pull request #4622 from gilles-peskine-arm/default-hashes-curves-2.16
Backport 2.16: Curve and hash selection for X.509 and TLS
2021-06-22 12:08:49 +02:00
Dave Rodgman
b3b1d4d9b4 Merge pull request #4684 from JoeSubbiani/2.16FixedMissingContextFree
Backport 2.16: Add Free context at the end of aes_crypt_xts_size()
2021-06-22 09:24:19 +01:00
Manuel Pégourié-Gonnard
58344efc91 Merge pull request #4689 from gilles-peskine-arm/winsock-fd-range-2.16
Backport 2.16: Fix net_sockets regression on Windows
2021-06-22 09:29:41 +02:00
Joe Subbiani
b047f99441 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:59:25 +01:00
Joe Subbiani
c8031855d0 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-21 09:30:50 +01:00
Gilles Peskine
3db875e66a Add missing parentheses
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-21 10:14:41 +02:00
Gilles Peskine
5eace4c826 Indicate that the truncation from size_t to int is deliberate
MPI sizes do fit in int. Let MSVC know this conversion is deliberate.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-21 10:14:41 +02:00
Gilles Peskine
e1a31284de Simplify mbedtls_debug_print_mpi and fix the case of empty bignums
Rewrite mbedtls_debug_print_mpi to be simpler and smaller. Leverage
mbedtls_mpi_bitlen() instead of manually looking for the leading
zeros.

Fix #4608: the old code made an invalid memory dereference when
X->n==0 (freshly initialized bignum with the value 0).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-21 10:14:41 +02:00
Gilles Peskine
6345e12161 Add mbedtls_debug_print_mpi test case for 0
There was already a test case for 0 but with a non-empty representation
(X->n == 1). Add a test case with X->n == 0 (freshly initialized mpi).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-21 10:13:43 +02:00
Gilles Peskine
d8aa3dbb04 Clarify test case descriptions
Reorder test cases and make their descriptions more explicit. No
change in test data.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-21 10:13:43 +02:00
Gilles Peskine
3257399efb SHA-1 is allowed for handshake signatures by default
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-06-21 09:54:03 +02:00
Gilles Peskine
8297657759 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:19:05 +02:00
Gilles Peskine
9065d786fd 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:19:04 +02:00
Joe Subbiani
dcdb277f34 Update changelog formatting - Missing Free Context
Trailing white space causing check_files.py to fail
issue4176.txt was also in dos format - this has been
changed to unix

Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
2021-06-18 18:59:01 +01:00
Joe Subbiani
cbe60337e3 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:23:34 +01:00
JoeSubbiani
402b1451c0 Changelog entry for Free Context in test_suite_aes fix
Signed-off-by: Joe Subbiani <joe.subbiani@arm.com>
2021-06-18 11:47:08 +01:00
JoeSubbiani
2f28c6b677 Free context 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.

Already resolved for 2.x and development - this is a backport for
2.16

Fixes #4176

Signed-off-by: JoeSubbiani <Joe.Subbiani@arm.com>
2021-06-18 11:05:47 +01:00
Manuel Pégourié-Gonnard
56efc52d6b Merge pull request #4628 from ronald-cron-arm/dhm-key-generation-bias
dhm: Fix bias in private key generation
2021-06-16 13:13:34 +02:00