Some functions within the X.509 module return an ASN.1 low level
error code where instead this error code should be wrapped by a
high-level X.509 error code as in the bulk of the module.
Specifically, the following functions are affected:
- mbedtls_x509_get_ext()
- x509_get_version()
- x509_get_uid()
This commit modifies these functions to always return an
X.509 high level error code.
Care has to be taken when adapting `mbetls_x509_get_ext()`:
Currently, the callers `mbedtls_x509_crt_ext()` treat the
return code `MBEDTLS_ERR_ASN1_UNEXPECTED_TAG` specially to
gracefully detect and continue if the extension structure is not
present. Wrapping the ASN.1 error with
`MBEDTLS_ERR_X509_INVALID_EXTENSIONS` and adapting the check
accordingly would mean that an unexpected tag somewhere
down the extension parsing would be ignored by the caller.
The way out of this is the following: Luckily, the extension
structure is always the last field in the surrounding structure,
so if there is some data remaining, it must be an Extension
structure, so we don't need to deal with a tag mismatch gracefully
in the first place.
We may therefore wrap the return code from the initial call to
`mbedtls_asn1_get_tag()` in `mbedtls_x509_get_ext()` by
`MBEDTLS_ERR_X509_INVALID_EXTENSIONS` and simply remove
the special treatment of `MBEDTLS_ERR_ASN1_UNEXPECTED_TAG`
in the callers `x509_crl_get_ext()` and `x509_crt_get_ext()`.
This renders `mbedtls_x509_get_ext()` unsuitable if it ever
happened that an Extension structure is optional and does not
occur at the end of its surrounding structure, but for CRTs
and CRLs, it's fine.
The following tests need to be adapted:
- "TBSCertificate v3, issuerID wrong tag"
The issuerID is optional, so if we look for its presence
but find a different tag, we silently continue and try
parsing the subjectID, and then the extensions. The tag '00'
used in this test doesn't match either of these, and the
previous code would hence return LENGTH_MISMATCH after
unsucessfully trying issuerID, subjectID and Extensions.
With the new code, any data remaining after issuerID and
subjectID _must_ be Extension data, so we fail with
UNEXPECTED_TAG when trying to parse the Extension data.
- "TBSCertificate v3, UIDs, invalid length"
The test hardcodes the expectation of
MBEDTLS_ERR_ASN1_INVALID_LENGTH, which needs to be
wrapped in MBEDTLS_ERR_X509_INVALID_FORMAT now.
Fixes#2431.
When parsing a substructure of an ASN.1 structure, no field within
the substructure must exceed the bounds of the substructure.
Concretely, the `end` pointer passed to the ASN.1 parsing routines
must be updated to point to the end of the substructure while parsing
the latter.
This was previously not the case for the routines
- x509_get_attr_type_and_value(),
- mbedtls_x509_get_crt_ext(),
- mbedtls_x509_get_crl_ext().
These functions kept using the end of the parent structure as the
`end` pointer and would hence allow substructure fields to cross
the substructure boundary. This could lead to successful parsing
of ill-formed X.509 CRTs.
This commit fixes this.
Care has to be taken when adapting `mbedtls_x509_get_crt_ext()`
and `mbedtls_x509_get_crl_ext()`, as the underlying function
`mbedtls_x509_get_ext()` returns `0` if no extensions are present
but doesn't set the variable which holds the bounds of the Extensions
structure in case the latter is present. This commit addresses
this by returning early from `mbedtls_x509_get_crt_ext()` and
`mbedtls_x509_get_crl_ext()` if parsing has reached the end of
the input buffer.
The following X.509 parsing tests need to be adapted:
- "TBSCertificate, issuer two inner set datas"
This test exercises the X.509 CRT parser with a Subject name
which has two empty `AttributeTypeAndValue` structures.
This is supposed to fail with `MBEDTLS_ERR_ASN1_OUT_OF_DATA`
because the parser should attempt to parse the first structure
and fail because of a lack of data. Previously, it failed to
obey the (0-length) bounds of the first AttributeTypeAndValue
structure and would try to interpret the beginning of the second
AttributeTypeAndValue structure as the first field of the first
AttributeTypeAndValue structure, returning an UNEXPECTED_TAG error.
- "TBSCertificate, issuer, no full following string"
This test exercises the parser's behaviour on an AttributeTypeAndValue
structure which contains more data than expected; it should therefore
fail with MBEDTLS_ERR_ASN1_LENGTH_MISMATCH. Because of the missing bounds
check, it previously failed with UNEXPECTED_TAG because it interpreted
the remaining byte in the first AttributeTypeAndValue structure as the
first byte in the second AttributeTypeAndValue structure.
- "SubjectAltName repeated"
This test should exercise two SubjectAltNames extensions in succession,
but a wrong length values makes the second SubjectAltNames extension appear
outside of the Extensions structure. With the new bounds in place, this
therefore fails with a LENGTH_MISMATCH error. This commit adapts the test
data to put the 2nd SubjectAltNames extension inside the Extensions
structure, too.
This commit temporarily comments the copying of the negotiated CIDs
into the established ::mbedtls_ssl_transform in mbedtls_ssl_derive_keys()
until the CID feature has been fully implemented.
While mbedtls_ssl_decrypt_buf() and mbedtls_ssl_encrypt_buf() do
support CID-based record protection by now and can be unit tested,
the following two changes in the rest of the stack are still missing
before CID-based record protection can be integrated:
- Parsing of CIDs in incoming records.
- Allowing the new CID record content type for incoming records.
- Dealing with a change of record content type during record
decryption.
Further, since mbedtls_ssl_get_peer_cid() judges the use of CIDs by
the CID fields in the currently transforms, this change also requires
temporarily disabling some grepping for ssl_client2 / ssl_server2
debug output in ssl-opt.sh.
Part of the record encryption/decryption tests is to gradually
increase the space available at the front and/or at the back of
a record and observe when encryption starts to succeed. If exactly
one of the two parameters is varied at a time, the expectation is
that encryption will continue to succeed once it has started
succeeding (that's not true if both pre- and post-space are varied
at the same time).
Moreover, previously the test would take turns when choosing which
transform should be used for encryption, and which for decryption.
With the introduction of the CID feaature, this switching of transforms
doesn't align with the expectation of eventual success of the encryption,
since the overhead of encryption might be different for the parties,
because both parties may use different CIDs for their outgoing records.
This commit modifies the tests to not take turns between transforms,
but to always use the same transforms for encryption and decryption
during a single round of the test.
The X.509 parsing test suite test_suite_x509parse contains a test
exercising X.509 verification for a valid MD4/MD5 certificate in a
profile which doesn't allow MD4/MD5. This commit adds an analogous
test for MD2.
requires_config_enabled doesn't support multiple config options.
Tests having multiple configuration dependencies must be prefixed
with multiple invocations of requires_config_enabled instead.
When running make with parallelization, running both "clean" and "lib"
with a single make invocation can lead to each target building in
parallel. It's bad if lib is partially done building something, and then
clean deletes what was built. This can lead to errors later on in the
lib target.
$ make -j9 clean lib
CC aes.c
CC aesni.c
CC arc4.c
CC aria.c
CC asn1parse.c
CC ./library/error.c
CC ./library/version.c
CC ./library/version_features.c
AR libmbedcrypto.a
ar: aes.o: No such file or directory
Makefile:120: recipe for target 'libmbedcrypto.a' failed
make[2]: *** [libmbedcrypto.a] Error 1
Makefile:152: recipe for target 'libmbedcrypto.a' failed
make[1]: *** [libmbedcrypto.a] Error 2
Makefile:19: recipe for target 'lib' failed
make: *** [lib] Error 2
make: *** Waiting for unfinished jobs....
To avoid this sort of trouble, always invoke clean by itself without
other targets throughout the library. Don't run clean in parallel with
other rules. The only place where clean was run in parallel with other
targets was in list-symbols.sh.
- Replace 'RSA with MD2' OID '2a864886f70d010102' by
'RSA with SHA-256' OID '2a864886f70d01010b':
Only the last byte determines the hash, and
`MBEDTLS_OID_PKCS1_MD2 == MBEDTLS_OID_PKCS1 "\x02"`
`MBEDTLS_OID_PKCS1_SHA256 == MBEDTLS_OID_PKCS1 "\x0b"`
See oid.h.
- Replace MD2 dependency by SHA256 dependency.
- Adapt expected CRT info output.
* origin/pr/2403: (24 commits)
crypto: Update to Mbed Crypto 8907b019e7
Create seedfile before running tests
crypto: Update to Mbed Crypto 81f9539037
ssl_cli.c : add explicit casting to unsigned char
Generating visualc files - let Mbed TLS take precedence over crypto
Add a link to the seedfile for out-of-tree cmake builds
Adjust visual studio file generation to always use the crypto submodule
all.sh: unparallelize mingw tests
all.sh - disable parallelization for shared target tests
config.pl: disable PSA_ITS_FILE and PSA_CRYPTO_STORAGE for baremetal
all.sh: unset crypto storage define in a psa full config cmake asan test
all.sh: unset FS_IO-dependent defines for tests that do not have it
curves.pl - change test script to not depend on the implementation
Export the submodule flag to sub-cmakes
Disable MBEDTLS_ECP_RESTARTABLE in full config
Export the submodule flag to sub-makes
Force the usage of crypto submodule
Fix crypto submodule usage in Makefile
Documentation rewording
Typo fixes in documentation
...
* origin/pr/2647:
list-symbols.sh: if the build fails, print the build transcript
Document "check-names.sh -v"
all.sh: invoke check-names.sh in print-trace-on-exit mode
Print a command trace if the check-names.sh exits unexpectedly
With MBEDTLS_USE_PSA_CRYPTO and MBEDTLS_ENTROPY_NV_SEED enabled, the
tests need a seedfile. Since test_suite_entropy is no longer there to
create it, and MBEDTLS_USE_PSA_CRYPTO is now enabled in the full
config, create tests/seedfile explicitly in basic-build-test.sh.
Sometimes, when building the shared target with crypto submodule, one could
get an "No rule to make target '../crypto/library/libmbedcrypto.so'" error.
This is due to two reasons - building in parallel and (probably) an
incomplete/incorrect list of dependencies for certain targets. The proposed
solution is to disable parallel builds with crypto submodule for now.
An issue has been raised here: https://github.com/ARMmbed/mbedtls/issues/2634
Sometimes, when building the shared target with crypto submodule, one could
get an "No rule to make target '../crypto/library/libmbedcrypto.so'" error.
This is due to two reasons - building in parallel and (probably) an
incomplete/incorrect list of dependencies for certain targets. The proposed
solution is to disable parallel builds with crypto submodule for now.
An issue has been raised here: https://github.com/ARMmbed/mbedtls/issues/2634
Currently the top-level makefile deploys commands to both Mbed TLS and
the submodule. Running make in the "tests" directory builds only the TLS tests.
The top level CMake on the other hand does not have the "tests" target defined,
so it also cannot be used, hence the raw "make".
Previously it was disabled as too experimental, which no longer holds. Also,
this option introduces new APIs, so it's not only about an internal
alternative (as the comment in config.pl used to state) - people who request a
full config should get all of the available APIs.
Adapt all.sh: now all builds with full config will also test this option, and
builds with the default config will test without it. Just to be sure, let's
have a build with full config minus this option.
Update documentation of MBEDTLS_USE_PSA_CRYPTO to reflect the status of the
new APIs it enables in Mbed TLS and why they're still opt-in.
Also enable it in scripts/config.pl full, as well as two storage options that
were only blacklisted from full config because they depended on
MBEDTLS_PSA_CRYPTO_C.
Adapt tests in all.sh:
- tests with submodule enabled (default) no longer need to enable it
explicitly, and no longer need runtime tests, as those are now handled by
all other test cases in this script
- tests with submodule disabled (old default) now need to disable it
explicitly, and execute some runtime tests, as those are no longer tested
anywhere else in this script
Adapt documentation in Readme: remove the section "building with submodule"
and replace it with a new section before the other building sections.
Purposefully don't document how to build not from the submodule, as that
option is going away soon.
* origin/pr/2530: (27 commits)
Style fix
Fix test data
Update test data
Add some negative test cases
Fix minor issues
Add ChangeLog entry about listing all SAN
Check that SAN is not malformed when parsing
Documentation fixes
Fix ChangeLog entry
Fail in case critical crt policy not supported
Update SAN parsing documentation
change the type of hardware_module_name member
Change mbedtls_x509_subject_alternative_name
Add length checking in certificate policy parsing
Rephrase x509_crt extension member description
Rephrase changeLog entries
Remove redundant memset()
Propogate error when parsing SubjectAltNames
Tidy up style in x509_info_subject_alt_name
Print unparseable SubjectAlternativeNames
...
* origin/pr/2538:
Remove unneeded whitespaces
Fix mingw CI failures
Initialize psa_crypto in ssl test
Fix missing tls version test failures
Fix typo
Fix ChangeLog entry location
Add changeLog entry
Add test for export keys functionality
Add function to retrieve the tls_prf type
Add tests for the public tls_prf API
Add public API for tls_prf
Add eap-tls key derivation in the examples.
Add ChangeLog entry
Add an extra key export function
Have the temporary buffer allocated dynamically
Zeroize secret data in the exit point
Add a single exit point in key derivation function
We've observed that sometimes check-names.sh exits unexpectedly with
status 2 and no error message. The failure is not reproducible. This
commits makes the script print a trace if it exits unexpectedly.
Make `mbedtls_x509_subject_alternative_name` to be a single item
rather than a list. Adapt the subject alternative name parsing function,
to receive a signle `mbedtls_x509_buf` item from the subject_alt_names
sequence of the certificate.
In x509_info_subject_alt_name() we silently dropped names that we
couldn't parse because they are not supported or are malformed. (Being
malformed might mean damaged file, but can be a sign of incompatibility
between applications.)
This commit adds code notifying the user that there is something, but
we can't parse it.
* origin/pr/1633: (26 commits)
Fix uninitialized variable access in debug output of record enc/dec
Adapt PSA code to ssl_transform changes
Ensure non-NULL key buffer when building SSL test transforms
Catch errors while building SSL test transforms
Use mbedtls_{calloc|free}() in SSL unit test suite
Improve documentation of mbedtls_record
Adapt record length value after encryption
Alternative between send/recv transform in SSL record test suite
Fix memory leak on failure in test_suite_ssl
Rename ssl_decrypt_buf() to mbedtls_ssl_decrypt_buf() in comment
Add record encryption/decryption tests for ARIA to SSL test suite
Improve documentation of mbedtls_ssl_transform
Double check that record expansion is as expected during decryption
Move debugging output after record decryption
Add encryption/decryption tests for small records
Add tests for record encryption/decryption
Reduce size of `ssl_transform` if no MAC ciphersuite is enabled
Remove code from `ssl_derive_keys` if relevant modes are not enabled
Provide standalone version of `ssl_decrypt_buf`
Provide standalone version of `ssl_encrypt_buf`
...
This commit adds tests to check the behavior of the record encryption
routine `ssl_encrypt_buf` when the buffer surrounding the plaintext is
too small to hold the expansion in the beginning and end (due to IV's,
padding, and MAC).
Each test starts successively increases the space available at the
beginning, end, or both, of the record buffer, and checks that the
record encryption either fails with a BUFFER_TOO_SMALL error, or
that it succeeds. Moreover, if it succeeds, it is checked that
decryption succeeds, too, and results in the original record.
This commit adds tests exercising mutually inverse pairs of
record encryption and decryption transformations for the various
transformation types allowed in TLS: Stream, CBC, and AEAD.
Resolve conflicts by performing the following:
- Ensure calls to mbedtls_x509_crt_verify_* are made with callbacks
* origin/pr/2539:
Make CRT callback tests more robust
Rename constant in client2.c
Fix typo
Add test for configuration specific CRT callback
Fix doxygen documentation of mbedtls_ssl_set_verify()
Add test exercising context-specific CRT callback to ssl-opt.sh
Add cmd to use context-specific CRT callback in ssl_client2
Implement context-specific verification callbacks
Add context-specific CRT verification callbacks
Improve documentation of mbedtls_ssl_conf_verify()
* origin/pr/2532: (29 commits)
Document and test flags in x509_verify
Fix style issues and a typo
Fix name to function call
Address comments for x509 tests
Address review comments regarding ssl_client2 and ssl tests
Remove mbedtls_ from the static function name
Change docs according to review comments
Change the verify function naming
Fix ssl_client2 and ssl_server2 if !PLATFORM_C
Correct placement of usage macro in ssl_client2
Update version_features.c
Remove trailing whitespace in test_suite_x509parse.function
Update query_config.c
Add ssl-opt.sh tests for trusted CA callbacks
Only run X.509 CRT verification tests with CA callback tests if !CRL
Minor fixes to CA callback tests
Declare CA callback type even if feature is disabled
Implement X.509 CRT verification using CA callback
Add prototype for CRT verification with static and dynamic CA list
Make use of CA callback if present when verifying peer CRT chain
...
The documentation doesn't explicitly say whether it's allowed or not.
This currently works with the default software implementation, but
only by accident. It isn't guaranteed to work with new ciphers or with
alternative implementations of individual ciphers, and it doesn't work
with the PSA wrappers. So don't do it.
Run ssl-opt.sh on x86_32 with ASan. This may detect bugs that only
show up on 32-bit platforms, for example due to size_t overflow.
For this component, turn off some memory management features that are
not useful, potentially slow, and may reduce ASan's effectiveness at
catching buffer overflows.
* origin/pr/2464:
Allow main() to lack a docstring.
Silence pylint
check-files.py: readability improvement in permission check
check-files.py: use class fields for class-wide constants
check-files.py: clean up class structure
abi_check.py: Document more methods
check-files.py: document some classes and methods
Fix pylint errors going uncaught
Call pylint3, not pylint
New, documented pylint configuration
When doing ABI/API checking, its useful to have a list of all the
identifiers that are defined in the internal header files, as we
do not promise compatibility for them. This option allows for a
simple method of getting them for use with the ABI checking script.
* origin/pr/2192:
Increase okm_hex buffer to contain null character
Minor modifications to hkdf test
Add explanation for okm_string size
Update ChangeLog
Reduce buffer size of okm
Reduce Stack usage of hkdf test function
It was failing to set the key in the ENCRYPT direction before encrypting.
This just happened to work for GCM and CCM.
After re-encrypting, compare the length to the expected ciphertext
length not the plaintext length. Again this just happens to work for
GCM and CCM since they do not perform any kind of padding.
* restricted/pr/551:
ECP: Clarify test descriptions
ECP: remove extra whitespaces
Fix ECDH secret export for Mongomery curves
Improve ECP test names
Make ecp_get_type public
Add more tests for ecp_read_key
ECP: Catch unsupported import/export
Improve documentation of mbedtls_ecp_read_key
Fix typo in ECP module
Remove unnecessary cast from ECP test
Improve mbedtls_ecp_point_read_binary tests
Add Montgomery points to ecp_point_write_binary
ECDH: Add test vectors for Curve25519
Add little endian export to Bignum
Add mbedtls_ecp_read_key
Add Montgomery points to ecp_point_read_binary
Add little endian import to Bignum
Ensure this merge passes tests by auto-generating query_config.c, adding
MBEDTLS_ECDH_LEGACY_CONTEXT to it.
* restricted/pr/552:
Fix mbedtls_ecdh_get_params with new ECDH context
Test undefining MBEDTLS_ECDH_LEGACY_CONTEXT in all.sh
Define MBEDTLS_ECDH_LEGACY_CONTEXT in config.h
Add changelog entry for mbedtls_ecdh_get_params robustness
Fix ecdh_get_params with mismatching group
Add test case for ecdh_get_params with mismatching group
Add test case for ecdh_calc_secret
Fix typo in documentation
Ensure tests pass when the submodule is used by updating the list of
crypto tests to include test_suite_oid in both tests/CMakeLists.txt and
tests/Makefile.
* origin/pr/2531:
Add changeLog entry
Add certificate policy of type any policy id
* origin/pr/2509:
all.sh: Generate seedfile for crypto submodule tests
Update crypto submodule to test with private headers
tests: Use globbing in test suite exclusion list
Update crypto submodule to Mbed Crypto development
tests: Test crypto via the crypto submodule
Test the crypto implementation via tests from the Mbed Crypto submodule
instead of at the Mbed TLS top level.
The version test is the only test that is tested from both TLS and
Crypto, despite being entirely in libmbedcrypto. This is because the
test data is code-gen'd from the version updating script and the version
between Mbed TLS and Mbed Crypto don't necessarily always agree. The
test data must come from the top level module, as only the top level
module will have test data that matches the expected version.
