Re: [Qemu-block] [PATCH 05/13] qcrypto-luks: clear the masterkey and password before freeing them always

[Nir Soffer]

On Wed, Aug 14, 2019, 23:23 Maxim Levitsky <address@hidden> wrote:

While there are other places where these are still stored in memory,
this is still one less key material area that can be sniffed with
various side channel attacks



Signed-off-by: Maxim Levitsky <address@hidden>
---
 crypto/block-luks.c | 52 ++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index e1a4df94b7..336e633df4 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1023,8 +1023,18 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
  cleanup:
     qcrypto_ivgen_free(ivgen);
     qcrypto_cipher_free(cipher);
-    g_free(splitkey);
-    g_free(possiblekey);
+
+    if (splitkey) {
+        memset(splitkey, 0, splitkeylen);

I think we need memset_s() or similar replacement, see

https://www.cryptologie.net/article/419/zeroing-memory-compiler-optimizations-and-memset_s/

+        g_free(splitkey);
+    }
+
+    if (possiblekey) {
+        memset(possiblekey, 0, masterkeylen(luks));
+        g_free(possiblekey);
+
+    }
+
     return ret;
 }

@@ -1161,16 +1171,34 @@ qcrypto_block_luks_open(QCryptoBlock *block,
     block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
     block->payload_offset = luks->header.payload_offset * block->sector_size;

-    g_free(masterkey);
-    g_free(password);
+    if (masterkey) {
+        memset(masterkey, 0, masterkeylen(luks));
+        g_free(masterkey);
+    }
+
+    if (password) {
+        memset(password, 0, strlen(password));
+        g_free(password);
+    }
+
     return 0;

  fail:
-    g_free(masterkey);
+
+    if (masterkey) {
+        memset(masterkey, 0, masterkeylen(luks));
+        g_free(masterkey);
+    }
+
+    if (password) {
+        memset(password, 0, strlen(password));
+        g_free(password);
+    }
+
     qcrypto_block_free_cipher(block);
     qcrypto_ivgen_free(block->ivgen);
+
     g_free(luks);
-    g_free(password);
     return ret;
 }

@@ -1459,7 +1487,10 @@ qcrypto_block_luks_create(QCryptoBlock *block,

     memset(masterkey, 0, luks->header.key_bytes);
     g_free(masterkey);
+
+    memset(password, 0, strlen(password));
     g_free(password);
+
     g_free(cipher_mode_spec);

     return 0;
@@ -1467,9 +1498,14 @@ qcrypto_block_luks_create(QCryptoBlock *block,
  error:
     if (masterkey) {
         memset(masterkey, 0, luks->header.key_bytes);
+        g_free(masterkey);
     }
-    g_free(masterkey);
-    g_free(password);
+
+    if (password) {
+        memset(password, 0, strlen(password));
+        g_free(password);
+    }
+
     g_free(cipher_mode_spec);

     qcrypto_block_free_cipher(block);
--
2.17.2