Bug 1613737 - Allow the inputvol to be encrypted when creating a volume from another volume
Summary: Allow the inputvol to be encrypted when creating a volume from another volume
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: libvirt
Version: 7.6
Hardware: x86_64
OS: Linux
unspecified
low
Target Milestone: rc
: 7.7
Assignee: John Ferlan
QA Contact: gaojianan
URL:
Whiteboard:
Depends On:
Blocks: 1598750 1638831 1645459
TreeView+ depends on / blocked
 
Reported: 2018-08-08 09:07 UTC by Meina Li
Modified: 2019-08-06 13:14 UTC (History)
11 users (show)

Fixed In Version: libvirt-4.5.0-12.el7
Doc Type: No Doc Update
Doc Text:
undefined
Clone Of:
: 1645459 (view as bug list)
Environment:
Last Closed: 2019-08-06 13:13:56 UTC
Target Upstream Version:


Attachments (Terms of Use)
code coverage (6.69 KB, text/html)
2019-04-23 09:31 UTC, gaojianan
no flags Details


Links
System ID Priority Status Summary Last Updated
IBM Linux Technology Center 173964 None None None 2019-07-26 06:01:06 UTC
Red Hat Bugzilla 1638831 None None None 2019-11-26 15:07:52 UTC
Red Hat Product Errata RHSA-2019:2294 None None None 2019-08-06 13:14:34 UTC

Internal Links: 1638831

Description Meina Li 2018-08-08 09:07:07 UTC
Description of problem:
Improve the error message when create an encrypted volume from another encrypted volume

Version-Release number of selected component (if applicable):
libvirt-4.5.0-6.el7.x86_64
qemu-kvm-rhev-2.12.0-9.el7.x86_64

How reproducible:
100%

Steps to Reproduce:
1. Prepare an encrypted volume encrypt1.img.
# virsh secret-list 
 UUID                                  Usage
--------------------------------------------------------------------------------
 33406385-1633-425a-a7e8-a02243603283  volume /var/lib/libvirt/images/encrypt1.img

# virsh vol-dumpxml encrypt1.img default
<volume type='file'>
  <name>encrypt1.img</name>
  <key>/var/lib/libvirt/images/encrypt1.img</key>
  <source>
  </source>
  <capacity unit='bytes'>209715200</capacity>
  <allocation unit='bytes'>262144</allocation>
  <physical unit='bytes'>211783680</physical>
  <target>
    <path>/var/lib/libvirt/images/encrypt1.img</path>
    <format type='raw'/>
    <permissions>
      <mode>0600</mode>
      <owner>0</owner>
      <group>0</group>
      <label>system_u:object_r:virt_image_t:s0</label>
    </permissions>
    <timestamps>
      <atime>1533714256.946794255</atime>
      <mtime>1533260873.922614416</mtime>
      <ctime>1533280739.802938868</ctime>
    </timestamps>
    <encryption format='luks'>
      <secret type='passphrase' uuid='33406385-1633-425a-a7e8-a02243603283'/>
    </encryption>
  </target>
</volume>

2. Prepare another xml of volume with encryption.
# cat encrypt2.xml 
<volume type='file'>
  <name>encrypt2.img</name>
  <key>/var/lib/libvirt/images/encrypt2.img</key>
  <source>
  </source>
  <capacity unit='bytes'>209715200</capacity>
  <allocation unit='bytes'>262144</allocation>
  <physical unit='bytes'>211783680</physical>
  <target>
    <path>/var/lib/libvirt/images/encrypt2.img</path>
    <format type='raw'/>
    <permissions>
      <mode>0600</mode>
      <owner>0</owner>
      <group>0</group>
      <label>system_u:object_r:virt_image_t:s0</label>
    </permissions>
    <timestamps>
      <atime>1533261677.171485324</atime>
      <mtime>1533260873.922614416</mtime>
      <ctime>1533260873.922614416</ctime>
    </timestamps>
    <encryption format='luks'>
      <secret type='passphrase' uuid='33406385-1633-425a-a7e8-a02243603283'/>
    </encryption>
  </target>
</volume>

