Rather than assume/pass the protocol to the qemuDomainSecretPlainSetup and qemuDomainSecretAESSetup, set and pass the secretUsageType based on the src->protocol type. This will eventually be used by the virSecretGetSecretString call Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 62 ++++++++++++++++---------------------------------- 1 file changed, 20 insertions(+), 42 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4a5378f..ca49db1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -814,7 +814,7 @@ qemuDomainHostdevPrivateDispose(void *obj) /* qemuDomainSecretPlainSetup: * @conn: Pointer to connection * @secinfo: Pointer to secret info - * @protocol: Protocol for secret + * @secretUsageType: The virSecretUsageType * @authdef: Pointer to auth data * * Taking a secinfo, fill in the plaintext information @@ -824,19 +824,15 @@ qemuDomainHostdevPrivateDispose(void *obj) static int qemuDomainSecretPlainSetup(virConnectPtr conn, qemuDomainSecretInfoPtr secinfo, - virStorageNetProtocol protocol, + virSecretUsageType secretUsageType, virStorageAuthDefPtr authdef) { - int secretType = VIR_SECRET_USAGE_TYPE_ISCSI; - secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN; if (VIR_STRDUP(secinfo->s.plain.username, authdef->username) < 0) return -1; - if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD) - secretType = VIR_SECRET_USAGE_TYPE_CEPH; - - return virSecretGetSecretString(conn, &authdef->seclookupdef, secretType, + return virSecretGetSecretString(conn, &authdef->seclookupdef, + secretUsageType, &secinfo->s.plain.secret, &secinfo->s.plain.secretlen); } @@ -847,7 +843,7 @@ qemuDomainSecretPlainSetup(virConnectPtr conn, * @priv: pointer to domain private object * @secinfo: Pointer to secret info * @srcalias: Alias of the disk/hostdev used to generate the secret alias - * @protocol: Protocol for secret + * @secretUsageType: The virSecretUsageType * @authdef: Pointer to auth data * * Taking a secinfo, fill in the AES specific information using the @@ -859,7 +855,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, qemuDomainObjPrivatePtr priv, qemuDomainSecretInfoPtr secinfo, const char *srcalias, - virStorageNetProtocol protocol, + virSecretUsageType secretUsageType, virStorageAuthDefPtr authdef) { int ret = -1; @@ -869,34 +865,11 @@ qemuDomainSecretAESSetup(virConnectPtr conn, size_t secretlen = 0; uint8_t *ciphertext = NULL; size_t ciphertextlen = 0; - int secretType = VIR_SECRET_USAGE_TYPE_NONE; secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_AES; if (VIR_STRDUP(secinfo->s.aes.username, authdef->username) < 0) return -1; - switch ((virStorageNetProtocol)protocol) { - case VIR_STORAGE_NET_PROTOCOL_RBD: - secretType = VIR_SECRET_USAGE_TYPE_CEPH; - break; - - case VIR_STORAGE_NET_PROTOCOL_NONE: - case VIR_STORAGE_NET_PROTOCOL_NBD: - case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: - case VIR_STORAGE_NET_PROTOCOL_GLUSTER: - case VIR_STORAGE_NET_PROTOCOL_ISCSI: - case VIR_STORAGE_NET_PROTOCOL_HTTP: - case VIR_STORAGE_NET_PROTOCOL_HTTPS: - case VIR_STORAGE_NET_PROTOCOL_FTP: - case VIR_STORAGE_NET_PROTOCOL_FTPS: - case VIR_STORAGE_NET_PROTOCOL_TFTP: - case VIR_STORAGE_NET_PROTOCOL_LAST: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("protocol '%s' cannot be used for encrypted secrets"), - virStorageNetProtocolTypeToString(protocol)); - return -1; - } - if (!(secinfo->s.aes.alias = qemuDomainGetSecretAESAlias(srcalias))) return -1; @@ -909,7 +882,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, goto cleanup; /* Grab the unencoded secret */ - if (virSecretGetSecretString(conn, &authdef->seclookupdef, secretType, + if (virSecretGetSecretString(conn, &authdef->seclookupdef, secretUsageType, &secret, &secretlen) < 0) goto cleanup; @@ -943,7 +916,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, * @priv: pointer to domain private object * @secinfo: Pointer to secret info * @srcalias: Alias of the disk/hostdev used to generate the secret alias - * @protocol: Protocol for secret + * @secretUsageType: The virSecretUsageType * @authdef: Pointer to auth data * * If we have the encryption API present and can support a secret object, then @@ -958,17 +931,18 @@ qemuDomainSecretSetup(virConnectPtr conn, qemuDomainObjPrivatePtr priv, qemuDomainSecretInfoPtr secinfo, const char *srcalias, - virStorageNetProtocol protocol, + virSecretUsageType secretUsageType, virStorageAuthDefPtr authdef) { if (virCryptoHaveCipher(VIR_CRYPTO_CIPHER_AES256CBC) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) && - protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { - if (qemuDomainSecretAESSetup(conn, priv, secinfo, - srcalias, protocol, authdef) < 0) + secretUsageType == VIR_SECRET_USAGE_TYPE_CEPH) { + if (qemuDomainSecretAESSetup(conn, priv, secinfo, srcalias, + secretUsageType, authdef) < 0) return -1; } else { - if (qemuDomainSecretPlainSetup(conn, secinfo, protocol, authdef) < 0) + if (qemuDomainSecretPlainSetup(conn, secinfo, secretUsageType, + authdef) < 0) return -1; } return 0; @@ -1015,13 +989,17 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI || src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) { + virSecretUsageType secretUsageType = VIR_SECRET_USAGE_TYPE_ISCSI; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); if (VIR_ALLOC(secinfo) < 0) return -1; + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) + secretUsageType = VIR_SECRET_USAGE_TYPE_CEPH; + if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias, - src->protocol, src->auth) < 0) + secretUsageType, src->auth) < 0) goto error; diskPriv->secinfo = secinfo; @@ -1086,7 +1064,7 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn, return -1; if (qemuDomainSecretSetup(conn, priv, secinfo, hostdev->info->alias, - VIR_STORAGE_NET_PROTOCOL_ISCSI, + VIR_SECRET_USAGE_TYPE_ISCSI, iscsisrc->auth) < 0) goto error; -- 2.5.5

