From 0038c5ff1c5e6b488f294704cb6b4e90d3f262f5 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 7 Jul 2022 06:49:01 +0000 Subject: [PATCH 01/16] Add ticket nonce setting Signed-off-by: Jerry Yu --- include/mbedtls/mbedtls_config.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 1c60ec8e4..37793a791 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -1541,6 +1541,15 @@ */ //#define MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE +/** + * \def MBEDTLS_SSL_TICKET_NONCE_LENGTH + * + * Size in bytes of a ticket nonce. This is not used in TLS 1.2. + * + * The default value is 32 bytes. + */ +#define MBEDTLS_SSL_TICKET_NONCE_LENGTH 32 + /** * \def MBEDTLS_SSL_PROTO_DTLS * From a270f6734080a618b1c4309b918b5090adeed93e Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 7 Jul 2022 06:51:06 +0000 Subject: [PATCH 02/16] Add tls13 session fields Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 17 +++++++++++++++++ library/ssl_misc.h | 4 ++++ 2 files changed, 21 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 7893edd13..24c9077b2 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1171,6 +1171,23 @@ struct mbedtls_ssl_session uint32_t MBEDTLS_PRIVATE(ticket_lifetime); /*!< ticket lifetime hint */ #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS) + uint8_t MBEDTLS_PRIVATE(ticket_flags); /*!< Ticket flags */ + uint32_t MBEDTLS_PRIVATE(ticket_age_add); /*!< Randomly generated value used to obscure the age of the ticket */ + uint8_t MBEDTLS_PRIVATE(key_len); /*!< PSK key length */ + +#if defined(MBEDTLS_SHA384_C) + unsigned char MBEDTLS_PRIVATE(key)[48]; /*!< key (48 byte) */ +#elif defined(MBEDTLS_SHA256_C) + unsigned char MBEDTLS_PRIVATE(key)[32]; /*!< key (32 byte) */ +#endif /* MBEDTLS_SHA256_C */ + +#if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_CLI_C) + time_t MBEDTLS_PRIVATE(ticket_received); /*!< time ticket was received */ +#endif /* MBEDTLS_HAVE_TIME && MBEDTLS_SSL_CLI_C */ + +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 && MBEDTLS_SSL_SESSION_TICKETS */ + #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) int MBEDTLS_PRIVATE(encrypt_then_mac); /*!< flag for EtM activation */ #endif diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 39a47cac7..777d44b88 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -101,6 +101,10 @@ #define MBEDTLS_SSL_EXT_SIG_ALG_CERT ( 1 << 20 ) #define MBEDTLS_SSL_EXT_KEY_SHARE ( 1 << 21 ) +#define MBEDTLS_SSL_TICKET_FLAG_ALLOW_EARLY_DATA ( 1 << 0 ) +#define MBEDTLS_SSL_TICKET_FLAG_ALLOW_DHE_RESUMPTION ( 1 << 1 ) +#define MBEDTLS_SSL_TICKET_FLAG_ALLOW_PSK_RESUMPTION ( 1 << 2 ) + /* * Helper macros for function call with return check. */ From c62ae5f5392eec8eabb9bd496c9a8253c4d25577 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 7 Jul 2022 09:42:26 +0000 Subject: [PATCH 03/16] Add new session ticket message check Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 1 + library/ssl_msg.c | 70 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 24c9077b2..8ea096648 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -658,6 +658,7 @@ typedef enum MBEDTLS_SSL_CLIENT_CCS_BEFORE_2ND_CLIENT_HELLO, MBEDTLS_SSL_SERVER_CCS_AFTER_SERVER_HELLO, MBEDTLS_SSL_SERVER_CCS_AFTER_HELLO_RETRY_REQUEST, + MBEDTLS_SSL_CLIENT_NEW_SESSION_TICKET, } mbedtls_ssl_states; diff --git a/library/ssl_msg.c b/library/ssl_msg.c index fb0b70997..4d7306813 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -5288,6 +5288,48 @@ static int ssl_check_ctr_renegotiate( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_RENEGOTIATION */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + +#if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) +static int ssl_tls13_check_new_session_ticket( mbedtls_ssl_context *ssl ) +{ + + if( ( ssl->in_hslen == mbedtls_ssl_hs_hdr_len( ssl ) ) || + ( ssl->in_msg[0] != MBEDTLS_SSL_HS_NEW_SESSION_TICKET ) ) + { + return( 0 ); + } + + ssl->keep_current_message = 1; + + MBEDTLS_SSL_DEBUG_MSG( 3, ( "NewSessionTicket received" ) ); + mbedtls_ssl_handshake_set_state( ssl, + MBEDTLS_SSL_CLIENT_NEW_SESSION_TICKET ); + + return( MBEDTLS_ERR_SSL_WANT_READ ); +} +#endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ + +static int ssl_tls13_handle_hs_message_post_handshake( mbedtls_ssl_context *ssl ) +{ + + MBEDTLS_SSL_DEBUG_MSG( 3, ( "received post-handshake message" ) ); + +#if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) + if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) + { + int ret = ssl_tls13_check_new_session_ticket( ssl ); + if( ret != 0 ) + return( ret ); + } +#endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ + + /* Fail in all other cases. */ + return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); +} +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ + +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) /* This function is called from mbedtls_ssl_read() when a handshake message is * received after the initial handshake. In this context, handshake messages * may only be sent for the purpose of initiating renegotiations. @@ -5297,8 +5339,7 @@ static int ssl_check_ctr_renegotiate( mbedtls_ssl_context *ssl ) * and having a helper function allows to distinguish between TLS <= 1.2 and * TLS 1.3 in the future without bloating the logic of mbedtls_ssl_read(). */ -MBEDTLS_CHECK_RETURN_CRITICAL -static int ssl_handle_hs_message_post_handshake( mbedtls_ssl_context *ssl ) +static int ssl_tls12_handle_hs_message_post_handshake( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -5380,18 +5421,39 @@ static int ssl_handle_hs_message_post_handshake( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "refusing renegotiation, sending alert" ) ); -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) if( ( ret = mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_WARNING, MBEDTLS_SSL_ALERT_MSG_NO_RENEGOTIATION ) ) != 0 ) { return( ret ); } -#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ } return( 0 ); } +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ + +MBEDTLS_CHECK_RETURN_CRITICAL +static int ssl_handle_hs_message_post_handshake( mbedtls_ssl_context *ssl ) +{ + /* Check protocol version and dispatch accordingly. */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + if( ssl->tls_version == MBEDTLS_SSL_VERSION_TLS1_3 ) + { + return( ssl_tls13_handle_hs_message_post_handshake( ssl ) ); + } +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ + +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if( ssl->tls_version <= MBEDTLS_SSL_VERSION_TLS1_2 ) + { + return( ssl_tls12_handle_hs_message_post_handshake( ssl ) ); + } +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ + + /* Should never happen */ + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); +} /* * Receive application data decrypted from the SSL layer From f8a4994ec7985c80e0e0d1921d441bae5851a86d Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 7 Jul 2022 11:32:32 +0000 Subject: [PATCH 04/16] Add tls13 new session ticket parser Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 2 + library/ssl_tls13_client.c | 237 +++++++++++++++++++++++++++++++++++++ 2 files changed, 239 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 8ea096648..0477e916a 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -98,6 +98,8 @@ /* Error space gap */ /** Processing of the Certificate handshake message failed. */ #define MBEDTLS_ERR_SSL_BAD_CERTIFICATE -0x7A00 +/** Received NewSessionTicket Post Handshake Message */ +#define MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET -0x7B00 /* Error space gap */ /* Error space gap */ /* Error space gap */ diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 2b68306f0..5e368a10e 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1876,6 +1876,234 @@ static int ssl_tls13_handshake_wrapup( mbedtls_ssl_context *ssl ) return( 0 ); } +#if defined(MBEDTLS_SSL_SESSION_TICKETS) + +static int ssl_tls13_parse_new_session_ticket_extensions( + mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + unsigned char *p = ( unsigned char * )buf; + + ((void) ssl); + + while( p < end ) + { + unsigned int extension_type; + size_t extension_data_len; + const unsigned char *extension_data_end; + + ((void) extension_data_end); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 4 ); + extension_type = MBEDTLS_GET_UINT16_BE( p, 0 ); + extension_data_len = MBEDTLS_GET_UINT16_BE( p, 2 ); + p += 4; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, extension_data_len ); + extension_data_end = p + extension_data_len; + + switch( extension_type ) + { + case MBEDTLS_TLS_EXT_EARLY_DATA: + MBEDTLS_SSL_DEBUG_MSG( 4, ( "early_data extension received" ) ); + break; + default: + break; + } + p += extension_data_len; + } + + return( 0 ); +} + +/* + * struct { + * uint32 ticket_lifetime; + * uint32 ticket_age_add; + * opaque ticket_nonce<0..255>; + * opaque ticket<1..2^16-1>; + * Extension extensions<0..2^16-2>; + * } NewSessionTicket; + * + */ +static int ssl_tls13_parse_new_session_ticket( mbedtls_ssl_context *ssl, + unsigned char *buf, + unsigned char *end ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + unsigned char *p = buf; + mbedtls_ssl_session *session = ssl->session; + size_t ticket_nonce_len; + unsigned char *ticket_nonce; + size_t ticket_len; + unsigned char *ticket; + size_t extensions_len; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info; + psa_algorithm_t psa_hash_alg; + int hash_length; + + /* + * ticket_lifetime 4 bytes + * ticket_age_add 4 bytes + * ticket_nonce >=1 byte + * ticket >=2 bytes + * extensions >=2 bytes + */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 13); + + session->ticket_lifetime = MBEDTLS_GET_UINT32_BE( p, 0 ); + p += 4; + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "ticket->lifetime: %u", + ( unsigned int )session->ticket_lifetime ) ); + + session->ticket_age_add = MBEDTLS_GET_UINT32_BE( p, 0 ); + p += 4; + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "ticket->ticket_age_add: %u", + ( unsigned int )session->ticket_age_add ) ); + + ticket_nonce_len = *p++; + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, ticket_nonce_len ); + ticket_nonce = p; + MBEDTLS_SSL_DEBUG_BUF( 3, "ticket_nonce:", ticket_nonce, ticket_nonce_len ); + p += ticket_nonce_len; + + /* Ticket */ + ticket_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, ticket_len ); + + MBEDTLS_SSL_DEBUG_BUF( 3, "received ticket", p, ticket_len ) ; + + /* Check if we previously received a ticket already. */ + if( session->ticket != NULL || session->ticket_len > 0 ) + { + mbedtls_free( session->ticket ); + session->ticket = NULL; + session->ticket_len = 0; + } + + if( ( ticket = mbedtls_calloc( 1, ticket_len ) ) == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "ticket alloc failed" ) ); + return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + } + memcpy( ticket, p, ticket_len ); + p += ticket_len; + session->ticket = ticket; + session->ticket_len = ticket_len; + MBEDTLS_SSL_DEBUG_BUF( 4, "stored ticket", ticket, ticket_len ); + + extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, extensions_len ); + + MBEDTLS_SSL_DEBUG_BUF( 3, "ticket->extension", p, extensions_len ); + + ret = ssl_tls13_parse_new_session_ticket_extensions( ssl, p, end ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, + "ssl_tls13_parse_new_session_ticket_extensions", + ret ); + return( ret ); + } + p += extensions_len; + + /* Compute PSK based on received nonce and resumption_master_secret + * in the following style: + * + * HKDF-Expand-Label( resumption_master_secret, + * "resumption", ticket_nonce, Hash.length ) + */ + ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( session->ciphersuite ); + if( ciphersuite_info == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + psa_hash_alg = mbedtls_psa_translate_md( ciphersuite_info->mac ); + hash_length = PSA_HASH_LENGTH( psa_hash_alg ); + if( hash_length == -1 ) + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + + MBEDTLS_SSL_DEBUG_BUF( 3, "resumption_master_secret", + session->app_secrets.resumption_master_secret, + hash_length ); + + /* Computer resumption key + * + * HKDF-Expand-Label( resumption_master_secret, + * "resumption", ticket_nonce, Hash.length ) + */ + ret = mbedtls_ssl_tls13_hkdf_expand_label( + psa_hash_alg, + session->app_secrets.resumption_master_secret, + hash_length, + MBEDTLS_SSL_TLS1_3_LBL_WITH_LEN( resumption ), + ticket_nonce, + ticket_nonce_len, + session->key, + hash_length ); + + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 2, + "Creating the ticket-resumed PSK failed", + ret ); + return( ret ); + } + + session->key_len = hash_length; + + MBEDTLS_SSL_DEBUG_BUF( 3, "Ticket-resumed PSK", + session->key, + session->key_len ); + +#if defined(MBEDTLS_HAVE_TIME) + /* Store ticket creation time */ + session->ticket_received = time( NULL ); +#endif + + return( 0 ); +} + +static int ssl_tls13_postprocess_new_session_ticket( mbedtls_ssl_context *ssl ) +{ + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HANDSHAKE_OVER ); + return( 0 ); +} + +/* + * Handler for MBEDTLS_SSL_CLIENT_NEW_SESSION_TICKET + */ +static int ssl_tls13_process_new_session_ticket( mbedtls_ssl_context *ssl ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + unsigned char *buf; + size_t buf_len; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse new session ticket" ) ); + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( + ssl, MBEDTLS_SSL_HS_NEW_SESSION_TICKET, + &buf, &buf_len ) ); + + MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_new_session_ticket( ssl, + buf, + buf + buf_len ) ); + + MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_new_session_ticket( ssl ) ); + +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse new session ticket" ) ); + return( ret ); +} +#endif /* MBEDTLS_SSL_SESSION_TICKETS */ + int mbedtls_ssl_tls13_handshake_client_step( mbedtls_ssl_context *ssl ) { int ret = 0; @@ -1956,6 +2184,15 @@ int mbedtls_ssl_tls13_handshake_client_step( mbedtls_ssl_context *ssl ) break; #endif /* MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE */ +#if defined(MBEDTLS_SSL_SESSION_TICKETS) + case MBEDTLS_SSL_CLIENT_NEW_SESSION_TICKET: + ret = ssl_tls13_process_new_session_ticket( ssl ); + if( ret != 0 ) + break; + ret = MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET; + break; +#endif /* MBEDTLS_SSL_SESSION_TICKETS */ + default: MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid state %d", ssl->state ) ); return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); From 2b4f02d7fb55de03a461482d19b8e07c320d8fdb Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 7 Jul 2022 11:41:58 +0000 Subject: [PATCH 05/16] Add new_session_ticket err handler Signed-off-by: Jerry Yu --- programs/ssl/ssl_client2.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index d6724dfb1..e8b8b1e3d 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -2645,8 +2645,6 @@ send_request: /* * 7. Read the HTTP response */ - mbedtls_printf( " < Read from server:" ); - fflush( stdout ); /* * TLS and DTLS need different reading styles (stream vs datagram) @@ -2694,6 +2692,18 @@ send_request: ret = 0; goto reconnect; +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + +#if defined(MBEDTLS_SSL_SESSION_TICKETS) + case MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET: + /* We were waiting for application data but got + * a NewSessionTicket instead. */ + mbedtls_printf( " got new session ticket.\n" ); + continue; +#endif /* MBEDTLS_SSL_SESSION_TICKETS */ + +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ + default: mbedtls_printf( " mbedtls_ssl_read returned -0x%x\n", (unsigned int) -ret ); @@ -2703,8 +2713,8 @@ send_request: len = ret; buf[len] = '\0'; - mbedtls_printf( " %d bytes read\n\n%s", len, (char *) buf ); - + mbedtls_printf( " < Read from server: %d bytes read\n\n%s", len, (char *) buf ); + fflush( stdout ); /* End of message should be detected according to the syntax of the * application protocol (eg HTTP), just use a dummy test here. */ if( ret > 0 && buf[len-1] == '\n' ) @@ -2767,7 +2777,7 @@ send_request: len = ret; buf[len] = '\0'; - mbedtls_printf( " %d bytes read\n\n%s", len, (char *) buf ); + mbedtls_printf( " < Read from server: %d bytes read\n\n%s", len, (char *) buf ); ret = 0; } From 29ab32d0e57ead1d51da715925c0d6ea7a71fc3c Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 7 Jul 2022 11:33:35 +0000 Subject: [PATCH 06/16] Add client side tests Signed-off-by: Jerry Yu --- tests/ssl-opt.sh | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 68a2d778b..a6c9708fc 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -12599,6 +12599,34 @@ run_test "TLS 1.3: Check client no signature algorithm, m->m" \ 1 \ -c "select_sig_alg_for_certificate_verify:no suitable signature algorithm found" +requires_openssl_tls1_3 +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_CLI_C +requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE +run_test "TLS 1.3: NewSessionTicket: Basic check, m->O" \ + "$O_NEXT_SRV -msg -tls1_3 -no_resume_ephemeral -no_cache " \ + "$P_CLI debug_level=4" \ + 0 \ + -c "Protocol is TLSv1.3" \ + -c "MBEDTLS_SSL_CLIENT_NEW_SESSION_TICKET" \ + -c "got new session ticket." \ + -c "HTTP/1.0 200 ok" + +requires_gnutls_tls1_3 +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_CLI_C +run_test "TLS 1.3: NewSessionTicket: Basic check, m->G" \ + "$G_NEXT_SRV --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+CIPHER-ALL --disable-client-cert" \ + "$P_CLI debug_level=4" \ + 0 \ + -c "Protocol is TLSv1.3" \ + -c "MBEDTLS_SSL_CLIENT_NEW_SESSION_TICKET" \ + -c "got new session ticket." \ + -c "HTTP/1.0 200 OK" + # Test heap memory usage after handshake requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_MEMORY_DEBUG From a357cf4d4c200d6b7790124e7cf38b9d3ac17509 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 12 Jul 2022 05:36:45 +0000 Subject: [PATCH 07/16] Rename new_session_ticket state Both client and server side use `MBEDTLS_SSL_NEW_SESSION_TICKET` now Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 3 +-- library/ssl_msg.c | 2 +- library/ssl_tls12_client.c | 4 ++-- library/ssl_tls13_client.c | 4 ++-- tests/ssl-opt.sh | 4 ++-- tests/suites/test_suite_ssl.data | 2 +- 6 files changed, 9 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 0477e916a..9703fcb16 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -651,7 +651,7 @@ typedef enum MBEDTLS_SSL_FLUSH_BUFFERS, MBEDTLS_SSL_HANDSHAKE_WRAPUP, MBEDTLS_SSL_HANDSHAKE_OVER, - MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET, + MBEDTLS_SSL_NEW_SESSION_TICKET, MBEDTLS_SSL_SERVER_HELLO_VERIFY_REQUEST_SENT, MBEDTLS_SSL_HELLO_RETRY_REQUEST, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS, @@ -660,7 +660,6 @@ typedef enum MBEDTLS_SSL_CLIENT_CCS_BEFORE_2ND_CLIENT_HELLO, MBEDTLS_SSL_SERVER_CCS_AFTER_SERVER_HELLO, MBEDTLS_SSL_SERVER_CCS_AFTER_HELLO_RETRY_REQUEST, - MBEDTLS_SSL_CLIENT_NEW_SESSION_TICKET, } mbedtls_ssl_states; diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 4d7306813..9c207948d 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -5304,7 +5304,7 @@ static int ssl_tls13_check_new_session_ticket( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "NewSessionTicket received" ) ); mbedtls_ssl_handshake_set_state( ssl, - MBEDTLS_SSL_CLIENT_NEW_SESSION_TICKET ); + MBEDTLS_SSL_NEW_SESSION_TICKET ); return( MBEDTLS_ERR_SSL_WANT_READ ); } diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index 7fa6443a0..240520854 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -3627,7 +3627,7 @@ int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ) if( ssl->state == MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC && ssl->handshake->new_session_ticket != 0 ) { - ssl->state = MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET; + ssl->state = MBEDTLS_SSL_NEW_SESSION_TICKET; } #endif @@ -3704,7 +3704,7 @@ int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ) * Finished */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) - case MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET: + case MBEDTLS_SSL_NEW_SESSION_TICKET: ret = ssl_parse_new_session_ticket( ssl ); break; #endif diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 5e368a10e..42653a310 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2077,7 +2077,7 @@ static int ssl_tls13_postprocess_new_session_ticket( mbedtls_ssl_context *ssl ) } /* - * Handler for MBEDTLS_SSL_CLIENT_NEW_SESSION_TICKET + * Handler for MBEDTLS_SSL_NEW_SESSION_TICKET */ static int ssl_tls13_process_new_session_ticket( mbedtls_ssl_context *ssl ) { @@ -2185,7 +2185,7 @@ int mbedtls_ssl_tls13_handshake_client_step( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) - case MBEDTLS_SSL_CLIENT_NEW_SESSION_TICKET: + case MBEDTLS_SSL_NEW_SESSION_TICKET: ret = ssl_tls13_process_new_session_ticket( ssl ); if( ret != 0 ) break; diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index a6c9708fc..942d70524 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -12609,7 +12609,7 @@ run_test "TLS 1.3: NewSessionTicket: Basic check, m->O" \ "$P_CLI debug_level=4" \ 0 \ -c "Protocol is TLSv1.3" \ - -c "MBEDTLS_SSL_CLIENT_NEW_SESSION_TICKET" \ + -c "MBEDTLS_SSL_NEW_SESSION_TICKET" \ -c "got new session ticket." \ -c "HTTP/1.0 200 ok" @@ -12623,7 +12623,7 @@ run_test "TLS 1.3: NewSessionTicket: Basic check, m->G" \ "$P_CLI debug_level=4" \ 0 \ -c "Protocol is TLSv1.3" \ - -c "MBEDTLS_SSL_CLIENT_NEW_SESSION_TICKET" \ + -c "MBEDTLS_SSL_NEW_SESSION_TICKET" \ -c "got new session ticket." \ -c "HTTP/1.0 200 OK" diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 19a1ae6f0..34f4d66c4 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -223,7 +223,7 @@ move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_SERVER_HELLO_VERIFY_RE Negative test moving servers ssl to state: NEW_SESSION_TICKET depends_on:MBEDTLS_SSL_PROTO_TLS1_2 -move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET:0 +move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_NEW_SESSION_TICKET:0 TLS 1.3:Test moving clients handshake to state: ENCRYPTED_EXTENSIONS depends_on:MBEDTLS_SSL_PROTO_TLS1_3:!MBEDTLS_SSL_PROTO_TLS1_2 From af2c0c8dd6d59e48fdfce962c2b23d6759c05a5a Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 12 Jul 2022 05:47:21 +0000 Subject: [PATCH 08/16] fix various comment/format issues Signed-off-by: Jerry Yu --- include/mbedtls/mbedtls_config.h | 2 +- library/ssl_tls13_client.c | 17 +++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 37793a791..ed673e7d7 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -1546,7 +1546,7 @@ * * Size in bytes of a ticket nonce. This is not used in TLS 1.2. * - * The default value is 32 bytes. + * This must be smaller or equal to 255. */ #define MBEDTLS_SSL_TICKET_NONCE_LENGTH 32 diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 42653a310..b61626d07 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1878,12 +1878,11 @@ static int ssl_tls13_handshake_wrapup( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_SESSION_TICKETS) -static int ssl_tls13_parse_new_session_ticket_extensions( - mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ) +static int ssl_tls13_parse_new_session_ticket_exts( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) { - unsigned char *p = ( unsigned char * )buf; + const unsigned char *p = buf; ((void) ssl); @@ -1891,22 +1890,20 @@ static int ssl_tls13_parse_new_session_ticket_extensions( { unsigned int extension_type; size_t extension_data_len; - const unsigned char *extension_data_end; - ((void) extension_data_end); MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 4 ); extension_type = MBEDTLS_GET_UINT16_BE( p, 0 ); extension_data_len = MBEDTLS_GET_UINT16_BE( p, 2 ); p += 4; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, extension_data_len ); - extension_data_end = p + extension_data_len; switch( extension_type ) { case MBEDTLS_TLS_EXT_EARLY_DATA: MBEDTLS_SSL_DEBUG_MSG( 4, ( "early_data extension received" ) ); break; + default: break; } @@ -2001,11 +1998,11 @@ static int ssl_tls13_parse_new_session_ticket( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_BUF( 3, "ticket->extension", p, extensions_len ); - ret = ssl_tls13_parse_new_session_ticket_extensions( ssl, p, end ); + ret = ssl_tls13_parse_new_session_ticket_exts( ssl, p, end ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, - "ssl_tls13_parse_new_session_ticket_extensions", + "ssl_tls13_parse_new_session_ticket_exts", ret ); return( ret ); } From cb3b1396f35cba60dfc969419fd61eed87c37f0f Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 12 Jul 2022 06:09:38 +0000 Subject: [PATCH 09/16] move resume psk ticket computation to end Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 73 +++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 32 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index b61626d07..586440a77 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1925,52 +1925,49 @@ static int ssl_tls13_parse_new_session_ticket_exts( mbedtls_ssl_context *ssl, */ static int ssl_tls13_parse_new_session_ticket( mbedtls_ssl_context *ssl, unsigned char *buf, - unsigned char *end ) + unsigned char *end, + unsigned char **ticket_nonce, + size_t *ticket_nonce_len ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *p = buf; mbedtls_ssl_session *session = ssl->session; - size_t ticket_nonce_len; - unsigned char *ticket_nonce; size_t ticket_len; unsigned char *ticket; size_t extensions_len; - const mbedtls_ssl_ciphersuite_t *ciphersuite_info; - psa_algorithm_t psa_hash_alg; - int hash_length; + *ticket_nonce = NULL; + *ticket_nonce_len = 0; /* * ticket_lifetime 4 bytes * ticket_age_add 4 bytes * ticket_nonce >=1 byte - * ticket >=2 bytes - * extensions >=2 bytes */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 13); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 9 ); session->ticket_lifetime = MBEDTLS_GET_UINT32_BE( p, 0 ); - p += 4; MBEDTLS_SSL_DEBUG_MSG( 3, ( "ticket->lifetime: %u", ( unsigned int )session->ticket_lifetime ) ); - session->ticket_age_add = MBEDTLS_GET_UINT32_BE( p, 0 ); - p += 4; + session->ticket_age_add = MBEDTLS_GET_UINT32_BE( p, 4 ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "ticket->ticket_age_add: %u", ( unsigned int )session->ticket_age_add ) ); - ticket_nonce_len = *p++; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, ticket_nonce_len ); - ticket_nonce = p; - MBEDTLS_SSL_DEBUG_BUF( 3, "ticket_nonce:", ticket_nonce, ticket_nonce_len ); - p += ticket_nonce_len; + *ticket_nonce_len = p[8]; + p += 9; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, *ticket_nonce_len ); + *ticket_nonce = p; + MBEDTLS_SSL_DEBUG_BUF( 3, "ticket_nonce:", *ticket_nonce, *ticket_nonce_len ); + p += *ticket_nonce_len; /* Ticket */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); ticket_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, ticket_len ); - MBEDTLS_SSL_DEBUG_BUF( 3, "received ticket", p, ticket_len ) ; /* Check if we previously received a ticket already. */ @@ -1992,6 +1989,7 @@ static int ssl_tls13_parse_new_session_ticket( mbedtls_ssl_context *ssl, session->ticket_len = ticket_len; MBEDTLS_SSL_DEBUG_BUF( 4, "stored ticket", ticket, ticket_len ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, extensions_len ); @@ -2008,6 +2006,24 @@ static int ssl_tls13_parse_new_session_ticket( mbedtls_ssl_context *ssl, } p += extensions_len; +#if defined(MBEDTLS_HAVE_TIME) + /* Store ticket creation time */ + session->ticket_received = time( NULL ); +#endif + + return( 0 ); +} + +static int ssl_tls13_postprocess_new_session_ticket( mbedtls_ssl_context *ssl, + unsigned char *ticket_nonce, + size_t ticket_nonce_len ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + mbedtls_ssl_session *session = ssl->session; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info; + psa_algorithm_t psa_hash_alg; + int hash_length; + /* Compute PSK based on received nonce and resumption_master_secret * in the following style: * @@ -2059,16 +2075,6 @@ static int ssl_tls13_parse_new_session_ticket( mbedtls_ssl_context *ssl, session->key, session->key_len ); -#if defined(MBEDTLS_HAVE_TIME) - /* Store ticket creation time */ - session->ticket_received = time( NULL ); -#endif - - return( 0 ); -} - -static int ssl_tls13_postprocess_new_session_ticket( mbedtls_ssl_context *ssl ) -{ mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HANDSHAKE_OVER ); return( 0 ); } @@ -2081,6 +2087,8 @@ static int ssl_tls13_process_new_session_ticket( mbedtls_ssl_context *ssl ) int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *buf; size_t buf_len; + unsigned char *ticket_nonce; + size_t ticket_nonce_len; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse new session ticket" ) ); @@ -2088,11 +2096,12 @@ static int ssl_tls13_process_new_session_ticket( mbedtls_ssl_context *ssl ) ssl, MBEDTLS_SSL_HS_NEW_SESSION_TICKET, &buf, &buf_len ) ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_new_session_ticket( ssl, - buf, - buf + buf_len ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_new_session_ticket( + ssl, buf, buf + buf_len, + &ticket_nonce, &ticket_nonce_len ) ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_new_session_ticket( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_new_session_ticket( + ssl, ticket_nonce, ticket_nonce_len ) ); cleanup: From 4e6c42a5339d3843896a82337cd4bb7fb15c630c Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 13 Jul 2022 11:16:51 +0800 Subject: [PATCH 10/16] fix various issues - wrong typo - unnecessary comments/debug code - wrong location Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 586440a77..0b243f491 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1947,12 +1947,12 @@ static int ssl_tls13_parse_new_session_ticket( mbedtls_ssl_context *ssl, session->ticket_lifetime = MBEDTLS_GET_UINT32_BE( p, 0 ); MBEDTLS_SSL_DEBUG_MSG( 3, - ( "ticket->lifetime: %u", + ( "ticket_lifetime: %u", ( unsigned int )session->ticket_lifetime ) ); session->ticket_age_add = MBEDTLS_GET_UINT32_BE( p, 4 ); MBEDTLS_SSL_DEBUG_MSG( 3, - ( "ticket->ticket_age_add: %u", + ( "ticket_age_add: %u", ( unsigned int )session->ticket_age_add ) ); *ticket_nonce_len = p[8]; @@ -1987,16 +1987,15 @@ static int ssl_tls13_parse_new_session_ticket( mbedtls_ssl_context *ssl, p += ticket_len; session->ticket = ticket; session->ticket_len = ticket_len; - MBEDTLS_SSL_DEBUG_BUF( 4, "stored ticket", ticket, ticket_len ); MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, extensions_len ); - MBEDTLS_SSL_DEBUG_BUF( 3, "ticket->extension", p, extensions_len ); + MBEDTLS_SSL_DEBUG_BUF( 3, "ticket extension", p, extensions_len ); - ret = ssl_tls13_parse_new_session_ticket_exts( ssl, p, end ); + ret = ssl_tls13_parse_new_session_ticket_exts( ssl, p, p + extensions_len ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, @@ -2004,12 +2003,6 @@ static int ssl_tls13_parse_new_session_ticket( mbedtls_ssl_context *ssl, ret ); return( ret ); } - p += extensions_len; - -#if defined(MBEDTLS_HAVE_TIME) - /* Store ticket creation time */ - session->ticket_received = time( NULL ); -#endif return( 0 ); } @@ -2024,12 +2017,11 @@ static int ssl_tls13_postprocess_new_session_ticket( mbedtls_ssl_context *ssl, psa_algorithm_t psa_hash_alg; int hash_length; - /* Compute PSK based on received nonce and resumption_master_secret - * in the following style: - * - * HKDF-Expand-Label( resumption_master_secret, - * "resumption", ticket_nonce, Hash.length ) - */ +#if defined(MBEDTLS_HAVE_TIME) + /* Store ticket creation time */ + session->ticket_received = time( NULL ); +#endif + ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( session->ciphersuite ); if( ciphersuite_info == NULL ) { @@ -2046,7 +2038,7 @@ static int ssl_tls13_postprocess_new_session_ticket( mbedtls_ssl_context *ssl, session->app_secrets.resumption_master_secret, hash_length ); - /* Computer resumption key + /* Compute resumption key * * HKDF-Expand-Label( resumption_master_secret, * "resumption", ticket_nonce, Hash.length ) @@ -2075,7 +2067,6 @@ static int ssl_tls13_postprocess_new_session_ticket( mbedtls_ssl_context *ssl, session->key, session->key_len ); - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HANDSHAKE_OVER ); return( 0 ); } @@ -2103,6 +2094,8 @@ static int ssl_tls13_process_new_session_ticket( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_new_session_ticket( ssl, ticket_nonce, ticket_nonce_len ) ); + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HANDSHAKE_OVER ); + cleanup: MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse new session ticket" ) ); From a0446a03448d2d1cbeba7b3d2f5eaa6e919155a0 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 13 Jul 2022 11:22:55 +0800 Subject: [PATCH 11/16] Add check_return flag Signed-off-by: Jerry Yu --- library/ssl_msg.c | 3 +++ library/ssl_tls13_client.c | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 9c207948d..dbef29b3f 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -5291,6 +5291,7 @@ static int ssl_check_ctr_renegotiate( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_TLS1_3) #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_check_new_session_ticket( mbedtls_ssl_context *ssl ) { @@ -5310,6 +5311,7 @@ static int ssl_tls13_check_new_session_ticket( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_handle_hs_message_post_handshake( mbedtls_ssl_context *ssl ) { @@ -5339,6 +5341,7 @@ static int ssl_tls13_handle_hs_message_post_handshake( mbedtls_ssl_context *ssl * and having a helper function allows to distinguish between TLS <= 1.2 and * TLS 1.3 in the future without bloating the logic of mbedtls_ssl_read(). */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls12_handle_hs_message_post_handshake( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 0b243f491..985c9af33 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1878,6 +1878,7 @@ static int ssl_tls13_handshake_wrapup( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_SESSION_TICKETS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_new_session_ticket_exts( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -1923,6 +1924,7 @@ static int ssl_tls13_parse_new_session_ticket_exts( mbedtls_ssl_context *ssl, * } NewSessionTicket; * */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_new_session_ticket( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -2007,6 +2009,7 @@ static int ssl_tls13_parse_new_session_ticket( mbedtls_ssl_context *ssl, return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_postprocess_new_session_ticket( mbedtls_ssl_context *ssl, unsigned char *ticket_nonce, size_t ticket_nonce_len ) @@ -2073,6 +2076,7 @@ static int ssl_tls13_postprocess_new_session_ticket( mbedtls_ssl_context *ssl, /* * Handler for MBEDTLS_SSL_NEW_SESSION_TICKET */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_process_new_session_ticket( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; From 08aed4def9ae0fb2f03c5c5fa7a822f4ba065ea0 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 20 Jul 2022 10:36:12 +0800 Subject: [PATCH 12/16] fix comments and time_t type issues Signed-off-by: Jerry Yu --- include/mbedtls/check_config.h | 5 +++++ include/mbedtls/mbedtls_config.h | 2 +- include/mbedtls/ssl.h | 2 +- library/ssl_tls13_client.c | 6 ++++-- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 5fe984984..b7ad7e98d 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -843,6 +843,11 @@ #error "MBEDTLS_SSL_TICKET_C defined, but not all prerequisites" #endif +#if defined(MBEDTLS_SSL_TICKET_NONCE_LENGTH) && \ + MBEDTLS_SSL_TICKET_NONCE_LENGTH >= 256 +#error "MBEDTLS_SSL_TICKET_NONCE_LENGTH must be less than 256" +#endif + #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && \ !defined(MBEDTLS_X509_CRT_PARSE_C) #error "MBEDTLS_SSL_SERVER_NAME_INDICATION defined, but not all prerequisites" diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index ed673e7d7..6d50c8d4e 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -1546,7 +1546,7 @@ * * Size in bytes of a ticket nonce. This is not used in TLS 1.2. * - * This must be smaller or equal to 255. + * This must be less than 256. */ #define MBEDTLS_SSL_TICKET_NONCE_LENGTH 32 diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 9703fcb16..ca727f983 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1185,7 +1185,7 @@ struct mbedtls_ssl_session #endif /* MBEDTLS_SHA256_C */ #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_CLI_C) - time_t MBEDTLS_PRIVATE(ticket_received); /*!< time ticket was received */ + mbedtls_time_t MBEDTLS_PRIVATE(ticket_received); /*!< time ticket was received */ #endif /* MBEDTLS_HAVE_TIME && MBEDTLS_SSL_CLI_C */ #endif /* MBEDTLS_SSL_PROTO_TLS1_3 && MBEDTLS_SSL_SESSION_TICKETS */ diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 985c9af33..93f58e052 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1915,6 +1915,8 @@ static int ssl_tls13_parse_new_session_ticket_exts( mbedtls_ssl_context *ssl, } /* + * From RFC8446, page 74 + * * struct { * uint32 ticket_lifetime; * uint32 ticket_age_add; @@ -1943,7 +1945,7 @@ static int ssl_tls13_parse_new_session_ticket( mbedtls_ssl_context *ssl, /* * ticket_lifetime 4 bytes * ticket_age_add 4 bytes - * ticket_nonce >=1 byte + * ticket_nonce_len 1 byte */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 9 ); @@ -2022,7 +2024,7 @@ static int ssl_tls13_postprocess_new_session_ticket( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_HAVE_TIME) /* Store ticket creation time */ - session->ticket_received = time( NULL ); + session->ticket_received = mbedtls_time( NULL ); #endif ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( session->ciphersuite ); From b14413804af9d3dfcbf569c95bc058848f01300c Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 20 Jul 2022 10:38:27 +0800 Subject: [PATCH 13/16] Remove ticket_flags It should be added later. Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 1 - library/ssl_misc.h | 4 ---- 2 files changed, 5 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index ca727f983..1a5cfca95 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1174,7 +1174,6 @@ struct mbedtls_ssl_session #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS) - uint8_t MBEDTLS_PRIVATE(ticket_flags); /*!< Ticket flags */ uint32_t MBEDTLS_PRIVATE(ticket_age_add); /*!< Randomly generated value used to obscure the age of the ticket */ uint8_t MBEDTLS_PRIVATE(key_len); /*!< PSK key length */ diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 777d44b88..39a47cac7 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -101,10 +101,6 @@ #define MBEDTLS_SSL_EXT_SIG_ALG_CERT ( 1 << 20 ) #define MBEDTLS_SSL_EXT_KEY_SHARE ( 1 << 21 ) -#define MBEDTLS_SSL_TICKET_FLAG_ALLOW_EARLY_DATA ( 1 << 0 ) -#define MBEDTLS_SSL_TICKET_FLAG_ALLOW_DHE_RESUMPTION ( 1 << 1 ) -#define MBEDTLS_SSL_TICKET_FLAG_ALLOW_PSK_RESUMPTION ( 1 << 2 ) - /* * Helper macros for function call with return check. */ From 0a430c8aaf67c9eb91a26986c2f851216e549183 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 20 Jul 2022 11:02:48 +0800 Subject: [PATCH 14/16] Rename resumption_key and the hardcode len `resumption_key` is better name. Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 16 +++++++++------- library/ssl_tls13_client.c | 8 ++++---- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 1a5cfca95..e665ec1b7 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -335,6 +335,13 @@ #define MBEDTLS_SSL_SRV_CIPHERSUITE_ORDER_CLIENT 1 #define MBEDTLS_SSL_SRV_CIPHERSUITE_ORDER_SERVER 0 +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS) +#if defined(MBEDTLS_SHA384_C) +#define MBEDTLS_SSL_TLS1_3_TICKET_RESUMPTION_KEY_LEN 48 +#elif defined(MBEDTLS_SHA256_C) +#define MBEDTLS_SSL_TLS1_3_TICKET_RESUMPTION_KEY_LEN 32 +#endif /* MBEDTLS_SHA256_C */ +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 && MBEDTLS_SSL_SESSION_TICKETS */ /* * Default range for DTLS retransmission timer value, in milliseconds. * RFC 6347 4.2.4.1 says from 1 second to 60 seconds. @@ -1175,13 +1182,8 @@ struct mbedtls_ssl_session #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS) uint32_t MBEDTLS_PRIVATE(ticket_age_add); /*!< Randomly generated value used to obscure the age of the ticket */ - uint8_t MBEDTLS_PRIVATE(key_len); /*!< PSK key length */ - -#if defined(MBEDTLS_SHA384_C) - unsigned char MBEDTLS_PRIVATE(key)[48]; /*!< key (48 byte) */ -#elif defined(MBEDTLS_SHA256_C) - unsigned char MBEDTLS_PRIVATE(key)[32]; /*!< key (32 byte) */ -#endif /* MBEDTLS_SHA256_C */ + uint8_t MBEDTLS_PRIVATE(resumption_key_len); /*!< resumption_key length */ + unsigned char MBEDTLS_PRIVATE(resumption_key)[MBEDTLS_SSL_TLS1_3_TICKET_RESUMPTION_KEY_LEN]; #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_CLI_C) mbedtls_time_t MBEDTLS_PRIVATE(ticket_received); /*!< time ticket was received */ diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 93f58e052..672cb0f53 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2055,7 +2055,7 @@ static int ssl_tls13_postprocess_new_session_ticket( mbedtls_ssl_context *ssl, MBEDTLS_SSL_TLS1_3_LBL_WITH_LEN( resumption ), ticket_nonce, ticket_nonce_len, - session->key, + session->resumption_key, hash_length ); if( ret != 0 ) @@ -2066,11 +2066,11 @@ static int ssl_tls13_postprocess_new_session_ticket( mbedtls_ssl_context *ssl, return( ret ); } - session->key_len = hash_length; + session->resumption_key_len = hash_length; MBEDTLS_SSL_DEBUG_BUF( 3, "Ticket-resumed PSK", - session->key, - session->key_len ); + session->resumption_key, + session->resumption_key_len ); return( 0 ); } From 9750f813a76635694563fa98af3d259d3018f210 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 20 Jul 2022 11:04:50 +0800 Subject: [PATCH 15/16] Rename MBEDTLS_SSL_TICKET_NONCE_LENGTH Signed-off-by: Jerry Yu --- include/mbedtls/check_config.h | 6 +++--- include/mbedtls/mbedtls_config.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index b7ad7e98d..cd17f9d02 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -843,9 +843,9 @@ #error "MBEDTLS_SSL_TICKET_C defined, but not all prerequisites" #endif -#if defined(MBEDTLS_SSL_TICKET_NONCE_LENGTH) && \ - MBEDTLS_SSL_TICKET_NONCE_LENGTH >= 256 -#error "MBEDTLS_SSL_TICKET_NONCE_LENGTH must be less than 256" +#if defined(MBEDTLS_SSL_TLS1_3_TICKET_NONCE_LENGTH) && \ + MBEDTLS_SSL_TLS1_3_TICKET_NONCE_LENGTH >= 256 +#error "MBEDTLS_SSL_TLS1_3_TICKET_NONCE_LENGTH must be less than 256" #endif #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && \ diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 6d50c8d4e..2bf24a0f6 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -1542,13 +1542,13 @@ //#define MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE /** - * \def MBEDTLS_SSL_TICKET_NONCE_LENGTH + * \def MBEDTLS_SSL_TLS1_3_TICKET_NONCE_LENGTH * * Size in bytes of a ticket nonce. This is not used in TLS 1.2. * * This must be less than 256. */ -#define MBEDTLS_SSL_TICKET_NONCE_LENGTH 32 +#define MBEDTLS_SSL_TLS1_3_TICKET_NONCE_LENGTH 32 /** * \def MBEDTLS_SSL_PROTO_DTLS From 3afdf36de704181cd12390363e24924e2f73b26f Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 20 Jul 2022 17:34:14 +0800 Subject: [PATCH 16/16] Add hash length check Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 672cb0f53..183b6ee06 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2036,8 +2036,12 @@ static int ssl_tls13_postprocess_new_session_ticket( mbedtls_ssl_context *ssl, psa_hash_alg = mbedtls_psa_translate_md( ciphersuite_info->mac ); hash_length = PSA_HASH_LENGTH( psa_hash_alg ); - if( hash_length == -1 ) + if( hash_length == -1 || + ( size_t )hash_length > sizeof( session->resumption_key ) ) + { return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + MBEDTLS_SSL_DEBUG_BUF( 3, "resumption_master_secret", session->app_secrets.resumption_master_secret,