mbedtls
mbedtls copied to clipboard
PSK check in mbedtls_ssl_tls13_key_schedule_stage_early
Suggested enhancement
I found one issue when we tested the following early data fallback 1-RTT test case:
- Client completes the initial full handshake with the server and receives NewSessionTicket that supports early data.
- Client initiates early data handshake but server rejects early data. We mocked it by faking the NewSessionTicket.
- We expect the client can fallback to 1-RTT and continue to have a full-handshake with the server.
- However we get
MBEDTLS_ERR_SSL_INTERNAL_ERROR
from mbedtls_ssl_tls13_key_schedule_stage_early whenssl_tls13_postprocess_server_hello
processes the second server hello that rejects early data.
Looking into the code, the psk will be clear out after ssl_tls13_write_early_data_postprocess. And psk may be set again in ssl_tls13_parse_server_pre_shared_key_ext only when the server accepts the psk.
So for this test case, it sounds reasonable that psk is not set when we call mbedtls_ssl_tls13_key_schedule_stage_early
for the 2nd server hello that rejects the early data. And we should allow such a case, and treat it as if there is no psk, and continue the handshake. It seems be the case in the old logic of tls13-prototype.
Here is our local patch to work around the issue by restoring old logic.
diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c
--- a/library/ssl_tls13_keys.c
+++ b/library/ssl_tls13_keys.c
@@ -1300,21 +1300,12 @@
hash_alg = handshake->ciphersuite_info->mac;
#endif /* MBEDTLS_USE_PSA_CRYPTO */
-
#if defined(MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_SOME_PSK_ENABLED)
- if( mbedtls_ssl_tls13_key_exchange_mode_with_psk( ssl ) ||
-#if defined(MBEDTLS_ZERO_RTT)
- ssl->handshake->early_data == MBEDTLS_SSL_EARLY_DATA_ON ||
-#endif
- 0 )
+ ret = mbedtls_ssl_tls13_export_handshake_psk( ssl, &psk, &psk_len );
+ if( ret != 0 )
{
- ret = mbedtls_ssl_tls13_export_handshake_psk( ssl, &psk, &psk_len );
- if( ret != 0 )
- {
- MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls13_export_handshake_psk",
- ret );
- return( ret );
- }
+ psk = NULL;
+ psk_len = 0;
}
#endif
cc @ronald-cron-arm , @yuhaoth
Justification
Mbed TLS needs this because