Commit Graph

9189 Commits

Author SHA1 Message Date
Gilles Peskine
eab4a7a05d
Merge pull request #3446 from mpg/use-all-sh-checks-for-pre-push-2.16
[backport 2.16] Use all.sh in pre-push hook
2020-06-23 14:37:20 +02:00
Manuel Pégourié-Gonnard
805b1461b8 all.sh: clean up some uses of "local" variables
While pure sh doesn't have a concept of local variables, we can partially
emulate them by unsetting variables before we exit the function, and use the
convention of giving them lowercase names to distinguish from global
variables.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-23 11:42:59 +02:00
Manuel Pégourié-Gonnard
d2b5ad871d Use all.sh in pre-push hook
The list in the pre-push hook was redundant with the list of `check_*`
components in all.sh, and unsurprisingly it was outdated.

Missing components were:

- check_recursion
- check_changelog
- check_test_cases
- check_python_files
- check_generate_test_code

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-23 11:37:04 +02:00
Manuel Pégourié-Gonnard
1ee685018a Make check_generate_test_code more -q friendly
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-23 11:36:32 +02:00
Manuel Pégourié-Gonnard
de4ad2da38 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:36:30 +02:00
Gilles Peskine
b0c260881f
Merge pull request #3442 from mpg/make-coverage-script-deterministic-2.16
[Backport 2.16] Make basic-build-test.sh more deterministic
2020-06-22 12:30:52 +02:00
Manuel Pégourié-Gonnard
1bff6847fb Adjust comments about SEED synchronisation
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-22 10:53:44 +02:00
Manuel Pégourié-Gonnard
c2400d38fe Make basic-build-test.sh deterministic
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-22 10:53:42 +02:00
Janos Follath
66e4dffa4e
Merge pull request #3357 from bensze01/license-2.16
[2.16] Update license headers to reflect the Apache-2.0 OR GPL-2.0-or-later licensing
2020-06-18 15:54:17 +01:00
Bence Szépkúti
948cd23845 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:49:36 +02:00
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
Bence Szépkúti
51b41d5422 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-15 12:18:56 +02:00
Bence Szépkúti
468a76f88a 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' '(' $exts ')' -print" | xargs grep -Li 'Copyright.*Arm'
# ========================

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
2020-06-15 12:18:56 +02:00
Manuel Pégourié-Gonnard
efb669c280
Merge pull request #3418 from ronald-cron-arm/ssl_write_client_hello-2.16
[Backport 2.16] Bounds checks in ssl_write_client_hello
2020-06-15 10:57:55 +02:00
Ronald Cron
9581fa3050 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 09:42:16 +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
Hanno Becker
7ea4b4d70a 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: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
danh-arm
dbe3d2b434
Merge pull request #3415 from bensze01/zeroize-2.16
[Backport 2.16] Remove hardcoded line number from the zeroize test
2020-06-10 11:31:30 +01:00
Bence Szépkúti
cd6fd060ec Remove hardcoded line number from the zeroize test
Instead, we insert a comment containing GDB_BREAK_HERE in the line we
want to break at, and let the gdb script search for it.

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
2020-06-10 11:13:41 +02:00
Bence Szépkúti
74fc3be396 Remove superfluous include from zeroize.c
This brings zeroize.c and test_zeroize.gdb in sync with development.

The include was introduced in 3b0c43063 (#2710).
Reverts ff8ae1115 from the same pull request.

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
2020-06-10 11:13:13 +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
5f5695077d Add changelog entry: fix #3394
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
471ae59783
Merge pull request #3396 from danh-arm/dh/branch-cov-2.16
Backport 2.16: Enable branch coverage in basic_build_test.sh
2020-06-08 10:15:17 +02:00
Manuel Pégourié-Gonnard
3a562d75fb
Merge pull request #3401 from mpg/fix-hmac-drbg-deps-2.16
[Backport 2.16] Fix undeclared dependencies on HMAC_DRBG
2020-06-05 11:50:11 +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
Manuel Pégourié-Gonnard
c5243c198c Add test for dependencies on HMAC_DRBG in all.sh
Similarly to the recently-added tests for dependencies on CTR_DRBG:
constrained environments will probably want only one DRBG module, and we
should make sure that tests pass in such a configuration.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-05 09:47:28 +02:00
Dan Handley
8cb1981acd Enable branch coverage in basic_build_test.sh
Enable branch coverage output in basic_build_test.sh. This
includes enabling branch coverage output to the lcov make target,
which is disabled by default.

Signed-off-by: Dan Handley <dan.handley@arm.com>
2020-06-04 16:31:59 +01:00
Jonas
7c0f1c3067 Add Changelog entry for #3318
Signed-off-by: Jonas <jonas.lejeune4420@gmail.com>
2020-06-04 13:38:32 +02:00
Manuel Pégourié-Gonnard
49d3dd0afc
Merge pull request #3384 from mpg/fix-ctr-drbg-deps-2.16
[Backport 2.16] Fix undeclared dependencies on CTR_DRBG (and add test)
2020-06-03 10:56:03 +02:00
Manuel Pégourié-Gonnard
014ff5b0d6 Add test for building without CTR_DRBG
People who prefer to rely on HMAC_DRBG (for example because they use it for
deterministic ECDSA and don't want a second DRBG for code size reasons) should
be able to build and run the tests suites without CTR_DRBG.

Ideally we should make sure the level of testing (SSL) is the same regardless
of which DRBG modules is enabled, but that's a more significant piece of work.
For now, just ensure everything builds and `make test` passes.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-03 09:54:19 +02:00
Manuel Pégourié-Gonnard
503b79e866
Merge pull request #3375 from gilles-peskine-arm/basic-build-test-status-2.16
Backport 2.16: Fix failure detection in basic-build-test.sh
2020-06-03 09:41:29 +02:00
Manuel Pégourié-Gonnard
f2c6e340de Fix undeclared deps on MBEDTLS_CTR_DRBG in tests
While at it, declare deps on ENTROPY as well.

A non-regression test will be added in a follow-up commit.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-06-02 12:16:07 +02:00