Commit graph

4332 commits

Author SHA1 Message Date
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 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 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 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 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
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
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
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
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
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 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
Janos Follath 69029cd29b Bump version to Mbed TLS 2.16.9
Executed ./scripts/bump_version.sh --version 2.16.9

Signed-off-by: Janos Follath <janos.follath@arm.com>
2020-12-09 01:21:50 +00:00
Janos Follath 15e860c639 Merge branch 'mbedtls-2.16-restricted' into mbedtls-2.16.9r0-pr 2020-12-08 21:00:17 +00:00
Paul Elliott 319b5939dd Remove Extraneous bytes from buffer post pem write
In order to remove large buffers from the stack, the der data is written
into the same buffer that the pem is eventually written into, however
although the pem data is zero terminated, there is now data left in the
buffer after the zero termination, which can cause
mbedtls_x509_crt_parse to fail to parse the same buffer if passed back
in. Patches also applied to mbedtls_pk_write_pubkey_pem, and
mbedtls_pk_write_key_pem, which use similar methods of writing der data
to the same buffer, and tests modified to hopefully catch any future
regression on this.

Signed-off-by: Paul Elliott <paul.elliott@arm.com>
2020-12-07 16:47:02 +00:00
Ronald Cron 3f35b87db9
Merge pull request #3938 from geecrypt/mbedtls-2.16
Backport to Mbedtls 2.16:  Support set *_drbg reseed interval before seed
2020-12-07 14:30:13 +01:00
Gilles Peskine 8ed9ac85e5
Merge pull request #3513 from gilles-peskine-arm/ecp-bignum-error-checks-2.16
Backport 2.16: add missing some error checks in ECP and bignum
2020-12-07 13:06:42 +01:00
Janos Follath bcfa41753d
Merge pull request #782 from chris-jones-arm/mbedtls-2.16-restricted
[Backport 2.16] Fix Diffie-Hellman large key size DoS
2020-12-07 09:27:55 +00:00
Gavin Acquroff 77cb30c3cb Support set *_drbg reseed interval before seed
mbedtls_ctr_drbg_set_reseed_interval() and
mbedtls_hmac_drbg_set_reseed_interval() can now be called before
their seed functions and the reseed_interval value will persist.
Previously it would be overwritten with the default value.

*_drbg_reseed_interval is now set in init() and free().

mbedtls_ctr_drbg_free() and mbedtls_hmac_drbg_free() now
reset the drbg context to the state immediately after init().

Tests:
- Added test to check that DRBG reseeds when reseed_counter
reaches reseed_interval, if reseed_interval set before seed
and reseed_interval is less than MBEDTLS_*_DRBG_RESEED_INTERVAL.

Signed-off-by: gacquroff <gavina352@gmail.com>
2020-12-03 13:30:36 -08:00
Chris Jones ad59a2a4a7 Fix potential DoS by limiting number sizes in exponentiation
Check that the exponent and modulus is below `MBEDTLS_MPI_MAX_BITS` before
performing a time expensive operation (modular exponentiation). This prevents
a potential DoS from Diffie-Hellman computations with extremely
large key sizes.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
2020-12-03 16:02:20 +00:00
Paul Elliott 027b601690 Add tag check to cert algorithm check
Add missing tag check for algorithm parameters when comparing the
signature in the description part of the cert against the actual
signature whilst loading a certificate. This was found by a
certificate (created by fuzzing) that openssl would not verify, but
mbedtls would.

Regression test added (one of the client certs modified accordingly)

