[libvirt] [PATCH 0/7] Create a util/virsecret and add key usage secret

Extracted from the LUKS series (patches 4-9 and patch 14): http://www.redhat.com/archives/libvir-list/2016-June/msg00804.html with a couple of modifications as I working through the TLS code... 1. Instead of looking the LookupDef 'secdef', call it 'seclookupdef' (it's just clearer that way I think) 2. Likewise, change the ParseSecret and FormatSecret to add "Lookup" John Ferlan (7): storage: Use virSecretGetSecretString secret: Move virStorageSecretType and rename util: Move and rename virStorageAuthDefParseSecret util: Introduce virSecretFormatSecret qemu: Change protocol parameter for secret setup qemu: Remove authdef from secret setup conf: Add new secret type "key" docs/aclpolkit.html.in | 4 ++ docs/formatsecret.html.in | 57 +++++++++++++++- docs/schemas/secret.rng | 10 +++ include/libvirt/libvirt-secret.h | 3 +- po/POTFILES.in | 1 + src/Makefile.am | 2 + src/access/viraccessdriverpolkit.c | 13 ++++ src/conf/secret_conf.c | 26 ++++++- src/conf/secret_conf.h | 3 +- src/conf/virsecretobj.c | 5 ++ src/libvirt_private.syms | 7 ++ src/libxl/libxl_conf.c | 2 +- src/qemu/qemu_domain.c | 131 ++++++++++++++++++++++-------------- src/secret/secret_util.c | 18 ++--- src/secret/secret_util.h | 10 +-- src/storage/storage_backend_iscsi.c | 54 +++------------ src/storage/storage_backend_rbd.c | 49 ++------------ src/util/virsecret.c | 124 ++++++++++++++++++++++++++++++++++ src/util/virsecret.h | 56 +++++++++++++++ src/util/virstoragefile.c | 103 +++++++--------------------- src/util/virstoragefile.h | 17 +---- tests/qemuargv2xmltest.c | 4 +- tests/secretxml2xmlin/usage-key.xml | 7 ++ tests/secretxml2xmltest.c | 1 + 24 files changed, 451 insertions(+), 256 deletions(-) create mode 100644 src/util/virsecret.c create mode 100644 src/util/virsecret.h create mode 100644 tests/secretxml2xmlin/usage-key.xml -- 2.5.5

