[libvirt] [PATCH v2 0/5] qemu: Forbid old qcow/qcow2 encryption
The old qcow/qcow2 encryption format is so broken that qemu decided to drop it completely. This series forbids the use of such images even with qemus prior to this and removes all the cruft necessary to support it. v2: - fixed check to include the qcow format too - reworded the error message slightly - split second patch into two with proper justification for the user-alias test since checking LUKS there actually makes sense Peter Krempa (5): tests: qemuxml2argv: Drop disk encryption from 'interface-server' test tests: qemuxml2argv: Verify that disk secret alias is correct with user-aliases tests: qemublock: Switch to qcow2+luks in test files qemu: domain: Forbid storage with old QCOW2 encryption qemu: Remove code for setting up disk passphrases src/qemu/qemu_domain.c | 10 ++ src/qemu/qemu_monitor.c | 13 --- src/qemu/qemu_monitor.h | 4 - src/qemu/qemu_monitor_json.c | 28 ------ src/qemu/qemu_monitor_json.h | 4 - src/qemu/qemu_process.c | 103 --------------------- .../file-qcow2-backing-chain-encryption.json | 2 +- .../file-qcow2-backing-chain-encryption.xml | 2 +- ...etwork-qcow2-backing-chain-encryption_auth.json | 2 +- ...network-qcow2-backing-chain-encryption_auth.xml | 2 +- tests/qemumonitorjsontest.c | 2 - tests/qemuxml2argvdata/encrypted-disk-usage.args | 8 +- tests/qemuxml2argvdata/encrypted-disk-usage.xml | 2 +- tests/qemuxml2argvdata/encrypted-disk.args | 8 +- tests/qemuxml2argvdata/encrypted-disk.xml | 2 +- tests/qemuxml2argvdata/interface-server.xml | 3 - tests/qemuxml2argvdata/user-aliases.args | 8 +- tests/qemuxml2argvdata/user-aliases.xml | 2 +- tests/qemuxml2argvtest.c | 7 +- tests/qemuxml2xmloutdata/encrypted-disk.xml | 2 +- tests/qemuxml2xmloutdata/interface-server.xml | 3 - tests/qemuxml2xmltest.c | 6 +- 22 files changed, 46 insertions(+), 177 deletions(-) -- 2.16.2
The disk encryption part is no way relevant to the rest of the test so drop it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvdata/interface-server.xml | 3 --- tests/qemuxml2xmloutdata/interface-server.xml | 3 --- 2 files changed, 6 deletions(-) diff --git a/tests/qemuxml2argvdata/interface-server.xml b/tests/qemuxml2argvdata/interface-server.xml index a92aff4218..7bf119197a 100644 --- a/tests/qemuxml2argvdata/interface-server.xml +++ b/tests/qemuxml2argvdata/interface-server.xml @@ -53,9 +53,6 @@ <driver name='qemu' type='qcow2'/> <source file='/var/lib/libvirt/images/OtherDemo.img'/> <target dev='vdb' bus='virtio'/> - <encryption format='qcow'> - <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> - </encryption> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </disk> <disk type='file' device='cdrom'> diff --git a/tests/qemuxml2xmloutdata/interface-server.xml b/tests/qemuxml2xmloutdata/interface-server.xml index 049b1472a8..75b12bf96f 100644 --- a/tests/qemuxml2xmloutdata/interface-server.xml +++ b/tests/qemuxml2xmloutdata/interface-server.xml @@ -53,9 +53,6 @@ <driver name='qemu' type='qcow2'/> <source file='/var/lib/libvirt/images/OtherDemo.img'/> <target dev='vdb' bus='virtio'/> - <encryption format='qcow'> - <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> - </encryption> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </disk> <disk type='file' device='cdrom'> -- 2.16.2
On Wed, May 23, 2018 at 04:13:26PM +0200, Peter Krempa wrote:
The disk encryption part is no way relevant to the rest of the test so drop it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvdata/interface-server.xml | 3 --- tests/qemuxml2xmloutdata/interface-server.xml | 3 --- 2 files changed, 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Change the disk encryption type to qcow2+luks so that the appropriate secret objects are generated. This tests that the proper alias is used for the passphrase secret object. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvdata/user-aliases.args | 8 +++++++- tests/qemuxml2argvdata/user-aliases.xml | 2 +- tests/qemuxml2argvtest.c | 3 ++- tests/qemuxml2xmltest.c | 2 +- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/qemuxml2argvdata/user-aliases.args b/tests/qemuxml2argvdata/user-aliases.args index 5ef52fc556..293dc919d5 100644 --- a/tests/qemuxml2argvdata/user-aliases.args +++ b/tests/qemuxml2argvdata/user-aliases.args @@ -7,6 +7,8 @@ QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-x86_64 \ -name gentoo \ -S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-gentoo/master-key.aes \ -machine pc-i440fx-1.4,accel=kvm,usb=off,dump-guest-core=off \ -m 4096 \ -smp 4,sockets=4,cores=1,threads=1 \ @@ -43,7 +45,11 @@ id=drive-ua-myDisk1,cache=none \ -drive file=/var/lib/libvirt/images/gentoo.qcow2,format=qcow2,if=none,\ id=drive-ua-myDisk2 \ -device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-ua-myDisk2,id=ua-myDisk2 \ --drive file=/var/lib/libvirt/images/OtherDemo.img,format=qcow2,if=none,\ +-object secret,id=ua-myEncryptedDisk1-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive file=/var/lib/libvirt/images/OtherDemo.img,encrypt.format=luks,\ +encrypt.key-secret=ua-myEncryptedDisk1-luks-secret0,format=qcow2,if=none,\ id=drive-ua-myEncryptedDisk1 \ -device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-ua-myEncryptedDisk1,\ id=ua-myEncryptedDisk1 \ diff --git a/tests/qemuxml2argvdata/user-aliases.xml b/tests/qemuxml2argvdata/user-aliases.xml index 9ce123b477..98b4845e52 100644 --- a/tests/qemuxml2argvdata/user-aliases.xml +++ b/tests/qemuxml2argvdata/user-aliases.xml @@ -55,7 +55,7 @@ <driver name='qemu' type='qcow2'/> <source file='/var/lib/libvirt/images/OtherDemo.img'/> <target dev='vdb' bus='virtio'/> - <encryption format='qcow'> + <encryption format='luks'> <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> </encryption> <alias name='ua-myEncryptedDisk1'/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1d023129ac..38530cdb5c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2823,7 +2823,8 @@ mymain(void) QEMU_CAPS_PIIX_DISABLE_S4, QEMU_CAPS_VNC, QEMU_CAPS_DEVICE_ISA_SERIAL, QEMU_CAPS_HDA_DUPLEX, - QEMU_CAPS_CCID_EMULATED); + QEMU_CAPS_CCID_EMULATED, + QEMU_CAPS_QCOW2_LUKS, QEMU_CAPS_OBJECT_SECRET); DO_TEST("user-aliases2", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI); DO_TEST("user-aliases-usb", QEMU_CAPS_KVM, QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4, diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e31d8212fe..b4f9161056 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1171,7 +1171,7 @@ mymain(void) DO_TEST("pseries-cpu-exact", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); - DO_TEST("user-aliases", NONE); + DO_TEST("user-aliases", QEMU_CAPS_QCOW2_LUKS); DO_TEST("input-virtio-ccw", QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_KEYBOARD, -- 2.16.2
On Wed, May 23, 2018 at 04:13:27PM +0200, Peter Krempa wrote:
Change the disk encryption type to qcow2+luks so that the appropriate secret objects are generated. This tests that the proper alias is used for the passphrase secret object.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvdata/user-aliases.args | 8 +++++++- tests/qemuxml2argvdata/user-aliases.xml | 2 +- tests/qemuxml2argvtest.c | 3 ++- tests/qemuxml2xmltest.c | 2 +- 4 files changed, 11 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The next patch will forbid the old qcow2 encryption completely. Remove it from the tests. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json | 2 +- .../qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml | 2 +- .../xml2json/network-qcow2-backing-chain-encryption_auth.json | 2 +- .../xml2json/network-qcow2-backing-chain-encryption_auth.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json index 3469c06654..376fce9f9e 100644 --- a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json +++ b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json @@ -21,7 +21,7 @@ "read-only": true, "driver": "qcow2", "encrypt": { - "format": "aes", + "format": "luks", "key-secret": "node-b-f-encalias" }, "file": "node-b-s", diff --git a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml index a1292284bf..75a3a8f029 100644 --- a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml +++ b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml @@ -20,7 +20,7 @@ <nodename type='format' name='node-b-f'/> </nodenames> </privateData> - <encryption format='qcow'> + <encryption format='luks'> <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> </encryption> </source> diff --git a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json index 6e5abbfbdd..fdb6f2ab1a 100644 --- a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json +++ b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json @@ -33,7 +33,7 @@ "read-only": true, "driver": "qcow2", "encrypt": { - "format": "aes", + "format": "luks", "key-secret": "node-b-f-encalias" }, "file": "node-b-s", diff --git a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml index bc2925b4cf..a62c0321ec 100644 --- a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml +++ b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml @@ -26,7 +26,7 @@ <nodename type='format' name='node-b-f'/> </nodenames> </privateData> - <encryption format='qcow'> + <encryption format='luks'> <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> </encryption> <auth username='testuser-iscsi'> -- 2.16.2
On Wed, May 23, 2018 at 04:13:28PM +0200, Peter Krempa wrote:
The next patch will forbid the old qcow2 encryption completely. Remove it from the tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json | 2 +- .../qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml | 2 +- .../xml2json/network-qcow2-backing-chain-encryption_auth.json | 2 +- .../xml2json/network-qcow2-backing-chain-encryption_auth.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The encryption was buggy and qemu actually dropped it upstream. Forbid it for all versions since it would cause other problems too. Problems with the old encryption include weak crypto, corruption of images with blockjobs and a lot of usability problems. This requires changing of the encryption type for the encrypted disk tests. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ tests/qemuxml2argvdata/encrypted-disk-usage.args | 8 +++++++- tests/qemuxml2argvdata/encrypted-disk-usage.xml | 2 +- tests/qemuxml2argvdata/encrypted-disk.args | 8 +++++++- tests/qemuxml2argvdata/encrypted-disk.xml | 2 +- tests/qemuxml2argvtest.c | 4 ++-- tests/qemuxml2xmloutdata/encrypted-disk.xml | 2 +- tests/qemuxml2xmltest.c | 4 ++-- 8 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee676a2789..23dd4dab0e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4349,6 +4349,16 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, return -1; } + if ((src->format == VIR_STORAGE_FILE_QCOW || + src->format == VIR_STORAGE_FILE_QCOW2) && + src->encryption && + (src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT || + src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_QCOW)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("old qcow/qcow2 encryption is not supported")); + return -1; + } + if (src->format == VIR_STORAGE_FILE_QCOW2 && src->encryption && src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && diff --git a/tests/qemuxml2argvdata/encrypted-disk-usage.args b/tests/qemuxml2argvdata/encrypted-disk-usage.args index 8c7ce3d653..32307cea71 100644 --- a/tests/qemuxml2argvdata/encrypted-disk-usage.args +++ b/tests/qemuxml2argvdata/encrypted-disk-usage.args @@ -7,6 +7,8 @@ QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name encryptdisk \ -S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-encryptdisk/master-key.aes \ -machine pc,accel=tcg,usb=off,dump-guest-core=off \ -m 1024 \ -smp 1,sockets=1,cores=1,threads=1 \ @@ -22,7 +24,11 @@ path=/tmp/lib/domain--1-encryptdisk/monitor.sock,server,nowait \ -no-acpi \ -boot c \ -usb \ --drive file=/storage/guest_disks/encryptdisk,format=qcow2,if=none,\ +-object secret,id=virtio-disk0-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive file=/storage/guest_disks/encryptdisk,encrypt.format=luks,\ +encrypt.key-secret=virtio-disk0-luks-secret0,format=qcow2,if=none,\ id=drive-virtio-disk0 \ -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\ id=virtio-disk0 \ diff --git a/tests/qemuxml2argvdata/encrypted-disk-usage.xml b/tests/qemuxml2argvdata/encrypted-disk-usage.xml index ad8f17e3df..205283b59d 100644 --- a/tests/qemuxml2argvdata/encrypted-disk-usage.xml +++ b/tests/qemuxml2argvdata/encrypted-disk-usage.xml @@ -18,7 +18,7 @@ <driver name='qemu' type='qcow2'/> <source file='/storage/guest_disks/encryptdisk'/> <target dev='vda' bus='virtio'/> - <encryption format='qcow'> + <encryption format='luks'> <secret type='passphrase' usage='/storage/guest_disks/encryptdisk'/> </encryption> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> diff --git a/tests/qemuxml2argvdata/encrypted-disk.args b/tests/qemuxml2argvdata/encrypted-disk.args index 8c7ce3d653..32307cea71 100644 --- a/tests/qemuxml2argvdata/encrypted-disk.args +++ b/tests/qemuxml2argvdata/encrypted-disk.args @@ -7,6 +7,8 @@ QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name encryptdisk \ -S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-encryptdisk/master-key.aes \ -machine pc,accel=tcg,usb=off,dump-guest-core=off \ -m 1024 \ -smp 1,sockets=1,cores=1,threads=1 \ @@ -22,7 +24,11 @@ path=/tmp/lib/domain--1-encryptdisk/monitor.sock,server,nowait \ -no-acpi \ -boot c \ -usb \ --drive file=/storage/guest_disks/encryptdisk,format=qcow2,if=none,\ +-object secret,id=virtio-disk0-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive file=/storage/guest_disks/encryptdisk,encrypt.format=luks,\ +encrypt.key-secret=virtio-disk0-luks-secret0,format=qcow2,if=none,\ id=drive-virtio-disk0 \ -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\ id=virtio-disk0 \ diff --git a/tests/qemuxml2argvdata/encrypted-disk.xml b/tests/qemuxml2argvdata/encrypted-disk.xml index 391461b200..275724bdaf 100644 --- a/tests/qemuxml2argvdata/encrypted-disk.xml +++ b/tests/qemuxml2argvdata/encrypted-disk.xml @@ -18,7 +18,7 @@ <driver name='qemu' type='qcow2'/> <source file='/storage/guest_disks/encryptdisk'/> <target dev='vda' bus='virtio'/> - <encryption format='qcow'> + <encryption format='luks'> <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> </encryption> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 38530cdb5c..f9ac79f4a4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1635,8 +1635,8 @@ mymain(void) DO_TEST("cpu-tsc-frequency", QEMU_CAPS_KVM); qemuTestSetHostCPU(driver.caps, NULL); - DO_TEST("encrypted-disk", NONE); - DO_TEST("encrypted-disk-usage", NONE); + DO_TEST("encrypted-disk", QEMU_CAPS_QCOW2_LUKS, QEMU_CAPS_OBJECT_SECRET); + DO_TEST("encrypted-disk-usage", QEMU_CAPS_QCOW2_LUKS, QEMU_CAPS_OBJECT_SECRET); # ifdef WITH_GNUTLS DO_TEST("luks-disks", QEMU_CAPS_OBJECT_SECRET); DO_TEST("luks-disks-source", QEMU_CAPS_OBJECT_SECRET); diff --git a/tests/qemuxml2xmloutdata/encrypted-disk.xml b/tests/qemuxml2xmloutdata/encrypted-disk.xml index 45b9fcca55..3c9d2fbafc 100644 --- a/tests/qemuxml2xmloutdata/encrypted-disk.xml +++ b/tests/qemuxml2xmloutdata/encrypted-disk.xml @@ -18,7 +18,7 @@ <driver name='qemu' type='qcow2'/> <source file='/storage/guest_disks/encryptdisk'/> <target dev='vda' bus='virtio'/> - <encryption format='qcow'> + <encryption format='luks'> <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> </encryption> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index b4f9161056..51e5d6cdfc 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -480,8 +480,8 @@ mymain(void) DO_TEST("pci-rom-disabled-invalid", NONE); DO_TEST("pci-serial-dev-chardev", NONE); - DO_TEST("encrypted-disk", NONE); - DO_TEST("encrypted-disk-usage", NONE); + DO_TEST("encrypted-disk", QEMU_CAPS_QCOW2_LUKS); + DO_TEST("encrypted-disk-usage", QEMU_CAPS_QCOW2_LUKS); DO_TEST("luks-disks", NONE); DO_TEST("luks-disks-source", NONE); DO_TEST("memtune", NONE); -- 2.16.2
On Wed, May 23, 2018 at 04:13:29PM +0200, Peter Krempa wrote:
The encryption was buggy and qemu actually dropped it upstream. Forbid it for all versions since it would cause other problems too.
Problems with the old encryption include weak crypto, corruption of images with blockjobs and a lot of usability problems.
This requires changing of the encryption type for the encrypted disk tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ tests/qemuxml2argvdata/encrypted-disk-usage.args | 8 +++++++- tests/qemuxml2argvdata/encrypted-disk-usage.xml | 2 +- tests/qemuxml2argvdata/encrypted-disk.args | 8 +++++++- tests/qemuxml2argvdata/encrypted-disk.xml | 2 +- tests/qemuxml2argvtest.c | 4 ++-- tests/qemuxml2xmloutdata/encrypted-disk.xml | 2 +- tests/qemuxml2xmltest.c | 4 ++-- 8 files changed, 31 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Now that the old qcow2 encryption is removed we can safely delete all this code since it's not needed any more. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.c | 13 ------ src/qemu/qemu_monitor.h | 4 -- src/qemu/qemu_monitor_json.c | 28 ------------ src/qemu/qemu_monitor_json.h | 4 -- src/qemu/qemu_process.c | 103 ------------------------------------------- tests/qemumonitorjsontest.c | 2 - 6 files changed, 154 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 43f1d2f816..88a9226e7f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3044,19 +3044,6 @@ qemuMonitorAddDrive(qemuMonitorPtr mon, } -int -qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon, - const char *alias, - const char *passphrase) -{ - VIR_DEBUG("alias=%s passphrase=%p(value hidden)", alias, passphrase); - - QEMU_CHECK_MONITOR(mon); - - return qemuMonitorJSONSetDrivePassphrase(mon, alias, passphrase); -} - - int qemuMonitorCreateSnapshot(qemuMonitorPtr mon, const char *name) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index c28db1a52b..6200908f25 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -811,10 +811,6 @@ int qemuMonitorAddDrive(qemuMonitorPtr mon, int qemuMonitorDriveDel(qemuMonitorPtr mon, const char *drivestr); -int qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon, - const char *alias, - const char *passphrase); - int qemuMonitorCreateSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorLoadSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 9f5c358795..80e710902c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4062,34 +4062,6 @@ int qemuMonitorJSONDelObject(qemuMonitorPtr mon, } -int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, - const char *alias, - const char *passphrase) -{ - int ret = -1; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - - cmd = qemuMonitorJSONMakeCommand("block_passwd", - "s:device", alias, - "s:password", passphrase, - NULL); - if (!cmd) - return -1; - - if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; - - if (qemuMonitorJSONCheckError(cmd, reply) < 0) - goto cleanup; - - ret = 0; - cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; -} - int qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr actions, const char *device, const char *file, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index f4ac8319ac..1c83760dc6 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -237,10 +237,6 @@ int qemuMonitorJSONAddObject(qemuMonitorPtr mon, int qemuMonitorJSONDelObject(qemuMonitorPtr mon, const char *objalias); -int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, - const char *alias, - const char *passphrase); - int qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr actions, const char *device, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ac2049b95d..d76e3e28a0 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -370,74 +370,6 @@ qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm, return NULL; } -static int -qemuProcessGetVolumeQcowPassphrase(virDomainDiskDefPtr disk, - char **secretRet, - size_t *secretLen) -{ - virConnectPtr conn = NULL; - char *passphrase; - unsigned char *data; - size_t size; - int ret = -1; - virStorageEncryptionPtr enc; - - if (!disk->src->encryption) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("disk %s does not have any encryption information"), - disk->src->path); - return -1; - } - enc = disk->src->encryption; - - if (!(conn = virGetConnectSecret())) - goto cleanup; - - if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW || - enc->nsecrets != 1 || - enc->secrets[0]->type != - VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid <encryption> for volume %s"), - virDomainDiskGetSource(disk)); - goto cleanup; - } - - if (virSecretGetSecretString(conn, &enc->secrets[0]->seclookupdef, - VIR_SECRET_USAGE_TYPE_VOLUME, - &data, &size) < 0) - goto cleanup; - - if (memchr(data, '\0', size) != NULL) { - memset(data, 0, size); - VIR_FREE(data); - virReportError(VIR_ERR_XML_ERROR, - _("format='qcow' passphrase for %s must not contain a " - "'\\0'"), virDomainDiskGetSource(disk)); - goto cleanup; - } - - if (VIR_ALLOC_N(passphrase, size + 1) < 0) { - memset(data, 0, size); - VIR_FREE(data); - goto cleanup; - } - memcpy(passphrase, data, size); - passphrase[size] = '\0'; - - memset(data, 0, size); - VIR_FREE(data); - - *secretRet = passphrase; - *secretLen = size; - - ret = 0; - - cleanup: - virObjectUnref(conn); - return ret; -} - static int qemuProcessHandleReset(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainObjPtr vm, @@ -2728,11 +2660,8 @@ qemuProcessInitPasswords(virQEMUDriverPtr driver, int asyncJob) { int ret = 0; - qemuDomainObjPrivatePtr priv = vm->privateData; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); size_t i; - char *alias = NULL; - char *secret = NULL; for (i = 0; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; @@ -2754,39 +2683,7 @@ qemuProcessInitPasswords(virQEMUDriverPtr driver, goto cleanup; } - for (i = 0; i < vm->def->ndisks; i++) { - size_t secretLen; - - if (!vm->def->disks[i]->src->encryption || - !virDomainDiskGetSource(vm->def->disks[i])) - continue; - - if (vm->def->disks[i]->src->encryption->format != - VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT && - vm->def->disks[i]->src->encryption->format != - VIR_STORAGE_ENCRYPTION_FORMAT_QCOW) - continue; - - VIR_FREE(secret); - if (qemuProcessGetVolumeQcowPassphrase(vm->def->disks[i], - &secret, &secretLen) < 0) - goto cleanup; - - VIR_FREE(alias); - if (!(alias = qemuAliasFromDisk(vm->def->disks[i]))) - goto cleanup; - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - goto cleanup; - ret = qemuMonitorSetDrivePassphrase(priv->mon, alias, secret); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - ret = -1; - if (ret < 0) - goto cleanup; - } - cleanup: - VIR_FREE(alias); - VIR_FREE(secret); virObjectUnref(cfg); return ret; } diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index add5ff0f19..3b494a1dba 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1343,7 +1343,6 @@ GEN_TEST_FUNC(qemuMonitorJSONAddNetdev, "id=net0,type=test") GEN_TEST_FUNC(qemuMonitorJSONRemoveNetdev, "net0") GEN_TEST_FUNC(qemuMonitorJSONDelDevice, "ide0") GEN_TEST_FUNC(qemuMonitorJSONAddDevice, "some_dummy_devicestr") -GEN_TEST_FUNC(qemuMonitorJSONSetDrivePassphrase, "drive-vda", "secret_passhprase") GEN_TEST_FUNC(qemuMonitorJSONDriveMirror, "vdb", "/foo/bar", NULL, 1024, 0, 0, VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) GEN_TEST_FUNC(qemuMonitorJSONBlockCommit, "vdb", "/foo/bar1", "/foo/bar2", NULL, 1024) @@ -2967,7 +2966,6 @@ mymain(void) DO_TEST_GEN(qemuMonitorJSONRemoveNetdev); DO_TEST_GEN(qemuMonitorJSONDelDevice); DO_TEST_GEN(qemuMonitorJSONAddDevice); - DO_TEST_GEN(qemuMonitorJSONSetDrivePassphrase); DO_TEST_GEN(qemuMonitorJSONDriveMirror); DO_TEST_GEN(qemuMonitorJSONBlockCommit); DO_TEST_GEN(qemuMonitorJSONDrivePivot); -- 2.16.2
On Wed, May 23, 2018 at 04:13:30PM +0200, Peter Krempa wrote:
Now that the old qcow2 encryption is removed we can safely delete all this code since it's not needed any more.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.c | 13 ------ src/qemu/qemu_monitor.h | 4 -- src/qemu/qemu_monitor_json.c | 28 ------------ src/qemu/qemu_monitor_json.h | 4 -- src/qemu/qemu_process.c | 103 ------------------------------------------- tests/qemumonitorjsontest.c | 2 - 6 files changed, 154 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On 05/23/2018 10:13 AM, Peter Krempa wrote:
The old qcow/qcow2 encryption format is so broken that qemu decided to drop it completely. This series forbids the use of such images even with qemus prior to this and removes all the cruft necessary to support it.
v2: - fixed check to include the qcow format too - reworded the error message slightly - split second patch into two with proper justification for the user-alias test since checking LUKS there actually makes sense
Peter Krempa (5): tests: qemuxml2argv: Drop disk encryption from 'interface-server' test tests: qemuxml2argv: Verify that disk secret alias is correct with user-aliases tests: qemublock: Switch to qcow2+luks in test files qemu: domain: Forbid storage with old QCOW2 encryption qemu: Remove code for setting up disk passphrases
Why not remove it from storage as well? It's not like anything could or would want to use whatever the storage driver created. There's always the fall back to indicate to use qemu-img for the die hards. John
src/qemu/qemu_domain.c | 10 ++ src/qemu/qemu_monitor.c | 13 --- src/qemu/qemu_monitor.h | 4 - src/qemu/qemu_monitor_json.c | 28 ------ src/qemu/qemu_monitor_json.h | 4 - src/qemu/qemu_process.c | 103 --------------------- .../file-qcow2-backing-chain-encryption.json | 2 +- .../file-qcow2-backing-chain-encryption.xml | 2 +- ...etwork-qcow2-backing-chain-encryption_auth.json | 2 +- ...network-qcow2-backing-chain-encryption_auth.xml | 2 +- tests/qemumonitorjsontest.c | 2 - tests/qemuxml2argvdata/encrypted-disk-usage.args | 8 +- tests/qemuxml2argvdata/encrypted-disk-usage.xml | 2 +- tests/qemuxml2argvdata/encrypted-disk.args | 8 +- tests/qemuxml2argvdata/encrypted-disk.xml | 2 +- tests/qemuxml2argvdata/interface-server.xml | 3 - tests/qemuxml2argvdata/user-aliases.args | 8 +- tests/qemuxml2argvdata/user-aliases.xml | 2 +- tests/qemuxml2argvtest.c | 7 +- tests/qemuxml2xmloutdata/encrypted-disk.xml | 2 +- tests/qemuxml2xmloutdata/interface-server.xml | 3 - tests/qemuxml2xmltest.c | 6 +- 22 files changed, 46 insertions(+), 177 deletions(-)
On Wed, May 30, 2018 at 16:14:31 -0400, John Ferlan wrote:
On 05/23/2018 10:13 AM, Peter Krempa wrote:
The old qcow/qcow2 encryption format is so broken that qemu decided to drop it completely. This series forbids the use of such images even with qemus prior to this and removes all the cruft necessary to support it.
v2: - fixed check to include the qcow format too - reworded the error message slightly - split second patch into two with proper justification for the user-alias test since checking LUKS there actually makes sense
Peter Krempa (5): tests: qemuxml2argv: Drop disk encryption from 'interface-server' test tests: qemuxml2argv: Verify that disk secret alias is correct with user-aliases tests: qemublock: Switch to qcow2+luks in test files qemu: domain: Forbid storage with old QCOW2 encryption qemu: Remove code for setting up disk passphrases
Why not remove it from storage as well? It's not like anything could or would want to use whatever the storage driver created. There's always the fall back to indicate to use qemu-img for the die hards.
If we've ever supported the use case of converting a qcow2 encrypted volume even into a unencrypted volume, we should keep that for allowing migration from those volumes.
On 05/31/2018 02:18 AM, Peter Krempa wrote:
On Wed, May 30, 2018 at 16:14:31 -0400, John Ferlan wrote:
On 05/23/2018 10:13 AM, Peter Krempa wrote:
The old qcow/qcow2 encryption format is so broken that qemu decided to drop it completely. This series forbids the use of such images even with qemus prior to this and removes all the cruft necessary to support it.
v2: - fixed check to include the qcow format too - reworded the error message slightly - split second patch into two with proper justification for the user-alias test since checking LUKS there actually makes sense
Peter Krempa (5): tests: qemuxml2argv: Drop disk encryption from 'interface-server' test tests: qemuxml2argv: Verify that disk secret alias is correct with user-aliases tests: qemublock: Switch to qcow2+luks in test files qemu: domain: Forbid storage with old QCOW2 encryption qemu: Remove code for setting up disk passphrases
Why not remove it from storage as well? It's not like anything could or would want to use whatever the storage driver created. There's always the fall back to indicate to use qemu-img for the die hards.
If we've ever supported the use case of converting a qcow2 encrypted volume even into a unencrypted volume, we should keep that for allowing migration from those volumes.
Without (at least parts of) for qemu's 2.9 or later: https://www.redhat.com/archives/libvir-list/2018-May/msg01268.html it won't work anyway because of qemu secret work. I think you probably need to make some documentation updates too. If not removing things, then updating certain pages to indicate as of libvirt 4.X.0 it won't be possible to use for domains (and possibly storage). John
On Thu, May 31, 2018 at 07:22:28 -0400, John Ferlan wrote:
On 05/31/2018 02:18 AM, Peter Krempa wrote:
On Wed, May 30, 2018 at 16:14:31 -0400, John Ferlan wrote:
On 05/23/2018 10:13 AM, Peter Krempa wrote:
The old qcow/qcow2 encryption format is so broken that qemu decided to drop it completely. This series forbids the use of such images even with qemus prior to this and removes all the cruft necessary to support it.
v2: - fixed check to include the qcow format too - reworded the error message slightly - split second patch into two with proper justification for the user-alias test since checking LUKS there actually makes sense
Peter Krempa (5): tests: qemuxml2argv: Drop disk encryption from 'interface-server' test tests: qemuxml2argv: Verify that disk secret alias is correct with user-aliases tests: qemublock: Switch to qcow2+luks in test files qemu: domain: Forbid storage with old QCOW2 encryption qemu: Remove code for setting up disk passphrases
Why not remove it from storage as well? It's not like anything could or would want to use whatever the storage driver created. There's always the fall back to indicate to use qemu-img for the die hards.
If we've ever supported the use case of converting a qcow2 encrypted volume even into a unencrypted volume, we should keep that for allowing migration from those volumes.
Without (at least parts of) for qemu's 2.9 or later:
https://www.redhat.com/archives/libvir-list/2018-May/msg01268.html
it won't work anyway because of qemu secret work.
Well, given that the required setup for qcow2 encryption is almost identical to luks I think we can support the old encryption on the input of the conversion process.
I think you probably need to make some documentation updates too. If not removing things, then updating certain pages to indicate as of libvirt 4.X.0 it won't be possible to use for domains (and possibly storage).
Hmm, yeah, there should be a section describing the <encryption> element. I'll add a note there and into news.xml. (probably as a separate patch.
John
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
John Ferlan -
Ján Tomko -
Peter Krempa