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.

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

How to handle TLS 1.3 new session ticket errors #8749

Closed
fargiolas opened this issue Jan 25, 2024 · 10 comments
Closed

How to handle TLS 1.3 new session ticket errors #8749

fargiolas opened this issue Jan 25, 2024 · 10 comments
Assignees
Labels
bug component-tls13 size-s Estimated task size: small (~2d)

Comments

@fargiolas
Copy link

fargiolas commented Jan 25, 2024

Lately I've been seeing some failed connection in TLS1.3 with mbedtls v3.3 because I was getting a MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET returned from a mbedtls_ssl_write() call.

According to the API doc write() should lead to a reset whenever an error different from WANT_READ, WANT_WRITE, ASYNC_IN, CRYPTO_IN_PROGRESS or CLIENT_RECONNECT is received. So the app was rightfully closing the connection with the new session ticket error. Note that this was both with session tickets enabled and disabled client side.

The only documentation I found about this error just says it's experimental and may go away but says nothing as far as I could find about how to handle it.

Now reading here and there it seems it is indeed safe to ignore this error, at least in mbedtls_ssl_handshake_step(), mbedtls_ssl_handshake() and mbedtls_ssl_read()) according to this comment.

Unfortunately mbedtls_ssl_write is not included in that list so it's still not conclusive about how to handle it in this case.

I'd say if this error is going to stay in future releases it deserves to be better documented.


Edit (mpg): status summary.

sfan5 added a commit to sfan5/ffmpeg that referenced this issue May 17, 2024
When TLSv1.3 and session tickets are enabled mbedtls_ssl_read()
will return an error code to inform about a received session ticket.
This can simply be handled like EAGAIN instead of errornously
aborting the connection.

ref: Mbed-TLS/mbedtls#8749
sfan5 added a commit to sfan5/ffmpeg that referenced this issue May 29, 2024
When TLSv1.3 and session tickets are enabled mbedtls_ssl_read()
will return an error code to inform about a received session ticket.
This can simply be handled like EAGAIN instead of errornously
aborting the connection.

ref: Mbed-TLS/mbedtls#8749
sfan5 added a commit to sfan5/ffmpeg that referenced this issue Jun 4, 2024
When TLSv1.3 and session tickets are enabled mbedtls_ssl_read()
will return an error code to inform about a received session ticket.
This can simply be handled like EAGAIN instead of errornously
aborting the connection.

ref: Mbed-TLS/mbedtls#8749
sfan5 added a commit to sfan5/ffmpeg that referenced this issue Jun 4, 2024
When TLSv1.3 and session tickets are enabled mbedtls_ssl_read()
will return an error code to inform about a received session ticket.
This can simply be handled like EAGAIN instead of errornously
aborting the connection.

ref: Mbed-TLS/mbedtls#8749
BtbN pushed a commit to FFmpeg/FFmpeg that referenced this issue Jun 11, 2024
When TLSv1.3 and session tickets are enabled mbedtls_ssl_read()
will return an error code to inform about a received session ticket.
This can simply be handled like EAGAIN instead of errornously
aborting the connection.

ref: Mbed-TLS/mbedtls#8749
Signed-off-by: Anton Khirnov <[email protected]>
richardpl pushed a commit to librempeg/librempeg that referenced this issue Jun 11, 2024
When TLSv1.3 and session tickets are enabled mbedtls_ssl_read()
will return an error code to inform about a received session ticket.
This can simply be handled like EAGAIN instead of errornously
aborting the connection.

ref: Mbed-TLS/mbedtls#8749
Signed-off-by: Anton Khirnov <[email protected]>
Signed-off-by: Paul B Mahol <[email protected]>
@gilles-peskine-arm gilles-peskine-arm moved this to 3.6.1 patch release in Mbed TLS Epics Aug 26, 2024
@gilles-peskine-arm gilles-peskine-arm added the size-s Estimated task size: small (~2d) label Aug 26, 2024
@gilles-peskine-arm gilles-peskine-arm changed the title Better document how to handle new session ticket errors How to handle TLS 1.3 new session ticket errors Aug 27, 2024
@gilles-peskine-arm
Copy link
Contributor

As of Mbed TLS 3.2–3.5, the error code is (very tersely) documented but its effect on a TLS connection is not. So applications should terminate the connection, since any error invalidates the mbedtls_ssl_context object except for a small documented list (MBEDTLS_ERR_SSL_WANT_WRITE, MBEDTLS_ERR_SSL_WANT_READ, MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS). This of course makes the error essentially useless. And it breaks many TLS 1.3 connections, including a default openssl s_server with OpenSSL 3.0 (and probably other versions as well).

In Mbed TLS 3.6.0, we enabled TLS 1.3 by default. So even applications that have no explicit support for TLS 1.3 receive this error. For backward compatibility, we have to stop returning this error by default. Our plan for Mbed TLS 3.6.1 is that this error will never be returned in the default configuration of an SSL context. This means that NewSessionTicket messages after the handshake will be ignored. Applications can declare that support for NewSessionTicket messages by calling a setup function on the SSL context (or the SSL configuration?) before the handshake. This will be a boolean, or maybe registering a callback.

@mpg
Copy link
Contributor

mpg commented Aug 27, 2024

Unfortunately mbedtls_ssl_write is not included in that list so it's still not conclusive about how to handle it in this case.

I think this error can be ignored from mbedtls_ssl_write() as well. If I'm not mistaken however, [edit: I was mistaken] the only circumstance this can happen is if you call mbedtls_ssl_write() before the handshake has completed. If you first complete the handshake (that is, call mbedtls_ssl_handshake() in a loop until in returns 0, see for example here or here), then I think mbedtls_ssl_write() should never return this error.

