Update Mbed Crypto with latest Mbed TLS changes as of 2019-11-15 #322

Merged

gilles-peskine-arm merged 421 commits from merge-crypto-development-20191115 into development

2019-11-18 07:59:12 -06:00

gilles-peskine-arm commented

2019-11-15 05:00:39 -06:00

(Migrated from github.com)

The previous update was https://github.com/ARMmbed/mbed-crypto/pull/223. Salient changes:

Reconcile config.py, in particular with MBEDTLS_MEMORY_BUFFER_ALLOC_C disabled in config.py full.
Bring in the test outcome collection code.
This update encompasses the removal of crypto files from mbedtls. This will make future merges easier.

This is a merge commit (9afbfdc833) and its follow-up.

CI passed and produced the expected outcome file.

Internal ref: IOTCRYPT-974

The previous update was https://github.com/ARMmbed/mbed-crypto/pull/223. Salient changes: * Reconcile `config.py`, in particular with `MBEDTLS_MEMORY_BUFFER_ALLOC_C` disabled in `config.py full`. * Bring in the test outcome collection code. * This update encompasses the removal of crypto files from mbedtls. This will make future merges easier. This is a merge commit (9afbfdc833726a31e5806494c661d6b83dc3a926) and its follow-up. CI passed and produced the expected outcome file. Internal ref: IOTCRYPT-974

dgreen-arm (Migrated from github.com) approved these changes

2019-11-18 05:42:14 -06:00

dgreen-arm (Migrated from github.com) left a comment

Having gone through the merge myself, the conflict resolution looks correct to me, approving.

Patater (Migrated from github.com) reviewed

2019-11-18 07:58:06 -06:00

Patater (Migrated from github.com) left a comment

I did the merge myself and ran into a few meaningful differences, listed below. I ran into other differences as well, but those are stylistic and I don't mind going with another style.

None of these are blockers.

The diffs are - for your PR version, and + for my merge.

I kept the /S in programs/Makefile. This provides consistency with what came before in the crypto branch, as /S was already present there. No TLS branch removes /S intentionally, so we shouldn't remove /S from Crypto by this merge.

diff --git a/programs/Makefile b/programs/Makefile
index f56df5f97fae..add1a8649b49 100644
--- a/programs/Makefile
+++ b/programs/Makefile
@@ -249,8 +249,8 @@ ifndef WINDOWS
        rm -f $(APPS) $(EXTRA_GENERATED)
        -rm -f test/cpp_dummy_build$(EXEXT)
 else
-       if exist *.o del /Q /F *.o
-       if exist *.exe del /Q /F *.exe
+       if exist *.o del /S /Q /F *.o
+       if exist *.exe del /S /Q /F *.exe
        if exist $(EXTRA_GENERATED) del /S /Q /F $(EXTRA_GENERATED)
 endif

In component_test_no_use_psa_crypto_full_cmake_asan() in all.sh, I aligned with TLS by unsetting MBEDTLS_PSA_CRYPTO_C. We shouldn't make this test test more different configurations than it currently does: the merge should align the configuration used by this test where applicable.

I did the merge myself and ran into a few meaningful differences, listed below. I ran into other differences as well, but those are stylistic and I don't mind going with another style. None of these are blockers. The diffs are `-` for your PR version, and `+` for my merge. 1. I kept the `/S` in `programs/Makefile`. This provides consistency with what came before in the crypto branch, as `/S` was already present there. No TLS branch removes `/S` intentionally, so we shouldn't remove `/S` from Crypto by this merge. ```diff diff --git a/programs/Makefile b/programs/Makefile index f56df5f97fae..add1a8649b49 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -249,8 +249,8 @@ ifndef WINDOWS rm -f $(APPS) $(EXTRA_GENERATED) -rm -f test/cpp_dummy_build$(EXEXT) else - if exist *.o del /Q /F *.o - if exist *.exe del /Q /F *.exe + if exist *.o del /S /Q /F *.o + if exist *.exe del /S /Q /F *.exe if exist $(EXTRA_GENERATED) del /S /Q /F $(EXTRA_GENERATED) endif ``` 2. In `component_test_no_use_psa_crypto_full_cmake_asan()` in `all.sh`, I aligned with TLS by unsetting `MBEDTLS_PSA_CRYPTO_C`. We shouldn't make this test test more different configurations than it currently does: the merge should align the configuration used by this test where applicable.

Patater commented

2019-11-18 07:59:03 -06:00

(Migrated from github.com)