[PATCHv2 0/5] update tls files without restarting libvirtd

2020-03-07 Thread Zhang Bo 
v1:
https://www.redhat.com/archives/libvir-list/2020-February/msg00370.html

v2:
according to Dienial's suggestion:
* update each tls file one time -> update all of them at one time
* forced to re-create the credentials object, rather than allowing
  to append to the original ones.

Zhang Bo (5):
  virnetserver: Introduce virNetServerUpdateTlsFiles
  tls: Add a mutex lock on 'tlsCtxt'
  admin: Introduce virAdmServerUpdateTlsFiles
  virt-admin: Introduce command srv-update-tls
  docs: update virt-admin.rst for server-update-tls

 docs/manpages/virt-admin.rst | 16 +++
 include/libvirt/libvirt-admin.h  |  3 ++
 src/admin/admin_protocol.x   | 12 +-
 src/admin/admin_server.c |  9 
 src/admin/admin_server.h |  3 ++
 src/admin/libvirt-admin.c| 30 +
 src/admin/libvirt_admin_private.syms |  1 +
 src/admin/libvirt_admin_public.syms  |  1 +
 src/libvirt_remote.syms  |  1 +
 src/rpc/virnetserver.c   | 51 ++
 src/rpc/virnetserver.h   |  2 +
 src/rpc/virnetserverclient.c |  4 ++
 src/rpc/virnettlscontext.c   | 46 
 src/rpc/virnettlscontext.h   |  3 ++
 tools/virt-admin.c   | 64 
 15 files changed, 245 insertions(+), 1 deletion(-)

-- 
2.23.0.windows.1

[PATCHv2 1/5] virnetserver: Introduce virNetServerUpdateTlsFiles

2020-03-07 Thread Zhang Bo 
Add an API to update server's tls context.
---
 src/libvirt_remote.syms|  1 +
 src/rpc/virnetserver.c | 51 ++
 src/rpc/virnetserver.h |  2 ++
 src/rpc/virnettlscontext.c | 46 ++
 src/rpc/virnettlscontext.h |  3 +++
 5 files changed, 103 insertions(+)

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 072ffdf5a3..0bfe94d3f8 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -21,6 +21,9 @@
 
 #include 
 
+#include 
+#include 
+
 #include "virnetserver.h"
 #include "virlog.h"
 #include "viralloc.h"
@@ -1205,3 +1208,51 @@ 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;
+}
+
+int
+virNetServerUpdateTlsFiles(virNetServerPtr srv)
+{
+int ret = -1;
+virNetTLSContextPtr ctxt = NULL;
+bool privileged = geteuid() == 0 ? true : false;
+
+ctxt = virNetServerGetTLSContext(srv);
+if (!ctxt) {
+VIR_ERROR(_("no tls svc found, unable to update tls files"));
+return -1;
+}
+
+virObjectLock(srv);
+virObjectLock(ctxt);
+
+if (virNetTLSContextReloadForServer(ctxt, !privileged)) {
+VIR_ERROR(_("failed to reload server's tls context"));
+goto cleanup;
+}
+
+VIR_INFO("update tls files success");
+ret = 0;
+
+ cleanup:
+virObjectUnlock(ctxt);
+virObjectUnlock(srv);
+return ret;
+}
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 260c99b22d..1c6a2efb6c 100644
--- a/src/rpc/virnetserver.h
+++ b/src/rpc/virnetserver.h
@@ -133,3 +133,5 @@ size_t virNetServerGetCurrentUnauthClients(virNetServerPtr 
srv);
 int virNetServerSetClientLimits(virNetServerPtr srv,
 long long int maxClients,
 long long int maxClientsUnauth);
+
+int virNetServerUpdateTlsFiles(virNetServerPtr srv);
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 44f0dfce77..02c17124a1 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -919,6 +919,52 @@ virNetTLSContextPtr virNetTLSContextNewServer(const char 
*cacert,
 }
 
 
