Commit Graph

9498 Commits

Author SHA1 Message Date
Janos Follath
1b6a24f759
Merge pull request #4180 from gilles-peskine-arm/net_poll-fd_setsize-2.16
Backport 2.16: Fix stack corruption in mbedtls_net_poll with large file descriptor
2021-03-04 12:15:53 +00:00
Gilles Peskine
388a9d3a8b Update error codes listed in the net_sockets documentation
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-03-03 12:35:46 +01:00
Gilles Peskine
121d7c7c14 Fix sloppy wording around stricly less-than vs less or equal
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-03-01 16:38:02 +01:00
Gilles Peskine
58ec378912 Document FD_SETSIZE limitation for mbedtls_net_{poll,recv_timeout}
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-03-01 16:37:53 +01:00
Gilles Peskine
f604240b1b Fix stack buffer overflow in net functions with large file descriptor
Fix a stack buffer overflow with mbedtls_net_poll() and
mbedtls_net_recv_timeout() when given a file descriptor that is beyond
FD_SETSIZE. The bug was due to not checking that the file descriptor
is within the range of an fd_set object.

Fix #4169

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-03-01 16:37:45 +01:00
Gilles Peskine
b01ce91745
Merge pull request #4154 from chris-jones-arm/test-mutex-usage-2.16
Backport 2.16: test and fix mutex usage
2021-02-23 15:14:48 +01:00
Gilles Peskine
57f8e9116e Make entropy double-free work
Although the library documentation does not guarantee that calling
mbedtls_entropy_free() twice works, it's a plausible assumption and it's
natural to write code that frees an object twice. While this is uncommon for
an entropy context, which is usually a global variable, it came up in our
own unit tests (random_twice tests in test_suite_random in the
development branch).

Announce this in the same changelog entry as for RSA because it's the same
bug in the two modules.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 11:29:25 +01:00
Gilles Peskine
210a0168d5 Add init-free tests for entropy
These tests validate that an entropy object can be reused and that
calling mbedtls_entropy_free() twice is ok.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 11:29:18 +01:00
Chris Jones
6855d1a457 Add MBEDTLS_TEST_HOOKS to query_config.c
Signed-off-by: Chris Jones <christopher.jones@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
ed9f7989f2 Fix typo in documentation
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
ce455ddb3e Document mutex usage for RSA
The mutex is now initialized iff ver != 0.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
3c30a7aeda Changelog entry for RSA mutex usage fix
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
718972e94e Fix mutex leak in RSA
mbedtls_rsa_gen_key() was not freeing the RSA object, and specifically
not freeing the mutex, in some error cases.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
b9fce3cea1 Fix mutex double-free in RSA
When MBEDTLS_THREADING_C is enabled, RSA code protects the use of the
key with a mutex. mbedtls_rsa_free() frees this mutex by calling
mbedtls_mutex_free(). This does not match the usage of
mbedtls_mutex_free(), which in general can only be done once.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
ab5849527d Add init-free tests for RSA
These tests are trivial except when compiling with MBEDTLS_THREADING_C
and a mutex implementation that are picky about matching each
mbedtls_mutex_init() with exactly one mbedtls_mutex_free().

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Chris Jones <christopher.jones@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
468ef4b3c7 Add missing cleanup in a test function
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
0c11622504 Changelog entry for DRBG mutex usage fix
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
fb6876a111 Document thread safety for HMAC_DRBG
random(), and only this function, is thread-safe.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Chris Jones <christopher.jones@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
b5e295d5c9 Document mutex invariant for HMAC_DRBG
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
05974893e6 Fix mutex leak in HMAC_DRBG
mbedtls_hmac_drbg_free() left a mutex in the initialized state. This
caused a resource leak on platforms where mbedtls_mutex_init()
allocates resources.

To fix this, mbedtls_hmac_drbg_free() no longer reinitializes the
mutex. To preserve the property that mbedtls_hmac_drbg_free() leaves
the object in an initialized state, which is generally true throughout
the library except regarding mutex objects on some platforms, no
longer initialize the mutex in mbedtls_hmac_drbg_init(). Since the
mutex is only used after seeding, and seeding is only permitted once,
call mbedtls_mutex_init() as part of the seeding process.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
831956980c Document thread safety for CTR_DRBG
random(), and only this function, is thread-safe.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Chris Jones <christopher.jones@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
2ecc0b89f3 Document mutex invariant for CTR_DRBG
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
89816bc020 Fix mutex leak in CTR_DRBG
mbedtls_ctr_drbg_free() left a mutex in the initialized state. This
caused a resource leak on platforms where mbedtls_mutex_init()
allocates resources.

