Commit graph

3209 commits

Author SHA1 Message Date
Gilles Peskine 66a28e991d Fix likely-harmless undefined behavior surrounding volatile
The code was making two unsequenced reads from volatile locations.
This is undefined behavior. It was probably harmless because we didn't
care in what order the reads happened and the reads were from ordinary
memory, but UB is UB and IAR8 complained.
2018-10-12 19:15:34 +02:00
Gilles Peskine 23d7ceaca9 PKCS#1 v1.5 decoding: fix empty payload case 2018-10-08 11:49:15 +02:00
Gilles Peskine 0b330f764f Remove a remaining sensitive memory access in PKCS#1 v1.5 decryption 2018-10-08 11:49:15 +02:00
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