Commit graph

3483 commits

Author SHA1 Message Date
Manuel Pégourié-Gonnard ab601d6a1c Fix memory leak on error path
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-24 11:57:47 +02:00
Manuel Pégourié-Gonnard 406c7aedc4 RSA: blind call to mpi_inv_mod() on secret value
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-24 11:57:47 +02:00
Manuel Pégourié-Gonnard 6ab924de1d RSA: remove redundant GCD call in prepare_blinding()
inv_mod() already returns a specific error code if the value is not
invertible, so no need to check in advance that it is. Also, this is a
preparation for blinding the call to inv_mod(), which is made easier by
avoiding the redundancy (otherwise the call to gcd() would need to be blinded
too).

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-24 11:57:47 +02:00
Manuel Pégourié-Gonnard a35e98a060 DHM: blind call to mpi_inv_mod() on secret value
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-24 11:57:46 +02:00
Manuel Pégourié-Gonnard f0f43c51c4 DHM: make drawing of blinding value a function
In the next commit, we'll need to draw a second random value, in order to
blind modular inversion. Having a function for that will avoid repetition.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-24 11:57:46 +02:00
Janos Follath 2a4f8991b3 Bump version to Mbed TLS 2.7.16
Executed "./scripts/bump_version.sh --version 2.7.16"

Signed-off-by: Janos Follath <janos.follath@arm.com>
2020-06-26 12:37:57 +01:00
Janos Follath 9cdda866bf Merge branch 'mbedtls-2.7-restricted' into mbedtls-2.7.16r0 2020-06-25 09:20:57 +01:00
Manuel Pégourié-Gonnard f2027b5c46
Merge pull request #705 from mpg/l13-hw-starts-finish-2.7-restricted
[backport 2.7] Use starts/finish around Lucky 13 dummy compressions
2020-06-23 10:43:22 +02:00
Manuel Pégourié-Gonnard 96951785fc Test multi-block output of the hash-based KDF
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-22 10:43:23 +02:00
Manuel Pégourié-Gonnard 138109133d Remove SHA-1 as a fallback option
- it's 2020, there shouldn't be too many systems out there where SHA-1 is the
  only available hash option, so its usefulness is limited
- OTOH testing configurations without SHA-2 reveal bugs that are not easy to
  fix in a fully compatible way

So overall, the benefit/cost ratio is not good enough to justify keeping SHA-1
as a fallback option here.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-19 11:00:19 +02:00
Manuel Pégourié-Gonnard 9797288383 Improve comment justifying a hard-coded limitation
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-19 11:00:11 +02:00
Manuel Pégourié-Gonnard 8745986699 Zeroize temporary stack buffer
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-19 10:59:59 +02:00
Manuel Pégourié-Gonnard 601128eb58 Fix potential memory overread in seed functions
The previous commit introduced a potential memory overread by reading
secret_len bytes from secret->p, while the is no guarantee that secret has
enough limbs for that.

Fix that by using an intermediate buffer and mpi_write_binary().

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-19 10:56:55 +02:00
Manuel Pégourié-Gonnard 6d61498e05 Add fall-back to hash-based KDF for internal ECP DRBG
The dependency on a DRBG module was perhaps a bit strict for LTS branches, so
let's have an option that works with no DRBG when at least one SHA module is
present.

This changes the internal API of ecp_drbg_seed() by adding the size of the
MPI as a parameter. Re-computing the size from the number of limbs doesn't
work too well here as we're writing out to a fixed-size buffer and for some
curves (P-521) that would round up too much. Using mbedtls_mpi_get_len() is
not entirely satisfactory either as it would mean using a variable-length
encoding, with could open side channels.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-19 10:56:55 +02:00
Manuel Pégourié-Gonnard 99bf33fa81 Fix typo in a comment
Co-authored-by: Janos Follath <janos.follath@arm.com>
2020-06-19 10:37:38 +02:00
Manuel Pégourié-Gonnard e2828c2d94 Use HMAC_DRBG by default for ECP internal DRBG
It results in smaller code than using CTR_DRBG (64 bytes smaller on ARMv6-M
with arm-none-eabi-gcc 7.3.1), so let's use this by default when both are
available.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-19 10:36:57 +02:00
Manuel Pégourié-Gonnard 22fe5236e9 Skip redundant checks for NULL f_rng
Unless MBEDTLS_ECP_NO_INTERNAL_RNG is defined, it's no longer possible for
f_rng to be NULL at the places that randomize coordinates.

