Commit Graph

8093 Commits

Author SHA1 Message Date
Andrzej Kurek
748face36f ssl_tls: fix maximum output length
set maximum output length to MBEDTLS_SSL_OUT_CONTENT_LEN instead of
MBEDTLS_SSL_MAX_CONTENT_LEN.
2018-10-11 07:20:19 -04:00
Andrzej Kurek
7c6df832ed ssl-opt.sh: revert unnecessary "autoreduction" checks
return to the initial check for "resend"
2018-10-11 07:06:12 -04:00
Andrzej Kurek
7311c78074 ssl-opt.sh: force ciphersuites to reduce mtu size 2018-10-11 06:49:41 -04:00
Hanno Becker
780f0a4cc1 Reinitialize PK ctx in mbedtls_pk_parse_key before reuse are free
Context: This commit makes a change to mbedtls_pk_parse_key() which
is responsible for parsing of private keys. The function doesn't know
the key format in advance (PEM vs. DER, encrypted vs. unencrypted) and
tries them one by one, resetting the PK context in between.

Issue: The previous code resets the PK context through a call to
mbedtls_pk_free() along, lacking the accompanying mbedtls_pk_init()
call. Practically, this is not an issue because functionally
mbedtls_pk_free() + mbedtls_pk_init() is equivalent to mbedtls_pk_free()
with the current implementation of these functions, but strictly
speaking it's nonetheless a violation of the API semantics according
to which xxx_free() functions leave a context in uninitialized state.
(yet not entirely random, because xxx_free() functions must be idempotent,
so they cannot just fill the context they operate on with garbage).

Change: The commit adds calls to mbedtls_pk_init() after those calls
to mbedtls_pk_free() within mbedtls_pk_parse_key() after which the
PK context might still be used.
2018-10-11 11:31:15 +01:00
Hanno Becker
cfa2e33fd2 Omit runtime configuration of calloc/free if macro config enabled
This commit removes the definition of the API function

`mbedtls_platform_set_calloc_free()`

from `library/platform.c` in case the macros

`MBEDTLS_PLATFORM_CALLOC_MACRO`
`MBEDTLS_PLATFORM_FREE_MACRO`

for compile time configuration of calloc/free are set.

This is in line with the corresponding header `mbedtls/platform.h`
which declares `mbedtls_platform_set_calloc_free()` only if
`MBEDTLS_PLATFORM_{CALLOC/FREE}_MACRO` are not defined.

Fixes #1642.
2018-10-11 11:04:20 +01:00
Hanno Becker
e5fecec7bf Add test for MBEDTLS_PLATFORM_{CALLOC/FREE}_MACRO to all.sh
This commit adds a test to tests/scripts/all.sh exercising an
ASan build of the default configuration with

MBEDTLS_PLATFORM_MEMORY enabled,
MBEDTLS_PLATFORM_CALLOC_MACRO set to std calloc
MBEDTLS_PLATFORM_FREE_MACRO   set to std free

(This should functionally be indistinguishable from a default build)
2018-10-11 11:04:20 +01:00
Hanno Becker
8df10232cf Add explicit unsigned-to-signed integer conversion
The previous code triggered a compiler warning because of a comparison
of a signed and an unsigned integer.

The conversion is safe because `len` is representable by 16-bits,
hence smaller than the maximum integer.
2018-10-10 15:48:39 +01:00
Darryl Green
ac2ead0e68 Improve deterministic test for prime testing
Extend the mbedtls_mpi_is_prime_det test to check that it reports
the number as prime when testing rounds-1 rounds, then reports the
number as composite when testing the full number of rounds.
2018-10-09 16:36:53 +01:00
Darryl Green
e3f95ed25b Fix bias in random number generation in Miller-Rabin test
When a random number is generated for the Miller-Rabin primality test,
if the bit length of the random number is larger than the number being
tested, the random number is shifted right to have the same bit length.
This introduces bias, as the random number is now guaranteed to be
larger than 2^(bit length-1).

