Commit Graph

10569 Commits

Author SHA1 Message Date
Jonathan Bennett
b9082ed820 Allow loading symlinked certificates
When mbedtls_x509_crt_parse_path() checks each object in the supplied path, it only processes regular files. This change makes it also accept a symlink to a file. Fixes #3005.

This was observed to be a problem on Fedora/CentOS/RHEL systems, where the ca-bundle in the default location is actually a symlink.
2020-03-13 15:37:54 +00:00
Gilles Peskine
0f14c15842 Add missing return code check on calls to mbedtls_md() 2020-03-13 15:37:54 +00:00
Gilles Peskine
a48fe01f15 Check that mbedtls_mpi_grow succeeds 2020-03-13 15:37:54 +00:00
Gilles Peskine
010efeb5a2 Remove redundant block_size validity check
Check the value only once, as soon as we've obtained it.
2020-03-13 15:37:54 +00:00
Manuel Pégourié-Gonnard
aa377cf111 Fix incrementing pointer instead of value
This was introduced by a hasty search-and-replace that didn't account for C's
operator precedence when changing those variables to pointer types.
2020-03-13 15:37:54 +00:00
Gilles Peskine
c0213a91ab Add changelog entry for the unchecked mbedtls_md call 2020-03-13 15:37:03 +00:00
Gilles Peskine
140f50206e Add missing return code check on call to mbedtls_md() 2020-03-13 15:36:05 +00:00
Janos Follath
e7b49d3cd1 Bump version to Mbed TLS 2.16.4 2020-03-13 15:36:05 +00:00
Manuel Pégourié-Gonnard
10a7f626d9 Add test for record compression in ssl-opt.sh
Deprecated but still needs to be tested.
2020-03-13 15:36:05 +00:00
Gilles Peskine
8b7f03f172 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.
2020-03-13 15:36:04 +00:00
Gilles Peskine
bcdd8bcfcf Enable more test cases without MBEDTLS_MEMORY_DEBUG
None of the test cases in tests_suite_memory_buffer_alloc actually
need MBEDTLS_MEMORY_DEBUG. Some have additional checks when
MBEDTLS_MEMORY_DEBUG but all are useful even without it. So enable
them all and #ifdef out the parts that require DEBUG.
2020-03-13 15:27:12 +00:00
Gilles Peskine
6a1ec6abea More accurate test case description 2020-03-13 15:27:12 +00:00
Gilles Peskine
8064dbb646 Clarify that the "FATAL" message is expected
The test case "Memory buffer small buffer" emits a message
"FATAL: verification of first header failed". In this test case, it's
actually expected, but it looks weird to see this message from a
passing test. Add a comment that states this explicitly, and modify
the test description to indicate that the failure is expected, and
change the test function name to be more accurate.

Fix #309
2020-03-13 15:27:12 +00:00
Andrzej Kurek
a8405447aa 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.
2020-03-13 15:27:12 +00:00
Janos Follath
e9db2aa5b4 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.
2020-03-13 15:25:40 +00:00
Janos Follath
47b56a159e 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.
2020-03-13 15:25:40 +00:00
Janos Follath
006b207de6 mpi_lt_mpi_ct: Fix test numbering 2020-03-13 15:25:40 +00:00
Janos Follath
d2aa4aa454 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.
2020-03-13 15:25:40 +00:00
Janos Follath
3d2b769d1c 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.
2020-03-13 15:25:40 +00:00
Janos Follath
44e40c0792 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.
2020-03-13 15:25:40 +00:00
Janos Follath
c8256e7020 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.
2020-03-13 15:25:40 +00:00
Janos Follath
ec4c42a95f Rename variable for better readability 2020-03-13 15:25:40 +00:00
Janos Follath
cf7eeef2cc mbedtls_mpi_lt_mpi_ct: Improve documentation 2020-03-13 15:25:40 +00:00
Janos Follath
aa9e7a4717 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.
2020-03-13 15:25:40 +00:00
Janos Follath
f8dbfd4f05 Bignum: Document assumptions about the sign field 2020-03-13 15:25:40 +00:00
Janos Follath
eb8fcf8181 Add more tests for mbedtls_mpi_lt_mpi_ct 2020-03-13 15:25:40 +00:00
Janos Follath
3be2fa44e1 mpi_lt_mpi_ct test: hardcode base 16 2020-03-13 15:25:40 +00:00
Janos Follath
3480947667 Document ct_lt_mpi_uint 2020-03-13 15:25:40 +00:00
Janos Follath
afa5342452 mpi_lt_mpi_ct: make use of unsigned consistent 2020-03-13 15:25:40 +00:00
Janos Follath
a830377142 ct_lt_mpi_uint: make use of biL 2020-03-13 15:25:40 +00:00
Janos Follath
8faf1d627b 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.
2020-03-13 15:25:40 +00:00
Janos Follath
81c9fe5f2c 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.
2020-03-13 15:25:40 +00:00
Janos Follath
fd9797b595 Remove excess vertical space 2020-03-13 15:25:40 +00:00
Janos Follath
78ed22b404 Remove declaration after statement
Visual Studio 2013 does not like it for some reason.
2020-03-13 15:25:40 +00:00
Janos Follath
fc2a826ab4 Fix side channel vulnerability in ECDSA 2020-03-13 15:25:39 +00:00
Janos Follath
7ce3a25316 Add tests to constant time mpi comparison 2020-03-13 15:25:39 +00:00
Janos Follath
c514ce474a Add new, constant time mpi comparison 2020-03-13 15:25:39 +00:00
Gilles Peskine
a5e2d86c3f Note that mbedtls_ctr_drbg_seed() must not be called twice
You can't reuse a CTR_DRBG context without free()ing it and
re-init()ing it. This generally happened to work, but was never
guaranteed. It could have failed with alternative implementations of
the AES module because mbedtls_ctr_drbg_seed() calls
mbedtls_aes_init() on a context which is already initialized if
mbedtls_ctr_drbg_seed() hasn't been called before, plausibly causing a
memory leak.

