Refactor key derivation structure for the driver interface

[gilles-peskine-arm]

This is the first step of the implementation of the interface for key derivation drivers introduced in #5451.

A salient feature of this interface is that a key derivation operation undergoes two phases. First the core collects inputs. When all initial inputs have been collected, the core invokes the driver's key_derivation_setup entry point, which starts phase 2. During phase 2, the core can retrieve outputs from drivers. In the key_derivation_setup entry point, the driver reads the inputs via getters on a psa_crypto_driver_key_derivation_inputs_t structure.

Goals of this issue:

• Refactor the key derivation data structures (struct psa_key_derivation_s and subtructures) to separate the data used by phase 1 (driver-independent) and the data used by phase 2 (specific to our implementation). Call the structure used for phase 1 psa_crypto_driver_key_derivation_inputs_t.
• Refactor the existing code accordingly, with no expected behavior change. Create a new function psa_crypto_key_derivation_driver_setup, which will become our implementation of the key_derivation_setup entry point, which copies data from the psa_crypto_driver_key_derivation_inputs_t structure to our implementation-specific structure.

Note that struct psa_key_derivation_s will now contain two different things before and after calling psa_crypto_key_derivation_driver_setup, so it should probably contain a union of psa_crypto_driver_key_derivation_inputs_t and something similar to the current unnamed union of psa_hkdf_key_derivation_t and psa_tls12_prf_key_derivation_t. This second structure will become psa_driver_key_derivation_context_t.

Note that psa_crypto_key_derivation_driver_setup can only be called once all the initial inputs are available. For HKDF, the salt is optional, which allows the following call sequence from the application: setup, input secret, input info, output, abort. The core doesn't know that the application didn't pass a salt until it calls an output function. Therefore, at least in this case, psa_crypto_key_derivation_driver_setup needs to be called from output_bytes. Currently, it's always correct to wait until the first output call to call psa_crypto_key_derivation_driver_setup, although this will not be the case in the future when we implement KDF algorithms with long inputs.

I think there are enough unit tests for this refactoring, but feel free to add more if you think it's warranted.

[mprse]

Currently we have the following inputs for psa_hkdf_key_derivation_t and psa_tls12_prf_key_derivation_t structures:

psa_hkdf_key_derivation_t	psa_tls12_prf_key_derivation_t
info	secret
info_length	secret_length
	seed
	seed_length
	label
	label_length
	other_secret
	other_secret_length

In case of psa_tls12_prf_key_derivation_t we have dedicated field for each input, but psa_hkdf_key_derivation_t has only info and seed and secret are processed on fly to generate prk (not stored into buffers). If we want to separate derivation process into two phases: collecting inputs and generating output I think we need to add also seed and secret to psa_hkdf_key_derivation_t ?

[gilles-peskine-arm]

Indeed, we won't be able to do the partial on-the-fly processing in HKDF anymore.

[mprse]

Note that struct psa_key_derivation_s will now contain two different things before and after calling psa_crypto_key_derivation_driver_setup, so it should probably contain a union of psa_crypto_driver_key_derivation_inputs_t and something similar to the current unnamed union of psa_hkdf_key_derivation_t and psa_tls12_prf_key_derivation_t. This second structure will become psa_driver_key_derivation_context_t.

This part is still unclear to me.

We will have 2 structures:

• new structure that holds inputs: psa_crypto_driver_key_derivation_inputs_t
• existing psa_key_derivation_s with added seed and secret to psa_hkdf_key_derivation_t

New flow:

• xxx_input() fills the requested input in psa_crypto_driver_key_derivation_inputs_t
• psa_key_derivation_setup() calls psa_crypto_key_derivation_driver_setup() that copies collected inputs from psa_crypto_driver_key_derivation_inputs_t to psa_key_derivation_s
• derivation of the output key

I'm missing something, because I can't see why psa_crypto_driver_key_derivation_inputs_t should be part of psa_key_derivation_s ?

[gilles-peskine-arm]

why psa_crypto_driver_key_derivation_inputs_t should be part of psa_key_derivation_s

Between setup() and the first output() call (which is when the core knows that all inputs are present, at least when the inputs don't all have to be passed in order), the inputs have to be stored somewhere. The only place is inside psa_key_derivation_s.

[mprse]

Note that struct psa_key_derivation_s will now contain two different things before and after calling psa_crypto_key_derivation_driver_setup, so it should probably contain a union of psa_crypto_driver_key_derivation_inputs_t and something similar to the current unnamed union of psa_hkdf_key_derivation_t and psa_tls12_prf_key_derivation_t. This second structure will become psa_driver_key_derivation_context_t.

I have problems with understanding the point of this issue. I understand that the goal is to separate inputs from implementation specific fields, but the note are unclear for me.

I think the first step here is to find agreement on the new the key derivation data structures, then maybe it become more clear to me.

For example regarding the union that consists of psa_crypto_driver_key_derivation_inputs_t and psa_crypto_driver_key_derivation_impl_t (something similar to the current unnamed union of psa_hkdf_key_derivation_t and psa_tls12_prf_key_derivation_t).
Inputs are provided as pointers to buffers and size. While collecting the inputs we will allocate memory for the inputs in psa_crypto_driver_key_derivation_inputs_t. Then when all inputs are collected and first output() is called we will use psa_crypto_key_derivation_driver_setup() to copy the inputs to the psa_crypto_driver_key_derivation_impl_t, so we will have to allocate new buffers, copy and free previous. This sounds odd especially we have a union here and it shares the memory for fields, so we could have shared inputs between psa_crypto_driver_key_derivation_inputs_t and psa_hkdf_key_derivation_t/psa_tls12_prf_key_derivation_t. Then no copy is needed.

Proposition for the new key derivation data structure can be found here: https://github.com/mprse/mbedtls/blob/8bd2f702935e7bd4b9042de7056a3bf88532298a/include/psa/crypto_struct.h#L185-L301

[gilles-peskine-arm]

The eventual goal is to implement the driver interface for key derivation. Currently the interface between the API functions and the implementations of individual KDFs is just ad hoc code sometimes broken into functions. We need to have a uniform interface that drivers can plug into, and that interface is specified in #5451.

The main change is that there can't be ad hoc consuming of inputs anymore.

Inputs are provided as pointers to buffers and size. While collecting the inputs we will allocate memory for the inputs in psa_crypto_driver_key_derivation_inputs_t. Then when all inputs are collected and first output() is called we will use psa_crypto_key_derivation_driver_setup() to copy the inputs to the psa_crypto_driver_key_derivation_impl_t, so we will have to allocate new buffers, copy and free previous. This sounds odd

I think the extra copy is necessary because the core doesn't know what the driver will do with the input — and that depends on the diver and the algorithm. It may be possible to optimize this copy away but that would complicate the interface: we'd trade a more efficient software implementation against less efficient driver implementations. I'm not sure that trade-off is worth it. I'm open to proposals, but I can't really tell anything from the type alone: I'd have to see how a driver can be implemented.

[mprse]

Created a draft PR: #5867 with the solution proposal (only last 2 commits).