Skip to content

Commit

Permalink
crypto: memzero_explicit - make sure to clear out sensitive data
Browse files Browse the repository at this point in the history
Recently, in commit 13aa93c ("random: add and use memzero_explicit()
for clearing data"), we have found that GCC may optimize some memset()
cases away when it detects a stack variable is not being used anymore
and going out of scope. This can happen, for example, in cases when we
are clearing out sensitive information such as keying material or any
e.g. intermediate results from crypto computations, etc.

With the help of Coccinelle, we can figure out and fix such occurences
in the crypto subsytem as well. Julia Lawall provided the following
Coccinelle program:

  @@
  type T;
  identifier x;
  @@

  T x;
  ... when exists
      when any
  -memset
  +memzero_explicit
     (&x,
  -0,
     ...)
  ... when != x
      when strict

  @@
  type T;
  identifier x;
  @@

  T x[...];
  ... when exists
      when any
  -memset
  +memzero_explicit
     (x,
  -0,
     ...)
  ... when != x
      when strict

Therefore, make use of the drop-in replacement memzero_explicit() for
exactly such cases instead of using memset().

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Julia Lawall <julia.lawall@lip6.fr>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Theodore Ts'o <tytso@mit.edu>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  • Loading branch information
Daniel Borkmann authored and tytso committed Oct 17, 2014
1 parent d4c5efd commit 7185ad2
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 13 deletions.
3 changes: 2 additions & 1 deletion crypto/cts.c
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ static int cts_cbc_decrypt(struct crypto_cts_ctx *ctx,
/* 5. Append the tail (BB - Ln) bytes of Xn (tmp) to Cn to create En */
memcpy(s + bsize + lastn, tmp + lastn, bsize - lastn);
/* 6. Decrypt En to create Pn-1 */
memset(iv, 0, sizeof(iv));
memzero_explicit(iv, sizeof(iv));

sg_set_buf(&sgsrc[0], s + bsize, bsize);
sg_set_buf(&sgdst[0], d, bsize);
err = crypto_blkcipher_decrypt_iv(&lcldesc, sgdst, sgsrc, bsize);
Expand Down
2 changes: 1 addition & 1 deletion crypto/sha1_generic.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ int crypto_sha1_update(struct shash_desc *desc, const u8 *data,
src = data + done;
} while (done + SHA1_BLOCK_SIZE <= len);

memset(temp, 0, sizeof(temp));
memzero_explicit(temp, sizeof(temp));
partial = 0;
}
memcpy(sctx->buffer + partial, src, len - done);
Expand Down
5 changes: 2 additions & 3 deletions crypto/sha256_generic.c
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,9 @@ static void sha256_transform(u32 *state, const u8 *input)

/* clear any sensitive info... */
a = b = c = d = e = f = g = h = t1 = t2 = 0;
memset(W, 0, 64 * sizeof(u32));
memzero_explicit(W, 64 * sizeof(u32));
}


static int sha224_init(struct shash_desc *desc)
{
struct sha256_state *sctx = shash_desc_ctx(desc);
Expand Down Expand Up @@ -316,7 +315,7 @@ static int sha224_final(struct shash_desc *desc, u8 *hash)
sha256_final(desc, D);

memcpy(hash, D, SHA224_DIGEST_SIZE);
memset(D, 0, SHA256_DIGEST_SIZE);
memzero_explicit(D, SHA256_DIGEST_SIZE);

return 0;
}
Expand Down
2 changes: 1 addition & 1 deletion crypto/sha512_generic.c
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ static int sha384_final(struct shash_desc *desc, u8 *hash)
sha512_final(desc, D);

memcpy(hash, D, 48);
memset(D, 0, 64);
memzero_explicit(D, 64);

return 0;
}
Expand Down
4 changes: 2 additions & 2 deletions crypto/tgr192.c
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ static int tgr160_final(struct shash_desc *desc, u8 * out)

tgr192_final(desc, D);
memcpy(out, D, TGR160_DIGEST_SIZE);
memset(D, 0, TGR192_DIGEST_SIZE);
memzero_explicit(D, TGR192_DIGEST_SIZE);

return 0;
}
Expand All @@ -623,7 +623,7 @@ static int tgr128_final(struct shash_desc *desc, u8 * out)

tgr192_final(desc, D);
memcpy(out, D, TGR128_DIGEST_SIZE);
memset(D, 0, TGR192_DIGEST_SIZE);
memzero_explicit(D, TGR192_DIGEST_SIZE);

return 0;
}
Expand Down
2 changes: 1 addition & 1 deletion crypto/vmac.c
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ static int vmac_final(struct shash_desc *pdesc, u8 *out)
}
mac = vmac(ctx->partial, ctx->partial_size, nonce, NULL, ctx);
memcpy(out, &mac, sizeof(vmac_t));
memset(&mac, 0, sizeof(vmac_t));
memzero_explicit(&mac, sizeof(vmac_t));
memset(&ctx->__vmac_ctx, 0, sizeof(struct vmac_ctx));
ctx->partial_size = 0;
return 0;
Expand Down
8 changes: 4 additions & 4 deletions crypto/wp512.c
Original file line number Diff line number Diff line change
Expand Up @@ -1102,8 +1102,8 @@ static int wp384_final(struct shash_desc *desc, u8 *out)
u8 D[64];

wp512_final(desc, D);
memcpy (out, D, WP384_DIGEST_SIZE);
memset (D, 0, WP512_DIGEST_SIZE);
memcpy(out, D, WP384_DIGEST_SIZE);
memzero_explicit(D, WP512_DIGEST_SIZE);

return 0;
}
Expand All @@ -1113,8 +1113,8 @@ static int wp256_final(struct shash_desc *desc, u8 *out)
u8 D[64];

wp512_final(desc, D);
memcpy (out, D, WP256_DIGEST_SIZE);
memset (D, 0, WP512_DIGEST_SIZE);
memcpy(out, D, WP256_DIGEST_SIZE);
memzero_explicit(D, WP512_DIGEST_SIZE);

return 0;
}
Expand Down

0 comments on commit 7185ad2

Please sign in to comment.