From bf4d0ce88d7b37c96e0dcf4d05b504ff653e2d70 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Jul 2021 17:12:53 +0200 Subject: [PATCH 1/5] Remove obsolete MBEDTLS_xxx dependencies This file had temporary MBEDTLS_xxx dependencies because it was created when support for PSA_WANT_xxx was still incomplete. Switch to the PSA_WANT_xxx dependencies This fixes the bug that "PSA storage read: AES-GCM+CTR" was never executed because there was a typo in a dependency. Signed-off-by: Gilles Peskine --- tests/suites/test_suite_psa_crypto_storage_format.misc.data | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_storage_format.misc.data b/tests/suites/test_suite_psa_crypto_storage_format.misc.data index 114c402a2..48e3804b4 100644 --- a/tests/suites/test_suite_psa_crypto_storage_format.misc.data +++ b/tests/suites/test_suite_psa_crypto_storage_format.misc.data @@ -3,11 +3,9 @@ # debugging changes to the test code or to the test case generation. PSA storage read: AES-GCM+CTR -#depends_on:PSA_WANT_KEY_TYPE_AES:PSA_WANT_ALG_GCM:PSA_WANT_ALG_CTR -depends_on:MBEDTLS_AES_C:MBEDTLS_GCM_C:MBEDTLS_CTR_C +depends_on:PSA_WANT_KEY_TYPE_AES:PSA_WANT_ALG_GCM:PSA_WANT_ALG_CTR key_storage_read:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_TYPE_AES:128:PSA_KEY_USAGE_EXPORT | PSA_KEY_USAGE_ENCRYPT:PSA_ALG_GCM:PSA_ALG_CTR:"404142434445464748494a4b4c4d4e4f":"505341004b45590000000000010000000024800001010000000250050010c00410000000404142434445464748494a4b4c4d4e4f":1 PSA storage save: AES-GCM+CTR -#depends_on:PSA_WANT_KEY_TYPE_AES -depends_on:MBEDTLS_AES_C +depends_on:PSA_WANT_KEY_TYPE_AES key_storage_save:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_TYPE_AES:128:PSA_KEY_USAGE_EXPORT | PSA_KEY_USAGE_ENCRYPT:PSA_ALG_GCM:PSA_ALG_CTR:"404142434445464748494a4b4c4d4e4f":"505341004b45590000000000010000000024800001010000000250050010c00410000000404142434445464748494a4b4c4d4e4f" From 5df77c63fba92d58c76161c959aa1db337db57c6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Jul 2021 17:22:58 +0200 Subject: [PATCH 2/5] Fix race condition when running generate_psa_tests.py Fix a race condition in parallel builds: when generating *.data files with generate_psa_tests.py, make instantiated the recipe once per output file, potentially resulting in multiple instances of generate_psa_tests.py running in parallel. This not only was inefficient, but occasionally caused the output to be corrupted (https://github.com/ARMmbed/mbedtls/issues/4773). Fix this by ensuring the recipe only runs once. Signed-off-by: Gilles Peskine --- tests/Makefile | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index c1620d6e7..6695437bf 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -63,18 +63,25 @@ GENERATED_DATA_FILES := $(patsubst tests/%,%,$(shell $(PYTHON) scripts/generate_ GENERATED_FILES := $(GENERATED_DATA_FILES) generated_files: $(GENERATED_FILES) -$(GENERATED_DATA_FILES): scripts/generate_psa_tests.py +# generate_psa_tests.py spends more time analyzing inputs than generating +# outputs. Its inputs are the same no matter which files are being generated. +# It's rare not to want all the outputs. So always generate all of its outputs. +# Use an intermediate phony dependency so that parallel builds don't run +# a separate instance of the recipe for each output file. +.SECONDARY: generated_psa_test_data +$(GENERATED_DATA_FILES): generated_psa_test_data +generated_psa_test_data: scripts/generate_psa_tests.py ## The generated file only depends on the options that are present in ## crypto_config.h, not on which options are set. To avoid regenerating this ## file all the time when switching between configurations, don't declare ## crypto_config.h as a dependency. Remove this file from your working tree ## if you've just added or removed an option in crypto_config.h. -#$(GENERATED_DATA_FILES): ../include/psa/crypto_config.h -$(GENERATED_DATA_FILES): ../include/psa/crypto_values.h -$(GENERATED_DATA_FILES): ../include/psa/crypto_extra.h -$(GENERATED_DATA_FILES): suites/test_suite_psa_crypto_metadata.data -$(GENERATED_DATA_FILES): - echo " Gen $@ ..." +#generated_psa_test_data: ../include/psa/crypto_config.h +generated_psa_test_data: ../include/psa/crypto_values.h +generated_psa_test_data: ../include/psa/crypto_extra.h +generated_psa_test_data: suites/test_suite_psa_crypto_metadata.data +generated_psa_test_data: + echo " Gen $(GENERATED_DATA_FILES) ..." $(PYTHON) scripts/generate_psa_tests.py # A test application is built for each suites/test_suite_*.data file. From 8b427c851e6e0979c598b95d2c04d5deab10352d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Jul 2021 18:14:25 +0200 Subject: [PATCH 3/5] Use python3 when building on non-Windows for Windows The makefiles look for python3 on Unix-like systems where python is often Python 2. This uses sh code so it doesn't work on Windows. On Windows, the makefiles just assume that python is Python 3. The code was incorrectly deciding not to try python3 based on WINDOWS_BUILD, which indicates that the build is *for* Windows. Switch to checking WINDOWS, which indicates that the build is *on* Windows. Fix #4774 Signed-off-by: Gilles Peskine --- ChangeLog.d/makefile-python-windows.txt | 4 ++++ programs/Makefile | 6 +++++- tests/Makefile | 6 +++++- 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 ChangeLog.d/makefile-python-windows.txt diff --git a/ChangeLog.d/makefile-python-windows.txt b/ChangeLog.d/makefile-python-windows.txt new file mode 100644 index 000000000..57ccc1a39 --- /dev/null +++ b/ChangeLog.d/makefile-python-windows.txt @@ -0,0 +1,4 @@ +Bugfix + * The GNU makefiles invoke python3 in preference to python except on Windows. + The check was accidentally not performed when cross-compiling for Windows + on Linux. Fix this. Fixes #4774. diff --git a/programs/Makefile b/programs/Makefile index 997c19871..55ebd601a 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -43,11 +43,15 @@ LOCAL_LDFLAGS += -lws2_32 ifdef SHARED SHARED_SUFFIX=.$(DLEXT) endif -PYTHON ?= python else DLEXT ?= so EXEXT= SHARED_SUFFIX= +endif + +ifdef WINDOWS +PYTHON ?= python +else PYTHON ?= $(shell if type python3 >/dev/null 2>/dev/null; then echo python3; else echo python; fi) endif diff --git a/tests/Makefile b/tests/Makefile index 6695437bf..251a9c6fc 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -50,11 +50,15 @@ LOCAL_LDFLAGS += -lws2_32 ifdef SHARED SHARED_SUFFIX=.$(DLEXT) endif -PYTHON ?= python else DLEXT ?= so EXEXT= SHARED_SUFFIX= +endif + +ifdef WINDOWS +PYTHON ?= python +else PYTHON ?= $(shell if type python3 >/dev/null 2>/dev/null; then echo python3; else echo python; fi) endif From e9ad95a63cb9063ea197a7b022b3148a55311bd8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Jul 2021 18:36:05 +0200 Subject: [PATCH 4/5] Error out if enumerating the generated data files fails Signed-off-by: Gilles Peskine --- tests/Makefile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/Makefile b/tests/Makefile index 251a9c6fc..449fca274 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -63,7 +63,13 @@ PYTHON ?= $(shell if type python3 >/dev/null 2>/dev/null; then echo python3; els endif .PHONY: generated_files -GENERATED_DATA_FILES := $(patsubst tests/%,%,$(shell $(PYTHON) scripts/generate_psa_tests.py --list)) +GENERATED_DATA_FILES := $(patsubst tests/%,%,$(shell \ + $(PYTHON) scripts/generate_psa_tests.py --list || \ + echo FAILED \ +)) +ifeq ($(GENERATED_DATA_FILES),FAILED) +$(error "$(PYTHON) scripts/generate_psa_tests.py --list" failed) +endif GENERATED_FILES := $(GENERATED_DATA_FILES) generated_files: $(GENERATED_FILES) From 6ee3bc09ed0baca12ec64b90ed0fa944dd2b0780 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Jul 2021 20:34:55 +0200 Subject: [PATCH 5/5] Fix typo in test dependencies Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 0729755f0..5a03f598f 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -6748,7 +6748,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SHA256_C -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_GCM_C run_test "DTLS fragmenting: both (MTU=512)" \ @@ -6779,7 +6779,7 @@ not_with_valgrind requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_GCM_C run_test "DTLS fragmenting: proxy MTU: auto-reduction (not valgrind)" \ @@ -6803,7 +6803,7 @@ only_with_valgrind requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_GCM_C run_test "DTLS fragmenting: proxy MTU: auto-reduction (with valgrind)" \ @@ -6855,7 +6855,7 @@ not_with_valgrind # spurious autoreduction due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_GCM_C run_test "DTLS fragmenting: proxy MTU, simple handshake (MTU=512)" \ @@ -6904,7 +6904,7 @@ not_with_valgrind # spurious autoreduction due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_GCM_C run_test "DTLS fragmenting: proxy MTU, simple handshake, nbio (MTU=512)" \ @@ -6940,7 +6940,7 @@ not_with_valgrind # spurious autoreduction due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_GCM_C run_test "DTLS fragmenting: proxy MTU, resumed handshake" \ @@ -6969,7 +6969,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SHA256_C -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED requires_config_enabled MBEDTLS_SSL_RENEGOTIATION requires_config_enabled MBEDTLS_CHACHAPOLY_C run_test "DTLS fragmenting: proxy MTU, ChachaPoly renego" \ @@ -7000,7 +7000,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SHA256_C -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED requires_config_enabled MBEDTLS_SSL_RENEGOTIATION requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_GCM_C @@ -7032,7 +7032,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SHA256_C -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED requires_config_enabled MBEDTLS_SSL_RENEGOTIATION requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_CCM_C @@ -7064,7 +7064,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SHA256_C -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED requires_config_enabled MBEDTLS_SSL_RENEGOTIATION requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_CIPHER_MODE_CBC @@ -7097,7 +7097,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SHA256_C -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED requires_config_enabled MBEDTLS_SSL_RENEGOTIATION requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_CIPHER_MODE_CBC @@ -7126,7 +7126,7 @@ run_test "DTLS fragmenting: proxy MTU, AES-CBC non-EtM renego" \ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_GCM_C client_needs_more_time 2 @@ -7150,7 +7150,7 @@ run_test "DTLS fragmenting: proxy MTU + 3d" \ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_GCM_C client_needs_more_time 2