Rather than inline code secret lookup for rbd/iscsi, use the common function. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/Makefile.am | 1 + src/storage/storage_backend_iscsi.c | 49 +++++-------------------------------- src/storage/storage_backend_rbd.c | 48 +++--------------------------------- 3 files changed, 10 insertions(+), 88 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 1aff57d..4333c2b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1618,6 +1618,7 @@ libvirt_driver_storage_impl_la_SOURCES = libvirt_driver_storage_impl_la_CFLAGS = \ -I$(srcdir)/access \ -I$(srcdir)/conf \ + -I$(srcdir)/secret \ $(AM_CFLAGS) libvirt_driver_storage_impl_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_storage_impl_la_LIBADD = diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index bccfba3..6cefd50 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -1,7 +1,7 @@ /* * storage_backend_iscsi.c: storage backend for iSCSI handling * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-2016 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -43,6 +43,7 @@ #include "virobject.h" #include "virstring.h" #include "viruuid.h" +#include "secret_util.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -277,11 +278,10 @@ virStorageBackendISCSISetAuth(const char *portal, virConnectPtr conn, virStoragePoolSourcePtr source) { - virSecretPtr secret = NULL; unsigned char *secret_value = NULL; + size_t secret_size; virStorageAuthDefPtr authdef = source->auth; int ret = -1; - char uuidStr[VIR_UUID_STRING_BUFLEN]; if (!authdef || authdef->authType == VIR_STORAGE_AUTH_TYPE_NONE) return 0; @@ -301,45 +301,9 @@ virStorageBackendISCSISetAuth(const char *portal, return -1; } - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) - secret = virSecretLookupByUUID(conn, authdef->secret.uuid); - else - secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_ISCSI, - authdef->secret.usage); - - if (secret) { - size_t secret_size; - secret_value = - conn->secretDriver->secretGetValue(secret, &secret_size, 0, - VIR_SECRET_GET_VALUE_INTERNAL_CALL); - if (!secret_value) { - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - virUUIDFormat(authdef->secret.uuid, uuidStr); - virReportError(VIR_ERR_INTERNAL_ERROR, - _("could not get the value of the secret " - "for username %s using uuid '%s'"), - authdef->username, uuidStr); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("could not get the value of the secret " - "for username %s using usage value '%s'"), - authdef->username, authdef->secret.usage); - } - goto cleanup; - } - } else { - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - virUUIDFormat(authdef->secret.uuid, uuidStr); - virReportError(VIR_ERR_NO_SECRET, - _("no secret matches uuid '%s'"), - uuidStr); - } else { - virReportError(VIR_ERR_NO_SECRET, - _("no secret matches usage value '%s'"), - authdef->secret.usage); - } + if (virSecretGetSecretString(conn, authdef, VIR_SECRET_USAGE_TYPE_ISCSI, + &secret_value, &secret_size) < 0) goto cleanup; - } if (virISCSINodeUpdate(portal, source->devices[0].path, @@ -358,8 +322,7 @@ virStorageBackendISCSISetAuth(const char *portal, ret = 0; cleanup: - virObjectUnref(secret); - VIR_FREE(secret_value); + VIR_DISPOSE_N(secret_value, secret_size); return ret; } diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index ac46b9d..64ec545 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -36,6 +36,7 @@ #include "virrandom.h" #include "rados/librados.h" #include "rbd/librbd.h" +#include "secret_util.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -62,8 +63,6 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, size_t secret_value_size = 0; char *rados_key = NULL; virBuffer mon_host = VIR_BUFFER_INITIALIZER; - virSecretPtr secret = NULL; - char secretUuid[VIR_UUID_STRING_BUFLEN]; size_t i; char *mon_buff = NULL; const char *client_mount_timeout = "30"; @@ -86,48 +85,9 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, return -1; } - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - virUUIDFormat(authdef->secret.uuid, secretUuid); - VIR_DEBUG("Looking up secret by UUID: %s", secretUuid); - secret = virSecretLookupByUUIDString(conn, secretUuid); - } else if (authdef->secret.usage != NULL) { - VIR_DEBUG("Looking up secret by usage: %s", - authdef->secret.usage); - secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_CEPH, - authdef->secret.usage); - } - - if (secret == NULL) { - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - virReportError(VIR_ERR_NO_SECRET, - _("no secret matches uuid '%s'"), - secretUuid); - } else { - virReportError(VIR_ERR_NO_SECRET, - _("no secret matches usage value '%s'"), - authdef->secret.usage); - } + if (virSecretGetSecretString(conn, authdef, VIR_SECRET_USAGE_TYPE_CEPH, + &secret_value, &secret_value_size) < 0) goto cleanup; - } - - secret_value = conn->secretDriver->secretGetValue(secret, - &secret_value_size, 0, - VIR_SECRET_GET_VALUE_INTERNAL_CALL); - - if (!secret_value) { - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("could not get the value of the secret " - "for username '%s' using uuid '%s'"), - authdef->username, secretUuid); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("could not get the value of the secret " - "for username '%s' using usage value '%s'"), - authdef->username, authdef->secret.usage); - } - goto cleanup; - } if (!(rados_key = virStringEncodeBase64(secret_value, secret_value_size))) goto cleanup; @@ -227,8 +187,6 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, VIR_DISPOSE_N(secret_value, secret_value_size); VIR_DISPOSE_STRING(rados_key); - virObjectUnref(secret); - virBufferFreeAndReset(&mon_host); VIR_FREE(mon_buff); return ret; -- 2.5.5

Move the enum into a new src/util/virsecret.h, rename it to be virSecretLookupType. Add a src/util/virsecret.h in order to perform a couple of simple operations on the secret XML and virSecretLookupTypeDef for clearing and copying. This includes quite a bit of collateral damage, but the goal is to remove the "virStorage*" and replace with the virSecretLookupType so that it's easier to to add new lookups that aren't necessarily storage pool related. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/Makefile.am | 1 + src/conf/secret_conf.h | 2 +- src/libvirt_private.syms | 5 ++++ src/libxl/libxl_conf.c | 2 +- src/qemu/qemu_domain.c | 4 +-- src/secret/secret_util.c | 18 ++++++------ src/secret/secret_util.h | 10 +++---- src/storage/storage_backend_iscsi.c | 7 +++-- src/storage/storage_backend_rbd.c | 3 +- src/util/virsecret.c | 57 +++++++++++++++++++++++++++++++++++++ src/util/virsecret.h | 50 ++++++++++++++++++++++++++++++++ src/util/virstoragefile.c | 32 +++++++++------------ src/util/virstoragefile.h | 17 ++--------- tests/qemuargv2xmltest.c | 4 +-- 14 files changed, 156 insertions(+), 56 deletions(-) create mode 100644 src/util/virsecret.c create mode 100644 src/util/virsecret.h diff --git a/src/Makefile.am b/src/Makefile.am index 4333c2b..ad80cc9 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -161,6 +161,7 @@ UTIL_SOURCES = \ util/virrotatingfile.h util/virrotatingfile.c \ util/virscsi.c util/virscsi.h \ util/virseclabel.c util/virseclabel.h \ + util/virsecret.c util/virsecret.h \ util/virsexpr.c util/virsexpr.h \ util/virsocketaddr.h util/virsocketaddr.c \ util/virstats.c util/virstats.h \ diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h index ca1afec..4584403 100644 --- a/src/conf/secret_conf.h +++ b/src/conf/secret_conf.h @@ -35,7 +35,7 @@ struct _virSecretDef { bool isprivate; unsigned char uuid[VIR_UUID_BUFLEN]; char *description; /* May be NULL */ - int usage_type; + int usage_type; /* virSecretUsageType */ union { char *volume; /* May be NULL */ char *ceph; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e939de3..8664e05 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2210,6 +2210,11 @@ virSecurityLabelDefFree; virSecurityLabelDefNew; +# util/virsecret.h +virSecretLookupDefClear; +virSecretLookupDefCopy; + + # util/virsexpr.h sexpr2string; sexpr_append; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 5989819..5a3e3a0 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -656,7 +656,7 @@ libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr) if (!(conn = virConnectOpen("xen:///system"))) goto cleanup; - if (virSecretGetSecretString(conn, src->auth, + if (virSecretGetSecretString(conn, &src->auth->seclookupdef, VIR_SECRET_USAGE_TYPE_CEPH, &secret, &secretlen) < 0) goto cleanup; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d1f8175..b44735d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -829,7 +829,7 @@ qemuDomainSecretPlainSetup(virConnectPtr conn, if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD) secretType = VIR_SECRET_USAGE_TYPE_CEPH; - return virSecretGetSecretString(conn, authdef, secretType, + return virSecretGetSecretString(conn, &authdef->seclookupdef, secretType, &secinfo->s.plain.secret, &secinfo->s.plain.secretlen); } @@ -902,7 +902,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, goto cleanup; /* Grab the unencoded secret */ - if (virSecretGetSecretString(conn, authdef, secretType, + if (virSecretGetSecretString(conn, &authdef->seclookupdef, secretType, &secret, &secretlen) < 0) goto cleanup; diff --git a/src/secret/secret_util.c b/src/secret/secret_util.c index 5602401..16e43ab 100644 --- a/src/secret/secret_util.c +++ b/src/secret/secret_util.c @@ -36,12 +36,12 @@ VIR_LOG_INIT("secret.secret_util"); /* virSecretGetSecretString: * @conn: Pointer to the connection driver to make secret driver call - * @authdef: Pointer to the disk storage authentication - * @secretUsageType: Type of secret usage for authdef lookup + * @seclookupdef: Secret lookup def + * @secretUsageType: Type of secret usage for usage lookup * @secret: returned secret as a sized stream of unsigned chars * @secret_size: Return size of the secret - either raw text or base64 * - * Lookup the secret for the authdef usage type and return it as raw text. + * Lookup the secret for the usage type and return it as raw text. * It is up to the caller to encode the secret further. * * Returns 0 on success, -1 on failure. On success the memory in secret @@ -49,7 +49,7 @@ VIR_LOG_INIT("secret.secret_util"); */ int virSecretGetSecretString(virConnectPtr conn, - virStorageAuthDefPtr authdef, + virSecretLookupTypeDefPtr seclookupdef, virSecretUsageType secretUsageType, uint8_t **secret, size_t *secret_size) @@ -57,14 +57,14 @@ virSecretGetSecretString(virConnectPtr conn, virSecretPtr sec = NULL; int ret = -1; - switch (authdef->secretType) { - case VIR_STORAGE_SECRET_TYPE_UUID: - sec = conn->secretDriver->secretLookupByUUID(conn, authdef->secret.uuid); + switch (seclookupdef->type) { + case VIR_SECRET_LOOKUP_TYPE_UUID: + sec = conn->secretDriver->secretLookupByUUID(conn, seclookupdef->u.uuid); break; - case VIR_STORAGE_SECRET_TYPE_USAGE: + case VIR_SECRET_LOOKUP_TYPE_USAGE: sec = conn->secretDriver->secretLookupByUsage(conn, secretUsageType, - authdef->secret.usage); + seclookupdef->u.usage); break; } diff --git a/src/secret/secret_util.h b/src/secret/secret_util.h index a039662..12b51b1 100644 --- a/src/secret/secret_util.h +++ b/src/secret/secret_util.h @@ -19,17 +19,17 @@ * */ -#ifndef __VIR_SECRET_H__ -# define __VIR_SECRET_H__ +#ifndef __VIR_SECRET_UTIL_H__ +# define __VIR_SECRET_UTIL_H__ # include "internal.h" -# include "virstoragefile.h" +# include "virsecret.h" int virSecretGetSecretString(virConnectPtr conn, - virStorageAuthDefPtr authdef, + virSecretLookupTypeDefPtr seclookupdef, virSecretUsageType secretUsageType, uint8_t **ret_secret, size_t *ret_secret_size) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK; -#endif /* __VIR_SECRET_H__ */ +#endif /* __VIR_SECRET_UTIL_H__ */ diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 6cefd50..e3a41b6 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -286,8 +286,8 @@ virStorageBackendISCSISetAuth(const char *portal, if (!authdef || authdef->authType == VIR_STORAGE_AUTH_TYPE_NONE) return 0; - VIR_DEBUG("username='%s' authType=%d secretType=%d", - authdef->username, authdef->authType, authdef->secretType); + VIR_DEBUG("username='%s' authType=%d seclookupdef.type=%d", + authdef->username, authdef->authType, authdef->seclookupdef.type); if (authdef->authType != VIR_STORAGE_AUTH_TYPE_CHAP) { virReportError(VIR_ERR_XML_ERROR, "%s", _("iscsi pool only supports 'chap' auth type")); @@ -301,7 +301,8 @@ virStorageBackendISCSISetAuth(const char *portal, return -1; } - if (virSecretGetSecretString(conn, authdef, VIR_SECRET_USAGE_TYPE_ISCSI, + if (virSecretGetSecretString(conn, &authdef->seclookupdef, + VIR_SECRET_USAGE_TYPE_ISCSI, &secret_value, &secret_size) < 0) goto cleanup; diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 64ec545..9665fbc 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -85,7 +85,8 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, return -1; } - if (virSecretGetSecretString(conn, authdef, VIR_SECRET_USAGE_TYPE_CEPH, + if (virSecretGetSecretString(conn, &authdef->seclookupdef, + VIR_SECRET_USAGE_TYPE_CEPH, &secret_value, &secret_value_size) < 0) goto cleanup; diff --git a/src/util/virsecret.c b/src/util/virsecret.c new file mode 100644 index 0000000..45ad996 --- /dev/null +++ b/src/util/virsecret.c @@ -0,0 +1,57 @@ +/* + * virsecret.c: secret utility functions + * + * Copyright (C) 2016 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * 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, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include "viralloc.h" +#include "virerror.h" +#include "virlog.h" +#include "virsecret.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("util.secret"); + + +void +virSecretLookupDefClear(virSecretLookupTypeDefPtr def) +{ + if (def->type == VIR_SECRET_LOOKUP_TYPE_USAGE) + VIR_FREE(def->u.usage); + else if (def->type == VIR_SECRET_LOOKUP_TYPE_UUID) + memset(&def->u.uuid, 0, VIR_UUID_BUFLEN); +} + + +int +virSecretLookupDefCopy(virSecretLookupTypeDefPtr dst, + const virSecretLookupTypeDef *src) +{ + dst->type = src->type; + if (dst->type == VIR_SECRET_LOOKUP_TYPE_UUID) { + memcpy(dst->u.uuid, src->u.uuid, VIR_UUID_BUFLEN); + } else if (dst->type == VIR_SECRET_LOOKUP_TYPE_USAGE) { + if (VIR_STRDUP(dst->u.usage, src->u.usage) < 0) + return -1; + } + return 0; +} diff --git a/src/util/virsecret.h b/src/util/virsecret.h new file mode 100644 index 0000000..fb3adb3 --- /dev/null +++ b/src/util/virsecret.h @@ -0,0 +1,50 @@ +/* + * virsecret.h: secret utility functions + * + * Copyright (C) 2016 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * 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, see + * <http://www.gnu.org/licenses/>. + * + */ + +#ifndef __VIR_SECRET_H__ +# define __VIR_SECRET_H__ + +# include "internal.h" + +typedef enum { + VIR_SECRET_LOOKUP_TYPE_NONE, + VIR_SECRET_LOOKUP_TYPE_UUID, + VIR_SECRET_LOOKUP_TYPE_USAGE, + + VIR_SECRET_LOOKUP_TYPE_LAST +} virSecretLookupType; + +typedef struct _virSecretLookupTypeDef virSecretLookupTypeDef; +typedef virSecretLookupTypeDef *virSecretLookupTypeDefPtr; +struct _virSecretLookupTypeDef { + int type; /* virSecretLookupType */ + union { + unsigned char uuid[VIR_UUID_BUFLEN]; + char *usage; + } u; + +}; + +void virSecretLookupDefClear(virSecretLookupTypeDefPtr def); +int virSecretLookupDefCopy(virSecretLookupTypeDefPtr dst, + const virSecretLookupTypeDef *src); + +#endif /* __VIR_SECRET_H__ */ diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d2da9e7..27b54a2 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1506,8 +1506,7 @@ virStorageAuthDefFree(virStorageAuthDefPtr authdef) VIR_FREE(authdef->username); VIR_FREE(authdef->secrettype); - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_USAGE) - VIR_FREE(authdef->secret.usage); + virSecretLookupDefClear(&authdef->seclookupdef); VIR_FREE(authdef); } @@ -1526,13 +1525,10 @@ virStorageAuthDefCopy(const virStorageAuthDef *src) if (VIR_STRDUP(ret->secrettype, src->secrettype) < 0) goto error; ret->authType = src->authType; - ret->secretType = src->secretType; - if (ret->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - memcpy(ret->secret.uuid, src->secret.uuid, sizeof(ret->secret.uuid)); - } else if (ret->secretType == VIR_STORAGE_SECRET_TYPE_USAGE) { - if (VIR_STRDUP(ret->secret.usage, src->secret.usage) < 0) - goto error; - } + + if (virSecretLookupDefCopy(&ret->seclookupdef, &src->seclookupdef) < 0) + goto error; + return ret; error: @@ -1573,16 +1569,16 @@ virStorageAuthDefParseSecret(xmlXPathContextPtr ctxt, } if (uuid) { - if (virUUIDParse(uuid, authdef->secret.uuid) < 0) { + if (virUUIDParse(uuid, authdef->seclookupdef.u.uuid) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid auth secret uuid")); goto cleanup; } - authdef->secretType = VIR_STORAGE_SECRET_TYPE_UUID; + authdef->seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID; } else { - authdef->secret.usage = usage; + authdef->seclookupdef.u.usage = usage; usage = NULL; - authdef->secretType = VIR_STORAGE_SECRET_TYPE_USAGE; + authdef->seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_USAGE; } ret = 0; @@ -1625,7 +1621,7 @@ virStorageAuthDefParseXML(xmlXPathContextPtr ctxt) VIR_FREE(authtype); } - authdef->secretType = VIR_STORAGE_SECRET_TYPE_NONE; + authdef->seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_NONE; if (virStorageAuthDefParseSecret(ctxt, authdef) < 0) goto error; @@ -1680,12 +1676,12 @@ virStorageAuthDefFormat(virBufferPtr buf, else virBufferAddLit(buf, "<secret"); - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - virUUIDFormat(authdef->secret.uuid, uuidstr); + if (authdef->seclookupdef.type == VIR_SECRET_LOOKUP_TYPE_UUID) { + virUUIDFormat(authdef->seclookupdef.u.uuid, uuidstr); virBufferAsprintf(buf, " uuid='%s'/>\n", uuidstr); - } else if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_USAGE) { + } else if (authdef->seclookupdef.type == VIR_SECRET_LOOKUP_TYPE_USAGE) { virBufferEscapeString(buf, " usage='%s'/>\n", - authdef->secret.usage); + authdef->seclookupdef.u.usage); } else { virBufferAddLit(buf, "/>\n"); } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index b88e715..71a8b3a 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -1,7 +1,7 @@ /* * virstoragefile.h: file utility functions for FS storage backend * - * Copyright (C) 2007-2009, 2012-2014 Red Hat, Inc. + * Copyright (C) 2007-2009, 2012-2016 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -28,6 +28,7 @@ # include "virseclabel.h" # include "virstorageencryption.h" # include "virutil.h" +# include "virsecret.h" /* Minimum header size required to probe all known formats with * virStorageFileProbeFormat, or obtain metadata from a known format. @@ -201,25 +202,13 @@ typedef enum { } virStorageAuthType; VIR_ENUM_DECL(virStorageAuth) -typedef enum { - VIR_STORAGE_SECRET_TYPE_NONE, - VIR_STORAGE_SECRET_TYPE_UUID, - VIR_STORAGE_SECRET_TYPE_USAGE, - - VIR_STORAGE_SECRET_TYPE_LAST -} virStorageSecretType; - typedef struct _virStorageAuthDef virStorageAuthDef; typedef virStorageAuthDef *virStorageAuthDefPtr; struct _virStorageAuthDef { char *username; char *secrettype; /* <secret type='%s' for disk source */ int authType; /* virStorageAuthType */ - int secretType; /* virStorageSecretType */ - union { - unsigned char uuid[VIR_UUID_BUFLEN]; - char *usage; - } secret; + virSecretLookupTypeDef seclookupdef; }; typedef struct _virStorageDriverData virStorageDriverData; diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index c5fe776..55dda01 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -36,8 +36,8 @@ static int testSanitizeDef(virDomainDefPtr vmdef) virDomainDiskDefPtr disk = vmdef->disks[i]; if (disk->src->auth) { - disk->src->auth->secretType = VIR_STORAGE_SECRET_TYPE_USAGE; - if (VIR_STRDUP(disk->src->auth->secret.usage, + disk->src->auth->seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_USAGE; + if (VIR_STRDUP(disk->src->auth->seclookupdef.u.usage, "qemuargv2xml_usage") < 0) goto fail; } -- 2.5.5

Move to virsecret.c and rename to virSecretParseSecret. Also convert to usage xmlNodePtr and virXMLPropString rather than virXPathString. Signed-off-by: John Ferlan <jferlan@redhat.com> --- po/POTFILES.in | 1 + src/libvirt_private.syms | 1 + src/util/virsecret.c | 44 +++++++++++++++++++++++++++++ src/util/virsecret.h | 5 +++- src/util/virstoragefile.c | 71 ++++++++++++----------------------------------- 5 files changed, 67 insertions(+), 55 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index 822cfbc..67838f5 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -233,6 +233,7 @@ src/util/virqemu.c src/util/virrandom.c src/util/virrotatingfile.c src/util/virscsi.c +src/util/virsecret.c src/util/virsexpr.c src/util/virsocketaddr.c src/util/virstats.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8664e05..d06e754 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2213,6 +2213,7 @@ virSecurityLabelDefNew; # util/virsecret.h virSecretLookupDefClear; virSecretLookupDefCopy; +virSecretParseSecretLookup; # util/virsexpr.h diff --git a/src/util/virsecret.c b/src/util/virsecret.c index 45ad996..a4eb22f 100644 --- a/src/util/virsecret.c +++ b/src/util/virsecret.c @@ -26,6 +26,7 @@ #include "virlog.h" #include "virsecret.h" #include "virstring.h" +#include "viruuid.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -55,3 +56,46 @@ virSecretLookupDefCopy(virSecretLookupTypeDefPtr dst, } return 0; } + + +int +virSecretParseSecretLookup(xmlNodePtr secretnode, + virSecretLookupTypeDefPtr def) +{ + char *uuid; + char *usage; + int ret = -1; + + uuid = virXMLPropString(secretnode, "uuid"); + usage = virXMLPropString(secretnode, "usage"); + if (uuid == NULL && usage == NULL) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing secret uuid or usage attribute")); + goto cleanup; + } + + if (uuid && usage) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("either secret uuid or usage expected")); + goto cleanup; + } + + if (uuid) { + if (virUUIDParse(uuid, def->u.uuid) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid secret uuid '%s'"), uuid); + goto cleanup; + } + def->type = VIR_SECRET_LOOKUP_TYPE_UUID; + } else { + def->u.usage = usage; + usage = NULL; + def->type = VIR_SECRET_LOOKUP_TYPE_USAGE; + } + ret = 0; + + cleanup: + VIR_FREE(uuid); + VIR_FREE(usage); + return ret; +} diff --git a/src/util/virsecret.h b/src/util/virsecret.h index fb3adb3..3c22be3 100644 --- a/src/util/virsecret.h +++ b/src/util/virsecret.h @@ -24,6 +24,8 @@ # include "internal.h" +# include "virxml.h" + typedef enum { VIR_SECRET_LOOKUP_TYPE_NONE, VIR_SECRET_LOOKUP_TYPE_UUID, @@ -46,5 +48,6 @@ struct _virSecretLookupTypeDef { void virSecretLookupDefClear(virSecretLookupTypeDefPtr def); int virSecretLookupDefCopy(virSecretLookupTypeDefPtr dst, const virSecretLookupTypeDef *src); - +int virSecretParseSecretLookup(xmlNodePtr secretnode, + virSecretLookupTypeDefPtr def); #endif /* __VIR_SECRET_H__ */ diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 27b54a2..963318f 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1537,62 +1537,11 @@ virStorageAuthDefCopy(const virStorageAuthDef *src) } -static int -virStorageAuthDefParseSecret(xmlXPathContextPtr ctxt, - virStorageAuthDefPtr authdef) -{ - char *uuid; - char *usage; - int ret = -1; - - /* Used by the domain disk xml parsing in order to ensure the - * <secret type='%s' value matches the expected secret type for - * the style of disk (iscsi is chap, nbd is ceph). For some reason - * the virSecretUsageType{From|To}String() cannot be linked here - * and because only the domain parsing code cares - just keep - * it as a string. - */ - authdef->secrettype = virXPathString("string(./secret/@type)", ctxt); - - uuid = virXPathString("string(./secret/@uuid)", ctxt); - usage = virXPathString("string(./secret/@usage)", ctxt); - if (uuid == NULL && usage == NULL) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing auth secret uuid or usage attribute")); - goto cleanup; - } - - if (uuid && usage) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("either auth secret uuid or usage expected")); - goto cleanup; - } - - if (uuid) { - if (virUUIDParse(uuid, authdef->seclookupdef.u.uuid) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("invalid auth secret uuid")); - goto cleanup; - } - authdef->seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID; - } else { - authdef->seclookupdef.u.usage = usage; - usage = NULL; - authdef->seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_USAGE; - } - ret = 0; - - cleanup: - VIR_FREE(uuid); - VIR_FREE(usage); - return ret; -} - - static virStorageAuthDefPtr virStorageAuthDefParseXML(xmlXPathContextPtr ctxt) { virStorageAuthDefPtr authdef = NULL; + xmlNodePtr secretnode = NULL; char *username = NULL; char *authtype = NULL; @@ -1621,8 +1570,22 @@ virStorageAuthDefParseXML(xmlXPathContextPtr ctxt) VIR_FREE(authtype); } - authdef->seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_NONE; - if (virStorageAuthDefParseSecret(ctxt, authdef) < 0) + if (!(secretnode = virXPathNode("./secret ", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing <secret> element in auth")); + goto error; + } + + /* Used by the domain disk xml parsing in order to ensure the + * <secret type='%s' value matches the expected secret type for + * the style of disk (iscsi is chap, nbd is ceph). For some reason + * the virSecretUsageType{From|To}String() cannot be linked here + * and because only the domain parsing code cares - just keep + * it as a string. + */ + authdef->secrettype = virXMLPropString(secretnode, "type"); + + if (virSecretParseSecretLookup(secretnode, &authdef->seclookupdef) < 0) goto error; return authdef; -- 2.5.5

Add utility to format the virSecretLookupTypeDefPtr in XML Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virsecret.c | 23 +++++++++++++++++++++++ src/util/virsecret.h | 3 +++ src/util/virstoragefile.c | 18 ++---------------- 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d06e754..5effca8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2211,6 +2211,7 @@ virSecurityLabelDefNew; # util/virsecret.h +virSecretFormatSecretLookup; virSecretLookupDefClear; virSecretLookupDefCopy; virSecretParseSecretLookup; diff --git a/src/util/virsecret.c b/src/util/virsecret.c index a4eb22f..532cbdc 100644 --- a/src/util/virsecret.c +++ b/src/util/virsecret.c @@ -99,3 +99,26 @@ virSecretParseSecretLookup(xmlNodePtr secretnode, VIR_FREE(usage); return ret; } + + +void +virSecretFormatSecretLookup(virBufferPtr buf, + const char *secrettype, + virSecretLookupTypeDefPtr def) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (secrettype) + virBufferAsprintf(buf, "<secret type='%s'", secrettype); + else + virBufferAddLit(buf, "<secret"); + + if (def->type == VIR_SECRET_LOOKUP_TYPE_UUID) { + virUUIDFormat(def->u.uuid, uuidstr); + virBufferAsprintf(buf, " uuid='%s'/>\n", uuidstr); + } else if (def->type == VIR_SECRET_LOOKUP_TYPE_USAGE) { + virBufferEscapeString(buf, " usage='%s'/>\n", def->u.usage); + } else { + virBufferAddLit(buf, "/>\n"); + } +} diff --git a/src/util/virsecret.h b/src/util/virsecret.h index 3c22be3..50a201f 100644 --- a/src/util/virsecret.h +++ b/src/util/virsecret.h @@ -50,4 +50,7 @@ int virSecretLookupDefCopy(virSecretLookupTypeDefPtr dst, const virSecretLookupTypeDef *src); int virSecretParseSecretLookup(xmlNodePtr secretnode, virSecretLookupTypeDefPtr def); +void virSecretFormatSecretLookup(virBufferPtr buf, + const char *secrettype, + virSecretLookupTypeDefPtr def); #endif /* __VIR_SECRET_H__ */ diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 963318f..4c9323f 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1623,8 +1623,6 @@ int virStorageAuthDefFormat(virBufferPtr buf, virStorageAuthDefPtr authdef) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - if (authdef->authType == VIR_STORAGE_AUTH_TYPE_NONE) { virBufferEscapeString(buf, "<auth username='%s'>\n", authdef->username); } else { @@ -1634,20 +1632,8 @@ virStorageAuthDefFormat(virBufferPtr buf, } virBufferAdjustIndent(buf, 2); - if (authdef->secrettype) - virBufferAsprintf(buf, "<secret type='%s'", authdef->secrettype); - else - virBufferAddLit(buf, "<secret"); - - if (authdef->seclookupdef.type == VIR_SECRET_LOOKUP_TYPE_UUID) { - virUUIDFormat(authdef->seclookupdef.u.uuid, uuidstr); - virBufferAsprintf(buf, " uuid='%s'/>\n", uuidstr); - } else if (authdef->seclookupdef.type == VIR_SECRET_LOOKUP_TYPE_USAGE) { - virBufferEscapeString(buf, " usage='%s'/>\n", - authdef->seclookupdef.u.usage); - } else { - virBufferAddLit(buf, "/>\n"); - } + virSecretFormatSecretLookup(buf, authdef->secrettype, + &authdef->seclookupdef); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</auth>\n"); -- 2.5.5

Rather than assume/pass the protocol to the qemuDomainSecretPlainSetup and qemuDomainSecretAESSetup, determine and pass the secretUsageType which is then used in the virSecretGetSecretString call For the two callers that convert from virStorageNetProtocol, add a new helper qemuDomainSecretProtocolGetUsageType. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 106 +++++++++++++++++++++++++++++-------------------- 1 file changed, 64 insertions(+), 42 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b44735d..ccd5ce8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -807,7 +807,7 @@ qemuDomainHostdevPrivateDispose(void *obj) /* qemuDomainSecretPlainSetup: * @conn: Pointer to connection * @secinfo: Pointer to secret info - * @protocol: Protocol for secret + * @secretUsageType: The virSecretUsageType * @authdef: Pointer to auth data * * Taking a secinfo, fill in the plaintext information @@ -817,19 +817,15 @@ qemuDomainHostdevPrivateDispose(void *obj) static int qemuDomainSecretPlainSetup(virConnectPtr conn, qemuDomainSecretInfoPtr secinfo, - virStorageNetProtocol protocol, + virSecretUsageType secretUsageType, virStorageAuthDefPtr authdef) { - int secretType = VIR_SECRET_USAGE_TYPE_ISCSI; - secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN; if (VIR_STRDUP(secinfo->s.plain.username, authdef->username) < 0) return -1; - if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD) - secretType = VIR_SECRET_USAGE_TYPE_CEPH; - - return virSecretGetSecretString(conn, &authdef->seclookupdef, secretType, + return virSecretGetSecretString(conn, &authdef->seclookupdef, + secretUsageType, &secinfo->s.plain.secret, &secinfo->s.plain.secretlen); } @@ -840,7 +836,7 @@ qemuDomainSecretPlainSetup(virConnectPtr conn, * @priv: pointer to domain private object * @secinfo: Pointer to secret info * @srcalias: Alias of the disk/hostdev used to generate the secret alias - * @protocol: Protocol for secret + * @secretUsageType: The virSecretUsageType * @authdef: Pointer to auth data * * Taking a secinfo, fill in the AES specific information using the @@ -852,7 +848,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, qemuDomainObjPrivatePtr priv, qemuDomainSecretInfoPtr secinfo, const char *srcalias, - virStorageNetProtocol protocol, + virSecretUsageType secretUsageType, virStorageAuthDefPtr authdef) { int ret = -1; @@ -862,34 +858,11 @@ qemuDomainSecretAESSetup(virConnectPtr conn, size_t secretlen = 0; uint8_t *ciphertext = NULL; size_t ciphertextlen = 0; - int secretType = VIR_SECRET_USAGE_TYPE_NONE; secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_AES; if (VIR_STRDUP(secinfo->s.aes.username, authdef->username) < 0) return -1; - switch ((virStorageNetProtocol)protocol) { - case VIR_STORAGE_NET_PROTOCOL_RBD: - secretType = VIR_SECRET_USAGE_TYPE_CEPH; - break; - - case VIR_STORAGE_NET_PROTOCOL_NONE: - case VIR_STORAGE_NET_PROTOCOL_NBD: - case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: - case VIR_STORAGE_NET_PROTOCOL_GLUSTER: - case VIR_STORAGE_NET_PROTOCOL_ISCSI: - case VIR_STORAGE_NET_PROTOCOL_HTTP: - case VIR_STORAGE_NET_PROTOCOL_HTTPS: - case VIR_STORAGE_NET_PROTOCOL_FTP: - case VIR_STORAGE_NET_PROTOCOL_FTPS: - case VIR_STORAGE_NET_PROTOCOL_TFTP: - case VIR_STORAGE_NET_PROTOCOL_LAST: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("protocol '%s' cannot be used for encrypted secrets"), - virStorageNetProtocolTypeToString(protocol)); - return -1; - } - if (!(secinfo->s.aes.alias = qemuDomainGetSecretAESAlias(srcalias))) return -1; @@ -902,7 +875,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, goto cleanup; /* Grab the unencoded secret */ - if (virSecretGetSecretString(conn, &authdef->seclookupdef, secretType, + if (virSecretGetSecretString(conn, &authdef->seclookupdef, secretUsageType, &secret, &secretlen) < 0) goto cleanup; @@ -936,7 +909,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, * @priv: pointer to domain private object * @secinfo: Pointer to secret info * @srcalias: Alias of the disk/hostdev used to generate the secret alias - * @protocol: Protocol for secret + * @secretUsageType: The virSecretUsageType * @authdef: Pointer to auth data * * If we have the encryption API present and can support a secret object, then @@ -951,17 +924,18 @@ qemuDomainSecretSetup(virConnectPtr conn, qemuDomainObjPrivatePtr priv, qemuDomainSecretInfoPtr secinfo, const char *srcalias, - virStorageNetProtocol protocol, + virSecretUsageType secretUsageType, virStorageAuthDefPtr authdef) { if (virCryptoHaveCipher(VIR_CRYPTO_CIPHER_AES256CBC) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) && - protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { - if (qemuDomainSecretAESSetup(conn, priv, secinfo, - srcalias, protocol, authdef) < 0) + secretUsageType == VIR_SECRET_USAGE_TYPE_CEPH) { + if (qemuDomainSecretAESSetup(conn, priv, secinfo, srcalias, + secretUsageType, authdef) < 0) return -1; } else { - if (qemuDomainSecretPlainSetup(conn, secinfo, protocol, authdef) < 0) + if (qemuDomainSecretPlainSetup(conn, secinfo, secretUsageType, + authdef) < 0) return -1; } return 0; @@ -985,6 +959,43 @@ qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) } +/* qemuDomainSecretGetProtocolUsageType: + * @protocol: The virStorageNetProtocol protocol type + * + * Convert the protocl into the expected virSecretUsageType for + * eventual usage to fetch the secret + * + * Returns matched protocol type or VIR_SECRET_USAGE_TYPE_NONE with an + * error message set on failure. + */ +static virSecretUsageType +qemuDomainSecretProtocolGetUsageType(virStorageNetProtocol protocol) +{ + switch ((virStorageNetProtocol)protocol) { + case VIR_STORAGE_NET_PROTOCOL_RBD: + return VIR_SECRET_USAGE_TYPE_CEPH; + + case VIR_STORAGE_NET_PROTOCOL_ISCSI: + return VIR_SECRET_USAGE_TYPE_ISCSI; + + case VIR_STORAGE_NET_PROTOCOL_NONE: + case VIR_STORAGE_NET_PROTOCOL_NBD: + case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + case VIR_STORAGE_NET_PROTOCOL_GLUSTER: + case VIR_STORAGE_NET_PROTOCOL_HTTP: + case VIR_STORAGE_NET_PROTOCOL_HTTPS: + case VIR_STORAGE_NET_PROTOCOL_FTP: + case VIR_STORAGE_NET_PROTOCOL_FTPS: + case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("protocol '%s' cannot be used for encrypted secrets"), + virStorageNetProtocolTypeToString(protocol)); + } + return VIR_SECRET_USAGE_TYPE_NONE; +} + + /* qemuDomainSecretDiskPrepare: * @conn: Pointer to connection * @priv: pointer to domain private object @@ -1008,13 +1019,19 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI || src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) { + virSecretUsageType secretUsageType; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); if (VIR_ALLOC(secinfo) < 0) return -1; + if ((secretUsageType = + qemuDomainSecretProtocolGetUsageType(src->protocol)) == + VIR_SECRET_USAGE_TYPE_NONE) + goto error; + if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias, - src->protocol, src->auth) < 0) + secretUsageType, src->auth) < 0) goto error; diskPriv->secinfo = secinfo; @@ -1072,14 +1089,19 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn, if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI && iscsisrc->auth) { + virSecretUsageType secretUsageType; qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev); if (VIR_ALLOC(secinfo) < 0) return -1; + if ((secretUsageType = + qemuDomainSecretProtocolGetUsageType(VIR_STORAGE_NET_PROTOCOL_ISCSI)) == VIR_SECRET_USAGE_TYPE_NONE) + goto error; + if (qemuDomainSecretSetup(conn, priv, secinfo, hostdev->info->alias, - VIR_STORAGE_NET_PROTOCOL_ISCSI, + secretUsageType, iscsisrc->auth) < 0) goto error; -- 2.5.5

Rather than pass authdef, pass the 'authdef->username' and the '&authdef->seclookupdef' Note that a username may be NULL. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ccd5ce8..c8f6d29 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -808,7 +808,8 @@ qemuDomainHostdevPrivateDispose(void *obj) * @conn: Pointer to connection * @secinfo: Pointer to secret info * @secretUsageType: The virSecretUsageType - * @authdef: Pointer to auth data + * @username: username to use for authentication (may be NULL) + * @seclookupdef: Pointer to seclookupdef data * * Taking a secinfo, fill in the plaintext information * @@ -818,14 +819,14 @@ static int qemuDomainSecretPlainSetup(virConnectPtr conn, qemuDomainSecretInfoPtr secinfo, virSecretUsageType secretUsageType, - virStorageAuthDefPtr authdef) + const char *username, + virSecretLookupTypeDefPtr seclookupdef) { secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN; - if (VIR_STRDUP(secinfo->s.plain.username, authdef->username) < 0) + if (VIR_STRDUP(secinfo->s.plain.username, username) < 0) return -1; - return virSecretGetSecretString(conn, &authdef->seclookupdef, - secretUsageType, + return virSecretGetSecretString(conn, seclookupdef, secretUsageType, &secinfo->s.plain.secret, &secinfo->s.plain.secretlen); } @@ -837,7 +838,8 @@ qemuDomainSecretPlainSetup(virConnectPtr conn, * @secinfo: Pointer to secret info * @srcalias: Alias of the disk/hostdev used to generate the secret alias * @secretUsageType: The virSecretUsageType - * @authdef: Pointer to auth data + * @username: username to use for authentication (may be NULL) + * @seclookupdef: Pointer to seclookupdef data * * Taking a secinfo, fill in the AES specific information using the * @@ -849,7 +851,8 @@ qemuDomainSecretAESSetup(virConnectPtr conn, qemuDomainSecretInfoPtr secinfo, const char *srcalias, virSecretUsageType secretUsageType, - virStorageAuthDefPtr authdef) + const char *username, + virSecretLookupTypeDefPtr seclookupdef) { int ret = -1; uint8_t *raw_iv = NULL; @@ -860,7 +863,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, size_t ciphertextlen = 0; secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_AES; - if (VIR_STRDUP(secinfo->s.aes.username, authdef->username) < 0) + if (VIR_STRDUP(secinfo->s.aes.username, username) < 0) return -1; if (!(secinfo->s.aes.alias = qemuDomainGetSecretAESAlias(srcalias))) @@ -875,7 +878,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, goto cleanup; /* Grab the unencoded secret */ - if (virSecretGetSecretString(conn, &authdef->seclookupdef, secretUsageType, + if (virSecretGetSecretString(conn, seclookupdef, secretUsageType, &secret, &secretlen) < 0) goto cleanup; @@ -910,7 +913,8 @@ qemuDomainSecretAESSetup(virConnectPtr conn, * @secinfo: Pointer to secret info * @srcalias: Alias of the disk/hostdev used to generate the secret alias * @secretUsageType: The virSecretUsageType - * @authdef: Pointer to auth data + * @username: username to use for authentication (may be NULL) + * @seclookupdef: Pointer to seclookupdef data * * If we have the encryption API present and can support a secret object, then * build the AES secret; otherwise, build the Plain secret. This is the magic @@ -925,17 +929,19 @@ qemuDomainSecretSetup(virConnectPtr conn, qemuDomainSecretInfoPtr secinfo, const char *srcalias, virSecretUsageType secretUsageType, - virStorageAuthDefPtr authdef) + const char *username, + virSecretLookupTypeDefPtr seclookupdef) { if (virCryptoHaveCipher(VIR_CRYPTO_CIPHER_AES256CBC) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) && secretUsageType == VIR_SECRET_USAGE_TYPE_CEPH) { if (qemuDomainSecretAESSetup(conn, priv, secinfo, srcalias, - secretUsageType, authdef) < 0) + secretUsageType, username, + seclookupdef) < 0) return -1; } else { if (qemuDomainSecretPlainSetup(conn, secinfo, secretUsageType, - authdef) < 0) + username, seclookupdef) < 0) return -1; } return 0; @@ -1031,7 +1037,8 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, goto error; if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias, - secretUsageType, src->auth) < 0) + secretUsageType, src->auth->username, + &src->auth->seclookupdef) < 0) goto error; diskPriv->secinfo = secinfo; @@ -1101,8 +1108,8 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn, goto error; if (qemuDomainSecretSetup(conn, priv, secinfo, hostdev->info->alias, - secretUsageType, - iscsisrc->auth) < 0) + secretUsageType, iscsisrc->auth->username, + &iscsisrc->auth->seclookupdef) < 0) goto error; hostdevPriv->secinfo = secinfo; -- 2.5.5

