[libvirt] [PATCH RFC] update cacrl without restarting libvirtd via virt-admin

Zhangbo (Oscar) oscar.zhangbo at huawei.com
Sat Dec 28 02:18:20 UTC 2019


This is an RFC request for supporting virt-admin to update cacrl without restarting libvirtd.

When a client wants to establish a TLS connection with libvirtd, a CRL file is used by libvirtd to verify the client's certificate. Right now, if the CRL file is changed, you must restart libvirtd to make it take effect. The restart behavior of libvirtd will cause clients connecting with libvirtd to fail.

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

This RFC patch adds a virt-admin command to update the server's CRL *online*.

This patch is not elegant enough, if this feature makes sense, I'd do more improvements.

---
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              | 33 ++++++++++++++++
src/admin/libvirt_admin_private.syms   |  1 +
src/admin/libvirt_admin_public.syms    |  1 +
src/rpc/virnetserver.c                 | 58 +++++++++++++++++++++++++++
src/rpc/virnetserver.h                 |  3 ++
src/rpc/virnettlscontext.c              | 33 ++++++++++++++++
src/rpc/virnettlscontext.h              |  3 ++
tools/virt-admin.c                    | 59 ++++++++++++++++++++++++++++
12 files changed, 224 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index abf2792926..2df43db567 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -402,6 +402,10 @@ int virAdmServerSetClientLimits(virAdmServerPtr srv,
                                 int nparams,
                                 unsigned int flags);

