Commit graph

174 commits

Author SHA1 Message Date
Gilles Peskine 6a26967382 Improve comments in mpi_shrink 2020-02-03 16:34:53 +01:00
Gilles Peskine 2aeab87cf7 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:34:53 +01:00
Janos Follath b4edac5616 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:27:36 +00:00
Janos Follath 5823961558 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:27:36 +00:00
Janos Follath cff9e6e03d 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:27:36 +00:00
Janos Follath 8ec2a953af Rename variable for better readability 2019-11-11 12:27:36 +00:00
Janos Follath a2b9a96fb8 mbedtls_mpi_lt_mpi_ct: Improve documentation 2019-11-11 12:27:36 +00:00
Janos Follath 51ed14e20f 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:27:36 +00:00
Janos Follath 3173a53fe9 Document ct_lt_mpi_uint 2019-11-11 12:27:36 +00:00
Janos Follath 782cbe592d mpi_lt_mpi_ct: make use of unsigned consistent 2019-11-11 12:27:36 +00:00
Janos Follath db9f449409 ct_lt_mpi_uint: make use of biL 2019-11-11 12:27:36 +00:00
Janos Follath c3b376e2f2 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:27:36 +00:00
Janos Follath 8461c0e2a8 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:27:36 +00:00
Janos Follath c587a32a9c Remove declaration after statement
Visual Studio 2013 does not like it for some reason.
2019-11-11 12:27:36 +00:00
Janos Follath e0187b95f0 Add new, constant time mpi comparison 2019-11-11 12:27:27 +00:00
Jaeden Amero 68cfefee34 Merge remote-tracking branch 'origin/pr/2399' into mbedtls-2.7
* origin/pr/2399:
  Add ChangeLog entry
  fix memory leak in mpi_miller_rabin()
2019-09-03 16:32:06 +01:00
Janos Follath 216e7385ef Fix typo 2019-03-06 14:00:44 +00:00
Hanno Becker a277d4cc82 Improve documentation of mbedtls_mpi_write_string() 2019-03-06 14:00:33 +00:00
Hanno Becker eff335d575 Fix 1-byte buffer overflow in mbedtls_mpi_write_string()
This can only occur for negative numbers. Fixes #2404.
2019-03-06 13:56:31 +00:00
Peter Kolbus f5d153daf0 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:42:27 +01:00
Jens Wiklander b2aa938ed6 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:10:01 +02:00
Jaeden Amero 9033e541a6 Merge remote-tracking branch 'origin/pr/2234' into mbedtls-2.7 2019-01-30 15:29:00 +00:00
Simon Butcher e34a4f5825 Merge remote-tracking branch 'restricted/pr/536' into mbedtls-2.7-restricted 2018-11-29 17:26:43 +00:00
Ron Eldor e6cbfc3622 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:44:03 +02:00
Gilles Peskine 220cc17165 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-26 12:45:35 +01:00
Darryl Green 94759f6593 Mark internal function as static 2018-10-19 09:21:15 +01:00
Darryl Green 56d7cc472f 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-19 09:21:15 +01:00
Janos Follath 72d555dd7c 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-11 15:38:46 +01:00
Janos Follath b39740e934 Bignum: Remove dead code
Both variables affected by the code are overwritten before their next
read.
2018-10-08 12:06:45 +01:00
Hanno Becker 930ec7dfe5 Minor fixes 2018-03-09 10:48:12 +00:00
Hanno Becker 888071184c Zeroize stack before returning from mpi_fill_random 2017-10-18 12:41:30 +01:00
Hanno Becker 073c199224 Make mpi_read_binary time constant
This commit modifies mpi_read_binary to always allocate the minimum number of
limbs required to hold the entire buffer provided to the function, regardless of
its content. Previously, leading zero bytes in the input data were detected and
used to reduce memory footprint and time, but this non-constant behavior turned
out to be non-tolerable for the cryptographic applications this function is used
for.
2017-10-17 15:17:27 +01:00
Manuel Pégourié-Gonnard db108ac944 Merge remote-tracking branch 'hanno/mpi_read_file_underflow' into development
* hanno/mpi_read_file_underflow:
  Fix potential stack underflow in mpi_read_file.
