Commit Graph

3206 Commits

Author SHA1 Message Date
Gilles Peskine
03fb3e36e4 mbedtls_rsa_rsaes_pkcs1_v15_decrypt: remove the variable p
Get rid of the variable p. This makes it more apparent where the code
accesses the buffer at an offset whose value is sensitive.

No intended behavior change in this commit.
2018-10-08 11:49:15 +02:00
Gilles Peskine
087544bc98 Minor optimization in the PKCS#1v1.5 unpadding step
Rather than doing the quadratic-time constant-memory-trace on the
whole working buffer, do it on the section of the buffer where the
data to copy has to lie, which can be significantly smaller if the
output buffer is significantly smaller than the working buffer, e.g.
for TLS RSA ciphersuites (48 bytes vs MBEDTLS_MPI_MAX_SIZE).
2018-10-08 11:49:15 +02:00
Gilles Peskine
cf1253e8f0 Use branch-free size comparison for the padding size
In mbedtls_rsa_rsaes_pkcs1_v15_decrypt, use size_greater_than (which
is based on bitwise operations) instead of the < operator to compare
sizes when the values being compared must not leak. Some compilers
compile < to a branch at least under some circumstances (observed with
gcc 5.4 for arm-gnueabi -O9 on a toy program).
2018-10-08 11:49:15 +02:00
Gilles Peskine
a04f8bbd0d Bleichenbacher fix: don't leak the plaintext length (step 2)
Replace memmove(to, to + offset, length) by a functionally equivalent
function that strives to make the same memory access patterns
regardless of the value of length. This fixes an information leak
through timing (especially timing of memory accesses via cache probes)
that leads to a Bleichenbacher-style attack on PKCS#1 v1.5 decryption
using the plaintext length as the observable.
2018-10-08 11:49:15 +02:00
Gilles Peskine
f9dd29e3a8 Bleichenbacher fix: don't leak the plaintext length (step 1)
mbedtls_rsa_rsaes_pkcs1_v15_decrypt takes care not to reveal whether
the padding is valid or not, even through timing or memory access
patterns. This is a defense against an attack published by
Bleichenbacher. The attacker can also obtain the same information by
observing the length of the plaintext. The current implementation
leaks the length of the plaintext through timing and memory access
patterns.

This commit is a first step towards fixing this leak. It reduces the
leak to a single memmove call inside the working buffer.
2018-10-08 11:49:15 +02:00
Gilles Peskine
b473916dcf Evolve choose_int_from_mask to if_int
Make the function more robust by taking an arbitrary zero/nonzero
argument instead of insisting on zero/all-bits-one. Update and fix its
documentation.
2018-10-08 11:49:15 +02:00
Gilles Peskine
dabe87cd71 Fix a timing-based Bleichenbacher attack on PKCS#1v1.5 decryption
mbedtls_rsa_rsaes_pkcs1_v15_decrypt took care of calculating the
padding length without leaking the amount of padding or the validity
of the padding. However it then skipped the copying of the data if the
padding was invalid, which could allow an adversary to find out
whether the padding was valid through precise timing measurements,
especially if for a local attacker who could observe memory access via
cache timings.

