RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1656255 - The lookup by uuid and usage of a secret should match the "expected" one
Summary: The lookup by uuid and usage of a secret should match the "expected" one
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Enterprise Linux 8
Classification: Red Hat
Component: libvirt
Version: ---
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: pre-dev-freeze
: ---
Assignee: John Ferlan
QA Contact: Meina Li
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-12-05 03:58 UTC by Meina Li
Modified: 2019-06-14 02:02 UTC (History)
10 users (show)

Fixed In Version: libvirt-4.5.0-16.el8
Doc Type: No Doc Update
Doc Text:
undefined
Clone Of:
Environment:
Last Closed: 2019-06-14 02:02:47 UTC
Type: Bug
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Meina Li 2018-12-05 03:58:39 UTC
Description of problem:
The UUID for an iscsi secret can be used to volume secret, which will affect the volume creation from an encrypted volume after pool refresh. 

Version-Release number of selected component (if applicable):
libvirt-4.5.0-14.module+el8+2210+474b8474.x86_64
qemu-kvm-2.12.0-41.el8+2104+3e32e6f8.x86_64

How reproducible:
100%

Steps to Reproduce:

1. Prepare a iscsi secret env and volume xml.
# virsh secret-list
 UUID                                  Usage
--------------------------------------------------------------------------------
 9d13f200-bd02-4470-9199-192145b558d8  iscsi libvirtiscsi

# cat test.xml 
<volume type='file'>
  <name>test.qcow2</name>
  <key>/var/lib/libvirt/images/test.qcow2</key>
  <source>
  </source>
  <target>
    <path>/var/lib/libvirt/images/test.qcow2</path>
    <format type='qcow2'/>
  </target>
</volume>

2. Create an encrypt luks volume.

# virsh vol-create default volume1-enc.xml 
Vol encrypt1.img created from volume1-enc.xml

# virsh vol-dumpxml encrypt1.img default
<volume type='file'>
  <name>encrypt1.img</name>
  <key>/var/lib/libvirt/images/encrypt1.img</key>
  <source>
  </source>
 ...
  <target>
    <path>/var/lib/libvirt/images/encrypt1.img</path>
    <format type='raw'/>
...
<encryption format='luks'>
      <secret type='passphrase' uuid='9d13f200-bd02-4470-9199-192145b558d8'/>
    </encryption>
  </target>
</volume>

3. At this time, refresh the pool and check the volume dumpxml.
# virsh pool-refresh default 
Pool default refreshed
# virsh vol-dumpxml encrypt1.img default
<volume type='file'>
  <name>encrypt1.img</name>
  <key>/var/lib/libvirt/images/encrypt1.img</key>
  <source>
  </source>
 ...
  <target>
    <path>/var/lib/libvirt/images/encrypt1.img</path>
    <format type='raw'/>
...
<encryption format='luks'>
    </encryption>
  </target>
</volume>

4. Create test.qcow2  from encrypt1.img.
# virsh vol-create-from default test.xml encrypt1.img --inputpool default
error: Failed to create vol from test.xml
error: invalid argument: A single <secret type='passphrase'...> element is expected in encryption description

Actual results:
We can use the uuid of iscsi secret to create volume and it will get error when vol-create-from this volume after pool-refresh

Expected results:
The uuid and usage of a secret that should match the "expected" one

Additional info:

1) https://bugzilla.redhat.com/show_bug.cgi?id=1645459#c9

2) John has posted a patch upstream to deal with that issue, see: 
https://www.redhat.com/archives/libvir-list/2018-December/msg00085.html

Comment 1 John Ferlan 2018-12-06 13:18:21 UTC
There is a patch posted upstream that will resolve this:

https://www.redhat.com/archives/libvir-list/2018-December/msg00085.html

Setting ITR to 8.0.0.0