Add a new secret type known as "key" - it will handle adding the secret objects that need a key (or passphrase) without a specific username. Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/aclpolkit.html.in | 4 +++ docs/formatsecret.html.in | 57 +++++++++++++++++++++++++++++++++++-- docs/schemas/secret.rng | 10 +++++++ include/libvirt/libvirt-secret.h | 3 +- src/access/viraccessdriverpolkit.c | 13 +++++++++ src/conf/secret_conf.c | 26 ++++++++++++++++- src/conf/secret_conf.h | 1 + src/conf/virsecretobj.c | 5 ++++ tests/secretxml2xmlin/usage-key.xml | 7 +++++ tests/secretxml2xmltest.c | 1 + 10 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 tests/secretxml2xmlin/usage-key.xml diff --git a/docs/aclpolkit.html.in b/docs/aclpolkit.html.in index dae0814..f3915e5 100644 --- a/docs/aclpolkit.html.in +++ b/docs/aclpolkit.html.in @@ -224,6 +224,10 @@ <td>secret_usage_target</td> <td>Name of the associated iSCSI target, if any</td> </tr> + <tr> + <td>secret_usage_key</td> + <td>Name to be associated with the key, if any</td> + </tr> </tbody> </table> diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in index 599cb38..3bb810a 100644 --- a/docs/formatsecret.html.in +++ b/docs/formatsecret.html.in @@ -41,8 +41,9 @@ <dd> Specifies what this secret is used for. A mandatory <code>type</code> attribute specifies the usage category, currently - only <code>volume</code>, <code>ceph</code> and <code>iscsi</code> - are defined. Specific usage categories are described below. + only <code>volume</code>, <code>ceph</code>, <code>iscsi</code>, + and <code>key</code> are defined. Specific usage categories + are described below. </dd> </dl> @@ -241,5 +242,57 @@ <secret usage='libvirtiscsi'/> </auth> </pre> + + <h3><a name="keyUsageType">Usage type "key"</a></h3> + + <p> + This secret is a general purpose secret to be used by various libvirt + objects to provide a single key (or passphrase) as required by the + object in order to perform its authentication. + <span class="since">Since 1.3.6</span>. The following is an example + of a key-secret.xml file: + </p> + + <pre> + # cat key-secret.xml + <secret ephemeral='no' private='yes'> + <description>sample key secret</description> + <usage type='key'> + <key>key_example</key> + </usage> + </secret> + + # virsh secret-define key-secret.xml + Secret 718c71bd-67b5-4a2b-87ec-a24e8ca200dc created + + # virsh secret-list + UUID Usage + ----------------------------------------------------------- + 718c71bd-67b5-4a2b-87ec-a24e8ca200dc key key_example + # + + </pre> + + <p> + A secret may also be defined via the + <a href="html/libvirt-libvirt-secret.html#virSecretDefineXML"> + <code>virSecretDefineXML</code></a> API. + + Once the secret is defined, a secret value will need to be set. This + value would be the same used to create and use the volume. + The following is a simple example of using + <code>virsh secret-set-value</code> to set the secret value. The + <a href="html/libvirt-libvirt-secret.html#virSecretSetValue"> + <code>virSecretSetValue</code></a> API may also be used to set + a more secure secret without using printable/readable characters. + </p> + + <pre> + # MYSECRET=`printf %s "letmein" | base64` + # virsh secret-set-value 718c71bd-67b5-4a2b-87ec-a24e8ca200dc $MYSECRET + Secret value set + + </pre> + </body> </html> diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng index e21e700..3d131eb 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -36,6 +36,7 @@ <ref name='usagevolume'/> <ref name='usageceph'/> <ref name='usageiscsi'/> + <ref name='usagekey'/> <!-- More choices later --> </choice> </element> @@ -71,4 +72,13 @@ </element> </define> + <define name='usagekey'> + <attribute name='type'> + <value>key</value> + </attribute> + <element name='key'> + <ref name='genericName'/> + </element> + </define> + </grammar> diff --git a/include/libvirt/libvirt-secret.h b/include/libvirt/libvirt-secret.h index 3e5cdf6..fadf811 100644 --- a/include/libvirt/libvirt-secret.h +++ b/include/libvirt/libvirt-secret.h @@ -4,7 +4,7 @@ * Description: Provides APIs for the management of secrets * Author: Daniel Veillard <veillard@redhat.com> * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2014, 2016 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -43,6 +43,7 @@ typedef enum { VIR_SECRET_USAGE_TYPE_VOLUME = 1, VIR_SECRET_USAGE_TYPE_CEPH = 2, VIR_SECRET_USAGE_TYPE_ISCSI = 3, + VIR_SECRET_USAGE_TYPE_KEY = 4, # ifdef VIR_ENUM_SENTINELS VIR_SECRET_USAGE_TYPE_LAST diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c index 89bc890..97419df 100644 --- a/src/access/viraccessdriverpolkit.c +++ b/src/access/viraccessdriverpolkit.c @@ -338,6 +338,19 @@ virAccessDriverPolkitCheckSecret(virAccessManagerPtr manager, virAccessPermSecretTypeToString(perm), attrs); } break; + case VIR_SECRET_USAGE_TYPE_KEY: { + const char *attrs[] = { + "connect_driver", driverName, + "secret_uuid", uuidstr, + "secret_usage_key", secret->usage.key, + NULL, + }; + + return virAccessDriverPolkitCheck(manager, + "secret", + virAccessPermSecretTypeToString(perm), + attrs); + } break; } } diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index de9e6cf..ccda2c3 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -29,6 +29,7 @@ #include "viralloc.h" #include "secret_conf.h" #include "virsecretobj.h" +#include "virstring.h" #include "virerror.h" #include "virxml.h" #include "viruuid.h" @@ -38,7 +39,7 @@ VIR_LOG_INIT("conf.secret_conf"); VIR_ENUM_IMPL(virSecretUsage, VIR_SECRET_USAGE_TYPE_LAST, - "none", "volume", "ceph", "iscsi") + "none", "volume", "ceph", "iscsi", "key") const char * virSecretUsageIDForDef(virSecretDefPtr def) @@ -56,6 +57,9 @@ virSecretUsageIDForDef(virSecretDefPtr def) case VIR_SECRET_USAGE_TYPE_ISCSI: return def->usage.target; + case VIR_SECRET_USAGE_TYPE_KEY: + return def->usage.key; + default: return NULL; } @@ -85,6 +89,10 @@ virSecretDefFree(virSecretDefPtr def) VIR_FREE(def->usage.target); break; + case VIR_SECRET_USAGE_TYPE_KEY: + VIR_FREE(def->usage.key); + break; + default: VIR_ERROR(_("unexpected secret usage type %d"), def->usage_type); break; @@ -92,6 +100,7 @@ virSecretDefFree(virSecretDefPtr def) VIR_FREE(def); } + static int virSecretDefParseUsage(xmlXPathContextPtr ctxt, virSecretDefPtr def) @@ -145,6 +154,14 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt, } break; + case VIR_SECRET_USAGE_TYPE_KEY: + if (!(def->usage.key = virXPathString("string(./usage/key)", ctxt))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("key usage specified, but key is missing")); + return -1; + } + break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected secret usage type %d"), @@ -305,6 +322,13 @@ virSecretDefFormatUsage(virBufferPtr buf, } break; + case VIR_SECRET_USAGE_TYPE_KEY: + if (def->usage.key != NULL) { + virBufferEscapeString(buf, "<key>%s</key>\n", + def->usage.key); + } + break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected secret usage type %d"), diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h index 4584403..352c57e 100644 --- a/src/conf/secret_conf.h +++ b/src/conf/secret_conf.h @@ -40,6 +40,7 @@ struct _virSecretDef { char *volume; /* May be NULL */ char *ceph; char *target; + char *key; } usage; }; diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index c46d22c..84a32b1 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -237,6 +237,11 @@ virSecretObjSearchName(const void *payload, if (STREQ(secret->def->usage.target, data->usageID)) found = 1; break; + + case VIR_SECRET_USAGE_TYPE_KEY: + if (STREQ(secret->def->usage.key, data->usageID)) + found = 1; + break; } cleanup: diff --git a/tests/secretxml2xmlin/usage-key.xml b/tests/secretxml2xmlin/usage-key.xml new file mode 100644 index 0000000..4b3afd5 --- /dev/null +++ b/tests/secretxml2xmlin/usage-key.xml @@ -0,0 +1,7 @@ +<secret ephemeral='no' private='no'> + <uuid>f52a81b2-424e-490c-823d-6bd4235bc572</uuid> + <description>Sample Key Secret</description> + <usage type='key'> + <key>mumblyfratz</key> + </usage> +</secret> diff --git a/tests/secretxml2xmltest.c b/tests/secretxml2xmltest.c index 8dcbb40..018fa7f 100644 --- a/tests/secretxml2xmltest.c +++ b/tests/secretxml2xmltest.c @@ -80,6 +80,7 @@ mymain(void) DO_TEST("usage-volume"); DO_TEST("usage-ceph"); DO_TEST("usage-iscsi"); + DO_TEST("usage-key"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.5.5

