crypto: memzero_explicit - make sure to clear out sensitive data
authorDaniel Borkmann <dborkman@redhat.com>
Sun, 7 Sep 2014 21:23:38 +0000 (23:23 +0200)
committerTheodore Ts'o <tytso@mit.edu>
Fri, 17 Oct 2014 15:44:07 +0000 (11:44 -0400)
Recently, in commit 13aa93c70e71 ("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>
crypto/cts.c
crypto/sha1_generic.c
crypto/sha256_generic.c
crypto/sha512_generic.c
crypto/tgr192.c
crypto/vmac.c
crypto/wp512.c

index 042223f8e73364529be44a850d0dce174352c858..133f0874c95eccc7e02c26f834afed08a504a0a9 100644 (file)
@@ -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);
index 42794803c480531a60cc465657741a42ea5485dc..7bb04743278216e99a011adafa8ce8d3d97c7b4d 100644 (file)
@@ -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);
index 5433667795249ca298059a136998b9c2624f2c97..32c5e5ea205a7e71f758a65336ac141158ad977d 100644 (file)
@@ -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);
@@ -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;
 }
index 6ed124f3ea0f064cbe3f757d09a85e9e44c7ac54..04d295a8bc08b15592040731b5f59f40b7d7014d 100644 (file)
@@ -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;
 }
index 87403556fd0bfa5c3a4ecd40a5a707b6892fbc04..3c7af0d1ff7a6f396538a41a89e0dd5a9405e99e 100644 (file)
@@ -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;
 }
@@ -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;
 }
index 2eb11a30c29cee93203a1b90a990cc4463368440..d84c24bd7ff7bb3752484a175aee7a7aadd28b21 100644 (file)
@@ -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;
index 180f1d6e03f48fc20ef787061d8866e6af429c68..ec64e7762fbb55a579fefd781757a9d29182f5da 100644 (file)
@@ -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;
 }
@@ -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;
 }