Commit graph

5029 commits

Author SHA1 Message Date
Andrzej Kurek ca60937cf9
Add buffer and context clearing upon suspected FI
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
2020-07-08 09:25:49 -04:00
Andrzej Kurek 0919b142b6
Formatting changes
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
2020-07-06 15:28:59 -04:00
Andrzej Kurek 84bde419e1
Add FI countermeasures to the ssl module
This commit adds mainly buffer pointer and length duplication and checks,
but also some hamming distance and return values checking improvements.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
2020-07-06 15:27:34 -04:00
Andrzej Kurek 74f7d0f03d
Duplicate sensitive buffer and buffer length information
Detect FI attacks on buffer pointers and buffer lengths.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
2020-07-06 14:28:12 -04:00
Andrzej Kurek 98c847a483
Merge pull request #3395 from AndrzejKurek/sha-flow_ctrl
Add flow control to sha256
2020-06-22 11:30:53 +01:00
Andrzej Kurek e5425a0944
Merge pull request #3408 from AndrzejKurek/hamming-distance-improvements
Hamming distance improvements
2020-06-22 08:28:55 +01:00
Piotr Nowicki f523c47578
Merge pull request #3403 from piotr-now/sca_memmove
Add mbedtls_platform_memmove() as a secured memcmp()
2020-06-10 14:52:02 +02:00
Piotr Nowicki ce0aab4474 Add new error code PLATFORM_ALLOC_FAILED for mbedtls_platform_memmove()
Signed-off-by: Piotr Nowicki <piotr.nowicki@arm.com>
2020-06-10 13:51:32 +02:00
Andrzej Kurek 78f77eb4e6
Add flow control to sha256
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
2020-06-10 05:12:34 -04:00
Piotr Nowicki 5d5841f450 Add mbedtls_platform_memmove() as a secured memcmp()
Signed-off-by: Piotr Nowicki <piotr.nowicki@arm.com>
2020-06-09 14:31:55 +02:00
Piotr Nowicki e048b91d25 Add returning a FAULT_DETECTED error on suspected FI attacks
The change applies to the places where we prevent double synchronous
FI attacks with random delay, and where we do not respond to their
detection. The response to such an attack should be to return the
appropriate error code.

Signed-off-by: Piotr Nowicki <piotr.nowicki@arm.com>
2020-06-09 12:00:42 +02:00
Andrzej Kurek 8f52a8a8c0
Improve the Hamming distance of ssl_hs_is_proper_fragment return values
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
2020-06-08 11:02:22 -04:00
Piotr Nowicki b06ec05dc0 Add comment for mbedtls_platform_random_delay()
Signed-off-by: Piotr Nowicki <piotr.nowicki@arm.com>
2020-06-08 14:21:38 +02:00
Andrzej Kurek 478b05c34c
Merge pull request #3355 from AndrzejKurek/fi_error_codes
Change the default value of status variables to an error
2020-06-08 08:57:33 +01:00
Andrzej Kurek fd56f409b3
Change the default value of status variables to an error
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
2020-06-05 12:26:07 -04:00
Piotr Nowicki e071e42480
Merge pull request #3336 from piotr-now/baremetal_flowmon
Increasing resistance to fault injection attacks related with memory operations.
2020-06-01 08:09:26 +02:00
Piotr Nowicki f0ab6d62ac Added some descriptions of functions
Signed-off-by: Piotr Nowicki <piotr.nowicki@arm.com>
2020-05-27 15:35:44 +02:00
Manuel Pégourié-Gonnard 13bebd0edb
Keep SSL context const when hw accel is disabled
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-05-21 10:13:26 -04:00
Manuel Pégourié-Gonnard 731d7c0dcc
Fix lack of cookie check on hard reconnect
Section 4.2.8 of RFC 6347 describes how to handle the case of a DTLS client
establishing a new connection using the same UDP quartet as an already active
connection, which we implement under the compile option
MBEDTLS_SSL_DLTS_CLIENT_PORT_REUSE. Relevant excerpts:

    [the server] MUST NOT destroy the existing
    association until the client has demonstrated reachability either by
    completing a cookie exchange or by completing a complete handshake
    including delivering a verifiable Finished message.
    [...]
    The reachability requirement prevents
    off-path/blind attackers from destroying associations merely by
    sending forged ClientHellos.

