Commit graph

2669 commits

Author SHA1 Message Date
Gilles Peskine 67c3c3fc1b all.sh: make the arm-gcc cross-compiler prefix configurable
Make it possible to use a compiler that isn't in $PATH, or that's
installed with a different name, or even a compiler for a different
target such as arm-linux-gnueabi.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-27 09:14:42 +00:00
Gilles Peskine 5bd9f56d15 all.sh: run selftest in the full config and with ASan
Almost everything the selftest program does is in the test suites. But
just in case run the selftest program itself once in the full
configuration, and once in the default configuration with ASan, in
addition to running it out of box.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-27 09:14:42 +00:00
Gilles Peskine ff873432d4
Merge pull request #3207 from mpg/check-bounds-of-test-intex-in-suites-2.16
[Backport 2.16] Add lower bound check to function index lookup
2020-04-20 15:19:47 +02:00
Gilles Peskine 15316fdb94
Merge pull request #3169 from gilles-peskine-arm/check-windows-files-2.16
Backport 2.16: Check Windows files for sanity as well
2020-04-20 13:59:18 +02:00
k-stachowiak 531b00bb39 Make negative function_id value impossible in test suites
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-04-20 10:04:25 +02:00
Gilles Peskine 12c299caea
Merge pull request #3184 from gilles-peskine-arm/pylint-up-to-2.4-2.16
Backport 2.16: Pass Pylint up to 2.4
2020-04-20 09:47:46 +02:00
Jaeden Amero da1d437389
Merge pull request #3197 from piotr-now/max_pathlen_overflow_mbedtls-2.16
Backport 2.16: Guard from undefined behaviour in case of an INT_MAX max_pathlen
2020-04-17 14:24:49 +01: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
Gilles Peskine a8bc32872a
Merge pull request #3164 from ronald-cron-arm/unmet-dependencies-buffer-overflow-fix-2.16
[backport 2.16] Unmet dependencies buffer overflow fix
2020-04-17 10:08:24 +02:00
Gilles Peskine 399b82f986 Pylint: minor code simplifications
Simplify the code in minor ways. Each of this changes fixes a warning
from Pylint 2.4 that doesn't appear with Pylint 1.7.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-11 20:51:08 +02:00
Gilles Peskine 5d1dfd4108 Pylint: abide by useless-object-inheritance warnings
Inheriting from object is a remainder of Python 2 habits and is just
clutter in Python 3.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-11 20:50:51 +02:00
Gilles Peskine a1bb3f86e9 mbedtls_test.py: drop compatibility with Python 2
Python 2 is no longer supported upstream. Actively drop compatibility
with Python 2.

Removing the inheritance of a class on object pacifies recent versions
of Pylint (useless-object-inheritance).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-11 20:50:51 +02:00
Gilles Peskine af67f8db77 Document more methods in Python scripts
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-11 20:50:49 +02:00
Gilles Peskine 8c3ad4bfcb Make check_python_files non-optional in all.sh
check_python_files was optional in all.sh because we used to have CI
machines where pylint wasn't available. But this had the downside that
check_python_files kept breaking because it wasn't checked in the CI.
Now our CI has pylint and check_python_files should not be optional.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-11 20:38:21 +02:00
Gilles Peskine c373000307 Make sure to use a Python 3 pylint
On some systems, such as Ubuntu up to 19.04, `pylint` is for Python 2
and `pylint3` is for Python 3, so we should not use `pylint` even if
it's available.

Use the Python module instead of the trivial shell wrapper. This way
we can make sure to use the correct Python version.

Fix #3111

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-11 20:38:21 +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
Gilles Peskine 368ccd416b In Windows files, detect CR without LF as well as LF without CR
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-06 16:28:53 +02:00
Gilles Peskine 0d5b016709 Check that Windows files have Windows line endings
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-06 16:28:52 +02:00
Gilles Peskine cecc726b91 Also check Windows files
Check Windows files for some issues, including permissions. Omit the
checks related to special characters (whitespace, line endings,
encoding) as appropriate.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-06 16:28:50 +02:00
Gilles Peskine 1978b68a2f Sort the list for easier maintenance
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-06 16:28:45 +02:00
Gilles Peskine eb9929e6e1 List each item on a separate line for easier maintenance
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-06 16:27:14 +02:00
Gilles Peskine d69f51b216 Clarify confusion between file names and suffixes of file names
To test a file name exactly, prepend a / to the base name.

files_to_check actually checks suffixes, not file names, so rename it
to extensions_to_check.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-06 16:20:09 +02:00
Ronald Cron b19ad118dd unit tests: Indicate missing unmet dependencies
The identifiers of the unmet dependencies of a test case are
stored in a buffer of fixed size that can be potentially too
small to store all the unmet dependencies. Indicate in test
reports if some unmet dependencies are missing.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-04-06 11:58:27 +02:00
Ronald Cron 69cc630750 unit tests: Fix potential buffer overflow
Fix potential buffer overflow when tracking the unmet dependencies
of a test case. The identifiers of unmet dependencies are stored
in an array of fixed size. Ensure that we don't overrun the array.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-04-06 11:58:27 +02:00
Gilles Peskine bde4d3045b Prefer unsigned types for non-negative numbers
Use size_t for some variables that are array indices.
Use unsigned for some variables that are counts of "small" things.