Rather than pass authdef, pass the 'authdef->username' and the '&authdef->secdef' Note that a username may be NULL. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ca49db1..dca8970 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -815,7 +815,8 @@ qemuDomainHostdevPrivateDispose(void *obj) * @conn: Pointer to connection * @secinfo: Pointer to secret info * @secretUsageType: The virSecretUsageType - * @authdef: Pointer to auth data + * @username: username to use for authentication (may be NULL) + * @seclookupdef: Pointer to seclookupdef data * * Taking a secinfo, fill in the plaintext information * @@ -825,14 +826,14 @@ static int qemuDomainSecretPlainSetup(virConnectPtr conn, qemuDomainSecretInfoPtr secinfo, virSecretUsageType secretUsageType, - virStorageAuthDefPtr authdef) + const char *username, + virSecretLookupTypeDefPtr seclookupdef) { secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN; - if (VIR_STRDUP(secinfo->s.plain.username, authdef->username) < 0) + if (VIR_STRDUP(secinfo->s.plain.username, username) < 0) return -1; - return virSecretGetSecretString(conn, &authdef->seclookupdef, - secretUsageType, + return virSecretGetSecretString(conn, seclookupdef, secretUsageType, &secinfo->s.plain.secret, &secinfo->s.plain.secretlen); } @@ -844,7 +845,8 @@ qemuDomainSecretPlainSetup(virConnectPtr conn, * @secinfo: Pointer to secret info * @srcalias: Alias of the disk/hostdev used to generate the secret alias * @secretUsageType: The virSecretUsageType - * @authdef: Pointer to auth data + * @username: username to use for authentication (may be NULL) + * @seclookupdef: Pointer to seclookupdef data * * Taking a secinfo, fill in the AES specific information using the * @@ -856,7 +858,8 @@ qemuDomainSecretAESSetup(virConnectPtr conn, qemuDomainSecretInfoPtr secinfo, const char *srcalias, virSecretUsageType secretUsageType, - virStorageAuthDefPtr authdef) + const char *username, + virSecretLookupTypeDefPtr seclookupdef) { int ret = -1; uint8_t *raw_iv = NULL; @@ -867,7 +870,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, size_t ciphertextlen = 0; secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_AES; - if (VIR_STRDUP(secinfo->s.aes.username, authdef->username) < 0) + if (VIR_STRDUP(secinfo->s.aes.username, username) < 0) return -1; if (!(secinfo->s.aes.alias = qemuDomainGetSecretAESAlias(srcalias))) @@ -882,7 +885,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, goto cleanup; /* Grab the unencoded secret */ - if (virSecretGetSecretString(conn, &authdef->seclookupdef, secretUsageType, + if (virSecretGetSecretString(conn, seclookupdef, secretUsageType, &secret, &secretlen) < 0) goto cleanup; @@ -917,7 +920,8 @@ qemuDomainSecretAESSetup(virConnectPtr conn, * @secinfo: Pointer to secret info * @srcalias: Alias of the disk/hostdev used to generate the secret alias * @secretUsageType: The virSecretUsageType - * @authdef: Pointer to auth data + * @username: username to use for authentication (may be NULL) + * @seclookupdef: Pointer to seclookupdef data * * 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 @@ -932,17 +936,19 @@ qemuDomainSecretSetup(virConnectPtr conn, qemuDomainSecretInfoPtr secinfo, const char *srcalias, virSecretUsageType secretUsageType, - virStorageAuthDefPtr authdef) + const char *username, + virSecretLookupTypeDefPtr seclookupdef) { 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, authdef) < 0) + secretUsageType, username, + seclookupdef) < 0) return -1; } else { if (qemuDomainSecretPlainSetup(conn, secinfo, secretUsageType, - authdef) < 0) + username, seclookupdef) < 0) return -1; } return 0; @@ -999,7 +1005,8 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, secretUsageType = VIR_SECRET_USAGE_TYPE_CEPH; if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias, - secretUsageType, src->auth) < 0) + secretUsageType, src->auth->username, + &src->auth->seclookupdef) < 0) goto error; diskPriv->secinfo = secinfo; @@ -1065,7 +1072,8 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn, if (qemuDomainSecretSetup(conn, priv, secinfo, hostdev->info->alias, VIR_SECRET_USAGE_TYPE_ISCSI, - iscsisrc->auth) < 0) + iscsisrc->auth->username, + &iscsisrc->auth->seclookupdef) < 0) goto error; hostdevPriv->secinfo = secinfo; -- 2.5.5

Add a new secret type known as "passphrase" - it will handle adding the secret objects that need a passphrase without a specific username. The format is: <secret ...> <uuid>...</uuid> ... <usage type='passphrase'> <id>mumblyfratz</id> </usage> </secret> Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- docs/aclpolkit.html.in | 4 +++ docs/formatsecret.html.in | 57 ++++++++++++++++++++++++++++-- docs/schemas/secret.rng | 10 ++++++ include/libvirt/libvirt-secret.h | 3 +- src/access/viraccessdriverpolkit.c | 13 +++++++ src/conf/secret_conf.c | 26 +++++++++++++- src/conf/secret_conf.h | 1 + src/conf/virsecretobj.c | 5 +++ tests/secretxml2xmlin/usage-passphrase.xml | 7 ++++ tests/secretxml2xmltest.c | 1 + 10 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 tests/secretxml2xmlin/usage-passphrase.xml diff --git a/docs/aclpolkit.html.in b/docs/aclpolkit.html.in index dae0814..1d31b6d 100644 --- a/docs/aclpolkit.html.in +++ b/docs/aclpolkit.html.in @@ -224,6 +224,10 @@ <td>secret_usage_target</td> <td>Name of the associated iSCSI target, if any</td> </tr> + <tr> + <td>secret_usage_id</td> + <td>Name of be associated passphrase secret, if any</td> + </tr> </tbody> </table> diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in index 599cb38..79c4082 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>passphrase</code> are defined. Specific usage categories + are described below. </dd> </dl> @@ -241,5 +242,57 @@ &lt;secret usage='libvirtiscsi'/&gt; &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. + <span class="since">Since 2.0.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;id&gt;id_example&lt;/id&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 id_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/schemas/secret.rng b/docs/schemas/secret.rng index e21e700..fc188ba 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -36,6 +36,7 @@ <ref name='usagevolume'/> <ref name='usageceph'/> <ref name='usageiscsi'/> + <ref name='usagepassphrase'/> <!-- More choices later --> </choice> </element> @@ -71,4 +72,13 @@ </element> </define> + <define name='usagepassphrase'> + <attribute name='type'> + <value>passphrase</value> + </attribute> + <element name='id'> + <ref name='genericName'/> + </element> + </define> + </grammar> diff --git a/include/libvirt/libvirt-secret.h b/include/libvirt/libvirt-secret.h index 3e5cdf6..55b11e0 100644 --- a/include/libvirt/libvirt-secret.h +++ b/include/libvirt/libvirt-secret.h @@ -4,7 +4,7 @@ * Description: Provides APIs for the management of secrets * Author: Daniel Veillard <veillard(a)redhat.com&gt; * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2014, 2016 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -43,6 +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, # ifdef VIR_ENUM_SENTINELS VIR_SECRET_USAGE_TYPE_LAST diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c index 89bc890..1f955f0 100644 --- a/src/access/viraccessdriverpolkit.c +++ b/src/access/viraccessdriverpolkit.c @@ -338,6 +338,19 @@ virAccessDriverPolkitCheckSecret(virAccessManagerPtr manager, virAccessPermSecretTypeToString(perm), attrs); } break; + case VIR_SECRET_USAGE_TYPE_PASSPHRASE: { + const char *attrs[] = { + "connect_driver", driverName, + "secret_uuid", uuidstr, + "secret_usage_id", secret->usage.id, + NULL, + }; + + return virAccessDriverPolkitCheck(manager, + "secret", + virAccessPermSecretTypeToString(perm), + attrs); + } break; } } diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index de9e6cf..77477b6 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -29,6 +29,7 @@ #include "viralloc.h" #include "secret_conf.h" #include "virsecretobj.h" +#include "virstring.h" #include "virerror.h" #include "virxml.h" #include "viruuid.h" @@ -38,7 +39,7 @@ VIR_LOG_INIT("conf.secret_conf"); VIR_ENUM_IMPL(virSecretUsage, VIR_SECRET_USAGE_TYPE_LAST, - "none", "volume", "ceph", "iscsi") + "none", "volume", "ceph", "iscsi", "passphrase") const char * virSecretUsageIDForDef(virSecretDefPtr def) @@ -56,6 +57,9 @@ virSecretUsageIDForDef(virSecretDefPtr def) case VIR_SECRET_USAGE_TYPE_ISCSI: return def->usage.target; + case VIR_SECRET_USAGE_TYPE_PASSPHRASE: + return def->usage.id; + default: return NULL; } @@ -85,6 +89,10 @@ virSecretDefFree(virSecretDefPtr def) VIR_FREE(def->usage.target); break; + case VIR_SECRET_USAGE_TYPE_PASSPHRASE: + VIR_FREE(def->usage.id); + break; + default: VIR_ERROR(_("unexpected secret usage type %d"), def->usage_type); break; @@ -92,6 +100,7 @@ virSecretDefFree(virSecretDefPtr def) VIR_FREE(def); } + static int virSecretDefParseUsage(xmlXPathContextPtr ctxt, virSecretDefPtr def) @@ -145,6 +154,14 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt, } break; + case VIR_SECRET_USAGE_TYPE_PASSPHRASE: + if (!(def->usage.id = virXPathString("string(./usage/id)", ctxt))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("passphrase usage specified, but id is missing")); + return -1; + } + break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected secret usage type %d"), @@ -305,6 +322,13 @@ virSecretDefFormatUsage(virBufferPtr buf, } break; + case VIR_SECRET_USAGE_TYPE_PASSPHRASE: + if (def->usage.id != NULL) { + virBufferEscapeString(buf, "<id>%s</id>\n", + def->usage.id); + } + break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected secret usage type %d"), diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h index 4584403..768e6e9 100644 --- a/src/conf/secret_conf.h +++ b/src/conf/secret_conf.h @@ -40,6 +40,7 @@ struct _virSecretDef { char *volume; /* May be NULL */ char *ceph; char *target; + char *id; } usage; }; diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index c46d22c..a2b4801 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -237,6 +237,11 @@ virSecretObjSearchName(const void *payload, if (STREQ(secret->def->usage.target, data->usageID)) found = 1; break; + + case VIR_SECRET_USAGE_TYPE_PASSPHRASE: + if (STREQ(secret->def->usage.id, data->usageID)) + found = 1; + break; } cleanup: diff --git a/tests/secretxml2xmlin/usage-passphrase.xml b/tests/secretxml2xmlin/usage-passphrase.xml new file mode 100644 index 0000000..7ebe0a4 --- /dev/null +++ b/tests/secretxml2xmlin/usage-passphrase.xml @@ -0,0 +1,7 @@ +<secret ephemeral='no' private='no'> + <uuid>f52a81b2-424e-490c-823d-6bd4235bc572</uuid> + <description>Sample Passphrase Secret</description> + <usage type='passphrase'> + <id>mumblyfratz</id> + </usage> +</secret> diff --git a/tests/secretxml2xmltest.c b/tests/secretxml2xmltest.c index 8dcbb40..c444e4d 100644 --- a/tests/secretxml2xmltest.c +++ b/tests/secretxml2xmltest.c @@ -80,6 +80,7 @@ mymain(void) DO_TEST("usage-volume"); DO_TEST("usage-ceph"); DO_TEST("usage-iscsi"); + DO_TEST("usage-passphrase"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.5.5

On 06/24/2016 06:28 AM, Peter Krempa wrote: Name is fine with me. Id is just shorter to type. Ironically the others are off too... This was some cut-n-paste, followed by deletion of a line in order to grab the output of the actual command that I ran... I'll fix it (and others too) - I assume those are trivial... True - but it follows other elements of the case. I could trivially change those as well. John

On Thu, Jun 23, 2016 at 13:29:00 -0400, John Ferlan wrote: I don't think it's necessary to describe how to use virsh here. This looks wrong. Passprhase type secrets list 'name' or 'id' as usage not the path. This contradicts changes in previous patch. Again. Mentioning the API is good enoguh. [...] Wrong type.

In order to read 16 bits of data in the native format and convert add the 16 bit macros to match existing 32 and 64 bit code. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/util/virendian.h | 24 ++++++++++++++++++++++++ tests/virendiantest.c | 18 ++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/util/virendian.h b/src/util/virendian.h index eefe48c..97940bd 100644 --- a/src/util/virendian.h +++ b/src/util/virendian.h @@ -90,4 +90,28 @@ ((uint32_t)(uint8_t)((buf)[2]) << 16) | \ ((uint32_t)(uint8_t)((buf)[3]) << 24)) +/** + * virReadBufInt16BE: + * @buf: byte to start reading at (can be 'char*' or 'unsigned char*'); + * evaluating buf must not have any side effects + * + * Read 2 bytes at BUF as a big-endian 16-bit number. Caller is + * responsible to avoid reading beyond array bounds. + */ +# define virReadBufInt16BE(buf) \ + (((uint16_t)(uint8_t)((buf)[0]) << 8) | \ + (uint16_t)(uint8_t)((buf)[1])) + +/** + * virReadBufInt16LE: + * @buf: byte to start reading at (can be 'char*' or 'unsigned char*'); + * evaluating buf must not have any side effects + * + * Read 2 bytes at BUF as a little-endian 16-bit number. Caller is + * responsible to avoid reading beyond array bounds. + */ +# define virReadBufInt16LE(buf) \ + ((uint16_t)(uint8_t)((buf)[0]) | \ + ((uint16_t)(uint8_t)((buf)[1]) << 8)) + #endif /* __VIR_ENDIAN_H__ */ diff --git a/tests/virendiantest.c b/tests/virendiantest.c index 4072507..f858e5c 100644 --- a/tests/virendiantest.c +++ b/tests/virendiantest.c @@ -50,6 +50,15 @@ test1(const void *data ATTRIBUTE_UNUSED) if (virReadBufInt32LE(array + 9) != 0x8d8c8b8aU) goto cleanup; + if (virReadBufInt16BE(array) != 0x0102U) + goto cleanup; + if (virReadBufInt16BE(array + 11) != 0x8c8dU) + goto cleanup; + if (virReadBufInt16LE(array) != 0x0201U) + goto cleanup; + if (virReadBufInt16LE(array + 11) != 0x8d8cU) + goto cleanup; + ret = 0; cleanup: return ret; @@ -81,6 +90,15 @@ test2(const void *data ATTRIBUTE_UNUSED) if (virReadBufInt32LE(array + 9) != 0x8d8c8b8aU) goto cleanup; + if (virReadBufInt16BE(array) != 0x0102U) + goto cleanup; + if (virReadBufInt16BE(array + 11) != 0x8c8dU) + goto cleanup; + if (virReadBufInt16LE(array) != 0x0201U) + goto cleanup; + if (virReadBufInt16LE(array + 11) != 0x8d8cU) + goto cleanup; + ret = 0; cleanup: return ret; -- 2.5.5

The version field historically has been a 4 byte data; however, an upcoming new type will use a 2 byte version. So let's adjust for that now. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/util/virstoragefile.c | 60 +++++++++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index de4955b..37e9798 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1,7 +1,7 @@ /* * virstoragefile.c: file utility functions for FS storage backend * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-2014, 2016 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -120,10 +120,12 @@ struct FileTypeInfo { * to check at head of file */ const char *extension; /* Optional file extension to check */ enum lv_endian endian; /* Endianness of file format */ + int versionOffset; /* Byte offset from start of file * where we find version number, * -1 to always fail the version test, * -2 to always pass the version test */ + int versionSize; /* Size in bytes of version data (0, 2, or 4) */ int versionNumbers[FILE_TYPE_VERSIONS_LAST]; /* Version numbers to validate. Zeroes are ignored. */ int sizeOffset; /* Byte offset from start of file @@ -189,15 +191,15 @@ qedGetBackingStore(char **, int *, const char *, size_t); static struct FileTypeInfo const fileTypeInfo[] = { [VIR_STORAGE_FILE_NONE] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, - -1, {0}, 0, 0, 0, 0, NULL, NULL }, + -1, 0, {0}, 0, 0, 0, 0, NULL, NULL }, [VIR_STORAGE_FILE_RAW] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, - -1, {0}, 0, 0, 0, 0, NULL, NULL }, + -1, 0, {0}, 0, 0, 0, 0, NULL, NULL }, [VIR_STORAGE_FILE_DIR] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, - -1, {0}, 0, 0, 0, 0, NULL, NULL }, + -1, 0, {0}, 0, 0, 0, 0, NULL, NULL }, [VIR_STORAGE_FILE_BOCHS] = { /*"Bochs Virtual HD Image", */ /* Untested */ 0, NULL, NULL, - LV_LITTLE_ENDIAN, 64, {0x20000}, + LV_LITTLE_ENDIAN, 64, 4, {0x20000}, 32+16+16+4+4+4+4+4, 8, 1, -1, NULL, NULL }, [VIR_STORAGE_FILE_CLOOP] = { @@ -206,7 +208,7 @@ static struct FileTypeInfo const fileTypeInfo[] = { modprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1 */ /* Untested */ 0, NULL, NULL, - LV_LITTLE_ENDIAN, -1, {0}, + LV_LITTLE_ENDIAN, -1, 0, {0}, -1, 0, 0, -1, NULL, NULL }, [VIR_STORAGE_FILE_DMG] = { @@ -214,60 +216,60 @@ static struct FileTypeInfo const fileTypeInfo[] = { * /usr/share/misc/magic lists double magic (both offsets * would have to match) but then disables that check. */ 0, NULL, ".dmg", - 0, -1, {0}, + 0, -1, 0, {0}, -1, 0, 0, -1, NULL, NULL }, [VIR_STORAGE_FILE_ISO] = { 32769, "CD001", ".iso", - LV_LITTLE_ENDIAN, -2, {0}, + LV_LITTLE_ENDIAN, -2, 0, {0}, -1, 0, 0, -1, NULL, NULL }, [VIR_STORAGE_FILE_VPC] = { 0, "conectix", NULL, - LV_BIG_ENDIAN, 12, {0x10000}, + LV_BIG_ENDIAN, 12, 4, {0x10000}, 8 + 4 + 4 + 8 + 4 + 4 + 2 + 2 + 4, 8, 1, -1, NULL, NULL }, /* TODO: add getBackingStore function */ [VIR_STORAGE_FILE_VDI] = { 64, "\x7f\x10\xda\xbe", ".vdi", - LV_LITTLE_ENDIAN, 68, {0x00010001}, + LV_LITTLE_ENDIAN, 68, 4, {0x00010001}, 64 + 5 * 4 + 256 + 7 * 4, 8, 1, -1, NULL, NULL}, /* Not direct file formats, but used for various drivers */ [VIR_STORAGE_FILE_FAT] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, - -1, {0}, 0, 0, 0, 0, NULL, NULL }, + -1, 0, {0}, 0, 0, 0, 0, NULL, NULL }, [VIR_STORAGE_FILE_VHD] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, - -1, {0}, 0, 0, 0, 0, NULL, NULL }, + -1, 0, {0}, 0, 0, 0, 0, NULL, NULL }, [VIR_STORAGE_FILE_PLOOP] = { 0, "WithouFreSpacExt", NULL, LV_LITTLE_ENDIAN, - -2, {0}, PLOOP_IMAGE_SIZE_OFFSET, 0, + -2, 0, {0}, PLOOP_IMAGE_SIZE_OFFSET, 0, PLOOP_SIZE_MULTIPLIER, -1, NULL, NULL }, /* All formats with a backing store probe below here */ [VIR_STORAGE_FILE_COW] = { 0, "OOOM", NULL, - LV_BIG_ENDIAN, 4, {2}, + LV_BIG_ENDIAN, 4, 4, {2}, 4+4+1024+4, 8, 1, -1, cowGetBackingStore, NULL }, [VIR_STORAGE_FILE_QCOW] = { 0, "QFI", NULL, - LV_BIG_ENDIAN, 4, {1}, + LV_BIG_ENDIAN, 4, 4, {1}, QCOWX_HDR_IMAGE_SIZE, 8, 1, QCOW1_HDR_CRYPT, qcow1GetBackingStore, NULL }, [VIR_STORAGE_FILE_QCOW2] = { 0, "QFI", NULL, - LV_BIG_ENDIAN, 4, {2, 3}, + LV_BIG_ENDIAN, 4, 4, {2, 3}, QCOWX_HDR_IMAGE_SIZE, 8, 1, QCOW2_HDR_CRYPT, qcow2GetBackingStore, qcow2GetFeatures }, [VIR_STORAGE_FILE_QED] = { /* http://wiki.qemu.org/Features/QED */ 0, "QED", NULL, - LV_LITTLE_ENDIAN, -2, {0}, + LV_LITTLE_ENDIAN, -2, 0, {0}, QED_HDR_IMAGE_SIZE, 8, 1, -1, qedGetBackingStore, NULL }, [VIR_STORAGE_FILE_VMDK] = { 0, "KDMV", NULL, - LV_LITTLE_ENDIAN, 4, {1, 2}, + LV_LITTLE_ENDIAN, 4, 4, {1, 2}, 4+4+4, 8, 512, -1, vmdk4GetBackingStore, NULL }, }; @@ -635,13 +637,25 @@ virStorageFileMatchesVersion(int format, if (fileTypeInfo[format].versionOffset == -2) return true; - if ((fileTypeInfo[format].versionOffset + 4) > buflen) + if ((fileTypeInfo[format].versionOffset + + fileTypeInfo[format].versionSize) > buflen) return false; - if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN) - version = virReadBufInt32LE(buf + fileTypeInfo[format].versionOffset); - else - version = virReadBufInt32BE(buf + fileTypeInfo[format].versionOffset); + if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN) { + if (fileTypeInfo[format].versionSize == 4) + version = virReadBufInt32LE(buf + + fileTypeInfo[format].versionOffset); + else + version = virReadBufInt16LE(buf + + fileTypeInfo[format].versionOffset); + } else { + if (fileTypeInfo[format].versionSize == 4) + version = virReadBufInt32BE(buf + + fileTypeInfo[format].versionOffset); + else + version = virReadBufInt16BE(buf + + fileTypeInfo[format].versionOffset); + } for (i = 0; i < FILE_TYPE_VERSIONS_LAST && fileTypeInfo[format].versionNumbers[i]; -- 2.5.5

On 06/24/2016 09:28 AM, Peter Krempa wrote: So you would like to see: if (fileTypeInfo[format].versionSize == 0) return false; It wouldn't be because for all size = 0, the versionOffset == -1 or -2 we will have returned false much sooner. I *did* have a check at one time for size == 0, then return false, but took it out. I can replace it though

Add the ability to detect a luks encrypted device. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/util/virstoragefile.c | 24 ++++++++++++++++++++++-- src/util/virstoragefile.h | 1 + 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 37e9798..59927ea 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -63,7 +63,7 @@ VIR_ENUM_IMPL(virStorageFileFormat, "cloop", "dmg", "iso", "vpc", "vdi", /* Not direct file formats, but used for various drivers */ - "fat", "vhd", "ploop", + "fat", "vhd", "ploop", "luks", /* Formats with backing file below here */ "cow", "qcow", "qcow2", "qed", "vmdk") @@ -189,6 +189,13 @@ qedGetBackingStore(char **, int *, const char *, size_t); #define PLOOP_IMAGE_SIZE_OFFSET 36 #define PLOOP_SIZE_MULTIPLIER 512 +#define LUKS_HDR_MAGIC_LEN 6 +#define LUKS_HDR_VERSION_LEN 2 + +/* Format described by qemu commit id '3e308f20e' */ +#define LUKS_HDR_VERSION_OFFSET LUKS_HDR_MAGIC_LEN + + static struct FileTypeInfo const fileTypeInfo[] = { [VIR_STORAGE_FILE_NONE] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, -1, 0, {0}, 0, 0, 0, 0, NULL, NULL }, @@ -244,6 +251,13 @@ static struct FileTypeInfo const fileTypeInfo[] = { -2, 0, {0}, PLOOP_IMAGE_SIZE_OFFSET, 0, PLOOP_SIZE_MULTIPLIER, -1, NULL, NULL }, + /* Magic is 'L','U','K','S', 0xBA, 0xBE + * Set sizeOffset = -1 and let hypervisor handle */ + [VIR_STORAGE_FILE_LUKS] = { + 0, "\x4c\x55\x4b\x53\xba\xbe", NULL, + LV_BIG_ENDIAN, LUKS_HDR_VERSION_OFFSET, 2, {1}, + -1, 0, 0, -1, NULL, NULL + }, /* All formats with a backing store probe below here */ [VIR_STORAGE_FILE_COW] = { 0, "OOOM", NULL, @@ -626,7 +640,7 @@ virStorageFileMatchesVersion(int format, char *buf, size_t buflen) { - int version; + int version = 0; size_t i; /* Validate version number info */ @@ -838,6 +852,12 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta, goto cleanup; } + if (meta->format == VIR_STORAGE_FILE_LUKS) { + /* By definition, this is encrypted */ + if (!meta->encryption && VIR_ALLOC(meta->encryption) < 0) + goto cleanup; + } + VIR_FREE(meta->backingStoreRaw); if (fileTypeInfo[meta->format].getBackingStore != NULL) { int store = fileTypeInfo[meta->format].getBackingStore(&meta->backingStoreRaw, diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 71a8b3a..78beaf4 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -74,6 +74,7 @@ typedef enum { VIR_STORAGE_FILE_FAT, VIR_STORAGE_FILE_VHD, VIR_STORAGE_FILE_PLOOP, + VIR_STORAGE_FILE_LUKS, /* Not a format, but a marker: all formats below this point have * libvirt support for following a backing chain */ -- 2.5.5

Add parse and format of the luks/passphrase secret including tests for volume XML parsing. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- docs/formatsecret.html.in | 7 +++- docs/formatstorageencryption.html.in | 26 ++++++++++++- docs/schemas/storagecommon.rng | 2 + src/qemu/qemu_process.c | 6 +++ src/storage/storage_backend.c | 3 +- src/storage/storage_backend_fs.c | 7 +++- src/storage/storage_backend_gluster.c | 2 + src/util/virstorageencryption.c | 2 +- src/util/virstorageencryption.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml | 41 ++++++++++++++++++++ .../qemuxml2xmlout-luks-disks.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + tests/storagevolxml2xmlin/vol-luks.xml | 21 ++++++++++ tests/storagevolxml2xmlout/vol-luks.xml | 21 ++++++++++ tests/storagevolxml2xmltest.c | 1 + 15 files changed, 180 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disks.xml create mode 100644 tests/storagevolxml2xmlin/vol-luks.xml create mode 100644 tests/storagevolxml2xmlout/vol-luks.xml diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in index 79c4082..578c80e 100644 --- a/docs/formatsecret.html.in +++ b/docs/formatsecret.html.in @@ -248,7 +248,12 @@ <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. + 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.0.0</span>. The following is an example of a secret.xml file: </p> diff --git a/docs/formatstorageencryption.html.in b/docs/formatstorageencryption.html.in index fae86eb..3a08192 100644 --- a/docs/formatstorageencryption.html.in +++ b/docs/formatstorageencryption.html.in @@ -59,8 +59,20 @@ the <code>secret</code> element is not present during volume creation, a secret is automatically generated and attached to the volume. </p> + <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. + <span class="since">Since 2.0.0</span>. + </p> - <h2><a name="example">Example</a></h2> + <h2><a name="example">Examples</a></h2> <p> Here is a simple example, specifying use of the <code>qcow</code> format: @@ -70,5 +82,17 @@ &lt;encryption format='qcow'&gt; &lt;secret type='passphrase' uuid='c1f11a6d-8c5d-4a3e-ac7a-4e171c5e0d4a' /&gt; &lt;/encryption&gt;</pre> + + <p> + Here is a simple example, specifying use of the <code>luks</code> format + where it's assumed that a <code>secret</code> has been defined using a + <code>usage</code> element with a <code>id</code> of "luks_example": + </p> + <pre> + &lt;encryption format='luks'&gt; + &lt;secret type='passphrase' usage='luks_example'/&gt; + &lt;/encryption&gt; + </pre> + </body> </html> diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng index c5b71de..63b55b4 100644 --- a/docs/schemas/storagecommon.rng +++ b/docs/schemas/storagecommon.rng @@ -12,6 +12,7 @@ <choice> <value>default</value> <value>qcow</value> + <value>luks</value> </choice> </attribute> <zeroOrMore> @@ -81,6 +82,7 @@ <value>fat</value> <value>vhd</value> <value>ploop</value> + <value>luks</value> <ref name='storageFormatBacking'/> </choice> </define> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7d56ec8..d4c49eb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2411,6 +2411,12 @@ qemuProcessInitPasswords(virConnectPtr conn, !virDomainDiskGetSource(vm->def->disks[i])) continue; + if (vm->def->disks[i]->src->encryption->format != + VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT && + vm->def->disks[i]->src->encryption->format != + VIR_STORAGE_ENCRYPTION_FORMAT_QCOW) + continue; + VIR_FREE(secret); if (qemuProcessGetVolumeQcowPassphrase(conn, vm->def->disks[i], diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index fd76e21..89d5962 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1027,8 +1027,7 @@ virStorageBackendCreateQemuImgCheckEncryption(int format, } } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("qcow volume encryption unsupported with " - "volume format %s"), type); + _("volume encryption unsupported with format %s"), type); return -1; } diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index cac6bb8..bc4f238 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -157,7 +157,12 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, case VIR_STORAGE_FILE_QCOW2: (*encryption)->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; break; - default: + + case VIR_STORAGE_FILE_LUKS: + (*encryption)->format = VIR_STORAGE_ENCRYPTION_FORMAT_LUKS; + break; + + case VIR_STORAGE_ENCRYPTION_FORMAT_LAST: break; } diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 0085052..eda060d 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -321,6 +321,8 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, if (vol->target.format == VIR_STORAGE_FILE_QCOW || vol->target.format == VIR_STORAGE_FILE_QCOW2) vol->target.encryption->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; + if (vol->target.format == VIR_STORAGE_FILE_LUKS) + vol->target.encryption->format = VIR_STORAGE_ENCRYPTION_FORMAT_LUKS; } vol->target.features = meta->features; meta->features = NULL; diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c index afb44da..8575416 100644 --- a/src/util/virstorageencryption.c +++ b/src/util/virstorageencryption.c @@ -43,7 +43,7 @@ VIR_ENUM_IMPL(virStorageEncryptionSecret, VIR_ENUM_IMPL(virStorageEncryptionFormat, VIR_STORAGE_ENCRYPTION_FORMAT_LAST, - "default", "qcow") + "default", "qcow", "luks") static void virStorageEncryptionSecretFree(virStorageEncryptionSecretPtr secret) diff --git a/src/util/virstorageencryption.h b/src/util/virstorageencryption.h index c68c66e..5e1be3b 100644 --- a/src/util/virstorageencryption.h +++ b/src/util/virstorageencryption.h @@ -48,6 +48,7 @@ typedef enum { /* "default" is only valid for volume creation */ VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT = 0, VIR_STORAGE_ENCRYPTION_FORMAT_QCOW, /* Both qcow and qcow2 */ + VIR_STORAGE_ENCRYPTION_FORMAT_LUKS, VIR_STORAGE_ENCRYPTION_FORMAT_LAST, } virStorageEncryptionFormatType; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml new file mode 100644 index 0000000..00399cf --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml @@ -0,0 +1,41 @@ +<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'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disks.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disks.xml new file mode 100644 index 0000000..9ce15c0 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disks.xml @@ -0,0 +1,45 @@ +<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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index d045fd4..6c566b3 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -502,6 +502,7 @@ mymain(void) DO_TEST("encrypted-disk"); DO_TEST("encrypted-disk-usage"); + DO_TEST("luks-disks"); DO_TEST("memtune"); DO_TEST("memtune-unlimited"); DO_TEST("blkiotune"); diff --git a/tests/storagevolxml2xmlin/vol-luks.xml b/tests/storagevolxml2xmlin/vol-luks.xml new file mode 100644 index 0000000..eb4dc41 --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-luks.xml @@ -0,0 +1,21 @@ +<volume> + <name>LuksDemo.img</name> + <key>/var/lib/libvirt/images/LuksDemo.img</key> + <source> + </source> + <capacity unit="G">5</capacity> + <allocation>294912</allocation> + <target> + <path>/var/lib/libvirt/images/LuksDemo.img</path> + <format type='luks'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='luks'> + <secret type='passphrase' usage='mumblyfratz'/> + </encryption> + </target> +</volume> diff --git a/tests/storagevolxml2xmlout/vol-luks.xml b/tests/storagevolxml2xmlout/vol-luks.xml new file mode 100644 index 0000000..5b764b7 --- /dev/null +++ b/tests/storagevolxml2xmlout/vol-luks.xml @@ -0,0 +1,21 @@ +<volume type='file'> + <name>LuksDemo.img</name> + <key>/var/lib/libvirt/images/LuksDemo.img</key> + <source> + </source> + <capacity unit='bytes'>5368709120</capacity> + <allocation unit='bytes'>294912</allocation> + <target> + <path>/var/lib/libvirt/images/LuksDemo.img</path> + <format type='luks'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='luks'> + <secret type='passphrase' usage='mumblyfratz'/> + </encryption> + </target> +</volume> diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index f722452..a36a706 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -105,6 +105,7 @@ mymain(void) DO_TEST("pool-dir", "vol-qcow2-lazy"); DO_TEST("pool-dir", "vol-qcow2-0.10-lazy"); DO_TEST("pool-dir", "vol-qcow2-nobacking"); + DO_TEST("pool-dir", "vol-luks"); DO_TEST("pool-disk", "vol-partition"); DO_TEST("pool-logical", "vol-logical"); DO_TEST("pool-logical", "vol-logical-backing"); -- 2.5.5

On 06/24/2016 09:45 AM, Peter Krempa wrote: OK - I know this was a "more recent" change to not have extra output files, but it wasn't something I followed closely enough to make it part of my "routine" when adding new tests. Tks - John

For a luks device, allow the configuration of a specific cipher to be used for encrypting the volume. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- docs/formatstorageencryption.html.in | 81 +++++++++++++- docs/schemas/storagecommon.rng | 44 +++++++- src/conf/domain_conf.c | 11 ++ src/util/virstorageencryption.c | 124 +++++++++++++++++++++ src/util/virstorageencryption.h | 13 +++ .../qemuxml2argv-luks-disk-cipher.xml | 41 +++++++ .../qemuxml2xmlout-luks-disk-cipher.xml | 45 ++++++++ tests/qemuxml2xmltest.c | 1 + tests/storagevolxml2xmlin/vol-luks-cipher.xml | 23 ++++ tests/storagevolxml2xmlout/vol-luks-cipher.xml | 23 ++++ tests/storagevolxml2xmltest.c | 1 + 11 files changed, 401 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml create mode 100644 tests/storagevolxml2xmlin/vol-luks-cipher.xml create mode 100644 tests/storagevolxml2xmlout/vol-luks-cipher.xml diff --git a/docs/formatstorageencryption.html.in b/docs/formatstorageencryption.html.in index 3a08192..80111e3 100644 --- a/docs/formatstorageencryption.html.in +++ b/docs/formatstorageencryption.html.in @@ -71,6 +71,58 @@ be used as the passphrase to decrypt the volume. <span class="since">Since 2.0.0</span>. </p> + <p> + For volume creation, it is possible to specify the encryption + algorithm used to encrypt the luks volume. The following two + optional elements may be provided for that purpose. It is hypervisor + dependent as to which algorithms are supported. The default algorithm + for QEMU is 'aes-256-cbc' using 'essiv' for initialization vector + generation and 'sha256' hash algorithm for both the cipher and the + initialization vector generation. + </p> + + <dl> + <dt><code>cipher</code></dt> + <dd>This element describes the cipher algorithm to be used to either + encrypt or decrypt the luks volume. This element has the following + attributes: + <dl> + <dt><code>name</code></dt> + <dd>The name of the cipher algorithm used for data encryption, + such as 'aes', 'des', 'cast5', 'serpent', 'twofish', etc. + Support of the specific algorithm is hypervisor dependent.</dd> + <dt><code>size</code></dt> + <dd>The size of the cipher in bits, such as '256', '192', '128', + etc. Support of the specific size for a specific cipher is + hypervisor dependent.</dd> + <dt><code>mode</code></dt> + <dd>An optional cipher algorithm mode such as 'cbc', 'xts', + 'ecb', etc. Support of the specific cipher mode is + hypervisor dependent.</dd> + <dt><code>hash</code></dt> + <dd>An optional master key hash algorithm such as 'md5', 'sha1', + 'sha256', etc. Support of the specific hash algorithm is + hypervisor dependent.</dd> + </dl> + </dd> + <dt><code>ivgen</code></dt> + <dd>This optional element describes the initialization vector + generation algorithm used in conjunction with the + <code>cipher</code>. If the <code>cipher</code> is not provided, + then an error will be generated by the parser. + <dl> + <dt><code>name</code></dt> + <dd>The name of the algorithm, such as 'plain', 'plain64', + 'essiv', etc. Support of the specific algorithm is hypervisor + dependent.</dd> + <dt><code>hash</code></dt> + <dd>An optional hash algorithm such as 'md5', 'sha1', 'sha256', + etc. Support of the specific ivgen hash algorithm is hypervisor + dependent.</dd> + </dl> + </dd> + </dl> + <h2><a name="example">Examples</a></h2> @@ -84,9 +136,12 @@ &lt;/encryption&gt;</pre> <p> - Here is a simple example, specifying use of the <code>luks</code> format - where it's assumed that a <code>secret</code> has been defined using a - <code>usage</code> element with a <code>id</code> of "luks_example": + Assuming a <a href="formatsecret.html#luksUsageType"> + <code>luks secret</code></a> is already defined using a + <code>usage</code> element with an <code>id</code> of "luks_example", + 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; @@ -94,5 +149,25 @@ &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: + </p> + <pre> + &lt;volume&gt; + &lt;name&gt;twofish.luks&lt;/name&gt; + &lt;capacity unit='G'&gt;5&lt;/capacity&gt; + &lt;target&gt; + &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;cipher name='twofish' size='256' mode='cbc' hash='sha256'/&gt; + &lt;ivgen name='plain64' hash='sha256'/&gt; + &lt;/encryption&gt; + &lt;/target&gt; + &lt;/volume&gt; + </pre> + </body> </html> diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng index 63b55b4..316fbae 100644 --- a/docs/schemas/storagecommon.rng +++ b/docs/schemas/storagecommon.rng @@ -15,9 +15,19 @@ <value>luks</value> </choice> </attribute> - <zeroOrMore> - <ref name='secret'/> - </zeroOrMore> + <interleave> + <zeroOrMore> + <ref name='secret'/> + </zeroOrMore> + <optional> + <element name='cipher'> + <ref name='keycipher'/> + </element> + <element name='ivgen'> + <ref name='keyivgen'/> + </element> + </optional> + </interleave> </element> </define> @@ -136,4 +146,32 @@ </optional> </define> + <define name='keycipher'> + <attribute name='name'> + <text/> + </attribute> + <attribute name='size'> + <ref name="unsignedInt"/> + </attribute> + <optional> + <attribute name='mode'> + <text/> + </attribute> + <attribute name='hash'> + <text/> + </attribute> + </optional> + </define> + + <define name='keyivgen'> + <attribute name='name'> + <text/> + </attribute> + <optional> + <attribute name='hash'> + <text/> + </attribute> + </optional> + </define> + </grammar> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9443281..1c3bdc4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7857,6 +7857,17 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, def->startupPolicy = val; } + if (encryption) { + if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && + encryption->cipher.name) { + + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("supplying the <cipher> for a domain is " + "unnecessary")); + goto error; + } + } + def->dst = target; target = NULL; def->src->auth = authdef; diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c index 8575416..6544123 100644 --- a/src/util/virstorageencryption.c +++ b/src/util/virstorageencryption.c @@ -35,6 +35,7 @@ #include "viruuid.h" #include "virfile.h" #include "virsecret.h" +#include "virstring.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -46,6 +47,15 @@ VIR_ENUM_IMPL(virStorageEncryptionFormat, "default", "qcow", "luks") static void +virStorageEncryptionInfoDefFree(virStorageEncryptionInfoDefPtr def) +{ + VIR_FREE(def->name); + VIR_FREE(def->mode); + VIR_FREE(def->hash); +} + + +static void virStorageEncryptionSecretFree(virStorageEncryptionSecretPtr secret) { if (!secret) @@ -63,6 +73,8 @@ virStorageEncryptionFree(virStorageEncryptionPtr enc) for (i = 0; i < enc->nsecrets; i++) virStorageEncryptionSecretFree(enc->secrets[i]); + virStorageEncryptionInfoDefFree(&enc->cipher); + virStorageEncryptionInfoDefFree(&enc->ivgen); VIR_FREE(enc->secrets); VIR_FREE(enc); } @@ -80,6 +92,20 @@ virStorageEncryptionSecretCopy(const virStorageEncryptionSecret *src) return ret; } + +static int +virStorageEncryptionInfoDefCopy(const virStorageEncryptionInfoDef *src, + virStorageEncryptionInfoDefPtr dst) +{ + if (VIR_STRDUP(dst->name, src->name) < 0 || + VIR_STRDUP(dst->mode, src->mode) < 0 || + VIR_STRDUP(dst->hash, src->hash) < 0) + return -1; + + return 0; +} + + virStorageEncryptionPtr virStorageEncryptionCopy(const virStorageEncryption *src) { @@ -100,6 +126,12 @@ virStorageEncryptionCopy(const virStorageEncryption *src) goto error; } + if (virStorageEncryptionInfoDefCopy(&src->cipher, &ret->cipher) < 0) + goto error; + + if (virStorageEncryptionInfoDefCopy(&src->ivgen, &ret->ivgen) < 0) + goto error; + return ret; error: @@ -153,6 +185,51 @@ virStorageEncryptionSecretParse(xmlXPathContextPtr ctxt, return NULL; } + +static int +virStorageEncryptionInfoParse(xmlNodePtr info_node, + virStorageEncryptionInfoDefPtr info, + bool require_size) +{ + int ret = -1; + char *size_str = NULL; + + if (!(info->name = virXMLPropString(info_node, "name"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing secret key info name string")); + goto cleanup; + } + + /* Check for a size string - it's required for cipher, but not for ivgen + * if provided for ivgen then just ignore */ + if (require_size) { + if ((size_str = virXMLPropString(info_node, "size")) && + virStrToLong_uip(size_str, NULL, 10, &info->size) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot parse secret key info size string '%s'"), + size_str); + goto cleanup; + } + + if (!size_str) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("secret key info missing 'size' attribute")); + goto cleanup; + } + } + + /* Optional */ + info->mode = virXMLPropString(info_node, "mode"); + info->hash = virXMLPropString(info_node, "hash"); + + ret = 0; + + cleanup: + VIR_FREE(size_str); + return ret; +} + + static virStorageEncryptionPtr virStorageEncryptionParseXML(xmlXPathContextPtr ctxt) { @@ -196,6 +273,28 @@ virStorageEncryptionParseXML(xmlXPathContextPtr ctxt) VIR_FREE(nodes); } + if (ret->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { + xmlNodePtr tmpnode; + + if ((tmpnode = virXPathNode("./cipher[1]", ctxt))) { + if (virStorageEncryptionInfoParse(tmpnode, &ret->cipher, true) < 0) + goto cleanup; + } + + if ((tmpnode = virXPathNode("./ivgen[1]", ctxt))) { + /* If no cipher node, then fail */ + if (!ret->cipher.name) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing storage encryption cipher")); + goto cleanup; + } + + if (virStorageEncryptionInfoParse(tmpnode, &ret->ivgen, false) < 0) + goto cleanup; + } + } + + return ret; cleanup: @@ -250,6 +349,28 @@ virStorageEncryptionSecretFormat(virBufferPtr buf, return 0; } + +static void +virStorageEncryptionInfoDefFormat(virBufferPtr buf, + const virStorageEncryption *enc) +{ + virBufferAsprintf(buf, "<cipher name='%s' size='%u'", + enc->cipher.name, enc->cipher.size); + if (enc->cipher.mode) + virBufferAsprintf(buf, " mode='%s'", enc->cipher.mode); + if (enc->cipher.hash) + virBufferAsprintf(buf, " hash='%s'", enc->cipher.hash); + virBufferAddLit(buf, "/>\n"); + + if (enc->ivgen.name) { + virBufferAsprintf(buf, "<ivgen name='%s'", enc->ivgen.name); + if (enc->ivgen.hash) + virBufferAsprintf(buf, " hash='%s'", enc->ivgen.hash); + virBufferAddLit(buf, "/>\n"); + } +} + + int virStorageEncryptionFormat(virBufferPtr buf, virStorageEncryptionPtr enc) @@ -270,6 +391,9 @@ virStorageEncryptionFormat(virBufferPtr buf, return -1; } + if (enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && enc->cipher.name) + virStorageEncryptionInfoDefFormat(buf, enc); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</encryption>\n"); diff --git a/src/util/virstorageencryption.h b/src/util/virstorageencryption.h index 5e1be3b..34bba03 100644 --- a/src/util/virstorageencryption.h +++ b/src/util/virstorageencryption.h @@ -44,6 +44,16 @@ struct _virStorageEncryptionSecret { virSecretLookupTypeDef seclookupdef; }; +/* For a key type it's possible to dictate the cipher and if necessary iv */ +typedef struct _virStorageEncryptionInfoDef virStorageEncryptionInfoDef; +typedef virStorageEncryptionInfoDef *virStorageEncryptionInfoDefPtr; +struct _virStorageEncryptionInfoDef { + unsigned int size; + char *name; + char *mode; + char *hash; +}; + typedef enum { /* "default" is only valid for volume creation */ VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT = 0, @@ -61,6 +71,9 @@ struct _virStorageEncryption { size_t nsecrets; virStorageEncryptionSecretPtr *secrets; + + virStorageEncryptionInfoDef cipher; + virStorageEncryptionInfoDef ivgen; }; virStorageEncryptionPtr virStorageEncryptionCopy(const virStorageEncryption *src) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml b/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml new file mode 100644 index 0000000..00399cf --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml @@ -0,0 +1,41 @@ +<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'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml new file mode 100644 index 0000000..9ce15c0 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml @@ -0,0 +1,45 @@ +<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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 6c566b3..2056c03 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -503,6 +503,7 @@ 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 new file mode 100644 index 0000000..009246f --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-luks-cipher.xml @@ -0,0 +1,23 @@ +<volume> + <name>LuksDemo.img</name> + <key>/var/lib/libvirt/images/LuksDemo.img</key> + <source> + </source> + <capacity unit="G">5</capacity> + <allocation>294912</allocation> + <target> + <path>/var/lib/libvirt/images/LuksDemo.img</path> + <format type='luks'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='luks'> + <secret type='passphrase' usage='mumblyfratz'/> + <cipher name='serpent' size='256' mode='cbc' hash='sha256'/> + <ivgen name='plain64' hash='sha256'/> + </encryption> + </target> +</volume> diff --git a/tests/storagevolxml2xmlout/vol-luks-cipher.xml b/tests/storagevolxml2xmlout/vol-luks-cipher.xml new file mode 100644 index 0000000..9014849 --- /dev/null +++ b/tests/storagevolxml2xmlout/vol-luks-cipher.xml @@ -0,0 +1,23 @@ +<volume type='file'> + <name>LuksDemo.img</name> + <key>/var/lib/libvirt/images/LuksDemo.img</key> + <source> + </source> + <capacity unit='bytes'>5368709120</capacity> + <allocation unit='bytes'>294912</allocation> + <target> + <path>/var/lib/libvirt/images/LuksDemo.img</path> + <format type='luks'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='luks'> + <secret type='passphrase' usage='mumblyfratz'/> + <cipher name='serpent' size='256' mode='cbc' hash='sha256'/> + <ivgen name='plain64' hash='sha256'/> + </encryption> + </target> +</volume> diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index a36a706..db82bea 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -106,6 +106,7 @@ mymain(void) DO_TEST("pool-dir", "vol-qcow2-0.10-lazy"); DO_TEST("pool-dir", "vol-qcow2-nobacking"); DO_TEST("pool-dir", "vol-luks"); + DO_TEST("pool-dir", "vol-luks-cipher"); DO_TEST("pool-disk", "vol-partition"); DO_TEST("pool-logical", "vol-logical"); DO_TEST("pool-logical", "vol-logical-backing"); -- 2.5.5

On 06/24/2016 10:10 AM, Peter Krempa wrote: Does this wording suffice: The default algorithm used by the storage driver backend when using qemu-img to create the volume is 'aes-256-cbc' using 'essiv' for initialization vector OK Right. Geez and I thought it was more useful to reuse code... I'll adjust though, no big deal. It's all one larger happy structure now. OK same as before... Fixed John

On Thu, Jun 23, 2016 at 13:29:06 -0400, John Ferlan wrote: ACK without the noise.

On Thu, Jun 23, 2016 at 13:29:07 -0400, John Ferlan wrote: [...] [1] I still don't understand this step. Owner [1] of the file is able to add the write permission back anyways. ACK with either a very good explanation or the chmod gone.

It's just a constant "secret" string anyway Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/qemu/qemu_command.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4fdb410..9331e65 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -510,7 +510,6 @@ qemuNetworkDriveGetPort(int protocol, /** * qemuBuildSecretInfoProps: * @secinfo: pointer to the secret info object - * @type: returns a pointer to a character string for object name * @props: json properties to return * * Build the JSON properties for the secret info type. @@ -520,14 +519,11 @@ qemuNetworkDriveGetPort(int protocol, */ static int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo, - const char **type, virJSONValuePtr *propsret) { int ret = -1; char *keyid = NULL; - *type = "secret"; - if (!(keyid = qemuDomainGetMasterKeyAlias())) return -1; @@ -565,13 +561,12 @@ qemuBuildObjectSecretCommandLine(virCommandPtr cmd, { int ret = -1; virJSONValuePtr props = NULL; - const char *type; char *tmp = NULL; - if (qemuBuildSecretInfoProps(secinfo, &type, &props) < 0) + if (qemuBuildSecretInfoProps(secinfo, &props) < 0) return -1; - if (!(tmp = virQEMUBuildObjectCommandlineFromJSON(type, + if (!(tmp = virQEMUBuildObjectCommandlineFromJSON("secret", secinfo->s.aes.alias, props))) goto cleanup; -- 2.5.5

