refactor AEAD API following changes in QUIC draft-11

[kazuho]

The AEAD API of picotls has been designed in hope that the only difference between the traffic key derivation functions of TLS 1.3 and QUIC will be the value of the base_label.

However, the function definition of QUIC has been changed recently, and draft-11 has shipped without reverting the change (see quicwg/base-drafts#1255).

This PR changes the AEAD API of picotls so that it would be easier for the users to implement draft-11-style AEAD, as well as removing the abstraction that is no longer necessary.

https://gist.github.com/kazuho/6ace6cb277e977b89f283be7631b977f contains a code snippet that can be used to create an AEAD context for QUIC draft-11.

[huitema]

I wrote my own key setting routines to match the QUIC specification. They call ptls_hkdf_expand() and aead->setup_crypto() for an aead context initialized with an algorithm. As long I these two APi are maintained, I am fine.

Of course there would be benefit in not having that code in Picoquic, but I need to better understand the intent of your API, and that's difficult to do from just the list of changes.

[huitema]

Is this function meant to be generic, or specific to TLS13?

[kazuho]

@huitema

Of course there would be benefit in not having that code in Picoquic, but I need to better understand the intent of your API, and that's difficult to do from just the list of changes.

Sorry for not giving a technical summary of what the change is.

I consider the setup_crypto callback to be a private interface (i.e. a contract only between picotls and its crypto bindings). Therefore, we need to change the current API in some way to support QUIC draft-11.

From the change in draft-11, we have learnt that each protocol that uses an AEAD cipher might define its own key deviation function, rather than reusing the one defined in TLS 1.3.

Therefore, we need to provide an API that looks like something that has been suggested in quicwg/base-drafts#1256 (review),

For mine, a more principled AEAD API would take two labels: one for the key and one for the label, and avoid baking in the specific details of the TLS HKDF info structure.

This PR makes such changes. Specifically:

• ptls_aead_new function is changed to take the encryption key as an argument, and users are required to setup the IV after calling the new function
• hkdf_expand_label becomes a private function. It is assumed that each application will have their own key deviation function
• for convenience, the key deviation function for QUIC draft-11 is provided on gist https://gist.github.com/kazuho/6ace6cb277e977b89f283be7631b977f

[larseggert]

I looked at the diff and the gist, but it's not clear to me how I would construct the cleartext secrets with these new functions?

[kazuho]

@larseggert Yeah it was not obvious.

Regarding the gist, what you might do is just copy the two functions on the gist to your source code, and replace existing calls to ptls_new_aead with new_aead, omitting the last argument that used to specify the base label.

get_traffic_key is the function used to construct the key and the IV from the secret, however it only need to be called from the function that creates the AEAD context (i.e. new_aead).

[larseggert]

Yes, I managed to figure out how to use new_aead instead of ptls_new_aead.

What I am missing is how to replace the call to ptls_hkdf_expand_label when constructing the handshake secret.

[kazuho]

@larseggert Ah. Now I see what you are asking for. Sorry for the confusion.

get_traffic_key is actually doing what qhkdf_expand is supposed to do. I have updated the gist renaming the function to qhkdf_expand.

As an example, quicly is at -09 at the moment. I expect that I would be making a change like the following to support -11.

diff --git a/lib/quicly.c b/lib/quicly.c
index 2bea619..7662290 100644
--- a/lib/quicly.c
+++ b/lib/quicly.c
@@ -545,9 +545,7 @@ static int setup_handshake_secret(ptls_aead_context_t **aead, ptls_cipher_suite_
     uint8_t aead_secret[PTLS_MAX_DIGEST_SIZE];
     int ret;
 
-    if ((ret = ptls_hkdf_expand_label(cs->hash, aead_secret, cs->hash->digest_size,
-                                      ptls_iovec_init(master_secret, cs->hash->digest_size), label, ptls_iovec_init(NULL, 0),
-                                      QHKDF_BASE_LABEL)) != 0)
+    if ((ret = qhkdf_expand(cs->hash, aead_secret, cs->hash->digest_size, master_secret, label)) != 0)
         goto Exit;
     if ((*aead = ptls_aead_new(cs->aead, cs->hash, is_enc, aead_secret, QHKDF_BASE_LABEL)) == NULL) {
         ret = PTLS_ERROR_NO_MEMORY;

[larseggert]

Thank you! That works great. So as far as I am concerned, this is fine to merge.

[kazuho]

Thank you for checking. Let's merge, now that we know it works.