Commit Graph

4215 Commits

Author SHA1 Message Date
Bence Szépkúti
f744bd72ee Update license headers to Apache-2.0 OR GPL-2.0-or-later
This will allow us to ship the LTS branches in a single archive

This commit was generated using the following script:

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

header1='\ *  SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later\
 *\
 *  This file is provided under the Apache License 2.0, or the\
 *  GNU General Public License v2.0 or later.\
 *\
 *  **********\
 *  Apache License 2.0:\
 *\
 *  Licensed under the Apache License, Version 2.0 (the "License"); you may\
 *  not use this file except in compliance with the License.\
 *  You may obtain a copy of the License at\
 *\
 *  http://www.apache.org/licenses/LICENSE-2.0\
 *\
 *  Unless required by applicable law or agreed to in writing, software\
 *  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT\
 *  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\
 *  See the License for the specific language governing permissions and\
 *  limitations under the License.\
 *\
 *  **********\
 *\
 *  **********\
 *  GNU General Public License v2.0 or later:\
 *\
 *  This program is free software; you can redistribute it and/or modify\
 *  it under the terms of the GNU General Public License as published by\
 *  the Free Software Foundation; either version 2 of the License, or\
 *  (at your option) any later version.\
 *\
 *  This program is distributed in the hope that it will be useful,\
 *  but WITHOUT ANY WARRANTY; without even the implied warranty of\
 *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the\
 *  GNU General Public License for more details.\
 *\
 *  You should have received a copy of the GNU General Public License along\
 *  with this program; if not, write to the Free Software Foundation, Inc.,\
 *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.\
 *\
 *  **********'

find -path './.git' -prune -o '(' -name '*.c' -o -name '*.cpp' -o -name '*.fmt' -o -name '*.h' ')' -print | xargs sed -i "
# Normalize the first line of the copyright headers (no text on the first line of a block comment)
/^\/\*.*Copyright.*Arm/I s/\/\*/&\n */

# Insert new copyright header
/SPDX-License-Identifier/ i\
$header1

# Delete old copyright header
/SPDX-License-Identifier/,$ {
  # Delete lines until the one preceding the mbedtls declaration
  N
  1,/This file is part of/ {
    /This file is part of/! D
  }
}
"

# Format copyright header for inclusion into scripts
header2=$(echo "$header1" | sed 's/^\\\? \* \?/#/')

find -path './.git' -prune -o '(' -name '*.gdb' -o -name '*.pl' -o -name '*.py' -o -name '*.sh' ')' -print | xargs sed -i "
# Insert new copyright header
/SPDX-License-Identifier/ i\
$header2

# Delete old copyright header
/SPDX-License-Identifier/,$ {
  # Delete lines until the one preceding the mbedtls declaration
  N
  1,/This file is part of/ {
    /This file is part of/! D
  }
}
"
# ========================

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
2020-06-15 12:48:48 +02:00
Ronald Cron
32b629dc99 ssl_client: Align line breaking with MBEDTLS_SSL_DEBUG_*
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-06-12 09:41:36 +02:00
Ronald Cron
157cffebab Use defines to check alpn ext list validity
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-06-12 09:40:07 +02:00
Hanno Becker
f250380df3 Return error in case of bad user configurations
This commits adds returns with the SSL_BAD_CONFIG error code
in case of bad user configurations.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-06-12 09:36:59 +02:00
Hanno Becker
d7296020a6 Add error condition for bad user configurations
This commit adds an error condition for bad user configurations
and updates the number of SSL module errors in error.h.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-06-12 09:36:39 +02:00
Hanno Becker
f8f61aad0f Uniformize bounds checks using new macro
This commit uses the previously defined macro to uniformize
bounds checks in several places. It also adds bounds checks to
the ClientHello writing function that were previously missing.
Also, the functions adding extensions to the ClientHello message
can now fail if the buffer is too small or a different error
condition occurs, and moreover they take an additional buffer
end parameter to free them from the assumption that one is
writing to the default output buffer.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-06-11 14:21:24 +02:00
Ronald Cron
711eea30b9 Remove unnecessary MBEDTLS_ECP_C preprocessor condition
The ssl_cli.c:ssl_write_supported_elliptic_curves_ext()
function is compiled only if MBEDTLS_ECDH_C, MBEDTLS_ECDSA_C
or MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED is defined which
implies that MBEDTLS_ECP_C is defined. Thus remove the
precompiler conditions on MBEDTLS_ECP_C in its code.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-06-11 14:21:24 +02:00
Hanno Becker
35f8a54405 Shorten lines in library/ssl_cli.c to at most 80 characters
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-06-11 14:21:24 +02:00
Hanno Becker
8f397268d3 Introduce macros for constants in SSL ticket implementation
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-06-11 14:21:11 +02:00
Janos Follath
001eb3cec4
Merge pull request #3411 from gilles-peskine-arm/montmul-cmp-branch-2.16
Backport 2.16: Remove a secret-dependent branch in Montgomery multiplication
2020-06-09 12:40:30 +01:00
Gilles Peskine
de719d5d69 Clean up some comments
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:31:30 +02:00
Gilles Peskine
08fd43c4f6 mbedtls_mpi_sub_abs: check the range of the result when it happens
The function mbedtls_mpi_sub_abs first checked that A >= B and then
performed the subtraction, relying on the fact that A >= B to
guarantee that the carry propagation would stop, and not taking
advantage of the fact that the carry when subtracting two numbers can
only be 0 or 1. This made the carry propagation code a little hard to
follow.