3.Create an encrypted volume encrypt2.img from encrypt1.img.
# virsh vol-create-from default encrypt2.xml encrypt1.img --inputpool default
error: Failed to create vol from luks-vol.xml
error: internal error: Child process (/usr/bin/qemu-img convert --image-opts -n --target-image-opts --object secret,id=encrypt2.img_encrypt0,file=/var/run/libvirt/storage/default.encrypt2.img.secret.G2v6A6 driver=raw,file.filename=/var/lib/libvirt/images/encrypt1.img driver=luks,file.filename=/var/lib/libvirt/images/encrypt2.img,key-secret=encrypt2.img_encrypt0) unexpected exit status 1: 2018-08-08 08:39:28.928+0000: 6580: debug : virFileClose:111 : Closed fd 25
2018-08-08 08:39:28.928+0000: 6580: debug : virFileClose:111 : Closed fd 27
2018-08-08 08:39:28.928+0000: 6580: debug : virFileClose:111 : Closed fd 22
qemu-img: output file is smaller than input file


Actual results:
As above step3.

Expected results:
Has an clear error message, for example:unsupported configuration: storage pool does not support building encrypted volumes from other encrypted volumes.

Additional info:
https://bugzilla.redhat.com/show_bug.cgi?id=1526382#c13 
There's no "mechanism" available to create an encrypted volume from another encrypted volume and a lot of that has to do with the way the qemu-img command works.

Comment 2 Meina Li 2018-08-08 10:58:45 UTC
Same phenomenon with vol-clone:
# virsh vol-clone --pool default encrypt1.img clone.img
error: Failed to clone vol from encrypt1.img
error: internal error: Child process (/usr/bin/qemu-img convert --image-opts -n --target-image-opts --object secret,id=clone.img_encrypt0,file=/var/run/libvirt/storage/default.clone.img.secret.1RNp7k driver=raw,file.filename=/var/lib/libvirt/images/encrypt1.img driver=luks,file.filename=/var/lib/libvirt/images/clone.img,key-secret=clone.img_encrypt0) unexpected exit status 1: 2018-08-08 10:53:59.547+0000: 14386: debug : virFileClose:111 : Closed fd 25
2018-08-08 10:53:59.547+0000: 14386: debug : virFileClose:111 : Closed fd 27
2018-08-08 10:53:59.547+0000: 14386: debug : virFileClose:111 : Closed fd 22
qemu-img: output file is smaller than input file

Comment 3 John Ferlan 2018-08-17 21:38:06 UTC
As it turns out creating a output volume using a specific secret is not possible. But if you used the same secret value for the output volume as is used for the input volume, then as long a you don't provide the secret for that volume in the output target XML, then things would work.

Let's try to create an output volume from some encrypted input volume. The trickier part is then assigning a secret to that output volume.

# virsh vol-dumpxml demo.luks default
<volume type='file'>
  <name>demo.luks</name>
  <key>/home/vm-images/demo.luks</key>
...
    <encryption format='luks'>
      <secret type='passphrase' uuid='6fd3f62d-9fe7-4a4d-a869-7acd6376d8ea'/>
    </encryption>
  </target>
</volume>

# virsh vol-info demo.luks default
Name:           demo.luks
Type:           file
Capacity:       100.00 MiB
Allocation:     256.00 KiB

# qemu-img info /home/vm-images/demo.luks
image: /home/vm-images/demo.luks
file format: luks
virtual size: 100M (104857600 bytes)
disk size: 256K
encrypted: yes
Format specific information:
    ivgen alg: plain64
    hash alg: sha256
    cipher alg: aes-256
    uuid: 46d202f8-da48-47b3-8e8d-3e210cbf1906
    cipher mode: xts
...

# cat volfromluks.xml
<volume type='file'>
  <name>volfromluks.img</name>
  <source>
  </source>
  <target>
    <format type='raw'/>
  </target>
</volume>

# virsh vol-create-from default volfromluks.xml demo.luks --inputpool default
Vol volfromluks.img created from input vol demo.luks

# virsh vol-dumpxml volfromluks.img default
<volume type='file'>
  <name>volfromluks.img</name>
  <key>/home/vm-images/volfromluks.img</key>
  <source>
