Fail multipart aead operations if a new key is found in the keyslot #5123
Conversation
AndrzejKurek
added
enhancement
mbed TLS team
needs-work
needs-design-approval
needs-review
| @@ -159,6 +159,8 @@ struct psa_aead_operation_s | |||
| * any driver (i.e. none of the driver contexts are active). */ | |||
| unsigned int MBEDTLS_PRIVATE(id); | |||
|
|
|||
| uint64_t MBEDTLS_PRIVATE(associated_key_id); | |||
| mbedtls_svc_key_id_t MBEDTLS_PRIVATE(key_id); | |||
There was a problem hiding this comment.
When MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER is active, this is a two-element structure (owner and id).
| rnd_status = psa_generate_random( (uint8_t *) &slot->key_id, | ||
| sizeof(slot->key_id) ); |
There was a problem hiding this comment.
How does setting a random key id help, compared to setting to some fixed value which is guaranteed not to be a valid key id?
There was a problem hiding this comment.
Why should we not set a valid ID? This random value just shouldn't be the one that's marked as the wrong, initial, "slot unused" one.
Random key ID makes it so that the ID generator is stateless, and the generated ID's have a larger hamming distance from each other.
This might not be inside the thread model for PSA (fault injection) however.
Cons - possible ID clash for two identical IDs. Even then, it's an issue if we generate the same number twice on the same slot, not if we generate two same IDs in two different slots.
| rnd_status = psa_generate_random( (uint8_t *) &slot->key_id, | ||
| sizeof(slot->key_id) ); |
There was a problem hiding this comment.
What if the slot is reused immediately afterwards?
There was a problem hiding this comment.
I'm not sure if I follow what's the problem here - if you mean immediately as in after calling this function - we should have a mutex in place in the future. If you mean after the call to slot wipe - no problem here.
The implementation prioritizes readers. Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
Make the multipart operation objects store key IDs of used keys to check if they changed before each multipart operation. Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
AndrzejKurek
force-pushed
the
keyslot-multipart-ids
branch
from
f08b25b
to
50cc174
Compare
|
We are now converting older PRs to draft PRs where the following conditions are met: They have not been updated in the last 3 months, and they need more than non-trivial work to complete. |
tom-daubney-arm
added
the
historical-reviewed
Make the multipart operation objects store key IDs of used keys to check if they changed before each multipart operation.
A randomly generated
uint64_tis added to each key slot and is populated in each call topsa_wipe_key_slot.This ID is copied to a multipart operation objects as a part of
psa_aead_setup.Do not merge yet: documentation needs an update and additional tests for
psa_aead_verifyhave to be added. PR created mainly to find any CI issues and serve as a base for discussion.Also: This PR is based on this one, implementing a global readers-writer lock.