Mechanism to error out on removed configuration options

include/mbedtls/build_info.h

@@ -68,6 +68,11 @@
 #include MBEDTLS_USER_CONFIG_FILE
 #endif
 
+/* For the sake of consistency checks in mbedtls_config.c */
+#if defined(MBEDTLS_INCLUDE_AFTER_RAW_CONFIG)
+#include MBEDTLS_INCLUDE_AFTER_RAW_CONFIG
+#endif
+
 /* Indicate that all configuration files have been read.
  * It is now time to adjust the configuration (follow through on dependencies,
  * make PSA and legacy crypto consistent, etc.).

library/CMakeLists.txt

@@ -118,6 +118,13 @@ if(GEN_FILES)
             ${CMAKE_CURRENT_BINARY_DIR}/ssl_debug_helpers_generated.c
             ${CMAKE_CURRENT_BINARY_DIR}/version_features.c
     )
+
+    # List generated headers as sources explicitly. Normally CMake finds
+    # headers by tracing include directives, but if that happens before the
+    # generated headers are generated, this process doesn't find them.
+    list(APPEND src_x509
+        ${MBEDTLS_GENERATED_CONFIG_CHECKS_HEADERS}
+    )
 endif()
 
 if(CMAKE_COMPILER_IS_GNUCC)
@@ -237,7 +244,9 @@ foreach(target IN LISTS target_libraries)
                $<INSTALL_INTERFACE:include/>
         PRIVATE ${MBEDTLS_DIR}/library/
                 ${MBEDTLS_DIR}/tf-psa-crypto/core
-                ${MBEDTLS_DIR}/tf-psa-crypto/drivers/builtin/src)
+                ${MBEDTLS_DIR}/tf-psa-crypto/drivers/builtin/src
+                # needed for generated headers
+                ${CMAKE_CURRENT_BINARY_DIR})
     set_config_files_compile_definitions(${target})
     install(
         TARGETS ${target}

library/Makefile

@@ -346,6 +346,8 @@ $(GENERATED_CONFIG_CHECK_FILES):
 	echo "  Gen   $(GENERATED_CONFIG_CHECK_FILES)"
 	$(PYTHON) ../scripts/generate_config_checks.py
 
+mbedtls_config.o: $(GENERATED_CONFIG_CHECK_FILES)
+
 TF_PSA_CRYPTO_GENERATED_CONFIG_CHECK_FILES = $(shell $(PYTHON) \
 	$(TF_PSA_CRYPTO_CORE_PATH)/../scripts/generate_config_checks.py \
 	--list $(TF_PSA_CRYPTO_CORE_PATH))

library/mbedtls_config.c

@@ -6,8 +6,29 @@
  *  SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
  */
 
+/* Apply the TF-PSA-Crypto configuration first. We need to do this
+ * before <mbedtls/build_info.h>, because "mbedtls_config_check_before.h"
+ * needs to run after the crypto config (including derived macros) is
+ * finalized, but before the user's mbedtls config is applied. This way
+ * it is possible to differentiate macros set by the user's mbedtls config
+ * from macros set or derived by the crypto config. */
+#include <tf-psa-crypto/build_info.h>
+
+/* Consistency checks on the user's configuration.
+ * Check that it doesn't define macros that we assume are under full
+ * control of the library, or options from past major versions that
+ * no longer have any effect.
+ * These headers are automatically generated. See
+ * framework/scripts/mbedtls_framework/config_checks_generator.py
+ */
+#include "mbedtls_config_check_before.h"
+#define MBEDTLS_INCLUDE_AFTER_RAW_CONFIG "mbedtls_config_check_user.h"
+
 #include <mbedtls/build_info.h>
 
 /* Consistency checks in the configuration: check for incompatible options,
  * missing options when at least one of a set needs to be enabled, etc. */
+/* Manually written checks */
 #include "mbedtls_check_config.h"
+/* Automatically generated checks */
+#include "mbedtls_config_check_final.h"

scripts/generate_config_checks.py

@@ -3,18 +3,50 @@
 """Generate C preprocessor code to check for bad configurations.
 """
 
+from typing import Iterator
+
 import framework_scripts_path # pylint: disable=unused-import
 from mbedtls_framework.config_checks_generator import * \
     #pylint: disable=wildcard-import,unused-wildcard-import
