Commit graph

2055 commits

Author SHA1 Message Date
Gilles Peskine cd7b0422ef 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 11:24:40 +02:00
Gilles Peskine bfda033f3d 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 11:23:11 +02:00
Gilles Peskine 5d32e64ad0
Merge pull request #3170 from gilles-peskine-arm/check-windows-files-2.7
Backport 2.7: Check Windows files for sanity as well
2020-04-20 13:59:22 +02:00
Gilles Peskine bc8c513ecb
Merge pull request #3185 from gilles-peskine-arm/pylint-up-to-2.4-2.7
Backport 2.7: Pass Pylint up to 2.4
2020-04-20 09:47:54 +02:00
Jaeden Amero 8aaf2c5861
Merge pull request #3196 from piotr-now/max_pathlen_overflow_mbedtls-2.7
Backport 2.7: Guard from undefined behaviour in case of an INT_MAX max_pathlen
2020-04-17 14:24:45 +01: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
Gilles Peskine e61988a766
Merge pull request #3165 from ronald-cron-arm/unmet-dependencies-buffer-overflow-fix-2.7
[backport 2.7] Unmet dependencies buffer overflow fix
2020-04-17 10:08:18 +02:00
Gilles Peskine b5847d20d3 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 21:08:26 +02:00
Gilles Peskine 558e26dbda Document more methods in Python scripts
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-11 21:07:43 +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
Gilles Peskine 70ef5c6c9a 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:39:06 +02:00
Gilles Peskine 783da6345e Check that Windows files have Windows line endings
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-06 16:39:04 +02:00
Gilles Peskine 227dfd435e 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:39:03 +02:00
Gilles Peskine 318f15ef92 Sort the list for easier maintenance
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-06 16:39:02 +02:00
Gilles Peskine c7153226f6 List each item on a separate line for easier maintenance
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-06 16:39:00 +02:00
Gilles Peskine c251e0d75a 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:38:58 +02:00
Ronald Cron 75d26b5d17 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
report if some unmet dependencies are missing.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-04-06 14:20:26 +02:00
Ronald Cron 1d3eab684c 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 14:20:26 +02:00
Ronald Cron 59f2139df0 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 14:20:18 +02:00
Ronald Cron eb5d0e9f4f unit tests: Backport ARRAY_LENGTH macro
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-04-06 10:34:54 +02: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
Gilles Peskine 67665509ab Note that unmet_dependencies is only filled in verbose mode
Warn about a gotcha that caused a bug in development.

Ensure that it's at least zeroed out, rather than uninitialized, in
non-verbose mode.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-04-01 09:59:54 +02:00
Manuel Pégourié-Gonnard b1ee30bfe9 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:57:45 +02:00
Manuel Pégourié-Gonnard a58b04649b 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:57:45 +02:00
Manuel Pégourié-Gonnard d6e4454de8 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:58:20 +01:00
Manuel Pégourié-Gonnard eb22ad2ddc
Merge pull request #2440 from andresag01/mbedtls-2.7-iotssl-2544-deprecate-record-accel
Backport 2.7: Fix compilation failure when MBEDTLS_SSL_HW_RECORD_ACCEL is enabled
2020-03-16 10:37:32 +01:00
Manuel Pégourié-Gonnard aa719e78ce Align some timeouts with the 2.16 branch in ssl-opt.sh
This commit only addresses the timeouts in the "DTLS proxy: 3d, ..." tests.
The discrepancy with the 2.16 branch became apparent for some of these tests
when backporting the previous commit (skip_close_nofity), so let's align the
whole series for consistency and to make future backporting easier.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-03-03 10:15:50 +01:00
Manuel Pégourié-Gonnard 5e261e958c 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-03-03 10:15:26 +01:00
Andres Amaya Garcia bb13e3b8e4 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: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 9c16ede466 Check for buffer overflow in test function 2020-02-19 09:45:02 +01:00
Manuel Pégourié-Gonnard 355bbc0e70 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 11:27:08 +01:00
Manuel Pégourié-Gonnard bfd0259d26 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 11:27:08 +01:00
Manuel Pégourié-Gonnard 37d0dfc0c5
Merge pull request #3030 from gilles-peskine-arm/test-opt-all-2.7
Backport 2.7: Fix and test the full config with gcc and clang
2020-02-11 09:17:14 +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 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
Gilles Peskine fa0e8b51c4 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:06:31 +01:00
Gilles Peskine c9247122e3 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:06:31 +01:00
Gilles Peskine 261aea1956 shrink tests: clearer description 2020-02-03 16:35:01 +01:00
Gilles Peskine 84b8e25426 Better coverage for copy and swap
Cover more cases: different signs, different zeronesses, repeated
argument.
2020-02-03 16:34:51 +01:00
Gilles Peskine 6f43c6038e Bignum copy/shrink: More precise test case descriptions 2020-02-03 16:27:56 +01:00
Manuel Pégourié-Gonnard 179c227203 Fix CA encoding issue with gnutls-cli
In the 2.7 branch, test-ca.crt has all the components of its Subject name
encoded as PrintableString, because it's generated with our cert_write
program, and our code writes all components that way until Mbed TLS 2.14.

