Merge pull request #9628 from gilles-peskine-arm/tls13-middlebox-compat-disabled-development

Fix interoperability when MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE is disabled

CMakeLists.txt

@@ -403,6 +403,9 @@ if(ENABLE_TESTING OR ENABLE_PROGRAMS)
 endif()
 
 if(ENABLE_PROGRAMS)
+    set(ssl_opt_target "${MBEDTLS_TARGET_PREFIX}ssl-opt")
+    add_custom_target(${ssl_opt_target})
+
     add_subdirectory(programs)
 endif()
 

ChangeLog.d/tls13-middlebox-compat-disabled.txt

@@ -0,0 +1,4 @@
+Bugfix
+   * When MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE is disabled, work with
+     peers that have middlebox compatibility enabled, as long as no
+     problematic middlebox is in the way. Fixes #9551.

Makefile

@@ -30,6 +30,7 @@ programs: lib mbedtls_test
 
 ssl-opt: lib mbedtls_test
 	$(MAKE) -C programs ssl-opt
+	$(MAKE) -C tests ssl-opt
 
 lib:
 	$(MAKE) -C library

library/ssl_msg.c

@@ -5066,15 +5066,9 @@ int mbedtls_ssl_handle_message_type(mbedtls_ssl_context *ssl)
 
 #if defined(MBEDTLS_SSL_PROTO_TLS1_3)
         if (ssl->tls_version == MBEDTLS_SSL_VERSION_TLS1_3) {
-#if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE)
-            MBEDTLS_SSL_DEBUG_MSG(1,
+            MBEDTLS_SSL_DEBUG_MSG(2,
                                   ("Ignore ChangeCipherSpec in TLS 1.3 compatibility mode"));
             return MBEDTLS_ERR_SSL_CONTINUE_PROCESSING;
-#else
-            MBEDTLS_SSL_DEBUG_MSG(1,
-                                  ("ChangeCipherSpec invalid in TLS 1.3 without compatibility mode"));
-            return MBEDTLS_ERR_SSL_INVALID_RECORD;
-#endif /* MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE */
         }
 #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */
     }

programs/CMakeLists.txt

@@ -1,9 +1,6 @@
 set(programs_target "${MBEDTLS_TARGET_PREFIX}programs")
 add_custom_target(${programs_target})
 
-set(ssl_opt_target "${MBEDTLS_TARGET_PREFIX}ssl-opt")
-add_custom_target(${ssl_opt_target})
-
 add_subdirectory(aes)
 add_subdirectory(cipher)
 if (NOT WIN32)

scripts/make_generated_files.bat

@@ -29,3 +29,4 @@ python framework\scripts\generate_ecp_tests.py --directory tf-psa-crypto\tests\s
 python framework\scripts\generate_psa_tests.py --directory tf-psa-crypto\tests\suites || exit /b 1
 python framework\scripts\generate_test_keys.py --output tests\src\test_keys.h || exit /b 1
 python framework\scripts\generate_test_cert_macros.py --output tests\src\test_certs.h || exit /b 1
+python tests\scripts\generate_tls13_compat_tests.py || exit /b 1

tests/.gitignore

@@ -18,6 +18,7 @@
 
 ###START_GENERATED_FILES###
 # Generated source files