+from mbedtls_framework import config_history
+
+class CryptoInternal(SubprojectInternal):
+    SUBPROJECT = 'TF-PSA-Crypto'
+
+class CryptoOption(SubprojectOption):
+    SUBPROJECT = 'psa/crypto_config.h'
+
+ALWAYS_ENABLED_SINCE_4_0 = frozenset([
+    'MBEDTLS_PSA_CRYPTO_CONFIG',
+    'MBEDTLS_USE_PSA_CRYPTO',
+])
+
+def checkers_for_removed_options() -> Iterator[Checker]:
+    """Discover removed options. Yield corresponding checkers."""
+    history = config_history.ConfigHistory()
+    old_public = history.options('mbedtls', '3.6')
+    new_public = history.options('mbedtls', '4.0')
+    crypto_public = history.options('tfpsacrypto', '1.0')
+    crypto_internal = history.internal('tfpsacrypto', '1.0')
+    for option in sorted(old_public - new_public):
+        if option in ALWAYS_ENABLED_SINCE_4_0:
+            continue
+        if option in crypto_public:
+            yield CryptoOption(option)
+        elif option in crypto_internal:
+            yield CryptoInternal(option)
+        else:
+            yield Removed(option, 'Mbed TLS 4.0')
+
+def all_checkers() -> Iterator[Checker]:
+    """Yield all checkers."""
+    yield from checkers_for_removed_options()
 
 MBEDTLS_CHECKS = BranchData(
     header_directory='library',
     header_prefix='mbedtls_',
     project_cpp_prefix='MBEDTLS',
-    checkers=[
-        Removed('MBEDTLS_KEY_EXCHANGE_RSA_ENABLED', 'Mbed TLS 4.0'),
-        Removed('MBEDTLS_PADLOCK_C', 'Mbed TLS 4.0'),
-    ],
+    checkers=list(all_checkers()),
 )
 
 if __name__ == '__main__':

tests/scripts/test_config_checks.py

@@ -22,12 +22,27 @@ class MbedtlsTestConfigChecks(unittest_config_checks.TestConfigChecks):
         'tf-psa-crypto/drivers/builtin/include',
     ]
 
+    ## Method naming convention:
+    ## * test_crypto_xxx when testing a tweak of crypto_config.h
+    ## * test_mbedtls_xxx when testing a tweak of mbedtls_config.h
+
+    def test_crypto_config_read(self) -> None:
+        """Check that crypto_config.h is read in mbedtls."""
+        self.bad_case('#error witness',
+                      None,
+                      error='witness')
+
+    def test_mbedtls_config_read(self) -> None:
+        """Check that mbedtls_config.h is read in mbedtls."""
+        self.bad_case(''
+                      '#error witness',
+                      error='witness')
+
     @unittest.skip("At this time, mbedtls does not go through crypto's check_config.h.")
-    def test_crypto_no_fs_io(self) -> None:
+    def test_crypto_undef_MBEDTLS_FS_IO(self) -> None:
         """A sample error expected from crypto's check_config.h."""
         self.bad_case('#undef MBEDTLS_FS_IO',
-                      None,
-                      error=('MBEDTLS_PSA_ITS_FILE_C'))
+                      error='MBEDTLS_PSA_ITS_FILE_C')
 
     def test_mbedtls_no_session_tickets_for_early_data(self) -> None:
         """An error expected from mbedtls_check_config.h based on the TLS configuration."""
@@ -36,9 +51,9 @@ def test_mbedtls_no_session_tickets_for_early_data(self) -> None:
                       #define MBEDTLS_SSL_EARLY_DATA
                       #undef MBEDTLS_SSL_SESSION_TICKETS
                       ''',
-                      error=('MBEDTLS_SSL_EARLY_DATA'))
+                      error='MBEDTLS_SSL_EARLY_DATA')
 
-    def test_mbedtls_no_ecdsa(self) -> None:
+    def test_crypto_mbedtls_no_ecdsa(self) -> None:
         """An error expected from mbedtls_check_config.h based on crypto+TLS configuration."""
         self.bad_case('''
                       #undef PSA_WANT_ALG_ECDSA