Write an ad hoc loop for the carry propagation, checking the size of
the result. This makes termination obvious.

The initial check that A >= B is no longer needed, since the function
now checks that the carry propagation terminates, which is equivalent.
This is a slight performance gain.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:31:30 +02:00
Gilles Peskine
635a374944 Simplify the final reduction in mpi_montmul
There was some confusion during review about when A->p[n] could be
nonzero. In fact, there is no need to set A->p[n]: only the
intermediate result d might need to extend to n+1 limbs, not the final
result A. So never access A->p[n]. Rework the explanation of the
calculation in a way that should be easier to follow.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:31:30 +02:00
Gilles Peskine
36acd547c5 Move carry propagation out of mpi_sub_hlp
The function mpi_sub_hlp had confusing semantics: although it took a
size parameter, it accessed the limb array d beyond this size, to
propagate the carry. This made the function difficult to understand
and analyze, with a potential buffer overflow if misused (not enough
room to propagate the carry).

Change the function so that it only performs the subtraction within
the specified number of limbs, and returns the carry.

Move the carry propagation out of mpi_sub_hlp and into its caller
mbedtls_mpi_sub_abs. This makes the code of subtraction very slightly
less neat, but not significantly different.

In the one other place where mpi_sub_hlp is used, namely mpi_montmul,
this is a net win because the carry is potentially sensitive data and
the function carefully arranges to not have to propagate it.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:31:30 +02:00
Gilles Peskine
46bf7da684 More logical parameter order for mpi_sub_hlp
mpi_sub_hlp performs a subtraction A - B, but took parameters in the
order (B, A). Swap the parameters so that they match the usual
mathematical syntax.

This has the additional benefit of putting the output parameter (A)
first, which is the normal convention in this module.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:31:30 +02:00
Gilles Peskine
6a9433ef34 Explicitly cast down from mbedtls_mpi_uint to unsigned char
Let code analyzers know that this is deliberate. For example MSVC
warns about the conversion if it's implicit.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:31:30 +02:00
Gilles Peskine
8f6726623a Remove a secret-dependent branch in Montgomery multiplication
In mpi_montmul, an auxiliary function for modular
exponentiation (mbedtls_mpi_mod_exp) that performs Montgomery
multiplication, the last step is a conditional subtraction to force
the result into the correct range. The current implementation uses a
branch and therefore may leak information about secret data to an
adversary who can observe what branch is taken through a side channel.

Avoid this potential leak by always doing the same subtraction and
doing a contant-trace conditional assignment to set the result.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:31:30 +02:00
Gilles Peskine
c81c5889e9 Separate out low-level mpi_safe_cond_assign
Separate out a version of mpi_safe_cond_assign that works on
equal-sized limb arrays, without worrying about allocation sizes or
signs.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:31:30 +02:00
Gilles Peskine
3ce3ddf1ac Document some internal bignum functions
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:31:30 +02:00
Gilles Peskine
bdcb39616d Revert "Shut up a clang-analyzer warning"
This reverts commit 2cc69fffcf.

