Commit graph

8910 commits

Author SHA1 Message Date
Gilles Peskine 86dd9501ba Catch AES failure in mbedtls_ctr_drbg_random
The functions mbedtls_ctr_drbg_random() and
mbedtls_ctr_drbg_random_with_add() could return 0 if an AES function
failed. This could only happen with alternative AES
implementations (the built-in implementation of the AES functions
involved never fail), typically due to a failure in a hardware
accelerator.

Bug reported and fix proposed by Johan Uppman Bruce and Christoffer
Lauri, Sectra.
2019-11-28 09:57:44 +01:00
Jaeden Amero f0486052b7 Merge remote-tracking branch 'restricted/pr/667' into mbedtls-2.16-restricted
* restricted/pr/667: (24 commits)
  Add ChangeLog entry
  mpi_lt_mpi_ct: fix condition handling
  mpi_lt_mpi_ct: Add further tests
  mpi_lt_mpi_ct: Fix test numbering
  mpi_lt_mpi_ct perform tests for both limb size
  ct_lt_mpi_uint: cast the return value explicitely
  mbedtls_mpi_lt_mpi_ct: add tests for 32 bit limbs
  mbedtls_mpi_lt_mpi_ct: simplify condition
  Rename variable for better readability
  mbedtls_mpi_lt_mpi_ct: Improve documentation
  Make mbedtls_mpi_lt_mpi_ct more portable
  Bignum: Document assumptions about the sign field
  Add more tests for mbedtls_mpi_lt_mpi_ct
  mpi_lt_mpi_ct test: hardcode base 16
  Document ct_lt_mpi_uint
  mpi_lt_mpi_ct: make use of unsigned consistent
  ct_lt_mpi_uint: make use of biL
  Change mbedtls_mpi_cmp_mpi_ct to check less than
  mbedtls_mpi_cmp_mpi_ct: remove multiplications
  Remove excess vertical space
  ...
2019-11-12 10:48:21 +00:00
Jaeden Amero baf23000e1 Merge remote-tracking branch 'restricted/pr/669' into mbedtls-2.16-restricted
* restricted/pr/669:
  Zeroize local AES variables before exiting the function
2019-11-12 10:43:57 +00:00
Jaeden Amero 3f1c68a1e2
Merge pull request #665 from ARMmbed/dev/yanesca/iotcrypt-958-ecdsa-side-channel-fix-2.16
Backport 2.16: ECDSA side channel fix
2019-11-12 10:37:22 +00:00
Andrzej Kurek f18de50b49 Zeroize local AES variables before exiting the function
This issue has been reported by Tuba Yavuz, Farhaan Fowze, Ken (Yihang) Bai,
Grant Hernandez, and Kevin Butler (University of Florida) and
Dave Tian (Purdue University).

