Commit Graph

3486 Commits

Author SHA1 Message Date
Gilles Peskine
5b1cb8873d
Merge pull request #3725 from gilles-peskine-arm/ecp-bignum-error-checks-2.7
Backport 2.7: add missing some error checks in ECP and bignum
2020-12-07 13:06:36 +01:00
Rodrigo Dias Correa
9c7e92b5db 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-29 14:45:10 -03:00
Rodrigo Dias Correa
f75fbab19f 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-29 14:44:41 -03:00
Rodrigo Dias Correa
d7853a847d 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-29 14:44:27 -03:00
Rodrigo Dias Correa
34018bef3d 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-29 14:43:45 -03:00
Rodrigo Dias Correa
375366a197 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-29 08:09:58 -03:00
Rodrigo Dias Correa
d103823ba2 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-29 08:09:28 -03:00
André Maroneze
9fc67f0e14 Backport 2.7: Fix use of uinitialized memory in ssl_parse_encrypted_pms
Signed-off-by: André Maroneze <maroneze@users.noreply.github.com>
2020-11-18 14:27:02 +01:00
Gilles Peskine
4159c7154c 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 16:09:41 +01:00
Bence Szépkúti
b4756c2e20 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:40:25 +01:00
Gilles Peskine
acbf9eccb5 Put local variables in a struct
This way we can have a single call to mbedtls_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>
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-10-22 10:34:20 +02:00
Gilles Peskine
5706e920a4 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:43:53 +02:00
Peter Kolbus
e634564381 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:39:15 -05:00
Gilles Peskine
b51c8a29b5 Fix uncaught error if fix_negative fails
fix_negative allocates memory for its result. The calling site didn't
check the return value, so an out-of-memory error could lead to an
incorrect calculation. Fix this.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-09-30 00:22:37 +02:00
Gilles Peskine
91070e43a6 Fix memory leak in mbedtls_mpi_sub_abs
Fix a memory leak in mbedtls_mpi_sub_abs when the output parameter is
aliased to the second operand (X = A - X) and the result is negative.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-09-30 00:22:37 +02:00
Jens Reimann
45a595845d fix return code
Signed-off-by: Jens Reimann <jreimann@redhat.com>
2020-09-22 16:19:25 +02:00
Daniel Otte
1939460417 adjusting size of sliding window array to correct size.
Probably the `W[2 << MBEDTLS_MPI_WINDOW_SIZE]` notation is based on a transcription of 2**MBEDTLS_MPI_WINDOW_SIZE.

Signed-off-by: Daniel Otte <d.otte@wut.de>
2020-09-08 12:24:31 +02:00
Janos Follath
0db765ac65 Bump version to Mbed TLS 2.7.17
Executed "./scripts/bump_version.sh --version 2.7.17"

Signed-off-by: Janos Follath <janos.follath@arm.com>
2020-08-26 15:28:48 +01:00
Janos Follath
93c784b356 Merge branch 'mbedtls-2.7-restricted' 2020-08-26 14:16:29 +01:00
Raoul Strackx
2a8e9587a7 Always revoke certificate on CRL
RFC5280 does not state that the `revocationDate` should be checked.

In addition, when no time source is available (i.e., when MBEDTLS_HAVE_TIME_DATE is not defined), `mbedtls_x509_time_is_past` always returns 0. This results in the CRL not being checked at all.

https://tools.ietf.org/html/rfc5280
Signed-off-by: Raoul Strackx <raoul.strackx@fortanix.com>
2020-08-26 11:38:41 +02:00
Manuel Pégourié-Gonnard
520e78b830 Fix a typo in a comment
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-08-25 12:01:11 +02:00
Manuel Pégourié-Gonnard
426c2d4a38 Add an option to test constant-flow with valgrind
Currently the new component in all.sh fails because
mbedtls_ssl_cf_memcpy_offset() is not actually constant flow - this is on
purpose to be able to verify that the new test works.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-08-25 12:01:09 +02:00
Manuel Pégourié-Gonnard
3b490a0a01 Add mbedtls_ssl_cf_memcpy_offset() with tests
The tests are supposed to be failing now (in all.sh component
test_memsan_constant_flow), but they don't as apparently MemSan doesn't
complain when the src argument of memcpy() is uninitialized, see
https://github.com/google/sanitizers/issues/1296

