From f4a6a05e9d1506a0af313d3b67af3f470aa8b2f1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Nov 2020 14:55:35 +0100 Subject: [PATCH 1/9] ssl_context_info: fix config requirements Revealed by attempting to build in configs/config-no-entropy.h. Signed-off-by: Gilles Peskine --- programs/ssl/ssl_context_info.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/programs/ssl/ssl_context_info.c b/programs/ssl/ssl_context_info.c index d109c1e6f..a204d9ead 100644 --- a/programs/ssl/ssl_context_info.c +++ b/programs/ssl/ssl_context_info.c @@ -26,10 +26,12 @@ #include #include -#if !defined(MBEDTLS_X509_CRT_PARSE_C) || !defined(MBEDTLS_ERROR_C) +#if !defined(MBEDTLS_X509_CRT_PARSE_C) || !defined(MBEDTLS_ERROR_C) || \ + !defined(MBEDTLS_SSL_TLS_C) int main( void ) { - printf("MBEDTLS_X509_CRT_PARSE_C and/or MBEDTLS_ERROR_C not defined.\n"); + printf("MBEDTLS_X509_CRT_PARSE_C and/or MBEDTLS_ERROR_C and/or " + "MBEDTLS_SSL_TLS_C not defined.\n"); return( 0 ); } #else From bce4dc028f439e7ead2a8786be75fd249a1eb711 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Nov 2020 15:06:57 +0100 Subject: [PATCH 2/9] Include config_psa.h from psa/crypto.h When the new PSA crypto configuration mechanism MBEDTLS_PSA_CRYPTO_CONFIG is disabled, legacy configurations must keep working, even if they don't include the new header file mbedtls/config_psa.h. Code that uses or implements PSA crypto interfaces needs some of the symbols defined by the new header file. Therefore, include the new header file via PSA crypto headers, which are included everywhere mbedtls/config_psa.h is needed. Include it early, in psa/crypto_platform.h, just after including mbedtls/config.h, so that its symbols are available wherever the symbols from mbedtls/config.h is available. This fixes the unit tests with configs/config-psa-crypto.h: some unit tests were failing, revealing that library features controlled with the new symbols were no longer getting built. Signed-off-by: Gilles Peskine --- include/psa/crypto_platform.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/psa/crypto_platform.h b/include/psa/crypto_platform.h index c64f61d58..4582a865f 100644 --- a/include/psa/crypto_platform.h +++ b/include/psa/crypto_platform.h @@ -41,6 +41,10 @@ #include MBEDTLS_CONFIG_FILE #endif +/* Translate between classic MBEDTLS_xxx feature symbols and PSA_xxx + * feature symbols. */ +#include "mbedtls/config_psa.h" + /* PSA requires several types which C99 provides in stdint.h. */ #include From b64e0fe5e35b174b8070827d92ec0a6aed6910f6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Nov 2020 15:14:10 +0100 Subject: [PATCH 3/9] Fix conditions for including string.h in error.c is actually needed when MBEDTLS_ERROR_C is enabled and not when only MBEDTLS_ERROR_STRERROR_DUMMY is enabled. Fix #3866. Signed-off-by: Gilles Peskine --- ChangeLog.d/error-include-string.txt | 2 ++ library/error.c | 2 +- scripts/data_files/error.fmt | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) create mode 100644 ChangeLog.d/error-include-string.txt diff --git a/ChangeLog.d/error-include-string.txt b/ChangeLog.d/error-include-string.txt new file mode 100644 index 000000000..0a12c7bec --- /dev/null +++ b/ChangeLog.d/error-include-string.txt @@ -0,0 +1,2 @@ +Bugfix + * Fix conditions for including string.h in error.c. Fixes #3866. diff --git a/library/error.c b/library/error.c index cba61e9e7..0d9f736b9 100644 --- a/library/error.c +++ b/library/error.c @@ -19,7 +19,7 @@ #include "common.h" -#if defined(MBEDTLS_ERROR_STRERROR_DUMMY) +#if defined(MBEDTLS_ERROR_C) #include #endif diff --git a/scripts/data_files/error.fmt b/scripts/data_files/error.fmt index fd72f8b5f..1c8a79036 100644 --- a/scripts/data_files/error.fmt +++ b/scripts/data_files/error.fmt @@ -19,7 +19,7 @@ #include "common.h" -#if defined(MBEDTLS_ERROR_STRERROR_DUMMY) +#if defined(MBEDTLS_ERROR_C) #include #endif From 7a78a1f47c6736de81ed8e1f3b3a0c2ee5b529bc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Nov 2020 14:44:04 +0100 Subject: [PATCH 4/9] Sort entries to make it easier to eyeball the list No semantic change. Signed-off-by: Gilles Peskine --- tests/scripts/test-ref-configs.pl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/scripts/test-ref-configs.pl b/tests/scripts/test-ref-configs.pl index 01edfe2fb..d38f5e718 100755 --- a/tests/scripts/test-ref-configs.pl +++ b/tests/scripts/test-ref-configs.pl @@ -28,6 +28,9 @@ use warnings; use strict; my %configs = ( + 'config-ccm-psk-tls1_2.h' => { + 'compat' => '-m tls1_2 -f \'^TLS-PSK-WITH-AES-...-CCM-8\'', + }, 'config-mini-tls1_1.h' => { 'compat' => '-m tls1_1 -f \'^DES-CBC3-SHA$\|^TLS-RSA-WITH-3DES-EDE-CBC-SHA$\'', #' }, @@ -36,9 +39,6 @@ my %configs = ( }, 'config-symmetric-only.h' => { }, - 'config-ccm-psk-tls1_2.h' => { - 'compat' => '-m tls1_2 -f \'^TLS-PSK-WITH-AES-...-CCM-8\'', - }, 'config-thread.h' => { 'opt' => '-f ECJPAKE.*nolog', }, From 25fdebf0c6c722623099633e6bc5616ba265542c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Nov 2020 15:15:17 +0100 Subject: [PATCH 5/9] Add missing configs from test-ref-configs.pl Two sample configuration file were not being tested: config-no-entropy.h and config-psa-crypto.h. Add them. Signed-off-by: Gilles Peskine --- tests/scripts/test-ref-configs.pl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/scripts/test-ref-configs.pl b/tests/scripts/test-ref-configs.pl index d38f5e718..cf4175af2 100755 --- a/tests/scripts/test-ref-configs.pl +++ b/tests/scripts/test-ref-configs.pl @@ -34,6 +34,10 @@ my %configs = ( 'config-mini-tls1_1.h' => { 'compat' => '-m tls1_1 -f \'^DES-CBC3-SHA$\|^TLS-RSA-WITH-3DES-EDE-CBC-SHA$\'', #' }, + 'config-no-entropy.h' => { + }, + 'config-psa-crypto.h' => { + }, 'config-suite-b.h' => { 'compat' => "-m tls1_2 -f 'ECDHE-ECDSA.*AES.*GCM' -p mbedTLS", }, From a26b3e56cb82e841a329ac6579bb9539fa37adcb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Nov 2020 15:19:32 +0100 Subject: [PATCH 6/9] Only include config_psa.h from config.h with new-style PSA configuration In old-style configuration, do not include mbedtls/config_psa.h from mbedtls/config.h. The inclusion should not and did not break any code, but it caused our testing to miss a break of backward compatibility (fixed in "Include config_psa.h from psa/crypto.h"). If users have their own config.h which enabled MBEDTLS_PSA_CRYPTO_C and worked prior to the creation of config_psa.h and MBEDTLS_PSA_CRYPTO_CONFIG, their config.h must keep working. By including config_psa.h from config.h in the legacy case, we weren't testing the legacy configuration mechanism adequately. Signed-off-by: Gilles Peskine --- include/mbedtls/config.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 2ac2cc696..5fd3c590e 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3856,7 +3856,9 @@ #include MBEDTLS_USER_CONFIG_FILE #endif +#if defined(MBEDTLS_PSA_CRYPTO_CONFIG) #include "mbedtls/config_psa.h" +#endif #include "mbedtls/check_config.h" From 58858b7ce4ea7da1168894c44ace993b2b8d9dfb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Nov 2020 15:26:09 +0100 Subject: [PATCH 7/9] Document that MBEDTLS_PSA_CRYPTO_CONFIG requires config_psa.h Document that enabling MBEDTLS_PSA_CRYPTO_CONFIG requires including mbedtls/config_psa.h from the configuration file (mbedtls/config.h or MBEDTLS_USER_CONFIG_FILE). Signed-off-by: Gilles Peskine --- include/mbedtls/config.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 5fd3c590e..98f88aebc 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -2057,7 +2057,11 @@ * API to be configured separately from support through the mbedtls API. * * Uncomment this to enable use of PSA Crypto configuration settings which - * can be found in include/psa/crypto_config.h + * can be found in include/psa/crypto_config.h. + * + * If you enable this option and write your own configuration file, you must + * include mbedtls/config_psa.h in your configuration file. The default + * provided mbedtls/config.h contains the necessary inclusion. * * This feature is still experimental and is not ready for production since * it is not completed. From 3809f5f70a23ced4c759beec42d574d26e92f899 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Nov 2020 15:40:05 +0100 Subject: [PATCH 8/9] Add a build with MBEDTLS_ERROR_STRERROR_DUMMY Add a build with MBEDTLS_ERROR_STRERROR_DUMMY but not MBEDTLS_ERROR_C. Previously, both options were enabled by default, but MBEDTLS_ERROR_STRERROR_DUMMY only matters when MBEDTLS_ERROR_C is enabled, so its effect was not tested. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 757a9ecc9..88a3eaf81 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1895,6 +1895,20 @@ component_test_no_64bit_multiplication () { make test } +component_test_no_strings () { + msg "build: no strings" # ~10s + scripts/config.py full + # Disable options that activate a large amount of string constants. + scripts/config.py unset MBEDTLS_DEBUG_C + scripts/config.py unset MBEDTLS_ERROR_C + scripts/config.py set MBEDTLS_ERROR_STRERROR_DUMMY + scripts/config.py unset MBEDTLS_VERSION_FEATURES + make CFLAGS='-Werror -Os' + + msg "test: no strings" # ~ 10s + make test +} + component_build_arm_none_eabi_gcc () { msg "build: ${ARM_NONE_EABI_GCC_PREFIX}gcc -O1" # ~ 10s scripts/config.py baremetal From 67aed9ada68437b614e9f54b0d2dfb0683a452cc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Nov 2020 15:14:10 +0100 Subject: [PATCH 9/9] Simplify conditional guards in error.c Simplify the guards on MBEDTLS_ERROR_C and MBEDTLS_ERROR_STRERROR_DUMMY. No longer include superfluous headers and definition: platform.h is only needed for MBEDTLS_ERROR_C; time_t is not needed at all. Signed-off-by: Gilles Peskine --- library/error.c | 16 +++++++--------- scripts/data_files/error.fmt | 16 +++++++--------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/library/error.c b/library/error.c index 0d9f736b9..901a3699a 100644 --- a/library/error.c +++ b/library/error.c @@ -19,20 +19,20 @@ #include "common.h" +#include "mbedtls/error.h" + +#if defined(MBEDTLS_ERROR_C) || defined(MBEDTLS_ERROR_STRERROR_DUMMY) + #if defined(MBEDTLS_ERROR_C) -#include -#endif #if defined(MBEDTLS_PLATFORM_C) #include "mbedtls/platform.h" #else #define mbedtls_snprintf snprintf -#define mbedtls_time_t time_t #endif -#if defined(MBEDTLS_ERROR_C) - #include +#include #if defined(MBEDTLS_AES_C) #include "mbedtls/aes.h" @@ -960,8 +960,6 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) #else /* MBEDTLS_ERROR_C */ -#if defined(MBEDTLS_ERROR_STRERROR_DUMMY) - /* * Provide an non-function in case MBEDTLS_ERROR_C is not defined */ @@ -973,6 +971,6 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) buf[0] = '\0'; } -#endif /* MBEDTLS_ERROR_STRERROR_DUMMY */ - #endif /* MBEDTLS_ERROR_C */ + +#endif /* MBEDTLS_ERROR_C || MBEDTLS_ERROR_STRERROR_DUMMY */ diff --git a/scripts/data_files/error.fmt b/scripts/data_files/error.fmt index 1c8a79036..9e479bbfd 100644 --- a/scripts/data_files/error.fmt +++ b/scripts/data_files/error.fmt @@ -19,20 +19,20 @@ #include "common.h" +#include "mbedtls/error.h" + +#if defined(MBEDTLS_ERROR_C) || defined(MBEDTLS_ERROR_STRERROR_DUMMY) + #if defined(MBEDTLS_ERROR_C) -#include -#endif #if defined(MBEDTLS_PLATFORM_C) #include "mbedtls/platform.h" #else #define mbedtls_snprintf snprintf -#define mbedtls_time_t time_t #endif -#if defined(MBEDTLS_ERROR_C) - #include +#include HEADER_INCLUDED @@ -149,8 +149,6 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) #else /* MBEDTLS_ERROR_C */ -#if defined(MBEDTLS_ERROR_STRERROR_DUMMY) - /* * Provide an non-function in case MBEDTLS_ERROR_C is not defined */ @@ -162,6 +160,6 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) buf[0] = '\0'; } -#endif /* MBEDTLS_ERROR_STRERROR_DUMMY */ - #endif /* MBEDTLS_ERROR_C */ + +#endif /* MBEDTLS_ERROR_C || MBEDTLS_ERROR_STRERROR_DUMMY */