[libvirt] [PATCH 7/8] qemu: add support for encrypted VNC TLS keys

Ján Tomko posted 8 patches 5 years, 10 months ago
[libvirt] [PATCH 7/8] qemu: add support for encrypted VNC TLS keys
Posted by Ján Tomko 5 years, 10 months ago
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@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..e17d7ddec7 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 && 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 6960f0569b..da9c4e566d 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);
 }
 
 
@@ -1750,6 +1751,7 @@ qemuDomainSecretGraphicsDestroy(virDomainGraphicsDefPtr graphics)
         return;
 
     VIR_FREE(gfxPriv->tlsAlias);
+    qemuDomainSecretInfoFree(&gfxPriv->secinfo);
 }
 
 
@@ -1773,6 +1775,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 01e47996f5..e706ddca31 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -490,6 +490,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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/8] qemu: add support for encrypted VNC TLS keys
Posted by John Ferlan 5 years, 10 months ago

On 1/16/19 2:41 AM, Ján Tomko wrote:
> 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@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..e17d7ddec7 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 && gfxPriv->secinfo) {

"gfxPriv" check is unnecessary, we would have already died dereffing
tlsAlias.

> +                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 6960f0569b..da9c4e566d 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);>  }
>  
>  
> @@ -1750,6 +1751,7 @@ qemuDomainSecretGraphicsDestroy(virDomainGraphicsDefPtr graphics)
>          return;
>  
>      VIR_FREE(gfxPriv->tlsAlias);
> +    qemuDomainSecretInfoFree(&gfxPriv->secinfo);

If you use virObjectUnref as noted in patch4, then the change in the
hunk above gives you this for free.

>  }
>  
>  
> @@ -1773,6 +1775,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;
>  }
>  

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list