...
 <target>
    <path>/home/vm-images/volfromluks.img</path>
    <format type='raw'/>
...
  </target>
</volume>

# virsh vol-info volfromluks.img default
Name:           volfromluks.img
Type:           file
Capacity:       101.97 MiB
Allocation:     256.00 KiB

# qemu-img info /home/vm-images/volfromluks.img
image: /home/vm-images/volfromluks.img
file format: luks
virtual size: 100M (104857600 bytes)
disk size: 256K
encrypted: yes
Format specific information:
    ivgen alg: plain64
    hash alg: sha256
    cipher alg: aes-256
    uuid: 46d202f8-da48-47b3-8e8d-3e210cbf1906
    cipher mode: xts
...

Using that volume would require creation of a libvirt volume secret to mimic the secret from the input volume, but using using the new path. Automagic creation of that secret is one of those tricky things because if the secret is private (which is probably the case for a luks volume), then fetching the secret from the input volume borders on the "don't do that". If of course you know the secret, then it's a simple follow the process exercise.

Because I was "curious"....

# virsh pool-refresh default
Pool default refreshed

# virsh vol-dumpxml volfromluks.img default
<volume type='file'>
  <name>volfromluks.img</name>
  <key>/home/vm-images/volfromluks.img</key>
...
    <encryption format='luks'>
    </encryption>
  </target>
</volume>


hmmmm...  So if the secret was already present it would have been found, but still would require the pool refresh in order to 'see' it in the output XML.

Let's look at the secrets I have for demo.luks

# virsh secret-list
 UUID                                  Usage
--------------------------------------------------------------------------------
...
 6fd3f62d-9fe7-4a4d-a869-7acd6376d8ea  volume /home/vm-images/demo.luks
...

# virsh secret-dumpxml 6fd3f62d-9fe7-4a4d-a869-7acd6376d8ea
<secret ephemeral='no' private='no'>
  <uuid>6fd3f62d-9fe7-4a4d-a869-7acd6376d8ea</uuid>
  <description>demo.luks</description>
  <usage type='volume'>
    <volume>/home/vm-images/demo.luks</volume>
  </usage>
</secret>

...
heh, heh... got lucky, it's not private ;-)

So, let's create a secret for /home/vm-images/volfromluks.img

# cat new-secret.xml
<secret ephemeral='no' private='no'>
  <description>volfromluks.img</description>
  <usage type='volume'>
    <volume>/home/vm-images/volfromluks.img</volume>
  </usage>
</secret>

# virsh secret-define new-secret.xml
Secret 520a2c46-d5cc-4676-ad98-a8191e2d0e87 created

# virsh secret-get-value 6fd3f62d-9fe7-4a4d-a869-7acd6376d8ea
bGV0bWVpbg==

# virsh secret-set-value 520a2c46-d5cc-4676-ad98-a8191e2d0e87 bGV0bWVpbg==
Secret value set

and here we have it in the output:

# virsh vol-dumpxml volfromluks.img default
<volume type='file'>
  <name>volfromluks.img</name>
  <key>/home/vm-images/volfromluks.img</key>
...
    <encryption format='luks'>
      <secret type='passphrase' uuid='520a2c46-d5cc-4676-ad98-a8191e2d0e87'/>
    </encryption>
  </target>
</volume>
#

The key is the pool-refresh in order to get the secret listed in the volume. Perhaps possible from code, but a different problem for a different day.

I will work on posting something that would at least disallow providing a secret for the output XML *and* the input volume.

Comment 4 John Ferlan 2018-08-20 21:32:57 UTC
Well looks like I wasn't totally correct in my initial assertion!  As I was digging through this with a fresh look on a Monday morning, I noted that the error message from qemu-img was:

"qemu-img: output file is smaller than input file"

So that got me to thinking I had to create a large enough output file if the input file was encrypted because the LUKS encryption header takes some space. Still qemu-img should know/handle that.

So, then I started thinking about the arguments to the qemu-img convert command:

driver=raw,file.filename=/var/lib/libvirt/images/encrypt1.img driver=luks,file.filename=/var/lib/libvirt/images/clone.img,key-secret=clone.img_encrypt0