A check was added in mpi_montmul because clang-analyzer warned about a
possibly null pointer. However this was a false positive. Recent
versions of clang-analyzer no longer emit a warning (3.6 does, 6
doesn't).

Incidentally, the size check was wrong: mpi_montmul needs
T->n >= 2 * (N->n + 1), not just T->n >= N->n + 1.

Given that this is an internal function which is only used from one
public function and in a tightly controlled way, remove both the null
check (which is of low value to begin with) and the size check (which
would be slightly more valuable, but was wrong anyway). This allows
the function not to need to return an error, which makes the source
code a little easier to read and makes the object code a little
smaller.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:31:30 +02:00
Gilles Peskine
e9073a6cb2 Add a const annotation to the non-changing argument of mpi_sub_mul
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:31:30 +02:00
Manuel Pégourié-Gonnard
62b293df5b
Merge pull request #3352 from gilles-peskine-arm/fix-ecp-mul-memory-leak-2.16
Backport 2.16: Fix potential memory leak in EC multiplication
2020-06-05 11:44:02 +02:00
Gilles Peskine
d2f0073c09 Re-generate error.c
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-05-28 11:59:59 +02:00
Jonas
6645fd31e7 Fix potential memory leak in EC multiplication
Signed-off-by: Jonas <jonas.lejeune4420@gmail.com>
2020-05-25 13:53:15 +02:00
Andrzej Kurek
acf7f2ce93 Guard from undefined behaviour in case of an INT_MAX max_pathlen
When parsing a certificate with the basic constraints extension
the max_pathlen that was read from it was incremented regardless
of its value. However, if the max_pathlen is equal to INT_MAX (which
is highly unlikely), an undefined behaviour would occur.
This commit adds a check to ensure that such value is not accepted
as valid. Relevant tests for INT_MAX and INT_MAX-1 are also introduced.
Certificates added in this commit were generated using the
test_suite_x509write, function test_x509_crt_check. Input data taken
from the "Certificate write check Server1 SHA1" test case, so the generated
files are like the "server1.crt", but with the "is_ca" field set to 1 and
max_pathlen as described by the file name.

Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
Signed-off-by: Piotr Nowicki <piotr.nowicki@arm.com>
2020-04-17 11:29:20 +02:00
Manuel Pégourié-Gonnard
dab3fd64ab Merge branch 'mbedtls-2.16-restricted' into prepare-rc-2.16.6-updated
* mbedtls-2.16-restricted:
  Parse HelloVerifyRequest buffer overread: add changelog entry
  Parse HelloVerifyRequest: avoid buffer overread at the start
  Parse HelloVerifyRequest: avoid buffer overread on the cookie
2020-04-09 12:27:20 +02:00
Manuel Pégourié-Gonnard
ef98d49997 Merge remote-tracking branch 'restricted/pr/670' into mbedtls-2.16-restricted
* restricted/pr/670:
  Parse HelloVerifyRequest buffer overread: add changelog entry
  Parse HelloVerifyRequest: avoid buffer overread at the start
  Parse HelloVerifyRequest: avoid buffer overread on the cookie
2020-04-09 11:56:09 +02:00
Janos Follath
c04703c58c Bump version to Mbed TLS 2.16.6
Signed-off-by: Janos Follath <janos.follath@arm.com>
2020-04-08 17:17:27 +01:00
Janos Follath
816305b8f5 Merge branch 'mbedtls-2.16-restricted' into mbedtls-2.16.6r0
Signed-off-by: Janos Follath <janos.follath@arm.com>
2020-04-08 15:12:15 +01:00
Gilles Peskine
5e65619669
Merge pull request #3142 from mpg/fix-reconnect-2.16
[Backport 2.16] Improve testing of DTLS client hard reconnect
2020-04-02 19:21:16 +02:00
Manuel Pégourié-Gonnard
f60041688c Fix leakage of projective coordinates in ECC
See the comments in the code for how an attack would go, and the ChangeLog
entry for an impact assessment. (For ECDSA, leaking a few bits of the scalar
over several signatures translates to full private key recovery using a
lattice attack.)

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-04-01 11:02:18 +02:00
Manuel Pégourié-Gonnard
b08a3344a5 Improve debug logging of client hard reconnect
The current logging was sub-standard, in particular there was no trace
whatsoever of the HelloVerifyRequest being sent. Now it's being logged with
the usual levels: 4 for full content, 2 return of f_send, 1 decision about
sending it (or taking other branches in the same function) because that's the
same level as state changes in the handshake, and also same as the "possible
client reconnect" message" to which it's the logical continuation (what are we
doing about it?).

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-03-31 12:32:38 +02:00
Simon Butcher
5cc087945a Correct comment on the configuration option in x509.c
In x509.c, the self-test code is dependent on MBEDTLS_CERTS_C and
MBEDTLS_SHA256_C being enabled. At some point in the recent past that dependency
was on MBEDTLS_SHA1_C but changed to SHA256, but the comment wasn't updated.

