[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] block: always fill entire LUKS header space with zeros

From: Eric Blake
Subject: Re: [PATCH v2] block: always fill entire LUKS header space with zeros
Date: Fri, 7 Feb 2020 08:39:44 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1

On 2/7/20 7:55 AM, Daniel P. Berrangé wrote:
When initializing the LUKS header the size with default encryption
parameters will currently be 2068480 bytes. This is rounded up to
a multiple of the cluster size, 2081792, with 64k sectors. If the
end of the header is not the same as the end of the cluster we fill
the extra space with zeros. This was forgetting that not even the
space allocated for the header will be fully initialized, as we
only write key material for the first key slot. The space left
for the other 7 slots is never written to.

The problem only exists when the disk image is entirely empty. Writing
data to the disk image payload will solve the problem by causing the
end of the file to be extended further.

The change fixes it by ensuring that the entire allocated LUKS header
region is fully initialized with zeros. The qemu-img check will still
fail for any pre-existing disk images created prior to this change,
unless at least 1 byte of the payload is written to.

Fully writing zeros to the entire LUKS header is a good idea regardless
as it ensures that space has been allocated on the host filesystem (or
whatever block storage backend is used).

What's more, we avoid a possible bug where creating a LUKS image backed by a block device protocol where the block device happens to already contain stale data from an earlier use of that block device in a different LUKS image, which could make it appear as though we have populated key slots. It's unlikely that those other slots would decode the current image correctly (as the stale keyslot would decode to a different master key), but being able to supply the passphrase to that stale keyslot to decode garbage out of the new image does not seem desirable.

Signed-off-by: Daniel P. Berrangé <address@hidden>

+++ b/block/qcow2.c
@@ -135,13 +135,16 @@ static ssize_t qcow2_crypto_hdr_init_func(QCryptoBlock 
*block, size_t headerlen,
      s->crypto_header.length = headerlen;
      s->crypto_header.offset = ret;
- /* Zero fill remaining space in cluster so it has predictable
-     * content in case of future spec changes */
+    /*
+     * Zero fill all space in cluster so it has predictable
+     * content, as we may not initialize some regions of the
+     * header (eg only 1 out of 8 key slots will be initialized)
+     */
      clusterlen = size_to_clusters(s, headerlen) * s->cluster_size;
      assert(qcow2_pre_write_overlap_check(bs, 0, ret, clusterlen, false) == 0);
      ret = bdrv_pwrite_zeroes(bs->file,
-                             ret + headerlen,
-                             clusterlen - headerlen, 0);
+                             ret,
+                             clusterlen, 0);
      if (ret < 0) {
          error_setg_errno(errp, -ret, "Could not zero fill encryption header");
          return -1;
Reviewed-by: Eric Blake <address@hidden>

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

reply via email to

[Prev in Thread] Current Thread [Next in Thread]