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.
Secure element support has its own source file, and in addition
requires many hooks in other files. This is a nontrivial amount of
code, so make it optional (but default on).
PSA_ERROR_BAD_STATE means that the function was called on a context in a
bad state.
This error is something that can't happen while only using the PSA API and
therefore a PSA_ERROR_CORRUPTION_DETECTED is a more appropriate error
code.
The macro initialiser might leave bytes in the union unspecified.
Zeroising it in setup makes sure that the behaviour is the same
independently of the initialisation method used.
The TLS 1.2 pseudorandom function does a lot of distinct HMAC operations
with the same key. To save the battery and CPU cycles spent on
calculating the paddings and hashing the inner padding, we keep the
hash context in the status right after the inner padding having been
hashed and clone it as needed.
Technically we could have reused the old one for the new API, but then
we had to set an extra field during setup. The new version works when
all the fields that haven't been set explicitely are zero-initialised.
The specific key derivation input functions support a subset of the
input options and need to check it anyway. Checking it at the top level
is redundant, it brings a very little value and comes with a cost in
code size and maintainability.
This change affects the psa_key_derivation_s structure. With the buffer
removed from the union, it is empty if MBEDTLS_MD_C is not defined.
We can avoid undefined behaviour by adding a new dummy field that is
always present or make the whole union conditional on MBEDTLS_MD_C.
In this latter case the initialiser macro has to depend on MBEDTLS_MD_C
as well. Furthermore the first structure would be either
psa_hkdf_key_derivation_t or psa_tls12_prf_key_derivation_t both of
which are very deep and would make the initialisation macro difficult
to maintain, therefore we go with the first option.
Some key derivation operation contexts (like
psa_tls12_prf_key_derivation_t) directly contain buffers with parts of
the derived key. Erase them safely as part of the abort.
Add the compile time option PSA_PRE_1_0_KEY_DERIVATION. If this is not
turned on, then the function `psa_key_derivation()` is removed.
Most of the tests regarding key derivation haven't been adapted to the
new API yet and some of them have only been adapted partially. When this
new option is turned off, the tests using the old API and test cases
using the old API of partially adapted tests are skipped.
The sole purpose of this option is to make the transition to the new API
smoother. Once the transition is complete it can and should be removed
along with the old API and its implementation.
When MBEDTLS_PSA_INJECT_ENTROPY is used, we now require also defining
MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES. When
MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES is defined, we do not add entropy
sources by default. This includes the NV seed entropy source, which the
PSA entropy injection API is built upon.
The PSA entropy injection feature depends on using NV seed as an entropy
source. Add NV seed as an entropy source for PSA entropy injection.
Fixes e3dbdd8d90 ("Gate entropy injection through a dedicated configuration option")
Now that psa_allocate_key() is no longer a public function, expose
psa_internal_allocate_key_slot() instead, which provides a pointer to
the slot to its caller.
Remove the key creation functions from before the attribute-based API,
i.e. the key creation functions that worked by allocating a slot, then
setting metadata through the handle and finally creating key material.
When importing a private elliptic curve key, require the input to have
exactly the right size. RFC 5915 requires the right size (you aren't
allowed to omit leading zeros). A different buffer size likely means
that something is wrong, e.g. a mismatch between the declared key type
and the actual data.
generate_key is a more classical name. The longer name was only
introduced to avoid confusion with getting a key from a generator,
which is key derivation, but we no longer use the generator
terminology so this reason no longer applies.
perl -i -pe 's/psa_generate_random_key/psa_generate_key/g' $(git ls-files)
“Tampering detected” was misleading because in the real world it can
also arise due to a software bug. “Corruption detected” is neutral and
more precisely reflects what can trigger the error.
perl -i -pe 's/PSA_ERROR_TAMPERING_DETECTED/PSA_ERROR_CORRUPTION_DETECTED/gi' $(git ls-files)
Generators are mostly about key derivation (currently: only about key
derivation). "Generator" is not a commonly used term in cryptography.
So favor "derivation" as terminology. Call a generator a key
derivation operation structure, since it behaves like other multipart
operation structures. Furthermore, the function names are not fully
consistent.
In this commit, I rename the functions to consistently have the prefix
"psa_key_derivation_". I used the following command:
perl -i -pe '%t = (
psa_crypto_generator_t => "psa_key_derivation_operation_t",
psa_crypto_generator_init => "psa_key_derivation_init",
psa_key_derivation_setup => "psa_key_derivation_setup",
psa_key_derivation_input_key => "psa_key_derivation_input_key",
psa_key_derivation_input_bytes => "psa_key_derivation_input_bytes",
psa_key_agreement => "psa_key_derivation_key_agreement",
psa_set_generator_capacity => "psa_key_derivation_set_capacity",
psa_get_generator_capacity => "psa_key_derivation_get_capacity",
psa_generator_read => "psa_key_derivation_output_bytes",
psa_generate_derived_key => "psa_key_derivation_output_key",
psa_generator_abort => "psa_key_derivation_abort",
PSA_CRYPTO_GENERATOR_INIT => "PSA_KEY_DERIVATION_OPERATION_INIT",
PSA_GENERATOR_UNBRIDLED_CAPACITY => "PSA_KEY_DERIVATION_UNLIMITED_CAPACITY",
); s/\b(@{[join("|", keys %t)]})\b/$t{$1}/ge' $(git ls-files)
When importing a private elliptic curve key, require the input to have
exactly the right size. RFC 5915 requires the right size (you aren't
allow to omit leading zeros). A different buffer size likely means
that something is wrong, e.g. a mismatch between the declared key type
and the actual data.
In psa_import_key, change the order of parameters to pass
the pointer where the newly created handle will be stored last.
This is consistent with most other library functions that put inputs
before outputs.
In psa_generate_derived_key, change the order of parameters to pass
the pointer where the newly created handle will be stored last.
This is consistent with most other library functions that put inputs
before outputs.
