If we disable or enable a message locally, it's by design. There's no
need to clutter the Pylint output with this information.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
If we take the trouble of using pass, it's because we think the code
is clearer that way. For example, Pylint 2.4 rejects pass in
def foo():
"""Do nothing."""
pass
But relying on a docstring as the sole code is weird, hence the use of
pass.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
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>
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>
Pylint warns about things like ``log.info('...'.format(...))``.
It insists on ``log.info('...', ...)``.
This is of minor utility (mainly a performance gain when there are
many messages that use formatting and are below the log level).
Some versions of Pylint (including 1.8, which is the version on
Ubuntu 18.04) only recognize old-style format strings using '%',
and complain about something like ``log.info('{}', foo)`` with
logging-too-many-args (Pylint supports new-style formatting if
declared globally with logging_format_style under [LOGGING] but
this requires Pylint >=2.2).
Disable this warning to remain compatible with Pylint 1.8 and not have
to change abi_check.py to use %-formats instead of {}-formats when
logging.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
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>
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>
Adding .function was necessary, as otherwise ctags would have no idea what to
do with those files.
Adding .h may not be necessary, as by default ctags considers them C++ which
is probably good enough, but since we're tuning the mapping anyway...
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The current logging was sub-standard, in particular there was no trace
whatsoever of the HelloVerifyRequest being sent. Now it's being logged with
the usual levels: 4 for full content, 2 return of f_send, 1 decision about
sending it (or taking other branches in the same function) because that's the
same level as state changes in the handshake, and also same as the "possible
client reconnect" message" to which it's the logical continuation (what are we
doing about it?).
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
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>
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>
In x509.c, the self-test code is dependent on MBEDTLS_CERTS_C and
MBEDTLS_SHA256_C being enabled. At some point in the recent past that dependency
was on MBEDTLS_SHA1_C but changed to SHA256, but the comment wasn't updated.
This commit updates the comment.
Signed-off-by: Simon Butcher <simon.butcher@arm.com>
Additional sanity checks in check_config.h to ensure:
* if test certificates are included (MBEDTLS_CERTS_C) there must be also be
support for the core X509 feature (MBEDTLS_X509_USE_C). This has a
secondary dependency on the public key abstraction layer (MBEDTLS_PK_C),
necessary as the certificates will either be signed by RSA or ECDSA, and
therefore need to be part of the library.
* if any of the TLS protocols are defined (MBEDTLS_SSL_PROTO_xxx) then a
key exchange method must also be defined (MBEDTLS_KEY_EXCHANGE_xxx).
Anyone who knows the library will probably not make these mistakes or will
quickly diagnose and fix them, but it is possible to compile and link both
configurations if you build only the library and not the example programs, and
therefore users may not realise immediately that there's a mistake, only
discovering it at runtime.
These checks may therefore save someone some time.
Signed-off-by: Simon Butcher <simon.butcher@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>
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>
`pylint3 --version` will output to stderr the status of the config file it's
using. This can be "No config file found" or "Using config file" or nothing.
This means the pylint version may or may not be on the first line.
Therefore this commit changes the filters on the pylint3 version output to first
strip out the config line, and then to select only the pylint line.
Signed-off-by: Simon Butcher <simon.butcher@arm.com>
Add the versions of Python, Perl, and Pylint to the version dump provided by
the output_env.sh script.
Signed-off-by: Simon Butcher <simon.butcher@arm.com>
A number of clean-up improvements following review.
* removal of redundant `` quotes
* removal of non-portable echo "\n", in favour of additional echo commands
* change to use of uname to detemine if the platform is Linux or not
* revised formatting of output
* change to dpkg-query from dpkg to find installed libasan variants
Co-Authored-By: Gilles Peskine <gilles.peskine@arm.com>
Signed-off-by: Simon Butcher <simon.butcher@arm.com>
This commit adds additional information to the output_env.sh script of:
* Linux distribution version (if available)
* GDB version (if available)
It also makes some information clearer:
* the type of OpenSSL/GNUTLS version (legacy/default/next)
* and whether certain versions are not installed, or not configured
And it simplifies the error messages for absent tools.
Signed-off-by: Simon Butcher <simon.butcher@arm.com>
The Mbed TLS project no longer requires a CLA. Contributions from now on
must be made under both Apache-2.0 AND GPL-2.0-or-later licenses, to enable
LTS (Long Term Support) branches of the software to continue to be provided
under either Apache-2.0 OR GPL-2.0-or-later. Contributors must accept the
terms of the Developer Certificate of Origin (DCO) by adding a Signed-off-by:
line to each commit message.
The software on the development branch continues to be provided under
Apache-2.0.
Update README.md, CONTRIBUTING.md and pull_request_template.md to explain
the new licensing model. Add a copy of the DCO to the project.
Signed-off-by: Dan Handley <dan.handley@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>
