ARM-software / psa-api

Documentation source and development of the PSA Certified API
https://arm-software.github.io/psa-api/
Other
49 stars 24 forks source link

Custom methods for key generation and key derivation #167

Open gilles-peskine-arm opened 5 months ago

gilles-peskine-arm commented 5 months ago

The PSA API should provide ways to customize how key material is created from random or pseudorandom inputs, i.e. key generation and key derivation. The scope here is for customizing the construction of keys (especially cooked keys, i.e. keys that aren't just chosen uniformly among bit-strings of a given length). Here are some possible use cases:

Urgency: Mbed TLS is interested in having a way to generate an RSA key with a chosen public exponent soon. Ideally we would like to include a beta version of this in our next release (code freeze: mid-March), but we don't know whether we'll have time to do the coding even.

My general idea is to define new functions psa_generate_key_ext() and psa_key_derivation_output_key_ext() (tentative names), which are similar to psa_generate_key() and psa_key_derivation_output_key() but take an extra parameter indicating the custom generation/derivation method. The type of the method parameter has a default value which means the same thing that the non-ext functions do.

gilles-peskine-arm commented 5 months ago

See https://github.com/Mbed-TLS/mbedtls/pull/8815 for an API proposal (with a prototype implementation).

gilles-peskine-arm commented 5 months ago

On Tuesday 13 Feb 2024, we decided to go ahead with the design in https://github.com/Mbed-TLS/mbedtls/pull/8815 at https://github.com/Mbed-TLS/mbedtls/pull/8815/commits/d8ba2b8716a1854c51b94ac69c24317b5ab65409 with the change from passing method_length to passing method_data_length as proposed in https://github.com/Mbed-TLS/mbedtls/pull/8815#discussion_r1487520828.

gilles-peskine-arm commented 4 months ago

A design question came up: if a function takes an input via parameters const psa_production_parameters_t *params, size_t params_length, it is not a “buffer” as defined by the parameter conventions. So according to the letter of the specification, as it stands, it would not allow overlap and would not require the implementation to take any precautions in an environment with shared memory.

However, *params does contain a variable-length buffer. Consider an implementation with domain separation where parameters are passed through shared memory (needing care in the implementation of the function) and small, fixed-size parameters are copied by the RPC code via one shared memory zone (so the function doesn't need any particular care). It would make sense for the RPC code to treat this variable-length parameter as a buffer that just happens to have a fixed-format header, and is passed directly as shared memory.

Alternatively, for the current use case, it would make sense for implementations to define a reasonably short PSA_PRODUCTION_PARAMETERS_MAX_SIZE (that's maybe something we should have anyway?), and then the RPC code can always transmit that many bytes as a fixed-size buffer. (Though it might still want to keep it variable-length because the max size could still be high by the standards of a constrained device — typically one RSA key length whereas most calls will actually use 0 bytes.)

What do you think @athoelke ?

athoelke commented 4 months ago

I think that as this has a variable length element, even if the current use cases do not have a wildly varying size, it might be best to categorise this as a 'buffer'-type parameter. This gives the implementation freedom to read-in-place, or to copy-to-cryptoprocessor (or decide at runtime based on the size of the parameter).

Note that this is an input parameter, and the calls in which it is used have (aside from the construction parameters):

Therefore, however we classify the construction parameters, it makes no material difference to the behavior of the implementation in terms of 'overlap' or 'stability' - it cannot reasonably overlap with the output key handle (without forcing the compiler to cast pointers), and overlap with other inputs is irrelevant.

gilles-peskine-arm commented 4 months ago

it makes no material difference to the behavior of the implementation in terms of 'overlap' or 'stability'

Overlap, no. But stability, yes. The implementation is allowed to assume stability for an input non-buffer (e.g. const psa_key_attributes_t *) but not for an input buffer (e.g. const uint8_t *key_data).

athoelke commented 4 months ago

Overlap, no. But stability, yes. The implementation is allowed to assume stability for an input non-buffer (e.g. const psa_key_attributes_t *) but not for an input buffer (e.g. const uint8_t *key_data).

But in this case what does 'assuming stability' look like?

For these two APIs, the only agent that might be modifying the buffer while the function is in progress is the application (or something else outside of the implementation) - there is no output parameter that can legally overlap with this input, so the implementation can assume that it will not corrupt this input parameter as a side-effect of its operation.

Are we suggesting that the implementation is permitted to be written in a way that might fail badly (i.e. crash, divulge secrets, generate an invalid/insecure key (but report success)) if the data in this parameter content is changed (by the caller) while the function is in progress? I would expect that implementations that do not trust the caller are written to mitigate such misbehavior.

athoelke commented 4 months ago

Perhaps the critical aspect here is that the flags field in the parameters is expected to control the behavior (code flow) within the implementation (e.g. by indicating a specific construction method), and the variable sized part is expected to provide data inputs to the construction methods. i.e. the flags will control the parsing of the variable length data.

Maybe ideally, the flags should be treated as a 'stable' parameter, but the data buffer not?

On the other hand, we already have an API that mixes control and data within a 'non-stable' parameter: for some key formats used with psa_import_key(), the input data includes control information that affects the parsing of the data in the buffer.

athoelke commented 2 months ago

As noted in https://github.com/ARM-software/psa-api/pull/194#discussion_r1557651496 and https://github.com/Mbed-TLS/mbedtls/issues/9020, this API is unsupportable in a project that might be included and called from C++ source code.

We will have to separate the structure-like parts of the production parameters, from any variable-sized parameters. The suggestion is to rework the API definition to the following:

typedef struct psa_key_production_parameters_t {
    uint32_t flags;
    /* imp-def */
} psa_key_production_parameters_t;

psa_status_t psa_generate_key_ext(const psa_key_attributes_t *attributes,
                                  const psa_key_production_parameters_t *params,
                                  const uint8_t *params_data,
                                  size_t params_data_length,
                                  mbedtls_svc_key_id_t *key);

psa_status_t psa_key_derivation_output_key_ext(const psa_key_attributes_t *attributes,
                                               psa_key_derivation_operation_t *operation,
                                               const psa_key_production_parameters_t *params,
                                               const uint8_t *params_data,
                                               size_t params_data_length,
                                               mbedtls_svc_key_id_t *key);

The unbounded array data[] is removed from the production parameter structure, and passed as a separate params_data buffer to the extended key generation and derivation functions. The psa_key_production_parameters_t structure is retained to initially contain the flags that indicate the remaining parameter content, and for addition of fixed-size parameter values in future specifications (or for implementation-specific key-production procedures).

If params.flags == 0 && params_data_length == 0 then these APIs will use the default key generation/derivation procedures. Note that (params_data, params_data_length) is now a standard input buffer parameter.

This alternative API is now compatible with C++ compilation, but also no longer requires special consideration of whether a parameter is a 'buffer-type parameter'.