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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
- “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>
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>
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>
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>
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>
Mbed OS now provides POSIX-like time functions, although not alarm() nor
signal(). It is possible to implement MBEDTLS_TIMING_ALT on Mbed OS, so
we should not artificially prevent this in check-config. Remove the the
check that prevents implementing MBEDTLS_TIMING_ALT on Mbed OS.
Note that this limitation originally was added in the following commit,
although there isn't much context around why the restriction was
imposed: 63e7ebaaa1 ("Add material for generating yotta module"). In
2015, Mbed OS was quite a different thing: no RTOS, no threads, just an
asynchronous event loop model. I'd suppose the asynchronous event loop
model made it difficult before to implement MBEDTLS_TIMING_C on Mbed OS,
but that is no longer the case.
Fixes#4633
Signed-off-by: Jaeden Amero <jaeden.amero@arm.com>
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>
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>
We stated that curves were listed "in order of preference", but we never
explained what the preference was, so this was not meaningful.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
MBEDTLS_ECP_WINDOW_SIZE is a compromise between memory usage (growing based
on the value) and performance (faster with larger values). There are
disminishing returns as the value grows larger. Based on Manuel's benchmarks
recorded in https://github.com/ARMmbed/mbedtls/issues/4127, 4 is a good
compromise point, with larger values bringing little advantage. So reduce
the default from 6 to 4.
Document the default value as in optimized for performance mostly, but don't
document the specific value, so we may change it later or make it
platform-dependent.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
