1:59 p.m.
For:
https://bugzilla.redhat.com/show_bug.cgi?id=1602418
v1:
https://www.redhat.com/archives/libvir-list/2019-January/msg00490.html
v2:
* fixed memory leaks pointed out by John in v1
Patches without R-b:
[4/8] cowardly refused to unref the private data in Secret.*Destroy
[8/8] fixed the logic to include checking whether the domain actually
has a VNC graphics and amended the commit message
Ján Tomko (8):
conf: introduce virDomainGraphicsNew
conf: add privateData to virDomainGraphicsDef
qemu: add qemuDomainGraphicsPrivate data with a tlsAlias
qemu: prepare secret for the graphics upfront
qemu_process: fix debug message
qemu.conf: add vnc_tls_x509_secret_uuid
qemu: add support for encrypted VNC TLS keys
qemu: error out when vnc vncTLSx509secretUUID is unsupported
src/conf/domain_conf.c | 30 ++++-
src/conf/domain_conf.h | 5 +
src/qemu/libvirtd_qemu.aug | 1 +
src/qemu/qemu.conf | 6 +
src/qemu/qemu_command.c | 19 +++-
src/qemu/qemu_conf.c | 4 +
src/qemu/qemu_conf.h | 1 +
src/qemu/qemu_domain.c | 107 ++++++++++++++++++
src/qemu/qemu_domain.h | 13 +++
src/qemu/qemu_process.c | 2 +-
src/qemu/test_libvirtd_qemu.aug.in | 1 +
...graphics-vnc-tls-secret.x86_64-latest.args | 36 ++++++
.../graphics-vnc-tls-secret.xml | 30 +++++
tests/qemuxml2argvtest.c | 5 +
14 files changed, 250 insertions(+), 10 deletions(-)
create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls-secret.xml
--
2.20.1
1:59 p.m.
New subject: [libvirt] [PATCHv2 1/8] conf: introduce virDomainGraphicsNew
A helper function for allocating the virDomainGraphicsDef structure.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/domain_conf.c | 23 +++++++++++++++++++----
src/conf/domain_conf.h | 2 ++
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 222bb8c482..761f9bffef 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14115,9 +14115,22 @@ virDomainGraphicsDefParseXMLEGLHeadless(virDomainGraphicsDefPtr
def,
}
+virDomainGraphicsDefPtr
+virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED)
+{
+ virDomainGraphicsDefPtr def = NULL;
+
+ if (VIR_ALLOC(def) < 0)
+ return NULL;
+
+ return def;
+}
+
+
/* Parse the XML definition for a graphics device */
static virDomainGraphicsDefPtr
-virDomainGraphicsDefParseXML(xmlNodePtr node,
+virDomainGraphicsDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr node,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
@@ -14125,7 +14138,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
char *type = NULL;
int typeVal;
- if (VIR_ALLOC(def) < 0)
+ if (!(def = virDomainGraphicsDefNew(xmlopt)))
return NULL;
type = virXMLPropString(node, "type");
@@ -16237,7 +16250,8 @@ virDomainDeviceDefParse(const char *xmlStr,
goto error;
break;
case VIR_DOMAIN_DEVICE_GRAPHICS:
- if (!(dev->data.graphics = virDomainGraphicsDefParseXML(node, ctxt, flags)))
+ if (!(dev->data.graphics = virDomainGraphicsDefParseXML(xmlopt, node,
+ ctxt, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_HUB:
@@ -20847,7 +20861,8 @@ virDomainDefParseXML(xmlDocPtr xml,
if (n && VIR_ALLOC_N(def->graphics, n) < 0)
goto error;
for (i = 0; i < n; i++) {
- virDomainGraphicsDefPtr graphics = virDomainGraphicsDefParseXML(nodes[i],
+ virDomainGraphicsDefPtr graphics = virDomainGraphicsDefParseXML(xmlopt,
+ nodes[i],
ctxt,
flags);
if (!graphics)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index fae130668f..2a97ad8ab3 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2954,6 +2954,8 @@ virDomainChrSourceDefNew(virDomainXMLOptionPtr xmlopt);
virDomainChrDefPtr virDomainChrDefNew(virDomainXMLOptionPtr xmlopt);
+virDomainGraphicsDefPtr
+virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt);
virDomainDefPtr virDomainDefNew(void);
void virDomainObjAssignDef(virDomainObjPtr domain,
--
2.20.1
1:59 p.m.
New subject: [libvirt] [PATCHv2 2/8] conf: add privateData to virDomainGraphicsDef
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/domain_conf.c | 9 ++++++++-
src/conf/domain_conf.h | 3 +++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 761f9bffef..54d6364f4f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1431,6 +1431,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
virDomainGraphicsListenDefClear(&def->listens[i]);
VIR_FREE(def->listens);
+ virObjectUnref(def->privateData);
VIR_FREE(def);
}
@@ -14116,13 +14117,19 @@ virDomainGraphicsDefParseXMLEGLHeadless(virDomainGraphicsDefPtr
def,
virDomainGraphicsDefPtr
-virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED)
+virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt)
{
virDomainGraphicsDefPtr def = NULL;
if (VIR_ALLOC(def) < 0)
return NULL;
+ if (xmlopt && xmlopt->privateData.graphicsNew &&
+ !(def->privateData = xmlopt->privateData.graphicsNew())) {
+ VIR_FREE(def);
+ def = NULL;
+ }
+
return def;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 2a97ad8ab3..7776a3afb2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1603,6 +1603,8 @@ struct _virDomainGraphicsListenDef {
};
struct _virDomainGraphicsDef {
+ virObjectPtr privateData;
+
/* Port value discipline:
* Value -1 is legacy syntax indicating that it should be auto-allocated.
* Value 0 means port wasn't specified in XML at all.
@@ -2783,6 +2785,7 @@ struct _virDomainXMLPrivateDataCallbacks {
virDomainXMLPrivateDataNewFunc vcpuNew;
virDomainXMLPrivateDataNewFunc chrSourceNew;
virDomainXMLPrivateDataNewFunc vsockNew;
+ virDomainXMLPrivateDataNewFunc graphicsNew;
virDomainXMLPrivateDataFormatFunc format;
virDomainXMLPrivateDataParseFunc parse;
/* following function shall return a pointer which will be used as the
--
2.20.1
1:59 p.m.
New subject: [libvirt] [PATCHv2 3/8] qemu: add qemuDomainGraphicsPrivate data with a tlsAlias
Also introduce the necessary callbacks.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_domain.c | 39 +++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 12 ++++++++++++
2 files changed, 51 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e07c1646f1..4b11cba1bd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1234,6 +1234,44 @@ qemuDomainVsockPrivateDispose(void *obj ATTRIBUTE_UNUSED)
}
+static virClassPtr qemuDomainGraphicsPrivateClass;
+static void qemuDomainGraphicsPrivateDispose(void *obj);
+
+static int
+qemuDomainGraphicsPrivateOnceInit(void)
+{
+ if (!VIR_CLASS_NEW(qemuDomainGraphicsPrivate, virClassForObject()))
+ return -1;
+
+ return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(qemuDomainGraphicsPrivate)
+
+static virObjectPtr
+qemuDomainGraphicsPrivateNew(void)
+{
+ qemuDomainGraphicsPrivatePtr priv;
+
+ if (qemuDomainGraphicsPrivateInitialize() < 0)
+ return NULL;
+
+ if (!(priv = virObjectNew(qemuDomainGraphicsPrivateClass)))
+ return NULL;
+
+ return (virObjectPtr) priv;
+}
+
+
+static void
+qemuDomainGraphicsPrivateDispose(void *obj)
+{
+ qemuDomainGraphicsPrivatePtr priv = obj;
+
+ VIR_FREE(priv->tlsAlias);
+}
+
+
/* qemuDomainSecretPlainSetup:
* @secinfo: Pointer to secret info
* @usageType: The virSecretUsageType
@@ -3020,6 +3058,7 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks =
{
.vcpuNew = qemuDomainVcpuPrivateNew,
.chrSourceNew = qemuDomainChrSourcePrivateNew,
.vsockNew = qemuDomainVsockPrivateNew,
+ .graphicsNew = qemuDomainGraphicsPrivateNew,
.parse = qemuDomainObjPrivateXMLParse,
.format = qemuDomainObjPrivateXMLFormat,
.getParseOpaque = qemuDomainObjPrivateXMLGetParseOpaque,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 0119de515a..6df355fe78 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -476,6 +476,18 @@ struct _qemuDomainVsockPrivate {
};
+# define QEMU_DOMAIN_GRAPHICS_PRIVATE(dev) \
+ ((qemuDomainGraphicsPrivatePtr) (dev)->privateData)
+
+typedef struct _qemuDomainGraphicsPrivate qemuDomainGraphicsPrivate;
+typedef qemuDomainGraphicsPrivate *qemuDomainGraphicsPrivatePtr;
+struct _qemuDomainGraphicsPrivate {
+ virObject parent;
+
+ char *tlsAlias;
+};
+
+
typedef enum {
QEMU_PROCESS_EVENT_WATCHDOG = 0,
QEMU_PROCESS_EVENT_GUESTPANIC,
--
2.20.1
1:59 p.m.
New subject: [libvirt] [PATCHv2 4/8] qemu: prepare secret for the graphics upfront
Instead of hardcoding the TLS creds alias in
qemuBuildGraphicsVNCCommandLine, store it
in the domain private data.
Given that we only support one VNC graphics
and thus have only one alias per-domain,
this is overengineered, but it will allow us
to prepare the secret upfront when we start
supporting encrypted server TLS keys.
Note that the alias is not formatted anywhere
since we won't need to access it after domain
startup.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 8 ++++----
src/qemu/qemu_domain.c | 44 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 822d5f8669..d130d0463c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8035,18 +8035,18 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
virBufferAddLit(&opt, ",password");
if (cfg->vncTLS) {
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) {
- const char *alias = "vnc-tls-creds0";
+ qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics);
+ if (gfxPriv->tlsAlias) {
if (qemuBuildTLSx509CommandLine(cmd,
cfg->vncTLSx509certdir,
true,
cfg->vncTLSx509verify,
NULL,
- alias,
+ gfxPriv->tlsAlias,
qemuCaps) < 0)
goto error;
- virBufferAsprintf(&opt, ",tls-creds=%s", alias);
+ virBufferAsprintf(&opt, ",tls-creds=%s",
gfxPriv->tlsAlias);
} else {
virBufferAddLit(&opt, ",tls");
if (cfg->vncTLSx509verify) {
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4b11cba1bd..b35c217d65 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1726,6 +1726,42 @@ qemuDomainSecretChardevPrepare(virQEMUDriverConfigPtr cfg,
}
+static void
+qemuDomainSecretGraphicsDestroy(virDomainGraphicsDefPtr graphics)
+{
+ qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics);
+
+ if (!gfxPriv)
+ return;
+
+ VIR_FREE(gfxPriv->tlsAlias);
+}
+
+
+static int
+qemuDomainSecretGraphicsPrepare(virQEMUDriverConfigPtr cfg,
+ qemuDomainObjPrivatePtr priv,
+ virDomainGraphicsDefPtr graphics)
+{
+ virQEMUCapsPtr qemuCaps = priv->qemuCaps;
+ qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics);
+
+ if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC)
+ return 0;
+
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509))
+ return 0;
+
+ if (!cfg->vncTLS)
+ return 0;
+
+ if (VIR_STRDUP(gfxPriv->tlsAlias, "vnc-tls-creds0") < 0)
+ return -1;
+
+ return 0;
+}
+
+
/* qemuDomainSecretDestroy:
* @vm: Domain object
*
@@ -1767,6 +1803,9 @@ qemuDomainSecretDestroy(virDomainObjPtr vm)
for (i = 0; i < vm->def->nredirdevs; i++)
qemuDomainSecretChardevDestroy(vm->def->redirdevs[i]->source);
+
+ for (i = 0; i < vm->def->ngraphics; i++)
+ qemuDomainSecretGraphicsDestroy(vm->def->graphics[i]);
}
@@ -1850,6 +1889,11 @@ qemuDomainSecretPrepare(virQEMUDriverPtr driver,
goto cleanup;
}
+ for (i = 0; i < vm->def->ngraphics; i++) {
+ if (qemuDomainSecretGraphicsPrepare(cfg, priv, vm->def->graphics[i]) <
0)
+ goto cleanup;
+ }
+
ret = 0;
cleanup:
--
2.20.1
12:14 a.m.
New subject: [libvirt] [PATCHv2 4/8] qemu: prepare secret for the graphics upfront
On 1/21/19 7:59 AM, Ján Tomko wrote:
Instead of hardcoding the TLS creds alias in
qemuBuildGraphicsVNCCommandLine, store it
in the domain private data.
Given that we only support one VNC graphics
and thus have only one alias per-domain,
this is overengineered, but it will allow us
to prepare the secret upfront when we start
supporting encrypted server TLS keys.
Note that the alias is not formatted anywhere
since we won't need to access it after domain
startup.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 8 ++++----
src/qemu/qemu_domain.c | 44 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 4 deletions(-)
Rather than respond in v1... Irony is:
sizeof(*gfxPriv) = 24
sizeof("vnc-tls-creds0") = 15
and the reason for the free during GraphicsDestroy is "to not have the
temporary data allocated during the whole time the domain is running",
but the algorithm leaves privateData allocated for every graphics device
and not just VNC one's.
It's used once and doesn't need to stick around. Of course who knows
what the future brings and the algorithm essentially follows storage and
chardev source (even though they are each a bit different).
Since it's not possible to have a struct with just "virObject parent;"
alone, an alternative is to have a "bool useTLSCreds;" and then continue
with the use of "const char *alias = "vnc-tls-creds0";" in
qemuBuildGraphicsVNCCommandLine.
Then again you did say overengineered above, so even though I may have
done things differently, what you have isn't wrong, so consider it,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
1:59 p.m.
New subject: [libvirt] [PATCHv2 5/8] qemu_process: fix debug message
Be generic instead of trying to enumerate all the involved
device types.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_process.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 855bd9cb14..4f45773dbf 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6051,7 +6051,7 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
VIR_DEBUG("Prepare chardev source backends for TLS");
qemuDomainPrepareChardevSource(vm->def, cfg);
- VIR_DEBUG("Add secrets to hostdevs and chardevs");
+ VIR_DEBUG("Prepare device secrets");
if (qemuDomainSecretPrepare(driver, vm) < 0)
goto cleanup;
--
2.20.1
1:59 p.m.
New subject: [libvirt] [PATCHv2 6/8] qemu.conf: add vnc_tls_x509_secret_uuid
Add an option that lets the user specify the secret
that unlocks the server TLS key.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/libvirtd_qemu.aug | 1 +
src/qemu/qemu.conf | 6 ++++++
src/qemu/qemu_conf.c | 4 ++++
src/qemu/qemu_conf.h | 1 +
src/qemu/test_libvirtd_qemu.aug.in | 1 +
5 files changed, 13 insertions(+)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 28bd851411..b311f02da6 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -35,6 +35,7 @@ module Libvirtd_qemu =
| bool_entry "vnc_auto_unix_socket"
| bool_entry "vnc_tls"
| str_entry "vnc_tls_x509_cert_dir"
+ | str_entry "vnc_tls_x509_secret_uuid"
| bool_entry "vnc_tls_x509_verify"
| str_entry "vnc_password"
| bool_entry "vnc_sasl"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 28e51b2c59..c1f1201134 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -95,6 +95,12 @@
#vnc_tls_x509_cert_dir = "/etc/pki/libvirt-vnc"
+# Uncomment and use the following option to override the default secret
+# UUID provided in the default_tls_x509_secret_uuid parameter.
+#
+#vnc_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000"
+
+
# The default TLS configuration only uses certificates for the server
# allowing the client to verify the server's identity and establish
# an encrypted channel.
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 256aad2c0b..1808fdd4cb 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -361,6 +361,7 @@ static void virQEMUDriverConfigDispose(void *obj)
VIR_FREE(cfg->defaultTLSx509secretUUID);
VIR_FREE(cfg->vncTLSx509certdir);
+ VIR_FREE(cfg->vncTLSx509secretUUID);
VIR_FREE(cfg->vncListen);
VIR_FREE(cfg->vncPassword);
VIR_FREE(cfg->vncSASLdir);
@@ -458,6 +459,8 @@ virQEMUDriverConfigLoadVNCEntry(virQEMUDriverConfigPtr cfg,
cfg->vncTLSx509verifyPresent = true;
if (virConfGetValueString(conf, "vnc_tls_x509_cert_dir",
&cfg->vncTLSx509certdir) < 0)
return -1;
+ if (virConfGetValueString(conf, "vnc_tls_x509_secret_uuid",
&cfg->vncTLSx509secretUUID) < 0)
+ return -1;
if (virConfGetValueString(conf, "vnc_listen", &cfg->vncListen) <
0)
return -1;
if (virConfGetValueString(conf, "vnc_password", &cfg->vncPassword)
< 0)
@@ -1189,6 +1192,7 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg)
} \
} while (0)
+ SET_TLS_SECRET_UUID_DEFAULT(vnc);
SET_TLS_SECRET_UUID_DEFAULT(chardev);
SET_TLS_SECRET_UUID_DEFAULT(migrate);
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index bce8364c5a..14c9d15a72 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -125,6 +125,7 @@ struct _virQEMUDriverConfig {
bool vncTLSx509verifyPresent;
bool vncSASL;
char *vncTLSx509certdir;
+ char *vncTLSx509secretUUID;
char *vncListen;
char *vncPassword;
char *vncSASLdir;
diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
index f1e8806ad2..4235464530 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -9,6 +9,7 @@ module Test_libvirtd_qemu =
{ "vnc_auto_unix_socket" = "1" }
{ "vnc_tls" = "1" }
{ "vnc_tls_x509_cert_dir" = "/etc/pki/libvirt-vnc" }
+{ "vnc_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000"
}
{ "vnc_tls_x509_verify" = "1" }
{ "vnc_password" = "XYZ12345" }
{ "vnc_sasl" = "1" }
--
2.20.1
1:59 p.m.
New subject: [libvirt] [PATCHv2 7/8] qemu: add support for encrypted VNC TLS keys
Use the password stored in the secret driver under
the uuid specified by the vnc_tls_x509_secret_uuid
option in qemu.conf.
https://bugzilla.redhat.com/show_bug.cgi?id=1602418
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 11 +++++-
src/qemu/qemu_domain.c | 9 +++++
src/qemu/qemu_domain.h | 1 +
...graphics-vnc-tls-secret.x86_64-latest.args | 36 +++++++++++++++++++
.../graphics-vnc-tls-secret.xml | 30 ++++++++++++++++
tests/qemuxml2argvtest.c | 5 +++
6 files changed, 91 insertions(+), 1 deletion(-)
create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls-secret.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d130d0463c..167b942196 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8037,11 +8037,20 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
if (cfg->vncTLS) {
qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics);
if (gfxPriv->tlsAlias) {
+ const char *secretAlias = NULL;
+
+ if (gfxPriv->secinfo) {
+ if (qemuBuildObjectSecretCommandLine(cmd,
+ gfxPriv->secinfo) < 0)
+ goto error;
+ secretAlias = gfxPriv->secinfo->s.aes.alias;
+ }
+
if (qemuBuildTLSx509CommandLine(cmd,
cfg->vncTLSx509certdir,
true,
cfg->vncTLSx509verify,
- NULL,
+ secretAlias,
gfxPriv->tlsAlias,
qemuCaps) < 0)
goto error;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b35c217d65..22d93d56f9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1269,6 +1269,7 @@ qemuDomainGraphicsPrivateDispose(void *obj)
qemuDomainGraphicsPrivatePtr priv = obj;
VIR_FREE(priv->tlsAlias);
+ qemuDomainSecretInfoFree(&priv->secinfo);
}
@@ -1735,6 +1736,7 @@ qemuDomainSecretGraphicsDestroy(virDomainGraphicsDefPtr graphics)
return;
VIR_FREE(gfxPriv->tlsAlias);
+ qemuDomainSecretInfoFree(&gfxPriv->secinfo);
}
@@ -1758,6 +1760,13 @@ qemuDomainSecretGraphicsPrepare(virQEMUDriverConfigPtr cfg,
if (VIR_STRDUP(gfxPriv->tlsAlias, "vnc-tls-creds0") < 0)
return -1;
+ if (cfg->vncTLSx509secretUUID) {
+ gfxPriv->secinfo = qemuDomainSecretInfoTLSNew(priv, gfxPriv->tlsAlias,
+ cfg->vncTLSx509secretUUID);
+ if (!gfxPriv->secinfo)
+ return -1;
+ }
+
return 0;
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 6df355fe78..defbffbf94 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -485,6 +485,7 @@ struct _qemuDomainGraphicsPrivate {
virObject parent;
char *tlsAlias;
+ qemuDomainSecretInfoPtr secinfo;
};
diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args
b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args
new file mode 100644
index 0000000000..737c4fe8fb
--- /dev/null
+++ b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args
@@ -0,0 +1,36 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+SASL_CONF_PATH=/root/.sasl2 \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
+-object secret,id=vnc-tls-creds0-secret0,\
+data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
+keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
+-object tls-creds-x509,id=vnc-tls-creds0,dir=/etc/pki/libvirt-vnc,\
+endpoint=server,verify-peer=yes,passwordid=vnc-tls-creds0-secret0 \
+-vnc 127.0.0.1:3,tls-creds=vnc-tls-creds0,sasl \
+-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls-secret.xml
b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.xml
new file mode 100644
index 0000000000..079f6241c4
--- /dev/null
+++ b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.xml
@@ -0,0 +1,30 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-i686</emulator>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <graphics type='vnc' port='5903' autoport='no'
listen='127.0.0.1'>
+ <listen type='address' address='127.0.0.1'/>
+ </graphics>
+ <video>
+ <model type='cirrus' vram='16384' heads='1'/>
+ </video>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 2cb8860d26..ba6fd4db35 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1290,6 +1290,11 @@ mymain(void)
DO_TEST("graphics-vnc-tls", QEMU_CAPS_VNC, QEMU_CAPS_DEVICE_CIRRUS_VGA);
DO_TEST_CAPS_VER("graphics-vnc-tls", "2.4.0");
DO_TEST_CAPS_LATEST("graphics-vnc-tls");
+ if (VIR_STRDUP_QUIET(driver.config->vncTLSx509secretUUID,
+ "6fd3f62d-9fe7-4a4e-a869-7acd6376d8ea") < 0)
+ return EXIT_FAILURE;
+ DO_TEST_CAPS_LATEST("graphics-vnc-tls-secret");
+ VIR_FREE(driver.config->vncTLSx509secretUUID);
driver.config->vncSASL = driver.config->vncTLSx509verify =
driver.config->vncTLS = 0;
VIR_FREE(driver.config->vncSASLdir);
VIR_FREE(driver.config->vncTLSx509certdir);
--
2.20.1
1:59 p.m.
New subject: [libvirt] [PATCHv2 8/8] qemu: error out when vnc vncTLSx509secretUUID is unsupported
Add a capability check to qemuDomainDefValidate and refuse to start
a domain with VNC graphics if the TLS secret was set in qemu.conf
and it's not supported.
Note that qemuDomainSecretGraphicsPrepare does not generate any
secret data if the capability is not present and qemuBuildTLSx509BackendProps
is not called at all.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_domain.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 22d93d56f9..32a43f2064 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4112,8 +4112,10 @@ qemuDomainDefValidate(const virDomainDef *def,
void *opaque)
{
virQEMUDriverPtr driver = opaque;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virQEMUCapsPtr qemuCaps = NULL;
int ret = -1;
+ size_t i;
if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
def->emulator)))
@@ -4234,10 +4236,23 @@ qemuDomainDefValidate(const virDomainDef *def,
if (qemuDomainDefValidateMemory(def, qemuCaps) < 0)
goto cleanup;
+ if (cfg->vncTLS && cfg->vncTLSx509secretUUID &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) {
+ for (i = 0; i < def->ngraphics; i++) {
+ if (def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("encrypted VNC TLS keys are not supported with
"
+ "this QEMU binary"));
+ goto cleanup;
+ }
+ }
+ }
+
ret = 0;
cleanup:
virObjectUnref(qemuCaps);
+ virObjectUnref(cfg);
return ret;
}
--
2.20.1
12:15 a.m.
New subject: [libvirt] [PATCHv2 8/8] qemu: error out when vnc vncTLSx509secretUUID is unsupported
On 1/21/19 7:59 AM, Ján Tomko wrote:
Add a capability check to qemuDomainDefValidate and refuse to start
a domain with VNC graphics if the TLS secret was set in qemu.conf
and it's not supported.
Note that qemuDomainSecretGraphicsPrepare does not generate any
secret data if the capability is not present and qemuBuildTLSx509BackendProps
is not called at all.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_domain.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
Some day I see a patch to create a qemuDomainDefValidateGraphics being
created ;-)
Whether you add the NB below or not,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 22d93d56f9..32a43f2064 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4112,8 +4112,10 @@ qemuDomainDefValidate(const virDomainDef *def,
void *opaque)
{
virQEMUDriverPtr driver = opaque;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virQEMUCapsPtr qemuCaps = NULL;
int ret = -1;
+ size_t i;
if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
def->emulator)))
@@ -4234,10 +4236,23 @@ qemuDomainDefValidate(const virDomainDef *def,
if (qemuDomainDefValidateMemory(def, qemuCaps) < 0)
goto cleanup;
/* NB: It is possible that vncTLS is set and we're using old style
* certificate processing without an X.509 object */
+ if (cfg->vncTLS && cfg->vncTLSx509secretUUID
&&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) {
+ for (i = 0; i < def->ngraphics; i++) {
+ if (def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("encrypted VNC TLS keys are not supported with
"
+ "this QEMU binary"));
+ goto cleanup;
+ }
+ }
+ }
+
ret = 0;
cleanup:
virObjectUnref(qemuCaps);
+ virObjectUnref(cfg);
return ret;
}
2069
days inactive
2069
days old
10 comments
2 participants
participants (2)
-
John Ferlan -
Ján Tomko