Using memset vs platform zeroize

[irwir]

mbedtls/tf-psa-crypto/core/psa_crypto.c

Lines 1256 to 1260 in 99ed26e

	* been wiped. Clear remaining metadata. We can call memset and not
	* zeroize because the metadata is not particularly sensitive.
	* This memset also sets the slot's state to PSA_SLOT_EMPTY. */
	memset(slot, 0, sizeof(*slot));

The given reason is strange. Non-sensitive data needs no wiping; sensitive data must be cleared. According to the comment, this memset was supposed to reset slot's state, but it would never happen if the call was optimized away.

Maybe in most cases memset(..., 0, ...) should be replaced with the reliable zeroize call.

[gilles-peskine-arm]

There are two reasons to clear data that is no longer in use:

• If the library code accesses it when it shouldn't. For example, we try not to keep copied of pointers to freed heap blocks around, to reduce the risk of a use-after-free. For this, memset is fine. A memset call can only be optimized away if the compiler is sure that the library code won't read that memory afterwards.
• If the memory becomes exposed even though, e.g. via a buffer overread. Cryptographic code tends to manipulate highly sensitive data such as keys, so we like to have a double line of defense: we both try to avoid buffer overreads, and try to reduce their impact. For this, memset is not adequate when the memory might not be reused as such, because a compiler can notice that the memory won't be read back and optimize away the memset call.

mbedtls_platform_zeroize is unfortunately more costly than memset(0) even when memset(0) isn't optimized away, so we tend to prefer memset(0). Also, there are circumstances where it would be ok to not call mbedtls_platform_zeroize — for example, mbedtls_platform_zeroize_and_free() could be just free() on platforms where free() itself wipes the memory. (Typically not the case on performance-oriented platforms such as Linux, but is often the case on security-oriented platforms such as many operating systems for secure enclaves.) Mind you, this isn't an optimization that we support at the moment, but it's fairly easy to patch for a specific platform by editing platform_util.c.

Here, the slot doesn't directly contain any sensitive memory, so we don't need the zeroize. We only want to ensure that if the slot is reused and the library code forgets to update part of it, the slot won't get into an inconsistent state.

Mind you, here, you could argue that zeroize is preferable because it's a second line of defense in case we do put sensitive content directly in the slot and we don't wipe it. We do in fact put key material directly in the slot when MBEDTLS_PSA_STATIC_KEY_SLOTS, but that part gets wiped by mbedtls_zeroize_and_free() in psa_remove_key_data_from_memory().

[irwir]

Thanks for clarifications.
Most probably, in this case memset would not be optimized away.
The problem then is in the comment.

We can call memset and not zeroize because the metadata is not particularly sensitive

This kind of suggests that it might be fine if the data was not wiped clean. Maybe this phrase should be removed.