Avoid this leak by always copying from the decryption buffer to the
output buffer, even when the padding is invalid. With invalid padding,
copy the same amount of data as what is expected on valid padding: the
minimum valid padding size if this fits in the output buffer,
otherwise the output buffer size. To avoid leaking payload data from
an unsuccessful decryption, zero the decryption buffer before copying
if the padding was invalid.
2018-10-08 11:49:15 +02:00
Gilles Peskine
cd500f3832 Minor readability improvement
Polish the beginning of mbedtls_rsa_rsaes_pkcs1_v15_decrypt a little,
to prepare for some behavior changes.
2018-10-08 11:49:15 +02:00
Simon Butcher
651902d062 Merge remote-tracking branch 'public/pr/1972' into mbedtls-2.7 2018-09-26 22:35:51 +01:00
Simon Butcher
34997fd291 Update library version number to 2.7.6 2018-08-31 16:07:23 +01:00
Simon Butcher
9877efb401 Merge remote-tracking branch 'restricted/pr/437' into mbedtls-2.7-restricted 2018-08-28 15:34:28 +01:00
Simon Butcher
242169bdc3 Merge remote-tracking branch 'restricted/pr/498' into mbedtls-2.7-restricted 2018-08-28 15:29:55 +01:00
Simon Butcher
6910201cd1 Merge remote-tracking branch 'restricted/pr/493' into mbedtls-2.7-restricted 2018-08-28 15:23:39 +01:00
Simon Butcher
4102b3d377 Merge remote-tracking branch 'public/pr/1888' into mbedtls-2.7 2018-08-28 12:25:12 +01:00
Simon Butcher
cc4f58d08c Merge remote-tracking branch 'public/pr/1956' into mbedtls-2.7 2018-08-28 12:16:11 +01:00
Simon Butcher
f7be6b029e Merge remote-tracking branch 'public/pr/1960' into mbedtls-2.7 2018-08-28 11:51:56 +01:00
Hanno Becker
6c794faa46 Fix bug in SSL ticket implementation removing keys of age < 1s
Fixes #1968.
2018-08-22 14:58:31 +01:00
Hanno Becker
12f7ede56e Compute record expansion in steps to ease readability 2018-08-17 15:30:03 +01:00
Hanno Becker
dbd3e88479 Fix mbedtls_ssl_get_record_expansion() for CBC modes
`mbedtls_ssl_get_record_expansion()` is supposed to return the maximum
difference between the size of a protected record and the size of the
encapsulated plaintext.

Previously, it did not correctly estimate the maximum record expansion
in case of CBC ciphersuites in (D)TLS versions 1.1 and higher, in which
case the ciphertext is prefixed by an explicit IV.

This commit fixes this bug. Fixes #1914.
2018-08-17 10:12:21 +01:00
Hanno Becker
78d5d8225e Fix overly strict bounds check in ssl_parse_certificate_request() 2018-08-16 15:53:02 +01:00
Hanno Becker
cd6a64a516 Reset session_in/out pointers in ssl_session_reset_int()
Fixes #1941.
2018-08-14 15:48:36 +01:00
Jaeden Amero
9eb78b4dab Merge remote-tracking branch 'upstream-public/pr/1900' into mbedtls-2.7
Add a Changelog entry
2018-08-10 11:26:15 +01:00
Jaeden Amero
f37a99e3fc Merge remote-tracking branch 'upstream-public/pr/1814' into mbedtls-2.7 2018-08-10 11:01:29 +01:00
Jaeden Amero
3b69174852 Merge remote-tracking branch 'upstream-public/pr/1886' into mbedtls-2.7 2018-08-10 10:50:34 +01:00
k-stachowiak
2c161144e2 Revert change of a return variable name 2018-07-31 17:02:56 +02:00
Ron Eldor
15b0a39322 enforce input and output of ccm selftest on stack
In `mbedtls_ccm_self_test()`, enforce input and output
buffers sent to the ccm API to be contigous and aligned,
by copying the test vectors to buffers on the stack.
2018-07-30 11:43:08 +03:00
Philippe Antoine
84cc74e82b Fix undefined shifts
- in x509_profile_check_pk_alg
- in x509_profile_check_md_alg
- in x509_profile_check_key

