[libvirt] Volume encryption missing pieces

Major changes since the previous submission: - Use one or two separate files per secret, using XML for metadata, instead of a single file for all secrets using a custom format. - Split <secret> XML handling from the local driver into secret_conf.[ch], use it when auto-generating secrets. - Use <usage type='volume'><volume/></usage> instead of <volume>. Also available at http://fedorapeople.org/gitweb?p=mitr/public_git/libvirt.git;a=summary .

* docs/formatsecret.html.in, docs/formatsecret.html: Document <usage type='volume'>, replacing stand-alone <volume>. * docs/schemas/secret.rng: Update schema to require <usage type='volume'> --- docs/formatsecret.html | 19 ++++++++++++++++--- docs/formatsecret.html.in | 25 ++++++++++++++++++++----- docs/schemas/secret.rng | 14 +++++++++++++- 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/docs/formatsecret.html b/docs/formatsecret.html index 929eb86..5fd6222 100644 --- a/docs/formatsecret.html +++ b/docs/formatsecret.html @@ -152,17 +152,30 @@ An unique identifier for this secret (not necessarily in the UUID format). If omitted when defining a new secret, a random UUID is generated. - </dd><dt><code>volume</code></dt><dd>Key of a volume this secret is associated with. It is safe to delete - the secret after the volume is deleted. </dd><dt><code>description</code></dt><dd>A human-readable description of the purpose of the secret. + </dd><dt><code>usage</code></dt><dd> + Specifies what this secret is used for. A mandatory + <code>type</code> attribute specifies the usage category, currently + only <code>volume</code> is defined. Specific usage categories are + described below. </dd></dl> + <h3>Usage type "volume"</h3> + <p> + This secret is associated with a volume, and it is safe to delete the + secret after the volume is deleted. The <code><usage + type='volume'></code> element must contain a + single <code>volume</code> element that specifies the key of the volume + this secret is associated with. + </p> <h2> <a name="example" id="example">Example</a> </h2> <pre> <secret ephemeral='no' private='yes'> - <volume>/var/lib/libvirt/images/mail.img</volume> <description>LUKS passphrase for the main hard drive of our mail server</description> + <usage type='volume'> + <volume>/var/lib/libvirt/images/mail.img</volume> + </usage> </secret></pre> </div> </div> diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in index 7471bf7..72022cb 100644 --- a/docs/formatsecret.html.in +++ b/docs/formatsecret.html.in @@ -32,21 +32,36 @@ format). If omitted when defining a new secret, a random UUID is generated. </dd> - <dt><code>volume</code></dt> - <dd>Key of a volume this secret is associated with. It is safe to delete - the secret after the volume is deleted. - </dd> <dt><code>description</code></dt> <dd>A human-readable description of the purpose of the secret. </dd> + <dt><code>usage</code></dt> + <dd> + Specifies what this secret is used for. A mandatory + <code>type</code> attribute specifies the usage category, currently + only <code>volume</code> is defined. Specific usage categories are + described below. + </dd> </dl> + <h3>Usage type "volume"</h3> + + <p> + This secret is associated with a volume, and it is safe to delete the + secret after the volume is deleted. The <code><usage + type='volume'></code> element must contain a + single <code>volume</code> element that specifies the key of the volume + this secret is associated with. + </p> + <h2><a name="example">Example</a></h2> <pre> <secret ephemeral='no' private='yes'> - <volume>/var/lib/libvirt/images/mail.img</volume> <description>LUKS passphrase for the main hard drive of our mail server</description> + <usage type='volume'> + <volume>/var/lib/libvirt/images/mail.img</volume> + </usage> </secret></pre> </body> </html> diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng index 05e04f2..8cfbd8f 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -34,11 +34,23 @@ </element> </optional> <optional> - <element name='volume'> + <element name='usage'> + <choice> + <ref name='usagevolume'> + </choice> <text/> </element> </optional> </interleave> </element> </define> + + <define name='usagevolume'> + <attribute name='type'> + <value>volume</value> + </attribute> + <element name='volume'> + <text/> + </element> + </define> </grammar> -- 1.6.2.5

On Mon, Sep 07, 2009 at 04:12:36PM +0200, Miloslav Trmač wrote:
* docs/formatsecret.html.in, docs/formatsecret.html: Document <usage type='volume'>, replacing stand-alone <volume>. * docs/schemas/secret.rng: Update schema to require <usage type='volume'>
Okay, sounds fine and if we need to change the format we need to do it before the release, ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Tue, Sep 08, 2009 at 10:43:30AM +0200, Daniel Veillard wrote:
On Mon, Sep 07, 2009 at 04:12:36PM +0200, Miloslav Trma?? wrote:
* docs/formatsecret.html.in, docs/formatsecret.html: Document <usage type='volume'>, replacing stand-alone <volume>. * docs/schemas/secret.rng: Update schema to require <usage type='volume'>
Okay, sounds fine and if we need to change the format we need to do it before the release,
ACK,
I pushed this change Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Add a VIR_SECRET_GET_VALUE_INTERNAL_CALL flag value, replacing the originally separate libvirt_internal_call parameter. The flag is used to differentiate external virSecretGetValue() calls from internal calls by libvirt drivers that need to use the secret even if it is private. * src/libvirt_internal.h (VIR_SECRET_GET_VALUE_FLAGS_MASK): New definition. * src/driver.h (VIR_SECRET_GET_VALUE_INTERNAL_CALL): New definition. * src/libvirt.c (virSecretGetValue): Don't allow the user to specify internal flags. * src/remote_internal.c (remoteSecretGetValue): Don't allow send internal flags over RPC. --- src/driver.h | 12 ++++++++++++ src/libvirt.c | 2 ++ src/libvirt_internal.h | 3 +++ src/remote_internal.c | 3 ++- 4 files changed, 19 insertions(+), 1 deletions(-) diff --git a/src/driver.h b/src/driver.h index 042c4af..28d7848 100644 --- a/src/driver.h +++ b/src/driver.h @@ -12,6 +12,8 @@ #include <libxml/uri.h> #include "internal.h" +#include "libvirt_internal.h" + /* * List of registered drivers numbers */ @@ -802,6 +804,16 @@ struct _virDeviceMonitor { virDrvNodeDeviceDestroy deviceDestroy; }; +enum { + /* This getValue call is inside libvirt, override the "private" flag. + This flag can not be set by outside callers. */ + VIR_SECRET_GET_VALUE_INTERNAL_CALL = 1 << 16 +}; + +/* Make sure ... INTERNAL_CALL can not be set by the caller */ +verify((VIR_SECRET_GET_VALUE_INTERNAL_CALL & + VIR_SECRET_GET_VALUE_FLAGS_MASK) == 0); + typedef virSecretPtr (*virDrvSecretLookupByUUIDString) (virConnectPtr conn, const char *uuid); diff --git a/src/libvirt.c b/src/libvirt.c index 45619e2..96d204c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -9066,6 +9066,8 @@ virSecretGetValue(virSecretPtr secret, size_t *value_size, unsigned int flags) goto error; } + flags &= VIR_SECRET_GET_VALUE_FLAGS_MASK; + if (conn->secretDriver != NULL && conn->secretDriver->getValue != NULL) { unsigned char *ret; diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 6976f34..60564d2 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -27,6 +27,9 @@ /* bits 16 and above of virDomainXMLFlags are for internal use */ #define VIR_DOMAIN_XML_FLAGS_MASK 0xffff +/* Bits 16 and above of virSecretGetValue flags are for internal use */ +#define VIR_SECRET_GET_VALUE_FLAGS_MASK 0xffff + #ifdef WITH_LIBVIRTD int virStateInitialize(int privileged); int virStateCleanup(void); diff --git a/src/remote_internal.c b/src/remote_internal.c index 3dd4609..745b128 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -76,6 +76,7 @@ # define AI_ADDRCONFIG 0 #endif +#include "libvirt_internal.h" #include "virterror_internal.h" #include "logging.h" #include "datatypes.h" @@ -6595,7 +6596,7 @@ remoteSecretGetValue (virSecretPtr secret, size_t *value_size, remoteDriverLock (priv); make_nonnull_secret (&args.secret, secret); - args.flags = flags; + args.flags = flags & VIR_SECRET_GET_VALUE_FLAGS_MASK; memset (&ret, 0, sizeof (ret)); if (call (secret->conn, priv, 0, REMOTE_PROC_SECRET_GET_VALUE, -- 1.6.2.5

