From 35c4387550aba1528389749d7a5dec71b500546a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 3 Mar 2025 16:20:52 +0100 Subject: [PATCH 1/8] Clarify test case description Signed-off-by: Gilles Peskine --- scripts/generate_tls_handshake_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/generate_tls_handshake_tests.py b/scripts/generate_tls_handshake_tests.py index 86bf9f5b0..3d6a43c9a 100755 --- a/scripts/generate_tls_handshake_tests.py +++ b/scripts/generate_tls_handshake_tests.py @@ -80,7 +80,7 @@ def write_tls_handshake_defragmentation_test( # or at runtime), the TLS 1.2 ClientHello parser only sees # the first fragment of the ClientHello. tc.requirements.append('requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3') - tc.description += ' TLS 1.3 ClientHello -> 1.2 Handshake' + tc.description += ' with 1.3 support' # To guarantee that the handhake messages are large enough and need to be # split into fragments, the tests require certificate authentication. From 67e3374460989d510da42ad648a1e1c78a5bf34e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 3 Mar 2025 16:37:00 +0100 Subject: [PATCH 2/8] Diversify handshake defragmentation tests in TLS 1.2 by encryption The symmetric encryption used for transportation matters for TLS 1.2 defragmentation, since the code is sensitive to the presence of an explicit IV. So have separate test cases for each class of symmetric encryption. Signed-off-by: Gilles Peskine --- scripts/generate_tls_handshake_tests.py | 55 +++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/scripts/generate_tls_handshake_tests.py b/scripts/generate_tls_handshake_tests.py index 3d6a43c9a..ad5c73673 100755 --- a/scripts/generate_tls_handshake_tests.py +++ b/scripts/generate_tls_handshake_tests.py @@ -14,6 +14,7 @@ from typing import Optional from mbedtls_framework import tls_test_case from mbedtls_framework import typing_util +import translate_ciphers from mbedtls_framework.tls_test_case import Side, Version @@ -29,7 +30,10 @@ def write_tls_handshake_defragmentation_test( out: typing_util.Writable, side: Side, length: Optional[int], - version: Optional[Version] = None + version: Optional[Version] = None, + cipher: Optional[str] = None, + etm: Optional[bool] = None, #encrypt-then-mac (only relevant for CBC) + variant: str = '' ) -> None: """Generate one TLS handshake defragmentation test. @@ -125,6 +129,31 @@ def write_tls_handshake_defragmentation_test( fr'waiting for more fragments ({length} of', ] + if cipher is not None: + if side == Side.CLIENT: + our_args += ' force_ciphersuite=' + translate_ciphers.translate_mbedtls(cipher) + if 'NULL' in cipher: + their_args += ' -cipher ALL@SECLEVEL=0:COMPLEMENTOFALL@SECLEVEL=0' + else: + # For TLS 1.2, when Mbed TLS is the server, we must force the + # cipher suite on the client side, because passing + # force_ciphersuite to ssl_server2 would force a TLS-1.2-only + # server, which does not support a fragmented ClientHello. + tc.requirements.append('requires_ciphersuite_enabled ' + cipher) + their_args += ' -cipher ' + translate_ciphers.translate_ossl(cipher) + if 'NULL' in cipher: + their_args += '@SECLEVEL=0' + + if etm is not None: + if etm: + tc.requirements.append('requires_config_enabled MBEDTLS_SSL_ENCRYPT_THEN_MAC') + our_args += ' etm=' + str(int(etm)) + (wanted_patterns if etm else forbidden_patterns)[0:0] = [ + 'using encrypt then mac', + ] + + tc.description += variant + if side == Side.CLIENT: tc.client = '$P_CLI debug_level=4' + our_args tc.server = '$O_NEXT_SRV' + their_args @@ -139,13 +168,33 @@ def write_tls_handshake_defragmentation_test( tc.forbidden_server_patterns = forbidden_patterns tc.write(out) + +CIPHERS_FOR_TLS12_HANDSHAKE_DEFRAGMENTATION = [ + (None, 'default', None), + ('TLS_ECDHE_ECDSA_WITH_NULL_SHA', 'null', None), + ('TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256', 'ChachaPoly', None), + ('TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256', 'GCM', None), + ('TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256', 'CBC, etm=n', False), + ('TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256', 'CBC, etm=y', True), +] + def write_tls_handshake_defragmentation_tests(out: typing_util.Writable) -> None: """Generate TLS handshake defragmentation tests.""" for side in Side.CLIENT, Side.SERVER: write_tls_handshake_defragmentation_test(out, side, None) for length in [512, 513, 256, 128, 64, 36, 32, 16, 13, 5, 4, 3]: - write_tls_handshake_defragmentation_test(out, side, length, Version.TLS13) - write_tls_handshake_defragmentation_test(out, side, length, Version.TLS12) + write_tls_handshake_defragmentation_test(out, side, length, + Version.TLS13) + if length == 4: + for (cipher_suite, nickname, etm) in \ + CIPHERS_FOR_TLS12_HANDSHAKE_DEFRAGMENTATION: + write_tls_handshake_defragmentation_test( + out, side, length, Version.TLS12, + cipher=cipher_suite, etm=etm, + variant=', '+nickname) + else: + write_tls_handshake_defragmentation_test(out, side, length, + Version.TLS12) def write_handshake_tests(out: typing_util.Writable) -> None: From d850a6fe3575eb8c5a02cf039264c9e05d11c697 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 3 Mar 2025 16:39:14 +0100 Subject: [PATCH 3/8] Tweak "waiting for more handshake fragments" log message In preparation for reworking mbedtls_ssl_prepare_handshake_record(), tweak the "waiting for more handshake fragments" log message in ssl_consume_current_message(), and add a similar one in mbedtls_ssl_prepare_handshake_record(). Assert both. Signed-off-by: Gilles Peskine --- scripts/generate_tls_handshake_tests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/generate_tls_handshake_tests.py b/scripts/generate_tls_handshake_tests.py index ad5c73673..a34152f28 100755 --- a/scripts/generate_tls_handshake_tests.py +++ b/scripts/generate_tls_handshake_tests.py @@ -125,8 +125,9 @@ def write_tls_handshake_defragmentation_test( forbidden_patterns = [] wanted_patterns = [ 'reassembled record', - fr'handshake fragment: 0 \.\. {length} of [0-9]\+ msglen {length}', - fr'waiting for more fragments ({length} of', + fr'handshake fragment: {length}, 0\.\.{length} of [0-9]\+', + fr'Prepare: waiting for more handshake fragments {length}/', + fr'Consume: waiting for more handshake fragments {length}/', ] if cipher is not None: From 3c889a4f3a85ca47ed1a89d8fb953bcdca92450d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 3 Mar 2025 16:48:31 +0100 Subject: [PATCH 4/8] Tweak log message assertions In preparation for reworking mbedtls_ssl_prepare_handshake_record(), don't assert negatively on "reassembled record". Signed-off-by: Gilles Peskine --- scripts/generate_tls_handshake_tests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/generate_tls_handshake_tests.py b/scripts/generate_tls_handshake_tests.py index a34152f28..6ab7367fe 100755 --- a/scripts/generate_tls_handshake_tests.py +++ b/scripts/generate_tls_handshake_tests.py @@ -104,7 +104,6 @@ def write_tls_handshake_defragmentation_test( if length is None: forbidden_patterns = [ - 'reassembled record', 'waiting for more fragments', ] wanted_patterns = [] From 52d8701b99b4eb3fc8edce14d73eb3691d330cf2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 3 Mar 2025 16:21:06 +0100 Subject: [PATCH 5/8] Don't skip defragmentation test cases wih length < 16 Require the bug with defragmentation of encrypted handshake messages to be fixed. Signed-off-by: Gilles Peskine --- scripts/generate_tls_handshake_tests.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/scripts/generate_tls_handshake_tests.py b/scripts/generate_tls_handshake_tests.py index 6ab7367fe..17ec13893 100755 --- a/scripts/generate_tls_handshake_tests.py +++ b/scripts/generate_tls_handshake_tests.py @@ -56,17 +56,6 @@ def write_tls_handshake_defragmentation_test( description = f'Handshake defragmentation on {side.name.lower()}: {description}' tc = tls_test_case.TestCase(description) - if version == Version.TLS12 and \ - length is not None and \ - length >= TLS_HANDSHAKE_FRAGMENT_MIN_LENGTH and \ - length < 16 and \ - side == side.CLIENT: - # Skip test cases where the Finished message is fragmented in TLS 1.2. - # This is currently buggy when the symmetric encryption used an - # explicit IV (CBC, GCM or CCM; Chachapoly and null work, as does - # TLS 1.3, because they use a purely implicit IV). - tc.requirements.append('skip_next_test') - if version is not None: their_args += ' ' + version.openssl_option() # Emit a version requirement, because we're forcing the version via From 38bc126e4bf658c9cf11f48b86678ce4967f588f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 3 Mar 2025 16:50:43 +0100 Subject: [PATCH 6/8] Assert log for both initial and subsequent handshake fragments Signed-off-by: Gilles Peskine --- scripts/generate_tls_handshake_tests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/generate_tls_handshake_tests.py b/scripts/generate_tls_handshake_tests.py index 17ec13893..abf731061 100755 --- a/scripts/generate_tls_handshake_tests.py +++ b/scripts/generate_tls_handshake_tests.py @@ -113,7 +113,8 @@ def write_tls_handshake_defragmentation_test( forbidden_patterns = [] wanted_patterns = [ 'reassembled record', - fr'handshake fragment: {length}, 0\.\.{length} of [0-9]\+', + fr'initial handshake fragment: {length}, 0\.\.{length} of [0-9]\+', + fr'subsequent handshake fragment: [0-9]\+, {length}\.\.', fr'Prepare: waiting for more handshake fragments {length}/', fr'Consume: waiting for more handshake fragments {length}/', ] From 8a534d9d7846ead7ad4843fa91adfc37442e3215 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 3 Mar 2025 21:19:41 +0100 Subject: [PATCH 7/8] Pacify pylint Signed-off-by: Gilles Peskine --- scripts/generate_tls_handshake_tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/generate_tls_handshake_tests.py b/scripts/generate_tls_handshake_tests.py index abf731061..a57c6979c 100755 --- a/scripts/generate_tls_handshake_tests.py +++ b/scripts/generate_tls_handshake_tests.py @@ -14,9 +14,8 @@ from typing import Optional from mbedtls_framework import tls_test_case from mbedtls_framework import typing_util -import translate_ciphers - from mbedtls_framework.tls_test_case import Side, Version +import translate_ciphers # Assume that a TLS 1.2 ClientHello used in these tests will be at most @@ -27,6 +26,7 @@ TLS12_CLIENT_HELLO_ASSUMED_MAX_LENGTH = 255 TLS_HANDSHAKE_FRAGMENT_MIN_LENGTH = 4 def write_tls_handshake_defragmentation_test( + #pylint: disable=too-many-arguments out: typing_util.Writable, side: Side, length: Optional[int], From 8d85112a44d052a5d89cb0a135e162384da42584 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 4 Mar 2025 10:27:21 +0100 Subject: [PATCH 8/8] Fix cipher suite syntax in requires call Signed-off-by: Gilles Peskine --- scripts/generate_tls_handshake_tests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/generate_tls_handshake_tests.py b/scripts/generate_tls_handshake_tests.py index a57c6979c..1e9dbb944 100755 --- a/scripts/generate_tls_handshake_tests.py +++ b/scripts/generate_tls_handshake_tests.py @@ -120,8 +120,9 @@ def write_tls_handshake_defragmentation_test( ] if cipher is not None: + mbedtls_cipher = translate_ciphers.translate_mbedtls(cipher) if side == Side.CLIENT: - our_args += ' force_ciphersuite=' + translate_ciphers.translate_mbedtls(cipher) + our_args += ' force_ciphersuite=' + mbedtls_cipher if 'NULL' in cipher: their_args += ' -cipher ALL@SECLEVEL=0:COMPLEMENTOFALL@SECLEVEL=0' else: @@ -129,7 +130,7 @@ def write_tls_handshake_defragmentation_test( # cipher suite on the client side, because passing # force_ciphersuite to ssl_server2 would force a TLS-1.2-only # server, which does not support a fragmented ClientHello. - tc.requirements.append('requires_ciphersuite_enabled ' + cipher) + tc.requirements.append('requires_ciphersuite_enabled ' + mbedtls_cipher) their_args += ' -cipher ' + translate_ciphers.translate_ossl(cipher) if 'NULL' in cipher: their_args += '@SECLEVEL=0'