Bug 1575578 - Failed to convert a source image to the qcow2 image encrypted by luks
Summary: Failed to convert a source image to the qcow2 image encrypted by luks
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: qemu-kvm-rhev
Version: 7.6
Hardware: Unspecified
OS: Unspecified
high
medium
Target Milestone: rc
: ---
Assignee: Kevin Wolf
QA Contact: Tingting Mao
URL:
Whiteboard:
Depends On:
Blocks: 1625604 1727821
TreeView+ depends on / blocked
 
Reported: 2018-05-07 10:42 UTC by Tingting Mao
Modified: 2019-07-08 09:20 UTC (History)
15 users (show)

Fixed In Version: qemu-kvm-rhev-2.12.0-13.el7
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1625604 1727821 (view as bug list)
Environment:
Last Closed: 2018-11-01 11:09:52 UTC
Target Upstream Version:


Attachments (Terms of Use)

Description Tingting Mao 2018-05-07 10:42:10 UTC
Description of problem:
Failed to convert a source image to the qcow2 image encrypted by luks

Version-Release number of selected component (if applicable):
kernel-3.10.0-880.el7
qemu-kvm-rhev-2.12.0-1.el7

How reproducible:
100%

Steps to Reproduce:
1. Create image with luks-inside-qcow2
# qemu-img create --object secret,id=sec0,data=backing -f qcow2 -o encrypt.format=luks,encrypt.key-secret=sec0 base.qcow2 20G
2.Convert the base.qcow2 to a qcow2 image with luks-inside-qcow2
# qemu-img convert -p --object secret,id=sec0,data=backing --object secret,id=sec2,data=convert --image-opts driver=qcow2,encrypt.key-secret=sec0,file.filename=base.qcow2 -O qcow2 -o encrypt.format=luks,encrypt.key-secret=sec2 convert.qcow2

Actual results:
Convert failed with the message as below:
qemu-img: Could not open 'convert.qcow2': Parameter 'encrypt.key-secret' is required for cipher

Expected results:
Convert successfully.

Additional info:
1.Convert executed successfully without encrypt option.
# qemu-img convert -p --object secret,id=sec0,data=backing --image-opts driver=qcow2,encrypt.key-secret=sec0,file.filename=base.qcow2 -O qcow2 convert.qcow2
    (100.00/100%)
2.Convert command executed successfully in the host of below environment.
Host environment:
kernel-3.10.0-862.el7
qemu-kvm-rhev-2.10.0-21.el7_5.1
# qemu-img create --object secret,id=sec0,data=backing -f qcow2 -o encrypt.format=luks,encrypt.key-secret=sec0 base.qcow2 2G
# qemu-img convert -p --object secret,id=sec0,data=backing --object secret,id=sec2,data=convert --image-opts driver=qcow2,encrypt.key-secret=sec0,file.filename=base.qcow2 -O qcow2 -o encrypt.format=luks,encrypt.key-secret=sec2 convert.qcow2
    (100.00/100%)

Comment 3 Tingting Mao 2018-06-19 06:31:41 UTC
For a luks image convert to another luks image, there is still the same bug.

Version-Release number of selected component:
qemu-kvm-rhev-2.12.0-3.el7
kernel-3.10.0-906.el7


Steps:
1.Create a luks image 
# qemu-img create -f luks --object secret,id=sec0,data=base -o key-secret=sec0 test.luks 10G
Formatting 'test.luks', fmt=luks size=10737418240 key-secret=sec0
2.Convert the luks image to another luks image
#  qemu-img convert -p --object secret,id=sec0,data=base --object secret,id=sec1,data=convert --image-opts driver=luks,key-secret=sec0,file.filename=test.luks -O luks -o key-secret=sec1 convert.luks
qemu-img: Could not open 'convert.luks': Parameter 'key-secret' is required for cipher