hmm... if we do this kind of copy, then it's just a pure raw copy of encrypt1.img into encrypt2.img and that isn't quite right.

So, going back to the raw qemu-img commands let's create a couple of raw encrypted images using different secrets, e.g.:

# qemu-img create -f luks --object secret,id=sec1,format=raw,data=pass1 -o key-secret=sec1 enc1.img 100K
Formatting 'enc1.img', fmt=luks size=102400 key-secret=sec1

# qemu-img create -f luks --object secret,id=sec2,format=raw,data=pass2 -o key-secret=sec2 enc2.img 100K
Formatting 'enc2.img', fmt=luks size=102400 key-secret=sec1

I figured/found that if I provided the secret object for the input vol as well as the output vol, then things would work:

# qemu-img convert --image-opts -n --target-image-opts --object secret,id=sec1,format=raw,data=pass1 --object secret,id=sec2,format=raw,data=pass2 driver=luks,file.filename=enc1.img,key-secret=sec1 driver=luks,file.filename=enc2.img,key-secret=sec2

# qemu-img dd --object secret,id=sec1,format=raw,data=pass1 --image-opts if=driver=luks,file.filename=enc1.img,key-secret=sec1 of=x.x
# qemu-img dd --object secret,id=sec2,format=raw,data=pass2 --image-opts if=driver=luks,file.filename=enc2.img,key-secret=sec2 of=y.y

# diff x.x y.y
# echo $?
0

Similarly, if someone wanted to create a raw output volume from an encrypted input volue that should work too:

# qemu-img create -f raw enc1-raw.img 100K
# qemu-img create -f raw enc2-raw.img 100K
# qemu-img convert --image-opts -n --target-image-opts --object secret,id=sec1,format=raw,data=pass1 driver=luks,file.filename=enc1.img,key-secret=sec1 driver=raw,file.filename=enc1-raw.img
# qemu-img convert --image-opts -n --target-image-opts --object secret,id=sec2,format=raw,data=pass2 driver=luks,file.filename=enc2.img,key-secret=sec2 driver=raw,file.filename=enc2-raw.img
# qemu-img dd if=enc1-raw.img of=c.c
# qemu-img dd if=enc2-raw.img of=d.d
# diff c.c d.d
# echo $?
0
#

and converting from raw to encrypted and back to raw:

# qemu-img create -f luks --object secret,id=sec1,format=raw,data=pass1 -o key-secret=sec1 encrypt1.img 100K
# qemu-img create -f raw encrypt1-raw.img 100K
# qemu-img convert --image-opts -n --target-image-opts --object secret,id=sec1,format=raw,data=pass1 driver=raw,file.filename=encrypt1-raw.img driver=luks,file.filename=encrypt1.img,key-secret=sec1
# qemu-img create -f raw encrypt1-raw-out.img 100K
Formatting 'encrypt1-raw-out.img', fmt=raw size=102400
# qemu-img convert --image-opts -n --target-image-opts --object secret,id=sec1,format=raw,data=pass1 driver=luks,file.filename=encrypt1.img,key-secret=sec1 driver=raw,file.filename=encrypt1-raw-out.img
# diff encrypt1-raw-out.img encrypt1-raw.img
# echo $?
0
#

So, I will carry this new knowledge into the libvirt code and implement.

Comment 5 John Ferlan 2018-08-21 16:24:12 UTC
Posted some patches upstream to resolve the issue found:

https://www.redhat.com/archives/libvir-list/2018-August/msg01306.html

Series of 3 patches - first 2 are mainly "setup" to reduce changes in last one.

NB: I also changed the title of the bz to reflect the research done.

Comment 6 John Ferlan 2018-08-30 11:46:31 UTC
Since this is not something that would block a current release, altering the flag to 7.7.

Comment 7 John Ferlan 2018-09-12 11:29:49 UTC

commit b975afc72504e636ec538ea119bee9a0cd7b63d8
Author: John Ferlan <jferlan@redhat.com>
Date:   Mon Aug 20 12:25:44 2018 -0400

    storage: Allow inputvol to be encrypted
    