The next commit will add an option to test constant flow with valgrind, which
will hopefully correctly flag the current non-constant-flow implementation.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-08-25 12:00:07 +02:00
Manuel Pégourié-Gonnard
bf7a49eacc Use temporary buffer to hold the peer's HMAC
This paves the way for a constant-flow implementation of HMAC checking, by
making sure that the comparison happens at a constant address. The missing
step is obviously to copy the HMAC from the secret offset to this temporary
buffer with constant flow, which will be done in the next few commits.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-08-25 12:00:07 +02:00
Manuel Pégourié-Gonnard
d863a67a74 Merge branch 'mbedtls-2.7' into mbedtls-2.7-restricted
* mbedtls-2.7: (28 commits)
  A different approach of signed-to-unsigned comparison
  Update the copy of tests/data_files/server2-sha256.crt in certs.c
  Fix bug in redirection of unit test outputs
  Backport e2k support to mbedtls-2.7
  Don't forget to free G, P, Q, ctr_drbg, and entropy
  Regenerate server2-sha256.crt with a PrintableString issuer
  Regenerate test client certificates with a PrintableString issuer
  cert_write: support all hash algorithms
  compat.sh: stop using allow_sha1
  compat.sh: quit using SHA-1 certificates
  compat.sh: enable CBC-SHA-2 suites for GnuTLS
  Fix license header in pre-commit hook
  Update copyright notices to use Linux Foundation guidance
  Fix building on NetBSD 9.0
  Remove obsolete buildbot reference in compat.sh
  Fix misuse of printf in shell script
  Fix added proxy command when IPv6 is used
  Simplify test syntax
  Fix logic error in setting client port
  ssl-opt.sh: include test name in log files
  ...
2020-08-25 10:59:51 +02:00
Gilles Peskine
84be024eb0
Merge pull request #3594 from gilles-peskine-arm/fix-compat.sh-with-ubuntu-16.04-gnutls-2.7
Backport 2.7: Fix compat.sh with ubuntu 16.04 gnutls 2.7
2020-08-25 10:00:54 +02:00
Gilles Peskine
46b3fc221e
Merge pull request #3599 from makise-homura/mbedtls-2.7
Backport 2.7: Support building on e2k (Elbrus) architecture
2020-08-25 09:46:42 +02:00
makise-homura
329fe7e043 A different approach of signed-to-unsigned comparison
Suggested by @hanno-arm

Signed-off-by: makise-homura <akemi_homura@kurisa.ch>
2020-08-24 18:39:56 +03:00
Gilles Peskine
1323fba357 Update the copy of tests/data_files/server2-sha256.crt in certs.c
Before this commit, certs.c had a copy of a different version of
tests/data_files/server2-sha256.crt (from the then development branch)
which was generated by cert_write. Update certs.c with the new
tests/data_files/server2-sha256.crt which is also generated by
cert_write.

The new copy has the same size as the old copy so there is no concern
about existing application binaries relying on the size. (The old
tests/data_files/server2-sha256.crt had a different size because it
had been generated by openssl and so had slightly different content.)

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-24 15:15:00 +02:00
makise-homura
03c2b8f1c7 Backport e2k support to mbedtls-2.7
Covers commits ac2fd65, 0be6aa9, e74f372, e559550
from `development` branch

Signed-off-by: makise-homura <akemi_homura@kurisa.ch>
2020-08-23 00:28:45 +03:00
Bence Szépkúti
44bfbe3b95 Update copyright notices to use Linux Foundation guidance
As a result, the copyright of contributors other than Arm is now
acknowledged, and the years of publishing are no longer tracked in the
source files.

Also remove the now-redundant lines declaring that the files are part of
MbedTLS.

This commit was generated using the following script:

# ========================
#!/bin/sh

# Find files
find '(' -path './.git' -o -path './3rdparty' ')' -prune -o -type f -print | xargs sed -bi '

# Replace copyright attribution line
s/Copyright.*Arm.*/Copyright The Mbed TLS Contributors/I

# Remove redundant declaration and the preceding line
$!N
/This file is part of Mbed TLS/Id
P
D
'
# ========================

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
2020-08-19 16:54:51 +02:00
Manuel Pégourié-Gonnard
30c1df3f84
Merge pull request #3570 from gufe44/net-sockets-fixes-2.7
[Backport 2.7] NetBSD 9.0 build fixes
2020-08-18 09:13:52 +02:00
gufe44
3ca3b9ea88 Fix building on NetBSD 9.0
Fixes #2310

Signed-off-by: gufe44 <gu981@protonmail.com>
2020-08-17 07:14:16 +02:00
Gilles Peskine
126b69aee5
Merge pull request #735 from gilles-peskine-arm/x509parse_crl-empty_entry-2.7
Backport 2.7: Fix buffer overflow in x509_get_entries (oss-fuzz 24123)
2020-08-14 23:22:19 +02:00
gufe44
206cb39116 Use arc4random_buf instead of rand on NetBSD
Avoid old implementation of rand returning numbers with cyclical lower bits. Allow tests to pass.