Eliminate the NULL check in this case:
- it makes it clearer to reviewers that randomization always happens (unless
  the user opted out at compile time)
- a NULL check in a place where it's easy to prove the value is never NULL
  might upset or confuse static analyzers (including humans)
- removing the check saves a bit of code size

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-19 10:36:16 +02:00
Manuel Pégourié-Gonnard 75036a0aff Implement use of internal DRBG for ecp_mul()
The case of MBEDTLS_ECP_RESTARTABLE isn't handled correctly yet: in that case
the DRBG instance should persist when resuming the operation. This will be
addressed in the next commit.

When both CTR_DRBG and HMAC_DRBG are available, CTR_DRBG is preferred since
both are suitable but CTR_DRBG tends to be faster and I needed a tie-breaker.

There are currently three possible cases to test:

- NO_INTERNAL_RNG is set -> tested in test_ecp_no_internal_rng
- it's unset and CTR_DRBG is available -> tested in the default config
- it's unset and CTR_DRBG is disabled -> tested in
  test_ecp_internal_rng_no_ctr_drbg

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-19 10:27:27 +02:00
Manuel Pégourié-Gonnard d90faf92b2 Add config.h option MBEDTLS_ECP_NO_INTERNAL_RNG
No effect so far, except on dependency checking, as the feature it's meant to
disable isn't implemented yet (so the descriptions in config.h and the
ChangeLog entry are anticipation for now).

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-19 10:05:16 +02:00
Janos Follath 44183d1548
Merge pull request #3409 from bensze01/license-2.7
[Backport 2.7] Update license headers to reflect the Apache-2.0 OR GPL-2.0-or-later licensing
2020-06-18 15:54:09 +01:00
Manuel Pégourié-Gonnard 8352797c44 Use starts/finish around Lucky 13 dummy compressions
Fixes #3246

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-18 11:48:55 +02:00
Bence Szépkúti 4e9f71227a 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:56:41 +02:00
Ronald Cron 904775da12 ssl_client: Align line breaking with MBEDTLS_SSL_DEBUG_*
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-06-12 10:00:16 +02:00
Ronald Cron a32236c813 Use defines to check alpn ext list validity
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-06-12 10:00:04 +02:00
Hanno Becker 2064355747 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:59:50 +02:00
Hanno Becker d8562b5e46 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:59:28 +02:00
Hanno Becker 0e8dc48cff 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:51:25 +02:00
Ronald Cron 29efc0f37d 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:35:14 +02:00
Hanno Becker 8cf6b49e6d 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:34:50 +02:00
Hanno Becker 910a751037 Introduce macros for constants in SSL ticket implementation
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-06-11 14:32:18 +02:00
Janos Follath 87e93d054d
Merge pull request #3412 from gilles-peskine-arm/montmul-cmp-branch-2.7
Backport 2.7: Remove a secret-dependent branch in Montgomery multiplication
2020-06-09 12:40:17 +01:00
Gilles Peskine f3317e6035 Clean up some comments
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:54:20 +02:00
Gilles Peskine fa85cc2da5 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:54:20 +02:00
Gilles Peskine cc6a6bfda7 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:53:46 +02:00
Gilles Peskine 6f3b68db74 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:53:46 +02:00
Gilles Peskine dc24cece12 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:50:44 +02:00
Gilles Peskine ea9ba77e55 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:50:44 +02:00
Gilles Peskine 7ff812e0d7 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:50:44 +02:00
Gilles Peskine 3c44c65fc1 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:50:44 +02:00
Gilles Peskine d108d07050 Document some internal bignum functions
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-06-09 11:50:44 +02:00
Gilles Peskine 8ff7cc9911 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:50:44 +02:00
Gilles Peskine d6496afa0b 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:48:25 +02:00
Manuel Pégourié-Gonnard 1539d15dd5
Merge pull request #3353 from gilles-peskine-arm/fix-ecp-mul-memory-leak-2.7
Backport 2.7: Fix potential memory leak in EC multiplication
2020-06-05 11:44:14 +02:00
Gilles Peskine ac631bb52a Re-generate error.c
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-05-28 12:06:10 +02:00
Jonas 86c5c61a91 Fix potential memory leak in EC multiplication
Signed-off-by: Jonas <jonas.lejeune4420@gmail.com>
2020-05-25 13:56:48 +02:00
Andrzej Kurek 3fd9297658 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:30:21 +02:00
Manuel Pégourié-Gonnard 0a997082fe Merge branch 'mbedtls-2.7-restricted' into prepare-rc-2.7.15-update
* mbedtls-2.7-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:31:52 +02:00
Manuel Pégourié-Gonnard 6e0806b338 Merge remote-tracking branch 'restricted/pr/671' into mbedtls-2.7-restricted
* restricted/pr/671:
  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:57:18 +02:00
Janos Follath b4b458fe01 Bump version to Mbed TLS 2.7.15
Signed-off-by: Janos Follath <janos.follath@arm.com>
2020-04-08 17:22:51 +01:00
Janos Follath e170ee7e18 Merge branch 'mbedtls-2.7-restricted' into mbedtls-2.7.15r0
Signed-off-by: Janos Follath <janos.follath@arm.com>
2020-04-08 15:17:55 +01:00
Gilles Peskine 29b7b9585b
Merge pull request #3145 from mpg/fix-reconnect-2.7
[backport 2.7] Fix issues in handling of client reconnecting from the same port
2020-04-02 19:21:22 +02:00
Manuel Pégourié-Gonnard 54587fcf9b 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:28:08 +02:00
Manuel Pégourié-Gonnard 6062b49d29 Fix bug in handling of DTLS client hard reconnect
We keep track of the current epoch and record sequence number in out_ctr,
which was overwritten when writing the record containing the
HelloVerifyRequest starting from out_buf. We can avoid that by only using the
rest of the buffer.

Using MBEDTLS_SSL_MAX_CONTENT_LEN as the buffer size is still correct, as it
was a pretty conservative value when starting from out_buf.

Note: this bug was also fixed unknowingly in 2.13 by introducing a new buffer
that holds the current value of the sequence number (including epoch), while
working on datagram packing: 198594709b

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-03-31 13:04:19 +02:00
Manuel Pégourié-Gonnard 4bbbdc36bc 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:46:23 +02:00
Simon Butcher 2c3351e54e 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:43:40 +00:00
Andres Amaya Garcia 8758053e80 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:26:02 +01:00
Janos Follath 0f22670243 Bump version to Mbed TLS 2.7.14 2020-02-19 12:08:10 +00:00
Manuel Pégourié-Gonnard 609d79ed8e 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 11:27:08 +01:00
Manuel Pégourié-Gonnard 869e9668dd Check public part when parsing private RSA key 2020-02-18 10:53:13 +01:00
Manuel Pégourié-Gonnard 8cc0491966 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:53:13 +01:00
Manuel Pégourié-Gonnard 6cf5931f1d
Merge pull request #3028 from gilles-peskine-arm/mpi_copy_shrink-2.7
Backport 2.7: Improve robustness and testing of mbedtls_mpi_copy
2020-02-06 09:52:18 +01:00
Janos Follath b40d60f096 Revert "Merge pull request #3011 from Patater/dev/jp-bennett/development-2.7"
This reverts commit 130e136439, reversing
changes made to 071b3e170e.

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

