The first half of the table is not used, let's reuse index 0 for the
result instead of appending it in the end.
Signed-off-by: Janos Follath <janos.follath@arm.com>
The table size was set before the configured window size bound was
applied which lead to out of bounds access when the configured window
size bound is less.
Signed-off-by: Janos Follath <janos.follath@arm.com>
The window size starts giving diminishing returns around 6 on most
platforms and highly unlikely to be more than 31 in practical use cases.
Still, compilers and static analysers might complain about this and
better to be pedantic.
Co-authored-by: Gilles Peskine <gilles.peskine@arm.com>
Signed-off-by: Janos Follath <janos.follath@arm.com>
With small exponents (for example, when doing RSA-1024 with CRT, each
prime is 512 bits and we'll use wsize = 5 which may be smaller that the
maximum - or even worse when doing public RSA operations which typically
have a 16-bit exponent so we'll use wsize = 1) the usage of W will have
pre-computed values, then empty space, then the accumulator at the very
end.
Move X next to the precomputed values to make accesses more efficient
and intuitive.
Signed-off-by: Janos Follath <janos.follath@arm.com>
Elements of W didn't all have the same owner: all were owned by this
function, except W[x_index]. It is more robust if we make a proper copy
of X.
Signed-off-by: Janos Follath <janos.follath@arm.com>
When x is the most negative value of a two's complement type,
`(unsigned_type)(-x)` has undefined behavior, whereas `-(unsigned_type)x`
has well-defined behavior and does what was intended.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Fix undefined behavior (typically harmless in practice) of
mbedtls_mpi_add_mpi(), mbedtls_mpi_add_abs() and mbedtls_mpi_add_int() when
both operands are 0 and the left operand is represented with 0 limbs.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
In mbedtls_mpi_add_mpi() and mbedtls_mpi_sub_mpi(), and by extention
mbedtls_mpi_add_int() and mbedtls_mpi_sub_int(), when the resulting value
was zero, the sign bit of the result was incorrectly set to -1 when the
left-hand operand was negative. This is not a valid mbedtls_mpi
representation. Fix this: always set the sign to +1 when the result is 0.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
mbedtls_mpi_add_mpi() and mbedtls_mpi_sub_mpi() have the same logic, just
with one bit to flip in the sign calculation. Move the shared logic to a new
auxiliary function. This slightly reduces the code size (if the compiler
doesn't inline) and reduces the maintenance burden.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Out of window zeroes were doing squaring on the output variable
directly. This leaks the position of windows and the out of window
zeroes.
Loading the output variable from the table in constant time removes this
leakage.
Signed-off-by: Janos Follath <janos.follath@arm.com>
We used to include platform.h only when MBEDTLS_PLATFORM_C was enabled, and
to define ad hoc replacements for mbedtls_xxx functions on a case-by-case
basis when MBEDTLS_PLATFORM_C was disabled. The only reason for this
complication was to allow building individual source modules without copying
platform.h. This is not something we support or recommend anymore, so get
rid of the complication: include platform.h unconditionally.
There should be no change in behavior since just including the header should
not change the behavior of a program.
This commit replaces most occurrences of conditional inclusion of
platform.h, using the following code:
```
perl -i -0777 -pe 's!#if.*\n#include "mbedtls/platform.h"\n(#else.*\n(#define (mbedtls|MBEDTLS)_.*\n|#include <(stdarg|stddef|stdio|stdlib|string|time)\.h>\n)*)?#endif.*!#include "mbedtls/platform.h"!mg' $(git grep -l '#include "mbedtls/platform.h"')
```
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Fix a null pointer dereference when performing some operations on zero
represented with 0 limbs: mbedtls_mpi_mod_int() dividing by 2, or
mbedtls_mpi_write_string() in base 2.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Elinimate macros defined by modules locally in the functions that are
moving to the new constant-time module.
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
There were multiple functions called mbedtls_cf_size_bool_eq. They had exactly
the same behavior, so move the one in bignum.c and remove the other.
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
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>
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>
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>