+/opt-testcases/tls13-compat.sh
 /suites/*.generated.data
 /suites/test_suite_config.mbedtls_boolean.data
 /src/test_keys.h

tests/CMakeLists.txt

@@ -64,6 +64,22 @@ if(GEN_FILES)
             # change too often in ways that don't affect the result
             # ((un)commenting some options).
     )
+    add_custom_command(
+        OUTPUT
+            ${CMAKE_CURRENT_SOURCE_DIR}/opt-testcases/tls13-compat.sh
+        WORKING_DIRECTORY
+            ${CMAKE_CURRENT_SOURCE_DIR}/..
+        COMMAND
+            "${MBEDTLS_PYTHON_EXECUTABLE}"
+            "${CMAKE_CURRENT_SOURCE_DIR}/scripts/generate_tls13_compat_tests.py"
+        DEPENDS
+            ${CMAKE_CURRENT_SOURCE_DIR}/scripts/generate_tls13_compat_tests.py
+    )
+    add_custom_target(tls13-compat.sh
+        DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/opt-testcases/tls13-compat.sh)
+    set_target_properties(tls13-compat.sh PROPERTIES EXCLUDE_FROM_ALL NO)
+    add_dependencies(${ssl_opt_target} tls13-compat.sh)
+
 else()
     foreach(file ${all_generated_data_files})
         link_to_source(${file})

tests/Makefile

@@ -59,6 +59,15 @@ GENERATED_CRYPTO_DATA_FILES += $(GENERATED_PSA_DATA_FILES)
 GENERATED_FILES = $(GENERATED_DATA_FILES) $(GENERATED_CRYPTO_DATA_FILES)
 GENERATED_FILES += src/test_keys.h src/test_certs.h
 
+# Generated files needed to (fully) run ssl-opt.sh
+.PHONY: ssl-opt
+
+opt-testcases/tls13-compat.sh: scripts/generate_tls13_compat_tests.py
+	echo "  Gen   $@"
+	$(PYTHON) scripts/generate_tls13_compat_tests.py -o $@
+GENERATED_FILES += opt-testcases/tls13-compat.sh
+ssl-opt: opt-testcases/tls13-compat.sh
+
 .PHONY: generated_files
 generated_files: $(GENERATED_FILES)
 
@@ -160,9 +169,11 @@ mbedtls_test: $(MBEDTLS_TEST_OBJS)
 
 src/test_certs.h: ../framework/scripts/generate_test_cert_macros.py \
 				  $($(PYTHON) ../framework/scripts/generate_test_cert_macros.py --list-dependencies)
+	echo "  Gen   $@"
 	$(PYTHON) ../framework/scripts/generate_test_cert_macros.py --output $@
 
 src/test_keys.h: ../framework/scripts/generate_test_keys.py
+	echo "  Gen   $@"
 	$(PYTHON) ../framework/scripts/generate_test_keys.py --output $@
 
 TEST_OBJS_DEPS = $(wildcard include/test/*.h include/test/*/*.h)

tests/scripts/check-generated-files.sh

@@ -174,6 +174,7 @@ if in_mbedtls_repo; then
     check scripts/generate_query_config.pl programs/test/query_config.c
     check scripts/generate_features.pl ${builtin_drivers_dir}/version_features.c
     check scripts/generate_ssl_debug_helpers.py library/ssl_debug_helpers_generated.c
+    check tests/scripts/generate_tls13_compat_tests.py tests/opt-testcases/tls13-compat.sh
     check framework/scripts/generate_test_cert_macros.py tests/src/test_certs.h
     # generate_visualc_files enumerates source files (library/*.c). It doesn't
     # care about their content, but the files must exist. So it must run after

tests/scripts/generate_tls13_compat_tests.py

@@ -66,7 +66,7 @@ class TLSProgram:
 
     # pylint: disable=too-many-arguments
     def __init__(self, ciphersuite=None, signature_algorithm=None, named_group=None,
-                 cert_sig_alg=None, compat_mode=True):
+                 cert_sig_alg=None):
         self._ciphers = []
         self._sig_algs = []
         self._named_groups = []
@@ -79,7 +79,6 @@ class TLSProgram:
             self.add_signature_algorithms(signature_algorithm)
         if cert_sig_alg:
             self.add_cert_signature_algorithms(cert_sig_alg)
-        self._compat_mode = compat_mode
 
     # add_ciphersuites should not override by sub class
     def add_ciphersuites(self, *ciphersuites):
@@ -157,8 +156,6 @@ class OpenSSLBase(TLSProgram):
             ret += ["-groups {named_groups}".format(named_groups=named_groups)]
 
         ret += ['-msg -tls1_3']
