[libvirt] [PATCH v2 0/5] qemu: Forbid old qcow/qcow2 encryption

Peter Krempa posted 5 patches 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cover.1527084603.git.pkrempa@redhat.com
Test syntax-check passed
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(-)
[libvirt] [PATCH v2 0/5] qemu: Forbid old qcow/qcow2 encryption
Posted by Peter Krempa 5 years, 10 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/5] qemu: Forbid old qcow/qcow2 encryption
Posted by John Ferlan 5 years, 10 months ago

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(-)
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/5] qemu: Forbid old qcow/qcow2 encryption
Posted by Peter Krempa 5 years, 10 months ago
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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/5] qemu: Forbid old qcow/qcow2 encryption
Posted by John Ferlan 5 years, 10 months ago

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/5] qemu: Forbid old qcow/qcow2 encryption
Posted by Peter Krempa 5 years, 10 months ago
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list