This is a backport of commit 3c1c8ea3e7.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-04-06 11:57:36 +02:00
Ronald Cron 07c83f27ad unit tests: Backport ARRAY_LENGTH macro
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-04-06 10:23:05 +02: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 901e87e5d1
Merge pull request #3124 from AndrzejKurek/typo_asn1_tests
[mbedtls-2.16] Typo fix in test_suite_asn1write.function
2020-04-01 12:27:10 +02:00
Gilles Peskine e8caf2634b Since unmet_dependencies stores integers, represent them as int
Since unmet_dependencies only ever contains strings that are integers
written out in decimal, store the integer instead. Do this
unconditionally since it doesn't cost any extra memory.

This commit saves a little memory and more importantly avoids a gotcha
with uninitialized pointers which caused a bug on development (the
array was only initialized in verbose mode).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-03-31 10:57:32 +02:00
Manuel Pégourié-Gonnard 34cbf106f3 Adjust timeout of tests with "no resend" assertions
There are currently 4 tests in ssl-opt.sh with either -C "resend" or -S
"resend", that is, asserting that no retransmission will occur. They sometimes
fail on loaded CI machines as one side doesn't send a message fast enough,
causing the other side to retransmit, causing the test to fail.

(For the "reconnect" test there was an other issue causing random failures,
fixed in a previous commit, but even after that fix the test would still
sometimes randomly fail, even if much more rarely.)

While it's a hard problem to fix in a general and perfect way, in practice the
probability of failures can be drastically reduced by making the timeout
values much larger.

For some tests, where retransmissions are actually expected, this would have
the negative effect of increasing the average running time of the test, as
each side would wait for longer before it starts retransmission, so we have a
trade-off between average running time and probability of spurious failures.

But for tests where retransmission is not expected, there is no such trade-off
as the expected running time of the test (assuming the code is correct most of
the time) is not impacted by the timeout value. So the only negative effect of
increasing the timeout value is on the worst-case running time on the test,
which is much less important, as test should only fail quite rarely.

This commit addresses the easy case of tests that don't expect retransmission
by increasing the value of their timeout range to 10s-20s. This value
corresponds to the value used for tests that assert `-S "autoreduction"` which
are in the same case and where the current value seems acceptable so far.

It also represents an increase, compared to the values before this commit, of
a factor 20 for the "reconnect" tests which were frequently observed to fail
in the CI, and of a factor 10 for the first two "DTLS proxy" tests, which were
observed to fail much less frequently, so hopefully the new values are enough
to reduce the probability of spurious failures to an acceptable level.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-03-31 09:37:31 +02:00
Manuel Pégourié-Gonnard b85ce9eaeb Add negative test for hard reconnect cookie check
The server must check client reachability (we chose to do that by checking a
cookie) before destroying the existing association (RFC 6347 section 4.2.8).
Let's make sure we do, by having a proxy-in-the-middle inject a ClientHello -
the server should notice, but not destroy the connection.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-03-31 09:37:28 +02:00
Andrzej Kurek 5c15847086
Typo fix in test_suite_asn1write.function
This commit fixes issue #2782, spotted by tmarti2.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
2020-03-25 07:48:11 -04:00
Manuel Pégourié-Gonnard 98a879a806 Improve some test names in ssl-opt.sh
- "Default" should only be used for tests that actually use the defaults (ie,
  not passing options on the command line, except maybe debug/dtls)
- All tests in the "Encrypt then MAC" group should start with that string as a
  common prefix

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-03-24 10:57:23 +01:00
Simon Butcher 47f728718f Change the use of pylint to optionally use pylint3
Pylint when installed as a distro package can be installed as pylint3, whilst as
a PEP egg, it can be installed as pylint.

This commit changes the scripts to first use pylint if installed, and optionally
look for pylint3 if not installed. This is to allow a preference for the PEP
version over the distro version, assuming the PEP one is more likely to be
the correct one.

Signed-off-by: Simon Butcher <simon.butcher@arm.com>
2020-03-16 13:53:06 +00:00
Manuel Pégourié-Gonnard acfa36890c
Merge pull request #2439 from andresag01/mbedtls-2.16-iotssl-2544-deprecate-record-accel
Backport 2.16: Fix compilation failure when MBEDTLS_SSL_HW_RECORD_ACCEL is enabled
2020-03-16 10:37:24 +01:00
Andres Amaya Garcia 9f3bdb87e5 Add test for MBEDTLS_SSL_HW_RECORD_ACCEL in all.sh
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-02-26 10:14:28 +01:00
Manuel Pégourié-Gonnard 498e632b08 Fix possible close_notify/ClientHello confusion
The ssl-opt.sh test cases using session resumption tend to fail occasionally
on the CI due to a race condition in how ssl_server2 and ssl_client2 handle
the reconnection cycle.

