Commit graph

202 commits

Author SHA1 Message Date
Gilles Peskine e9073a6cb2 Add a const annotation to the non-changing argument of mpi_sub_mul
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:31:30 +02:00
Gilles Peskine 56427c2d2b Minor comment improvement 2020-02-03 16:21:31 +01:00
Gilles Peskine 27c15c7853 Improve comments in mpi_shrink 2020-02-03 16:21:31 +01:00
Gilles Peskine 3e9f5228c8 mpi_copy: make the 0 case slightly more robust
If Y was constructed through functions in this module, then Y->n == 0
iff Y->p == NULL. However we do not prevent filling mpi structures
manually, and zero may be represented with n=0 and p a valid pointer.
Most of the code can cope with such a representation, but for the
source of mbedtls_mpi_copy, this would cause an integer underflow.
Changing the test for zero from Y->p==NULL to Y->n==0 causes this case
to work at no extra cost.
2020-02-03 16:21:31 +01:00
Janos Follath e25f1ee44d mpi_lt_mpi_ct: fix condition handling
The code previously only set the done flag if the return value was one.
This led to overriding the correct return value later on.
2019-11-11 12:32:12 +00:00
Janos Follath 359a01e07c ct_lt_mpi_uint: cast the return value explicitely
The return value is always either one or zero and therefore there is no
risk of losing precision. Some compilers can't deduce this and complain.
2019-11-11 12:32:12 +00:00
Janos Follath fbe4c947cd mbedtls_mpi_lt_mpi_ct: simplify condition
In the case of *ret we might need to preserve a 0 value throughout the
loop and therefore we need an extra condition to protect it from being
overwritten.

The value of done is always 1 after *ret has been set and does not need
to be protected from overwriting. Therefore in this case the extra
condition can be removed.
2019-11-11 12:32:12 +00:00
Janos Follath 1f21c1d519 Rename variable for better readability 2019-11-11 12:32:12 +00:00
Janos Follath bd87a59007 mbedtls_mpi_lt_mpi_ct: Improve documentation 2019-11-11 12:32:12 +00:00
Janos Follath 58525180fb Make mbedtls_mpi_lt_mpi_ct more portable
The code relied on the assumptions that CHAR_BIT is 8 and that unsigned
does not have padding bits.

In the Bignum module we already assume that the sign of an MPI is either
-1 or 1. Using this, we eliminate the above mentioned dependency.
2019-11-11 12:32:12 +00:00
Janos Follath 45ec990711 Document ct_lt_mpi_uint 2019-11-11 12:32:12 +00:00
Janos Follath b11ce0ec2d mpi_lt_mpi_ct: make use of unsigned consistent 2019-11-11 12:32:12 +00:00
Janos Follath 7a34bcffef ct_lt_mpi_uint: make use of biL 2019-11-11 12:32:12 +00:00
Janos Follath 867a3abff5 Change mbedtls_mpi_cmp_mpi_ct to check less than
The signature of mbedtls_mpi_cmp_mpi_ct() meant to support using it in
place of mbedtls_mpi_cmp_mpi(). This meant full comparison functionality
and a signed result.

To make the function more universal and friendly to constant time
coding, we change the result type to unsigned. Theoretically, we could
encode the comparison result in an unsigned value, but it would be less
intuitive.

Therefore we won't be able to represent the result as unsigned anymore
and the functionality will be constrained to checking if the first
operand is less than the second. This is sufficient to support the
current use case and to check any relationship between MPIs.

The only drawback is that we need to call the function twice when
checking for equality, but this can be optimised later if an when it is
needed.
2019-11-11 12:32:12 +00:00
Janos Follath 4f6cf38016 mbedtls_mpi_cmp_mpi_ct: remove multiplications
Multiplication is known to have measurable timing variations based on
the operands. For example it typically is much faster if one of the
operands is zero. Remove them from constant time code.
2019-11-11 12:32:12 +00:00
Janos Follath 4ea2319726 Remove declaration after statement
Visual Studio 2013 does not like it for some reason.
2019-11-11 12:32:12 +00:00
Janos Follath b9f6f9bc97 Add new, constant time mpi comparison 2019-11-11 12:32:12 +00:00
Jaeden Amero da5930654e Merge remote-tracking branch 'origin/pr/2578' into mbedtls-2.16
* origin/pr/2578:
  Remove a redundant function call
2019-10-02 17:59:28 +01:00
Jaeden Amero c3bfb20a41 Merge remote-tracking branch 'origin/pr/2645' into mbedtls-2.16
* origin/pr/2645:
  Adapt ChangeLog
  Fix mpi_bigendian_to_host() on bigendian systems
2019-09-05 17:37:13 +01:00
Jaeden Amero adb4fa5921 Merge remote-tracking branch 'origin/pr/2398' into mbedtls-2.16
* origin/pr/2398:
  Add ChangeLog entry
  fix memory leak in mpi_miller_rabin()