-        if not self._compat_mode:
-            ret += ['-no_middlebox']
 
         return ret
 
@@ -248,8 +245,7 @@ class GnuTLSBase(TLSProgram):
 
     def pre_checks(self):
         return ["requires_gnutls_tls1_3",
-                "requires_gnutls_next_no_ticket",
-                "requires_gnutls_next_disable_tls13_compat", ]
+                "requires_gnutls_next_no_ticket"]
 
     def cmd(self):
         ret = super().cmd()
@@ -288,9 +284,6 @@ class GnuTLSBase(TLSProgram):
         priority_string = ':+'.join(priority_string_list)
         priority_string += ':%NO_TICKETS'
 
-        if not self._compat_mode:
-            priority_string += [':%DISABLE_TLS13_COMPAT_MODE']
-
         ret += ['--priority={priority_string}'.format(
             priority_string=priority_string)]
         return ret
@@ -370,9 +363,6 @@ class MbedTLSBase(TLSProgram):
         ret = ['requires_config_enabled MBEDTLS_DEBUG_C',
                'requires_config_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED']
 
-        if self._compat_mode:
-            ret += ['requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE']
-
         if 'rsa_pss_rsae_sha256' in self._sig_algs + self._cert_sig_algs:
             ret.append(
                 'requires_config_enabled MBEDTLS_X509_RSASSA_PSS_SUPPORT')
@@ -533,21 +523,13 @@ def generate_hrr_compat_test(client=None, server=None,
                      client_object.pre_checks() +
                      [cmd])
 
-SSL_OUTPUT_HEADER = '''#!/bin/sh
-
-# {filename}
+SSL_OUTPUT_HEADER = '''\
+# TLS 1.3 interoperability test cases (equivalent of compat.sh for TLS 1.3).
 #
+# Automatically generated by {cmd}. Do not edit!
+
 # Copyright The Mbed TLS Contributors
 # SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
-#
-# Purpose
-#
-# List TLS1.3 compat test cases. They are generated by
-# `{cmd}`.
-#
-# PLEASE DO NOT EDIT THIS FILE. IF NEEDED, PLEASE MODIFY `generate_tls13_compat_tests.py`
-# AND REGENERATE THIS FILE.
-#
 '''
 DATA_FILES_PATH_VAR = '''
 DATA_FILES_PATH=../framework/data_files
@@ -559,11 +541,16 @@ def main():
     """
     parser = argparse.ArgumentParser()
 
-    parser.add_argument('-o', '--output', nargs='?',
-                        default=None, help='Output file path if `-a` was set')
+    parser.add_argument('-o', '--output',
+                        default='tests/opt-testcases/tls13-compat.sh',
+                        help='Output file path (not used with -1)')
 
-    parser.add_argument('-a', '--generate-all-tls13-compat-tests', action='store_true',
-                        default=False, help='Generate all available tls13 compat tests')
+    parser.add_argument('-1', '--single', action='store_true',
+                        help='Print a single test case')
+    # Single mode used to be the default.
+    parser.add_argument('-a', '--generate-all-tls13-compat-tests',
+                        action='store_false', dest='single',
+                        help='Generate all test cases (negates -1) (default)')
 
     parser.add_argument('--list-ciphers', action='store_true',
                         default=False, help='List supported ciphersuites')
@@ -626,11 +613,12 @@ def main():
                                                server_named_group=server_named_group,
                                                cert_sig_alg="ecdsa_secp256r1_sha256")
 
-    if args.generate_all_tls13_compat_tests:
+    if not args.single:
         if args.output:
             with open(args.output, 'w', encoding="utf-8") as f:
                 f.write(SSL_OUTPUT_HEADER.format(
-                    filename=os.path.basename(args.output), cmd=' '.join(sys.argv)))
+                    filename=os.path.basename(args.output),
+                    cmd=os.path.basename(sys.argv[0])))
                 f.write(DATA_FILES_PATH_VAR)
                 f.write('\n\n'.join(get_all_test_cases()))
                 f.write('\n')