In AES encrypt and decrypt some variables were left on the stack. The value
of these variables can be used to recover the last round key. To follow best
practice and to limit the impact of buffer overread vulnerabilities (like
Heartbleed) we need to zeroize them before exiting the function.
2019-11-12 03:34:03 -05:00
Janos Follath d71f31bfb7 Add ChangeLog entry 2019-11-11 14:15:00 +00: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 b159ae8409 mpi_lt_mpi_ct: Add further tests
The existing tests did not catch a failure that came up at integration
testing. Adding the missing test cases to trigger the bug.
2019-11-11 12:32:12 +00:00
Janos Follath b2fddfd3f1 mpi_lt_mpi_ct: Fix test numbering 2019-11-11 12:32:12 +00:00
Janos Follath 14528628c8 mpi_lt_mpi_ct perform tests for both limb size
The corner case tests were designed for 32 and 64 bit limbs
independently and performed only on the target platform. On the other
platform they are not corner cases anymore, but we can still exercise
them.
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 798e394943 mbedtls_mpi_lt_mpi_ct: add tests for 32 bit limbs
The corner case tests were designed for 64 bit limbs and failed on 32
bit platforms because the numbers in the test ended up being stored in a
different number of limbs and the function (correctly) returnd an error
upon receiving them.
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 aac48d1b3d Bignum: Document assumptions about the sign field 2019-11-11 12:32:12 +00:00
Janos Follath e1bf02ae26 Add more tests for mbedtls_mpi_lt_mpi_ct 2019-11-11 12:32:12 +00:00
Janos Follath 27d221a1aa mpi_lt_mpi_ct test: hardcode base 16 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 3d826456f5 Remove excess vertical space 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 4c3408b140 Fix side channel vulnerability in ECDSA 2019-11-11 12:32:12 +00:00
Janos Follath e9ae6305ea Add tests to constant time mpi comparison 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
Janos Follath 1baed82774 Add ChangeLog entry 2019-10-25 11:21:30 +01:00
Janos Follath d65df1fa67 ECDSA: Fix side channel vulnerability
The blinding applied to the scalar before modular inversion is
inadequate. Bignum is not constant time/constant trace, side channel
attacks can retrieve the blinded value, factor it (it is smaller than
RSA keys and not guaranteed to have only large prime factors). Then the
key can be recovered by brute force.

Reducing the blinded value makes factoring useless because the adversary
can only recover pk*t+z*N instead of pk*t.
2019-10-25 09:11:21 +01:00
Jaeden Amero ec904e4b57
Merge pull request #2899 from gilles-peskine-arm/asan-test-fail-2.16
Backport 2.16: Make sure Asan failures are detected in 'make test'
2019-10-22 16:30:45 +01:00
Jaeden Amero 8fedeaacd5
Merge pull request #2871 from gilles-peskine-arm/test_malloc_0_null-2.16
Backport 2.16: Test the library when malloc(0) returns NULL
2019-10-22 13:41:48 +01:00
Gilles Peskine 33685f51f3 'make test' must fail if Asan fails
When running 'make test' with GNU make, if a test suite program
displays "PASSED", this was automatically counted as a pass. This
would in particular count as passing:
* A test suite with the substring "PASSED" in a test description.
* A test suite where all the test cases succeeded, but the final
  cleanup failed, in particular if a sanitizer reported a memory leak.

Use the test executable's return status instead to determine whether
the test suite passed. It's always 0 on PASSED unless the executable's
cleanup code fails, and it's never 0 on any failure.

Fix ARMmbed/mbed-crypto#303
2019-10-21 20:48:05 +02:00
Gilles Peskine ac479065f0 Asan make builds: avoid sanitizer recovery
Some sanitizers default to displaying an error message and recovering.
This could result in a test being recorded as passing despite a
complaint from the sanitizer. Turn off sanitizer recovery to avoid
this risk.
2019-10-21 20:48:05 +02:00
Gilles Peskine b1478e8ebc Use UBsan in addition to Asan with 'make test'
When building with make with the address sanitizer enabled, also
enable the undefined behavior sanitizer.
2019-10-21 20:48:05 +02:00
Gilles Peskine ff26b04fe3 Unify ASan options in make builds
Use a common set of options when building with Asan without CMake.
2019-10-21 20:48:02 +02:00
Jaeden Amero 069fb0e09a Merge remote-tracking branch 'origin/pr/2860' into mbedtls-2.16
* origin/pr/2860: (26 commits)
  config.pl full: exclude MBEDTLS_CTR_DRBG_USE_128_BIT_KEY
  mbedtls_hmac_drbg_set_entropy_len() only matters when reseeding
  mbedtls_ctr_drbg_set_entropy_len() only matters when reseeding
  mbedtls_ctr_drbg_seed: correct maximum for len
  Add a note about CTR_DRBG security strength to config.h
  Move MBEDTLS_CTR_DRBG_USE_128_BIT_KEY to the correct section
  CTR_DRBG: more consistent formatting and wording
  CTR_DRBG documentation: further wording improvements
  CTR_DRBG: Improve the explanation of security strength
  CTR_DRBG: make it easier to understand the security strength
  HMAC_DRBG: note that the initial seeding grabs entropy for the nonce
  Use standard terminology to describe the personalization string
  Do note that xxx_drbg_random functions reseed with PR enabled
  Consistently use \c NULL and \c 0
  Also mention HMAC_DRBG in the changelog entry
  HMAC_DRBG: improve the documentation of the entropy length
  HMAC_DRBG documentation improvements clarifications
  More CTR_DRBG documentation improvements and clarifications
  Fix wording
  Remove warning that the previous expanded discussion has obsoleted
  ...