2019-09-03 16:32:54 +01:00
Hanno Becker 92c98931f2 Fix mpi_bigendian_to_host() on bigendian systems
The previous implementation of mpi_bigendian_to_host() did
a byte-swapping regardless of the endianness of the system.

Fixes #2622.
2019-05-15 13:12:29 +01:00
Ron Eldor ff8d8d72aa Remove a redundant function call
Remove a call to `mbedtls_mpi_bitlen()` since the returned value is
overwritten in the line after. This is redundant since da31fa137a.
Fixes #2377.
2019-05-01 17:13:48 +03:00
Janos Follath 870ed0008a Fix typo 2019-03-06 13:51:30 +00:00
Hanno Becker c1fa6cdab6 Improve documentation of mbedtls_mpi_write_string() 2019-03-06 13:51:19 +00:00
Hanno Becker af97cae27d Fix 1-byte buffer overflow in mbedtls_mpi_write_string()
This can only occur for negative numbers. Fixes #2404.
2019-03-06 13:50:54 +00:00
Peter Kolbus b83d41d828 Fix DEADCODE in mbedtls_mpi_exp_mod()
In mbedtls_mpi_exp_mod(), the limit check on wsize is never true when
MBEDTLS_MPI_WINDOW_SIZE is at least 6. Wrap in a preprocessor guard
to remove the dead code and resolve a Coverity finding from the
DEADCODE checker.

Change-Id: Ice7739031a9e8249283a04de11150565b613ae89
2019-02-05 16:44:03 +01:00
Jens Wiklander dfd447e836 fix memory leak in mpi_miller_rabin()
Fixes memory leak in mpi_miller_rabin() that occurs when the function has
failed to obtain a usable random 'A' 30 turns in a row.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
2019-01-31 19:09:17 +02:00
Jaeden Amero 4002ada9f3 Merge remote-tracking branch 'origin/pr/2214' into mbedtls-2.16 2019-01-30 15:03:02 +00:00
Hanno Becker 0e810b9648 Don't call memcpy with NULL pointer in mbedtls_mpi_read_binary()
mbedtls_mpi_read_binary() calls memcpy() with the source pointer being
the source pointer passed to mbedtls_mpi_read_binary(), the latter may
be NULL if the buffer length is 0 (and this happens e.g. in the ECJPAKE
test suite). The behavior of memcpy(), in contrast, is undefined when
called with NULL source buffer, even if the length of the copy operation
is 0.

This commit fixes this by explicitly checking that the source pointer is
not NULL before calling memcpy(), and skipping the call otherwise.
2019-01-03 17:13:11 +00:00
Hanno Becker 9f6d16ad79 Fix preprocessor macro existence check in bignum.c 2019-01-02 17:15:06 +00:00
Hanno Becker 6dab6200c6 Fix typo after rebase 2019-01-02 16:42:29 +00:00
Hanno Becker 5d91c0bbee Add missing macro existence check in byte swapping code in bignum.c 2019-01-02 11:38:14 +00:00
Hanno Becker f872007782 Optimize mpi_bigendian_to_host() for speed and size
Use GCC / Clang builtins for byte swapping.
2019-01-02 11:38:14 +00:00
Hanno Becker 2be8a55f72 Change signature of mpi_bigendian_to_host() to reflect usage 2019-01-02 11:37:25 +00:00
Hanno Becker da1655a48e Remove temporary stack-buffer from mbedtls_mpi_fill_random()
Context: The function `mbedtls_mpi_fill_random()` uses a temporary stack
buffer to hold the random data before reading it into the target MPI.

Problem: This is inefficient both computationally and memory-wise.
Memory-wise, it may lead to a stack overflow on constrained devices with
limited stack.

Fix: This commit introduces the following changes to get rid of the
temporary stack buffer entirely:

1. It modifies the call to the PRNG to output the random data directly
   into the target MPI's data buffer.

This alone, however, constitutes a change of observable behaviour:
The previous implementation guaranteed to interpret the bytes emitted by
the PRNG in a big-endian fashion, while rerouting the PRNG output into the
target MPI's limb array leads to an interpretation that depends on the
endianness of the host machine.
As a remedy, the following change is applied, too:

2. Reorder the bytes emitted from the PRNG within the target MPI's
   data buffer to ensure big-endian semantics.

Luckily, the byte reordering was already implemented as part of
`mbedtls_mpi_read_binary()`, so:

3. Extract bigendian-to-host byte reordering from
   `mbedtls_mpi_read_binary()` to a separate internal function
   `mpi_bigendian_to_host()` to be used by `mbedtls_mpi_read_binary()`
   and `mbedtls_mpi_fill_random()`.