...
    
    When processing the inputvol for encryption, we need to handle
    the case where the inputvol is encrypted. This then allows for
    the encrypted inputvol to be used either for an output encrypted
    volume or an output volume of some XML provided type.
    
    Add tests to show the various conversion options when either input
    or output is encrypted. This includes when both are encrypted.
    
$ git describe b975afc72504e636ec538ea119bee9a0cd7b63d8
v4.7.0-97-gb975afc725
$

Comment 12 gaojianan 2019-04-23 09:31:28 UTC
Verified on version:
qemu-kvm-rhev-2.12.0-25.el7.x86_64
libvirt-4.5.0-12.virtcov.el7.x86_64

Step:
1.Prepare an encrypted volume luks_1.img.
# virsh vol-dumpxml --pool luks luks_1.img 
<volume type='file'>
  <name>luks_1.img</name>
  <key>/var/lib/libvirt/images/luks/luks_1.img</key>
  <source>
  </source>
  <capacity unit='bytes'>1048576</capacity>
  <allocation unit='bytes'>69632</allocation>
  <physical unit='bytes'>1576960</physical>
  <target>
    <path>/var/lib/libvirt/images/luks/luks_1.img</path>
    <format type='raw'/>
    <permissions>
      <mode>0600</mode>
      <owner>0</owner>
      <group>0</group>
      <label>system_u:object_r:virt_var_lib_t:s0</label>
    </permissions>
    <timestamps>
      <atime>1556010549.248869270</atime>
      <mtime>1556010549.229869233</mtime>
      <ctime>1556010549.247869268</ctime>
    </timestamps>
    <encryption format='luks'>
      <secret type='passphrase' uuid='f981dd17-143f-45bc-88e6-ed1fe20ce9da'/>
    </encryption>
  </target>
</volume>

2.Prepare another xml of volume with encryption.
# cat luks2.xml
<volume>
         <name>luks_2.img</name>
         <capacity unit='M'>1</capacity>
         <target>
           <path>/var/lib/libvirt/images/luks/luks_2.img</path>
           <format type='raw'/>
          <encryption format='luks'>
              <secret type='passphrase' uuid='f981dd17-143f-45bc-88e6-ed1fe20ce9da'/>
              <cipher name='twofish' size='128' mode='cbc' hash='sha256'/>
              <ivgen name='plain64' hash='sha1'/>
           </encryption>
          </target>
 </volume>


3.Create an encrypted volume luks_2.img from luks_1.img.
# virsh vol-create-from luks luks2.xml luks_1.img --inputpool luks
Vol luks_2.img created from input vol luks_1.img



Work as expected
Additional info:
code coverage in the attachment(coverage 96%)
But for this piece of code ,i don't know how to cover it

    }

    if (inputenc && convertStep == VIR_STORAGE_VOL_ENCRYPT_CONVERT) {
        if (!inputSecretPath) {
            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                           _("path to inputvol secret data file is required"));  <<<<<<<<???
            goto error;
        }
        if (virAsprintf(&inputSecretAlias, "%s_encrypt0",

Comment 13 gaojianan 2019-04-23 09:31:56 UTC
Created attachment 1557516 [details]
code coverage

Comment 14 John Ferlan 2019-04-23 12:42:04 UTC
Short answer - there isn't a way to generate a via normal/existing API calls to trigger the failure.

The check is designed as an assumption made while developing the code in lieu of using other techniques to utilize ATTRIBUTE_NONNULL on certain arguments that are not expected to be NULL. A technique that has a drawback of only checking if NULL is passed to a method, but not if an argument is NULL. That is the ATTRIBUTE_NONNULL can detect:

method(arg1, arg2, NULL)

but cannot detect the case where

method(arg1, arg2, arg3) where arg3 == NULL

So, one would hope that the path isn't reachable, but if someone made a change such that virStorageBackendCreateQemuImgCmdFromVol() was called with a NULL @inputSecretPath argument, then that check would cause a failure.  Similarly if arguments @create_tool or @secretPath were NULL, there'd be a failure.

I suppose someone could add a few negative tests in tests/storagevolxml2argvtest.c to pass NULL arguments and ensure the proper error is returned, but that's beyond the scope of this bz.

Comment 16 errata-xmlrpc 2019-08-06 13:13:56 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/RHSA-2019:2294


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