2019-10-18 14:22:42 +01:00
Gilles Peskine 16ee3c15a3 config.pl full: exclude MBEDTLS_CTR_DRBG_USE_128_BIT_KEY
This is a variant toggle, not an extra feature, so it should be tested
separately.
2019-10-08 15:19:25 +02:00
Gilles Peskine 8b424397b9 mbedtls_hmac_drbg_set_entropy_len() only matters when reseeding
The documentation of HMAC_DRBG erroneously claimed that
mbedtls_hmac_drbg_set_entropy_len() had an impact on the initial
seeding. This is in fact not the case: mbedtls_hmac_drbg_seed() forces
the entropy length to its chosen value. Fix the documentation.
2019-10-04 18:28:51 +02:00
Gilles Peskine cc74872ba9 mbedtls_ctr_drbg_set_entropy_len() only matters when reseeding
The documentation of CTR_DRBG erroneously claimed that
mbedtls_ctr_drbg_set_entropy_len() had an impact on the initial
seeding. This is in fact not the case: mbedtls_ctr_drbg_seed() forces
the initial seeding to grab MBEDTLS_CTR_DRBG_ENTROPY_LEN bytes of
entropy. Fix the documentation and rewrite the discussion of the
entropy length and the security strength accordingly.
2019-10-04 18:25:05 +02:00
Gilles Peskine e215a4d05e mbedtls_ctr_drbg_seed: correct maximum for len 2019-10-04 18:22:50 +02:00
Gilles Peskine f6c2061af2 Add a note about CTR_DRBG security strength to config.h 2019-10-04 11:21:25 +02:00
Gilles Peskine 1989218456 Move MBEDTLS_CTR_DRBG_USE_128_BIT_KEY to the correct section
It's an on/off feature, so it should be listed in version_features.
2019-10-04 11:21:25 +02:00
Gilles Peskine dd5b67b4f4 CTR_DRBG: more consistent formatting and wording
In particular, don't use #MBEDTLS_xxx on macros that are undefined in
some configurations, since this would be typeset with a literal '#'.
2019-10-04 11:21:25 +02:00
Gilles Peskine e3d8cf1966 CTR_DRBG documentation: further wording improvements 2019-10-02 19:02:13 +02:00
Gilles Peskine 596fdfd6cf CTR_DRBG: Improve the explanation of security strength
Separate the cases that achieve a 128-bit strength and the cases that
achieve a 256-bit strength.
2019-10-02 19:01:31 +02:00
Jaeden Amero b9fc0798d2 Merge remote-tracking branch 'origin/pr/2864' into mbedtls-2.16
* origin/pr/2864:
  Fix compilation error
  Add const to variable
  Fix endianity issue when reading uint32
  Increase test suite timeout
  Reduce stack usage of test_suite_pkcs1_v15
  Reduce stack usage of test_suite_pkcs1_v21
  Reduce stack usage of test_suite_rsa
  Reduce stack usage of test_suite_pk
2019-10-02 18:00:31 +01: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 b0328ba8a3 Merge remote-tracking branch 'origin/pr/2323' into mbedtls-2.16
* origin/pr/2323:
  Add missing dependencies in test_suite_cipher.gcm
  Adapt ChangeLog
  Add NIST AES GCM test vectors to single-step cipher API test suite
2019-10-02 17:57:37 +01:00