Re: [patch v2 1/1] virt-aa-helper: Add support for smartcard host-certificates

2020-02-09 Thread Christian Ehrhardt 
On Fri, Jan 24, 2020 at 2:15 PM Arnaud Patard  wrote:

> Christian Ehrhardt  writes:
>
> > On Thu, Dec 5, 2019 at 6:25 PM Arnaud Patard 
> wrote:
> >
> >> When emulating smartcard with host certificates, qemu needs to
> >> be able to read the certificates files. Add necessary code to
> >> add the smartcard certificates file path to the apparmor profile.
> >>
> >> Passthrough support has been tested with spicevmc and remote-viewer.
> >>
> >> v2:
> >> - Fix CodingStyle
> >> - Add support for 'host' case.
> >> - Add a comment to mention that the passthrough case doesn't need
> >>   some configuration
> >> - Use one rule with '{,*}' instead of two rules.
> >>
> >> Signed-off-by: Arnaud Patard 
> >> Index: libvirt/src/security/virt-aa-helper.c
> >> ===
> >> --- libvirt.orig/src/security/virt-aa-helper.c
> >> +++ libvirt/src/security/virt-aa-helper.c
> >> @@ -1271,6 +1271,39 @@ get_files(vahControl * ctl)
> >>  }
> >>  }
> >>
> >> +for (i = 0; i < ctl->def->nsmartcards; i++) {
> >> +virDomainSmartcardDefPtr sc = ctl->def->smartcards[i];
> >> +virDomainSmartcardType sc_type = sc->type;
> >> +char *sc_db = (char *)VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE;
> >> +if (sc->data.cert.database)
> >> +sc_db = sc->data.cert.database;
> >> +switch (sc_type) {
> >> +/*
> >> + * Note: At time of writing, to get this working, qemu
> >> seccomp sandbox has
> >> + * to be disabled or the host must be running QEMU with
> commit
> >> + * 9a1565a03b79d80b236bc7cc2dbce52a2ef3a1b8.
> >> + * It's possibly due to
> >> libcacard:vcard_emul_new_event_thread(), which calls
> >> + * PR_CreateThread(), which calls {g,s}etpriority(). And
> >> resourcecontrol seccomp
> >> + * filter forbids it (cf src/qemu/qemu_command.c which
> seems
> >> to always use
> >> + * resourcecontrol=deny).
> >> + */
> >> +case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
> >> +virBufferAddLit(, "  \"/etc/pki/nssdb/{,*}\"
> rk,\n");
> >>
> >
> > That path matches the examples in libvirt/qemu and also the fedora nss
> > package
> > [root@fedora~]# rpm -qf /etc/pki/nssdb/
> > nss-3.47.0-2.fc29.x86_64
> > [root@fedora ~]# ll /etc/pki/nssdb/
> > total 8
> > -rw-r--r-- 1 root root 65536 Jan  6  2017 cert8.db
> > -rw-r--r-- 1 root root  9216 Jan  6  2017 cert9.db
> > -rw-r--r-- 1 root root 16384 Jan  6  2017 key3.db
> > -rw-r--r-- 1 root root 11264 Jan  6  2017 key4.db
> > -rw-r--r-- 1 root root   451 Oct 23 11:23 pkcs11.txt
> > -rw-r--r-- 1 root root 16384 Jan  6  2017 secmod.db
> >
> > But on Debian/Ubuntu the paths are slightly different.
> > root@x:~# dpkg -L libnss3-nssdb
> > [...]
> > /var/lib/nssdb/key4.db
> > /var/lib/nssdb/cert9.db
> > /var/lib/nssdb/pkcs11.txt
> > /var/lib/nssdb/secmod.db
> >
> > Therefore I'd ask you to add that path as well here.
> >
>
> I don't remember seeing this /var/lib/nssdb path on my Debian. I don't
> even find the libnss3-nssdb package. Is it Ubuntu-specific and
> documented somewhere ?
>

Hmm, I was just checking paths on a multitude of containers and which
package it belongs.
After you wondered I checked more in detail - the package and the /var...
path are only in older releases.
So please feel free to ignore that part of my question.


> I can add this path, I've no problem with that. Nevertheless, I'm
> wondering that if it's really distribution specific (like done only on
> Ubuntu), wouldn't it be better to have the distribution patch libvirt to
> add this path ?
>
> >
> > +break;
> >> +case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
> >> +virBufferAsprintf(, "  \"%s/{,*}\" rk,\n", sc_db);
> >> +break;
> >>
> >
> >>From https://libvirt.org/formatdomain.html#elementsSmartcard
> > "An additional sub-element  can specify the absolute path to an
> > alternate directory ... if not present, it defaults to /etc/pki/nssdb."
> > That is in "VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE".
> > Have you tested if sc_db is actually set to
> > "VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE"
> > in that case?
> > If it is e.g. undefined we need to check for that and add
> > "VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE"
> > instead.
>
> I remember testing the case with certificates inside /etc/pki/nssdb and
> not specifying the  element at the early version of the
> patch.
>
> >
> > Furthermore actually this lets us define:
> >   
> > cert1
> > cert2
> > cert3
> > /etc/pki/nssdb/
> >   
> >
> > There could be two guests using rather different cert[1-3] and there is
> no
> > need letting them cross read right?
> > So instead of
> >   virBufferAsprintf(, "  \"%s/{,*}\" rk,\n", sc_db);
> > maybe something like this is safer:
> > iterate on certs-that-are-defined => cert
> > virBufferAsprintf(, "  \"%s/%s\" rk,\n", sc_db, cert);
>
> Not

撤回: [PATCH 6/6] docs: update virt-admin.rst for server-update-tls

2020-02-09 Thread Zhangbo (Oscar) 
Zhangbo (Oscar) 将撤回邮件“[PATCH 6/6] docs: update virt-admin.rst for 
server-update-tls”。

撤回: [PATCH 5/6] virt-admin: Introduce command srv-update-tls

2020-02-09 Thread Zhangbo (Oscar) 
Zhangbo (Oscar) 将撤回邮件“[PATCH 5/6] virt-admin: Introduce command srv-update-tls”。

撤回: [PATCH 1/6] virnettlscontext: refactoring virNetTLSContextLoadCredentials

2020-02-09 Thread Zhangbo (Oscar) 
Zhangbo (Oscar) 将撤回邮件“[PATCH 1/6] virnettlscontext: refactoring 
virNetTLSContextLoadCredentials”。

[PATCH 3/6] admin: Introduce virAdmServerUpdateTlsFiles

2020-02-09 Thread Zhang Bo 
The server needs to use CA certificate, CRL, server certificate/key to
complete the TLS handshake. If these files change, we need to restart
libvirtd for them to take effect. This API can update the TLS context
without restarting libvirtd.
---
 include/libvirt/libvirt-admin.h  |  4 
 src/admin/admin_protocol.x   | 13 ++-
 src/admin/admin_server.c | 13 +++
 src/admin/admin_server.h |  4 
 src/admin/libvirt-admin.c| 34 
 src/admin/libvirt_admin_private.syms |  1 +
 src/admin/libvirt_admin_public.syms  |  1 +
 7 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index 3edc044490..6e38261129 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -410,6 +410,10 @@ int virAdmServerSetClientLimits(virAdmServerPtr srv,
 int nparams,
 unsigned int flags);
 
+int virAdmServerUpdateTlsFiles(virAdmServerPtr srv,
+   unsigned int filetypes,
+   unsigned int flags);
+
 int virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn,
char **outputs,
unsigned int flags);
diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
index 42e215d23a..0fc8c54c80 100644
--- a/src/admin/admin_protocol.x
+++ b/src/admin/admin_protocol.x
@@ -181,6 +181,12 @@ struct admin_server_set_client_limits_args {
 unsigned int flags;
 };
 
+struct admin_server_update_tls_files_args {
+admin_nonnull_server srv;
+unsigned int filetypes;
+unsigned int flags;
+};
+
 struct admin_connect_get_logging_outputs_args {
 unsigned int flags;
 };
@@ -314,5 +320,10 @@ enum admin_procedure {
 /**
  * @generate: both
  */
-ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17
+ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17,
+
+/**
+ * @generate: both
+ */
+ADMIN_PROC_SERVER_UPDATE_TLS_FILES = 18
 };
diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c
index ba87f701c3..558913367b 100644
--- a/src/admin/admin_server.c
+++ b/src/admin/admin_server.c
@@ -367,3 +367,16 @@ adminServerSetClientLimits(virNetServerPtr srv,
 
 return 0;
 }
+
+int
+adminServerUpdateTlsFiles(virNetServerPtr srv,
+  unsigned int filetypes,
+  unsigned int flags)
+{
+virCheckFlags(0, -1);
+
+if (virNetServerUpdateTlsFiles(srv, filetypes) < 0)
+return -1;
+
+return 0;
+}
diff --git a/src/admin/admin_server.h b/src/admin/admin_server.h
index 1d5cbec55f..bd355017f2 100644
--- a/src/admin/admin_server.h
+++ b/src/admin/admin_server.h
@@ -67,3 +67,7 @@ int adminServerSetClientLimits(virNetServerPtr srv,
virTypedParameterPtr params,
int nparams,
unsigned int flags);
+
+int adminServerUpdateTlsFiles(virNetServerPtr srv,
+  unsigned int filetypes,
+  unsigned int flags);
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 4099a54854..f3f92ed91c 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -1082,6 +1082,40 @@ virAdmServerSetClientLimits(virAdmServerPtr srv,
 return ret;
 }
 
+/**
+ * virAdmServerUpdateTlsFiles:
+ * @srv: a valid server object reference
+ * @filetypes: bitwise-OR of virServerTlsFiletype
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Notify server to update tls file, such as cacert, cacrl, server cert / key.
+ * Mark the files that need to be updated by the @filetypes parameter.
+ * See virServerTlsFiletype for detailed description of accepted filetypes.
+ *
+ * Returns 0 if the TLS files have been updated successfully or -1 in case of 
an
+ * error.
+ */
+int
+virAdmServerUpdateTlsFiles(virAdmServerPtr srv,
+  unsigned int filetypes,
+  unsigned int flags)
+{
+int ret = -1;
+
+VIR_DEBUG("srv=%p, filetypes=%u, flags=0x%x", srv, filetypes, flags);
+virResetLastError();
+
+virCheckAdmServerGoto(srv, error);
+
+if ((ret = remoteAdminServerUpdateTlsFiles(srv, filetypes, flags)) < 0)
+goto error;
+
+return ret;
+ error:
+virDispatchError(NULL);
+return ret;
+}
+
 /**
  * virAdmConnectGetLoggingOutputs:
  * @conn: pointer to an active admin connection
diff --git a/src/admin/libvirt_admin_private.syms 
b/src/admin/libvirt_admin_private.syms
index 9526412de8..157a45341e 100644
--- a/src/admin/libvirt_admin_private.syms
+++ b/src/admin/libvirt_admin_private.syms
@@ -31,6 +31,7 @@ xdr_admin_server_lookup_client_args;
 xdr_admin_server_lookup_client_ret;
 xdr_admin_server_set_client_limits_args;
 xdr_admin_server_set_threadpool_parameters_args;

撤回: [PATCH 4/6] admin: support server cert update mode

2020-02-09 Thread Zhangbo (Oscar) 
Zhangbo (Oscar) 将撤回邮件“[PATCH 4/6] admin: support server cert update mode”。

[PATCH 0/6] update tls files without restarting libvirtd

2020-02-09 Thread Zhang Bo 
When a client wants to establish a TLS connection with libvirtd, a CRL
file， CA cert and server cert/key are used. Right
now, if these files are changed, you must restart libvirtd to make them
take effect. The restart behavior of libvirtd will cause clients
connecting with libvirtd to fail.

In a server cluster, these files, mostly the CRL, may be updated
quite frequently dueto the large amount of certificates.  If the new
file does not take effect in time, there are security risks. So you
may need to restart libvirtd frequently to make the CRL etc. take
effect in time. However, frequent restarts will affect the reliability
of cluster virtual machine management(such as openstack) services.

These patches add a virt-admin command to update the tls related files
*online*.

Zhang Bo (6):
  virnettlscontext: refactoring virNetTLSContextLoadCredentials
  virnetserver: Introduce virNetServerUpdateTlsFiles
  admin: Introduce virAdmServerUpdateTlsFiles
  admin: support server cert update mode
  virt-admin: Introduce command srv-update-tls
  docs: update virt-admin.rst for server-update-tls

 docs/manpages/virt-admin.rst |  21 
 include/libvirt/libvirt-admin.h  |  26 
 src/admin/admin_protocol.x   |  13 +-
 src/admin/admin_server.c |   8 ++
 src/admin/admin_server.h |   4 +
 src/admin/libvirt-admin.c|  39 ++
 src/admin/libvirt_admin_private.syms |   1 +
 src/admin/libvirt_admin_public.syms  |   1 +
 src/libvirt_remote.syms  |   1 +
 src/rpc/virnetserver.c   |  81 
 src/rpc/virnetserver.h   |   4 +
 src/rpc/virnetserverclient.c |   4 +
 src/rpc/virnettlscontext.c   | 179 +++
 src/rpc/virnettlscontext.h   |   3 +
 tools/virt-admin.c   |  88 +
 15 files changed, 419 insertions(+), 54 deletions(-)

-- 
2.23.0.windows.1

[PATCH 4/6] admin: support server cert update mode

2020-02-09 Thread Zhang Bo 
virAdmServerUpdateTlsFiles:
@flags specifies how to update server cert/key in tls service.
Two modes are currently supported: append mode and clear mode, means
whether to clear the original cert then add the new one, or just append
to the original one.
---
 include/libvirt/libvirt-admin.h | 14 ++
 src/admin/admin_server.c|  7 +--
 src/admin/libvirt-admin.c   |  7 ++-
 src/rpc/virnetserver.c  | 17 +
 src/rpc/virnetserver.h  |  3 ++-
 src/rpc/virnettlscontext.c  |  7 +--
 src/rpc/virnettlscontext.h  |  3 ++-
 7 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index 6e38261129..dfdd81ae83 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -392,6 +392,20 @@ int virAdmClientClose(virAdmClientPtr client, unsigned int 
flags);
 
 # define VIR_SERVER_CLIENTS_UNAUTH_CURRENT "nclients_unauth"
 
+typedef enum {
+/* free old credentials and then set new tls context.
+ */
+VIR_TLS_UPDATE_CLEAR  = 0,
+
+/* do not clear original certificates and keys.
+ */
+VIR_TLS_UPDATE_APPEND = 1,
+
+/* boundary value for flag check (unreachable).
+ */
+VIR_TLS_UPDATE_FLAG_MAX   = 2,
+} virServerTlsUpdateFlag;
+
 /* tls related filetype flags. */
 typedef enum {
 VIR_TLS_FILE_TYPE_CA_CERT = (1U << 0),
diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c
index 558913367b..43c7e00d90 100644
--- a/src/admin/admin_server.c
+++ b/src/admin/admin_server.c
@@ -373,10 +373,5 @@ adminServerUpdateTlsFiles(virNetServerPtr srv,
   unsigned int filetypes,
   unsigned int flags)
 {
-virCheckFlags(0, -1);
-
-if (virNetServerUpdateTlsFiles(srv, filetypes) < 0)
-return -1;
-
-return 0;
+return virNetServerUpdateTlsFiles(srv, filetypes, flags);
 }
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index f3f92ed91c..b6ba72b577 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -1086,12 +1086,17 @@ virAdmServerSetClientLimits(virAdmServerPtr srv,
  * virAdmServerUpdateTlsFiles:
  * @srv: a valid server object reference
  * @filetypes: bitwise-OR of virServerTlsFiletype
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: mode that specifies the update method
  *
  * Notify server to update tls file, such as cacert, cacrl, server cert / key.
  * Mark the files that need to be updated by the @filetypes parameter.
  * See virServerTlsFiletype for detailed description of accepted filetypes.
  *
+ * @flags specifies how to update server cert/key in tls service,
+ * and is either the value VIR_TLS_UPDATE_APPEND, or VIR_TLS_UPDATE_CLEAR.
+ * The default value is VIR_TLS_UPDATE_CLEAR. See virServerTlsUpdateFlag for
+ * detailed description.
+ *
  * Returns 0 if the TLS files have been updated successfully or -1 in case of 
an
  * error.
  */
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 65ec677d0a..72c4d37bc6 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -1226,7 +1226,8 @@ virNetServerGetTLSContext(virNetServerPtr srv)
 return ctxt;
 }
 
-static int virNetServerUpdateTlsFilesCheckParams(unsigned int filetypes)
+static int virNetServerUpdateTlsFilesCheckParams(unsigned int filetypes,
+ unsigned int flags)
 {
 bool haveSrvCert = filetypes & VIR_TLS_FILE_TYPE_SERVER_CERT;
 bool haveSrvKey = filetypes & VIR_TLS_FILE_TYPE_SERVER_KEY;
@@ -1239,12 +1240,20 @@ static int 
virNetServerUpdateTlsFilesCheckParams(unsigned int filetypes)
 return -1;
 }
 
+if (flags >= VIR_TLS_UPDATE_FLAG_MAX) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("don not support flags: %d"),
+   flags);
+return -1;
+}
+
 return 0;
 }
 
 int
 virNetServerUpdateTlsFiles(virNetServerPtr srv,
-   unsigned int filetypes)
+   unsigned int filetypes,
+   unsigned int flags)
 {
 int ret = -1;
 #ifndef WITH_GNUTLS
@@ -1254,7 +1263,7 @@ virNetServerUpdateTlsFiles(virNetServerPtr srv,
 #else
 virNetTLSContextPtr ctxt = NULL;
 
-if (virNetServerUpdateTlsFilesCheckParams(filetypes))
+if (virNetServerUpdateTlsFilesCheckParams(filetypes, flags))
 return -1;
 
 virObjectLock(srv);
@@ -1266,7 +1275,7 @@ virNetServerUpdateTlsFiles(virNetServerPtr srv,
 goto cleanup;
 }
 
-if (virNetTLSContextReload(ctxt, filetypes)) {
+if (virNetTLSContextReload(ctxt, filetypes, flags)) {
 VIR_ERROR(_("reload server's tls context fail"));
 goto cleanup;
 }
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 99466dd041..1a905aa483 100644
--- a/src/rpc/virnetserver.h
+++

[PATCH 1/6] virnettlscontext: refactoring virNetTLSContextLoadCredentials

2020-02-09 Thread Zhang Bo 
Encapsulate the code for setting TLS-related files into functions,
which is convenient for other modules to call.
---
 src/rpc/virnettlscontext.c | 135 ++---
 1 file changed, 82 insertions(+), 53 deletions(-)

diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 44f0dfce77..12811bed78 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -594,6 +594,85 @@ static int virNetTLSContextSanityCheckCredentials(bool 
isServer,
 return ret;
 }
 
+static int virNetTLSContextSetCACert(virNetTLSContextPtr ctxt,
+ const char *cacert,
+ bool allowMissing)
+{
+int err;
+if (virNetTLSContextCheckCertFile("CA certificate", cacert, allowMissing) 
< 0)
+return -1;
+
+VIR_DEBUG("loading CA cert from %s", cacert);
+err = gnutls_certificate_set_x509_trust_file(ctxt->x509cred,
+ cacert,
+ GNUTLS_X509_FMT_PEM);
+if (err < 0) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("Unable to set x509 CA certificate: %s: %s"),
+   cacert, gnutls_strerror(err));
+return -1;
+}
+
+return 0;
+}
+
+static int virNetTLSContextSetCACRL(virNetTLSContextPtr ctxt,
+const char *cacrl,
+bool allowMissing)
+{
+int rv, err;
+if ((rv = virNetTLSContextCheckCertFile("CA revocation list", cacrl, 
allowMissing)) < 0)
+return -1;
+
+if (rv == 0) {
+VIR_DEBUG("loading CRL from %s", cacrl);
+err = gnutls_certificate_set_x509_crl_file(ctxt->x509cred,
+   cacrl,
+   GNUTLS_X509_FMT_PEM);
+if (err < 0) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("Unable to set x509 certificate revocation list: 
%s: %s"),
+   cacrl, gnutls_strerror(err));
+return -1;
+}
+} else {
+VIR_DEBUG("Skipping non-existent CA CRL %s", cacrl);
+}
+
+return 0;
+}
+
+static int virNetTLSContextSetCertAndKey(virNetTLSContextPtr ctxt,
+ const char *cert,
+ const char *key,
+ bool allowMissing)
+{
+int rv, err;
+if ((rv = virNetTLSContextCheckCertFile("certificate", cert, 
allowMissing)) < 0)
+return -1;
+if (rv == 0 &&
+(rv = virNetTLSContextCheckCertFile("private key", key, allowMissing)) 
< 0)
+return -1;
+
+if (rv == 0) {
+VIR_DEBUG("loading cert and key from %s and %s", cert, key);
+err =
+gnutls_certificate_set_x509_key_file(ctxt->x509cred,
+ cert, key,
+ GNUTLS_X509_FMT_PEM);
+if (err < 0) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("Unable to set x509 key and certificate: %s, %s: 
%s"),
+   key, cert, gnutls_strerror(err));
+return -1;
+}
+} else {
+VIR_DEBUG("Skipping non-existent cert %s key %s on client",
+  cert, key);
+}
+
+return 0;
+}
 
 static int virNetTLSContextLoadCredentials(virNetTLSContextPtr ctxt,
bool isServer,
@@ -602,69 +681,19 @@ static int 
virNetTLSContextLoadCredentials(virNetTLSContextPtr ctxt,
const char *cert,
const char *key)
 {
-int err;
-
 if (cacert && cacert[0] != '\0') {
-if (virNetTLSContextCheckCertFile("CA certificate", cacert, false) < 0)
-return -1;
-
-VIR_DEBUG("loading CA cert from %s", cacert);
-err = gnutls_certificate_set_x509_trust_file(ctxt->x509cred,
- cacert,
- GNUTLS_X509_FMT_PEM);
-if (err < 0) {
-virReportError(VIR_ERR_SYSTEM_ERROR,
-   _("Unable to set x509 CA certificate: %s: %s"),
-   cacert, gnutls_strerror(err));
+if (virNetTLSContextSetCACert(ctxt, cacert, false))
 return -1;
-}
 }
 
 if (cacrl && cacrl[0] != '\0') {
-int rv;
-if ((rv = virNetTLSContextCheckCertFile("CA revocation list", cacrl, 
true)) < 0)
+if (virNetTLSContextSetCACRL(ctxt, cacrl, true))
 return -1;
-
-if (rv == 0) {
-VIR_DEBUG("loading CRL from %s", cacrl);
-err = gnutls_certificate_set_x509_crl_file(ctxt->x509cred,
-   cacrl,

撤回: [PATCH 3/6] admin: Introduce virAdmServerUpdateTlsFiles

2020-02-09 Thread Zhangbo (Oscar) 
Zhangbo (Oscar) 将撤回邮件“[PATCH 3/6] admin: Introduce virAdmServerUpdateTlsFiles”。

撤回: [PATCH 2/6] virnetserver: Introduce virNetServerUpdateTlsFiles

2020-02-09 Thread Zhangbo (Oscar) 
Zhangbo (Oscar) 将撤回邮件“[PATCH 2/6] virnetserver: Introduce 
virNetServerUpdateTlsFiles”。

[PATCH 5/6] virt-admin: Introduce command srv-update-tls

2020-02-09 Thread Zhang Bo 
wire-up virAdmServerUpdateTlsFiles API into virt-admin client.
---
 tools/virt-admin.c | 88 ++
 1 file changed, 88 insertions(+)

diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 32edfe5757..85235ae03d 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -957,6 +957,84 @@ cmdSrvClientsSet(vshControl *ctl, const vshCmd *cmd)
 goto cleanup;
 }
 
+/* 
+ *  Command srv-update-tls
+ * 
+ */
+static const vshCmdInfo info_srv_update_tls_file[] = {
+{.name = "help",
+ .data = N_("notify server to update TLS related files online.")
+},
+{.name = "desc",
+ .data = N_("notify server to update the CA cert, "
+"CA CRL, server cert / key without restarts. "
+"See OPTIONS for currently supported attributes.")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_srv_update_tls_file[] = {
+{.name = "server",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("Available servers on a daemon. "
+"Currently only supports 'libvirtd'.")
+},
+{.name = "filetypes",
+ .type = VSH_OT_INT,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("filetypes that need to be updated. "
+"bitwise-OR of tls filetypes flags.\n"
+"  parameter Description:\n"
+"--filetypes 1  ===>  cacert\n"
+"--filetypes 2  ===>  cacrl\n"
+"--filetypes 4  ===>  server-cert\n"
+"--filetypes 8  ===>  server-key\n"
+"  or a combination of several values. eg:\n"
+"--filetypes 3  ===>  cacert | cacrl\n"
+"  notice:\n"
+"server cert and key must be updated together.\n")
+},
+{.name = NULL}
+};
+
+static bool
+cmdSrvUpdateTlsFiles(vshControl *ctl, const vshCmd *cmd)
+{
+bool ret = false;
+const char *srvname = NULL;
+unsigned int filetypes;
+
+virAdmServerPtr srv = NULL;
+vshAdmControlPtr priv = ctl->privData;
+
+if (vshCommandOptStringReq(ctl, cmd, "server", ) < 0)
+return false;
+
+if (vshCommandOptUInt(ctl, cmd, "filetypes", ) < 0)
+return false;
+
+if (filetypes == 0) {
+vshError(ctl, "%s", _("filetypes can not be 0."));
+goto cleanup;
+}
+
+if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0)))
+goto cleanup;
+
+if (virAdmServerUpdateTlsFiles(srv, filetypes, VIR_TLS_UPDATE_CLEAR) < 0) {
+vshError(ctl, "%s", _("Unable to update server's tls related files."));
+goto cleanup;
+}
+
+ret = true;
+vshPrint(ctl, "update tls related files succeed\n");
+
+ cleanup:
+virAdmServerFree(srv);
+return ret;
+}
+
 /* --
  * Command daemon-log-filters
  * --
@@ -1436,6 +1514,16 @@ static const vshCmdDef managementCmds[] = {
  .info = info_srv_clients_set,
  .flags = 0
 },
+{.name = "srv-update-tls",
+ .flags = VSH_CMD_FLAG_ALIAS,
+ .alias = "server-update-tls"
+},
+{.name = "server-update-tls",
+ .handler = cmdSrvUpdateTlsFiles,
+ .opts = opts_srv_update_tls_file,
+ .info = info_srv_update_tls_file,
+ .flags = 0
+},
 {.name = "daemon-log-filters",
  .handler = cmdDaemonLogFilters,
  .opts = opts_daemon_log_filters,
-- 
2.23.0.windows.1

[PATCH 2/6] virnetserver: Introduce virNetServerUpdateTlsFiles

2020-02-09 Thread Zhang Bo 
Add an API to update server's tls context before admin method can be
introduced.
---
 include/libvirt/libvirt-admin.h |  8 
 src/libvirt_remote.syms |  1 +
 src/rpc/virnetserver.c  | 72 +
 src/rpc/virnetserver.h  |  3 ++
 src/rpc/virnetserverclient.c|  4 ++
 src/rpc/virnettlscontext.c  | 41 +++
 src/rpc/virnettlscontext.h  |  2 +
 7 files changed, 131 insertions(+)

diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index abf2792926..3edc044490 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -392,6 +392,14 @@ int virAdmClientClose(virAdmClientPtr client, unsigned int 
flags);
 
 # define VIR_SERVER_CLIENTS_UNAUTH_CURRENT "nclients_unauth"
 
+/* tls related filetype flags. */
+typedef enum {
+VIR_TLS_FILE_TYPE_CA_CERT = (1U << 0),
+VIR_TLS_FILE_TYPE_CA_CRL  = (1U << 1),
+VIR_TLS_FILE_TYPE_SERVER_CERT = (1U << 2),
+VIR_TLS_FILE_TYPE_SERVER_KEY  = (1U << 3),
+} virServerTlsFiletype;
+
 int virAdmServerGetClientLimits(virAdmServerPtr srv,
 virTypedParameterPtr *params,
 int *nparams,
diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index 0493467f46..0018a0c41d 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -137,6 +137,7 @@ virNetServerSetClientLimits;
 virNetServerSetThreadPoolParameters;
 virNetServerSetTLSContext;
 virNetServerUpdateServices;
+virNetServerUpdateTlsFiles;
 
 
 # rpc/virnetserverclient.h
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index c87dade1a8..65ec677d0a 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -1207,3 +1207,75 @@ virNetServerSetClientLimits(virNetServerPtr srv,
 virObjectUnlock(srv);
 return ret;
 }
+
+static virNetTLSContextPtr
+virNetServerGetTLSContext(virNetServerPtr srv)
+{
+size_t i;
+virNetTLSContextPtr ctxt = NULL;
+virNetServerServicePtr svc = NULL;
+
+/* find svcTLS from srv, get svcTLS->tls */
+for (i = 0; i < srv->nservices; i++) {
+svc = srv->services[i];
+ctxt = virNetServerServiceGetTLSContext(svc);
+if (ctxt != NULL)
+break;
+}
+
+return ctxt;
+}
+
+static int virNetServerUpdateTlsFilesCheckParams(unsigned int filetypes)
+{
+bool haveSrvCert = filetypes & VIR_TLS_FILE_TYPE_SERVER_CERT;
+bool haveSrvKey = filetypes & VIR_TLS_FILE_TYPE_SERVER_KEY;
+
+if ((haveSrvCert && !haveSrvKey) ||
+(!haveSrvCert && haveSrvKey)) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("server cert/key must be updated together. "
+   "filetypes: %d"), filetypes);
+return -1;
+}
+
+return 0;
+}
+
+int
+virNetServerUpdateTlsFiles(virNetServerPtr srv,
+   unsigned int filetypes)
+{
+int ret = -1;
+#ifndef WITH_GNUTLS
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("Don't support GNUTLS, can't to update filetypes: %d"),
+   filetypes);
+#else
+virNetTLSContextPtr ctxt = NULL;
+
+if (virNetServerUpdateTlsFilesCheckParams(filetypes))
+return -1;
+
+virObjectLock(srv);
+
+ctxt = virNetServerGetTLSContext(srv);
+if (!ctxt) {
+VIR_ERROR(_("no tls svc found, can't to update filetypes: %d"),
+  filetypes);
+goto cleanup;
+}
+
+if (virNetTLSContextReload(ctxt, filetypes)) {
+VIR_ERROR(_("reload server's tls context fail"));
+goto cleanup;
+}
+
+VIR_INFO("update all tls files complete, filetypes: %d", filetypes);
+ret = 0;
+
+ cleanup:
+virObjectUnlock(srv);
+#endif
+return ret;
+}
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 260c99b22d..99466dd041 100644
--- a/src/rpc/virnetserver.h
+++ b/src/rpc/virnetserver.h
@@ -133,3 +133,6 @@ size_t virNetServerGetCurrentUnauthClients(virNetServerPtr 
srv);
 int virNetServerSetClientLimits(virNetServerPtr srv,
 long long int maxClients,
 long long int maxClientsUnauth);
+
+int virNetServerUpdateTlsFiles(virNetServerPtr srv,
+   unsigned int filetypes);
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 67b3bf9531..f0952cadde 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1117,7 +1117,9 @@ int virNetServerClientInit(virNetServerClientPtr client)
   client->tls);
 
 /* Begin the TLS handshake. */
