Cannot destroy a corrupted key file #544

Open
opened 2025-10-23 06:51:27 -05:00 by gilles-peskine-arm · 0 comments
gilles-peskine-arm commented 2025-10-23 06:51:27 -05:00 (Migrated from github.com)

If a file in the PSA persistent key store is corrupted, it may be impossible to destroy it with psa_destroy_key(). This generally happens when the format of the metadata is invalid, e.g. because the file was truncated. It does not happen for “minor” bad formats, in particular a key file with an unsupported key type can be deleted.

In theory, it should be impossible to create such invalid files. But in practice, it could happen in several circumstances:

  • The system crashed while writing a file, on a platform where writing a file is not atomic.
  • Data corruption on physical media that isn't caught by a checksum.
  • Consequence of a bug in Mbed TLS or TF-PSA-Crypto (possibly a bug that is now fixed, since what matters is the version of the library when the file was created, not when you try to destroy it).

I wrote some tests in https://github.com/gilles-peskine-arm/TF-PSA-Crypto/tree/psa-test-load-key-id-ranges-1.0 ("Test with bad data in storage: bad key metadata"). Observed in both 3.6.5 and 1.0.0. Given the history of the code, I think it's been like this either since we added multithreading support in 3.6.0, or since we moved to the “openless” interface for persistent keys back in some 2.2x.

The reason this happens is that psa_destroy_key starts by attempting to access the key, and this involves loading the key in memory if it's persistent. Naively, the code should instead first destroy the key in memory if it's already loaded, and then remove the key file. However, this naive approach doesn't work in a multithreaded build, where another thread could load the key midway through.

Definition of done for this issue: psa_destroy_key always destroys the corresponding file, if it's given an existing persistent key ID. (Out of scope: if a file exists but the key ID is not in the persistent range where a file should exist, as in https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/488.)

If a file in the PSA persistent key store is corrupted, it may be impossible to destroy it with `psa_destroy_key()`. This generally happens when the format of the metadata is invalid, e.g. because the file was truncated. It does not happen for “minor” bad formats, in particular a key file with an unsupported key type can be deleted. In theory, it should be impossible to create such invalid files. But in practice, it could happen in several circumstances: * The system crashed while writing a file, on a platform where writing a file is not atomic. * Data corruption on physical media that isn't caught by a checksum. * Consequence of a bug in Mbed TLS or TF-PSA-Crypto (possibly a bug that is now fixed, since what matters is the version of the library when the file was created, not when you try to destroy it). I wrote some tests in https://github.com/gilles-peskine-arm/TF-PSA-Crypto/tree/psa-test-load-key-id-ranges-1.0 ("Test with bad data in storage: bad key metadata"). Observed in both 3.6.5 and 1.0.0. Given the history of the code, I think it's been like this either since we added multithreading support in 3.6.0, or since we moved to the “openless” interface for persistent keys back in some 2.2x. The reason this happens is that `psa_destroy_key` starts by attempting to access the key, and this involves loading the key in memory if it's persistent. Naively, the code should instead first destroy the key in memory if it's already loaded, and then remove the key file. However, this naive approach doesn't work in a multithreaded build, where another thread could load the key midway through. Definition of done for this issue: `psa_destroy_key` always destroys the corresponding file, if it's given an existing persistent key ID. (Out of scope: if a file exists but the key ID is not in the persistent range where a file should exist, as in https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/488.)
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-Mbed-TLS/TF-PSA-Crypto#544
No description provided.