Review: Key generation

[hanno-becker]

The purpose of this task is to review the following functions and prepare them for upstreaming:

• mbedtls_ssl_generate_handshake_traffic_keys()
• mbedtls_ssl_tls1_3_derive_master_secret()
• mbedtls_ssl_generate_resumption_master_secret()
• mbedtls_ssl_generate_application_traffic_keys()
• mbedtls_ssl_generate_early_data_keys()
• mbedtls_ssl_create_binder()

Those are mostly simple wrappers around the HKDF-based key derivation functions of TLS 1.3 which have already been upstreamed.

Those functions operate on the whole SSL context. Ideally, they'd be standalone like the other key schedule functions. If that's no feasible, it should be documented which fields of the SSL context are required as input, and which fields are being set -- this gives us a chance to write some unit tests nonetheless.

Acceptance criterion:

• PRs have been filed and merged to the prototype to prepare the above functions for upstreaming.
• A PR is open on upstream Mbed TLS, upstreaming all of ssl_tls13_keys.[ch] as it is in the prototype.

[hanno-becker]

TODOs noted on first review pass:

About mbedtls_ssl_tls1_3_derive_master_secret()` has been removed

• Add internal structure mbedtls_ssl_handshake_secrets (or similar) holding triple of secrets: 0-RTT, HS, Master
• Compute PSK and ECDHE secret prior to mbedtls_ssl_tls1_3_derive_master_secret()
• Make mbedtls_ssl_tls1_3_derive_master_secret() standalone by passing PSK + ECDHE secret as input, and taking a pointer to a mbedtls_ssl_handshake_secrets structure as the output.
• Add KATs to the SSL unit test suite.

About: mbedtls_ssl_create_binder()

• Reuse mbedtls_ssl_tls1_3_evolve_secret() in the early_keygeneration in mbedtls_ssl_create_binder(). Currently this is calling the HKDF API directly.
• Make the current handshake transcript an explicit parameter for mbedtls_ssl_create_binder(): This makes it easier to unit test, and avoids re-calculations the handshake transcript every time in case we offer multiple PSKs.
• Rename to mbedtls_ssl_tls1_3_create_psk_binder()

About mbedtls_ssl_generate_resumption_master_secret():

• Use mbedtls_ssl_get_handshake_transcript() in mbedtls_ssl_generate_resumption_master_secret(), or make the handshake transcript a parameter.

About mbedtls_ssl_generate_handshake_traffic_keys():

• Use mbedtls_ssl_get_handshake_transcript() in mbedtls_ssl_generate_handshake_traffic_keys(), or make the handshake transcript a parameter.
• Remove calculation of exporter master secret (functionally wrong here, see Fix calculation of Exporter Master Secret  #203)
• Don't set verify hash function pointers in this function - this is unrelated.

About mbedtls_ssl_handshake_key_derivation():

• mbedtls_ssl_handshake_key_derivation() shouldn't modify the messaging layer. Move that up to the caller.
  About mbedtls_ssl_generate_resumption_master_secret():

About mbedtls_ssl_generate_early_data_keys():

• Use mbedtls_ssl_get_handshake_transcript(), or make transcript a parameter

About mbedtls_ssl_generate_application_traffic_keys():

• Add calculation of exporter master secret.
• Application traffic secrets cannot be stored in the handshake structure because they are needed beyond the initial handshake (for key update).

[hanno-becker]

The early data key generation is hard to follow at the moment. The logic is this: When establishing which PSK to use, the peers call mbedtls_ssl_create_binder() to compute the PSK binder from a PSK. As a somewhat hidden intermediate step in this derivation, the early secret is computed and stored in the handshake structure, overwriting the previous one if present, that is, when multiple PSKs are considered. In particular, only the one from the last call to mbedtls_ssl_create_binder() will survive. Once a PSK is chosen, mbedtls_ssl_generate_early_data_keys() is called which then builds on this early secret to derive 0-RTT keys.

While it may be true that the server stops parsing PSK binders immediately once it found an acceptable PSK, and thus the last call to mbedtls_ssl_create_binder() is indeed the one establishing the early secret to use, this logic is difficult to follow because you can't tell from the function invocations when the early secret is actually established.
Moreover, should the client ever offer more than one PSK, it simply doesn't work anymore, and we have to re-generate the early secret once we know which PSK the server has chosen.

Finally, the early secret is then re-generated in mbedtls_ssl_derive_master_secret(), which is unnecessary and slightly confusing.

[hanno-becker]

Suggestion: mbedtls_ssl_create_binder() should use a local buffer to hold the early secret for the PSK under consideration. Once it's clear which PSK will be used, mbedtls_ssl_generate_early_keys() will be called to establish both the early secret, as well as keys derived from it. Finally, mbedtls_ssl_derive_master_secret() would build on the early secret instead of re-calculating it.

[hanno-becker]

Logically, we need to take care of the following layers of key schedule:

1. Key evolution: Empty -> Early Secret -> Handshake Secret -> Master Secret
2. Secret derivation per
   • Early Secret: Binder (not needed), client 0-RTT keys, early exporter
   • Handshake: Client and server handshake secrets
   • Master: Client and server application secrets, resumption master, exporter master
3. Key derivation per secret

Plus, one function specifically for the PSK -> PSK binder derivation.

The core functions for those steps are

1. mbedtls_ssl_tls1_3_evolve_secret()
2. mbedtls_ssl_tls1_3_derive_secret()
3. mbedtls_ssl_tls1_3_make_traffic_keys()

all of which have already been upstreamed.

The functions that this issue is about are the convenience functions built on top of this. Specifically, the following functions provide some flattening of (2)+(3):

• mbedtls_ssl_generate_handshake_traffic_keys()
• mbedtls_ssl_generate_application_traffic_keys()
• mbedtls_ssl_generate_early_data_keys()

Nitpick: Not every secret generated in (2) is converted to a traffic secret in (3). For example, the exporter secrets aren't. Should we reflect this in the naming of the functions? E.g. mbedtls_ssl_tls1_3_derive_{handshake,application,early_data}_key_material()?

Beyond that, we currently have the following:

• mbedtls_ssl_tls1_3_derive_master_secret()
  This is a concatenation of all key evolutions, which as explained above leads to duplicated early secret computation. We should either split this in two functions, one for the handshake secret, and one for the master secret, or at least remove the duplicated early secret computation.
• mbedtls_ssl_generate_resumption_master_secret()
  This should be removed and the code integrated into mbedtls_ssl_generate_application_traffic_keys().
• mbedtls_ssl_create_binder()
  This stands apart as it needs to be called multiple times, once per PSK. It should be kept but improved on as detailed above.