[libvirt] [RFC PATCH 0/4] qemu: Forbid old qcow2 encryption
This applies on top of the text monitor cleanup. See explanation in 3/4 for justification. Peter Krempa (4): tests: qemublock: Switch to qcow2+luks in test files tests: qemu: Modernize/remove qcow2 encryption from tests not related to storage qemu: domain: Forbid storage with old QCOW2 encryption qemu: Remove code for setting up disk passphrases src/qemu/qemu_domain.c | 20 ++-- 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, 50 insertions(+), 183 deletions(-) -- 2.16.2
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 94e2ecd1e2..7be2894812 100644 --- a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json +++ b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json @@ -20,7 +20,7 @@ "read-only": true, "driver": "qcow2", "encrypt": { - "format": "aes", + "format": "luks", "key-secret": "node-b-f-encalias" }, "file": { 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 7e7a4e44f7..364d6066e6 100644 --- a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json +++ b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json @@ -32,7 +32,7 @@ "read-only": true, "driver": "qcow2", "encrypt": { - "format": "aes", + "format": "luks", "key-secret": "node-b-f-encalias" }, "file": { 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
Remove the storage encryption completely from interface-server.xml as it serves no purpose there. For the user-alias test use the 'luks' encryption type for qcow to test that the names of the secret objects are generated properly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvdata/interface-server.xml | 3 --- tests/qemuxml2argvdata/user-aliases.args | 8 +++++++- tests/qemuxml2argvdata/user-aliases.xml | 2 +- tests/qemuxml2argvtest.c | 3 ++- tests/qemuxml2xmloutdata/interface-server.xml | 3 --- tests/qemuxml2xmltest.c | 2 +- 6 files changed, 11 insertions(+), 10 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/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 78454acb1a..ee2b0ccff8 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2819,7 +2819,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/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'> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 7cedc2b999..5755800dcf 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
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. Replace it with a message hinting that users should convert the image to e.g. LUKS. 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 | 20 ++++++++++++++------ 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, 35 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d3beee5d87..f64b69cc3d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4179,12 +4179,20 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, } if (src->format == VIR_STORAGE_FILE_QCOW2 && - src->encryption && - src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_QCOW2_LUKS)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("LUKS encrypted QCOW2 images are not suppored by this QEMU")); - return -1; + src->encryption) { + if (src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_QCOW2_LUKS)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("LUKS encrypted QCOW2 images are not suppored by this QEMU")); + return -1; + } + + if (src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT || + src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_QCOW) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("old QCOW2 encryption is not supported, please convert images")); + return -1; + } } if (src->format == VIR_STORAGE_FILE_FAT && 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 ee2b0ccff8..860e23f41a 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1631,8 +1631,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 5755800dcf..2610dfe086 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
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 02d2629eb0..c4a77be9c5 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3039,19 +3039,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 212d1e3e16..5024cb75a5 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 42afa6201f..1fee9e8ff6 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4034,34 +4034,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 8a9c214c82..056e0f144c 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 2ba432630f..bb5cc3c310 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -368,74 +368,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, @@ -2726,11 +2658,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]; @@ -2752,39 +2681,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 05/22/2018 10:04 AM, Peter Krempa wrote:
This applies on top of the text monitor cleanup. See explanation in 3/4 for justification.
Peter Krempa (4): tests: qemublock: Switch to qcow2+luks in test files tests: qemu: Modernize/remove qcow2 encryption from tests not related to storage qemu: domain: Forbid storage with old QCOW2 encryption qemu: Remove code for setting up disk passphrases
This would be nice, but based on this series: https://www.redhat.com/archives/libvir-list/2018-May/msg01268.html I believe there are quite a few more tests/files to modify/delete in order to remove qcow[2] from the source tree. There's also the formatstorageencryption and formatsecret documentation that would need updating. Based only on the effort from the above series to convert/consume a non encrypted image to result in a qcow[2] encrypted image - I assume conversion of qcow[2] images is not a simple exercise. Not sure whether anyone really uses qcow[2] encryption anymore in the wild, but just telling them they have to convert (without providing a shred of details as to what that entails isn't very friendly. Also not sure it's possible to just convert to using LUKS since at one time at least usage required having code/tests inside a "# ifdef WITH_GNUTLS" (something that can be seen in the diffs from tests/qemuxml2argvtest.c in patch 3). John
src/qemu/qemu_domain.c | 20 ++-- 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, 50 insertions(+), 183 deletions(-)
On Tue, May 22, 2018 at 10:40:39 -0400, John Ferlan wrote:
On 05/22/2018 10:04 AM, Peter Krempa wrote:
This applies on top of the text monitor cleanup. See explanation in 3/4 for justification.
Peter Krempa (4): tests: qemublock: Switch to qcow2+luks in test files tests: qemu: Modernize/remove qcow2 encryption from tests not related to storage qemu: domain: Forbid storage with old QCOW2 encryption qemu: Remove code for setting up disk passphrases
This would be nice, but based on this series:
https://www.redhat.com/archives/libvir-list/2018-May/msg01268.html
I believe there are quite a few more tests/files to modify/delete in order to remove qcow[2] from the source tree.
Yes, because the check in 3/4 only does this for qcow2, but it also should be done for qcow.
There's also the formatstorageencryption and formatsecret documentation that would need updating.
Yep.
Based only on the effort from the above series to convert/consume a non encrypted image to result in a qcow[2] encrypted image - I assume conversion of qcow[2] images is not a simple exercise. Not sure whether anyone really uses qcow[2] encryption anymore in the wild, but just telling them they have to convert (without providing a shred of details as to what that entails isn't very friendly.
Starting with qemu 2.7 qcow[2] encryption can't be used with system emulators only with qemu-img. It was deprecated since 2.3. While this breaks compatibility with old qemus the upstream support for this is declared dead. With these patches you get a failure even with old qemus and you know that you have to fix your images rather than waiting for the doom which can happen. commit 8c0dcbc4ad2bf4f9f3b27c637b357e87cad70ec7 Author: Daniel P. Berrange <berrange@redhat.com> Date: Mon Jun 13 12:30:09 2016 +0100 block: drop support for using qcow[2] encryption with system emulators Back in the 2.3.0 release we declared qcow[2] encryption as deprecated, warning people that it would be removed in a future release. commit a1f688f4152e65260b94f37543521ceff8bfebe4 Author: Markus Armbruster <armbru@redhat.com> Date: Fri Mar 13 21:09:40 2015 +0100 block: Deprecate QCOW/QCOW2 encryption
Also not sure it's possible to just convert to using LUKS since at one time at least usage required having code/tests inside a "# ifdef WITH_GNUTLS" (something that can be seen in the diffs from tests/qemuxml2argvtest.c in patch 3).
Well, without gnutls this will not work, but in that case even qemu encryption will most probably not work.
participants (2)
-
John Ferlan -
Peter Krempa