From bbaa2b784a17505810a22824e51006ee5bf16494 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 12 Apr 2020 13:33:57 +0200 Subject: [PATCH 01/22] Move long lists out of functions Signed-off-by: Gilles Peskine --- scripts/config.py | 102 ++++++++++++++++++++++++---------------------- 1 file changed, 53 insertions(+), 49 deletions(-) diff --git a/scripts/config.py b/scripts/config.py index b7a9a080e..2557cf194 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -159,42 +159,44 @@ def realfull_adapter(_name, active, section): return active return True +EXCLUDE_FROM_FULL = frozenset([ + 'MBEDTLS_CTR_DRBG_USE_128_BIT_KEY', + 'MBEDTLS_DEPRECATED_REMOVED', + 'MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED', + 'MBEDTLS_ECP_RESTARTABLE', + 'MBEDTLS_ENTROPY_FORCE_SHA256', # Variant toggle, tested separately + 'MBEDTLS_HAVE_SSE2', + 'MBEDTLS_MEMORY_BACKTRACE', + 'MBEDTLS_MEMORY_BUFFER_ALLOC_C', + 'MBEDTLS_MEMORY_DEBUG', + 'MBEDTLS_NO_64BIT_MULTIPLICATION', + 'MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES', + 'MBEDTLS_NO_PLATFORM_ENTROPY', + 'MBEDTLS_NO_UDBL_DIVISION', + 'MBEDTLS_PKCS11_C', + 'MBEDTLS_PLATFORM_NO_STD_FUNCTIONS', + 'MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER', + 'MBEDTLS_PSA_CRYPTO_SE_C', + 'MBEDTLS_PSA_CRYPTO_SPM', + 'MBEDTLS_PSA_INJECT_ENTROPY', + 'MBEDTLS_REMOVE_3DES_CIPHERSUITES', + 'MBEDTLS_REMOVE_ARC4_CIPHERSUITES', + 'MBEDTLS_RSA_NO_CRT', + 'MBEDTLS_SHA512_NO_SHA384', + 'MBEDTLS_SSL_HW_RECORD_ACCEL', + 'MBEDTLS_SSL_PROTO_SSL3', + 'MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO', + 'MBEDTLS_TEST_NULL_ENTROPY', + 'MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3', + 'MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION', + 'MBEDTLS_ZLIB_SUPPORT', +]) + def include_in_full(name): """Rules for symbols in the "full" configuration.""" if re.search(r'PLATFORM_[A-Z0-9]+_ALT', name): return True - if name in [ - 'MBEDTLS_CTR_DRBG_USE_128_BIT_KEY', - 'MBEDTLS_DEPRECATED_REMOVED', - 'MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED', - 'MBEDTLS_ECP_RESTARTABLE', - 'MBEDTLS_ENTROPY_FORCE_SHA256', # Variant toggle, tested separately - 'MBEDTLS_HAVE_SSE2', - 'MBEDTLS_MEMORY_BACKTRACE', - 'MBEDTLS_MEMORY_BUFFER_ALLOC_C', - 'MBEDTLS_MEMORY_DEBUG', - 'MBEDTLS_NO_64BIT_MULTIPLICATION', - 'MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES', - 'MBEDTLS_NO_PLATFORM_ENTROPY', - 'MBEDTLS_NO_UDBL_DIVISION', - 'MBEDTLS_PKCS11_C', - 'MBEDTLS_PLATFORM_NO_STD_FUNCTIONS', - 'MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER', - 'MBEDTLS_PSA_CRYPTO_SE_C', - 'MBEDTLS_PSA_CRYPTO_SPM', - 'MBEDTLS_PSA_INJECT_ENTROPY', - 'MBEDTLS_REMOVE_3DES_CIPHERSUITES', - 'MBEDTLS_REMOVE_ARC4_CIPHERSUITES', - 'MBEDTLS_RSA_NO_CRT', - 'MBEDTLS_SHA512_NO_SHA384', - 'MBEDTLS_SSL_HW_RECORD_ACCEL', - 'MBEDTLS_SSL_PROTO_SSL3', - 'MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO', - 'MBEDTLS_TEST_NULL_ENTROPY', - 'MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3', - 'MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION', - 'MBEDTLS_ZLIB_SUPPORT', - ]: + if name in EXCLUDE_FROM_FULL: return False if name.endswith('_ALT'): return False @@ -206,25 +208,27 @@ def full_adapter(name, active, section): return active return include_in_full(name) +EXCLUDE_FROM_BAREMETAL = frozenset([ + 'MBEDTLS_DEPRECATED_WARNING', + 'MBEDTLS_ENTROPY_NV_SEED', + 'MBEDTLS_FS_IO', + 'MBEDTLS_HAVEGE_C', + 'MBEDTLS_HAVE_TIME', + 'MBEDTLS_HAVE_TIME_DATE', + 'MBEDTLS_NET_C', + 'MBEDTLS_PLATFORM_FPRINTF_ALT', + 'MBEDTLS_PLATFORM_TIME_ALT', + 'MBEDTLS_PSA_CRYPTO_SE_C', + 'MBEDTLS_PSA_CRYPTO_STORAGE_C', + 'MBEDTLS_PSA_ITS_FILE_C', + 'MBEDTLS_THREADING_C', + 'MBEDTLS_THREADING_PTHREAD', + 'MBEDTLS_TIMING_C', +]) + def keep_in_baremetal(name): """Rules for symbols in the "baremetal" configuration.""" - if name in [ - 'MBEDTLS_DEPRECATED_WARNING', - 'MBEDTLS_ENTROPY_NV_SEED', - 'MBEDTLS_FS_IO', - 'MBEDTLS_HAVEGE_C', - 'MBEDTLS_HAVE_TIME', - 'MBEDTLS_HAVE_TIME_DATE', - 'MBEDTLS_NET_C', - 'MBEDTLS_PLATFORM_FPRINTF_ALT', - 'MBEDTLS_PLATFORM_TIME_ALT', - 'MBEDTLS_PSA_CRYPTO_SE_C', - 'MBEDTLS_PSA_CRYPTO_STORAGE_C', - 'MBEDTLS_PSA_ITS_FILE_C', - 'MBEDTLS_THREADING_C', - 'MBEDTLS_THREADING_PTHREAD', - 'MBEDTLS_TIMING_C', - ]: + if name in EXCLUDE_FROM_BAREMETAL: return False return True From cfffc28a8066c3364185279013d9611812e4746e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 12 Apr 2020 13:55:45 +0200 Subject: [PATCH 02/22] Document the full and baremetal configurations For each excluded symbol, explain why it's excluded. Signed-off-by: Gilles Peskine --- scripts/config.py | 107 +++++++++++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 44 deletions(-) diff --git a/scripts/config.py b/scripts/config.py index 2557cf194..d09353fd1 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -159,46 +159,58 @@ def realfull_adapter(_name, active, section): return active return True +# The goal of the full configuration is to have everything that can be tested +# together. This includes deprecated or insecure options. It excludes: +# * Options that require additional build dependencies or unusual hardware. +# * Options that make testing less effective. +# * Options that are incompatible with other options. +# * Options that remove features. +# * Options that are variants, so that we need to test both with and without. EXCLUDE_FROM_FULL = frozenset([ - 'MBEDTLS_CTR_DRBG_USE_128_BIT_KEY', - 'MBEDTLS_DEPRECATED_REMOVED', - 'MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED', - 'MBEDTLS_ECP_RESTARTABLE', - 'MBEDTLS_ENTROPY_FORCE_SHA256', # Variant toggle, tested separately - 'MBEDTLS_HAVE_SSE2', - 'MBEDTLS_MEMORY_BACKTRACE', - 'MBEDTLS_MEMORY_BUFFER_ALLOC_C', - 'MBEDTLS_MEMORY_DEBUG', - 'MBEDTLS_NO_64BIT_MULTIPLICATION', - 'MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES', - 'MBEDTLS_NO_PLATFORM_ENTROPY', - 'MBEDTLS_NO_UDBL_DIVISION', - 'MBEDTLS_PKCS11_C', - 'MBEDTLS_PLATFORM_NO_STD_FUNCTIONS', - 'MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER', + #pylint: disable=line-too-long + 'MBEDTLS_CTR_DRBG_USE_128_BIT_KEY', # variant toggle + 'MBEDTLS_DEPRECATED_REMOVED', # conflicts with deprecated options + 'MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED', # variant toggle + 'MBEDTLS_ECP_RESTARTABLE', # incompatible with USE_PSA_CRYPTO + 'MBEDTLS_ENTROPY_FORCE_SHA256', # variant toggle + 'MBEDTLS_HAVE_SSE2', # hardware dependency + 'MBEDTLS_MEMORY_BACKTRACE', # depends on MEMORY_BUFFER_ALLOC_C + 'MBEDTLS_MEMORY_BUFFER_ALLOC_C', # makes sanitizers (e.g. ASan) less effective + 'MBEDTLS_MEMORY_DEBUG', # depends on MEMORY_BUFFER_ALLOC_C + 'MBEDTLS_NO_64BIT_MULTIPLICATION', # variant toggle + 'MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES', # removes a feature + 'MBEDTLS_NO_PLATFORM_ENTROPY', # removes a feature + 'MBEDTLS_NO_UDBL_DIVISION', # variant toggle + 'MBEDTLS_PKCS11_C', # build dependecy (libpkcs11-helper) + 'MBEDTLS_PLATFORM_NO_STD_FUNCTIONS', # removes a feature + 'MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER', # variant toggle 'MBEDTLS_PSA_CRYPTO_SE_C', - 'MBEDTLS_PSA_CRYPTO_SPM', - 'MBEDTLS_PSA_INJECT_ENTROPY', - 'MBEDTLS_REMOVE_3DES_CIPHERSUITES', - 'MBEDTLS_REMOVE_ARC4_CIPHERSUITES', - 'MBEDTLS_RSA_NO_CRT', - 'MBEDTLS_SHA512_NO_SHA384', - 'MBEDTLS_SSL_HW_RECORD_ACCEL', + 'MBEDTLS_PSA_CRYPTO_SPM', # platform dependency (PSA SPM) + 'MBEDTLS_PSA_INJECT_ENTROPY', # build dependency (hook functions) + 'MBEDTLS_REMOVE_3DES_CIPHERSUITES', # removes a feature + 'MBEDTLS_REMOVE_ARC4_CIPHERSUITES', # removes a feature + 'MBEDTLS_RSA_NO_CRT', # variant toggle + 'MBEDTLS_SHA512_NO_SHA384', # removes a feature + 'MBEDTLS_SSL_HW_RECORD_ACCEL', # build dependency (hook functions) 'MBEDTLS_SSL_PROTO_SSL3', 'MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO', - 'MBEDTLS_TEST_NULL_ENTROPY', + 'MBEDTLS_TEST_NULL_ENTROPY', # removes a feature 'MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3', - 'MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION', - 'MBEDTLS_ZLIB_SUPPORT', + 'MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION', # variant toggle + 'MBEDTLS_ZLIB_SUPPORT', # build dependency (libz) ]) def include_in_full(name): """Rules for symbols in the "full" configuration.""" if re.search(r'PLATFORM_[A-Z0-9]+_ALT', name): + # Include configurable functions that default to the built-in function. + # This way we test that they're in place without changing the behavior. return True if name in EXCLUDE_FROM_FULL: return False if name.endswith('_ALT'): + # Exclude alt implementations since they require an implementation + # of the relevant functions. return False return True @@ -208,22 +220,28 @@ def full_adapter(name, active, section): return active return include_in_full(name) +# The baremetal configuration excludes options that require a library or +# operating system feature that is typically not present on bare metal +# systems. Features that are excluded from "full" won't be in "baremetal" +# either (unless explicitly turned on in baremetal_adapter) so they don't +# need to be repeated here. EXCLUDE_FROM_BAREMETAL = frozenset([ + #pylint: disable=line-too-long 'MBEDTLS_DEPRECATED_WARNING', - 'MBEDTLS_ENTROPY_NV_SEED', - 'MBEDTLS_FS_IO', - 'MBEDTLS_HAVEGE_C', - 'MBEDTLS_HAVE_TIME', - 'MBEDTLS_HAVE_TIME_DATE', - 'MBEDTLS_NET_C', - 'MBEDTLS_PLATFORM_FPRINTF_ALT', - 'MBEDTLS_PLATFORM_TIME_ALT', - 'MBEDTLS_PSA_CRYPTO_SE_C', - 'MBEDTLS_PSA_CRYPTO_STORAGE_C', - 'MBEDTLS_PSA_ITS_FILE_C', - 'MBEDTLS_THREADING_C', - 'MBEDTLS_THREADING_PTHREAD', - 'MBEDTLS_TIMING_C', + 'MBEDTLS_ENTROPY_NV_SEED', # requires FS_IO or alternate NV seed hooks + 'MBEDTLS_FS_IO', # requires a filesystem + 'MBEDTLS_HAVEGE_C', # requires a clock + 'MBEDTLS_HAVE_TIME', # requires a clock + 'MBEDTLS_HAVE_TIME_DATE', # requires a clock + 'MBEDTLS_NET_C', # requires POSIX-like networking + 'MBEDTLS_PLATFORM_FPRINTF_ALT', # requires FILE* from stdio.h + 'MBEDTLS_PLATFORM_TIME_ALT', # requires timing + 'MBEDTLS_PSA_CRYPTO_SE_C', # requires a filesystem + 'MBEDTLS_PSA_CRYPTO_STORAGE_C', # requires a filesystem + 'MBEDTLS_PSA_ITS_FILE_C', # requires a filesystem + 'MBEDTLS_THREADING_C', # requires a threading interface + 'MBEDTLS_THREADING_PTHREAD', # requires pthread + 'MBEDTLS_TIMING_C', # requires a clock ]) def keep_in_baremetal(name): @@ -237,6 +255,7 @@ def baremetal_adapter(name, active, section): if not is_full_section(section): return active if name == 'MBEDTLS_NO_PLATFORM_ENTROPY': + # No OS-provided entropy source return True return include_in_full(name) and keep_in_baremetal(name) @@ -247,10 +266,10 @@ def include_in_crypto(name): name.startswith('MBEDTLS_KEY_EXCHANGE_'): return False if name in [ - 'MBEDTLS_CERTS_C', - 'MBEDTLS_DEBUG_C', - 'MBEDTLS_NET_C', - 'MBEDTLS_PKCS11_C', + 'MBEDTLS_CERTS_C', # part of libmbedx509 + 'MBEDTLS_DEBUG_C', # part of libmbedtls + 'MBEDTLS_NET_C', # part of libmbedtls + 'MBEDTLS_PKCS11_C', # part of libmbedx509 ]: return False return True From 32e889dfc3c7808d254750f09f11f0c397482368 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 12 Apr 2020 23:43:28 +0200 Subject: [PATCH 03/22] Document and fix the MBEDTLS_xxx_ALT logic for the full config The intended logic around MBEDTLS_xxx_ALT is to exclude them from full because they require the alternative implementation of one or more library functions, except that MBEDTLS_PLATFORM_xxx_ALT are different: they're alternative implementations of a platform function and they have a built-in default, so they should be included in full. Document this. Fix a bug whereby MBEDTLS_PLATFORM_xxx_ALT didn't catch symbols where xxx contains an underscore. As a consequence, MBEDTLS_PLATFORM_GMTIME_R_ALT and MBEDTLS_PLATFORM_NV_SEED_ALT are now enabled in the full config. Explicitly exclude MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT because it behaves like the non-platform ones, requiring an extra build-time dependency. Explicitly exclude MBEDTLS_PLATFORM_NV_SEED_ALT from baremetal because it requires MBEDTLS_ENTROPY_NV_SEED, and likewise explicitly unset it from builds that unset MBEDTLS_ENTROPY_NV_SEED. Signed-off-by: Gilles Peskine --- scripts/config.py | 24 +++++++++++++++++------- tests/scripts/all.sh | 4 ++++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/scripts/config.py b/scripts/config.py index d09353fd1..cd69ebfde 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -183,6 +183,7 @@ EXCLUDE_FROM_FULL = frozenset([ 'MBEDTLS_NO_UDBL_DIVISION', # variant toggle 'MBEDTLS_PKCS11_C', # build dependecy (libpkcs11-helper) 'MBEDTLS_PLATFORM_NO_STD_FUNCTIONS', # removes a feature + 'MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT', # similar to non-platform xxx_ALT, requires platform_alt.h 'MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER', # variant toggle 'MBEDTLS_PSA_CRYPTO_SE_C', 'MBEDTLS_PSA_CRYPTO_SPM', # platform dependency (PSA SPM) @@ -200,18 +201,26 @@ EXCLUDE_FROM_FULL = frozenset([ 'MBEDTLS_ZLIB_SUPPORT', # build dependency (libz) ]) +def is_seamless_alt(name): + """Include xxx_ALT symbols that don't have external dependencies. + + Include alternative implementations of platform functions, which are + configurable function pointers that default to the built-in function. + This way we test that the function pointers exist and build correctly + without changing the behavior, and tests can verify that the function + pointers are used by modifying those pointers. + + Exclude alternative implementations of library functions since they require + an implementation of the relevant functions and an xxx_alt.h header. + """ + return name.startswith('MBEDTLS_PLATFORM_') + def include_in_full(name): """Rules for symbols in the "full" configuration.""" - if re.search(r'PLATFORM_[A-Z0-9]+_ALT', name): - # Include configurable functions that default to the built-in function. - # This way we test that they're in place without changing the behavior. - return True if name in EXCLUDE_FROM_FULL: return False if name.endswith('_ALT'): - # Exclude alt implementations since they require an implementation - # of the relevant functions. - return False + return is_seamless_alt(name) return True def full_adapter(name, active, section): @@ -235,6 +244,7 @@ EXCLUDE_FROM_BAREMETAL = frozenset([ 'MBEDTLS_HAVE_TIME_DATE', # requires a clock 'MBEDTLS_NET_C', # requires POSIX-like networking 'MBEDTLS_PLATFORM_FPRINTF_ALT', # requires FILE* from stdio.h + 'MBEDTLS_PLATFORM_NV_SEED_ALT', # requires a filesystem 'MBEDTLS_PLATFORM_TIME_ALT', # requires timing 'MBEDTLS_PSA_CRYPTO_SE_C', # requires a filesystem 'MBEDTLS_PSA_CRYPTO_STORAGE_C', # requires a filesystem diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 9b69aa204..1085de0a2 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1023,6 +1023,7 @@ component_test_check_params_without_platform () { scripts/config.py unset MBEDTLS_PLATFORM_TIME_ALT scripts/config.py unset MBEDTLS_PLATFORM_FPRINTF_ALT scripts/config.py unset MBEDTLS_PLATFORM_MEMORY + scripts/config.py unset MBEDTLS_PLATFORM_NV_SEED_ALT scripts/config.py unset MBEDTLS_PLATFORM_PRINTF_ALT scripts/config.py unset MBEDTLS_PLATFORM_SNPRINTF_ALT scripts/config.py unset MBEDTLS_ENTROPY_NV_SEED @@ -1052,6 +1053,7 @@ component_test_no_platform () { scripts/config.py unset MBEDTLS_PLATFORM_SNPRINTF_ALT scripts/config.py unset MBEDTLS_PLATFORM_TIME_ALT scripts/config.py unset MBEDTLS_PLATFORM_EXIT_ALT + scripts/config.py unset MBEDTLS_PLATFORM_NV_SEED_ALT scripts/config.py unset MBEDTLS_ENTROPY_NV_SEED scripts/config.py unset MBEDTLS_FS_IO scripts/config.py unset MBEDTLS_PSA_CRYPTO_SE_C @@ -1069,6 +1071,7 @@ component_build_no_std_function () { scripts/config.py full scripts/config.py set MBEDTLS_PLATFORM_NO_STD_FUNCTIONS scripts/config.py unset MBEDTLS_ENTROPY_NV_SEED + scripts/config.py unset MBEDTLS_PLATFORM_NV_SEED_ALT make CC=gcc CFLAGS='-Werror -Wall -Wextra -Os' } @@ -1252,6 +1255,7 @@ component_test_null_entropy () { scripts/config.py set MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES scripts/config.py set MBEDTLS_ENTROPY_C scripts/config.py unset MBEDTLS_ENTROPY_NV_SEED + scripts/config.py unset MBEDTLS_PLATFORM_NV_SEED_ALT scripts/config.py unset MBEDTLS_ENTROPY_HARDWARE_ALT scripts/config.py unset MBEDTLS_HAVEGE_C CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan -D UNSAFE_BUILD=ON . From 72d40fc6cab30872a3b0c5bbdc8fa27fe50369b0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 14 Apr 2020 21:28:42 +0200 Subject: [PATCH 04/22] Fix build failure with MBEDTLS_PLATFORM_NV_SEED_ALT Signed-off-by: Gilles Peskine --- tests/suites/test_suite_entropy.function | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/suites/test_suite_entropy.function b/tests/suites/test_suite_entropy.function index 9f10a9043..d9ea44149 100644 --- a/tests/suites/test_suite_entropy.function +++ b/tests/suites/test_suite_entropy.function @@ -1,6 +1,7 @@ /* BEGIN_HEADER */ #include "mbedtls/entropy.h" #include "mbedtls/entropy_poll.h" +#include "mbedtls/md.h" #include "string.h" typedef enum From 6710e159216515f9fa0eea462e617539b042fbb5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 12 Apr 2020 14:21:30 +0200 Subject: [PATCH 05/22] Enable X509_ALLOW_EXTENSIONS_NON_V3 in config full and fix tests Enable MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 in the full config. There's no reason to keep it out. We weren't testing it at all on the CI. Add a missing dependency on !MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 to some test cases that either were testing that v3 extensions are only accepted in v3 certificates, or where parsing returns a different error when MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 is enabled. Add a few positive and negative test cases with MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 enabled. Fix one test case with MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 where the intended output of mbedtls_x509_crt_info had changed in 890819a597bd7a67f0b8185a4409733504f792b3 but the test case was missed because it was never executed. Signed-off-by: Gilles Peskine --- scripts/config.py | 1 - tests/suites/test_suite_x509parse.data | 32 +++++++++++++++++++++----- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/scripts/config.py b/scripts/config.py index cd69ebfde..34b79e032 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -196,7 +196,6 @@ EXCLUDE_FROM_FULL = frozenset([ 'MBEDTLS_SSL_PROTO_SSL3', 'MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO', 'MBEDTLS_TEST_NULL_ENTROPY', # removes a feature - 'MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3', 'MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION', # variant toggle 'MBEDTLS_ZLIB_SUPPORT', # build dependency (libz) ]) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 3099e632d..6ae13cb0d 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -176,7 +176,7 @@ x509_cert_info:"data_files/bitstring-in-dn.pem":"cert. version \: 3\nserial X509 certificate v1 with extension depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3:MBEDTLS_SHA1_C -x509_cert_info:"data_files/cert_v1_with_ext.crt":"cert. version \: 1\nserial number \: BD\:ED\:44\:C7\:D2\:3E\:C2\:A4\nissuer name \: C=XX, ST=XX, L=XX, O=XX, OU=XX, emailAddress=admin@identity-check.org, CN=identity-check.org\nsubject name \: C=XX, ST=XX, L=XX, O=XX, OU=XX, emailAddress=admin@identity-check.org, CN=identity-check.org\nissued on \: 2013-07-04 16\:17\:02\nexpires on \: 2014-07-04 16\:17\:02\nsigned using \: RSA with SHA1\nRSA key size \: 2048 bits\nsubject alt name \:\n dNSName \: identity-check.org\n dNSName \: www.identity-check.org\n" +x509_cert_info:"data_files/cert_v1_with_ext.crt":"cert. version \: 1\nserial number \: BD\:ED\:44\:C7\:D2\:3E\:C2\:A4\nissuer name \: C=XX, ST=XX, L=XX, O=XX, OU=XX, emailAddress=admin@identity-check.org, CN=identity-check.org\nsubject name \: C=XX, ST=XX, L=XX, O=XX, OU=XX, emailAddress=admin@identity-check.org, CN=identity-check.org\nissued on \: 2013-07-04 16\:17\:02\nexpires on \: 2014-07-04 16\:17\:02\nsigned using \: RSA with SHA1\nRSA key size \: 2048 bits\nsubject alt name \:\n dNSName \: identity-check.org\n dNSName \: www.identity-check.org\n \n" X509 SAN parsing otherName depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C @@ -1563,7 +1563,7 @@ depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C x509parse_crt:"308198308182a0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa201300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_OUT_OF_DATA X509 CRT ASN1 (TBS, valid IssuerID, inv SubjectID, inv tag) -depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +depends_on:!MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3:MBEDTLS_RSA_C:MBEDTLS_SHA256_C x509parse_crt:"30819a308184a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa1000500300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH X509 CRT ASN1 (TBSCertificate v3, ext SubjectAlternativeName malformed) @@ -1583,13 +1583,21 @@ depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C x509parse_crt:"30819a308184a0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa100a201300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_OUT_OF_DATA X509 CRT ASN1 (TBS, IssuerID unsupported in v1 CRT) -depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +depends_on:!MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3:MBEDTLS_RSA_C:MBEDTLS_SHA256_C x509parse_crt:"30819a308184a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa100a201300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH +X509 CRT ASN1 (TBS, IssuerID unsupported in v1 CRT, ALLOW_EXTENSIONS_NON_V3) +depends_on:MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +x509parse_crt:"30819a308184a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa100a201300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG + X509 CRT ASN1 (TBS, SubjectID unsupported in v1 CRT) -depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +depends_on:!MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3:MBEDTLS_RSA_C:MBEDTLS_SHA256_C x509parse_crt:"30819a308184a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa200a201300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH +X509 CRT ASN1 (TBS, SubjectID unsupported in v1 CRT, ALLOW_EXTENSIONS_NON_V3) +depends_on:MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +x509parse_crt:"30819a308184a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa200a201300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG + X509 CRT ASN1 (TBS, inv v3Ext, inv tag) depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C x509parse_crt:"30819c308186a0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa100a2000500300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG @@ -1830,12 +1838,24 @@ X509 CRT ASN1 (TBS, inv v3Ext, SubjectAltName repeated outside Extensions) depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C x509parse_crt:"3081dc3081c6a0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa100a200a321301f301d0603551d11041630148208666f6f2e7465737482086261722e74657374301d0603551d11041630148208666f6f2e7465737482086261722e74657374300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH -X509 CRT ASN1 (TBS, valid v3Ext in v1 CRT) +X509 CRT (TBS, valid v3Ext in v1 CRT, ALLOW_EXTENSIONS_NON_V3) +depends_on:MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +x509parse_crt:"3081b93081a3a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa321301f301d0603551d11041630148208666f6f2e7465737482086261722e74657374300d06092a864886f70d01010b0500030200ff":"cert. version \: 1\nserial number \: DE\:AD\:BE\:EF\nissuer name \: ??=Test\nsubject name \: ??=Test\nissued on \: 2009-01-01 00\:00\:00\nexpires on \: 2009-12-31 23\:59\:59\nsigned using \: RSA with SHA-256\nRSA key size \: 128 bits\nsubject alt name \:\n dNSName \: foo.test\n dNSName \: bar.test\n":0 + +X509 CRT (TBS, valid v3Ext in v2 CRT, ALLOW_EXTENSIONS_NON_V3) +depends_on:MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +x509parse_crt:"3081b93081a3a0030201018204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa321301f301d0603551d11041630148208666f6f2e7465737482086261722e74657374300d06092a864886f70d01010b0500030200ff":"cert. version \: 2\nserial number \: DE\:AD\:BE\:EF\nissuer name \: ??=Test\nsubject name \: ??=Test\nissued on \: 2009-01-01 00\:00\:00\nexpires on \: 2009-12-31 23\:59\:59\nsigned using \: RSA with SHA-256\nRSA key size \: 128 bits\nsubject alt name \:\n dNSName \: foo.test\n dNSName \: bar.test\n":0 + +X509 CRT (TBS, valid v3Ext in v3 CRT) depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +x509parse_crt:"3081b93081a3a0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa321301f301d0603551d11041630148208666f6f2e7465737482086261722e74657374300d06092a864886f70d01010b0500030200ff":"cert. version \: 3\nserial number \: DE\:AD\:BE\:EF\nissuer name \: ??=Test\nsubject name \: ??=Test\nissued on \: 2009-01-01 00\:00\:00\nexpires on \: 2009-12-31 23\:59\:59\nsigned using \: RSA with SHA-256\nRSA key size \: 128 bits\nsubject alt name \:\n dNSName \: foo.test\n dNSName \: bar.test\n":0 + +X509 CRT ASN1 (TBS, valid v3Ext in v1 CRT) +depends_on:!MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3:MBEDTLS_RSA_C:MBEDTLS_SHA256_C x509parse_crt:"3081b93081a3a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa321301f301d0603551d11041630148208666f6f2e7465737482086261722e74657374300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH X509 CRT ASN1 (TBS, valid v3Ext in v2 CRT) -depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +depends_on:!MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3:MBEDTLS_RSA_C:MBEDTLS_SHA256_C x509parse_crt:"3081b93081a3a0030201018204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa321301f301d0603551d11041630148208666f6f2e7465737482086261722e74657374300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH X509 CRT ASN1 (TBS, valid SubjectID, valid IssuerID, inv v3Ext, SubjectAltName repeated outside Extensions, inv SubjectAltNames tag) From dc6d838a73316736c6a094319be3363d7f4afd62 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 12 Apr 2020 14:21:55 +0200 Subject: [PATCH 06/22] Enable MBEDTLS_PSA_CRYPTO_SE_C in config full It started out as be experimental, but it is now robust enough not to break the rest, so there's no reason to leave it out. Signed-off-by: Gilles Peskine --- scripts/config.py | 1 - tests/scripts/all.sh | 11 +---------- tests/scripts/basic-build-test.sh | 3 +-- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/scripts/config.py b/scripts/config.py index 34b79e032..79e8e4e09 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -185,7 +185,6 @@ EXCLUDE_FROM_FULL = frozenset([ 'MBEDTLS_PLATFORM_NO_STD_FUNCTIONS', # removes a feature 'MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT', # similar to non-platform xxx_ALT, requires platform_alt.h 'MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER', # variant toggle - 'MBEDTLS_PSA_CRYPTO_SE_C', 'MBEDTLS_PSA_CRYPTO_SPM', # platform dependency (PSA SPM) 'MBEDTLS_PSA_INJECT_ENTROPY', # build dependency (hook functions) 'MBEDTLS_REMOVE_3DES_CIPHERSUITES', # removes a feature diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 1085de0a2..34e71c79e 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -985,6 +985,7 @@ component_test_no_use_psa_crypto_full_cmake_asan() { scripts/config.py unset MBEDTLS_PSA_CRYPTO_C scripts/config.py unset MBEDTLS_USE_PSA_CRYPTO scripts/config.py unset MBEDTLS_PSA_ITS_FILE_C + scripts/config.py unset MBEDTLS_PSA_CRYPTO_SE_C scripts/config.py unset MBEDTLS_PSA_CRYPTO_STORAGE_C CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . make @@ -1373,16 +1374,6 @@ component_test_se_default () { make test } -component_test_se_full () { - msg "build: full config + MBEDTLS_PSA_CRYPTO_SE_C" - scripts/config.py full - scripts/config.py set MBEDTLS_PSA_CRYPTO_SE_C - make CC=gcc CFLAGS="$ASAN_CFLAGS -O2" LDFLAGS="$ASAN_CFLAGS" - - msg "test: full config + MBEDTLS_PSA_CRYPTO_SE_C" - make test -} - component_test_make_shared () { msg "build/test: make shared" # ~ 40s make SHARED=1 all check diff --git a/tests/scripts/basic-build-test.sh b/tests/scripts/basic-build-test.sh index aca2f11fb..5080c5790 100755 --- a/tests/scripts/basic-build-test.sh +++ b/tests/scripts/basic-build-test.sh @@ -68,10 +68,9 @@ export LDFLAGS=' --coverage' make clean cp "$CONFIG_H" "$CONFIG_BAK" scripts/config.py full -# Enable some deprecated or experimental features that are not in the +# Enable some deprecated features that are not in the # full config, but are compatible with it and have tests. scripts/config.py set MBEDTLS_SSL_PROTO_SSL3 -scripts/config.py set MBEDTLS_PSA_CRYPTO_SE_C make -j From 01fd875b32223f5ca38ac9b14e92d0c83fbb0167 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 14 Apr 2020 19:31:52 +0200 Subject: [PATCH 07/22] Strict C99: Don't repeat the typedef for psa_se_drv_table_entry_t GCC and Clang accept ``` typedef struct foo foo_t; typedef struct foo { ... } foo_t; ``` But this is not valid ISO C due to the redefinition of `foo_t`. Signed-off-by: Gilles Peskine --- library/psa_crypto_se.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index b7fa0c5c5..9418b5f45 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -64,7 +64,7 @@ typedef struct uintptr_t transient_data; } psa_drv_se_internal_context_t; -typedef struct psa_se_drv_table_entry_s +struct psa_se_drv_table_entry_s { psa_key_lifetime_t lifetime; const psa_drv_se_t *methods; @@ -73,7 +73,7 @@ typedef struct psa_se_drv_table_entry_s psa_drv_se_internal_context_t internal; psa_drv_se_context_t context; }; -} psa_se_drv_table_entry_t; +}; static psa_se_drv_table_entry_t driver_table[PSA_MAX_SE_DRIVERS]; From 1a75d0c15566f0d34d3b8eac92438f7a0d7e6153 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 14 Apr 2020 19:33:25 +0200 Subject: [PATCH 08/22] Strict C99: don't use an anonymous union field GCC and Clang accept anonymous union fields, but this is not valid ISO C. Use a named field. Signed-off-by: Gilles Peskine --- library/psa_crypto_se.c | 42 ++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index 9418b5f45..b9f186a96 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -72,7 +72,7 @@ struct psa_se_drv_table_entry_s { psa_drv_se_internal_context_t internal; psa_drv_se_context_t context; - }; + } u; }; static psa_se_drv_table_entry_t driver_table[PSA_MAX_SE_DRIVERS]; @@ -104,7 +104,7 @@ const psa_drv_se_t *psa_get_se_driver_methods( psa_drv_se_context_t *psa_get_se_driver_context( psa_se_drv_table_entry_t *driver ) { - return( &driver->context ); + return( &driver->u.context ); } int psa_get_se_driver( psa_key_lifetime_t lifetime, @@ -115,7 +115,7 @@ int psa_get_se_driver( psa_key_lifetime_t lifetime, if( p_methods != NULL ) *p_methods = ( driver ? driver->methods : NULL ); if( p_drv_context != NULL ) - *p_drv_context = ( driver ? &driver->context : NULL ); + *p_drv_context = ( driver ? &driver->u.context : NULL ); return( driver != NULL ); } @@ -134,7 +134,7 @@ static psa_status_t psa_get_se_driver_its_file_uid( #if SIZE_MAX > UINT32_MAX /* ITS file sizes are limited to 32 bits. */ - if( driver->internal.persistent_data_size > UINT32_MAX ) + if( driver->u.internal.persistent_data_size > UINT32_MAX ) return( PSA_ERROR_NOT_SUPPORTED ); #endif @@ -162,8 +162,8 @@ psa_status_t psa_load_se_persistent_data( * persistent_data_size is in range, but compilers don't know that, * so cast to reassure them. */ return( psa_its_get( uid, 0, - (uint32_t) driver->internal.persistent_data_size, - driver->internal.persistent_data, + (uint32_t) driver->u.internal.persistent_data_size, + driver->u.internal.persistent_data, &length ) ); } @@ -181,8 +181,8 @@ psa_status_t psa_save_se_persistent_data( * persistent_data_size is in range, but compilers don't know that, * so cast to reassure them. */ return( psa_its_set( uid, - (uint32_t) driver->internal.persistent_data_size, - driver->internal.persistent_data, + (uint32_t) driver->u.internal.persistent_data_size, + driver->u.internal.persistent_data, 0 ) ); } @@ -221,8 +221,8 @@ psa_status_t psa_find_se_slot_for_key( driver->methods->key_management->p_validate_slot_number; if( p_validate_slot_number == NULL ) return( PSA_ERROR_NOT_SUPPORTED ); - status = p_validate_slot_number( &driver->context, - driver->internal.persistent_data, + status = p_validate_slot_number( &driver->u.context, + driver->u.internal.persistent_data, attributes, method, *slot_number ); } @@ -240,8 +240,8 @@ psa_status_t psa_find_se_slot_for_key( driver->methods->key_management->p_allocate; if( p_allocate == NULL ) return( PSA_ERROR_NOT_SUPPORTED ); - status = p_allocate( &driver->context, - driver->internal.persistent_data, + status = p_allocate( &driver->u.context, + driver->u.internal.persistent_data, attributes, method, slot_number ); } @@ -265,8 +265,8 @@ psa_status_t psa_destroy_se_key( psa_se_drv_table_entry_t *driver, driver->methods->key_management->p_destroy == NULL ) return( PSA_ERROR_NOT_PERMITTED ); status = driver->methods->key_management->p_destroy( - &driver->context, - driver->internal.persistent_data, + &driver->u.context, + driver->u.internal.persistent_data, slot_number ); storage_status = psa_save_se_persistent_data( driver ); return( status == PSA_SUCCESS ? storage_status : status ); @@ -284,8 +284,8 @@ psa_status_t psa_init_all_se_drivers( void ) if( methods->p_init != NULL ) { psa_status_t status = methods->p_init( - &driver->context, - driver->internal.persistent_data, + &driver->u.context, + driver->u.internal.persistent_data, driver->lifetime ); if( status != PSA_SUCCESS ) return( status ); @@ -341,14 +341,14 @@ psa_status_t psa_register_se_driver( driver_table[i].lifetime = lifetime; driver_table[i].methods = methods; - driver_table[i].internal.persistent_data_size = + driver_table[i].u.internal.persistent_data_size = methods->persistent_data_size; if( methods->persistent_data_size != 0 ) { - driver_table[i].internal.persistent_data = + driver_table[i].u.internal.persistent_data = mbedtls_calloc( 1, methods->persistent_data_size ); - if( driver_table[i].internal.persistent_data == NULL ) + if( driver_table[i].u.internal.persistent_data == NULL ) { status = PSA_ERROR_INSUFFICIENT_MEMORY; goto error; @@ -373,8 +373,8 @@ void psa_unregister_all_se_drivers( void ) size_t i; for( i = 0; i < PSA_MAX_SE_DRIVERS; i++ ) { - if( driver_table[i].internal.persistent_data != NULL ) - mbedtls_free( driver_table[i].internal.persistent_data ); + if( driver_table[i].u.internal.persistent_data != NULL ) + mbedtls_free( driver_table[i].u.internal.persistent_data ); } memset( driver_table, 0, sizeof( driver_table ) ); } From a5fc939bddbf71f1f80897649ff633763e82cdde Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 14 Apr 2020 19:34:19 +0200 Subject: [PATCH 09/22] Strict C99: don't use a signed* when an unsigned* is expected It works in practice on almost every platform, given that we're only using the wrong type in cases where the value is guaranteed to stay within the value bits of a signed int. But even in this case it may or may not be strictly conforming. Anyway `gcc -std=c99 -pedantic` rejects it. Signed-off-by: Gilles Peskine --- programs/aes/crypt_and_hash.c | 3 ++- programs/pkey/pk_decrypt.c | 3 ++- programs/pkey/rsa_decrypt.c | 2 +- programs/pkey/rsa_verify.c | 3 ++- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/programs/aes/crypt_and_hash.c b/programs/aes/crypt_and_hash.c index a5acf5b8b..472c28aa8 100644 --- a/programs/aes/crypt_and_hash.c +++ b/programs/aes/crypt_and_hash.c @@ -85,7 +85,8 @@ int main( void ) int main( int argc, char *argv[] ) { - int ret = 1, i, n; + int ret = 1, i; + unsigned n; int exit_code = MBEDTLS_EXIT_FAILURE; int mode; size_t keylen, ilen, olen; diff --git a/programs/pkey/pk_decrypt.c b/programs/pkey/pk_decrypt.c index bf425079e..998c7ac89 100644 --- a/programs/pkey/pk_decrypt.c +++ b/programs/pkey/pk_decrypt.c @@ -64,7 +64,8 @@ int main( void ) int main( int argc, char *argv[] ) { FILE *f; - int ret = 1, c; + int ret = 1; + unsigned c; int exit_code = MBEDTLS_EXIT_FAILURE; size_t i, olen = 0; mbedtls_pk_context pk; diff --git a/programs/pkey/rsa_decrypt.c b/programs/pkey/rsa_decrypt.c index ff71bd055..c725f7ca1 100644 --- a/programs/pkey/rsa_decrypt.c +++ b/programs/pkey/rsa_decrypt.c @@ -65,7 +65,7 @@ int main( int argc, char *argv[] ) FILE *f; int ret = 1; int exit_code = MBEDTLS_EXIT_FAILURE; - int c; + unsigned c; size_t i; mbedtls_rsa_context rsa; mbedtls_mpi N, P, Q, D, E, DP, DQ, QP; diff --git a/programs/pkey/rsa_verify.c b/programs/pkey/rsa_verify.c index 94f0ef9ce..74030e45e 100644 --- a/programs/pkey/rsa_verify.c +++ b/programs/pkey/rsa_verify.c @@ -59,7 +59,8 @@ int main( void ) int main( int argc, char *argv[] ) { FILE *f; - int ret = 1, c; + int ret = 1; + unsigned c; int exit_code = MBEDTLS_EXIT_FAILURE; size_t i; mbedtls_rsa_context rsa; From 31f88a29dee0b8ab5d93c34e83d118daf01a7b21 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 14 Apr 2020 19:39:56 +0200 Subject: [PATCH 10/22] Strict C99: make sure that fileno() is declared only declares the non-ISO-C function fileno() if an appropriate POSIX symbol is defined or if using a compiler such as GCC in non-pedantic mode. Define the appropriate POSIX symbol. Signed-off-by: Gilles Peskine --- tests/suites/main_test.function | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index a1ba61058..db1c88a80 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -19,6 +19,12 @@ * This file is part of Mbed TLS (https://tls.mbed.org) */ +#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) +#if !defined(_POSIX_C_SOURCE) +#define _POSIX_C_SOURCE 1 // for fileno() from +#endif +#endif + #if !defined(MBEDTLS_CONFIG_FILE) #include #else From b2971ff942f69094594274e858075bd2ca55fe5f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 14 Apr 2020 19:41:01 +0200 Subject: [PATCH 11/22] Strict C99: don't use extremely large string literals Don't use string literals that are longer than 4095 bytes, which is the minimum that C99 compilers are required to support. Compilers are extremely likely to support longer literals, but `gcc -std=c99 -pedantic` complains. Signed-off-by: Gilles Peskine --- programs/ssl/ssl_client2.c | 18 +++++++++++++----- programs/ssl/ssl_server2.c | 18 +++++++++++++----- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index f6284feeb..97088916f 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -370,7 +370,9 @@ int main( void ) #define USAGE_SERIALIZATION "" #endif -#define USAGE \ +/* USAGE is arbitrarily split to stay under the portable string literal + * length limit: 4095 bytes in C99. */ +#define USAGE1 \ "\n usage: ssl_client2 param=<>...\n" \ "\n acceptable parameters:\n" \ " server_name=%%s default: localhost\n" \ @@ -394,7 +396,8 @@ int main( void ) "\n" \ USAGE_DTLS \ USAGE_CID \ - "\n" \ + "\n" +#define USAGE2 \ " auth_mode=%%s default: (library default: none)\n" \ " options: none, optional, required\n" \ USAGE_IO \ @@ -404,7 +407,8 @@ int main( void ) USAGE_PSK \ USAGE_ECJPAKE \ USAGE_ECRESTART \ - "\n" \ + "\n" +#define USAGE3 \ " allow_legacy=%%d default: (library default: no)\n" \ USAGE_RENEGO \ " exchanges=%%d default: 1\n" \ @@ -427,7 +431,8 @@ int main( void ) USAGE_CURVES \ USAGE_RECSPLIT \ USAGE_DHMLEN \ - "\n" \ + "\n" +#define USAGE4 \ " arc4=%%d default: (library default: 0)\n" \ " allow_sha1=%%d default: 0\n" \ " min_version=%%s default: (library default: tls1)\n" \ @@ -1234,7 +1239,10 @@ int main( int argc, char *argv[] ) if( ret == 0 ) ret = 1; - mbedtls_printf( USAGE ); + mbedtls_printf( USAGE1 ); + mbedtls_printf( USAGE2 ); + mbedtls_printf( USAGE3 ); + mbedtls_printf( USAGE4 ); list = mbedtls_ssl_list_ciphersuites(); while( *list ) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 845881f93..76b1ba6cc 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -456,7 +456,9 @@ int main( void ) #define USAGE_SERIALIZATION "" #endif -#define USAGE \ +/* USAGE is arbitrarily split to stay under the portable string literal + * length limit: 4095 bytes in C99. */ +#define USAGE1 \ "\n usage: ssl_server2 param=<>...\n" \ "\n acceptable parameters:\n" \ " server_addr=%%s default: (all interfaces)\n" \ @@ -477,7 +479,8 @@ int main( void ) USAGE_COOKIES \ USAGE_ANTI_REPLAY \ USAGE_BADMAC_LIMIT \ - "\n" \ + "\n" +#define USAGE2 \ " auth_mode=%%s default: (library default: none)\n" \ " options: none, optional, required\n" \ " cert_req_ca_list=%%d default: 1 (send ca list)\n" \ @@ -489,7 +492,8 @@ int main( void ) USAGE_PSK \ USAGE_CA_CALLBACK \ USAGE_ECJPAKE \ - "\n" \ + "\n" +#define USAGE3 \ " allow_legacy=%%d default: (library default: no)\n" \ USAGE_RENEGO \ " exchanges=%%d default: 1\n" \ @@ -506,7 +510,8 @@ int main( void ) USAGE_EMS \ USAGE_ETM \ USAGE_CURVES \ - "\n" \ + "\n" +#define USAGE4 \ " arc4=%%d default: (library default: 0)\n" \ " allow_sha1=%%d default: 0\n" \ " min_version=%%s default: (library default: tls1)\n" \ @@ -1900,7 +1905,10 @@ int main( int argc, char *argv[] ) if( ret == 0 ) ret = 1; - mbedtls_printf( USAGE ); + mbedtls_printf( USAGE1 ); + mbedtls_printf( USAGE2 ); + mbedtls_printf( USAGE3 ); + mbedtls_printf( USAGE4 ); list = mbedtls_ssl_list_ciphersuites(); while( *list ) From e094a18f6bf2e8effffffa2a644d7207ae2f4a3d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 14 Apr 2020 20:08:41 +0200 Subject: [PATCH 12/22] Strict C99: check it in the full config Ensure that there is a build with -pedantic in the full config, not just in "exotic" configurations. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 34e71c79e..9fd6522e7 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1392,7 +1392,7 @@ test_build_opt () { info=$1 cc=$2; shift 2 for opt in "$@"; do msg "build/test: $cc $opt, $info" # ~ 30s - make CC="$cc" CFLAGS="$opt -Wall -Wextra -Werror" + make CC="$cc" CFLAGS="$opt -std=c99 -pedantic -Wall -Wextra -Werror" # We're confident enough in compilers to not run _all_ the tests, # but at least run the unit tests. In particular, runs with # optimizations use inline assembly whereas runs with -O0 From 90581ee629dd1926154cc62f38091e2d8b80daca Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 12 Apr 2020 14:02:47 +0200 Subject: [PATCH 13/22] Turn off DEPRECATED_WARNING in full and baremetal MBEDTLS_DEPRECATED_REMOVED is turned off in full since we don't want to turn off deprecated features. Also turn off MBEDTLS_DEPRECATED_WARNING since we wouldn't want expected warnings: we're aware that we're enabling deprecated modules. Since MBEDTLS_DEPRECATED_WARNING is excluded from full, it doesn't need to be excluded from baremetal explicitly. Signed-off-by: Gilles Peskine --- scripts/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/config.py b/scripts/config.py index 79e8e4e09..acc92100a 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -170,6 +170,7 @@ EXCLUDE_FROM_FULL = frozenset([ #pylint: disable=line-too-long 'MBEDTLS_CTR_DRBG_USE_128_BIT_KEY', # variant toggle 'MBEDTLS_DEPRECATED_REMOVED', # conflicts with deprecated options + 'MBEDTLS_DEPRECATED_WARNING', # conflicts with deprecated options 'MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED', # variant toggle 'MBEDTLS_ECP_RESTARTABLE', # incompatible with USE_PSA_CRYPTO 'MBEDTLS_ENTROPY_FORCE_SHA256', # variant toggle @@ -234,7 +235,6 @@ def full_adapter(name, active, section): # need to be repeated here. EXCLUDE_FROM_BAREMETAL = frozenset([ #pylint: disable=line-too-long - 'MBEDTLS_DEPRECATED_WARNING', 'MBEDTLS_ENTROPY_NV_SEED', # requires FS_IO or alternate NV seed hooks 'MBEDTLS_FS_IO', # requires a filesystem 'MBEDTLS_HAVEGE_C', # requires a clock From be1d609c19d97843f8212132c1a2b22337ceee68 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 12 Apr 2020 14:17:16 +0200 Subject: [PATCH 14/22] New config: full_non_deprecated Enable everything that can be tested together and isn't deprecated. Signed-off-by: Gilles Peskine --- scripts/config.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/scripts/config.py b/scripts/config.py index acc92100a..62891ce95 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -296,6 +296,28 @@ def crypto_adapter(adapter): return adapter(name, active, section) return continuation +DEPRECATED = frozenset([ + 'MBEDTLS_SSL_PROTO_SSL3', + 'MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO', +]) + +def non_deprecated_adapter(adapter): + """Modify an adapter to disable deprecated symbols. + + ``non_deprecated_adapter(adapter)(name, active, section)`` is like + ``adapter(name, active, section)``, but unsets all deprecated symbols + and sets ``MBEDTLS_DEPRECATED_REMOVED``. + """ + def continuation(name, active, section): + if name == 'MBEDTLS_DEPRECATED_REMOVED': + return True + if name in DEPRECATED: + return False + if adapter is None: + return active + return adapter(name, active, section) + return continuation + class ConfigFile(Config): """Representation of the Mbed TLS configuration read for a file. @@ -457,6 +479,10 @@ if __name__ == '__main__': Exclude alternative implementations and platform support options, as well as some options that are awkward to test. """) + add_adapter('full_non_deprecated', non_deprecated_adapter(full_adapter), + """Uncomment most non-deprecated features. + Like "full", but without deprecated features. + """) add_adapter('realfull', realfull_adapter, """Uncomment all boolean #defines. Suitable for generating documentation, but not for building.""") From 3a584aecca23de40129d0596a3b0409f135d904a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 12 Apr 2020 23:15:51 +0200 Subject: [PATCH 15/22] Enable SSLv3 in the full config It's deprecated, but not otherwise counter-indicated for the full config: it doesn't conflict with anything and enabling it doesn't make testing harder (especially since it defaults off in compat.sh). Signed-off-by: Gilles Peskine --- scripts/config.py | 2 -- tests/scripts/basic-build-test.sh | 3 --- 2 files changed, 5 deletions(-) diff --git a/scripts/config.py b/scripts/config.py index 62891ce95..aeba64f26 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -193,8 +193,6 @@ EXCLUDE_FROM_FULL = frozenset([ 'MBEDTLS_RSA_NO_CRT', # variant toggle 'MBEDTLS_SHA512_NO_SHA384', # removes a feature 'MBEDTLS_SSL_HW_RECORD_ACCEL', # build dependency (hook functions) - 'MBEDTLS_SSL_PROTO_SSL3', - 'MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO', 'MBEDTLS_TEST_NULL_ENTROPY', # removes a feature 'MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION', # variant toggle 'MBEDTLS_ZLIB_SUPPORT', # build dependency (libz) diff --git a/tests/scripts/basic-build-test.sh b/tests/scripts/basic-build-test.sh index 5080c5790..08c141052 100755 --- a/tests/scripts/basic-build-test.sh +++ b/tests/scripts/basic-build-test.sh @@ -68,9 +68,6 @@ export LDFLAGS=' --coverage' make clean cp "$CONFIG_H" "$CONFIG_BAK" scripts/config.py full -# Enable some deprecated features that are not in the -# full config, but are compatible with it and have tests. -scripts/config.py set MBEDTLS_SSL_PROTO_SSL3 make -j From 1093d9f9afa89f310ef0b569a53c28b13f4b004a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 13 Apr 2020 01:03:21 +0200 Subject: [PATCH 16/22] all.sh: reorganize testing around deprecated features build_deprecated combined the testing of deprecated features, and testing of the build without deprecated features. Also, it violated the component naming convention by being called build_xxx but running tests. Replace it by: * test_default_no_deprecated: check that you can remove deprecated features from the default build. * test_full_no_deprecated: check that the library builds when deprecated features are disabled (and incidentally that the tests run). * test_no_deprecated_warning: check that there are no warnings when deprecated features are disabled and MBEDTLS_DEPRECATED_WARNING is enabled. * test_deprecated: test the deprecated features. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 55 +++++++++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 9fd6522e7..83d2d1bfe 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -897,26 +897,49 @@ component_test_full_cmake_clang () { if_build_succeeded env OPENSSL_CMD="$OPENSSL_NEXT" tests/compat.sh -e '^$' -f 'ARIA\|CHACHA' } -component_build_deprecated () { - msg "build: make, full config + DEPRECATED_WARNING, gcc -O" # ~ 30s +component_test_default_no_deprecated () { + msg "build: make, default + MBEDTLS_DEPRECATED_REMOVED" # ~ 30s + scripts/config.py set MBEDTLS_DEPRECATED_REMOVED + make CC=gcc CFLAGS='-O -Werror -Wall -Wextra' + + msg "test: make, default + MBEDTLS_DEPRECATED_REMOVED" # ~ 5s + make test +} + +component_test_full_no_deprecated () { + msg "build: make, full_non_deprecated config" # ~ 30s + scripts/config.py full_non_deprecated + make CC=gcc CFLAGS='-O -Werror -Wall -Wextra' + + msg "test: make, full_non_deprecated config" # ~ 5s + make test +} + +component_test_no_deprecated_warning () { + msg "build: make, full_non_deprecated config, MBEDTLS_DEPRECATED_WARNING" # ~ 30s + scripts/config.py full_non_deprecated + scripts/config.py unset MBEDTLS_DEPRECATED_REMOVED + scripts/config.py set MBEDTLS_DEPRECATED_WARNING + make CC=gcc CFLAGS='-O -Werror -Wall -Wextra' + + msg "test: make, full_non_deprecated config, MBEDTLS_DEPRECATED_WARNING" # ~ 5s + make test +} + +component_test_deprecated () { + msg "build: make, full config + MBEDTLS_DEPRECATED_WARNING, expect warnings" # ~ 30s scripts/config.py full scripts/config.py set MBEDTLS_DEPRECATED_WARNING - # Build with -O -Wextra to catch a maximum of issues. - make CC=gcc CFLAGS='-O -Werror -Wall -Wextra' lib programs - make CC=gcc CFLAGS='-O -Werror -Wall -Wextra -Wno-unused-function' tests + # Expect warnings from '#warning' directives in check_config.h. + make CC=gcc CFLAGS='-O -Werror -Wall -Wextra -Wno-error=cpp' lib programs - msg "test: make, full config + DEPRECATED_WARNING, expect warnings" # ~ 30s - make -C tests clean - make CC=gcc CFLAGS='-O -Werror -Wall -Wextra -Wno-error=deprecated-declarations -DMBEDTLS_TEST_DEPRECATED' tests + msg "build: make tests, full config + MBEDTLS_TEST_DEPRECATED, expect warnings" # ~ 30s + # Expect warnings from '#warning' directives in check_config.h and + # from the use of deprecated functions in test suites. + make CC=gcc CFLAGS='-O -Werror -Wall -Wextra -Wno-error=deprecated-declarations -Wno-error=cpp -DMBEDTLS_TEST_DEPRECATED' tests - msg "build: make, full config + DEPRECATED_REMOVED, clang -O" # ~ 30s - # No cleanup, just tweak the configuration and rebuild - make clean - scripts/config.py unset MBEDTLS_DEPRECATED_WARNING - scripts/config.py set MBEDTLS_DEPRECATED_REMOVED - # Build with -O -Wextra to catch a maximum of issues. - make CC=clang CFLAGS='-O -Werror -Wall -Wextra' lib programs - make CC=clang CFLAGS='-O -Werror -Wall -Wextra -Wno-unused-function' tests + msg "test: full config + MBEDTLS_TEST_DEPRECATED" # ~ 30s + make test } # Check that the specified libraries exist and are empty. From c9d0433ecef36120bee71deefcf98c0b2f9513c6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 16 Apr 2020 20:50:17 +0200 Subject: [PATCH 17/22] Improve the description of EXCLUDED_FROM_FULL Every boolean (defined/undefined) symbol is a "variant toggle" in some sense, even enabling a module with MBEDTLS_xxx_C. What matters is whether the symbol influences some other part of the system in such a way that we need to run tests separately with and without it being defined. Signed-off-by: Gilles Peskine --- scripts/config.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/scripts/config.py b/scripts/config.py index aeba64f26..bc9a5147c 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -163,38 +163,39 @@ def realfull_adapter(_name, active, section): # together. This includes deprecated or insecure options. It excludes: # * Options that require additional build dependencies or unusual hardware. # * Options that make testing less effective. -# * Options that are incompatible with other options. +# * Options that are incompatible with other options, or more generally that +# interact with other parts of the code in such a way that a bulk enabling +# is not a good way to test them. # * Options that remove features. -# * Options that are variants, so that we need to test both with and without. EXCLUDE_FROM_FULL = frozenset([ #pylint: disable=line-too-long - 'MBEDTLS_CTR_DRBG_USE_128_BIT_KEY', # variant toggle + 'MBEDTLS_CTR_DRBG_USE_128_BIT_KEY', # interacts with ENTROPY_FORCE_SHA256 'MBEDTLS_DEPRECATED_REMOVED', # conflicts with deprecated options 'MBEDTLS_DEPRECATED_WARNING', # conflicts with deprecated options - 'MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED', # variant toggle + 'MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED', # influences the use of ECDH in TLS 'MBEDTLS_ECP_RESTARTABLE', # incompatible with USE_PSA_CRYPTO - 'MBEDTLS_ENTROPY_FORCE_SHA256', # variant toggle + 'MBEDTLS_ENTROPY_FORCE_SHA256', # interacts with CTR_DRBG_128_BIT_KEY 'MBEDTLS_HAVE_SSE2', # hardware dependency 'MBEDTLS_MEMORY_BACKTRACE', # depends on MEMORY_BUFFER_ALLOC_C 'MBEDTLS_MEMORY_BUFFER_ALLOC_C', # makes sanitizers (e.g. ASan) less effective 'MBEDTLS_MEMORY_DEBUG', # depends on MEMORY_BUFFER_ALLOC_C - 'MBEDTLS_NO_64BIT_MULTIPLICATION', # variant toggle + 'MBEDTLS_NO_64BIT_MULTIPLICATION', # influences anything that uses bignum 'MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES', # removes a feature 'MBEDTLS_NO_PLATFORM_ENTROPY', # removes a feature - 'MBEDTLS_NO_UDBL_DIVISION', # variant toggle + 'MBEDTLS_NO_UDBL_DIVISION', # influences anything that uses bignum 'MBEDTLS_PKCS11_C', # build dependecy (libpkcs11-helper) 'MBEDTLS_PLATFORM_NO_STD_FUNCTIONS', # removes a feature 'MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT', # similar to non-platform xxx_ALT, requires platform_alt.h - 'MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER', # variant toggle + 'MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER', # platform dependency (PSA SPM) (at this time) 'MBEDTLS_PSA_CRYPTO_SPM', # platform dependency (PSA SPM) 'MBEDTLS_PSA_INJECT_ENTROPY', # build dependency (hook functions) 'MBEDTLS_REMOVE_3DES_CIPHERSUITES', # removes a feature 'MBEDTLS_REMOVE_ARC4_CIPHERSUITES', # removes a feature - 'MBEDTLS_RSA_NO_CRT', # variant toggle + 'MBEDTLS_RSA_NO_CRT', # influences the use of RSA in X.509 and TLS 'MBEDTLS_SHA512_NO_SHA384', # removes a feature 'MBEDTLS_SSL_HW_RECORD_ACCEL', # build dependency (hook functions) 'MBEDTLS_TEST_NULL_ENTROPY', # removes a feature - 'MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION', # variant toggle + 'MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION', # influences the use of X.509 in TLS 'MBEDTLS_ZLIB_SUPPORT', # build dependency (libz) ]) From 659db005176b3872b2de3f1edbbd48a0245542ab Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 16 Apr 2020 20:54:01 +0200 Subject: [PATCH 18/22] Clarify that EXCLUDE_FROM_FULL has exceptions from is_seamless_alt Signed-off-by: Gilles Peskine --- scripts/config.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/config.py b/scripts/config.py index bc9a5147c..ecf95d165 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -210,6 +210,9 @@ def is_seamless_alt(name): Exclude alternative implementations of library functions since they require an implementation of the relevant functions and an xxx_alt.h header. + + If a symbol matches this naming pattern but doesn't behave like the others, + list it in EXCLUDE_FROM_FULL. """ return name.startswith('MBEDTLS_PLATFORM_') From 98f8f95208ab634bb6af9f0c95d776cf27278fbc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Apr 2020 15:38:39 +0200 Subject: [PATCH 19/22] Minor improvements to the description of full and baremetal Signed-off-by: Gilles Peskine --- scripts/config.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/config.py b/scripts/config.py index ecf95d165..8aca1f48b 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -183,7 +183,7 @@ EXCLUDE_FROM_FULL = frozenset([ 'MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES', # removes a feature 'MBEDTLS_NO_PLATFORM_ENTROPY', # removes a feature 'MBEDTLS_NO_UDBL_DIVISION', # influences anything that uses bignum - 'MBEDTLS_PKCS11_C', # build dependecy (libpkcs11-helper) + 'MBEDTLS_PKCS11_C', # build dependency (libpkcs11-helper) 'MBEDTLS_PLATFORM_NO_STD_FUNCTIONS', # removes a feature 'MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT', # similar to non-platform xxx_ALT, requires platform_alt.h 'MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER', # platform dependency (PSA SPM) (at this time) @@ -237,16 +237,16 @@ def full_adapter(name, active, section): # need to be repeated here. EXCLUDE_FROM_BAREMETAL = frozenset([ #pylint: disable=line-too-long - 'MBEDTLS_ENTROPY_NV_SEED', # requires FS_IO or alternate NV seed hooks + 'MBEDTLS_ENTROPY_NV_SEED', # requires a filesystem and FS_IO or alternate NV seed hooks 'MBEDTLS_FS_IO', # requires a filesystem 'MBEDTLS_HAVEGE_C', # requires a clock 'MBEDTLS_HAVE_TIME', # requires a clock 'MBEDTLS_HAVE_TIME_DATE', # requires a clock 'MBEDTLS_NET_C', # requires POSIX-like networking 'MBEDTLS_PLATFORM_FPRINTF_ALT', # requires FILE* from stdio.h - 'MBEDTLS_PLATFORM_NV_SEED_ALT', # requires a filesystem - 'MBEDTLS_PLATFORM_TIME_ALT', # requires timing - 'MBEDTLS_PSA_CRYPTO_SE_C', # requires a filesystem + 'MBEDTLS_PLATFORM_NV_SEED_ALT', # requires a filesystem and ENTROPY_NV_SEED + 'MBEDTLS_PLATFORM_TIME_ALT', # requires a clock and HAVE_TIME + 'MBEDTLS_PSA_CRYPTO_SE_C', # requires a filesystem and PSA_CRYPTO_STORAGE_C 'MBEDTLS_PSA_CRYPTO_STORAGE_C', # requires a filesystem 'MBEDTLS_PSA_ITS_FILE_C', # requires a filesystem 'MBEDTLS_THREADING_C', # requires a threading interface From c34faba8fcad18bdf1e1e8480987f522f4c0d99e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Apr 2020 15:44:14 +0200 Subject: [PATCH 20/22] List MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT in is_seamless_alt Group all the logic about _ALT symbols in one place, even the lone exception. Signed-off-by: Gilles Peskine --- scripts/config.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/scripts/config.py b/scripts/config.py index 8aca1f48b..8f69f9fb0 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -185,7 +185,6 @@ EXCLUDE_FROM_FULL = frozenset([ 'MBEDTLS_NO_UDBL_DIVISION', # influences anything that uses bignum 'MBEDTLS_PKCS11_C', # build dependency (libpkcs11-helper) 'MBEDTLS_PLATFORM_NO_STD_FUNCTIONS', # removes a feature - 'MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT', # similar to non-platform xxx_ALT, requires platform_alt.h 'MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER', # platform dependency (PSA SPM) (at this time) 'MBEDTLS_PSA_CRYPTO_SPM', # platform dependency (PSA SPM) 'MBEDTLS_PSA_INJECT_ENTROPY', # build dependency (hook functions) @@ -200,9 +199,9 @@ EXCLUDE_FROM_FULL = frozenset([ ]) def is_seamless_alt(name): - """Include xxx_ALT symbols that don't have external dependencies. + """Whether the xxx_ALT symbol should be included in the full configuration. - Include alternative implementations of platform functions, which are + Include alternative implementations of platform functions, which are configurable function pointers that default to the built-in function. This way we test that the function pointers exist and build correctly without changing the behavior, and tests can verify that the function @@ -210,10 +209,10 @@ def is_seamless_alt(name): Exclude alternative implementations of library functions since they require an implementation of the relevant functions and an xxx_alt.h header. - - If a symbol matches this naming pattern but doesn't behave like the others, - list it in EXCLUDE_FROM_FULL. """ + if name == 'MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT': + # Similar to non-platform xxx_ALT, requires platform_alt.h + return False return name.startswith('MBEDTLS_PLATFORM_') def include_in_full(name): From 30de2e84ef9c11564d64c54d6594fdccbe805355 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Apr 2020 21:39:22 +0200 Subject: [PATCH 21/22] Make no_deprecated naming more consistent Use "no_deprecated" both in the name of the configuration and in the name of all.sh components, rather than a mixture of "no_deprecated" and "non_deprecated". Make all.sh component names more consistent. Signed-off-by: Gilles Peskine --- scripts/config.py | 6 +++--- tests/scripts/all.sh | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/scripts/config.py b/scripts/config.py index 8f69f9fb0..63d052ba6 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -302,10 +302,10 @@ DEPRECATED = frozenset([ 'MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO', ]) -def non_deprecated_adapter(adapter): +def no_deprecated_adapter(adapter): """Modify an adapter to disable deprecated symbols. - ``non_deprecated_adapter(adapter)(name, active, section)`` is like + ``no_deprecated_adapter(adapter)(name, active, section)`` is like ``adapter(name, active, section)``, but unsets all deprecated symbols and sets ``MBEDTLS_DEPRECATED_REMOVED``. """ @@ -480,7 +480,7 @@ if __name__ == '__main__': Exclude alternative implementations and platform support options, as well as some options that are awkward to test. """) - add_adapter('full_non_deprecated', non_deprecated_adapter(full_adapter), + add_adapter('full_no_deprecated', no_deprecated_adapter(full_adapter), """Uncomment most non-deprecated features. Like "full", but without deprecated features. """) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 83d2d1bfe..4a142c7df 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -907,22 +907,22 @@ component_test_default_no_deprecated () { } component_test_full_no_deprecated () { - msg "build: make, full_non_deprecated config" # ~ 30s - scripts/config.py full_non_deprecated + msg "build: make, full_no_deprecated config" # ~ 30s + scripts/config.py full_no_deprecated make CC=gcc CFLAGS='-O -Werror -Wall -Wextra' - msg "test: make, full_non_deprecated config" # ~ 5s + msg "test: make, full_no_deprecated config" # ~ 5s make test } -component_test_no_deprecated_warning () { - msg "build: make, full_non_deprecated config, MBEDTLS_DEPRECATED_WARNING" # ~ 30s - scripts/config.py full_non_deprecated +component_test_full_no_deprecated_warning () { + msg "build: make, full_no_deprecated config, MBEDTLS_DEPRECATED_WARNING" # ~ 30s + scripts/config.py full_no_deprecated scripts/config.py unset MBEDTLS_DEPRECATED_REMOVED scripts/config.py set MBEDTLS_DEPRECATED_WARNING make CC=gcc CFLAGS='-O -Werror -Wall -Wextra' - msg "test: make, full_non_deprecated config, MBEDTLS_DEPRECATED_WARNING" # ~ 5s + msg "test: make, full_no_deprecated config, MBEDTLS_DEPRECATED_WARNING" # ~ 5s make test } From 8386ea22b26f300aa3946a62f883abcfd8017740 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 30 Apr 2020 09:07:29 +0200 Subject: [PATCH 22/22] all.sh: explain the testing around deprecated features Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 4a142c7df..a06dc1b61 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -898,6 +898,8 @@ component_test_full_cmake_clang () { } component_test_default_no_deprecated () { + # Test that removing the deprecated features from the default + # configuration leaves something consistent. msg "build: make, default + MBEDTLS_DEPRECATED_REMOVED" # ~ 30s scripts/config.py set MBEDTLS_DEPRECATED_REMOVED make CC=gcc CFLAGS='-O -Werror -Wall -Wextra' @@ -915,7 +917,10 @@ component_test_full_no_deprecated () { make test } -component_test_full_no_deprecated_warning () { +component_test_full_no_deprecated_deprecated_warning () { + # Test that there is nothing deprecated in "full_no_deprecated". + # A deprecated feature would trigger a warning (made fatal) from + # MBEDTLS_DEPRECATED_WARNING. msg "build: make, full_no_deprecated config, MBEDTLS_DEPRECATED_WARNING" # ~ 30s scripts/config.py full_no_deprecated scripts/config.py unset MBEDTLS_DEPRECATED_REMOVED @@ -926,14 +931,18 @@ component_test_full_no_deprecated_warning () { make test } -component_test_deprecated () { +component_test_full_deprecated_warning () { + # Test that when MBEDTLS_DEPRECATED_WARNING is enabled, the build passes + # with only certain whitelisted types of warnings. msg "build: make, full config + MBEDTLS_DEPRECATED_WARNING, expect warnings" # ~ 30s scripts/config.py full scripts/config.py set MBEDTLS_DEPRECATED_WARNING # Expect warnings from '#warning' directives in check_config.h. make CC=gcc CFLAGS='-O -Werror -Wall -Wextra -Wno-error=cpp' lib programs - msg "build: make tests, full config + MBEDTLS_TEST_DEPRECATED, expect warnings" # ~ 30s + msg "build: make tests, full config + MBEDTLS_DEPRECATED_WARNING, expect warnings" # ~ 30s + # Set MBEDTLS_TEST_DEPRECATED to enable tests for deprecated features. + # By default those are disabled when MBEDTLS_DEPRECATED_WARNING is set. # Expect warnings from '#warning' directives in check_config.h and # from the use of deprecated functions in test suites. make CC=gcc CFLAGS='-O -Werror -Wall -Wextra -Wno-error=deprecated-declarations -Wno-error=cpp -DMBEDTLS_TEST_DEPRECATED' tests