+int virNetTLSContextReloadForServer(virNetTLSContextPtr ctxt,
+bool tryUserPkiPath)
+{
+gnutls_certificate_credentials_t x509credBak;
+int err;
+char *cacert = NULL;
+char *cacrl = NULL;
+char *cert = NULL;
+char *key = NULL;
+
+x509credBak = ctxt->x509cred;
+ctxt->x509cred = NULL;
+
+if (virNetTLSContextLocateCredentials(NULL, tryUserPkiPath, true,
+  , , , ))
+goto error;
+
+err = gnutls_certificate_allocate_credentials(>x509cred);
+if (err) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+   _("Unable to allocate x509 credentials: %s"),
+   gnutls_strerror(err));
+goto error;
+}
+
+if (virNetTLSContextSanityCheckCredentials(true, cacert, cert))
+goto error;
+
+if (virNetTLSContextLoadCredentials(ctxt, true, cacert, cacrl, cert, key))
+goto error;
+
+gnutls_certificate_set_dh_params(ctxt->x509cred,
+ ctxt->dhParams);
+
+gnutls_certificate_free_credentials(x509credBak);
+
+return 0;
+
+ error:
+if (ctxt->x509cred)
+gnutls_certificate_free_credentials(ctxt->x509cred);
+ctxt->x509cred = x509credBak;
+return -1;
+}
+
+
 virNetTLSContextPtr virNetTLSContextNewClient(const char *cacert,
   const char *cacrl,
   const char *cert,
diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h
index f3273bc26a..fe885aed9a 100644
--- a/src/rpc/virnettlscontext.h
+++ b/src/rpc/virnettlscontext.h
@@ -62,6 +62,9 @@ virNetTLSContextPtr virNetTLSContextNewClient(const char 
*cacert,
   bool sanityCheckCert,
   bool requireValidCert);
 
+int virNetTLSContextReloadForServer(virNetTLSContextPtr ctxt,
+bool tryUserPkiPath);
+
 int

[PATCHv2 4/5] virt-admin: Introduce command srv-update-tls

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

diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 32edfe5757..a8e5e0a5af 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -957,6 +957,60 @@ 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 = NULL}
+};
+
+static bool
+cmdSrvUpdateTlsFiles(vshControl *ctl, const vshCmd *cmd)
+{
+bool ret = false;
+const char *srvname = NULL;
+
+virAdmServerPtr srv = NULL;
+vshAdmControlPtr priv = ctl->privData;
+
+if (vshCommandOptStringReq(ctl, cmd, "server", ) < 0)
+return false;
+
+if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0)))
+goto cleanup;
+
+if (virAdmServerUpdateTlsFiles(srv, 0) < 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 +1490,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

[PATCHv2 2/5] tls: Add a mutex lock on 'tlsCtxt'

2020-03-07 Thread Zhang Bo 
Prevent the handshake function from reading 'tlsCtxt' while
updating 'tlsCtxt'.
---
 src/rpc/virnetserverclient.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 4d85ee25d7..657108239f 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1114,7 +1114,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)
