309 lines
10 KiB
Diff
309 lines
10 KiB
Diff
From 67f36d057aa71ca56ebc17ef28a7cb70bac6c6b6 Mon Sep 17 00:00:00 2001
|
|
From: "Daniel P. Berrange" <berrange@redhat.com>
|
|
Date: Tue, 5 May 2020 16:46:01 +0100
|
|
Subject: [PATCH 01/12] block: always fill entire LUKS header space with zeros
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
RH-Author: Daniel P. Berrange <berrange@redhat.com>
|
|
Message-id: <20200505164601.1059974-2-berrange@redhat.com>
|
|
Patchwork-id: 96277
|
|
O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH 1/1] block: always fill entire LUKS header space with zeros
|
|
Bugzilla: 1775462
|
|
RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>
|
|
RH-Acked-by: John Snow <jsnow@redhat.com>
|
|
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
|
|
|
|
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.
|
|
|
|
An optimization to the ref count checking code:
|
|
|
|
commit a5fff8d4b4d928311a5005efa12d0991fe3b66f9 (refs/bisect/bad)
|
|
Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
|
|
Date: Wed Feb 27 16:14:30 2019 +0300
|
|
|
|
qcow2-refcount: avoid eating RAM
|
|
|
|
made the assumption that every cluster which was allocated would
|
|
have at least some data written to it. This was violated by way
|
|
the LUKS header is only partially written, with much space simply
|
|
reserved for future use.
|
|
|
|
Depending on the cluster size this problem was masked by the
|
|
logic which wrote zeros between the end of the LUKS header and
|
|
the end of the cluster.
|
|
|
|
$ qemu-img create --object secret,id=cluster_encrypt0,data=123456 \
|
|
-f qcow2 -o cluster_size=2k,encrypt.iter-time=1,\
|
|
encrypt.format=luks,encrypt.key-secret=cluster_encrypt0 \
|
|
cluster_size_check.qcow2 100M
|
|
Formatting 'cluster_size_check.qcow2', fmt=qcow2 size=104857600
|
|
encrypt.format=luks encrypt.key-secret=cluster_encrypt0
|
|
encrypt.iter-time=1 cluster_size=2048 lazy_refcounts=off refcount_bits=16
|
|
|
|
$ qemu-img check --object secret,id=cluster_encrypt0,data=redhat \
|
|
'json:{"driver": "qcow2", "encrypt.format": "luks", \
|
|
"encrypt.key-secret": "cluster_encrypt0", \
|
|
"file.driver": "file", "file.filename": "cluster_size_check.qcow2"}'
|
|
ERROR: counting reference for region exceeding the end of the file by one cluster or more: offset 0x2000 size 0x1f9000
|
|
Leaked cluster 4 refcount=1 reference=0
|
|
...snip...
|
|
Leaked cluster 130 refcount=1 reference=0
|
|
|
|
1 errors were found on the image.
|
|
Data may be corrupted, or further writes to the image may corrupt it.
|
|
|
|
127 leaked clusters were found on the image.
|
|
This means waste of disk space, but no harm to data.
|
|
Image end offset: 268288
|
|
|
|
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).
|
|
|
|
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
|
|
Message-Id: <20200207135520.2669430-1-berrange@redhat.com>
|
|
Reviewed-by: Eric Blake <eblake@redhat.com>
|
|
Signed-off-by: Max Reitz <mreitz@redhat.com>
|
|
(cherry picked from commit 087ab8e775f48766068e65de1bc99d03b40d1670)
|
|
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
|
|
|
|
Conflicts:
|
|
tests/qemu-iotests/group: no test 283 in downstream
|
|
|
|
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
|
|
---
|
|
block/qcow2.c | 11 ++++--
|
|
tests/qemu-iotests/284 | 97 ++++++++++++++++++++++++++++++++++++++++++++++
|
|
tests/qemu-iotests/284.out | 62 +++++++++++++++++++++++++++++
|
|
tests/qemu-iotests/group | 1 +
|
|
4 files changed, 167 insertions(+), 4 deletions(-)
|
|
create mode 100755 tests/qemu-iotests/284
|
|
create mode 100644 tests/qemu-iotests/284.out
|
|
|
|
diff --git a/block/qcow2.c b/block/qcow2.c
|
|
index 71067c6..af0ad4a 100644
|
|
--- a/block/qcow2.c
|
|
+++ 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;
|
|
diff --git a/tests/qemu-iotests/284 b/tests/qemu-iotests/284
|
|
new file mode 100755
|
|
index 0000000..071e89b
|
|
--- /dev/null
|
|
+++ b/tests/qemu-iotests/284
|
|
@@ -0,0 +1,97 @@
|
|
+#!/usr/bin/env bash
|
|
+#
|
|
+# Test ref count checks on encrypted images
|
|
+#
|
|
+# Copyright (C) 2019 Red Hat, Inc.
|
|
+#
|
|
+# This program is free software; you can redistribute it and/or modify
|
|
+# it under the terms of the GNU General Public License as published by
|
|
+# the Free Software Foundation; either version 2 of the License, or
|
|
+# (at your option) any later version.
|
|
+#
|
|
+# This program is distributed in the hope that it will be useful,
|
|
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
+# GNU General Public License for more details.
|
|
+#
|
|
+# You should have received a copy of the GNU General Public License
|
|
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
|
|
+#
|
|
+
|
|
+# creator
|
|
+owner=berrange@redhat.com
|
|
+
|
|
+seq=`basename $0`
|
|
+echo "QA output created by $seq"
|
|
+
|
|
+status=1 # failure is the default!
|
|
+
|
|
+_cleanup()
|
|
+{
|
|
+ _cleanup_test_img
|
|
+}
|
|
+trap "_cleanup; exit \$status" 0 1 2 3 15
|
|
+
|
|
+# get standard environment, filters and checks
|
|
+. ./common.rc
|
|
+. ./common.filter
|
|
+
|
|
+_supported_fmt qcow2
|
|
+_supported_proto generic
|
|
+_supported_os Linux
|
|
+
|
|
+
|
|
+size=1M
|
|
+
|
|
+SECRET="secret,id=sec0,data=astrochicken"
|
|
+
|
|
+IMGSPEC="driver=$IMGFMT,file.filename=$TEST_IMG,encrypt.key-secret=sec0"
|
|
+QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
|
|
+
|
|
+_run_test()
|
|
+{
|
|
+ IMGOPTSSYNTAX=true
|
|
+ OLD_TEST_IMG="$TEST_IMG"
|
|
+ TEST_IMG="driver=$IMGFMT,file.filename=$TEST_IMG,encrypt.key-secret=sec0"
|
|
+ QEMU_IMG_EXTRA_ARGS="--image-opts --object $SECRET"
|
|
+
|
|
+ echo
|
|
+ echo "== cluster size $csize"
|
|
+ echo "== checking image refcounts =="
|
|
+ _check_test_img
|
|
+
|
|
+ echo
|
|
+ echo "== writing some data =="
|
|
+ $QEMU_IO -c "write -P 0x9 0 1" $QEMU_IMG_EXTRA_ARGS $TEST_IMG | _filter_qemu_io | _filter_testdir
|
|
+ echo
|
|
+ echo "== rechecking image refcounts =="
|
|
+ _check_test_img
|
|
+
|
|
+ echo
|
|
+ echo "== writing some more data =="
|
|
+ $QEMU_IO -c "write -P 0x9 $csize 1" $QEMU_IMG_EXTRA_ARGS $TEST_IMG | _filter_qemu_io | _filter_testdir
|
|
+ echo
|
|
+ echo "== rechecking image refcounts =="
|
|
+ _check_test_img
|
|
+
|
|
+ TEST_IMG="$OLD_TEST_IMG"
|
|
+ QEMU_IMG_EXTRA_ARGS=
|
|
+ IMGOPTSSYNTAX=
|
|
+}
|
|
+
|
|
+
|
|
+echo
|
|
+echo "testing LUKS qcow2 encryption"
|
|
+echo
|
|
+
|
|
+for csize in 512 2048 32768
|
|
+do
|
|
+ _make_test_img --object $SECRET -o "encrypt.format=luks,encrypt.key-secret=sec0,encrypt.iter-time=10,cluster_size=$csize" $size
|
|
+ _run_test
|
|
+ _cleanup_test_img
|
|
+done
|
|
+
|
|
+# success, all done
|
|
+echo "*** done"
|
|
+rm -f $seq.full
|
|
+status=0
|
|
diff --git a/tests/qemu-iotests/284.out b/tests/qemu-iotests/284.out
|
|
new file mode 100644
|
|
index 0000000..48216f5
|
|
--- /dev/null
|
|
+++ b/tests/qemu-iotests/284.out
|
|
@@ -0,0 +1,62 @@
|
|
+QA output created by 284
|
|
+
|
|
+testing LUKS qcow2 encryption
|
|
+
|
|
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
|
|
+
|
|
+== cluster size 512
|
|
+== checking image refcounts ==
|
|
+No errors were found on the image.
|
|
+
|
|
+== writing some data ==
|
|
+wrote 1/1 bytes at offset 0
|
|
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
|
|
+
|
|
+== rechecking image refcounts ==
|
|
+No errors were found on the image.
|
|
+
|
|
+== writing some more data ==
|
|
+wrote 1/1 bytes at offset 512
|
|
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
|
|
+
|
|
+== rechecking image refcounts ==
|
|
+No errors were found on the image.
|
|
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
|
|
+
|
|
+== cluster size 2048
|
|
+== checking image refcounts ==
|
|
+No errors were found on the image.
|
|
+
|
|
+== writing some data ==
|
|
+wrote 1/1 bytes at offset 0
|
|
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
|
|
+
|
|
+== rechecking image refcounts ==
|
|
+No errors were found on the image.
|
|
+
|
|
+== writing some more data ==
|
|
+wrote 1/1 bytes at offset 2048
|
|
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
|
|
+
|
|
+== rechecking image refcounts ==
|
|
+No errors were found on the image.
|
|
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
|
|
+
|
|
+== cluster size 32768
|
|
+== checking image refcounts ==
|
|
+No errors were found on the image.
|
|
+
|
|
+== writing some data ==
|
|
+wrote 1/1 bytes at offset 0
|
|
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
|
|
+
|
|
+== rechecking image refcounts ==
|
|
+No errors were found on the image.
|
|
+
|
|
+== writing some more data ==
|
|
+wrote 1/1 bytes at offset 32768
|
|
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
|
|
+
|
|
+== rechecking image refcounts ==
|
|
+No errors were found on the image.
|
|
+*** done
|
|
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
|
|
index e47cbfc..9c565cf 100644
|
|
--- a/tests/qemu-iotests/group
|
|
+++ b/tests/qemu-iotests/group
|
|
@@ -289,3 +289,4 @@
|
|
277 rw quick
|
|
280 rw migration quick
|
|
281 rw quick
|
|
+284 rw
|
|
--
|
|
1.8.3.1
|
|
|