@@ -52,7 +67,75 @@ def test_mbedtls_no_ecdsa(self) -> None:
                       #error PSA_WANT_ALG_DETERMINSTIC_ECDSA unexpected
                       #endif
                       ''',
-                      error=('MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED'))
+                      error='MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED')
+
+    def test_crypto_define_MBEDTLS_KEY_EXCHANGE_RSA_ENABLED(self) -> None:
+        """Error when setting a removed option via crypto_config.h."""
+        self.bad_case('#define MBEDTLS_KEY_EXCHANGE_RSA_ENABLED',
+                      error='MBEDTLS_KEY_EXCHANGE_RSA_ENABLED was removed')
+
+    def test_mbedtls_define_MBEDTLS_KEY_EXCHANGE_RSA_ENABLED(self) -> None:
+        """Error when setting a removed option via mbedtls_config.h."""
+        self.bad_case(None,
+                      '#define MBEDTLS_KEY_EXCHANGE_RSA_ENABLED',
+                      error='MBEDTLS_KEY_EXCHANGE_RSA_ENABLED was removed')
+
+    def test_crypto_exempt_define_MBEDTLS_KEY_EXCHANGE_RSA_ENABLED(self) -> None:
+        """Bypassed error when setting a removed option via crypto_config.h."""
+        self.good_case('#define MBEDTLS_KEY_EXCHANGE_RSA_ENABLED',
+                       extra_options=['-DMBEDTLS_CONFIG_CHECK_BYPASS'])
+
+    def test_mbedtls_exempt_define_MBEDTLS_KEY_EXCHANGE_RSA_ENABLED(self) -> None:
+        """Bypassed error when setting a removed option via mbedtls_config.h."""
+        self.good_case(None,
+                       '#define MBEDTLS_KEY_EXCHANGE_RSA_ENABLED',
+                       extra_options=['-DMBEDTLS_CONFIG_CHECK_BYPASS'])
+
+    def test_mbedtls_define_MBEDTLS_MD5_C_redundant(self) -> None:
+        """Error when redundantly setting a subproject internal option."""
+        self.bad_case('#define PSA_WANT_ALG_MD5 1',
+                      '#define MBEDTLS_MD5_C',
+                      error=r'MBEDTLS_MD5_C is an internal macro')
+
+    def test_mbedtls_define_MBEDTLS_MD5_C_added(self) -> None:
+        """Error when setting a subproject internal option that was disabled."""
+        self.bad_case('''
+                      #undef PSA_WANT_ALG_MD5
+                      #undef MBEDTLS_MD5_C
+                      ''',
+                      '#define MBEDTLS_MD5_C',
+                      error=r'MBEDTLS_MD5_C is an internal macro')
+
+    def test_mbedtls_define_MBEDTLS_BASE64_C_redundant(self) -> None:
+        """Ok to redundantly set a subproject option."""
+        self.good_case(None,
+                       '#define MBEDTLS_BASE64_C')
+
+    def test_mbedtls_define_MBEDTLS_BASE64_C_added(self) -> None:
+        """Error when setting a subproject option that was disabled."""
+        self.bad_case('''
+                      #undef MBEDTLS_BASE64_C
+                      #undef MBEDTLS_PEM_PARSE_C
+                      #undef MBEDTLS_PEM_WRITE_C
+                      ''',
+                      '#define MBEDTLS_BASE64_C',
+                      error=r'MBEDTLS_BASE64_C .*psa/crypto_config\.h')
+
+    @unittest.skip("Checks for #undef are not implemented yet.")
+    def test_mbedtls_define_MBEDTLS_BASE64_C_unset(self) -> None:
+        """Error when unsetting a subproject option that was enabled."""
+        self.bad_case(None,
+                      '#undef MBEDTLS_BASE64_C',
+                      error=r'MBEDTLS_BASE64_C .*psa/crypto_config\.h')
+
+    def test_crypto_define_MBEDTLS_USE_PSA_CRYPTO(self) -> None:
+        """It's ok to set MBEDTLS_USE_PSA_CRYPTO (now effectively always on)."""
+        self.good_case('#define MBEDTLS_USE_PSA_CRYPTO')
+
+    def test_mbedtls_define_MBEDTLS_USE_PSA_CRYPTO(self) -> None:
+        """It's ok to set MBEDTLS_USE_PSA_CRYPTO (now effectively always on)."""
+        self.good_case(None,
+                       '#define MBEDTLS_USE_PSA_CRYPTO')
 
 
 if __name__ == '__main__':