Archive-Team
/
zfs
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 103 Wiki Activity

ICP: AES-GCM: Refactor gcm_clear_ctx() 

Currently the temporary buffer in which decryption takes place
isn't cleared on context destruction. Further in some routines we
fail to call gcm_clear_ctx() on error exit. Both flaws may result
in leaking sensitive data.

We follow best practices and zero out the plaintext buffer before
freeing the memory holding it. Also move all cleanup into
gcm_clear_ctx() and call it on any context destruction.

The performance impact should be negligible.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Rob Norris <robn@despairlabs.com>
Signed-off-by: Attila Fülöp <attila@fueloep.org>
Closes #14528
This commit is contained in:
Attila Fülöp 2023-02-27 23:38:12 +01:00 committed by GitHub
parent 3b9309aabe
commit f58e513f74
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 36 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
module/icp/algs/modes/gcm.c
View File

@ -1127,24 +1127,6 @@ gcm_simd_get_htab_size(boolean_t simd_mode)
}
}
/*
* Clear sensitive data in the context.
*
* ctx->gcm_remainder may contain a plaintext remainder. ctx->gcm_H and
* ctx->gcm_Htable contain the hash sub key which protects authentication.
*
* Although extremely unlikely, ctx->gcm_J0 and ctx->gcm_tmp could be used for
* a known plaintext attack, they consists of the IV and the first and last
* counter respectively. If they should be cleared is debatable.
*/
static inline void
gcm_clear_ctx(gcm_ctx_t *ctx)
{
memset(ctx->gcm_remainder, 0, sizeof (ctx->gcm_remainder));
memset(ctx->gcm_H, 0, sizeof (ctx->gcm_H));
memset(ctx->gcm_J0, 0, sizeof (ctx->gcm_J0));
memset(ctx->gcm_tmp, 0, sizeof (ctx->gcm_tmp));
}
/* Increment the GCM counter block by n. */
static inline void
@ -1367,8 +1349,6 @@ gcm_encrypt_final_avx(gcm_ctx_t *ctx, crypto_data_t *out, size_t block_size)
return (rv);
out->cd_offset += ctx->gcm_tag_len;
/* Clear sensitive data in the context before returning. */
gcm_clear_ctx(ctx);
return (CRYPTO_SUCCESS);
}
@ -1478,7 +1458,6 @@ gcm_decrypt_final_avx(gcm_ctx_t *ctx, crypto_data_t *out, size_t block_size)
return (rv);
}
out->cd_offset += pt_len;
gcm_clear_ctx(ctx);
return (CRYPTO_SUCCESS);
}

13
module/icp/algs/modes/modes.c
View File

@ -150,18 +150,7 @@ crypto_free_mode_ctx(void *ctx)
case GCM_MODE:
case GMAC_MODE:
if (((gcm_ctx_t *)ctx)->gcm_pt_buf != NULL)
vmem_free(((gcm_ctx_t *)ctx)->gcm_pt_buf,
((gcm_ctx_t *)ctx)->gcm_pt_buf_len);
#ifdef CAN_USE_GCM_ASM
if (((gcm_ctx_t *)ctx)->gcm_Htable != NULL) {
gcm_ctx_t *gcm_ctx = (gcm_ctx_t *)ctx;
memset(gcm_ctx->gcm_Htable, 0, gcm_ctx->gcm_htab_len);
kmem_free(gcm_ctx->gcm_Htable, gcm_ctx->gcm_htab_len);
}
#endif
gcm_clear_ctx((gcm_ctx_t *)ctx);
kmem_free(ctx, sizeof (gcm_ctx_t));
}
}

32
module/icp/include/modes/modes.h
View File

@ -244,6 +244,38 @@ typedef struct gcm_ctx {
#define AES_GMAC_IV_LEN 12
#define AES_GMAC_TAG_BITS 128
/*
* Clear sensitive data in the context and free allocated memory.
*
* ctx->gcm_remainder may contain a plaintext remainder. ctx->gcm_H and
* ctx->gcm_Htable contain the hash sub key which protects authentication.
* ctx->gcm_pt_buf contains the plaintext result of decryption.
*
* Although extremely unlikely, ctx->gcm_J0 and ctx->gcm_tmp could be used for
* a known plaintext attack, they consists of the IV and the first and last
* counter respectively. If they should be cleared is debatable.
*/
static inline void
gcm_clear_ctx(gcm_ctx_t *ctx)
{
memset(ctx->gcm_remainder, 0, sizeof (ctx->gcm_remainder));
memset(ctx->gcm_H, 0, sizeof (ctx->gcm_H));
#if defined(CAN_USE_GCM_ASM)
if (ctx->gcm_use_avx == B_TRUE) {
ASSERT3P(ctx->gcm_Htable, !=, NULL);
memset(ctx->gcm_Htable, 0, ctx->gcm_htab_len);
kmem_free(ctx->gcm_Htable, ctx->gcm_htab_len);
}
#endif
if (ctx->gcm_pt_buf != NULL) {
memset(ctx->gcm_pt_buf, 0, ctx->gcm_pt_buf_len);
vmem_free(ctx->gcm_pt_buf, ctx->gcm_pt_buf_len);
}
/* Optional */
memset(ctx->gcm_J0, 0, sizeof (ctx->gcm_J0));
memset(ctx->gcm_tmp, 0, sizeof (ctx->gcm_tmp));
}
typedef struct aes_ctx {
union {
ecb_ctx_t acu_ecb;

25
module/icp/io/aes.c
View File

@ -945,17 +945,9 @@ out:
memset(aes_ctx.ac_keysched, 0, aes_ctx.ac_keysched_len);
kmem_free(aes_ctx.ac_keysched, aes_ctx.ac_keysched_len);
}
#ifdef CAN_USE_GCM_ASM
if (aes_ctx.ac_flags & (GCM_MODE|GMAC_MODE) &&
((gcm_ctx_t *)&aes_ctx)->gcm_Htable != NULL) {
gcm_ctx_t *ctx = (gcm_ctx_t *)&aes_ctx;
memset(ctx->gcm_Htable, 0, ctx->gcm_htab_len);
kmem_free(ctx->gcm_Htable, ctx->gcm_htab_len);
if (aes_ctx.ac_flags & (GCM_MODE|GMAC_MODE)) {
gcm_clear_ctx((gcm_ctx_t *)&aes_ctx);
}
#endif
return (ret);
}
@ -1101,18 +1093,7 @@ out:
vmem_free(aes_ctx.ac_pt_buf, aes_ctx.ac_data_len);
}
} else if (aes_ctx.ac_flags & (GCM_MODE|GMAC_MODE)) {
if (((gcm_ctx_t *)&aes_ctx)->gcm_pt_buf != NULL) {
vmem_free(((gcm_ctx_t *)&aes_ctx)->gcm_pt_buf,
((gcm_ctx_t *)&aes_ctx)->gcm_pt_buf_len);
}
#ifdef CAN_USE_GCM_ASM
if (((gcm_ctx_t *)&aes_ctx)->gcm_Htable != NULL) {
gcm_ctx_t *ctx = (gcm_ctx_t *)&aes_ctx;
memset(ctx->gcm_Htable, 0, ctx->gcm_htab_len);
kmem_free(ctx->gcm_Htable, ctx->gcm_htab_len);
}
#endif
gcm_clear_ctx((gcm_ctx_t *)&aes_ctx);
}
return (ret);