TLS 1.3 client: Writing of the early data indication extension

[ronald-cron-arm]

This issue tracks the up-streaming on client side of the writing of the early data indication extension

Source: https://github.com/hannestschofenig/mbedtls/tree/tls13-prototype

PLEASE READ THOROUGHLY section 4.2.10 of the specification

• Write the early data indication extension in the ClientHello if the early data feature is enabled on the client and the client has been configured with a first PSK that:

1. was established via a NewSessionTicket (no support for externally established PSK for the time being)
2. is associated to the TLS 1.3 version of the protocol
3. is associated to a ciphersuite that the client proposes to the server
4. can be used for early data (see session->ticket_flags).
   The check of ALPN is out of the scope of this PR.

• Introduce, document and initialize properly the ssl context early_data_status field indicating the status of the negotiation of early data with the server. No need to add the mbedtls_ssl_get_early_data_status() API for the time being: to be defined as part of Define the API for writing early data on client side in TLS 1.3 #4902 if we need this API or not.

Testing:
test (ssl-opt.sh test) with an OpenSSL/GnuTLS server that a ClientHello containing an early data indication extension is successfully parsed. The resumption handshake should fail if the server accepts early data as long as the client does not write the End of Early Data message in response (another issue dedicated to that).

Depends on #6330

[yuhaoth]

was established via a NewSessionTicket (no support for externally established PSK for the time being)

For time being, I prefer PSK firstly, that can provide a suitable client for testing ASAP. @ronald-cron-arm and @xkqian, what's your opinion?

[yuhaoth]

can be used for early data (see session->ticket_flags).

I do not get any property about ticket_flags From RFC 8446. I think the definition of ticket_flags in prototype is out of data. I suggest replace it with max_early_data_size and max_early_data_size=0 means the ticket does not allow early data. @ronald-cron-arm and @xkqian, what's your opinion?

In prototype, the valid values of ticket_flags are allow_early_data, allow_dhe_resumption and allow_psk_resumption . It does not match section 4.6.1. And mbedtls_ssl_session does not include property for max_early_data_size.

[ronald-cron-arm]

Looking at TLS 1.3 specification, the flag allow_early_data is still relevant. The flag allow_early_data should be raised when the NewSessionTicket message defining a ticket contains an early data indication extension thus in ssl_tls13_parse_new_session_ticket_exts.

[xkqian]

Seems Jerry prefer external psk firstly, How about your opinion? @ronald-cron-arm
And, how about the relationship of MBEDTLS_SSL_SESSION_TICKETS and MBEDTLS_SSL_EARLY_DATA? Should they be binded together?

[yuhaoth]

Looking at TLS 1.3 specification, the flag allow_early_data is still relevant. The flag allow_early_data should be raised when the NewSessionTicket message defining a ticket contains an early data indication extension thus in ssl_tls13_parse_new_session_ticket_exts.

But I think that can be represent by max_early_data_size == 0 . I do not think we need ticket_flags.

[yuhaoth]

how about the relationship of MBEDTLS_SSL_SESSION_TICKETS and MBEDTLS_SSL_EARLY_DATA? Should they be binded together?

I prefer mandatory MBEDTLS_SSL_SESSION_TICKETS and external PSK optional. That's due to replay risk in https://datatracker.ietf.org/doc/html/rfc8446#section-8

[xkqian]

how about the relationship of MBEDTLS_SSL_SESSION_TICKETS and MBEDTLS_SSL_EARLY_DATA? Should they be binded together?

I prefer mandatory MBEDTLS_SSL_SESSION_TICKETS and external PSK optional. That's due to replay risk in https://datatracker.ietf.org/doc/html/rfc8446#section-8

---

--If we bind them together, there will be no optional external psk.

[ronald-cron-arm]

Regarding the support of early data through ticket and/or external PSK, the main use case is to me the ticket case. Thus I prefer to add support for it first. I know it is more complicated to test than using an external PSK but anyway we need to do it. As @yuhaoth pointed there are quite complex security issues to have in mind when using early data with an external PSK and if we want a quite complete support we would need to extend somehow mbedtls_ssl_conf_psk and mbedtls_ssl_conf_psk_opaque to define the TLS version and ciphersuite associated to the PSK. That's why I had in mind to not support early data through an external PSK (at least) to start with. As of now, I still think this is a good tradeoff thus I rather prefer to not add the support for early data through an external PSK. For some early data exchanges tests locally on your machines, you can always add the mbedtls_ssl_conf_has_static_psk( ssl->conf ) == 1 check in mbedtls_ssl_tls13_write_client_hello_exts().