To fix this, mbedtls_ctr_drbg_free() no longer reinitializes the
mutex. To preserve the property that mbedtls_ctr_drbg_free() leaves
the object in an initialized state, which is generally true throughout
the library except regarding mutex objects on some platforms, no
longer initialize the mutex in mbedtls_ctr_drbg_init(). Since the
mutex is only used after seeding, and seeding is only permitted once,
call mbedtls_mutex_init() as part of the seeding process.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
7ba73e5756 Explain the usage of is_valid in pthread mutexes
Document the usage inside the library, and relate it with how it's
additionally used in the test code.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
7252ec3947 Count and report non-freed mutexes
Subtract the number of calls to mbedtls_mutex_free() from the number
of calls to mbedtls_mutex_init(). A mutex leak will manifest as a
positive result at the end of the test case.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Chris Jones <christopher.jones@arm.com>
2021-02-22 19:40:41 +01:00
Gilles Peskine
cd2e248fdd Detect and report mutex usage errors
If the mutex usage verification framework is enabled and it detects a
mutex usage error, report this error and mark the test as failed.

This detects most usage errors, but not all cases of using
uninitialized memory (which is impossible in full generality) and not
leaks due to missing free (which will be handled in a subsequent commit).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Chris Jones <christopher.jones@arm.com>
2021-02-22 19:40:29 +01:00
Gilles Peskine
6c91b7c91e
Merge pull request #4155 from gilles-peskine-arm/ccm-test-iv-overflow-warning-2.16
Backport 2.16: Silence gcc-10 warning in test_suite_ccm
2021-02-20 00:12:26 +01:00
Gilles Peskine
e8d7e6c6e4 More robust code to set the IV
Check that the source address and the frame counter have the expected
length. Otherwise, if the test data was invalid, the test code could
build nonsensical inputs, potentially overflowing the iv buffer.

The primary benefit of this change is that it also silences a warning
from compiling with `gcc-10 -O3` (observed with GCC 10.2.0 on
Linux/amd64). GCC unrolled the loops and complained about a buffer
overflow with warnings like:
```
suites/test_suite_ccm.function: In function 'test_mbedtls_ccm_star_auth_decrypt':
suites/test_suite_ccm.function:271:15: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
  271 |         iv[i] = source_address->x[i];
      |         ~~~~~~^~~~~~~~~~~~~~~~~~~~~~
suites/test_suite_ccm.function:254:19: note: at offset [13, 14] to object 'iv' with size 13 declared here
  254 |     unsigned char iv[13];
```
Just using memcpy instead of loops bypasses this warnings. The added
checks are a bonus.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-17 18:08:06 +01:00
Gilles Peskine
c071373842 Mutex usage testing: set up wrapper functions
When using pthread mutexes (MBEDTLS_THREADING_C and
MBEDTLS_THREADING_PTHREAD enabled), and when test hooks are
enabled (MBEDTLS_TEST_HOOKS), set up wrappers around the
mbedtls_mutex_xxx abstraction. In this commit, the wrapper functions
don't do anything yet.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Chris Jones <christopher.jones@arm.com>
2021-02-17 13:10:42 +00:00
Gilles Peskine
96a7064754 Remove reference to a document that doesn't exist in this branch
Don't reference the architecture document. This is an LTS branch and
new invasive tests are only going to be introduced as (perhaps partial
or adapted) backports of invasive tests from development anyway.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-17 12:53:44 +00:00
Gilles Peskine
44e89c547f Declare MBEDTLS_TEST_HOOKS in config.h
When this option is enabled, the product includes additional
interfaces that enable additional tests. This option should not be
enabled in production, but is included in the "full" build to enable
the extra tests.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-17 12:50:52 +00:00
Gilles Peskine
7f652adc48 Use $ASAN_FLAGS instead of repeating its contents
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-17 12:48:33 +00:00
Manuel Pégourié-Gonnard
47e4035e98
Merge pull request #4134 from gilles-peskine-arm/ssl-opt-server-failure-2.16
Backport 2.16: ssl-opt.sh: if the server fails, do treat it as a test failure
2021-02-12 12:16:09 +01:00
Gilles Peskine
2cf44b6941 ssl-opt.sh: Only check the server exit for Mbed TLS
We care about the exit code of our server, for example if it's
reporting a memory leak after having otherwise executed correctly.

We don't care about the exit code of the servers we're using for
interoperability testing (openssl s_server, gnutls-serv). We assume
that they're working correctly anyway, and they return 1 (gnutls-serv)
or die by the signal handle the signal (openssl) when killed by a
signal.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-10 13:05:21 +01:00
Gilles Peskine
634fe27a12 ssl-opt.sh: if the server fails, do treat it as a test failure
This used to be the case a long time ago but was accidentally broken.