Our code chooses to use a cookie exchange for establishing reachability, but
unfortunately that check was effectively removed in a recent refactoring,
which changed what value ssl_handle_possible_reconnect() needs to return in
order for ssl_get_next_record() (introduced in that refactoring) to take the
proper action. Unfortunately, in addition to changing the value, the
refactoring also changed a return statement to an assignment to the ret
variable, causing the function to reach the code for a valid cookie, which
immediately destroys the existing association, effectively bypassing the
cookie verification.

This commit fixes that by immediately returning after sending a
HelloVerifyRequest when a ClientHello without a valid cookie is found. It also
updates the description of the function to reflect the new return value
convention (the refactoring updated the code but not the documentation).

The commit that changed the return value convention (and introduced the bug)
is 2fddd3765e, whose commit message explains the
change.

Note: this bug also indirectly caused the ssl-opt.sh test case "DTLS client
reconnect from same port: reconnect" to occasionally fail due to a race
condition between the reception of the ClientHello carrying a valid cookie and
the closure of the connection by the server after noticing the ClientHello
didn't carry a valid cookie after it incorrectly destroyed the previous
connection, that could cause that ClientHello to be invisible to the server
(if that message reaches the server just before it does `net_close()`). A
welcome side effect of this commit is to remove that race condition, as the
new connection will immediately start with a ClientHello carrying a valid
cookie in the SSL input buffer, so the server will not call `net_close()` and
not risk discarding a better ClientHello that arrived in the meantime.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-05-21 10:12:25 -04:00
Piotr Nowicki 4aaa34c03f Add flow monitor protection to mbedtls_platform_memcmp()
Signed-off-by: Piotr Nowicki <piotr.nowicki@arm.com>
2020-05-20 16:50:24 +02:00
Andrzej Kurek 825ebd483f
Merge mbedtls 2.16.6 into baremetal
Conflicts:
mbedtls.doxyfile - PROJECT_NAME - mbed TLS v2.16.6 chosen.
doc_mainpage.h - mbed TLS v2.16.6 version chosen.
hmac_drbg.h - line 260, extended description chosen.
            - line 313, extended description chosen.
            - line 338, extended description chosen.
version.h - 2.16.6 chosen.
CMakeLists.txt - 2.16.6 chosen.
test_suite_version.data - 2.16.6 chosen.
Makefile - 141 - manual correction - baremetal version of C_SOURCE_FILES
                 with variables for directories plus 2.16.6 CTAGS addition.
pkparse.c - lines 846 onwards - the asn1_get_nonzero_mpi implementation chosen.
ssl_tls.c - line 5269 - edited manually, left the ret=0, because baremetal has
            a different behaviour since commit 87b5626, but added a debug
            message that's new in 2.16.6.    
all.sh:
- component_build_deprecated - chosen the refactored version from 2.16.6,
                               but with extra flags from baremetal.
- rest of the _no_xxx tests - merged make options to have PTHREAD=1 and
                              other changes from 2.16.6 (like -O1 instead of -O0).
- component_build_arm_none_eabi_gcc_no_64bit_multiplication - added 
                              TINYCRYPT_BUILD=0 to the 2.16.6 version of make.

x509/req_app.c - left baremetal log but with mbedtls_exit( 0 ) call.
x509/crl_app.c - left baremetal log but with mbedtls_exit( 0 ) call.
x509/cert_app.c - left baremetal log but with mbedtls_exit( 0 ) call.
ssl/ssl_mail_client.c - left baremetal log but with mbedtls_exit( 0 ) call.
ssl/ssl_pthread_server.c - left baremetal log but with mbedtls_exit( 0 ) call.
ssl/ssl_fork_server.c - left baremetal log but with mbedtls_exit( 0 ) call.
ssl_client1.c - line 54 - left baremetal log but with mbedtls_exit( 0 ) call.
ssl_client2.c - line 54 - left baremetal log but with mbedtls_exit( 0 ) call.
              - line 132 - new options of both branches added.
              - skip close notify handled as in 2.16.6, but with `ssl` instead of `&ssl`.
              - Merged the 2.16.6 usage split with additional baremetal usages.
              - Merged options from baremetal and 2.16.6.
ssl_server.c - left baremetal log but with mbedtls_exit( 0 ) call.
ssl_server2.c - Merged the 2.16.6 usage split with additional baremetal usages.
config.pl - fixed missing defines from the documentation, removed duplicates,
            and reorganised so that the documentation and excluded list
            are ordered in the same way.