This commit updates the comment.

Signed-off-by: Simon Butcher <simon.butcher@arm.com>
2020-03-28 00:35:49 +00:00
Andres Amaya Garcia
52dbda62a0 Fix compilation issue when DTLS and SSL_HW_RECORD_ACCEL are on
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-02-26 10:13:27 +01:00
Janos Follath
334cf59930 Bump version to Mbed TLS 2.16.5 2020-02-19 12:03:00 +00:00
Manuel Pégourié-Gonnard
d09fcdedb9 Fix pkparse bug wrt MBEDTLS_RSA_ALT
Some code paths want to access members of the mbedtls_rsa_context structure.
We can only do that when using our own implementation, as otherwise we don't
know anything about that structure.
2020-02-18 10:49:06 +01:00
Manuel Pégourié-Gonnard
25bb8dc228 Check public part when parsing private RSA key 2020-02-18 10:49:06 +01:00
Manuel Pégourié-Gonnard
9ab0305700 Don't pass zero to rsa_complete() as a param
When parsing a PKCS#1 RSAPrivateKey structure, all parameters are always
present. After importing them, we need to call rsa_complete() for the sake of
alternative implementations. That function interprets zero as a signal for
"this parameter was not provided". As that's never the case, we mustn't pass
any zero value to that function, so we need to explicitly check for it.
2020-02-18 10:49:06 +01:00
Manuel Pégourié-Gonnard
8d9b1c471c
Merge pull request #3027 from gilles-peskine-arm/mpi_copy_shrink-2.16
Backport 2.16: Improve robustness and testing of mbedtls_mpi_copy
2020-02-06 09:52:11 +01:00
Janos Follath
cabf0eed66 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-02-04 14:42:15 +00:00
Gilles Peskine
56427c2d2b Minor comment improvement 2020-02-03 16:21:31 +01:00
Gilles Peskine
27c15c7853 Improve comments in mpi_shrink 2020-02-03 16:21:31 +01:00
Gilles Peskine
3e9f5228c8 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-02-03 16:21:31 +01:00
Jack Lloyd
b10fd065be 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-01-29 13:09:55 -05:00
Janos Follath
bac9f1bfb0 Merge pull request #3001 from from gilles-peskine-arm/coverity-20200115-2.16 into mbedtls-2.16 2020-01-29 14:49:23 +00:00
Jaeden Amero
7550e857bf
Merge pull request #3012 from Patater/dev/jp-bennett/development-2.16
Backport 2.16: Allow loading symlinked certificates
2020-01-28 15:55:54 +00:00
Janos Follath
d0c2575324
Merge pull request #2992 from yanesca/bump-version-2.16.4
Bump version to Mbed TLS 2.16.4
2020-01-28 11:32:46 +00:00
Jonathan Bennett
6f45cb995b 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-01-28 11:28:42 +00:00
Gilles Peskine
83a5672ae1 Remove redundant block_size validity check
Check the value only once, as soon as we've obtained it.
2020-01-27 15:16:16 +01:00
Manuel Pégourié-Gonnard
42b8194b53 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-01-24 12:35:47 +01:00