+virObjectLock(client->tlsCtxt);
 ret = virNetTLSSessionHandshake(client->tls);
+virObjectUnlock(client->tlsCtxt);
 if (ret == 0) {
 /* Unlikely, but ...  Next step is to check the certificate. */
 if (virNetServerClientCheckAccess(client) < 0)
@@

[PATCH 6/6] docs: update virt-admin.rst for server-update-tls

2020-02-09 Thread Zhang Bo 
Update the manpage for the 'server-update-tls' command
---
 docs/manpages/virt-admin.rst | 21 +
 1 file changed, 21 insertions(+)

diff --git a/docs/manpages/virt-admin.rst b/docs/manpages/virt-admin.rst
index 51c3d3917e..e19d1f1577 100644
--- a/docs/manpages/virt-admin.rst
+++ b/docs/manpages/virt-admin.rst
@@ -442,6 +442,27 @@ Set new client-related limits on *server*.
   *--max-clients*.
 
 
+server-update-tls
+-
+
+**Syntax:**
+
+.. code-block::
+
+   server-update-tls server [--filetypes types]
+
+Update tls context on *server*.
+
+- *server*
+
+  Available servers on a daemon. Currently only supports 'libvirtd'.
+
+- *--filetypes*
+
+  Indicate which TLS related files need to be updated, such as CA cert, CA CRL,
+  server cert/key. ``types`` is bitwise-OR of tls related files.
+
+
 CLIENT COMMANDS
 ===
 
-- 
2.23.0.windows.1

撤回: [PATCH 0/6] update tls files without restarting libvirtd

2020-02-09 Thread Zhangbo (Oscar) 
Zhangbo (Oscar) 将撤回邮件“[PATCH 0/6] update tls files without restarting libvirtd”。

[PATCH 5/6] virt-admin: Introduce command srv-update-tls

2020-02-09 Thread Zhangbo (Oscar) 
wire-up virAdmServerUpdateTlsFiles API into virt-admin client.
---
 tools/virt-admin.c | 88 ++
 1 file changed, 88 insertions(+)

diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 32edfe5757..85235ae03d 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -957,6 +957,84 @@ cmdSrvClientsSet(vshControl *ctl, const vshCmd *cmd)
 goto cleanup;
 }

