-
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
PSK check in mbedtls_ssl_tls13_key_schedule_stage_early #406
Comments
I think I did not understand the problem. Could you share your test scripts?
I did not know why mbedtls_ssl_tls13_key_schedule_stage_early is called in this case.
As my understand, if early data indication of EE is not received, client SHOULD skip |
If 2nd client hello is received, early data MUST be disabled. mbedtls_ssl_tls13_key_schedule_stage_early should not be called in ClientHello, it should be called in ServerHello. Then the reported issue should not exists. |
Sorry, this is from our own integration test. It is a case for QUIC and we use gtest to mock the case. The following is the mbedtls debug log from client. The issue is in the last few lines.
The rough flow is like this:
I touch it in step 8 above. mbedtls_ssl_tls13_key_schedule_stage_early is called from
|
Suggested enhancement
I found one issue when we tested the following early data fallback 1-RTT test case:
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.
cc @ronald-cron-arm , @yuhaoth
Justification
Mbed TLS needs this because
The text was updated successfully, but these errors were encountered: