Commit id's '9bbf0d7e6' and '2552fec24' added some XML parsing tests for a LUKS volume to use a 'passphrase' secret format. After commit, this was deemed to be incorrect, so covert the various tests to use the volume usage format where the 'usage' is the path to the volume rather than a user defined name string. Also, removed the qemuxml2argv-luks-disk-cipher.xml since it was just a duplicate of qemuxml2argv-luks-disks.xml. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- .../qemuxml2argv-luks-disk-cipher.xml | 45 ---------------------- tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml | 2 +- .../qemuxml2xmlout-luks-disk-cipher.xml | 1 - tests/qemuxml2xmltest.c | 1 - tests/storagevolxml2xmlin/vol-luks-cipher.xml | 2 +- tests/storagevolxml2xmlin/vol-luks.xml | 2 +- tests/storagevolxml2xmlout/vol-luks-cipher.xml | 2 +- tests/storagevolxml2xmlout/vol-luks.xml | 2 +- 8 files changed, 5 insertions(+), 52 deletions(-) delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml delete mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml b/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml deleted file mode 100644 index 9ce15c0..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml +++ /dev/null @@ -1,45 +0,0 @@ -<domain type='qemu'> - <name>encryptdisk</name> - <uuid>496898a6-e6ff-f7c8-5dc2-3cf410945ee9</uuid> - <memory unit='KiB'>1048576</memory> - <currentMemory unit='KiB'>524288</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='x86_64' machine='pc-i440fx-2.1'>hvm</type> - <boot dev='hd'/> - </os> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu</emulator> - <disk type='file' device='disk'> - <driver name='qemu' type='luks'/> - <source file='/storage/guest_disks/encryptdisk'/> - <target dev='vda' bus='virtio'/> - <encryption format='luks'> - <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> - </encryption> - <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> - </disk> - <disk type='file' device='disk'> - <driver name='qemu' type='luks'/> - <source file='/storage/guest_disks/encryptdisk2'/> - <target dev='vdb' bus='virtio'/> - <encryption format='luks'> - <secret type='passphrase' usage='mycluster_myname'/> - </encryption> - <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> - </disk> - <controller type='usb' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> - </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <memballoon model='virtio'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> - </memballoon> - </devices> -</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml index 9ce15c0..4c9c4c7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml @@ -28,7 +28,7 @@ <source file='/storage/guest_disks/encryptdisk2'/> <target dev='vdb' bus='virtio'/> <encryption format='luks'> - <secret type='passphrase' usage='mycluster_myname'/> + <secret type='passphrase' usage='/storage/guest_disks/encryptdisk2'/> </encryption> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </disk> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml deleted file mode 120000 index fa55233..0000000 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml +++ /dev/null @@ -1 +0,0 @@ -../qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index eb810e3..d62e536 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -557,7 +557,6 @@ mymain(void) DO_TEST("encrypted-disk"); DO_TEST("encrypted-disk-usage"); DO_TEST("luks-disks"); - DO_TEST("luks-disk-cipher"); DO_TEST("memtune"); DO_TEST("memtune-unlimited"); DO_TEST("blkiotune"); diff --git a/tests/storagevolxml2xmlin/vol-luks-cipher.xml b/tests/storagevolxml2xmlin/vol-luks-cipher.xml index 009246f..da28a27 100644 --- a/tests/storagevolxml2xmlin/vol-luks-cipher.xml +++ b/tests/storagevolxml2xmlin/vol-luks-cipher.xml @@ -15,7 +15,7 @@ <label>unconfined_u:object_r:virt_image_t:s0</label> </permissions> <encryption format='luks'> - <secret type='passphrase' usage='mumblyfratz'/> + <secret type='passphrase' uuid='f52a81b2-424e-490c-823d-6bd4235bc572'/> <cipher name='serpent' size='256' mode='cbc' hash='sha256'/> <ivgen name='plain64' hash='sha256'/> </encryption> diff --git a/tests/storagevolxml2xmlin/vol-luks.xml b/tests/storagevolxml2xmlin/vol-luks.xml index eb4dc41..bf3c519 100644 --- a/tests/storagevolxml2xmlin/vol-luks.xml +++ b/tests/storagevolxml2xmlin/vol-luks.xml @@ -15,7 +15,7 @@ <label>unconfined_u:object_r:virt_image_t:s0</label> </permissions> <encryption format='luks'> - <secret type='passphrase' usage='mumblyfratz'/> + <secret type='passphrase' uuid='f52a81b2-424e-490c-823d-6bd4235bc572'/> </encryption> </target> </volume> diff --git a/tests/storagevolxml2xmlout/vol-luks-cipher.xml b/tests/storagevolxml2xmlout/vol-luks-cipher.xml index 9014849..1ac7424 100644 --- a/tests/storagevolxml2xmlout/vol-luks-cipher.xml +++ b/tests/storagevolxml2xmlout/vol-luks-cipher.xml @@ -15,7 +15,7 @@ <label>unconfined_u:object_r:virt_image_t:s0</label> </permissions> <encryption format='luks'> - <secret type='passphrase' usage='mumblyfratz'/> + <secret type='passphrase' uuid='f52a81b2-424e-490c-823d-6bd4235bc572'/> <cipher name='serpent' size='256' mode='cbc' hash='sha256'/> <ivgen name='plain64' hash='sha256'/> </encryption> diff --git a/tests/storagevolxml2xmlout/vol-luks.xml b/tests/storagevolxml2xmlout/vol-luks.xml index 5b764b7..7b82866 100644 --- a/tests/storagevolxml2xmlout/vol-luks.xml +++ b/tests/storagevolxml2xmlout/vol-luks.xml @@ -15,7 +15,7 @@ <label>unconfined_u:object_r:virt_image_t:s0</label> </permissions> <encryption format='luks'> - <secret type='passphrase' usage='mumblyfratz'/> + <secret type='passphrase' uuid='f52a81b2-424e-490c-823d-6bd4235bc572'/> </encryption> </target> </volume> -- 2.5.5

Commit id's 'c8438010', '9bbf0d7e', and '2552fec24' altered the documentation to describe adding a 'passphrase' type secret usage model in order to reference the secret for a luks volume. After commit, it was deemed that a 'volume' usage model should be used, so adjust the various documents in order rephrase descriptions in order to follow the correct usage model. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- docs/formatsecret.html.in | 104 +++++++++++++---------------------- docs/formatstorage.html.in | 16 ++++++ docs/formatstorageencryption.html.in | 29 ++++------ 3 files changed, 66 insertions(+), 83 deletions(-) diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in index ffaa7dc..216a83c 100644 --- a/docs/formatsecret.html.in +++ b/docs/formatsecret.html.in @@ -41,19 +41,20 @@ <dd> Specifies what this secret is used for. A mandatory <code>type</code> attribute specifies the usage category, currently - only <code>volume</code>, <code>ceph</code>, <code>iscsi</code>, - and <code>passphrase</code> are defined. Specific usage categories - are described below. + only <code>volume</code>, <code>ceph</code>, and <code>iscsi</code> + are defined. Specific usage categories are described below. </dd> </dl> <h3><a name="VolumeUsageType">Usage type "volume"</a></h3> <p> - This secret is associated with a volume, and it is safe to delete the - secret after the volume is deleted. The <code>&lt;usage - type='volume'&gt;</code> element must contain a - single <code>volume</code> element that specifies the key of the volume + This secret is associated with a volume, whether the format is either + for a "qcow" or a "luks" encrypted volume. Each volume will have a + unique secret associated with it and it is safe to delete the + secret after the volume is deleted. The + <code>&lt;usage type='volume'&gt;</code> element must contain a + single <code>volume</code> element that specifies the path of the volume this secret is associated with. For example, create a volume-secret.xml file as follows: </p> @@ -69,7 +70,7 @@ </pre> <p> - Define the secret and set the pass phrase as follows: + Define the secret and set the passphrase as follows: </p> <pre> # virsh secret-define volume-secret.xml @@ -82,8 +83,8 @@ </pre> <p> - The volume type secret can then be used in the XML for a storage volume - <a href="formatstorageencryption.html">encryption</a> as follows: + The volume type secret can be supplied in domain XML for a qcow storage + volume <a href="formatstorageencryption.html">encryption</a> as follows: </p> <pre> &lt;encryption format='qcow'&gt; @@ -91,6 +92,33 @@ &lt;/encryption&gt; </pre> + <p> + The volume type secret can be supplied either in volume XML during + creation of a <a href="formatstorage.html#StorageVol">storage volume</a> + in order to provide the passphrase to encrypt the volume or in + domain XML <a href="formatdomain.html#elementsDisks">disk device</a> + in order to provide the passphrase to decrypt the volume, + <span class="since">since 2.1.0</span>. An example follows: + </p> + <pre> + # cat luks-secret.xml + &lt;secret ephemeral='no' private='yes'&gt; + &lt;description&gt;LUKS Sample Secret&lt;/description&gt; + &lt;uuid&gt;f52a81b2-424e-490c-823d-6bd4235bc57&lt;/uuid&gt; + &lt;usage type='volume'&gt; + &lt;volume&gt;/var/lib/libvirt/images/luks-sample.img&lt;/volume&gt; + &lt;/usage&gt; + &lt;/secret&gt; + + # virsh secret-define luks-secret.xml + Secret f52a81b2-424e-490c-823d-6bd4235bc57 created + # + # MYSECRET=`printf %s "letmein" | base64` + # virsh secret-set-value f52a81b2-424e-490c-823d-6bd4235bc57 $MYSECRET + Secret value set + # + </pre> + <h3><a name="CephUsageType">Usage type "ceph"</a></h3> <p> This secret is associated with a Ceph RBD (rados block device). @@ -243,61 +271,5 @@ &lt;/auth&gt; </pre> - <h3><a name="passphraseUsageType">Usage type "passphrase"</a></h3> - - <p> - This secret is a general purpose secret to be used by various libvirt - objects to provide a single passphrase as required by the object in - order to perform its authentication. For example, this secret will - be used either by the - <a href="formatstorage.html#StorageVol">storage volume</a> in order to - provide the passphrase to encrypt a luks volume or by the - <a href="formatdomain.html#elementsDisks">disk device</a> in order to - provide the passphrase to decrypt the luks volume for usage. - <span class="since">Since 2.1.0</span>. The following is an example - of a secret.xml file: - </p> - - <pre> - # cat secret.xml - &lt;secret ephemeral='no' private='yes'&gt; - &lt;description&gt;sample passphrase secret&lt;/description&gt; - &lt;usage type='passphrase'&gt; - &lt;name&gt;name_example&lt;/name&gt; - &lt;/usage&gt; - &lt;/secret&gt; - - # virsh secret-define secret.xml - Secret 718c71bd-67b5-4a2b-87ec-a24e8ca200dc created - - # virsh secret-list - UUID Usage - ----------------------------------------------------------- - 718c71bd-67b5-4a2b-87ec-a24e8ca200dc passphrase name_example - # - - </pre> - - <p> - A secret may also be defined via the - <a href="html/libvirt-libvirt-secret.html#virSecretDefineXML"> - <code>virSecretDefineXML</code></a> API. - - Once the secret is defined, a secret value will need to be set. This - value would be the same used to create and use the volume. - The following is a simple example of using - <code>virsh secret-set-value</code> to set the secret value. The - <a href="html/libvirt-libvirt-secret.html#virSecretSetValue"> - <code>virSecretSetValue</code></a> API may also be used to set - a more secure secret without using printable/readable characters. - </p> - - <pre> - # MYSECRET=`printf %s "letmein" | base64` - # virsh secret-set-value 718c71bd-67b5-4a2b-87ec-a24e8ca200dc $MYSECRET - Secret value set - - </pre> - </body> </html> diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 94277a1..a700e85 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -752,5 +752,21 @@ &lt;/permissions&gt; &lt;/target&gt; &lt;/volume&gt;</pre> + + <h3><a name="exampleLuks">Storage volume using LUKS</a></h3> + + <pre> + &lt;volume&gt; + &lt;name&gt;MyLuks.img&lt;/name&gt; + &lt;capacity unit="G"&gt;5&lt;/capacity&gt; + &lt;target&gt; + &lt;path&gt;/var/lib/virt/images/MyLuks.img&lt;/path&gt; + &lt;format type='luks'/&gt; + &lt;encryption format='luks'&gt; + &lt;secret type='passphrase' uuid='f52a81b2-424e-490c-823d-6bd4235bc572'/&gt; + &lt;/encryption&gt; + &lt;/target&gt; + &lt;/volume&gt; + </pre> </body> </html> diff --git a/docs/formatstorageencryption.html.in b/docs/formatstorageencryption.html.in index f2b0ffd..11af97e 100644 --- a/docs/formatstorageencryption.html.in +++ b/docs/formatstorageencryption.html.in @@ -27,9 +27,9 @@ <code>secret</code> tags, each with mandatory attributes <code>type</code> and either <code>uuid</code> or <code>usage</code> (<span class="since">since 2.1.0</span>). The only currently defined - value of <code>type</code> is <code>passphrase</code>. The + value of <code>type</code> is <code>volume</code>. The <code>uuid</code> is "uuid" of the <code>secret</code> while - <code>usage</code> is the value "usage" subelement field. + <code>usage</code> is the "usage" subelement field. A secret value can be set in libvirt by the <a href="html/libvirt-libvirt-secret.html#virSecretSetValue"> <code>virSecretSetValue</code></a> API. Alternatively, if supported @@ -40,7 +40,7 @@ <h3><a name="StorageEncryptionDefault">"default" format</a></h3> <p> <code>&lt;encryption format="default"/&gt;</code> can be specified only - when creating a volume. If the volume is successfully created, the + when creating a qcow volume. If the volume is successfully created, the encryption formats, parameters and secrets will be auto-generated by libvirt and the attached <code>encryption</code> tag will be updated. The unmodified contents of the <code>encryption</code> tag can be used @@ -59,13 +59,9 @@ <h3><a name="StorageEncryptionLuks">"luks" format</a></h3> <p> The <code>luks</code> format is specific to a luks encrypted volume - and the secret used in order to either encrypt or decrypt the volume. - A single <code>&lt;secret type='passphrase'...&gt;</code> element is - expected. The secret may be referenced via either a <code>uuid</code> or - <code>usage</code> attribute. One of the two must be present. When - present for volume creation, the secret will be used in order for - volume encryption. When present for domain usage, the secret will - be used as the passphrase to decrypt the volume. + and the secret is used in order to either encrypt during volume creation + or decrypt the volume for usage by the domain. A single + <code>&lt;secret type='passphrase'...&gt;</code> element is expected. <span class="since">Since 2.1.0</span>. </p> <p> @@ -135,22 +131,21 @@ &lt;/encryption&gt;</pre> <p> - Assuming a <a href="formatsecret.html#luksUsageType"> - <code>luks secret</code></a> is already defined using a - <code>usage</code> element with an <code>name</code> of "luks_example", + Assuming a <a href="formatsecret.html#VolumeUsageType"> + <code>luks volume type secret</code></a> is already defined, a simple example specifying use of the <code>luks</code> format for either volume creation without a specific cipher being defined or as part of a domain volume definition: </p> <pre> &lt;encryption format='luks'&gt; - &lt;secret type='passphrase' usage='luks_example'/&gt; + &lt;secret type='passphrase' uuid='f52a81b2-424e-490c-823d-6bd4235bc572'/&gt; &lt;/encryption&gt; </pre> <p> - Here is an example, specifying use of the <code>luks</code> format for - a specific cipher algorihm for volume creation: + Here is an example specifying use of the <code>luks</code> format for + a specific cipher algorithm for volume creation: </p> <pre> &lt;volume&gt; @@ -160,7 +155,7 @@ &lt;path&gt;/var/lib/libvirt/images/demo.luks&lt;/path&gt; &lt;format type='luks'/&gt; &lt;encryption format='luks'&gt; - &lt;secret type='passphrase' usage='luks_example'/&gt; + &lt;secret type='passphrase' uuid='f52a81b2-424e-490c-823d-6bd4235bc572'/&gt; &lt;cipher name='twofish' size='256' mode='cbc' hash='sha256'/&gt; &lt;ivgen name='plain64' hash='sha256'/&gt; &lt;/encryption&gt; -- 2.5.5

Commit id 'c84380106' added support for a secret usage type 'passphrase' that was designed to be used for both LUKS encryption and TLS credentials since both used a 'simple' passphrase in order to handle the authentication. However, a post commit review deemed that usage model to be invalid. This patch repurposes the 'passphrase' usage type for 'tls' specific usage. A previous patch has already adjusted the various LUKS usages to utilize a 'volume' secret. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- docs/aclpolkit.html.in | 2 +- docs/formatsecret.html.in | 59 +++++++++++++++++++++++++++++- docs/schemas/secret.rng | 6 +-- include/libvirt/libvirt-secret.h | 2 +- src/access/viraccessdriverpolkit.c | 2 +- src/conf/secret_conf.c | 12 +++--- src/conf/virsecretobj.c | 2 +- tests/secretxml2xmlin/usage-passphrase.xml | 7 ---- tests/secretxml2xmlin/usage-tls.xml | 7 ++++ tests/secretxml2xmltest.c | 2 +- 10 files changed, 78 insertions(+), 23 deletions(-) delete mode 100644 tests/secretxml2xmlin/usage-passphrase.xml create mode 100644 tests/secretxml2xmlin/usage-tls.xml diff --git a/docs/aclpolkit.html.in b/docs/aclpolkit.html.in index 4d0307d..7a1e302 100644 --- a/docs/aclpolkit.html.in +++ b/docs/aclpolkit.html.in @@ -226,7 +226,7 @@ </tr> <tr> <td>secret_usage_name</td> - <td>Name of be associated passphrase secret, if any</td> + <td>Name of the associated tls secret, if any</td> </tr> </tbody> </table> diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in index 216a83c..de1af20 100644 --- a/docs/formatsecret.html.in +++ b/docs/formatsecret.html.in @@ -41,8 +41,9 @@ <dd> Specifies what this secret is used for. A mandatory <code>type</code> attribute specifies the usage category, currently - only <code>volume</code>, <code>ceph</code>, and <code>iscsi</code> - are defined. Specific usage categories are described below. + only <code>volume</code>, <code>ceph</code>, <code>iscsi</code>, + and <code>tls</code> are defined. Specific usage categories + are described below. </dd> </dl> @@ -271,5 +272,59 @@ &lt;/auth&gt; </pre> + <h3><a name="tlsUsageType">Usage type "tls"</a></h3> + + <p> + This secret may be used in order to provide the passphrase for the + private key used to provide TLS credentials. + The <code>&lt;usage type='tls'&gt;</code> element must contain a + single <code>name</code> element that specifies a usage name + for the secret. + <span class="since">Since 2.1.0</span>. + The following is an example of the expected XML and processing to + define the secret: + </p> + + <pre> + # cat tls-secret.xml + &lt;secret ephemeral='no' private='yes'&gt; + &lt;description&gt;sample tls secret&lt;/description&gt; + &lt;usage type='tls'&gt; + &lt;name&gt;TLS_example&lt;/name&gt; + &lt;/usage&gt; + &lt;/secret&gt; + + # virsh secret-define tls-secret.xml + Secret 718c71bd-67b5-4a2b-87ec-a24e8ca200dc created + + # virsh secret-list + UUID Usage + ----------------------------------------------------------- + 718c71bd-67b5-4a2b-87ec-a24e8ca200dc tls TLS_example + # + + </pre> + + <p> + A secret may also be defined via the + <a href="html/libvirt-libvirt-secret.html#virSecretDefineXML"> + <code>virSecretDefineXML</code></a> API. + + Once the secret is defined, a secret value will need to be set. The + secret would be the passphrase used to access the TLS credentials. + The following is a simple example of using + <code>virsh secret-set-value</code> to set the secret value. The + <a href="html/libvirt-libvirt-secret.html#virSecretSetValue"> + <code>virSecretSetValue</code></a> API may also be used to set + a more secure secret without using printable/readable characters. + </p> + + <pre> + # MYSECRET=`printf %s "letmein" | base64` + # virsh secret-set-value 718c71bd-67b5-4a2b-87ec-a24e8ca200dc $MYSECRET + Secret value set + + </pre> + </body> </html> diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng index cac8560..1e94d66 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -36,7 +36,7 @@ <ref name='usagevolume'/> <ref name='usageceph'/> <ref name='usageiscsi'/> - <ref name='usagepassphrase'/> + <ref name='usagetls'/> <!-- More choices later --> </choice> </element> @@ -72,9 +72,9 @@ </element> </define> - <define name='usagepassphrase'> + <define name='usagetls'> <attribute name='type'> - <value>passphrase</value> + <value>tls</value> </attribute> <element name='name'> <ref name='genericName'/> diff --git a/include/libvirt/libvirt-secret.h b/include/libvirt/libvirt-secret.h index 55b11e0..2ae36f6 100644 --- a/include/libvirt/libvirt-secret.h +++ b/include/libvirt/libvirt-secret.h @@ -43,7 +43,7 @@ typedef enum { VIR_SECRET_USAGE_TYPE_VOLUME = 1, VIR_SECRET_USAGE_TYPE_CEPH = 2, VIR_SECRET_USAGE_TYPE_ISCSI = 3, - VIR_SECRET_USAGE_TYPE_PASSPHRASE = 4, + VIR_SECRET_USAGE_TYPE_TLS = 4, # ifdef VIR_ENUM_SENTINELS VIR_SECRET_USAGE_TYPE_LAST diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c index 99b867f..67f2a57 100644 --- a/src/access/viraccessdriverpolkit.c +++ b/src/access/viraccessdriverpolkit.c @@ -338,7 +338,7 @@ virAccessDriverPolkitCheckSecret(virAccessManagerPtr manager, virAccessPermSecretTypeToString(perm), attrs); } break; - case VIR_SECRET_USAGE_TYPE_PASSPHRASE: { + case VIR_SECRET_USAGE_TYPE_TLS: { const char *attrs[] = { "connect_driver", driverName, "secret_uuid", uuidstr, diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index a973aa9..ce1ad92 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -39,7 +39,7 @@ VIR_LOG_INIT("conf.secret_conf"); VIR_ENUM_IMPL(virSecretUsage, VIR_SECRET_USAGE_TYPE_LAST, - "none", "volume", "ceph", "iscsi", "passphrase") + "none", "volume", "ceph", "iscsi", "tls") const char * virSecretUsageIDForDef(virSecretDefPtr def) @@ -57,7 +57,7 @@ virSecretUsageIDForDef(virSecretDefPtr def) case VIR_SECRET_USAGE_TYPE_ISCSI: return def->usage.target; - case VIR_SECRET_USAGE_TYPE_PASSPHRASE: + case VIR_SECRET_USAGE_TYPE_TLS: return def->usage.name; default: @@ -89,7 +89,7 @@ virSecretDefFree(virSecretDefPtr def) VIR_FREE(def->usage.target); break; - case VIR_SECRET_USAGE_TYPE_PASSPHRASE: + case VIR_SECRET_USAGE_TYPE_TLS: VIR_FREE(def->usage.name); break; @@ -153,10 +153,10 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt, } break; - case VIR_SECRET_USAGE_TYPE_PASSPHRASE: + case VIR_SECRET_USAGE_TYPE_TLS: if (!(def->usage.name = virXPathString("string(./usage/name)", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("passphrase usage specified, but name is missing")); + _("tls usage specified, but name is missing")); return -1; } break; @@ -313,7 +313,7 @@ virSecretDefFormatUsage(virBufferPtr buf, virBufferEscapeString(buf, "<target>%s</target>\n", def->usage.target); break; - case VIR_SECRET_USAGE_TYPE_PASSPHRASE: + case VIR_SECRET_USAGE_TYPE_TLS: virBufferEscapeString(buf, "<name>%s</name>\n", def->usage.name); break; diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index 6714a00..2bdfe08 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -238,7 +238,7 @@ virSecretObjSearchName(const void *payload, found = 1; break; - case VIR_SECRET_USAGE_TYPE_PASSPHRASE: + case VIR_SECRET_USAGE_TYPE_TLS: if (STREQ(secret->def->usage.name, data->usageID)) found = 1; break; diff --git a/tests/secretxml2xmlin/usage-passphrase.xml b/tests/secretxml2xmlin/usage-passphrase.xml deleted file mode 100644 index 2b94b80..0000000 --- a/tests/secretxml2xmlin/usage-passphrase.xml +++ /dev/null @@ -1,7 +0,0 @@ -<secret ephemeral='no' private='no'> - <uuid>f52a81b2-424e-490c-823d-6bd4235bc572</uuid> - <description>Sample Passphrase Secret</description> - <usage type='passphrase'> - <name>mumblyfratz</name> - </usage> -</secret> diff --git a/tests/secretxml2xmlin/usage-tls.xml b/tests/secretxml2xmlin/usage-tls.xml new file mode 100644 index 0000000..8203681 --- /dev/null +++ b/tests/secretxml2xmlin/usage-tls.xml @@ -0,0 +1,7 @@ +<secret ephemeral='no' private='no'> + <uuid>f52a81b2-424e-490c-823d-6bd4235bc572</uuid> + <description>Sample TLS Secret</description> + <usage type='tls'> + <name>TLS-Example</name> + </usage> +</secret> diff --git a/tests/secretxml2xmltest.c b/tests/secretxml2xmltest.c index c444e4d..714c709 100644 --- a/tests/secretxml2xmltest.c +++ b/tests/secretxml2xmltest.c @@ -80,7 +80,7 @@ mymain(void) DO_TEST("usage-volume"); DO_TEST("usage-ceph"); DO_TEST("usage-iscsi"); - DO_TEST("usage-passphrase"); + DO_TEST("usage-tls"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.5.5

Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1301021 If the volume xml was looking to create a luks volume take the necessary steps in order to make that happen. The processing will be: 1. create a temporary file (virStorageBackendCreateQemuImgSecretPath) 1a. use the storage driver state dir path that uses the pool and volume name as a base. 2. create a secret object (virStorageBackendCreateQemuImgSecretObject) 2a. use an alias combinding the volume name and "_luks0" 2b. add the file to the object 3. create/add luks options to the commandline (virQEMUBuildLuksOpts) 3a. at the very least a "key-secret=%s" using the secret object alias 3b. if found in the XML the various "cipher" and "ivgen" options Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/storage/storage_backend.c | 218 ++++++++++++++++++++++++++++++++++++++--- src/storage/storage_backend.h | 3 +- src/util/virqemu.c | 23 +++++ src/util/virqemu.h | 6 ++ tests/storagevolxml2argvtest.c | 3 +- 6 files changed, 240 insertions(+), 14 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ad0af76..7066e91 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2180,6 +2180,7 @@ virProcessWait; # util/virqemu.h +virQEMUBuildLuksOpts; virQEMUBuildObjectCommandlineFromJSON; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 97f6ffe..1acee0d 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -55,11 +55,14 @@ #include "viralloc.h" #include "internal.h" #include "secret_conf.h" +#include "secret_util.h" #include "viruuid.h" #include "virstoragefile.h" #include "storage_backend.h" #include "virlog.h" #include "virfile.h" +#include "virjson.h" +#include "virqemu.h" #include "stat-time.h" #include "virstring.h" #include "virxml.h" @@ -907,6 +910,7 @@ virStorageBackendQemuImgSupportsCompat(const char *qemuimg) return ret; } + static int virStorageBackendQEMUImgBackingFormat(const char *qemuimg) { @@ -925,6 +929,10 @@ virStorageBackendQEMUImgBackingFormat(const char *qemuimg) return ret; } +/* The _virStorageBackendQemuImgInfo separates the command line building from + * the volume definition so that qemuDomainSnapshotCreateInactiveExternal can + * use it without needing to deal with a volume. + */ struct _virStorageBackendQemuImgInfo { int format; const char *path; @@ -941,21 +949,36 @@ struct _virStorageBackendQemuImgInfo { const char *inputPath; const char *inputFormatStr; int inputFormat; + + char *secretAlias; + const char *secretPath; }; + static int -virStorageBackendCreateQemuImgOpts(char **opts, +virStorageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc, + char **opts, struct _virStorageBackendQemuImgInfo info) { virBuffer buf = VIR_BUFFER_INITIALIZER; - if (info.backingPath) - virBufferAsprintf(&buf, "backing_fmt=%s,", - virStorageFileFormatTypeToString(info.backingFormat)); - if (info.encryption) - virBufferAddLit(&buf, "encryption=on,"); - if (info.preallocate) - virBufferAddLit(&buf, "preallocation=metadata,"); + if (info.format == VIR_STORAGE_FILE_LUKS) { + if (!enc) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("missing luks encryption information")); + goto error; + } + virQEMUBuildLuksOpts(&buf, enc, info.secretAlias); + } else { + if (info.backingPath) + virBufferAsprintf(&buf, "backing_fmt=%s,", + virStorageFileFormatTypeToString(info.backingFormat)); + if (info.encryption) + virBufferAddLit(&buf, "encryption=on,"); + if (info.preallocate) + virBufferAddLit(&buf, "preallocation=metadata,"); + } + if (info.nocow) virBufferAddLit(&buf, "nocow=on,"); @@ -1025,6 +1048,22 @@ virStorageBackendCreateQemuImgCheckEncryption(int format, if (virStorageGenerateQcowEncryption(conn, vol) < 0) return -1; } + } else if (format == VIR_STORAGE_FILE_LUKS) { + if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported volume encryption format %d"), + vol->target.encryption->format); + return -1; + } + if (enc->nsecrets > 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("too many secrets for luks encryption")); + return -1; + } + if (enc->nsecrets == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("no secret provided for luks encryption")); + } } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("volume encryption unsupported with format %s"), type); @@ -1069,6 +1108,12 @@ virStorageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool, int accessRetCode = -1; char *absolutePath = NULL; + if (info->format == VIR_STORAGE_FILE_LUKS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cannot set backing store for luks volume")); + return -1; + } + info->backingFormat = vol->target.backingStore->format; info->backingPath = vol->target.backingStore->path; @@ -1122,6 +1167,7 @@ virStorageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool, static int virStorageBackendCreateQemuImgSetOptions(virCommandPtr cmd, int imgformat, + virStorageEncryptionInfoDefPtr enc, struct _virStorageBackendQemuImgInfo info) { char *opts = NULL; @@ -1130,7 +1176,7 @@ virStorageBackendCreateQemuImgSetOptions(virCommandPtr cmd, imgformat >= QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT) info.compat = "0.10"; - if (virStorageBackendCreateQemuImgOpts(&opts, info) < 0) + if (virStorageBackendCreateQemuImgOpts(enc, &opts, info) < 0) return -1; if (opts) virCommandAddArgList(cmd, "-o", opts, NULL); @@ -1140,6 +1186,43 @@ virStorageBackendCreateQemuImgSetOptions(virCommandPtr cmd, } +/* Add a secret object to the command line: + * --object secret,id=$secretAlias,file=$secretPath + * + * NB: format=raw is assumed + */ +static int +virStorageBackendCreateQemuImgSecretObject(virCommandPtr cmd, + virStorageVolDefPtr vol, + struct _virStorageBackendQemuImgInfo *info) +{ + char *str = NULL; + virJSONValuePtr props = NULL; + char *commandStr = NULL; + + if (virAsprintf(&info->secretAlias, "%s_luks0", vol->name) < 0) { + VIR_FREE(str); + return -1; + } + VIR_FREE(str); + + if (virJSONValueObjectCreate(&props, "s:file", info->secretPath, NULL) < 0) + return -1; + + if (!(commandStr = virQEMUBuildObjectCommandlineFromJSON("secret", + info->secretAlias, + props))) { + virJSONValueFree(props); + return -1; + } + virJSONValueFree(props); + + virCommandAddArgList(cmd, "--object", commandStr, NULL); + + return 0; +} + + /* Create a qemu-img virCommand from the supplied binary path, * volume definitions and imgformat */ @@ -1150,7 +1233,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, virStorageVolDefPtr inputvol, unsigned int flags, const char *create_tool, - int imgformat) + int imgformat, + const char *secretPath) { virCommandPtr cmd = NULL; const char *type; @@ -1162,7 +1246,10 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, .compat = vol->target.compat, .features = vol->target.features, .nocow = vol->target.nocow, + .secretPath = secretPath, + .secretAlias = NULL, }; + virStorageEncryptionInfoDefPtr enc = NULL; virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); @@ -1192,6 +1279,18 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, _("format features only available with qcow2")); return NULL; } + if (info.format == VIR_STORAGE_FILE_LUKS) { + if (inputvol) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cannot use inputvol with luks volume")); + return NULL; + } + if (!info.encryption) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing encryption description")); + return NULL; + } + } if (inputvol && virStorageBackendCreateQemuImgSetInput(inputvol, &info) < 0) @@ -1226,10 +1325,22 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, if (info.backingPath) virCommandAddArgList(cmd, "-b", info.backingPath, NULL); - if (virStorageBackendCreateQemuImgSetOptions(cmd, imgformat, info) < 0) { + if (info.format == VIR_STORAGE_FILE_LUKS) { + if (virStorageBackendCreateQemuImgSecretObject(cmd, vol, &info) < 0) { + VIR_FREE(info.secretAlias); + virCommandFree(cmd); + return NULL; + } + enc = &vol->target.encryption->encinfo; + } + + if (virStorageBackendCreateQemuImgSetOptions(cmd, imgformat, + enc, info) < 0) { + VIR_FREE(info.secretAlias); virCommandFree(cmd); return NULL; } + VIR_FREE(info.secretAlias); if (info.inputPath) virCommandAddArg(cmd, info.inputPath); @@ -1240,6 +1351,77 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, return cmd; } + +static char * +virStorageBackendCreateQemuImgSecretPath(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol) +{ + virStorageEncryptionPtr enc = vol->target.encryption; + char *secretPath = NULL; + int fd = -1; + uint8_t *secret = NULL; + size_t secretlen = 0; + + if (!enc) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing encryption description")); + return NULL; + } + + if (!conn || !conn->secretDriver || + !conn->secretDriver->secretLookupByUUID || + !conn->secretDriver->secretLookupByUsage || + !conn->secretDriver->secretGetValue) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to look up encryption secret")); + return NULL; + } + + if (!(secretPath = virStoragePoolObjBuildTempFilePath(pool, vol))) + goto cleanup; + + if ((fd = mkostemp(secretPath, O_CLOEXEC)) < 0) { + virReportSystemError(errno, "%s", + _("failed to open luks secret file for write")); + goto error; + } + + if (virSecretGetSecretString(conn, &enc->secrets[0]->seclookupdef, + VIR_SECRET_USAGE_TYPE_VOLUME, + &secret, &secretlen) < 0) + goto error; + + if (safewrite(fd, secret, secretlen) < 0) { + virReportSystemError(errno, "%s", + _("failed to write luks secret file")); + goto error; + } + VIR_FORCE_CLOSE(fd); + + if ((vol->target.perms->uid != (uid_t) -1) && + (vol->target.perms->gid != (gid_t) -1)) { + if (chown(secretPath, vol->target.perms->uid, + vol->target.perms->gid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to chown luks secret file")); + goto error; + } + } + + cleanup: + VIR_DISPOSE_N(secret, secretlen); + VIR_FORCE_CLOSE(fd); + + return secretPath; + + error: + unlink(secretPath); + VIR_FREE(secretPath); + goto cleanup; +} + + int virStorageBackendCreateQemuImg(virConnectPtr conn, virStoragePoolObjPtr pool, @@ -1251,6 +1433,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, char *create_tool; int imgformat; virCommandPtr cmd; + char *secretPath = NULL; virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1); @@ -1266,8 +1449,15 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, if (imgformat < 0) goto cleanup; + if (vol->target.format == VIR_STORAGE_FILE_LUKS) { + if (!(secretPath = + virStorageBackendCreateQemuImgSecretPath(conn, pool, vol))) + goto cleanup; + } + cmd = virStorageBackendCreateQemuImgCmdFromVol(conn, pool, vol, inputvol, - flags, create_tool, imgformat); + flags, create_tool, + imgformat, secretPath); if (!cmd) goto cleanup; @@ -1275,6 +1465,10 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, virCommandFree(cmd); cleanup: + if (secretPath) { + unlink(secretPath); + VIR_FREE(secretPath); + } VIR_FREE(create_tool); return ret; } diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 5bc622c..28e1a65 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -241,7 +241,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, virStorageVolDefPtr inputvol, unsigned int flags, const char *create_tool, - int imgformat); + int imgformat, + const char *secretPath); /* ------- virStorageFile backends ------------ */ typedef struct _virStorageFileBackend virStorageFileBackend; diff --git a/src/util/virqemu.c b/src/util/virqemu.c index 895168e..dd7a59f 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -140,3 +140,26 @@ virQEMUBuildObjectCommandlineFromJSON(const char *type, virBufferFreeAndReset(&buf); return ret; } + + +void +virQEMUBuildLuksOpts(virBufferPtr buf, + virStorageEncryptionInfoDefPtr enc, + const char *alias) +{ + virBufferAsprintf(buf, "key-secret=%s,", alias); + + /* If there's any cipher, then add that to the command line */ + if (enc->cipher_name) { + virBufferEscapeString(buf, "cipher-alg=%s-", enc->cipher_name); + virBufferAsprintf(buf, "%u,", enc->cipher_size); + if (enc->cipher_mode) + virBufferEscapeString(buf, "cipher-mode=%s,", enc->cipher_mode); + if (enc->cipher_hash) + virBufferEscapeString(buf, "hash-alg=%s,", enc->cipher_hash); + if (enc->ivgen_name) + virBufferEscapeString(buf, "ivgen-alg=%s,", enc->ivgen_name); + if (enc->ivgen_hash) + virBufferEscapeString(buf, "ivgen-hash-alg=%s,", enc->ivgen_hash); + } +} diff --git a/src/util/virqemu.h b/src/util/virqemu.h index af4ec1d..46e7ae2 100644 --- a/src/util/virqemu.h +++ b/src/util/virqemu.h @@ -26,9 +26,15 @@ # include "internal.h" # include "virjson.h" +# include "virstorageencryption.h" char *virQEMUBuildObjectCommandlineFromJSON(const char *type, const char *alias, virJSONValuePtr props); +void virQEMUBuildLuksOpts(virBufferPtr buf, + virStorageEncryptionInfoDefPtr enc, + const char *alias) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + #endif /* __VIR_QEMU_H_ */ diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index ccfe9ab..e300821 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -83,7 +83,8 @@ testCompareXMLToArgvFiles(bool shouldFail, cmd = virStorageBackendCreateQemuImgCmdFromVol(conn, &poolobj, vol, inputvol, flags, - create_tool, imgformat); + create_tool, imgformat, + NULL); if (!cmd) { if (shouldFail) { virResetLastError(); -- 2.5.5

On Mon, Jul 11, 2016 at 02:07:56PM -0400, John Ferlan wrote: I would rather set it to NULL right after the AddObject call and track whether we need to remove the object in a separate variable. This way when qemuDomainObjExitMonitor fails, we jump to 'failexitmonitor' without setting it to NULL and free it again. This will need to be repeated on every label. In hindsight, using a set of bools for rollback (like qemuDomainAttachHostUSBDevice does) might have been more readable than separate labels. Can't we access the disk private info here, to use the same conditions as we did for adding it? ACK with the double free fixed. Jan

Soon we will be adding luks encryption support. Since a volume could require both a luks secret and a secret to give to the server to use of the device, alter the alias generation to create a slightly different alias so that we don't have two objects with the same alias. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/qemu/qemu_alias.c | 10 ++++++++-- src/qemu/qemu_alias.h | 3 ++- src/qemu/qemu_domain.c | 17 ++++++++++------- src/qemu/qemu_hotplug.c | 3 ++- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index d624071..51a654a 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -485,13 +485,16 @@ qemuDomainGetMasterKeyAlias(void) /* qemuDomainGetSecretAESAlias: + * @srcalias: Source alias used to generate the secret alias + * @isLuks: True when we are generating a secret for LUKS encrypt/decrypt * * Generate and return an alias for the encrypted secret * * Returns NULL or a string containing the alias */ char * -qemuDomainGetSecretAESAlias(const char *srcalias) +qemuDomainGetSecretAESAlias(const char *srcalias, + bool isLuks) { char *alias; @@ -501,7 +504,10 @@ qemuDomainGetSecretAESAlias(const char *srcalias) return NULL; } - ignore_value(virAsprintf(&alias, "%s-secret0", srcalias)); + if (isLuks) + ignore_value(virAsprintf(&alias, "%s-luks-secret0", srcalias)); + else + ignore_value(virAsprintf(&alias, "%s-secret0", srcalias)); return alias; } diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index e328a9b..d1c6ba8 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -69,6 +69,7 @@ char *qemuAliasFromDisk(const virDomainDiskDef *disk); char *qemuDomainGetMasterKeyAlias(void); -char *qemuDomainGetSecretAESAlias(const char *srcalias); +char *qemuDomainGetSecretAESAlias(const char *srcalias, + bool isLuks); #endif /* __QEMU_ALIAS_H__*/ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 286f096..d046633 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -895,6 +895,7 @@ qemuDomainSecretPlainSetup(virConnectPtr conn, * @secretUsageType: The virSecretUsageType * @username: username to use for authentication (may be NULL) * @seclookupdef: Pointer to seclookupdef data + * @isLuks: True/False for is for luks (alias generation) * * Taking a secinfo, fill in the AES specific information using the * @@ -907,7 +908,8 @@ qemuDomainSecretAESSetup(virConnectPtr conn, const char *srcalias, virSecretUsageType secretUsageType, const char *username, - virSecretLookupTypeDefPtr seclookupdef) + virSecretLookupTypeDefPtr seclookupdef, + bool isLuks) { int ret = -1; uint8_t *raw_iv = NULL; @@ -921,7 +923,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, if (VIR_STRDUP(secinfo->s.aes.username, username) < 0) return -1; - if (!(secinfo->s.aes.alias = qemuDomainGetSecretAESAlias(srcalias))) + if (!(secinfo->s.aes.alias = qemuDomainGetSecretAESAlias(srcalias, isLuks))) return -1; /* Create a random initialization vector */ @@ -970,6 +972,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, * @secretUsageType: The virSecretUsageType * @username: username to use for authentication (may be NULL) * @seclookupdef: Pointer to seclookupdef data + * @isLuks: True when is luks (generates different alias) * * If we have the encryption API present and can support a secret object, then * build the AES secret; otherwise, build the Plain secret. This is the magic @@ -985,14 +988,15 @@ qemuDomainSecretSetup(virConnectPtr conn, const char *srcalias, virSecretUsageType secretUsageType, const char *username, - virSecretLookupTypeDefPtr seclookupdef) + virSecretLookupTypeDefPtr seclookupdef, + bool isLuks) { if (virCryptoHaveCipher(VIR_CRYPTO_CIPHER_AES256CBC) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) && secretUsageType == VIR_SECRET_USAGE_TYPE_CEPH) { if (qemuDomainSecretAESSetup(conn, priv, secinfo, srcalias, secretUsageType, username, - seclookupdef) < 0) + seclookupdef, isLuks) < 0) return -1; } else { if (qemuDomainSecretPlainSetup(conn, secinfo, secretUsageType, @@ -1052,7 +1056,6 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, qemuDomainSecretInfoPtr secinfo = NULL; if (conn && qemuDomainSecretDiskCapable(src)) { - virSecretUsageType secretUsageType = VIR_SECRET_USAGE_TYPE_ISCSI; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); @@ -1064,7 +1067,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias, secretUsageType, src->auth->username, - &src->auth->seclookupdef) < 0) + &src->auth->seclookupdef, false) < 0) goto error; diskPriv->secinfo = secinfo; @@ -1131,7 +1134,7 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn, if (qemuDomainSecretSetup(conn, priv, secinfo, hostdev->info->alias, VIR_SECRET_USAGE_TYPE_ISCSI, iscsisrc->auth->username, - &iscsisrc->auth->seclookupdef) < 0) + &iscsisrc->auth->seclookupdef, false) < 0) goto error; hostdevPriv->secinfo = secinfo; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 004d6e3..376e6aa 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2853,7 +2853,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) && qemuDomainSecretDiskCapable(disk->src)) { - if (!(objAlias = qemuDomainGetSecretAESAlias(disk->info.alias))) { + if (!(objAlias = + qemuDomainGetSecretAESAlias(disk->info.alias, false))) { VIR_FREE(drivestr); return -1; } -- 2.5.5

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1301021 Generate the luks command line using the AES secret key to encrypt the luks secret. A luks secret object will be in addition to a an AES secret. For hotplug, check if the encinfo exists and if so, add the AES secret for the passphrase for the secret object used to decrypt the device. Modify/augment the fakeSecret* in qemuxml2argvtest in order to handle find a uuid or a volume usage with a specific path prefix in the XML (corresponds to the already generated XML tests). Add error message when the 'usageID' is not 'mycluster_myname'. Commit id '1d632c39' altered the error message generation to rely on the errors from the secret_driver (or it's faked replacement). Add the .args output for adding the LUKS disk to the domain Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/qemu/qemu_command.c | 9 +++ src/qemu/qemu_domain.c | 25 +++++++- src/qemu/qemu_hotplug.c | 74 +++++++++++++++++++++- .../qemuxml2argvdata/qemuxml2argv-luks-disks.args | 36 +++++++++++ tests/qemuxml2argvtest.c | 24 ++++++- 5 files changed, 160 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c97d0f6..c356450 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1098,6 +1098,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, int actualType = virStorageSourceGetActualType(disk->src); qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; + qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo; bool emitDeviceSyntax = qemuDiskBusNeedsDeviceArg(disk->bus); if (idx < 0) { @@ -1237,6 +1238,10 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, secinfo->s.aes.alias); } + if (encinfo) + virQEMUBuildLuksOpts(&opt, &disk->src->encryption->encinfo, + encinfo->s.aes.alias); + if (disk->src->format > 0 && disk->src->type != VIR_STORAGE_TYPE_DIR) virBufferAsprintf(&opt, "format=%s,", @@ -1940,6 +1945,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, virDomainDiskDefPtr disk = def->disks[i]; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; + qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo; /* PowerPC pseries based VMs do not support floppy device */ if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && @@ -1976,6 +1982,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0) return -1; + if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0) + return -1; + virCommandAddArg(cmd, "-drive"); if (!(optstr = qemuBuildDriveStr(disk, driveBoot, qemuCaps))) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d046633..ef7e3c3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -993,7 +993,8 @@ qemuDomainSecretSetup(virConnectPtr conn, { if (virCryptoHaveCipher(VIR_CRYPTO_CIPHER_AES256CBC) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) && - secretUsageType == VIR_SECRET_USAGE_TYPE_CEPH) { + (secretUsageType == VIR_SECRET_USAGE_TYPE_CEPH || + secretUsageType == VIR_SECRET_USAGE_TYPE_VOLUME)) { if (qemuDomainSecretAESSetup(conn, priv, secinfo, srcalias, secretUsageType, username, seclookupdef, isLuks) < 0) @@ -1053,11 +1054,14 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, virDomainDiskDefPtr disk) { virStorageSourcePtr src = disk->src; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainSecretInfoPtr secinfo = NULL; - if (conn && qemuDomainSecretDiskCapable(src)) { + if (!conn) + return 0; + + if (qemuDomainSecretDiskCapable(src)) { virSecretUsageType secretUsageType = VIR_SECRET_USAGE_TYPE_ISCSI; - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); if (VIR_ALLOC(secinfo) < 0) return -1; @@ -1073,6 +1077,21 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, diskPriv->secinfo = secinfo; } + if (!virStorageSourceIsEmpty(src) && src->encryption && + src->format == VIR_STORAGE_FILE_LUKS) { + + if (VIR_ALLOC(secinfo) < 0) + return -1; + + if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias, + VIR_SECRET_USAGE_TYPE_VOLUME, NULL, + &src->encryption->secrets[0]->seclookupdef, + true) < 0) + goto error; + + diskPriv->encinfo = secinfo; + } + return 0; error: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 376e6aa..d8a9fee 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -311,8 +311,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *src = virDomainDiskGetSource(disk); virJSONValuePtr secobjProps = NULL; + virJSONValuePtr encProps = NULL; qemuDomainDiskPrivatePtr diskPriv; qemuDomainSecretInfoPtr secinfo; + qemuDomainSecretInfoPtr encinfo; if (!disk->info.type) { if (qemuDomainMachineIsS390CCW(vm->def) && @@ -352,6 +354,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, goto error; } + encinfo = diskPriv->encinfo; + if (encinfo && qemuBuildSecretInfoProps(encinfo, &encProps) < 0) + goto error; + if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) goto error; @@ -371,6 +377,11 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, secobjProps) < 0) goto exit_monitor; + if (encProps && qemuMonitorAddObject(priv->mon, "secret", + encinfo->s.aes.alias, + encProps) < 0) + goto failaddencsecret; + if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) goto failadddrive; @@ -386,6 +397,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, * for successful exit from monitor to clear; otherwise, error * paths wouldn't clean up properly */ secobjProps = NULL; + encProps = NULL; virDomainAuditDisk(vm, NULL, disk->src, "attach", true); @@ -394,6 +406,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, cleanup: virJSONValueFree(secobjProps); + virJSONValueFree(encProps); qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); VIR_FREE(drivestr); @@ -409,6 +422,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, } failadddrive: + if (encProps) { + if (!orig_err) + orig_err = virSaveLastError(); + ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias)); + } + + failaddencsecret: if (secobjProps) { if (!orig_err) orig_err = virSaveLastError(); @@ -423,7 +443,9 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, exit_monitor: if (qemuDomainObjExitMonitor(driver, vm) < 0) releaseaddr = false; - secobjProps = NULL; /* qemuMonitorAddObject consumes props on failure too */ + /* qemuMonitorAddObject consumes props on failure too */ + secobjProps = NULL; + encProps = NULL; failexitmonitor: virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -575,10 +597,14 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, { size_t i; qemuDomainObjPrivatePtr priv = vm->privateData; + virErrorPtr orig_err = NULL; char *drivestr = NULL; char *devstr = NULL; int ret = -1; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virJSONValuePtr encProps = NULL; + qemuDomainDiskPrivatePtr diskPriv; + qemuDomainSecretInfoPtr encinfo; if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0) goto cleanup; @@ -609,6 +635,11 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (qemuDomainSecretDiskPrepare(conn, priv, disk) < 0) goto error; + diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + encinfo = diskPriv->encinfo; + if (encinfo && qemuBuildSecretInfoProps(encinfo, &encProps) < 0) + goto error; + if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) goto error; @@ -618,9 +649,13 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) goto error; - /* Attach the device - 2 step process */ qemuDomainObjEnterMonitor(driver, vm); + if (encProps && qemuMonitorAddObject(priv->mon, "secret", + encinfo->s.aes.alias, + encProps) < 0) + goto exit_monitor; + if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) goto failadddrive; @@ -630,6 +665,11 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (qemuDomainObjExitMonitor(driver, vm) < 0) goto failexitmonitor; + /* qemuMonitorAddObject consumes the props, but we need to wait + * for successful exit from monitor to clear; otherwise, error + * paths wouldn't clean up properly */ + encProps = NULL; + virDomainAuditDisk(vm, NULL, disk->src, "attach", true); virDomainDiskInsertPreAlloced(vm->def, disk); @@ -647,7 +687,17 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, VIR_WARN("qemuMonitorAddDevice failed on %s (%s)", drivestr, devstr); failadddrive: + orig_err = virSaveLastError(); + if (encProps) + ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias)); + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } + + exit_monitor: ignore_value(qemuDomainObjExitMonitor(driver, vm)); + encProps = NULL; /* qemuMonitorAddObject consumes props on failure too */ failexitmonitor: virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -2838,6 +2888,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; char *drivestr; char *objAlias = NULL; + char *encAlias = NULL; VIR_DEBUG("Removing disk %s from domain %p %s", disk->info.alias, vm, vm->def->name); @@ -2860,6 +2911,20 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, } } + /* Similarly, if this is possible a device using LUKS encryption, we + * can remove the luks object password too + */ + if (!virStorageSourceIsEmpty(disk->src) && disk->src->encryption && + disk->src->format == VIR_STORAGE_FILE_LUKS) { + + if (!(encAlias = + qemuDomainGetSecretAESAlias(disk->info.alias, true))) { + VIR_FREE(objAlias); + VIR_FREE(drivestr); + return -1; + } + } + qemuDomainObjEnterMonitor(driver, vm); /* If it fails, then so be it - it was a best shot */ @@ -2867,6 +2932,11 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, ignore_value(qemuMonitorDelObject(priv->mon, objAlias)); VIR_FREE(objAlias); + /* If it fails, then so be it - it was a best shot */ + if (encAlias) + ignore_value(qemuMonitorDelObject(priv->mon, encAlias)); + VIR_FREE(encAlias); + qemuMonitorDriveDel(priv->mon, drivestr); VIR_FREE(drivestr); if (qemuDomainObjExitMonitor(driver, vm) < 0) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args new file mode 100644 index 0000000..270322f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args @@ -0,0 +1,36 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name encryptdisk \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-encryptdisk/master-key.aes \ +-M pc-i440fx-2.1 \ +-m 1024 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 496898a6-e6ff-f7c8-5dc2-3cf410945ee9 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-encryptdisk/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-object secret,id=virtio-disk0-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive file=/storage/guest_disks/encryptdisk,\ +key-secret=virtio-disk0-luks-secret0,format=luks,if=none,id=drive-virtio-disk0 \ +-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-object secret,id=virtio-disk1-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive file=/storage/guest_disks/encryptdisk2,\ +key-secret=virtio-disk1-luks-secret0,format=luks,if=none,id=drive-virtio-disk1 \ +-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk1,\ +id=virtio-disk1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2bfd1ca..500e78c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -49,12 +49,22 @@ fakeSecretGetValue(virSecretPtr obj ATTRIBUTE_UNUSED, static virSecretPtr fakeSecretLookupByUsage(virConnectPtr conn, - int usageType ATTRIBUTE_UNUSED, + int usageType, const char *usageID) { unsigned char uuid[VIR_UUID_BUFLEN]; - if (STRNEQ(usageID, "mycluster_myname")) + if (usageType == VIR_SECRET_USAGE_TYPE_VOLUME) { + if (!STRPREFIX(usageID, "/storage/guest_disks/")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "test provided invalid volume storage prefix '%s'", + usageID); + return NULL; + } + } else if (STRNEQ(usageID, "mycluster_myname")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "test provided incorrect usage '%s'", usageID); return NULL; + } if (virUUIDGenerate(uuid) < 0) return NULL; @@ -62,10 +72,17 @@ fakeSecretLookupByUsage(virConnectPtr conn, return virGetSecret(conn, uuid, usageType, usageID); } +static virSecretPtr +fakeSecretLookupByUUID(virConnectPtr conn, + const unsigned char *uuid) +{ + return virGetSecret(conn, uuid, 0, ""); +} + static virSecretDriver fakeSecretDriver = { .connectNumOfSecrets = NULL, .connectListSecrets = NULL, - .secretLookupByUUID = NULL, + .secretLookupByUUID = fakeSecretLookupByUUID, .secretLookupByUsage = fakeSecretLookupByUsage, .secretDefineXML = NULL, .secretGetXMLDesc = NULL, @@ -1345,6 +1362,7 @@ mymain(void) DO_TEST("encrypted-disk", NONE); DO_TEST("encrypted-disk-usage", NONE); + DO_TEST("luks-disks", QEMU_CAPS_OBJECT_SECRET); DO_TEST("memtune", NONE); DO_TEST("memtune-unlimited", NONE); -- 2.5.5

[...] This is now cleaner with the bool-based cleanup... Much cleaner now with bool-based cleanup John