[crypto] Re-mask AES-GCM keys #27647
Conversation
nasahlpa
force-pushed
the
aes_gcm_refresh_key
branch
2 times, most recently
from
3e46299 to
e04ff10
Compare
nasahlpa
requested review from
engdoreis,
moidx,
h-filali,
andrea-caforio,
felixmiller,
vogelpi and
cfrantz
and removed request for
a team and
engdoreis
nasahlpa
added
the
CherryPick:earlgrey_1.0.0
| HARDENED_TRY(hardened_xor((uint32_t *)internal_ctx->key.key_shares[1], mask, | ||
| internal_ctx->key.key_len)); | ||
| } else { | ||
| HARDENED_CHECK_EQ(internal_ctx->key.sideload, kHardenedBoolTrue); |
There was a problem hiding this comment.
Shall we return error upon calling remask with a sideload key to notify user?
sideloaded keys are always remasked on every power cycle iiuc
There was a problem hiding this comment.
gcm_remask_key() cannot be called by the user - it is only called by the CL internally in otcrypto_aes_gcm_update_encrypted_data, otcrypto_aes_gcm_decrypt_final, or otcrypto_aes_gcm_encrypt_final.
In this case, I think just returning OTCRYPTO_OK for this function in the sideloaded key case should be fine.
sw/device/lib/crypto/impl/aes_gcm.c
Outdated
| @@ -427,6 +455,8 @@ otcrypto_status_t otcrypto_aes_gcm_update_encrypted_data( | |||
| aes_gcm_context_t internal_ctx; | |||
| gcm_context_restore(ctx, &internal_ctx); | |||
| HARDENED_TRY(load_key_if_sideloaded(internal_ctx.key)); | |||
| // Remask the key if needed. | |||
There was a problem hiding this comment.
if needed refers to whether not sideloaded, right? we could say this explicitly
There was a problem hiding this comment.
Thanks, good point. I've updated the comment.
nasahlpa
force-pushed
the
aes_gcm_refresh_key
branch
from
e04ff10 to
8ffb16e
Compare
sw/device/lib/crypto/impl/keyblob.h
Outdated
| * @param word_len Length in words of each operand. | ||
| * @return Result of the operation. | ||
| */ | ||
| status_t hardened_xor(uint32_t *OT_RESTRICT x, const uint32_t *OT_RESTRICT y, |
There was a problem hiding this comment.
Need to check if it is OK to expose this function here or move to the hardened lib.
There was a problem hiding this comment.
I think there are two options:
- Move it to hardened lib, which will require creating a couple of hook functions to initialize and get entropy.
- Move it to
random_orderas the function implementation already relies onrandom_orderprimitives.
It may be easier to go with (2) for now.
There was a problem hiding this comment.
Thank you for your suggestion @moidx
I tried adding it to random_order but there I am creating a cycle dependency as I would need to include hardened_memory as a dependency, which depends on random_order.
Would moving it to hardened_memory, as I just did in the first commit of this PR, be OK as well?
sw/device/lib/crypto/impl/keyblob.h
Outdated
| * @param word_len Length in words of each operand. | ||
| * @return Result of the operation. | ||
| */ | ||
| status_t hardened_xor(uint32_t *OT_RESTRICT x, const uint32_t *OT_RESTRICT y, |
There was a problem hiding this comment.
I think there are two options:
- Move it to hardened lib, which will require creating a couple of hook functions to initialize and get entropy.
- Move it to
random_orderas the function implementation already relies onrandom_orderprimitives.
It may be easier to go with (2) for now.
As this function is useful also outside of keyblob, so move it to the hardened_memory library. Signed-off-by: Pascal Nasahl <nasahlpa@lowrisc.org>
nasahlpa
force-pushed
the
aes_gcm_refresh_key
branch
from
8ffb16e to
6dd1b96
Compare
As suggested in lowRISC#27243, this commit regularly remasks the key that is used for AES-GCM. Signed-off-by: Pascal Nasahl <nasahlpa@lowrisc.org>
nasahlpa
force-pushed
the
aes_gcm_refresh_key
branch
from
6dd1b96 to
7251046
Compare
As suggested in #27243, this commit regularly remasks the key that is used for AES-GCM.