[ronald-cron-arm]

how about the relationship of MBEDTLS_SSL_SESSION_TICKETS and MBEDTLS_SSL_EARLY_DATA? Should they be binded together?

I prefer mandatory MBEDTLS_SSL_SESSION_TICKETS and external PSK optional. That's due to replay risk in https://datatracker.ietf.org/doc/html/rfc8446#section-8

--If we bind them together, there will be no optional external psk.

If we support early data only through tickets (my preferred option) to start with, yes MBEDTLS_SSL_SESSION_TICKETS should be a requisite to MBEDTLS_SSL_EARLY_DATA. I do not think this would be a problem for local testing with external PSKs (as mentioned just above).

[ronald-cron-arm]

But I think that can be represent by max_early_data_size == 0 . I do not think we need ticket_flags.

Yes that could work but then we state that a NewSessionTicket without the early data indication extension is equivalent to a NewSessionTicket with an early data indication extension and max_early_data_size == 0. This is probably fine but if we could avoid I think I prefer. We will need a ticket_flags field anyway for allow_dhe_resumption and allow_psk_resumption flags as it seems to me that we miss them for the time being: "In order to use PSKs, clients MUST also send a "psk_key_exchange_modes" extension. The semantics of this extension are that the client only supports the use of PSKs with these modes, which restricts both the use of PSKs offered in this ClientHello and those which the server might supply via NewSessionTicket." (TLS 1.3 spec 4.2.9).

[yuhaoth]

Yes that could work but then we state that a NewSessionTicket without the early data indication extension is equivalent to a NewSessionTicket with an early data indication extension and max_early_data_size == 0. This is probably fine but if we could avoid I think I prefer.

As my understand, I think your meaning is

• when early_data not exists in NewSessionTicket, allow_early_data =1
• when early_data exists in NewSessionTicket and max_early_data_size == 0 , allow_early_data = 0
• when early_data exists in NewSessionTicket and max_early_data_size != 0 , allow_early_data = 1
  If yes, I agree allow_early_data should not be removed.

[yuhaoth]

We will need a ticket_flags field anyway for allow_dhe_resumption and allow_psk_resumption flags as it seems to me that we miss them for the time being

Understand. I misunderstand allow_dhe_resumption and allow_psk_resumption. And I just create #6551 for it. I post some actions in #6551, please check them.

[ronald-cron-arm]

Yes that could work but then we state that a NewSessionTicket without the early data indication extension is equivalent to a NewSessionTicket with an early data indication extension and max_early_data_size == 0. This is probably fine but if we could avoid I think I prefer.

As my understand, I think your meaning is

* when early_data not exists in NewSessionTicket, `allow_early_data =1`

* when early_data exists in NewSessionTicket and `max_early_data_size == 0` , `allow_early_data = 0 `

* when early_data exists in NewSessionTicket and `max_early_data_size != 0` ,  `allow_early_data = 1`
  If yes, I agree `allow_early_data` should not be removed.

I guess you rather mean "when early_data not exists in NewSessionTicket, allow_early_data = 0."
Otherwise I am not sure why we would handle max_early_data_size == 0 differently. For sure it is not very useful but not much less than max_early_data_size == 1 or 2. Thus I was rather thinking: allow_early_data = 1 <=> (equivalent) the early data extension is present in the NewSessionTicket.

[yuhaoth]

I guess you rather mean "when early_data not exists in NewSessionTicket, allow_early_data = 0." Otherwise I am not sure why we would handle max_early_data_size == 0 differently. For sure it is not very useful but not much less than max_early_data_size == 1 or 2. Thus I was rather thinking: allow_early_data = 1 <=> (equivalent) the early data extension is present in the NewSessionTicket.

Understand. I think we should rename allow_early_data to early_data_present .

[xkqian]

I guess you rather mean "when early_data not exists in NewSessionTicket, allow_early_data = 0." Otherwise I am not sure why we would handle max_early_data_size == 0 differently. For sure it is not very useful but not much less than max_early_data_size == 1 or 2. Thus I was rather thinking: allow_early_data = 1 <=> (equivalent) the early data extension is present in the NewSessionTicket.

Understand. I think we should rename allow_early_data to early_data_present .

What name should we used? MBEDTLS_SSL_TLS1_3_TICKET_ALLOW_EARLY_DATA? MBEDTLS_SSL_TLS1_3_TICKET_EARLY_DATA_PRESENT ? or MBEDTLS_SSL_TLS1_3_TICKET_HAS_EARLY_DATA_INDACTION? I am very confused about the naming rule, seems every review will have one different point, which one should we follow up?