Commit id 'a1344f70a' added AES secret processing for RBD when starting up a guest. As such, when the hotplug code calls qemuDomainSecretDiskPrepare an AES secret could be added to the disk about to be hotplugged. If an AES secret was added, then the hotplug code would need to generate the secret object because qemuBuildDriveStr would add the "password-secret=" to the returned 'driveStr' rather than the base64 encoded password. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 44 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f695903..235cb73 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -310,6 +310,9 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, bool releaseaddr = false; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *src = virDomainDiskGetSource(disk); + virJSONValuePtr secobjProps = NULL; + qemuDomainDiskPrivatePtr diskPriv; + qemuDomainSecretInfoPtr secinfo; if (!disk->info.type) { if (qemuDomainMachineIsS390CCW(vm->def) && @@ -342,6 +345,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (qemuDomainSecretDiskPrepare(conn, priv, disk) < 0) goto error; + diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + secinfo = diskPriv->secinfo; + if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { + if (qemuBuildSecretInfoProps(secinfo, &secobjProps) < 0) + goto error; + } + if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) goto error; @@ -354,9 +364,15 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) goto error; - /* Attach the device - 2 step process */ + /* Attach the device - possible 3 step process */ qemuDomainObjEnterMonitor(driver, vm); + if (secobjProps && qemuMonitorAddObject(priv->mon, "secret", + secinfo->s.aes.alias, + secobjProps) < 0) + goto failaddobjsecret; + secobjProps = NULL; + if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) goto failadddrive; @@ -374,6 +390,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, ret = 0; cleanup: + virJSONValueFree(secobjProps); qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); VIR_FREE(drivestr); @@ -393,8 +410,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, } failadddrive: + if (secobjProps) + ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias)); + + failaddobjsecret: if (qemuDomainObjExitMonitor(driver, vm) < 0) releaseaddr = false; + secobjProps = NULL; /* qemuMonitorAddObject consumes props on failure too */ failexitmonitor: virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -3389,6 +3411,8 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, { int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(detach); + qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { virReportError(VIR_ERR_OPERATION_FAILED, @@ -3422,12 +3446,14 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, qemuDomainMarkDeviceForRemoval(vm, &detach->info); qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto cleanup; - virDomainAuditDisk(vm, detach->src, NULL, "detach", false); - goto cleanup; + if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { + if (qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias) < 0) + goto faildel; } + + if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) + goto faildel; + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; @@ -3437,6 +3463,12 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, cleanup: qemuDomainResetDeviceRemoval(vm); return ret; + + faildel: + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; + virDomainAuditDisk(vm, detach->src, NULL, "detach", false); + goto cleanup; } static int -- 2.5.5

On 06/24/2016 08:51 AM, Peter Krempa wrote: RemoveDiskDevice I thought was for SCSI and USB All those *still* confound me... DetachDisk/RemoveDisk... Right... I'll post a v3 shortly with changes that weren't officially ACK'd, but it seems they were close, plus a few that were ACK w/ changes, plus two new ones borne out of review, and of course these last 2 patches. Thanks for the persistence! John