Changing this to instead zero all bits higher than the tested numbers
bit length will remove this bias and keep the random number being
uniformly generated.
2018-10-09 16:36:53 +01:00
Janos Follath
e0e7ddf99e Changelog: Add entry for prime validation fix 2018-10-09 16:36:53 +01:00
Janos Follath
a0b67c2f3e Bignum: Deprecate mbedtls_mpi_is_prime()
When using a primality testing function the tolerable error rate depends
on the scheme in question, the required security strength and wether it
is used for key generation or parameter validation. To support all use
cases we need more flexibility than what the old API provides.
2018-10-09 16:36:53 +01:00
Janos Follath
da31fa137a Bignum: Fix prime validation vulnerability
The input distribution to primality testing functions is completely
different when used for generating primes and when for validating
primes. The constants used in the library are geared towards the prime
generation use case and are weak when used for validation. (Maliciously
constructed composite numbers can pass the test with high probability)

The mbedtls_mpi_is_prime() function is in the public API and although it
is not documented, it is reasonable to assume that the primary use case
is validating primes. The RSA module too uses it for validating key
material.
2018-10-09 16:36:53 +01:00
Janos Follath
64eca05ec2 Bignum: Add tests for primality testing
Primality tests have to deal with different distribution when generating
primes and when validating primes.
These new tests are testing if mbedtls_mpi_is_prime() is working
properly in the latter setting.

The new tests involve pseudoprimes with maximum number of
non-witnesses. The non-witnesses were generated by printing them
from mpi_miller_rabin(). The pseudoprimes were generated by the
following function:

void gen_monier( mbedtls_mpi* res, int nbits )
{
    mbedtls_mpi p_2x_plus_1, p_4x_plus_1, x, tmp;

    mbedtls_mpi_init( &p_2x_plus_1 );
    mbedtls_mpi_init( &p_4x_plus_1 );
    mbedtls_mpi_init( &x ); mbedtls_mpi_init( &tmp );

    do
    {
        mbedtls_mpi_gen_prime( &p_2x_plus_1, nbits >> 1, 0,
                               rnd_std_rand, NULL );
        mbedtls_mpi_sub_int( &x, &p_2x_plus_1, 1 );
        mbedtls_mpi_div_int( &x, &tmp, &x, 2 );

        if( mbedtls_mpi_get_bit( &x, 0 ) == 0 )
            continue;

        mbedtls_mpi_mul_int( &p_4x_plus_1, &x, 4 );
        mbedtls_mpi_add_int( &p_4x_plus_1, &p_4x_plus_1, 1 );

        if( mbedtls_mpi_is_prime( &p_4x_plus_1, rnd_std_rand,
                                  NULL ) == 0 )
            break;

    } while( 1 );

    mbedtls_mpi_mul_mpi( res, &p_2x_plus_1, &p_4x_plus_1 );
}
2018-10-09 16:36:53 +01:00
Janos Follath
b728c29114 Bignum: Remove dead code
Both variables affected by the code are overwritten before their next
read.
2018-10-09 16:33:27 +01:00
Janos Follath
3332937538 Changelog: Add entry for prime test improvement 2018-10-09 16:33:27 +01:00
Janos Follath
b8fc1b02ee RSA: Use MBEDTLS_MPI_GEN_PRIME_FLAG_LOW_ERR 2018-10-09 16:33:27 +01:00
Janos Follath
a3cb7eb8ad Bignum: Add test for improved prime generation 2018-10-09 16:33:27 +01:00
Janos Follath
f301d23ceb Bignum: Improve primality test for FIPS primes
The FIPS 186-4 RSA key generation prescribes lower failure probability
in primality testing and this makes key generation slower. We enable the
caller to decide between compliance/security and performance.

This python script calculates the base two logarithm of the formulas in
HAC Fact 4.48 and was used to determine the breakpoints and number of
rounds:

