Richard W.M. Jones
2014-Oct-31 14:27 UTC
[Libguestfs] [PATCH v2] launch: libvirt: Implement drive secrets (RHBZ#1159016).
Since v1: - Base64 decode the Ceph secret before passing it to libvirt. - Don't call virSecretFree (NULL) [libvirt bug?] - Small cleanups.
Richard W.M. Jones
2014-Oct-31 14:27 UTC
[Libguestfs] [PATCH v2] launch: libvirt: Implement drive secrets (RHBZ#1159016).
Implement the GUESTFS_ADD_DRIVE_OPTS_SECRET argument of
guestfs_add_drive_opts. For libvirt we have to save the secret in
libvirtd first, get a UUID, and then pass the UUID back through the
domain XML.
---
bootstrap | 1 +
generator/actions.ml | 2 +-
m4/.gitignore | 7 +-
src/launch-libvirt.c | 271 ++++++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 274 insertions(+), 7 deletions(-)
diff --git a/bootstrap b/bootstrap
index 402ac4e..292f454 100755
--- a/bootstrap
+++ b/bootstrap
@@ -41,6 +41,7 @@ accept4
areadlink
areadlinkat
arpa_inet
+base64
byteswap
c-ctype
cloexec
diff --git a/generator/actions.ml b/generator/actions.ml
index ece3f88..83f4734 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -1494,7 +1494,7 @@ specify the remote username you want.
=item C<secret>
For the C<rbd> protocol only, this specifies the 'secret' to use
when
-connecting to the remote device.
+connecting to the remote device. It must be base64 encoded.
If not given, then a secret matching the given username will be looked up in
the
default keychain locations, or if no username is given, then no authentication
diff --git a/m4/.gitignore b/m4/.gitignore
index 605cd26..b0f8d4b 100644
--- a/m4/.gitignore
+++ b/m4/.gitignore
@@ -5,6 +5,7 @@
/argmatch.m4
/arpa_inet_h.m4
/asm-underscore.m4
+/base64.m4
/btowc.m4
/byteswap.m4
/canonicalize-lgpl.m4
@@ -104,6 +105,7 @@
/inttypes-pri.m4
/ioctl.m4
/i-ring.m4
+/isatty.m4
/isc-posix.m4
/largefile.m4
/lchown.m4
@@ -166,6 +168,7 @@
/printf-posix.m4
/priv-set.m4
/progtest.m4
+/ptsname_r.m4
/putenv.m4
/quotearg.m4
/quote.m4
@@ -236,6 +239,7 @@
/thread.m4
/time_h.m4
/timespec.m4
+/ttyname_r.m4
/uintmax_t.m4
/ulonglong.m4
/ungetc.m4
@@ -271,6 +275,3 @@
/xstrtol.m4
/xvasprintf.m4
/yield.m4
-/isatty.m4
-/ptsname_r.m4
-/ttyname_r.m4
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 7206b33..743e97f 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -50,6 +50,7 @@
#endif
#include "glthread/lock.h"
+#include "base64.h"
#include "guestfs.h"
#include "guestfs-internal.h"
@@ -97,6 +98,27 @@ xmlBufferDetach (xmlBufferPtr buf)
}
#endif
+#ifdef HAVE_ATTRIBUTE_CLEANUP
+#define CLEANUP_VIRSECRETFREE __attribute__((cleanup(cleanup_virSecretFree)))
+
+static void
+cleanup_virSecretFree (void *ptr)
+{
+ virSecretPtr secret_obj = * (virSecretPtr *) ptr;
+ if (secret_obj)
+ virSecretFree (secret_obj);
+}
+
+#else /* !HAVE_ATTRIBUTE_CLEANUP */
+#define CLEANUP_VIRSECRETFREE
+#endif
+
+/* Use to store a mapping of secret to libvirt secret UUID. */
+struct secret {
+ char *secret;
+ char uuid[VIR_UUID_STRING_BUFLEN];
+};
+
#define DOMAIN_NAME_LEN (8+16+1) /* "guestfs-" + random + \0 */
/* Per-handle data. */
@@ -110,6 +132,8 @@ struct backend_libvirt_data {
char name[DOMAIN_NAME_LEN]; /* random name */
bool is_kvm; /* false = qemu, true = kvm (from
capabilities)*/
unsigned long qemu_version; /* qemu version (from libvirt) */
+ struct secret *secrets; /* list of secrets */
+ size_t nr_secrets;
};
/* Parameters passed to construct_libvirt_xml and subfunctions. We
@@ -130,6 +154,9 @@ struct libvirt_xml_params {
};
static int parse_capabilities (guestfs_h *g, const char *capabilities_xml,
struct backend_libvirt_data *data);
+static int add_secret (guestfs_h *g, struct backend_libvirt_data *data, const
struct drive *drv);
+static int find_secret (guestfs_h *g, const struct backend_libvirt_data *data,
const struct drive *drv, const char **type, const char **uuid);
+static int have_secret (guestfs_h *g, const struct backend_libvirt_data *data,
const struct drive *drv);
static xmlChar *construct_libvirt_xml (guestfs_h *g, const struct
libvirt_xml_params *params);
static void debug_appliance_permissions (guestfs_h *g);
static void debug_socket_permissions (guestfs_h *g);
@@ -224,6 +251,8 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
CLEANUP_FREE xmlChar *xml = NULL;
CLEANUP_FREE char *appliance = NULL;
struct sockaddr_un addr;
+ struct drive *drv;
+ size_t i;
int r;
uint32_t size;
CLEANUP_FREE void *buf = NULL;
@@ -458,6 +487,14 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
debug (g, "cannot find group 'qemu'");
}
+ /* Store any secrets in libvirtd, keeping a mapping from the secret
+ * to its UUID.
+ */
+ ITER_DRIVES (g, i, drv) {
+ if (add_secret (g, data, drv) == -1)
+ goto cleanup;
+ }
+
/* Construct the libvirt XML. */
if (g->verbose)
guestfs___print_timestamped_message (g, "create libvirt XML");
@@ -1272,6 +1309,8 @@ construct_libvirt_xml_disk (guestfs_h *g,
CLEANUP_FREE char *path = NULL;
int is_host_device;
CLEANUP_FREE char *format = NULL;
+ const char *type, *uuid;
+ int r;
/* XXX We probably could support this if we thought about it some more. */
if (drv->iface) {
@@ -1383,9 +1422,15 @@ construct_libvirt_xml_disk (guestfs_h *g,
if (drv->src.username != NULL) {
start_element ("auth") {
attribute ("username", drv->src.username);
- /* TODO: write the drive secret, after first storing it separately
- * in libvirt
- */
+ r = find_secret (g, data, drv, &type, &uuid);
+ if (r == -1)
+ return -1;
+ if (r == 1) {
+ start_element ("secret") {
+ attribute ("type", type);
+ attribute ("uuid", uuid);
+ } end_element ();
+ }
} end_element ();
}
break;
@@ -1657,6 +1702,215 @@ construct_libvirt_xml_qemu_cmdline (guestfs_h *g,
}
static int
+construct_libvirt_xml_secret (guestfs_h *g,
+ const struct backend_libvirt_data *data,
+ const struct drive *drv,
+ xmlTextWriterPtr xo)
+{
+ start_element ("secret") {
+ attribute ("ephemeral", "yes");
+ attribute ("private", "yes");
+ start_element ("description") {
+ string_format ("guestfs secret associated with %s %s",
+ data->name, drv->src.u.path);
+ } end_element ();
+ } end_element ();
+
+ return 0;
+}
+
+/* If drv->src.secret != NULL, store the secret in libvirt, and save
+ * the UUID so we can retrieve it later. Also there is some slight
+ * variation depending on the protocol. See
+ * http://libvirt.org/formatsecret.html
+ */
+static int
+add_secret (guestfs_h *g, struct backend_libvirt_data *data, const struct drive
*drv)
+{
+ CLEANUP_XMLBUFFERFREE xmlBufferPtr xb = NULL;
+ xmlOutputBufferPtr ob;
+ CLEANUP_XMLFREETEXTWRITER xmlTextWriterPtr xo = NULL;
+ CLEANUP_FREE xmlChar *xml = NULL;
+ CLEANUP_VIRSECRETFREE virSecretPtr secret_obj = NULL;
+ const char *secret = drv->src.secret;
+ CLEANUP_FREE unsigned char *secret_raw = NULL;
+ size_t secret_raw_len = 0;
+ size_t i;
+
+ if (secret == NULL)
+ return 0;
+
+ /* If it was already stored, don't create another secret. */
+ if (have_secret (g, data, drv))
+ return 0;
+
+ /* Create the XML for the secret. */
+ xb = xmlBufferCreate ();
+ if (xb == NULL) {
+ perrorf (g, "xmlBufferCreate");
+ return -1;
+ }
+ ob = xmlOutputBufferCreateBuffer (xb, NULL);
+ if (ob == NULL) {
+ perrorf (g, "xmlOutputBufferCreateBuffer");
+ return -1;
+ }
+ xo = xmlNewTextWriter (ob);
+ if (xo == NULL) {
+ perrorf (g, "xmlNewTextWriter");
+ return -1;
+ }
+
+ if (xmlTextWriterSetIndent (xo, 1) == -1 ||
+ xmlTextWriterSetIndentString (xo, BAD_CAST " ") == -1) {
+ perrorf (g, "could not set XML indent");
+ return -1;
+ }
+ if (xmlTextWriterStartDocument (xo, NULL, NULL, NULL) == -1) {
+ perrorf (g, "xmlTextWriterStartDocument");
+ return -1;
+ }
+
+ if (construct_libvirt_xml_secret (g, data, drv, xo) == -1)
+ return -1;
+
+ if (xmlTextWriterEndDocument (xo) == -1) {
+ perrorf (g, "xmlTextWriterEndDocument");
+ return -1;
+ }
+ xml = xmlBufferDetach (xb);
+ if (xml == NULL) {
+ perrorf (g, "xmlBufferDetach");
+ return -1;
+ }
+
+ debug (g, "libvirt secret XML:\n%s", xml);
+
+ /* Pass the XML to libvirt. */
+ secret_obj = virSecretDefineXML (data->conn, (const char *) xml, 0);
+ if (secret_obj == NULL) {
+ libvirt_error (g, _("could not define libvirt secret"));
+ return -1;
+ }
+
+ /* For Ceph, we have to base64 decode the secret. For others, we
+ * currently just pass the secret straight through.
+ */
+ switch (drv->src.protocol) {
+ case drive_protocol_rbd:
+ if (!base64_decode_alloc (secret, strlen (secret),
+ (char **) &secret_raw, &secret_raw_len))
{
+ error (g, _("rbd protocol secret must be base64 encoded"));
+ return -1;
+ }
+ if (secret_raw == NULL) {
+ error (g, _("base64_decode_alloc: %m"));
+ return -1;
+ }
+ break;
+ case drive_protocol_file:
+ case drive_protocol_ftp:
+ case drive_protocol_ftps:
+ case drive_protocol_gluster:
+ case drive_protocol_http:
+ case drive_protocol_https:
+ case drive_protocol_iscsi:
+ case drive_protocol_nbd:
+ case drive_protocol_sheepdog:
+ case drive_protocol_ssh:
+ case drive_protocol_tftp:
+ secret_raw = (unsigned char *) safe_strdup (g, secret);
+ secret_raw_len = strlen (secret);
+ }
+
+ /* Set the secret. */
+ if (virSecretSetValue (secret_obj, secret_raw, secret_raw_len, 0) == -1) {
+ libvirt_error (g, _("could not set libvirt secret value"));
+ return -1;
+ }
+
+ /* Get back the UUID and save it in the private data. */
+ i = data->nr_secrets;
+ data->nr_secrets++;
+ data->secrets + safe_realloc (g, data->secrets, sizeof (struct
secret) * data->nr_secrets);
+
+ data->secrets[i].secret = safe_strdup (g, secret);
+
+ if (virSecretGetUUIDString (secret_obj, data->secrets[i].uuid) == -1) {
+ libvirt_error (g, _("could not get UUID from libvirt secret"));
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+have_secret (guestfs_h *g,
+ const struct backend_libvirt_data *data, const struct drive *drv)
+{
+ size_t i;
+
+ if (drv->src.secret == NULL)
+ return 0;
+
+ for (i = 0; i < data->nr_secrets; ++i) {
+ if (STREQ (data->secrets[i].secret, drv->src.secret))
+ return 1;
+ }
+
+ return 0;
+}
+
+/* Find a secret previously stored in libvirt. Returns the
+ * <secret type=... uuid=...> attributes. This function returns -1
+ * if there was an error, 0 if there is no secret, and 1 if the
+ * secret was found and returned.
+ */
+static int
+find_secret (guestfs_h *g,
+ const struct backend_libvirt_data *data, const struct drive *drv,
+ const char **type, const char **uuid)
+{
+ size_t i;
+
+ if (drv->src.secret == NULL)
+ return 0;
+
+ for (i = 0; i < data->nr_secrets; ++i) {
+ if (STREQ (data->secrets[i].secret, drv->src.secret)) {
+ *uuid = data->secrets[i].uuid;
+
+ *type = "volume";
+
+ switch (drv->src.protocol) {
+ case drive_protocol_rbd:
+ *type = "ceph";
+ break;
+ case drive_protocol_iscsi:
+ *type = "iscsi";
+ break;
+ case drive_protocol_file:
+ case drive_protocol_ftp:
+ case drive_protocol_ftps:
+ case drive_protocol_gluster:
+ case drive_protocol_http:
+ case drive_protocol_https:
+ case drive_protocol_nbd:
+ case drive_protocol_sheepdog:
+ case drive_protocol_ssh:
+ case drive_protocol_tftp:
+ /* set to a default value above */ ;
+ }
+
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+static int
is_blk (const char *path)
{
struct stat statbuf;
@@ -1678,6 +1932,7 @@ shutdown_libvirt (guestfs_h *g, void *datav, int
check_for_errors)
struct backend_libvirt_data *data = datav;
virConnectPtr conn = data->conn;
virDomainPtr dom = data->dom;
+ size_t i;
int ret = 0;
int flags;
@@ -1710,6 +1965,12 @@ shutdown_libvirt (guestfs_h *g, void *datav, int
check_for_errors)
free (data->network_bridge);
data->network_bridge = NULL;
+ for (i = 0; i < data->nr_secrets; ++i)
+ free (data->secrets[i].secret);
+ free (data->secrets);
+ data->secrets = NULL;
+ data->nr_secrets = 0;
+
return ret;
}
@@ -1814,6 +2075,10 @@ hot_add_drive_libvirt (guestfs_h *g, void *datav,
return -1;
}
+ /* If the drive has an associated secret, store it in libvirt. */
+ if (add_secret (g, data, drv) == -1)
+ return -1;
+
/* Create the XML for the new disk. */
xml = construct_libvirt_xml_hot_add_disk (g, data, drv, drv_index);
if (xml == NULL)
--
2.0.4