In psa_asymmetric_sign, immediately reject an empty signature buffer.
This can never be right.
Add test cases (one RSA and one ECDSA).
Change the SE HAL mock tests not to use an empty signature buffer.
Zero-length keys are rejected at creation time, so we don't need any
special handling internally.
When exporting a key, we do need to take care of the case where the
output buffer is empty, but this is easy: an empty output buffer is
never valid.
At the end of `psa_hmac_setup_internal()`, the ipad is cleared.
However, the size that was given to clear was `key_len` which is larger
than the size of `ipad`.
In psa_import_key, the key bits value was uninitialized before
calling the secure element driver import function. There is a
potential issue if the driver returns PSA_SUCCESS without setting
the key bits. This shouldn't happen, but shouldn't be discounted
either, so we initialize the key bits to an invalid issue.
Adopt a simple method for tracking whether there was a failure: each
fallible operation sets overall_status, unless overall_status is
already non-successful. Thus in case of multiple failures, the
function always reports whatever failed first. This may not always be
the right thing, but it's simple.
This revealed a bug whereby if the only failure was the call to
psa_destroy_se_key(), i.e. if the driver reported a failure or if the
driver lacked support for destroying keys, psa_destroy_key() would
ignore that failure.
For a key in a secure element, if creating a transaction file fails,
don't touch storage, but close the key in memory. This may not be
right, but it's no wronger than it was before. Tracked in
https://github.com/ARMmbed/mbed-crypto/issues/215
When a key slot is wiped, a copy of the key material may remain in
operations. This is undesirable, but does not violate the safety of
the code. Tracked in https://github.com/ARMmbed/mbed-crypto/issues/86
The methods to import and generate a key in a secure element drivers
were written for an earlier version of the application-side interface.
Now that there is a psa_key_attributes_t structure that combines all
key metadata including its lifetime (location), type, size, policy and
extra type-specific data (domain parameters), pass that to drivers
instead of separate arguments for each piece of metadata. This makes
the interface less cluttered.
Update parameter names and descriptions to follow general conventions.
Document the public-key output on key generation more precisely.
Explain that it is optional in a driver, and when a driver would
implement it. Declare that it is optional in the core, too (which
means that a crypto core might not support drivers for secure elements
that do need this feature).
Update the implementation and the tests accordingly.
Register an existing key in a secure element.
Minimal implementation that doesn't call any driver method and just
lets the application declare whatever it wants.
Pass the key creation method (import/generate/derive/copy) to the
driver methods to allocate or validate a slot number. This allows
drivers to enforce policies such as "this key slot can only be used
for keys generated inside the secure element".
Let psa_start_key_creation know what type of key creation this is. This
will be used at least for key registration in a secure element, which
is a peculiar kind of creation since it uses existing key material.
Allow the application to choose the slot number in a secure element,
rather than always letting the driver choose.
With this commit, any application may request any slot. In an
implementation with isolation, it's up to the service to filter key
creation requests and apply policies to limit which applications can
request which slot.
This function no longer modifies anything, so it doesn't actually
allocate the slot. Now, it just returns the empty key slot, and it's
up to the caller to cause the slot to be in use (or not).
Add a slot_number field to psa_key_attributes_t and getter/setter
functions. Since slot numbers can have the value 0, indicate the
presence of the field via a separate flag.
In psa_get_key_attributes(), report the slot number if the key is in a
secure element.
When creating a key, for now, applications cannot choose a slot
number. A subsequent commit will add this capability in the secure
element HAL.
Add infrastructure for internal, external and dual-use flags, with a
compile-time check (if static_assert is available) to ensure that the
same numerical value doesn't get declared for two different purposes
in crypto_struct.h (external or dual-use) and
psa_crypto_core.h (internal).
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.
Only allow creating keys in the application (user) range. Allow
opening keys in the implementation (vendor) range as well.
Compared with what the implementation allowed, which was undocumented:
0 is now allowed; values from 0x40000000 to 0xfffeffff are now
forbidden.
Make it a little easier to add ChaCha20-Poly1305.
This also fixes the error code in case mbedtls_gcm_setkey() fails with
a status that doesn't map to INVALID_ARGUMENT.
In psa_import_key and psa_copy_key, some information comes from the
key data (input buffer or source key) rather than from the attributes:
key size for import, key size and type and domain parameters for copy.
If an unused attribute is nonzero in the attribute structure, check
that it matches the correct value. This protects against application
errors.
Read extra data from the domain parameters in the attribute structure
instead of taking an argument on the function call.
Implement this for RSA key generation, where the public exponent can
be set as a domain parameter.
Add tests that generate RSA keys with various public exponents.
Change psa_get_domain_parameters() and psa_set_domain_parameters() to
access a psa_key_attributes_t structure rather than a key handle.
In psa_get_key_attributes(), treat the RSA public exponent as a domain
parameter and read it out. This is in preparation for removing the
`extra` parameter of psa_generate_key() and setting the RSA public
exponent for key generation via domain parameters.
In this commit, the default public exponent 65537 is not treated
specially, which allows us to verify that test code that should be
calling psa_reset_key_attributes() after retrieving the attributes of
an RSA key is doing so properly (if it wasn't, there would be a memory
leak), even if the test data happens to use an RSA key with the
default public exponent.
Instead of passing a separate parameter for the key size to
psa_generate_key and psa_generator_import_key, set it through the
attributes, like the key type and other metadata.
Implement attribute querying.
Test attribute getters and setters. Use psa_get_key_attributes instead
of the deprecated functions psa_get_key_policy or
psa_get_key_information in most tests.
Implement the new, attribute-based psa_import_key and some basic
functions to access psa_key_attributes_t. Replace
psa_import_key_to_handle by psa_import_key in a few test functions.
This commit does not handle persistence attributes yet.