Comment 4 Ademar Reis 2018-06-19 21:19:31 UTC
(In reply to timao from comment #3)
> For a luks image convert to another luks image, there is still the same bug.
> 
> Version-Release number of selected component:
> qemu-kvm-rhev-2.12.0-3.el7
> kernel-3.10.0-906.el7
> 
> 
> Steps:
> 1.Create a luks image 
> # qemu-img create -f luks --object secret,id=sec0,data=base -o
> key-secret=sec0 test.luks 10G
> Formatting 'test.luks', fmt=luks size=10737418240 key-secret=sec0
> 2.Convert the luks image to another luks image
> #  qemu-img convert -p --object secret,id=sec0,data=base --object
> secret,id=sec1,data=convert --image-opts
> driver=luks,key-secret=sec0,file.filename=test.luks -O luks -o
> key-secret=sec1 convert.luks
> qemu-img: Could not open 'convert.luks': Parameter 'key-secret' is required
> for cipher

I've reproduced and confirmed it as a regression between qemu 2.11 and 2.12. Didn't bisect it though.

Comment 5 Cathy Avery 2018-07-17 17:12:27 UTC
The problem is patch

commit 1ec4f4160a1a94cf3d13b43551fff2792bd5056e 
Author: Kevin Wolf <kwolf@redhat.com>
Date:   Fri Mar 2 14:16:36 2018 +0100

    luks: Create block_crypto_co_create_generic()

when running qemu-img convert.

block_crypto_co_create_opts_luks() uses qemu_opts_to_qdict_filtered() replacing the previous version's use of qemu_opts_to_qdict() and ends up deleting the key-secret option in opts. Subsequently qcrypto_block_luks_open errors with the key-secret (sec1) as being missing.

Not sure why this was done. If I set qemu_opts_to_qdict_filtered so it does not delete the key-secret option everything works fine. I'll speak to Kevin about it.

Comment 7 Daniel Berrangé 2018-07-23 10:54:44 UTC
(In reply to timao from comment #0)
> Description of problem:
> Failed to convert a source image to the qcow2 image encrypted by luks
> 
> Version-Release number of selected component (if applicable):
> kernel-3.10.0-880.el7
> qemu-kvm-rhev-2.12.0-1.el7
> 
> How reproducible:
> 100%
> 
> Steps to Reproduce:
> 1. Create image with luks-inside-qcow2
> # qemu-img create --object secret,id=sec0,data=backing -f qcow2 -o
> encrypt.format=luks,encrypt.key-secret=sec0 base.qcow2 20G
> 2.Convert the base.qcow2 to a qcow2 image with luks-inside-qcow2
> # qemu-img convert -p --object secret,id=sec0,data=backing --object
> secret,id=sec2,data=convert --image-opts
> driver=qcow2,encrypt.key-secret=sec0,file.filename=base.qcow2 -O qcow2 -o
> encrypt.format=luks,encrypt.key-secret=sec2 convert.qcow2
> 
> Actual results:
> Convert failed with the message as below:
> qemu-img: Could not open 'convert.qcow2': Parameter 'encrypt.key-secret' is
> required for cipher
> 
> Expected results:
> Convert successfully.
> 
> Additional info:
> 1.Convert executed successfully without encrypt option.
> # qemu-img convert -p --object secret,id=sec0,data=backing --image-opts
> driver=qcow2,encrypt.key-secret=sec0,file.filename=base.qcow2 -O qcow2
> convert.qcow2
>     (100.00/100%)
> 2.Convert command executed successfully in the host of below environment.
> Host environment:
> kernel-3.10.0-862.el7
> qemu-kvm-rhev-2.10.0-21.el7_5.1
> # qemu-img create --object secret,id=sec0,data=backing -f qcow2 -o
> encrypt.format=luks,encrypt.key-secret=sec0 base.qcow2 2G
> # qemu-img convert -p --object secret,id=sec0,data=backing --object
> secret,id=sec2,data=convert --image-opts
> driver=qcow2,encrypt.key-secret=sec0,file.filename=base.qcow2 -O qcow2 -o
> encrypt.format=luks,encrypt.key-secret=sec2 convert.qcow2
>     (100.00/100%)

This is a known limitation of the 'convert' command.  The options specified give the images for creating the target image, but there's no way to pass options for opening the target image at the same time.

To use 'convert' with luks, you must pre-create the *target* image and then use '-n' arg to 'convert'.  You can then use --target-image-opts to specify the right options for 'convert' to open this pre-created target image.

Comment 8 Tingting Mao 2018-07-23 11:39:40 UTC
(In reply to Daniel Berrange from comment #7)

> This is a known limitation of the 'convert' command.  The options specified
> give the images for creating the target image, but there's no way to pass
> options for opening the target image at the same time.

Firstly, thanks for your information.

However, for qemu-kvm-rhev-2.10.0, we test this scenario is exactly like comment 0,it passed (the details is in Additional info in comment 0). So I wonder to know why it does not work in the new build - qemu-kvm-rhev-2.12.0. Thanks in advance.
 
> To use 'convert' with luks, you must pre-create the *target* image and then
> use '-n' arg to 'convert'.  You can then use --target-image-opts to specify
> the right options for 'convert' to open this pre-created target image.

Yeah, it works. And if we should use 'convert' with luks like you mentioned, it is better to add more instruction in the manpage, would you?

# qemu-img create --object secret,id=sec0,data=backing -f qcow2 -o encrypt.format=luks,encrypt.key-secret=sec0 base.qcow2 20G
# qemu-img create --object secret,id=sec1,data=convert -f qcow2 -o encrypt.format=luks,encrypt.key-secret=sec1 target.qcow2 20G
# qemu-img convert --object secret,id=sec0,data=backing --object secret,id=sec2,data=convert --image-opts driver=qcow2,encrypt.key-secret=sec0,file.filename=base.qcow2 --target-image-opts driver=qcow2,encrypt.key-secret=sec2,file.filename=target.qcow2 -n -p
    (100.00/100%)
# qemu-img compare --object secret,id=sec0,data=backing --object secret,id=sec2,data=convert --image-opts driver=qcow2,encrypt.key-secret=sec0,file.filename=base.qcow2 driver=qcow2,encrypt.key-secret=sec2,file.filename=target.qcow2 -p
Images are identical.

Comment 9 Daniel Berrangé 2018-07-23 11:42:20 UTC
(In reply to timao from comment #8)
> (In reply to Daniel Berrange from comment #7)
> 
> > This is a known limitation of the 'convert' command.  The options specified
> > give the images for creating the target image, but there's no way to pass
> > options for opening the target image at the same time.
> 
> Firstly, thanks for your information.
> 
> However, for qemu-kvm-rhev-2.10.0, we test this scenario is exactly like
> comment 0,it passed (the details is in Additional info in comment 0). So I
> wonder to know why it does not work in the new build - qemu-kvm-rhev-2.12.0.

I don't really understand how it works in that old version - it is *not* expected to work, so I wonder if it actually did what it claimed to do.

Comment 10 Cathy Avery 2018-07-23 11:53:47 UTC
(In reply to Daniel Berrange from comment #9)

I had no problem reproducing this and I root sourced it as explained. Prior to the patch there was no error. After the patch there was the error which was fixed by not deleting key-secret = sec1. The version I used was 2.12.90. As for it doing what it claimed with or without the error that is something I don't know how to verify.

Comment 11 Daniel Berrangé 2018-07-23 11:56:56 UTC
Oh actually there was a hack I completely forgot about in qemu-img that attempted to deal with this:

commit 29cf933635a50a4f1c51b022b323089997471e38
Author: Daniel P. Berrange <berrange@redhat.com>
Date:   Mon May 15 17:47:12 2017 +0100

    qemu-img: copy *key-secret opts when opening newly created files
    
    The qemu-img dd/convert commands will create an image file and
    then try to open it. Historically it has been possible to open
    new files without passing any options. With encrypted files
    though, the *key-secret options are mandatory, so we need to
    provide those options when opening the newly created file.
    
    Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
    Message-id: 20170515164712.6643-5-berrange@redhat.com
    Reviewed-by: Max Reitz <mreitz@redhat.com>
    Signed-off-by: Max Reitz <mreitz@redhat.com>

So that is possibly what made it work originally. If so, I wonder how that got broken though...

Comment 12 Cathy Avery 2018-07-23 12:08:00 UTC
(In reply to Daniel Berrange from comment #11)
Daniel could you please address my findings?

Comment 13 Daniel Berrangé 2018-07-23 13:05:10 UTC
Git bisect has blamed the following commit for the problem

commit b76b4f604521e59f857d6177bc55f6f2e41fd392 (HEAD, refs/bisect/bad)
Author: Kevin Wolf <kwolf@redhat.com>
Date:   Thu Jan 11 16:18:08 2018 +0100

    qcow2: Use visitor for options in qcow2_create()
    
    Instead of manually creating the BlockdevCreateOptions object, use a
    visitor to parse the given options into the QAPI object.
    
    This involves translation from the old command line syntax to the syntax
    mandated by the QAPI schema. Option names are still checked against
    qcow2_create_opts, so only the old option names are allowed on the
    command line, even if they are translated in qcow2_create().
    
    In contrast, new option values are optionally recognised besides the old
    values: 'compat' accepts 'v2'/'v3' as an alias for '0.10'/'1.1', and
    'encrypt.format' accepts 'qcow' as an alias for 'aes' now.
    
    Signed-off-by: Kevin Wolf <kwolf@redhat.com>
    Reviewed-by: Max Reitz <mreitz@redhat.com>
    Reviewed-by: Eric Blake <eblake@redhat.com>


And this makes some sense when I look at it in context of qemu-img convert code and commit 29cf933635a50a4f1c51b022b323089997471e38 that I mentioned previously.

The 'img_convert' method in qemu-img.c has a QemuOpts variable called "opts" that contains all the options used when creating the target image.

Before opening the target image, it processes this "opts" variable an extracts any values matching "*key-secret", and puts them into the QDict that is used when opening the target image.

IOW, we're copying the encrypt.key-secret parameter.

It turns out that this code was relying on a bug in the qcow2_co_create() method's previous impl.

When qcow2_co_create() runs, it is supposed to remove all values from the QemuOpts parameter that it receives, so when it returns the QemuOpts list should be empty.  For some reason in the old impl, we never removed the 'encrypt.key-secret' parameter.

So encrypt.key-secret  was still present in the QemuOpts when qemu-img.c does a call to img_open_new_file().

With Kevin's change, qcow2_co_create() correctly purges all entries in the QemuOpts it receives. Thus when img_open_new_file() is called later, we're missing the encrypt.key-secret.

So we need a fix in qemu-img, img_convert method so that it doesn't rely on the 'QemuOpts *opts' variable being populated after bdrv_create() returns.

Comment 14 Cathy Avery 2018-07-23 13:28:27 UTC
(In reply to Daniel Berrange from comment #13)

I should mention that I was using the method from comment 3 to reproduce this issue and track it down so I am seeing the same problem in block_crypto_co_create_opts_luks that you just described for qcow2_co_create. 

Are Timao's test cases valid? There initially seemed to be some question about this. Now it appears that this is a legitimate issue. Is that correct?

Comment 16 Daniel Berrangé 2018-08-08 07:39:02 UTC
@Cathy Yes, this does look like a genuine issue

Comment 25 Daniel Berrangé 2018-08-14 09:57:21 UTC
Posted upstream as https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg02366.html

Comment 30 Miroslav Rezanina 2018-09-04 14:33:00 UTC
Fix included in qemu-kvm-rhev-2.12.0-13.el7

Comment 32 Tingting Mao 2018-09-05 05:21:14 UTC
Verified this issue as below.


Tested packages:
kernel-3.10.0-944.el7
qemu-kvm-rhev-2.12.0-13.el7


Scenario1 (luks-inside-qcow2)
1. Create image
# qemu-img create --object secret,id=sec0,data=backing -f qcow2 -o encrypt.format=luks,encrypt.key-secret=sec0 base.qcow2 20G
Formatting 'base.qcow2', fmt=qcow2 size=21474836480 encrypt.format=luks encrypt.key-secret=sec0 cluster_size=65536 lazy_refcounts=off refcount_bits=16

2. Convert to a encrypted image, successfully.
# qemu-img convert -p --object secret,id=sec0,data=backing --object secret,id=sec2,data=convert --image-opts driver=qcow2,encrypt.key-secret=sec0,file.filename=base.qcow2 -O qcow2 -o encrypt.format=luks,encrypt.key-secret=sec2 convert.qcow2
    (100.00/100%)

3. Compare the source and target images, they are identical.
# qemu-img compare -p --object secret,id=sec0,data=backing --object secret,id=sec1,data=convert --image-opts driver=qcow2,encrypt.key-secret=sec0,file.filename=base.qcow2 driver=qcow2,encrypt.key-secret=sec1,file.filename=convert.qcow2
Images are identical. 


Scenario2 (luks)
1. Create image with luks format
# qemu-img create -f luks --object secret,id=sec0,data=base -o key-secret=sec0 test.luks 10G
Formatting 'test.luks', fmt=luks size=10737418240 key-secret=sec0

2. Convert to another luks image, successfully
# qemu-img convert -p --object secret,id=sec0,data=base --object secret,id=sec1,data=convert --image-opts driver=luks,key-secret=sec0,file.filename=test.luks -O luks -o key-secret=sec1 convert.luks
    (100.00/100%)

3. Compare the two images, and they are identical.
# qemu-img compare -p --object secret,id=sec0,data=base --object secret,id=sec1,data=convert --image-opts driver=luks,key-secret=sec0,file.filename=test.luks driver=luks,key-secret=sec1,file.filename=convert.luks
Images are identical.

Comment 33 errata-xmlrpc 2018-11-01 11:09:52 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:3443


Note You need to log in before you can comment on or make changes to this bug.