+/* 
+ *  Command srv-update-tls
+ * 
+ */
+static const vshCmdInfo info_srv_update_tls_file[] = {
+{.name = "help",
+ .data = N_("notify server to update TLS related files online.")
+},
+{.name = "desc",
+ .data = N_("notify server to update the CA cert, "
+"CA CRL, server cert / key without restarts. "
+"See OPTIONS for currently supported attributes.")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_srv_update_tls_file[] = {
+{.name = "server",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("Available servers on a daemon. "
+"Currently only supports 'libvirtd'.")
+},
+{.name = "filetypes",
+ .type = VSH_OT_INT,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("filetypes that need to be updated. "
+"bitwise-OR of tls filetypes flags.\n"
+"  parameter Description:\n"
+"--filetypes 1  ===>  cacert\n"
+"--filetypes 2  ===>  cacrl\n"
+"--filetypes 4  ===>  server-cert\n"
+"--filetypes 8  ===>  server-key\n"
+"  or a combination of several values. eg:\n"
+"--filetypes 3  ===>  cacert | cacrl\n"
+"  notice:\n"
+"server cert and key must be updated together.\n")
+},
+{.name = NULL}
+};
+
+static bool
+cmdSrvUpdateTlsFiles(vshControl *ctl, const vshCmd *cmd)
+{
+bool ret = false;
+const char *srvname = NULL;
+unsigned int filetypes;
+
+virAdmServerPtr srv = NULL;
+vshAdmControlPtr priv = ctl->privData;
+
+if (vshCommandOptStringReq(ctl, cmd, "server", ) < 0)
+return false;
+
+if (vshCommandOptUInt(ctl, cmd, "filetypes", ) < 0)
+return false;
+
+if (filetypes == 0) {
+vshError(ctl, "%s", _("filetypes can not be 0."));
+goto cleanup;
+}
+
+if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0)))
+goto cleanup;
+
+if (virAdmServerUpdateTlsFiles(srv, filetypes, VIR_TLS_UPDATE_CLEAR) < 0) {
+vshError(ctl, "%s", _("Unable to update server's tls related files."));
+goto cleanup;
+}
+
+ret = true;
+vshPrint(ctl, "update tls related files succeed\n");
+
+ cleanup:
+virAdmServerFree(srv);
+return ret;
+}
+
 /* --
  * Command daemon-log-filters
  * --
@@ -1436,6 +1514,16 @@ static const vshCmdDef managementCmds[] = {
  .info = info_srv_clients_set,
  .flags = 0
 },
+{.name = "srv-update-tls",
+ .flags = VSH_CMD_FLAG_ALIAS,
+ .alias = "server-update-tls"
+},
+{.name = "server-update-tls",
+ .handler = cmdSrvUpdateTlsFiles,
+ .opts = opts_srv_update_tls_file,
+ .info = info_srv_update_tls_file,
+ .flags = 0
+},
 {.name = "daemon-log-filters",
  .handler = cmdDaemonLogFilters,
  .opts = opts_daemon_log_filters,
--
2.23.0.windows.1

[PATCH 6/6] docs: update virt-admin.rst for server-update-tls

2020-02-09 Thread Zhangbo (Oscar) 
Update the manpage for the 'server-update-tls' command
---
 docs/manpages/virt-admin.rst | 21 +
 1 file changed, 21 insertions(+)

diff --git a/docs/manpages/virt-admin.rst b/docs/manpages/virt-admin.rst
index 51c3d3917e..e19d1f1577 100644
--- a/docs/manpages/virt-admin.rst
+++ b/docs/manpages/virt-admin.rst
@@ -442,6 +442,27 @@ Set new client-related limits on *server*.
   *--max-clients*.


+server-update-tls
+-
+
+**Syntax:**
+
+.. code-block::
+
+   server-update-tls server [--filetypes types]
+
+Update tls context on *server*.
+
+- *server*
+
+  Available servers on a daemon. Currently only supports 'libvirtd'.
+
+- *--filetypes*
+
+  Indicate which TLS related files need to be updated, such as CA cert, CA CRL,
+  server cert/key. ``types`` is bitwise-OR of tls related files.
+
+
 CLIENT COMMANDS
 ===

--
2.23.0.windows.1

[PATCH 4/6] admin: support server cert update mode

2020-02-09 Thread Zhangbo (Oscar) 
virAdmServerUpdateTlsFiles:
@flags specifies how to update server cert/key in tls service.
Two modes are currently supported: append mode and clear mode, means
whether to clear the original cert then add the new one, or just append
to the original one.
---
 include/libvirt/libvirt-admin.h | 14 ++
 src/admin/admin_server.c|  7 +--
 src/admin/libvirt-admin.c   |  7 ++-
 src/rpc/virnetserver.c  | 17 +
 src/rpc/virnetserver.h  |  3 ++-
 src/rpc/virnettlscontext.c  |  7 +--
 src/rpc/virnettlscontext.h  |  3 ++-
 7 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index 6e38261129..dfdd81ae83 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -392,6 +392,20 @@ int virAdmClientClose(virAdmClientPtr client, unsigned int 
flags);

 # define VIR_SERVER_CLIENTS_UNAUTH_CURRENT "nclients_unauth"

+typedef enum {
+/* free old credentials and then set new tls context.
+ */
+VIR_TLS_UPDATE_CLEAR  = 0,
+
+/* do not clear original certificates and keys.
+ */
+VIR_TLS_UPDATE_APPEND = 1,
+
+/* boundary value for flag check (unreachable).
+ */
+VIR_TLS_UPDATE_FLAG_MAX   = 2,
+} virServerTlsUpdateFlag;
+
 /* tls related filetype flags. */
 typedef enum {
 VIR_TLS_FILE_TYPE_CA_CERT = (1U << 0),
diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c
index 558913367b..43c7e00d90 100644
--- a/src/admin/admin_server.c
+++ b/src/admin/admin_server.c
@@ -373,10 +373,5 @@ adminServerUpdateTlsFiles(virNetServerPtr srv,
   unsigned int filetypes,
   unsigned int flags)
 {
-virCheckFlags(0, -1);
-
-if (virNetServerUpdateTlsFiles(srv, filetypes) < 0)
-return -1;
-
-return 0;
+return virNetServerUpdateTlsFiles(srv, filetypes, flags);
 }
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index f3f92ed91c..b6ba72b577 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -1086,12 +1086,17 @@ virAdmServerSetClientLimits(virAdmServerPtr srv,
  * virAdmServerUpdateTlsFiles:
  * @srv: a valid server object reference
  * @filetypes: bitwise-OR of virServerTlsFiletype
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: mode that specifies the update method
  *
  * Notify server to update tls file, such as cacert, cacrl, server cert / key.
  * Mark the files that need to be updated by the @filetypes parameter.
  * See virServerTlsFiletype for detailed description of accepted filetypes.
  *
+ * @flags specifies how to update server cert/key in tls service,
+ * and is either the value VIR_TLS_UPDATE_APPEND, or VIR_TLS_UPDATE_CLEAR.
+ * The default value is VIR_TLS_UPDATE_CLEAR. See virServerTlsUpdateFlag for
+ * detailed description.
+ *
  * Returns 0 if the TLS files have been updated successfully or -1 in case of 
an
  * error.
  */
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 65ec677d0a..72c4d37bc6 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -1226,7 +1226,8 @@ virNetServerGetTLSContext(virNetServerPtr srv)
 return ctxt;
 }

-static int virNetServerUpdateTlsFilesCheckParams(unsigned int filetypes)
+static int virNetServerUpdateTlsFilesCheckParams(unsigned int filetypes,
+ unsigned int flags)
 {
 bool haveSrvCert = filetypes & VIR_TLS_FILE_TYPE_SERVER_CERT;
 bool haveSrvKey = filetypes & VIR_TLS_FILE_TYPE_SERVER_KEY;
@@ -1239,12 +1240,20 @@ static int 
virNetServerUpdateTlsFilesCheckParams(unsigned int filetypes)
 return -1;
 }

+if (flags >= VIR_TLS_UPDATE_FLAG_MAX) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("don not support flags: %d"),
+   flags);
+return -1;
+}
+
 return 0;
 }

 int
 virNetServerUpdateTlsFiles(virNetServerPtr srv,
-   unsigned int filetypes)
+   unsigned int filetypes,
+   unsigned int flags)
 {
 int ret = -1;
 #ifndef WITH_GNUTLS
@@ -1254,7 +1263,7 @@ virNetServerUpdateTlsFiles(virNetServerPtr srv,
 #else
 virNetTLSContextPtr ctxt = NULL;

-if (virNetServerUpdateTlsFilesCheckParams(filetypes))
+if (virNetServerUpdateTlsFilesCheckParams(filetypes, flags))
 return -1;

 virObjectLock(srv);
@@ -1266,7 +1275,7 @@ virNetServerUpdateTlsFiles(virNetServerPtr srv,
 goto cleanup;
 }

-if (virNetTLSContextReload(ctxt, filetypes)) {
+if (virNetTLSContextReload(ctxt, filetypes, flags)) {
 VIR_ERROR(_("reload server's tls context fail"));
 goto cleanup;
 }
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 99466dd041..1a905aa483 100644
--- a/src/rpc/virnetserver.h
+++

[PATCH 3/6] admin: Introduce virAdmServerUpdateTlsFiles

2020-02-09 Thread Zhangbo (Oscar) 
The server needs to use CA certificate, CRL, server certificate/key to
complete the TLS handshake. If these files change, we need to restart
libvirtd for them to take effect. This API can update the TLS context
without restarting libvirtd.
---
 include/libvirt/libvirt-admin.h  |  4 
 src/admin/admin_protocol.x   | 13 ++-
 src/admin/admin_server.c | 13 +++
 src/admin/admin_server.h |  4 
 src/admin/libvirt-admin.c| 34 
 src/admin/libvirt_admin_private.syms |  1 +
 src/admin/libvirt_admin_public.syms  |  1 +
 7 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index 3edc044490..6e38261129 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -410,6 +410,10 @@ int virAdmServerSetClientLimits(virAdmServerPtr srv,
 int nparams,
 unsigned int flags);

+int virAdmServerUpdateTlsFiles(virAdmServerPtr srv,
+   unsigned int filetypes,
+   unsigned int flags);
+
 int virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn,
char **outputs,
unsigned int flags);
diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
index 42e215d23a..0fc8c54c80 100644
--- a/src/admin/admin_protocol.x
+++ b/src/admin/admin_protocol.x
@@ -181,6 +181,12 @@ struct admin_server_set_client_limits_args {
 unsigned int flags;
 };

+struct admin_server_update_tls_files_args {
+admin_nonnull_server srv;
+unsigned int filetypes;
+unsigned int flags;
+};
+
 struct admin_connect_get_logging_outputs_args {
 unsigned int flags;
 };
@@ -314,5 +320,10 @@ enum admin_procedure {
 /**
  * @generate: both
  */
-ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17
+ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17,
+
+/**
+ * @generate: both
+ */
+ADMIN_PROC_SERVER_UPDATE_TLS_FILES = 18
 };
diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c
index ba87f701c3..558913367b 100644
--- a/src/admin/admin_server.c
+++ b/src/admin/admin_server.c
@@ -367,3 +367,16 @@ adminServerSetClientLimits(virNetServerPtr srv,

 return 0;
 }
+
+int
+adminServerUpdateTlsFiles(virNetServerPtr srv,
+  unsigned int filetypes,
+  unsigned int flags)
+{
+virCheckFlags(0, -1);
+
+if (virNetServerUpdateTlsFiles(srv, filetypes) < 0)
+return -1;
+
+return 0;
+}
diff --git a/src/admin/admin_server.h b/src/admin/admin_server.h
index 1d5cbec55f..bd355017f2 100644
--- a/src/admin/admin_server.h
+++ b/src/admin/admin_server.h
@@ -67,3 +67,7 @@ int adminServerSetClientLimits(virNetServerPtr srv,
virTypedParameterPtr params,
int nparams,
unsigned int flags);
+
+int adminServerUpdateTlsFiles(virNetServerPtr srv,
+  unsigned int filetypes,
+  unsigned int flags);
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 4099a54854..f3f92ed91c 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -1082,6 +1082,40 @@ virAdmServerSetClientLimits(virAdmServerPtr srv,
 return ret;
 }

+/**
+ * virAdmServerUpdateTlsFiles:
+ * @srv: a valid server object reference
+ * @filetypes: bitwise-OR of virServerTlsFiletype
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Notify server to update tls file, such as cacert, cacrl, server cert / key.
+ * Mark the files that need to be updated by the @filetypes parameter.
+ * See virServerTlsFiletype for detailed description of accepted filetypes.
+ *
+ * Returns 0 if the TLS files have been updated successfully or -1 in case of 
an
+ * error.
+ */
+int
+virAdmServerUpdateTlsFiles(virAdmServerPtr srv,
+  unsigned int filetypes,
+  unsigned int flags)
+{
+int ret = -1;
+
+VIR_DEBUG("srv=%p, filetypes=%u, flags=0x%x", srv, filetypes, flags);
+virResetLastError();
+
+virCheckAdmServerGoto(srv, error);
+
+if ((ret = remoteAdminServerUpdateTlsFiles(srv, filetypes, flags)) < 0)
+goto error;
+
+return ret;
+ error:
+virDispatchError(NULL);
+return ret;
+}
+
 /**
  * virAdmConnectGetLoggingOutputs:
  * @conn: pointer to an active admin connection
diff --git a/src/admin/libvirt_admin_private.syms 
b/src/admin/libvirt_admin_private.syms
index 9526412de8..157a45341e 100644
--- a/src/admin/libvirt_admin_private.syms
+++ b/src/admin/libvirt_admin_private.syms
@@ -31,6 +31,7 @@ xdr_admin_server_lookup_client_args;
 xdr_admin_server_lookup_client_ret;
 xdr_admin_server_set_client_limits_args;
 xdr_admin_server_set_threadpool_parameters_args;

[PATCH 1/6] virnettlscontext: refactoring virNetTLSContextLoadCredentials

2020-02-09 Thread Zhangbo (Oscar) 
Encapsulate the code for setting TLS-related files into functions,
which is convenient for other modules to call.
---
 src/rpc/virnettlscontext.c | 135 ++---
 1 file changed, 82 insertions(+), 53 deletions(-)

diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 44f0dfce77..12811bed78 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -594,6 +594,85 @@ static int virNetTLSContextSanityCheckCredentials(bool 
isServer,
 return ret;
 }

+static int virNetTLSContextSetCACert(virNetTLSContextPtr ctxt,
+ const char *cacert,
+ bool allowMissing)
+{
+int err;
+if (virNetTLSContextCheckCertFile("CA certificate", cacert, allowMissing) 
< 0)
+return -1;
+
+VIR_DEBUG("loading CA cert from %s", cacert);
+err = gnutls_certificate_set_x509_trust_file(ctxt->x509cred,
+ cacert,
+ GNUTLS_X509_FMT_PEM);
+if (err < 0) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("Unable to set x509 CA certificate: %s: %s"),
+   cacert, gnutls_strerror(err));
+return -1;
+}
+
+return 0;
+}
+
+static int virNetTLSContextSetCACRL(virNetTLSContextPtr ctxt,
+const char *cacrl,
+bool allowMissing)
+{
+int rv, err;
+if ((rv = virNetTLSContextCheckCertFile("CA revocation list", cacrl, 
allowMissing)) < 0)
+return -1;
+
+if (rv == 0) {
+VIR_DEBUG("loading CRL from %s", cacrl);
+err = gnutls_certificate_set_x509_crl_file(ctxt->x509cred,
+   cacrl,
+   GNUTLS_X509_FMT_PEM);
+if (err < 0) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("Unable to set x509 certificate revocation list: 
%s: %s"),
+   cacrl, gnutls_strerror(err));
+return -1;
+}
+} else {
+VIR_DEBUG("Skipping non-existent CA CRL %s", cacrl);
+}
+
+return 0;
+}
+
+static int virNetTLSContextSetCertAndKey(virNetTLSContextPtr ctxt,
+ const char *cert,
+ const char *key,
+ bool allowMissing)
+{
+int rv, err;
+if ((rv = virNetTLSContextCheckCertFile("certificate", cert, 
allowMissing)) < 0)
+return -1;
+if (rv == 0 &&
+(rv = virNetTLSContextCheckCertFile("private key", key, allowMissing)) 
< 0)
+return -1;
+
+if (rv == 0) {
+VIR_DEBUG("loading cert and key from %s and %s", cert, key);
+err =
+gnutls_certificate_set_x509_key_file(ctxt->x509cred,
+ cert, key,
+ GNUTLS_X509_FMT_PEM);
+if (err < 0) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("Unable to set x509 key and certificate: %s, %s: 
%s"),
+   key, cert, gnutls_strerror(err));
+return -1;
+}
+} else {
+VIR_DEBUG("Skipping non-existent cert %s key %s on client",
+  cert, key);
+}
+
+return 0;
+}

 static int virNetTLSContextLoadCredentials(virNetTLSContextPtr ctxt,
bool isServer,
@@ -602,69 +681,19 @@ static int 
virNetTLSContextLoadCredentials(virNetTLSContextPtr ctxt,
const char *cert,
const char *key)
 {
-int err;
-
 if (cacert && cacert[0] != '\0') {
-if (virNetTLSContextCheckCertFile("CA certificate", cacert, false) < 0)
-return -1;
-
-VIR_DEBUG("loading CA cert from %s", cacert);
-err = gnutls_certificate_set_x509_trust_file(ctxt->x509cred,
- cacert,
- GNUTLS_X509_FMT_PEM);
-if (err < 0) {
-virReportError(VIR_ERR_SYSTEM_ERROR,
-   _("Unable to set x509 CA certificate: %s: %s"),
-   cacert, gnutls_strerror(err));
+if (virNetTLSContextSetCACert(ctxt, cacert, false))
 return -1;
-}
 }

 if (cacrl && cacrl[0] != '\0') {
-int rv;
-if ((rv = virNetTLSContextCheckCertFile("CA revocation list", cacrl, 
true)) < 0)
+if (virNetTLSContextSetCACRL(ctxt, cacrl, true))
 return -1;
-
-if (rv == 0) {
-VIR_DEBUG("loading CRL from %s", cacrl);
-err = gnutls_certificate_set_x509_crl_file(ctxt->x509cred,
-   cacrl,
-

[PATCH 2/6] virnetserver: Introduce virNetServerUpdateTlsFiles

2020-02-09 Thread Zhangbo (Oscar) 
Add an API to update server's tls context before admin method can be
introduced.
---
 include/libvirt/libvirt-admin.h |  8 
 src/libvirt_remote.syms |  1 +
 src/rpc/virnetserver.c  | 72 +
 src/rpc/virnetserver.h  |  3 ++
 src/rpc/virnetserverclient.c|  4 ++
 src/rpc/virnettlscontext.c  | 41 +++
 src/rpc/virnettlscontext.h  |  2 +
 7 files changed, 131 insertions(+)

diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index abf2792926..3edc044490 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -392,6 +392,14 @@ int virAdmClientClose(virAdmClientPtr client, unsigned int 
flags);

 # define VIR_SERVER_CLIENTS_UNAUTH_CURRENT "nclients_unauth"

+/* tls related filetype flags. */
+typedef enum {
+VIR_TLS_FILE_TYPE_CA_CERT = (1U << 0),
+VIR_TLS_FILE_TYPE_CA_CRL  = (1U << 1),
+VIR_TLS_FILE_TYPE_SERVER_CERT = (1U << 2),
+VIR_TLS_FILE_TYPE_SERVER_KEY  = (1U << 3),
+} virServerTlsFiletype;
+
 int virAdmServerGetClientLimits(virAdmServerPtr srv,
 virTypedParameterPtr *params,
 int *nparams,
diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index 0493467f46..0018a0c41d 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -137,6 +137,7 @@ virNetServerSetClientLimits;
 virNetServerSetThreadPoolParameters;
 virNetServerSetTLSContext;
 virNetServerUpdateServices;
+virNetServerUpdateTlsFiles;


 # rpc/virnetserverclient.h
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index c87dade1a8..65ec677d0a 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -1207,3 +1207,75 @@ virNetServerSetClientLimits(virNetServerPtr srv,
 virObjectUnlock(srv);
 return ret;
 }
+
+static virNetTLSContextPtr
+virNetServerGetTLSContext(virNetServerPtr srv)
+{
+size_t i;
+virNetTLSContextPtr ctxt = NULL;
+virNetServerServicePtr svc = NULL;
+
+/* find svcTLS from srv, get svcTLS->tls */
+for (i = 0; i < srv->nservices; i++) {
+svc = srv->services[i];
+ctxt = virNetServerServiceGetTLSContext(svc);
+if (ctxt != NULL)
+break;
+}
+
+return ctxt;
+}
+
+static int virNetServerUpdateTlsFilesCheckParams(unsigned int filetypes)
+{
+bool haveSrvCert = filetypes & VIR_TLS_FILE_TYPE_SERVER_CERT;
+bool haveSrvKey = filetypes & VIR_TLS_FILE_TYPE_SERVER_KEY;
+
+if ((haveSrvCert && !haveSrvKey) ||
+(!haveSrvCert && haveSrvKey)) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("server cert/key must be updated together. "
+   "filetypes: %d"), filetypes);
+return -1;
+}
+
+return 0;
+}
+
+int
+virNetServerUpdateTlsFiles(virNetServerPtr srv,
+   unsigned int filetypes)
+{
+int ret = -1;
+#ifndef WITH_GNUTLS
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("Don't support GNUTLS, can't to update filetypes: %d"),
+   filetypes);
+#else
+virNetTLSContextPtr ctxt = NULL;
+
+if (virNetServerUpdateTlsFilesCheckParams(filetypes))
+return -1;
+
+virObjectLock(srv);
+
+ctxt = virNetServerGetTLSContext(srv);
+if (!ctxt) {
+VIR_ERROR(_("no tls svc found, can't to update filetypes: %d"),
+  filetypes);
+goto cleanup;
+}
+
+if (virNetTLSContextReload(ctxt, filetypes)) {
+VIR_ERROR(_("reload server's tls context fail"));
+goto cleanup;
+}
+
+VIR_INFO("update all tls files complete, filetypes: %d", filetypes);
+ret = 0;
+
+ cleanup:
+virObjectUnlock(srv);
+#endif
+return ret;
+}
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 260c99b22d..99466dd041 100644
--- a/src/rpc/virnetserver.h
+++ b/src/rpc/virnetserver.h
@@ -133,3 +133,6 @@ size_t virNetServerGetCurrentUnauthClients(virNetServerPtr 
srv);
 int virNetServerSetClientLimits(virNetServerPtr srv,
 long long int maxClients,
 long long int maxClientsUnauth);
