From db8c5faed7ea7770f59512e8a30802fa583af243 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 3 Aug 2022 12:10:13 +0800 Subject: [PATCH 01/22] Add getting session ticket for client - Move ssl_get_psk_to_offer to `ssl_tls13_client.c` - Rename to `ssl_tls13_get_psk_to_offer` - Add session ticket parser Signed-off-by: Jerry Yu --- library/ssl_misc.h | 10 ----- library/ssl_tls13_client.c | 83 +++++++++++++++++++++++++++++++------ library/ssl_tls13_generic.c | 37 ----------------- 3 files changed, 70 insertions(+), 60 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 4842135bf..c2ded59b5 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2457,16 +2457,6 @@ int mbedtls_ssl_check_dtls_clihlo_cookie( #endif #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) -/* Check if we have any PSK to offer, returns 0 if PSK is available. - * Assign the psk and ticket if pointers are present. - */ -MBEDTLS_CHECK_RETURN_CRITICAL -int mbedtls_ssl_get_psk_to_offer( - const mbedtls_ssl_context *ssl, - int *psk_type, - const unsigned char **psk, size_t *psk_len, - const unsigned char **psk_identity, size_t *psk_identity_len ); - /** * \brief Given an SSL context and its associated configuration, write the TLS * 1.3 specific Pre-Shared key extension. diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 505f8dda8..6d87cffc1 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -664,10 +664,68 @@ static int ssl_tls13_write_psk_key_exchange_modes_ext( mbedtls_ssl_context *ssl, ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_PSK_KEY_EXCHANGE_MODES; return ( 0 ); } -#endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ + +/* Check if we have any PSK to offer, returns 0 if PSK is available. + * Assign the psk and ticket if pointers are present. + */ +MBEDTLS_CHECK_RETURN_CRITICAL +static int ssl_tls13_get_psk_to_offer( + const mbedtls_ssl_context *ssl, + int *psk_type, + const unsigned char **psk, size_t *psk_len, + const unsigned char **psk_identity, size_t *psk_identity_len ) +{ + if( psk_type == NULL || + psk == NULL || psk_len == NULL || + psk_identity == NULL || psk_identity_len == NULL ) + { + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + } + + *psk = NULL; + *psk_len = 0; + *psk_identity = NULL; + *psk_identity_len = 0; + +#if defined(MBEDTLS_SSL_SESSION_TICKETS) + /* Check if a ticket has been configured. */ + if( ssl->session_negotiate != NULL && + ssl->session_negotiate->ticket != NULL ) + { +#if defined(MBEDTLS_HAVE_TIME) + mbedtls_time_t now = mbedtls_time( NULL ); + + if( ( ssl->session_negotiate->ticket_received <= now && + now - ssl->session_negotiate->ticket_received < 7 * 86400 * 1000 ) ) + { + *psk_type = MBEDTLS_SSL_TLS1_3_PSK_RESUMPTION; + *psk = ssl->session_negotiate->resumption_key; + *psk_len = ssl->session_negotiate->resumption_key_len; + *psk_identity = ssl->session_negotiate->ticket; + *psk_identity_len = ssl->session_negotiate->ticket_len; + return( 0 ); + } +#endif /* MBEDTLS_HAVE_TIME */ + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ticket expired" ) ); + } +#endif + + /* Check if an external PSK has been configured. */ + if( ssl->conf->psk != NULL ) + { + *psk_type = MBEDTLS_SSL_TLS1_3_PSK_EXTERNAL; + *psk = ssl->conf->psk; + *psk_len = ssl->conf->psk_len; + *psk_identity = ssl->conf->psk_identity; + *psk_identity_len = ssl->conf->psk_identity_len; + return( 0 ); + } + + return( 1 ); +} /* - * mbedtls_ssl_tls13_write_pre_shared_key_ext() structure: + * mbedtls_ssl_tls13_write_identities_of_pre_shared_key_ext() structure: * * struct { * opaque identity<1..2^16-1>; @@ -689,9 +747,6 @@ static int ssl_tls13_write_psk_key_exchange_modes_ext( mbedtls_ssl_context *ssl, * } PreSharedKeyExtension; * */ - -#if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) - int mbedtls_ssl_tls13_write_identities_of_pre_shared_key_ext( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -725,9 +780,8 @@ int mbedtls_ssl_tls13_write_identities_of_pre_shared_key_ext( * configured, offer that. * - Otherwise, skip the PSK extension. */ - - if( mbedtls_ssl_get_psk_to_offer( ssl, &psk_type, &psk, &psk_len, - &psk_identity, &psk_identity_len ) != 0 ) + if( ssl_tls13_get_psk_to_offer( ssl, &psk_type, &psk, &psk_len, + &psk_identity, &psk_identity_len ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "skip pre_shared_key extensions" ) ); return( 0 ); @@ -831,8 +885,8 @@ int mbedtls_ssl_tls13_write_binders_of_pre_shared_key_ext( unsigned char transcript[MBEDTLS_MD_MAX_SIZE]; size_t transcript_len; - if( mbedtls_ssl_get_psk_to_offer( ssl, &psk_type, &psk, &psk_len, - &psk_identity, &psk_identity_len ) != 0 ) + if( ssl_tls13_get_psk_to_offer( ssl, &psk_type, &psk, &psk_len, + &psk_identity, &psk_identity_len ) != 0 ) { return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } @@ -1266,15 +1320,15 @@ static int ssl_tls13_parse_server_pre_shared_key_ext( mbedtls_ssl_context *ssl, size_t psk_len; const unsigned char *psk_identity; size_t psk_identity_len; - + int psk_type; /* Check which PSK we've offered. * * NOTE: Ultimately, we want to offer multiple PSKs, and in this * case, we need to iterate over them here. */ - if( mbedtls_ssl_get_psk_to_offer( ssl, NULL, &psk, &psk_len, - &psk_identity, &psk_identity_len ) != 0 ) + if( ssl_tls13_get_psk_to_offer( ssl, &psk_type, &psk, &psk_len, + &psk_identity, &psk_identity_len ) != 0 ) { /* If we haven't offered a PSK, the server must not send * a PSK identity extension. */ @@ -2405,6 +2459,9 @@ static int ssl_tls13_parse_new_session_ticket( mbedtls_ssl_context *ssl, return( ret ); } + /* session has been updated, allow export */ + session->exported = 0; + return( 0 ); } diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 2b9ac5c57..aa843a74e 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1505,41 +1505,4 @@ int mbedtls_ssl_tls13_generate_and_write_ecdh_key_exchange( } #endif /* MBEDTLS_ECDH_C */ -#if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) -/* Check if we have any PSK to offer, returns 0 if PSK is available. - * Assign the psk and ticket if pointers are present. - */ -int mbedtls_ssl_get_psk_to_offer( - const mbedtls_ssl_context *ssl, - int *psk_type, - const unsigned char **psk, size_t *psk_len, - const unsigned char **psk_identity, size_t *psk_identity_len ) -{ - int ptrs_present = 0; - - if( psk_type != NULL && psk != NULL && psk_len != NULL && - psk_identity != NULL && psk_identity_len != NULL ) - { - ptrs_present = 1; - } - - /* Check if an external PSK has been configured. */ - if( ssl->conf->psk != NULL ) - { - if( ptrs_present ) - { - *psk_type = MBEDTLS_SSL_TLS1_3_PSK_EXTERNAL; - *psk = ssl->conf->psk; - *psk_len = ssl->conf->psk_len; - *psk_identity = ssl->conf->psk_identity; - *psk_identity_len = ssl->conf->psk_identity_len; - } - - return( 0 ); - } - - return( 1 ); -} -#endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ - #endif /* MBEDTLS_SSL_TLS_C && MBEDTLS_SSL_PROTO_TLS1_3 */ From 49d63f8c36669d9dc67e644ea859a73e6ed55482 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 3 Aug 2022 12:28:08 +0800 Subject: [PATCH 02/22] Implement generate resumption master secret Signed-off-by: Jerry Yu --- library/ssl_tls13_keys.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 32a4f2ab3..d0809fc1c 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -1507,9 +1507,42 @@ cleanup: int mbedtls_ssl_tls13_generate_resumption_master_secret( mbedtls_ssl_context *ssl ) { + int ret = 0; + + mbedtls_md_type_t md_type; + + unsigned char transcript[MBEDTLS_MD_MAX_SIZE]; + size_t transcript_len; + + MBEDTLS_SSL_DEBUG_MSG( 2, + ( "=> mbedtls_ssl_tls13_generate_resumption_master_secret" ) ); + + md_type = ssl->handshake->ciphersuite_info->mac; + + ret = mbedtls_ssl_get_handshake_transcript( ssl, md_type, + transcript, sizeof( transcript ), + &transcript_len ); + if( ret != 0 ) + return( ret ); + + ret = mbedtls_ssl_tls13_derive_resumption_master_secret( + mbedtls_psa_translate_md( md_type ), + ssl->handshake->tls13_master_secrets.app, + transcript, transcript_len, + &ssl->session_negotiate->app_secrets ); + if( ret != 0 ) + return( ret ); + /* Erase master secrets */ mbedtls_platform_zeroize( &ssl->handshake->tls13_master_secrets, sizeof( ssl->handshake->tls13_master_secrets ) ); + + MBEDTLS_SSL_DEBUG_BUF( 4, "Resumption master secret", + ssl->session_negotiate->app_secrets.resumption_master_secret, + mbedtls_md_get_size( mbedtls_md_info_from_type( md_type ) ) ); + + MBEDTLS_SSL_DEBUG_MSG( 2, + ( "<= mbedtls_ssl_tls13_generate_resumption_master_secret" ) ); return( 0 ); } From e6527512d248fc26040c150e0852cefbfd57c3e3 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 3 Aug 2022 13:39:05 +0800 Subject: [PATCH 03/22] Add obfuscated_ticket_age write Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 6d87cffc1..56cd0608d 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -811,6 +811,25 @@ int mbedtls_ssl_tls13_write_identities_of_pre_shared_key_ext( break; } } + else +#if defined(MBEDTLS_SSL_SESSION_TICKETS) + if( psk_type == MBEDTLS_SSL_TLS1_3_PSK_RESUMPTION ) + { +#if defined(MBEDTLS_HAVE_TIME) + mbedtls_time_t now = mbedtls_time( NULL ); + obfuscated_ticket_age = + (uint32_t)( now - ssl->session_negotiate->ticket_received ) + + ssl->session_negotiate->ticket_age_add; +#endif + } + else +#endif /* MBEDTLS_SSL_SESSION_TICKETS */ + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "write_identities_of_pre_shared_key_ext: " + "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( ssl->session_negotiate->ciphersuite ); From 0203534c6437cf63f840152d8c20c87a44abf6f8 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 3 Aug 2022 12:43:06 +0800 Subject: [PATCH 04/22] Add session save after got new session ticket Signed-off-by: Jerry Yu --- programs/ssl/ssl_client2.c | 129 +++++++++++++++++++++++++------------ 1 file changed, 89 insertions(+), 40 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 253752186..c1f347b87 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -657,6 +657,57 @@ int report_cid_usage( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ +static int ssl_save_session_serialize( mbedtls_ssl_context *ssl, + unsigned char **session_data, + size_t *session_data_len ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + mbedtls_ssl_session exported_session; + + /* free any previously saved data */ + if( *session_data != NULL ) + { + mbedtls_platform_zeroize( *session_data, *session_data_len ); + mbedtls_free( *session_data ); + *session_data = NULL; + *session_data_len = 0; + } + + mbedtls_ssl_session_init( &exported_session ); + ret = mbedtls_ssl_get_session( ssl, &exported_session ); + if( ret != 0 ) + { + mbedtls_printf( + "failed\n ! mbedtls_ssl_get_session() returned -%#02x\n", + (unsigned) -ret ); + goto exit; + } + + /* get size of the buffer needed */ + mbedtls_ssl_session_save( &exported_session, NULL, 0, session_data_len ); + *session_data = mbedtls_calloc( 1, *session_data_len ); + if( *session_data == NULL ) + { + mbedtls_printf( " failed\n ! alloc %u bytes for session data\n", + (unsigned) *session_data_len ); + ret = MBEDTLS_ERR_SSL_ALLOC_FAILED; + goto exit; + } + + /* actually save session data */ + if( ( ret = mbedtls_ssl_session_save( &exported_session, + *session_data, *session_data_len, + session_data_len ) ) != 0 ) + { + mbedtls_printf( " failed\n ! mbedtls_ssl_session_saved returned -0x%04x\n\n", + (unsigned int) -ret ); + goto exit; + } + +exit: + mbedtls_ssl_session_free( &exported_session ); + return( ret ); +} int main( int argc, char *argv[] ) { int ret = 0, len, tail_len, i, written, frags, retry_left; @@ -2360,57 +2411,21 @@ int main( int argc, char *argv[] ) } } #endif /* MBEDTLS_SSL_DTLS_SRTP */ - if( opt.reconnect != 0 ) + if( opt.reconnect != 0 && ssl.tls_version != MBEDTLS_SSL_VERSION_TLS1_3 ) { mbedtls_printf(" . Saving session for reuse..." ); fflush( stdout ); if( opt.reco_mode == 1 ) { - mbedtls_ssl_session exported_session; - - /* free any previously saved data */ - if( session_data != NULL ) + if( ( ret = ssl_save_session_serialize( &ssl, + &session_data, &session_data_len ) ) != 0 ) { - mbedtls_platform_zeroize( session_data, session_data_len ); - mbedtls_free( session_data ); - session_data = NULL; - } - - mbedtls_ssl_session_init( &exported_session ); - ret = mbedtls_ssl_get_session( &ssl, &exported_session ); - if( ret != 0 ) - { - mbedtls_printf( - "failed\n ! mbedtls_ssl_get_session() returned -%#02x\n", - (unsigned) -ret ); - goto exit; - } - - /* get size of the buffer needed */ - mbedtls_ssl_session_save( &exported_session, NULL, 0, &session_data_len ); - session_data = mbedtls_calloc( 1, session_data_len ); - if( session_data == NULL ) - { - mbedtls_printf( " failed\n ! alloc %u bytes for session data\n", - (unsigned) session_data_len ); - mbedtls_ssl_session_free( &exported_session ); - ret = MBEDTLS_ERR_SSL_ALLOC_FAILED; - goto exit; - } - - /* actually save session data */ - if( ( ret = mbedtls_ssl_session_save( &exported_session, - session_data, session_data_len, - &session_data_len ) ) != 0 ) - { - mbedtls_printf( " failed\n ! mbedtls_ssl_session_saved returned -0x%04x\n\n", + mbedtls_printf( " failed\n ! ssl_save_session_serialize returned -0x%04x\n\n", (unsigned int) -ret ); - mbedtls_ssl_session_free( &exported_session ); goto exit; } - mbedtls_ssl_session_free( &exported_session ); } else { @@ -2700,6 +2715,40 @@ send_request: /* We were waiting for application data but got * a NewSessionTicket instead. */ mbedtls_printf( " got new session ticket.\n" ); + if( opt.reconnect != 0 ) + { + mbedtls_printf(" . Saving session for reuse..." ); + fflush( stdout ); + + if( opt.reco_mode == 1 ) + { + if( ( ret = ssl_save_session_serialize( &ssl, + &session_data, &session_data_len ) ) != 0 ) + { + mbedtls_printf( " failed\n ! ssl_save_session_serialize returned -0x%04x\n\n", + (unsigned int) -ret ); + goto exit; + } + } + else + { + if( ( ret = mbedtls_ssl_get_session( &ssl, &saved_session ) ) != 0 ) + { + mbedtls_printf( " failed\n ! mbedtls_ssl_get_session returned -0x%x\n\n", + (unsigned int) -ret ); + goto exit; + } + } + + mbedtls_printf( " ok\n" ); + + if( opt.reco_mode == 1 ) + { + mbedtls_printf( " [ Saved %u bytes of session data]\n", + (unsigned) session_data_len ); + } + } + continue; #endif /* MBEDTLS_SSL_SESSION_TICKETS */ From e976492a11bb838a4230601c320fb5a7a76b6296 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 3 Aug 2022 14:34:24 +0800 Subject: [PATCH 05/22] Add session ticket tests for client Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 3 ++ tests/ssl-opt.sh | 81 ++++++++++++++++++++------------------ 2 files changed, 45 insertions(+), 39 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 56cd0608d..febf441c9 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1695,16 +1695,19 @@ static int ssl_tls13_postprocess_server_hello( mbedtls_ssl_context *ssl ) /* Only the pre_shared_key extension was received */ case MBEDTLS_SSL_EXT_PRE_SHARED_KEY: handshake->key_exchange_mode = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "key exchange mode: psk" ) ); break; /* Only the key_share extension was received */ case MBEDTLS_SSL_EXT_KEY_SHARE: handshake->key_exchange_mode = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "key exchange mode: ephemeral" ) ); break; /* Both the pre_shared_key and key_share extensions were received */ case ( MBEDTLS_SSL_EXT_PRE_SHARED_KEY | MBEDTLS_SSL_EXT_KEY_SHARE ): handshake->key_exchange_mode = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "key exchange mode: psk_ephemeral" ) ); break; /* Neither pre_shared_key nor key_share extension was received */ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index c4628b017..0e5a5daab 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11987,48 +11987,55 @@ run_test "TLS 1.3: Server side check - mbedtls with sni" \ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_SSL_SRV_C requires_config_enabled MBEDTLS_SSL_CLI_C -run_test "TLS 1.3, default suite, PSK" \ - "$P_SRV nbio=2 debug_level=5 force_version=tls13 psk=010203 psk_identity=0a0b0c tls13_kex_modes=psk" \ - "$P_CLI nbio=2 debug_level=5 force_version=tls13 psk=010203 psk_identity=0a0b0c tls13_kex_modes=psk" \ - 1 \ +run_test "TLS 1.3: PSK: default suite. m->m" \ + "$P_SRV debug_level=5 force_version=tls13 psk=6162636465666768696a6b6c6d6e6f70 psk_identity=Client_identity tls13_kex_modes=psk_all" \ + "$P_CLI debug_level=5 force_version=tls13 psk=6162636465666768696a6b6c6d6e6f70 psk_identity=Client_identity tls13_kex_modes=psk_all" \ + 0 \ -c "=> write client hello" \ -c "client hello, adding pre_shared_key extension, omitting PSK binder list" \ -c "client hello, adding psk_key_exchange_modes extension" \ -c "client hello, adding PSK binder list" \ - -c "<= write client hello" + -c "<= write client hello" \ + -s "found psk key exchange modes extension" \ + -s "found pre_shared_key extension" \ + -s "Found PSK_EPHEMERAL KEX MODE" \ + -s "Found PSK KEX MODE" \ + -s "Pre shared key found" requires_openssl_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, default suite, PSK - openssl" \ - "$O_NEXT_SRV -msg -debug -tls1_3 -psk_identity 0a0b0c -psk 010203 -allow_no_dhe_kex -nocert" \ - "$P_CLI debug_level=4 psk=010203 psk_identity=0a0b0c tls13_kex_modes=psk" \ - 1 \ +run_test "TLS 1.3: PSK: default suite. m->O" \ + "$O_NEXT_SRV -msg -debug -tls1_3 -psk_identity Client_identity -psk 6162636465666768696a6b6c6d6e6f70 -allow_no_dhe_kex -nocert" \ + "$P_CLI debug_level=4 psk=6162636465666768696a6b6c6d6e6f70 psk_identity=Client_identity tls13_kex_modes=psk_all force_version=tls13" \ + 0 \ -c "=> write client hello" \ -c "client hello, adding pre_shared_key extension, omitting PSK binder list" \ -c "client hello, adding psk_key_exchange_modes extension" \ -c "client hello, adding PSK binder list" \ -c "<= write client hello" -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_gnutls_tls1_3 requires_gnutls_next_no_ticket +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, default suite, PSK - gnutls" \ - "$G_NEXT_SRV -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+PSK:+CIPHER-ALL:%NO_TICKETS --pskhint=0a0b0c" \ - "$P_CLI debug_level=4 psk=010203 psk_identity=0a0b0c tls13_kex_modes=psk" \ - 1 \ +run_test "TLS 1.3: PSK: default suite. m->G" \ + "$G_NEXT_SRV -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+PSK:+CIPHER-ALL --pskpasswd data_files/passwd.psk" \ + "$P_CLI debug_level=4 psk=6162636465666768696a6b6c6d6e6f70 psk_identity=Client_identity tls13_kex_modes=all force_version=tls13" \ + 0 \ -c "=> write client hello" \ -c "client hello, adding pre_shared_key extension, omitting PSK binder list" \ -c "client hello, adding psk_key_exchange_modes extension" \ -c "client hello, adding PSK binder list" \ + -c "<= write client hello" \ -s "Parsing extension 'PSK Key Exchange Modes/45'" \ -s "Parsing extension 'Pre Shared Key/41'" \ - -c "<= write client hello" + -s "PSK authentication. Connected as 'Client_identity'" + for i in opt-testcases/*.sh do @@ -12650,13 +12657,13 @@ 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 reco_mode=1 reconnect=1" \ + "$O_NEXT_SRV -msg -tls1_3 -no_resume_ephemeral -no_cache --num_tickets 4" \ + "$P_CLI debug_level=1 reco_mode=1 reconnect=1" \ 0 \ -c "Protocol is TLSv1.3" \ - -c "MBEDTLS_SSL_NEW_SESSION_TICKET" \ -c "got new session ticket." \ -c "Saving session for reuse... ok" \ + -c "Reconnecting with saved session" \ -c "HTTP/1.0 200 ok" requires_gnutls_tls1_3 @@ -12665,27 +12672,15 @@ 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:+PSK --disable-client-cert" \ - "$P_CLI debug_level=4 reco_mode=1 reconnect=1" \ + "$G_NEXT_SRV -d 10 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+CIPHER-ALL:+PSK --disable-client-cert" \ + "$P_CLI debug_level=1 reco_mode=1 reconnect=1" \ 0 \ -c "Protocol is TLSv1.3" \ - -c "MBEDTLS_SSL_NEW_SESSION_TICKET" \ -c "got new session ticket." \ -c "Saving session for reuse... ok" \ - -c "HTTP/1.0 200 OK" - -requires_openssl_tls1_3 -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 -requires_config_enabled MBEDTLS_SSL_SESSION_TICKETS -requires_config_enabled MBEDTLS_SSL_SRV_C -requires_config_enabled MBEDTLS_DEBUG_C -run_test "TLS 1.3: NewSessionTicket: Basic check, O->m" \ - "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=1" \ - "$O_NEXT_CLI -msg -debug -tls1_3 -no_middlebox" \ - 0 \ - -s "=> write NewSessionTicket msg" \ - -s "server state: MBEDTLS_SSL_NEW_SESSION_TICKET" \ - -s "server state: MBEDTLS_SSL_NEW_SESSION_TICKET_FLUSH" + -c "Reconnecting with saved session" \ + -c "HTTP/1.0 200 OK" \ + -s "This is a resumed session" requires_gnutls_tls1_3 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 @@ -12694,12 +12689,16 @@ requires_config_enabled MBEDTLS_SSL_SRV_C requires_config_enabled MBEDTLS_DEBUG_C 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=1" \ - "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%DISABLE_TLS13_COMPAT_MODE -V" \ + "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%DISABLE_TLS13_COMPAT_MODE -V -r" \ 0 \ + -c "Connecting again- trying to resume previous session" \ + -c "NEW SESSION TICKET (4) was received" \ -s "=> write NewSessionTicket msg" \ -s "server state: MBEDTLS_SSL_NEW_SESSION_TICKET" \ -s "server state: MBEDTLS_SSL_NEW_SESSION_TICKET_FLUSH" \ - -c "NEW SESSION TICKET (4) was received" + -s "key exchange mode: ephemeral" \ + -s "key exchange mode: psk_ephemeral" \ + -s "found pre_shared_key extension" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_SSL_SESSION_TICKETS @@ -12711,13 +12710,17 @@ run_test "TLS 1.3: NewSessionTicket: Basic check, m->m" \ "$P_CLI debug_level=4 reco_mode=1 reconnect=1" \ 0 \ -c "Protocol is TLSv1.3" \ - -c "MBEDTLS_SSL_NEW_SESSION_TICKET" \ -c "got new session ticket." \ -c "Saving session for reuse... ok" \ + -c "Reconnecting with saved session" \ -c "HTTP/1.0 200 OK" \ -s "=> write NewSessionTicket msg" \ -s "server state: MBEDTLS_SSL_NEW_SESSION_TICKET" \ - -s "server state: MBEDTLS_SSL_NEW_SESSION_TICKET_FLUSH" + -s "server state: MBEDTLS_SSL_NEW_SESSION_TICKET_FLUSH" \ + -s "key exchange mode: ephemeral" \ + -s "key exchange mode: psk_ephemeral" \ + -s "found pre_shared_key extension" + requires_openssl_tls1_3 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 From 661dd943b6a61ef79d32cff08a96cc9c7e6f850b Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 3 Aug 2022 14:50:01 +0800 Subject: [PATCH 06/22] Add dummy server name extension paser Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index febf441c9..7b26bbbfb 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1895,7 +1895,12 @@ static int ssl_tls13_parse_encrypted_extensions( mbedtls_ssl_context *ssl, */ switch( extension_type ) { + case MBEDTLS_TLS_EXT_SERVERNAME: + MBEDTLS_SSL_DEBUG_MSG( 3, ( "found server_name extension" ) ); + /* The server_name extension should be an empty extension */ + + break; case MBEDTLS_TLS_EXT_SUPPORTED_GROUPS: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found extensions supported groups" ) ); break; From 89eb95a2218b6c64899f117cf09d9f0aee160ee4 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Sun, 21 Aug 2022 19:21:45 +0800 Subject: [PATCH 07/22] Add ticket age tolerance config option Signed-off-by: Jerry Yu --- include/mbedtls/mbedtls_config.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index e9487b28f..19efff7d9 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -1549,6 +1549,14 @@ */ //#define MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE +/** + * \def MBEDTLS_SSL_TLS1_3_TICKET_NONCE_LENGTH + * + * Time in seconds of max ticket lifetime. This is not used in TLS 1.2. + * + */ +#define MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE 6000 + /** * \def MBEDTLS_SSL_TLS1_3_TICKET_NONCE_LENGTH * From 95699e72f3fa8bc402b040192560fabf4dee2cd0 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Sun, 21 Aug 2022 19:22:23 +0800 Subject: [PATCH 08/22] Add session ticket identity check Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 141 +++++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index a5b414e97..02a476080 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -121,14 +121,155 @@ static int ssl_tls13_parse_key_exchange_modes_ext( mbedtls_ssl_context *ssl, #define SSL_TLS1_3_OFFERED_PSK_NOT_MATCH 1 #define SSL_TLS1_3_OFFERED_PSK_MATCH 0 + +#if defined(MBEDTLS_SSL_SESSION_TICKETS) + +MBEDTLS_CHECK_RETURN_CRITICAL +static int ssl_tls13_offered_psks_check_identity_match_ticket( + mbedtls_ssl_context *ssl, + mbedtls_ssl_session *session, + const unsigned char *identity, + size_t identity_len, + uint32_t obfuscated_ticket_age ) +{ + int ret; + unsigned char *ticket_buffer; + + ((void) obfuscated_ticket_age); + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> check_identity_match_ticket" ) ); + + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ticket length: %" MBEDTLS_PRINTF_SIZET + ". ticket_parse is %sconfigured. " + "ticket_write is %sconfigured.", + identity_len, + ssl->conf->f_ticket_parse == NULL ? "NOT " : "", + ssl->conf->f_ticket_write == NULL ? "NOT " : "" ) ); + + if( ssl->conf->f_ticket_parse == NULL || + identity_len == 0 ) + { + /* Ticket parser is not configured, Skip */ + return( 0 ); + } + + /* We create a copy of the encrypted ticket since decrypting + * it into the same buffer will wipe-out the original content. + * We do, however, need the original buffer for computing the + * psk binder value. + */ + ticket_buffer = mbedtls_calloc( 1, identity_len ); + if( ticket_buffer == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); + return ( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + } + memcpy( ticket_buffer, identity, identity_len ); + + if( ( ret = ssl->conf->f_ticket_parse( ssl->conf->p_ticket, + session, + ticket_buffer, identity_len ) ) != 0 ) + { + if( ret == MBEDTLS_ERR_SSL_INVALID_MAC ) + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ticket is not authentic" ) ); + else if( ret == MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED ) + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ticket is expired" ) ); + else + MBEDTLS_SSL_DEBUG_RET( 1, "ticket_parse", ret ); + } + + /* We delete the temporary buffer */ + mbedtls_free( ticket_buffer ); + + if( ret == 0 ) + { +#if defined(MBEDTLS_HAVE_TIME) + mbedtls_time_t now; + int64_t diff; +#endif + ret = SSL_TLS1_3_OFFERED_PSK_MATCH; +#if defined(MBEDTLS_HAVE_TIME) + now = mbedtls_time( NULL ); + + /* Check #1: + * Is the time when the ticket was issued later than now? + */ + if( now < session->start ) + { + MBEDTLS_SSL_DEBUG_MSG( + 3, ( "Ticket expired: now=%" MBEDTLS_PRINTF_LONGLONG + ", start=%" MBEDTLS_PRINTF_LONGLONG, + (long long)now, (long long)session->start ) ); + ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; + } + + /* Check #2: + * Is the ticket age for the selected PSK identity + * (computed by subtracting ticket_age_add from + * PskIdentity.obfuscated_ticket_age modulo 2^32 ) + * within a small tolerance of the time since the + * ticket was issued? + */ + diff = ( now - session->start ) - + ( obfuscated_ticket_age - session->ticket_age_add ); + + if( diff > MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "Ticket age outside tolerance window ( diff=%" + MBEDTLS_PRINTF_LONGLONG" )", + (long long)diff ) ); + ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; + } + +#endif /* MBEDTLS_HAVE_TIME */ + } + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= check_identity_match_ticket" ) ); + return( ret ); +} +#endif /* MBEDTLS_SSL_SESSION_TICKETS */ + MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_offered_psks_check_identity_match( mbedtls_ssl_context *ssl, const unsigned char *identity, size_t identity_len, + uint32_t obfuscated_ticket_age, + void *session, int *psk_type ) { + ((void) session); + ((void) obfuscated_ticket_age); *psk_type = MBEDTLS_SSL_TLS1_3_PSK_EXTERNAL; + + MBEDTLS_SSL_DEBUG_BUF( 4, "identity", identity, identity_len ); + ssl->handshake->resume = 0; + + + +#if defined(MBEDTLS_SSL_SESSION_TICKETS) + if( ssl_tls13_offered_psks_check_identity_match_ticket( + ssl, (mbedtls_ssl_session *)session, + identity, identity_len, + obfuscated_ticket_age ) == SSL_TLS1_3_OFFERED_PSK_MATCH ) + { + mbedtls_ssl_session *i_session=(mbedtls_ssl_session *)session; + ssl->handshake->resume = 1; + *psk_type = MBEDTLS_SSL_TLS1_3_PSK_RESUMPTION; + mbedtls_ssl_set_hs_psk( ssl, + i_session->resumption_key, + i_session->resumption_key_len ); + + MBEDTLS_SSL_DEBUG_BUF( 4, "Ticket-resumed PSK:", + i_session->resumption_key, + i_session->resumption_key_len ); + MBEDTLS_SSL_DEBUG_MSG( 4, ( "ticket: obfuscated_ticket_age: %u", + (unsigned)obfuscated_ticket_age ) ); + return( SSL_TLS1_3_OFFERED_PSK_MATCH ); + } +#endif /* MBEDTLS_SSL_SESSION_TICKETS */ + /* Check identity with external configured function */ if( ssl->conf->f_psk != NULL ) { From 8253486c4ffe58eaed7906f5a473749246b07f80 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 30 Aug 2022 10:42:33 +0800 Subject: [PATCH 09/22] Add session ticket support for server Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 86 +++++++++++++++++++++++++++++++++----- 1 file changed, 75 insertions(+), 11 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 02a476080..c139fb536 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -397,6 +397,7 @@ static int ssl_tls13_select_ciphersuite_for_psk( return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } +#if defined(MBEDTLS_SSL_SESSION_TICKETS) MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_select_ciphersuite_for_resumption( mbedtls_ssl_context *ssl, @@ -406,15 +407,51 @@ static int ssl_tls13_select_ciphersuite_for_resumption( uint16_t *selected_ciphersuite, const mbedtls_ssl_ciphersuite_t **selected_ciphersuite_info ) { - ((void) ssl); - ((void) session); - ((void) cipher_suites); - ((void) cipher_suites_end); + *selected_ciphersuite = 0; *selected_ciphersuite_info = NULL; - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + for( const unsigned char *p = cipher_suites; p < cipher_suites_end; p += 2 ) + { + uint16_t cipher_suite = MBEDTLS_GET_UINT16_BE( p, 0 ); + const mbedtls_ssl_ciphersuite_t *ciphersuite_info; + + if( cipher_suite != session->ciphersuite ) + continue; + + ciphersuite_info = ssl_tls13_validate_peer_ciphersuite( ssl, + cipher_suite ); + if( ciphersuite_info == NULL ) + continue; + + *selected_ciphersuite = session->ciphersuite; + *selected_ciphersuite_info = ciphersuite_info; + + return( 0 ); + + } + + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } +MBEDTLS_CHECK_RETURN_CRITICAL +static int ssl_tls13_session_copy( mbedtls_ssl_session *dst, + mbedtls_ssl_session *src ) +{ + dst->endpoint = src->endpoint; + dst->ciphersuite = src->ciphersuite; + dst->ticket_age_add = src->ticket_age_add; + dst->ticket_flags = src->ticket_flags; + dst->resumption_key_len = src->resumption_key_len; + if( src->resumption_key_len == 0 ) + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + memcpy( dst->resumption_key, src->resumption_key, src->resumption_key_len ); +#if defined(MBEDTLS_HAVE_TIME) + dst->start = src->start; +#endif + return( 0 ); +} +#endif /* MBEDTLS_SSL_SESSION_TICKETS */ + /* Parser for pre_shared_key extension in client hello * struct { * opaque identity<1..2^16-1>; @@ -484,17 +521,23 @@ static int ssl_tls13_parse_pre_shared_key_ext( mbedtls_ssl_context *ssl, { const unsigned char *identity; size_t identity_len; + uint32_t obfuscated_ticket_age; const unsigned char *binder; size_t binder_len; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; int psk_type; uint16_t cipher_suite; const mbedtls_ssl_ciphersuite_t *ciphersuite_info; +#if defined(MBEDTLS_SSL_SESSION_TICKETS) + mbedtls_ssl_session session; + memset( &session, 0, sizeof( session ) ); +#endif MBEDTLS_SSL_CHK_BUF_READ_PTR( p_identity_len, identities_end, 2 + 1 + 4 ); identity_len = MBEDTLS_GET_UINT16_BE( p_identity_len, 0 ); identity = p_identity_len + 2; MBEDTLS_SSL_CHK_BUF_READ_PTR( identity, identities_end, identity_len + 4 ); + obfuscated_ticket_age = MBEDTLS_GET_UINT32_BE( identity , identity_len ); p_identity_len += identity_len + 6; MBEDTLS_SSL_CHK_BUF_READ_PTR( p_binder_len, binders_end, 1 + 32 ); @@ -508,7 +551,8 @@ static int ssl_tls13_parse_pre_shared_key_ext( mbedtls_ssl_context *ssl, continue; ret = ssl_tls13_offered_psks_check_identity_match( - ssl, identity, identity_len, &psk_type ); + ssl, identity, identity_len, obfuscated_ticket_age, + &session, &psk_type ); if( ret != SSL_TLS1_3_OFFERED_PSK_MATCH ) continue; @@ -521,9 +565,13 @@ static int ssl_tls13_parse_pre_shared_key_ext( mbedtls_ssl_context *ssl, &cipher_suite, &ciphersuite_info ); break; case MBEDTLS_SSL_TLS1_3_PSK_RESUMPTION: +#if defined(MBEDTLS_SSL_SESSION_TICKETS) ret = ssl_tls13_select_ciphersuite_for_resumption( - ssl, ciphersuites, ciphersuites_end, NULL, + ssl, ciphersuites, ciphersuites_end, &session, &cipher_suite, &ciphersuite_info ); +#else + ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; +#endif break; default: return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); @@ -547,6 +595,9 @@ static int ssl_tls13_parse_pre_shared_key_ext( mbedtls_ssl_context *ssl, /* For security reasons, the handshake should be aborted when we * fail to validate a binder value. See RFC 8446 section 4.2.11.2 * and appendix E.6. */ +#if defined(MBEDTLS_SSL_SESSION_TICKETS) + mbedtls_ssl_session_free( &session ); +#endif MBEDTLS_SSL_DEBUG_MSG( 3, ( "Invalid binder." ) ); MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_offered_psks_check_binder_match" , ret ); @@ -559,11 +610,24 @@ static int ssl_tls13_parse_pre_shared_key_ext( mbedtls_ssl_context *ssl, matched_identity = identity_id; /* Update handshake parameters */ - ssl->session_negotiate->ciphersuite = cipher_suite; ssl->handshake->ciphersuite_info = ciphersuite_info; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "overwrite ciphersuite: %04x - %s", - cipher_suite, ciphersuite_info->name ) ); - + if( psk_type == MBEDTLS_SSL_TLS1_3_PSK_EXTERNAL ) + { + ssl->session_negotiate->ciphersuite = cipher_suite; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "overwrite ciphersuite: %04x - %s", + cipher_suite, + ciphersuite_info->name ) ); + } +#if defined(MBEDTLS_SSL_SESSION_TICKETS) + else + if( psk_type == MBEDTLS_SSL_TLS1_3_PSK_RESUMPTION ) + { + ret = ssl_tls13_session_copy(ssl->session_negotiate, &session ); + mbedtls_ssl_session_free( &session ); + if( ret != 0 ) + return( ret ); + } +#endif /* MBEDTLS_SSL_SESSION_TICKETS */ } if( p_identity_len != identities_end || p_binder_len != binders_end ) From fd310ebf2d99fc6ccbfb70dff5eca203c94dbd28 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 6 Sep 2022 09:16:35 +0800 Subject: [PATCH 10/22] fix coding style issues Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 2 +- library/ssl_tls13_server.c | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 7b26bbbfb..c3ee44d0b 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -689,7 +689,7 @@ static int ssl_tls13_get_psk_to_offer( #if defined(MBEDTLS_SSL_SESSION_TICKETS) /* Check if a ticket has been configured. */ - if( ssl->session_negotiate != NULL && + if( ssl->session_negotiate != NULL && ssl->session_negotiate->ticket != NULL ) { #if defined(MBEDTLS_HAVE_TIME) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index c139fb536..0818a8f18 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -132,7 +132,7 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( size_t identity_len, uint32_t obfuscated_ticket_age ) { - int ret; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *ticket_buffer; ((void) obfuscated_ticket_age); @@ -146,12 +146,9 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( ssl->conf->f_ticket_parse == NULL ? "NOT " : "", ssl->conf->f_ticket_write == NULL ? "NOT " : "" ) ); - if( ssl->conf->f_ticket_parse == NULL || - identity_len == 0 ) - { - /* Ticket parser is not configured, Skip */ + /* Ticket parser is not configured, Skip */ + if( ssl->conf->f_ticket_parse == NULL || identity_len == 0 ) return( 0 ); - } /* We create a copy of the encrypted ticket since decrypting * it into the same buffer will wipe-out the original content. From a02841bb8aff4e7ab58c88fcd5e5fab445c97c89 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 13 Sep 2022 10:59:21 +0800 Subject: [PATCH 11/22] revert changes on PSK tests Signed-off-by: Jerry Yu --- tests/ssl-opt.sh | 55 +++++++++++++++++++++--------------------------- 1 file changed, 24 insertions(+), 31 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 0e5a5daab..d601eea65 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11987,29 +11987,9 @@ run_test "TLS 1.3: Server side check - mbedtls with sni" \ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_SSL_SRV_C requires_config_enabled MBEDTLS_SSL_CLI_C -run_test "TLS 1.3: PSK: default suite. m->m" \ - "$P_SRV debug_level=5 force_version=tls13 psk=6162636465666768696a6b6c6d6e6f70 psk_identity=Client_identity tls13_kex_modes=psk_all" \ - "$P_CLI debug_level=5 force_version=tls13 psk=6162636465666768696a6b6c6d6e6f70 psk_identity=Client_identity tls13_kex_modes=psk_all" \ - 0 \ - -c "=> write client hello" \ - -c "client hello, adding pre_shared_key extension, omitting PSK binder list" \ - -c "client hello, adding psk_key_exchange_modes extension" \ - -c "client hello, adding PSK binder list" \ - -c "<= write client hello" \ - -s "found psk key exchange modes extension" \ - -s "found pre_shared_key extension" \ - -s "Found PSK_EPHEMERAL KEX MODE" \ - -s "Found PSK KEX MODE" \ - -s "Pre shared key found" - -requires_openssl_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: PSK: default suite. m->O" \ - "$O_NEXT_SRV -msg -debug -tls1_3 -psk_identity Client_identity -psk 6162636465666768696a6b6c6d6e6f70 -allow_no_dhe_kex -nocert" \ - "$P_CLI debug_level=4 psk=6162636465666768696a6b6c6d6e6f70 psk_identity=Client_identity tls13_kex_modes=psk_all force_version=tls13" \ +run_test "TLS 1.3, default suite, PSK" \ + "$P_SRV nbio=2 debug_level=5 force_version=tls13 psk=010203 psk_identity=0a0b0c tls13_kex_modes=psk" \ + "$P_CLI nbio=2 debug_level=5 force_version=tls13 psk=010203 psk_identity=0a0b0c tls13_kex_modes=psk" \ 0 \ -c "=> write client hello" \ -c "client hello, adding pre_shared_key extension, omitting PSK binder list" \ @@ -12017,25 +11997,38 @@ run_test "TLS 1.3: PSK: default suite. m->O" \ -c "client hello, adding PSK binder list" \ -c "<= write client hello" -requires_gnutls_tls1_3 -requires_gnutls_next_no_ticket +requires_openssl_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: PSK: default suite. m->G" \ - "$G_NEXT_SRV -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+PSK:+CIPHER-ALL --pskpasswd data_files/passwd.psk" \ - "$P_CLI debug_level=4 psk=6162636465666768696a6b6c6d6e6f70 psk_identity=Client_identity tls13_kex_modes=all force_version=tls13" \ +run_test "TLS 1.3, default suite, PSK - openssl" \ + "$O_NEXT_SRV -msg -debug -tls1_3 -psk_identity 0a0b0c -psk 010203 -allow_no_dhe_kex -nocert" \ + "$P_CLI debug_level=4 psk=010203 psk_identity=0a0b0c tls13_kex_modes=psk" \ 0 \ -c "=> write client hello" \ -c "client hello, adding pre_shared_key extension, omitting PSK binder list" \ -c "client hello, adding psk_key_exchange_modes extension" \ -c "client hello, adding PSK binder list" \ - -c "<= write client hello" \ + -c "<= write client hello" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_gnutls_tls1_3 +requires_gnutls_next_no_ticket +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, default suite, PSK - gnutls" \ + "$G_NEXT_SRV -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+PSK:+CIPHER-ALL:%NO_TICKETS --pskhint=0a0b0c" \ + "$P_CLI debug_level=4 psk=010203 psk_identity=0a0b0c tls13_kex_modes=psk" \ + 1 \ + -c "=> write client hello" \ + -c "client hello, adding pre_shared_key extension, omitting PSK binder list" \ + -c "client hello, adding psk_key_exchange_modes extension" \ + -c "client hello, adding PSK binder list" \ -s "Parsing extension 'PSK Key Exchange Modes/45'" \ -s "Parsing extension 'Pre Shared Key/41'" \ - -s "PSK authentication. Connected as 'Client_identity'" - + -c "<= write client hello" for i in opt-testcases/*.sh do From 6cf85a4bb08e627e17a2f1e8e76831a8261e4c68 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 13 Sep 2022 11:14:42 +0800 Subject: [PATCH 12/22] update document abourt maximum ticket_age_tolerance Signed-off-by: Jerry Yu --- include/mbedtls/mbedtls_config.h | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 19efff7d9..679f7eb86 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -1550,9 +1550,21 @@ //#define MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE /** - * \def MBEDTLS_SSL_TLS1_3_TICKET_NONCE_LENGTH + * \def MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE * - * Time in seconds of max ticket lifetime. This is not used in TLS 1.2. + * Maximum time difference in milliseconds tolerated between the age of a + * ticket from the server and client point of view. + * From the client point of view, the age of a ticket is the time difference + * between the time when the client proposes to the server to use the ticket + * (time of writing of the Pre-Shared Key Extension including the ticket) and + * the time the client received the ticket from the server. + * From the server point of view, the age of a ticket is the time difference + * between the time when the server receives a proposition from the client + * to use the ticket and the time when the ticket was created by the server. + * The server age is expected to be always greater than the client one and + * MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE defines the + * maximum difference tolerated for the server to accept the ticket. + * This is not used in TLS 1.2. * */ #define MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE 6000 From 466dda855356443a1225652df5d1ffad069c6bbf Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 13 Sep 2022 11:20:20 +0800 Subject: [PATCH 13/22] Rename resumption master secret compute function Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 4 ++-- library/ssl_tls13_keys.c | 7 +++---- library/ssl_tls13_keys.h | 3 +-- library/ssl_tls13_server.c | 4 ++-- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index c3ee44d0b..e92a10c0a 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2318,11 +2318,11 @@ static int ssl_tls13_write_client_finished( mbedtls_ssl_context *ssl ) if( ret != 0 ) return( ret ); - ret = mbedtls_ssl_tls13_generate_resumption_master_secret( ssl ); + ret = mbedtls_ssl_tls13_compute_resumption_master_secret( ssl ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, - "mbedtls_ssl_tls13_generate_resumption_master_secret ", ret ); + "mbedtls_ssl_tls13_compute_resumption_master_secret ", ret ); return ( ret ); } diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index d0809fc1c..504d89789 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -1504,8 +1504,7 @@ cleanup: return( ret ); } -int mbedtls_ssl_tls13_generate_resumption_master_secret( - mbedtls_ssl_context *ssl ) +int mbedtls_ssl_tls13_compute_resumption_master_secret( mbedtls_ssl_context *ssl ) { int ret = 0; @@ -1515,7 +1514,7 @@ int mbedtls_ssl_tls13_generate_resumption_master_secret( size_t transcript_len; MBEDTLS_SSL_DEBUG_MSG( 2, - ( "=> mbedtls_ssl_tls13_generate_resumption_master_secret" ) ); + ( "=> mbedtls_ssl_tls13_compute_resumption_master_secret" ) ); md_type = ssl->handshake->ciphersuite_info->mac; @@ -1542,7 +1541,7 @@ int mbedtls_ssl_tls13_generate_resumption_master_secret( mbedtls_md_get_size( mbedtls_md_info_from_type( md_type ) ) ); MBEDTLS_SSL_DEBUG_MSG( 2, - ( "<= mbedtls_ssl_tls13_generate_resumption_master_secret" ) ); + ( "<= mbedtls_ssl_tls13_compute_resumption_master_secret" ) ); return( 0 ); } diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index b1155fb2a..b80e02c97 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -636,8 +636,7 @@ int mbedtls_ssl_tls13_generate_application_keys( * \returns A negative error code on failure. */ MBEDTLS_CHECK_RETURN_CRITICAL -int mbedtls_ssl_tls13_generate_resumption_master_secret( - mbedtls_ssl_context *ssl ); +int mbedtls_ssl_tls13_compute_resumption_master_secret( mbedtls_ssl_context *ssl ); /** * \brief Calculate the verify_data value for the client or server TLS 1.3 diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 0818a8f18..080415202 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -2564,11 +2564,11 @@ static int ssl_tls13_process_client_finished( mbedtls_ssl_context *ssl ) if( ret != 0 ) return( ret ); - ret = mbedtls_ssl_tls13_generate_resumption_master_secret( ssl ); + ret = mbedtls_ssl_tls13_compute_resumption_master_secret( ssl ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, - "mbedtls_ssl_tls13_generate_resumption_master_secret ", ret ); + "mbedtls_ssl_tls13_compute_resumption_master_secret", ret ); } mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HANDSHAKE_WRAPUP ); From 46bffe0e821638908f76530e1570376d5d3eb5f1 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 13 Sep 2022 11:25:28 +0800 Subject: [PATCH 14/22] Refine rsumption master secret compute function Signed-off-by: Jerry Yu --- library/ssl_tls13_keys.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 504d89789..28090ebcb 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -1506,17 +1506,16 @@ cleanup: int mbedtls_ssl_tls13_compute_resumption_master_secret( mbedtls_ssl_context *ssl ) { - int ret = 0; - + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_md_type_t md_type; - - unsigned char transcript[MBEDTLS_MD_MAX_SIZE]; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; + unsigned char transcript[MBEDTLS_TLS1_3_MD_MAX_SIZE]; size_t transcript_len; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_tls13_compute_resumption_master_secret" ) ); - md_type = ssl->handshake->ciphersuite_info->mac; + md_type = handshake->ciphersuite_info->mac; ret = mbedtls_ssl_get_handshake_transcript( ssl, md_type, transcript, sizeof( transcript ), @@ -1526,20 +1525,21 @@ int mbedtls_ssl_tls13_compute_resumption_master_secret( mbedtls_ssl_context *ssl ret = mbedtls_ssl_tls13_derive_resumption_master_secret( mbedtls_psa_translate_md( md_type ), - ssl->handshake->tls13_master_secrets.app, + handshake->tls13_master_secrets.app, transcript, transcript_len, &ssl->session_negotiate->app_secrets ); if( ret != 0 ) return( ret ); /* Erase master secrets */ - mbedtls_platform_zeroize( &ssl->handshake->tls13_master_secrets, - sizeof( ssl->handshake->tls13_master_secrets ) ); + mbedtls_platform_zeroize( &handshake->tls13_master_secrets, + sizeof( handshake->tls13_master_secrets ) ); MBEDTLS_SSL_DEBUG_BUF( 4, "Resumption master secret", ssl->session_negotiate->app_secrets.resumption_master_secret, mbedtls_md_get_size( mbedtls_md_info_from_type( md_type ) ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_tls13_compute_resumption_master_secret" ) ); return( 0 ); From 8d4bbbae4f0b1039ae0ed90764f79961120e7249 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 13 Sep 2022 14:15:48 +0800 Subject: [PATCH 15/22] fix ticket age check issues - Ticket age and ticket age add, obfuscated age use different unit. Align the units to million seconds. - Add maximum ticket age check. Until now, ticket_lifetime is not recorded in server side. Check it with maximum ticket_lifetime. - Free session when error found. Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 108 ++++++++++++++++++++++--------------- 1 file changed, 66 insertions(+), 42 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 080415202..adedb6df5 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -127,13 +127,17 @@ static int ssl_tls13_parse_key_exchange_modes_ext( mbedtls_ssl_context *ssl, MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_offered_psks_check_identity_match_ticket( mbedtls_ssl_context *ssl, - mbedtls_ssl_session *session, const unsigned char *identity, size_t identity_len, - uint32_t obfuscated_ticket_age ) + uint32_t obfuscated_ticket_age, + mbedtls_ssl_session *session ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *ticket_buffer; +#if defined(MBEDTLS_HAVE_TIME) + mbedtls_time_t now; + uint64_t age_in_s, age_in_ms, client_age_in_ms; +#endif ((void) obfuscated_ticket_age); @@ -178,49 +182,70 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( /* We delete the temporary buffer */ mbedtls_free( ticket_buffer ); - if( ret == 0 ) + if( ret != 0 ) + goto exit; + + ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; +#if defined(MBEDTLS_HAVE_TIME) + now = mbedtls_time( NULL ); + + ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; + if( now < session->start ) { -#if defined(MBEDTLS_HAVE_TIME) - mbedtls_time_t now; - int64_t diff; -#endif - ret = SSL_TLS1_3_OFFERED_PSK_MATCH; -#if defined(MBEDTLS_HAVE_TIME) - now = mbedtls_time( NULL ); + MBEDTLS_SSL_DEBUG_MSG( + 3, ( "Ticket expired: now=%" MBEDTLS_PRINTF_LONGLONG + ", start=%" MBEDTLS_PRINTF_LONGLONG, + (long long)now, (long long)session->start ) ); + goto exit; + } - /* Check #1: - * Is the time when the ticket was issued later than now? - */ - if( now < session->start ) - { - MBEDTLS_SSL_DEBUG_MSG( - 3, ( "Ticket expired: now=%" MBEDTLS_PRINTF_LONGLONG - ", start=%" MBEDTLS_PRINTF_LONGLONG, - (long long)now, (long long)session->start ) ); - ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; - } + age_in_s = (uint64_t)( now - session->start ); - /* Check #2: - * Is the ticket age for the selected PSK identity - * (computed by subtracting ticket_age_add from - * PskIdentity.obfuscated_ticket_age modulo 2^32 ) - * within a small tolerance of the time since the - * ticket was issued? - */ - diff = ( now - session->start ) - - ( obfuscated_ticket_age - session->ticket_age_add ); + /* RFC 8446 section 4.6.1 + * + * Servers MUST NOT use any value greater than 604800 seconds (7 days). + * + * RFC 8446 section 4.2.11.1 + * + * Clients MUST NOT attempt to use tickets which have ages greater than + * the "ticket_lifetime" value which was provided with the ticket. + * + * For time being, the age MUST be less than 604800 seconds (7 days). + */ + if( age_in_s > 604800 ) + { + MBEDTLS_SSL_DEBUG_MSG( + 3, ( "Ticket expired: Ticket age exceed limitation ticket_age=%lu", + (long unsigned int)age_in_s ) ); + goto exit; + } - if( diff > MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE ) - { - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "Ticket age outside tolerance window ( diff=%" - MBEDTLS_PRINTF_LONGLONG" )", - (long long)diff ) ); - ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; - } + /* RFC 8446 section 4.2.10 + * + * For PSKs provisioned via NewSessionTicket, a server MUST validate that + * the ticket age for the selected PSK identity (computed by subtracting + * ticket_age_add from PskIdentity.obfuscated_ticket_age modulo 2^32) is + * within a small tolerance of the time since the ticket was issued. + */ + age_in_ms = age_in_s * 1000; + client_age_in_ms = obfuscated_ticket_age - session->ticket_age_add; + if( age_in_ms < client_age_in_ms || + ( age_in_ms - client_age_in_ms ) > MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE ) + { + MBEDTLS_SSL_DEBUG_MSG( + 3, ( "Ticket expired: Ticket age outside tolerance window " + "( diff=%d )", + (int)(age_in_ms - client_age_in_ms ) ) ); + goto exit; + } + + ret = 0; #endif /* MBEDTLS_HAVE_TIME */ - } + +exit: + if( ret != 0 ) + mbedtls_ssl_session_free( session ); MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= check_identity_match_ticket" ) ); return( ret ); @@ -247,9 +272,8 @@ static int ssl_tls13_offered_psks_check_identity_match( #if defined(MBEDTLS_SSL_SESSION_TICKETS) if( ssl_tls13_offered_psks_check_identity_match_ticket( - ssl, (mbedtls_ssl_session *)session, - identity, identity_len, - obfuscated_ticket_age ) == SSL_TLS1_3_OFFERED_PSK_MATCH ) + ssl, identity, identity_len, obfuscated_ticket_age, + (mbedtls_ssl_session *)session ) == SSL_TLS1_3_OFFERED_PSK_MATCH ) { mbedtls_ssl_session *i_session=(mbedtls_ssl_session *)session; ssl->handshake->resume = 1; From 4746b10c2e8806df173046cb4ab9998587fbb3af Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 13 Sep 2022 11:11:48 +0800 Subject: [PATCH 16/22] fix various issues - Format issues - Possible memory leak - Improve naming and comment issues Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 53 +++++++++++++------------------------- programs/ssl/ssl_client2.c | 1 + 2 files changed, 19 insertions(+), 35 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index adedb6df5..67d685bb0 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -143,21 +143,14 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> check_identity_match_ticket" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "ticket length: %" MBEDTLS_PRINTF_SIZET - ". ticket_parse is %sconfigured. " - "ticket_write is %sconfigured.", - identity_len, - ssl->conf->f_ticket_parse == NULL ? "NOT " : "", - ssl->conf->f_ticket_write == NULL ? "NOT " : "" ) ); - /* Ticket parser is not configured, Skip */ if( ssl->conf->f_ticket_parse == NULL || identity_len == 0 ) return( 0 ); - /* We create a copy of the encrypted ticket since decrypting - * it into the same buffer will wipe-out the original content. - * We do, however, need the original buffer for computing the - * psk binder value. + /* We create a copy of the encrypted ticket since the ticket parsing + * function is allowed to use its input buffer as an output buffer + * (in-place decryption). We do, however, need the original buffer for + * computing the PSK binder value. */ ticket_buffer = mbedtls_calloc( 1, identity_len ); if( ticket_buffer == NULL ) @@ -258,8 +251,8 @@ static int ssl_tls13_offered_psks_check_identity_match( const unsigned char *identity, size_t identity_len, uint32_t obfuscated_ticket_age, - void *session, - int *psk_type ) + int *psk_type, + mbedtls_ssl_session *session ) { ((void) session); ((void) obfuscated_ticket_age); @@ -268,8 +261,6 @@ static int ssl_tls13_offered_psks_check_identity_match( MBEDTLS_SSL_DEBUG_BUF( 4, "identity", identity, identity_len ); ssl->handshake->resume = 0; - - #if defined(MBEDTLS_SSL_SESSION_TICKETS) if( ssl_tls13_offered_psks_check_identity_match_ticket( ssl, identity, identity_len, obfuscated_ticket_age, @@ -444,31 +435,26 @@ static int ssl_tls13_select_ciphersuite_for_resumption( if( ciphersuite_info == NULL ) continue; - *selected_ciphersuite = session->ciphersuite; + *selected_ciphersuite = cipher_suite; *selected_ciphersuite_info = ciphersuite_info; return( 0 ); - } return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } MBEDTLS_CHECK_RETURN_CRITICAL -static int ssl_tls13_session_copy( mbedtls_ssl_session *dst, - mbedtls_ssl_session *src ) +static int ssl_tls13_session_copy_ticket( mbedtls_ssl_session *dst, + const mbedtls_ssl_session *src ) { - dst->endpoint = src->endpoint; - dst->ciphersuite = src->ciphersuite; dst->ticket_age_add = src->ticket_age_add; dst->ticket_flags = src->ticket_flags; dst->resumption_key_len = src->resumption_key_len; if( src->resumption_key_len == 0 ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); memcpy( dst->resumption_key, src->resumption_key, src->resumption_key_len ); -#if defined(MBEDTLS_HAVE_TIME) - dst->start = src->start; -#endif + return( 0 ); } #endif /* MBEDTLS_SSL_SESSION_TICKETS */ @@ -551,7 +537,7 @@ static int ssl_tls13_parse_pre_shared_key_ext( mbedtls_ssl_context *ssl, const mbedtls_ssl_ciphersuite_t *ciphersuite_info; #if defined(MBEDTLS_SSL_SESSION_TICKETS) mbedtls_ssl_session session; - memset( &session, 0, sizeof( session ) ); + mbedtls_ssl_session_init( &session ); #endif MBEDTLS_SSL_CHK_BUF_READ_PTR( p_identity_len, identities_end, 2 + 1 + 4 ); @@ -573,7 +559,7 @@ static int ssl_tls13_parse_pre_shared_key_ext( mbedtls_ssl_context *ssl, ret = ssl_tls13_offered_psks_check_identity_match( ssl, identity, identity_len, obfuscated_ticket_age, - &session, &psk_type ); + &psk_type, &session ); if( ret != SSL_TLS1_3_OFFERED_PSK_MATCH ) continue; @@ -590,6 +576,8 @@ static int ssl_tls13_parse_pre_shared_key_ext( mbedtls_ssl_context *ssl, ret = ssl_tls13_select_ciphersuite_for_resumption( ssl, ciphersuites, ciphersuites_end, &session, &cipher_suite, &ciphersuite_info ); + if( ret != 0 ) + mbedtls_ssl_session_free( &session ); #else ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; #endif @@ -632,18 +620,13 @@ static int ssl_tls13_parse_pre_shared_key_ext( mbedtls_ssl_context *ssl, /* Update handshake parameters */ ssl->handshake->ciphersuite_info = ciphersuite_info; - if( psk_type == MBEDTLS_SSL_TLS1_3_PSK_EXTERNAL ) - { - ssl->session_negotiate->ciphersuite = cipher_suite; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "overwrite ciphersuite: %04x - %s", - cipher_suite, - ciphersuite_info->name ) ); - } + ssl->session_negotiate->ciphersuite = cipher_suite; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "overwrite ciphersuite: %04x - %s", + cipher_suite, ciphersuite_info->name ) ); #if defined(MBEDTLS_SSL_SESSION_TICKETS) - else if( psk_type == MBEDTLS_SSL_TLS1_3_PSK_RESUMPTION ) { - ret = ssl_tls13_session_copy(ssl->session_negotiate, &session ); + ret = ssl_tls13_session_copy_ticket(ssl->session_negotiate, &session ); mbedtls_ssl_session_free( &session ); if( ret != 0 ) return( ret ); diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index c1f347b87..94f6d18ff 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -708,6 +708,7 @@ exit: mbedtls_ssl_session_free( &exported_session ); return( ret ); } + int main( int argc, char *argv[] ) { int ret = 0, len, tail_len, i, written, frags, retry_left; From 95db17ed5f016b6767942655c40c6285f14af42b Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 14 Sep 2022 10:30:53 +0800 Subject: [PATCH 17/22] fix various issues - improve obfuscated ticket age generator - improve psk getter Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index e92a10c0a..2c7d94638 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -675,17 +675,11 @@ static int ssl_tls13_get_psk_to_offer( const unsigned char **psk, size_t *psk_len, const unsigned char **psk_identity, size_t *psk_identity_len ) { - if( psk_type == NULL || - psk == NULL || psk_len == NULL || - psk_identity == NULL || psk_identity_len == NULL ) - { - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); - } - *psk = NULL; *psk_len = 0; *psk_identity = NULL; *psk_identity_len = 0; + *psk_type = MBEDTLS_SSL_TLS1_3_PSK_EXTERNAL; #if defined(MBEDTLS_SSL_SESSION_TICKETS) /* Check if a ticket has been configured. */ @@ -694,9 +688,9 @@ static int ssl_tls13_get_psk_to_offer( { #if defined(MBEDTLS_HAVE_TIME) mbedtls_time_t now = mbedtls_time( NULL ); - - if( ( ssl->session_negotiate->ticket_received <= now && - now - ssl->session_negotiate->ticket_received < 7 * 86400 * 1000 ) ) + if( ssl->session_negotiate->ticket_received <= now && + (uint64_t)( now - ssl->session_negotiate->ticket_received ) + <= ssl->session_negotiate->ticket_lifetime ) { *psk_type = MBEDTLS_SSL_TLS1_3_PSK_RESUMPTION; *psk = ssl->session_negotiate->resumption_key; @@ -713,7 +707,6 @@ static int ssl_tls13_get_psk_to_offer( /* Check if an external PSK has been configured. */ if( ssl->conf->psk != NULL ) { - *psk_type = MBEDTLS_SSL_TLS1_3_PSK_EXTERNAL; *psk = ssl->conf->psk; *psk_len = ssl->conf->psk_len; *psk_identity = ssl->conf->psk_identity; @@ -721,7 +714,7 @@ static int ssl_tls13_get_psk_to_offer( return( 0 ); } - return( 1 ); + return( MBEDTLS_ERR_ERROR_GENERIC_ERROR ); } /* @@ -817,9 +810,12 @@ int mbedtls_ssl_tls13_write_identities_of_pre_shared_key_ext( { #if defined(MBEDTLS_HAVE_TIME) mbedtls_time_t now = mbedtls_time( NULL ); + uint64_t age_in_ms = + ( now - ssl->session_negotiate->ticket_received ) * 1000; + obfuscated_ticket_age = - (uint32_t)( now - ssl->session_negotiate->ticket_received ) + - ssl->session_negotiate->ticket_age_add; + (uint32_t)( ( age_in_ms + ssl->session_negotiate->ticket_age_add ) + & ( ( 1LL << 32 ) - 1 ) ); #endif } else From acff823846f06663e8a7dd67348f20ccb217b435 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 14 Sep 2022 14:35:11 +0800 Subject: [PATCH 18/22] Add negative tolerance window If `now == session->start` or the timer of client is faster than server, client age might be bigger than server. Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 67d685bb0..55cb67067 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -136,7 +136,8 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( unsigned char *ticket_buffer; #if defined(MBEDTLS_HAVE_TIME) mbedtls_time_t now; - uint64_t age_in_s, age_in_ms, client_age_in_ms; + uint64_t age_in_s; + int64_t diff_in_ms; #endif ((void) obfuscated_ticket_age); @@ -220,15 +221,14 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( * ticket_age_add from PskIdentity.obfuscated_ticket_age modulo 2^32) is * within a small tolerance of the time since the ticket was issued. */ - age_in_ms = age_in_s * 1000; - client_age_in_ms = obfuscated_ticket_age - session->ticket_age_add; - if( age_in_ms < client_age_in_ms || - ( age_in_ms - client_age_in_ms ) > MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE ) + diff_in_ms = age_in_s * 1000; + diff_in_ms -= ( obfuscated_ticket_age - session->ticket_age_add ); + diff_in_ms += MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE / 2; + if( diff_in_ms < 0 || diff_in_ms > MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "Ticket expired: Ticket age outside tolerance window " - "( diff=%d )", - (int)(age_in_ms - client_age_in_ms ) ) ); + "( diff=%d )", (int)diff_in_ms ) ); goto exit; } From f7dad3cfbe49696dc2cd20642b1a05e372f3f699 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 14 Sep 2022 22:31:39 +0800 Subject: [PATCH 19/22] fix various issues - Naming - format - Reduce negative tolerance window Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 10 +++------- library/ssl_tls13_server.c | 23 ++++++++++++++--------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 2c7d94638..33e8cc6aa 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -665,9 +665,7 @@ static int ssl_tls13_write_psk_key_exchange_modes_ext( mbedtls_ssl_context *ssl, return ( 0 ); } -/* Check if we have any PSK to offer, returns 0 if PSK is available. - * Assign the psk and ticket if pointers are present. - */ +/* Check if we have any PSK to offer, returns 0 if a PSK is available. */ MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_get_psk_to_offer( const mbedtls_ssl_context *ssl, @@ -810,12 +808,10 @@ int mbedtls_ssl_tls13_write_identities_of_pre_shared_key_ext( { #if defined(MBEDTLS_HAVE_TIME) mbedtls_time_t now = mbedtls_time( NULL ); - uint64_t age_in_ms = - ( now - ssl->session_negotiate->ticket_received ) * 1000; obfuscated_ticket_age = - (uint32_t)( ( age_in_ms + ssl->session_negotiate->ticket_age_add ) - & ( ( 1LL << 32 ) - 1 ) ); + ( (uint32_t)( now - ssl->session_negotiate->ticket_received ) * 1000 ) + + ssl->session_negotiate->ticket_age_add; #endif } else diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 55cb67067..b73b39c2a 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -137,7 +137,7 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( #if defined(MBEDTLS_HAVE_TIME) mbedtls_time_t now; uint64_t age_in_s; - int64_t diff_in_ms; + int64_t age_diff_in_ms; #endif ((void) obfuscated_ticket_age); @@ -183,7 +183,6 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( #if defined(MBEDTLS_HAVE_TIME) now = mbedtls_time( NULL ); - ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED; if( now < session->start ) { MBEDTLS_SSL_DEBUG_MSG( @@ -220,15 +219,20 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( * the ticket age for the selected PSK identity (computed by subtracting * ticket_age_add from PskIdentity.obfuscated_ticket_age modulo 2^32) is * within a small tolerance of the time since the ticket was issued. + * + * NOTE: When `now == session->start`, `age_diff_in_ms` will get a negative + * result. That's reasonable, the age units are different between + * server and client sides. Add a -1000 tolerance window to resolve + * that. */ - diff_in_ms = age_in_s * 1000; - diff_in_ms -= ( obfuscated_ticket_age - session->ticket_age_add ); - diff_in_ms += MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE / 2; - if( diff_in_ms < 0 || diff_in_ms > MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE ) + age_diff_in_ms = age_in_s * 1000; + age_diff_in_ms -= ( obfuscated_ticket_age - session->ticket_age_add ); + if( age_diff_in_ms <= -1000 || + age_diff_in_ms > MBEDTLS_SSL_TLS1_3_TICKET_AGE_TOLERANCE ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "Ticket expired: Ticket age outside tolerance window " - "( diff=%d )", (int)diff_in_ms ) ); + "( diff=%d )", (int)age_diff_in_ms ) ); goto exit; } @@ -264,7 +268,7 @@ static int ssl_tls13_offered_psks_check_identity_match( #if defined(MBEDTLS_SSL_SESSION_TICKETS) if( ssl_tls13_offered_psks_check_identity_match_ticket( ssl, identity, identity_len, obfuscated_ticket_age, - (mbedtls_ssl_session *)session ) == SSL_TLS1_3_OFFERED_PSK_MATCH ) + session ) == SSL_TLS1_3_OFFERED_PSK_MATCH ) { mbedtls_ssl_session *i_session=(mbedtls_ssl_session *)session; ssl->handshake->resume = 1; @@ -626,7 +630,8 @@ static int ssl_tls13_parse_pre_shared_key_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_SESSION_TICKETS) if( psk_type == MBEDTLS_SSL_TLS1_3_PSK_RESUMPTION ) { - ret = ssl_tls13_session_copy_ticket(ssl->session_negotiate, &session ); + ret = ssl_tls13_session_copy_ticket( ssl->session_negotiate, + &session ); mbedtls_ssl_session_free( &session ); if( ret != 0 ) return( ret ); From 0a55cc647ccbcba34b9ab982978a0006bdbbb118 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 15 Sep 2022 16:15:06 +0800 Subject: [PATCH 20/22] Remove unnecessary var and improve comment Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index b73b39c2a..71716ad44 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -220,10 +220,10 @@ static int ssl_tls13_offered_psks_check_identity_match_ticket( * ticket_age_add from PskIdentity.obfuscated_ticket_age modulo 2^32) is * within a small tolerance of the time since the ticket was issued. * - * NOTE: When `now == session->start`, `age_diff_in_ms` will get a negative - * result. That's reasonable, the age units are different between - * server and client sides. Add a -1000 tolerance window to resolve - * that. + * NOTE: When `now == session->start`, `age_diff_in_ms` may be negative + * as the age units are different on the server (s) and in the + * client (ms) side. Add a -1000 ms tolerance window to take this + * into account. */ age_diff_in_ms = age_in_s * 1000; age_diff_in_ms -= ( obfuscated_ticket_age - session->ticket_age_add ); @@ -270,16 +270,15 @@ static int ssl_tls13_offered_psks_check_identity_match( ssl, identity, identity_len, obfuscated_ticket_age, session ) == SSL_TLS1_3_OFFERED_PSK_MATCH ) { - mbedtls_ssl_session *i_session=(mbedtls_ssl_session *)session; ssl->handshake->resume = 1; *psk_type = MBEDTLS_SSL_TLS1_3_PSK_RESUMPTION; mbedtls_ssl_set_hs_psk( ssl, - i_session->resumption_key, - i_session->resumption_key_len ); + session->resumption_key, + session->resumption_key_len ); MBEDTLS_SSL_DEBUG_BUF( 4, "Ticket-resumed PSK:", - i_session->resumption_key, - i_session->resumption_key_len ); + session->resumption_key, + session->resumption_key_len ); MBEDTLS_SSL_DEBUG_MSG( 4, ( "ticket: obfuscated_ticket_age: %u", (unsigned)obfuscated_ticket_age ) ); return( SSL_TLS1_3_OFFERED_PSK_MATCH ); From a5df584d87a3bf6f929a60dba84906eb0f886cd3 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 16 Sep 2022 11:27:57 +0800 Subject: [PATCH 21/22] fix build fail for test_psa_crypto_config_accel_hash_use_psa Signed-off-by: Jerry Yu --- library/ssl_tls13_keys.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 28090ebcb..0f9c07ada 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -1537,7 +1537,8 @@ int mbedtls_ssl_tls13_compute_resumption_master_secret( mbedtls_ssl_context *ssl MBEDTLS_SSL_DEBUG_BUF( 4, "Resumption master secret", ssl->session_negotiate->app_secrets.resumption_master_secret, - mbedtls_md_get_size( mbedtls_md_info_from_type( md_type ) ) ); + PSA_HASH_LENGTH( + PSA_ALG_HMAC( mbedtls_hash_info_psa_from_md( md_type ) ) ) ); MBEDTLS_SSL_DEBUG_MSG( 2, From 6ee726e1abb1ac4cf5a81fc38b5cc5c6a4272354 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 16 Sep 2022 16:32:27 +0800 Subject: [PATCH 22/22] Replace md translation function Signed-off-by: Jerry Yu --- library/ssl_tls13_keys.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 0f9c07ada..48de3d008 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -1537,9 +1537,7 @@ int mbedtls_ssl_tls13_compute_resumption_master_secret( mbedtls_ssl_context *ssl MBEDTLS_SSL_DEBUG_BUF( 4, "Resumption master secret", ssl->session_negotiate->app_secrets.resumption_master_secret, - PSA_HASH_LENGTH( - PSA_ALG_HMAC( mbedtls_hash_info_psa_from_md( md_type ) ) ) ); - + PSA_HASH_LENGTH( mbedtls_psa_translate_md( md_type ) ) ) ; MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_tls13_compute_resumption_master_secret" ) );