Allow enabling TLS for the NBD server used to do pull-mode backups. Note
that documentation already mentions 'tls', so this just implements the
schema and XML bits.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 docs/schemas/domainbackup.rng                   |  9 ++++++++-
 src/conf/backup_conf.c                          | 17 +++++++++++++++++
 src/conf/backup_conf.h                          |  1 +
 .../backup-pull-encrypted.xml                   |  2 +-
 .../backup-pull-internal-invalid.xml            |  2 +-
 .../backup-pull-encrypted.xml                   |  2 +-
 6 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
index 650f5cd4c3..c0ca3c3038 100644
--- a/docs/schemas/domainbackup.rng
+++ b/docs/schemas/domainbackup.rng
@@ -51,6 +51,14 @@
             </attribute>
             <interleave>
               <element name='server'>
+                <optional>
+                  <attribute name='tls'>
+                    <choice>
+                      <value>yes</value>
+                      <value>no</value>
+                    </choice>
+                  </attribute>
+                </optional>
                 <choice>
                   <group>
                     <optional>
@@ -69,7 +77,6 @@
                         <ref name='unsignedInt'/>
                       </attribute>
                     </optional>
-                    <!-- add tls? -->
                   </group>
                   <group>
                     <attribute name='transport'>
diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
index 74f6e4b020..59d7e1dfaf 100644
--- a/src/conf/backup_conf.c
+++ b/src/conf/backup_conf.c
@@ -260,6 +260,8 @@ virDomainBackupDefParse(xmlXPathContextPtr ctxt,
     def->incremental = virXPathString("string(./incremental)", ctxt);

     if ((node = virXPathNode("./server", ctxt))) {
+        g_autofree char *tls = NULL;
+
         if (def->type != VIR_DOMAIN_BACKUP_TYPE_PULL) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("use of <server> requires pull mode backup"));
@@ -284,6 +286,19 @@ virDomainBackupDefParse(xmlXPathContextPtr ctxt,
                            def->server->socket);
             return NULL;
         }
+
+        if ((tls = virXMLPropString(node, "tls"))) {
+            int tmp;
+
+            if ((tmp = virTristateBoolTypeFromString(tls)) <= 0) {
+                virReportError(VIR_ERR_XML_ERROR,
+                               _("unknown value '%s' of 'tls' attribute"),\
+                               tls);
+                return NULL;
+            }
+
+            def->tls = tmp;
+        }
     }

     if ((n = virXPathNodeSet("./disks/*", ctxt, &nodes)) < 0)
