This commit adds multiple test cases to the X.509 CRT parsing test suite
exercising the stack's behaviour when facing CertificatePolicy extensions
that are malformed for a variety of reasons. It follows the same scheme
as in other negative parsing tests: For each ASN.1 component, have test
cases for (a) unexpected tag, (b) missing length, (c) invalid length
encoding, (d) length out of bounds.
This commit modifies the test
X509 CRT ASN1 (TBSCertificate v3, inv CertificatePolicies, data missing)
which exercises the behaviour of the X.509 CRT parser when facing a
CertificatePolicy extension with empty data field.
The following adaptations are made:
- The subject ID and issuer ID are modified to have length 0.
The previous values `aa` and `bb` are OK, but a generic ASN.1
parser will try to interpret them as ASN.1 tags and fail. For
maintainability, it's therefore better to use something that
can be parsed as ASN.1, and an empty ID is the easiest solution
here.
- The TBS part of the certificate wasn't followed by signature
algorithm and signature fields, which makes the test incompatible
with future changes swapping to breadth-first parsing of
certificates.
This commit moves the X.509 negative parsing tests for the
CertificatePolicy extension to the place where negative
testing of other extensions happens.
Judging from its name, the purpose of the test
TBSCertificate v3, ext CertificatePolicies tag, bool len missing
in test_suite_x509parse.data is to exercise the X.509 parsing stack's
behaviour when parsing a CertificatePolicy extension which lacks the
length field of the boolean 'Criticality' value.
However, the test fails at an earlier stage due to a mismatch of inner
and outer length of the explicit ASN.1 extensions structure.
Since we already have tests exercising
- mismatch of inner and outer length in the extensions structure, namely
'X509 CRT ASN1 (TBS, inv v3Ext, inner tag invalid)'
- missing length of the 'Criticality' field in an extension, namely
'X509 CRT ASN1 (TBS, inv v3Ext, critical length missing)'
and since for both tests there's no relevance to the use of the
policy extension OID, the test
'TBSCertificate v3, ext CertificatePolicies tag, bool len missing'
can be dropped.
Normally a valueless symbol remains valueless and a symbol with a
value keeps having one. But just in case a symbol does get changed
from valueless to having a value, make sure there's a space between
the symbol and the value. And if a symbol gets changed from having a
value to valueless, strip trailing whitespace.
Add corresponding tests.
Also fix the case of a valueless symbol added with the set method,
which would have resulted in attempting to use None as a string. This
only happened with the Python API, not with the command line API.
The test suite generator has been a Python script for a long time,
but tests/CMakeLists.txt still looked for Perl. The reference to
PYTHON_INTERP only worked due to a call to find_package(PythonInterp)
in the toplevel CMakeLists.txt, and cmake would not have printed the
expected error message if python was not available.
Run config.py with various options and store the results in files.
This script also supports the now-removed config.pl.
This is a framework to run non-regression tests on config.py: run it
with the old version, run it with the new version, and compare the
output.
This is deliberately not a functional test suite so that we don't need
to maintain a set of known outputs. When something changes in
config.py (or config.h), run the script before, run it after, and
check manually whether any differences in the output are acceptable.
git grep -Fl /config.pl | xargs sed -i -e 's!/config\.pl!/config.py!g'
Also:
* Change one comment in include/mbedtls/check_config.h.
* Change PERL to PYTHON in CMakeLists.txt.
* origin/pr/2469:
Enable MBEDTLS_MEMORY_DEBUG in memory buffer alloc test in all.sh
Remove unnecessary memory buffer alloc unsets
Disable DTLS proxy tests for MEMORY_BUFFER_ALLOC test
all.sh: restructure memory allocator tests
Add missing dependency in memory buffer alloc set in all.sh
Don't set MBEDTLS_MEMORY_DEBUG through `scripts/config.pl full`
Add cfg dep MBEDTLS_MEMORY_DEBUG->MBEDTLS_MEMORY_BUFFER_ALLOC_C
Fix memory leak in CSR test suite on failure
Fix a memory leak in x509write test suite
Add all.sh run with full config and ASan enabled
Add all.sh run with MBEDTLS_MEMORY_BUFFER_ALLOC_C enabled
Update documentation of exceptions for `config.pl full`
Adapt all.sh to removal of buffer allocator from full config
Disable memory buffer allocator in full config
Check dependencies of MBEDTLS_MEMORY_BACKTRACE in check_config.h
Tweak test data for one test case to not rely on mbedtls_asn1_get_int
lacking support for leading zeros. Instead, use a number that is
actually out of range for int.
Tweak test data for one test case to not rely on
mbedtls_asn1_get_bitstring_null rejecting bitstrings shorter than two
octets. Instead, try bit strings that are genuinely invalid, or have a
nonzero number of unused bits.
Add a test case with a correct empty signature. This is commented out
because asn1parse currently does not support this. Uncomment it when
asn1parse is updated to support this.
This leak wasn't discovered by the CI because the only test in
all.sh exercising the respective path enabled the custom memory
buffer allocator implementations of calloc() and free(), hence
bypassing ASan.
With the removal of MBEDTLS_MEMORY_BUFFER_ALLOC_C from the
full config, there are no tests for it remaining in all.sh.
This commit adds a build as well as runs of `make test` and
`ssl-opt.sh` with MBEDTLS_MEMORY_BUFFER_ALLOC_C enabled to all.sh.
Previously, numerous all.sh tests manually disabled the buffer allocator
or memory backtracting after setting a full config as the starting point.
With the removal of MBEDTLS_MEMORY_BACKTRACE and MBEDTLS_MEMORY_BUFFER_ALLOC_C
from full configs, this is no longer necessary.
* origin/development:
Fix copypasta in msg
When not using PSA crypto, disable it
Disable MEMORY_BUFFER_ALLOC with ASan
Remove config.pl calls with no effect
ssl-opt.sh: wait for proxy to start before running the script further
Adapt ChangeLog
Fix mpi_bigendian_to_host() on bigendian systems
* origin/pr/2771:
Fix copypasta in msg
When not using PSA crypto, disable it
Disable MEMORY_BUFFER_ALLOC with ASan
Remove config.pl calls with no effect
* origin/development: (42 commits)
Handle deleting non-existant files on Windows
Update submodule
Use 3rdparty headers from the submodule
Add Everest components to all.sh
3rdparty: Add config checks for Everest
Fix macros in benchmark.c
Update generated files
3rdparty: Fix inclusion order of CMakeLists.txt
Fix trailing whitespace
ECDH: Fix inclusion of platform.h for proper use of MBEDTLS_ERR_PLATFORM_FEATURE_UNSUPPORTED
ECDH: Fix use of ECDH API in full handshake benchmark
ECDH: Removed unnecessary calls to mbedtls_ecp_group_load in ECDH benchmark
ECDH: Fix Everest x25519 make_public
Fix file permissions
3rdparty: Rename THIRDPARTY_OBJECTS
3rdparty: Update description of MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED
3rdparty: Fix Makefile coding conventions
ECDSA: Refactor return value checks for mbedtls_ecdsa_can_do
Add a changelog entry for Everest ECDH (X25519)
Document that curve lists can include partially-supported curves
...
Manually edit ChangeLog to ensure correct placement of ChangeLog notes.
* origin/pr/2799: (42 commits)
Handle deleting non-existant files on Windows
Update submodule
Use 3rdparty headers from the submodule
Add Everest components to all.sh
3rdparty: Add config checks for Everest
Fix macros in benchmark.c
Update generated files
3rdparty: Fix inclusion order of CMakeLists.txt
Fix trailing whitespace
ECDH: Fix inclusion of platform.h for proper use of MBEDTLS_ERR_PLATFORM_FEATURE_UNSUPPORTED
ECDH: Fix use of ECDH API in full handshake benchmark
ECDH: Removed unnecessary calls to mbedtls_ecp_group_load in ECDH benchmark
ECDH: Fix Everest x25519 make_public
Fix file permissions
3rdparty: Rename THIRDPARTY_OBJECTS
3rdparty: Update description of MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED
3rdparty: Fix Makefile coding conventions
ECDSA: Refactor return value checks for mbedtls_ecdsa_can_do
Add a changelog entry for Everest ECDH (X25519)
Document that curve lists can include partially-supported curves
...
If we try to delete a non-existant file using del on Windows, as
can happen when running make clean, del will throw an error. Make
the Makefiles more robust by only deleting files if they exist.
This commit splits each test in ssl-opt.sh related to context serialization
in three tests, exercising the use of CCM, GCM and ChaChaPoly separately.
The reason is that the choice of primitive affects the presence and size
of an explicit IV, and we should test that space for those IVs is correctly
restored during context deserialization; in fact, this was not the case
previously, as fixed in the last commit, and was not caught by the tests
because only ChaChaPoly was tested.
* origin/development:
Update the crypto submodule
Use multipart PSA key derivation API
platform: Include stdarg.h where needed
Update Mbed Crypto to contain mbed-crypto#152
CMake: Add a subdirectory build regression test
README: Enable builds as a CMake subproject
ChangeLog: Enable builds as a CMake subproject
Remove use of CMAKE_SOURCE_DIR
Update library version to 2.18.0
Bring Mbed TLS 2.18.0 and 2.18.1 release changes back into the
development branch. We had branched to release 2.18.0 and 2.18.1 in
order to allow those releases to go out without having to block work on
the `development` branch.
Manually resolve conflicts in the Changelog by moving all freshly addded
changes to a new, unreleased version entry.
Reject changes to include/mbedtls/platform.h made in the mbedtls-2.18
branch, as that file is now sourced from Mbed Crypto.
* mbedtls-2.18:
platform: Include stdarg.h where needed
Update Mbed Crypto to contain mbed-crypto#152
CMake: Add a subdirectory build regression test
README: Enable builds as a CMake subproject
ChangeLog: Enable builds as a CMake subproject
Remove use of CMAKE_SOURCE_DIR
Update library version to 2.18.0
* origin/development: (114 commits)
Don't redefine calloc and free
Add changelog entry to record checking
Fix compiler warning
Add debug messages
Remove duplicate entries from ChangeLog
Fix parameter name in doxygen
Add missing guards for mac usage
Improve reability and debugability of large if
Fix a typo in a comment
Fix MSVC warning
Fix compile error in reduced configurations
Avoid duplication of session format header
Implement config-checking header to context s11n
Provide serialisation API only if it's enabled
Fix compiler warning: comparing signed to unsigned
Actually reset the context on save as advertised
Re-use buffer allocated by handshake_init()
Enable serialisation tests in ssl-opt.sh
Change requirements for setting timer callback
Add setting of forced fields when deserializing
...
This commit introduces a new SSL error code
`MBEDTLS_ERR_SSL_VERSION_MISMATCH`
which can be used to indicate operation failure due to a
mismatch of version or configuration.
It is put to use in the implementation of `mbedtls_ssl_session_load()`
to signal the attempt to de-serialize a session which has been serialized
in a build of Mbed TLS using a different version or configuration.
This commit improves the test exercising the behaviour of
session deserialization when facing an unexpected version
or config, by testing ver/cfg corruption at any bit in the
ver/cfg header of the serialized data; previously, it had
only tested the first bit of each byte.
The size of the ticket used in this test dropped from 192 to 143 bytes, so
move all sizes used in this test down 50 bytes. Also, we now need to adapt the
server response size as the default size would otherwise collide with the new
mtu value.
The chosen fix matches what's currently done in the baremetal branch - except
the `#ifdef` have been adapted because now in baremetal the digest is not kept
if renegotiation is disabled.
We have explicit recommendations to use US spelling for technical writing, so
let's apply this to code as well for uniformity. (My fingers tend to prefer UK
spelling, so this needs to be fixed in many places.)
sed -i 's/\([Ss]eriali\)s/\1z/g' **/*.[ch] **/*.function **/*.data ChangeLog
This test works regardless of the serialisation format and embedded pointers
in it, contrary to the load-save test, though it requires more maintenance of
the test code (sync the member list with the struct definition).
This uncovered a bug that led to a double-free (in practice, in general could
be free() on any invalid value): initially the session structure is loaded
with `memcpy()` which copies the previous values of pointers peer_cert and
ticket to heap-allocated buffers (or any other value if the input is
attacker-controlled). Now if we exit before we got a chance to replace those
invalid values with valid ones (for example because the input buffer is too
small, or because the second malloc() failed), then the next call to
session_free() is going to call free() on invalid pointers.
This bug is fixed in this commit by always setting the pointers to NULL right
after they've been read from the serialised state, so that the invalid values
can never be used.
(An alternative would be to NULL-ify them when writing, which was rejected
mostly because we need to do it when reading anyway (as the consequences of
free(invalid) are too severe to take any risk), so doing it when writing as
well is redundant and a waste of code size.)
Also, while thinking about what happens in case of errors, it became apparent
to me that it was bad practice to leave the session structure in an
half-initialised state and rely on the caller to call session_free(), so this
commit also ensures we always clear the structure when loading failed.
This test appeared to be passing for the wrong reason, it's actually not
appropriate for the current implementation. The serialised data contains
values of pointers to heap-allocated buffers. There is no reason these should
be identical after a load-save pair. They just happened to be identical when I
first ran the test due to the place of session_free() in the test code and the
fact that the libc's malloc() reused the same buffers. The test no longer
passes if other malloc() implementations are used (for example, when compiling
with asan which avoids re-using the buffer, probably for better error
detection).
So, disable this test for now (we can re-enable it when we changed how
sessions are serialised, which will be done in a future PR, hence the name of
the dummy macro in depends_on). In the next commit we're going to add a test
that save-load is the identity instead - which will be more work in testing as
it will require checking each field manually, but at least is reliable.
This initial test ensures that a load-save function is the identity. It is so
far incomplete in that it only tests sessions without tickets or certificate.
This will be improved in the next commits.
While 'session hash' is currently unique, so suitable to prove that the
intended code path has been taken, it's a generic enough phrase that in the
future we might add other debug messages containing it in completely unrelated
code paths. In order to future-proof the accuracy of the test, let's use a
more specific string.
* origin/development: (51 commits)
Fix possibly-lossy conversion warning from MSVC
Reintroduce length 0 check for records
Don't use memcpy() for 2-byte copy operation
Remove integer parsing macro
Fix alignment in record header parsing routine
Don't disallow 'record from another epoch' log msg in proxy ref test
Make sure 'record from another epoch' is displayed for next epoch
Implement record checking API
Mark ssl_parse_record_header() as `const` in SSL context
Make mbedtls_ssl_in_hdr_len() CID-unaware
Remove duplicate setting of ssl->in_msgtype and ssl->in_msglen
Move update of in_xxx fields in ssl_get_next_record()
Move update of in_xxx fields outside of ssl_prepare_record_content()
Reduce dependency of ssl_prepare_record_content() on in_xxx fields
Move ssl_update_in_pointers() to after record hdr parsing
Mark DTLS replay check as `const` on the SSL context
Move updating the internal rec ptrs to outside of rec hdr parsing
Mark ssl_decrypt_buf() as `const in the input SSL context
Adapt ssl_prepare_record_content() to use SSL record structure
Use record length from record structure when fetching content in TLS
...
* origin/pr/2790: (40 commits)
Fix possibly-lossy conversion warning from MSVC
Reintroduce length 0 check for records
Don't use memcpy() for 2-byte copy operation
Remove integer parsing macro
Fix alignment in record header parsing routine
Don't disallow 'record from another epoch' log msg in proxy ref test
Make sure 'record from another epoch' is displayed for next epoch
Implement record checking API
Mark ssl_parse_record_header() as `const` in SSL context
Make mbedtls_ssl_in_hdr_len() CID-unaware
Remove duplicate setting of ssl->in_msgtype and ssl->in_msglen
Move update of in_xxx fields in ssl_get_next_record()
Move update of in_xxx fields outside of ssl_prepare_record_content()
Reduce dependency of ssl_prepare_record_content() on in_xxx fields
Move ssl_update_in_pointers() to after record hdr parsing
Mark DTLS replay check as `const` on the SSL context
Move updating the internal rec ptrs to outside of rec hdr parsing
Mark ssl_decrypt_buf() as `const in the input SSL context
Adapt ssl_prepare_record_content() to use SSL record structure
Use record length from record structure when fetching content in TLS
...
* development:
Update crypto to a repo with latest crypto
Update Mbed Crypto
tls: Remove duplicate psa_util.h include
Remove unused cryptography test files
Remove crypto C files
Remove files sourced from Mbed Crypto
config: Fix Doxygen link to MBEDTLS_PARAM_FAILED
Use mbedtls-based path for includes
check-names: Consider crypto-sourced header files
It happens regularly in test runs that the server example application
shuts down a connection, goes into waiting mode for a new connection,
and then receives the encrypted ClosureAlert from the client. The only
reason why this does currently not trigger the 'record from another epoch'
message is that we handle ClientHello parsing outside of the main record
stack because we want to be able to detect SSLv2 ClientHellos. However,
this is likely to go away, and once it happens, we'll see the log message.
Further, when record checking is used, every record, including the mentioned
closure alert, is passed to the record checking API before being passed to
the rest of the stack, which leads to the log message being printed.
In summary, grepping for 'record from another epoch' is a fragile way
of checking whether a reordered message has arrived. A more reliable
way is to grep for 'Buffer record from epoch' which is printed when
a record from a future epoch is actually buffered, and 'ssl_buffer_message'
which is the function buffering a future handshake message.
compat.sh used to skip OpenSSL altogether for DTLS 1.2, because older
versions of OpenSSL didn't support it. But these days it is supported.
We don't want to use DTLS 1.2 with OpenSSL unconditionally, because we
still use legacy versions of OpenSSL to test with legacy ciphers. So
check whether the version we're using supports it.
Without any -O option, the default is -O0, and then the assembly code
is not used, so this would not be a non-regression test for the
assembly code that doesn't build.
