-
Notifications
You must be signed in to change notification settings - Fork 8
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.
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
base: tls13-prototype
Are you sure you want to change the base?
[0-RTT] Use trial decryption if server rejects client 0-RTT attempt #386
Conversation
@@ -876,6 +876,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; |
There was a problem hiding this comment.
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.
return( 0 ); | ||
|
||
/* | ||
* Drop record iff: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
ae02c5e
to
0feb28b
Compare
Description
This PR adds server support for handling trial decryption in order to make test output accessible for the early data rejected case by using
ssl_server2.c
. Without trial decryption logic, early data rejected tests that usessl_server2.c
will fail due to decryption errors caused by the client sending early data before receiving and parsing the server's extensions.Status
READY
Requires Backporting
NO
Additional comments
The need for the
skip_failed_decryption
flag is to ensure we no longer drop records which fail deprotection once a record is deprotected successfully with the 1-RTT keys. The motivation was taken from Fizz, but I'd like to know if (1) there's a more appropriate place thanmbedtls_ssl_read_record()
to disable the flag, and (2) if there's other existing state we can leverage to avoid using an additional flag.Todos
Steps to test or reproduce
ssl-opt.sh -f 'early data status - rejected'
and verify it passes.ssl-opt.sh -f 'early data status - rejected'
- verify it fails due toMBEDTLS_ERR_SSL_INVALID_MAC
.