Fix the TLS 1.3 obfuscated_ticket_age caclulation #377
Conversation
Summary: Fix the calcuation of (obfuscated_ticket_age)[https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.11.1], it is milliseconds. Test Plan: ``` tests/ssl-opt.sh ``` Reviewers: Subscribers: Tasks: Tags:
| @@ -939,7 +939,7 @@ int mbedtls_ssl_write_pre_shared_key_ext( mbedtls_ssl_context *ssl, | |||
| } | |||
|
|
|||
| obfuscated_ticket_age = | |||
| (uint32_t)( now - ssl->session_negotiate->ticket_received ) + | |||
| (uint32_t)( ( now - ssl->session_negotiate->ticket_received ) * 1000 ) + | |||
There was a problem hiding this comment.
It seems a fix is needed at line 934 as well as now - ssl->session_negotiate->ticket_received (age in seconds) seems to be compared to the number of milliseconds in one week.
There was a problem hiding this comment.
Agreed. The intention is to keep the fix simple.
Regarding the L934, the fix is not straightforward. If we just return MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED, the request will fail. But a better approach is to fall back to full handshake (as if there is no session ticket). What do you think on this approach?
There was a problem hiding this comment.
Falling back to the full handshake is certainly better than having the exchange fail
There was a problem hiding this comment.
I agree with Ron that this change needs to be aligned with the calculation in line 934
Summary:
Courtesy to Bill Warshaw. Fix the calcuation of
obfuscated_ticket_age, it is milliseconds.Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Notes:
Signed-off-by:line from the committer to certify that the contribution is made under the terms of the Developer Certificate of Origin.Description
A few sentences describing the overall goals of the pull request's commits.
Status
READY/IN DEVELOPMENT/HOLD
Requires Backporting
When there is a bug fix, it should be backported to all maintained and supported branches.
Changes do not have to be backported if:
Yes | NO
Which branch?
Migrations
If there is any API change, what's the incentive and logic for it.
YES | NO
Additional comments
Any additional information that could be of interest
Todos
Steps to test or reproduce
Outline the steps to test or reproduce the PR here.