Commit graph

7411 commits

Author SHA1 Message Date
Manuel Pégourié-Gonnard 73341a0f84 Add a --quiet option to all.sh
The primary purpose is to use it to run all.sh -k -q in the pre-push hook, but
this can be useful in any circumstance where you're not interested in the full
output from each component and just want a short summary of which components
were run (and if any failed).

Note that only stdout from components is suppressed, stderr is preserved so
that errors are reported. This means components should avoid printing to
stderr in normal usage (ie in the absence of errors).

Currently all the `check_*` components obey this convention except:
- check_generate_test_code: unittest prints progress to stderr
- check_test_cases: lots of non-fatal warnings printed to stderr

These components will be fixed in follow-up commits.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-23 11:53:07 +02: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
Janos Follath 3f44eb5ac5
Merge pull request #706 from mpg/ecp-mul-null-rng-2.7-restricted
[Backport 2.7] Use internal RNG in ecp_mul when none was provided
2020-06-22 15:06:44 +01:00
Gilles Peskine eaf31e39c2
Merge pull request #3443 from mpg/make-coverage-script-deterministic-2.7
[Backport 2.7] Make basic-build-test.sh more deterministic
2020-06-22 12:30:54 +02:00
Manuel Pégourié-Gonnard e4a5c05d49 Adjust comments about SEED synchronisation
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-22 10:54:38 +02:00
Manuel Pégourié-Gonnard 54d95b1722 Make basic-build-test.sh deterministic
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-22 10:54:38 +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 f1aca9fdba Update dependencies documentation
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-19 10:57:36 +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 6d059bf051 Add Security ChangeLog entry for lack of blinding
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-19 10:32:45 +02:00
Manuel Pégourié-Gonnard 966cb796c4 Update documentation about optional f_rng parameter
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-19 10:32:34 +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 eb7f35d18e Update LICENSE and README.md to reflect licensing
SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later

Also add a copy of the GPL to the repo

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
2020-06-15 12:57:29 +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
Manuel Pégourié-Gonnard 693768f8e9
Merge pull request #3424 from ronald-cron-arm/ssl_write_client_hello-2.7
[Backport 2.7] Bounds checks in ssl_write_client_hello
2020-06-15 10:57:58 +02:00
Ronald Cron 4206bd4a4f Align with check-like function return value convention
By convention, in the project, functions that have a
check or similar in the name return 0 if the check
succeeds, non-zero otherwise. Align with this for
mbedtls_ssl_chk_buf_ptr().

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-06-12 10:00:29 +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
Hanno Becker dc7b5b97a1 Add macro for bounds checking
This commit adds a macro for buffer bounds checks in the SSL
module. It takes the buffer's current and end position as the
first argument(s), followed by the needed space; if the
available space is too small, it returns an SSL_BUFFER_TOO_SMALL
error.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-06-11 14:35:27 +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 70529abbac Add changelog entry: fix #3394
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
Bence Szépkúti 09b4f19f94 Add Apache-2.0 headers to all scripts
This commit was generated using the following script:

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

# Find scripts
find -path './.git' -prune -o '(' -name '*.gdb' -o -name '*.pl' -o -name '*.py' -o -name '*.sh' ')' -print | xargs sed -i '

# Remove Mbed TLS declaration if it occurs before the copyright line
1,/Copyright.*Arm/I {
  /This file is part of/,$ {
    /Copyright.*Arm/I! d
  }
}

# Convert non-standard header in scripts/abi_check.py to the format used in the other scripts
/"""/,/"""/ {

  # Cut copyright declaration
  /Copyright.*Arm/I {
    h
    N
    d
  }

  # Paste copyright declaration
  /"""/ {
    x
    /./ {
      s/^/# /    # Add #
      x          # Replace orignal buffer with Copyright declaration
      p          # Print original buffer, insert newline
      i\

      s/.*//     # Clear original buffer
    }
    x
  }
}

/Copyright.*Arm/I {

  # Print copyright declaration
  p

  # Read the two lines immediately following the copyright declaration
  N
  N

  # Insert Apache header if it is missing
  /SPDX/! {
    i\
# SPDX-License-Identifier: Apache-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.

    # Insert Mbed TLS declaration if it is missing
    /This file is part of/! i\
#\
# This file is part of Mbed TLS (https://tls.mbed.org)
  }

  # Clear copyright declaration from buffer
  D
}
'
# ========================

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
2020-06-08 17:46:40 +02:00
Bence Szépkúti b7246ad938 Add copyright dates to all scripts
To find any files with a missing copyright declaration, use the following script:

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

# Find files with copyright declarations, and list their file extensions
exts=$(grep -Ril --exclude-dir .git 'Copyright.*Arm' | sed '
  s/.*\./-name "*./
  s/$/"/
' | sort -u | sed -n '
  :l
    N
    $!bl
  s/\n/ -o /gp
')

# Find files with file extensions that ususally include copyright extensions,
# but don't include a copyright declaration themselves.
eval "find -path './.git' -prune -o\
  ! -path './tests/data_files/format_pkcs12.fmt'\
  ! -path './programs/psa/psa_constant_names_generated.c'\
  '(' $exts ')' -print" | xargs grep -Li 'Copyright.*Arm'
# ========================

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
2020-06-08 17:26:24 +02:00