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..1d3406854 --- /dev/null +++ b/ChangeLog.d/tls13-reorder-ciphersuite-preference-list.txt @@ -0,0 +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). The preferred cipher suite is now + TLS_CHACHA20_POLY1305_SHA256. + +Bugfix + * 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/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..7d9643ee4 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 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) && \ + 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_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/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 81c289aee..005a1d799 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; } @@ -1323,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. * @@ -1352,8 +1371,11 @@ 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_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); @@ -1366,6 +1388,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) { @@ -1373,6 +1396,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..b1ee65493 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-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 +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-CHACHA20-POLY1305-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 @@ -11488,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" \ @@ -11522,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" \ @@ -11555,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" \ @@ -11591,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" \ @@ -13235,6 +13250,31 @@ run_test "TLS 1.3: NewSessionTicket: Basic check, G->m" \ -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 +# 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" \ + "$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 requires_config_enabled MBEDTLS_SSL_CLI_C