Signed-off-by: Paul Elliott <paul.elliott@arm.com>
2020-12-02 11:03:58 +00:00
Gilles Peskine e98bbbeb06
Merge pull request #3925 from rodrigo-dc/mbedtls-2.16
Backport 2.16: Fix build failure on gcc-11
2020-12-01 13:26:23 +01:00
Rodrigo Dias Correa d2d0e70276 Move declaration to fix C90 warning
"declaration-after-statement" was generated because that code was
backported from the development branch, which currently uses C99.

Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
2020-11-28 14:59:56 -03:00
Rodrigo Dias Correa 0b9bc0bd77 Change function casting in ssl_calc_finished_tls_sha384
`finish_sha384_t` was made more generic by using `unsigned char*`
instead of `unsigned char[48]` as the second parameter.
This change tries to make the function casting more robust against
future improvements of gcc analysis.

Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
2020-11-28 14:39:08 -03:00
Rodrigo Dias Correa 671600cd44 Fix GCC warning in ssl_calc_finished_tls_sha384
This commit fixes the same warning fixed by baeedbf9, but without
wasting RAM. By casting `mbedtls_sha512_finish_ret()`, `padbuf`
could be kept 48 bytes long without triggering any warnings.

Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
2020-11-28 14:38:30 -03:00
Rodrigo Dias Correa 56ad24cad7 Fix GCC warning in ssl_calc_finished_tls_sha384
GCC 11 generated a warning because `padbuf` was too small to be
used as an argument for `mbedtls_sha512_finish_ret`. The `output`
parameter of `mbedtls_sha512_finish_ret` has the type
`unsigned char[64]`, but `padbuf` was only 48 bytes long.

Even though `ssl_calc_finished_tls_sha384` uses only 48 bytes for
the hash output, the size of `padbuf` was increased to 64 bytes.

Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
2020-11-28 14:27:03 -03:00
Rodrigo Dias Correa 1a0c7fb383 Fix mismatched function parameters (prototype/definition)
In GCC 11, parameters declared as arrays in function prototypes
cannot be declared as pointers in the function definition. The
same is true for the other way around.

The definition of `mbedtls_aes_cmac_prf_128` was changed to match
its public prototype in `cmac.h`. The type `output` was
`unsigned char *`, now is `unsigned char [16]`.

In `ssl_tls.c`, all the `ssl_calc_verify_*` variants now use pointers
for the output `hash` parameter. The array parameters were removed
because those functions must be compatible with the function pointer
`calc_verify` (defined in `ssl_internal.h`).

Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
2020-11-28 12:14:57 -03:00
Rodrigo Dias Correa 0391ea39c1 Fix build failure on gcc-11
Function prototypes changed to use array parameters instead of
pointers.

Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
2020-11-28 09:26:52 -03:00
Gilles Peskine 0525114752 Handle random generator failure in mbedtls_mpi_fill_random()
Discuss the impact in a changelog entry.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-11-25 16:18:39 +01:00
Gilles Peskine debf3ae54e
Merge pull request #3879 from chris-jones-arm/mbedtls-2.16
Backport 2.16: Fix _POSIX_C_SOURCE typos
2020-11-23 23:41:28 +01:00
André Maroneze e78a0c3e14 Fix another use of uinitialized memory in ssl_parse_encrypted_pms
Complement to 0a8352b4: peer_pmslen is not initialized when decryption
fails, so '|= peer_pmslen' may access uninitialized memory, as indicated
by Frama-C/Eva.

Co-authored-by: Gilles Peskine <gilles.peskine@arm.com>
Signed-off-by: André Maroneze <maroneze@users.noreply.github.com>
2020-11-18 13:54:38 +01:00
Gilles Peskine 076c97e29a Simplify conditional guards in error.c
Simplify the guards on MBEDTLS_ERROR_C and MBEDTLS_ERROR_STRERROR_DUMMY.

No longer include superfluous headers and definition: string.h and
platform.h are only needed for MBEDTLS_ERROR_C; time_t is not needed
at all.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-11-16 13:07:51 +01:00
Chris Jones 0a635600ba Fixes two _POSIX_C_SOURCE typos.
Contribution from gufe44.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
2020-11-13 09:17:16 +00:00
Gilles Peskine 23cb65e32f
Merge pull request #3863 from gabor-mezei-arm/3818_bp216_MBEDTLS_AES_SETKEY_DEC_ALT_excludes_MBEDTLS_CIPHER_MODE_XTS
[Backport 2.16] Make the aes xts methods independent from MBEDTLS_AES_SETKEY_DEC_ALT
2020-11-09 20:44:12 +01:00
gabor-mezei-arm 13b8a26730
Make the aes xts methods independent from MBEDTLS_AES_SETKEY_DEC_ALT
This is fix for the MBEDTLS_AES_SETKEY_DEC_ALT macro switch is including the aes xts methods
and building with a custom mbedtls_aes_setkey_dec function will disable the aes xts methods.
The fix is separating the aes xts methods and the MBEDTLS_AES_SETKEY_DEC_ALT can only
switch the presence of the mbedtls_aes_setkey_dec function.

