Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better error handling when destroying a key in a secure element #215

Closed
gilles-peskine-arm opened this issue Aug 13, 2019 · 2 comments
Closed

Comments

@gilles-peskine-arm
Copy link
Collaborator

Description

Consider psa_destroy_key on a key which is located in a secure element. The normal process is:

  1. Create a transaction file.
  2. Call the driver to destroy the key in the secure element.
  3. Destroy the key metadata in internal storage.
  4. Remove the transaction file.

If an error occurs, we normally do as much as we can anyway. But if creating the transaction file fails, we do nothing. This is undesirable because psa_destroy_key should make the best effort to destroy all key material.

In particular, it's currently impossible to destroy a key if the storage is full, because creating the transaction file fails.

Issue request type

[ ] Question
[x] Enhancement
[ ] Bug
@gilles-peskine-arm gilles-peskine-arm added the enhancement New feature or request label Aug 13, 2019
gilles-peskine-arm added a commit to gilles-peskine-arm/mbed-crypto that referenced this issue Aug 13, 2019
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
ARMmbed#215
@ciarmcom
Copy link
Member

Internal Jira reference: https://jira.arm.com/browse/IOTCRYPT-865

@gilles-peskine-arm
Copy link
Collaborator Author

The secure element driver interface enabled by MBEDTLS_PSA_CRYPTO_SE_C is deprecated. We will not work on it any further.

@gilles-peskine-arm gilles-peskine-arm closed this as not planned Won't fix, can't repro, duplicate, stale May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants