Update Mbed Crypto with latest Mbed TLS changes as of 2020-02-03 #357
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
This commit moves the X.509 negative parsing tests for the CertificatePolicy extension to the place where negative testing of other extensions happens.
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 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.
…constant integer number)
…case MBEDTLS_SSL_PROTO_DTLS was undefined
…t value was never used)
X.509: Enhance negative testing for CertificatePolicy extension
…xample Fix potential resource leak in sslserver2 example
* origin/pr/2854: Shorter version of mbedtls_ssl_send_fatal_handshake_failure Resolve #2801 - remove repetitive assignment to ssl->in_msg (the first value was never used) Resolve #2800 - move declaration to avoid unused variable warning in case MBEDTLS_SSL_PROTO_DTLS was undefined Resolve #2717 - remove erroneous sizeof (the operator was applied to constant integer number)
…tricted * restricted/pr/661: Fix buffer size in an AES example
…stricted * origin/development: Remove unused test data file Remove component designed to test MAX_SIGNATURE_SIZE Use MBEDTLS_PK_SIGNATURE_MAX_SIZE in pkey sample programs Use MBEDTLS_PK_SIGNATURE_MAX_SIZE in X.509 Update crypto submodule x509write_csr: Reduce stack usage of mbedtls_x509write_csr_pem() Fix mbedtls_ssl_check_record usage with ext buf Shorter version of mbedtls_ssl_send_fatal_handshake_failure Resolve #2801 - remove repetitive assignment to ssl->in_msg (the first value was never used) Resolve #2800 - move declaration to avoid unused variable warning in case MBEDTLS_SSL_PROTO_DTLS was undefined Resolve #2717 - remove erroneous sizeof (the operator was applied to constant integer number) Fix potential resource leak in sslserver2 example X.509: Add numerous negative parsing tests for CertificatePolicy ext X.509: Adapt negative parsing test for no data in CrtPolicy ext X.509: Move negative tests for CertificatePolicy parsing X.509: Remove CRT policy parsing test 'bool len missing'
fix error when calloc is called with size 0
Avoid allocating 0-length buffers for PSK. Add memory debug information to ssl_client2.
The new macro ASSERT_ALLOC allocates memory with mbedtls_calloc and fails the test if the allocation fails. It outputs a null pointer if the requested size is 0. It is meant to replace existing calls to mbedtls_calloc.
`ASSERT_ALLOC(p, length)` now allocates `length` elements, i.e. `length * sizeof(*p)` bytes.
The assert() macro in test is not available anymore. It is superseeded by TEST_HELPER_ASSERT().
Declare include headers as `PUBLIC` to propagate to project consumers
In a unit test we want to avoid accessing the network. To test the handshake in the unit test suite we need to implement a connection between the server and the client. This ring buffer implementation will serve as the said connection.
…larity config.pl: If python3 fails, make it clear that this isn't fatal
* ARMmbed#321: Replace config.pl by config.py * ARMmbed#322: Update Mbed Crypto with latest Mbed TLS changes as of 2019-11-15 * ARMmbed#308: Small performance improvement of mbedtls_mpi_div_mpi() * ARMmbed#324: test_psa_constant_names: support key agreement, better code structure * ARMmbed#320: Link to the PSA crypto portal page from README.md * ARMmbed#293: Always gather MBEDTLS_ENTROPY_BLOCK_SIZE bytes of entropy * ARMmbed#310: Clarify test descriptions in test_suite_memory_buffer_alloc * ARMmbed#307: Add ASN.1 ENUMERATED tag support * ARMmbed#328: Remove dependency of crypto_values.h on crypto_extra.h * ARMmbed#325: Rename psa_asymmetric_{sign_verify} to psa_{sign,verify}_hash Missed listing in the previous submodule update: * ARMmbed#304: Make sure Asan failures are detected in 'make test'
In a unit test we want to avoid accessing the network. To test the handshake in the unit test suite we need to implement a connection between the server and the client. This socket implementation uses two ring buffers to mock the transport layer.
The recent update changed the Mbed Crypto SO version, get Mbed TLS in sync.
Add changelog entries for the crypto changes in 2.20.0
Bump version to Mbed TLS 2.20.0
Because two buffers were aliased too early in the code, it was possible that after an allocation failure, free() would be called twice for the same pointer.
…ks-to-ssl-unit-test Changes in custom IO callbacks used in unit tests
Allow loading symlinked certificates
…s into development
[cmake] Propagate public headers
…port-simulated Message transport mocks in ssl tests
Add zlib tests and fix runtime bug
Previously in d875285: * ARMmbed#333: Streamline PSA key type encodings: prepare * ARMmbed#323: Initialise return values to an error Previously in dbcb442: * ARMmbed#291: Test MBEDTLS_CTR_DRBG_USE_128_BIT_KEY * ARMmbed#334: Fix some pylint warnings Previously in ceceedb: * ARMmbed#348: Bump version to Mbed TLS 2.20.0 and crypto SO version to 4 * ARMmbed#354: Fix incrementing pointer instead of value In this commit: * ARMmbed#349: Fix minor defects found by Coverity * ARMmbed#179: Add option to build SHA-512 without SHA-384 * ARMmbed#327: Implement psa_hash_compute and psa_hash_compare * ARMmbed#330: Streamline PSA key type and curve encodings
Adapt to the change of encoding of elliptic curve key types in PSA crypto. Before, an EC key type encoded the TLS curve identifier. Now the EC key type only includes an ad hoc curve family identifier, and determining the exact curve requires both the key type and size. This commit moves from the old encoding and old definitions from crypto/include/mbedtls/psa_util.h to the new encoding and definitions from the immediately preceding crypto submodule update.
…-prescribed-state Add test for prescribed states of handshake with the custom IO callbacks
…ings-types_and_curves-ls USE_PSA_CRYPTO: update elliptic curve encoding
Checks mbedtls_rsa_export return in fuzz targets
gilles-peskine-arm
added
the
needs: review
…opment
Files deleted by us: keep them deleted.
```
git rm $(git status -s | sed -n 's/^DU //p')
```
Individual files with conflicts:
* `README.md`: keep the crypto version.
* `doxygen/input/doc_mainpage.h`: keep the crypto version (with an obsolete Mbed Crypto version number).
* `include/mbedtls/error.h`:
* `ERROR`: similar additions made through parallel commits, with only whitespace differences. Align with the tls version.
* `library/CMakeLists.txt`: keep the crypto version.
* `library/Makefile`: keep the crypto version.
* `scripts/generate_errors.pl`: keep the crypto version (the relevant changes were made through parallel commits).
* `tests/scripts/check-test-cases.py`:
* `Results`: keep the crypto version, which has both the new argument to the constructor (added in crypto only) and the class docstring (added through parallel commits).
* `tests/suites/helpers.function`:
* `ARRAY_LENGTH`, `ASSERT_ALLOC`: additions in the same location. Keep both, in indifferent order.
* `tests/suites/target_test.function`:
* `receive_uint32`: keep the crypto version which has an additional bug fix. The tls changes made in tls are irrelevant after this bug fix.
* `visualc/VS2010/mbedTLS.vcxproj`: run `scripts/generate_visualc_files.pl`.
Review of non-conflicting changes:
* `all.sh`: 1 change.
* zlib test components: don't add them.
* `include/CMakeLists.txt`: 1 change.
* `target_include_directories`: doesn't work as is (different target name). Don't take the change.
* All other non-conflicting changes: take them.
gilles-peskine-arm
force-pushed
the
merge-crypto-development-20200203
branch
from
a8dff88 to
2579675
Compare
mpg
approved these changes
Feb 4, 2020
Contributor
mpg
left a comment
mpg
left a comment
There was a problem hiding this comment.
I reviewed the diff, and checked that the merge commit indeed merges mbed-crypto's current development head with mbedtls's yesterday's development head.
Contributor
|
I checked that the CI only fails the Mbed OS tests, which are a known issue independent from this PR. |
yanesca
approved these changes
Feb 4, 2020
Collaborator
yanesca
left a comment
yanesca
left a comment
There was a problem hiding this comment.
I have done the merge locally and arrived at the same result.
yanesca
added
ready for merge
Collaborator
|
Double checked that the CI only fails on Mbed OS tests. |
gilles-peskine-arm
added a commit
to gilles-peskine-arm/mbed-crypto
that referenced
this pull request
Mar 23, 2020
* ARMmbed#352: Parse RSA parameters DP, DQ and QP from PKCS1 private keys * ARMmbed#263: Introduce ASN.1 SEQUENCE traversal API * ARMmbed#345: Fix possible error code mangling in psa_mac_verify_finish * ARMmbed#357: Update Mbed Crypto with latest Mbed TLS changes as of 2020-02-03 * ARMmbed#350: test_suite_asn1parse: improve testing of trailing garbage in parse_prefixes * ARMmbed#346: Improve robustness and testing of mbedtls_mpi_copy
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
12 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previous update was #322. Not much going on this time: a few bug fixes;
ASSERT_ALLOCin test suites.This is just one merge commit.