Signed-off-by: gabor-mezei-arm <gabor.mezei@arm.com>
2020-11-06 16:24:19 +01:00
Bence Szépkúti 567bd958ff Do not set IV size for ECB mode ciphers
ECB mode ciphers do not use IVs

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
2020-11-06 15:26:52 +01:00
Gilles Peskine 91ee76380c Put local variables in a struct
This way we can have a single call to mbedtls_platform_zeroize, which
saves a few bytes of code size.

Additionally, on my PC, I notice a significant speed improvement
(x86_64 build with MBEDTLS_AESNI_C disabled, gcc 5.4.0 -O3). I don't
have an explanation for that (I expected no measurable difference).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-10-22 09:21:11 +02:00
Gilles Peskine 02c7b48a70 Remove a useless zeroization
Remove the zeroization of a pointer variable in the AES block
functions. The code was valid but spurious and misleading since it
looked like a mistaken attempt to zeroize the pointed-to buffer.
Reported by Antonio de la Piedra, CEA Leti, France.

Note that we do not zeroize the buffer here because these are the
round keys, and they need to stay until all the blocks are processed.
They will be zeroized in mbedtls_aes_free().

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-10-22 09:20:57 +02:00
Gilles Peskine 90b545881c Fix the build when _GNU_SOURCE is defined to a non-empty value
Fix #3432.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-10-01 17:17:37 +02:00
Peter Kolbus b2aeb75509 Restore retry in rsa_prepare_blinding()
Starting with commit 49e94e3, the do/while loop in
`rsa_prepare_blinding()` was changed to a `do...while(0)`, which
prevents retry from being effective and leaves dead code.

Restore the while condition to retry, and lift the calls to finish the
computation out of the while loop by by observing that they are
performed only when `mbedtls_mpi_inv_mod()` returns zero.

Signed-off-by: Peter Kolbus <peter.kolbus@garmin.com>
2020-09-30 07:22:42 -05:00
Jens Reimann 84d4f022e2 fix return code
Signed-off-by: Jens Reimann <jreimann@redhat.com>
2020-09-22 16:22:27 +02:00
gabor-mezei-arm 70f7f67133
Put local variables and buffers in a struct
This reduces the impact of the code size increase due to the addition
of calls to mbedtls_platform_zeroize.

Signed-off-by: gabor-mezei-arm <gabor.mezei@arm.com>
2020-09-09 15:18:05 +02:00
gabor-mezei-arm f21639fc2f
Zeroize internal buffers and variables in MD hashes
Zeroising of local buffers and variables which are used for calculations in
mbedtls_internal_md*_process() and mbedtls_internal_ripemd160_process()
functions to erase sensitive data from memory.
Checked all function for possible missing zeroisation in MD.

Signed-off-by: gabor-mezei-arm <gabor.mezei@arm.com>
2020-09-09 15:17:11 +02:00
gabor-mezei-arm 5feba8dae1
Force cleanup before return
Signed-off-by: gabor-mezei-arm <gabor.mezei@arm.com>
2020-09-09 15:17:11 +02:00
gabor-mezei-arm d5253bba32
Zeroize internal buffers and variables in PKCS and SHA
Zeroising of local buffers and variables which are used for calculations in
mbedtls_pkcs5_pbkdf2_hmac() and mbedtls_internal_sha*_process() functions
to erase sensitive data from memory.
Checked all function for possible missing zeroisation in PKCS and SHA.

Signed-off-by: gabor-mezei-arm <gabor.mezei@arm.com>
2020-09-09 15:16:01 +02:00