@@ -1435,7 +1437,9 @@ virNetServerClientDispatchHandshake(virNetServerClientPtr 
client)
 {
 int ret;
 /* Continue the handshake. */
+virObjectLock(client->tlsCtxt);
 ret = virNetTLSSessionHandshake(client->tls);
+virObjectUnlock(client->tlsCtxt);
 if (ret == 0) {
 /* Finished.  Next step is to check the certificate. */
 if (virNetServerClientCheckAccess(client) < 0)
-- 
2.23.0.windows.1

[PATCHv2 3/5] admin: Introduce virAdmServerUpdateTlsFiles

2020-03-07 Thread Zhang Bo 
The server needs to use CA certificate, CRL, server certificate/key to
complete the TLS handshake. If these files change, we needed to restart
libvirtd for them to take effect. This API can update the TLS context
*ONLINE* without restarting libvirtd.
---
 include/libvirt/libvirt-admin.h  |  3 +++
 src/admin/admin_protocol.x   | 12 ++-
 src/admin/admin_server.c |  9 +
 src/admin/admin_server.h |  3 +++
 src/admin/libvirt-admin.c| 30 
 src/admin/libvirt_admin_private.syms |  1 +
 src/admin/libvirt_admin_public.syms  |  1 +
 7 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index abf2792926..e414f776e4 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -402,6 +402,9 @@ int virAdmServerSetClientLimits(virAdmServerPtr srv,
 int nparams,
 unsigned int flags);
 
+int virAdmServerUpdateTlsFiles(virAdmServerPtr srv,
+   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..7dc6724032 100644
--- a/src/admin/admin_protocol.x
+++ b/src/admin/admin_protocol.x
@@ -181,6 +181,11 @@ struct admin_server_set_client_limits_args {
 unsigned int flags;
 };
 
+struct admin_server_update_tls_files_args {
+admin_nonnull_server srv;
+unsigned int flags;
+};
+
 struct admin_connect_get_logging_outputs_args {
 unsigned int flags;
 };
@@ -314,5 +319,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..ebc0cfb045 100644
--- a/src/admin/admin_server.c
+++ b/src/admin/admin_server.c
@@ -367,3 +367,12 @@ adminServerSetClientLimits(virNetServerPtr srv,
 
 return 0;
 }
+
+int
+adminServerUpdateTlsFiles(virNetServerPtr srv,
+  unsigned int flags)
+{
+virCheckFlags(0, -1);
+
+return virNetServerUpdateTlsFiles(srv);
+}
diff --git a/src/admin/admin_server.h b/src/admin/admin_server.h
index 1d5cbec55f..08877a8edc 100644
--- a/src/admin/admin_server.h
+++ b/src/admin/admin_server.h
@@ -67,3 +67,6 @@ int adminServerSetClientLimits(virNetServerPtr srv,
virTypedParameterPtr params,
int nparams,
unsigned int flags);
+
+int adminServerUpdateTlsFiles(virNetServerPtr srv,
+  unsigned int flags);
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index a8592ebfd3..835b5560d2 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -1078,6 +1078,36 @@ virAdmServerSetClientLimits(virAdmServerPtr srv,
 return ret;
 }
 
+/**
+ * virAdmServerUpdateTlsFiles:
+ * @srv: a valid server object reference
+ * @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.
+ *
+ * Returns 0 if the TLS files have been updated successfully or -1 in case of 
an
+ * error.
+ */
+int
+virAdmServerUpdateTlsFiles(virAdmServerPtr srv,
+   unsigned int flags)
+{
+int ret = -1;
+
+VIR_DEBUG("srv=%p, flags=0x%x", srv, flags);
+virResetLastError();
+
+virCheckAdmServerGoto(srv, error);
+
+if ((ret = remoteAdminServerUpdateTlsFiles(srv, 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;
+xdr_admin_server_update_tls_files_args;
 
 # datatypes.h
 virAdmClientClass;
diff --git a/src/admin/libvirt_admin_public.syms 
b/src/admin/libvirt_admin_public.syms
index 9a3f843780..8126973e5b 100644
--- a/src/admin/libvirt_admin_public.syms
+++ b/src/admin/libvirt_admin_public.syms
@@ -38,6 +38,7 @@ LIBVIRT_ADMIN_2.0.0 {
 virAdmClientClose;
 virAdmServerGetClientLimits;
 virAdmServerSetClientLimits;
+virAdmServerUpdateTlsFiles;
 };
 
 LIBVIRT_ADMIN_3.0.0 {
-- 
2.23.0.windows.1

[PATCHv2 5/5] docs: update virt-admin.rst for server-update-tls

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

diff --git a/docs/manpages/virt-admin.rst b/docs/manpages/virt-admin.rst
index 51c3d3917e..3e0d127790 100644
--- a/docs/manpages/virt-admin.rst
+++ b/docs/manpages/virt-admin.rst
@@ -442,6 +442,22 @@ Set new client-related limits on *server*.
   *--max-clients*.
 
 
+server-update-tls
+-
+
+**Syntax:**
+
+.. code-block::
+
+   server-update-tls server
+
+Update tls context on *server*.
+
+- *server*
+
+  Available servers on a daemon. Currently only supports 'libvirtd'.
+
+
 CLIENT COMMANDS
 ===
 
-- 
2.23.0.windows.1