Fix <github:nogrep> #4103 for ssl-opt.sh.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-10 13:05:19 +01:00
Janos Follath
fee234afcd
Merge pull request #4100 from d-otte/mbedtls-2.16
Backport 2.16: wrong RSA_PRV_DER_MAX_BYTES for odd MBEDTLS_MPI_MAX_SIZE
2021-02-02 16:14:59 +00:00
Janos Follath
9039f16c48
Merge pull request #4097 from gilles-peskine-arm/mpi_sub_abs-buffer_overflow-2.16
Backport 2.16: Fix buffer overflow in mbedtls_mpi_sub_abs negative case
2021-02-02 13:10:22 +00:00
Daniel Otte
80fa1b4d8f adding changelog entry for issue #4093
Signed-off-by: Daniel Otte <d.otte@wut.de>
2021-02-02 12:57:48 +01:00
Daniel Otte
9c6cb217f1 adding parentheses to macro definitions.
Avoid confusion and possible mistakes in usage of macros.

Signed-off-by: Daniel Otte <d.otte@wut.de>
2021-02-02 12:52:18 +01:00
Daniel Otte
80a2c2a5f9 avoid errorneous computation of RSA_PRV_DER_MAX_BYTES.
if MBEDTLS_MPI_MAX_SIZE is odd then RSA_PRV_DER_MAX_BYTES will be two less than expected, since the macros are lacking parentheses.


Signed-off-by: Daniel Otte <d.otte@wut.de>
2021-02-02 12:51:02 +01:00
Gilles Peskine
6260b70717 mbedtls_mpi_sub_abs: fix buffer overflow in error case
Fix a buffer overflow in mbedtls_mpi_sub_abs() when calculating
|A| - |B| where |B| is larger than |A| and has more limbs (so the
function should return MBEDTLS_ERR_MPI_NEGATIVE_VALUE).

Fix #4042

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-01 17:27:06 +01:00
Gilles Peskine
9a3cf3174d Add mpi_sub_abs negative tests with a larger-in-size second operand
Add test cases for mbedtls_mpi_sub_abs() where the second operand has
more limbs than the first operand (which, if the extra limbs are not
all zero, implies that the function returns
MBEDTLS_ERR_MPI_NEGATIVE_VALUE).

This exposes a buffer overflow (reported in #4042).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-01 17:27:04 +01:00
Janos Follath
5d453ee882
Merge pull request #4068 from stevew817/backport/pr-4008
[Backport 2.16] Avoid unreferenced items in ECDSA when ALT is in use
2021-01-29 12:54:35 +00:00
Ronald Cron
226626fd42
Merge pull request #4021 from gilles-peskine-arm/ssl-test_without_hmac_drbg-2.16
Backport 2.16: Test SSL with non-deterministic ECDSA
2021-01-29 09:10:11 +01:00
Steven Cooreman
a82e56aa91 Avoid unreferenced item warnings in ECDSA when ALT is in use
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
2021-01-26 18:04:10 +01:00
Janos Follath
01c69377bd
Merge pull request #4057 from stevew817/backport/pr-4007
[backport 2.16] Skip known entropy tests for ECJPAKE ALT implementations
2021-01-25 12:38:53 +00:00
Steven Cooreman
0b7cb319cd Skip tests requiring known entropy for ECJPAKE ALT implementations
These implementations don't necessarily consume entropy the same way the
mbed TLS internal software implementation does, and the 'reference
handshake' test vectors can thus not be applied to an ALT implementation.

Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
2021-01-25 10:36:37 +01:00
Gilles Peskine
629fd9362c Test SSL with non-deterministic ECDSA
In component_test_no_hmac_drbg, the fact that HMAC_DRBG is disabled
doesn't affect the SSL code, but the fact that deterministic ECDSA is
disabled does. So run some ECDSA-related SSL tests.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-01-13 20:38:13 +01:00
Gilles Peskine
15c39e53e5
Merge pull request #3988 from gilles-peskine-arm/rsa_private-ret-2.16
Backport 2.16: Fix an incorrect error code if RSA private operation glitched
2021-01-13 11:10:08 +01:00
Gilles Peskine
3b7523e11e Fix an incorrect error code if RSA private operation glitched
mbedtls_rsa_private() could return the sum of two RSA error codes
instead of a valid error code in some rare circumstances:

* If rsa_prepare_blinding() returned  MBEDTLS_ERR_RSA_RNG_FAILED
  (indicating a misbehaving or misconfigured RNG).
* If the comparison with the public value failed (typically indicating
  a glitch attack).

Make sure not to add two high-level error codes.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-01-06 20:55:34 +01:00