2017-06-08 19:48:03 +02:00
Hanno Becker b2034b7785 Fix potential stack underflow in mpi_read_file.
When provided with an empty line, mpi_read_file causes a numeric
underflow resulting in a stack underflow. This commit fixes this and
adds some documentation to mpi_read_file.
2017-05-09 10:29:06 +01:00
Hanno Becker 4bcb4914c5 Abort modular inversion when modulus is one.
The modular inversion function hangs when provided with the modulus 1. This commit refuses this modulus with a BAD_INPUT error code. It also adds a test for this case.
2017-05-08 14:47:04 +01:00
Hanno Becker a4af1c47d2 Correct sign in modular exponentiation algorithm.
The modular exponentiation function  handled the sign incorrectly. This commit fixes this and a test case which should have caught it.
2017-04-18 09:07:45 +01:00
Andres AG d1cc7f6f34 Fix buffer overflow in mbedtls_mpi_write_string()
Fix a buffer overflow when writting a string representation of an MPI
number to a buffer in hexadecimal. The problem occurs because hex
digits are written in pairs and this is not accounted for in the
calculation of the required buffer size when the number of digits is
odd.
2017-03-02 21:34:21 +00:00
Simon Butcher 29176897a1 Adds additional casts to calloc calls
Casts added to allow compilation of the library as C++
2016-05-23 14:29:33 +01:00
Nicholas Wilson 91c68a5e15 Shut up a clang-analyzer warning
The function appears to be safe, since grow() is called with sensible
arguments in previous functions.  Ideally Clang would be clever enough to
realise this.  Even if N has size MBEDTLS_MPI_MAX_LIMBS, which will
cause the grow to fail, the affected lines in montmul won't be reached.
Having this sanity check can hardly hurt though.
2016-05-23 14:29:28 +01:00
Alexey Skalozub e17a8da17e Rename MPI zeroize function to mbedtls_mpi_zeroize
Avoid naming confusion
2016-04-25 16:01:07 +01:00
Alexey Skalozub 3d53f41638 Faster mbedtls_zeroize for MPI
Writes in `sizeof(mbedtls_mpi_uint)` units perform faster than plain chars, also eliminates multiplication by `ciL`
2016-04-25 16:00:50 +01:00
Manuel Pégourié-Gonnard e9c1b1a3bf Merge remote-tracking branch 'yanesca/iss309' into development
* yanesca/iss309:
  Improved on the previous fix and added a test case to cover both types of carries.
  Removed recursion from fix #309.
  Improved on the fix of #309 and extended the test to cover subroutines.
  Tests and fix added for #309 (inplace mpi doubling).
2016-01-07 13:22:27 +01:00
Simon Butcher 9803d07a63 Fix for MPI divide on MSVC
Resolves multiple platform issues when building bignum.c with Microsoft
Visual Studio.
2016-01-03 00:24:34 +00:00
Simon Butcher f5ba04541e Fix for compiler warnings and style
Changes for C90 compliance, and style following review
2015-12-27 23:01:55 +00:00
Manuel Pégourié-Gonnard 1630888aa0 Fix two more compiler warnings
- declaration after statement
- always true comparison due to limited range of operand
2015-12-01 10:27:00 +01:00
Manuel Pégourié-Gonnard e3e8edfa51 Fix potential integer overflow in prev. commit
Found by Clang's -Wshift-count-overflow
2015-12-01 09:34:36 +01:00
Simon Butcher 15b15d1361 Added integer divide by as separate function
Added 64bit integer divided by 32bit integer, with remainder
2015-11-26 19:35:03 +00:00
Janos Follath 6c92268093 Improved on the previous fix and added a test case to cover both types
of carries.
2015-10-30 17:50:12 +01:00
Janos Follath 3fc644f246 Removed recursion from fix #309. 2015-10-25 14:24:10 +01:00
Janos Follath 8483e28e21 Merge remote-tracking branch 'upstream/development' into iss309 2015-10-25 12:36:03 +01:00