2019-01-02 11:37:25 +00:00
Hanno Becker f25ee7f79d Fix parameter validation for mbedtls_mpi_lsb()
The MPI_VALIDATE_RET() macro cannot be used for parameter
validation of mbedtls_mpi_lsb() because this function returns
a size_t.

Use the underlying MBEDTLS_INTERNAL_VALIDATE_RET() insteaed,
returning 0 on failure.

Also, add a test for this behaviour.
2018-12-19 16:51:50 +00:00
Hanno Becker 8ce11a323e Minor improvements to bignum module 2018-12-19 16:18:52 +00:00
Hanno Becker 54c91dd235 Remove double semicolon from bignum.c 2018-12-18 18:12:13 +00:00
Hanno Becker 73d7d79bc1 Implement parameter validation for MPI module 2018-12-18 18:12:13 +00:00
Simon Butcher b9eb7866eb Merge remote-tracking branch 'restricted/pr/535' into development 2018-11-29 16:54:51 +00:00
Ron Eldor a16fa297f7 Refactor mpi_write_hlp to not be recursive
Refactor `mpi_write_hlp()` to not be recursive, to fix stack overflows.
Iterate over the `mbedtls_mpi` division of the radix requested,
until it is zero. Each iteration, put the residue in the next LSB
of the output buffer. Fixes #2190
2018-11-27 10:34:36 +02:00
Gilles Peskine 11cdb0559e mbedtls_mpi_write_binary: don't leak the exact size of the number
In mbedtls_mpi_write_binary, avoid leaking the size of the number
through timing or branches, if possible. More precisely, if the number
fits in the output buffer based on its allocated size, the new code's
trace doesn't depend on the value of the number.
2018-11-20 17:09:27 +01:00
Darryl Green e3f95ed25b Fix bias in random number generation in Miller-Rabin test
When a random number is generated for the Miller-Rabin primality test,
if the bit length of the random number is larger than the number being
tested, the random number is shifted right to have the same bit length.
This introduces bias, as the random number is now guaranteed to be
larger than 2^(bit length-1).

Changing this to instead zero all bits higher than the tested numbers
bit length will remove this bias and keep the random number being
uniformly generated.
2018-10-09 16:36:53 +01:00
Janos Follath a0b67c2f3e Bignum: Deprecate mbedtls_mpi_is_prime()
When using a primality testing function the tolerable error rate depends
on the scheme in question, the required security strength and wether it
is used for key generation or parameter validation. To support all use
cases we need more flexibility than what the old API provides.
2018-10-09 16:36:53 +01:00
Janos Follath da31fa137a Bignum: Fix prime validation vulnerability
The input distribution to primality testing functions is completely
different when used for generating primes and when for validating
primes. The constants used in the library are geared towards the prime
generation use case and are weak when used for validation. (Maliciously
constructed composite numbers can pass the test with high probability)

The mbedtls_mpi_is_prime() function is in the public API and although it
is not documented, it is reasonable to assume that the primary use case
is validating primes. The RSA module too uses it for validating key
material.
2018-10-09 16:36:53 +01:00
Janos Follath b728c29114 Bignum: Remove dead code
Both variables affected by the code are overwritten before their next
read.
2018-10-09 16:33:27 +01:00
Janos Follath f301d23ceb Bignum: Improve primality test for FIPS primes
The FIPS 186-4 RSA key generation prescribes lower failure probability
in primality testing and this makes key generation slower. We enable the
caller to decide between compliance/security and performance.

This python script calculates the base two logarithm of the formulas in
HAC Fact 4.48 and was used to determine the breakpoints and number of
rounds:

def mrpkt_log_2(k, t):
    if t <= k/9.0:
        return 3*math.log(k,2)/2+t-math.log(t,2)/2+4-2*math.sqrt(t*k)
    elif t <= k/4.0:
        c1 = math.log(7.0*k/20,2)-5*t
        c2 = math.log(1/7.0,2)+15*math.log(k,2)/4.0-k/2.0-2*t
        c3 = math.log(12*k,2)-k/4.0-3*t
        return max(c1, c2, c3)
    else:
        return math.log(1/7.0)+15*math.log(k,2)/4.0-k/2.0-2*t
2018-10-09 16:33:27 +01:00
Janos Follath 7c025a9f50 Generalize dh_flag in mbedtls_mpi_gen_prime
Setting the dh_flag to 1 used to indicate that the caller requests safe
primes from mbedtls_mpi_gen_prime. We generalize the functionality to
make room for more flags in that parameter.
2018-09-21 16:30:07 +01:00
Jaeden Amero a331e0f0af Merge remote-tracking branch 'upstream-restricted/pr/421' into development-proposed 2018-05-04 14:39:24 +01:00
Andres Amaya Garcia 6698d2fc5c Fix style for mbedtls_mpi_zeroize() 2018-04-24 08:39:07 -05:00