Comment 2 Michal Privoznik 2018-12-13 13:06:57 UTC
(In reply to Meina Li from comment #0)
> Description of problem:
> The UUID for an iscsi secret can be used to volume secret, which will affect
> the volume creation from an encrypted volume after pool refresh. 
> 
> Version-Release number of selected component (if applicable):
> libvirt-4.5.0-14.module+el8+2210+474b8474.x86_64
> qemu-kvm-2.12.0-41.el8+2104+3e32e6f8.x86_64
> 
> How reproducible:
> 100%
> 
> Steps to Reproduce:
> 
> 1. Prepare a iscsi secret env and volume xml.
> # virsh secret-list
>  UUID                                  Usage
> -----------------------------------------------------------------------------
> ---
>  9d13f200-bd02-4470-9199-192145b558d8  iscsi libvirtiscsi
> 
> # cat test.xml 
> <volume type='file'>
>   <name>test.qcow2</name>
>   <key>/var/lib/libvirt/images/test.qcow2</key>
>   <source>
>   </source>
>   <target>
>     <path>/var/lib/libvirt/images/test.qcow2</path>
>     <format type='qcow2'/>
>   </target>
> </volume>
> 
> 2. Create an encrypt luks volume.
> 
> # virsh vol-create default volume1-enc.xml 
> Vol encrypt1.img created from volume1-enc.xml
> 
> # virsh vol-dumpxml encrypt1.img default
> <volume type='file'>
>   <name>encrypt1.img</name>
>   <key>/var/lib/libvirt/images/encrypt1.img</key>
>   <source>
>   </source>
>  ...
>   <target>
>     <path>/var/lib/libvirt/images/encrypt1.img</path>
>     <format type='raw'/>
> ...
> <encryption format='luks'>
>       <secret type='passphrase' uuid='9d13f200-bd02-4470-9199-192145b558d8'/>
>     </encryption>
>   </target>
> </volume>
> 
> 3. At this time, refresh the pool and check the volume dumpxml.
> # virsh pool-refresh default 
> Pool default refreshed
> # virsh vol-dumpxml encrypt1.img default
> <volume type='file'>
>   <name>encrypt1.img</name>
>   <key>/var/lib/libvirt/images/encrypt1.img</key>
>   <source>
>   </source>
>  ...
>   <target>
>     <path>/var/lib/libvirt/images/encrypt1.img</path>
>     <format type='raw'/>
> ...
> <encryption format='luks'>
>     </encryption>

Why is the secret not reported here? Can it be that because we've lost it the subsequent vol-create-from fails (as it finds two or more secrets and therefore is rightfully confused)? In fact, if I don't refresh the pool, the secret is not lost and vol-create-from succeeds.

Comment 3 John Ferlan 2018-12-13 14:04:28 UTC
(In reply to Michal Privoznik from comment #2)
> (In reply to Meina Li from comment #0)
> > Description of problem:
> > The UUID for an iscsi secret can be used to volume secret, which will affect
> > the volume creation from an encrypted volume after pool refresh. 
> > 
> > Version-Release number of selected component (if applicable):
> > libvirt-4.5.0-14.module+el8+2210+474b8474.x86_64
> > qemu-kvm-2.12.0-41.el8+2104+3e32e6f8.x86_64
> > 
> > How reproducible:
> > 100%
> > 
> > Steps to Reproduce:
> > 
> > 1. Prepare a iscsi secret env and volume xml.
> > # virsh secret-list
> >  UUID                                  Usage
> > -----------------------------------------------------------------------------
> > ---
> >  9d13f200-bd02-4470-9199-192145b558d8  iscsi libvirtiscsi
> > 
> > # cat test.xml 
> > <volume type='file'>
> >   <name>test.qcow2</name>
> >   <key>/var/lib/libvirt/images/test.qcow2</key>
> >   <source>
> >   </source>
> >   <target>
> >     <path>/var/lib/libvirt/images/test.qcow2</path>
> >     <format type='qcow2'/>
> >   </target>
> > </volume>
> > 
> > 2. Create an encrypt luks volume.
> > 
> > # virsh vol-create default volume1-enc.xml 
> > Vol encrypt1.img created from volume1-enc.xml
> > 
> > # virsh vol-dumpxml encrypt1.img default
> > <volume type='file'>
> >   <name>encrypt1.img</name>
> >   <key>/var/lib/libvirt/images/encrypt1.img</key>
> >   <source>
> >   </source>
> >  ...
> >   <target>
> >     <path>/var/lib/libvirt/images/encrypt1.img</path>
> >     <format type='raw'/>
> > ...
> > <encryption format='luks'>
> >       <secret type='passphrase' uuid='9d13f200-bd02-4470-9199-192145b558d8'/>
> >     </encryption>
> >   </target>
> > </volume>
> > 
> > 3. At this time, refresh the pool and check the volume dumpxml.
> > # virsh pool-refresh default 
> > Pool default refreshed
> > # virsh vol-dumpxml encrypt1.img default
> > <volume type='file'>
> >   <name>encrypt1.img</name>
> >   <key>/var/lib/libvirt/images/encrypt1.img</key>
> >   <source>
> >   </source>
> >  ...
> >   <target>
> >     <path>/var/lib/libvirt/images/encrypt1.img</path>
> >     <format type='raw'/>
> > ...
> > <encryption format='luks'>
> >     </encryption>
> 
> Why is the secret not reported here? Can it be that because we've lost it
> the subsequent vol-create-from fails (as it finds two or more secrets and
> therefore is rightfully confused)? In fact, if I don't refresh the pool, the
> secret is not lost and vol-create-from succeeds.

Maybe it'd help to read all from where this bug was sourced, see bz 1645459.

Still the short answer is the secret is not reported there because during storageBackendLoadDefaultSecrets we expected to find a 'volume' secretType for a LUKS encrypted volume, but did not find one by the "path" to the volume.

The 'vol-create-from' had incorrectly allowed using an 'iscsi' secret to create the LUKS volume because the lookup by UUID was successful and we never checked on creation that the usageType for the found secret was 'volume'. We assumed that someone using a UUID for lookup would pass a 'volume' secret.

The proposed patch says, let's make sure the usageType matches the expected usageType. For a secretLookupByUsage, it would be correct - there's no way it couldn't be. For secretLookupByUUID, different story.

NB: Secrets are stored by "UUID" and "usageID", where "usageID" is like a name for a domain. Unfortunately the "usageID" and the "usageType" are different things that are similarly named. The usageID is a char* name and the usageType is an enum type. The processing in the storage pool reload code uses virSecretLookupByUsage using the path to the volume because when creating a volume secret you must provide the path to the volume that would require the secret.

Perhaps an example will help. Here's my 'virsh secret-list' output:

 UUID                                   Usage                                         
--------------------------------------------------------------------------------------
 1a2e881d-2e88-411c-b817-fd02bfa30bd5   volume /home/vm-images/encrypt1.img           
 20297a81-1317-4822-8b16-8d6f4a6c4aca   volume /home/vm-images/demo-twofish.luks      
 2a682ffe-2c95-43c3-8cdb-e690d407371e   iscsi notPrivateSecret                        
 520a2c46-d5cc-4676-ad98-a8191e2d0e87   volume /home/vm-images/volfromluks.img        
 565f077a-ec0e-4d2d-9b4b-f230f5028cbd   volume /home/vm-storage/test/luks.img         
 584aff01-f718-4c86-b593-2ee3f111e0d4   iscsi notEphemeralSecret                      
 5e94e1d5-a8da-4a77-bc72-0b19614db485   volume /home/vm-images/luks.img               
 60cebcc1-20f8-493a-b236-61f7f1211435   volume /dev/bz1427049/vol1                    
 6fd3f62d-9fe7-4a4d-a869-7acd6376d8ea   volume /home/vm-images/demo.luks              
 9e2f99fe-31a3-4d5f-8870-03eae4a684ae   volume /dev/sde1                              
 ad2cc57a-1a34-48be-8844-132c35c82f78   volume /home/vm-images/encrypt2.img           
 e05512e9-4894-4ec2-834e-698e6b407ffe   iscsi libvirtiscsi                            
 e7a85a05-d90d-492c-95cc-946f4f0d26b6   volume /home/vm-images/f18-cvt-qcow2luks.img  
 ee8950b7-aadf-4c26-9c9f-133497af4bab   volume /dev/LVM_Test/new_vol                  
 f52a81b2-424e-490c-823d-6bd4235bc578   volume /home/vm-images/f18-luks.img           

So if the input query from/for a 'create' operation was:

   <encryption format='luks'>
      <secret type='passphrase' uuid='2a682ffe-2c95-43c3-8cdb-e690d407371e'/>
    </encryption>

The "existing" code would return the notPrivateSecret which is a secret for an 'iscsi' using usageType 'iscsi' w/ no path to the volume. It's a secret meant for the iSCSI server.

With the existing code, we'd be fat, dumb, and happy to do that because our "lookup" was done by UUID and we found something even though we shouldn't be using it for the creation of a volume for LUKS because LUKS requires usage of a 'volume' secret (for example, see UUID 1a2e881d-2e88-411c-b817-fd02bfa30bd for usageType 'volume' and usageID '/home/vm-images/encrypt1.img').

Comment 4 John Ferlan 2018-12-13 14:25:53 UTC
Oh and something I noted in the upstream review reply that is probably important to keep here for posterity as well:

Creating a volume expects a usageType volume when calling virSecretGetSecretString to get the secret (see storageBackendCreateQemuImgSecretPath).

The call is:

    if (virSecretGetSecretString(conn, &enc->secrets[0]->seclookupdef,
                                 VIR_SECRET_USAGE_TYPE_VOLUME,
                                 &secret, &secretlen) < 0)

where the lookupdef is sourced from how the secret appears in the volume creation XML, in this case "uuid='2a682ffe-2c95-43c3-8cdb-e690d407371e'" was supplied... The "proper" thing would have been to use "uuid='1a2e881d-2e88-411c-b817-fd02bfa30bd5' for usageType 'volume' for usageID '/home/vm-images/encrypt1.img' in order to create the LUKS volume for /home/vm-images/encrypt1.img.

Comment 5 John Ferlan 2018-12-13 15:25:29 UTC
A patch has been pushed upstream:

commit e0eb8a8a696ee334fa33281b880e480e76348052 
Author: John Ferlan <jferlan>
Date:   Tue Dec 4 15:15:22 2018 -0500

    secret: Add check/validation for correct usage when LookupByUUID
    
    ...
    
    If virSecretGetSecretString is using by secretLookupByUUID,
    then it's possible the found sec->usageType doesn't match the
    desired @secretUsageType. If this occurs for the encrypted
    volume creation processing and a subsequent pool refresh is
    executed, then the secret used to create the volume will not
    be found by the storageBackendLoadDefaultSecrets which expects
    to find secrets by VIR_SECRET_USAGE_TYPE_VOLUME.
    
    Add a check to virSecretGetSecretString to avoid the possibility
    along with an error indicating the incorrect matched types.
    
    Signed-off-by: John Ferlan <jferlan>
    ACKed-by: Michal Privoznik <mprivozn>

$ git describe e0eb8a8a696ee334fa33281b880e480e76348052
v4.10.0-65-ge0eb8a8a69
$

Comment 8 Meina Li 2018-12-28 09:09:39 UTC
Verified env:
libvirt-4.5.0-16.module+el8+2586+bf759444.x86_64
qemu-kvm-2.12.0-51.module+el8+2608+a17c4bfe.x86_64

Verified Steps:
1. Prepare different type of secret.
# virsh secret-list 
 UUID                                  Usage
--------------------------------------------------------------------------------
 2586c3da-1f2b-4579-aaa8-ff5736d2fbac  ceph ceph_example
 5d0e41de-2bda-4bf4-975f-a60c5dde72c8  tls TLS_example
 8d13f200-bd02-4470-9199-192145b558d8  volume /var/lib/libvirt/images/encrypt1.img
 8d13f200-bd02-4470-9199-192145b669a9  iscsi libvirtiscsi

2. Prepare an volume xml.
# cat volume.xml
<volume type='file'>
  <name>test.qcow2</name>
  <key>/var/lib/libvirt/images/test.qcow2</key>
  <source>
  </source>
  <capacity unit='bytes'>10737418240</capacity>
  <target>
    <path>/var/lib/libvirt/images/test.qcow2</path>
    <encryption format='luks'>
      <secret type='passphrase' uuid='***8d13f200-bd02-4470-9199-192145b558d8***'/>
    </encryption>
  </target>
</volume>

3. Create volume with different type of uuid.
1) "volume"
# virsh vol-create default volume.xml
Vol test.qcow2 created from volume.xml

2) "iscsi"
# virsh vol-create default volume.xml
error: Failed to create vol from volume.xml
error: invalid argument: secret with uuid 8d13f200-bd02-4470-9199-192145b669a9 is of type 'iscsi' not expected 'volume' type

3) "tls"
# virsh vol-create default volume.xml
error: Failed to create vol from volume.xml
error: invalid argument: secret with uuid 5d0e41de-2bda-4bf4-975f-a60c5dde72c8 is of type 'tls' not expected 'volume' type

4) "ceph"
# virsh vol-create default volume.xml
error: Failed to create vol from volume.xml
error: invalid argument: secret with uuid 2586c3da-1f2b-4579-aaa8-ff5736d2fbac is of type 'ceph' not expected 'volume' type

So move this bug to be verified.


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