mbedtls_ctr_drbg_random can only return up to
MBEDTLS_CTR_DRBG_MAX_REQUEST (normally 1024) bytes at a time. So if
more than that is requested, call mbedtls_ctr_drbg_random in a loop.
When psa_generate_random fails, psa_generate_key_internal frees the
key buffer but a the pointer to the now-freed buffer in the slot. Then
psa_generate_key calls psa_fail_key_creation which sees the pointer
and calls free() again.
This bug was introduced by ff5f0e7221
"Implement atomic-creation psa_{generate,generator_import}_key" which
changed how psa_generate_key() cleans up on errors. I went through the
code and could not find a similar bug in cleanup on an error during
key creation.
Fix#207
Conflict resolution:
* `scripts/config.pl`:
Take the exclusion of `MBEDTLS_PSA_CRYPTO_SE_C` from the API branch.
Take the removal of `MBEDTLS_PSA_CRYPTO_STORAGE_ITS_C` (obsolete) from
the development branch.
* `tests/scripts/all.sh`:
Multiple instances of factoring a sequence of `config.pl` calls into
a mere `config.pl baremetal` in the development branch, and a change in
the composition of `baremetal` in the API branch. In each case, take the
version from development.
* `tests/suites/test_suite_psa_crypto_slot_management.function`:
A function became non-static in development and disappeared in the API
branch. Keep the version from the API branch. Functions need to be
non-static if they're defined but unused in some configurations,
which is not the case for any function in this file at the moment.
* `tests/suites/test_suite_psa_crypto.function`:
Consecutive changes in the two branches, reconciled.
The flag to mark key slots as allocated was introduced to mark slots
that are claimed and in use, but do not have key material yet, at a
time when creating a key used several API functions: allocate a slot,
then progressively set its metadata, and finally create the key
material. Now that all of these steps are combined into a single
API function call, the notion of allocated-but-not-filled slot is no
longer relevant. So remove the corresponding flag.
A slot is occupied iff there is a key in it. (For a key in a secure
element, the key material is not present, but the slot contains the
key metadata.) This key must have a type which is nonzero, so use this
as an indicator that a slot is in use.
There is now a field for the key size in the key slot in memory. Use
it.
This makes psa_get_key_attributes() marginally faster at the expense
of memory that is available anyway in the current memory layout (16
bits for the size, 16 bits for flags). That's not the goal, though:
the goal is to simplify the code, in particular to make it more
uniform between transparent keys (whose size can be recomputed) and
keys in secure elements (whose size cannot be recomputed).
For keys in a secure element, the bit size is now saved by serializing
the type psa_key_bits_t (which is an alias for uint16_t) rather than
size_t.
Change the type of key slots in memory to use
psa_core_key_attributes_t rather than separate fields. The goal is to
simplify some parts of the code. This commit only does the mechanical
replacement, not the substitution.
The bit-field `allocate` is now a flag `PSA_KEY_SLOT_FLAG_ALLOCATED`
in the `flags` field.
Write accessor functions for flags.
Key slots now contain a bit size field which is currently unused.
Subsequent commits will make use of it.
65528 bits is more than any reasonable key until we start supporting
post-quantum cryptography.
This limit is chosen to allow bit-sizes to be stored in 16 bits, with
65535 left to indicate an invalid value. It's a whole number of bytes,
which facilitates some calculations, in particular allowing a key of
exactly PSA_CRYPTO_MAX_STORAGE_SIZE to be created but not one bit
more.
As a resource usage limit, this is arguably too large, but that's out
of scope of the current commit.
Test that key import, generation and derivation reject overly large
sizes.
Move the "core attributes" to a substructure of psa_key_attribute_t.
The motivation is to be able to use the new structure
psa_core_key_attributes_t internally.
For a key in a secure element, save the bit size alongside the slot
number.
This is a quick-and-dirty implementation where the storage format
depends on sizeof(size_t), which is fragile. This should be replaced
by a more robust implementation before going into production.
Add a parameter to the key import method of a secure element driver to
make it report the key size in bits. This is necessary (otherwise the
core has no idea what the bit-size is), and making import report it is
easier than adding a separate method (for other key creation methods,
this information is an input, not an output).
Nothing has been saved to disk yet, but there is stale data in
psa_crypto_transaction. This stale data should not be reused, but do
wipe it to reduce the risk of it mattering somehow in the future.
Introduce a new function psa_get_transparent_key which returns
NOT_SUPPORTED if the key is in a secure element. Use this function in
functions that don't support keys in a secure element.
After this commit, all functions that access a key slot directly via
psa_get_key_slot or psa_get_key_from_slot rather than via
psa_get_transparent_key have at least enough support for secure
elements not to crash or otherwise cause undefined behavior. Lesser
bad behavior such as wrong results or resource leakage is still
possible in error cases.
Pass information via a key attribute structure rather than as separate
parameters to psa_crypto_storage functions. This makes it easier to
maintain the code when the metadata of a key evolves.
This has negligible impact on code size (+4B with "gcc -Os" on x86_64).
Key creation and key destruction for a key in a secure element both
require updating three pieces of data: the key data in the secure
element, the key metadata in internal storage, and the SE driver's
persistent data. Perform these actions in a transaction so that
recovery is possible if the action is interrupted midway.
When creating a key with a lifetime that places it in a secure
element, retrieve the appropriate driver table entry.
This commit doesn't yet achieve behavior: so far the code only
retrieves the driver, it doesn't call the driver.
The psa_tls12_prf_set_seed() and psa_tls12_prf_set_label() functions did
not work on platforms where malloc(0) returns NULL.
It does not affect the TLS use case but these PRFs are used in other
protocols as well and might not be used the same way. For example EAP
uses the TLS PRF with an empty secret. (This would not trigger the bug,
but is a strong indication that it is not safe to assume that certain
inputs to this function are not zero length.)
The conditional block includes the memcpy() call as well to avoid
passing a NULL pointer as a parameter resulting in undefined behaviour.
The current tests are already using zero length label and seed, there is
no need to add new test for this bug.