test_suite_x509parse.data - only added the two new pathlen tests.
x509_crt.c - change the return code by removing
             MBEDTLS_ERR_X509_INVALID_EXTENSIONS, since it's added by
             x509_crt_frame_parse_ext not by an "or", but by "+=".
Changelog - Assigned all entries to appropriate sections.
ssl-opt.sh - line 8263 - merged options.
           - removed lines 1165 - 1176 - there was a duplicate test, probably
             an artifact of previous merges.
check-files.py - sticked to old formatting.

Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
2020-05-18 11:47:25 -04:00
Janos Follath f3a13486f4 Revert "Merge pull request #3012 from Patater/dev/jp-bennett/development-2.16"
This reverts commit 7550e857bf, reversing
changes made to d0c2575324.

stat() will never return S_IFLNK as the file type, as stat()
explicitly follows symlinks.

Fixes #3005.
2020-03-13 17:08:43 +00:00
Gilles Peskine 8830bd2447 Minor comment improvement 2020-03-13 16:23:45 +00:00
Gilles Peskine 0660747057 Improve comments in mpi_shrink 2020-03-13 16:23:45 +00:00
Gilles Peskine 51c2e06eb8 mpi_copy: make the 0 case slightly more robust
If Y was constructed through functions in this module, then Y->n == 0
iff Y->p == NULL. However we do not prevent filling mpi structures
manually, and zero may be represented with n=0 and p a valid pointer.
Most of the code can cope with such a representation, but for the
source of mbedtls_mpi_copy, this would cause an integer underflow.
Changing the test for zero from Y->p==NULL to Y->n==0 causes this case
to work at no extra cost.
2020-03-13 16:23:45 +00:00
Jack Lloyd 32b6e6984d Parse RSA parameters DP, DQ and QP from PKCS1 private keys
Otherwise these values are recomputed in mbedtls_rsa_deduce_crt, which
currently suffers from side channel issues in the computation of QP
(see https://eprint.iacr.org/2020/055). By loading the pre-computed
values not only is the side channel avoided, but runtime overhead of
loading RSA keys is reduced.

Discussion in https://github.com/ARMmbed/mbed-crypto/issues/347

Backport of https://github.com/ARMmbed/mbed-crypto/pull/352
2020-03-13 16:21:44 +00:00
Jonathan Bennett b9082ed820 Allow loading symlinked certificates
When mbedtls_x509_crt_parse_path() checks each object in the supplied path, it only processes regular files. This change makes it also accept a symlink to a file. Fixes #3005.

This was observed to be a problem on Fedora/CentOS/RHEL systems, where the ca-bundle in the default location is actually a symlink.
2020-03-13 15:37:54 +00:00
Gilles Peskine 010efeb5a2 Remove redundant block_size validity check
Check the value only once, as soon as we've obtained it.
2020-03-13 15:37:54 +00:00
Manuel Pégourié-Gonnard aa377cf111 Fix incrementing pointer instead of value
This was introduced by a hasty search-and-replace that didn't account for C's
operator precedence when changing those variables to pointer types.
2020-03-13 15:37:54 +00:00
Gilles Peskine 140f50206e Add missing return code check on call to mbedtls_md() 2020-03-13 15:36:05 +00:00
Janos Follath e7b49d3cd1 Bump version to Mbed TLS 2.16.4 2020-03-13 15:36:05 +00:00
Gilles Peskine 8b7f03f172 Catch AES failure in mbedtls_ctr_drbg_random
The functions mbedtls_ctr_drbg_random() and
mbedtls_ctr_drbg_random_with_add() could return 0 if an AES function
failed. This could only happen with alternative AES
implementations (the built-in implementation of the AES functions
involved never fail), typically due to a failure in a hardware
accelerator.

Bug reported and fix proposed by Johan Uppman Bruce and Christoffer
Lauri, Sectra.
2020-03-13 15:36:04 +00:00
Andrzej Kurek a8405447aa Zeroize local AES variables before exiting the function
This issue has been reported by Tuba Yavuz, Farhaan Fowze, Ken (Yihang) Bai,
Grant Hernandez, and Kevin Butler (University of Florida) and
Dave Tian (Purdue University).

In AES encrypt and decrypt some variables were left on the stack. The value
of these variables can be used to recover the last round key. To follow best
practice and to limit the impact of buffer overread vulnerabilities (like
Heartbleed) we need to zeroize them before exiting the function.
2020-03-13 15:27:12 +00:00
Janos Follath e9db2aa5b4 mpi_lt_mpi_ct: fix condition handling
The code previously only set the done flag if the return value was one.
This led to overriding the correct return value later on.
2020-03-13 15:25:40 +00:00
Janos Follath 3d2b769d1c ct_lt_mpi_uint: cast the return value explicitely
The return value is always either one or zero and therefore there is no
risk of losing precision. Some compilers can't deduce this and complain.
2020-03-13 15:25:40 +00:00
Janos Follath c8256e7020 mbedtls_mpi_lt_mpi_ct: simplify condition
In the case of *ret we might need to preserve a 0 value throughout the
loop and therefore we need an extra condition to protect it from being
overwritten.

The value of done is always 1 after *ret has been set and does not need
to be protected from overwriting. Therefore in this case the extra
condition can be removed.
2020-03-13 15:25:40 +00:00
Janos Follath ec4c42a95f Rename variable for better readability 2020-03-13 15:25:40 +00:00
Janos Follath cf7eeef2cc mbedtls_mpi_lt_mpi_ct: Improve documentation 2020-03-13 15:25:40 +00:00
Janos Follath aa9e7a4717 Make mbedtls_mpi_lt_mpi_ct more portable
The code relied on the assumptions that CHAR_BIT is 8 and that unsigned
does not have padding bits.

In the Bignum module we already assume that the sign of an MPI is either
-1 or 1. Using this, we eliminate the above mentioned dependency.
2020-03-13 15:25:40 +00:00
Janos Follath 3480947667 Document ct_lt_mpi_uint 2020-03-13 15:25:40 +00:00
Janos Follath afa5342452 mpi_lt_mpi_ct: make use of unsigned consistent 2020-03-13 15:25:40 +00:00
Janos Follath a830377142 ct_lt_mpi_uint: make use of biL 2020-03-13 15:25:40 +00:00
Janos Follath 8faf1d627b Change mbedtls_mpi_cmp_mpi_ct to check less than
The signature of mbedtls_mpi_cmp_mpi_ct() meant to support using it in
place of mbedtls_mpi_cmp_mpi(). This meant full comparison functionality
and a signed result.

To make the function more universal and friendly to constant time
coding, we change the result type to unsigned. Theoretically, we could
encode the comparison result in an unsigned value, but it would be less
intuitive.

Therefore we won't be able to represent the result as unsigned anymore
and the functionality will be constrained to checking if the first
operand is less than the second. This is sufficient to support the
current use case and to check any relationship between MPIs.

The only drawback is that we need to call the function twice when
checking for equality, but this can be optimised later if an when it is
needed.
2020-03-13 15:25:40 +00:00
Janos Follath 81c9fe5f2c mbedtls_mpi_cmp_mpi_ct: remove multiplications
Multiplication is known to have measurable timing variations based on
the operands. For example it typically is much faster if one of the
operands is zero. Remove them from constant time code.
2020-03-13 15:25:40 +00:00
Janos Follath fd9797b595 Remove excess vertical space 2020-03-13 15:25:40 +00:00
Janos Follath 78ed22b404 Remove declaration after statement
Visual Studio 2013 does not like it for some reason.
2020-03-13 15:25:40 +00:00
Janos Follath fc2a826ab4 Fix side channel vulnerability in ECDSA 2020-03-13 15:25:39 +00:00
Janos Follath c514ce474a Add new, constant time mpi comparison 2020-03-13 15:25:39 +00:00
Janos Follath 6bd8c0ae2a ECDSA: Fix side channel vulnerability
The blinding applied to the scalar before modular inversion is
inadequate. Bignum is not constant time/constant trace, side channel
attacks can retrieve the blinded value, factor it (it is smaller than
RSA keys and not guaranteed to have only large prime factors). Then the
key can be recovered by brute force.

Reducing the blinded value makes factoring useless because the adversary
can only recover pk*t+z*N instead of pk*t.
2020-03-13 15:25:39 +00:00
Gilles Peskine 20dbfb9938 CTR_DRBG: support set_entropy_len() before seed()
mbedtls_ctr_drbg_seed() always set the entropy length to the default,
so a call to mbedtls_ctr_drbg_set_entropy_len() before seed() had no
effect. Change this to the more intuitive behavior that
set_entropy_len() sets the entropy length and seed() respects that and
only uses the default entropy length if there was no call to
set_entropy_len().

The former test-only function mbedtls_ctr_drbg_seed_entropy_len() is
no longer used, but keep it for strict ABI compatibility.
2020-03-13 15:25:39 +00:00