Bug 1613737

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

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.

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.

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.

Since this is not something that would block a current release, altering the flag to 7.7.

commit b975afc72504e636ec538ea119bee9a0cd7b63d8
Author: John Ferlan <jferlan>
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
$

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",

Created attachment 1557516 [details]
code coverage

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.

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