On 06/16/2016 07:08 AM, John Ferlan wrote:
Extracted from the LUKS series (patches 4-9 and patch 14):
http://www.redhat.com/archives/libvir-list/2016-June/msg00804.html
with a couple of modifications as I working through the TLS code...
1. Instead of looking the LookupDef 'secdef', call it 'seclookupdef' (it's just clearer that way I think)
2. Likewise, change the ParseSecret and FormatSecret to add "Lookup"
John Ferlan (7): storage: Use virSecretGetSecretString secret: Move virStorageSecretType and rename util: Move and rename virStorageAuthDefParseSecret util: Introduce virSecretFormatSecret qemu: Change protocol parameter for secret setup qemu: Remove authdef from secret setup conf: Add new secret type "key"
docs/aclpolkit.html.in | 4 ++ docs/formatsecret.html.in | 57 +++++++++++++++- docs/schemas/secret.rng | 10 +++ include/libvirt/libvirt-secret.h | 3 +- po/POTFILES.in | 1 + src/Makefile.am | 2 + src/access/viraccessdriverpolkit.c | 13 ++++ src/conf/secret_conf.c | 26 ++++++- src/conf/secret_conf.h | 3 +- src/conf/virsecretobj.c | 5 ++ src/libvirt_private.syms | 7 ++ src/libxl/libxl_conf.c | 2 +- src/qemu/qemu_domain.c | 131 ++++++++++++++++++++++-------------- src/secret/secret_util.c | 18 ++--- src/secret/secret_util.h | 10 +-- src/storage/storage_backend_iscsi.c | 54 +++------------ src/storage/storage_backend_rbd.c | 49 ++------------ src/util/virsecret.c | 124 ++++++++++++++++++++++++++++++++++ src/util/virsecret.h | 56 +++++++++++++++ src/util/virstoragefile.c | 103 +++++++--------------------- src/util/virstoragefile.h | 17 +---- tests/qemuargv2xmltest.c | 4 +- tests/secretxml2xmlin/usage-key.xml | 7 ++ tests/secretxml2xmltest.c | 1 + 24 files changed, 451 insertions(+), 256 deletions(-) create mode 100644 src/util/virsecret.c create mode 100644 src/util/virsecret.h create mode 100644 tests/secretxml2xmlin/usage-key.xml
With Peter's review of the LUKS series, these no longer are necessary to review separately. John
participants (1)
-
John Ferlan