On Mon, Sep 07, 2009 at 04:12:37PM +0200, Miloslav Trmač wrote:
Add a VIR_SECRET_GET_VALUE_INTERNAL_CALL flag value, replacing the originally separate libvirt_internal_call parameter. The flag is used to differentiate external virSecretGetValue() calls from internal calls by libvirt drivers that need to use the secret even if it is private.
* src/libvirt_internal.h (VIR_SECRET_GET_VALUE_FLAGS_MASK): New definition. * src/driver.h (VIR_SECRET_GET_VALUE_INTERNAL_CALL): New definition. * src/libvirt.c (virSecretGetValue): Don't allow the user to specify internal flags. * src/remote_internal.c (remoteSecretGetValue): Don't allow send internal flags over RPC.
Sounds fine,
+/* Make sure ... INTERNAL_CALL can not be set by the caller */ +verify((VIR_SECRET_GET_VALUE_INTERNAL_CALL & + VIR_SECRET_GET_VALUE_FLAGS_MASK) == 0);
??? what's that ? an assert at compile time ? I don't know that construct I would rather avoid it if it's not portable.
typedef virSecretPtr (*virDrvSecretLookupByUUIDString) (virConnectPtr conn, const char *uuid); diff --git a/src/libvirt.c b/src/libvirt.c index 45619e2..96d204c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -9066,6 +9066,8 @@ virSecretGetValue(virSecretPtr secret, size_t *value_size, unsigned int flags) goto error; }
+ flags &= VIR_SECRET_GET_VALUE_FLAGS_MASK; + if (conn->secretDriver != NULL && conn->secretDriver->getValue != NULL) { unsigned char *ret;
diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 6976f34..60564d2 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -27,6 +27,9 @@ /* bits 16 and above of virDomainXMLFlags are for internal use */ #define VIR_DOMAIN_XML_FLAGS_MASK 0xffff
+/* Bits 16 and above of virSecretGetValue flags are for internal use */ +#define VIR_SECRET_GET_VALUE_FLAGS_MASK 0xffff + #ifdef WITH_LIBVIRTD int virStateInitialize(int privileged); int virStateCleanup(void); diff --git a/src/remote_internal.c b/src/remote_internal.c index 3dd4609..745b128 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -76,6 +76,7 @@ # define AI_ADDRCONFIG 0 #endif
+#include "libvirt_internal.h" #include "virterror_internal.h" #include "logging.h" #include "datatypes.h" @@ -6595,7 +6596,7 @@ remoteSecretGetValue (virSecretPtr secret, size_t *value_size, remoteDriverLock (priv);
make_nonnull_secret (&args.secret, secret); - args.flags = flags; + args.flags = flags & VIR_SECRET_GET_VALUE_FLAGS_MASK;
memset (&ret, 0, sizeof (ret)); if (call (secret->conn, priv, 0, REMOTE_PROC_SECRET_GET_VALUE,
Except that verify() surprise, patch looks fine to me. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Mon, Sep 07, 2009 at 04:12:37PM +0200, Miloslav Trma?? wrote:
Add a VIR_SECRET_GET_VALUE_INTERNAL_CALL flag value, replacing the originally separate libvirt_internal_call parameter. The flag is used to differentiate external virSecretGetValue() calls from internal calls by libvirt drivers that need to use the secret even if it is private.
* src/libvirt_internal.h (VIR_SECRET_GET_VALUE_FLAGS_MASK): New definition. * src/driver.h (VIR_SECRET_GET_VALUE_INTERNAL_CALL): New definition. * src/libvirt.c (virSecretGetValue): Don't allow the user to specify internal flags. * src/remote_internal.c (remoteSecretGetValue): Don't allow send internal flags over RPC. --- src/driver.h | 12 ++++++++++++ src/libvirt.c | 2 ++ src/libvirt_internal.h | 3 +++ src/remote_internal.c | 3 ++- 4 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/src/driver.h b/src/driver.h index 042c4af..28d7848 100644 --- a/src/driver.h +++ b/src/driver.h @@ -12,6 +12,8 @@ #include <libxml/uri.h>
#include "internal.h" +#include "libvirt_internal.h" + /* * List of registered drivers numbers */ @@ -802,6 +804,16 @@ struct _virDeviceMonitor { virDrvNodeDeviceDestroy deviceDestroy; };
+enum { + /* This getValue call is inside libvirt, override the "private" flag. + This flag can not be set by outside callers. */ + VIR_SECRET_GET_VALUE_INTERNAL_CALL = 1 << 16 +}; + +/* Make sure ... INTERNAL_CALL can not be set by the caller */ +verify((VIR_SECRET_GET_VALUE_INTERNAL_CALL & + VIR_SECRET_GET_VALUE_FLAGS_MASK) == 0); + typedef virSecretPtr (*virDrvSecretLookupByUUIDString) (virConnectPtr conn, const char *uuid); diff --git a/src/libvirt.c b/src/libvirt.c index 45619e2..96d204c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -9066,6 +9066,8 @@ virSecretGetValue(virSecretPtr secret, size_t *value_size, unsigned int flags) goto error; }
+ flags &= VIR_SECRET_GET_VALUE_FLAGS_MASK; + if (conn->secretDriver != NULL && conn->secretDriver->getValue != NULL) { unsigned char *ret;
diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 6976f34..60564d2 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -27,6 +27,9 @@ /* bits 16 and above of virDomainXMLFlags are for internal use */ #define VIR_DOMAIN_XML_FLAGS_MASK 0xffff
+/* Bits 16 and above of virSecretGetValue flags are for internal use */ +#define VIR_SECRET_GET_VALUE_FLAGS_MASK 0xffff + #ifdef WITH_LIBVIRTD int virStateInitialize(int privileged); int virStateCleanup(void); diff --git a/src/remote_internal.c b/src/remote_internal.c index 3dd4609..745b128 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -76,6 +76,7 @@ # define AI_ADDRCONFIG 0 #endif
+#include "libvirt_internal.h" #include "virterror_internal.h" #include "logging.h" #include "datatypes.h" @@ -6595,7 +6596,7 @@ remoteSecretGetValue (virSecretPtr secret, size_t *value_size, remoteDriverLock (priv);
make_nonnull_secret (&args.secret, secret); - args.flags = flags; + args.flags = flags & VIR_SECRET_GET_VALUE_FLAGS_MASK;
memset (&ret, 0, sizeof (ret)); if (call (secret->conn, priv, 0, REMOTE_PROC_SECRET_GET_VALUE,
This is redundant, since libvirt.c has already masked the flags out by the time we get here I think the mask lives better in driver.h, so going to push the following slight re-arrangement instead (also moving the existing flag) Regards, Daniel commit 94a7da7de17a6355bcfc4ffce4b0c1053a5eb081 Author: Miloslav TrmaÄ <mitr@redhat.com> Date: Fri Aug 14 21:42:19 2009 +0200 Mask out flags used internally for virSecretGetValue Add a VIR_SECRET_GET_VALUE_INTERNAL_CALL flag value, replacing the originally separate libvirt_internal_call parameter. The flag is used to differentiate external virSecretGetValue() calls from internal calls by libvirt drivers that need to use the secret even if it is private. * src/libvirt_internal.h Remove VIR_DOMAIN_XML_FLAGS_MASK * src/driver.h Add VIR_SECRET_GET_VALUE_FLAGS_MASK constant and VIR_SECRET_GET_VALUE_INTERNAL_CALL. Re-add the VIR_DOMAIN_XML_FLAGS_MASK constant * src/libvirt.c (virSecretGetValue): Don't allow the user to specify internal flags. diff --git a/src/driver.h b/src/driver.h index 042c4af..447b7a2 100644 --- a/src/driver.h +++ b/src/driver.h @@ -802,6 +802,22 @@ struct _virDeviceMonitor { virDrvNodeDeviceDestroy deviceDestroy; }; +/* bits 16 and above of virDomainXMLFlags are for internal use */ +#define VIR_DOMAIN_XML_FLAGS_MASK 0xffff + +/* Bits 16 and above of virSecretGetValue flags are for internal use */ +#define VIR_SECRET_GET_VALUE_FLAGS_MASK 0xffff + +enum { + /* This getValue call is inside libvirt, override the "private" flag. + This flag can not be set by outside callers. */ + VIR_SECRET_GET_VALUE_INTERNAL_CALL = 1 << 16 +}; + +/* Make sure ... INTERNAL_CALL can not be set by the caller */ +verify((VIR_SECRET_GET_VALUE_INTERNAL_CALL & + VIR_SECRET_GET_VALUE_FLAGS_MASK) == 0); + typedef virSecretPtr (*virDrvSecretLookupByUUIDString) (virConnectPtr conn, const char *uuid); diff --git a/src/libvirt.c b/src/libvirt.c index 45619e2..96d204c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -9066,6 +9066,8 @@ virSecretGetValue(virSecretPtr secret, size_t *value_size, unsigned int flags) goto error; } + flags &= VIR_SECRET_GET_VALUE_FLAGS_MASK; + if (conn->secretDriver != NULL && conn->secretDriver->getValue != NULL) { unsigned char *ret; diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 6976f34..5913798 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -24,9 +24,6 @@ #include "internal.h" -/* bits 16 and above of virDomainXMLFlags are for internal use */ -#define VIR_DOMAIN_XML_FLAGS_MASK 0xffff - #ifdef WITH_LIBVIRTD int virStateInitialize(int privileged); int virStateCleanup(void); Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Add a <secret> XML handling API, separate from the local driver, to avoid manually generating XML in other parts of libvirt. * src/secret_conf.c, src/secret_conf.h: New files. * po/POTFILES.in, src/Makefile.am: Add secret_conf. --- po/POTFILES.in | 1 + src/Makefile.am | 7 +- src/libvirt_private.syms | 5 + src/secret_conf.c | 309 ++++++++++++++++++++++++++++++++++++++++++++++ src/secret_conf.h | 59 +++++++++ 5 files changed, 380 insertions(+), 1 deletions(-) create mode 100644 src/secret_conf.c create mode 100644 src/secret_conf.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 0a53dab..30cb08a 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -30,6 +30,7 @@ src/proxy_internal.c src/qemu_conf.c src/qemu_driver.c src/remote_internal.c +src/secret_conf.c src/security.c src/security_selinux.c src/storage_backend.c diff --git a/src/Makefile.am b/src/Makefile.am index 8e186a6..d6d9a6b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -91,6 +91,10 @@ STORAGE_CONF_SOURCES = \ INTERFACE_CONF_SOURCES = \ interface_conf.c interface_conf.h +# Secret driver generic impl APIs +SECRET_CONF_SOURCES = \ + secret_conf.h secret_conf.c + # The remote RPC driver, covering domains, storage, networks, etc REMOTE_DRIVER_SOURCES = \ gnutls_1_0_compat.h \ @@ -247,7 +251,8 @@ libvirt_driver_la_SOURCES = \ $(NETWORK_CONF_SOURCES) \ $(STORAGE_CONF_SOURCES) \ $(INTERFACE_CONF_SOURCES) \ - $(NODE_DEVICE_CONF_SOURCES) + $(NODE_DEVICE_CONF_SOURCES) \ + $(SECRET_CONF_SOURCES) libvirt_driver_la_CFLAGS = $(XEN_CFLAGS) $(NUMACTL_CFLAGS) libvirt_driver_la_LDFLAGS = $(XEN_LIBS) $(NUMACTL_LIBS) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index eec8be6..01d52d1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -300,6 +300,11 @@ qparam_get_query; qparam_query_parse; free_qparam_set; +# secret_conf.h +virSecretDefFree; +virSecretDefParseString; +virSecretDefParseFile; +virSecretDefFormat; # security.h virSecurityDriverVerify; diff --git a/src/secret_conf.c b/src/secret_conf.c new file mode 100644 index 0000000..c7b11a9 --- /dev/null +++ b/src/secret_conf.c @@ -0,0 +1,309 @@ +/* + * secret_conf.c: internal <secret> XML handling + * + * Copyright (C) 2009 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Red Hat Author: Miloslav Trmač <mitr@redhat.com> + */ + +#include <config.h> + +#include "internal.h" +#include "buf.h" +#include "datatypes.h" +#include "logging.h" +#include "memory.h" +#include "secret_conf.h" +#include "virterror_internal.h" +#include "util.h" +#include "xml.h" + +#define VIR_FROM_THIS VIR_FROM_SECRET + +VIR_ENUM_IMPL(virSecretUsageType, VIR_SECRET_USAGE_TYPE_LAST, "none", "volume") + +void +virSecretDefFree(virSecretDefPtr def) +{ + if (def == NULL) + return; + + VIR_FREE(def->id); + VIR_FREE(def->description); + switch (def->usage_type) { + case VIR_SECRET_USAGE_TYPE_NONE: + break; + + case VIR_SECRET_USAGE_TYPE_VOLUME: + VIR_FREE(def->usage.volume); + break; + + default: + VIR_ERROR(_("unexpected secret usage type %d"), def->usage_type); + break; + } + VIR_FREE(def); +} + +static int +virSecretDefParseUsage(virConnectPtr conn, xmlXPathContextPtr ctxt, + virSecretDefPtr def) +{ + char *type_str; + int type; + + type_str = virXPathString(conn, "string(./usage/@type)", ctxt); + if (type_str == NULL) { + virSecretReportError(conn, VIR_ERR_XML_ERROR, "%s", + _("unknown secret usage type")); + return -1; + } + type = virSecretUsageTypeTypeFromString(type_str); + if (type < 0) { + virSecretReportError(conn, VIR_ERR_XML_ERROR, + _("unknown secret usage type %s"), type_str); + VIR_FREE(type_str); + return -1; + } + VIR_FREE(type_str); + def->usage_type = type; + switch (def->usage_type) { + case VIR_SECRET_USAGE_TYPE_NONE: + break; + + case VIR_SECRET_USAGE_TYPE_VOLUME: + def->usage.volume = virXPathString(conn, "string(./usage/volume)", + ctxt); + break; + + default: + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unexpected secret usage type %d"), + def->usage_type); + return -1; + } + return 0; +} + +static virSecretDefPtr +secretXMLParseNode(virConnectPtr conn, xmlDocPtr xml, xmlNodePtr root) +{ + xmlXPathContextPtr ctxt = NULL; + virSecretDefPtr def = NULL, ret = NULL; + char *prop = NULL; + + if (!xmlStrEqual(root->name, BAD_CAST "secret")) { + virSecretReportError(conn, VIR_ERR_XML_ERROR, "%s", + _("incorrect root element")); + goto cleanup; + } + + ctxt = xmlXPathNewContext(xml); + if (ctxt == NULL) { + virReportOOMError(conn); + goto cleanup; + } + ctxt->node = root; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(conn); + goto cleanup; + } + + prop = virXPathString(conn, "string(./@ephemeral)", ctxt); + if (prop != NULL) { + if (STREQ(prop, "yes")) + def->ephemeral = 1; + else if (STREQ(prop, "no")) + def->ephemeral = 0; + else { + virSecretReportError(conn, VIR_ERR_XML_ERROR, "%s", + _("invalid value of 'ephemeral'")); + goto cleanup; + } + VIR_FREE(prop); + } + + prop = virXPathString(conn, "string(./@private)", ctxt); + if (prop != NULL) { + if (STREQ(prop, "yes")) + def->private = 1; + else if (STREQ(prop, "no")) + def->private = 0; + else { + virSecretReportError(conn, VIR_ERR_XML_ERROR, "%s", + _("invalid value of 'private'")); + goto cleanup; + } + VIR_FREE(prop); + } + + def->id = virXPathString(conn, "string(./uuid)", ctxt); + def->description = virXPathString(conn, "string(./description)", ctxt); + if (virXPathNode(conn, "./usage", ctxt) != NULL + && virSecretDefParseUsage(conn, ctxt, def) < 0) + goto cleanup; + ret = def; + def = NULL; + + cleanup: + VIR_FREE(prop); + virSecretDefFree(def); + xmlXPathFreeContext(ctxt); + return ret; +} + +/* Called from SAX on parsing errors in the XML. */ +static void +catchXMLError(void *ctx, const char *msg ATTRIBUTE_UNUSED, ...) +{ + xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr) ctx; + + if (ctxt) { + virConnectPtr conn = ctxt->_private; + + if (virGetLastError() == NULL && + ctxt->lastError.level == XML_ERR_FATAL && + ctxt->lastError.message != NULL) { + virSecretReportError(conn, VIR_ERR_XML_DETAIL, _("at line %d: %s"), + ctxt->lastError.line, ctxt->lastError.message); + } + } +} + +static virSecretDefPtr +virSecretDefParse(virConnectPtr conn, const char *xmlStr, const char *filename) +{ + xmlParserCtxtPtr pctxt; + xmlDocPtr xml = NULL; + xmlNodePtr root; + virSecretDefPtr ret = NULL; + + pctxt = xmlNewParserCtxt(); + if (pctxt == NULL || pctxt->sax == NULL) + goto cleanup; + pctxt->sax->error = catchXMLError; + pctxt->_private = conn; + + if (filename != NULL) + xml = xmlCtxtReadFile(pctxt, filename, NULL, + XML_PARSE_NOENT | XML_PARSE_NONET | + XML_PARSE_NOWARNING); + else + xml = xmlCtxtReadDoc(pctxt, BAD_CAST xmlStr, "secret.xml", NULL, + XML_PARSE_NOENT | XML_PARSE_NONET | + XML_PARSE_NOWARNING); + if (xml == NULL) { + if (conn->err.code == VIR_ERR_NONE) + virSecretReportError(conn, VIR_ERR_XML_ERROR, "%s", + _("failed to parse xml document")); + goto cleanup; + } + + root = xmlDocGetRootElement(xml); + if (root == NULL) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("missing root element")); + goto cleanup; + } + + ret = secretXMLParseNode(conn, xml, root); + + cleanup: + xmlFreeDoc(xml); + xmlFreeParserCtxt(pctxt); + return ret; +} + +virSecretDefPtr +virSecretDefParseString(virConnectPtr conn, const char *xmlStr) +{ + return virSecretDefParse(conn, xmlStr, NULL); +} + +virSecretDefPtr +virSecretDefParseFile(virConnectPtr conn, const char *filename) +{ + return virSecretDefParse(conn, NULL, filename); +} + +static int +virSecretDefFormatUsage(virConnectPtr conn, virBufferPtr buf, + const virSecretDefPtr def) +{ + const char *type; + + type = virSecretUsageTypeTypeToString(def->usage_type); + if (type == NULL) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unexpected secret usage type %d"), + def->usage_type); + return -1; + } + virBufferVSprintf(buf, " <usage type='%s'>\n", type); + switch (def->usage_type) { + case VIR_SECRET_USAGE_TYPE_NONE: + break; + + case VIR_SECRET_USAGE_TYPE_VOLUME: + if (def->usage.volume != NULL) + virBufferEscapeString(buf, " <volume>%s</volume>\n", + def->usage.volume); + break; + + default: + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unexpected secret usage type %d"), + def->usage_type); + return -1; + } + virBufferAddLit(buf, " </usage>\n"); + + return 0; +} + +char * +virSecretDefFormat(virConnectPtr conn, const virSecretDefPtr def) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *tmp; + + virBufferVSprintf(&buf, "<secret ephemeral='%s' private='%s'>\n", + def->ephemeral ? "yes" : "no", + def->private ? "yes" : "no"); + if (def->id != NULL) + virBufferEscapeString(&buf, " <uuid>%s</uuid>\n", def->id); + if (def->description != NULL) + virBufferEscapeString(&buf, " <description>%s</description>\n", + def->description); + if (def->usage_type != VIR_SECRET_USAGE_TYPE_NONE && + virSecretDefFormatUsage(conn, &buf, def) < 0) + goto error; + virBufferAddLit(&buf, "</secret>\n"); + + if (virBufferError(&buf)) + goto no_memory; + + return virBufferContentAndReset(&buf); + + no_memory: + virReportOOMError(conn); + error: + tmp = virBufferContentAndReset(&buf); + VIR_FREE(tmp); + return NULL; +} diff --git a/src/secret_conf.h b/src/secret_conf.h new file mode 100644 index 0000000..95beedf --- /dev/null +++ b/src/secret_conf.h @@ -0,0 +1,59 @@ +/* + * secret_conf.h: internal <secret> XML handling API + * + * Copyright (C) 2009 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Red Hat Author: Miloslav Trmač <mitr@redhat.com> + */ + +#ifndef __VIR_SECRET_CONF_H__ +#define __VIR_SECRET_CONF_H__ + +#include "internal.h" +#include "util.h" + +#define virSecretReportError(conn, code, fmt...) \ + virReportErrorHelper(conn, VIR_FROM_SECRET, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) + +enum virSecretUsageType { + VIR_SECRET_USAGE_TYPE_NONE = 0, /* default when zero-initialized */ + VIR_SECRET_USAGE_TYPE_VOLUME, + + VIR_SECRET_USAGE_TYPE_LAST +}; +VIR_ENUM_DECL(virSecretUsageType) + +typedef struct _virSecretDef virSecretDef; +typedef virSecretDef *virSecretDefPtr; +struct _virSecretDef { + unsigned ephemeral : 1; + unsigned private : 1; + char *id; /* May be NULL */ + char *description; /* May be NULL */ + int usage_type; + union { + char *volume; /* May be NULL */ + } usage; +}; + +void virSecretDefFree(virSecretDefPtr def); +virSecretDefPtr virSecretDefParseString(virConnectPtr conn, const char *xml); +virSecretDefPtr virSecretDefParseFile(virConnectPtr conn, const char *filename); +char *virSecretDefFormat(virConnectPtr conn, const virSecretDefPtr def); + +#endif -- 1.6.2.5

On Mon, Sep 07, 2009 at 04:12:38PM +0200, Miloslav Trmač wrote:
Add a <secret> XML handling API, separate from the local driver, to avoid manually generating XML in other parts of libvirt.
* src/secret_conf.c, src/secret_conf.h: New files. * po/POTFILES.in, src/Makefile.am: Add secret_conf. [...] +VIR_ENUM_IMPL(virSecretUsageType, VIR_SECRET_USAGE_TYPE_LAST, "none", "volume") + +void +virSecretDefFree(virSecretDefPtr def) +{ + if (def == NULL) + return; + + VIR_FREE(def->id); + VIR_FREE(def->description); + switch (def->usage_type) { + case VIR_SECRET_USAGE_TYPE_NONE: + break; + + case VIR_SECRET_USAGE_TYPE_VOLUME: + VIR_FREE(def->usage.volume); + break; + + default: + VIR_ERROR(_("unexpected secret usage type %d"), def->usage_type); + break;
Hum, since the virSecretDefPtr is allocated by our own code, it's probably better to remove the default so that the compiler can tell us we missed one enum case if new ones gets added.
+ } + VIR_FREE(def); +} + +static int +virSecretDefParseUsage(virConnectPtr conn, xmlXPathContextPtr ctxt, + virSecretDefPtr def) +{ + char *type_str; + int type; + + type_str = virXPathString(conn, "string(./usage/@type)", ctxt); + if (type_str == NULL) { + virSecretReportError(conn, VIR_ERR_XML_ERROR, "%s", + _("unknown secret usage type"));
_("missing secret usage type") would be more appropriate I guess
+ return -1; + } + type = virSecretUsageTypeTypeFromString(type_str); + if (type < 0) { + virSecretReportError(conn, VIR_ERR_XML_ERROR, + _("unknown secret usage type %s"), type_str); + VIR_FREE(type_str); + return -1; + } + VIR_FREE(type_str); + def->usage_type = type; + switch (def->usage_type) { + case VIR_SECRET_USAGE_TYPE_NONE: + break; + + case VIR_SECRET_USAGE_TYPE_VOLUME: + def->usage.volume = virXPathString(conn, "string(./usage/volume)", + ctxt); + break; + + default:
Again default: here means a mismatch between virSecretUsageTypeTypeFromString and this function, best handled statically IMHO.
+ virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unexpected secret usage type %d"), + def->usage_type); + return -1; + } + return 0; +} [...]
But this is mostly stylistic, ACK with the error message change Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

This implementation stores the secrets in an unencrypted text file, for simplicity in implementation and debugging. (Symmetric encryption, e.g. using gpgme, will not be difficult to add. Because the TLS private key used by libvirtd is stored unencrypted, encrypting the secrets file does not currently provide much additional security.) Changes since the fourth submission: - Rewrite storage mechanism to use one or two files per secret - Use the separate virSecretDef API for XML manipulation - Update for <usage type='volume'><volume/></usage> - Replace the "libvirt_internal_call" parameter of setValue() by VIR_SECRET_GET_VALUE_INTERNAL_CALL. - Fix comment in src/libvirt_private.syms * include/libvirt/virterror.h, src/virterror.c (VIR_ERR_NO_SECRET): New error number. * po/POTFILES.in, src/Makefile.am: Add secret_driver. * bootstrap: Use gnulib's base64 module. * src/secret_driver.c, src.secret_driver.h, src/libvirt_private.syms: Add local secret driver. * qemud/qemud.c (qemudInitialize): Use the local secret driver. --- bootstrap | 1 + include/libvirt/virterror.h | 1 + po/POTFILES.in | 1 + qemud/qemud.c | 3 + src/Makefile.am | 14 + src/libvirt_private.syms | 3 + src/secret_driver.c | 1060 +++++++++++++++++++++++++++++++++++++++++++ src/secret_driver.h | 28 ++ src/virterror.c | 5 + 9 files changed, 1116 insertions(+), 0 deletions(-) create mode 100644 src/secret_driver.c create mode 100644 src/secret_driver.h diff --git a/bootstrap b/bootstrap index 8b81e0e..885b299 100755 --- a/bootstrap +++ b/bootstrap @@ -65,6 +65,7 @@ gnulib_tool=$GNULIB_SRCDIR/gnulib-tool <$gnulib_tool || exit modules=' +base64 c-ctype close connect diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 62cad88..fa5cac4 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -169,6 +169,7 @@ typedef enum { VIR_ERR_MULTIPLE_INTERFACES, /* more than one matching interface found */ VIR_WAR_NO_SECRET, /* failed to start secret storage */ VIR_ERR_INVALID_SECRET, /* invalid secret */ + VIR_ERR_NO_SECRET, /* secret not found */ } virErrorNumber; /** diff --git a/po/POTFILES.in b/po/POTFILES.in index 30cb08a..5a23893 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -31,6 +31,7 @@ src/qemu_conf.c src/qemu_driver.c src/remote_internal.c src/secret_conf.c +src/secret_driver.c src/security.c src/security_selinux.c src/storage_backend.c diff --git a/qemud/qemud.c b/qemud/qemud.c index df275e6..00b9859 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -92,6 +92,7 @@ #ifdef WITH_NODE_DEVICES #include "node_device.h" #endif +#include "secret_driver.h" #endif @@ -814,6 +815,7 @@ static struct qemud_server *qemudInitialize(int sigread) { virDriverLoadModule("network"); virDriverLoadModule("storage"); virDriverLoadModule("nodedev"); + virDriverLoadModule("secret"); virDriverLoadModule("qemu"); virDriverLoadModule("lxc"); virDriverLoadModule("uml"); @@ -832,6 +834,7 @@ static struct qemud_server *qemudInitialize(int sigread) { (defined(HAVE_HAL) || defined(HAVE_DEVKIT)) nodedevRegister(); #endif + secretRegister(); #ifdef WITH_QEMU qemuRegister(); #endif diff --git a/src/Makefile.am b/src/Makefile.am index d6d9a6b..ceaae35 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -181,6 +181,9 @@ NETWORK_DRIVER_SOURCES = \ INTERFACE_DRIVER_SOURCES = \ interface_driver.h interface_driver.c +SECRET_DRIVER_SOURCES = \ + secret_driver.h secret_driver.c + # Storage backend specific impls STORAGE_DRIVER_SOURCES = \ storage_driver.h storage_driver.c \ @@ -454,6 +457,17 @@ endif libvirt_driver_interface_la_SOURCES = $(INTERFACE_DRIVER_SOURCES) endif +if WITH_DRIVER_MODULES +mod_LTLIBRARIES += libvirt_driver_secret.la +else +noinst_LTLIBRARIES += libvirt_driver_secret.la +libvirt_la_LIBADD += libvirt_driver_secret.la +endif +if WITH_DRIVER_MODULES +libvirt_driver_secret_la_LDFLAGS = -module -avoid-version +endif +libvirt_driver_secret_la_SOURCES = $(SECRET_DRIVER_SOURCES) + # Needed to keep automake quiet about conditionals libvirt_driver_storage_la_SOURCES = if WITH_STORAGE_DIR diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 01d52d1..cd22b96 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -306,6 +306,9 @@ virSecretDefParseString; virSecretDefParseFile; virSecretDefFormat; +# secret_driver.h +secretRegister; + # security.h virSecurityDriverVerify; virSecurityDriverStartup; diff --git a/src/secret_driver.c b/src/secret_driver.c new file mode 100644 index 0000000..40c3ede --- /dev/null +++ b/src/secret_driver.c @@ -0,0 +1,1060 @@ +/* + * secret_driver.c: local driver for secret manipulation API + * + * Copyright (C) 2009 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Red Hat Author: Miloslav Trmač <mitr@redhat.com> + */ + +#include <config.h> + +#include <dirent.h> +#include <fcntl.h> +#include <stdbool.h> +#include <string.h> +#include <sys/stat.h> +#include <unistd.h> + +#include "internal.h" +#include "base64.h" +#include "datatypes.h" +#include "driver.h" +#include "logging.h" +#include "memory.h" +#include "secret_conf.h" +#include "secret_driver.h" +#include "threads.h" +#include "util.h" +#include "uuid.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_SECRET + +enum { SECRET_MAX_XML_FILE = 10*1024*1024 }; + + /* Internal driver state */ + +typedef struct _virSecretEntry virSecretEntry; +typedef virSecretEntry *virSecretEntryPtr; +struct _virSecretEntry { + virSecretEntryPtr next; + virSecretDefPtr def; + unsigned char *value; /* May be NULL */ + size_t value_size; +}; + +typedef struct _virSecretDriverState virSecretDriverState; +typedef virSecretDriverState *virSecretDriverStatePtr; +struct _virSecretDriverState { + virMutex lock; + virSecretEntry *secrets; + char *directory; +}; + +static virSecretDriverStatePtr driverState; + +static void +secretDriverLock(virSecretDriverStatePtr driver) +{ + virMutexLock(&driver->lock); +} + +static void +secretDriverUnlock(virSecretDriverStatePtr driver) +{ + virMutexUnlock(&driver->lock); +} + +static virSecretEntryPtr +listUnlink(virSecretEntryPtr *pptr) +{ + virSecretEntryPtr secret; + + secret = *pptr; + *pptr = secret->next; + return secret; +} + +static void +listInsert(virSecretEntryPtr *pptr, virSecretEntryPtr secret) +{ + secret->next = *pptr; + *pptr = secret; +} + +static void +secretFree(virSecretEntryPtr secret) +{ + if (secret == NULL) + return; + + virSecretDefFree(secret->def); + if (secret->value != NULL) { + memset(secret->value, 0, secret->value_size); + VIR_FREE(secret->value); + } + VIR_FREE(secret); +} + +static virSecretEntryPtr * +secretFind(virSecretDriverStatePtr driver, const char *uuid) +{ + virSecretEntryPtr *pptr, s; + + for (pptr = &driver->secrets; *pptr != NULL; pptr = &s->next) { + s = *pptr; + if (STREQ(s->def->id, uuid)) + return pptr; + } + return NULL; +} + +static virSecretEntryPtr +secretCreate(virConnectPtr conn, virSecretDriverStatePtr driver, + const char *uuid) +{ + virSecretEntryPtr secret = NULL; + + if (VIR_ALLOC(secret) < 0 || VIR_ALLOC(secret->def)) + goto no_memory; + secret->def->id = strdup(uuid); + if (secret->def->id == NULL) + goto no_memory; + listInsert(&driver->secrets, secret); + return secret; + + no_memory: + virReportOOMError(conn); + secretFree(secret); + return NULL; +} + +static virSecretEntryPtr +secretFindOrCreate(virConnectPtr conn, virSecretDriverStatePtr driver, + const char *uuid, bool *created_new) +{ + virSecretEntryPtr *pptr, secret; + + pptr = secretFind(driver, uuid); + if (pptr != NULL) { + if (created_new != NULL) + *created_new = false; + return *pptr; + } + + secret = secretCreate(conn, driver, uuid); + if (secret != NULL && created_new != NULL) + *created_new = true; + return secret; +} + + /* Permament secret storage */ + +/* Secrets are stored in virSecretDriverStatePtr->directory. Each secret + has virSecretDef stored as XML in "$basename.xml". If a value of the + secret is defined, it is stored as base64 (with no formatting) in + "$basename.base64". "$basename" is in both cases the base64-encoded UUID. */ + +static int +replaceFile(virConnectPtr conn, const char *filename, void *data, size_t size) +{ + char *tmp_path = NULL; + int fd = -1, ret = -1; + + if (virAsprintf(&tmp_path, "%sXXXXXX", filename) < 0) { + virReportOOMError(conn); + goto cleanup; + } + fd = mkstemp (tmp_path); + if (fd == -1) { + virReportSystemError(conn, errno, _("mkstemp('%s') failed"), tmp_path); + goto cleanup; + } + if (fchmod(fd, S_IRUSR | S_IWUSR) != 0) { + virReportSystemError(conn, errno, _("fchmod('%s') failed"), tmp_path); + goto cleanup; + } + + ret = safewrite(fd, data, size); + if (ret < 0) { + virReportSystemError(conn, errno, _("error writing to '%s'"), + tmp_path); + goto cleanup; + } + if (close(fd) < 0) { + virReportSystemError(conn, errno, _("error closing '%s'"), tmp_path); + goto cleanup; + } + fd = -1; + + if (rename(tmp_path, filename) < 0) { + virReportSystemError(conn, errno, _("rename(%s, %s) failed"), tmp_path, + filename); + goto cleanup; + } + VIR_FREE(tmp_path); + ret = 0; + +cleanup: + if (fd != -1) + close(fd); + if (tmp_path != NULL) { + unlink(tmp_path); + VIR_FREE(tmp_path); + } + return ret; +} + +static char * +secretComputePath(virConnectPtr conn, virSecretDriverStatePtr driver, + const virSecretEntry *secret, const char *suffix) +{ + char *ret, *base64_id; + + base64_encode_alloc(secret->def->id, strlen(secret->def->id), &base64_id); + if (base64_id == NULL) { + virReportOOMError(conn); + return NULL; + } + + if (virAsprintf(&ret, "%s/%s%s", driver->directory, base64_id, suffix) < 0) + /* ret is NULL */ + virReportOOMError(conn); + VIR_FREE(base64_id); + return ret; +} + +static char * +secretXMLPath(virConnectPtr conn, virSecretDriverStatePtr driver, + const virSecretEntry *secret) +{ + return secretComputePath(conn, driver, secret, ".xml"); +} + +static char * +secretBase64Path(virConnectPtr conn, virSecretDriverStatePtr driver, + const virSecretEntry *secret) +{ + return secretComputePath(conn, driver, secret, ".base64"); +} + +static int +secretEnsureDirectory(virConnectPtr conn, virSecretDriverStatePtr driver) +{ + if (mkdir(driver->directory, S_IRWXU) < 0 && errno != EEXIST) { + virReportSystemError(conn, errno, _("cannot create '%s'"), + driver->directory); + return -1; + } + return 0; +} + +static int +secretSaveDef(virConnectPtr conn, virSecretDriverStatePtr driver, + const virSecretEntry *secret) +{ + char *filename = NULL, *xml = NULL; + int ret = -1; + + if (secretEnsureDirectory(conn, driver) < 0) + goto cleanup; + + filename = secretXMLPath(conn, driver, secret); + if (filename == NULL) + goto cleanup; + xml = virSecretDefFormat(conn, secret->def); + if (xml == NULL) + goto cleanup; + + if (replaceFile(conn, filename, xml, strlen(xml)) < 0) + goto cleanup; + + ret = 0; + +cleanup: + VIR_FREE(xml); + VIR_FREE(filename); + return ret; +} + +static int +secretSaveValue(virConnectPtr conn, virSecretDriverStatePtr driver, + const virSecretEntry *secret) +{ + char *filename = NULL, *base64 = NULL; + int ret = -1; + + if (secret->value == NULL) + return 0; + + if (secretEnsureDirectory(conn, driver) < 0) + goto cleanup; + + filename = secretBase64Path(conn, driver, secret); + if (filename == NULL) + goto cleanup; + base64_encode_alloc((const char *)secret->value, secret->value_size, + &base64); + if (base64 == NULL) { + virReportOOMError(conn); + goto cleanup; + } + + if (replaceFile(conn, filename, base64, strlen(base64)) < 0) + goto cleanup; + + ret = 0; + +cleanup: + VIR_FREE(base64); + VIR_FREE(filename); + return ret; +} + +static int +secretDeleteSaved(virConnectPtr conn, virSecretDriverStatePtr driver, + const virSecretEntry *secret) +{ + char *xml_filename = NULL, *value_filename = NULL; + int ret = -1; + + xml_filename = secretXMLPath(conn, driver, secret); + if (xml_filename == NULL) + goto cleanup; + value_filename = secretBase64Path(conn, driver, secret); + if (value_filename == NULL) + goto cleanup; + + if (unlink(xml_filename) < 0 && errno != ENOENT) + goto cleanup; + /* When the XML is missing, the rest may waste disk space, but the secret + won't be loaded again, so we have succeeded already. */ + ret = 0; + + (void)unlink(value_filename); + +cleanup: + VIR_FREE(value_filename); + VIR_FREE(xml_filename); + return ret; +} + +static int +secretLoadValidateUUID(virConnectPtr conn, virSecretDefPtr def, + const char *xml_basename) +{ + char *base64_id; + + if (def->id == NULL) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("<uuid> missing in secret description in '%s'"), + xml_basename); + return -1; + } + + base64_encode_alloc(def->id, strlen(def->id), &base64_id); + if (base64_id == NULL) { + virReportOOMError(conn); + return -1; + } + if (!virFileMatchesNameSuffix(xml_basename, base64_id, ".xml")) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("<uuid> does not match secret file name '%s'"), + xml_basename); + VIR_FREE(base64_id); + return -1; + } + VIR_FREE(base64_id); + + return 0; +} + +static int +secretLoadValue(virConnectPtr conn, virSecretDriverStatePtr driver, + virSecretEntryPtr secret) +{ + int ret = -1, fd = -1; + struct stat st; + char *filename = NULL, *contents = NULL, *value = NULL; + size_t value_size; + + filename = secretBase64Path(conn, driver, secret); + if (filename == NULL) + goto cleanup; + + fd = open(filename, O_RDONLY); + if (fd == -1) { + if (errno == ENOENT) { + ret = 0; + goto cleanup; + } + virReportSystemError (conn, errno, _("cannot open '%s'"), filename); + goto cleanup; + } + if (fstat(fd, &st) < 0) { + virReportSystemError (conn, errno, _("cannot stat '%s'"), filename); + goto cleanup; + } + if ((size_t)st.st_size != st.st_size) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("'%s' file does not fit in memory"), filename); + goto cleanup; + } + + if (VIR_ALLOC_N(contents, st.st_size) < 0) { + virReportOOMError(conn); + goto cleanup; + } + if (saferead(fd, contents, st.st_size) != st.st_size) { + virReportSystemError (conn, errno, _("cannot read '%s'"), filename); + goto cleanup; + } + close(fd); + fd = -1; + + if (!base64_decode_alloc(contents, st.st_size, &value, &value_size)) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("invalid base64 in '%s'"), filename); + goto cleanup; + } + if (value == NULL) { + virReportOOMError(conn); + goto cleanup; + } + + secret->value = (unsigned char *)value; + value = NULL; + secret->value_size = value_size; + + ret = 0; + +cleanup: + if (value != NULL) { + memset(value, 0, value_size); + VIR_FREE(value); + } + if (contents != NULL) { + memset(contents, 0, st.st_size); + VIR_FREE(contents); + } + if (fd != -1) + close(fd); + VIR_FREE(filename); + return ret; +} + +static virSecretEntryPtr +secretLoad(virConnectPtr conn, virSecretDriverStatePtr driver, + const char *xml_basename) +{ + virSecretDefPtr def; + virSecretEntryPtr secret = NULL, ret = NULL; + char *xml_filename; + + if (virAsprintf(&xml_filename, "%s/%s", driver->directory, + xml_basename) < 0) { + virReportOOMError(conn); + goto cleanup; + } + def = virSecretDefParseFile(conn, xml_filename); + if (def == NULL) + goto cleanup; + VIR_FREE(xml_filename); + + if (secretLoadValidateUUID(conn, def, xml_basename) < 0) + goto cleanup; + + if (VIR_ALLOC(secret) < 0) + goto cleanup; + secret->def = def; + def = NULL; + + if (secretLoadValue(conn, driver, secret) < 0) + goto cleanup; + + ret = secret; + secret = NULL; + +cleanup: + secretFree(secret); + virSecretDefFree(def); + VIR_FREE(xml_filename); + return ret; +} + +static int +loadSecrets(virConnectPtr conn, virSecretDriverStatePtr driver, + virSecretEntryPtr *dest) +{ + int ret = -1; + DIR *dir = NULL; + struct dirent *de; + virSecretEntryPtr list = NULL; + + dir = opendir(driver->directory); + if (dir == NULL) { + if (errno == ENOENT) + return 0; + virReportSystemError(conn, errno, _("cannot open '%s'"), + driver->directory); + goto cleanup; + } + while ((de = readdir(dir)) != NULL) { + virSecretEntryPtr secret; + + if (STREQ(de->d_name, ".") || STREQ(de->d_name, "..")) + continue; + if (!virFileHasSuffix(de->d_name, ".xml")) + continue; + + secret = secretLoad(conn, driver, de->d_name); + if (secret == NULL) { + virErrorPtr err = virGetLastError(); + + VIR_ERROR(_("Error reading secret: %s\n"), + err != NULL ? err->message: ""); + virResetError(err); + continue; + } + listInsert(&list, secret); + } + /* Ignore error reported by readdir(), if any. It's better to keep the + secrets we managed to find. */ + + while (list != NULL) { + virSecretEntryPtr s; + + s = listUnlink(&list); + listInsert(dest, s); + } + + ret = 0; + +cleanup: + while (list != NULL) { + virSecretEntryPtr s; + + s = listUnlink(&list); + secretFree(s); + } + if (dir != NULL) + closedir(dir); + return ret; +} + + /* Driver functions */ + +static virDrvOpenStatus +secretOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + int flags ATTRIBUTE_UNUSED) { + if (driverState == NULL) + return VIR_DRV_OPEN_DECLINED; + + conn->secretPrivateData = driverState; + return VIR_DRV_OPEN_SUCCESS; +} + +static int +secretClose(virConnectPtr conn) { + conn->secretPrivateData = NULL; + return 0; +} + +static int +secretNumOfSecrets(virConnectPtr conn) +{ + virSecretDriverStatePtr driver = conn->secretPrivateData; + int i; + virSecretEntryPtr secret; + + secretDriverLock(driver); + + i = 0; + for (secret = driver->secrets; secret != NULL; secret = secret->next) + i++; + + secretDriverUnlock(driver); + return i; +} + +static int +secretListSecrets(virConnectPtr conn, char **uuids, int maxuuids) +{ + virSecretDriverStatePtr driver = conn->secretPrivateData; + int i; + virSecretEntryPtr secret; + + memset(uuids, 0, maxuuids * sizeof(*uuids)); + + secretDriverLock(driver); + + i = 0; + for (secret = driver->secrets; secret != NULL; secret = secret->next) { + if (i == maxuuids) + break; + uuids[i] = strdup(secret->def->id); + if (uuids[i] == NULL) + goto cleanup; + i++; + } + + secretDriverUnlock(driver); + return i; + +cleanup: + secretDriverUnlock(driver); + + for (i = 0; i < maxuuids; i++) + VIR_FREE(uuids[i]); + + return -1; +} + +static virSecretPtr +secretLookupByUUIDString(virConnectPtr conn, const char *uuid) +{ + virSecretDriverStatePtr driver = conn->secretPrivateData; + virSecretPtr ret = NULL; + virSecretEntryPtr *pptr; + + secretDriverLock(driver); + + pptr = secretFind(driver, uuid); + if (pptr == NULL) { + virSecretReportError(conn, VIR_ERR_NO_SECRET, + _("no secret with matching id '%s'"), uuid); + goto cleanup; + } + + ret = virGetSecret(conn, (*pptr)->def->id); + +cleanup: + secretDriverUnlock(driver); + return ret; +} + +static char * +secretGenerateUUID(virConnectPtr conn, virSecretDriverStatePtr driver) +{ + char *uuid = NULL; + unsigned attempt; + + if (VIR_ALLOC_N(uuid, VIR_UUID_STRING_BUFLEN) < 0) { + virReportOOMError(conn); + goto error; + } + + for (attempt = 0; attempt < 65536; attempt++) { + unsigned char uuid_data[VIR_UUID_BUFLEN]; + + if (virUUIDGenerate(uuid_data) < 0) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to generate uuid")); + goto error; + } + virUUIDFormat(uuid_data, uuid); + if (secretFind(driver, uuid) == NULL) + return uuid; + } + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("too many conflicts when generating an uuid")); + goto error; + +error: + VIR_FREE(uuid); + return NULL; +} + +static virSecretPtr +secretDefineXML(virConnectPtr conn, const char *xml, + unsigned int flags ATTRIBUTE_UNUSED) +{ + virSecretDriverStatePtr driver = conn->secretPrivateData; + virSecretPtr ret = NULL; + virSecretEntryPtr secret; + virSecretDefPtr backup, new_attrs; + bool secret_is_new; + + new_attrs = virSecretDefParseString(conn, xml); + if (new_attrs == NULL) + return NULL; + + secretDriverLock(driver); + + if (new_attrs->id != NULL) + secret = secretFindOrCreate(conn, driver, new_attrs->id, + &secret_is_new); + else { + new_attrs->id = secretGenerateUUID(conn, driver); + if (new_attrs->id == NULL) + goto cleanup; + secret = secretCreate(conn, driver, new_attrs->id); + secret_is_new = true; + } + if (secret == NULL) + goto cleanup; + + /* Save old values of the attributes */ + backup = secret->def; + + if (backup->private && !new_attrs->private) { + virSecretReportError(conn, VIR_ERR_OPERATION_DENIED, "%s", + virErrorMsg(VIR_ERR_OPERATION_DENIED, NULL)); + goto cleanup; + } + + secret->def = new_attrs; + if (!new_attrs->ephemeral) { + if (backup->ephemeral) { + if (secretSaveValue(conn, driver, secret) < 0) + goto restore_backup; + } + if (secretSaveDef(conn, driver, secret) < 0) { + if (backup->ephemeral) { + char *filename; + + /* Undo the secretSaveValue() above; ignore errors */ + filename = secretBase64Path(conn, driver, secret); + if (filename != NULL) + (void)unlink(filename); + VIR_FREE(filename); + } + goto restore_backup; + } + } else if (!backup->ephemeral) { + if (secretDeleteSaved(conn, driver, secret) < 0) + goto restore_backup; + } + /* Saved succesfully - drop old values */ + new_attrs = NULL; + virSecretDefFree(backup); + + ret = virGetSecret(conn, secret->def->id); + goto cleanup; + +restore_backup: + /* Error - restore previous state and free new attributes */ + secret->def = backup; + if (secret_is_new) { + /* "secret" was added to the head of the list above */ + if (listUnlink(&driverState->secrets) != secret) + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("list of secrets is inconsistent")); + else + secretFree(secret); + } + +cleanup: + virSecretDefFree(new_attrs); + secretDriverUnlock(driver); + + return ret; +} + +static char * +secretGetXMLDesc(virSecretPtr obj, unsigned int flags ATTRIBUTE_UNUSED) +{ + virSecretDriverStatePtr driver = obj->conn->secretPrivateData; + char *ret = NULL; + virSecretEntryPtr *pptr; + + secretDriverLock(driver); + + pptr = secretFind(driver, obj->uuid); + if (pptr == NULL) { + virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, + _("no secret with matching id '%s'"), obj->uuid); + goto cleanup; + } + + ret = virSecretDefFormat(obj->conn, (*pptr)->def); + +cleanup: + secretDriverUnlock(driver); + + return ret; +} + +static int +secretSetValue(virSecretPtr obj, const unsigned char *value, + size_t value_size, unsigned int flags ATTRIBUTE_UNUSED) +{ + virSecretDriverStatePtr driver = obj->conn->secretPrivateData; + int ret = -1; + unsigned char *old_value, *new_value; + size_t old_value_size; + virSecretEntryPtr secret, *pptr; + + if (VIR_ALLOC_N(new_value, value_size) < 0) { + virReportOOMError(obj->conn); + return -1; + } + + secretDriverLock(driver); + + pptr = secretFind(driver, obj->uuid); + if (pptr == NULL) { + virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, + _("no secret with matching id '%s'"), obj->uuid); + goto cleanup; + } + secret = *pptr; + + old_value = secret->value; + old_value_size = secret->value_size; + + memcpy(new_value, value, value_size); + secret->value = new_value; + secret->value_size = value_size; + if (!secret->def->ephemeral) { + if (secretSaveValue(obj->conn, driver, secret) < 0) + goto restore_backup; + } + /* Saved succesfully - drop old value */ + if (old_value != NULL) { + memset(old_value, 0, old_value_size); + VIR_FREE(old_value); + } + new_value = NULL; + + ret = 0; + goto cleanup; + +restore_backup: + /* Error - restore previous state and free new value */ + secret->value = old_value; + secret->value_size = old_value_size; + memset(new_value, 0, value_size); + +cleanup: + secretDriverUnlock(driver); + + VIR_FREE(new_value); + + return ret; +} + +static unsigned char * +secretGetValue(virSecretPtr obj, size_t *value_size, unsigned int flags) +{ + virSecretDriverStatePtr driver = obj->conn->secretPrivateData; + unsigned char *ret = NULL; + virSecretEntryPtr *pptr, secret; + + secretDriverLock(driver); + + pptr = secretFind(driver, obj->uuid); + if (pptr == NULL) { + virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, + _("no secret with matching id '%s'"), obj->uuid); + goto cleanup; + } + secret = *pptr; + if (secret->value == NULL) { + virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, + _("secret '%s' does not have a value"), obj->uuid); + goto cleanup; + } + + if ((flags & VIR_SECRET_GET_VALUE_INTERNAL_CALL) == 0 && + secret->def->private) { + virSecretReportError(obj->conn, VIR_ERR_OPERATION_DENIED, "%s", + _("secret is private")); + goto cleanup; + } + + if (VIR_ALLOC_N(ret, secret->value_size) < 0) { + virReportOOMError(obj->conn); + goto cleanup; + } + memcpy(ret, secret->value, secret->value_size); + *value_size = secret->value_size; + +cleanup: + secretDriverUnlock(driver); + + return ret; +} + +static int +secretUndefine(virSecretPtr obj) +{ + virSecretDriverStatePtr driver = obj->conn->secretPrivateData; + int ret = -1; + virSecretEntryPtr *pptr, secret; + + secretDriverLock(driver); + + pptr = secretFind(driver, obj->uuid); + if (pptr == NULL) { + virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, + _("no secret with matching id '%s'"), obj->uuid); + goto cleanup; + } + + secret = listUnlink(pptr); + if (!secret->def->ephemeral) { + if (secretDeleteSaved(obj->conn, driver, secret) < 0) + goto restore_backup; + } + secretFree(secret); + + ret = 0; + goto cleanup; + +restore_backup: + /* This may change the order of secrets in the list. We don't care. */ + listInsert(&driver->secrets, secret); + +cleanup: + secretDriverUnlock(driver); + + return ret; +} + +static int +secretDriverCleanup(void) +{ + if (driverState == NULL) + return -1; + + secretDriverLock(driverState); + + while (driverState->secrets != NULL) { + virSecretEntryPtr s; + + s = listUnlink(&driverState->secrets); + secretFree(s); + } + VIR_FREE(driverState->directory); + + secretDriverUnlock(driverState); + virMutexDestroy(&driverState->lock); + VIR_FREE(driverState); + + return 0; +} + +static int +secretDriverStartup(int privileged) +{ + char *base = NULL; + + if (VIR_ALLOC(driverState) < 0) + return -1; + + if (virMutexInit(&driverState->lock) < 0) { + VIR_FREE(driverState); + return -1; + } + secretDriverLock(driverState); + + if (privileged) { + base = strdup(SYSCONF_DIR "/libvirt"); + if (base == NULL) + goto out_of_memory; + } else { + uid_t uid = geteuid(); + char *userdir = virGetUserDirectory(NULL, uid); + + if (!userdir) + goto error; + + if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) { + VIR_FREE(userdir); + goto out_of_memory; + } + VIR_FREE(userdir); + } + if (virAsprintf(&driverState->directory, "%s/secrets", base) == -1) + goto out_of_memory; + VIR_FREE(base); + + if (loadSecrets(NULL, driverState, &driverState->secrets) < 0) + goto error; + + secretDriverUnlock(driverState); + return 0; + + out_of_memory: + VIR_ERROR0(_("Out of memory initializing secrets")); + error: + VIR_FREE(base); + secretDriverUnlock(driverState); + secretDriverCleanup(); + return -1; +} + +static int +secretDriverReload(void) +{ + virSecretEntryPtr new_secrets = NULL; + + if (!driverState) + return -1; + + secretDriverLock(driverState); + + if (loadSecrets(NULL, driverState, &new_secrets) < 0) + goto end; + + /* Keep ephemeral secrets from current state. Discard non-ephemeral secrets + that were removed by the secrets directory. */ + while (driverState->secrets != NULL) { + virSecretEntryPtr s; + + s = listUnlink(&driverState->secrets); + if (s->def->ephemeral) + listInsert(&new_secrets, s); + else + secretFree(s); + } + driverState->secrets = new_secrets; + + end: + secretDriverUnlock(driverState); + return 0; +} + +static virSecretDriver secretDriver = { + .name = "secret", + .open = secretOpen, + .close = secretClose, + .numOfSecrets = secretNumOfSecrets, + .listSecrets = secretListSecrets, + .lookupByUUIDString = secretLookupByUUIDString, + .defineXML = secretDefineXML, + .getXMLDesc = secretGetXMLDesc, + .setValue = secretSetValue, + .getValue = secretGetValue, + .undefine = secretUndefine +}; + +static virStateDriver stateDriver = { + .initialize = secretDriverStartup, + .cleanup = secretDriverCleanup, + .reload = secretDriverReload, + .active = NULL /* All persistent state is immediately saved to disk */ +}; + +int +secretRegister(void) +{ + virRegisterSecretDriver(&secretDriver); + virRegisterStateDriver(&stateDriver); + return 0; +} diff --git a/src/secret_driver.h b/src/secret_driver.h new file mode 100644 index 0000000..0d0b80a --- /dev/null +++ b/src/secret_driver.h @@ -0,0 +1,28 @@ +/* + * secret_driver.h: local driver for secret manipulation API + * + * Copyright (C) 2009 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Red Hat Author: Miloslav Trmač <mitr@redhat.com> + */ + +#ifndef __VIR_SECRET_DRIVER_H__ +#define __VIR_SECRET_DRIVER_H__ + +int secretRegister(void); + +#endif /* __VIR_SECRET_DRIVER_H__ */ diff --git a/src/virterror.c b/src/virterror.c index 2a3cdaf..77b295c 100644 --- a/src/virterror.c +++ b/src/virterror.c @@ -1082,6 +1082,11 @@ virErrorMsg(virErrorNumber error, const char *info) errmsg = _("Invalid secret"); else errmsg = _("Invalid secret: %s"); + case VIR_ERR_NO_SECRET: + if (info == NULL) + errmsg = _("Secret not found"); + else + errmsg = _("Secret not found: %s"); break; } return (errmsg); -- 1.6.2.5

On Mon, Sep 07, 2009 at 04:12:39PM +0200, Miloslav Trmač wrote:
This implementation stores the secrets in an unencrypted text file, for simplicity in implementation and debugging.
(Symmetric encryption, e.g. using gpgme, will not be difficult to add. Because the TLS private key used by libvirtd is stored unencrypted, encrypting the secrets file does not currently provide much additional security.)
Changes since the fourth submission: - Rewrite storage mechanism to use one or two files per secret - Use the separate virSecretDef API for XML manipulation - Update for <usage type='volume'><volume/></usage> - Replace the "libvirt_internal_call" parameter of setValue() by VIR_SECRET_GET_VALUE_INTERNAL_CALL. - Fix comment in src/libvirt_private.syms
* include/libvirt/virterror.h, src/virterror.c (VIR_ERR_NO_SECRET): New error number. * po/POTFILES.in, src/Makefile.am: Add secret_driver. * bootstrap: Use gnulib's base64 module. * src/secret_driver.c, src.secret_driver.h, src/libvirt_private.syms: Add local secret driver. * qemud/qemud.c (qemudInitialize): Use the local secret driver. --- bootstrap | 1 + include/libvirt/virterror.h | 1 + po/POTFILES.in | 1 + qemud/qemud.c | 3 + src/Makefile.am | 14 + src/libvirt_private.syms | 3 + src/secret_driver.c | 1060 +++++++++++++++++++++++++++++++++++++++++++ src/secret_driver.h | 28 ++ src/virterror.c | 5 + 9 files changed, 1116 insertions(+), 0 deletions(-) create mode 100644 src/secret_driver.c create mode 100644 src/secret_driver.h
diff --git a/bootstrap b/bootstrap index 8b81e0e..885b299 100755 --- a/bootstrap +++ b/bootstrap @@ -65,6 +65,7 @@ gnulib_tool=$GNULIB_SRCDIR/gnulib-tool <$gnulib_tool || exit
modules=' +base64
Argh ! .gnulib/lib/base64.c "This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 2, or (at your option) any later version." Looks like GPL code not LGPL, I'm not sure we can actually use it, Jim any opinion ? We have some base64 code in libxml2 but it's not part of the exported APIs [...]
+ if (rename(tmp_path, filename) < 0) { + virReportSystemError(conn, errno, _("rename(%s, %s) failed"), tmp_path, + filename); + goto cleanup; + } + VIR_FREE(tmp_path); + ret = 0;
Okay, I had to convince myself this was correct but with the initialization of tmp_path fine :-) [...]
+static int +secretLoadValue(virConnectPtr conn, virSecretDriverStatePtr driver, + virSecretEntryPtr secret) +{ + int ret = -1, fd = -1; + struct stat st; + char *filename = NULL, *contents = NULL, *value = NULL; + size_t value_size; + + filename = secretBase64Path(conn, driver, secret); + if (filename == NULL) + goto cleanup; + + fd = open(filename, O_RDONLY); + if (fd == -1) { + if (errno == ENOENT) { + ret = 0; + goto cleanup; + } + virReportSystemError (conn, errno, _("cannot open '%s'"), filename); + goto cleanup; + } + if (fstat(fd, &st) < 0) { + virReportSystemError (conn, errno, _("cannot stat '%s'"), filename); + goto cleanup; + } + if ((size_t)st.st_size != st.st_size) {
shouldn't we chaeck against SECRET_MAX_XML_FILE instead ?
+ virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("'%s' file does not fit in memory"), filename); + goto cleanup; + }
ACK once we have cleared the status of the base64 gnulib dependancy Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

Daniel Veillard wrote:
On Mon, Sep 07, 2009 at 04:12:39PM +0200, Miloslav Trmač wrote:
This implementation stores the secrets in an unencrypted text file, for simplicity in implementation and debugging.
(Symmetric encryption, e.g. using gpgme, will not be difficult to add. Because the TLS private key used by libvirtd is stored unencrypted, encrypting the secrets file does not currently provide much additional security.)
Changes since the fourth submission: - Rewrite storage mechanism to use one or two files per secret - Use the separate virSecretDef API for XML manipulation - Update for <usage type='volume'><volume/></usage> - Replace the "libvirt_internal_call" parameter of setValue() by VIR_SECRET_GET_VALUE_INTERNAL_CALL. - Fix comment in src/libvirt_private.syms
* include/libvirt/virterror.h, src/virterror.c (VIR_ERR_NO_SECRET): New error number. * po/POTFILES.in, src/Makefile.am: Add secret_driver. * bootstrap: Use gnulib's base64 module. * src/secret_driver.c, src.secret_driver.h, src/libvirt_private.syms: Add local secret driver. * qemud/qemud.c (qemudInitialize): Use the local secret driver. --- bootstrap | 1 + include/libvirt/virterror.h | 1 + po/POTFILES.in | 1 + qemud/qemud.c | 3 + src/Makefile.am | 14 + src/libvirt_private.syms | 3 + src/secret_driver.c | 1060 +++++++++++++++++++++++++++++++++++++++++++ src/secret_driver.h | 28 ++ src/virterror.c | 5 + 9 files changed, 1116 insertions(+), 0 deletions(-) create mode 100644 src/secret_driver.c create mode 100644 src/secret_driver.h
diff --git a/bootstrap b/bootstrap index 8b81e0e..885b299 100755 --- a/bootstrap +++ b/bootstrap @@ -65,6 +65,7 @@ gnulib_tool=$GNULIB_SRCDIR/gnulib-tool <$gnulib_tool || exit
modules=' +base64
Argh ! .gnulib/lib/base64.c "This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 2, or (at your option) any later version."
Looks like GPL code not LGPL, I'm not sure we can actually use it, Jim any opinion ?
We have some base64 code in libxml2 but it's not part of the exported APIs
That module's license is LGPLv2+, so we're fine. $ grep -A1 Lic .gnulib/modules/base64 License: LGPLv2+ Yes, it can definitely be confusing if you don't realize that the module file is where the *real* license is recorded, and the comment in the code is automatically substituted by gnulib-tool when it imports the code into your project. So best is to look at files (post-bootstrap) in gnulib/, not .gnulib/. Note that any patch that adds a module and passes the check in ./bootstrap should be fine, because we've told gnulib-tool to enforce libvirt's LGPLv2+ requirement: $ grep -A1 '^$gnulib_to' bootstrap $gnulib_tool \ --lgpl=2 \

On Tue, Sep 08, 2009 at 12:39:44PM +0200, Jim Meyering wrote:
Daniel Veillard wrote:
On Mon, Sep 07, 2009 at 04:12:39PM +0200, Miloslav Trmač wrote:
This implementation stores the secrets in an unencrypted text file, for simplicity in implementation and debugging.
(Symmetric encryption, e.g. using gpgme, will not be difficult to add. Because the TLS private key used by libvirtd is stored unencrypted, encrypting the secrets file does not currently provide much additional security.)
Changes since the fourth submission: - Rewrite storage mechanism to use one or two files per secret - Use the separate virSecretDef API for XML manipulation - Update for <usage type='volume'><volume/></usage> - Replace the "libvirt_internal_call" parameter of setValue() by VIR_SECRET_GET_VALUE_INTERNAL_CALL. - Fix comment in src/libvirt_private.syms
* include/libvirt/virterror.h, src/virterror.c (VIR_ERR_NO_SECRET): New error number. * po/POTFILES.in, src/Makefile.am: Add secret_driver. * bootstrap: Use gnulib's base64 module. * src/secret_driver.c, src.secret_driver.h, src/libvirt_private.syms: Add local secret driver. * qemud/qemud.c (qemudInitialize): Use the local secret driver. --- bootstrap | 1 + include/libvirt/virterror.h | 1 + po/POTFILES.in | 1 + qemud/qemud.c | 3 + src/Makefile.am | 14 + src/libvirt_private.syms | 3 + src/secret_driver.c | 1060 +++++++++++++++++++++++++++++++++++++++++++ src/secret_driver.h | 28 ++ src/virterror.c | 5 + 9 files changed, 1116 insertions(+), 0 deletions(-) create mode 100644 src/secret_driver.c create mode 100644 src/secret_driver.h
diff --git a/bootstrap b/bootstrap index 8b81e0e..885b299 100755 --- a/bootstrap +++ b/bootstrap @@ -65,6 +65,7 @@ gnulib_tool=$GNULIB_SRCDIR/gnulib-tool <$gnulib_tool || exit
modules=' +base64
Argh ! .gnulib/lib/base64.c "This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 2, or (at your option) any later version."
Looks like GPL code not LGPL, I'm not sure we can actually use it, Jim any opinion ?
We have some base64 code in libxml2 but it's not part of the exported APIs
That module's license is LGPLv2+, so we're fine.
$ grep -A1 Lic .gnulib/modules/base64 License: LGPLv2+
Yes, it can definitely be confusing if you don't realize that the module file is where the *real* license is recorded, and the comment in the code is automatically substituted by gnulib-tool when it imports the code into your project. So best is to look at files (post-bootstrap) in gnulib/, not .gnulib/.
okay, but this weren't imported yet :-)
Note that any patch that adds a module and passes the check in ./bootstrap should be fine, because we've told gnulib-tool to enforce libvirt's LGPLv2+ requirement:
$ grep -A1 '^$gnulib_to' bootstrap $gnulib_tool \ --lgpl=2 \
Okay, cool :-) Thanks for clearing the issue ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

* src/virsh.c: Add virsh commands. * docs/virsh.pod, virsh.1: Update documentation. --- docs/virsh.pod | 43 ++++++++ src/virsh.c | 323 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ virsh.1 | 34 ++++++- 3 files changed, 399 insertions(+), 1 deletions(-) diff --git a/docs/virsh.pod b/docs/virsh.pod index 10bb991..55ec64a 100644 --- a/docs/virsh.pod +++ b/docs/virsh.pod @@ -543,6 +543,49 @@ Convert a network name to network UUID. =back +=head1 SECRET COMMMANDS + +The following commands manipulate "secrets" (e.g. passwords, passphrases and +encryption keys). Libvirt can store secrets independently from their use, and +other objects (e.g. volumes or domains) can refer to the secrets for encryption +or possibly other uses. Secrets are identified using an UUID. See +L<http://libvirt.org/formatsecret.html> for documentation of the XML format +used to represent properties of secrets. + +=over 4 + +=item B<secret-define> I<file> + +Create a secret with the properties specified in I<file>, with no associated +secret value. If I<file> does not specify a UUID, choose one automatically. +If I<file> specifies an UUID of an existing secret, replace its properties by +properties defined in I<file>, without affecting the secret value. + +=item B<secret-dumpxml> I<secret> + +Output properties of I<secret> (specified by its UUID) as an XML dump to stdout. + +=item B<secret-set-value> I<secret> I<base64> + +Set the value associated with I<secret> (specified by its UUID) to the value +Base64-encoded value I<base64>. + +=item B<secret-get-value> I<secret> + +Output the value associated with I<secret> (specified by its UUID) to stdout, +encoded using Base64. + +=item B<secret-undefine> I<secret> + +Delete a I<secret> (specified by its UUID), including the associated value, if +any. + +=item B<secret-list> + +Output a list of UUIDs of known secrets to stdout. + +=back + =head1 ENVIRONMENT The following environment variables can be set to alter the behaviour diff --git a/src/virsh.c b/src/virsh.c index 910d860..9dc8857 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -41,6 +41,7 @@ #endif #include "internal.h" +#include "base64.h" #include "buf.h" #include "console.h" #include "util.h" @@ -271,6 +272,9 @@ static virStorageVolPtr vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd, vshCommandOptVolBy(_ctl, _cmd, _optname, _pooloptname, _name, \ VSH_BYUUID|VSH_BYNAME) +static virSecretPtr vshCommandOptSecret(vshControl *ctl, const vshCmd *cmd, + char **name); + static void vshPrintExtra(vshControl *ctl, const char *format, ...) ATTRIBUTE_FMT_PRINTF(2, 3); static void vshDebug(vshControl *ctl, int level, const char *format, ...) @@ -5249,9 +5253,291 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd) } +/* + * "secret-define" command + */ +static const vshCmdInfo info_secret_define[] = { + {"help", gettext_noop("define or modify a secret from an XML file")}, + {"desc", gettext_noop("Define or modify a secret.")}, + {NULL, NULL} +}; +static const vshCmdOptDef opts_secret_define[] = { + {"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("file containing secret attributes in XML")}, + {NULL, 0, 0, NULL} +}; +static int +cmdSecretDefine(vshControl *ctl, const vshCmd *cmd) +{ + char *from, *buffer, *uuid; + virSecretPtr res; + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + from = vshCommandOptString(cmd, "file", NULL); + if (!from) + return FALSE; + + if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) + return FALSE; + + res = virSecretDefineXML(ctl->conn, buffer, 0); + free (buffer); + + if (res == NULL) { + vshError(ctl, FALSE, _("Failed to set attributes from %s"), from); + return FALSE; + } + uuid = virSecretGetUUIDString(res); + if (uuid == NULL) { + vshError(ctl, FALSE, "%s", + _("Failed to get UUID of created secret")); + virSecretFree(res); + return FALSE; + } + vshPrint(ctl, _("Secret %s created\n"), uuid); + free(uuid); + virSecretFree(res); + return TRUE; +} + +/* + * "secret-dumpxml" command + */ +static const vshCmdInfo info_secret_dumpxml[] = { + {"help", gettext_noop("secret attributes in XML")}, + {"desc", gettext_noop("Output attributes of a secret as an XML dump to stdout.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_secret_dumpxml[] = { + {"secret", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("secret UUID")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdSecretDumpXML(vshControl *ctl, const vshCmd *cmd) +{ + virSecretPtr secret; + int ret = FALSE; + char *xml; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + secret = vshCommandOptSecret(ctl, cmd, NULL); + if (secret == NULL) + return FALSE; + + xml = virSecretGetXMLDesc(secret, 0); + if (xml == NULL) + goto cleanup; + printf("%s", xml); + free(xml); + ret = TRUE; + +cleanup: + virSecretFree(secret); + return ret; +} + +/* + * "secret-set-value" command + */ +static const vshCmdInfo info_secret_set_value[] = { + {"help", gettext_noop("set a secret value")}, + {"desc", gettext_noop("Set a secret value.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_secret_set_value[] = { + {"secret", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("secret UUID")}, + {"base64", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("base64-encoded secret value")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd) +{ + virSecretPtr secret; + size_t value_size; + char *base64, *value; + int found, res, ret = FALSE; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + secret = vshCommandOptSecret(ctl, cmd, NULL); + if (secret == NULL) + return FALSE; + + base64 = vshCommandOptString(cmd, "base64", &found); + if (!base64) + goto cleanup; + + if (!base64_decode_alloc(base64, strlen(base64), &value, &value_size)) { + vshError(ctl, FALSE, _("Invalid base64 data")); + goto cleanup; + } + if (value == NULL) { + vshError(ctl, FALSE, "%s", _("Failed to allocate memory")); + return FALSE; + } + + res = virSecretSetValue(secret, (unsigned char *)value, value_size, 0); + memset(value, 0, value_size); + free (value); + + if (res != 0) { + vshError(ctl, FALSE, "%s", _("Failed to set secret value")); + goto cleanup; + } + vshPrint(ctl, "%s", _("Secret value set\n")); + ret = TRUE; + +cleanup: + virSecretFree(secret); + return ret; +} + +/* + * "secret-get-value" command + */ +static const vshCmdInfo info_secret_get_value[] = { + {"help", gettext_noop("Output a secret value")}, + {"desc", gettext_noop("Output a secret value to stdout.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_secret_get_value[] = { + {"secret", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("secret UUID")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdSecretGetValue(vshControl *ctl, const vshCmd *cmd) +{ + virSecretPtr secret; + char *base64; + unsigned char *value; + size_t value_size; + int ret = FALSE; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + secret = vshCommandOptSecret(ctl, cmd, NULL); + if (secret == NULL) + return FALSE; + + value = virSecretGetValue(secret, &value_size, 0); + if (value == NULL) + goto cleanup; + + base64_encode_alloc((char *)value, value_size, &base64); + memset(value, 0, value_size); + free(value); + + if (base64 == NULL) { + vshError(ctl, FALSE, "%s", _("Failed to allocate memory")); + goto cleanup; + } + printf("%s", base64); + memset(base64, 0, strlen(base64)); + free(base64); + ret = TRUE; + +cleanup: + virSecretFree(secret); + return ret; +} + +/* + * "secret-undefine" command + */ +static const vshCmdInfo info_secret_undefine[] = { + {"help", gettext_noop("undefine a secret")}, + {"desc", gettext_noop("Undefine a secret.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_secret_undefine[] = { + {"secret", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("secret UUID")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdSecretUndefine(vshControl *ctl, const vshCmd *cmd) +{ + virSecretPtr secret; + int ret = FALSE; + char *uuid; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + secret = vshCommandOptSecret(ctl, cmd, &uuid); + if (secret == NULL) + return FALSE; + + if (virSecretUndefine(secret) < 0) { + vshError(ctl, FALSE, _("Failed to delete secret %s"), uuid); + goto cleanup; + } + vshPrint(ctl, _("Secret %s deleted\n"), uuid); + ret = TRUE; + +cleanup: + virSecretFree(secret); + return ret; +} + +/* + * "secret-list" command + */ +static const vshCmdInfo info_secret_list[] = { + {"help", gettext_noop("list secrets")}, + {"desc", gettext_noop("Returns a list of secrets")}, + {NULL, NULL} +}; + +static int +cmdSecretList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + int maxuuids = 0, i; + char **uuids = NULL; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + maxuuids = virConnectNumOfSecrets(ctl->conn); + if (maxuuids < 0) { + vshError(ctl, FALSE, "%s", _("Failed to list secrets")); + return FALSE; + } + uuids = vshMalloc(ctl, sizeof(*uuids) * maxuuids); + + maxuuids = virConnectListSecrets(ctl->conn, uuids, maxuuids); + if (maxuuids < 0) { + vshError(ctl, FALSE, "%s", _("Failed to list secrets")); + free(uuids); + return FALSE; + } + + qsort(uuids, maxuuids, sizeof(char *), namesorter); + + vshPrintExtra(ctl, "%s\n", _("UUID")); + vshPrintExtra(ctl, "-----------------------------------------\n"); + + for (i = 0; i < maxuuids; i++) { + vshPrint(ctl, "%-36s\n", uuids[i]); + free(uuids[i]); + } + free(uuids); + return TRUE; +} /* @@ -6931,6 +7217,14 @@ static const vshCmdDef commands[] = { {"pool-undefine", cmdPoolUndefine, opts_pool_undefine, info_pool_undefine}, {"pool-uuid", cmdPoolUuid, opts_pool_uuid, info_pool_uuid}, + {"secret-define", cmdSecretDefine, opts_secret_define, info_secret_define}, + {"secret-dumpxml", cmdSecretDumpXML, opts_secret_dumpxml, info_secret_dumpxml}, + {"secret-set-value", cmdSecretSetValue, opts_secret_set_value, info_secret_set_value}, + {"secret-get-value", cmdSecretGetValue, opts_secret_get_value, info_secret_get_value}, + {"secret-undefine", cmdSecretUndefine, opts_secret_undefine, info_secret_undefine}, + {"secret-list", cmdSecretList, NULL, info_secret_list}, + + #ifndef WIN32 {"pwd", cmdPwd, NULL, info_pwd}, #endif @@ -7490,6 +7784,35 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd, return vol; } +static virSecretPtr +vshCommandOptSecret(vshControl *ctl, const vshCmd *cmd, char **name) +{ + virSecretPtr secret = NULL; + char *n; + const char *optname = "secret"; + + if (!cmd_has_option (ctl, cmd, optname)) + return NULL; + + n = vshCommandOptString(cmd, optname, NULL); + if (n == NULL) { + vshError(ctl, FALSE, "%s", _("undefined secret UUID")); + return NULL; + } + + vshDebug(ctl, 5, "%s: found option <%s>: %s\n", cmd->def->name, optname, n); + + if (name != NULL) + *name = n; + + secret = virSecretLookupByUUIDString(ctl->conn, n); + + if (secret == NULL) + vshError(ctl, FALSE, _("failed to get secret '%s'"), n); + + return secret; +} + /* * Executes command(s) and returns return code from last command */ diff --git a/virsh.1 b/virsh.1 index 0a5b1c1..5731b4c 100644 --- a/virsh.1 +++ b/virsh.1 @@ -132,7 +132,7 @@ .\" ======================================================================== .\" .IX Title "VIRSH 1" -.TH VIRSH 1 "2009-08-11" "libvirt-0.7.0" "Virtualization Support" +.TH VIRSH 1 "2009-08-20" "libvirt-0.7.0" "Virtualization Support" .\" For nroff, turn off justification. Always turn off hyphenation; it makes .\" way too many mistakes in technical documents. .if n .ad l @@ -593,6 +593,38 @@ Undefine the configuration for an inactive network. .IP "\fBnet-uuid\fR \fInetwork-name\fR" 4 .IX Item "net-uuid network-name" Convert a network name to network \s-1UUID\s0. +.SH "SECRET COMMMANDS" +.IX Header "SECRET COMMMANDS" +The following commands manipulate \*(L"secrets\*(R" (e.g. passwords, passphrases and +encryption keys). Libvirt can store secrets independently from their use, and +other objects (e.g. volumes or domains) can refer to the secrets for encryption +or possibly other uses. Secrets are identified using an \s-1UUID\s0. See +<http://libvirt.org/formatsecret.html> for documentation of the \s-1XML\s0 format +used to represent properties of secrets. +.IP "\fBsecret-define\fR \fIfile\fR" 4 +.IX Item "secret-define file" +Create a secret with the properties specified in \fIfile\fR, with no associated +secret value. If \fIfile\fR does not specify a \s-1UUID\s0, choose one automatically. +If \fIfile\fR specifies an \s-1UUID\s0 of an existing secret, replace its properties by +properties defined in \fIfile\fR, without affecting the secret value. +.IP "\fBsecret-dumpxml\fR \fIsecret\fR" 4 +.IX Item "secret-dumpxml secret" +Output properties of \fIsecret\fR (specified by its \s-1UUID\s0) as an \s-1XML\s0 dump to stdout. +.IP "\fBsecret-set-value\fR \fIsecret\fR \fIbase64\fR" 4 +.IX Item "secret-set-value secret base64" +Set the value associated with \fIsecret\fR (specified by its \s-1UUID\s0) to the value +Base64\-encoded value \fIbase64\fR. +.IP "\fBsecret-get-value\fR \fIsecret\fR" 4 +.IX Item "secret-get-value secret" +Output the value associated with \fIsecret\fR (specified by its \s-1UUID\s0) to stdout, +encoded using Base64. +.IP "\fBsecret-undefine\fR \fIsecret\fR" 4 +.IX Item "secret-undefine secret" +Delete a \fIsecret\fR (specified by its \s-1UUID\s0), including the associated value, if +any. +.IP "\fBsecret-list\fR" 4 +.IX Item "secret-list" +Output a list of UUIDs of known secrets to stdout. .SH "ENVIRONMENT" .IX Header "ENVIRONMENT" The following environment variables can be set to alter the behaviour -- 1.6.2.5

On Mon, Sep 07, 2009 at 04:12:40PM +0200, Miloslav Trmač wrote:
* src/virsh.c: Add virsh commands. * docs/virsh.pod, virsh.1: Update documentation. --- docs/virsh.pod | 43 ++++++++ src/virsh.c | 323 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ virsh.1 | 34 ++++++- 3 files changed, 399 insertions(+), 1 deletions(-)
ACK, makes sense and looks good, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

* storage_encryption_conf.c: Fix a pasto in comment. --- src/storage_encryption_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage_encryption_conf.c b/src/storage_encryption_conf.c index 7ec24af..7b9ee88 100644 --- a/src/storage_encryption_conf.c +++ b/src/storage_encryption_conf.c @@ -1,5 +1,5 @@ /* - * storage_encryption_conf.h: volume encryption information + * storage_encryption_conf.c: volume encryption information * * Copyright (C) 2009 Red Hat, Inc. * -- 1.6.2.5

On Mon, Sep 07, 2009 at 04:12:41PM +0200, Miloslav Trmač wrote:
* storage_encryption_conf.c: Fix a pasto in comment. --- src/storage_encryption_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/storage_encryption_conf.c b/src/storage_encryption_conf.c index 7ec24af..7b9ee88 100644 --- a/src/storage_encryption_conf.c +++ b/src/storage_encryption_conf.c @@ -1,5 +1,5 @@ /* - * storage_encryption_conf.h: volume encryption information + * storage_encryption_conf.c: volume encryption information * * Copyright (C) 2009 Red Hat, Inc. *
Trivial, applied, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

If the <encryption format='qcow'> element does not specify a secret during volume creation, generate a suitable secret and add it to the <encryption> tag. The caller can view the updated <encryption> tag using virStorageVolGetXMLDesc(). Similarly, when <encryption format='default'/> is specified while creating a qcow or qcow2-formatted volume, change the format to "qcow" and generate a secret as described above. Changes since the fourth submission: - Use virSecretDefPtr API to create a <secret> XML string - Update for <usage type='volume'><volume/></usage> - Add "flags" parameter to virSecretDefineXML(), virSecretGetXMLDesc(), virSecretGetValue(), virSecretSetValue(), and all derived interfaces. - Split qcow passphrase generation into a separate function, move it into storage_encryption_conf.[ch]. * src/storage_encryption_conf.h (VIR_STORAGE_QCOW_PASSPHRASE_SIZE, virStorageGenerateQcowPasphrase), src/storage_encryption_conf.c (virStorageGenerateQcowPasphrase), src/libvirt_private.syms: Add virStorageGenerateQcowPasphrase(). * src/storage_backend.c (virStoragegenerateQcowEncryption, virStorageBackendCreateQemuImg): Generate a passphrase and <encryption> when creating a qcow-formatted encrypted volume and the user did not supply the information. --- src/libvirt_private.syms | 1 + src/storage_backend.c | 113 +++++++++++++++++++++++++++++++++++++++- src/storage_encryption_conf.c | 37 +++++++++++++ src/storage_encryption_conf.h | 7 +++ 4 files changed, 155 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cd22b96..3cb707e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -357,6 +357,7 @@ virStorageEncryptionFree; virStorageEncryptionDropSecrets; virStorageEncryptionParseNode; virStorageEncryptionFormat; +virStorageGenerateQcowPassphrase; # threads.h diff --git a/src/storage_backend.c b/src/storage_backend.c index 77243ef..afcce67 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -43,11 +43,13 @@ #include <selinux/selinux.h> #endif +#include "datatypes.h" #include "virterror_internal.h" #include "util.h" #include "memory.h" #include "node_device.h" #include "internal.h" +#include "secret_conf.h" #include "storage_backend.h" @@ -330,6 +332,103 @@ cleanup: } static int +virStorageGenerateQcowEncryption(virConnectPtr conn, + virStorageVolDefPtr vol) +{ + virSecretDefPtr def = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + virStorageEncryptionPtr enc; + virStorageEncryptionSecretPtr enc_secret = NULL; + virSecretPtr secret = NULL; + char *uuid = NULL, *xml; + unsigned char value[VIR_STORAGE_QCOW_PASSPHRASE_SIZE]; + int ret = -1; + + if (conn->secretDriver == NULL || conn->secretDriver->defineXML == NULL || + conn->secretDriver->setValue == NULL) { + virStorageReportError(conn, VIR_ERR_NO_SUPPORT, "%s", + _("secret storage not supported")); + goto cleanup; + } + + enc = vol->target.encryption; + if (enc->nsecrets != 0) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("secrets already defined")); + goto cleanup; + } + + if (VIR_ALLOC(enc_secret) < 0 || VIR_REALLOC_N(enc->secrets, 1) < 0 || + VIR_ALLOC(def) < 0) { + virReportOOMError(conn); + goto cleanup; + } + + def->ephemeral = 0; + def->private = 0; + def->id = NULL; /* Chosen by the secret driver */ + if (virAsprintf(&def->description, "qcow passphrase for %s", + vol->target.path) == -1) { + virReportOOMError(conn); + goto cleanup; + } + def->usage_type = VIR_SECRET_USAGE_TYPE_VOLUME; + def->usage.volume = strdup(vol->target.path); + if (def->usage.volume == NULL) { + virReportOOMError(conn); + goto cleanup; + } + xml = virSecretDefFormat(conn, def); + virSecretDefFree(def); + def = NULL; + if (xml == NULL) + goto cleanup; + + secret = conn->secretDriver->defineXML(conn, xml, 0); + if (secret == NULL) { + VIR_FREE(xml); + goto cleanup; + } + VIR_FREE(xml); + + uuid = strdup(secret->uuid); + if (uuid == NULL) { + virReportOOMError(conn); + goto cleanup; + } + + if (virStorageGenerateQcowPassphrase(conn, value) < 0) + goto cleanup; + + if (conn->secretDriver->setValue(secret, value, sizeof(value), 0) < 0) + goto cleanup; + secret = NULL; + + enc_secret->type = VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE; + enc_secret->uuid = uuid; + uuid = NULL; + enc->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; + enc->secrets[0] = enc_secret; /* Space for secrets[0] allocated above */ + enc_secret = NULL; + enc->nsecrets = 1; + + ret = 0; + +cleanup: + VIR_FREE(uuid); + if (secret != NULL) { + if (conn->secretDriver->undefine != NULL) + conn->secretDriver->undefine(secret); + virSecretFree(secret); + } + xml = virBufferContentAndReset(&buf); + VIR_FREE(xml); + virSecretDefFree(def); + VIR_FREE(enc_secret); + return ret; +} + +static int virStorageBackendCreateQemuImg(virConnectPtr conn, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, @@ -427,6 +526,8 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, } if (vol->target.encryption != NULL) { + virStorageEncryptionPtr enc; + if (vol->target.format != VIR_STORAGE_VOL_FILE_QCOW && vol->target.format != VIR_STORAGE_VOL_FILE_QCOW2) { virStorageReportError(conn, VIR_ERR_NO_SUPPORT, @@ -434,18 +535,24 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, "volume format %s"), type); return -1; } - if (vol->target.encryption->format != - VIR_STORAGE_ENCRYPTION_FORMAT_QCOW) { + enc = vol->target.encryption; + if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW && + enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) { virStorageReportError(conn, VIR_ERR_NO_SUPPORT, _("unsupported volume encryption format %d"), vol->target.encryption->format); return -1; } - if (vol->target.encryption->nsecrets > 1) { + if (enc->nsecrets > 1) { virStorageReportError(conn, VIR_ERR_INVALID_STORAGE_VOL, _("too many secrets for qcow encryption")); return -1; } + if (enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT || + enc->nsecrets == 0) { + if (virStorageGenerateQcowEncryption(conn, vol) < 0) + return -1; + } } if ((create_tool = virFindFileInPath("kvm-img")) != NULL) diff --git a/src/storage_encryption_conf.c b/src/storage_encryption_conf.c index 7b9ee88..1ce76b5 100644 --- a/src/storage_encryption_conf.c +++ b/src/storage_encryption_conf.c @@ -22,6 +22,9 @@ #include <config.h> +#include <fcntl.h> +#include <unistd.h> + #include "internal.h" #include "buf.h" @@ -242,3 +245,37 @@ virStorageEncryptionFormat(virConnectPtr conn, return 0; } + +int +virStorageGenerateQcowPassphrase(virConnectPtr conn, unsigned char *dest) +{ + int fd; + size_t i; + + /* A qcow passphrase is up to 16 bytes, with any data following a NUL + ignored. Prohibit control and non-ASCII characters to avoid possible + unpleasant surprises with the qemu monitor input mechanism. */ + fd = open("/dev/urandom", O_RDONLY); + if (fd < 0) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot open /dev/urandom")); + return -1; + } + i = 0; + while (i < VIR_STORAGE_QCOW_PASSPHRASE_SIZE) { + ssize_t r; + + while ((r = read(fd, dest + i, 1)) == -1 && errno == EINTR) + ; + if (r <= 0) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot read from /dev/urandom")); + close(fd); + return -1; + } + if (dest[i] >= 0x20 && dest[i] <= 0x7E) + i++; /* Got an acceptable character */ + } + close(fd); + return 0; +} diff --git a/src/storage_encryption_conf.h b/src/storage_encryption_conf.h index 3e653b5..cdcf625 100644 --- a/src/storage_encryption_conf.h +++ b/src/storage_encryption_conf.h @@ -69,4 +69,11 @@ virStorageEncryptionPtr virStorageEncryptionParseNode(virConnectPtr conn, int virStorageEncryptionFormat(virConnectPtr conn, virBufferPtr buf, virStorageEncryptionPtr enc); +/* A helper for VIR_STORAGE_ENCRYPTION_FORMAT_QCOW */ +enum { + VIR_STORAGE_QCOW_PASSPHRASE_SIZE = 16 +}; + +int virStorageGenerateQcowPassphrase(virConnectPtr conn, unsigned char *dest); + #endif /* __VIR_STORAGE_ENCRYPTION_H__ */ -- 1.6.2.5

The interface allows qemudMonitorSendCont() to report errors that are not overridden by its callers. Also fix a potential infinite loop in qemuDomainCoreDump() if sending cont repeatedly fails. Changes since the fourth submission: - Replace the "error_reported" parameter by using virGetLastError() in callers * src/qemu_driver.c (qemudMonitorSendCont): New function. (qemudAutostartConfigs): Reset error before each call to qemudStartVMDaemon(). (qemudInitCpus, qemudDomainResume, qemudDomainCoreDump, qemudDomainRestore, qemudDomainMigratePerform, qemudDomainMigrateFinish2): Use qemudMonitorSendCont(). --- src/qemu_driver.c | 79 +++++++++++++++++++++++++++------------------------- 1 files changed, 41 insertions(+), 38 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 2a8e6d1..d8a98c0 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -28,6 +28,7 @@ #include <dirent.h> #include <limits.h> #include <string.h> +#include <stdbool.h> #include <stdio.h> #include <strings.h> #include <stdarg.h> @@ -132,6 +133,8 @@ static int qemudMonitorCommandExtra(const virDomainObjPtr vm, const char *extraPrompt, int scm_fd, char **reply); +static int qemudMonitorSendCont(virConnectPtr conn, + const virDomainObjPtr vm); static int qemudDomainSetMemoryBalloon(virConnectPtr conn, virDomainObjPtr vm, unsigned long newmem); @@ -247,7 +250,10 @@ qemudAutostartConfigs(struct qemud_driver *driver) { virDomainObjLock(vm); if (vm->autostart && !virDomainIsActive(vm)) { - int ret = qemudStartVMDaemon(conn, driver, vm, NULL, -1); + int ret; + + virResetLastError(); + ret = qemudStartVMDaemon(conn, driver, vm, NULL, -1); if (ret < 0) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s\n"), @@ -1308,7 +1314,6 @@ static int qemudInitCpus(virConnectPtr conn, virDomainObjPtr vm, const char *migrateFrom) { - char *info = NULL; #if HAVE_SCHED_GETAFFINITY cpu_set_t mask; int i, maxcpu = QEMUD_CPUMASK_LEN; @@ -1344,12 +1349,12 @@ qemudInitCpus(virConnectPtr conn, if (migrateFrom == NULL) { /* Allow the CPUS to start executing */ - if (qemudMonitorCommand(vm, "cont", &info) < 0) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("resume operation failed")); + if (qemudMonitorSendCont(conn, vm) < 0) { + if (virGetLastError() == NULL) + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("resume operation failed")); return -1; } - VIR_FREE(info); } return 0; @@ -2651,6 +2656,18 @@ qemudMonitorCommand(const virDomainObjPtr vm, return qemudMonitorCommandWithFd(vm, cmd, -1, reply); } +static int +qemudMonitorSendCont(virConnectPtr conn ATTRIBUTE_UNUSED, + const virDomainObjPtr vm) { + char *reply; + + if (qemudMonitorCommand(vm, "cont", &reply) < 0) + return -1; + qemudDebug ("%s: cont reply: %s", vm->def->name, info); + VIR_FREE(reply); + return 0; +} + static virDrvOpenStatus qemudOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { @@ -3135,7 +3152,6 @@ cleanup: static int qemudDomainResume(virDomainPtr dom) { struct qemud_driver *driver = dom->conn->privateData; - char *info; virDomainObjPtr vm; int ret = -1; virDomainEventPtr event = NULL; @@ -3156,17 +3172,16 @@ static int qemudDomainResume(virDomainPtr dom) { goto cleanup; } if (vm->state == VIR_DOMAIN_PAUSED) { - if (qemudMonitorCommand(vm, "cont", &info) < 0) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, - "%s", _("resume operation failed")); + if (qemudMonitorSendCont(dom->conn, vm) < 0) { + if (virGetLastError() == NULL) + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("resume operation failed")); goto cleanup; } vm->state = VIR_DOMAIN_RUNNING; - qemudDebug("Reply %s", info); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_RESUMED, VIR_DOMAIN_EVENT_RESUMED_UNPAUSED); - VIR_FREE(info); } if (virDomainSaveStatus(dom->conn, driver->stateDir, vm) < 0) goto cleanup; @@ -3906,13 +3921,11 @@ cleanup: will support synchronous operations so we always get here after the migration is complete. */ if (resume && paused) { - if (qemudMonitorCommand(vm, "cont", &info) < 0) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, - "%s", _("resuming after dump failed")); - goto cleanup; + if (qemudMonitorSendCont(dom->conn, vm) < 0) { + if (virGetLastError() == NULL) + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("resuming after dump failed")); } - DEBUG ("%s: cont reply: %s", vm->def->name, info); - VIR_FREE(info); } if (vm) virDomainObjUnlock(vm); @@ -4428,13 +4441,12 @@ static int qemudDomainRestore(virConnectPtr conn, /* If it was running before, resume it now. */ if (header.was_running) { - char *info; - if (qemudMonitorCommand(vm, "cont", &info) < 0) { - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - "%s", _("failed to resume domain")); + if (qemudMonitorSendCont(conn, vm) < 0) { + if (virGetLastError() == NULL) + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("failed to resume domain")); goto cleanup; } - VIR_FREE(info); vm->state = VIR_DOMAIN_RUNNING; virDomainSaveStatus(conn, driver->stateDir, vm); } @@ -7144,14 +7156,9 @@ qemudDomainMigratePerform (virDomainPtr dom, ret = 0; cleanup: - /* Note that we have to free info *first*, since we are re-using the - * variable below (and otherwise might cause a memory leak) - */ - VIR_FREE(info); - if (paused) { /* we got here through some sort of failure; start the domain again */ - if (qemudMonitorCommand (vm, "cont", &info) < 0) { + if (qemudMonitorSendCont(dom->conn, vm) < 0) { /* Hm, we already know we are in error here. We don't want to * overwrite the previous error, though, so we just throw something * to the logs and hope for the best @@ -7159,16 +7166,13 @@ cleanup: VIR_ERROR(_("Failed to resume guest %s after failure\n"), vm->def->name); } - else { - DEBUG ("%s: cont reply: %s", vm->def->name, info); - VIR_FREE(info); - } event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_RESUMED, VIR_DOMAIN_EVENT_RESUMED_MIGRATED); } + VIR_FREE(info); if (vm) virDomainObjUnlock(vm); if (event) @@ -7191,7 +7195,6 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn, virDomainObjPtr vm; virDomainPtr dom = NULL; virDomainEventPtr event = NULL; - char *info = NULL; qemuDriverLock(driver); vm = virDomainFindByName(&driver->domains, dname); @@ -7211,12 +7214,12 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn, * >= 0.10.6 to work properly. This isn't strictly necessary on * older qemu's, but it also doesn't hurt anything there */ - if (qemudMonitorCommand(vm, "cont", &info) < 0) { - qemudReportError(dconn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("resume operation failed")); + if (qemudMonitorSendCont(dconn, vm) < 0) { + if (virGetLastError() == NULL) + qemudReportError(dconn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("resume operation failed")); goto cleanup; } - VIR_FREE(info); vm->state = VIR_DOMAIN_RUNNING; event = virDomainEventNewFromObj(vm, -- 1.6.2.5

On Mon, Sep 07, 2009 at 04:12:43PM +0200, Miloslav Trma?? wrote:
The interface allows qemudMonitorSendCont() to report errors that are not overridden by its callers.
Also fix a potential infinite loop in qemuDomainCoreDump() if sending cont repeatedly fails.
Changes since the fourth submission: - Replace the "error_reported" parameter by using virGetLastError() in callers
* src/qemu_driver.c (qemudMonitorSendCont): New function. (qemudAutostartConfigs): Reset error before each call to qemudStartVMDaemon(). (qemudInitCpus, qemudDomainResume, qemudDomainCoreDump, qemudDomainRestore, qemudDomainMigratePerform, qemudDomainMigrateFinish2): Use qemudMonitorSendCont(). --- src/qemu_driver.c | 79 +++++++++++++++++++++++++++------------------------- 1 files changed, 41 insertions(+), 38 deletions(-)
ACK, pushed this change Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Changes since the fourth submission: - Update for removed "error_reported" parameter to qemudMonitorSendCont() - Replace the "libvirt_internal_call" parameter of setValue() by VIR_SECRET_GET_VALUE_INTERNAL_CALL. * src/qemu_driver.c (findDomainDiskEncryption, * findVolumeQcowPassphrase, qemudMonitorSendVolumePassphrase, qemudMonitorSendCont): Send a volume passphrase if qemu asks for it. * src/qemu_driver.c (qemudInitPasswords): Remove obsolete comment. --- src/qemu_driver.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 143 insertions(+), 8 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index d8a98c0..0e7bb4a 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -1367,12 +1367,6 @@ qemudInitPasswords(virConnectPtr conn, virDomainObjPtr vm) { char *info = NULL; - /* - * NB: Might have more passwords to set in the future. eg a qcow - * disk decryption password, but there's no monitor command - * for that yet... - */ - if ((vm->def->ngraphics == 1) && vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && (vm->def->graphics[0]->data.vnc.passwd || driver->vncPassword)) { @@ -2656,12 +2650,153 @@ qemudMonitorCommand(const virDomainObjPtr vm, return qemudMonitorCommandWithFd(vm, cmd, -1, reply); } +static virStorageEncryptionPtr +findDomainDiskEncryption(virConnectPtr conn, virDomainObjPtr vm, + const char *path) +{ + bool seen_volume; + int i; + + seen_volume = false; + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk; + + disk = vm->def->disks[i]; + if (disk->src != NULL && STREQ(disk->src, path)) { + seen_volume = true; + if (disk->encryption != NULL) + return disk->encryption; + } + } + if (seen_volume) + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN, + _("missing <encryption> for volume %s"), path); + else + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unexpected passphrase request for volume %s"), + path); + return NULL; +} + +static char * +findVolumeQcowPassphrase(virConnectPtr conn, virDomainObjPtr vm, + const char *path, size_t *passphrase_len) +{ + virStorageEncryptionPtr enc; + virSecretPtr secret; + char *passphrase; + unsigned char *data; + size_t size; + + if (conn->secretDriver == NULL || + conn->secretDriver->lookupByUUIDString == NULL || + conn->secretDriver->getValue == NULL) { + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s", + _("secret storage not supported")); + return NULL; + } + + enc = findDomainDiskEncryption(conn, vm, path); + if (enc == NULL) + return NULL; + + if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW || + enc->nsecrets != 1 || + enc->secrets[0]->type != + VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN, + _("invalid <encryption> for volume %s"), path); + return NULL; + } + + if (enc->secrets[0]->uuid == NULL) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN, + _("missing secret uuid for volume %s"), path); + return NULL; + } + secret = conn->secretDriver->lookupByUUIDString(conn, + enc->secrets[0]->uuid); + if (secret == NULL) + return NULL; + data = conn->secretDriver->getValue(secret, &size, + VIR_SECRET_GET_VALUE_INTERNAL_CALL); + virUnrefSecret(secret); + if (data == NULL) + return NULL; + + if (memchr(data, '\0', size) != NULL) { + memset(data, 0, size); + VIR_FREE(data); + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_SECRET, + _("format='qcow' passphrase for %s must not contain a " + "'\\0'"), path); + return NULL; + } + + if (VIR_ALLOC_N(passphrase, size + 1) < 0) { + memset(data, 0, size); + VIR_FREE(data); + virReportOOMError(conn); + return NULL; + } + memcpy(passphrase, data, size); + passphrase[size] = '\0'; + + memset(data, 0, size); + VIR_FREE(data); + + *passphrase_len = size; + return passphrase; +} + +static int +qemudMonitorSendVolumePassphrase(const virDomainObjPtr vm, + const char *buf, + const char *prompt, + void *data) +{ + virConnectPtr conn = data; + char *passphrase, *path; + const char *prompt_path; + size_t path_len, passphrase_len = 0; + int res; + + /* The complete prompt looks like this: + ide0-hd0 (/path/to/volume) is encrypted. + Password: + "prompt" starts with ") is encrypted". Extract /path/to/volume. */ + for (prompt_path = prompt; prompt_path > buf && prompt_path[-1] != '('; + prompt_path--) + ; + if (prompt_path == buf) + return -1; + path_len = prompt - prompt_path; + if (VIR_ALLOC_N(path, path_len + 1) < 0) + return -1; + memcpy(path, prompt_path, path_len); + path[path_len] = '\0'; + + passphrase = findVolumeQcowPassphrase(conn, vm, path, &passphrase_len); + VIR_FREE(path); + if (passphrase == NULL) + return -1; + + res = qemudMonitorSend(vm, passphrase, -1); + + memset(passphrase, 0, passphrase_len); + VIR_FREE(passphrase); + + return res; +} + static int -qemudMonitorSendCont(virConnectPtr conn ATTRIBUTE_UNUSED, +qemudMonitorSendCont(virConnectPtr conn, const virDomainObjPtr vm) { char *reply; - if (qemudMonitorCommand(vm, "cont", &reply) < 0) + if (qemudMonitorCommandWithHandler(vm, "cont", ") is encrypted.", + qemudMonitorSendVolumePassphrase, conn, + -1, &reply) < 0) return -1; qemudDebug ("%s: cont reply: %s", vm->def->name, info); VIR_FREE(reply); -- 1.6.2.5

... to be more consistent with other sections. * src/libvirt_private.syms: Add a missing comment. --- src/libvirt_private.syms | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3cb707e..73a3a80 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -353,6 +353,7 @@ virStoragePartedFsTypeTypeToString; virStoragePoolObjLock; virStoragePoolObjUnlock; +# storage_encryption_conf.h virStorageEncryptionFree; virStorageEncryptionDropSecrets; virStorageEncryptionParseNode; -- 1.6.2.5

On Mon, Sep 07, 2009 at 04:12:45PM +0200, Miloslav Trmač wrote:
... to be more consistent with other sections.
* src/libvirt_private.syms: Add a missing comment.
Trivial, applied too, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (4)
-
Daniel P. Berrange
-
Daniel Veillard
-
Jim Meyering
-
Miloslav Trmač