Calling free() and seed() with no intervening init fails when
MBEDTLS_THREADING_C is enabled and all-bits-zero is not a valid mutex
representation.
2020-03-13 15:25:39 +00:00
Gilles Peskine
216040d46f Fix CTR_DRBG benchmark
You can't reuse a CTR_DRBG context without free()ing it and
re-init()ing. This generally happened to work, but was never
guaranteed. It could have failed with alternative implementations of
the AES module because mbedtls_ctr_drbg_seed() calls
mbedtls_aes_init() on a context which is already initialized if
mbedtls_ctr_drbg_seed() hasn't been called before, plausibly causing a
memory leak. Calling free() and seed() with no intervening init fails
when MBEDTLS_THREADING_C is enabled and all-bits-zero is not a valid
mutex representation. So add the missing free() and init().
2020-03-13 15:25:39 +00:00
Janos Follath
d69ae8c21d Add ChangeLog entry 2020-03-13 15:25:39 +00:00
Janos Follath
6bd8c0ae2a 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.
2020-03-13 15:25:39 +00:00
Gilles Peskine
bb3d55665e Changelog entry for xxx_drbg_set_entropy_len before xxx_drbg_seed 2020-03-13 15:25:39 +00:00
Gilles Peskine
20dbfb9938 CTR_DRBG: support set_entropy_len() before seed()
mbedtls_ctr_drbg_seed() always set the entropy length to the default,
so a call to mbedtls_ctr_drbg_set_entropy_len() before seed() had no
effect. Change this to the more intuitive behavior that
set_entropy_len() sets the entropy length and seed() respects that and
only uses the default entropy length if there was no call to
set_entropy_len().

The former test-only function mbedtls_ctr_drbg_seed_entropy_len() is
no longer used, but keep it for strict ABI compatibility.
2020-03-13 15:25:39 +00:00
Gilles Peskine
f0bf757f9c CTR_DRBG: Don't use functions before they're defined
Move the definitions of mbedtls_ctr_drbg_seed_entropy_len() and
mbedtls_ctr_drbg_seed() to after they are used. This makes the code
easier to read and to maintain.
2020-03-13 15:25:39 +00:00
Gilles Peskine
1d2a9e88c3 HMAC_DRBG: support set_entropy_len() before seed()
mbedtls_hmac_drbg_seed() always set the entropy length to the default,
so a call to mbedtls_hmac_drbg_set_entropy_len() before seed() had no
effect. Change this to the more intuitive behavior that
set_entropy_len() sets the entropy length and seed() respects that and
only uses the default entropy length if there was no call to
set_entropy_len().
2020-03-13 15:24:20 +00:00
Gilles Peskine
6e2cb64a97 '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
2020-03-13 15:24:20 +00:00
Gilles Peskine
6eec4ab323 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.
2020-03-13 15:24:20 +00:00
Gilles Peskine
b3e54396fa Use UBsan in addition to Asan with 'make test'
When building with make with the address sanitizer enabled, also
enable the undefined behavior sanitizer.
2020-03-13 15:24:19 +00:00
Gilles Peskine
3ccb7f18e0 Unify ASan options in make builds
Use a common set of options when building with Asan without CMake.
2020-03-13 15:24:19 +00:00
Gilles Peskine
6b5e60c26c 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.
2020-03-13 15:22:14 +00:00