+int virAdmServerUpdateTlsFile(virAdmServerPtr srv,
+                              unsigned int filetype,
+                              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..c74c0d777b 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_file_args {
+    admin_nonnull_server srv;
+    unsigned int filetype;
+    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_FILE = 18
};
diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c
index ba87f701c3..2f695eea4f 100644
--- a/src/admin/admin_server.c
+++ b/src/admin/admin_server.c
@@ -367,3 +367,16 @@ adminServerSetClientLimits(virNetServerPtr srv,

     return 0;
}
+
+int
+adminServerUpdateTlsFile(virNetServerPtr srv,
+                         unsigned int filetype,
+                         unsigned int flags)
+{
+    virCheckFlags(0, -1);
+
+    if (virNetServerUpdateTlsFile(srv, filetype) < 0)
+        return -1;
+
+    return 0;
+}
diff --git a/src/admin/admin_server.h b/src/admin/admin_server.h
index 1d5cbec55f..748235811a 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 adminServerUpdateTlsFile(virNetServerPtr srv,
+                             unsigned int filetype,
+                             unsigned int flags);
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 4099a54854..13c8db016d 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -1082,6 +1082,39 @@ virAdmServerSetClientLimits(virAdmServerPtr srv,
     return ret;
}

+/**
+ * virAdmServerUpdateTlsFile:
+ * @srv: a valid server object reference
+ * @filetype: TLS file type, such as crl, cert, key
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * update TLS Context in TLS service.
+ *
+ * Returns 0 if the TLS files have been updated successfully or -1 in case of an
+ * error.
+ */
+int
+virAdmServerUpdateTlsFile(virAdmServerPtr srv,
+                          unsigned int filetype,
+                          unsigned int flags)
+{
+    int ret = -1;
+
+    VIR_DEBUG("srv=%p, filetype=%u flags=%x", srv, filetype, flags);
+    virResetLastError();
+
+    virCheckAdmServerGoto(srv, error);
+
+    /* rpc call to update tls file */
+    if ((ret = remoteAdminServerUpdateTlsFile(srv, filetype, 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..d563757482 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_file_args;

 # datatypes.h
virAdmClientClass;
diff --git a/src/admin/libvirt_admin_public.syms b/src/admin/libvirt_admin_public.syms
index 9a3f843780..97b223bfba 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;
+        virAdmServerUpdateTlsFile;
};

 LIBVIRT_ADMIN_3.0.0 {
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 4122636805..fc18b2a224 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -1209,3 +1209,61 @@ virNetServerSetClientLimits(virNetServerPtr srv,
     virObjectUnlock(srv);
     return ret;
}
+
+int
+virNetServerUpdateTlsFile(virNetServerPtr srv,
+                          unsigned int filetype)
+{
+    int ret = -1;
+#if WITH_GNUTLS
+    size_t i;
+    int cnt = 0;
+    virNetTLSContextPtr ctxt = NULL;
+    virNetServerServicePtr svc = NULL;
+
+    if (filetype != TYPE_CACRL_LIBVIRT) {
+        virReportError(VIR_ERR_SYSTEM_ERROR,
+                       _("Don't support CRL filetype: %d"),
+                       filetype);
+        return ret;
+    }
+
+    virObjectLock(srv);
+
+    /* find svcTLS from srv */
+    for (i = 0; i < srv->nservices; i++) {
+        svc = srv->services[i];
+        /* find tls from svc */
+        ctxt = virNetServerServiceGetTLSContext(svc);
+        if (ctxt == NULL)
+            continue;
+
+        ret = virNetTLSContextUpdateCRL(ctxt);
+        if (ret < 0) {
+            VIR_ERROR(_("update tls file fail, "
+                        "filetype: %d, svcID: %zu"), filetype, i);
+            ret = -1;
+            goto cleanup;
+        }
+        VIR_INFO("update success, filetype: %d, svcID: %zu", filetype, i);
+        cnt++;
+    }
+
+    if (cnt == 0) {
+        virReportError(VIR_ERR_SYSTEM_ERROR,
+                       _("no tls server found, don't need update %d"),
+                       filetype);
+    } else {
+        VIR_INFO("update tls file complete, filetype: %d",
+                 filetype);
+    }
+
+ cleanup:
+    virObjectUnlock(srv);
+#else
+    virReportError(VIR_ERR_SYSTEM_ERROR,
+                   _("Don't support GNUTLS: %d"),
+                   filetype);
+#endif
+    return ret;
+}
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 260c99b22d..d0138d250f 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 virNetServerUpdateTlsFile(virNetServerPtr srv,
+                              unsigned int filetype);
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 08944f6771..1cc3cb8620 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -1106,6 +1106,39 @@ int virNetTLSContextCheckCertificate(virNetTLSContextPtr ctxt,
     return ret;
}

+int virNetTLSContextUpdateCRL(virNetTLSContextPtr ctxt)
+{
+    int ret = -1;
+    char *cacrl = NULL;
+
+    if (VIR_STRDUP(cacrl, LIBVIRT_CACRL) < 0)
+        return -1;
+
+    if (!virFileExists(cacrl)) {
+        virReportSystemError(errno, _("%s not exist"), cacrl);
+        goto cleanup;
+    }
+
+    virObjectLock(ctxt);
+
+    ret = gnutls_certificate_set_x509_crl_file(ctxt->x509cred,
+                                               cacrl,
+                                               GNUTLS_X509_FMT_PEM);
+    if (ret < 0) {
+        virReportError(VIR_ERR_SYSTEM_ERROR,
+                       _("Unable to update x509 CRL %s: %s"),
+                       cacrl, gnutls_strerror(ret));
+    } else {
+        VIR_INFO("Load %d CRL from %s", ret, cacrl);
+        ret = 0;
+    }
+
+    virObjectUnlock(ctxt);
+ cleanup:
+    VIR_FREE(cacrl);
+    return ret;
+}
+
void virNetTLSContextDispose(void *obj)
{
     virNetTLSContextPtr ctxt = obj;
diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h
index f3273bc26a..b823ab2c3f 100644
--- a/src/rpc/virnettlscontext.h
+++ b/src/rpc/virnettlscontext.h
@@ -23,6 +23,8 @@
#include "internal.h"
#include "virobject.h"

+#define TYPE_CACRL_LIBVIRT    13
+
typedef struct _virNetTLSContext virNetTLSContext;
typedef virNetTLSContext *virNetTLSContextPtr;

@@ -65,6 +67,7 @@ virNetTLSContextPtr virNetTLSContextNewClient(const char *cacert,
int virNetTLSContextCheckCertificate(virNetTLSContextPtr ctxt,
                                      virNetTLSSessionPtr sess);

+int virNetTLSContextUpdateCRL(virNetTLSContextPtr ctxt);

 typedef ssize_t (*virNetTLSSessionWriteFunc)(const char *buf, size_t len,
                                              void *opaque);
diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 30106d1971..25971dbf27 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -50,6 +50,8 @@
/* we don't need precision to milliseconds in this module */
#define VIRT_ADMIN_TIME_BUFLEN VIR_TIME_STRING_BUFLEN - 3

+#define LIBVIRT_CACRL_TYPE    13
+
static char *progname;

 static const vshCmdGrp cmdGroups[];
@@ -1103,6 +1105,57 @@ cmdDaemonLogOutputs(vshControl *ctl, const vshCmd *cmd)
     return true;
}

+/* ------------------------
+ *  Command update-crl-file
+ * ------------------------
+ */
+static const vshCmdInfo info_srv_update_tls_file[] = {
+    {.name = "help",
+     .data = N_("notify tls file type, current only support libvirt cacrl(13)")
+    },
+    {.name = "desc",
+     .data = N_("notify libvirtd TLS server hot update CACRL.")
+    },
+    {.name = NULL}
+};
+
+static const vshCmdOptDef opts_srv_update_tls_file[] = {
+    {.name = "server",
+     .type = VSH_OT_DATA,
+     .flags = VSH_OFLAG_REQ,
+     .help = N_("Server to reload the CRL file."),
+    },
+    {.name = NULL}
+};
+
+static bool
+cmdSrvUpdateTlsFile(vshControl *ctl, const vshCmd *cmd)
+{
+    bool ret = false;
+    const char *srvname = NULL;
+    unsigned int filetype = LIBVIRT_CACRL_TYPE;
+
+    virAdmServerPtr srv = NULL;
+    vshAdmControlPtr priv = ctl->privData;
+
+    if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0)
+        return false;
+
+    if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0)))
+        goto cleanup;
+
+    if (virAdmServerUpdateTlsFile(srv, filetype, 0) < 0) {
+        vshError(ctl, "%s", _("Unable to update server's tls file."));
+        goto cleanup;
+    }
+
+    ret = true;
+
+ cleanup:
+    virAdmServerFree(srv);
+    return ret;
+}
+
static void *
vshAdmConnectionHandler(vshControl *ctl)
{
@@ -1486,6 +1539,12 @@ static const vshCmdDef managementCmds[] = {
      .info = info_daemon_log_outputs,
      .flags = 0
     },
+    {.name = "update-crl-file",
+     .handler = cmdSrvUpdateTlsFile,
+     .opts = opts_srv_update_tls_file,
+     .info = info_srv_update_tls_file,
+     .flags = 0
+    },
     {.name = NULL}
};


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191228/198be825/attachment-0001.htm>


More information about the libvir-list mailing list