Fixes #3005.
2020-02-04 14:47:45 +00:00
Janos Follath 5d1171268c
Merge pull request #3020 from mpg/fix-ssl-opt-gnutls-no-sha1-2.7
[backport 2.7] Fix ssl-opt.sh for GnuTLS versions rejecting SHA-1
2020-02-04 11:19:18 +00:00
Manuel Pégourié-Gonnard ac3c80673f Fix comment to match reality
We can't fix the code to match the comment, so have it the other way round.
2020-02-04 09:52:27 +01:00
Manuel Pégourié-Gonnard 918b25d8fd Revert "Fix certs.c to match the content of the files"
This reverts commit 205e88cb20.
2020-02-04 09:48:08 +01:00
Gilles Peskine 774c163eae Minor comment improvement 2020-02-03 16:34:53 +01:00
Gilles Peskine 6a26967382 Improve comments in mpi_shrink 2020-02-03 16:34:53 +01:00
Gilles Peskine 2aeab87cf7 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:34:53 +01:00
Manuel Pégourié-Gonnard 205e88cb20 Fix certs.c to match the content of the files
The comment on TEST_SRV_CRT_RSA_SHA256 that it was
tests/data_files/server2-sha256.crt was a lie, the contents were actually
those of the mbedtls-2.16 version of the same file.

While it didn't have a noticeable impact on its own, it was confusing and
distracting while investigating an issue that cause gnutls-cli to not trust
the default RSA-SHA256 cert given test-ca.crt as a root, so worth fixing.
2020-02-03 15:54:11 +01:00
Jack Lloyd 100e147c71 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:13:04 -05:00
Janos Follath a67508e066 Merge pull request #3002 from gilles-peskine-arm/coverity-20200115-2.7 into mbedtls-2.7 2020-01-29 14:53:48 +00:00
Jaeden Amero 130e136439
Merge pull request #3011 from Patater/dev/jp-bennett/development-2.7
Backport 2.7: Allow loading symlinked certificates
2020-01-28 15:55:41 +00:00
Jonathan Bennett 791babcbb9 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:26:47 +00:00
Gilles Peskine 0f595f714a Remove redundant block_size validity check
Check the value only once, as soon as we've obtained it.
2020-01-22 19:09:05 +01:00
Gilles Peskine d22a7933d2 Add missing return code check on call to mbedtls_md() 2020-01-22 19:06:32 +01:00
Janos Follath ee88f8145d Bump version to Mbed TLS 2.7.13 2020-01-20 14:28:41 +00:00
Jaeden Amero d8180f8d84 Merge remote-tracking branch 'origin/mbedtls-2.7' into mbedtls-2.7-restricted
* origin/mbedtls-2.7:
  Enable more test cases without MBEDTLS_MEMORY_DEBUG
  More accurate test case description
  Clarify that the "FATAL" message is expected
  Note that mbedtls_ctr_drbg_seed() must not be called twice
  Fix CTR_DRBG benchmark
  Changelog entry for xxx_drbg_set_entropy_len before xxx_drbg_seed
  CTR_DRBG: support set_entropy_len() before seed()
  CTR_DRBG: Don't use functions before they're defined
  HMAC_DRBG: support set_entropy_len() before seed()