def mrpkt_log_2(k, t):
    if t <= k/9.0:
        return 3*math.log(k,2)/2+t-math.log(t,2)/2+4-2*math.sqrt(t*k)
    elif t <= k/4.0:
        c1 = math.log(7.0*k/20,2)-5*t
        c2 = math.log(1/7.0,2)+15*math.log(k,2)/4.0-k/2.0-2*t
        c3 = math.log(12*k,2)-k/4.0-3*t
        return max(c1, c2, c3)
    else:
        return math.log(1/7.0)+15*math.log(k,2)/4.0-k/2.0-2*t
2018-10-09 16:33:27 +01:00
Andrzej Kurek
35f2f300ca ssl-opt.sh: change expected pattern from 'resend' to 'autoreduction' 2018-10-09 08:52:14 -04:00
Andrzej Kurek
ef43ce6e25 Dtls: change the way unlimited mtu is set for client hello messages 2018-10-09 08:24:12 -04:00
Hanno Becker
abe6003f5a Adapt ChangeLog 2018-10-09 12:44:47 +01:00
Hanno Becker
095d9cf58e Fix ordering of free()ing of internal structures in ssl_server2
If `MBEDTLS_MEMORY_BUFFER_ALLOC_C` is configured and Mbed TLS'
custom buffer allocator is used for calloc() and free(), the
read buffer used by the server example application is allocated
from the buffer allocator, but freed after the buffer allocator
has been destroyed. If memory backtracing is enabled, this leaves
a memory leak in the backtracing structure allocated for the buffer,
as found by valgrind.

Fixes #2069.
2018-10-09 12:44:42 +01:00
Andrzej Kurek
0fc9cf40cf ssl-opt.sh: increase proxy mtu sizes to contain entire datagrams 2018-10-09 03:09:41 -04:00
Hanno Becker
1624e2e8bb Avoid overly long lines X.509 DN attr array def in x509_create.c 2018-10-08 14:52:20 +01:00
Hanno Becker
35b6854e54 Replace reference to RFC 3280 by reference to newer RFC 5280 2018-10-08 14:47:38 +01:00
Hanno Becker
cfc47bab69 Correct some indentation and line lengths in x509_create.c 2018-10-08 14:45:42 +01:00
Hanno Becker
d355e69aed Rename tag to default_tag in x509_attr_descriptor_t 2018-10-08 14:44:32 +01:00
Hanno Becker
d0e21fbd27 Improve documentation of ASN.1 string-writing functions
- mbedtls_asn1_write_tagged_string()
- mbedtls_asn1_write_printable_string()
- mbedtls_asn1_write_utf8_string()
- mbedtls_asn1_write_ia5_string()
2018-10-08 14:44:28 +01:00
Hanno Becker
d2c9009e5a Improve documentation of x509_attr_descriptor_t 2018-10-08 14:42:34 +01:00
Hanno Becker
dc71ef8fcc Adapt ChangeLog 2018-10-08 13:51:38 +01:00
Hanno Becker
0c161d1956 Fix bounds check in ssl_parse_server_psk_hint()
In the previous bounds check `(*p) > end - len`, the computation
of `end - len` might underflow if `end` is within the first 64KB
of the address space (note that the length `len` is controlled by
the peer). In this case, the bounds check will be bypassed, leading
to `*p` exceed the message bounds by up to 64KB when leaving
`ssl_parse_server_psk_hint()`. In a pure PSK-based handshake,
this doesn't seem to have any consequences, as `*p*` is not accessed
afterwards. In a PSK-(EC)DHE handshake, however, `*p` is read from
in `ssl_parse_server_ecdh_params()` and `ssl_parse_server_dh_params()`
which might lead to an application crash of information leakage.
2018-10-08 13:40:50 +01:00
Gilles Peskine
695a34654a Add tests for PKCS#1 v1.5 decoding
Functional tests for various payload sizes and output buffer sizes.

