Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[0-RTT] Use trial decryption if server rejects client 0-RTT attempt #386

Open
wants to merge 1 commit into
base: tls13-prototype
Choose a base branch
from
Open

[0-RTT] Use trial decryption if server rejects client 0-RTT attempt #386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions library/ssl_misc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,9 @@ struct mbedtls_ssl_handshake_params
1 -- MBEDTLS_SSL_EARLY_DATA_ENABLED (for use early data)
*/
int early_data;
#if defined(MBEDTLS_SSL_SRV_C)
int skip_failed_decryption;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to indicate (in the comments) what the allowed value for skip_failed_decryption are.

#endif /* MBEDTLS_SSL_SRV_C */
#endif /* MBEDTLS_ZERO_RTT */

#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */
Expand Down
68 changes: 62 additions & 6 deletions library/ssl_msg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4114,10 +4114,21 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl,
return( ret );
}

if( ssl->in_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE &&
update_hs_digest == 1 )
if( ssl->in_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE )
{
mbedtls_ssl_update_handshake_status( ssl );
if( update_hs_digest == 1)
mbedtls_ssl_update_handshake_status( ssl );

#if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_ZERO_RTT)
if( ssl->handshake != NULL &&
ssl->handshake->skip_failed_decryption != 0 &&
ssl->transform_in != NULL )
{
/* Record deprotected successfully */
ssl->handshake->skip_failed_decryption = 0;
MBEDTLS_SSL_DEBUG_MSG( 4, ( "disabling skip_failed_decryption" ) );
}
#endif /* MBEDTLS_SSL_SRV_C && MBEDTLS_ZERO_RTT */
}
}
else
Expand Down Expand Up @@ -4711,6 +4722,41 @@ static int ssl_buffer_future_record( mbedtls_ssl_context *ssl,

#endif /* MBEDTLS_SSL_PROTO_DTLS */

/*
* RFC 8446:
* "If the client attempts a 0-RTT handshake but the server
* rejects it, the server will generally not have the 0-RTT record
* protection keys and must instead use trial decryption (either with
* the 1-RTT handshake keys or by looking for a cleartext ClientHello in
* the case of a HelloRetryRequest) to find the first non-0-RTT message."
*/
#if defined(MBEDTLS_SSL_SRV_C)
static int ssl_should_drop_record( mbedtls_ssl_context *ssl )
{
#if defined(MBEDTLS_ZERO_RTT) && !defined(MBEDTLS_SSL_USE_MPS)
if( ssl->conf->endpoint != MBEDTLS_SSL_IS_SERVER || ssl->handshake == NULL )
return( 0 );

/*
* Drop record iff:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand how the 4 cases are distinguished in the code below.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skip_failed_decryption is set to 1 in one place if the client sends early data indication but server can't/won't support early data (1 and 2), and is set to 0 once a record is deprotected successfully (3).

Checking transform_in ensures handshake traffic key was used when attempting to deprotect (4). I added this condition as an invariant check since there should never be a case where skip_failed_decryption == 1 but we're using other keys.

I agree it's not immediately obvious, do you think the implementation could be more clear or just the comments?

* 1. Client indicated early data use (skip_failed_decryption).
* 2. Server does not have early data enabled (skip_failed_decryption).
* 3. First non-0-RTT record has not yet been found (skip_failed_decryption).
* 4. 1-RTT handshake keys are in use.
*/
if( ssl->handshake->skip_failed_decryption == 1 &&
ssl->transform_in == ssl->handshake->transform_handshake )
{
return( 1 );
}

#endif /* MBEDTLS_ZERO_RTT && !MBEDTLS_SSL_USE_MPS */
((void) ssl);

return( 0 );
}
#endif /* MBEDTLS_SSL_SRV_C */

static int ssl_get_next_record( mbedtls_ssl_context *ssl )
{
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
Expand Down Expand Up @@ -4879,15 +4925,25 @@ static int ssl_get_next_record( mbedtls_ssl_context *ssl )
else
#endif
{
/* Error out (and send alert) on invalid records */
#if defined(MBEDTLS_SSL_ALL_ALERT_MESSAGES)
if( ret == MBEDTLS_ERR_SSL_INVALID_MAC )
{
#if defined(MBEDTLS_SSL_SRV_C)
if( ssl->handshake != NULL &&
ssl_should_drop_record( ssl ) != 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid record (mac), dropping 0-RTT message" ) );
return( MBEDTLS_ERR_SSL_CONTINUE_PROCESSING );
}
#endif /* MBEDTLS_SSL_SRV_C */

/* Error out (and send alert) on invalid records */
#if defined(MBEDTLS_SSL_ALL_ALERT_MESSAGES)
mbedtls_ssl_send_alert_message( ssl,
MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_BAD_RECORD_MAC );
#endif /* MBEDTLS_SSL_ALL_ALERT_MESSAGES */
}
#endif

return( ret );
}
}
Expand Down
1 change: 1 addition & 0 deletions library/ssl_tls13_generic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2130,6 +2130,7 @@ int mbedtls_ssl_tls13_write_early_data_ext( mbedtls_ssl_context *ssl,
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write early_data extension" ) );
ssl->handshake->early_data = MBEDTLS_SSL_EARLY_DATA_OFF;
ssl->handshake->skip_failed_decryption = 1;
return( 0 );
}
}
Expand Down
3 changes: 3 additions & 0 deletions library/ssl_tls13_server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2754,6 +2754,9 @@ static int ssl_tls13_client_hello_postprocess( mbedtls_ssl_context *ssl,
int ret = 0;
#if defined(MBEDTLS_ZERO_RTT)
mbedtls_ssl_key_set traffic_keys;

if( ssl->handshake->hello_retry_requests_sent == 1 )
ssl->handshake->skip_failed_decryption = 0;
#endif /* MBEDTLS_ZERO_RTT */

if( ssl->handshake->hello_retry_requests_sent == 0 &&
Expand Down
13 changes: 13 additions & 0 deletions tests/ssl-opt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2188,6 +2188,19 @@ run_test "TLS 1.3, TLS1-3-AES-256-GCM-SHA384, ext PSK, early data status - no
0 \
-c "early data status = 0" \

# test early data status - rejected
requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3
requires_config_enabled MBEDTLS_DEBUG_C
requires_config_enabled MBEDTLS_SSL_SRV_C
requires_config_enabled MBEDTLS_SSL_CLI_C
requires_config_enabled MBEDTLS_ZERO_RTT
requires_config_disabled MBEDTLS_SSL_USE_MPS
run_test "TLS 1.3, TLS1-3-AES-256-GCM-SHA384, ext PSK, early data status - rejected" \
"$P_SRV nbio=2 debug_level=5 force_version=tls13 early_data=0 tls13_kex_modes=psk psk=010203 psk_identity=0a0b0c" \
"$P_CLI nbio=2 debug_level=5 force_version=tls13 force_ciphersuite=TLS1-3-AES-256-GCM-SHA384 tls13_kex_modes=psk early_data=1 psk=010203 psk_identity=0a0b0c" \
0 \
-c "early data status = 1" \

# test early data status - accepted
requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3
requires_config_enabled MBEDTLS_DEBUG_C
Expand Down