2020-01-15 16:59:10 +00:00
Gilles Peskine b2be1fca2c 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.
2019-11-28 09:55:25 +01:00
Gilles Peskine 2414ce1a5e Parse HelloVerifyRequest: avoid buffer overread at the start
In ssl_parse_hello_verify_request, we read 3 bytes (version and cookie
length) without checking that there are that many bytes left in
ssl->in_msg. This could potentially read from memory outside of the
ssl->receive buffer (which would be a remotely exploitable
crash).
2019-11-21 14:18:27 +01:00
Gilles Peskine 99b6777b72 Parse HelloVerifyRequest: avoid buffer overread on the cookie
In ssl_parse_hello_verify_request, we print cookie_len bytes without
checking that there are that many bytes left in ssl->in_msg. This
could potentially log data outside the received message (not a big
deal) and could potentially read from memory outside of the receive
buffer (which would be a remotely exploitable crash).
2019-11-21 14:18:26 +01:00
Jaeden Amero c5a016dde1 Merge remote-tracking branch 'restricted/pr/666' into mbedtls-2.7-restricted
* restricted/pr/666: (24 commits)
  Add ChangeLog entry
  mpi_lt_mpi_ct: fix condition handling
  mpi_lt_mpi_ct: Add further tests
  mpi_lt_mpi_ct: Fix test numbering
  mpi_lt_mpi_ct perform tests for both limb size
  ct_lt_mpi_uint: cast the return value explicitely
  mbedtls_mpi_lt_mpi_ct: add tests for 32 bit limbs
  mbedtls_mpi_lt_mpi_ct: simplify condition
  Rename variable for better readability
  mbedtls_mpi_lt_mpi_ct: Improve documentation
  Make mbedtls_mpi_lt_mpi_ct more portable
  Bignum: Document assumptions about the sign field
  Add more tests for mbedtls_mpi_lt_mpi_ct
  mpi_lt_mpi_ct test: hardcode base 16
  Document ct_lt_mpi_uint
  mpi_lt_mpi_ct: make use of unsigned consistent
  ct_lt_mpi_uint: make use of biL
  Change mbedtls_mpi_cmp_mpi_ct to check less than
  mbedtls_mpi_cmp_mpi_ct: remove multiplications
  Remove excess vertical space
  ...
2019-11-12 10:47:55 +00:00
Jaeden Amero e70059df85 Merge remote-tracking branch 'restricted/pr/668' into mbedtls-2.7-restricted
* restricted/pr/668:
  Zeroize local AES variables before exiting the function
2019-11-12 10:42:45 +00:00
Andrzej Kurek 07597365cd 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.
2019-11-12 03:23:51 -05:00
Janos Follath b4edac5616 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.
2019-11-11 12:27:36 +00:00
Janos Follath 5823961558 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.
2019-11-11 12:27:36 +00:00
Janos Follath cff9e6e03d 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.
2019-11-11 12:27:36 +00:00
Janos Follath 8ec2a953af Rename variable for better readability 2019-11-11 12:27:36 +00:00
Janos Follath a2b9a96fb8 mbedtls_mpi_lt_mpi_ct: Improve documentation 2019-11-11 12:27:36 +00:00
Janos Follath 51ed14e20f 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.
2019-11-11 12:27:36 +00:00
Janos Follath 3173a53fe9 Document ct_lt_mpi_uint 2019-11-11 12:27:36 +00:00
Janos Follath 782cbe592d mpi_lt_mpi_ct: make use of unsigned consistent 2019-11-11 12:27:36 +00:00
Janos Follath db9f449409 ct_lt_mpi_uint: make use of biL 2019-11-11 12:27:36 +00:00
Janos Follath c3b376e2f2 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.
2019-11-11 12:27:36 +00:00
Janos Follath 8461c0e2a8 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.
2019-11-11 12:27:36 +00:00
Janos Follath 8de2d45cd7 Remove excess vertical space 2019-11-11 12:27:36 +00:00
Janos Follath c587a32a9c Remove declaration after statement
Visual Studio 2013 does not like it for some reason.
2019-11-11 12:27:36 +00:00
Janos Follath 5f3019b298 Fix side channel vulnerability in ECDSA 2019-11-11 12:27:36 +00:00
Janos Follath e0187b95f0 Add new, constant time mpi comparison 2019-11-11 12:27:27 +00:00
Janos Follath 82debf8332 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.
2019-10-25 09:01:34 +01:00
Gilles Peskine b729e1b9ba 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.
2019-10-23 18:01:25 +02:00