Re: [Libguestfs] [PATCH 2/2] libvirt: read secrets of disks (RHBZ#1392798)
On Wed, Nov 16, 2016 at 12:59:39PM +0100, Pino Toscano wrote:
> Read also the secrets associated to disks ( tag within ),
> so qemu can properly open them later on.
Looks good, ACK.
Rich.
> src/libvirt-domain.c | 130
> +++
> 1 file changed, 122 insertions(+), 8 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index baab307..696a264 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -33,6 +33,8 @@
> #include
> #include
>
> +#include "base64.h"
> +
> #include "guestfs.h"
> #include "guestfs-internal.h"
> #include "guestfs-internal-actions.h"
> @@ -40,7 +42,7 @@
> #if defined(HAVE_LIBVIRT)
>
> static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom);
> -static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr
> doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int
> readonly, const char *protocol, char *const *server, const char *username,
> void *data), void *data);
> +static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr
> doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int
> readonly, const char *protocol, char *const *server, const char *username,
> const char *secret, void *data), void *data);
> static int libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc, char
> **label_rtn, char **imagelabel_rtn);
> static char *filename_from_pool (guestfs_h *g, virConnectPtr conn, const
> char *pool_nane, const char *volume_name);
> static bool xPathObjectIsEmpty (xmlXPathObjectPtr obj);
> @@ -95,8 +97,12 @@ guestfs_impl_add_domain (guestfs_h *g, const char
> *domain_name,
> return -1;
>}
>
> - /* Connect to libvirt, find the domain. */
> - conn = guestfs_int_open_libvirt_connection (g, libvirturi, VIR_CONNECT_RO);
> + /* Connect to libvirt, find the domain. We cannot open the connection
> + * in read-only mode (VIR_CONNECT_RO), as that kind of connection
> + * is considered untrusted, and thus libvirt will prevent to read
> + * the values of secrets.
> + */
> + conn = guestfs_int_open_libvirt_connection (g, libvirturi, 0);
>if (!conn) {
> err = virGetLastError ();
> error (g, _("could not connect to libvirt (code %d, domain %d): %s"),
> @@ -163,7 +169,7 @@ guestfs_impl_add_domain (guestfs_h *g, const char
> *domain_name,
>return r;
> }
>
> -static int add_disk (guestfs_h *g, const char *filename, const char *format,
> int readonly, const char *protocol, char *const *server, const char
> *username, void *data);
> +static int add_disk (guestfs_h *g, const char *filename, const char *format,
> int readonly, const char *protocol, char *const *server, const char
> *username, const char *secret, void *data);
> static int connect_live (guestfs_h *g, virDomainPtr dom);
>
> enum readonlydisk {
> @@ -325,7 +331,7 @@ static int
> add_disk (guestfs_h *g,
>const char *filename, const char *format, int readonly_in_xml,
>const char *protocol, char *const *server, const char *username,
> - void *datavp)
> + const char *secret, void *datavp)
> {
>struct add_disk_data *data = datavp;
>/* Copy whole struct so we can make local changes: */
> @@ -382,6 +388,10 @@ add_disk (guestfs_h *g,
> optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_USERNAME_BITMASK;
> optargs.username = username;
>}
> + if (secret) {
> +optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK;
> +optargs.secret = secret;
> + }
>
>return guestfs_add_drive_opts_argv (g, filename, );
> }
> @@ -475,7 +485,7 @@ for_each_disk (guestfs_h *g,
> const char *filename, const char *format,
> int readonly,
> const char *protocol, char *const *server,
> - const char *username,
> + const char *username, const char *secret,
> void *data),
> void *data)
> {
> @@ -504,7 +514,7 @@ for_each_disk (guestfs_h *g,
>if (nodes != NULL) {
> nr_nodes = nodes->nodeNr;
> for (i = 0; i < nr_nodes; ++i) {
> - CLEANUP_FREE char *type = NULL, *filename = NULL, *format = NULL,
> *protocol = NULL, *username = NULL;
> + CLEANUP_FREE char *type = NULL, *filename = NULL, *format = NULL,
> *protocol = NULL, *username = NULL, *secret = NULL;
>CLEANUP_FREE_STRING_LIST char **server = NULL;
>CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xptype = NULL;
>CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpformat = NULL;
> @@ -517,6 +527,7 @@ for_each_disk (guestfs_h *g,
>CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpvolume = NULL;
>int readonly;
>int t;
> + virErrorPtr err;
>
>/* Change the context to the current node.
> * DV advises to reset this before each search since older versions of
> @@ -569,8 +580,111
[Libguestfs] [PATCH 2/2] libvirt: read secrets of disks (RHBZ#1392798)
Read also the secrets associated to disks ( tag within ),
so qemu can properly open them later on.
---
src/libvirt-domain.c | 130 +++
1 file changed, 122 insertions(+), 8 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index baab307..696a264 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -33,6 +33,8 @@
#include
#include
+#include "base64.h"
+
#include "guestfs.h"
#include "guestfs-internal.h"
#include "guestfs-internal-actions.h"
@@ -40,7 +42,7 @@
#if defined(HAVE_LIBVIRT)
static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom);
-static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr doc,
int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly,
const char *protocol, char *const *server, const char *username, void *data),
void *data);
+static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr doc,
int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly,
const char *protocol, char *const *server, const char *username, const char
*secret, void *data), void *data);
static int libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc, char
**label_rtn, char **imagelabel_rtn);
static char *filename_from_pool (guestfs_h *g, virConnectPtr conn, const char
*pool_nane, const char *volume_name);
static bool xPathObjectIsEmpty (xmlXPathObjectPtr obj);
@@ -95,8 +97,12 @@ guestfs_impl_add_domain (guestfs_h *g, const char
*domain_name,
return -1;
}
- /* Connect to libvirt, find the domain. */
- conn = guestfs_int_open_libvirt_connection (g, libvirturi, VIR_CONNECT_RO);
+ /* Connect to libvirt, find the domain. We cannot open the connection
+ * in read-only mode (VIR_CONNECT_RO), as that kind of connection
+ * is considered untrusted, and thus libvirt will prevent to read
+ * the values of secrets.
+ */
+ conn = guestfs_int_open_libvirt_connection (g, libvirturi, 0);
if (!conn) {
err = virGetLastError ();
error (g, _("could not connect to libvirt (code %d, domain %d): %s"),
@@ -163,7 +169,7 @@ guestfs_impl_add_domain (guestfs_h *g, const char
*domain_name,
return r;
}
-static int add_disk (guestfs_h *g, const char *filename, const char *format,
int readonly, const char *protocol, char *const *server, const char *username,
void *data);
+static int add_disk (guestfs_h *g, const char *filename, const char *format,
int readonly, const char *protocol, char *const *server, const char *username,
const char *secret, void *data);
static int connect_live (guestfs_h *g, virDomainPtr dom);
enum readonlydisk {
@@ -325,7 +331,7 @@ static int
add_disk (guestfs_h *g,
const char *filename, const char *format, int readonly_in_xml,
const char *protocol, char *const *server, const char *username,
- void *datavp)
+ const char *secret, void *datavp)
{
struct add_disk_data *data = datavp;
/* Copy whole struct so we can make local changes: */
@@ -382,6 +388,10 @@ add_disk (guestfs_h *g,
optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_USERNAME_BITMASK;
optargs.username = username;
}
+ if (secret) {
+optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK;
+optargs.secret = secret;
+ }
return guestfs_add_drive_opts_argv (g, filename, );
}
@@ -475,7 +485,7 @@ for_each_disk (guestfs_h *g,
const char *filename, const char *format,
int readonly,
const char *protocol, char *const *server,
- const char *username,
+ const char *username, const char *secret,
void *data),
void *data)
{
@@ -504,7 +514,7 @@ for_each_disk (guestfs_h *g,
if (nodes != NULL) {
nr_nodes = nodes->nodeNr;
for (i = 0; i < nr_nodes; ++i) {
- CLEANUP_FREE char *type = NULL, *filename = NULL, *format = NULL,
*protocol = NULL, *username = NULL;
+ CLEANUP_FREE char *type = NULL, *filename = NULL, *format = NULL,
*protocol = NULL, *username = NULL, *secret = NULL;
CLEANUP_FREE_STRING_LIST char **server = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xptype = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpformat = NULL;
@@ -517,6 +527,7 @@ for_each_disk (guestfs_h *g,
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpvolume = NULL;
int readonly;
int t;
+ virErrorPtr err;
/* Change the context to the current node.
* DV advises to reset this before each search since older versions of
@@ -569,8 +580,111 @@ for_each_disk (guestfs_h *g,
xpusername = xmlXPathEvalExpression (BAD_CAST "./auth/@username",
xpathCtx);
if (!xPathObjectIsEmpty (xpusername)) {
+ CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpsecrettype = NULL;
+