Signed-off-by: gufe44 <gu981@protonmail.com>
2020-08-13 06:22:45 +02:00
Gilles Peskine
691bed7cce
Merge pull request #733 from gabor-mezei-arm/689_bp27_zeroising_of_plaintext_buffers
[Backport 2.7] Zeroising of plaintext buffers in mbedtls_ssl_read()
2020-08-12 18:51:47 +02:00
Gilles Peskine
78e54b9b1d x509_crl_parse: fix 1-byte buffer overflow and entry->raw.tag
In the entries (mbedtls_x509_crl_entry values) on the list constructed
by mbedtls_x509_crl_parse_der(), set entry->raw.tag to
(SEQUENCE | CONSTRUCTED) rather than to the tag of the first ASN.1
element of the entry (which happens to be the tag of the serial
number, so INTEGER or INTEGER | CONTEXT_SPECIFIC). This is doesn't
really matter in practice (and in particular the value is never used
in Mbed TLS itself), and isn't documented, but at least it's
consistent with how mbedtls_x509_buf is normally used.

The primary importance of this change is that the old code tried to
access the tag of the first element of the entry even when the entry
happened to be empty. If the entry was empty and not followed by
anything else in the CRL, this could cause a read 1 byte after the end
of the buffer containing the CRL.

The test case "X509 CRL ASN1 (TBSCertList, single empty entry at end)"
hit the problematic buffer overflow, which is detected with ASan.

Credit to OSS-Fuzz for detecting the problem.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-12 12:51:43 +02:00
Manuel Pégourié-Gonnard
cd542a5453
Merge pull request #729 from mpg/ct-varlen-hmac-2.7
[Backport 2.7] Add constant-flow variable-length HMAC function
2020-08-10 12:40:53 +02:00
gabor-mezei-arm
ef73875913
Zeroising of plaintext buffers to erase unused application data from memory
Signed-off-by: gabor-mezei-arm <gabor.mezei@arm.com>
2020-08-03 10:53:48 +02:00
Manuel Pégourié-Gonnard
7cf5ebc90f Add comment that was lost while backporting
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-29 13:01:05 +02:00
Manuel Pégourié-Gonnard
e05e57619b Remove use of C99 construct
This is an LTS branch, C99 isn't allowed yet, it breaks versions of MSVC that
we still support for this branch.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-29 10:06:39 +02:00
Manuel Pégourié-Gonnard
2f484bd979 Add missing const for consistency
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-28 13:03:29 +02:00
Manuel Pégourié-Gonnard
2da9a54559 Fix typos in comments
Co-authored-by: Janos Follath <janos.follath@arm.com>
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-28 13:03:29 +02:00
Manuel Pégourié-Gonnard
0cd0c731fd Check errors from the MD layer
Could be out-of-memory for some functions, accelerator issues for others.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-28 13:03:28 +02:00
Manuel Pégourié-Gonnard
c9ef5a2b76 Remove unnecessary cast
This is C, not C++, casts between void * and other pointer types are free.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-28 13:03:28 +02:00
Manuel Pégourié-Gonnard
ec956b1861 Improve some comments and internal documentation
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-28 13:03:28 +02:00
Manuel Pégourié-Gonnard
41df0f2bca Factor repeated condition to its own macro
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-28 13:03:28 +02:00
Manuel Pégourié-Gonnard
4508c67c42 Implement cf_hmac() actually with constant flow
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-28 13:03:28 +02:00
Manuel Pégourié-Gonnard
40597cef01 Add MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN
This option allows to test the constant-flow nature of selected code, using
MemSan and the fundamental observation behind ctgrind that the set of
operations allowed on undefined memory by dynamic analysers is the same as the
set of operations allowed on secret data to avoid leaking it to a local
attacker via side channels, namely, any operation except branching and
dereferencing.

(This isn't the full story, as on some CPUs some instructions have variable
execution depending on the inputs, most notably division and on some cores
multiplication. However, testing that no branch or memory access depends on
secret data is already a good start.)

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-28 13:03:28 +02:00
Manuel Pégourié-Gonnard
d11971875a Use existing implementation of cf_hmac()
Just move code from ssl_decrypt_buf() to the new cf_hmac() function and then
call cf_hmac() from there.

This makes the new cf_hmac() function used and validates that its interface
works for using it in ssl_decrypt_buf().

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-28 13:03:28 +02:00