and in ssl_cli.c : unsigned char gets promoted to signed integer
2018-07-26 22:49:42 +01:00
Angus Gratton
cb7a5b0b0c Fix memory leak in ecp_mul_comb() if ecp_precompute_comb() fails
In ecp_mul_comb(), if (!p_eq_g && grp->T == NULL) and then ecp_precompute_comb() fails (which can
happen due to OOM), then the new array of points T will be leaked (as it's newly allocated, but
hasn't been asigned to grp->T yet).

Symptom was a memory leak in ECDHE key exchange under low memory conditions.
2018-07-26 11:08:06 +03:00
Jaeden Amero
8385110ae8 Update version to 2.7.5 2018-07-25 15:43:21 +01:00
Simon Butcher
7daacda940 Merge remote-tracking branch 'restricted/pr/494' into mbedtls-2.7 2018-07-24 23:40:53 +01:00
Simon Butcher
d5a3ed36b8 Merge remote-tracking branch 'public/pr/1863' into mbedtls-2.7 2018-07-24 12:57:15 +01:00
k-stachowiak
f4a668870f Fix code formatting 2018-07-24 12:54:39 +02:00
Simon Butcher
b65d6ce83f Merge remote-tracking branch 'public/pr/1870' into mbedtls-2.7 2018-07-24 10:30:11 +01:00
Dawid Drozd
2ba7d8ed2d Remove unnecessary mark as unused #1098 (backport) 2018-07-20 14:08:02 +02:00
Simon Butcher
bc5ec41c01 Merge remote-tracking branch 'public/pr/1847' into mbedtls-2.7 2018-07-19 19:48:25 +01:00
Angus Gratton
8946b0dd30 Check for invalid short Alert messages
(Short Change Cipher Spec & Handshake messages are already checked for.)
2018-07-16 20:12:56 +01:00
Angus Gratton
b91cb6e1e6 TLSv1.2: Treat zero-length fragments as invalid, unless they are application data
TLS v1.2 explicitly disallows other kinds of zero length fragments (earlier standards
don't mention zero-length fragments at all).
2018-07-16 20:12:55 +01:00
Angus Gratton
1ba8e911ec CBC mode: Allow zero-length message fragments (100% padding)
Fixes https://github.com/ARMmbed/mbedtls/issues/1632
2018-07-16 20:12:47 +01:00
k-stachowiak
6978949cd0 Prevent buffer overread by one byte 2018-07-16 12:30:39 +02:00
Manuel Pégourié-Gonnard
7c34432b2d Avoid debug message that might leak length
The length to the debug message could conceivably leak through the time it
takes to print it, and that length would in turn reveal whether padding was
correct or not.
2018-07-12 10:18:37 +02:00
Manuel Pégourié-Gonnard
aeeaaf271c Add counter-measure to cache-based Lucky 13
The basis for the Lucky 13 family of attacks is for an attacker to be able to
distinguish between (long) valid TLS-CBC padding and invalid TLS-CBC padding.
Since our code sets padlen = 0 for invalid padding, the length of the input to
the HMAC function, and the location where we read the MAC, give information
about that.

A local attacker could gain information about that by observing via a
cache attack whether the bytes at the end of the record (at the location of
would-be padding) have been read during MAC verification (computation +
comparison).

Let's make sure they're always read.
2018-07-12 10:18:37 +02:00
Manuel Pégourié-Gonnard
5fcfd0345d Fix Lucky 13 cache attack on MD/SHA padding
The basis for the Lucky 13 family of attacks is for an attacker to be able to
distinguish between (long) valid TLS-CBC padding and invalid TLS-CBC padding.
Since our code sets padlen = 0 for invalid padding, the length of the input to
the HMAC function gives information about that.

Information about this length (modulo the MD/SHA block size) can be deduced
from how much MD/SHA padding (this is distinct from TLS-CBC padding) is used.
If MD/SHA padding is read from a (static) buffer, a local attacker could get
information about how much is used via a cache attack targeting that buffer.

Let's get rid of this buffer. Now the only buffer used is the internal MD/SHA
one, which is always read fully by the process() function.
2018-07-12 10:18:37 +02:00
Simon Butcher
28f68a3d15 Merge remote-tracking branch 'public/pr/1809' into mbedtls-2.7 2018-07-10 14:58:51 +01:00
Simon Butcher
a159d64e86 Merge remote-tracking branch 'public/pr/1827' into mbedtls-2.7 2018-07-10 12:50:16 +01:00
k-stachowiak
c2eddee456 Fix memory leak in ssl_setup 2018-07-09 10:39:20 +02:00
Philippe Antoine
33e5c32a5b Fixes different off by ones 2018-07-09 10:39:02 +02:00
Brendan Shanks
b32233319b x509.c: Remove unused includes
Remove unused includes guarded by MBEDTLS_FS_IO, which doesn't appear
anywhere else in the file.
2018-07-02 12:13:26 +01:00
niisato
a35dbf155c about a issue Replace "new" variable #1782 2018-06-29 11:17:41 +01:00
Ron Eldor
c32b3b73c4 Add ecc extensions only if ecc ciphersuite is used
Fix compliancy to RFC4492. ECC extensions should be included
only if ec ciphersuites are used. Interoperability issue with
bouncy castle. #1157
2018-06-28 15:49:34 +03:00
Ron Eldor
8c02dd1709 Move definition of MBEDTLS_CIPHER_MODE_STREAM
Move definition of `MBEDTLS_CIPHER_MODE_STREAM` to header file
(`mbedtls_cipher_internal.h`), because it is used by more than
one file. Raised by TrinityTonic in #1719
2018-06-28 08:44:47 +03:00