When the padding is bad or the plaintext is too large for the output
buffer, verify that function writes some outputs. This doesn't
validate that the implementation is time-constant, but it at least
validates that it doesn't just return early without outputting anything.
2018-10-08 11:13:21 +02:00
Andrzej Kurek
948fe80f42 ssl-opt.sh: adjust test timeouts to fit slower targets 2018-10-06 05:07:47 -04:00
Gilles Peskine
ec2a5fdee1 PKCS#1 v1.5 decoding: fix empty payload case 2018-10-05 20:48:20 +02:00
Gilles Peskine
c5ccd7a1e7 Indicate the memory access variations in the changelog entry 2018-10-05 15:42:52 +02:00
Gilles Peskine
40b57f4acd Remove a remaining sensitive memory access in PKCS#1 v1.5 decryption 2018-10-05 15:35:03 +02:00
Gilles Peskine
85a7442753 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-05 14:50:21 +02:00
Andrzej Kurek
6290dae909 Disable dtls fragmentation for ClientHello messages
Set the handshake mtu to unlimited when encountering a ClienHello message and
reset it to its previous value after writing the record.
2018-10-05 08:06:01 -04:00
Andrzej Kurek
52f8491dc2 ssl-opt.sh: adjust tests to fit slower targets
Adjust mtu sizes to be able to pass tests using a full configuration
2018-10-05 07:53:40 -04:00
Hanno Becker
617a321ed9 Adapt ChangeLog 2018-10-05 09:52:59 +01:00
Hanno Becker
30a95102b1 Fix memory leak and freeing without initialization in cert_write
* The variables `csr` and `issuer_crt` are initialized but not freed.
* The variable `entropy` is unconditionally freed in the cleanup section
  but there's a conditional jump to that section before its initialization.
  This cmmot Moves it to the other initializations happening before the
  first conditional jump to the cleanup section.

Fixes #1422.
2018-10-05 09:52:31 +01:00
Gilles Peskine
eeedabe25e 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-04 22:45:13 +02:00
Gilles Peskine
8c9440a2cb 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-04 21:44:40 +02:00
Gilles Peskine
a1af5c8ea2 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-04 21:44:29 +02:00
Gilles Peskine
9265ff4ee6 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-04 21:38:22 +02:00
Gilles Peskine
331d80e162 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-04 21:36:34 +02:00
Aurelien Jarno
16b1bd8932 bn_mul.h: add ARM DSP optimized MULADDC code
The Cortex M4, M7 MCUs and the Cortex A CPUs support the ARM DSP
instructions, and especially the umaal instruction which greatly
speed up MULADDC code. In addition the patch switched the ASM
constraints to registers instead of memory, giving the opportunity
for the compiler to load them the best way.

The speed improvement is variable depending on the crypto operation
and the CPU. Here are the results on a Cortex M4, a Cortex M7 and a
Cortex A8. All tests have been done with GCC 6.3 using -O2. RSA uses a
RSA-4096 key. ECDSA uses a secp256r1 curve EC key pair.

                 +--------+--------+--------+
                 |   M4   |   M7   |   A8   |
+----------------+--------+--------+--------+
| ECDSA signing  |  +6.3% |  +7.9% |  +4.1% |
+----------------+--------+--------+--------+
| RSA signing    | +43.7% | +68.3% | +26.3% |
+----------------+--------+--------+--------+
| RSA encryption |  +3.4% |  +9.7% |  +3.6% |
+----------------+--------+--------+--------+
| RSA decryption | +43.0% | +67.8% | +22.8% |
+----------------+--------+--------+--------+

I ran the whole testsuite on the Cortex A8 Linux environment, and it
all passes.
2018-10-04 16:09:27 +02:00
Simon Butcher
d2642584cb Make inclusion of stdio.h conditional in x509_crt.c
stdio.h was being included both conditionally if MBEDTLS_FS_IO was
defined, and also unconditionally, which made at least one of them
redundant.

This change removes the unconditional inclusion of stdio.h and makes it
conditional on MBEDTLS_PLATFORM_C.
2018-10-03 15:11:19 +01:00
Gilles Peskine
ddffa06501 Add ChangeLog entry 2018-10-03 13:40:16 +02:00
Gilles Peskine
e2a10de275 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-03 01:03:05 +02:00