@@ -445,6 +460,8 @@ virDomainBackupDefFormat(virBufferPtr buf,
     if (def->server) {
         virBufferAsprintf(&serverAttrBuf, " transport='%s'",
                           virStorageNetHostTransportTypeToString(def->server->transport));
+        if (def->tls != VIR_TRISTATE_BOOL_ABSENT)
+            virBufferAsprintf(&serverAttrBuf, " tls='%s'", virTristateBoolTypeToString(def->tls));
         virBufferEscapeString(&serverAttrBuf, " name='%s'", def->server->name);
         if (def->server->port)
             virBufferAsprintf(&serverAttrBuf, " port='%u'", def->server->port);
diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h
index a1d1e453c1..bda2bdcfe4 100644
--- a/src/conf/backup_conf.h
+++ b/src/conf/backup_conf.h
@@ -81,6 +81,7 @@ struct _virDomainBackupDef {
     int type; /* virDomainBackupType */
     char *incremental;
     virStorageNetHostDefPtr server; /* only when type == PULL */
+    virTristateBool tls; /* use TLS for NBD */

     size_t ndisks; /* should not exceed dom->ndisks */
     virDomainBackupDiskDef *disks;
diff --git a/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml b/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml
index 1469189a37..48232aa0fe 100644
--- a/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml
+++ b/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml
@@ -1,6 +1,6 @@
 <domainbackup mode="pull">
   <incremental>1525889631</incremental>
-  <server transport='tcp' name='localhost' port='10809'/>
+  <server transport='tcp' tls='yes' name='localhost' port='10809'/>
   <disks>
     <disk name='vda' type='file' exportname='test-vda' exportbitmap='blah'>
       <driver type='qcow2'/>
diff --git a/tests/domainbackupxml2xmlin/backup-pull-internal-invalid.xml b/tests/domainbackupxml2xmlin/backup-pull-internal-invalid.xml
index 261dec0eea..ba8f7ca3ab 100644
--- a/tests/domainbackupxml2xmlin/backup-pull-internal-invalid.xml
+++ b/tests/domainbackupxml2xmlin/backup-pull-internal-invalid.xml
@@ -1,6 +1,6 @@
 <domainbackup mode='pull'>
   <incremental>1525889631</incremental>
-  <server transport='tcp' name='localhost' port='10809'/>
+  <server transport='tcp' tls='yes' name='localhost' port='10809'/>
   <disks>
     <disk name='vda' backup='yes' state='running' type='file' exportname='test-vda' exportbitmap='blah'>
       <driver type='qcow2'/>
diff --git a/tests/domainbackupxml2xmlout/backup-pull-encrypted.xml b/tests/domainbackupxml2xmlout/backup-pull-encrypted.xml
index 81519bfcb5..ea9dcf72b9 100644
--- a/tests/domainbackupxml2xmlout/backup-pull-encrypted.xml
+++ b/tests/domainbackupxml2xmlout/backup-pull-encrypted.xml
@@ -1,6 +1,6 @@
 <domainbackup mode='pull'>
   <incremental>1525889631</incremental>
-  <server transport='tcp' name='localhost' port='10809'/>
+  <server transport='tcp' tls='yes' name='localhost' port='10809'/>
   <disks>
     <disk name='vda' backup='yes' type='file' exportname='test-vda' exportbitmap='blah'>
       <driver type='qcow2'/>
-- 
2.26.2

On 7/2/20 9:40 AM, Peter Krempa wrote:
> Allow enabling TLS for the NBD server used to do pull-mode backups. Note
> that documentation already mentions 'tls', so this just implements the
> schema and XML bits.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---

> +++ b/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml
> @@ -1,6 +1,6 @@
>   <domainbackup mode="pull">
>     <incremental>1525889631</incremental>
> -  <server transport='tcp' name='localhost' port='10809'/>
> +  <server transport='tcp' tls='yes' name='localhost' port='10809'/>

So this doesn't say what files are actually feeding the TLS 
configuration; the docs already mentioned 'tls', but do we need to add a 
cross-reference that states when tls='yes' is in effect then the server 
uses the files as configured in qemu.conf?  Knowing how the server is 
keyed is important for writing a client that can connect over TLS to the 
server.

But the overall idea makes sense.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On Thu, Jul 02, 2020 at 14:53:28 -0500, Eric Blake wrote:
> On 7/2/20 9:40 AM, Peter Krempa wrote:
> > Allow enabling TLS for the NBD server used to do pull-mode backups. Note
> > that documentation already mentions 'tls', so this just implements the
> > schema and XML bits.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> 
> > +++ b/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml
> > @@ -1,6 +1,6 @@
> >   <domainbackup mode="pull">
> >     <incremental>1525889631</incremental>
> > -  <server transport='tcp' name='localhost' port='10809'/>
> > +  <server transport='tcp' tls='yes' name='localhost' port='10809'/>
> 
> So this doesn't say what files are actually feeding the TLS configuration;
> the docs already mentioned 'tls', but do we need to add a cross-reference
> that states when tls='yes' is in effect then the server uses the files as
> configured in qemu.conf?  Knowing how the server is keyed is important for
> writing a client that can connect over TLS to the server.

Note that patch 22 actually adds the following paragraph to
formatbackup.rst into the NBD section:

+   Note that for the QEMU hypervisor the TLS environment in controlled using
+   ``backup_tls_x509_cert_dir``, ``backup_tls_x509_verify``, and
+   ``backup_tls_x509_secret_uuid`` properties in ``/etc/libvirt/qemu.conf``.


> But the overall idea makes sense.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>