But the default RSA SHA-256 certificate, server2-sha256.crt, has the O and CN
components of its Issuer name encoded as UTF8String, because it was generated
with OpenSSL and that's what OpenSSL does, regardless of how those components
were encoded in the CA's Subject name.

This triggers some overly strict behaviour in some libraries, most notably NSS
and GnuTLS (of interest to us in ssl-opt.sh) which won't recognize the trusted
root as a possible parent for the presented certificate, see for example:
https://github.com/ARMmbed/mbedtls/issues/1033

Fortunately, we have at our disposal a version of test-ca.crt with encodings
matching the ones in server2-sha256.crt, in the file test-ca_utf8.crt. So
let's append that to gnutls-cli's list of trusted roots, so that it recognizes
certs signed by this CA but with the O and CN components as UTF8String.

Note: Since https://github.com/ARMmbed/mbedtls/pull/1641 was merged (in Mbed
TLS 2.14), we changed how we encode those components, so in the 2.16 branch,
cert_write generates test-ca.crt with encodings that matches the ones used by
openssl when generating server2-sha256.crt, so the issue of gnutls-cli
rejecting server2-sha256.crt is specific to the 2.7 branch.
2020-02-03 15:55:43 +01:00
Manuel Pégourié-Gonnard 350823d0da
Merge pull request #2976 from mpg/add-zlib-tests-2.7
[2.7] Add zlib tests
2020-01-31 09:22:36 +01:00
Manuel Pégourié-Gonnard 37abf12631 De-duplicate SHA1-independent test in ssl-opt.sh
The splitting of this test into two versions depending on whether SHA-1 was
allowed by the server was a mistake in
5d2511c4d4 - the test has nothing to do with
SHA-1 in the first place, as the server doesn't request a certificate from
the client so it doesn't matter if the server accepts SHA-1 or not.
2020-01-30 12:49:43 +01:00
Manuel Pégourié-Gonnard a92990a082 Fix ssl-opt.sh for GnuTLS versions rejecting SHA-1
While the whole script makes (often implicit) assumptions about the version of
GnuTLS used, generally speaking it should work out of the box with the version
packaged on our reference testing platform, which is Ubuntu 16.04 so far.

With the update from Jan 8 2020 (3.4.10-4ubuntu1.6), the patches for rejecting
SHA-1 in certificate signatures were backported, so we should avoid presenting
SHA-1 signed certificates to a GnuTLS peer in ssl-opt.sh.
2020-01-30 11:19:45 +01: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
Manuel Pégourié-Gonnard 2150fb22c8 Add detection for zlib headers to all.sh 2020-01-29 09:51:56 +01:00
Gilles Peskine 16ba09c621 Check that mbedtls_mpi_grow succeeds 2020-01-22 19:09:05 +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