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>
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>
mbedtls_mpi_read_binary{,_le} (in https://github.com/ARMmbed/mbedtls/pull/4276)
and mbedtls_mpi_read_string (in https://github.com/ARMmbed/mbedtls/pull/4644)
changed their behavior on an empty input from constructing an MPI object with
one limb to not allocating a limb. In principle, this change should be
transparent to applications, however it caused a bug in the library and it does
affect the value when writing back out, so list the change in the changelog.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
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>
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>
- “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>
Introduce psa_mac_compute_internal with an
additional `is_sign` parameter compared to
the psa_mac_compute API. The intent is to
call psa_mac_compute_internal() from
psa_mac_verify() as well to compute the
message MAC.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
