From b1176f2583407bb80838f8760e0824eae50cc0fb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 22 Feb 2023 22:07:28 +0100 Subject: [PATCH 1/4] Allow alternative names for overridden PSA headers Integrators of Mbed TLS may override the header files "psa/crypto_platform.h" and "psa/crypto_struct.h" by overwriting the files or by placing alternative versions earlier in the include file search path. These two methods are sometimes inconvenient, so allow a third method which doesn't require overwriting files or having a precise order for the include path: integrators can now specify alternative names for the headers. Signed-off-by: Gilles Peskine --- ChangeLog.d/psa-alt-headers.txt | 4 +++ include/mbedtls/mbedtls_config.h | 47 ++++++++++++++++++++++++++++++++ include/psa/crypto.h | 8 ++++++ include/psa/crypto_types.h | 4 +++ 4 files changed, 63 insertions(+) create mode 100644 ChangeLog.d/psa-alt-headers.txt diff --git a/ChangeLog.d/psa-alt-headers.txt b/ChangeLog.d/psa-alt-headers.txt new file mode 100644 index 000000000..95556290a --- /dev/null +++ b/ChangeLog.d/psa-alt-headers.txt @@ -0,0 +1,4 @@ +Features + * The configuration macros MBEDTLS_PSA_CRYPTO_PLATFORM_FILE and + MBEDTLS_PSA_CRYPTO_STRUCT_FILE specify alternative locations for + the headers "psa/crypto_platform.h" and "psa/crypto_struct.h". diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 9ae51c964..f596417ff 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -3537,6 +3537,53 @@ */ //#define MBEDTLS_PSA_CRYPTO_USER_CONFIG_FILE "/dev/null" +/** + * \def MBEDTLS_PSA_CRYPTO_PLATFORM_FILE + * + * If defined, this is a header which will be included instead of + * `"psa/crypto_platform.h"`. This file should declare the same identifiers + * as the one in Mbed TLS, but with definitions adapted to the platform on + * which the library code will run. + * + * \note The required content of this header can vary from one version of + * Mbed TLS to the next. Integrators who provide an alternative file + * should review the changes in the original file whenever they + * upgrade Mbed TLS. + * + * This macro is expanded after an \#include directive. This is a popular but + * non-standard feature of the C language, so this feature is only available + * with compilers that perform macro expansion on an \#include line. + * + * The value of this symbol is typically a path in double quotes, either + * absolute or relative to a directory on the include search path. + */ +//#define MBEDTLS_PSA_CRYPTO_PLATFORM_FILE "psa/crypto_platform_alt.h" + +/** + * \def MBEDTLS_PSA_CRYPTO_STRUCT_FILE + * + * If defined, this is a header which will be included instead of + * `"psa/crypto_struct.h"`. This file should declare the same identifiers + * as the one in Mbed TLS, but with definitions adapted to the environment + * in which the library code will run. The typical use for this feature + * is to provide alternative type definitions on the client side in + * client-server integrations of PSA crypto, where operation structures + * contain handles instead of cryptographic data. + * + * \note The required content of this header can vary from one version of + * Mbed TLS to the next. Integrators who provide an alternative file + * should review the changes in the original file whenever they + * upgrade Mbed TLS. + * + * This macro is expanded after an \#include directive. This is a popular but + * non-standard feature of the C language, so this feature is only available + * with compilers that perform macro expansion on an \#include line. + * + * The value of this symbol is typically a path in double quotes, either + * absolute or relative to a directory on the include search path. + */ +//#define MBEDTLS_PSA_CRYPTO_STRUCT_FILE "psa/crypto_struct_alt.h" + /** \} name SECTION: General configuration options */ /** diff --git a/include/psa/crypto.h b/include/psa/crypto.h index 80bf5c969..bd544224d 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -22,7 +22,11 @@ #ifndef PSA_CRYPTO_H #define PSA_CRYPTO_H +#if defined(MBEDTLS_PSA_CRYPTO_PLATFORM_FILE) +#include MBEDTLS_PSA_CRYPTO_PLATFORM_FILE +#else #include "crypto_platform.h" +#endif #include @@ -4677,7 +4681,11 @@ psa_status_t psa_verify_hash_abort( /* The file "crypto_struct.h" contains definitions for * implementation-specific structs that are declared above. */ +#if defined(MBEDTLS_PSA_CRYPTO_STRUCT_FILE) +#include MBEDTLS_PSA_CRYPTO_STRUCT_FILE +#else #include "crypto_struct.h" +#endif /* The file "crypto_extra.h" contains vendor-specific definitions. This * can include vendor-defined algorithms, extra functions, etc. */ diff --git a/include/psa/crypto_types.h b/include/psa/crypto_types.h index 95bf32fd9..dd4d4fca3 100644 --- a/include/psa/crypto_types.h +++ b/include/psa/crypto_types.h @@ -34,7 +34,11 @@ #define PSA_CRYPTO_TYPES_H #include "mbedtls/private_access.h" +#if defined(MBEDTLS_PSA_CRYPTO_PLATFORM_FILE) +#include MBEDTLS_PSA_CRYPTO_PLATFORM_FILE +#else #include "crypto_platform.h" +#endif /* If MBEDTLS_PSA_CRYPTO_C is defined, make sure MBEDTLS_PSA_CRYPTO_CLIENT * is defined as well to include all PSA code. From df6e84a4471e257d6db13986db592c959886ea01 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 22 Feb 2023 22:09:51 +0100 Subject: [PATCH 2/4] Test the PSA alternative header configuration macros Test that MBEDTLS_PSA_CRYPTO_PLATFORM_FILE and MBEDTLS_PSA_CRYPTO_STRUCT_FILE can be set to files in a directory that comes after the standard directory in the include file search path. Signed-off-by: Gilles Peskine --- programs/.gitignore | 1 + programs/Makefile | 5 ++++ programs/test/CMakeLists.txt | 1 + programs/test/query_included_headers.c | 41 ++++++++++++++++++++++++++ tests/.gitignore | 2 ++ tests/Makefile | 5 ++++ tests/include/alt-extra/psa/crypto.h | 7 +++++ tests/scripts/all.sh | 21 +++++++++++++ 8 files changed, 83 insertions(+) create mode 100644 programs/test/query_included_headers.c create mode 100644 tests/include/alt-extra/psa/crypto.h diff --git a/programs/.gitignore b/programs/.gitignore index 44e904a95..398152dcb 100644 --- a/programs/.gitignore +++ b/programs/.gitignore @@ -64,6 +64,7 @@ test/cpp_dummy_build.cpp test/dlopen test/ecp-bench test/query_compile_time_config +test/query_included_headers test/selftest test/ssl_cert_test test/udp_proxy diff --git a/programs/Makefile b/programs/Makefile index fdfece72a..3509fc374 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -123,6 +123,7 @@ APPS = \ ssl/ssl_server2 \ test/benchmark \ test/query_compile_time_config \ + test/query_included_headers \ test/selftest \ test/udp_proxy \ test/zeroize \ @@ -403,6 +404,10 @@ test/query_config.o: test/query_config.c test/query_config.h $(DEP) echo " CC test/query_config.c" $(CC) $(LOCAL_CFLAGS) $(CFLAGS) -c test/query_config.c -o $@ +test/query_included_headers$(EXEXT): test/query_included_headers.c $(DEP) + echo " CC test/query_included_headers.c" + $(CC) $(LOCAL_CFLAGS) $(CFLAGS) test/query_included_headers.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@ + test/selftest$(EXEXT): test/selftest.c $(DEP) echo " CC test/selftest.c" $(CC) $(LOCAL_CFLAGS) $(CFLAGS) test/selftest.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@ diff --git a/programs/test/CMakeLists.txt b/programs/test/CMakeLists.txt index c3e7d2e98..735684ebf 100644 --- a/programs/test/CMakeLists.txt +++ b/programs/test/CMakeLists.txt @@ -3,6 +3,7 @@ set(libs ) set(executables_libs + query_included_headers selftest udp_proxy ) diff --git a/programs/test/query_included_headers.c b/programs/test/query_included_headers.c new file mode 100644 index 000000000..383a2ffc8 --- /dev/null +++ b/programs/test/query_included_headers.c @@ -0,0 +1,41 @@ +/* Ad hoc report on included headers. */ +/* + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +int main(void) +{ + + /* Which PSA platform header? */ +#if defined(PSA_CRYPTO_PLATFORM_H) + mbedtls_printf("PSA_CRYPTO_PLATFORM_H\n"); +#endif +#if defined(PSA_CRYPTO_PLATFORM_ALT_H) + mbedtls_printf("PSA_CRYPTO_PLATFORM_ALT_H\n"); +#endif + + /* Which PSA struct header? */ +#if defined(PSA_CRYPTO_STRUCT_H) + mbedtls_printf("PSA_CRYPTO_STRUCT_H\n"); +#endif +#if defined(PSA_CRYPTO_STRUCT_ALT_H) + mbedtls_printf("PSA_CRYPTO_STRUCT_ALT_H\n"); +#endif + +} diff --git a/tests/.gitignore b/tests/.gitignore index 15fce6888..b85d66aa4 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -13,6 +13,8 @@ data_files/hmac_drbg_seed data_files/ctr_drbg_seed data_files/entropy_seed +include/alt-extra/psa/crypto_platform_alt.h +include/alt-extra/psa/crypto_struct_alt.h include/test/instrument_record_status.h src/*.o diff --git a/tests/Makefile b/tests/Makefile index c9283c984..26947f4b9 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -219,6 +219,7 @@ ifndef WINDOWS rm -rf $(BINARIES) *.c *.datax rm -f src/*.o src/drivers/*.o src/libmbed* rm -f include/test/instrument_record_status.h + rm -f include/alt-extra/*/*_alt.h rm -rf libtestdriver1 rm -f ../library/libtestdriver1.a else @@ -244,6 +245,10 @@ check: $(BINARIES) test: check +# Generate variants of some headers for testing +include/alt-extra/%_alt.h: ../include/%.h + perl -p -e 's/^(# *(define|ifndef) +\w+_)H\b/$${1}ALT_H/' $< >$@ + # Generate test library # Perl code that is executed to transform each original line from a library diff --git a/tests/include/alt-extra/psa/crypto.h b/tests/include/alt-extra/psa/crypto.h new file mode 100644 index 000000000..005f3aeea --- /dev/null +++ b/tests/include/alt-extra/psa/crypto.h @@ -0,0 +1,7 @@ +/* The goal of the include/alt-extra directory is to test what happens + * if certain files come _after_ the normal include directory. + * Make sure that if the alt-extra directory comes before the normal + * directory (so we wouldn't be achieving our test objective), the build + * will fail. + */ +#error "The normal include directory must come first in the include path" diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 7d91fa27d..a851d0e7b 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -3258,6 +3258,27 @@ component_build_psa_config_file () { rm -f psa_test_config.h psa_user_config.h } +component_build_psa_alt_headers () { + msg "build: make with PSA alt headers" # ~20s + + # Generate alternative versions of the substitutable headers with the + # same content except different include guards. + make -C tests include/alt-extra/psa/crypto_platform_alt.h include/alt-extra/psa/crypto_struct_alt.h + + # Build the library and some programs. + # Don't build the fuzzers to avoid having to go through hoops to set + # a correct include path for programs/fuzz/Makefile. + make CFLAGS="-I ../tests/include/alt-extra -DMBEDTLS_PSA_CRYPTO_PLATFORM_FILE='\"psa/crypto_platform_alt.h\"' -DMBEDTLS_PSA_CRYPTO_STRUCT_FILE='\"psa/crypto_struct_alt.h\"'" lib + make -C programs -o fuzz CFLAGS="-I ../tests/include/alt-extra -DMBEDTLS_PSA_CRYPTO_PLATFORM_FILE='\"psa/crypto_platform_alt.h\"' -DMBEDTLS_PSA_CRYPTO_STRUCT_FILE='\"psa/crypto_struct_alt.h\"'" + + # Check that we're getting the alternative include guards and not the + # original include guards. + programs/test/query_included_headers | grep -x PSA_CRYPTO_PLATFORM_ALT_H + programs/test/query_included_headers | grep -x PSA_CRYPTO_STRUCT_ALT_H + programs/test/query_included_headers | not grep -x PSA_CRYPTO_PLATFORM_H + programs/test/query_included_headers | not grep -x PSA_CRYPTO_STRUCT_H +} + component_test_m32_o0 () { # Build without optimization, so as to use portable C code (in a 32-bit # build) and not the i386-specific inline assembly. From 361b5f992ff215af199b7922088fad2ab8907cd9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 22 Feb 2023 22:15:18 +0100 Subject: [PATCH 3/4] Make sure the configuration is always included Before, if psa/crypto_platform.h was overridden and the override didn't include "mbedtls/build_info.h", it was possible to end up with parts of the headers not taking the library configuration into account, if no mbedtls header was included before "psa/crypto.h". Make sure that the mbedtls configuration is visible from the start, no matter what is or is not in the platform header. Signed-off-by: Gilles Peskine --- include/psa/crypto_types.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/psa/crypto_types.h b/include/psa/crypto_types.h index dd4d4fca3..5e2e334a1 100644 --- a/include/psa/crypto_types.h +++ b/include/psa/crypto_types.h @@ -32,6 +32,10 @@ #ifndef PSA_CRYPTO_TYPES_H #define PSA_CRYPTO_TYPES_H + +/* Make sure the Mbed TLS configuration is visible. */ +#include "mbedtls/build_info.h" +/* Define the MBEDTLS_PRIVATE macro. */ #include "mbedtls/private_access.h" #if defined(MBEDTLS_PSA_CRYPTO_PLATFORM_FILE) From 95c915201e76a9bac790722bedf12fe0c3c07948 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 22 Feb 2023 22:20:24 +0100 Subject: [PATCH 4/4] Move the implication of MBEDTLS_PSA_CRYPTO_CLIENT where it belongs If MBEDTLS_PSA_CRYPTO_C is enabled, we always enable MBEDTLS_PSA_CRYPTO_CLIENT, since the client-side functions are part of the full PSA crypto feature set. Historically, we didn't have a good place for configuration modification, so we did this early in the crypto.h include tree. Since Mbed TLS 3.0, we have mbedtls/build_info.h for that. Addresses https://github.com/Mbed-TLS/mbedtls/issues/7144 . Signed-off-by: Gilles Peskine --- include/mbedtls/build_info.h | 7 +++++++ include/psa/crypto_types.h | 7 ------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/mbedtls/build_info.h b/include/mbedtls/build_info.h index bbfd5d48d..2edf01519 100644 --- a/include/mbedtls/build_info.h +++ b/include/mbedtls/build_info.h @@ -80,6 +80,13 @@ #include MBEDTLS_USER_CONFIG_FILE #endif +/* If MBEDTLS_PSA_CRYPTO_C is defined, make sure MBEDTLS_PSA_CRYPTO_CLIENT + * is defined as well to include all PSA code. + */ +#if defined(MBEDTLS_PSA_CRYPTO_C) +#define MBEDTLS_PSA_CRYPTO_CLIENT +#endif /* MBEDTLS_PSA_CRYPTO_C */ + /* The PK wrappers need pk_write functions to format RSA key objects * when they are dispatching to the PSA API. This happens under USE_PSA_CRYPTO, * and also even without USE_PSA_CRYPTO for mbedtls_pk_sign_ext(). diff --git a/include/psa/crypto_types.h b/include/psa/crypto_types.h index 5e2e334a1..a5154fcd6 100644 --- a/include/psa/crypto_types.h +++ b/include/psa/crypto_types.h @@ -44,13 +44,6 @@ #include "crypto_platform.h" #endif -/* If MBEDTLS_PSA_CRYPTO_C is defined, make sure MBEDTLS_PSA_CRYPTO_CLIENT - * is defined as well to include all PSA code. - */ -#if defined(MBEDTLS_PSA_CRYPTO_C) -#define MBEDTLS_PSA_CRYPTO_CLIENT -#endif /* MBEDTLS_PSA_CRYPTO_C */ - #include /** \defgroup error Error codes