-
Notifications
You must be signed in to change notification settings - Fork 871
[crypto] Re-mask AES-GCM keys #27647
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
base: master
Are you sure you want to change the base?
Conversation
3e46299
to
e04ff10
Compare
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks sgtm
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if needed refers to whether not sideloaded, right? we could say this explicitly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good point. I've updated the comment.
e04ff10
to
8ffb16e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @nasahlpa, I will take a second look after you respond to the hardened_xor
comment.
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>
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>
6dd1b96
to
7251046
Compare
As suggested in #27243, this commit regularly remasks the key that is used for AES-GCM.