Note that while the API allows you to directly call mbedtls_ssl_read() or mbedtls_ssl_write() without first calling mbedtls_ssl_handshake() (in a loop until it returns 0), my personal recommendation (and what all our example do, I think) is to call mbedtls_ssl_handshake() first, as simplifies error handling by reducing the range of errors that mbedtls_ssl_read() and mbedtls_ssl_write() may return. (Note: this is unfortunately not true with TLS 1.2 with renegotiation enabled, but it's true with TLS 1.3, or TLS 1.2 with renegotiation disabled.)

@mpg
Copy link
Contributor

mpg commented Aug 27, 2024

For the record, here's my reasoning about which public functions can return MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET. Starting from the bottom up:

  • MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET can only be returned by ssl_tls13_process_new_session_ticket(),
  • which is only called by mbedtls_ssl_tls13_handshake_client_step() (when ssl->state is MBEDTLS_SSL_TLS1_3_NEW_SESSION_TICKET [edit: which happens when receiving a new session ticket]),
  • which is only called by mbedtls_ssl_handshake_step(), which may be called directly by the application, but in the library it's only called by mbedtls_ssl_write_early_data(), which is public but never called from the library, and mbedtls_ssl_handshake() which is public but also called in from the library in:
    • mbedtls_ssl_write_early_data() already listed above;
    • mbedtls_ssl_read() but only when ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER [edit: in particular when it's MBEDTLS_SSL_TLS1_3_NEW_SESSION_TICKET` see above]
    • mbedtls_ssl_write() but only when ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER; [edit: in particular when it's MBEDTLS_SSL_TLS1_3_NEW_SESSION_TICKET` see above]
    • mbedtls_ssl_start_renegotiation() but it's a 1.2-only thing;
    • mbedtls_ssl_renegotiate() but it's a 1.2-only thing again.

So, long story short, the public functions that might return MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET are:

  • mbedtls_ssl_handshake_step() and mbedtls_ssl_handshake();
  • mbedtls_ssl_read();
  • mbedtls_ssl_write() possibly to be investigated (can we get to ssl->state == MBEDTLS_SSL_TLS1_3_NEW_SESSION_TICKET after the handshake is over? would need a read at some point) (edit: yes we can, if we call mbedtls_ssl_read() and it returns WANT_READ, then for some reason we call ssl_write() before calling ssl_read() again).
  • possibly mbedtls_ssl_write_early_data() except perhaps it's impossible due to the state machine, I'd have to dig deeper.

@fargiolas
Copy link
Author

fargiolas commented Aug 27, 2024

It's been a while but I remember distinctly to have seen this error in a mbedtls_ssl_write after the handshake was over. App wasn't doing any write before the handshake was over.

If that helps, since this bug was reported I followed the suggestion of ignoring MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET in write read and handshake/handshake_step just like we do for the other non fatal errors listed by @gilles-peskine-arm and it's been working pretty well for me so far.

I agree with with the proposed path forward to make this opt-in so it doesn't break existing applications.

@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented Aug 29, 2024

In Mbed TLS 3.6.1

In #9507 we added documentation on handling the error. We also made a backward-incompatible change for applications that were already handling the error: it is now disabled by default, you'll need to call mbedtls_ssl_conf_tls13_enable_signal_new_session_tickets(1) to enable the pre-3.6.1 behavior. We did that to avoid breaking applications that were not handling the error, since clients that were working with TLS 1.2 could stop working when they connected to a TLS 1.3 server.

In Mbed TLS 4.0

We're going to change the API: instead of returning an error, the TLS layer will call a callback when it receives a NewSessionTicket message. This was previously suggested in #6640.

@mpg
Copy link
Contributor

mpg commented Aug 29, 2024

you'll need to call mbedtls_ssl_conf_tls13_enable_signal_new_session_tickets(1) to enable the pre-3.6.0 behavior

ITYM pre-3.6.1 here.

@fargiolas
Copy link
Author

fargiolas commented Aug 29, 2024

We also made a backward-incompatible change for applications that were already handling the error

If "handling the error" here is just ignoring it and retrying, apps will continue to work, am I correct? We just won't receive the error anymore?

@gilles-peskine-arm
Copy link
Contributor

If "handling the error" here is just ignoring it and retrying, apps will continue to work, am I correct? We just won't receive the error anymore?

Right. Applications that want the session tickets will need to call mbedtls_ssl_conf_tls13_enable_signal_new_session_tickets. Applications that don't care about session tickets will not need to do anything, whether they were treating MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET as a “try again” error or as a fatal error.

@mpg
Copy link
Contributor

mpg commented Sep 2, 2024

This is fixed in Mbed TLS 3.6.1 which has now been released.

However I'm not closing this ticket as the following work remains:

@mpg mpg self-assigned this Sep 2, 2024
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Sep 25, 2024
TLS 1.3 session tickets require additional handling in the client.
Mbed-TLS#8749

Disable session tickets for ssl_client1 when using TLS 1.3
until Mbed-TLS#6640 is resolved
and (if relevant) implemented in ssl_client1.

Signed-off-by: Gilles Peskine <[email protected]>
@mpg
Copy link
Contributor

mpg commented Oct 2, 2024

Closing this issue as the problem was fixed in 3.6.1 and while it has not been resolved for 4.0 yet, that part is already tracked by #6640 which has now been added to the 4.0 board.

@mpg mpg closed this as completed Oct 2, 2024
@github-project-automation github-project-automation bot moved this to Done in Barriers Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-tls13 size-s Estimated task size: small (~2d)
Projects
Archived in project
Status: 3.6.1 patch release
Development

Successfully merging a pull request may close this issue.

5 participants