psa_cipher_encrypt() and psa_cipher_decrypt() sometimes add a zero offset to
a null pointer when the cipher does not use an IV. This is undefined
behavior, although it works as naively expected on most platforms. This
can cause a crash with modern Clang+ASan (depending on compiler optimizations).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
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>
The transport is allowed to be MBEDTLS_SSL_TRANSPORT_DATAGRAM when the
compression is MBEDTLS_SSL_COMPRESS_NULL.
Signed-off-by: David Horstmann <david.horstmann@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>
Change is_compression_ok() to is_compression_bad() for more semantics
that are a better match for what's really going on in the case of no
compression support.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
The bit length of m is leaked through through timing in ecp_mul_mxz().
Initially found by Manuel Pégourié-Gonnard on ecp_mul_edxyz(), which has
been inspired from ecp_mul_mxz(), during initial review of the EdDSA PR.
See: https://github.com/Mbed-TLS/mbedtls/pull/3245#discussion_r490827996
Fix that by using grp->nbits + 1 instead, which anyway is very close to
the length of m, which means there is no significant performance impact.
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
