An earlier botched backport (d56ca658ab)
had the wrong name for a variable and a missing header inclusion.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
The intended logic around MBEDTLS_xxx_ALT is to exclude them from full
because they require the alternative implementation of one or more
library functions, except that MBEDTLS_PLATFORM_xxx_ALT are different:
they're alternative implementations of a platform function and they
have a built-in default, so they should be included in full. Document
this.
Fix a bug whereby MBEDTLS_PLATFORM_xxx_ALT didn't catch symbols where
xxx contains an underscore. As a consequence, MBEDTLS_PLATFORM_NV_SEED_ALT
is now enabled in the full config. Explicitly exclude
MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT because it behaves like the
non-platform ones, requiring an extra build-time dependency.
Explicitly exclude MBEDTLS_PLATFORM_NV_SEED_ALT from baremetal
because it requires MBEDTLS_ENTROPY_NV_SEED, and likewise explicitly
unset it from builds that unset MBEDTLS_ENTROPY_NV_SEED.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
- "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>
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>
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>
(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
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.
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.
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.
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.
* 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()