+
+int virNetServerUpdateTlsFiles(virNetServerPtr srv,
+   unsigned int filetypes);
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 67b3bf9531..f0952cadde 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1117,7 +1117,9 @@ int virNetServerClientInit(virNetServerClientPtr client)
   client->tls);

 /* Begin the TLS handshake. */
+virObjectLock(client->tlsCtxt);
 ret = virNetTLSSessionHandshake(client->tls);
+virObjectUnlock(client->tlsCtxt);
 if (ret == 0) {
 /* Unlikely, but ...  Next step is to check the certificate. */
 if (virNetServerClientCheckAccess(client) < 0)
@@ -1438,7

[PATCH 0/6] update tls files without restarting libvirtd

2020-02-09 Thread Zhangbo (Oscar) 
When a client wants to establish a TLS connection with libvirtd, a CRL
file， CA cert and server cert/key are used. Right
now, if these files are changed, you must restart libvirtd to make them
take effect. The restart behavior of libvirtd will cause clients
connecting with libvirtd to fail.

In a server cluster, these files, mostly the CRL, may be updated
quite frequently dueto the large amount of certificates.  If the new
file does not take effect in time, there are security risks. So you
may need to restart libvirtd frequently to make the CRL etc. take
effect in time. However, frequent restarts will affect the reliability
of cluster virtual machine management(such as openstack) services.

These patches add a virt-admin command to update the tls related files
*online*.

Zhang Bo (6):
  virnettlscontext: refactoring virNetTLSContextLoadCredentials
  virnetserver: Introduce virNetServerUpdateTlsFiles
  admin: Introduce virAdmServerUpdateTlsFiles
  admin: support server cert update mode
  virt-admin: Introduce command srv-update-tls
  docs: update virt-admin.rst for server-update-tls

 docs/manpages/virt-admin.rst |  21 
 include/libvirt/libvirt-admin.h  |  26 
 src/admin/admin_protocol.x   |  13 +-
 src/admin/admin_server.c |   8 ++
 src/admin/admin_server.h |   4 +
 src/admin/libvirt-admin.c|  39 ++
 src/admin/libvirt_admin_private.syms |   1 +
 src/admin/libvirt_admin_public.syms  |   1 +
 src/libvirt_remote.syms  |   1 +
 src/rpc/virnetserver.c   |  81 
 src/rpc/virnetserver.h   |   4 +
 src/rpc/virnetserverclient.c |   4 +
 src/rpc/virnettlscontext.c   | 179 +++
 src/rpc/virnettlscontext.h   |   3 +
 tools/virt-admin.c   |  88 +
 15 files changed, 419 insertions(+), 54 deletions(-)

--
2.23.0.windows.1