From 606671e06e91fd660c02db4878cbf8799551ed09 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 17 Feb 2023 11:36:33 +0100 Subject: [PATCH 01/11] tls13: server: Check mbedtls_ssl_set_hs_psk returned value Signed-off-by: Ronald Cron --- library/ssl_tls13_server.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 81c289aee..b91cde637 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -258,6 +258,8 @@ static int ssl_tls13_offered_psks_check_identity_match( int *psk_type, mbedtls_ssl_session *session) { + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + ((void) session); ((void) obfuscated_ticket_age); *psk_type = MBEDTLS_SSL_TLS1_3_PSK_EXTERNAL; @@ -271,9 +273,13 @@ static int ssl_tls13_offered_psks_check_identity_match( session) == SSL_TLS1_3_OFFERED_PSK_MATCH) { ssl->handshake->resume = 1; *psk_type = MBEDTLS_SSL_TLS1_3_PSK_RESUMPTION; - mbedtls_ssl_set_hs_psk(ssl, - session->resumption_key, - session->resumption_key_len); + ret = mbedtls_ssl_set_hs_psk(ssl, + session->resumption_key, + session->resumption_key_len); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_ssl_set_hs_psk", ret); + return ret; + } MBEDTLS_SSL_DEBUG_BUF(4, "Ticket-resumed PSK:", session->resumption_key, @@ -299,7 +305,11 @@ static int ssl_tls13_offered_psks_check_identity_match( identity_len == ssl->conf->psk_identity_len && mbedtls_ct_memcmp(ssl->conf->psk_identity, identity, identity_len) == 0) { - mbedtls_ssl_set_hs_psk(ssl, ssl->conf->psk, ssl->conf->psk_len); + ret = mbedtls_ssl_set_hs_psk(ssl, ssl->conf->psk, ssl->conf->psk_len); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_ssl_set_hs_psk", ret); + return ret; + } return SSL_TLS1_3_OFFERED_PSK_MATCH; } From fc7ae87ad49308024dd76519efe2b54ff0904a90 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 16 Feb 2023 15:32:19 +0100 Subject: [PATCH 02/11] tls13: server: Check ciphersuite list length parity once Check ciphersuite list length parity once, mainly to enable the possibility of getting out of the loop of the ciphersuites whenever we want. Signed-off-by: Ronald Cron --- library/ssl_tls13_server.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index b91cde637..f557d7f40 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1333,6 +1333,15 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, cipher_suites_len = MBEDTLS_GET_UINT16_BE(p, 0); p += 2; + /* + * The length of the ciphersuite list has to be even. + */ + if (cipher_suites_len & 1) { + MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_DECODE_ERROR); + return MBEDTLS_ERR_SSL_DECODE_ERROR; + } + /* Check we have enough data for the ciphersuite list, the legacy * compression methods and the length of the extensions. * @@ -1362,8 +1371,6 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, uint16_t cipher_suite; const mbedtls_ssl_ciphersuite_t *ciphersuite_info; - MBEDTLS_SSL_CHK_BUF_READ_PTR(p, cipher_suites_end, 2); - cipher_suite = MBEDTLS_GET_UINT16_BE(p, 0); ciphersuite_info = ssl_tls13_validate_peer_ciphersuite( ssl, cipher_suite); From 25e9ec61f07cd1d6fa8f483e03dbde8787f4f235 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 16 Feb 2023 15:35:16 +0100 Subject: [PATCH 03/11] tls13: server: Select preferred cipher suite Signed-off-by: Ronald Cron --- library/ssl_tls13_server.c | 2 ++ tests/ssl-opt.sh | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index f557d7f40..8aff19111 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1383,6 +1383,7 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG(2, ("selected ciphersuite: %04x - %s", cipher_suite, ciphersuite_info->name)); + break; } if (handshake->ciphersuite_info == NULL) { @@ -1390,6 +1391,7 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE); return MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE; } + p = cipher_suites_end; /* ... * opaque legacy_compression_methods<1..2^8-1>; diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index c176d0d62..b15fe16f7 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -13222,7 +13222,7 @@ requires_config_enabled MBEDTLS_DEBUG_C requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED -run_test "TLS 1.3: NewSessionTicket: Basic check, G->m" \ +run_test "TLS 1.3: NewSessionTicket: resumption failure, PSK len too big, G->m" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=4" \ "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3 -V -r" \ 0 \ @@ -13231,9 +13231,9 @@ run_test "TLS 1.3: NewSessionTicket: Basic check, G->m" \ -s "=> write NewSessionTicket msg" \ -s "server state: MBEDTLS_SSL_TLS1_3_NEW_SESSION_TICKET" \ -s "server state: MBEDTLS_SSL_TLS1_3_NEW_SESSION_TICKET_FLUSH" \ + -s "mbedtls_ssl_set_hs_psk() returned" \ -s "key exchange mode: ephemeral" \ - -s "key exchange mode: psk_ephemeral" \ - -s "found pre_shared_key extension" + -S "key exchange mode: psk_ephemeral" requires_config_enabled MBEDTLS_SSL_SESSION_TICKETS requires_config_enabled MBEDTLS_SSL_SRV_C From 0a1c50415636405ab17a4b5a310d83d7fba0a64f Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 20 Feb 2023 10:44:22 +0100 Subject: [PATCH 04/11] tls13: Fix session resumption with 384 bits PSKs MBEDTLS_PSK_MAX_LEN main purpose is to determine a miximum size for the TLS 1.2 pre-master secret. This is not relevant to TLS 1.3 thus disable in TLS 1.3 case the check against MBEDTLS_PSK_MAX_LEN when setting during the handshake the PSK through mbedtls_ssl_set_hs_psk(). This fixes the session resumption with 384 bits PSKs when MBEDTLS_PSK_MAX_LEN is smaller than that. Signed-off-by: Ronald Cron --- library/ssl_tls.c | 5 ++++- tests/ssl-opt.sh | 28 +++++++++++++++++++++++++--- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 86f5c0b55..4312f154a 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2145,9 +2145,12 @@ int mbedtls_ssl_set_hs_psk(mbedtls_ssl_context *ssl, return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } - if (psk_len > MBEDTLS_PSK_MAX_LEN) { +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if (ssl->tls_version == MBEDTLS_SSL_VERSION_TLS1_2 && + psk_len > MBEDTLS_PSK_MAX_LEN) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ ssl_remove_psk(ssl); diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index b15fe16f7..3f8b203a7 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -13222,7 +13222,7 @@ requires_config_enabled MBEDTLS_DEBUG_C requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED -run_test "TLS 1.3: NewSessionTicket: resumption failure, PSK len too big, G->m" \ +run_test "TLS 1.3: NewSessionTicket: Basic check" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=4" \ "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3 -V -r" \ 0 \ @@ -13231,9 +13231,31 @@ run_test "TLS 1.3: NewSessionTicket: resumption failure, PSK len too big, G-> -s "=> write NewSessionTicket msg" \ -s "server state: MBEDTLS_SSL_TLS1_3_NEW_SESSION_TICKET" \ -s "server state: MBEDTLS_SSL_TLS1_3_NEW_SESSION_TICKET_FLUSH" \ - -s "mbedtls_ssl_set_hs_psk() returned" \ -s "key exchange mode: ephemeral" \ - -S "key exchange mode: psk_ephemeral" + -s "key exchange mode: psk_ephemeral" \ + -s "found pre_shared_key extension" + +requires_gnutls_tls1_3 +requires_config_enabled MBEDTLS_SSL_SESSION_TICKETS +requires_config_enabled MBEDTLS_SSL_SRV_C +requires_config_enabled MBEDTLS_DEBUG_C +requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED \ + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED +requires_ciphersuite_enabled TLS1-3-AES-256-GCM-SHA384 +run_test "TLS 1.3: NewSessionTicket: Basic check with AES-256-GCM only, G->m" \ + "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=4" \ + "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:-CIPHER-ALL:+AES-256-GCM -V -r" \ + 0 \ + -c "Connecting again- trying to resume previous session" \ + -c "NEW SESSION TICKET (4) was received" \ + -s "Ciphersuite is TLS1-3-AES-256-GCM-SHA384" \ + -s "=> write NewSessionTicket msg" \ + -s "server state: MBEDTLS_SSL_TLS1_3_NEW_SESSION_TICKET" \ + -s "server state: MBEDTLS_SSL_TLS1_3_NEW_SESSION_TICKET_FLUSH" \ + -s "key exchange mode: ephemeral" \ + -s "key exchange mode: psk_ephemeral" \ + -s "found pre_shared_key extension" requires_config_enabled MBEDTLS_SSL_SESSION_TICKETS requires_config_enabled MBEDTLS_SSL_SRV_C From b18c67af5fc7c47f6251c6ea0b64fcec12109ee7 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 16 Feb 2023 16:57:16 +0100 Subject: [PATCH 05/11] tls13: ssl-opt.sh: Add test of default crypto algo Signed-off-by: Ronald Cron --- tests/ssl-opt.sh | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 3f8b203a7..68641385a 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11468,6 +11468,21 @@ run_test "TLS 1.3: Test gnutls tls1_3 feature" \ -c "Version: TLS1.3" # TLS1.3 test cases +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_config_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED +requires_ciphersuite_enabled TLS1-3-AES-128-GCM-SHA256 +requires_config_enabled MBEDTLS_ECP_DP_CURVE25519_ENABLED +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED +requires_config_enabled MBEDTLS_ECDSA_C +run_test "TLS 1.3: Default" \ + "$P_SRV allow_sha1=0 debug_level=3 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13" \ + "$P_CLI allow_sha1=0" \ + 0 \ + -s "Protocol is TLSv1.3" \ + -s "Ciphersuite is TLS1-3-AES-128-GCM-SHA256" \ + -s "ECDH group: x25519" \ + -s "selected signature algorithm ecdsa_secp256r1_sha256" + requires_openssl_tls1_3 requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_CLI_C From 4bb67736400540f3f2afb38e2abfae6cfd02018a Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 16 Feb 2023 15:51:18 +0100 Subject: [PATCH 06/11] tls13: Apply same preference rules for ciphersuites as for TLS 1.2 Signed-off-by: Ronald Cron --- library/ssl_ciphersuites.c | 4 ++-- tests/ssl-opt.sh | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/library/ssl_ciphersuites.c b/library/ssl_ciphersuites.c index 33789c463..501608a5a 100644 --- a/library/ssl_ciphersuites.c +++ b/library/ssl_ciphersuites.c @@ -52,9 +52,9 @@ static const int ciphersuite_preference[] = #else #if defined(MBEDTLS_SSL_PROTO_TLS1_3) /* TLS 1.3 ciphersuites */ - MBEDTLS_TLS1_3_AES_128_GCM_SHA256, - MBEDTLS_TLS1_3_AES_256_GCM_SHA384, MBEDTLS_TLS1_3_CHACHA20_POLY1305_SHA256, + MBEDTLS_TLS1_3_AES_256_GCM_SHA384, + MBEDTLS_TLS1_3_AES_128_GCM_SHA256, MBEDTLS_TLS1_3_AES_128_CCM_SHA256, MBEDTLS_TLS1_3_AES_128_CCM_8_SHA256, #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 68641385a..3b493ee39 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11470,7 +11470,7 @@ run_test "TLS 1.3: Test gnutls tls1_3 feature" \ # TLS1.3 test cases requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED -requires_ciphersuite_enabled TLS1-3-AES-128-GCM-SHA256 +requires_ciphersuite_enabled TLS1-3-CHACHA20-POLY1305-SHA256 requires_config_enabled MBEDTLS_ECP_DP_CURVE25519_ENABLED requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED requires_config_enabled MBEDTLS_ECDSA_C @@ -11479,7 +11479,7 @@ run_test "TLS 1.3: Default" \ "$P_CLI allow_sha1=0" \ 0 \ -s "Protocol is TLSv1.3" \ - -s "Ciphersuite is TLS1-3-AES-128-GCM-SHA256" \ + -s "Ciphersuite is TLS1-3-CHACHA20-POLY1305-SHA256" \ -s "ECDH group: x25519" \ -s "selected signature algorithm ecdsa_secp256r1_sha256" @@ -11503,7 +11503,7 @@ run_test "TLS 1.3: minimal feature sets - openssl" \ -c "client state: MBEDTLS_SSL_FLUSH_BUFFERS" \ -c "client state: MBEDTLS_SSL_HANDSHAKE_WRAPUP" \ -c "<= ssl_tls13_process_server_hello" \ - -c "server hello, chosen ciphersuite: ( 1301 ) - TLS1-3-AES-128-GCM-SHA256" \ + -c "server hello, chosen ciphersuite: ( 1303 ) - TLS1-3-CHACHA20-POLY1305-SHA256" \ -c "ECDH curve: x25519" \ -c "=> ssl_tls13_process_server_hello" \ -c "<= parse encrypted extensions" \ @@ -11537,7 +11537,7 @@ run_test "TLS 1.3: minimal feature sets - gnutls" \ -c "client state: MBEDTLS_SSL_FLUSH_BUFFERS" \ -c "client state: MBEDTLS_SSL_HANDSHAKE_WRAPUP" \ -c "<= ssl_tls13_process_server_hello" \ - -c "server hello, chosen ciphersuite: ( 1301 ) - TLS1-3-AES-128-GCM-SHA256" \ + -c "server hello, chosen ciphersuite: ( 1303 ) - TLS1-3-CHACHA20-POLY1305-SHA256" \ -c "ECDH curve: x25519" \ -c "=> ssl_tls13_process_server_hello" \ -c "<= parse encrypted extensions" \ @@ -11570,7 +11570,7 @@ run_test "TLS 1.3: alpn - openssl" \ -c "client state: MBEDTLS_SSL_FLUSH_BUFFERS" \ -c "client state: MBEDTLS_SSL_HANDSHAKE_WRAPUP" \ -c "<= ssl_tls13_process_server_hello" \ - -c "server hello, chosen ciphersuite: ( 1301 ) - TLS1-3-AES-128-GCM-SHA256" \ + -c "server hello, chosen ciphersuite: ( 1303 ) - TLS1-3-CHACHA20-POLY1305-SHA256" \ -c "ECDH curve: x25519" \ -c "=> ssl_tls13_process_server_hello" \ -c "<= parse encrypted extensions" \ @@ -11606,7 +11606,7 @@ run_test "TLS 1.3: alpn - gnutls" \ -c "client state: MBEDTLS_SSL_FLUSH_BUFFERS" \ -c "client state: MBEDTLS_SSL_HANDSHAKE_WRAPUP" \ -c "<= ssl_tls13_process_server_hello" \ - -c "server hello, chosen ciphersuite: ( 1301 ) - TLS1-3-AES-128-GCM-SHA256" \ + -c "server hello, chosen ciphersuite: ( 1303 ) - TLS1-3-CHACHA20-POLY1305-SHA256" \ -c "ECDH curve: x25519" \ -c "=> ssl_tls13_process_server_hello" \ -c "<= parse encrypted extensions" \ From 675d97d42e16fa4e55d80d76596b36c56730589b Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 17 Feb 2023 15:49:03 +0100 Subject: [PATCH 07/11] Add change log Signed-off-by: Ronald Cron --- .../tls13-reorder-ciphersuite-preference-list.txt | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 ChangeLog.d/tls13-reorder-ciphersuite-preference-list.txt diff --git a/ChangeLog.d/tls13-reorder-ciphersuite-preference-list.txt b/ChangeLog.d/tls13-reorder-ciphersuite-preference-list.txt new file mode 100644 index 000000000..948bc882a --- /dev/null +++ b/ChangeLog.d/tls13-reorder-ciphersuite-preference-list.txt @@ -0,0 +1,11 @@ +Default behavior changes + * The default priority order of TLS 1.3 cipher suites has been modified to + follow the same rules as the TLS 1.2 cipher suites (see + ssl_ciphersuites.c). + +Bugfix + * In the TLS 1.3 server, select the prefered client cipher suite, not the + least prefered. The selection error was introduced in Mbed TLS 3.3.0. + * Fix TLS 1.3 session resumption when the established pre-shared key is + 384 bits long. That is the length of pre-shared keys created under a + session where the cipher suite is TLS_AES_256_GCM_SHA384. From d89360b87bb5bcf09a21c74a799056c49260b260 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 21 Feb 2023 08:53:33 +0100 Subject: [PATCH 08/11] Fix and improve documentation, comments and logs Signed-off-by: Ronald Cron --- ChangeLog.d/tls13-reorder-ciphersuite-preference-list.txt | 7 ++++--- library/ssl_tls13_server.c | 5 +++++ tests/ssl-opt.sh | 5 ++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/ChangeLog.d/tls13-reorder-ciphersuite-preference-list.txt b/ChangeLog.d/tls13-reorder-ciphersuite-preference-list.txt index 948bc882a..1d3406854 100644 --- a/ChangeLog.d/tls13-reorder-ciphersuite-preference-list.txt +++ b/ChangeLog.d/tls13-reorder-ciphersuite-preference-list.txt @@ -1,11 +1,12 @@ Default behavior changes * The default priority order of TLS 1.3 cipher suites has been modified to follow the same rules as the TLS 1.2 cipher suites (see - ssl_ciphersuites.c). + ssl_ciphersuites.c). The preferred cipher suite is now + TLS_CHACHA20_POLY1305_SHA256. Bugfix - * In the TLS 1.3 server, select the prefered client cipher suite, not the - least prefered. The selection error was introduced in Mbed TLS 3.3.0. + * In the TLS 1.3 server, select the preferred client cipher suite, not the + least preferred. The selection error was introduced in Mbed TLS 3.3.0. * Fix TLS 1.3 session resumption when the established pre-shared key is 384 bits long. That is the length of pre-shared keys created under a session where the cipher suite is TLS_AES_256_GCM_SHA384. diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 8aff19111..005a1d799 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1371,6 +1371,11 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, uint16_t cipher_suite; const mbedtls_ssl_ciphersuite_t *ciphersuite_info; + /* + * "cipher_suite_end - p is even" is an invariant of the loop. As + * cipher_suites_end - p > 0, we have cipher_suites_end - p >= 2 and + * it is thus safe to read two bytes. + */ cipher_suite = MBEDTLS_GET_UINT16_BE(p, 0); ciphersuite_info = ssl_tls13_validate_peer_ciphersuite( ssl, cipher_suite); diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 3b493ee39..b1ee65493 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -13237,7 +13237,7 @@ requires_config_enabled MBEDTLS_DEBUG_C requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED -run_test "TLS 1.3: NewSessionTicket: Basic check" \ +run_test "TLS 1.3: NewSessionTicket: Basic check, G->m" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=4" \ "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3 -V -r" \ 0 \ @@ -13257,6 +13257,9 @@ requires_config_enabled MBEDTLS_DEBUG_C requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED +# Test the session resumption when the cipher suite for the original session is +# TLS1-3-AES-256-GCM-SHA384. In that case, the PSK is 384 bits long and not +# 256 bits long as with all the other TLS 1.3 cipher suites. requires_ciphersuite_enabled TLS1-3-AES-256-GCM-SHA384 run_test "TLS 1.3: NewSessionTicket: Basic check with AES-256-GCM only, G->m" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=4" \ From 1aa6e8d6e9f5af25c24a2484d774f9a879aa873a Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 23 Feb 2023 09:46:54 +0100 Subject: [PATCH 09/11] Restore same PSK length enforcement Restore same PSK length enforcement in conf_psk and set_hs_psk, whether the negotiated protocol is TLS 1.2 or TLS 1.3. Signed-off-by: Ronald Cron --- include/mbedtls/mbedtls_config.h | 2 +- include/mbedtls/ssl.h | 16 +++++++++++++++- library/ssl_tls.c | 5 +---- tests/ssl-opt.sh | 2 +- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 9ae51c964..4c676c520 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -3761,7 +3761,7 @@ */ //#define MBEDTLS_SSL_DTLS_MAX_BUFFERING 32768 -//#define MBEDTLS_PSK_MAX_LEN 32 /**< Max size of TLS pre-shared keys, in bytes (default 256 bits) */ +//#define MBEDTLS_PSK_MAX_LEN 32 /**< Max size of TLS pre-shared keys, in bytes (default 256 or 384 bits) */ //#define MBEDTLS_SSL_COOKIE_TIMEOUT 60 /**< Default expiration delay of DTLS cookies, in seconds if HAVE_TIME, or in number of cookies issued */ /** diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 4b954bb45..0df142d68 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -599,8 +599,22 @@ * Size defines */ #if !defined(MBEDTLS_PSK_MAX_LEN) -#define MBEDTLS_PSK_MAX_LEN 32 /* 256 bits */ +/* + * If the library supports TLS 1.3 tickets and the cipher suite + * TLS1-3-AES-256-GCM-SHA384, set the PSK maximum length to 48 instead of 32. + * That way, the TLS 1.3 client and server are able to resume sessions where + * the cipher suite was TLS1-3-AES-256-GCM-SHA384 (pre-shared keys are 48 + * bytes long is that case). + */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ + defined(MBEDTLS_SSL_SESSION_TICKETS) && \ + defined(MBEDTLS_AES_C) && defined(MBEDTLS_GCM_C) && \ + defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) +#define MBEDTLS_PSK_MAX_LEN 48 /* 384 bits */ +#else +#define MBEDTLS_PSK_MAX_LEN 32 /* 256 bits */ #endif +#endif /* !MBEDTLS_PSK_MAX_LEN */ /* Dummy type used only for its size */ union mbedtls_ssl_premaster_secret { diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 4312f154a..86f5c0b55 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2145,12 +2145,9 @@ int mbedtls_ssl_set_hs_psk(mbedtls_ssl_context *ssl, return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if (ssl->tls_version == MBEDTLS_SSL_VERSION_TLS1_2 && - psk_len > MBEDTLS_PSK_MAX_LEN) { + if (psk_len > MBEDTLS_PSK_MAX_LEN) { return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } -#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ ssl_remove_psk(ssl); diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index b1ee65493..679406873 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -13239,7 +13239,7 @@ requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED run_test "TLS 1.3: NewSessionTicket: Basic check, G->m" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=4" \ - "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3 -V -r" \ + "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:-AES-256-GCM -V -r" \ 0 \ -c "Connecting again- trying to resume previous session" \ -c "NEW SESSION TICKET (4) was received" \ From ee54de02b196145295037142dee589476d47f94c Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 24 Feb 2023 12:06:30 +0100 Subject: [PATCH 10/11] Fix comments Signed-off-by: Ronald Cron --- include/mbedtls/ssl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 0df142d68..7d9643ee4 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -603,8 +603,8 @@ * If the library supports TLS 1.3 tickets and the cipher suite * TLS1-3-AES-256-GCM-SHA384, set the PSK maximum length to 48 instead of 32. * That way, the TLS 1.3 client and server are able to resume sessions where - * the cipher suite was TLS1-3-AES-256-GCM-SHA384 (pre-shared keys are 48 - * bytes long is that case). + * the cipher suite is TLS1-3-AES-256-GCM-SHA384 (pre-shared keys are 48 + * bytes long in that case). */ #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ defined(MBEDTLS_SSL_SESSION_TICKETS) && \ From 7dc413021006df09e99f2cf6804ef46139cddd36 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 24 Feb 2023 12:10:09 +0100 Subject: [PATCH 11/11] Improve GnuTLS client priority for resumption basic check Signed-off-by: Ronald Cron --- tests/ssl-opt.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 679406873..b1ee65493 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -13239,7 +13239,7 @@ requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_ENABLED run_test "TLS 1.3: NewSessionTicket: Basic check, G->m" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=4" \ - "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:-AES-256-GCM -V -r" \ + "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3 -V -r" \ 0 \ -c "Connecting again- trying to resume previous session" \ -c "NEW SESSION TICKET (4) was received" \