[Qemu-devel] [PATCH v3] virtio-crypto: zeroize the key material before free

[Qemu-devel] [PATCH v3] virtio-crypto: zeroize the key material before free

[Gonglei]

Common practice with sensitive information (key material, passwords,
etc). Prevents sensitive information from being exposed by accident later in
coredumps, memory disclosure bugs when heap memory is reused, etc.

Sensitive information is sometimes also held in mlocked pages to prevent
it being swapped to disk but that's not being done here.

Let's zeroize the memory of CryptoDevBackendSymOpInfo structure pointed
for key material security.

[Thanks to Stefan for help with crafting the commit message]

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 v3:
 - rework the commit message [Eric]
 - add Eric's R-by tag.

 hw/virtio/virtio-crypto.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 4f11fee..48288e8 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -337,7 +337,18 @@ static void virtio_crypto_free_request(VirtIOCryptoReq *req)
 {
     if (req) {
         if (req->flags == CRYPTODEV_BACKEND_ALG_SYM) {
-            g_free(req->u.sym_op_info);
+            size_t max_len;
+            CryptoDevBackendSymOpInfo *op_info = req->u.sym_op_info;
+
+            max_len = op_info->iv_len +
+                      op_info->aad_len +
+                      op_info->src_len +
+                      op_info->dst_len +
+                      op_info->digest_result_len;
+
+            /* Zeroize and free request data structure */
+            memset(op_info, 0, sizeof(*op_info) + max_len);
+            g_free(op_info);
         }
         g_free(req);
     }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3] virtio-crypto: zeroize the key material before free

[Gonglei (Arei)]

Hi Michael,

Ping...


Regards,
-Gonglei


> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Thursday, December 22, 2016 11:01 AM
> To: qemu-devel@nongnu.org
> Cc: mst@redhat.com; Gonglei (Arei)
> Subject: [PATCH v3] virtio-crypto: zeroize the key material before free
> 
> Common practice with sensitive information (key material, passwords,
> etc). Prevents sensitive information from being exposed by accident later in
> coredumps, memory disclosure bugs when heap memory is reused, etc.
> 
> Sensitive information is sometimes also held in mlocked pages to prevent
> it being swapped to disk but that's not being done here.
> 
> Let's zeroize the memory of CryptoDevBackendSymOpInfo structure pointed
> for key material security.
> 
> [Thanks to Stefan for help with crafting the commit message]
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  v3:
>  - rework the commit message [Eric]
>  - add Eric's R-by tag.
> 
>  hw/virtio/virtio-crypto.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
> index 4f11fee..48288e8 100644
> --- a/hw/virtio/virtio-crypto.c
> +++ b/hw/virtio/virtio-crypto.c
> @@ -337,7 +337,18 @@ static void
> virtio_crypto_free_request(VirtIOCryptoReq *req)
>  {
>      if (req) {
>          if (req->flags == CRYPTODEV_BACKEND_ALG_SYM) {
> -            g_free(req->u.sym_op_info);
> +            size_t max_len;
> +            CryptoDevBackendSymOpInfo *op_info = req->u.sym_op_info;
> +
> +            max_len = op_info->iv_len +
> +                      op_info->aad_len +
> +                      op_info->src_len +
> +                      op_info->dst_len +
> +                      op_info->digest_result_len;
> +
> +            /* Zeroize and free request data structure */
> +            memset(op_info, 0, sizeof(*op_info) + max_len);
> +            g_free(op_info);
>          }
>          g_free(req);
>      }
> --
> 1.8.3.1
>