Bug 1656255
Summary: | The lookup by uuid and usage of a secret should match the "expected" one | ||
---|---|---|---|
Product: | Red Hat Enterprise Linux 8 | Reporter: | Meina Li <meili> |
Component: | libvirt | Assignee: | John Ferlan <jferlan> |
Status: | CLOSED CURRENTRELEASE | QA Contact: | Meina Li <meili> |
Severity: | unspecified | Docs Contact: | |
Priority: | unspecified | ||
Version: | --- | CC: | dyuan, hhan, jdenemar, jferlan, jsuchane, lmen, mprivozn, rbalakri, xuzhang, yisun |
Target Milestone: | pre-dev-freeze | ||
Target Release: | --- | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | libvirt-4.5.0-16.el8 | Doc Type: | No Doc Update |
Doc Text: |
undefined
|
Story Points: | --- |
Clone Of: | Environment: | ||
Last Closed: | 2019-06-14 02:02:47 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Meina Li
2018-12-05 03:58:39 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 (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. (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'). 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. 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 $ 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. |