The server does the following in order:
- S1 send application data
- S2 send a close_notify alert
- S3 close the client socket
- S4 wait for a "new connection" (actually a new datagram)
- S5 start a handshake

The client does the following in order:
- C1 wait for and read application data from the server
- C2 send a close_notify alert
- C3 close the server socket
- C4 reset session data and re-open a server socket
- C5 start a handshake

If the client has been able to send the close_notify (C2) and if has been
delivered to the server before if closes the client socket (S3), when the
server reaches S4, the datagram that we start the new connection will be the
ClientHello and everything will be fine.

However if S3 wins the race and happens before the close_notify is delivered,
in S4 the close_notify is what will be seen as the first datagram in a new
connection, and then in S5 this will rightfully be rejected as not being a
valid ClientHello and the server will close the connection (and go wait for
another one). The client will then fail to read from the socket and exit
non-zero and the ssl-opt.sh harness will correctly report this as a failure.

In order to avoid this race condition in test using ssl_client2 and
ssl_server2, this commits introduces a new command-line option
skip_close_notify to ssl_client2 and uses it in all ssl-opt.sh tests that use
session resumption with DTLS and ssl_server2.

This works because ssl_server2 knows how many messages it expects in each
direction and in what order, and closes the connection after that rather than
relying on close_notify (which is also why there was a race in the first
place).

Tests that use another server (in practice there are two of them, using
OpenSSL as a server) wouldn't work with skip_close_notify, as the server won't
close the connection until the client sends a close_notify, but for the same
reason they don't need it (there is no race between receiving close_notify and
closing as the former is the cause of the later).

An alternative approach would be to make ssl_server2 keep the connection open
until it receives a close_notify. Unfortunately it creates problems for tests
where we simulate a lossy network, as the close_notify could be lost (and the
client can't retransmit it). We could modify udp_proxy with an option to never
drop alert messages, but when TLS 1.3 comes that would no longer work as the
type of messages will be encrypted.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-02-26 09:33:05 +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 96ed13e21b Test each failure mode of pk_parse_key_pkcs1_der()
(Only the top-level ones, ie, for each call to eg asn1_get_mpi(), ensure
there's at least one test case that makes this call fail in one way, but don't
test the various ways to make asn1_get_mpi fail - that should be covered
elsewhere.)

- the new checks added by the previous commits needed exercising
- existing tests sometimes had wrong descriptions or where passing for the
  wrong reason (eg with the "length mismatch" test, the function actually
failed before reaching the length check)
- while at it, add tests for the rest as well

The valid minimal-size key was generated with:

openssl genrsa 128 2>/dev/null | openssl rsa -outform der 2>/dev/null | xxd -p
2020-02-18 10:49:06 +01:00
Manuel Pégourié-Gonnard 12fb9c383d Clean up test function pk_parse_key
- remove incorrect compile-time dependency (the individual cases already have
  correct run-time dependency information)
- remove unused argument
- remove unused stack buffer
- remove useless code block
2020-02-18 10:49:06 +01:00
Manuel Pégourié-Gonnard a59988e197
Merge pull request #3029 from gilles-peskine-arm/test-opt-all-2.16
Backport 2.16: Fix and test the full config with gcc and clang
2020-02-11 09:17:09 +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 82ebf511e9
Merge pull request #3019 from mpg/fix-ssl-opt-gnutls-no-sha1-2.16
[backport 2.16] Fix ssl-opt.sh for GnuTLS versions rejecting SHA-1
2020-02-04 11:18:30 +00:00
Gilles Peskine 0fe92c2f4f Test GCC and Clang with common build options
Goals:
* Build with common compilers with common options, so that we don't
  miss a (potentially useful) warning only triggered with certain
  build options.
* A previous commit removed -O0 test jobs, leaving only the one with
  -m32. We have inline assembly that is disabled with -O0, falling
  back to generic C code. This commit restores a test that runs the
  generic C code on a 64-bit platform.
2020-02-03 20:03:39 +01:00
Gilles Peskine 99d70d8cb1 Replace -O0 by -O1 or -Os in most components
Gcc skips some analyses when compiling with -O0, so we may miss
warnings about things like uninitialized variables.
2020-02-03 20:03:39 +01:00
Gilles Peskine 8ece8572b2 shrink tests: clearer description 2020-02-03 16:21:32 +01:00
Gilles Peskine e0ced3a3d6 Move test functions from Lilliput to Blefuscu
We normally represent bignums in big-endian order and there is no
reason to deviate here.
2020-02-03 16:21:31 +01:00
Gilles Peskine 8fe3b79cdb Better coverage for copy and swap
Cover more cases: different signs, different zeronesses, repeated
argument.
2020-02-03 16:21:31 +01:00
Gilles Peskine e6cca7c937 Bignum copy/shrink: More precise test case descriptions 2020-02-03 16:21:31 +01:00
Manuel Pégourié-Gonnard a7b9007d60
Merge pull request #2972 from mpg/add-zlib-tests-2.16
[2.16] Add zlib tests
2020-01-31 09:22:30 +01:00